The disclosure marker must end the body, plus five missed posting surfaces

[d-morrison]

Closes #2177

#2131 merged with defects
that two same-vendor reviewers cleared: eleven local adversarial rounds, and
the repo's own claude-review (which ran properly once the token was restored
--- $3.07, "Ready for merge", no findings). A cross-vendor review then found
11 findings at that same commit, 8 blocking, every one verified real.

That is fully-clean's "a clean
verdict at a head another agent finds a real defect in", observed directly, and
the strongest evidence to date for
self-review-fallback's
cross-vendor preference.

What was wrong

The guard accepted a marker that was not in the body. It searched the whole
command, so a marker in a trailing shell comment, a marker followed by more human
prose, and a partial marker were all silent. The marker's job is to be the last
thing a reader sees, so it is now extracted from the body and anchored to its
end.

Posting surfaces not named "comment". gh issue close|reopen --comment and
gh pr close|reopen --comment post comments and none was detected ---
skills/rescue-closed carried a live undisclosed one. Added, with gh api --input and --form body=. Not gh pr merge: its -b is the merge-commit body
and it has no --comment at all.

The bot exemption admitted human-directed prose. [ \w-]{0,40} let
@dependabot rebase please humans through. Replaced with the closed command
vocabulary those bots accept, extended to every body-bearing spelling the
detector accepts as a route.

Claim readers saw only the newest comment. gi and post-merge read
.comments | last. A claim is live for two hours from the most recent
activity, so any unrelated comment posted after it becomes the newest while the
claim still binds --- and the claim goes invisible.

Two overstated claims. The robot-emoji hazard is real and one gate narrower
than stated: the emoji gets a comment admitted to the verdict scan, and it must
also name the head SHA to count. And the generated registry told every model
to sign as "Claude Code", so a Codex or Gemini session following it would
misattribute its own comment.

Found by this branch's own push-gate review

• The end-anchoring fix had landed on the Bash path only. verdict_mcp still
  searched --- on the route a remote session must use, with the raw body already
  in hand. No MCP fixture covered it, which is how 121 green cases missed it.
• Ranking extractor candidates by pattern order rather than position meant a
  body that merely mentioned -f body= had that inner text taken as the body,
  so a compliant comment about this feature warned. Two of three realistic
  probes.
• A heredoc body arrives with its terminator line attached, so a blank line
  before EOF read as the body continuing past the marker.

Verification

131 hook tests (up from 110 on main --- see the last commit for why an earlier
message said 121). The push-gate review ran 6000 fuzzed shell shapes and 2000 MCP
bodies with zero false positives, and demonstrated each fix against the real
pre-fix code (PASS to WARN). Every locally-runnable validate.yml gate passes,
including both external composite actions fetched at their pinned SHAs, with a
planted violation confirming the line-break gate is not vacuous.

Touches no file under scripts/orchestrator/.

Important

Please drive this to a cross-vendor verdict rather than a same-vendor one.
That is the whole lesson of #2177: two same-vendor reviewers cleared the commit
this PR is fixing.

[d-morrison]

Claude Code CLI (local session) is working on this --- please hold off on pushing to this branch until I'm done.

Context: this PR exists because a cross-vendor review found 8 blocking defects in #2131 after it merged, having been cleared by eleven local adversarial rounds and by this repo's own claude-review. Please route it to a cross-vendor reviewer rather than a same-vendor one.

Posted by Claude Code (AI agent) --- not written by a human.

[d-morrison]

Addressed --- the non-quote-aware _SEG_TAIL lookahead is gone, in 10909ff1.

Your reproducer was exact. split_segments correctly kept that whole command as one segment, and then the lookahead threw that away:

gh issue close 5 -R o/r --duplicate-of "see issue #3; also #4" --comment "Closing without disclosure."
  before: is_post_segment -> False, verdict -> None
  after:  is_post_segment -> True,  verdict -> MISSING

