ux(steering): make every hook deny / tool refusal terminal or forwarding — no guard cycles, no dead ends, explicit "nothing executed" + "do not retry raw git" text #15

New issue

Closed

opened 2026-07-11 21:35:05 +00:00 by hexajon · 1 comment

hexajon commented

2026-07-11 21:35:05 +00:00

(Migrated from codeberg.org)

Problem — agents hammer the guards because the texts are context-blind and can point at each other

Live case study (2026-07-10, a foreign-repo worktree flow on another repo; companion to #14 which covers the reconcile-mode gap itself):

Agent runs git merge origin/main in its linked worktree (allowed) → conflict → worktree now has merge-in-progress state.
Agent runs cd <wt> && git add <file> && git commit → PreToolUse hook (hooks/scripts/check-git-local.sh) blocks the WHOLE invocation and steers to madt_commit(dir=…). The text does not say that no part of the command executed — the agent later assumed the git add had run.
madt_commit dir=<wt> → refused (internal/mcp/tools_localgit.go): foreign repo has a merge in progress, "resolve or abort that operation directly in the target repo". That text reads as "use raw git here" — the exact thing the hook in step 2 steers away from. The two guards point at each other.
Agent tries git merge --continue → fails with "unmerged files" — not a guard at all, but the agent can't tell: the staging from step 2 never happened (see step 2's missing "nothing executed"). From the agent's seat this is a third consecutive wall, indistinguishable from more steering.
Working exit (found by trial): madt_add dir=<wt> + git merge --continue + madt_push dir= branch= + madt_prs action=merge owner_repo=. Nothing in any guard text names this path.

Net effect: a well-resourced caller bounced 4 times; a lower-capability caller would still be hammering. The failure mode is not too little text — it's non-terminal text: rails that don't end in an executable next step for the state the repo is actually in.

Design rules for every deny/steer/refusal (the ask)

The guard graph must be a DAG that terminates in executable commands. Every deny/steer/refusal either (a) gives the exact replacement call for the current arguments and repo state, or (b) forwards to a tool whose own refusal text continues the chain. Two guards may never reference each other's territory without naming the state that breaks the loop. Add a test: for each steer target × each documented refusal state of that target, the refusal text names a next hop that is not the steered-away-from form.
Compound-command denials state execution semantics explicitly: "This ENTIRE invocation was blocked; none of it ran (the git add did not execute)." One sentence kills the step-4 class of confusion.
State-aware steers where cheap: check-git-local.sh can detect MERGE_HEAD in the target repo and emit the merge-in-progress rail directly (madt_add the resolutions + git merge --continue are the sanctioned pair — same rails madt_orchestrate reconcile already prints), instead of the generic madt_commit redirect that will bounce.
Say what NOT to do next: deny texts add one line — "Do not retry any raw git commit variant; every form is steered, every time." Agents probe variants when the boundary is implied rather than stated.
State the rule itself, self-contained — never cite internal references. One plain sentence of WHY per text ("raw git against a credentialed remote can leak the token", "there is no stage-everything mode — declare the paths you mean", "madtea never force-pushes") so agents can generalize the boundary instead of rediscovering it per command shape. Steering/error text must NOT name ADR numbers, internal issue numbers, or internal repo/host names: those references don't travel with the binary — for any user outside this org they are noise at best, broken pointers at worst. Rationale citations live in docs/ADRs; shipped strings carry the rule, not the reference.
Refusal texts get the same treatment as steers: tools_localgit.go's foreign merge-in-progress refusal names the working exit verbatim (madt_add dir= + git merge --continue, or reconcile once #14 lands dir=), not "directly in the target repo".

Acceptance

Hook-tests (hooks/tests) cover: compound-block text contains the nothing-executed sentence; merge-in-progress steer emits the madt_add + merge --continue rail; commit-steer contains the do-not-retry line.
madt_commit foreign merge-in-progress refusal names the executable exit; a table-driven test walks steer→refusal chains for cycles (no refusal may recommend a form the hooks block).
A short doc section (docs/architecture or the steering ADR) records the six rules so future guards are written to them.
A lint/test over the hook scripts and steer/refusal string constants rejects internal-reference literals (ADR \d+, #\d+-style issue citations, internal repo/host names); existing violations swept in the same change. Dynamic user-facing numbers (the caller's own PR/issue in a rail like reconcile=29) are exempt — the lint targets hardcoded literals only. Note: tool DESCRIPTIONS ship with the binary too and currently carry the same internal issue citations — decide in this issue whether the sweep covers them or that splits out.

## Problem — agents hammer the guards because the texts are context-blind and can point at each other Live case study (2026-07-10, a foreign-repo worktree flow on another repo; companion to #14 which covers the reconcile-mode gap itself): 1. Agent runs `git merge origin/main` in its linked worktree (allowed) → conflict → worktree now has merge-in-progress state. 2. Agent runs `cd <wt> && git add <file> && git commit` → PreToolUse hook (`hooks/scripts/check-git-local.sh`) blocks the WHOLE invocation and steers to `madt_commit(dir=…)`. The text does not say that **no part of the command executed** — the agent later assumed the `git add` had run. 3. `madt_commit dir=<wt>` → refused (`internal/mcp/tools_localgit.go`): foreign repo has a merge in progress, "resolve or abort that operation directly in the target repo". That text reads as "use raw git here" — the exact thing the hook in step 2 steers away from. **The two guards point at each other.** 4. Agent tries `git merge --continue` → fails with "unmerged files" — not a guard at all, but the agent can't tell: the staging from step 2 never happened (see step 2's missing "nothing executed"). From the agent's seat this is a third consecutive wall, indistinguishable from more steering. 5. Working exit (found by trial): `madt_add dir=<wt>` + `git merge --continue` + `madt_push dir= branch=` + `madt_prs action=merge owner_repo=`. Nothing in any guard text names this path. Net effect: a well-resourced caller bounced 4 times; a lower-capability caller would still be hammering. The failure mode is not too little text — it's **non-terminal text**: rails that don't end in an executable next step for the state the repo is actually in. ## Design rules for every deny/steer/refusal (the ask) 1. **The guard graph must be a DAG that terminates in executable commands.** Every deny/steer/refusal either (a) gives the exact replacement call for the *current arguments and repo state*, or (b) forwards to a tool whose own refusal text continues the chain. Two guards may never reference each other's territory without naming the state that breaks the loop. Add a test: for each steer target × each documented refusal state of that target, the refusal text names a next hop that is not the steered-away-from form. 2. **Compound-command denials state execution semantics explicitly:** "This ENTIRE invocation was blocked; none of it ran (the `git add` did not execute)." One sentence kills the step-4 class of confusion. 3. **State-aware steers where cheap:** `check-git-local.sh` can detect `MERGE_HEAD` in the target repo and emit the merge-in-progress rail directly (madt_add the resolutions + `git merge --continue` are the sanctioned pair — same rails madt_orchestrate reconcile already prints), instead of the generic madt_commit redirect that will bounce. 4. **Say what NOT to do next:** deny texts add one line — "Do not retry any raw `git commit` variant; every form is steered, every time." Agents probe variants when the boundary is implied rather than stated. 5. **State the rule itself, self-contained — never cite internal references.** One plain sentence of WHY per text ("raw git against a credentialed remote can leak the token", "there is no stage-everything mode — declare the paths you mean", "madtea never force-pushes") so agents can generalize the boundary instead of rediscovering it per command shape. Steering/error text must NOT name ADR numbers, internal issue numbers, or internal repo/host names: those references don't travel with the binary — for any user outside this org they are noise at best, broken pointers at worst. Rationale citations live in docs/ADRs; shipped strings carry the rule, not the reference. 6. **Refusal texts get the same treatment as steers:** `tools_localgit.go`'s foreign merge-in-progress refusal names the working exit verbatim (madt_add dir= + git merge --continue, or reconcile once #14 lands `dir=`), not "directly in the target repo". ## Acceptance - Hook-tests (hooks/tests) cover: compound-block text contains the nothing-executed sentence; merge-in-progress steer emits the madt_add + merge --continue rail; commit-steer contains the do-not-retry line. - madt_commit foreign merge-in-progress refusal names the executable exit; a table-driven test walks steer→refusal chains for cycles (no refusal may recommend a form the hooks block). - A short doc section (docs/architecture or the steering ADR) records the six rules so future guards are written to them. - A lint/test over the hook scripts and steer/refusal string constants rejects internal-reference literals (`ADR \d+`, `#\d+`-style issue citations, internal repo/host names); existing violations swept in the same change. Dynamic user-facing numbers (the caller's own PR/issue in a rail like `reconcile=29`) are exempt — the lint targets hardcoded literals only. Note: tool DESCRIPTIONS ship with the binary too and currently carry the same internal issue citations — decide in this issue whether the sweep covers them or that splits out.

hexajon commented

2026-07-14 21:33:20 +00:00

(Migrated from codeberg.org)

Evidence handoff from the #39 sweep (adversarial-review finding, deliberately deferred here because #39's text assigns the steering/refusal string class to this issue):

hooks/ ships to plugin users via scripts/assemble-plugin-artifact.sh (hooks/hooks.json + hooks/scripts/*.sh) and carries 208 dangling 3+-digit tracker refs, including user-facing refusal text — e.g. hooks/scripts/check-foreign-primary.sh:49 and :57 inside the reason= denial strings ("#2032, ADR 0019" / "Launch-directory scope (#2063)"). Those numbers don't resolve on this tracker, so every hook denial a plugin user sees cites a ghost issue.

#39 landed the shared static rule for exactly this: internal/danglingref exports Findings/Pattern/allowedNumbers, and its package doc notes this issue should call the same rule. Design rule 5's lint over hook scripts and steer/refusal constants can extend that scan set (hooks/hooks.json, hooks/scripts/*.sh) and sweep the refusal strings to carry the rule, not the reference.

Evidence handoff from the #39 sweep (adversarial-review finding, deliberately deferred here because #39's text assigns the steering/refusal string class to this issue): `hooks/` ships to plugin users via `scripts/assemble-plugin-artifact.sh` (`hooks/hooks.json` + `hooks/scripts/*.sh`) and carries **208 dangling 3+-digit tracker refs**, including user-facing refusal text — e.g. `hooks/scripts/check-foreign-primary.sh:49` and `:57` inside the `reason=` denial strings ("#2032, ADR 0019" / "Launch-directory scope (#2063)"). Those numbers don't resolve on this tracker, so every hook denial a plugin user sees cites a ghost issue. #39 landed the shared static rule for exactly this: `internal/danglingref` exports `Findings`/`Pattern`/`allowedNumbers`, and its package doc notes this issue should call the same rule. Design rule 5's lint over hook scripts and steer/refusal constants can extend that scan set (`hooks/hooks.json`, `hooks/scripts/*.sh`) and sweep the refusal strings to carry the rule, not the reference.

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

sixfold-space/madtea#15

No description provided.

Rows
Columns