What happened
PR #2761 migrated retro.yaml from deprecated runner_env to env.runner/env.sandbox per ADR 0055. The companion integration test (internal/harness/scaffold_integration_test.go, lines 323-328) was correctly updated with empty topLevelKeys/forgeGithubKeys and all keys in envRunnerKeys. However, the unit test TestHarnessForgeRunnerEnvMerge in internal/scaffold/scaffold_test.go (lines 684-688) was not updated — it still lists retro keys under topLevelKeys (FULLSEND_OUTPUT_SCHEMA) and forgeGithubKeys (ORIGINATING_URL, REPO_FULL_NAME, GH_TOKEN). The review agent flagged this as a medium-severity test-integrity finding on commit f1d276a. The human reviewer (rh-hemartin) approved without comments on 2026-06-30, and the PR was merged on 2026-07-02 with the finding unresolved.
What could go better
The test passes today because the combined map merging logic happens to include Env.Runner keys, but it does not verify the keys landed in the correct location. If a future change regresses retro.yaml back to the old pattern, this test would not catch it. The review agent correctly identified the gap, but the finding was posted as a COMMENTED review rather than CHANGES_REQUESTED, making it easy for the human reviewer to overlook. Confidence is high — the discrepancy between scaffold_test.go and scaffold_integration_test.go is confirmed by code inspection.
Proposed change
Update the retro test case in internal/scaffold/scaffold_test.go (around line 684-688) to match the integration test pattern: set topLevelKeys and forgeGithubKeys to empty slices, and add envRunnerKeys: []string{"FULLSEND_OUTPUT_SCHEMA", "ORIGINATING_URL", "REPO_FULL_NAME", "GH_TOKEN"}. This aligns the unit test with both the actual harness YAML and the integration test.
Validation criteria
The retro test case in scaffold_test.go has empty topLevelKeys and forgeGithubKeys with all keys under envRunnerKeys. All tests pass. A deliberate regression (moving a key back to runner_env in retro.yaml) causes the test to fail.
Generated by retro agent from #2761
What happened
PR #2761 migrated retro.yaml from deprecated
runner_envtoenv.runner/env.sandboxper ADR 0055. The companion integration test (internal/harness/scaffold_integration_test.go, lines 323-328) was correctly updated with emptytopLevelKeys/forgeGithubKeysand all keys inenvRunnerKeys. However, the unit testTestHarnessForgeRunnerEnvMergeininternal/scaffold/scaffold_test.go(lines 684-688) was not updated — it still lists retro keys undertopLevelKeys(FULLSEND_OUTPUT_SCHEMA) andforgeGithubKeys(ORIGINATING_URL, REPO_FULL_NAME, GH_TOKEN). The review agent flagged this as a medium-severity test-integrity finding on commit f1d276a. The human reviewer (rh-hemartin) approved without comments on 2026-06-30, and the PR was merged on 2026-07-02 with the finding unresolved.What could go better
The test passes today because the combined map merging logic happens to include
Env.Runnerkeys, but it does not verify the keys landed in the correct location. If a future change regresses retro.yaml back to the old pattern, this test would not catch it. The review agent correctly identified the gap, but the finding was posted as a COMMENTED review rather than CHANGES_REQUESTED, making it easy for the human reviewer to overlook. Confidence is high — the discrepancy between scaffold_test.go and scaffold_integration_test.go is confirmed by code inspection.Proposed change
Update the retro test case in
internal/scaffold/scaffold_test.go(around line 684-688) to match the integration test pattern: settopLevelKeysandforgeGithubKeysto empty slices, and addenvRunnerKeys: []string{"FULLSEND_OUTPUT_SCHEMA", "ORIGINATING_URL", "REPO_FULL_NAME", "GH_TOKEN"}. This aligns the unit test with both the actual harness YAML and the integration test.Validation criteria
The retro test case in scaffold_test.go has empty topLevelKeys and forgeGithubKeys with all keys under envRunnerKeys. All tests pass. A deliberate regression (moving a key back to runner_env in retro.yaml) causes the test to fail.
Generated by retro agent from #2761