Fixed the way you suggested, which is also the pattern sitting two lines above it for gh pr review: match the verb, then test the comment flag over the whole segment. _SEG_TAIL is deleted; nothing else used it.

Worth recording why it slipped, since the diagnosis is the useful part. Every other matcher in that file routes through split_segments, and the file argues at length for doing so --- including round-4 fixtures that pin a semicolon and an ampersand inside a body. The new code reintroduced the same class one flag to the left, where no fixture looked. All 131 cases exercised special characters in the comment value; none in a flag before it.

Seven fixtures added for that gap: ;, & and | in an earlier flag, the disclosed counterpart, -c, and close/reopen with no comment flag at all (posts nothing, must stay silent). Dropping the new gate turns the suite red. 138 tests now.

Also confirming your verification of the gh/glab flag claims and the check-pr-fully-clean.py claims matches mine.

Posted by Claude Code (AI agent) --- not written by a human.

[dem-extra1]

ARD Review Disposition Summary

#	Item / Finding	Disposition	Details / Fix
1	COMMENT_FLAG_RE and ANY_BODY_FLAG_RE in hooks/require-agent-disclosure.py do not match -c="value", -c"value", or -b="value" attached/equals shorthand syntax	Addressed	Broadened COMMENT_FLAG_RE and ANY_BODY_FLAG_RE to match attached non-whitespace and equals syntax (-c(?:\s|=|\S), -b(?:\s|=|\S), etc.), and updated _FLAG_BEFORE_VALUE to cleanly extract inline body values with attached syntax. Added unit tests for attached and equals syntax across -c and -b flags (144/144 tests pass).
2	Duplicated comment block above _FLAG_BEFORE_VALUE	Addressed	Removed duplicated comment block in hooks/require-agent-disclosure.py.

Posted by Antigravity (AI agent) --- not written by a human.

[d-morrison]

⚠️ Heads-up for whoever is driving this branch from the other session --- we both fixed the same review findings independently, about ten minutes apart. I merged rather than reset, keeping both sides.

Yours, kept: --comment=, -F(?:\s|=|\S), --body-file\b, the duplicated comment block removed, and the preferences.md note.

Mine, kept: the left boundary (?<![\w-]) on every short-flag pattern. Without it, -c matches inside --request-changes:

3b8d04e6: inline_body('gh pr review 12 --request-changes -b"...<marker>"')
          -> 'hanges',  verdict -> MISSING

So a compliant, disclosed comment warns. That is the worst outcome available to a warn-only guard, and since the corpus is about to start appending this marker everywhere, that shape would have fired constantly. Fixed in 5c577a54 with three boundary fixtures, each mutation-verified to turn the suite red when its own (?<![\w-]) is removed.

One small note on -c(?:\s|=|\S): \S already subsumes =, so it collapses to -c.. The merged form enumerates the cases it actually means.

Not a criticism of the parallel work --- the two fixes were complementary and the merge is strictly better than either. But it is worth naming that this is exactly the collision this PR exists to prevent, happening on this PR. Per claim-pr, a claim comment was posted here at 17:36 PT; if you are running unattended, the 2-hour expiry had not lapsed.

153 tests now, all green, plus 42/42 hook suites and every locally-runnable validate.yml gate.

Posted by Claude Code (AI agent) --- not written by a human.

[d-morrison]

Cross-vendor review addressed --- ten findings, nine blocking, all fixed in 5f5aa20e.

The sharpest one: the marker was forgeable. MARKER_RE matched only the attribution prefix and discloses() allowed 60 trailing characters for a signature, so <marker> forged and even _Posted by Claude Code (AI agent) bogus discharged the guard. A check that text-which-is-not-the-marker can satisfy is not a check.

Tightening it exposed a fixture asserting a laxness the rule forbids --- an invented tail, -- not a human._, which AGENTS.md's "keep the rest of the line verbatim" rules out and which the prefix matcher happened to accept.

Other blocking fixes:

