fix(#197): add fallback path resolution for companion scripts in post-fix.sh#228
fix(#197): add fallback path resolution for companion scripts in post-fix.sh#228fullsend-ai-coder[bot] wants to merge 1 commit into
Conversation
…-fix.sh
When post-fix.sh runs from a content-addressed cache path
(sha256/<hash>/content), the BASH_SOURCE[0] dirname does not
contain companion files like process-fix-result.py. This causes
the structured output processing to be silently skipped.
Add a fallback: after computing SCRIPT_DIR from BASH_SOURCE[0],
check if process-fix-result.py exists there. If not, fall back
to ${FULLSEND_DIR:-.}/scripts where companion scripts are
co-located in the standard workspace layout. The existing
safety net (warning if PROCESS_SCRIPT still does not exist)
remains unchanged.
Closes #197
E2E tests did not runE2E tests run automatically for org/repo members and collaborators on pull requests. For other contributors, a maintainer must add the See E2E testing guide for details. |
Review — approvePR: #228 SummarySmall, well-scoped bug fix that adds a fallback path resolution for AnalysisCorrectness ✅ — The fallback logic is safe. All edge cases degrade gracefully:
Security ✅ — No security concerns. Intent & Coherence ✅ — The PR correctly traces to issue #197 and implements Option A (minimal post-script fallback). While issue #197 mentions both Style ✅ — The added code matches existing conventions: comment style, conditional formatting ( Findings (low-severity, non-blocking)
Labels: PR modifies post-fix dispatch script companion resolution. Previous runReview — commentPR: fix(#197): add fallback path resolution for companion scripts in post-fix.sh SummaryThe PR adds a fallback path resolution for Findings1.
|
|
/fs-review |
| # Locate process-fix-result.py relative to this script. | ||
| # When this script runs from a content-addressed cache path | ||
| # (sha256/<hash>/content), companion files are not co-located. | ||
| # Fall back to ${FULLSEND_DIR}/scripts if the companion is missing. |
There was a problem hiding this comment.
[low] docs-currency
FULLSEND_DIR is now used for fallback companion script resolution but is not listed in the Optional environment variables section of the file header (lines 39-42). Adding it would help operators understand the fallback behavior.
Suggested fix: Add FULLSEND_DIR to the Optional environment variables comment block with a description like: FULLSEND_DIR — path to fullsend workspace directory for companion script fallback resolution (default: current directory).
| # Locate process-fix-result.py relative to this script. | ||
| # When this script runs from a content-addressed cache path | ||
| # (sha256/<hash>/content), companion files are not co-located. | ||
| # Fall back to ${FULLSEND_DIR}/scripts if the companion is missing. |
There was a problem hiding this comment.
[low] edge-case
The ${FULLSEND_DIR:-.} default resolves to . (current working directory after cd into REPO_DIR), which is unlikely to contain the companion script. The fallback is ineffective when FULLSEND_DIR is unset, though the existing safety net at line 322 prevents any harm. In CI, FULLSEND_DIR is always set via action.yml.
When post-fix.sh runs from a content-addressed cache path (sha256//content), the BASH_SOURCE[0] dirname does not contain companion files like process-fix-result.py. This causes the structured output processing to be silently skipped.
Add a fallback: after computing SCRIPT_DIR from BASH_SOURCE[0], check if process-fix-result.py exists there. If not, fall back to ${FULLSEND_DIR:-.}/scripts where companion scripts are co-located in the standard workspace layout. The existing safety net (warning if PROCESS_SCRIPT still does not exist) remains unchanged.
Closes #197
Post-script verification
agent/197-post-script-companion-fallback)f96750babbed5ada406a9ae04e8068449701d9c7..HEAD)