40 KiB
name, description
| name | description |
|---|---|
| find-and-fix | Run a multi-agent team that audits a codebase, verifies each finding, fixes the survivors, reviews the patches, and merges them — with every claim gated on reproducible evidence. Use when asked to hunt for bugs/vulnerabilities across a codebase and land fixes for them, or when the user says "find and fix", "audit and fix", "hardening pass", "review the codebase and fix what you find", or wants a team of auditors plus implementers plus a reviewer working a bug backlog end to end. |
find-and-fix
Partition the tree → audit in parallel → verify before you fix → fix in isolated worktrees → review → merge. The hard parts are refusing to fix findings that are not real, and refusing to merge changes you have not actually tested.
Do not use this for a single known defect (just fix it) or pure exploration with no intent to land changes.
0. The contract — read this first
This is the executable core. Everything after it is rationale and detail.
The git semantics asserted in §6 and §7 are verified by verify-claims.sh next
to this file — run it after editing those sections. Its scope is deliberate: it
proves local git behaviour only, not GitHub CLI behaviour, team-tool
behaviour, or platform limits. A claim in this skill that is not covered by the
script is unverified and should be labelled as operational knowledge rather than
fact — which is the exact standard this document applies to everything else.
FREEZE git fetch origin --prune # WITHOUT this, origin/master may be stale
base=$(git rev-parse origin/master) # one SHA, used everywhere
mkdir -p .omo/audits # the ledger dir must exist first
# Write the ledger HEADER (see §4) and record the base in it. Do not
# append a bare line before the header, or §4's contract is violated:
# base-sha: $base
# date: <iso8601>
# repo: <owner/name>
Every branch and every diff uses $base. When a step needs "current master"
later (the post-merge check), resolve a NEW sha, record it as
master-after, and label it — never silently re-resolve $base.
Placeholder convention: $base is this frozen SHA; <branch>, <n>, <sha>,
<path> are literal arguments you substitute.
SPAWN team_create({ inline_spec }) # see §5 for the shape
One team_task_create per auditor, confirmer, implementer, and reviewer —
not just auditors. Record each returned task id in the ledger.
team_send_message({ teamRunId, to, body }) # assignment; include the task id
"Work starts on dispatch" means the ASSIGNMENT starts the member; if all
parallel slots are busy, the work queues — so check capacity first.
TASKS Task state machine is strict: pending → claimed → in_progress → completed
Direct pending→completed and claimed→completed are REJECTED.
So: team_task_update(status:"claimed", owner:"<name>")
team_task_update(status:"in_progress")
team_task_update(status:"completed")
Readback after each transition: team_task_list — confirm the state changed.
EVIDENCE Every command whose output you rely on goes in the ledger file
(see §4). Not in chat. The ledger is the audit trail.
REPORTS Auditors write to a gitignored path YOU assign before spawning, e.g.
.omo/audits/<auditor>.md — the path is a spawn-time decision, not a
placeholder. Reports go to team_send_message per batch.
FIX git worktree add <path> -b fix/<slug> $base
implementer pushes, then: gh pr create --base master --title ... --body-file ...
A PR body has three sections: What Problem is / What changes / Not fixed.
REVIEW pr-reviewer posts a verdict: MERGEABLE or CHANGES REQUIRED.
gh pr comment <n> --body-file <f> # NOT gh pr review — see §6
MERGE Only when BOTH hold:
1. CI green AND the step list shows the relevant step EXECUTED
2. review verdict MERGEABLE
gh pr merge <n> --squash --delete-branch
Then confirm state=MERGED and that origin/master advanced, before §7.
CLOSE All tasks terminal → team_shutdown_request + team_approve_shutdown per
member → team_delete. Same turn. Write the handover (§7) at a path you name.
0.1 The tool calls — shapes verified by use
team_create ({ inline_spec: { name, members:[...] } }) # §5 for members
team_task_create ({ teamRunId, subject, description }) # returns a task id
team_task_update ({ teamRunId, taskId, status, owner }) # status per §0 TASKS
team_task_list ({ teamRunId }) # readback + closure check
team_send_message({ teamRunId, to:"<member>", body:"..." }) # delivery is automatic
team_status ({ teamRunId }) # who is active / idle
team_shutdown_request({ teamRunId, targetMemberName })
team_approve_shutdown({ teamRunId, memberName })
team_delete ({ teamRunId })
Notes that cost real time when unknown:
team_send_messagedelivers automatically as a new turn; a member that goes idle after replying is normal, not stalled.- The task board rejects
pending→completedandclaimed→completed; you must pass throughin_progress(§0 TASKS). Readback withteam_task_listafter each transition — that is how you observe the change rather than assume it. - Record each returned task id in the ledger next to its finding id, so "which task tracked which finding" is answerable later.
- Closure requires every task terminal and no shutdown request outstanding.
Read
team_task_listbefore you try to close; if it errors onteam_delete, re-readteam_statusrather than forcing.
Everything below this line is rationale and detail. §0 plus §0.1 is the control skeleton, not a self-contained procedure — it tells you the sequence and the invariants, but you must read §2 (team shape and prompts), §3 (audit and confirmation), §4 (ledger), §5 (team spec), §6 (fix/review/merge commands) and §7 (post-merge checks) to actually execute it. Do not hand §0 to an agent and expect a complete run.
Two gates, and they are different gates — do not collapse them:
- Claim gate. A finding is actionable when its mechanism is derived with
concrete values: type widths, offsets, index values, and who writes each
shared field, with
file:line. Otherwise it is a hypothesis. - Execution gate. A verification counts when there is evidence the relevant path actually ran. A green check whose path never executed proves nothing (§3.1, vacuous verification).
1. Phase 0 — Recon before you spawn
Cheap here, expensive later. Do it yourself, with tools.
git remote -v; git rev-parse --abbrev-ref HEAD; git log --oneline -10
git rev-list --count $base..HEAD
gh repo view --json viewerPermission,visibility # permission, not proof
gh api repos/<o>/<r>/branches/master/protection # see caveat below
gh run list --limit 10 # does CI actually run?
rg -n '^[[:space:]]*(pull_request|push)[[:space:]]*:' .github/workflows --glob '*.{yml,yaml}'
Do not read a 404 from the protection endpoint as "unprotected". It is
ambiguous: the repo or branch name may be wrong, the token may lack access, or the
endpoint may be unavailable. Confirm the repo and branch resolve first (e.g. the
gh repo view above succeeds) before drawing any conclusion, and treat the answer
as advisory — it decides whether force-push and direct merges are safe, so get it
right rather than inferring from a status code.
- Does CI run at all? A run stuck queued with no runner may mean a missing
self-hosted runner — or just capacity, labels, or an outage. Investigate; do
not conclude. The
rgabove finds event keys, but it is reconnaissance: it misseson: [pull_request]list syntax and comments it out nothing. If nothing triggers onpull_request, you have no PR-level verification: build it before auditing (that outranks every bug), or find another gate. - What can be verified locally? Build trees, compile database, whether LSP tooling can see the tree (test it — some reject paths outside their own cwd), and the pinned toolchain versions.
- Conventions. Read
AGENTS.md/CONTRIBUTING.md/MAINTAINERS. Get the commit-subject format, the feature-id list, the style checker, and a real accepted commit body:git log -1 --format='%s%n%b' <sha>.
If the project has no working verification, that becomes task 1 and every fix waits on it. Give it an owner, a deliverable, and an exit condition — a phase with none of those is a dead branch.
2. Phase 1 — Partition and spawn
Split into non-overlapping file domains. A workable shape:
| Member | Scope |
|---|---|
| auditor-core | core libs, memory, IPC, API layers |
| auditor-datapath | dataplane, plugins, drivers, tests |
| implementer-a / -b | fixes (assigned by you) |
| pr-reviewer | every PR |
| confirmer | the independent second pass (§3) |
Two things people forget, both of which cause stalls:
- Staff the confirmer. The verification gate needs a second agent, and the team bounds cap parallelism (observed: 8 members, 4 concurrent workers). Decide up front whether the confirmer is a dedicated member, a rotated role, or the reviewer — and if the reviewers are saturated, the confirmation work queues. Say which.
- Cap open PRs. CI is usually the bottleneck, not the agents. Decide how many PRs may be open at once, and merge before opening more.
Standing-prompt rules:
- Explicit, exclusive path list per member; tell each what a peer owns. Cross-domain leads go to the lead as leads, not as findings.
- Diff-first if the tree is a fork or has a hardening history:
git diff --stat $base...HEAD -- <domain>. In an already-hardened tree the highest-value defects are incomplete guards — hardening applied to some call sites and not others. - Auditors are read-only: no edits, no commits, no builds. Outputs are one
markdown report at a path you name, plus
team_send_messageper batch. - If local builds are banned, tell the implementers up front and say verification comes from CI. Otherwise they burn hours trying to compile.
2.1 Producer contract — state this once, verbatim
Applies to every producer of claims (auditors, confirmers, reviewers).
Per finding:
Location file:line, plus every other call site of the symbol
Mechanism <field/type at file:line> -> <value or offset> -> <the read or
write it enables> -> <the exact input that produces it>
Reachability dataplane / API / CLI / config — and whether proven or assumed
Ownership for each shared field (counters, head/tail, refcounts,
generations): which side writes it, with file:line
Severity crash / DoS / memory safety / leak / robustness
Fix shape smallest correct change, direction only, not implemented
Confidence HIGH / MEDIUM / LOW + one line why (unmarked is treated LOW)
VERIFY FIRST yes/no; if yes, the missing fact and what would settle it
Report must also end with:
## Retracted
<id> is FALSE: <the evidence that killed it, in plain language>
Self-correction is a duty, not a failure. Re-derive your own earlier findings and
retract with the evidence. A retraction section is a deliverable, and a growing
one is a POSITIVE signal about the producer. An agent that marks confidence and
flags what it cannot prove is trusted and kept; an agent that reports everything
at equal volume gets its whole batch re-verified by someone else.
That last paragraph is the highest-leverage thing in the prompt: it is the producer's calibration, not its recall, that decides how much of the batch survives.
3. Phase 2 — Audit, then verify
Auditors report incrementally, per batch — define what "batch" means (e.g. "every ~5 findings or when a file is finished"), because the lead must be able to tell "still working" from "done". Speed of reporting beats completeness of sweep.
The gate, in order:
- Spot-check the top findings yourself. Read the source at the cited lines. Reproduce the arithmetic. Do not accept a finding you cannot see.
- Send an independent confirmation pass for survivors. Give it your settled
verdicts so it does not redo them, and ask for
CONFIRMED / FALSE / UNRESOLVEDplus a derivation, and for confirmed items the minimal fix shape and the caller blast radius. - Correct the producer. Send back what was killed and the evidence, and require the report file to be corrected so it does not stand as a list of live defects.
Tell the confirmers: killing a finding is a success, and a well-derived FALSE counts as much as a CONFIRMED.
Two failure modes recur; name them in the prompt:
- Assumed ownership — the finding assumes a peer or callee can move a value
the local side (or another owner) actually controls. Require the write-owner
with
file:linefor every shared field. - Whole-file reasoning — conclusions from running a tool over entire files rather than the changed hunks. Gate scripts are usually hunk-scoped.
3.1 Vacuous verification — the failure that looks like success
A check "passes" because its precondition never fired: the loop iterated zero times, the branch was never taken, the test step was skipped. Formal verification calls this vacuous satisfaction.
It is uniquely dangerous because a normal defect shows up as failure; this shows up as success. The information you need is not in the source — it is in which branch executed. That is why re-reading the code never finds it. Note "never" is not absolute: source review can spot a missing event trigger or a filter that matches nothing. But you cannot rely on it; you need execution evidence.
A worked example (one run, but the shape generalises): a CI workflow carried
three defects on a single path, none findable by re-reading the YAML. Steps ran
under sh instead of bash and died with Bad substitution; fixing that let the
step complete and emit a test filter with no matching module; and the probe added
to exercise the path produced zero runs because its branch was already merged.
Those were layers on one unexecuted path, each hiding the next — so expect
layers, not a single bug, on any path that has never actually run.
The criterion: do not ask "did it pass?" Ask "did it execute?"
- Demand log evidence the branch ran. Good evidence is a value only the
executed path could produce — and a result, not just a banner. A line like
Running TEST suites:nat64proves the step completed and selected something non-empty; it does not by itself prove the tests ran or passed, so pair it with the test-result output. Contrastconclusion=successwith the test stepskipped: nothing was proven. - Require a negative control where you can: show the gate can fail. A gate that has never failed is weakly validated, not automatically invalid — some gates cannot be safely broken, and static proof carries some claims. But treat an unexercised-failure gate as weaker evidence and label it that way.
- Mutation testing is the systematic form of this. Mutate the code under test and confirm the test fails. A surviving mutant points at a coverage or assertion gap — unless it is equivalent or deliberately out of contract.
When you cannot construct the input, say so and treat the run as non-evidence. "I could not make the acceptance run exercise this path" is a finding. "The run was green" is not, when the path never ran.
4. The evidence ledger
"Show the command and its output" is not enough if the output lives in chat and
scrolls away. Keep one file, e.g. .omo/audits/LEDGER.md (gitignored).
Header, once, before anything else:
base-sha: <sha> # the frozen $base from §0, recorded
date: <iso8601>
repo: <owner/name>
One block per finding. The evidence field must carry provenance, not a
bare output line — a line copied from the wrong run or wrong job satisfies a
one-line schema and proves nothing:
LEDGER-FINDING <id> <status: hypothesis|confirmed|false|fixed|merged>
- claim: one line
- derivation: the arithmetic / ownership facts, with file:line
- confidence: HIGH|MEDIUM|LOW (from the producer; VERIFY FIRST if flagged)
- verdict: who confirmed it, how, and what settled it
- evidence: <exact command>
exit=<code> at=<iso8601> by=<who/where>
<key output lines — the value that only this path could produce>
- ci: run=<id> job=<id> head-sha=<sha> step="<name>" = <conclusion>
- fix: branch=<name> commits=<sha[,sha]> pr=<n>
- merge: merge-commit=<sha> master-before=<sha> master-after=<sha>
Two fields earn their keep by being easy to omit: exit= (a command that
failed and printed something plausible is not evidence) and head-sha= (the
run you inspected must be the run for the commit you tested — a branch can have
several). If you find yourself unable to fill head-sha= or step=, you have not
actually verified anything.
This is what makes the run auditable and what you hand over. It is also how you survive context exhaustion: raw logs go to files, the ledger holds conclusions.
5. Team spec
{
"name": "<project>-hardening",
"members": [
{ "name": "auditor-core", "kind": "category", "category": "deep",
"prompt": "<role> <exclusive domain> <method, in order> <report path>
<report format §2.1 verbatim> <prohibitions>" },
{ "name": "auditor-datapath", "kind": "category", "category": "deep",
"prompt": "<same, disjoint domain>" },
{ "name": "confirmer", "kind": "category", "category": "deep",
"prompt": "<read-only> Verify findings handed to you by the lead: verdict
CONFIRMED/FALSE/UNRESOLVED with a derivation, plus the minimal
fix shape and caller blast radius for confirmed items. Killing a
finding is a success. Work from <report path> and the source;
never edit. The lead hands you settled verdicts to avoid redoing
them." },
{ "name": "implementer-a", "kind": "category", "category": "deep",
"prompt": "<fix assigned defects only; minimal diffs; atomic commits;
push + gh pr create; honest 'could not verify' reporting>" },
{ "name": "implementer-b", "kind": "category", "category": "deep",
"prompt": "<same, own worktree>" },
{ "name": "pr-reviewer", "kind": "category", "category": "ultrabrain",
"prompt": "<review every PR; caller-classification first; verdict
MERGEABLE or CHANGES REQUIRED>" }
]
}
Six members is deliberate and fits the observed bounds (8 members, 4 concurrent
workers). If you have fewer slots, do not drop the confirmer — the
verification gate is the point of the pipeline. Instead drop one auditor and give
its domain to the other, or make pr-reviewer also the confirmer and accept that
review work queues behind confirmation. Say which you chose. A team spec with no
confirmer contradicts §2 and silently removes the gate that makes findings
trustworthy.
Prompts must be self-contained — a member cannot see your audit conversation, so a brief that says "fix the bug we discussed" is unusable. Every prompt carries: role, scope, method, deliverable path, report format, and prohibitions. Vague prompts produce vague work.
6. Phases 3–5 — fix, review, merge
Fix. Create the worktree and branch yourself, from the frozen base:
git worktree add <path> -b fix/<slug> $base. Do not let two implementers share
one, and do not let them create their own (they collide or pick a stale base).
The brief carries the derivation, the required shape, an explicit
out-of-scope list, and the acceptance criteria. Constrain it: minimal, no
refactor, no drive-by reformatting. Atomic commits, one per independent logical
unit, verified against the project's own gate script. Then:
git push -u origin fix/<slug>
gh pr create --base master --title '<feature>: <subject>' --body-file /tmp/body.md
The body has three sections: ## What Problem is, ## What changes,
## Not fixed. The last is where honesty lives.
If a fix depends on an unmerged PR's new symbol, stack it: branch from that
PR's branch and gh pr create --base <that-branch>. After the base merges,
retarget with gh api -X PATCH repos/<o>/<r>/pulls/<n> -f base=master
(gh pr edit is unreliable on some repos).
Never rebase a pushed branch by default — but that is a policy default, not a law. After a base PR is squash-merged, a stacked branch often needs rebasing or recreation to drop the base's pre-squash commits; a merge commit can leave duplicate logical commits and a confusing diff.
Concrete procedure once the base is merged. Record the base branch's tip before
it merges — call it B; call the base's merge commit on master M:
git fetch origin
# Option A (preferred where history rewriting is allowed): rebase the stacked
# branch's own commits onto M, dropping the base's pre-squash commits.
git switch <stacked-branch>
git rebase --onto $M $B <stacked-branch>
git push --force-with-lease origin <stacked-branch>
gh api -X PATCH repos/<o>/<r>/pulls/<n> -f base=master # retarget
# Option B (where force-push is forbidden): merge master in, and review the result.
git merge origin/master
git diff --name-status HEAD^..HEAD # confirm nothing reverted
git push origin <stacked-branch>
Verify the outcome either way — this is the check that catches a botched rewrite:
gh pr diff <n> --name-only # should list ONLY the stacked branch's files
gh pr view <n> --json mergeable,baseRefName
Two traps, both verified against a scratch repo:
git log --no-merges M..<branch>does not isolate "the real work". After a squash merge it lists the base's original commits as well, because the squash produced a different SHA — the base's work appears twice (once asM, once as the branch's own commit). Do not use that range to decide what to cherry-pick.git diff M...<branch>likewise shows the base's files too, for the same reason:Mis not an ancestor of the branch, so the merge base is still the old branch point. Onlygit rebase --onto $M $B(or an equivalent explicit cherry-pick of commits in$B..<branch>) yields a branch whose diff againstMis just its own work.
If a D appears for a file the branch never touched, see §7 — it may be staleness,
not a revert. State in the PR which option you used and why.
Review. One reviewer, every PR, verdict exactly MERGEABLE or
CHANGES REQUIRED, with per-finding file:line, mechanism, required change.
Require the reviewer to emit the verdict as a bare token on its own line, in a fixed prefix form, so it can be read back mechanically:
VERDICT: MERGEABLE
not prose containing the word ("I would not call this MERGEABLE yet" defeats a
grep-based readback — the §6 check greps for the token and would extract the
wrong one). The reviewer must also name the head SHA it reviewed, so a verdict
issued before a later push is visibly stale.
This is what the merge gate reads in §6 step 2b; without the fixed form the readback is a guess.
First check, and it is the one that catches this whole pipeline's defect class:
Enumerate every reference of the symbol being changed, then classify which paths the fix protects. Not "every site must appear in the diff" — a root-level accessor fix or a wrapper change correctly protects callers without touching them. And grep misses macro-expanded, generated, and indirect references. Require: every vulnerable path is covered, and say how many sites exist and how many the fix reaches.
Then: does it remove the defect at the root for every reachable path (a guard validating the wrong thing, or sitting after the dereference, is a rejection); is the value genuinely untrusted there; does the test fail before and pass after (a test that cannot distinguish is a rejection); scope honesty; project style and commit gates.
Operational notes: gh pr review is rejected when reviewer and author share an
account — use gh pr comment <n> --body-file <f> and say so, or a reader will
think no review exists. gh pr edit may fail with a GraphQL Projects-classic
error; use the gh api -X PATCH form.
Merge. Two conditions, both required:
- CI green, read from the step list — and the relevant step executed.
- Review verdict
MERGEABLE.
Every check below fails closed: it exits non-zero, so you cannot slide past a
missing run or an unmerged PR. That matters because the obvious forms do not —
jq 'select(...)' emits nothing and exits 0 when nothing matches, last on an
empty array yields null, and [ cond ] || echo "warning" returns 0. Verified:
$ echo '{"a":1}' | jq 'select(.b=="x")'; echo $? # no output, 0
$ echo '[]' | jq 'sort_by(.x)|last|.y'; echo $? # null, 0
$ bash -c '[ 1 = 2 ] || { echo warn; }'; echo $? # warn, 0 <- does NOT fail closed
set -euo pipefail
# 0. Record the master SHA before merging, so you can prove it advanced.
# §7 reuses this as $BEFORE; keep it in the ledger as master-before.
BEFORE=$(git rev-parse origin/master)
# 1. Select the run deterministically: pin commit AND workflow, filter to the
# pull_request event, take the newest. A commit can have several runs.
HEAD=$(gh pr view <n> --json headRefOid --jq .headRefOid)
RUN=$(gh run list --commit "$HEAD" --workflow "<workflow-name>" \
--json databaseId,event \
--jq '[.[] | select(.event=="pull_request")] | sort_by(.databaseId) | last | .databaseId')
[ -n "$RUN" ] && [ "$RUN" != "null" ] || { echo "no run for $HEAD — do NOT merge" >&2; exit 1; }
# 2. Assert THAT run finished successfully and belongs to this head.
# 'select' emits nothing on no match, so assert the output is non-empty.
MATCH=$(gh run view "$RUN" --json status,conclusion,headSha \
--jq 'select(.headSha=="'"$HEAD"'" and .status=="completed" and .conclusion=="success")')
[ -n "$MATCH" ] || { echo "run $RUN is not a completed success for $HEAD" >&2; exit 1; }
# 2b. Read back the review verdict instead of remembering it. The reviewer emits
# "VERDICT: <token>" on its own line (see the Review section), so anchor the
# grep to that prefix — a bare token search would match prose like
# "I would not call this MERGEABLE yet" and extract the wrong verdict.
VERDICT=$(gh api repos/<o>/<r>/issues/<n>/comments --jq '[.[].body] | join("\n")' \
| grep -oE '^VERDICT: (MERGEABLE|CHANGES REQUIRED)$' | tail -1 \
| sed 's/^VERDICT: //')
[ "$VERDICT" = "MERGEABLE" ] || { echo "review verdict is '$VERDICT', not MERGEABLE — do not merge" >&2; exit 1; }
# Also confirm the verdict postdates the head: if the reviewer named a head SHA,
# it must equal $HEAD, or the verdict is stale and the PR needs a re-review.
# 3. Assert the dependent step ran and succeeded. grep -qx fails on "skipped",
# on "failure", and on a step that does not exist — that is the point.
JOB=$(gh run view "$RUN" --json jobs --jq '.jobs[]|select(.name=="<job>")|.databaseId')
[ -n "$JOB" ] || { echo "job <job> not found in run $RUN" >&2; exit 1; }
gh api repos/<o>/<r>/actions/jobs/"$JOB" \
--jq '.steps[] | select(.name=="<step>") | .conclusion' | grep -qx success \
|| { echo "step <step> did not succeed (skipped/failed/missing)" >&2; exit 1; }
# Record run id, job id, step and its conclusion in the ledger.
# 4. Merge, then poll until it is ACTUALLY merged (a queue or auto-merge returns
# before master moves). Exit non-zero if it never merges.
gh pr merge <n> --squash --delete-branch
state=""
for i in $(seq 1 30); do
state=$(gh pr view <n> --json state --jq .state)
[ "$state" = "MERGED" ] && break
sleep 10
done
[ "$state" = "MERGED" ] || { echo "not MERGED after 5min — stop here" >&2; exit 1; }
# 5. Prove master advanced, rather than assuming.
git fetch origin
AFTER=$(git rev-parse origin/master)
[ "$BEFORE" != "$AFTER" ] || { echo "master did NOT advance — the merge did not land" >&2; exit 1; }
git log --oneline -3 origin/master
Step 3 is the one that enforces this document's whole thesis: it asserts the
step's conclusion instead of inviting the reader to eyeball it, so a skipped
test step fails loudly rather than merging silently.
An agent can satisfy "inspect the steps" while inspecting the wrong run's job, so
pin by commit and workflow, take the newest matching run, and record the ids.
Note step 3 asserts the step's conclusion rather than eyeballing it: a skipped
test step is the exact failure this document exists to prevent, so make the check
fail loudly instead of relying on the reader to notice.
A skipped test step is disqualifying when that test is the claimed behavioural evidence, and acceptable only for changes whose correctness is independently established and whose scope does not need it. Say which clause you applied.
Do not merge to unblock. If the gate is broken, fix the gate — merging "the one that passes" while the rest are blocked by your own defect hides the defect and teaches everyone to ignore red.
7. After the merge — check the merged tree
Merging is not finished when the command returns. Check that master actually moved and that the merged tree contains what you intended. Two traps live here: the squash-captures-a-stale-head trap, and the two-tree-diff trap.
So after merging:
git fetch origin && git log --oneline origin/master -3
git diff --name-status $BEFORE origin/master # $BEFORE is the pre-merge master from §6
And for any PR whose branch predates a recent master merge, know what each
command does and does not tell you before trusting MERGEABLE (MERGEABLE means
"no conflict", not "changes only what it says"):
# 1. Does the branch contain the recent master state?
git merge-base --is-ancestor <master-sha> <branch> \
|| echo "branch predates it"
# 2. The branch's own patch, relative to the merge base (three-dot):
git diff --name-status <master-sha>...<branch>
# 3. Two final trees compared (whole-tree divergence):
git diff --name-status origin/master <branch>
# 4. The ACTUAL merge result, without merging (authoritative):
git merge-tree --write-tree origin/master <branch> # tree oid, or conflict report
git ls-tree -r --name-only <tree-oid> # what the merged tree contains
Command 3 does NOT predict what a merge will do. This is a trap worth naming.
A branch cut before a file was added to master shows D <file> in command 3 —
yet a three-way merge preserves that file, because the branch never deleted
it. Verified:
$ git diff --name-status master feature
D master-only.txt # looks like a deletion
$ git merge-tree --write-tree master feature
$ git ls-tree -r --name-only <tree>
base.txt
master-only.txt # actually preserved
So do not read a D as "the merge will delete this". Use command 4 for the merge
result, and command 2 to see what the branch itself changed. A genuine revert is
the branch modifying a path that master changed — visible in command 2, not as a
bare D in command 3. (A D in command 3 is still worth looking at: it means
the branch lacks something master has, which may be staleness or may be a real
deletion. Command 2 distinguishes them.)
git merge-tree --write-tree needs Git ≥ 2.38. On older Git, fall back to a
throwaway worktree: git worktree add /tmp/probe <branch> && git -C /tmp/probe merge --no-commit --no-ff origin/master and inspect, then remove it.
After merging, confirm the merge actually happened before checking the tree —
gh pr merge can enter a merge queue or enable auto-merge rather than completing:
gh pr view <n> --json state,mergeCommit # state must be MERGED
git fetch origin && git log --oneline origin/master -3
git diff --name-status $BEFORE origin/master # $BEFORE is the pre-merge master from §6
A squash merge captures the head SHA actually selected and merged. A commit pushed to the branch after that head was chosen is not included — so a cleanup commit can silently miss the merge. (Say "after the selected head", not "after the PR was queued": a queued or auto-merge PR may be revalidated against a later head, in which case the later commit is included.)
Real case: a probe file was added so a PR's own diff would exercise a code path;
the removal commit was pushed seconds after the head was selected and never
entered the merge. The probe file landed on master. (Note the naive explanation
"squash resurrected a deleted file" is wrong — squash cannot do that. And note
that this is a different phenomenon from the D-in-command-3 trap above; do not
conflate the two.)
Clean up, and verify the cleanup rather than assuming it:
git worktree list # only the main checkout should remain
git worktree prune
git branch -a --list 'fix/*' # merged branches should be gone
git ls-remote --heads origin 'fix/*' # including on the remote
Then close the team: all tasks terminal → team_shutdown_request +
team_approve_shutdown per member → team_delete, in the same turn as the last
task completes. Read team_task_list first: if anything is still
pending/claimed/in_progress, the closure contract is not met.
Handover (write it to a path you name, e.g. .omo/audits/HANDOVER.md) — the
next session reads this, not your chat:
master: <sha>
merged: <PR list with commit shas>
open: <PR list, each with its exact blocker>
left: <findings recorded but not tasked, with file:line and fix direction>
verify: <what is proven, what is unproven, and how to re-check>
next: <exact commands to continue>
Every claim in it needs a command that reproduces it. A handover that says "verified" without the command is the same vacuous evidence the rest of this document warns about.
8. Handling the things that go wrong
| Situation | Do this |
|---|---|
| Member goes idle with no report | Idle is normal after a turn, not an error. But a member that never reports and never completes is stalled: ask it directly, with a bounded question. If it produces nothing twice, reassign the work — do not wait on the wall clock. |
| Audit finds nothing | That is a valid outcome, but it needs an artifact: a zero-findings report listing what was covered and what was not. "Nothing found" must not look like an abandoned run. |
| Context filling up | Push conclusions to the ledger (§4), raw logs to files. Summarise and discard. A long multi-agent run will exhaust the lead's context if everything stays in chat. |
| Wall-clock budget | Team runs have a wall-clock limit (observed: 120 min). Budget it: recon, audit, fix, CI waits. CI is the long pole — start runs early, keep several in flight, and do not serialise behind one build. |
| CI slow or flaky | Distinguish: PR defect / infra failure / flaky test / queued runner / missing required check. Merge independent fast-checked fixes first; cancel superseded runs. Do not merge on a flake — re-run and see it twice. |
| Two findings touch the same file | Non-overlapping audit domains do not imply non-overlapping fixes. Combine them into one branch, or serialise them — never two open PRs editing the same lines. |
| A probe produces no run | Verify the run exists before trusting it — but first read the workflow's on: block and its filters, because the trigger is conditional. reopened fires only if the workflow lists it in types:; a branch push fires only if the workflow file exists on that ref and its branches:/paths: filters match. Check rg -n -A6 '^on:' .github/workflows/<wf>.yml, then confirm a new run id appears. |
| Empty or absent report | Treat "no report, no completion signal" as unfinished, not as clean. Ask, then escalate. |
9. Anti-patterns
Observed examples from one run; the fix is the general rule.
| Anti-pattern | Looks like | Do instead |
|---|---|---|
| Vacuous verification | A check passes because its precondition never fired — loop ran zero times, test step skipped. Looks like success, so re-reading code cannot find it |
Ask "did it execute?" not "did it pass?" (§3.1) |
| Layered masking | Fixing one defect on an untested path reveals the next it was hiding; looks like a run of independent mistakes | Expect layers on any path that never ran; treat each green as provisional until the whole path has executed |
| Vacuous probe | A commit added to exercise a path produces zero runs (branch can't fire an event) — the verification attempt is itself unverified | Check the run id exists before trusting it |
| Uniform-confidence reporting | Every finding equally certain; no confidence marks, no VERIFY FIRST, no retractions |
Put calibration duties in the prompt at spawn (§2.1) |
| Accepting a workflow on a run that could not exercise it | A CI change merged on a green run whose diff never reached the new path | Make the acceptance run touch a path that exercises the change, or treat the run as non-evidence |
| Whole-file tool reasoning | Running a formatter over a whole file, seeing hundreds of diffs, concluding "the tool is unreliable" and dismissing a correct finding | Run the project's own gate script, hunk-scoped, with the pinned version; calibrate a local tool against a file the change does not touch |
| Merging on a green conclusion | conclusion=success while Run the tests is skipped |
Read the step list |
Trusting MERGEABLE alone |
A PR reports no conflict and builds fine, but its branch predates a master merge and the merge would revert a file | git diff --name-status origin/master <branch>; check ancestry with git merge-base --is-ancestor |
| Commit pushed after the merge queued | A cleanup commit sits on the branch but is absent from master, because squash captured the earlier head | Check the merged tree after every merge (§7) |
| Dispatch trigger that cannot fire | gh workflow run --ref <branch> returns 422 because the branch predates the workflow file |
Close and reopen the PR to fire reopened; confirm a new run id |
| Merge commit failing the commit gate | Contributor merges the base branch in; the gate rejects Merge remote-tracking branch ... for having no feature id |
Iterate the PR range with git rev-list --reverse --no-merges <base>..<head> |
| Sentinel collision | A "not set" sentinel of 0 colliding with a legitimate open() returning 0 |
Use -1, or force the descriptor above 0 |
| A check that can never fire | if (!ptr) right after a raw accessor that never returns NULL — the wild dereference already happened |
Validate inside the accessor, or promote a checked accessor and adopt it at trust boundaries |
| Overclaiming in a PR body | "The existing test would fail before this change" when the test never reaches the changed function | Verify against the call chain, or state the gap |
10. Checklist
- Recon: remotes, permission, does CI run, local verification, conventions
- Base SHA frozen and recorded once; every branch made from it
- Domains partitioned, no overlap, each member told what a peer owns
- Confirmer staffed; open-PR cap decided
- Standing prompts carry scope, report path, format, and calibration duties
- Auditors read-only; reporting per batch with a defined batch size
- Evidence ledger started; commands and outputs recorded there
- Every finding spot-checked, then independently confirmed; producers corrected
- Every green gate confirmed to have executed the path it claims to check
- Worktree + branch created by the lead per confirmed finding
- Briefs self-contained, with out-of-scope and honest-verification clauses
- PRs created with
gh pr create; body has all three sections - Every PR reviewed with the caller-classification check first
- Merged only on executed-CI-green + MERGEABLE; merged tree checked afterwards
- Zero-finding or abandoned-domain outcomes have an artifact
- Worktrees and branches cleaned; team closed in the same turn as the last task
- Handover written: what merged, what is left, exact next commands