[malibio]

Summary

Both PreToolUse hooks described a process the agent guide replaced. They now summarise the guide's worktree flow and point at its sections.

• .claude/hooks/check-feature-branch.ts (before Edit / Write): on main it prints the startup sequence in the guide's order, with EnterWorktree({name: "issue-<N>-brief-desc"}) in place of a git checkout -b branch. It says a docs-only or mechanical change may be committed straight to main, and no longer says STOP. It stays silent inside a worktree, where the branch is never main.
• .claude/hooks/remind-plan-startup-sequence.ts (before EnterPlanMode): Step 0 is the same worktree sequence. The final steps are bun run test:changed, bun run quality:fix and commit, git push origin HEAD:issue-<N>-brief-desc, gh pr create --head issue-<N>-brief-desc, and bun run merge <PR#> after the user approves. It names the guide as AGENTS.md.

Both still print to stderr and exit 0. .claude/settings.json is unchanged.

Testing

• Ran both hooks: the branch check prints nothing inside the worktree and exits 0; the plan-mode reminder prints the new text and exits 0.
• The branch check's main output was not run on a main checkout; the condition (branch === 'main') is unchanged.
• bun run test:changed: no change reaches a test tier.
• bun run quality:fix: no further changes.

🤖 Generated with Claude Code

[malibio]

Pragmatic Code Review: PR #3736

Review Type: Initial Review (no prior reviews or comments on the PR). Reviewed commit 4d9e8aa79.

Summary

Two files changed, both hook text: .claude/hooks/check-feature-branch.ts and .claude/hooks/remind-plan-startup-sequence.ts. .claude/settings.json is unchanged and still registers both (Edit|Write, EnterPlanMode).

The change is a clear net improvement: both hooks stopped giving instructions the guide forbids. I checked the new text line by line against AGENTS.md ("Mandatory Startup Sequence", "Plan Mode", "Implementation Workflow" steps 3, 4, 6) and nodespace-docs/development/startup-sequence.md. Step order and command spellings match in both hooks. One sentence in the branch check states a rule I could not find in the guide (finding 1).

Requirements Check

• ✅ Both hooks kept, say EnterWorktree, in the guide's order. Both list: git status → git fetch origin && git pull origin main → EnterWorktree({name: "issue-<N>-brief-desc"}) → bun install → bun run test → gh:comment → gh:assign → gh:status. This is the order of steps 1-8 in AGENTS.md and startup-sequence.md.
• ✅ No banned wording. A grep of both files for feature/, checkout -b, test:all, gh:pr, CLAUDE.md and STOP returns nothing.
• ✅ Branch check silent in a worktree; on main it reminds without telling the agent to stop. Ran it in this worktree: no output, exit 0. The condition (branch === 'main') is unchanged. The main output was read, not run on a main checkout (the PR says the same). See finding 1 on how the "does not stop" part is worded.
• ✅ Plan-mode final steps. bun run test:changed, bun run quality:fix then commit, git push origin HEAD:issue-<N>-brief-desc, gh pr create --head issue-<N>-brief-desc, bun run merge <PR#> after the user approves, then ExitWorktree({action: "remove", discard_changes: true}). Matches "Plan Mode" item 2. Ran it: prints the new text, exit 0.
• ✅ bun run quality:fix (not independently verified). quality:fix does not lint .claude/ (quality:scripts:fix, packages/desktop-app, packages/skill, Rust), so the PR's "no further changes" is the expected result. I did not run it because it rewrites files.

Code Review Findings

1. 🟡 Important: the branch check states a permission the guide does not state
📁 .claude/hooks/check-feature-branch.ts:8-9, 32

A docs-only or mechanical change may be committed straight to main: carry on.

I searched AGENTS.md and nodespace-docs/development/ for this rule and did not find it. What the guide does say points the other way:

• "Git: Branch per issue";
• "main moves only through the queue", and a direct push "bypasses the queue, and the queue stops landing when it sees one and retests";
• the only direct push it names is a release's version bump.

