aboutsummaryrefslogtreecommitdiff
path: root/todo.org
diff options
context:
space:
mode:
authorCraig Jennings <c@cjennings.net>2026-07-28 21:28:29 -0500
committerCraig Jennings <c@cjennings.net>2026-07-28 21:28:29 -0500
commit80624606b8a60d97d8c7600882ddc35f5225469f (patch)
tree4c4e56089c1293ab75863fe50bafefcf5084565e /todo.org
parent3a933a26c12b6350fa46faf89bf86e99bfab0f30 (diff)
downloadrulesets-80624606b8a60d97d8c7600882ddc35f5225469f.tar.gz
rulesets-80624606b8a60d97d8c7600882ddc35f5225469f.zip
feat(publish): isolate the pre-commit review and loop it to approval
The review already ran before every commit. It ran in the context that wrote the diff, which is the part that didn't work. A self-review checks the change against the author's own model of it and can't check the model, so the errors that survive are the ones that were never in the diff: a scope inherited from whoever reported the problem, a blast radius estimated instead of measured. So the reviewer is now a subagent, on every commit including a one-line one. It gets the diff, a one-line claim of what the change does, and the ticket where one exists. It doesn't get the conversation or my reasoning, because those are the model it exists not to hold. The ticket stays in for the opposite reason: it was written before the work and is the only thing that can contradict my claim about my own diff. Findings loop back to the same reviewer until it approves, rather than to a fresh one that can't tell an addressed finding from one that never existed. Three rounds, or a finding that recurs after being reported fixed, and it stops for a human instead. The unattended callers park the task rather than wait for an answer nobody is awake to give. I gave the stance a substantiation floor. An agent told to attack will manufacture findings to satisfy the instruction, and a manufactured finding costs a round and teaches the author to discount the next review. subagents.md said three separate times not to dispatch work this small, so it now carries an Isolation Override: the size heuristics assume the main thread could do the job equally well, and they lapse when its own context is what makes its answer untrustworthy. Field 2 of the Prompt Contract inverts there. Paste your context in and you hand over the very model you spawned the agent to escape. I wrote it by running it on itself: three rounds and thirteen findings. Two were things I'd have shipped. Withholding the ticket made my own claim self-certifying, and the override reaffirmed the contract field that would have destroyed the isolation.
Diffstat (limited to 'todo.org')
-rw-r--r--todo.org31
1 files changed, 31 insertions, 0 deletions
diff --git a/todo.org b/todo.org
index 21e1e98..be3f707 100644
--- a/todo.org
+++ b/todo.org
@@ -39,6 +39,37 @@ Tags are assigned and refreshed by =task-audit=; =task-review= keeps them honest
* Rulesets Open Work
+** DOING [#B] Hostile subagent review before every agent commit :feature:
+:PROPERTIES:
+:LAST_REVIEWED: 2026-07-28
+:END:
+From the roam inbox, 2026-07-28: "code reviews must occur before every commit an agent does, and they should be hostile reviews from a subagent without the agent's context."
+
+Two asks, and only the second is new. The publish flow already mandates a review before every commit (Step 1). What changes is *who reviews*: today the reviewing agent is the one that wrote the change, so it inherits the author's mental model, and =review-code= only *suggests* subagent dispatch, and only "for substantive reviews on large diffs".
+
+Decisions settled with Craig, 2026-07-28, and shipped:
+- *Scope* — every commit. The reviewer's own Phase 0 rules a diff trivial and returns Skipped, which satisfies the gate; the author never rules on their own diff.
+- *Stance* — "adversarial", not "hostile" (Craig's call). An agent told to attack manufactures findings, so the stance carries a substantiation floor: a finding not substantiated against the diff is dropped.
+- *What the reviewer gets* — the diff, a one-line claim of what it does, and the requirement source (ticket, plan, task body) where one exists. Withheld: the conversation, the exploration, the author's rationale. The requirement source stays *in* because it is the only artifact that can contradict the author's claim; withholding it makes the claim self-certifying.
+- *Loop* — re-review until the reviewer approves, turning on blocking findings rather than the verdict token. Bounded at three rounds, and stopped early on a finding that recurs after being reported fixed. Both bounds hand the decision to Craig; the unattended callers park instead.
+- *Adjudication* — Craig, never the author overruling the reviewer.
+- *Home* — the =publish= skill Step 1, with =review-code= carrying the adversarial contract and re-review mode.
+- *The =subagents.md= tension* — resolved with an Isolation Override section: the size heuristics assume the main thread could do the task equally well, and they lapse when its own context is what makes its answer untrustworthy.
+
+Remaining: nothing on the design. The change shipped in this session.
+
+** TODO [#C] start-work Phase 7 still summarizes the old publish flow :chore:solo:
+:PROPERTIES:
+:LAST_REVIEWED: 2026-07-28
+:END:
+=.claude/commands/start-work.md:336-339= hands off with "Follow =commits.md= exactly" and "Run =/review-code --staged= before each commit" — no isolated reviewer, no re-review loop. It was already stale before the 2026-07-28 review change, because =commits.md= moved the publish flow into the =publish= skill in an earlier commit, so the pointer names a file that no longer holds the flow.
+
+Grading: Cosmetic severity (a stale summary beside a correct canonical, and start-work is attended so the escalation target exists) x most users frequently = P3 = [#C].
+
+Fix is to point Phase 7 at the =publish= skill rather than restate the flow, which is what let it drift in the first place. Worth a sweep for other files that restate the flow instead of pointing at it.
+
+Found by the adversarial reviewer during the 2026-07-28 review-flow change, and correctly filed rather than fixed there: the line was untouched by that diff.
+
** DONE [#C] sentry.org calls one loop cycle a "fire" :chore:solo:
CLOSED: [2026-07-28 Tue]
:PROPERTIES: