Skip to content

session-start: re-copy the Stop hook when the setup script did not (infra#112) - #88

Merged
bdelanghe merged 1 commit into
mainfrom
claude/front-desk-not-working-6993hp
Aug 1, 2026
Merged

session-start: re-copy the Stop hook when the setup script did not (infra#112)#88
bdelanghe merged 1 commit into
mainfrom
claude/front-desk-not-working-6993hp

Conversation

@bdelanghe

Copy link
Copy Markdown
Contributor

Row 3 of #85's table, closed the way #84 closed row 2. The setup script's cp of stop-hook-git-check.sh had gone missing along with its register-mcp.mjs call, so the dispatcher now re-copies it.

Measured 2026-08-01 in this container:

d124f7e8844ce1bd…  .claude/stop-hook-git-check.sh          5458 bytes   (matches SUM_stop_hook_git_check_sh)
1e1c49718621d862…  /root/.claude/stop-hook-git-check.sh    3262 bytes   (the platform's stock hook)

Why this one outlasted the MCP break

It fails quietly. The stock hook scopes its check to origin/<branch>..HEAD, which after a squash merge includes GitHub's own merge commit — so it warns "Unverified" after every successful merge and advises an --amend that would rewrite already-merged history.

Nothing looks broken, so nothing gets investigated. And a hook that cries wolf on every merge is worse than no hook, because it teaches you to ignore the one time it is right. #84's warning predicted this — "a field that has stopped calling register-mcp.mjs has probably stopped doing the rest of its job too" — and it was correct.

Design notes

  • Compares bytes, not presence. The failure here is a wrong file, not a missing one, so an [ -f ]-style check would have reported this session healthy. stopHookAction is a pure function over the two files' bytes and is tested directly.
  • Only the script is replaced. launcher-settings.json declares the Stop hook, is platform-managed, and gets rewritten — so the dispatcher swaps the file it already points at rather than touching the wiring.
  • Late is not too late. Hooks are invoked per event, so a copy written during SessionStart is in force from that session's first Stop. Unlike the MCP case there was no ordering claim to disprove.
  • homedir() is correct here and wrong for the session root: /root/.claude really is where the user settings live while the repos are under /home/user. That asymmetry is what sessionRootFrom exists for, so the comment says explicitly not to "fix" one to match the other.
  • Degrades like the rest of the file. The bootstrap verifies each fetched file independently, so a refused digest can leave the dispatcher present and this one absent. That case is reported and leaves the platform's hook intact rather than overwriting it with an empty file.

Verification

  • 116 tests pass across the six suites the schema job runs.
  • 8 new tests: the three stopHookAction cases, plus real-fs runs covering replace-the-stock-hook, executable mode, target directory created, re-run is a no-op, and a missing source leaving the installed hook byte-identical. One further test asserts against this repo's actual file, so a rename or move fails rather than passing against a fixture — which is exactly how the setup script's copy failed.
  • End-to-end against a scratch session root: stock hook in place → replaced with the 5458-byte copy at mode 755, warning on stderr; re-run → silent.

Not fixed here

The setup-script field itself, and the two remaining rows of #85. The CLAUDE_SESSION_ROOT prefix is benign with .github attached, and the settings.json write is the irreducible bootstrap — the hook that would self-heal it is the thing being installed. See the comment on #85 for why the field can't be CI-owned, and for the server-managed-settings route that might remove it entirely.


Generated by Claude Code

…nfra#112)

Row 3 of #85's table, closed the same way #84 closed row 2. The setup script's
`cp` of stop-hook-git-check.sh had gone missing along with its register-mcp.mjs
call; measured 2026-08-01, $HOME/.claude held the platform's stock hook at 3262
bytes against this repo's 5458.

This one outlasted the MCP break because it fails quietly. The stock hook scopes
its check to `origin/<branch>..HEAD`, which after a squash merge includes
GitHub's own merge commit — so it warns "Unverified" after every successful
merge and advises an --amend that would rewrite already-merged history. Nothing
looks broken, so nothing gets investigated, and a hook that cries wolf on every
merge is worse than no hook.

Compares BYTES, not presence: the failure mode here is a wrong file rather than
a missing one, so `[ -f ]` would have called this healthy. Only the script is
replaced — launcher-settings.json is platform-managed and rewritten, so the
dispatcher swaps the file it already points at. Hooks run per event, so a copy
written at SessionStart is in force from that session's first Stop.

Degrades the way the rest of the file does: a refused digest can leave the
dispatcher present and this file absent, and that case is reported and leaves
the platform's hook alone rather than overwriting it with nothing.

Note homedir() is correct here and wrong for the session root — /root/.claude
really is where the user settings live, while the repos are under /home/user.
That asymmetry is what sessionRootFrom exists for, so the comment says not to
"fix" one to match the other.
@bdelanghe
bdelanghe requested a review from a team as a code owner August 1, 2026 15:25
@bdelanghe
bdelanghe merged commit 7ee3d5c into main Aug 1, 2026
9 checks passed
@bdelanghe
bdelanghe deleted the claude/front-desk-not-working-6993hp branch August 1, 2026 15:27
@github-project-automation github-project-automation Bot moved this from Todo to Done in Front Desk Aug 1, 2026
bdelanghe added a commit that referenced this pull request Aug 1, 2026
By hand again, for #88, because the `pin` job still cannot push its bump branch
(#87 — the App installation lacks `contents: write`).

Second hand-bump in one afternoon. The pin only goes stale when a fetched file
changes, so this is not noise: it is the designed hand-off running with its
automated half broken, and every PR touching `.claude/` will need this until #87
is fixed.

Co-authored-by: Claude <noreply@anthropic.com>
bdelanghe added a commit that referenced this pull request Aug 3, 2026
…gainst the canonical field text (#97)

Closes #91. Implements I1 of docs/session-capability-invariants.md, which moves
from Partial to Enforced.

ensureMcpRegistered (#84) and syncStopHook (#88) were two bespoke implementations
of one idea, written a few hours apart. They are now entries in one MANIFEST
driven by a single loop.

The gate is the point, not the refactor. parseSteps in gen-bootstrap-pin.mjs,
which already parsed the canonical setup-script block for PIN and the SUM_*
lines, now also enumerates that field's steps, and bootstrap-steps.test.mjs
asserts each maps to a manifest entry or an IRREDUCIBLE declaration carrying a
reason. A step added to the field with no fallback now fails CI instead of going
unnoticed until it breaks in production, which is how #85 happened.

Held onto deliberately: the comparison stays per-entry (the Stop hook compares
BYTES because its failure was a wrong file; MCP compares a predicate over JSON),
the parse REFUSES an unclassifiable verb rather than skipping it, and the two
irreducible steps are declared with reasons rather than merely absent. The
scar-tissue comments moved onto the entries they explain — they are why the two
entries are asymmetric.

Verified with a negative test: a cp step added to the canonical text with no
manifest entry takes the suite from 99 pass to 98 pass / 1 fail, naming the
artifact and the offending line. Two mutants of the loop are both caught. Run end
to end against a fake two-repo session root.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants