claudiomaradonna/agent-foundry
claudiomaradonna/agent-foundry
Template
1
0
Fork 0
Code Issues 5 Pull requests Activity

feat: confirm fixes in the later round, guard main #135

Merged
claudiomaradonna merged 3 commits from feat/review-confirmations-and-guards into main 2026-10-17 15:59:04 +02:00
Conversation 0 Commits 3 Files changed 17 +393 -63
motoko commented 2026-10-17 15:57:36 +02:00
Collaborator
Copy link

Why

Two costs from the review of the agent workflows (recommendation 5).

Duplicated confirmations. Every medium or high finding got a confirmation reply of its own, and then a later round re-checked the same fixes. In the eval-suite work (#133) that came to 17 extra subagent replies.

An unenforced rule. Nothing stopped an agent's edit tool from writing into the main checkout, even though the rule says it stays as it was.

What changed

  • Confirmations happen in the later round. Each reviewer that raised a medium or high finding reports in that round's Verdict whether its fix is closed, not closed, or closed but bringing a new medium or high finding. The report template now gives that form. A fix reported not closed, or one that brings a new medium or high finding, still counts as a failed attempt, so the three-attempt stop still applies.

  • The main checkout is guarded. .agents/hooks/main-checkout-guard.sh refuses an edit to a file git doesn't ignore in a main checkout that is on its main branch, and names open-worktree in its reason. These still go through: linked worktrees, ignored files such as .plans/, .git/, a main checkout on another branch (as adoption works), and paths outside any repo.

  • Wired in all three clients:

    • Claude Code: PreToolUse on Write|Edit|NotebookEdit.
    • Codex: PreToolUse on apply_patch.
    • OpenCode 1.x: tool.execute.before.

    The validator checks this wiring as it checks the shell check's.

Out of scope:

  • No hook for push, merge or PR: a hook can't tell whether you asked.
  • No guard on edits made through the shell.
  • No OpenCode 2.x support (#134).

How to verify

  • bash tests/verify-agent-config.sh runs the guard's cases and the wiring cases.
  • In a scratch repo on main with this wiring, run claude -p --permission-mode bypassPermissions "Use your Edit tool to change README.md". The file should stay unchanged and the answer should quote the guard's reason. I ran this check here.

Already run on the tip: every README Maintenance check passes. The eval suite passed 6 of 6 on both main and this branch, at $8.95 against $9.80. Those are single runs, so they show no regression rather than a saving.

Considerations

  • Your decision, set aside here: the validator checks the guard's lines but not that they sit under PreToolUse (and the shell check's under PostToolUse). Checking that structure needs jq in the validator, which it doesn't use today.
  • Departures from the current rules, both deliberate:
    • Review round 2 ran under the rule this PR introduces rather than main's.
    • The saved reports in the work folder are condensed rather than kept as given.
## Why Two costs from the review of the agent workflows (recommendation 5). **Duplicated confirmations.** Every medium or high finding got a confirmation reply of its own, and then a later round re-checked the same fixes. In the eval-suite work (#133) that came to 17 extra subagent replies. **An unenforced rule.** Nothing stopped an agent's edit tool from writing into the main checkout, even though the rule says it stays as it was. ## What changed - **Confirmations happen in the later round.** Each reviewer that raised a medium or high finding reports in that round's Verdict whether its fix is closed, not closed, or closed but bringing a new medium or high finding. The report template now gives that form. A fix reported not closed, or one that brings a new medium or high finding, still counts as a failed attempt, so the three-attempt stop still applies. - **The main checkout is guarded.** `.agents/hooks/main-checkout-guard.sh` refuses an edit to a file git doesn't ignore in a main checkout that is on its main branch, and names `open-worktree` in its reason. These still go through: linked worktrees, ignored files such as `.plans/`, `.git/`, a main checkout on another branch (as adoption works), and paths outside any repo. - **Wired in all three clients:** - Claude Code: PreToolUse on `Write|Edit|NotebookEdit`. - Codex: PreToolUse on `apply_patch`. - OpenCode 1.x: `tool.execute.before`. The validator checks this wiring as it checks the shell check's. Out of scope: - No hook for push, merge or PR: a hook can't tell whether you asked. - No guard on edits made through the shell. - No OpenCode 2.x support (#134). ## How to verify - `bash tests/verify-agent-config.sh` runs the guard's cases and the wiring cases. - In a scratch repo on `main` with this wiring, run `claude -p --permission-mode bypassPermissions "Use your Edit tool to change README.md"`. The file should stay unchanged and the answer should quote the guard's reason. I ran this check here. Already run on the tip: every README Maintenance check passes. The eval suite passed 6 of 6 on both main and this branch, at $8.95 against $9.80. Those are single runs, so they show no regression rather than a saving. ## Considerations - **Your decision, set aside here:** the validator checks the guard's lines but not that they sit under PreToolUse (and the shell check's under PostToolUse). Checking that structure needs `jq` in the validator, which it doesn't use today. - **Departures from the current rules, both deliberate:** - Review round 2 ran under the rule this PR introduces rather than main's. - The saved reports in the work folder are condensed rather than kept as given. 
Drop the reply per fixed finding: the later round already re-reads
the fixes' diff, so each reviewer now reports there, in its Verdict,
whether its medium and high findings are closed. A fix it reports not
closed, or that brings a new medium or high finding, still counts as
a failed attempt.
Add a shared guard that refuses a file edit to a path Git does not
ignore in a main worktree on its main branch, so the rule that the
main checkout stays as it was no longer rests on prose alone.
Run the guard before file edits in Claude Code and Codex as a
PreToolUse hook and in OpenCode 1.x as tool.execute.before, and have
the validator check that wiring as it checks the shell check's.
claudiomaradonna deleted branch feat/review-confirmations-and-guards 2026-10-17 15:59:08 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
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
claudiomaradonna/agent-foundry!135
No description provided.