Fix StopFailure Hook Uninstall Testing

Claude Code·Opus 5.5·gtrrz-victor·yesterday·2min·1 Checkpoint·1 file change·+12·5K tokens

I'd approve trail #1480 once two things are done: commit the uncommitted test change in your worktree, and add a description to the trail (it has none). I found no bugs. The new stop-failure hook goes through the same code as Stop and ends the turn. It's also included everywhere the other hooks are: install, uninstall, the outdated-config check in status/doctor, the hook-name list the CLI registers commands from, the repo's own .claude/settings.json, and the docs.

Trail status: 17/17 checks pass and the agent review has no findings. The only gate still failing is approvals, which needs a reviewer.

Before approving

  1. Uncommitted test. Your worktree has an edit to hooks_test.go that checks uninstall removes the StopFailure hook. It matters because AreHooksInstalled only looks at Stop, so a leftover stop-failure hook would go unnoticed. The package tests pass with it; commit and push it.
  2. Older Claude Code versions. Entire now writes a StopFailure key into users' .claude/settings.json. I can't confirm from the repo how a Claude Code release that predates this hook handles a key it doesn't know: it might ignore just that hook, or reject the whole settings file. If it rejects the file, every Entire hook would stop working for people on old versions. The repo doesn't check the Claude Code version anywhere. Worth confirming which version added StopFailure and whether that risk is acceptable.

Smaller points

  • Transcript wait matches by accident. When a turn ends, Entire waits for Claude Code to finish writing the transcript. It does that by looking for the text "hooks claude-code stop" in the file (lifecycle.go:275), and "hooks claude-code stop-failure" contains that text. So if Claude Code logs the failure hook there, the wait ends early as it does for Stop. If it doesn't, the wait falls back to "file size stopped changing". Either way works, but the match is accidental. A comment or test would stop someone later tightening the check and quietly losing this.
  • Test coverage. Only the parsing is unit-tested. One integration test that fires stop-failure after a prompt and checks the session goes back to idle would cover the real bug being fixed.
  • Checkpoint summary. A failed turn's saved step may take the API error message as its summary. That's acceptable, just worth knowing.
  • Nits: docs/architecture/agent-guide.md packs an explanation into a table cell (stop + stop-failure (StopFailure: …)), which reads awkwardly. The new TestCheckHookConfig_Outdated_MissingStopFailure repeats most of the setup in the MissingSubagentStop test right above it; a shared helper would remove the duplication.