What happened
PR #69 added non-empty guards (if [ -n "${GITHUB_WORKSPACE:-}" ]) to the workspace fallback loops in scripts/pre-code.sh (line 140) and scripts/pre-fix.sh (line 120), plus similar guards in scripts/post-code.sh and scripts/post-fix.sh. The post-scripts have test files (post-code-test.sh, post-fix-test.sh) and are run by make script-test (via .github/workflows/script-test.yml). However, no test files exist for pre-scripts. The Makefile's script-test target runs 8 test files — all post-scripts and validate-output-schema-test.sh — but no pre-*.sh tests. This means the guard fix in pre-code.sh and pre-fix.sh has zero regression test coverage. The code agent could not have added tests even if it tried, because there was no test infrastructure for pre-scripts to add tests to.
What could go better
The code-implementation skill mandates that every behavioral change have a corresponding test change. The code agent followed the issue spec (which only required existing tests to pass) and the AGENTS.md principle of surgical changes. But the absence of pre-script test files creates a structural gap: when the code agent fixes a bug in a pre-script, it has no way to add a regression test. If someone later removes the -n guard, no test will catch the regression.
Confidence is high that the gap exists: ls scripts/pre-*-test.sh returns nothing, and the Makefile's script-test target has no pre-script entries. Confidence is medium on the practical risk: pre-scripts run input validation on the GitHub Actions runner before sandbox creation, and broken validation could allow malformed inputs to reach the agent sandbox.
Proposed change
Create test files for the pre-scripts that contain testable logic, starting with scripts/pre-code-test.sh and scripts/pre-fix-test.sh. Each should at minimum:
- Extract the input validation logic (ISSUE_NUMBER, REPO_FULL_NAME, GITHUB_ISSUE_URL format checks) into testable functions.
- Include a test case that verifies the script does not crash with
GITHUB_WORKSPACE unset under set -u (similar to post-triage-test.sh lines 260–267).
- Follow the existing mock pattern used in post-script tests (mock
gh commands, test in isolation).
Add the new test files to the Makefile script-test target:
script-test:
$(call run-timed,bash scripts/pre-code-test.sh)
$(call run-timed,bash scripts/pre-fix-test.sh)
# ... existing entries ...
The remaining pre-scripts (pre-retro.sh, pre-review.sh, pre-scribe.sh, pre-triage.sh, pre-prioritize.sh) also lack tests but have simpler logic; prioritize pre-code.sh and pre-fix.sh since they contain the most validation logic (input checks, workspace fallback, branch detection).
Validation criteria
After implementation: (1) scripts/pre-code-test.sh and scripts/pre-fix-test.sh exist and pass when run via make script-test. (2) Each test file includes at least one test case for the GITHUB_WORKSPACE-unset scenario. (3) Deliberately removing the -n "${GITHUB_WORKSPACE:-}" guard from pre-code.sh causes the corresponding test to fail. (4) The script-test.yml CI workflow passes on the PR that adds the tests.
Generated by retro agent from #69
What happened
PR #69 added non-empty guards (
if [ -n "${GITHUB_WORKSPACE:-}" ]) to the workspace fallback loops inscripts/pre-code.sh(line 140) andscripts/pre-fix.sh(line 120), plus similar guards inscripts/post-code.shandscripts/post-fix.sh. The post-scripts have test files (post-code-test.sh,post-fix-test.sh) and are run bymake script-test(via.github/workflows/script-test.yml). However, no test files exist for pre-scripts. The Makefile'sscript-testtarget runs 8 test files — all post-scripts andvalidate-output-schema-test.sh— but nopre-*.shtests. This means the guard fix in pre-code.sh and pre-fix.sh has zero regression test coverage. The code agent could not have added tests even if it tried, because there was no test infrastructure for pre-scripts to add tests to.What could go better
The code-implementation skill mandates that every behavioral change have a corresponding test change. The code agent followed the issue spec (which only required existing tests to pass) and the AGENTS.md principle of surgical changes. But the absence of pre-script test files creates a structural gap: when the code agent fixes a bug in a pre-script, it has no way to add a regression test. If someone later removes the
-nguard, no test will catch the regression.Confidence is high that the gap exists:
ls scripts/pre-*-test.shreturns nothing, and the Makefile'sscript-testtarget has no pre-script entries. Confidence is medium on the practical risk: pre-scripts run input validation on the GitHub Actions runner before sandbox creation, and broken validation could allow malformed inputs to reach the agent sandbox.Proposed change
Create test files for the pre-scripts that contain testable logic, starting with
scripts/pre-code-test.shandscripts/pre-fix-test.sh. Each should at minimum:GITHUB_WORKSPACEunset underset -u(similar topost-triage-test.shlines 260–267).ghcommands, test in isolation).Add the new test files to the Makefile
script-testtarget:The remaining pre-scripts (
pre-retro.sh,pre-review.sh,pre-scribe.sh,pre-triage.sh,pre-prioritize.sh) also lack tests but have simpler logic; prioritize pre-code.sh and pre-fix.sh since they contain the most validation logic (input checks, workspace fallback, branch detection).Validation criteria
After implementation: (1)
scripts/pre-code-test.shandscripts/pre-fix-test.shexist and pass when run viamake script-test. (2) Each test file includes at least one test case for the GITHUB_WORKSPACE-unset scenario. (3) Deliberately removing the-n "${GITHUB_WORKSPACE:-}"guard from pre-code.sh causes the corresponding test to fail. (4) Thescript-test.ymlCI workflow passes on the PR that adds the tests.Generated by retro agent from #69