The issue describes this as "a change the guide allows on main", and there is practice behind it (35fe90244 landed on main without a PR). But the acceptance criterion only asks that the hook not tell the agent to stop. The hook goes further: it tells every agent editing on main that "mechanical" changes are allowed there and to carry on. "Mechanical" is undefined, so an agent can apply it to a small code fix and skip the worktree, the review and the merge gate.

This is the drift the issue set out to remove: a hook asserting process the guide does not contain. Either:

• make the sentence neutral, e.g. "If this is not issue work, ignore this reminder", which satisfies the criterion without granting anything; or
• add the rule to AGENTS.md in this PR, with what counts as mechanical, and keep the hook's sentence.

The header comment at lines 8-9 ("A docs-only or mechanical change is committed straight to main") needs the same change.

2. 🟢 Suggestion: check that exit-0 stderr reaches the agent (pre-existing, not introduced here)
📁 .claude/hooks/check-feature-branch.ts:11-12, .claude/hooks/remind-plan-startup-sequence.ts:6-8

Both header comments say Claude Code surfaces the stderr text "as a message to the agent". As I understand Claude Code's hook contract, a PreToolUse hook that exits 0 does not feed stderr to the model; stderr reaches the model on exit 2, which blocks, and context can be added without blocking through JSON on stdout (hookSpecificOutput.additionalContext). I could not observe this from the review session, so treat it as unverified. If it is right, neither reminder is ever read by the agent, and the corrected text has no effect. The issue says to keep stderr and exit 0, so this is not a change for this PR; it is worth one check before #3712 decides the hooks' fate.

3. 🟢 Suggestion: "of the primary checkout" is inferred, and the hook fires for files outside the repository (pre-existing)
📁 .claude/hooks/check-feature-branch.ts:17-28, 30

The hook reads the branch of its working directory, not the edited file's path. On main it also fires for an edit to a file outside this repository (a nodespace-docs file, a scratchpad file). The softer wording makes that cheap to ignore. The hook receives the tool input on stdin, so it could compare file_path with the repository root if the noise matters later. No change needed here.

Nit: 📁 .claude/hooks/remind-plan-startup-sequence.ts:14 The reminder's first line still names only the startup sequence as required, while the body now also requires the final steps and inline standards. "MUST include the startup sequence as Step 0 and the final steps" would match the header comment.

Other checks

• Security: no input is read, no new command is run; the one execSync call is a fixed string and is unchanged. Nothing to flag.
• Testing: none needed. The change is message text, and scripts/gate-scope.ts routes .claude/ to no test tier.
• Documentation: no doc mentions these hooks by name; none needed, unless finding 1 is resolved by adding the rule to AGENTS.md.
• Sync boundary / node types: not touched.

Recommendation

APPROVE, with finding 1 to address before merge (a one-sentence wording change, plus the header comment). Findings 2 and 3 are pre-existing and need no change in this PR.

[malibio]

Automated pragmatic-code-review: APPROVE (posted as a comment — GitHub blocks self-approval). One Important finding to address before merge: the branch check tells the agent a "docs-only or mechanical change may be committed straight to main", a rule AGENTS.md does not state. Full findings in the review comment above.

[malibio]

Review addressed

Addressed

• Finding 1 (Important): the branch check no longer says a docs-only or mechanical change may be committed straight to main. The guide states no such rule, so the hook was granting a permission of its own. It now says "If it is not issue work, ignore this reminder", and the header comment says the same. The reminder still does not tell the agent to stop.
• Nit: the plan-mode reminder's first line now names the final steps as well as Step 0.

Skipped

• Finding 2 (Suggestion): whether stderr from a PreToolUse hook that exits 0 reaches the agent. Unverified, pre-existing, and the issue says to keep stderr and exit 0. It belongs with the decision on the hooks' long-term fate.
• Finding 3 (Suggestion): the hook reads the working directory's branch, not the edited file's path. Pre-existing; the reviewer asked for no change.

Re-Review Decision

Decision: NO RE-REVIEW NEEDED

Rationale: the follow-up commit changes three sentences of message text and one comment, each the wording the review asked for. No logic changed.