chore(source-control): review-thread resolution has no audit trail, and the guarded wrapper is routinely bypassed

[kyle-sexton]

Filed by AI, then independently attacked by a second session. Four claims held, one was overstated,
and the remedy ordering was wrong on the original filing's own logic. Revised below; the corrections
are marked so the earlier reasoning is not silently replaced.

What happened

#2100 and #2115 merged with all review threads resolved. Two Claude sessions were working
that sweep; neither resolved them, and both disclosed their full write surface. The resolutions
satisfied the required_review_thread_resolution merge gate.

Attribution is structurally impossible today

resolvedBy reports kyle-sexton on all nine threads across both PRs — and __typename: User,
not Bot, which matters (below). That field cannot discriminate: every agent, both sessions and
the human act under that one account.

resolvedAt does not exist. Settled by GraphQL introspection rather than by failing to find it:
PullRequestReviewThread exposes exactly 19 fields — comments, diffSide, id, isCollapsed, isOutdated, isResolved, line, originalLine, originalStartLine, path, pullRequest, repository, resolvedBy, startDiffSide, startLine, subjectType, viewerCanReply, viewerCanResolve, viewerCanUnresolve. There is no timestamp. So a resolution cannot even be placed on a timeline
relative to a process's lifetime.

What is excluded, with evidence

1. The CI / GitHub App route — excluded positively. .github/workflows/claude-review.yml and
   claude-security-review.yml are thin callers into melodic-software/ci-workflows, both granting
   pull-requests: write and id-token: write to mint a Claude GitHub App token. An App-token
   resolution attributes as a Bot. All nine threads are __typename: User. (Correction: the
   original filing excluded this by grepping the local .github/ for resolveReviewThread, which
   was insufficient — the behaviour lives in another repository. The __typename reading is the
   evidence that actually carries it.)
2. A second write-capable principal. repos/.../collaborators returns exactly one — kyle-sexton,
   admin.
3. Recorded local Claude sessions. No transcript contains a resolve mutation for any of fix(guardrails): replay a wrapper's chdir in the lease hash probe and rescope skill-reference-verify reconstruction #2100's
   four thread IDs. The search capability was verified positively: a genuine issued-mutation line
   was taken from the corpus, a real fix(guardrails): replay a wrapper's chdir in the lease hash probe and rescope skill-reference-verify reconstruction #2100 thread ID substituted in, and the searches re-run against
   it — all matched. So the absence is meaningful. (Correction: the original filing said "across
   ~/.claude/projects/…/*.jsonl". That glob is not recursive and subagent transcripts live in
   <session-id>/subagents/, so it reached 463 of 7,436 files — about 6%. A recursive sweep finds
   the IDs in 4 transcripts, 3 of them subagents of the investigating session created after filing.
   The conclusion survives; the stated reach did not.)
4. Subagent activity, separately: none of the four IDs appears in any agent transcript. One gap —
   a duplicate agent killed mid-run left a 0-byte transcript.
5. The audit log. GET /orgs/melodic-software/audit-log → 404 with read:audit_log present;
   the endpoint requires Enterprise Cloud.

The hypothesis nothing here can touch

A PAT belonging to kyle-sexton, used by any tool on any machine, produces exactly the observed
User/kyle-sexton attribution and is not enumerable through the API. Nor is a session on another
device, a web session, or the GitHub UI — the transcript argument is local-disk only. This is not a
gap in the search; it is the limit of what any post-hoc search could establish.

The systemic finding, which is larger than these two PRs — and worse than first filed

plugins/source-control/bin/source-control-babysit-resolve-thread guards resolution: owner
allowlist, bot-only bright line, TOCTOU pins, severity bright line, no self-resolution when
autonomous. The raw mutation bypasses all of it.

Occurrences classified by JSONL block structure — a tool_use block is an issued call;
tool_result, prose and Write/Edit inputs are mentions:

files matching `mutation{resolveReviewThread`      : 54
  shell tool_use (ISSUED)        occ=205  files=49
  tool_result (echoed output)    occ=14   files=5
  prose                          occ=10   files=5
  other tool_use (Write/Edit/…)  occ=9    files=…

files with >=1 ISSUED shell mutation : 49  (91%)
top single transcript                : 17

So the headline is not contaminated. It is an undercount — that was one project directory's top
level. Corpus-wide there are 249 project directories and 7,436 transcripts; a recursive classified
sweep finds 310 transcripts with issued mutations, ~2,230 occurrences, top transcript ≈100.
(The corpus-wide figures use a heuristic that is known to over-count slightly — it caught a
git commit -F - heredoc quoting the mutation. The 54-scope numbers above are structural and are
the ones to rely on.)

Some of those same transcripts carry prose warning against exactly this. A guard whose unguarded
one-line alternative is already the established habit is not a guard.

Remedies, reordered — the original ordering was self-defeating

1. Enable a pull_request_review_thread webhook at the org or repo level. GitHub documents this
   event with resolved / unresolved actions and an updated_at. It records at GitHub's side, so
   it captures every resolution regardless of route — wrapper, raw mutation, web UI, or app —
   and cannot be bypassed by reaching past the wrapper. It requires no code.

   It is not configured, at either level:

   gh api repos/melodic-software/claude-code-plugins/hooks -> []
   gh api orgs/melodic-software/hooks                      -> []
   

   So the surface that would have recorded these resolutions was never expired — it was never
   enabled. The existing token already carries admin:repo_hook and admin:org_hook.

   Limit, stated honestly: sender is not in that event's documented parameter table, so this
   guarantees action + timestamp — which GraphQL conclusively cannot give — but the actor is
   unconfirmed until a real delivery is inspected.

2. Close the bypass, or stop claiming it is closed. Either deny the raw mutation at the
   permission layer so the wrapper is the only route, or drop the claim that the wrapper's guards
   constrain resolution here.

3. Then a wrapper-side append-only record — pins validated, disposition, severity decision —
   for the guard-level detail a webhook cannot see.

(Correction: the original filing put the wrapper log first. That is self-defeating and this issue's
own evidence proves it — a wrapper log records only calls that went through the wrapper, which is
precisely the set that was never the problem. It becomes worth having only after (2).)

Interim, already in use

Any thread that must stay open has its hold written into the comment text, because an unresolved
state alone has twice proven not to hold. That patches the symptom.

Related

• fix(guardrails): replay a wrapper's chdir in the lease hash probe and rescope skill-reference-verify reconstruction #2100, fix(session-flow): distinguish a completed background continuation from a failed one #2115 — the two PRs
• Triage the remaining stranded P2 review findings from the post-merge sweep #1938 — the stranded post-merge review-findings sweep

[kyle-sexton]

A second surface for the same root cause — commits, not just thread resolutions

This issue frames the problem around review-thread resolution, but the root cause is broader and there is now a concrete second instance worth recording.

What happened. Two agents worked the same branch and the same worktree concurrently on PR #2147. One committed d40420de, posted two thread replies, resolved both, and merged. The other — the agent that had authored everything before that point — found HEAD moved to a commit it had not written, with a clean git status, and reported:

I had a git status-clean tree and a moved HEAD, which is indistinguishable from my own work until you look.

It resolved cleanly, but only because the intruding change happened to be additive and correct. That is luck, not a property of the arrangement. The failure mode it names is real: one agent committing on top of another's half-finished edit, with neither noticing.

Why it belongs here. It is the same defect as the one this issue documents, on a different surface. A single shared identity — kyle-sexton for every agent, both sessions, and the human — makes actors indistinguishable after the fact. There it was resolutions clearing a merge gate with no attributable actor. Here it is commits in a shared worktree. In both cases the artifact records what happened and cannot record who.

That matters for scoping the remedies. The pull_request_review_thread webhook (remedy 1) fixes attribution for resolutions and does nothing for this. The exposure is not limited to a GitHub-side setting, so a complete answer needs the local half too: the append-only wrapper record proposed as remedy 3 should carry an agent or session identifier, not merely a timestamp, or it reproduces the same blind spot one layer down.

Worth noting how it was caught, because the mechanism generalises. The second agent did not accept a same-identity commit as its own. It diffed ab6b3da8..d40420de, stripped comment lines, confirmed the remainder was empty, and re-checked both replies against its own measurements. Had the intruding commit been subtly wrong, that check is what would have caught it — and nothing else in the system would have.

Suggested addition to the remedies: a lock, or an explicit hand-off, before any parallel run shares a worktree. Cheap, and it removes the ambiguity at the source rather than reconstructing it afterwards.

[kyle-sexton]

Audit trail shipped in #2431: babysit_resolve_thread.py now appends every successful guarded resolve to resolve-thread-audit.jsonl with PR, thread, disposition, and metadata.

The remaining gap — operators bypassing the wrapper — is operational discipline, not a missing code path. The wrapper bypass concern is noted in the issue body comment from the #2147 concurrent-worktree incident; a worktree lock/hand-off remedy is tracked separately (#1774 / #2257). Closing the audit-trail portion as done.