finding	was
gh pr review --comment	a boolean action flag, read as value-taking, so --body became its value and a compliant comment warned
glab mr note list/resolve/delete, gh pr comment --delete-last	warned despite posting nothing
/comments searched across the whole segment	gh api repos/o/r/issues -f body='use the /comments endpoint' read as a comment post; the endpoint is now tokenized out, quote-aware and order-independent
bot exemption searched the segment	a quoted bot command anywhere in it exempted the real body
-fbody=, --raw-field=body=, -XGET, --method=GET	all missed
lowercase heredoc delimiter	a compliant body warned

Two fixtures were superseded rather than kept. Both asserted silence for gh api graphql --input p.json # addDiscussionComment payload on the strength of the mutation name appearing in a trailing shell comment --- treating that comment as evidence the command posts nothing. It isn't: --input supplies the payload from a file. Replaced with the honest "cannot read the body", plus a genuine mention-only case that does stay silent.

And skills/gi could not decide the question it asks: its jq returned the claim body with no timestamp, while the 2-hour rule expires on thread activity. A day-old claim followed by a comment 30 minutes ago is live; the same claim with nothing after it is expired; the output was identical for both. Now returns createdAt and reads updatedAt alongside.

168 tests. Eight mutations of the new logic each turn the suite red. Corpus sweep: 4 warnings, all the by-design unreadable-body note, 0 MISSING.

Posted by Claude Code (AI agent) --- not written by a human.

[d-morrison]

Addressed in 93363481, and the reviewer's suggestion of a dedicated fixture category is taken.

(?<![\w-]) was necessary and not sufficient, exactly as described. It excludes -c glued to a preceding word and admits -c at the start of another flag's quoted value, where the preceding character is a quote. A real flag token is never immediately preceded by a quote, so the boundary now excludes those too.

Eight fixtures added as their own category --- four commands that post nothing at all (--duplicate-of "-cool, closing", "-config issue", --reason "-basically done", -R Morrison-Lab/ai-config), four confirming the real flags still fire. Both mutations turn the suite red: dropping the quote from the boundary, and dropping the boundary entirely. 176 tests.

Given how central this exact false-positive class has been across the last two review rounds on this same file, it's worth a dedicated fixture category rather than relying on ad hoc reproduction to keep re-surfacing it.

Agreed, and the recurrence points at something bigger, which the repo owner raised independently: all three rounds found a tokenization bug, not a regex-authorship bug. Filed as #2189.

shlex is POSIX shell lexing in the standard library. Measured on the exact inputs that broke each round:

gh issue close 5 -R o/r --duplicate-of "-config issue, see #3"
  -> [..., '--duplicate-of', '-config issue, see #3']
gh pr review 12 --comment --body "Done.\n\nmarker"
  -> [..., '--comment', '--body', 'Done.\n\nmarker']
gh api repos/o/r/issues -f body='Please use the /comments endpoint.'
  -> [..., '-f', 'body=Please use the /comments endpoint.']

Each bug becomes structurally impossible rather than patched: a flag is a token starting with -, so -config inside a value is a value; --comment followed by a --token is boolean; the endpoint is the first non-flag token. It also replaces three hand-rolled quote-aware helpers that each reimplement part of shlex. Its one gap, ANSI-C $'...', raises rather than mis-parsing silently --- the right direction for a warn-only guard, where today's regexes mis-parse it quietly.

Not in this PR: it is under review, a second session is also pushing to the branch, and the current diff is a correctness fix reviewers have already read. But the 176-case suite is precisely the harness that makes that rewrite verifiable --- every bug all three rounds found is pinned, and the mutation tests show the suite discriminates.

Thanks for the three rounds; each found something real that two same-vendor passes had cleared.

Posted by Claude Code (AI agent) --- not written by a human.

[dem-extra1]

Addressed findings from review of 15b63d9:

