Repository navigation
fix: reconcile stale Git hook overrides on worker relaunch - #23
Merged
Merged
Conversation
…urce-only launch assertions
…le serial 7. The new multiline launch prefix exposed tests that parsed semicolon positions or matched anchored executable source. Replaced those assertions with model-free workers executing the complete captured launch and observing delivered argv/environment, including sibling Claude brief/credential and trace-cleanup paths. Production reconciliation, shell selection, and allowlist behavior remain unchanged. Updated the existing verification documentation. Local verification: bash bin/fm-test-run.sh --jobs 1 tests/fm-control-relaunch.test.sh tests/fm-trace-context-spawn.test.sh tests/fm-claude-trust.test.sh tests/fm-git-strip-ai-trailers.test.sh — all four suites passed, zero failures and zero gate skips. Private Bash/ksh tmux proofs passed, including persistent-pane cleanup across the allowlisted relaunch transition. Two non-fatal BSD sed diagnostics appeared in an unrelated Pi-session case. The full portable CI shard was not rerun
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
We need to get rid of or fix bad guards. We can't allow them to continue encumbering our work. I want judgement applied. The better answer lies in better engineering instead of more engineering.
Context: the existing ST-A3 publication recovery stops before publishing because its supported relaunch retains an obsolete Firstmate-generated git-hook override from the original terminal shell, even though keep-ai-trailers now selects the repository's own hooks. Git's command-scope output proves the stale override. The preserved work is unchanged; this task fixes that specific launcher defect, not Vernant code or its guards.
What Changed
Risk Assessment
✅ Low: The fix is narrowly scoped to task-owned Git hook routing, reconciles both persistent and filtered launch environments, and introduces no substantiated material defect or unnecessary behavioral component.
Testing
Reproduced the original stale Git-hook override, then passed the target's real fm-spawn/private-tmux/Git transitions in Bash and ksh, including allowlisted persistent-shell reconciliation, configuration preservation, and idempotent strip re-enablement. Related executable environment/argv checks also passed. The socket-path setup issue was resolved locally, four product-output logs were retained, and disposable fixtures were removed. No full suite, linters, live vendor workers, publication actions, or Herdr lifecycle actions were run.
Evidence: Base launcher reproduces stale command-scope hook selection
Source: Base launcher reproduces stale command-scope hook selection
Evidence: Target Bash and ksh relaunch Git scopes and persistent-pane environment
Source: Target Bash and ksh relaunch Git scopes and persistent-pane environment
Evidence: Delivered Claude environment and permission arguments plus compound-launch commit object
Source: Delivered Claude environment and permission arguments plus compound-launch commit object
Evidence: Delivered compact-adviser environment across both relaunch allowlist postures
Source: Delivered compact-adviser environment across both relaunch allowlist postures
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 2 issues found → auto-fixed (2) ✅
bin/fm-spawn.sh:5257- The new destination-shell function assumeslocalexists, but accepted ksh panes do not provide that builtin. tmux preserves the configured pane shell, and ksh is explicitly classified as an agent-free shell in bin/fm-agent-process-lib.sh:54. With the allowlist absent, the staged launch is sourced directly in that shell. Both declarations at bin/fm-spawn.sh:5257-5258 fail, leaving fm_count empty; line 5259 then returns successfully without reconciling anything. A strip-to-keep relaunch retains the stale generated hooksPath, while a fresh strip-enabled launch never selects the strip hooks. The invocation at bin/fm-spawn.sh:5287 continues into the harness in both cases. Make variable handling in this shared prefix compatible with the accepted destination shells while retaining reconciliation in the persistent pane shell.tests/fm-spawn-dispatch-profile.test.sh:1635- The replacement assertions still search emitted executable shell source rather than verify the claimed worker behavior, contrary to the required test-quality rule. The new permission assertions at tests/fm-spawn-dispatch-profile.test.sh:1635-1636 can pass when matching flags occur in comments or unexecuted commands; the changed CLAUDE_CONFIG_DIR assertion at :1061 likewise does not prove which credential store reaches the worker. Directly related retained assertions have the same issue at :1651-1652 (scout permissions) and tests/fm-kimi-harness.test.sh:297-299 (delivered effort and turn-end arguments). Remove these source-content-only assertions or execute the captured launch with environment/argv probes and assert the actual delivered values. The new claude_launch_arg helper already demonstrates the executable approach.🔧 Fix applied.
2 errors still open:
bin/fm-spawn.sh:5257- The new destination-shell function assumeslocalexists, but accepted ksh panes do not provide that builtin. tmux preserves the configured pane shell, and ksh is explicitly classified as an agent-free shell in bin/fm-agent-process-lib.sh:54. With the allowlist absent, the staged launch is sourced directly in that shell. Both declarations at bin/fm-spawn.sh:5257-5258 fail, leaving fm_count empty; line 5259 then returns successfully without reconciling anything. A strip-to-keep relaunch retains the stale generated hooksPath, while a fresh strip-enabled launch never selects the strip hooks. The invocation at bin/fm-spawn.sh:5287 continues into the harness in both cases. Make variable handling in this shared prefix compatible with the accepted destination shells while retaining reconciliation in the persistent pane shell.bin/fm-spawn.sh:5286- Round 1 fixes ksh portability but leaves the allowlisted path inconsistent with the human requirement to "reconcile in the same persistent destination shell." The changed hunk at :5286-5287 places reconciliation inside LAUNCH, which :5411 wraps in the existing env -i /bin/sh child. Concrete supported sequence: launch with stripping and no allowlist, stop the agent, enable keep-ai-trailers and an empty launch-env-allowlist, then relaunch the same task. The replacement worker is correctly filtered, but the persistent pane sourced at :5462 retains its obsolete generated Git override because the child cannot unset its parent's exports. This is an acceptance-criterion contradiction, not a demonstrated worker-publication failure. Reconcile the task-owned stale entry at the shared persistent-shell boundary before the existing filter, retaining filtered-worker hook selection, unrelated entries, and operator overrides. Related sites: docs/configuration.md:978 promises destination-shell reconciliation; tests/fm-git-strip-ai-trailers.test.sh:398-416 covers only the unfiltered transition.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
git diff 1070d7ed698014e49087d7182d10485baf2974ab cbe2ce79662de87182c08d23ed2503924705e2a3for the changed launcher, documentation, and targeted tests.VALIDATION_BASELINE=1 bash tests/.gate-stale-hooks-live.test.sh— disposable observation driver using the base commit's fm-spawn.sh; reproduced the obsolete command-scope hooks override after keep-ai-trailers relaunch.VALIDATION_BASELINE=0 bash tests/.gate-stale-hooks-live.test.sh— executed the existing private-tmux relaunch scenarios against the target launcher in Bash and ksh, observing worker Git scopes, persistent-pane environment, and unchanged task endpoint, pane, and worktree.bash tests/.gate-profile-probes.test.sh— selected tests from fm-spawn-dispatch-profile.test.sh for executable Claude config-directory forwarding, ship/scout auto-permission arguments, and compound-launch trailer stripping.bash tests/.gate-compact-relaunch.test.sh— selected test_relaunch_rebuilds_the_switch from fm-spawn-compact-adviser-disable.test.sh, executing replacement launch probes with the allowlist absent and enabled.Used a short worktree-local private tmux socket after the initial fixture socket exceeded macOS's socket-path limit; removed disposable drivers, baseline launcher, fixture directories, and private tmux servers after validation.✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.