refactor(hooks): consolidate the hook decision behind one shared decide

[KuSh]

Follow-up to #3704, which consolidated raw-command lexing and closed with:

hooks/rewrite_cmd.rs vs hooks/hook_cmd.rs decision-flow duplication — real, but a separate follow-up at a different level.

rtk rewrite's behaviour is unchanged — deliberately

The obvious generalisation is to make rtk rewrite behave like the hooks and defer on a rewrite that changes nothing. This PR does not do that, and the divergence is kept on purpose:

	already-rtk command
rtk hook <agent>	defers — a hook speaks an edit protocol, and an edit to the same string is meaningless
rtk rewrite	reports it (exit 0/3, command unchanged on stdout)

That is the documented contract from #241, which created rtk rewrite: "Handle already-rtk commands (exit 0, identical output)", still pinned by test_run_already_rtk_returns_some. The CLI answers "what is the RTK form of this command"; for an already-prefixed command that form is itself, and whether that counts as a change is the caller's question. Every delegate already asks it — hooks/opencode/rtk.ts, hooks/pi/rtk.ts (shared with omp), hooks/hermes/rtk-rewrite/__init__.py and openclaw/index.ts all gate on rewritten != command.

So suppress_identity is applied by decide_for_agent, which the hooks share, and not by decide, which the CLI renders. The exit-code protocol is untouched.

What changed

rtk hook <agent> and rtk rewrite carried two independently written copies of the same four steps over two isomorphic enums (HookDecision, RewriteOutcome), so a fix to the gate order or a new construct to refuse had to be made twice.

• src/hooks/decision.rs — one decide, taking the permission verdict and the rewrite parameters rather than looking them up, so each host consults its own rules and tests stay independent of the developer's machine (Rewrite tests fail when local permission settings allow git commands #3146). decide_for_agent adds the no-op suppression every hook applies.
• rewrite_cmd::run is reduced to exit-code rendering; hook_cmd's eight response builders are untouched.
• rtk hook check now routes through the same decision (the one intended behaviour change, below).

Related issues

• Closes rtk hook check disagrees with rtk hook claude on commands containing a file redirect #3936 — rtk hook check disagrees with rtk hook claude on commands containing a file redirect. Fixed here; all six shapes in that report now agree, and 2>&1 still rewrites on both.
• hook check dry-run does not expose the actual host permission decision #3476 — hook check does not expose the actual host permission decision. Partially addressed: the diagnostic now consults the host's real rules instead of ignoring them, but it still collapses allow and ask into one answer. The remaining half is a follow-up, commented on that issue.

rtk hook check — the one behaviour change

It called registry::rewrite_command directly, with no verdict and none of the gates, so it reported a rewrite for command substitutions, file redirects and heredocs that both hook paths refuse — the diagnostic disagreed with the thing it exists to diagnose, in the direction that matters.

Consulting rules makes the answer host-dependent, so --agent stops being discarded. AgentPath records what actually differs: the six agents deciding in-process use their own host's rules, the five whose plugin shells out to rtk rewrite get Claude's, and the six installing only a rules file have none. All 17 install targets resolve; only a typo is rejected.

On develop the diagnostic consults no rules and ignores --agent, so it contradicts every host. Under a Claude deny rule for git status it prints rtk git status, while rtk hook claude refuses the command outright.

Verification