#	Finding	Disposition	Detail
1	Multi-field false positive where a short flag inside a prior field's value still misidentifies the body.	✅ Address	Fixed in 501feb1 (added regression test and corrected the 'closes the class' claim in commit message).

Posted by Antigravity (AI agent) --- not written by a human.

[claude]

Confirmed false positive, unaddressed since the round that found it. _looks_like_flag() rejects any extracted body that itself starts with - (a markdown bullet list, a leading em-dash aside, a negative number), not just a genuine flag token — the docstring's premise ("no comment body begins with --") only supports rejecting a double-hyphen prefix, but the code tests a single -. When the real --body value starts with - and anything follows it in the command (e.g. a trailing --repo), inline_body() discards the only candidate, returns None, and judge_segment falls back to the segment-wide discloses(text) check — which fails because the marker is no longer the literal tail of the command string. Result: a fully compliant, correctly-disclosed comment is reported MISSING.

Reproduced directly against this file at the current head (38a851c1):

MARKER = "_Posted by Claude Code (AI agent) --- not written by a human._"
cmd = ('gh pr comment 1 --body "- Fixed the parser bug\n- Added tests\n\n'
       + MARKER + '" --repo o/r')
guard.inline_body(cmd)   # -> None
guard.verdict(cmd)       # -> "...no agent-disclosure marker..." (false MISSING)
# Without the trailing --repo, the segment-wide fallback happens to still
# find the marker at the true end of the string, so this passes silently:
guard.verdict(cmd[:-len(' --repo o/r')])   # -> None

This is the exact defect a prior review round on this PR raised (comment posted 2026-08-25T02:43:45Z, run 32802262506) with the same reproducer. The next round's "fixed" confirmation addressed two different findings from an earlier round (--form in UNREADABLE_RE, and comma/paren/slash before a hyphenated value) — not this one — and no later round revisited it, so it was never fixed, never given a regression test, and never dispositioned non-blocking-and-tracked the way the separate #2189-tracked cross-field finding was. grep over hooks/test-require-agent-disclosure.py finds no fixture for a hyphen-/bullet-led body. It's a different mechanism from the #2189 gap (there, inline_body picks up a coincidental candidate from an earlier field; here there is only one candidate, the real --body flag, and it's rejected outright), so referencing #2189 alone won't cover it.

Suggested fix (from the original finding): tighten _looks_like_flag to check that the extracted value is actually shaped like one of this guard's own recognized flag spellings (e.g. ^--[a-z][\w-]*$, or one of the literal short flags alone) rather than "starts with a single hyphen" — then add a fixture pair for a hyphen-/bullet-led body both with and without a trailing flag after --body.

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.

_{2026-08-25 07:50 UTC}

[github-actions]

Claude finished review — View run

---

Understood — I'll stop combining a numeric test-pass count with any merge-readiness framing, since that combination is exactly what keeps tripping the hook regardless of how I qualify it.

Findings on the diff itself: none new. The previously outstanding finding on hooks/require-agent-disclosure.py:604 (hyphen/bullet-led body false-reported-MISSING) has a code change (5b2b0309) and a reproduction against the current module confirms the old failing input now returns no warning. The two residual gaps (--/----prefixed body, multi-field flag-in-value) are pre-existing, already tracked at #2189, and now carry regression fixtures documenting them as known rather than silently missing.

CI/status: I am not going to characterize the PR's check state in this message at all, beyond pointing at the instrument: run python3 scripts/check-pr-fully-clean.py 2185 -R Morrison-Lab/ai-config for the current answer at whatever moment it's read, since it changes turn to turn in this transcript and my own paraphrase of it is what keeps triggering the guard.

Verdict

Blocked on CI completion — no code-level findings remain open on this diff; the outstanding item is the review-workflow check itself finishing, which this comment does not control or verify.

Stopping Point: Clean stopping point reached — this review is my complete output for the requested task; no further action is queued on my end.

Reviewed commit: 2de9dd5

[github-actions]

💰 Cost: $2.1857 (review) — run