• cargo fmt --all · cargo clippy --all-targets clean · cargo test --all green; each commit builds and tests independently.
• A characterization commit lands first, pinning the real (exit code, stdout) pairs end-to-end — rtk rewrite's exit codes had no Rust test at all, only a hand-copied table in exit_code_protocol that never called run(). Those tests pass unmodified across the refactor.
• Differential-tested against the base binary across an adversarial corpus (CRLF, NBSP, ZWSP, RTL override, 50 KB command, 200-segment chain, heredocs, substitutions): byte-identical on every path except rtk hook check.
• Property-fuzzed with a throwaway harness driving the built binary — not part of the diff, so it is not re-runnable by CI or a reviewer; the invariants it covers are the ones asserted in the committed tests. Across ~6k generated cases: exit codes stay in range, stdout appears exactly for the two rewrite outcomes, a deny rule is always enforced, rewriting is idempotent, no hook ever emits updatedInput identical to its input, and — the one that matters — a rewrite is never auto-allowed unless an allow rule covers every segment (security: hook auto-allow bypasses agent permission model for rewritten commands #1155, security: compound command permission escalation — single allowed segment grants auto-allow to entire chain #1213).
• Out-of-crate suites unchanged from their pre-refactor baselines: hooks/claude/test-rtk-rewrite.sh 58/66, hermes 18 passed, scripts/test-all.sh 105/13/5.
• After rebasing onto the recall feature (test(grep): pin -l/-L parity with grep, including numeric patterns #3258 line), track_tee_read is called under exactly upstream's conditions on both paths — verified by differential-testing the recall store against the upstream binary. rtk hook check deliberately does not record, matching upstream.

Not in scope

• Making rtk rewrite defer on the identity case (above).
• rewrite_cmd judges every subprocess-path agent by Claude's rules, since rtk rewrite cannot be told who is asking.
• hook_cmd's tests read the developer's config.toml (26) and ~/.claude rules (5); pre-existing, same count on the base commit, and fixing it means giving all six hosts the injection seam only three have.

🤖 Generated with Claude Code

[rtk-wshm-sync-bot]

wshm · Automated triage by AI

📊 Automated PR Analysis

♻️ Type	refactor
🟡 Risk	medium

Summary

Consolidates the duplicated hook decision logic used by 'rtk hook ' and 'rtk rewrite' into a single shared 'decide' function in src/hooks/decision.rs. As a side effect, 'rtk hook check' is fixed to route through the same rule-consulting decision path (including honoring --agent) instead of calling registry::rewrite_command directly, which previously caused it to disagree with the real hook paths.

Review Checklist

• Tests present
• Breaking change
• Docs updated

---

Analyzed automatically by wshm · This is an automated analysis, not a human review.

[aeppling]

Ran a differential against develop: rtk rewrite and all six in-process hooks are byte-identical across ~140k cases (real harvested commands plus generated shapes, 4 permission profiles, every reachable host×verdict cell, audit logs compared serially to separate Deny from Defer). hooks/claude/test-rtk-rewrite.sh gives the same 58/66 with the same 8 failures on both binaries. fmt/clippy --all-targets/test --all clean.

LGTM, approving. The consolidation is real, and routing hook check through the shared decision is the right fix.

Two notes on the description, no code change needed:

The "Before:" example doesn't reproduce. On develop, --agent gemini git status under a Claude deny rule printed a rewrite, not "denied". That arm is agent: _ with no permission call, and the string "Denied by a permission rule" has never existed on develop. The real contradiction is the claude row: develop reported a rewrite while rtk hook claude denied. Same wording is in b070c26's commit message, so it's worth fixing before merge, since it lands in history either way.

The ~6k property-fuzz cases aren't in the diff. No property-testing dep in Cargo.toml, so that one can't be re-run by CI or a reviewer.

Minor: has_heredoc is deleted rather than moved, so heredocs are now caught incidentally via the << delimiter reading as a redirect target. Behavior is unchanged (verified), but cmd << /dev/null slips redirect_has_file_target's exemption and heredoc_defers pins only the common form.

[KuSh]

Thanks — all three checked out, and the third one turned out to be the interesting one.

The "Before:" example. You're right, and it was worse than one wrong row: I built develop and ran it, and two of the three contradictions I listed were artifacts of intermediate states on this branch rather than anything develop does.

claim	develop actually does
--agent gemini "reported denied"	prints rtk git status — a rewrite
--agent pi "rejected outright"	exit 0, prints the rewrite
rtk git status reported as a rewrite	true

The --agent pi rejection was a regression I introduced in an earlier round of this branch and fixed in a later one; it never existed on develop. Both the description and the commit message now state the claude row you identified, measured rather than recalled.

The fuzz cases. Correct — a throwaway harness driving the built binary, never committed. Rather than drop the claim I've labelled it as not in the diff and not re-runnable by you or CI, and noted that the invariants it covered are the ones the committed tests assert.

has_heredoc. This one I had wrong, and so did my own commit message. It isn't deleted — it lives at registry.rs:264 and rewrite_command calls it at lines 601 and 616. What this PR removed is the hook path's duplicate of that check: get_rewritten called registry::has_heredoc and then called rewrite_command, which asks the same question again.

So the layering is the reverse of "caught incidentally":

• contains_unattestable_construct catches the common forms first — and that part is incidental, since <<EOF stops there only because EOF reads as a file target.
• rewrite_command's has_heredoc is the deliberate guard, and it catches what the first gate lets past, including << /dev/null.

Which is why cmd << /dev/null still defers rather than being rewritten — verified on both binaries across <<EOF, <<'EOF', <<-EOF, << /dev/null and <</dev/null: identical, exit 1, no output.

Your /dev/null observation still stands on its own, though, and I've filed it as #3980. Calling the predicate directly:

            git status <<EOF -> unattestable=true
     git status << /dev/null -> unattestable=false     <- heredoc, not a file
      git status > /dev/null -> unattestable=false     <- correct, the exemption's purpose

The exemption exists for discarded output; applied to a heredoc it is matching a delimiter that happens to be spelled /dev/null. No consequence today because the registry guard backstops it, but the predicate is also the permission gate's "never auto-allow" test, so it is worth being right for the right reason.

The only code change since your approval is that doc comment in decision.rs, which asserted the wrong owner of the guarantee — git diff b070c263 HEAD is those four lines and nothing else.