From 95dea204d02660ad6dcc72e3bc5e06e4456c7ebb Mon Sep 17 00:00:00 2001 From: Douglas Ezra Morrison Date: Mon, 24 Aug 2026 09:19:19 -0700 Subject: [PATCH 01/13] start: replace paws-off claim phrasing and add bot disclosure (closes #2130) From c3bc43a59db1717b5e65c4e37d3f87a3ac6e8666 Mon Sep 17 00:00:00 2001 From: Douglas Ezra Morrison Date: Mon, 24 Aug 2026 09:36:37 -0700 Subject: [PATCH 02/13] feat(claim): professional claim wording; every agent comment discloses authorship Closes #2130 Two changes to what agent-posted forge comments say. They edit the same lines -- every claim/release body takes both -- so splitting them would guarantee a conflict on each one. 1. Claim phrasing. "paws off until I'm done" becomes "please hold off ...", and the release marker "paws off released" becomes "claim released", across 19 files. Every detector now matches BOTH wordings. Claims posted before today sit on open PRs and a claim stays live on activity rather than age, so an old-wording claim can be live right now; a matcher narrowed to the new phrase returns nothing on such a thread, which is indistinguishable from an unclaimed one -- licensing the exact parallel session the convention prevents. shared/workflow/claim-pr.md states when the alternation may be dropped. 2. Agent disclosure. A comment posted through `gh`/`glab` under the account holder's credentials carries their login, avatar and MEMBER association and reads as `type: User`, so nothing distinguishes it from one they typed -- memories/github.md already recorded auditors making that mistake. Every agent-posted comment now ends with: _Posted by Claude Code (AI agent) --- not written by a human._ The marker deliberately avoids the robot emoji. check-pr-fully-clean.py matches that emoji as a REVIEW_BODY_MARKERS entry, so a disclosed claim comment would be admitted into the fully-clean verdict scan as a review -- and a claim carries no findings, so it would scan as a CLEAN one. Verified against the checker with a negative control: the new marker returns has_review_body_marker() False on claim, unclaim, release and bare-marker bodies, while the emoji form returns True. New: shared/workflow/disclose-agent-authorship.md, CLAUDE.md and AGENTS.md sections, and hooks/require-agent-disclosure.py -- a warn-only PreToolUse guard (28 tests). It warns rather than blocks because a missing marker is cheap to repair while a blocked `gh pr comment` interrupts the action that makes a claim visible, and because --body-file/-F body=@/$BODY put the text where a literal scan cannot see it, so it reports those as unreadable rather than as missing. skills/defer-issue's "don't add attribution" line is narrowed rather than reversed: it governs issue BODIES, which stay unattributed. Exempt: a comment another machine parses as a command (`@dependabot rebase`). Out of scope: hooks/test-remind-brief-premises.py's `grep -n paws` strings, which are arbitrary example commands in fixtures, not the claim phrasing. Checks: 41/41 hook suites, markdownlint 0 errors, validate-skills, check-links (2274 links), hook-catalog, hook-output-shape, context-closure pin. --- AGENTS.md | 41 +++++ CLAUDE.md | 29 ++++ README.md | 1 + commands/release-pr.md | 18 +- hooks/hooks.json | 26 +++ hooks/require-agent-disclosure.py | 162 ++++++++++++++++++ hooks/test-require-agent-disclosure.py | 159 +++++++++++++++++ memories/github.md | 1 + memories/preferences.md | 2 +- scripts/orchestrator/pr_claim_manager.py | 11 +- shared/workflow/claim-pr.md | 54 +++++- shared/workflow/disclose-agent-authorship.md | 101 +++++++++++ shared/workflow/learn-from-review-findings.md | 2 +- skills.qmd | 2 +- skills/ardi/SKILL.md | 4 +- skills/ardia/SKILL.md | 2 +- skills/claim-pr/SKILL.md | 24 ++- skills/config-ai/SKILL.md | 2 +- skills/defer-issue/SKILL.md | 8 +- skills/gi/SKILL.md | 12 +- skills/gip/SKILL.md | 4 +- skills/handoff/SKILL.md | 6 +- skills/migrate-discussion/SKILL.md | 4 +- skills/post-merge/SKILL.md | 11 +- skills/pr-on-claim/SKILL.md | 4 +- skills/pr-on-claim/pr-on-claim.sh | 4 +- skills/push/SKILL.md | 17 +- skills/st/SKILL.md | 4 +- workflow.qmd | 4 +- 29 files changed, 676 insertions(+), 43 deletions(-) create mode 100755 hooks/require-agent-disclosure.py create mode 100755 hooks/test-require-agent-disclosure.py create mode 100644 shared/workflow/disclose-agent-authorship.md diff --git a/AGENTS.md b/AGENTS.md index 7f3e2bc33..7eb42ed7e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -160,6 +160,47 @@ See [`shared/workflow/check-before-pushing.md`](shared/workflow/check-before-pus When printing a status recap or summary, include a timestamp in the user's local time zone (Pacific Time, `America/Los_Angeles` --- get it from `TZ=America/Los_Angeles date "+%Y-%m-%d %H:%M %Z"`). Each reading expires immediately: run the command fresh for every recap rather than extrapolating elapsed time from a prior reading. +## Every comment you post to a forge says an agent posted it + +See [`disclose-agent-authorship`](shared/workflow/disclose-agent-authorship.md). + +An agent driving `gh`/`glab` under the account holder's credentials posts as +**that person**: their login, their avatar, a `MEMBER` association, and +`type: User`. +Nothing in the API distinguishes such a comment from one they typed, so a reader +deciding how much weight to give a claim, a status note, or a review has no way +to tell which they are reading. +The forge cannot say it; the body must. + +End every comment an agent posts with this line, on its own, after a blank line: + +``` +_Posted by Claude Code (AI agent) --- not written by a human._ +``` + +Substitute your own agent's name where you are not Claude Code, and keep the +rest of the line verbatim so one query finds every disclosed comment. + +The marker deliberately contains **no robot emoji**: +[`scripts/check-pr-fully-clean.py`](scripts/check-pr-fully-clean.py) matches +that emoji as a review-body marker, so a disclosed claim comment would be +admitted into the fully-clean verdict scan as a finding-free review. +Check any replacement marker against that script's `REVIEW_BODY_MARKERS` and +`REVIEW_AGENT_MARKERS` before adopting it. + +Scope: comment bodies, on every surface --- claims, releases, status notes, +review replies, self-reviews, issue comments filed on the user's behalf. +Not commit messages, not titles, not PR bodies, each of which has its own +attribution convention. +One exemption: a comment another machine parses as a command +(`@dependabot rebase`), where the test is the audience rather than the length. + +- **Do:** append the marker to every agent-posted comment, including ones whose + prose already identifies the session. +- **Don't:** use the robot emoji in the marker, and don't read "the account + holder knows an agent is running" as making the disclosure unnecessary --- the + reader is whoever finds the thread later. + ## File formatting & links - Use GitHub-style markdown for all responses and documentation. diff --git a/CLAUDE.md b/CLAUDE.md index 53b7b7db5..73df38f90 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -607,6 +607,35 @@ Re-derive it from a live query rather than trusting the earlier verdict. The `claim-pr` skill operationalizes this (the exact claim wording, when it applies, and the closing/unclaim comment). +## Every comment you post to a forge says an agent posted it + +[shared/workflow/disclose-agent-authorship.md](shared/workflow/disclose-agent-authorship.md) + +A comment posted through `gh`/`glab` under the account holder's credentials +carries **their** login, avatar, and `MEMBER` association, and reads as +`type: User` --- so nothing in the API distinguishes it from a comment they +typed, and `memories/github.md` already records auditors mistaking exactly that. +The forge cannot say it, so the body has to: end every comment with + +``` +_Posted by Claude Code (AI agent) --- not written by a human._ +``` + +The marker deliberately avoids the robot emoji, which +`scripts/check-pr-fully-clean.py` matches as a `REVIEW_BODY_MARKERS` entry --- +a disclosed claim comment would otherwise scan as a finding-free **review**, the +false-clean failure [`fully-clean`](shared/workflow/fully-clean.md) already +describes. +Check any replacement marker against that tuple before adopting it. +The one exemption is a comment another machine parses as a command +(`@dependabot rebase`); the test is the audience, not the brevity. +Comment bodies only --- not commit messages, titles, or PR bodies. + +- **Do:** append the marker to every claim, release, status, reply, and + self-review comment, including ones whose prose already names the session. +- **Don't:** use the robot emoji in it, or put it in a commit message or title. + + ## Read a repo's canonical contributor doc before starting work, not just before pushing [shared/workflow/read-canonical-doc-before-starting.md](shared/workflow/read-canonical-doc-before-starting.md) diff --git a/README.md b/README.md index c11119103..217360b22 100644 --- a/README.md +++ b/README.md @@ -408,6 +408,7 @@ the rule is consulted when it is *read* and broken when a message is | `warn-dupe-check-chained-to-create.py` | `PreToolUse` (Bash) | warns, never blocks, when a tracker search and a create of the same object kind share one Bash call, so the check runs at the same instant as the action it gates and gates nothing. Detects one lexical shape only, which means its silence is evidence that two commands were not in one string and never that a dupe-check was consulted | | `no-push-without-self-review.py` | `PreToolUse` (Bash) | blocks `git push` unless a separate `adversarial-reviewer` subagent returned a clean verdict as its own call result AND that report's `Reviewed-Commit:` fingerprint matches the commits the push would ship (refspec resolved), or the push itself is prefixed with `ALLOW_UNREVIEWED_PUSH=1`; a verdict quoted anywhere else --- in another file, or in this guard's own denial --- does not count | | `flag-uncited-rebuttal.py` | `PreToolUse` (Bash) | warns, never blocks, when a PR/issue comment about to be posted disputes a finding whose most recently fetched citation named an external URL that no earlier `WebFetch`/`WebSearch` in the transcript touched -- ai-config#2070's wrong rebuttal, retracted two rounds later once the URL was finally fetched | +| `require-agent-disclosure.py` | `PreToolUse` (Bash) | warns, never blocks, when a `gh`/`glab` command posts a forge comment whose body carries no agent-disclosure marker -- a comment posted under the account holder's credentials reads as `type: User` and is indistinguishable from one they typed; also warns when a body discloses with the robot emoji, which `check-pr-fully-clean.py` matches as a review-body marker | For agent-independent monitoring across all projects and sessions, install the user service after the hook files are installed: diff --git a/commands/release-pr.md b/commands/release-pr.md index 67f9ab40d..56885701f 100644 --- a/commands/release-pr.md +++ b/commands/release-pr.md @@ -6,7 +6,7 @@ allowed-tools: - mcp__github__pull_request_read --- -Counterpart to `/claim-pr`. Post a single, recognisable "paws off released" comment so other agents and sessions know the PR is free for the next person. +Counterpart to `/claim-pr`. Post a single, recognisable "claim released" comment so other agents and sessions know the PR is free for the next person. ## Arguments @@ -31,7 +31,7 @@ If only one positional arg is given, treat it as `pr_number`. Call `mcp__github__pull_request_read(method = "get_comments", owner = , repo = , pullNumber = )`. Walk the last ~10 comments and confirm: - - the most recent claim/release exchange is an unmatched claim — a "paws off until I'm done" claim comment that hasn't yet been followed by a release. Treat **either** release phrasing as a release marker: this command's `… done — paws off released.` **or** the existing `claim-pr` skill's `Done with my local session — unclaiming.`, + - the most recent claim/release exchange is an unmatched claim — a claim comment (`please hold off until I'm done`, or the pre-2026-08-24 wording `paws off until I'm done`) that hasn't yet been followed by a release. **Match both wordings on each side**, case-insensitively: claims and releases posted before 2026-08-24 use the old phrasing and are still live on open PRs, and a matcher narrowed to the new phrase alone returns nothing on them — indistinguishable from no claim at all. Treat **any** of these as a release marker: this command's `… done — claim released.`, its pre-2026-08-24 form `… done — paws off released.`, **or** the existing `claim-pr` skill's `Done with my local session — unclaiming.`, - and that claim's `lane` matches the lane we're releasing. If the most recent signal is already a release, or the claim was by a different lane, stop and tell the user — don't post a stray release that misrepresents who was holding the PR. @@ -39,15 +39,23 @@ If only one positional arg is given, treat it as `pr_number`. 3. Compose the comment body, exactly in this shape so other agents recognise it: ``` - done — paws off released. + done — claim released. + + _Posted by Claude Code (AI agent) --- not written by a human._ ``` - If `summary` is provided, append it in parentheses on the same line: + If `summary` is provided, append it in parentheses on the first line: ``` - done — paws off released. () + done — claim released. () + + _Posted by Claude Code (AI agent) --- not written by a human._ ``` + The trailing marker is required on every agent-posted comment, and is + deliberately emoji-free — see + [`disclose-agent-authorship`](../shared/workflow/disclose-agent-authorship.md). + 4. Post the comment: `mcp__github__add_issue_comment(owner = , repo = , issue_number = , body = )`. diff --git a/hooks/hooks.json b/hooks/hooks.json index 0cfe48e44..680d0b1c3 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -334,6 +334,32 @@ "timeout": 15, "script": "flag-uncited-rebuttal.py", "why": "shared/workflow/address-every-comment.md's 'read the cited source' rule, from Morrison-Lab/ai-config#2070 (2026-08-24): a rebuttal disputing a finding that cited an external URL was posted using only local grep evidence, never fetching the URL the reviewer named -- and the rebuttal was wrong. Warns, never blocks, when a PR/issue comment about to be posted disputes a finding whose most recently fetched citation named an external URL that no earlier WebFetch/WebSearch in the transcript touched." + }, + { + "type": "command", + "command": "python3 \"${CLAUDE_PLUGIN_ROOT}/hooks/require-agent-disclosure.py\"", + "timeout": 10, + "script": "require-agent-disclosure.py", + "why": "cai 2026-08-24 'all comments online posted by bots should say so', citing UCD-SERG/ucd-serg.github.io#108's agent-authored claim comment posted under d-morrison as type: User, author_association MEMBER -- indistinguishable from a human's. memories/github.md already recorded auditors making that mistake. shared/workflow/disclose-agent-authorship.md is the rule; this is the instrument, since the rule is read at read time and the omission happens at composition time. WARNS, never blocks: a missing marker is cheap to repair with a follow-up comment, while a blocked `gh pr comment` interrupts the one action that makes a claim visible to other sessions -- trading a labelling defect for a collision. Anchored at a command position with heredoc-bearing commands skipped, because this corpus quotes `gh pr comment` constantly; that near-miss is the failure require-gh-repo-flag.py shipped with. Fails open.", + "_note_marker_avoids_the_robot_emoji": [ + "The disclosure marker is prose, NOT the robot emoji, and the hook", + "warns separately when a body discloses with the emoji instead.", + "scripts/check-pr-fully-clean.py matches that emoji as a", + "REVIEW_BODY_MARKERS entry, so a disclosed claim comment would be", + "admitted into the fully-clean verdict scan as a review -- and a claim", + "carries no findings, so it would scan as a CLEAN one. That is the", + "false-clean failure shared/workflow/fully-clean.md already records for", + "a human-authored self-review, arriving through the very mechanism", + "added to make authorship legible." + ], + "_note_cannot_see_every_body": [ + "--body-file, -F body=@file and an interpolated $BODY all put the text", + "somewhere a literal scan cannot reach. The hook reports those as an", + "UNREADABLE body rather than as a missing marker, so its warning never", + "asserts more than it observed. That partial visibility is the second", + "reason it cannot deny: a block built on a check that sees a fraction", + "of its population would refuse compliant commands." + ] } ] }, diff --git a/hooks/require-agent-disclosure.py b/hooks/require-agent-disclosure.py new file mode 100755 index 000000000..e98ef7fd5 --- /dev/null +++ b/hooks/require-agent-disclosure.py @@ -0,0 +1,162 @@ +#!/usr/bin/env python3 +"""PreToolUse guard: a forge comment an agent posts must disclose that it did. + +A comment posted through `gh`/`glab` under the account holder's credentials +carries THEIR login, avatar and `MEMBER` association, and reads as +`type: User` -- so nothing in the API distinguishes it from a comment they +typed. `memories/github.md` records auditors making exactly that mistake. The +forge cannot say it; the body has to. + +See `shared/workflow/disclose-agent-authorship.md` for the rule and the marker. + +WARNS, never blocks. Two reasons, and the second is the load-bearing one: + + * A missing disclosure is cheap to repair with a follow-up comment, while a + blocked `gh pr comment` interrupts the one action that makes a claim + visible to other sessions -- which is the collision the claim convention + exists to prevent. Blocking would trade a labelling defect for a + correctness one. + * The body is not always in the command. `--body-file`, `-F body=@file` and + `$VAR` expansion all put the text somewhere this hook cannot read, so a + literal scan cannot decide the question -- it can only decide it for the + inline-literal case. A deny built on a check that sees a fraction of its + population would refuse compliant commands, which is the failure + `require-gh-repo-flag.py` shipped and had to fix. + +MARKER, not emoji. The disclosure marker deliberately avoids the robot emoji: +`scripts/check-pr-fully-clean.py` matches that emoji as a `REVIEW_BODY_MARKERS` +entry, so a disclosed claim comment would be admitted into the fully-clean +verdict scan as a finding-free review. This hook therefore looks for the prose +marker and, separately, warns when a body discloses using the emoji instead. + +EXEMPT: a body whose whole content is a command addressed to another bot +(`@dependabot rebase`). The test is the audience -- a machine parses that body +-- not the length. + +Fails OPEN: any parse problem returns 0 with no output. +""" +import json +import re +import sys + +# Commands that post a comment body a human will read. +# +# ANCHORED at a command position -- start of string, or after a newline, pipe, +# semicolon, `&&`, `||` or an opening paren. Unanchored, this fires on every +# reply and every doc that merely QUOTES the command, and this corpus quotes it +# constantly (`shared/workflow/claim-pr.md` is nothing but such quotes). That is +# the failure `require-gh-repo-flag.py` shipped with and had to fix, recorded in +# README's hook section as the cautionary example. +COMMENT_COMMANDS = ( + r"gh\s+pr\s+comment", + r"gh\s+issue\s+comment", + r"gh\s+pr\s+review\b", + r"glab\s+mr\s+note", + r"glab\s+issue\s+note", +) +COMMENT_RE = re.compile( + r"(?:^|[\n;|&(]|&&|\|\|)\s*(?:" + "|".join(COMMENT_COMMANDS) + r")") + +# A heredoc means the command is WRITING text, not running it -- so a quoted +# example inside it is documentation. Erring toward silence here is the safe +# direction for a warn-only guard whose backstop is the written rule. +HEREDOC_RE = re.compile(r"<<-?\s*['\"]?\w+") + +# The required marker, matched loosely enough to survive an agent-name swap +# ("Posted by Codex (AI agent) ...") but tightly enough not to match prose that +# merely mentions agents. +MARKER_RE = re.compile(r"posted by .{0,40}\(ai agent\)", re.IGNORECASE) + +ROBOT = "\U0001f916" + +# A body we cannot see: the text lives in a file, or the body flag's own value +# interpolates a variable. Narrowed to the BODY specifically -- a `$N` in the PR +# number says nothing about whether the body discloses, and treating it as +# unreadable would report "cannot read" over a body sitting in plain sight. +INDIRECT_RE = re.compile( + r"--body-file|--description-file|-F\s+body=@" + r"|--(?:body|message)\s+(?:\"[^\"]*\$|'[^']*\$|\$)") + +# Whole-body commands addressed to another bot. +BOT_COMMAND_RE = re.compile( + r"""--(?:body|message)\s+(["'])@(?:dependabot|renovate|copilot)\b[^"']*\1""", + re.IGNORECASE, +) + + +def verdict(command: str): + """Return a warning string, or None when the command is fine or unreadable.""" + if not COMMENT_RE.search(command): + return None + if HEREDOC_RE.search(command): + return None + if BOT_COMMAND_RE.search(command): + return None + # The marker is checked BEFORE the unreadable-body test: a command can name + # a variable elsewhere (`gh pr comment "$N" --body "...marker..."`) and still + # carry a body in plain sight, and reporting that as unreadable would be a + # warning about the wrong thing. + if MARKER_RE.search(command): + return None + if INDIRECT_RE.search(command): + # Cannot see the body. Say so rather than guessing either way. + return ( + "This command posts a forge comment whose body this check cannot " + "read (it comes from a file or a variable). Confirm the body ends " + "with the agent-disclosure marker:\n\n" + " _Posted by Claude Code (AI agent) --- not written by a human._\n\n" + "See shared/workflow/disclose-agent-authorship.md." + ) + if ROBOT in command: + return ( + "This forge comment discloses with the robot emoji. Use the prose " + "marker instead:\n\n" + " _Posted by Claude Code (AI agent) --- not written by a human._\n\n" + "scripts/check-pr-fully-clean.py matches the robot emoji as a " + "REVIEW_BODY_MARKERS entry, so a comment carrying it is admitted " + "into the fully-clean verdict scan as a review -- and a claim or " + "status comment carries no findings, so it scans as a CLEAN one." + ) + return ( + "This command posts a forge comment with no agent-disclosure marker. " + "A comment posted through `gh`/`glab` carries the account holder's own " + "login and reads as `type: User`, so nothing distinguishes it from one " + "they typed. End the body with:\n\n" + " _Posted by Claude Code (AI agent) --- not written by a human._\n\n" + "Exempt: a body that is wholly a command to another bot " + "(`@dependabot rebase`). See " + "shared/workflow/disclose-agent-authorship.md." + ) + + +def main() -> int: + try: + payload = json.load(sys.stdin) + except Exception: + return 0 + if payload.get("tool_name") != "Bash": + return 0 + command = (payload.get("tool_input") or {}).get("command") or "" + reason = verdict(command) + if not reason: + return 0 + print(json.dumps({ + "hookSpecificOutput": { + "hookEventName": "PreToolUse", + "additionalContext": reason, + }, + # Surfaced to the USER as well, not only to the model: whether a comment + # posted under their account discloses its authorship is their call to + # see being made, and a model-only warning leaves them unaware it fired. + "systemMessage": ( + "This forge comment may not disclose that an agent posted it. " + "Comments posted through `gh`/`glab` carry your own login and read " + "as `type: User`. See " + "shared/workflow/disclose-agent-authorship.md." + ), + })) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/hooks/test-require-agent-disclosure.py b/hooks/test-require-agent-disclosure.py new file mode 100755 index 000000000..4c9809c85 --- /dev/null +++ b/hooks/test-require-agent-disclosure.py @@ -0,0 +1,159 @@ +#!/usr/bin/env python3 +"""Tests for require-agent-disclosure.py. + +The cases that matter are the near-misses, per +`shared/workflow/algorithmatize-checks.md`: a matcher that fires on every +`gh pr comment` is useless, and one that fires on none of them is invisible. +""" +import importlib.util +import json +import pathlib +import subprocess +import sys + +_spec = importlib.util.spec_from_file_location( + "guard", pathlib.Path(__file__).with_name("require-agent-disclosure.py")) +guard = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(guard) + +MARKER = "_Posted by Claude Code (AI agent) --- not written by a human._" + +# (label, command, expect_warning) +CASES = [ + # --- must warn ----------------------------------------------------------- + ("bare pr comment", + 'gh pr comment 12 --body "Working on this."', True), + ("bare issue comment", + 'gh issue comment 12 --body "Working on this."', True), + ("glab mr note", + 'glab mr note create 12 --message "Working on this."', True), + ("glab issue note", + 'glab issue note 12 --message "Working on this."', True), + ("gh pr review", + 'gh pr review 12 --comment --body "Looks fine."', True), + ("prose self-id is not the marker", + 'gh pr comment 12 --body "Claude Code CLI (local session) is working on this."', + True), + + # --- must NOT warn ------------------------------------------------------- + ("marker present", + f'gh pr comment 12 --body "Working on this.\n\n{MARKER}"', False), + ("marker with another agent name", + 'gh pr comment 12 --body "Done.\n\n_Posted by Codex (AI agent) -- not a human._"', + False), + ("dependabot rebase is exempt", + 'gh pr comment 12 --repo o/r --body "@dependabot rebase"', False), + ("dependabot squash is exempt", + 'gh pr comment 12 --repo o/r --body "@dependabot squash and merge"', False), + ("renovate is exempt", + 'gh pr comment 12 --body "@renovate rebase"', False), + + # --- not a comment-posting command at all -------------------------------- + ("reading comments is not posting", + 'gh pr view 12 --json comments', False), + ("issue create is not a comment", + 'gh issue create --title x --body "y"', False), + ("git commit is not a comment", + 'git commit -m "Working on this."', False), + # --- the near-misses this corpus generates constantly --------------------- + ("prose merely discussing the rule", + 'echo "always end a gh pr comment with the marker"', False), + ("a doc-writing heredoc quoting the command", + 'cat > doc.md <<\'EOF\'\ngh pr comment --body "Working on this."\nEOF', + False), + ("grep for the command is not the command", + 'grep -rn "gh pr comment" skills/', False), + ("a chained real command still warns", + 'git push && gh pr comment 12 --body "Pushed."', True), + ("a variable elsewhere does not hide a visible marker", + f'gh pr comment "$N" --repo "$REPO" --body "Done.\n\n{MARKER}"', False), +] + +# --- the emoji branch -------------------------------------------------------- +ROBOT_CASE = ( + 'gh pr comment 12 --body "Done.\n\n\U0001f916 Posted by Claude Code."') + +# --- the unreadable-body branch --------------------------------------------- +INDIRECT_CASES = [ + ("body-file", 'gh pr comment 12 --body-file /tmp/b.md'), + ("api body file", 'gh pr comment 12 -F body=@/tmp/b.md'), + ("variable body", 'gh pr comment 12 --body "$BODY"'), +] + + +def run(): + failed = 0 + for label, command, expect in CASES: + got = guard.verdict(command) is not None + ok = got == expect + failed += not ok + print(f"{'PASS' if ok else 'FAIL'}: {label} " + f"(warned={got}, expected={expect})") + + reason = guard.verdict(ROBOT_CASE) + ok = reason is not None and "robot emoji" in reason + failed += not ok + print(f"{'PASS' if ok else 'FAIL'}: a robot-emoji disclosure is named as " + f"the wrong marker") + + for label, command in INDIRECT_CASES: + reason = guard.verdict(command) + ok = reason is not None and "cannot read" in reason + failed += not ok + print(f"{'PASS' if ok else 'FAIL'}: {label} reports an unreadable body " + f"rather than a missing marker") + + # The hook must never block. Its only output shape is additionalContext. + src = pathlib.Path(__file__).with_name( + "require-agent-disclosure.py").read_text(encoding="utf-8") + body = src.split('"""', 2)[-1] + ok = "permissionDecision" not in body + failed += not ok + print(f"{'PASS' if ok else 'FAIL'}: the hook warns and never denies") + + # End-to-end through stdin, because `verdict()` returning a string proves + # only that the text was COMPUTED. Whether the harness ever surfaces it is a + # fact about the emitted JSON, and a test asserting bool(verdict) cannot + # tell a surfaced warning from discarded output. + for label, payload, expect_warning in ( + ("a bare comment emits additionalContext", + {"tool_name": "Bash", + "tool_input": {"command": 'gh pr comment 12 --body "Working on this."'}}, + True), + ("a disclosed comment emits nothing", + {"tool_name": "Bash", + "tool_input": {"command": f'gh pr comment 12 --body "Done.\n\n{MARKER}"'}}, + False), + ("a non-Bash tool emits nothing", + {"tool_name": "Edit", "tool_input": {"command": "gh pr comment 1 --body x"}}, + False), + ("malformed stdin fails open", "not json at all", False), + ): + stdin = payload if isinstance(payload, str) else json.dumps(payload) + proc = subprocess.run( + [sys.executable, + str(pathlib.Path(__file__).with_name("require-agent-disclosure.py"))], + input=stdin, capture_output=True, text=True) + out = proc.stdout.strip() + if expect_warning: + try: + emitted = json.loads(out)["hookSpecificOutput"] + except Exception: + emitted = {} + ok = (proc.returncode == 0 + and emitted.get("hookEventName") == "PreToolUse" + and "additionalContext" in emitted + and "permissionDecision" not in emitted + and "disclosure marker" in emitted.get("additionalContext", "")) + else: + ok = proc.returncode == 0 and out == "" + failed += not ok + print(f"{'PASS' if ok else 'FAIL'}: {label}") + + total = len(CASES) + 1 + len(INDIRECT_CASES) + 1 + 4 + print(f"\n{total - failed} passed, {failed} failed") + return 1 if failed else 0 + + +if __name__ == "__main__": + sys.exit(run()) diff --git a/memories/github.md b/memories/github.md index a31e43421..59bb0f202 100644 --- a/memories/github.md +++ b/memories/github.md @@ -193,6 +193,7 @@ in [`github-repo-transfers.md`](github-repo-transfers.md). Also note `gh api ... --jq ''` returns raw unquoted strings for string scalar expressions (such as `.head.sha`), which are not valid JSON on their own and fail `jq empty`. Fetch the endpoint JSON first and parse with `jq -r` instead. (Learned on gha#518.) - **A reply posted via `gh pr comment`/`gh api` from within a session shows up under the *human user's own* GitHub account, not a bot identity — don't mistake it for an independent human review when auditing a PR's review state.** `gh` authenticates as whatever account is logged in locally (often the user's own, e.g. seen as `dem-extra1` on `Lacaedemon/sparta`), so when an agent (or a dispatched subagent) replies to an inline review comment on the user's behalf, `gh api repos///pulls//reviews` lists it as a `COMMENTED` review authored by the user — indistinguishable at a glance from the user genuinely opening the PR in a browser and typing a reply themselves. + **Since 2026-08-24 the fix is on the posting side:** every comment an agent posts carries a trailing `_Posted by Claude Code (AI agent) --- not written by a human._` marker, so the body says what the author field cannot. See [`disclose-agent-authorship`](../shared/workflow/disclose-agent-authorship.md). That makes agent-authored comments identifiable **going forward**; comments posted before that date carry no marker, so this warning still governs when auditing older threads. Before treating an unexpected review entry as a signal that the human intervened, check whether its body/inline-comment content reads like the agent's own scripted reply (referencing a specific commit SHA, restating verification numbers) rather than free-form human commentary — if so, it's the session's own tooling, not new human input. **The same ambiguity runs the other way, and there it arrives as a positive claim rather than an inference you might draw.** An automated reviewer reading the PR's own history sees that same bot-account commit and can describe it *in its review body* as the work of a human, e.g. "that finding was confirmed and fixed by a human reviewer (`dem-extra1`) in commit ``", stating as fact something no API field asserts. That is worse than the inference case above, because the claim is now published prose a later reader inherits, and "a human already verified this" is precisely the sentence that stops the next person checking. diff --git a/memories/preferences.md b/memories/preferences.md index c7d11a12a..ade24fb94 100644 --- a/memories/preferences.md +++ b/memories/preferences.md @@ -786,7 +786,7 @@ Skill-builder encodes steps that are easy to skip when done ad hoc: the extend-first check, running the four local validation scripts (`validate-skills.py`, `check-links.py`, `check-vendored-drift.py`, `markdownlint-cli2`) before pushing, registering any cited MCP tool in `tool-mappings.yml`, updating `skills.qmd`'s count from the actual `skills/` directory count (not a manual +1), cross-linking related skills, and explicitly requesting a human reviewer after AI review passes. (Learned on ai-config#338 --- the `prompt-me`/`pm` skill was built and shipped without invoking `skill-builder`, so none of those steps ran; CI happened to catch what the scripts would have. Reinforced on ai-config#347 --- `resolve-pr-threads` was hand-authored and needed a review round to catch a `tool-mappings.yml` gap `skill-builder` already documented from a near-identical miss in `push-memory` #311.) - Claim a PR before pushing iterative commits to it, even when you opened the PR yourself in the same session --- this repo's `@claude` review workflow can fire and interleave with an in-flight push. - Post the "paws off" comment from `claim-pr` right after opening the PR, not just for PRs you're joining mid-flight. (Missed on ai-config#338: several commits were pushed across an ARDI-style review loop with no claim comment posted.) + Post the claim comment from `claim-pr` right after opening the PR, not just for PRs you're joining mid-flight. (Missed on ai-config#338: several commits were pushed across an ARDI-style review loop with no claim comment posted.) - Default dispatched subagents (`Agent({...})`, or `agent(prompt, {...})` inside a `Workflow` script) to a mid-tier model like Sonnet, not whatever model the conductor itself is running as. Reserve the top tier for genuinely judgment-heavy work (cross-cutting design docs, remediation on a disputed finding) and escalate only on explicit user request --- most fleet work (implement a spec, drive an ARDI loop, checklist-verify a "fully clean" claim against fresh queries) doesn't need the most capable tier. The concrete cost of skipping this: a session that dispatched ~30 unscoped subagents nearly exhausted a model-specific weekly quota (85-86% used, reset days out) while the all-models weekly pool sat at ~56-57% --- a wide gap between a model-specific usage bar and the all-models bar is the diagnostic signature of this exact mistake. diff --git a/scripts/orchestrator/pr_claim_manager.py b/scripts/orchestrator/pr_claim_manager.py index 589bf9374..8ca35e296 100644 --- a/scripts/orchestrator/pr_claim_manager.py +++ b/scripts/orchestrator/pr_claim_manager.py @@ -145,7 +145,16 @@ def claim_issue_and_open_draft_pr( logger.info("Successfully opened draft PR for issue #%d: %s", issue_number, result["pr_url"]) # 7. Post claim comment on issue - claim_body = f"Orchestrator worker is working on this via draft PR #{result.get('pr_number')} -- paws off until done." + # The trailing marker discloses agent authorship on every comment we + # post, and is deliberately emoji-free: check-pr-fully-clean.py + # matches the robot emoji as a review-body marker, so a claim + # carrying it would scan as a finding-free review. + # See shared/workflow/disclose-agent-authorship.md. + claim_body = ( + f"Orchestrator worker is working on this via draft PR " + f"#{result.get('pr_number')} -- please hold off until done." + "\n\n_Posted by Claude Code (AI agent) -- not written by a human._" + ) comment_cmd = ["gh", "issue", "comment", str(issue_number), "--body", claim_body] if effective_repo: comment_cmd.extend(["-R", effective_repo]) diff --git a/shared/workflow/claim-pr.md b/shared/workflow/claim-pr.md index 68a57f4fa..3b5a72557 100644 --- a/shared/workflow/claim-pr.md +++ b/shared/workflow/claim-pr.md @@ -6,10 +6,21 @@ to start a conflicting parallel session. Use: ``` -gh pr comment --body "Working on this --- paws off until I'm done." -gh issue comment --body "Working on this --- paws off until I'm done." +gh pr comment --body "Working on this --- please hold off on pushing to this branch until I'm done. + +_Posted by Claude Code (AI agent) --- not written by a human._" +gh issue comment --body "Working on this --- please hold off until I'm done. + +_Posted by Claude Code (AI agent) --- not written by a human._" ``` +Both halves of that body are load-bearing, and they answer different questions. +The first line says the thread is claimed. +The trailing line says **who claimed it**, and it is required on every comment an +agent posts to a forge --- not only on a claim. +See [`disclose-agent-authorship`](disclose-agent-authorship.md), which carries the +rule, the exact marker, and why the marker deliberately avoids the robot emoji. + Then proceed with the work. After the session ends (PR merged, issue closed, or work otherwise paused), follow up with a closing comment so the PR/issue is unclaimed for the next person. @@ -34,7 +45,7 @@ PR is a stronger "in-flight" signal than a comment alone. **A claim expires 2 hours after the most recent push or comment on the PR/issue --- reassert it rather than resuming under a stale one.** A claim comment with no expiry binds the thread indefinitely: a crashed or -abandoned session leaves its "paws off" standing forever, and a second session +abandoned session leaves its "hold off" standing forever, and a second session has no rule for when the claim stops blocking. So the convention is time-boxed and keyed to observable activity: a claim is **live for 2 hours from the most recent push or comment** on the PR/issue, and @@ -87,6 +98,41 @@ while under-respecting a live one costs a collision. issue claims last 2 hours from the most recent push or comment; if it's been longer than that, reassert your claim.") +**Every detector of a claim matches the OLD wording as well as the new one, and +dropping the old alternation is the one edit that fails silently.** +The claim body said "paws off until I'm done" until 2026-08-24 and now says +"please hold off ...". +Claims posted before that are still sitting on open PRs and issues, and a claim +stays live on activity rather than on age --- so a thread claimed under the old +wording and pushed to this morning is live right now. + +A detector narrowed to the new phrase alone still returns cleanly on such a +thread. +It returns **nothing**, which is indistinguishable from an unclaimed thread, and +that reading licenses exactly the parallel session this whole convention exists +to prevent. +Nothing in the output announces the miss: a claim search that finds no claim +looks the same whether the thread is free or the matcher went blind. + +So match the alternation, case-insensitively, everywhere a claim is read: + +```bash +gh pr view --json comments \ + -q '.comments[] | select(.body | test("hold off|paws off"; "i"))' # READ_PR_COMMENTS +``` + +Keep the old alternative until no claim under the old wording can plausibly +still be live --- which, given the 2-hour rule keys on activity and not on the +comment's own age, means until every PR and issue open on 2026-08-24 has closed. +Removing it is a deliberate later edit, not tidying to do in passing. + +- **Do:** match `hold off|paws off` case-insensitively wherever a claim is read. +- **Do:** treat the old alternative as load-bearing until the threads carrying it + have closed. +- **Don't:** read an empty claim search as an unclaimed thread without first + confirming the matcher covers both wordings --- the two results are identical. +- **Don't:** drop the back-compat alternative as part of an unrelated change. + **Verify a mid-task "already done" claim against real PR state before trusting or redoing it.** A PR you claimed and are actively driving can still gain commits from a **second, independently-running session** under the same @@ -105,7 +151,7 @@ about claiming a PR before starting, but about **re-verifying you're still the sole active driver** once work has been under way for a while --- especially when you picked up the PR mid-session (e.g. by answering a diagnostic question about it) rather than through the normal claim-then-branch -flow, so no fresh "paws off" check ever ran right before you started pushing. +flow, so no fresh claim check ever ran right before you started pushing. (`d-morrison/gha#286`, 2026-07-24: a webhook event delivered a review-comment reply attributed to `d-morrison` reading exactly like a Claude-authored reply, claiming a fix "Addressed... Pushed in 3fb8c5b" that this session diff --git a/shared/workflow/disclose-agent-authorship.md b/shared/workflow/disclose-agent-authorship.md new file mode 100644 index 000000000..e52298ccb --- /dev/null +++ b/shared/workflow/disclose-agent-authorship.md @@ -0,0 +1,101 @@ +Every comment an agent posts to a forge --- GitHub, GitLab, or any other --- +says, in the body, that an agent posted it. + +The reason is that the forge cannot say it for you. +An agent driving `gh` under the account holder's credentials posts as **that +person**: the comment carries their avatar, their login, and a `MEMBER` or +`OWNER` association, and nothing in the API response distinguishes it from a +comment they typed. +`type` reads `User`, not `Bot`, because the token belongs to a user. +So a reader deciding how much weight to give a claim, a status report, or a +review has no way to tell which of the two they are reading, and the default +reading is the wrong one. + +The marker is one line, on its own, at the end of the body: + +``` +_Posted by Claude Code (AI agent) --- not written by a human._ +``` + +**It deliberately does not use the robot emoji.** +That looks like the obvious choice, and it is the one thing the marker must +avoid: `scripts/check-pr-fully-clean.py` matches the bare emoji as a +`REVIEW_BODY_MARKERS` entry, so any comment carrying it is admitted into the +verdict scan as a review item. +A disclosure footer on every agent comment would therefore turn every claim, +every status note, and every deferral into something the fully-clean checker +reads as a review --- and a claim comment carries no findings, so it would scan +as a **clean** one. +That is the false-clean failure [`fully-clean`](fully-clean.md) already +describes for a human-authored self-review, arriving through the very mechanism +added to make authorship legible. + +The marker above collides with none of the checker's `REVIEW_BODY_MARKERS` +(the robot emoji, `### ` plus that emoji, `code review`, `**claude finished`, +`### verdict`, `verdict:`) nor with any `REVIEW_AGENT_MARKERS` entry, verified +against `scripts/check-pr-fully-clean.py` on 2026-08-24. +Check a replacement marker against both tuples before changing it. + +**The scope is every comment, not every review.** +Review comments are the case that already discloses, since a review body +announces its own agent. +The comments that need this are the ones that read most like a person: a claim, +a release, a status update, a reply on a review thread, an issue filed on the +user's behalf, a paraphrase of the user's own in-chat feedback. +Each of those is short, conversational, and posted under a human login, which +is exactly the shape that gets mistaken for a human. + +**A prose self-identification is not a substitute for the marker.** +"Claude Code CLI (local session) is working on this" already discloses, so +appending the footer to it looks redundant. +Keep both. +A convention worth anything has to be checkable by one query rather than by +reading each body and judging whether its prose happened to disclose --- and a +uniform trailing marker is what makes a sweep, or a hook, possible at all. + +**Where the marker must NOT go: content that is not a comment.** +A commit message, a PR title, or an issue title has its own attribution +conventions and its own consumers, and a trailing italic line in a commit +message corrupts a changelog. +PR bodies already carry the harness's own generated-with footer. +This rule governs comment bodies. + +**One exemption, and it is narrow: a comment another MACHINE parses as a +command.** +`@dependabot rebase`, `@dependabot squash and merge`, and their equivalents are +not addressed to a reader at all --- they are an API call wearing a comment, and +the receiving bot parses the body. +Appending prose to one risks changing what it parses, for no reader's benefit, +since nobody mistakes `@dependabot rebase` for a human's considered opinion. + +The exemption is about the **audience**, not about brevity. +A one-line status comment is short and still has a human reader, so it carries +the marker. +`skills/chores/SKILL.md`'s two Dependabot commands are the only instances of the +exemption in this corpus as of 2026-08-24. + +- **Do:** omit the marker on a comment whose whole body is a command addressed + to another bot. +- **Don't:** widen that to any comment that happens to be short, or to any + comment posted by automation --- the test is whether a machine parses the + body, not whether a machine wrote it. + +- **Do:** end every agent-posted forge comment with the marker line, on its + own, after a blank line. +- **Do:** keep the marker on comments whose prose already identifies the + session, so one query finds all of them. +- **Do:** check a proposed replacement marker against + `check-pr-fully-clean.py`'s `REVIEW_BODY_MARKERS` and `REVIEW_AGENT_MARKERS` + before adopting it. +- **Don't:** use the robot emoji in the marker --- it is a review-body marker, + and it converts every disclosed comment into a finding-free review item. +- **Don't:** treat a comment posted under a human login as self-evidently + agent-authored because the account holder knows an agent is running. + The reader is whoever finds the thread later. +- **Don't:** put the marker in a commit message, a title, or a PR body. + +(Directive from the user, 2026-08-24: "all comments online posted by bots +should say so", citing + +--- an agent-authored claim comment posted under `d-morrison`, `type: User`, +`author_association: MEMBER`, reading exactly like a human's.) diff --git a/shared/workflow/learn-from-review-findings.md b/shared/workflow/learn-from-review-findings.md index e8ed6023d..a4e73a33f 100644 --- a/shared/workflow/learn-from-review-findings.md +++ b/shared/workflow/learn-from-review-findings.md @@ -82,7 +82,7 @@ Both remedies the round-3 review suggested were a fourth narrowing, and each is A manual action by someone else, on a PR you claimed, is the same trigger with nothing labelled a finding. The trigger above still needs a reviewer to state a defect. This is the version where nobody states anything. -You claimed the PR --- posted the "paws off" comment, per [`claim-pr`](claim-pr.md) --- and while you are driving it, another actor performs an action that was already yours to have taken. +You claimed the PR --- posted the claim comment, per [`claim-pr`](claim-pr.md) --- and while you are driving it, another actor performs an action that was already yours to have taken. A human requests the review you should have requested the moment the PR opened or went ready, per [`pr-on-claim`](pr-on-claim.md)'s "Request the external reviewer in the same stride". A maintainer merges `main` in to clear a conflict [`sync-with-main`](sync-with-main.md) says you should have been watching for. A bot resolves a thread [`address-every-comment`](address-every-comment.md) says you should have resolved on Address. diff --git a/skills.qmd b/skills.qmd index cf4dc7486..caf8b8293 100644 --- a/skills.qmd +++ b/skills.qmd @@ -32,7 +32,7 @@ field (e.g. "grab an issue", "drive to clean", "update memories and skills"). | [`ardi`](skills/ardi/SKILL.md) | `dc`, `drive`, `clean`, `iterate` | Read the latest review, address/rebut/defer every finding, push, re-request review — loop until clean | | [`ard`](skills/ard/SKILL.md) | `adr` | Single ARD pass: address, rebut, defer, or acknowledge — without the iterate loop | | [`resolve-pr-threads`](skills/resolve-pr-threads/SKILL.md) | | Resolve inline review threads that are already settled, without a full ARD pass | -| [`claim-pr`](skills/claim-pr/SKILL.md) | | Post a "paws off" comment before a work session; unclaim when done | +| [`claim-pr`](skills/claim-pr/SKILL.md) | | Post a claim comment before a work session; unclaim when done | | [`request-pr-review`](skills/request-pr-review/SKILL.md) | | Add a human reviewer and @-mention them with context | | [`pr-status`](skills/pr-status/SKILL.md) | | Report the current PR's CI and review state | | [`pr-status-all`](skills/pr-status-all/SKILL.md) | | Report status across all open PRs | diff --git a/skills/ardi/SKILL.md b/skills/ardi/SKILL.md index 5b7623dcd..710851ee8 100644 --- a/skills/ardi/SKILL.md +++ b/skills/ardi/SKILL.md @@ -19,7 +19,7 @@ finding → push → post summary → re-request review → repeat until clean. 1. **Identify and claim the PR/MR.** Use the current branch's open MR, or the one the user specified. -Post a brief claim comment (`COMMENT_PR`) so a parallel `@claude` CI run or another person doesn't start a colliding session: `gh pr comment --body "Driving this PR to clean --- back off until done."` +Post a brief claim comment (`COMMENT_PR`) so a parallel `@claude` CI run or another person doesn't start a colliding session: `gh pr comment --body "Driving this PR to clean --- please hold off until done.\n\n_Posted by Claude Code (AI agent) --- not written by a human._"` Skip if your most recent comment already says so and is still live --- claims expire 2 hours after the most recent push or comment, and an expired one needs reasserting, per [`claim-pr`](../../shared/workflow/claim-pr.md). (`COMMENT_PR` and the other bracketed tokens below are abstract operation tokens --- resolve to your model's tool via [`tool-mappings.md`](../../tool-mappings.md).) @@ -383,7 +383,7 @@ The loop stopped to ask twice under the old guard; both times the answer was to ## On clean -Post an unclaim comment (`COMMENT_PR` --- `gh pr comment --body "Done --- PR is free."`) to +Post an unclaim comment (`COMMENT_PR` --- `gh pr comment --body "Done --- PR is free.\n\n_Posted by Claude Code (AI agent) --- not written by a human._"`) to unblock any parallel sessions that backed off in step 1. **Then run `ums`, before reporting ready.** diff --git a/skills/ardia/SKILL.md b/skills/ardia/SKILL.md index 227ff7cf4..b575bb319 100644 --- a/skills/ardia/SKILL.md +++ b/skills/ardia/SKILL.md @@ -38,7 +38,7 @@ mutates a PR stays serial. - **Skip a driven draft.** Any of these marks one: the head commit is still the `start:` scaffold (the implementer is mid-flight), a - still-live "paws off" claim comment stands (claims expire after 2 + still-live claim comment stands (claims expire after 2 hours with no push or comment --- [`claim-pr`](../../shared/workflow/claim-pr.md)), another actor pushed recently, or the draft is deliberately held as a merge-order diff --git a/skills/claim-pr/SKILL.md b/skills/claim-pr/SKILL.md index 39df7e017..3dded4004 100644 --- a/skills/claim-pr/SKILL.md +++ b/skills/claim-pr/SKILL.md @@ -34,8 +34,12 @@ Past 2 hours the claim has expired; re-post it before resuming. ### GitHub ```bash -gh pr comment --body "Claude Code CLI (local session) is working on this — paws off until I'm done." # COMMENT_PR -gh issue comment --body "Claude Code CLI (local session) is working on this — paws off until I'm done." # COMMENT_ISSUE +gh pr comment --body "Claude Code CLI (local session) is working on this — please hold off on pushing to this branch until I'm done. + +_Posted by Claude Code (AI agent) --- not written by a human._" # COMMENT_PR +gh issue comment --body "Claude Code CLI (local session) is working on this — please hold off until I'm done. + +_Posted by Claude Code (AI agent) --- not written by a human._" # COMMENT_ISSUE ``` (`COMMENT_PR` / `COMMENT_ISSUE` are abstract operation tokens — resolve to your @@ -47,7 +51,9 @@ On GitLab, post the claim as a **resolvable discussion** (not a plain note) so it can be resolved later: ```bash -glab mr note create --message "Claude Code CLI (local session) is working on this — paws off until I'm done." +glab mr note create --message "Claude Code CLI (local session) is working on this — please hold off on pushing to this branch until I'm done. + +_Posted by Claude Code (AI agent) --- not written by a human._" ``` > GitLab MR notes are resolvable discussions by default. @@ -102,7 +108,9 @@ DISCUSSION_ID=$(glab api "projects//merge_requests//discussi import json, sys for d in json.load(sys.stdin): for n in d.get('notes', []): - if 'paws off' in n.get('body', '') and not n.get('resolved'): + body = n.get('body', '').lower() + # Both wordings: claims posted before 2026-08-24 say 'paws off'. + if ('hold off' in body or 'paws off' in body) and not n.get('resolved'): print(d['id']); break else: continue break @@ -117,8 +125,12 @@ glab api --method PUT \ ### GitHub — post a closing comment ```bash -gh pr comment --body "Done with my local session — unclaiming." # COMMENT_PR -gh issue comment --body "Done with my local session — unclaiming." # COMMENT_ISSUE +gh pr comment --body "Done with my local session — unclaiming. + +_Posted by Claude Code (AI agent) --- not written by a human._" # COMMENT_PR +gh issue comment --body "Done with my local session — unclaiming. + +_Posted by Claude Code (AI agent) --- not written by a human._" # COMMENT_ISSUE ``` ## Notes diff --git a/skills/config-ai/SKILL.md b/skills/config-ai/SKILL.md index 9907677c6..53ef2d65c 100644 --- a/skills/config-ai/SKILL.md +++ b/skills/config-ai/SKILL.md @@ -142,7 +142,7 @@ last two. implementation form chosen in Step 1 and why, the target file(s)/path(s), and enough detail that a future `gi`/`grab-issue` session -- which does have push access -- can build it unattended. Don't claim it (`claim-pr`'s - "paws off" comment is for a session about to implement); leave it open + claim comment is for a session about to implement); leave it open for whoever picks it up. 3. **No access to the target repo at all** (not in the session's GitHub scope, no network path, issue creation itself fails) → file the issue in diff --git a/skills/defer-issue/SKILL.md b/skills/defer-issue/SKILL.md index d8646424c..9a0962bd7 100644 --- a/skills/defer-issue/SKILL.md +++ b/skills/defer-issue/SKILL.md @@ -130,9 +130,15 @@ EOF - Check `gh label list` for an existing `followup`, `deferred`, or `tech-debt` label and add it with `--label`. **Don't fabricate labels that don't exist** — `gh` will fail and you'll have to retry. -- Don't add `🤖 Generated with Claude Code` attribution to the issue body +- Don't add `🤖 Generated with Claude Code` attribution to the issue **body** unless the user asks. Issue attribution isn't covered by the global `attribution` setting. + This is not an exemption from the agent-disclosure rule, which governs + **comments** rather than issue bodies and uses a different, deliberately + emoji-free marker --- see + [`disclose-agent-authorship`](../../shared/workflow/disclose-agent-authorship.md). + So a comment this skill posts on the deferring PR carries the marker, while + the filed issue's own body stays unattributed. **GitLab:** diff --git a/skills/gi/SKILL.md b/skills/gi/SKILL.md index 959ac4080..c96c47961 100644 --- a/skills/gi/SKILL.md +++ b/skills/gi/SKILL.md @@ -87,7 +87,9 @@ check both explicitly here). gh issue view --json comments --jq '.comments | last | .body' | cat # READ_ISSUE_COMMENTS ``` -If it contains "Working on this" / "paws off" (or an equivalent claim), skip +If it contains "Working on this" / "hold off" / "paws off" (the last being the +pre-2026-08-24 wording, still live on threads claimed before then), or an +equivalent claim, skip the issue --- unless the claim has expired: no push or comment on the issue in over 2 hours, per [`claim-pr`](../../shared/workflow/claim-pr.md)'s expiration rule. @@ -124,10 +126,14 @@ MRs/PRs that touched the same area. Don't undo past progress. ```bash # GitHub -gh issue comment --body "Claude Code CLI (local session) is working on this — paws off until I'm done." # COMMENT_ISSUE +gh issue comment --body "Claude Code CLI (local session) is working on this — please hold off until I'm done. + +_Posted by Claude Code (AI agent) --- not written by a human._" # COMMENT_ISSUE # GitLab -glab issue note --message "Claude Code CLI (local session) is working on this — paws off until I'm done." +glab issue note --message "Claude Code CLI (local session) is working on this — please hold off until I'm done. + +_Posted by Claude Code (AI agent) --- not written by a human._" ``` ### 7. Create a branch diff --git a/skills/gip/SKILL.md b/skills/gip/SKILL.md index a6f70d110..d68960c61 100644 --- a/skills/gip/SKILL.md +++ b/skills/gip/SKILL.md @@ -140,7 +140,9 @@ and the default branch for each issue: > entirely within this worktree and touch only files relevant to this issue. > > 1. **Claim it** so no one else double-works it: post a brief "Working on this -> --- paws off until I'm done." comment on the issue +> --- please hold off until I'm done." comment on the issue, ending with the +> agent-disclosure marker every agent-posted comment carries +> (`_Posted by Claude Code (AI agent) --- not written by a human._`) > (`gh issue comment --body "..."`, or the MCP > `mcp__github__add_issue_comment` equivalent in a remote session). > 2. **Check history** — before writing code, scan merged/closed PRs that diff --git a/skills/handoff/SKILL.md b/skills/handoff/SKILL.md index 78ddffdc6..80f2cd61f 100644 --- a/skills/handoff/SKILL.md +++ b/skills/handoff/SKILL.md @@ -79,11 +79,13 @@ short note so the `@claude` bot and other sessions don't push conflicting changes — especially when you have unpushed local commits or running jobs. ```bash -gh pr comment --body "⏸️ **Local session paused** () — still claimed, paws off. +gh pr comment --body "⏸️ **Local session paused** () — still claimed. <2-4 bullets: in-flight jobs + IDs, unpushed local commits and why held, what runs next> -Please don't push to this branch in the meantime." # COMMENT_PR +Please hold off on pushing to this branch in the meantime. + +_Posted by Claude Code (AI agent) --- not written by a human._" # COMMENT_PR ``` If the work is genuinely *finished* (merged/closed, nothing outstanding), post a diff --git a/skills/migrate-discussion/SKILL.md b/skills/migrate-discussion/SKILL.md index 96f5173d5..e6ea62949 100644 --- a/skills/migrate-discussion/SKILL.md +++ b/skills/migrate-discussion/SKILL.md @@ -139,7 +139,9 @@ this is outward-facing and hard to reverse. not planned: ```bash - gh issue comment --body 'Moved to — this is better suited to Discussions.' # COMMENT_ISSUE + gh issue comment --body 'Moved to — this is better suited to Discussions. + + _Posted by Claude Code (AI agent) --- not written by a human._' # COMMENT_ISSUE gh issue close --reason 'not planned' # CLOSE_ISSUE ``` diff --git a/skills/post-merge/SKILL.md b/skills/post-merge/SKILL.md index 4618a2558..6a892d22d 100644 --- a/skills/post-merge/SKILL.md +++ b/skills/post-merge/SKILL.md @@ -313,7 +313,8 @@ conflicting PR can sit in `UNKNOWN` and get missed if you filter for and "A stacked PR is the one conflict that intersection cannot attribute". 3. **Check claim status.** Read the most recent comment. - If it says "Working on this --- paws off" (or equivalent) and the claim is + If it says "Working on this --- please hold off" (or the pre-2026-08-24 + wording "paws off", or any equivalent claim) and the claim is still live --- a push or comment within the last 2 hours --- skip it --- another session owns it. An expired claim (over 2 idle hours) no longer blocks; take over with a @@ -321,7 +322,9 @@ conflicting PR can sit in `UNKNOWN` and get missed if you filter for [`claim-pr`](../../shared/workflow/claim-pr.md)'s expiration rule. 4. **Claim it.** ```bash - gh pr comment --body "Working on this — paws off until I'm done." # COMMENT_PR + gh pr comment --body "Working on this — please hold off on pushing to this branch until I'm done. + + _Posted by Claude Code (AI agent) --- not written by a human._" # COMMENT_PR ``` 5. **Create an isolated worktree**, fetch the latest `main` (the squash-merge commit that caused the conflict), and merge: @@ -354,7 +357,9 @@ conflicting PR can sit in `UNKNOWN` and get missed if you filter for ``` 8. **Unclaim** with a brief resolution summary: ```bash - gh pr comment --body "Conflict resolved — branch is now mergeable. " # COMMENT_PR + gh pr comment --body "Conflict resolved — branch is now mergeable. + + _Posted by Claude Code (AI agent) --- not written by a human._" # COMMENT_PR ``` Resolve PRs one at a time — not because worktrees race each other (each diff --git a/skills/pr-on-claim/SKILL.md b/skills/pr-on-claim/SKILL.md index 6d61acf32..bbdde977a 100644 --- a/skills/pr-on-claim/SKILL.md +++ b/skills/pr-on-claim/SKILL.md @@ -28,7 +28,9 @@ Operationalizes the strong form of the claim workflow: branch → empty commit 4. Open a **draft PR** with: - Title: issue title (or override) - Body: `"Closes #\n\nWIP — opened up front to claim the issue; implementing now."` -5. Post a claim comment on the issue: `"Claude Code CLI (local session) is working on this — paws off until I'm done."` +5. Post a claim comment on the issue: `"Claude Code CLI (local session) is working on this — please hold off until I'm done.\n\n_Posted by Claude Code (AI agent) --- not written by a human._"` + (the trailing marker is required on every agent-posted comment --- see + [`disclose-agent-authorship`](../../shared/workflow/disclose-agent-authorship.md)) ## Why draft? diff --git a/skills/pr-on-claim/pr-on-claim.sh b/skills/pr-on-claim/pr-on-claim.sh index a3fc2b83c..046945927 100644 --- a/skills/pr-on-claim/pr-on-claim.sh +++ b/skills/pr-on-claim/pr-on-claim.sh @@ -41,7 +41,9 @@ WIP — opened up front to claim the issue; implementing now." \ --draft echo "Posting claim comment on issue..." -gh issue comment "$ISSUE_NUM" --body "Claude Code CLI (local session) is working on this — paws off until I'm done." +gh issue comment "$ISSUE_NUM" --body "Claude Code CLI (local session) is working on this — please hold off until I'm done. + +_Posted by Claude Code (AI agent) --- not written by a human._" echo "✓ PR opened and issue claimed." git log --oneline -1 diff --git a/skills/push/SKILL.md b/skills/push/SKILL.md index 5b7d73727..d99d1a8fd 100644 --- a/skills/push/SKILL.md +++ b/skills/push/SKILL.md @@ -89,7 +89,7 @@ git log --oneline HEAD.."origin/$BRANCH" 2>/dev/null An object you cannot resolve locally is the **stronger** signal, not the milder one: the remote moved after your last fetch and you cannot see what is there. -### 3. "Paws off" claim by someone else +### 3. Claim comment by someone else Look at the open PR for this branch for a claim comment posted by **another** session or person. (Your own most-recent "I'm working on this" comment is fine — @@ -98,10 +98,17 @@ that's your claim.) ```bash PR=$(gh pr view --json number,headRefName -q .number 2>/dev/null) # VIEW_PR gh pr view "$PR" --json comments \ - -q '.comments[] | select(.body | test("paws off"; "i")) | "\(.author.login): \(.body)"' # READ_PR_COMMENTS + -q '.comments[] | select(.body | test("hold off|paws off"; "i")) | "\(.author.login): \(.body)"' # READ_PR_COMMENTS ``` -If the latest "paws off" comment is from someone **other than you**, hasn't +The alternation is deliberate: claims posted before 2026-08-24 say "paws off", +and a claim stays live on activity rather than on age, so an old-wording claim +can be live right now. +A matcher narrowed to the new phrase returns nothing on such a thread, which +reads exactly like an unclaimed one --- see +[`claim-pr`](../../shared/workflow/claim-pr.md). + +If the latest claim comment is from someone **other than you**, hasn't been unclaimed, and is still live --- the PR shows a push or comment within the last 2 hours, per [`claim-pr`](../../shared/workflow/claim-pr.md)'s expiration rule --- @@ -190,7 +197,7 @@ review, not a draft). ## Relationship to other skills -- **`claim-pr`** — posts/removes the "paws off" claim comment this skill reads +- **`claim-pr`** — posts/removes the claim comment this skill reads in check #3. `push` is the read side; `claim-pr` is the write side. - **`session-lock` / `deconflict-sessions`** — the local-checkout counterpart: it keeps parallel sessions on one machine from sharing a working tree. `push` @@ -208,7 +215,7 @@ review, not a draft). - ❌ Force-pushing over commits another session added (check #2) - ❌ Bare `git push --force` instead of `--force-with-lease --force-if-includes` (check #6) - ❌ Reusing an earlier fetch as the check --- the reading has to be taken immediately before the push (check #2) -- ❌ Pushing past a fresh "paws off" claim from someone else (check #3) +- ❌ Pushing past a fresh claim comment from someone else (check #3) - ❌ Pushing onto a `do-not-merge` / `hold` PR without asking (check #4) - ❌ Pushing while a `@claude` run is mid-session on the branch (check #5) - ❌ Pushing directly to `main` / the default branch (check #1) diff --git a/skills/st/SKILL.md b/skills/st/SKILL.md index 3f8d57128..dd6124437 100644 --- a/skills/st/SKILL.md +++ b/skills/st/SKILL.md @@ -86,7 +86,9 @@ glab issue create --title "" --description " - Then **claim it** (`claim-pr` pattern) so a parallel session / the `@claude` bot doesn't collide: ```bash - gh issue comment --body "Claude Code CLI (local session) is working on this — paws off until I'm done." # COMMENT_ISSUE + gh issue comment --body "Claude Code CLI (local session) is working on this — please hold off until I'm done. + + _Posted by Claude Code (AI agent) --- not written by a human._" # COMMENT_ISSUE ``` ### 4. Check history diff --git a/workflow.qmd b/workflow.qmd index 0bbcbc2b1..38f9117e7 100644 --- a/workflow.qmd +++ b/workflow.qmd @@ -27,7 +27,9 @@ Before fetching a branch, editing files, or triggering a review loop, post a brief comment so other people and bots know not to start a conflicting session: ```sh -gh pr comment --body "Working on this --- paws off until I'm done." +gh pr comment --body "Working on this --- please hold off on pushing to this branch until I'm done. + +_Posted by Claude Code (AI agent) --- not written by a human._" ``` After the session ends (PR merged, paused, or closed), follow up with a From c41f0d34e926fac29701b3472d67106430ebba72 Mon Sep 17 00:00:00 2001 From: Douglas Ezra Morrison Date: Mon, 24 Aug 2026 09:58:42 -0700 Subject: [PATCH 03/13] fix(claim,disclosure): address adversarial review findings Fifteen findings from the adversarial self-review on c3bc43a5. All addressed except one deferred with a tracking issue. Detectors - commands/release-pr.md matched the ISSUE-form claim sentence while reading PR comments, and the PR-form claim is not a superstring of it -- so /release-pr on a PR claimed today would have found no claim and refused to post. Both it and shared/workflow/claim-pr.md now match the two-word invariant `hold off` (plus `paws off`) rather than any whole sentence, and claim-pr.md says why a whole-sentence matcher is the trap the rename introduces. Hook (hooks/require-agent-disclosure.py), 28 -> 52 tests - Segmentation was newline-splitting, which cut the trailing marker off the very command it discloses -- every correctly-disclosed multi-line comment warned. Split is now quote-aware. - Judged per call, so one disclosed body vouched for an undisclosed sibling in a batched round. Now judged per command-position segment. - Heredoc handling skipped the whole command on any heredoc, silencing `--body-file -` and `$(cat < --body "..." +gh pr comment --body " + +_Posted by Claude Code (AI agent) --- not written by a human._" ``` One to three sentences is enough. +The trailing marker is required, per the section above: this comment paraphrases the user in the user's own voice under the user's own login, which is the shape most easily read as their own writing. Don't quote verbatim — paraphrase so it reads naturally in the PR thread. Skip trivial acknowledgments or conversational exchanges with nothing to act on. @@ -611,28 +614,19 @@ The `claim-pr` skill operationalizes this (the exact claim wording, when it appl [shared/workflow/disclose-agent-authorship.md](shared/workflow/disclose-agent-authorship.md) -A comment posted through `gh`/`glab` under the account holder's credentials -carries **their** login, avatar, and `MEMBER` association, and reads as -`type: User` --- so nothing in the API distinguishes it from a comment they -typed, and `memories/github.md` already records auditors mistaking exactly that. +A comment posted through `gh`/`glab` under the account holder's credentials carries **their** login, avatar, and `MEMBER` association, and reads as `type: User` --- so nothing in the API distinguishes it from a comment they typed, and `memories/github.md` already records auditors mistaking exactly that. The forge cannot say it, so the body has to: end every comment with ``` _Posted by Claude Code (AI agent) --- not written by a human._ ``` -The marker deliberately avoids the robot emoji, which -`scripts/check-pr-fully-clean.py` matches as a `REVIEW_BODY_MARKERS` entry --- -a disclosed claim comment would otherwise scan as a finding-free **review**, the -false-clean failure [`fully-clean`](shared/workflow/fully-clean.md) already -describes. +The marker deliberately avoids the robot emoji, which `scripts/check-pr-fully-clean.py` matches as a `REVIEW_BODY_MARKERS` entry --- a disclosed claim comment would otherwise scan as a finding-free **review**, the false-clean failure [`fully-clean`](shared/workflow/fully-clean.md) already describes. Check any replacement marker against that tuple before adopting it. -The one exemption is a comment another machine parses as a command -(`@dependabot rebase`); the test is the audience, not the brevity. +The one exemption is a comment another machine parses as a command (`@dependabot rebase`); the test is the audience, not the brevity. Comment bodies only --- not commit messages, titles, or PR bodies. -- **Do:** append the marker to every claim, release, status, reply, and - self-review comment, including ones whose prose already names the session. +- **Do:** append the marker to every claim, release, status, reply, and self-review comment, including ones whose prose already names the session. - **Don't:** use the robot emoji in it, or put it in a commit message or title. @@ -640,11 +634,7 @@ Comment bodies only --- not commit messages, titles, or PR bodies. [shared/workflow/read-canonical-doc-before-starting.md](shared/workflow/read-canonical-doc-before-starting.md) -When a short `CLAUDE.md` names a fuller document as the actual authority ---- `.github/copilot-instructions.md`, `CONTRIBUTING.md`, a linked style -guide --- read that document before the first edit, and front-load its -pre-PR requirements into the first commit rather than discovering them via -a red CI check. +When a short `CLAUDE.md` names a fuller document as the actual authority --- `.github/copilot-instructions.md`, `CONTRIBUTING.md`, a linked style guide --- read that document before the first edit, and front-load its pre-PR requirements into the first commit rather than discovering them via a red CI check. ## Open a PR immediately after claiming an issue diff --git a/commands/release-pr.md b/commands/release-pr.md index 56885701f..6649e1b83 100644 --- a/commands/release-pr.md +++ b/commands/release-pr.md @@ -6,7 +6,8 @@ allowed-tools: - mcp__github__pull_request_read --- -Counterpart to `/claim-pr`. Post a single, recognisable "claim released" comment so other agents and sessions know the PR is free for the next person. +Counterpart to `/claim-pr`. +Post a single, recognisable "claim released" comment so other agents and sessions know the PR is free for the next person. ## Arguments @@ -31,7 +32,11 @@ If only one positional arg is given, treat it as `pr_number`. Call `mcp__github__pull_request_read(method = "get_comments", owner = , repo = , pullNumber = )`. Walk the last ~10 comments and confirm: - - the most recent claim/release exchange is an unmatched claim — a claim comment (`please hold off until I'm done`, or the pre-2026-08-24 wording `paws off until I'm done`) that hasn't yet been followed by a release. **Match both wordings on each side**, case-insensitively: claims and releases posted before 2026-08-24 use the old phrasing and are still live on open PRs, and a matcher narrowed to the new phrase alone returns nothing on them — indistinguishable from no claim at all. Treat **any** of these as a release marker: this command's `… done — claim released.`, its pre-2026-08-24 form `… done — paws off released.`, **or** the existing `claim-pr` skill's `Done with my local session — unclaiming.`, + - the most recent claim/release exchange is an unmatched claim that hasn't yet been followed by a release. + **Match the two-word invariant `hold off` (case-insensitively), never a full sentence** — the PR claim reads `please hold off on pushing to this branch until I'm done` while the issue claim reads `please hold off until I'm done`, so neither sentence is a substring of the other and a matcher keyed on either one misses the other. + **Also match the pre-2026-08-24 invariant `paws off`**: claims posted before that date are still live on open PRs, since a claim expires on activity rather than on age, and a matcher narrowed to the new wording returns nothing on them — indistinguishable from no claim at all. + So the claim test is `test("hold off|paws off"; "i")`. + Treat **any** of these as a release marker: this command's `… done — claim released.`, its pre-2026-08-24 form `… done — paws off released.`, **or** the existing `claim-pr` skill's `Done with my local session — unclaiming.`, - and that claim's `lane` matches the lane we're releasing. If the most recent signal is already a release, or the claim was by a different lane, stop and tell the user — don't post a stray release that misrepresents who was holding the PR. @@ -52,9 +57,7 @@ If only one positional arg is given, treat it as `pr_number`. _Posted by Claude Code (AI agent) --- not written by a human._ ``` - The trailing marker is required on every agent-posted comment, and is - deliberately emoji-free — see - [`disclose-agent-authorship`](../shared/workflow/disclose-agent-authorship.md). + The trailing marker is required on every agent-posted comment, and is deliberately emoji-free — see [`disclose-agent-authorship`](../shared/workflow/disclose-agent-authorship.md). 4. Post the comment: diff --git a/hooks/hooks.json b/hooks/hooks.json index 680d0b1c3..8eea1b7fe 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -353,12 +353,33 @@ "added to make authorship legible." ], "_note_cannot_see_every_body": [ - "--body-file, -F body=@file and an interpolated $BODY all put the text", - "somewhere a literal scan cannot reach. The hook reports those as an", + "--body-file, --editor, `-F ` and an interpolated $BODY all put the", + "text where a literal scan cannot reach. The hook reports those as an", "UNREADABLE body rather than as a missing marker, so its warning never", "asserts more than it observed. That partial visibility is the second", - "reason it cannot deny: a block built on a check that sees a fraction", - "of its population would refuse compliant commands." + "reason it cannot deny: a block built on a check that sees a fraction of", + "its population would refuse compliant commands.", + "", + "skills/ard/SKILL.md's per-round summary is exactly that shape -- posted", + "via --body-file on every round of every PR -- so it states the marker", + "requirement in its own text rather than relying on this guard.", + "", + "Registered under Bash AND matched against the mcp__github__* comment", + "tools, because a remote/web session has no `gh` at all and MCP is its", + "only path there. A Bash-only guard would be silent exactly where the CLI", + "is absent." + ], + "_note_segmentation_is_quote_aware": [ + "Segments are split on shell separators OUTSIDE quotes. A plain", + "re.split(r'[;&|\\n]+', ...) looked adequate and was not: the marker sits", + "on its own line at the END of a body, so splitting on every newline cuts", + "the marker off the very command it discloses, and every correctly", + "disclosed multi-line comment -- the normal shape -- warned. Caught by the", + "test suite before this hook was ever registered.", + "", + "Judging PER SEGMENT rather than per call is what stops one disclosed body", + "vouching for an undisclosed sibling in a batched round, which", + "shared/workflow/efficient-pr-babysitting.md actively encourages." ] } ] diff --git a/hooks/require-agent-disclosure.py b/hooks/require-agent-disclosure.py index e98ef7fd5..371f4a98d 100755 --- a/hooks/require-agent-disclosure.py +++ b/hooks/require-agent-disclosure.py @@ -16,22 +16,35 @@ visible to other sessions -- which is the collision the claim convention exists to prevent. Blocking would trade a labelling defect for a correctness one. - * The body is not always in the command. `--body-file`, `-F body=@file` and - `$VAR` expansion all put the text somewhere this hook cannot read, so a - literal scan cannot decide the question -- it can only decide it for the - inline-literal case. A deny built on a check that sees a fraction of its - population would refuse compliant commands, which is the failure - `require-gh-repo-flag.py` shipped and had to fix. + * The body is not always visible. `--body-file`, `--editor`, `-F ` and + `$VAR` expansion all put the text somewhere a literal scan cannot read, so + the check decides the question only for the inline-literal case. A deny + built on a check that sees a fraction of its population would refuse + compliant commands, which is why this stays advisory. (The kindred + incident in `README.md`'s hook section is the opposite shape: an early + `require-gh-repo-flag.py` matched too MUCH, firing on a heredoc that merely + documented a gated command. Both directions end in a guard that refuses + correct work; they arrive by different routes.) MARKER, not emoji. The disclosure marker deliberately avoids the robot emoji: `scripts/check-pr-fully-clean.py` matches that emoji as a `REVIEW_BODY_MARKERS` entry, so a disclosed claim comment would be admitted into the fully-clean verdict scan as a finding-free review. This hook therefore looks for the prose -marker and, separately, warns when a body discloses using the emoji instead. +marker and, separately, points out a body that discloses with the emoji. -EXEMPT: a body whose whole content is a command addressed to another bot -(`@dependabot rebase`). The test is the audience -- a machine parses that body --- not the length. +EXEMPT: a body whose WHOLE content is a command addressed to another bot +(`@dependabot rebase`, the review re-request `ardi` mandates). The test is the +audience -- a machine parses that body -- not the length, so the pattern is +anchored to the whole body rather than to its first token. + +PER SEGMENT, not per call. A batched round posting several comments in one +Bash call is the encouraged shape (`shared/workflow/efficient-pr-babysitting.md`), +so one disclosed body must not vouch for an undisclosed sibling. Each +command-position segment is judged on its own text. + +Covers the Bash CLI forms and the `mcp__github__*` comment tools, since a +remote/web session has no `gh` at all and MCP is its only path there +(`CLAUDE.md`, "Skills that call gh/glab: fall back to tool-mappings.md"). Fails OPEN: any parse problem returns 0 with no output. """ @@ -39,28 +52,51 @@ import re import sys -# Commands that post a comment body a human will read. +# --- what counts as posting a comment --------------------------------------- # -# ANCHORED at a command position -- start of string, or after a newline, pipe, -# semicolon, `&&`, `||` or an opening paren. Unanchored, this fires on every -# reply and every doc that merely QUOTES the command, and this corpus quotes it -# constantly (`shared/workflow/claim-pr.md` is nothing but such quotes). That is -# the failure `require-gh-repo-flag.py` shipped with and had to fix, recorded in -# README's hook section as the cautionary example. -COMMENT_COMMANDS = ( +# ANCHORED at a command position -- start of string, after a separator, or +# after a shell keyword that introduces a command. Unanchored, this fires on +# every reply and every doc that merely QUOTES the command, and this corpus +# quotes it constantly (`shared/workflow/claim-pr.md` is nothing but such +# quotes). +_POST_CMDS = ( r"gh\s+pr\s+comment", r"gh\s+issue\s+comment", r"gh\s+pr\s+review\b", - r"glab\s+mr\s+note", - r"glab\s+issue\s+note", + # `glab ... comment` is a real alias of `... note`; both spellings ship. + r"glab\s+mr\s+(?:note|comment)", + r"glab\s+issue\s+(?:note|comment)", + # The raw-API form `memories/git.md` prescribes for bodies carrying + # backticks, and the only way to post a REVIEW-THREAD reply. + r"gh\s+api\s+[^\n;|&]*(?:/comments|/replies)", +) +_ANCHOR = ( + r"(?:^|[;&|\n({`]|\b(?:then|else|elif|do|if|while|until)\s|!\s*)\s*" + r"(?:[A-Za-z_][A-Za-z0-9_]*=\S*\s+)*" ) -COMMENT_RE = re.compile( - r"(?:^|[\n;|&(]|&&|\|\|)\s*(?:" + "|".join(COMMENT_COMMANDS) + r")") +POST_RE = re.compile(_ANCHOR + r"(?:" + "|".join(_POST_CMDS) + r")", + re.MULTILINE) -# A heredoc means the command is WRITING text, not running it -- so a quoted -# example inside it is documentation. Erring toward silence here is the safe -# direction for a warn-only guard whose backstop is the written rule. -HEREDOC_RE = re.compile(r"<<-?\s*['\"]?\w+") +# Segment boundaries, split QUOTE-AWARE. +# +# A plain `re.split(r"[;&|\n]+", ...)` looked adequate and was not: the marker +# sits on its own line at the END of a body, so splitting on every newline cuts +# the marker off the very command it discloses, and every correctly-disclosed +# multi-line comment -- the normal shape -- warns. Tracking quote state is the +# minimum needed to keep a body intact. It is still not a shell parser; an +# unbalanced quote degrades to treating the remainder as quoted, which merges +# segments and can only silence a warning, never invent one. +SEG_SEPARATORS = ";&|\n" + +# A heredoc body is prose when it is being WRITTEN and is the comment body when +# it is being PIPED (`--body-file -`, `$(cat < int: @@ -134,10 +286,14 @@ def main() -> int: payload = json.load(sys.stdin) except Exception: return 0 - if payload.get("tool_name") != "Bash": + tool_name = payload.get("tool_name") + tool_input = payload.get("tool_input") or {} + if not isinstance(tool_input, dict): return 0 - command = (payload.get("tool_input") or {}).get("command") or "" - reason = verdict(command) + if tool_name == "Bash": + reason = verdict_bash(tool_input.get("command") or "") + else: + reason = verdict_mcp(tool_name, tool_input) if not reason: return 0 print(json.dumps({ @@ -151,8 +307,7 @@ def main() -> int: "systemMessage": ( "This forge comment may not disclose that an agent posted it. " "Comments posted through `gh`/`glab` carry your own login and read " - "as `type: User`. See " - "shared/workflow/disclose-agent-authorship.md." + "as `type: User`. " + SEE ), })) return 0 diff --git a/hooks/test-require-agent-disclosure.py b/hooks/test-require-agent-disclosure.py index 4c9809c85..d0f1dd468 100755 --- a/hooks/test-require-agent-disclosure.py +++ b/hooks/test-require-agent-disclosure.py @@ -67,6 +67,59 @@ 'git push && gh pr comment 12 --body "Pushed."', True), ("a variable elsewhere does not hide a visible marker", f'gh pr comment "$N" --repo "$REPO" --body "Done.\n\n{MARKER}"', False), + + # --- forms the first version missed entirely (review findings 2, 12) ------ + ("gh api issues comments", + 'gh api repos/o/r/issues/12/comments -f body="Working on this."', True), + ("gh api review-thread reply", + 'gh api repos/o/r/pulls/12/comments/9/replies -f body="Addressed."', True), + ("gh api reply WITH marker", + f'gh api repos/o/r/pulls/12/comments/9/replies -f body="Addressed.\n\n{MARKER}"', + False), + ("glab mr comment alias", + 'glab mr comment 12 --message "Working on this."', True), + ("glab issue comment alias", + 'glab issue comment 12 --message "Working on this."', True), + ("command after then", + 'if true; then gh pr comment 12 --body "bare"; fi', True), + ("negated command", + '! gh pr comment 12 --body "bare"', True), + ("command inside a do-loop", + 'for n in 1 2; do gh pr comment $n --body "bare"; done', True), + + # --- one marker must not vouch for a sibling (review finding 4) ----------- + ("a disclosed comment does not vouch for an undisclosed sibling", + f'gh pr comment 1 --body "a\n\n{MARKER}" && gh pr comment 2 --body "b"', + True), + ("both disclosed is silent", + f'gh pr comment 1 --body "a\n\n{MARKER}" && gh pr comment 2 --body "b\n\n{MARKER}"', + False), + ("a grep for the marker does not vouch for a bare comment", + 'grep -rn "Posted by Claude Code (AI agent)" . ; gh pr comment 2 --body "bare"', + True), + + # --- heredocs: body when piped, prose when written (review finding 3) ----- + ("heredoc IS the body, and discloses", + 'gh pr comment 12 --body-file - <<\'EOF\'\nDone.\n\n' + MARKER + + '\nEOF', False), + ("heredoc IS the body, and does not disclose", + 'gh pr comment 12 --body-file - <<\'EOF\'\nDone, undisclosed.\nEOF', True), + ("a doc heredoc does not silence a real sibling command", + 'cat > d.md <<\'EOF\'\ngh pr comment --body "x"\nEOF\ngh pr comment 2 --body "bare"', + True), + + # --- the exemption is whole-body, not first-token (review finding 8) ------ + ("a bot handle followed by prose for humans is NOT exempt", + 'gh pr comment 12 --body "@dependabot rebase please, and a note for the ' + 'humans reading this thread: I will also rerun CI"', True), + ("the review re-request is exempt", + 'gh pr comment 12 --body "@' + 'claude review"', False), + + # --- unreadable vs missing must not be confused (review finding 9) ------- + ("gh pr comment -F is a body-file, reported unreadable", + 'gh pr comment 12 -F /tmp/body.md', None), + ("--editor is unreadable", + 'gh pr comment 12 --editor', None), ] # --- the emoji branch -------------------------------------------------------- @@ -84,11 +137,20 @@ def run(): failed = 0 for label, command, expect in CASES: - got = guard.verdict(command) is not None - ok = got == expect + reason = guard.verdict(command) + if expect is None: + # Must warn, and specifically about a body it could not read -- + # accusing a command of omitting a marker never seen is the + # misdiagnosis review finding 9 named. + ok = reason is not None and "cannot read" in reason + print(f"{'PASS' if ok else 'FAIL'}: {label} " + f"(reported unreadable={ok})") + else: + got = reason is not None + ok = got == expect + print(f"{'PASS' if ok else 'FAIL'}: {label} " + f"(warned={got}, expected={expect})") failed += not ok - print(f"{'PASS' if ok else 'FAIL'}: {label} " - f"(warned={got}, expected={expect})") reason = guard.verdict(ROBOT_CASE) ok = reason is not None and "robot emoji" in reason @@ -96,6 +158,15 @@ def run(): print(f"{'PASS' if ok else 'FAIL'}: a robot-emoji disclosure is named as " f"the wrong marker") + # Review finding 14: a body merely MENTIONING the emoji discloses nothing, + # so the emoji advice would be inapplicable and would displace the real one. + mention = 'gh pr comment 12 --body "The \U0001f916 badge broke; rerunning."' + reason = guard.verdict(mention) + ok = reason is not None and "robot emoji" not in reason + failed += not ok + print(f"{'PASS' if ok else 'FAIL'}: merely mentioning the emoji is not " + f"treated as disclosing with it") + for label, command in INDIRECT_CASES: reason = guard.verdict(command) ok = reason is not None and "cannot read" in reason @@ -150,7 +221,27 @@ def run(): failed += not ok print(f"{'PASS' if ok else 'FAIL'}: {label}") - total = len(CASES) + 1 + len(INDIRECT_CASES) + 1 + 4 + # Review finding 10: a remote/web session has no `gh`, so MCP is its only + # path -- a Bash-only guard is silent exactly where the CLI is absent. + for label, tool, body, expect in ( + ("MCP add_issue_comment bare", "mcp__github__add_issue_comment", + "Working on this.", True), + ("MCP add_issue_comment disclosed", "mcp__github__add_issue_comment", + "Working on this.\n\n" + MARKER, False), + ("MCP review reply bare", + "mcp__github__add_reply_to_pull_request_comment", "Addressed.", True), + ("MCP bot-command body is exempt", "mcp__github__add_issue_comment", + "@dependabot rebase", False), + ("a non-comment MCP tool is out of scope", + "mcp__github__create_pull_request", "Closes #1", False), + ): + got = guard.verdict_mcp(tool, {"body": body}) is not None + ok = got == expect + failed += not ok + print(f"{'PASS' if ok else 'FAIL'}: {label} " + f"(warned={got}, expected={expect})") + + total = len(CASES) + 2 + len(INDIRECT_CASES) + 1 + 4 + 5 print(f"\n{total - failed} passed, {failed} failed") return 1 if failed else 0 diff --git a/memories/github.md b/memories/github.md index 59bb0f202..127e4a6ea 100644 --- a/memories/github.md +++ b/memories/github.md @@ -192,10 +192,14 @@ in [`github-repo-transfers.md`](github-repo-transfers.md). `jq -e .` does separate empty from valid input, but then rejects the legitimate `null`/`false` bodies `gh api` returns routinely. Also note `gh api ... --jq ''` returns raw unquoted strings for string scalar expressions (such as `.head.sha`), which are not valid JSON on their own and fail `jq empty`. Fetch the endpoint JSON first and parse with `jq -r` instead. (Learned on gha#518.) -- **A reply posted via `gh pr comment`/`gh api` from within a session shows up under the *human user's own* GitHub account, not a bot identity — don't mistake it for an independent human review when auditing a PR's review state.** `gh` authenticates as whatever account is logged in locally (often the user's own, e.g. seen as `dem-extra1` on `Lacaedemon/sparta`), so when an agent (or a dispatched subagent) replies to an inline review comment on the user's behalf, `gh api repos///pulls//reviews` lists it as a `COMMENTED` review authored by the user — indistinguishable at a glance from the user genuinely opening the PR in a browser and typing a reply themselves. - **Since 2026-08-24 the fix is on the posting side:** every comment an agent posts carries a trailing `_Posted by Claude Code (AI agent) --- not written by a human._` marker, so the body says what the author field cannot. See [`disclose-agent-authorship`](../shared/workflow/disclose-agent-authorship.md). That makes agent-authored comments identifiable **going forward**; comments posted before that date carry no marker, so this warning still governs when auditing older threads. +- **A reply posted via `gh pr comment`/`gh api` from within a session shows up under the *human user's own* GitHub account, not a bot identity — don't mistake it for an independent human review when auditing a PR's review state.** + `gh` authenticates as whatever account is logged in locally (often the user's own, e.g. seen as `dem-extra1` on `Lacaedemon/sparta`), so when an agent (or a dispatched subagent) replies to an inline review comment on the user's behalf, `gh api repos///pulls//reviews` lists it as a `COMMENTED` review authored by the user — indistinguishable at a glance from the user genuinely opening the PR in a browser and typing a reply themselves. + **Since 2026-08-24 the fix is on the posting side:** every comment an agent posts carries a trailing `_Posted by Claude Code (AI agent) --- not written by a human._` marker, so the body says what the author field cannot. + See [`disclose-agent-authorship`](../shared/workflow/disclose-agent-authorship.md). + That makes agent-authored comments identifiable **going forward**; comments posted before that date carry no marker, so this warning still governs when auditing older threads. Before treating an unexpected review entry as a signal that the human intervened, check whether its body/inline-comment content reads like the agent's own scripted reply (referencing a specific commit SHA, restating verification numbers) rather than free-form human commentary — if so, it's the session's own tooling, not new human input. - **The same ambiguity runs the other way, and there it arrives as a positive claim rather than an inference you might draw.** An automated reviewer reading the PR's own history sees that same bot-account commit and can describe it *in its review body* as the work of a human, e.g. "that finding was confirmed and fixed by a human reviewer (`dem-extra1`) in commit ``", stating as fact something no API field asserts. + **The same ambiguity runs the other way, and there it arrives as a positive claim rather than an inference you might draw.** + An automated reviewer reading the PR's own history sees that same bot-account commit and can describe it *in its review body* as the work of a human, e.g. "that finding was confirmed and fixed by a human reviewer (`dem-extra1`) in commit ``", stating as fact something no API field asserts. That is worse than the inference case above, because the claim is now published prose a later reader inherits, and "a human already verified this" is precisely the sentence that stops the next person checking. Correct it in the thread when you see it, naming which account is actually a session identity; don't let it stand just because the surrounding verdict was clean. (`ucdavis/bcs#532`, 2026-07-31: a `claude-review` pass reported a fix as human-confirmed when `dem-extra1` was the Claude session that made it, and no human had touched the PR at that point.) - **`gh pr view --json` does not accept `merged` as a field.** Use `state` (returns `"MERGED"`) and `mergedAt` (ISO timestamp, null if not merged) to check merge status. Example: `gh pr view --json state,mergedAt`. diff --git a/shared/workflow/claim-pr.md b/shared/workflow/claim-pr.md index 3b5a72557..b26779b1d 100644 --- a/shared/workflow/claim-pr.md +++ b/shared/workflow/claim-pr.md @@ -16,14 +16,11 @@ _Posted by Claude Code (AI agent) --- not written by a human._" Both halves of that body are load-bearing, and they answer different questions. The first line says the thread is claimed. -The trailing line says **who claimed it**, and it is required on every comment an -agent posts to a forge --- not only on a claim. -See [`disclose-agent-authorship`](disclose-agent-authorship.md), which carries the -rule, the exact marker, and why the marker deliberately avoids the robot emoji. +The trailing line says **who claimed it**, and it is required on every comment an agent posts to a forge --- not only on a claim. +See [`disclose-agent-authorship`](disclose-agent-authorship.md), which carries the rule, the exact marker, and why the marker deliberately avoids the robot emoji. -Then proceed with the work. After the session ends (PR merged, issue closed, or -work otherwise paused), follow up with a closing comment so the PR/issue is -unclaimed for the next person. +Then proceed with the work. +After the session ends (PR merged, issue closed, or work otherwise paused), follow up with a closing comment so the PR/issue is unclaimed for the next person. Skip the claim step if the most recent comment already says you are working on it **and that claim is still live under the expiration rule below**. @@ -44,12 +41,8 @@ PR is a stronger "in-flight" signal than a comment alone. **A claim expires 2 hours after the most recent push or comment on the PR/issue --- reassert it rather than resuming under a stale one.** -A claim comment with no expiry binds the thread indefinitely: a crashed or -abandoned session leaves its "hold off" standing forever, and a second session -has no rule for when the claim stops blocking. -So the convention is time-boxed and keyed to observable activity: a claim is -**live for 2 hours from the most recent push or comment** on the PR/issue, and -**expired** past that. +A claim comment with no expiry binds the thread indefinitely: a crashed or abandoned session leaves its "hold off" standing forever, and a second session has no rule for when the claim stops blocking. +So the convention is time-boxed and keyed to observable activity: a claim is **live for 2 hours from the most recent push or comment** on the PR/issue, and **expired** past that. The rule cuts both ways. @@ -98,21 +91,18 @@ while under-respecting a live one costs a collision. issue claims last 2 hours from the most recent push or comment; if it's been longer than that, reassert your claim.") -**Every detector of a claim matches the OLD wording as well as the new one, and -dropping the old alternation is the one edit that fails silently.** -The claim body said "paws off until I'm done" until 2026-08-24 and now says -"please hold off ...". -Claims posted before that are still sitting on open PRs and issues, and a claim -stays live on activity rather than on age --- so a thread claimed under the old -wording and pushed to this morning is live right now. - -A detector narrowed to the new phrase alone still returns cleanly on such a -thread. -It returns **nothing**, which is indistinguishable from an unclaimed thread, and -that reading licenses exactly the parallel session this whole convention exists -to prevent. -Nothing in the output announces the miss: a claim search that finds no claim -looks the same whether the thread is free or the matcher went blind. +**Every detector of a claim matches the OLD wording as well as the new one, and dropping the old alternation is the one edit that fails silently.** +The claim body said "paws off until I'm done" until 2026-08-24 and now says "please hold off ...". +Claims posted before that are still sitting on open PRs and issues, and a claim stays live on activity rather than on age --- so a thread claimed under the old wording and pushed to this morning is live right now. + +A detector narrowed to the new phrase alone still returns cleanly on such a thread. +It returns **nothing**, which is indistinguishable from an unclaimed thread, and that reading licenses exactly the parallel session this whole convention exists to prevent. +Nothing in the output announces the miss: a claim search that finds no claim looks the same whether the thread is free or the matcher went blind. + +**Match the two-word invariant, never a whole sentence.** +The claim body varies by target --- a PR claim says "please hold off on pushing to this branch until I'm done" and an issue claim says "please hold off until I'm done" --- so neither sentence contains the other, and a detector keyed on either one is blind to half the claims. +Under the old single-string wording that distinction did not exist, which is exactly why it is easy to carry a whole-sentence matcher across the rename without noticing it has narrowed. +`hold off` is the invariant; `paws off` is its predecessor. So match the alternation, case-insensitively, everywhere a claim is read: @@ -121,51 +111,27 @@ gh pr view --json comments \ -q '.comments[] | select(.body | test("hold off|paws off"; "i"))' # READ_PR_COMMENTS ``` -Keep the old alternative until no claim under the old wording can plausibly -still be live --- which, given the 2-hour rule keys on activity and not on the -comment's own age, means until every PR and issue open on 2026-08-24 has closed. +Keep the old alternative until no claim under the old wording can plausibly still be live --- which, given the 2-hour rule keys on activity and not on the comment's own age, means until every PR and issue open on 2026-08-24 has closed. Removing it is a deliberate later edit, not tidying to do in passing. - **Do:** match `hold off|paws off` case-insensitively wherever a claim is read. -- **Do:** treat the old alternative as load-bearing until the threads carrying it - have closed. -- **Don't:** read an empty claim search as an unclaimed thread without first - confirming the matcher covers both wordings --- the two results are identical. +- **Do:** treat the old alternative as load-bearing until the threads carrying it have closed. +- **Don't:** read an empty claim search as an unclaimed thread without first confirming the matcher covers both wordings --- the two results are identical. - **Don't:** drop the back-compat alternative as part of an unrelated change. -**Verify a mid-task "already done" claim against real PR state before trusting -or redoing it.** A PR you claimed and are actively driving can still gain -commits from a **second, independently-running session** under the same -account --- a `` review-comment-reply event can -describe work ("Addressed... Pushed in ``") that this session never did. -Don't assume it's fabricated or injected, and don't reflexively redo the same -fix: cross-check the PR's actual commit list (`gh pr view --json commits` / -`pull_request_read` `get_commits`) and review threads before either (a) -trusting the claim, or (b) starting the same fix yourself. If a commit with -that SHA genuinely exists, authored close to when the event arrived, treat it -as confirmation a live parallel session owns this PR right now --- stop -pushing further speculative fixes yourself, and, if genuinely in doubt, ask -whether to keep driving or step back, rather than racing the other session's -pushes. This gap is distinct from the initial claim check above: it's not -about claiming a PR before starting, but about **re-verifying you're still -the sole active driver** once work has been under way for a while --- -especially when you picked up the PR mid-session (e.g. by answering a -diagnostic question about it) rather than through the normal claim-then-branch -flow, so no fresh claim check ever ran right before you started pushing. -(`d-morrison/gha#286`, 2026-07-24: a webhook event delivered a review-comment -reply attributed to `d-morrison` reading exactly like a Claude-authored -reply, claiming a fix "Addressed... Pushed in 3fb8c5b" that this session -hadn't made; verified real via `get_commits` before proceeding --- a second -live session, not injection.) - -**The git-level variant of that check: a rejected push whose remote commit is -byte-for-byte what you were about to push.** +**Verify a mid-task "already done" claim against real PR state before trusting or redoing it.** +A PR you claimed and are actively driving can still gain commits from a **second, independently-running session** under the same account --- a `` review-comment-reply event can describe work ("Addressed... +Pushed in ``") that this session never did. +Don't assume it's fabricated or injected, and don't reflexively redo the same fix: cross-check the PR's actual commit list (`gh pr view --json commits` / `pull_request_read` `get_commits`) and review threads before either (a) trusting the claim, or (b) starting the same fix yourself. +If a commit with that SHA genuinely exists, authored close to when the event arrived, treat it as confirmation a live parallel session owns this PR right now --- stop pushing further speculative fixes yourself, and, if genuinely in doubt, ask whether to keep driving or step back, rather than racing the other session's pushes. +This gap is distinct from the initial claim check above: it's not about claiming a PR before starting, but about **re-verifying you're still the sole active driver** once work has been under way for a while --- especially when you picked up the PR mid-session (e.g. by answering a diagnostic question about it) rather than through the normal claim-then-branch flow, so no fresh claim check ever ran right before you started pushing. (`d-morrison/gha#286`, 2026-07-24: a webhook event delivered a review-comment reply attributed to `d-morrison` reading exactly like a Claude-authored reply, claiming a fix "Addressed... +Pushed in 3fb8c5b" that this session hadn't made; verified real via `get_commits` before proceeding --- a second live session, not injection.) + +**The git-level variant of that check: a rejected push whose remote commit is byte-for-byte what you were about to push.** The section above covers a *comment* claiming work was done. Here the parallel session makes no claim at all. -Your `git push` is simply rejected because it pushed first, and what it -pushed is the same merge you just made. -The reflex on a rejected push is to merge again, which would stack a -redundant merge commit on top of an identical one. +Your `git push` is simply rejected because it pushed first, and what it pushed is the same merge you just made. +The reflex on a rejected push is to merge again, which would stack a redundant merge commit on top of an identical one. Four reads settle it before you touch anything: @@ -176,19 +142,16 @@ git show -s --format=%P HEAD # your merge's parents git show -s --format=%P origin/ # its parents ``` -An identical tree plus identical parents means the two merges are the same -merge, so the right action is `git reset --hard origin/`. +An identical tree plus identical parents means the two merges are the same merge, so the right action is `git reset --hard origin/`. - **Do:** compare trees and parents before deciding what a rejected push means. - **Do:** discard your local merge with `git reset --hard origin/` once both match. -- **Don't:** re-merge reflexively on a rejected push --- that is what - produces the redundant merge commit. +- **Don't:** re-merge reflexively on a rejected push --- that is what produces the redundant merge commit. - **Don't:** force-push over the other session's commit. -(`Morrison-Lab/ai-config#965`, 2026-07-31: `main` moved one commit, a local -`git merge origin/main` was made, and the push was rejected. +(`Morrison-Lab/ai-config#965`, 2026-07-31: `main` moved one commit, a local `git merge origin/main` was made, and the push was rejected. The remote carried `b8d2273`, a merge of the same two parents, with tree `1bda1bc`, identical to the local merge's.) diff --git a/shared/workflow/disclose-agent-authorship.md b/shared/workflow/disclose-agent-authorship.md index e52298ccb..57b1a9c40 100644 --- a/shared/workflow/disclose-agent-authorship.md +++ b/shared/workflow/disclose-agent-authorship.md @@ -1,15 +1,9 @@ -Every comment an agent posts to a forge --- GitHub, GitLab, or any other --- -says, in the body, that an agent posted it. +Every comment an agent posts to a forge --- GitHub, GitLab, or any other --- says, in the body, that an agent posted it. The reason is that the forge cannot say it for you. -An agent driving `gh` under the account holder's credentials posts as **that -person**: the comment carries their avatar, their login, and a `MEMBER` or -`OWNER` association, and nothing in the API response distinguishes it from a -comment they typed. +An agent driving `gh` under the account holder's credentials posts as **that person**: the comment carries their avatar, their login, and a `MEMBER` or `OWNER` association, and nothing in the API response distinguishes it from a comment they typed. `type` reads `User`, not `Bot`, because the token belongs to a user. -So a reader deciding how much weight to give a claim, a status report, or a -review has no way to tell which of the two they are reading, and the default -reading is the wrong one. +So a reader deciding how much weight to give a claim, a status report, or a review has no way to tell which of the two they are reading, and the default reading is the wrong one. The marker is one line, on its own, at the end of the body: @@ -18,84 +12,62 @@ _Posted by Claude Code (AI agent) --- not written by a human._ ``` **It deliberately does not use the robot emoji.** -That looks like the obvious choice, and it is the one thing the marker must -avoid: `scripts/check-pr-fully-clean.py` matches the bare emoji as a -`REVIEW_BODY_MARKERS` entry, so any comment carrying it is admitted into the -verdict scan as a review item. -A disclosure footer on every agent comment would therefore turn every claim, -every status note, and every deferral into something the fully-clean checker -reads as a review --- and a claim comment carries no findings, so it would scan -as a **clean** one. -That is the false-clean failure [`fully-clean`](fully-clean.md) already -describes for a human-authored self-review, arriving through the very mechanism -added to make authorship legible. - -The marker above collides with none of the checker's `REVIEW_BODY_MARKERS` -(the robot emoji, `### ` plus that emoji, `code review`, `**claude finished`, -`### verdict`, `verdict:`) nor with any `REVIEW_AGENT_MARKERS` entry, verified -against `scripts/check-pr-fully-clean.py` on 2026-08-24. +That looks like the obvious choice, and it is the one thing the marker must avoid: `scripts/check-pr-fully-clean.py` matches the bare emoji as a `REVIEW_BODY_MARKERS` entry, so any comment carrying it is admitted into the verdict scan as a review item. +A disclosure footer on every agent comment would therefore turn every claim, every status note, and every deferral into something the fully-clean checker reads as a review --- and a claim comment carries no findings, so it would scan as a **clean** one. +That is the false-clean failure [`fully-clean`](fully-clean.md) already describes for a human-authored self-review, arriving through the very mechanism added to make authorship legible. + +The marker above collides with none of the checker's `REVIEW_BODY_MARKERS` (the robot emoji, `### ` plus that emoji, `code review`, `**claude finished`, `### verdict`, `verdict:`) nor with any `REVIEW_AGENT_MARKERS` entry, verified against `scripts/check-pr-fully-clean.py` on 2026-08-24. Check a replacement marker against both tuples before changing it. **The scope is every comment, not every review.** -Review comments are the case that already discloses, since a review body -announces its own agent. -The comments that need this are the ones that read most like a person: a claim, -a release, a status update, a reply on a review thread, an issue filed on the -user's behalf, a paraphrase of the user's own in-chat feedback. -Each of those is short, conversational, and posted under a human login, which -is exactly the shape that gets mistaken for a human. +Review comments are the case that already discloses, since a review body announces its own agent. +The comments that need this are the ones that read most like a person: a claim, a release, a status update, a reply on a review thread, an issue filed on the user's behalf, a paraphrase of the user's own in-chat feedback. +Each of those is short, conversational, and posted under a human login, which is exactly the shape that gets mistaken for a human. **A prose self-identification is not a substitute for the marker.** -"Claude Code CLI (local session) is working on this" already discloses, so -appending the footer to it looks redundant. +"Claude Code CLI (local session) is working on this" already discloses, so appending the footer to it looks redundant. Keep both. -A convention worth anything has to be checkable by one query rather than by -reading each body and judging whether its prose happened to disclose --- and a -uniform trailing marker is what makes a sweep, or a hook, possible at all. +A convention worth anything has to be checkable by one query rather than by reading each body and judging whether its prose happened to disclose --- and a uniform trailing marker is what makes a sweep, or a hook, possible at all. + +**Search for the marker as a substring, never as a whole line.** +A body composed inside an indented code fence carries that fence's indentation into the posted comment, so the marker can arrive with leading spaces. +Dedenting the source to column 0 is not the fix: a column-0 line ends the enclosing list item, which closes the fence and turns the marker into prose --- markdownlint MD049 catches it, and the comment stops being shown as a command at all. +So the source keeps its indentation and the query drops its anchor. + +**The guard cannot see every body, and the gaps are worth knowing.** +`hooks/require-agent-disclosure.py` reads the command text and the `mcp__github__*` comment tools, so it is silent on a body it cannot reach: a `--body-file`, an `--editor` session, an interpolated `$BODY`. +It reports those as an **unreadable** body rather than as a missing marker, so its warning never asserts more than it observed --- but a `--body-file` comment that genuinely omits the marker draws only the weaker note. +`skills/ard/SKILL.md`'s per-round summary is exactly that shape, which is why it states the requirement in its own text rather than relying on the guard. **Where the marker must NOT go: content that is not a comment.** -A commit message, a PR title, or an issue title has its own attribution -conventions and its own consumers, and a trailing italic line in a commit -message corrupts a changelog. +A commit message, a PR title, or an issue title has its own attribution conventions and its own consumers, and a trailing italic line in a commit message corrupts a changelog. PR bodies already carry the harness's own generated-with footer. This rule governs comment bodies. -**One exemption, and it is narrow: a comment another MACHINE parses as a -command.** -`@dependabot rebase`, `@dependabot squash and merge`, and their equivalents are -not addressed to a reader at all --- they are an API call wearing a comment, and -the receiving bot parses the body. -Appending prose to one risks changing what it parses, for no reader's benefit, -since nobody mistakes `@dependabot rebase` for a human's considered opinion. +**One exemption, and it is narrow: a comment another MACHINE parses as a command.** +`@dependabot rebase`, `@dependabot squash and merge`, and their equivalents are not addressed to a reader at all --- they are an API call wearing a comment, and the receiving bot parses the body. +Appending prose to one risks changing what it parses, for no reader's benefit, since nobody mistakes `@dependabot rebase` for a human's considered opinion. The exemption is about the **audience**, not about brevity. -A one-line status comment is short and still has a human reader, so it carries -the marker. -`skills/chores/SKILL.md`'s two Dependabot commands are the only instances of the -exemption in this corpus as of 2026-08-24. - -- **Do:** omit the marker on a comment whose whole body is a command addressed - to another bot. -- **Don't:** widen that to any comment that happens to be short, or to any - comment posted by automation --- the test is whether a machine parses the - body, not whether a machine wrote it. - -- **Do:** end every agent-posted forge comment with the marker line, on its - own, after a blank line. -- **Do:** keep the marker on comments whose prose already identifies the - session, so one query finds all of them. -- **Do:** check a proposed replacement marker against - `check-pr-fully-clean.py`'s `REVIEW_BODY_MARKERS` and `REVIEW_AGENT_MARKERS` - before adopting it. -- **Don't:** use the robot emoji in the marker --- it is a review-body marker, - and it converts every disclosed comment into a finding-free review item. -- **Don't:** treat a comment posted under a human login as self-evidently - agent-authored because the account holder knows an agent is running. +A one-line status comment is short and still has a human reader, so it carries the marker. + +As of 2026-08-24 the exemption covers three sites: `skills/chores/SKILL.md`'s two Dependabot commands, and the review re-request `skills/ardi/SKILL.md` mandates, whose whole body is the reviewer's own `@`-mention. +That third one is worth naming because the first draft of this rule missed it and asserted the other two were the only instances --- an enumeration of a population nobody had queried, which is [`metacognitive-monitoring`](metacognitive-monitoring.md)'s scope-claim failure. +Derive the set before restating it: + +```bash +grep -rn -- '--body "@\|--message "@' --include="*.md" --include="*.sh" . +``` + +- **Do:** omit the marker on a comment whose whole body is a command addressed to another bot. +- **Don't:** widen that to any comment that happens to be short, or to any comment posted by automation --- the test is whether a machine parses the body, not whether a machine wrote it. + +- **Do:** end every agent-posted forge comment with the marker line, on its own, after a blank line. +- **Do:** keep the marker on comments whose prose already identifies the session, so one query finds all of them. +- **Do:** check a proposed replacement marker against `check-pr-fully-clean.py`'s `REVIEW_BODY_MARKERS` and `REVIEW_AGENT_MARKERS` before adopting it. +- **Don't:** use the robot emoji in the marker --- it is a review-body marker, and it converts every disclosed comment into a finding-free review item. +- **Don't:** treat a comment posted under a human login as self-evidently agent-authored because the account holder knows an agent is running. The reader is whoever finds the thread later. - **Don't:** put the marker in a commit message, a title, or a PR body. -(Directive from the user, 2026-08-24: "all comments online posted by bots -should say so", citing - ---- an agent-authored claim comment posted under `d-morrison`, `type: User`, -`author_association: MEMBER`, reading exactly like a human's.) +(Directive from the user, 2026-08-24: "all comments online posted by bots should say so", citing --- an agent-authored claim comment posted under `d-morrison`, `type: User`, `author_association: MEMBER`, reading exactly like a human's.) diff --git a/skills/ard/SKILL.md b/skills/ard/SKILL.md index 8cc26437f..122a9b261 100644 --- a/skills/ard/SKILL.md +++ b/skills/ard/SKILL.md @@ -112,6 +112,15 @@ gh pr comment --body-file ard-summary.md # COMMENT_PR — GitHub glab mr note -F ard-summary.md # GitLab ``` +**End `ard-summary.md` with the agent-disclosure marker**, on its own line after a blank line: + +``` +_Posted by Claude Code (AI agent) --- not written by a human._ +``` + +This comment is posted under the account holder's own login on every round of every PR, so it is among the likeliest in the corpus to be mistaken for their own writing --- see [`disclose-agent-authorship`](../../shared/workflow/disclose-agent-authorship.md). +The body arrives via `--body-file`, which the disclosure guard cannot read, so nothing will warn you if it is missing. + **Keep the bot's trigger phrase out of the summary body.** The `issue_comment` trigger fires on the bare bot `@`-mention **anywhere** in a comment — even in a sentence saying you're *not* re-requesting a review. Refer to it obliquely diff --git a/skills/ardi/SKILL.md b/skills/ardi/SKILL.md index 710851ee8..de693ed23 100644 --- a/skills/ardi/SKILL.md +++ b/skills/ardi/SKILL.md @@ -19,7 +19,14 @@ finding → push → post summary → re-request review → repeat until clean. 1. **Identify and claim the PR/MR.** Use the current branch's open MR, or the one the user specified. -Post a brief claim comment (`COMMENT_PR`) so a parallel `@claude` CI run or another person doesn't start a colliding session: `gh pr comment --body "Driving this PR to clean --- please hold off until done.\n\n_Posted by Claude Code (AI agent) --- not written by a human._"` +Post a brief claim comment (`COMMENT_PR`) so a parallel `@claude` CI run or another person doesn't start a colliding session. +The body carries a real blank line before the disclosure marker --- `\n` inside a bash double-quoted string is the two characters, not a newline: + +```bash +gh pr comment --body "Driving this PR to clean --- please hold off until done. + +_Posted by Claude Code (AI agent) --- not written by a human._" # COMMENT_PR +``` Skip if your most recent comment already says so and is still live --- claims expire 2 hours after the most recent push or comment, and an expired one needs reasserting, per [`claim-pr`](../../shared/workflow/claim-pr.md). (`COMMENT_PR` and the other bracketed tokens below are abstract operation tokens --- resolve to your model's tool via [`tool-mappings.md`](../../tool-mappings.md).) @@ -383,8 +390,13 @@ The loop stopped to ask twice under the old guard; both times the answer was to ## On clean -Post an unclaim comment (`COMMENT_PR` --- `gh pr comment --body "Done --- PR is free.\n\n_Posted by Claude Code (AI agent) --- not written by a human._"`) to -unblock any parallel sessions that backed off in step 1. +Post an unclaim comment (`COMMENT_PR`) to unblock any parallel sessions that backed off in step 1: + +```bash +gh pr comment --body "Done --- PR is free. + +_Posted by Claude Code (AI agent) --- not written by a human._" # COMMENT_PR +``` **Then run `ums`, before reporting ready.** The clean verdict is the proactive-UMS checkpoint for this PR, not the merge; diff --git a/skills/defer-issue/SKILL.md b/skills/defer-issue/SKILL.md index 9a0962bd7..0df07e95d 100644 --- a/skills/defer-issue/SKILL.md +++ b/skills/defer-issue/SKILL.md @@ -130,15 +130,10 @@ EOF - Check `gh label list` for an existing `followup`, `deferred`, or `tech-debt` label and add it with `--label`. **Don't fabricate labels that don't exist** — `gh` will fail and you'll have to retry. -- Don't add `🤖 Generated with Claude Code` attribution to the issue **body** - unless the user asks. Issue attribution isn't covered by the global - `attribution` setting. - This is not an exemption from the agent-disclosure rule, which governs - **comments** rather than issue bodies and uses a different, deliberately - emoji-free marker --- see - [`disclose-agent-authorship`](../../shared/workflow/disclose-agent-authorship.md). - So a comment this skill posts on the deferring PR carries the marker, while - the filed issue's own body stays unattributed. +- Don't add `🤖 Generated with Claude Code` attribution to the issue **body** unless the user asks. + Issue attribution isn't covered by the global `attribution` setting. + This is not an exemption from the agent-disclosure rule, which governs **comments** rather than issue bodies and uses a different, deliberately emoji-free marker --- see [`disclose-agent-authorship`](../../shared/workflow/disclose-agent-authorship.md). + So a comment this skill posts on the deferring PR carries the marker, while the filed issue's own body stays unattributed. **GitLab:** diff --git a/skills/gi/SKILL.md b/skills/gi/SKILL.md index c96c47961..e969b821c 100644 --- a/skills/gi/SKILL.md +++ b/skills/gi/SKILL.md @@ -87,14 +87,8 @@ check both explicitly here). gh issue view --json comments --jq '.comments | last | .body' | cat # READ_ISSUE_COMMENTS ``` -If it contains "Working on this" / "hold off" / "paws off" (the last being the -pre-2026-08-24 wording, still live on threads claimed before then), or an -equivalent claim, skip -the issue --- unless the claim has expired: no push or comment on the issue in -over 2 hours, per [`claim-pr`](../../shared/workflow/claim-pr.md)'s expiration -rule. -An expired claim is taken over by posting your own claim comment, never -silently. +If it contains "Working on this" / "hold off" / "paws off" (the last being the pre-2026-08-24 wording, still live on threads claimed before then), or an equivalent claim, skip the issue --- unless the claim has expired: no push or comment on the issue in over 2 hours, per [`claim-pr`](../../shared/workflow/claim-pr.md)'s expiration rule. +An expired claim is taken over by posting your own claim comment, never silently. **(2) No open PR already references the issue:** diff --git a/skills/gip/SKILL.md b/skills/gip/SKILL.md index d68960c61..21394a394 100644 --- a/skills/gip/SKILL.md +++ b/skills/gip/SKILL.md @@ -140,11 +140,10 @@ and the default branch for each issue: > entirely within this worktree and touch only files relevant to this issue. > > 1. **Claim it** so no one else double-works it: post a brief "Working on this -> --- please hold off until I'm done." comment on the issue, ending with the -> agent-disclosure marker every agent-posted comment carries -> (`_Posted by Claude Code (AI agent) --- not written by a human._`) +> --- please hold off until I'm done." comment on the issue > (`gh issue comment --body "..."`, or the MCP > `mcp__github__add_issue_comment` equivalent in a remote session). +> End the body with the agent-disclosure marker every agent-posted comment carries: `_Posted by Claude Code (AI agent) --- not written by a human._` > 2. **Check history** — before writing code, scan merged/closed PRs that > touched the same area so you don't undo past work or reintroduce a fixed > bug (`gh pr list --state all --search ""`). If a past PR already diff --git a/skills/post-merge/SKILL.md b/skills/post-merge/SKILL.md index 6a892d22d..3978f6af1 100644 --- a/skills/post-merge/SKILL.md +++ b/skills/post-merge/SKILL.md @@ -313,21 +313,15 @@ conflicting PR can sit in `UNKNOWN` and get missed if you filter for and "A stacked PR is the one conflict that intersection cannot attribute". 3. **Check claim status.** Read the most recent comment. - If it says "Working on this --- please hold off" (or the pre-2026-08-24 - wording "paws off", or any equivalent claim) and the claim is - still live --- a push or comment within the last 2 hours --- - skip it --- another session owns it. - An expired claim (over 2 idle hours) no longer blocks; take over with a - fresh claim comment of your own, per - [`claim-pr`](../../shared/workflow/claim-pr.md)'s expiration rule. + If it says "Working on this --- please hold off" (or the pre-2026-08-24 wording "paws off", or any equivalent claim) and the claim is still live --- a push or comment within the last 2 hours --- skip it --- another session owns it. + An expired claim (over 2 idle hours) no longer blocks; take over with a fresh claim comment of your own, per [`claim-pr`](../../shared/workflow/claim-pr.md)'s expiration rule. 4. **Claim it.** ```bash gh pr comment --body "Working on this — please hold off on pushing to this branch until I'm done. _Posted by Claude Code (AI agent) --- not written by a human._" # COMMENT_PR ``` -5. **Create an isolated worktree**, fetch the latest `main` (the squash-merge - commit that caused the conflict), and merge: +5. **Create an isolated worktree**, fetch the latest `main` (the squash-merge commit that caused the conflict), and merge: ```bash git fetch origin main # FETCH — fetch both: we need the new main tip git worktree add .claude/worktrees/pr- origin/ @@ -362,11 +356,9 @@ conflicting PR can sit in `UNKNOWN` and get missed if you filter for _Posted by Claude Code (AI agent) --- not written by a human._" # COMMENT_PR ``` -Resolve PRs one at a time — not because worktrees race each other (each -worktree is an independent checkout), but because the same human or bot may be -actively working a PR between your claim and your push. One-at-a-time keeps -the blast radius small. Skip any PR whose conflict is in a file you can't -understand without more context — comment asking for clarification instead. +Resolve PRs one at a time — not because worktrees race each other (each worktree is an independent checkout), but because the same human or bot may be actively working a PR between your claim and your push. +One-at-a-time keeps the blast radius small. +Skip any PR whose conflict is in a file you can't understand without more context — comment asking for clarification instead. **Match the response to standing, not only to cause.** Step 2 says whether a conflict is yours; it does not say the branch is. diff --git a/skills/pr-on-claim/SKILL.md b/skills/pr-on-claim/SKILL.md index bbdde977a..1c7e9d94d 100644 --- a/skills/pr-on-claim/SKILL.md +++ b/skills/pr-on-claim/SKILL.md @@ -28,9 +28,8 @@ Operationalizes the strong form of the claim workflow: branch → empty commit 4. Open a **draft PR** with: - Title: issue title (or override) - Body: `"Closes #\n\nWIP — opened up front to claim the issue; implementing now."` -5. Post a claim comment on the issue: `"Claude Code CLI (local session) is working on this — please hold off until I'm done.\n\n_Posted by Claude Code (AI agent) --- not written by a human._"` - (the trailing marker is required on every agent-posted comment --- see - [`disclose-agent-authorship`](../../shared/workflow/disclose-agent-authorship.md)) +5. Post a claim comment on the issue, whose body is the claim line, a blank line, and the agent-disclosure marker every agent-posted comment carries (see [`disclose-agent-authorship`](../../shared/workflow/disclose-agent-authorship.md)). + `\n` here is notation for a real newline, as in step 4 --- inside a bash double-quoted string those two characters stay two characters, so [`pr-on-claim.sh`](pr-on-claim.sh) writes the body with actual line breaks: `"Claude Code CLI (local session) is working on this — please hold off until I'm done.\n\n_Posted by Claude Code (AI agent) --- not written by a human._"` ## Why draft? diff --git a/skills/push/SKILL.md b/skills/push/SKILL.md index d99d1a8fd..afbbdd0c2 100644 --- a/skills/push/SKILL.md +++ b/skills/push/SKILL.md @@ -101,21 +101,12 @@ gh pr view "$PR" --json comments \ -q '.comments[] | select(.body | test("hold off|paws off"; "i")) | "\(.author.login): \(.body)"' # READ_PR_COMMENTS ``` -The alternation is deliberate: claims posted before 2026-08-24 say "paws off", -and a claim stays live on activity rather than on age, so an old-wording claim -can be live right now. -A matcher narrowed to the new phrase returns nothing on such a thread, which -reads exactly like an unclaimed one --- see -[`claim-pr`](../../shared/workflow/claim-pr.md). - -If the latest claim comment is from someone **other than you**, hasn't -been unclaimed, and is still live --- the PR shows a push or comment within -the last 2 hours, per -[`claim-pr`](../../shared/workflow/claim-pr.md)'s expiration rule --- -**do not push.** Ask the user. -An expired claim (over 2 idle hours) no longer blocks on its own, but take it -over with a fresh claim comment and run this skill's other checks (branch-head -advance, `@claude` run in flight) before pushing. +The alternation is deliberate: claims posted before 2026-08-24 say "paws off", and a claim stays live on activity rather than on age, so an old-wording claim can be live right now. +A matcher narrowed to the new phrase returns nothing on such a thread, which reads exactly like an unclaimed one --- see [`claim-pr`](../../shared/workflow/claim-pr.md). + +If the latest claim comment is from someone **other than you**, hasn't been unclaimed, and is still live --- the PR shows a push or comment within the last 2 hours, per [`claim-pr`](../../shared/workflow/claim-pr.md)'s expiration rule --- **do not push.** +Ask the user. +An expired claim (over 2 idle hours) no longer blocks on its own, but take it over with a fresh claim comment and run this skill's other checks (branch-head advance, `@claude` run in flight) before pushing. ### 4. Hold / block labels @@ -197,14 +188,13 @@ review, not a draft). ## Relationship to other skills -- **`claim-pr`** — posts/removes the claim comment this skill reads - in check #3. `push` is the read side; `claim-pr` is the write side. +- **`claim-pr`** — posts/removes the claim comment this skill reads in check #3. + `push` is the read side; `claim-pr` is the write side. - **`session-lock` / `deconflict-sessions`** — the local-checkout counterpart: it keeps parallel sessions on one machine from sharing a working tree. `push` guards the *remote* branch; `session-lock` guards the *local* tree. -- **`sync-pr-branch` / `merge-main`** — when check #2 fires because `main` (not - the branch) moved ahead, sync the branch first, then push. `sync-pr-branch` - ends in a push and should itself honor these checks. +- **`sync-pr-branch` / `merge-main`** — when check #2 fires because `main` (not the branch) moved ahead, sync the branch first, then push. + `sync-pr-branch` ends in a push and should itself honor these checks. - **`ardi`** — its push step should run these checks; the "detect an active parallel session before pushing" note in `claim-pr` is the same guard. - **[`check-before-pushing`](../../shared/workflow/check-before-pushing.md)** --- the standing rule these checks implement, and the home of the immediacy argument and the `--force-if-includes` mechanism. From cd3b626e1d149ffde8c5baed20ea533aa09bc000 Mon Sep 17 00:00:00 2001 From: Douglas Ezra Morrison Date: Mon, 24 Aug 2026 10:28:20 -0700 Subject: [PATCH 04/13] fix(disclosure): address second-round adversarial review findings Twelve findings on c41f0d34. All addressed except one rebutted. Guard was silent or wrong where it mattered most - Registered under matcher `Bash` only, so `MCP_POST_TOOLS` and `verdict_mcp()` were unreachable at runtime -- dead code in exactly the population they were written for, since a remote/web session has no `gh` at all. hooks.json matches by tool name, so the fix is a second entry under `mcp__github__.*`. Three statements asserting MCP coverage were false until it landed. - A heredoc body anywhere in a call vouched for every segment, so writing a doc that QUOTES the marker silenced a bare comment posted in the same call -- the exact per-segment property the notes claimed. Heredoc bodies are now replaced by position-keyed placeholders, so each body reaches only the segment that opened it. `strip_heredocs` also erased `<<`, which had been the only signal a segment used one. - `gh api .../comments` matched with no method or field discriminator, so the review-READ that CLAUDE.md prescribes and every ARDI round runs warned. So did `gh pr review --approve`, which posts no prose. Both now require a body field. A guard that cries wolf on the common read is one nobody reads on the rare write. - The bot exemption anchored to end-of-segment, so BOTH corpus sites failed: chores' commands carry a trailing `# COMMENT_PR`. Acting on that warning would mean appending prose to a body Dependabot parses. - Missed `glab api .../notes` and `gh api graphql addDiscussionComment` entirely. Sites and claims - Review-thread replies (both forges), discussion comments, and migrate-discussion's step 3 carried no marker though the fragment names them in scope; two were invisible to the guard as well. - release-pr enumerated 3 of the 5 release markers the corpus posts, so an ARDI release (`Done --- PR is free.`) read as unreleased and would draw a stray release comment. It now lists all five and gives the query to re-derive them. - push's claim query could no longer see a release at all: the old wording made it free, since `paws off released` contains `paws off`, while `claim released` contains no claim term. A released PR read as live-claimed and the skill would refuse a legitimate push. - The orchestrator posted `--` where the canonical marker has `---`, so a literal grep found every site but that one. - The exemption's "derive it with this grep" could not find the third site: the review handle is deliberately never spelled contiguously in a source file. The fragment now says so and gives both queries. - Two test labels claimed coverage their assertions did not provide. Measured rather than asserted - The MD049 claim was wrong. Re-measured repo-wide (the CI condition, not a single-file lint, which reports differently -- verify-the-right-artifact): the dedent raises MD049 in post-merge, MD046 in release-pr, and nothing in st. Worse, the marker is underscore-emphasised, so one that escapes its fence sets MD049's `consistent` expectation and fails untouched lines elsewhere -- which is what my own migrate-discussion edit did, breaking two lines 70 below it. Marker indentation now tracks the enclosing fence. - Reviewer reproducers for findings 2, 3, 5 and 8 are now permanent test cases. 61 hook tests, up from 52. Rebutted: em dashes on 26 added lines (finding 12). Every one is in a file that already carries 8-43 of them, and the three new files this PR adds carry none. Converting only mine would make those files internally inconsistent. The corpus-wide condition is already tracked as #685, #731 and #735; a fourth issue would be a duplicate. Checks: 41/41 hook suites (61 in this one), markdownlint 0, links, skills, hook-catalog, hook-output-shape, context-closure pin. Merge-tree against the 7 commits main gained mid-session is clean, verified against a negative control that returns rc=1 on a real conflict. --- commands/release-pr.md | 10 ++- hooks/hooks.json | 18 +++-- hooks/require-agent-disclosure.py | 85 ++++++++++++++++---- hooks/test-require-agent-disclosure.py | 42 +++++++++- scripts/orchestrator/pr_claim_manager.py | 2 +- shared/workflow/disclose-agent-authorship.md | 20 ++++- skills/ard/SKILL.md | 4 + skills/discussions/SKILL.md | 8 +- skills/migrate-discussion/SKILL.md | 4 +- skills/push/SKILL.md | 8 +- 10 files changed, 170 insertions(+), 31 deletions(-) diff --git a/commands/release-pr.md b/commands/release-pr.md index 6649e1b83..ad5f160b1 100644 --- a/commands/release-pr.md +++ b/commands/release-pr.md @@ -36,7 +36,15 @@ If only one positional arg is given, treat it as `pr_number`. **Match the two-word invariant `hold off` (case-insensitively), never a full sentence** — the PR claim reads `please hold off on pushing to this branch until I'm done` while the issue claim reads `please hold off until I'm done`, so neither sentence is a substring of the other and a matcher keyed on either one misses the other. **Also match the pre-2026-08-24 invariant `paws off`**: claims posted before that date are still live on open PRs, since a claim expires on activity rather than on age, and a matcher narrowed to the new wording returns nothing on them — indistinguishable from no claim at all. So the claim test is `test("hold off|paws off"; "i")`. - Treat **any** of these as a release marker: this command's `… done — claim released.`, its pre-2026-08-24 form `… done — paws off released.`, **or** the existing `claim-pr` skill's `Done with my local session — unclaiming.`, + Treat **any** of these as a release marker --- the corpus posts five, and enumerating only this command's own is what makes it post a stray release over somebody else's completed handover: + - this command's `… done — claim released.` + - its pre-2026-08-24 form `… done — paws off released.` + - `claim-pr`'s `Done with my local session — unclaiming.` + - `ardi`'s on-clean unclaim, `Done --- PR is free.` + - `post-merge`'s conflict unclaim, `Conflict resolved — branch is now mergeable. …` + + Derive that list rather than trusting this one, since a skill may add a + sixth: `grep -rn "unclaim\|released\|PR is free\|now mergeable" skills/ commands/`. - and that claim's `lane` matches the lane we're releasing. If the most recent signal is already a release, or the claim was by a different lane, stop and tell the user — don't post a stray release that misrepresents who was holding the PR. diff --git a/hooks/hooks.json b/hooks/hooks.json index 8eea1b7fe..460dc64ff 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -340,7 +340,7 @@ "command": "python3 \"${CLAUDE_PLUGIN_ROOT}/hooks/require-agent-disclosure.py\"", "timeout": 10, "script": "require-agent-disclosure.py", - "why": "cai 2026-08-24 'all comments online posted by bots should say so', citing UCD-SERG/ucd-serg.github.io#108's agent-authored claim comment posted under d-morrison as type: User, author_association MEMBER -- indistinguishable from a human's. memories/github.md already recorded auditors making that mistake. shared/workflow/disclose-agent-authorship.md is the rule; this is the instrument, since the rule is read at read time and the omission happens at composition time. WARNS, never blocks: a missing marker is cheap to repair with a follow-up comment, while a blocked `gh pr comment` interrupts the one action that makes a claim visible to other sessions -- trading a labelling defect for a collision. Anchored at a command position with heredoc-bearing commands skipped, because this corpus quotes `gh pr comment` constantly; that near-miss is the failure require-gh-repo-flag.py shipped with. Fails open.", + "why": "cai 2026-08-24 'all comments online posted by bots should say so', citing UCD-SERG/ucd-serg.github.io#108's agent-authored claim comment posted under d-morrison as type: User, author_association MEMBER -- indistinguishable from a human's. memories/github.md already recorded auditors making that mistake. shared/workflow/disclose-agent-authorship.md is the rule; this is the instrument, since the rule is read at read time and the omission happens at composition time. WARNS, never blocks: a missing marker is cheap to repair with a follow-up comment, while a blocked `gh pr comment` interrupts the one action that makes a claim visible to other sessions -- trading a labelling defect for a collision. Anchored at a command position over text whose heredoc BODIES have been stripped -- the opener tail is kept, so piping a heredoc into `--body-file -` still fires while a doc-writing heredoc quoting `gh pr comment` does not. This corpus quotes that command constantly, and matching such a quote is the near-miss require-gh-repo-flag.py shipped with. Fails open.", "_note_marker_avoids_the_robot_emoji": [ "The disclosure marker is prose, NOT the robot emoji, and the hook", "warns separately when a body discloses with the emoji instead.", @@ -364,10 +364,11 @@ "via --body-file on every round of every PR -- so it states the marker", "requirement in its own text rather than relying on this guard.", "", - "Registered under Bash AND matched against the mcp__github__* comment", - "tools, because a remote/web session has no `gh` at all and MCP is its", - "only path there. A Bash-only guard would be silent exactly where the CLI", - "is absent." + "REGISTERED TWICE, under Bash and under mcp__github__.*, because", + "hooks.json matches by TOOL NAME: the MCP tool list inside the script is", + "unreachable from a Bash-only entry however complete it is. A remote/web", + "session has no `gh` at all, so a Bash-only registration would be silent", + "in exactly the population the MCP branch was written for." ], "_note_segmentation_is_quote_aware": [ "Segments are split on shell separators OUTSIDE quotes. A plain", @@ -465,6 +466,13 @@ "timeout": 10, "script": "warn-pr-create-without-dupe-check.py", "why": "Dual-registered with the Bash entry, following no-unauthorized-merge.py's precedent. tool-mappings.md maps CREATE_PR to mcp__github__create_pull_request, and CLAUDE.md mandates that substitute in remote sessions -- so registering only under Bash let a remote session discharge the guard via MCP reads while never being able to trip it via MCP creation." + }, + { + "type": "command", + "command": "python3 \"${CLAUDE_PLUGIN_ROOT}/hooks/require-agent-disclosure.py\"", + "timeout": 10, + "script": "require-agent-disclosure.py", + "why": "The MCP half of the Bash entry above. A remote/web session has no `gh` on PATH, so mcp__github__add_issue_comment and its siblings are the ONLY way it posts a comment -- and hooks.json matches by tool name, so the Bash entry never sees them. Same script, same warn-only behaviour; it reads tool_input.body directly rather than parsing a command line, and stays silent when no string body is present (some pull_request_review_write methods submit without one)." } ] } diff --git a/hooks/require-agent-disclosure.py b/hooks/require-agent-disclosure.py index 371f4a98d..7fa8a07e1 100755 --- a/hooks/require-agent-disclosure.py +++ b/hooks/require-agent-disclosure.py @@ -45,6 +45,9 @@ Covers the Bash CLI forms and the `mcp__github__*` comment tools, since a remote/web session has no `gh` at all and MCP is its only path there (`CLAUDE.md`, "Skills that call gh/glab: fall back to tool-mappings.md"). +That second half needs a SECOND registration: `hooks.json` matches hooks by +tool name, so an entry under `Bash` alone never reaches an MCP call however +many MCP tool names this file lists. Fails OPEN: any parse problem returns 0 with no output. """ @@ -62,13 +65,26 @@ _POST_CMDS = ( r"gh\s+pr\s+comment", r"gh\s+issue\s+comment", - r"gh\s+pr\s+review\b", + # A review needs a BODY flag to be a comment. `gh pr review 12 --approve` + # posts no prose at all, so there is nothing to disclose and warning on it + # spends the guard's credibility on a command it cannot be about. + r"gh\s+pr\s+review\b[^\n;|&]*(?:--body\b|--body-file\b|-b\s|-F\s)", # `glab ... comment` is a real alias of `... note`; both spellings ship. r"glab\s+mr\s+(?:note|comment)", r"glab\s+issue\s+(?:note|comment)", - # The raw-API form `memories/git.md` prescribes for bodies carrying - # backticks, and the only way to post a REVIEW-THREAD reply. - r"gh\s+api\s+[^\n;|&]*(?:/comments|/replies)", + # The raw-API forms `memories/git.md` prescribes for bodies carrying + # backticks, and the only route to a REVIEW-THREAD reply or a discussion + # comment. A body-supplying FIELD FLAG is required rather than just the + # path: `gh api .../comments` with no field is the review-READ that + # `CLAUDE.md`'s own re-check section prescribes and every ARDI round runs, + # and a guard that fires on the common read is one nobody reads on the + # rare write. + r"gh\s+api\s+[^\n;|&]*(?:/comments|/replies|/notes)" + r"[^\n;|&]*(?:-f|-F|--field|--raw-field)\s+body=", + r"gh\s+api\s+graphql[^\n;|&]*(?:addDiscussionComment|addComment)", + # GitLab's raw-API discussion-note form, the counterpart to the above. + r"glab\s+api\s+[^\n;|&]*/(?:notes|discussions)" + r"[^\n;|&]*(?:-f|--field|--raw-field)\s+body=", ) _ANCHOR = ( r"(?:^|[;&|\n({`]|\b(?:then|else|elif|do|if|while|until)\s|!\s*)\s*" @@ -137,9 +153,15 @@ # GitHub renders, and a diff view renders this file -- so spelling it here # would summon the bot from a source file. _BOT_HANDLES = "|".join(["dependabot", "renovate", "copilot", "cl" + "aude"]) +# The `\1` closing quote ends it -- NOT `$`. Anchoring to end-of-segment +# required `--body` to be the last flag, so the corpus's own two Dependabot +# sites both warned: `gh pr comment "$N" --repo "$REPO" --body "@dependabot +# rebase" # COMMENT_PR` has a trailing comment, and `--body "..." --repo o/r` +# has a trailing flag. Acting on that warning would mean appending prose to a +# body Dependabot parses, which is the harm the exemption exists to avoid. BOT_COMMAND_RE = re.compile( r"--(?:body|message)\s+([\"'])\s*@(?:" + _BOT_HANDLES + r")\b" - r"[ \w-]{0,40}\s*\1\s*$", + r"[ \w-]{0,40}\s*\1", re.IGNORECASE, ) @@ -210,24 +232,51 @@ def split_segments(text): return segments +# Placeholder standing in for a heredoc body. Carries no shell separator, so it +# survives segmentation intact and keeps its body attached to the ONE segment +# that opened it. +SLOT = "\x00HEREDOC{}\x00" +SLOT_RE = re.compile(r"\x00HEREDOC(\d+)\x00") + + def strip_heredocs(command): - """Remove heredoc BODIES, keeping the rest of the opener line. + """Remove heredoc BODIES, keeping the opener tail and a body placeholder. Only the body is prose. The opener line's tail is still shell and routinely carries the very command being looked for -- piping a heredoc into `--body-file -` is the idiomatic way to post a multi-line body. Same reasoning, and the same lesson, as `warn-pr-create-without-dupe-check.py`. + + Returns (stripped_text, bodies). A plain strip was not enough: the body a + segment consumes has to stay attached to THAT segment, or a doc-writing + heredoc quoting the marker vouches for a bare comment elsewhere in the call. + Dropping `<<` along with the body also erased the only signal that a segment + had a heredoc at all, so the placeholder carries it. """ - return HEREDOC_RE.sub(lambda m: m.group(2), command) + bodies = [] + + def take(m): + bodies.append(m.group(0)) + return m.group(2) + " " + SLOT.format(len(bodies) - 1) + return HEREDOC_RE.sub(take, command), bodies -def heredoc_bodies(command): - """The heredoc bodies, which may themselves be the comment body.""" - return "\n".join(m.group(0) for m in HEREDOC_RE.finditer(command)) + +def bodies_for(segment, bodies): + """The heredoc bodies this segment actually opened.""" + return "\n".join(bodies[int(i)] for i in SLOT_RE.findall(segment) + if int(i) < len(bodies)) def judge_segment(segment, extra): - """Return a warning for one command-position segment, or None.""" + """Return a warning for one command-position segment, or None. + + `extra` is supplied ONLY when this segment actually references a heredoc. + Passing every heredoc body to every segment let a doc-writing heredoc that + merely QUOTED the marker vouch for a bare comment posted later in the same + call -- which is exactly the per-segment property this function exists to + provide, defeated by the argument meant to support it. + """ text = segment + "\n" + extra if BOT_COMMAND_RE.search(segment): return None @@ -235,6 +284,13 @@ def judge_segment(segment, extra): return None if EMOJI_DISCLOSURE_RE.search(text): return EMOJI + # A heredoc body we actually READ settles it: the body is in hand and + # carries no marker, so this is a missing marker rather than an unseen one. + # `--body-file -` is unreadable BY FLAG and readable in fact when its stdin + # is the heredoc, and reporting "cannot read" over a body just read is the + # same misdiagnosis the `-F ` case produced. + if extra: + return MISSING if UNREADABLE_RE.search(segment) or not HAS_INLINE_BODY_RE.search(segment): return UNREADABLE return MISSING @@ -242,15 +298,16 @@ def judge_segment(segment, extra): def verdict_bash(command): """Return a warning string for a Bash command, or None.""" - stripped = strip_heredocs(command) + stripped, bodies = strip_heredocs(command) if not POST_RE.search(stripped): return None - extra = heredoc_bodies(command) warnings = [] for segment in split_segments(stripped): if not POST_RE.search("\n" + segment): continue - found = judge_segment(segment, extra) + # A heredoc body IS this segment's comment body only when this segment + # opened it. Elsewhere it is somebody else's prose. + found = judge_segment(segment, bodies_for(segment, bodies)) if found and found not in warnings: warnings.append(found) if not warnings: diff --git a/hooks/test-require-agent-disclosure.py b/hooks/test-require-agent-disclosure.py index d0f1dd468..9ca1e9ab3 100755 --- a/hooks/test-require-agent-disclosure.py +++ b/hooks/test-require-agent-disclosure.py @@ -102,11 +102,19 @@ ("heredoc IS the body, and discloses", 'gh pr comment 12 --body-file - <<\'EOF\'\nDone.\n\n' + MARKER + '\nEOF', False), + # `--body-file -` is genuinely unreadable-by-flag, and the heredoc makes the + # body visible anyway -- so this must report MISSING, not "cannot read". ("heredoc IS the body, and does not disclose", - 'gh pr comment 12 --body-file - <<\'EOF\'\nDone, undisclosed.\nEOF', True), + 'gh pr comment 12 --body-file - <<\'EOF\'\nDone, undisclosed.\nEOF', "missing"), ("a doc heredoc does not silence a real sibling command", 'cat > d.md <<\'EOF\'\ngh pr comment --body "x"\nEOF\ngh pr comment 2 --body "bare"', True), + # The fixture above proves only that the sibling is SEEN. This one proves + # the heredoc cannot vouch for it: the doc being written quotes the marker + # verbatim, which is the normal shape when editing this very corpus. + ("a heredoc quoting the marker does not vouch for a bare sibling", + 'cat > frag.md <<\'EOF\'\nEnd every body with:\n\n' + MARKER + + '\nEOF\ngh pr comment 2 --body "bare claim"', True), # --- the exemption is whole-body, not first-token (review finding 8) ------ ("a bot handle followed by prose for humans is NOT exempt", @@ -115,6 +123,32 @@ ("the review re-request is exempt", 'gh pr comment 12 --body "@' + 'claude review"', False), + # --- a READ is not a post; round-2 review finding 3 ---------------------- + ("gh api GET of comments is a read, not a post", + 'gh api repos/o/r/issues/12/comments --paginate | jq -s \'.\'', False), + ("gh pr review --approve posts no prose", + 'gh pr review 12 --approve', False), + ("gh pr review WITH a body is a post", + 'gh pr review 12 --request-changes --body-file /tmp/r.md', None), + + # --- forge-API comment routes; round-2 review finding 8 ------------------ + ("glab api discussion note", + 'glab api -X POST "projects/:id/merge_requests/5/discussions/9/notes" ' + '-f body="Addressed."', True), + ("gh api graphql addDiscussionComment", + "gh api graphql -f body='Moved.' -f query='mutation { " + "addDiscussionComment(input:{}) { comment { url } } }'", True), + ("gh api graphql addDiscussionComment WITH marker", + "gh api graphql -f body='Moved.\n\n" + MARKER + "' -f query='mutation { " + "addDiscussionComment(input:{}) { comment { url } } }'", False), + + # --- the exemption must survive a trailing token; round-2 finding 5 ------ + ("chores site verbatim, with its trailing comment", + 'gh pr comment "$N" --repo "$REPO" --body "@dependabot rebase" ' + '# COMMENT_PR', False), + ("bot body followed by another flag", + 'gh pr comment 12 --body "@dependabot rebase" --repo o/r', False), + # --- unreadable vs missing must not be confused (review finding 9) ------- ("gh pr comment -F is a body-file, reported unreadable", 'gh pr comment 12 -F /tmp/body.md', None), @@ -138,7 +172,11 @@ def run(): failed = 0 for label, command, expect in CASES: reason = guard.verdict(command) - if expect is None: + if expect == "missing": + ok = reason is not None and "no agent-disclosure marker" in reason + print(f"{'PASS' if ok else 'FAIL'}: {label} " + f"(reported missing={ok})") + elif expect is None: # Must warn, and specifically about a body it could not read -- # accusing a command of omitting a marker never seen is the # misdiagnosis review finding 9 named. diff --git a/scripts/orchestrator/pr_claim_manager.py b/scripts/orchestrator/pr_claim_manager.py index 8ca35e296..869a0e24d 100644 --- a/scripts/orchestrator/pr_claim_manager.py +++ b/scripts/orchestrator/pr_claim_manager.py @@ -153,7 +153,7 @@ def claim_issue_and_open_draft_pr( claim_body = ( f"Orchestrator worker is working on this via draft PR " f"#{result.get('pr_number')} -- please hold off until done." - "\n\n_Posted by Claude Code (AI agent) -- not written by a human._" + "\n\n_Posted by Claude Code (AI agent) --- not written by a human._" ) comment_cmd = ["gh", "issue", "comment", str(issue_number), "--body", claim_body] if effective_repo: diff --git a/shared/workflow/disclose-agent-authorship.md b/shared/workflow/disclose-agent-authorship.md index 57b1a9c40..97be313d3 100644 --- a/shared/workflow/disclose-agent-authorship.md +++ b/shared/workflow/disclose-agent-authorship.md @@ -31,7 +31,15 @@ A convention worth anything has to be checkable by one query rather than by read **Search for the marker as a substring, never as a whole line.** A body composed inside an indented code fence carries that fence's indentation into the posted comment, so the marker can arrive with leading spaces. -Dedenting the source to column 0 is not the fix: a column-0 line ends the enclosing list item, which closes the fence and turns the marker into prose --- markdownlint MD049 catches it, and the comment stops being shown as a command at all. +Dedenting the source to column 0 is not the fix: a column-0 line ends the enclosing list item, which closes the fence and turns the marker into prose, so the comment stops being shown as a command at all. +Whether markdownlint notices depends on the file --- measured 2026-08-24 with `markdownlint-cli2@0.22.1` in a repo-wide run under this repo's config, the same dedent raises `MD049` (emphasis-style) in `skills/post-merge/SKILL.md`, `MD046` (code-block-style) in `commands/release-pr.md`, and **nothing at all** in `skills/st/SKILL.md`. +Measure that repo-wide rather than by linting one file: passing a single path changes which globs and which config apply, and it reported `MD049` for all three here --- a different artifact answering a different question, per [`verify-the-right-artifact`](verify-the-right-artifact.md). +`MD049` defaults to `consistent`, so it fires only where the document already established asterisk emphasis --- which makes the linter a partial detector here rather than the check. + +**That `consistent` default cuts the other way too, and it is the sharper hazard.** +The marker is underscore-emphasised, so a marker that escapes its fence into prose becomes the document's FIRST emphasis and sets the expectation to underscore --- whereupon every pre-existing `*asterisk*` in that file starts failing. +Measured here: adding the marker to `skills/migrate-discussion/SKILL.md` at column 0 closed the fence and turned two untouched lines 70 lines below into `MD049` errors, which read as defects in prose nobody had edited. +So the linter does catch this file, and it reports the wrong lines. So the source keeps its indentation and the query drops its anchor. **The guard cannot see every body, and the gaps are worth knowing.** @@ -53,10 +61,16 @@ A one-line status comment is short and still has a human reader, so it carries t As of 2026-08-24 the exemption covers three sites: `skills/chores/SKILL.md`'s two Dependabot commands, and the review re-request `skills/ardi/SKILL.md` mandates, whose whole body is the reviewer's own `@`-mention. That third one is worth naming because the first draft of this rule missed it and asserted the other two were the only instances --- an enumeration of a population nobody had queried, which is [`metacognitive-monitoring`](metacognitive-monitoring.md)'s scope-claim failure. -Derive the set before restating it: + +**No single query derives that set, and pretending otherwise repeats the error one level up.** +A literal `--body "@...` grep finds the two Dependabot sites and cannot find the third: per [`memories/mention-triggers.md`](../../memories/mention-triggers.md), the reviewer's handle is deliberately never spelled contiguously in a source file, because a diff view renders the file and the mention gate is a raw substring test. +So the handle that most needs the exemption is the one no handle-based search can see. + +Run both, and read the second as the reason the first is incomplete: ```bash -grep -rn -- '--body "@\|--message "@' --include="*.md" --include="*.sh" . +grep -rn -- '--body "@' --include='*.md' --include='*.sh' . # literal handles +grep -rn 'review re-request\|re-request review\|trigger phrase' skills/ # the assembled one ``` - **Do:** omit the marker on a comment whose whole body is a command addressed to another bot. diff --git a/skills/ard/SKILL.md b/skills/ard/SKILL.md index 122a9b261..7fa4e8cbf 100644 --- a/skills/ard/SKILL.md +++ b/skills/ard/SKILL.md @@ -171,6 +171,8 @@ resolve via GraphQL (`RESOLVE_REVIEW_THREAD`): # /tmp/reply-.md silently writes to the wrong file) cat > "/tmp/reply-.md" <<'EOF' ✅ Addressed in ``. + +_Posted by Claude Code (AI agent) --- not written by a human._ EOF gh api "repos/{owner}/{repo}/pulls//comments" \ -F in_reply_to="" -F body="@/tmp/reply-.md" # REPLY_REVIEW_COMMENT @@ -191,6 +193,8 @@ In a remote/web session without `gh`, resolve `RESOLVE_REVIEW_THREAD` via ```bash cat > "/tmp/reply-.md" <<'EOF' Addressed in ``. + +_Posted by Claude Code (AI agent) --- not written by a human._ EOF glab api -X POST "projects/:id/merge_requests//discussions//notes" \ -F body="@/tmp/reply-.md" diff --git a/skills/discussions/SKILL.md b/skills/discussions/SKILL.md index 914421293..e56d2950e 100644 --- a/skills/discussions/SKILL.md +++ b/skills/discussions/SKILL.md @@ -181,7 +181,9 @@ Top-level comment on the discussion (uses the discussion `id` from step 3, `COMMENT_DISCUSSION`): ```bash -gh api graphql -f discussionId='' -f body='' -f query=' +gh api graphql -f discussionId='' -f body=' + +_Posted by Claude Code (AI agent) --- not written by a human._' -f query=' mutation($discussionId: ID!, $body: String!) { addDiscussionComment(input: {discussionId: $discussionId, body: $body}) { comment { id url } @@ -193,7 +195,9 @@ Threaded reply to a specific comment — add `replyToId` (the comment `id`, also `COMMENT_DISCUSSION`): ```bash -gh api graphql -f discussionId='' -f replyToId='' -f body='' -f query=' +gh api graphql -f discussionId='' -f replyToId='' -f body=' + +_Posted by Claude Code (AI agent) --- not written by a human._' -f query=' mutation($discussionId: ID!, $replyToId: ID!, $body: String!) { addDiscussionComment(input: {discussionId: $discussionId, replyToId: $replyToId, body: $body}) { comment { id url } diff --git a/skills/migrate-discussion/SKILL.md b/skills/migrate-discussion/SKILL.md index e6ea62949..b312816aa 100644 --- a/skills/migrate-discussion/SKILL.md +++ b/skills/migrate-discussion/SKILL.md @@ -87,7 +87,9 @@ this is outward-facing and hard to reverse. 3. Comment on the discussion pointing at the new issue, then close it: ```bash - gh api graphql -f discussionId='' -f body='Moved to to track the actionable work.' -f query=' + gh api graphql -f discussionId='' -f body='Moved to to track the actionable work. + + _Posted by Claude Code (AI agent) --- not written by a human._' -f query=' mutation($discussionId: ID!, $body: String!) { addDiscussionComment(input: {discussionId: $discussionId, body: $body}) { comment { url } diff --git a/skills/push/SKILL.md b/skills/push/SKILL.md index afbbdd0c2..14e2852d4 100644 --- a/skills/push/SKILL.md +++ b/skills/push/SKILL.md @@ -98,10 +98,14 @@ that's your claim.) ```bash PR=$(gh pr view --json number,headRefName -q .number 2>/dev/null) # VIEW_PR gh pr view "$PR" --json comments \ - -q '.comments[] | select(.body | test("hold off|paws off"; "i")) | "\(.author.login): \(.body)"' # READ_PR_COMMENTS + -q '.comments[] | select(.body | test("hold off|paws off|unclaim|released|PR is free"; "i")) | "\(.author.login): \(.body)"' # READ_PR_COMMENTS ``` -The alternation is deliberate: claims posted before 2026-08-24 say "paws off", and a claim stays live on activity rather than on age, so an old-wording claim can be live right now. +The alternation is deliberate, and it covers RELEASES as well as claims. +Claims posted before 2026-08-24 say "paws off", and a claim stays live on activity rather than on age, so an old-wording claim can be live right now. +The release terms matter because the old wording made them free: `paws off released` contains `paws off`, so one grep surfaced both sides of the exchange. +`claim released` contains neither claim term, so a claim-only query returns the claim and not its release --- and step 3 below asks whether the claim "hasn't been unclaimed", which that output cannot answer. +A released PR would read as live-claimed, and this skill would refuse a legitimate push. A matcher narrowed to the new phrase returns nothing on such a thread, which reads exactly like an unclaimed one --- see [`claim-pr`](../../shared/workflow/claim-pr.md). If the latest claim comment is from someone **other than you**, hasn't been unclaimed, and is still live --- the PR shows a push or comment within the last 2 hours, per [`claim-pr`](../../shared/workflow/claim-pr.md)'s expiration rule --- **do not push.** From fffc9aac79fc80cd538716ebca9f0f7ebada0777 Mon Sep 17 00:00:00 2001 From: Douglas Ezra Morrison Date: Mon, 24 Aug 2026 10:49:15 -0700 Subject: [PATCH 05/13] fix(disclosure): address third-round adversarial review findings Twelve findings on d8216c3a. All addressed. The guard matched nothing the corpus actually writes Two detectors added last round passed their own tests and fired on no real site, which is fixtures-are-not-evidence exactly: both fixtures were single-line inventions, and the WITH-marker one was vacuous -- removing the marker did not change its result, because the marker check never ran. - `[^\n;|&]*` cannot cross a backslash line-continuation, and every review-thread reply in skills/ard uses one. Both raw-API detectors were silent on the only sites they exist for. - Every GraphQL comment site puts `addDiscussionComment` several lines below `gh api graphql`, inside a quoted query that spans newlines. - The first repair -- skipping quoted spans atomically -- was worse: the path segment these detectors match (`/notes`, `/comments`) is INSIDE the quoted URL, so the quote-skipping alternative swallowed the thing being matched. Now two gap classes: `_GAP` admits a line continuation, `_GAP_ML` is bounded and non-greedy for an argument that genuinely spans lines. The fixtures are replaced with verbatim corpus command lines, and the WITH-marker case now flips when the marker is removed. Misdiagnoses, both directions - `-F body=` and `--raw-field body=` were accepted as posting routes but omitted from HAS_INLINE_BODY_RE, so a body in plain sight drew "cannot read" -- the weaker note, which invites no correction. - The variable-expansion clause covered `--body`/`--message` but not `-b`/`-m`, so a compliant `-b "$BODY"` was told its marker was missing. - `-F body="@file"` reported MISSING; the `@` sigil is routinely quoted. - EMOJI_DISCLOSURE_RE lacked word boundaries, so "CI regenerated" and "badge rewritten" matched and got told to swap markers rather than add one. - split_segments applied plain single-quote rules to ANSI-C `$'...'`, reading the quote as closed and splitting the body on its newlines. Detectors and sites - push's alternation missed post-merge's release wording, so a released PR read as live-claimed and push would refuse a legitimate push -- the exact outcome the prose beside it says the alternation prevents. - post-merge matched a whole claim sentence, which the rule added four files away forbids, and its quoted prefix used `---` while its own emitter four steps below posts an em-dash. - tool-mappings.md -- the registry CLAUDE.md designates the single source of truth, and the substitution point for the very remote sessions the MCP registration exists to cover -- had no marker in any template. False claims corrected - The markdownlint cause claim was wrong. markdownlint-cli2 UNIONS a command-line path with the config globs (verified: 573 files either way), so there is no single-file scope to get wrong. The real cause was my own measurement: it grepped rule codes from the whole run without filtering by filename, so a different broken file's MD049 was attributed to each file in turn. metacognitive-monitoring's cause check, gone unasked. - ard's "nothing will warn you" was false; the guard warns, weakly. - The marker's searchable invariant is `(ai agent)`, not the full line -- the literal full line cannot find a Codex- or Gemini-posted comment, returning nothing, indistinguishable from an undisclosed thread. Named in the fragment with the query. - A comment posted under a genuine BOT token needs no marker, since the forge discloses it there. The fragment's scope sentence did not carve that out. The test is the token, not the workflow, with the query to settle it. - release-pr said the corpus "posts five" release markers; one is retired. Checks: 41/41 hook suites (70 in this one, up from 61), markdownlint 0, links, skills, hook-catalog, hook-output-shape, context-closure pin, codex wrappers re-synced. --- commands/release-pr.md | 5 +- hooks/require-agent-disclosure.py | 62 +++++++++++++++++--- hooks/test-require-agent-disclosure.py | 36 ++++++++++++ shared/workflow/disclose-agent-authorship.md | 23 +++++++- skills/ard/SKILL.md | 4 +- skills/post-merge/SKILL.md | 5 +- skills/push/SKILL.md | 8 +-- 7 files changed, 124 insertions(+), 19 deletions(-) diff --git a/commands/release-pr.md b/commands/release-pr.md index ad5f160b1..2c5897131 100644 --- a/commands/release-pr.md +++ b/commands/release-pr.md @@ -36,15 +36,14 @@ If only one positional arg is given, treat it as `pr_number`. **Match the two-word invariant `hold off` (case-insensitively), never a full sentence** — the PR claim reads `please hold off on pushing to this branch until I'm done` while the issue claim reads `please hold off until I'm done`, so neither sentence is a substring of the other and a matcher keyed on either one misses the other. **Also match the pre-2026-08-24 invariant `paws off`**: claims posted before that date are still live on open PRs, since a claim expires on activity rather than on age, and a matcher narrowed to the new wording returns nothing on them — indistinguishable from no claim at all. So the claim test is `test("hold off|paws off"; "i")`. - Treat **any** of these as a release marker --- the corpus posts five, and enumerating only this command's own is what makes it post a stray release over somebody else's completed handover: + Treat **any** of these as a release marker --- four the corpus posts today plus one retired form still sitting on open PRs, and enumerating only this command's own is what makes it post a stray release over somebody else's completed handover: - this command's `… done — claim released.` - its pre-2026-08-24 form `… done — paws off released.` - `claim-pr`'s `Done with my local session — unclaiming.` - `ardi`'s on-clean unclaim, `Done --- PR is free.` - `post-merge`'s conflict unclaim, `Conflict resolved — branch is now mergeable. …` - Derive that list rather than trusting this one, since a skill may add a - sixth: `grep -rn "unclaim\|released\|PR is free\|now mergeable" skills/ commands/`. + Derive that list rather than trusting this one, since a skill may add a sixth: `grep -rn "unclaim\|released\|PR is free\|now mergeable" skills/ commands/`. - and that claim's `lane` matches the lane we're releasing. If the most recent signal is already a release, or the claim was by a different lane, stop and tell the user — don't post a stray release that misrepresents who was holding the PR. diff --git a/hooks/require-agent-disclosure.py b/hooks/require-agent-disclosure.py index 7fa8a07e1..35752eed4 100755 --- a/hooks/require-agent-disclosure.py +++ b/hooks/require-agent-disclosure.py @@ -62,6 +62,24 @@ # every reply and every doc that merely QUOTES the command, and this corpus # quotes it constantly (`shared/workflow/claim-pr.md` is nothing but such # quotes). +# The gap between two parts of ONE command. +# +# A plain `[^\n;|&]*` was wrong twice over: a backslash line-continuation lives +# inside a single command, and every GraphQL comment site in this corpus puts +# the mutation name several lines below `gh api graphql` -- so both raw-API +# detectors matched nothing the corpus actually writes while their single-line +# test fixtures passed. +# +# Skipping quoted spans atomically was the wrong repair: the path segment these +# detectors look for (`/notes`, `/comments`) sits INSIDE the quoted URL, so the +# quote-skipping alternative swallowed the very thing being matched. +_GAP = r"(?:[^\n;&|]|\\\n)*" + +# The multi-line variant, for an argument that genuinely spans lines: a GraphQL +# query is a single quoted string containing newlines. Bounded and non-greedy so +# it cannot run away across a whole script; `;` and `&` still end it. +_GAP_ML = r"[^;&]{0,400}?" + _POST_CMDS = ( r"gh\s+pr\s+comment", r"gh\s+issue\s+comment", @@ -79,12 +97,12 @@ # `CLAUDE.md`'s own re-check section prescribes and every ARDI round runs, # and a guard that fires on the common read is one nobody reads on the # rare write. - r"gh\s+api\s+[^\n;|&]*(?:/comments|/replies|/notes)" - r"[^\n;|&]*(?:-f|-F|--field|--raw-field)\s+body=", - r"gh\s+api\s+graphql[^\n;|&]*(?:addDiscussionComment|addComment)", + r"gh\s+api\s+" + _GAP + r"(?:/comments|/replies|/notes)" + + _GAP + r"(?:-f|-F|--field|--raw-field)\s+body=", + r"gh\s+api\s+graphql" + _GAP_ML + r"(?:addDiscussionComment|addComment)", # GitLab's raw-API discussion-note form, the counterpart to the above. - r"glab\s+api\s+[^\n;|&]*/(?:notes|discussions)" - r"[^\n;|&]*(?:-f|--field|--raw-field)\s+body=", + r"glab\s+api\s+" + _GAP + r"/(?:notes|discussions)" + + _GAP + r"(?:-f|-F|--field|--raw-field)\s+body=", ) _ANCHOR = ( r"(?:^|[;&|\n({`]|\b(?:then|else|elif|do|if|while|until)\s|!\s*)\s*" @@ -124,9 +142,14 @@ # disclosing with the wrong marker. A body merely MENTIONING the emoji # ("the robot badge broke") discloses nothing, and telling its author to swap # markers would replace the one applicable instruction with an inapplicable one. +# `\b` on both sides: without it "CI regenerated the snapshots" and "badge +# rewritten" matched, and because this branch returns before MISSING, a body +# that carried the emoji incidentally AND omitted the marker was told to swap +# markers rather than to add one -- the inapplicable advice this branch exists +# to avoid giving. +_ATTRIB = r"\b(?:posted|generated|written|authored)\b" EMOJI_DISCLOSURE_RE = re.compile( - ROBOT + r"[^\n]{0,80}(?:posted|generated|written|authored)" - r"|(?:posted|generated|written|authored)[^\n]{0,80}" + ROBOT, + ROBOT + r"[^\n]{0,80}" + _ATTRIB + r"|" + _ATTRIB + r"[^\n]{0,80}" + ROBOT, re.IGNORECASE, ) @@ -138,11 +161,23 @@ r"--body-file|--description-file|--editor\b|--web\b" r"|-F\s+body=@|(?/comments" \\\n' + ' -F in_reply_to="" -F body="@/tmp/reply-.md"', + None), + ("ard's GitLab discussion note, with its line continuation", + 'glab api -X POST "projects/:id/merge_requests//discussions//notes" \\\n' + ' -F body="@/tmp/reply-.md"', None), + ("discussions' multi-line addDiscussionComment, no marker", + "gh api graphql -f discussionId='' -f body='' -f query='\n" + " mutation($discussionId: ID!, $body: String!) {\n" + " addDiscussionComment(input: {discussionId: $discussionId, body: $body}) {\n" + " comment { id url }\n }\n }'", "missing"), + ("discussions' multi-line addDiscussionComment, WITH marker", + "gh api graphql -f discussionId='' -f body='\n\n" + MARKER + + "' -f query='\n mutation($discussionId: ID!, $body: String!) {\n" + " addDiscussionComment(input: {discussionId: $discussionId, body: $body}) {\n" + " comment { id url }\n }\n }'", False), + + # --- precision on ordinary corpus reads; round-3 findings 3, 4, 11 ------- + ("the review-verdict read CLAUDE.md prescribes", + "gh api repos/o/r/issues/12/comments --paginate | jq -s '.'", False), + ("an incidental robot emoji is a missing marker, not a wrong one", + 'gh pr comment 5 --body "\U0001f916 CI regenerated the snapshots."', + "missing"), + ("-b with an expanded variable is unreadable, like --body", + 'gh pr comment 5 -b "$BODY"', None), + ("--raw-field body= is inline and readable", + 'gh api repos/o/r/issues/12/comments --raw-field body="hi"', "missing"), + ("-F body=@file is a file reference, so unreadable", + 'gh api repos/o/r/issues/12/comments -F body="@/tmp/b.md"', None), + # --- unreadable vs missing must not be confused (review finding 9) ------- ("gh pr comment -F is a body-file, reported unreadable", 'gh pr comment 12 -F /tmp/body.md', None), diff --git a/shared/workflow/disclose-agent-authorship.md b/shared/workflow/disclose-agent-authorship.md index 97be313d3..626f1e3ca 100644 --- a/shared/workflow/disclose-agent-authorship.md +++ b/shared/workflow/disclose-agent-authorship.md @@ -24,6 +24,14 @@ Review comments are the case that already discloses, since a review body announc The comments that need this are the ones that read most like a person: a claim, a release, a status update, a reply on a review thread, an issue filed on the user's behalf, a paraphrase of the user's own in-chat feedback. Each of those is short, conversational, and posted under a human login, which is exactly the shape that gets mistaken for a human. +**A comment posted under a genuine BOT identity needs no marker.** +The whole argument above is that the forge cannot report authorship when the token belongs to a person. +Where it belongs to an app --- a workflow posting as `github-actions[bot]`, so the API returns `type: Bot` and the UI shows a bot badge --- the forge already says it, and a marker adds nothing. +`skills/claude-agent-workflow/SKILL.md`'s in-workflow reply is that case. + +The test is the **token**, not the author: an agent driving `gh` with the account holder's PAT posts as a `User` however automated the surrounding workflow is. +Check `.user.type` on a posted comment if unsure --- `gh api repos///issues/comments/ --jq .user.type` --- rather than reasoning from how the comment was produced. + **A prose self-identification is not a substitute for the marker.** "Claude Code CLI (local session) is working on this" already discloses, so appending the footer to it looks redundant. Keep both. @@ -33,7 +41,11 @@ A convention worth anything has to be checkable by one query rather than by read A body composed inside an indented code fence carries that fence's indentation into the posted comment, so the marker can arrive with leading spaces. Dedenting the source to column 0 is not the fix: a column-0 line ends the enclosing list item, which closes the fence and turns the marker into prose, so the comment stops being shown as a command at all. Whether markdownlint notices depends on the file --- measured 2026-08-24 with `markdownlint-cli2@0.22.1` in a repo-wide run under this repo's config, the same dedent raises `MD049` (emphasis-style) in `skills/post-merge/SKILL.md`, `MD046` (code-block-style) in `commands/release-pr.md`, and **nothing at all** in `skills/st/SKILL.md`. -Measure that repo-wide rather than by linting one file: passing a single path changes which globs and which config apply, and it reported `MD049` for all three here --- a different artifact answering a different question, per [`verify-the-right-artifact`](verify-the-right-artifact.md). +Read the per-file lines, not the run's whole output. +A first pass at this measurement dedented one file at a time and grepped the rule codes out of the entire run, which reported `MD049` for all three --- because a *different* file was broken at the time and contributed its own `MD049` to every run. +The cause was never the invocation. +`markdownlint-cli2` unions a command-line path with the config's globs, so `markdownlint-cli2 commands/release-pr.md` lints all 573 files under the same config as a bare run; there is no single-file scope to get wrong. +The mistake was attributing an error to whichever file the loop happened to be testing, which is [`metacognitive-monitoring`](metacognitive-monitoring.md)'s cause check going unasked. `MD049` defaults to `consistent`, so it fires only where the document already established asterisk emphasis --- which makes the linter a partial detector here rather than the check. **That `consistent` default cuts the other way too, and it is the sharper hazard.** @@ -42,6 +54,15 @@ Measured here: adding the marker to `skills/migrate-discussion/SKILL.md` at colu So the linter does catch this file, and it reports the wrong lines. So the source keeps its indentation and the query drops its anchor. +**The searchable invariant is `(AI agent)`, not the full line.** +`AGENTS.md` says to swap in your own agent's name and keep the rest verbatim, which leaves the literal full line unable to find a Codex- or Gemini-posted comment --- returning nothing, which is indistinguishable from an undisclosed thread. +That is the same failure this rule's sibling diagnoses for the claim wording, one artifact over. +`hooks/require-agent-disclosure.py` already encodes the invariant as `posted by .{0,40}\(ai agent\)`; match that, case-insensitively: + +```bash +grep -rn -i "posted by .*(ai agent)" . +``` + **The guard cannot see every body, and the gaps are worth knowing.** `hooks/require-agent-disclosure.py` reads the command text and the `mcp__github__*` comment tools, so it is silent on a body it cannot reach: a `--body-file`, an `--editor` session, an interpolated `$BODY`. It reports those as an **unreadable** body rather than as a missing marker, so its warning never asserts more than it observed --- but a `--body-file` comment that genuinely omits the marker draws only the weaker note. diff --git a/skills/ard/SKILL.md b/skills/ard/SKILL.md index 7fa4e8cbf..890fed912 100644 --- a/skills/ard/SKILL.md +++ b/skills/ard/SKILL.md @@ -119,7 +119,9 @@ _Posted by Claude Code (AI agent) --- not written by a human._ ``` This comment is posted under the account holder's own login on every round of every PR, so it is among the likeliest in the corpus to be mistaken for their own writing --- see [`disclose-agent-authorship`](../../shared/workflow/disclose-agent-authorship.md). -The body arrives via `--body-file`, which the disclosure guard cannot read, so nothing will warn you if it is missing. +The body arrives via `--body-file`, which the disclosure guard cannot read. +It still warns --- but only that the body is unreadable, never that the marker is absent, and that weaker note reads as a formality rather than a finding. +So this step states the requirement itself. **Keep the bot's trigger phrase out of the summary body.** The `issue_comment` trigger fires on the bare bot `@`-mention **anywhere** in a comment — even in a diff --git a/skills/post-merge/SKILL.md b/skills/post-merge/SKILL.md index 3978f6af1..7adca4f71 100644 --- a/skills/post-merge/SKILL.md +++ b/skills/post-merge/SKILL.md @@ -313,7 +313,10 @@ conflicting PR can sit in `UNKNOWN` and get missed if you filter for and "A stacked PR is the one conflict that intersection cannot attribute". 3. **Check claim status.** Read the most recent comment. - If it says "Working on this --- please hold off" (or the pre-2026-08-24 wording "paws off", or any equivalent claim) and the claim is still live --- a push or comment within the last 2 hours --- skip it --- another session owns it. + Match the two-word invariant, `hold off` or the pre-2026-08-24 `paws off`, case-insensitively --- never a whole sentence. + The PR and issue claims differ after those two words, and the dash between them is an em-dash in this file's own emitter four steps below, so a quoted prefix misses claims this very skill posts. + See [`claim-pr`](../../shared/workflow/claim-pr.md)'s "Match the two-word invariant". + If a live claim stands --- a push or comment within the last 2 hours --- skip the PR; another session owns it. An expired claim (over 2 idle hours) no longer blocks; take over with a fresh claim comment of your own, per [`claim-pr`](../../shared/workflow/claim-pr.md)'s expiration rule. 4. **Claim it.** ```bash diff --git a/skills/push/SKILL.md b/skills/push/SKILL.md index 14e2852d4..7db24f35d 100644 --- a/skills/push/SKILL.md +++ b/skills/push/SKILL.md @@ -98,7 +98,7 @@ that's your claim.) ```bash PR=$(gh pr view --json number,headRefName -q .number 2>/dev/null) # VIEW_PR gh pr view "$PR" --json comments \ - -q '.comments[] | select(.body | test("hold off|paws off|unclaim|released|PR is free"; "i")) | "\(.author.login): \(.body)"' # READ_PR_COMMENTS + -q '.comments[] | select(.body | test("hold off|paws off|unclaim|released|PR is free|now mergeable"; "i")) | "\(.author.login): \(.body)"' # READ_PR_COMMENTS ``` The alternation is deliberate, and it covers RELEASES as well as claims. @@ -106,6 +106,7 @@ Claims posted before 2026-08-24 say "paws off", and a claim stays live on activi The release terms matter because the old wording made them free: `paws off released` contains `paws off`, so one grep surfaced both sides of the exchange. `claim released` contains neither claim term, so a claim-only query returns the claim and not its release --- and step 3 below asks whether the claim "hasn't been unclaimed", which that output cannot answer. A released PR would read as live-claimed, and this skill would refuse a legitimate push. +Derive the release terms rather than copying this list, which is a snapshot of what the corpus posts today: `grep -rn "unclaim\|released\|PR is free\|now mergeable" skills/ commands/`. A matcher narrowed to the new phrase returns nothing on such a thread, which reads exactly like an unclaimed one --- see [`claim-pr`](../../shared/workflow/claim-pr.md). If the latest claim comment is from someone **other than you**, hasn't been unclaimed, and is still live --- the PR shows a push or comment within the last 2 hours, per [`claim-pr`](../../shared/workflow/claim-pr.md)'s expiration rule --- **do not push.** @@ -194,9 +195,8 @@ review, not a draft). - **`claim-pr`** — posts/removes the claim comment this skill reads in check #3. `push` is the read side; `claim-pr` is the write side. -- **`session-lock` / `deconflict-sessions`** — the local-checkout counterpart: - it keeps parallel sessions on one machine from sharing a working tree. `push` - guards the *remote* branch; `session-lock` guards the *local* tree. +- **`session-lock` / `deconflict-sessions`** — the local-checkout counterpart: it keeps parallel sessions on one machine from sharing a working tree. + `push` guards the *remote* branch; `session-lock` guards the *local* tree. - **`sync-pr-branch` / `merge-main`** — when check #2 fires because `main` (not the branch) moved ahead, sync the branch first, then push. `sync-pr-branch` ends in a push and should itself honor these checks. - **`ardi`** — its push step should run these checks; the "detect an active From a9ff133655a8b7f811a32b2d54c037f242c5f47f Mon Sep 17 00:00:00 2001 From: Douglas Ezra Morrison Date: Mon, 24 Aug 2026 11:12:18 -0700 Subject: [PATCH 06/13] fix(disclosure): address fourth-round adversarial review findings Eleven findings on fffc9aac. All addressed. The fix from round 3 introduced a bug the code it replaced did not have post-merge's claim check now matches the two-word invariant `paws off`, and the retired release marker is `... done --- paws off released.`, which contains it. The sentence matcher that preceded it did not collide. So a released PR read as claimed, the conflict sweep skipped it, and the conflict went unresolved with nothing reporting why. It now checks the same comment for a release term first. The raw-API detectors were still wrong, for a third distinct reason Rounds 2 and 3 each tried one regex spanning the gap between `gh api` and its comment path. `[^\n;|&]*` could not cross a line continuation. `[^;&]{0,400}?` put the COMMENT BODY inside the gap, so an ordinary semicolon in the prose, an ampersand, or a body over 400 characters made the detector silent -- on exactly the longer, human-looking replies the rule targets. And the round-3 fixture added to stop testing "single-line inventions" used a 12-character placeholder with no punctuation, so it passed on the one input that concealed it. Replaced with a two-part test over the whole segment, which the quote-aware splitter has already bounded to one command: an anchored `gh|glab api`, a comment-bearing target, and a body field or a GraphQL comment mutation. Order independence falls out, which also fixes `gh api -f body=... `. As a side effect the polynomial blowup the reviewer measured (7.47s on 8000 `/comments` substrings) is gone with the gap regexes -- 0.008s on the same input. Other misdiagnoses - `(?` shorthand and fired on any `-F`, so `gh api ... -F in_reply_to=5 -F body="..."` -- the corpus's own review-reply shape -- reported "cannot read" over a visible body. Narrowed to a bare token with no `=`. - verdict_mcp synthesized `--body ""` to reuse the shell-shaped exemption pattern, so a `"` inside the body closed the synthetic argument early and `@dependabot rebase" and a long note for the humans...` took the exemption. The MCP path now matches the raw body. The Bash path had a fixture against this shape; reconstructing syntax to reuse a matcher is what reopened it. False claims corrected - "there is no single-file scope to get wrong" is false: `--no-globs` is in markdownlint-cli2's own `--help` and gives exactly that (573 files versus 1, measured). The union half was right. Named the flag, since it removes the contamination the weaker remedy works around. - The `.user.type` check gave only the issues endpoint, which 404s on a review-thread comment -- a surface the fragment names in scope two paragraphs above. Both routes now given, with what the 404 means. - The scope list said "an issue filed on the user's behalf" while three other sites in this diff say issue BODIES stay unattributed. Narrowed to comments. - tool-mappings' COMMENT_DISCUSSION row said no MCP tool exists; `mcp__github__discussion_comment_write` does, and this hook covers it. - The MCP_POST_TOOLS comment cited tool-mappings as the source of its five-tool list; that file names four. Said so rather than implying derivation. - The README row described one of the three verdicts the hook emits, omitting the unreadable-body one, which is the commoner in this corpus. Checks: 41/41 hook suites (77 in this one, up from 70), markdownlint 0, links, skills, hook-catalog, hook-output-shape, context-closure pin, codex wrappers re-synced, and `bash -n` parses all 17 marker-carrying shell blocks. --- README.md | 2 +- hooks/require-agent-disclosure.py | 116 ++++++++++++------- hooks/test-require-agent-disclosure.py | 43 ++++++- shared/workflow/disclose-agent-authorship.md | 16 ++- skills/post-merge/SKILL.md | 5 + 5 files changed, 134 insertions(+), 48 deletions(-) diff --git a/README.md b/README.md index 217360b22..eb5041831 100644 --- a/README.md +++ b/README.md @@ -408,7 +408,7 @@ the rule is consulted when it is *read* and broken when a message is | `warn-dupe-check-chained-to-create.py` | `PreToolUse` (Bash) | warns, never blocks, when a tracker search and a create of the same object kind share one Bash call, so the check runs at the same instant as the action it gates and gates nothing. Detects one lexical shape only, which means its silence is evidence that two commands were not in one string and never that a dupe-check was consulted | | `no-push-without-self-review.py` | `PreToolUse` (Bash) | blocks `git push` unless a separate `adversarial-reviewer` subagent returned a clean verdict as its own call result AND that report's `Reviewed-Commit:` fingerprint matches the commits the push would ship (refspec resolved), or the push itself is prefixed with `ALLOW_UNREVIEWED_PUSH=1`; a verdict quoted anywhere else --- in another file, or in this guard's own denial --- does not count | | `flag-uncited-rebuttal.py` | `PreToolUse` (Bash) | warns, never blocks, when a PR/issue comment about to be posted disputes a finding whose most recently fetched citation named an external URL that no earlier `WebFetch`/`WebSearch` in the transcript touched -- ai-config#2070's wrong rebuttal, retracted two rounds later once the URL was finally fetched | -| `require-agent-disclosure.py` | `PreToolUse` (Bash) | warns, never blocks, when a `gh`/`glab` command posts a forge comment whose body carries no agent-disclosure marker -- a comment posted under the account holder's credentials reads as `type: User` and is indistinguishable from one they typed; also warns when a body discloses with the robot emoji, which `check-pr-fully-clean.py` matches as a review-body marker | +| `require-agent-disclosure.py` | `PreToolUse` (Bash, mcp__github__.*) | warns, never blocks, on a `gh`/`glab` command or MCP call that posts a forge comment without the agent-disclosure marker -- such a comment carries the account holder's own login and reads as `type: User`, indistinguishable from one they typed. Three verdicts, not one: the marker is missing, the body is somewhere the check cannot read (`--body-file`, `--editor`, `$BODY`) so it says so rather than accusing, or the body discloses with the robot emoji, which `check-pr-fully-clean.py` matches as a review-body marker | For agent-independent monitoring across all projects and sessions, install the user service after the hook files are installed: diff --git a/hooks/require-agent-disclosure.py b/hooks/require-agent-disclosure.py index 35752eed4..75a577815 100755 --- a/hooks/require-agent-disclosure.py +++ b/hooks/require-agent-disclosure.py @@ -62,54 +62,71 @@ # every reply and every doc that merely QUOTES the command, and this corpus # quotes it constantly (`shared/workflow/claim-pr.md` is nothing but such # quotes). -# The gap between two parts of ONE command. -# -# A plain `[^\n;|&]*` was wrong twice over: a backslash line-continuation lives -# inside a single command, and every GraphQL comment site in this corpus puts -# the mutation name several lines below `gh api graphql` -- so both raw-API -# detectors matched nothing the corpus actually writes while their single-line -# test fixtures passed. +# --- what counts as posting a comment --------------------------------------- # -# Skipping quoted spans atomically was the wrong repair: the path segment these -# detectors look for (`/notes`, `/comments`) sits INSIDE the quoted URL, so the -# quote-skipping alternative swallowed the very thing being matched. -_GAP = r"(?:[^\n;&|]|\\\n)*" - -# The multi-line variant, for an argument that genuinely spans lines: a GraphQL -# query is a single quoted string containing newlines. Bounded and non-greedy so -# it cannot run away across a whole script; `;` and `&` still end it. -_GAP_ML = r"[^;&]{0,400}?" +# ANCHORED at a command position -- start of string, after a separator, or after +# a shell keyword that introduces a command. Unanchored, this fires on every +# reply and every doc that merely QUOTES the command, and this corpus quotes it +# constantly (`shared/workflow/claim-pr.md` is nothing but such quotes). +_ANCHOR = ( + r"(?:^|[;&|\n({`]|\b(?:then|else|elif|do|if|while|until)\s|!\s*)\s*" + r"(?:[A-Za-z_][A-Za-z0-9_]*=\S*\s+)*" +) +# The named CLI verbs, where the command word alone settles it. _POST_CMDS = ( r"gh\s+pr\s+comment", r"gh\s+issue\s+comment", # A review needs a BODY flag to be a comment. `gh pr review 12 --approve` - # posts no prose at all, so there is nothing to disclose and warning on it - # spends the guard's credibility on a command it cannot be about. - r"gh\s+pr\s+review\b[^\n;|&]*(?:--body\b|--body-file\b|-b\s|-F\s)", + # posts no prose, so there is nothing to disclose and warning on it spends + # the guard's credibility on a command it cannot be about. + r"gh\s+pr\s+review\b(?=[^\n;|&]*(?:--body\b|--body-file\b|-b\s|-F\s))", # `glab ... comment` is a real alias of `... note`; both spellings ship. r"glab\s+mr\s+(?:note|comment)", r"glab\s+issue\s+(?:note|comment)", - # The raw-API forms `memories/git.md` prescribes for bodies carrying - # backticks, and the only route to a REVIEW-THREAD reply or a discussion - # comment. A body-supplying FIELD FLAG is required rather than just the - # path: `gh api .../comments` with no field is the review-READ that - # `CLAUDE.md`'s own re-check section prescribes and every ARDI round runs, - # and a guard that fires on the common read is one nobody reads on the - # rare write. - r"gh\s+api\s+" + _GAP + r"(?:/comments|/replies|/notes)" - + _GAP + r"(?:-f|-F|--field|--raw-field)\s+body=", - r"gh\s+api\s+graphql" + _GAP_ML + r"(?:addDiscussionComment|addComment)", - # GitLab's raw-API discussion-note form, the counterpart to the above. - r"glab\s+api\s+" + _GAP + r"/(?:notes|discussions)" - + _GAP + r"(?:-f|-F|--field|--raw-field)\s+body=", ) -_ANCHOR = ( - r"(?:^|[;&|\n({`]|\b(?:then|else|elif|do|if|while|until)\s|!\s*)\s*" - r"(?:[A-Za-z_][A-Za-z0-9_]*=\S*\s+)*" -) -POST_RE = re.compile(_ANCHOR + r"(?:" + "|".join(_POST_CMDS) + r")", - re.MULTILINE) +POST_RE = re.compile(_ANCHOR + r"(?:" + "|".join(_POST_CMDS) + r")", re.MULTILINE) + +# The raw-API routes need a TWO-PART test rather than one regex spanning the +# gap between their parts. +# +# A single pattern was tried twice and failed twice. `[^\n;|&]*` cannot cross the +# backslash line-continuation every review-thread reply in `skills/ard` uses. +# Widening it to `[^;&]{0,400}?` put the COMMENT BODY inside the gap, so an +# ordinary semicolon in the prose, or a body over 400 characters, made the +# detector silent -- on exactly the longer, more human-looking replies the rule +# is for. Both times the fixture was short and punctuation-free and passed. +# +# So test the parts independently over the whole segment, which the quote-aware +# splitter has already bounded to one command. Order-independent by +# construction, which also fixes `gh api -f body=... `. +API_CMD_RE = re.compile(_ANCHOR + r"(?:gh|glab)\s+api\b", re.MULTILINE) +# A body-supplying field is what separates a POST from the review-READ that +# `CLAUDE.md` prescribes and every ARDI round runs. +API_BODY_FIELD_RE = re.compile(r"(?:-f|-F|--field|--raw-field)\s+body=") +# The comment-bearing endpoints, and the GraphQL comment mutations. +API_COMMENT_TARGET_RE = re.compile( + r"/comments|/replies|/notes|/discussions" + r"|addDiscussionComment|addComment", re.IGNORECASE) + + +def is_api_post(segment): + """True when this segment posts a comment through a raw forge API.""" + if not API_CMD_RE.search("\n" + segment): + return False + if not API_COMMENT_TARGET_RE.search(segment): + return False + # A GraphQL comment mutation carries its body as a variable, so the + # `body=` field test would reject it; the mutation name is the evidence. + if re.search(r"addDiscussionComment|addComment", segment, re.IGNORECASE): + return True + return bool(API_BODY_FIELD_RE.search(segment)) + + +def is_post_segment(segment): + """True when this segment posts a forge comment by any route.""" + return bool(POST_RE.search("\n" + segment)) or is_api_post(segment) + # Segment boundaries, split QUOTE-AWARE. # @@ -159,7 +176,7 @@ # being assumed to cover the other. UNREADABLE_RE = re.compile( r"--body-file|--description-file|--editor\b|--web\b" - r"|-F\s+body=@|(?"` so it could reuse the +# shell-shaped pattern below; a `"` inside the body then closed that synthetic +# argument early, and `@dependabot rebase" and a long note for the humans ...` +# took the exemption. Reconstructing syntax to reuse a matcher is what reopened +# a hole the Bash path had a fixture against. +BOT_BODY_RE = re.compile( + r"^\s*@(?:" + _BOT_HANDLES + r")\b[ \w-]{0,40}\s*$", re.IGNORECASE) + + +# MCP comment-posting tools. `tool-mappings.md` names COMMENT_PR/COMMENT_ISSUE, +# REPLY_REVIEW_COMMENT and COMMENT_DISCUSSION, which is four of these; the +# pending-review tool has no registry row, so this list is wider than that file +# rather than derived from it. MCP_POST_TOOLS = ( "mcp__github__add_issue_comment", "mcp__github__add_comment_to_pending_review", @@ -343,11 +373,11 @@ def judge_segment(segment, extra): def verdict_bash(command): """Return a warning string for a Bash command, or None.""" stripped, bodies = strip_heredocs(command) - if not POST_RE.search(stripped): + if not any(is_post_segment(seg) for seg in split_segments(stripped)): return None warnings = [] for segment in split_segments(stripped): - if not POST_RE.search("\n" + segment): + if not is_post_segment(segment): continue # A heredoc body IS this segment's comment body only when this segment # opened it. Elsewhere it is somebody else's prose. @@ -368,7 +398,7 @@ def verdict_mcp(tool_name, tool_input): # `pull_request_review_write` submits without a body on some methods, # and a body we never saw is not a body we can judge. return None - if BOT_COMMAND_RE.search('--body "' + body.strip() + '"'): + if BOT_BODY_RE.match(body): return None if MARKER_RE.search(body): return None diff --git a/hooks/test-require-agent-disclosure.py b/hooks/test-require-agent-disclosure.py index 14278a20a..943a4afa6 100755 --- a/hooks/test-require-agent-disclosure.py +++ b/hooks/test-require-agent-disclosure.py @@ -18,6 +18,15 @@ MARKER = "_Posted by Claude Code (AI agent) --- not written by a human._" + +def GQL(body): + """The corpus's verbatim addDiscussionComment command, with `body` in it.""" + return ("gh api graphql -f discussionId='' -f body='" + body + + "' -f query='\n" + " mutation($discussionId: ID!, $body: String!) {\n" + " addDiscussionComment(input: {discussionId: $discussionId, " + "body: $body}) {\n comment { id url }\n }\n }'") + # (label, command, expect_warning) CASES = [ # --- must warn ----------------------------------------------------------- @@ -185,6 +194,31 @@ ("-F body=@file is a file reference, so unreadable", 'gh api repos/o/r/issues/12/comments -F body="@/tmp/b.md"', None), + # --- round-4: the body sits INSIDE what used to be the gap --------------- + # + # The round-3 GraphQL fixture used the 12-character placeholder + # ``, which has no `;`, no `&`, and fits any length bound -- so + # it passed on the one input that concealed the bug. These vary exactly the + # properties the old gap regex was sensitive to. + ("GraphQL body containing a semicolon", + GQL("Addressed; pushed."), "missing"), + ("GraphQL body containing an ampersand", + GQL("Fixed A & B."), "missing"), + ("GraphQL body longer than the old 400-char bound", + GQL("x" * 320), "missing"), + ("GraphQL long body WITH marker", + GQL("x" * 320 + "\n\n" + MARKER), False), + + # --- round-4: a typed -F field is not a body-file ------------------------ + ("a typed -F field does not hide a visible body", + 'gh api repos/o/r/pulls/1/comments -F in_reply_to=5 ' + '-F body="Addressed, undisclosed."', "missing"), + + # --- round-4: argument order must not decide it -------------------------- + ("gh api with the body flag before the path", + 'gh api -X POST -f body="Working on this." repos/o/r/issues/12/comments', + "missing"), + # --- unreadable vs missing must not be confused (review finding 9) ------- ("gh pr comment -F is a body-file, reported unreadable", 'gh pr comment 12 -F /tmp/body.md', None), @@ -306,6 +340,13 @@ def run(): "mcp__github__add_reply_to_pull_request_comment", "Addressed.", True), ("MCP bot-command body is exempt", "mcp__github__add_issue_comment", "@dependabot rebase", False), + # Round-4: verdict_mcp used to synthesize `--body ""` to reuse the + # shell-shaped pattern, so a quote INSIDE the body closed that synthetic + # argument early and faked the exemption. + ("MCP body with an embedded quote does not fake the exemption", + "mcp__github__add_issue_comment", + '@dependabot rebase" and a long note for the humans reading this', + True), ("a non-comment MCP tool is out of scope", "mcp__github__create_pull_request", "Closes #1", False), ): @@ -315,7 +356,7 @@ def run(): print(f"{'PASS' if ok else 'FAIL'}: {label} " f"(warned={got}, expected={expect})") - total = len(CASES) + 2 + len(INDIRECT_CASES) + 1 + 4 + 5 + total = len(CASES) + 2 + len(INDIRECT_CASES) + 1 + 4 + 6 print(f"\n{total - failed} passed, {failed} failed") return 1 if failed else 0 diff --git a/shared/workflow/disclose-agent-authorship.md b/shared/workflow/disclose-agent-authorship.md index 626f1e3ca..b8785bc6b 100644 --- a/shared/workflow/disclose-agent-authorship.md +++ b/shared/workflow/disclose-agent-authorship.md @@ -21,7 +21,8 @@ Check a replacement marker against both tuples before changing it. **The scope is every comment, not every review.** Review comments are the case that already discloses, since a review body announces its own agent. -The comments that need this are the ones that read most like a person: a claim, a release, a status update, a reply on a review thread, an issue filed on the user's behalf, a paraphrase of the user's own in-chat feedback. +The comments that need this are the ones that read most like a person: a claim, a release, a status update, a reply on a review thread, a comment filed on an issue on the user's behalf, a paraphrase of the user's own in-chat feedback. +An issue's own **body** is not a comment and stays unattributed, per [`defer-issue`](../../skills/defer-issue/SKILL.md) --- the scope line below says so, and reading this list as covering issue bodies is the contradiction it would create. Each of those is short, conversational, and posted under a human login, which is exactly the shape that gets mistaken for a human. **A comment posted under a genuine BOT identity needs no marker.** @@ -30,7 +31,14 @@ Where it belongs to an app --- a workflow posting as `github-actions[bot]`, so t `skills/claude-agent-workflow/SKILL.md`'s in-workflow reply is that case. The test is the **token**, not the author: an agent driving `gh` with the account holder's PAT posts as a `User` however automated the surrounding workflow is. -Check `.user.type` on a posted comment if unsure --- `gh api repos///issues/comments/ --jq .user.type` --- rather than reasoning from how the comment was produced. +Check `.user.type` on a posted comment if unsure, rather than reasoning from how the comment was produced --- and pick the endpoint by comment KIND, since an issue-or-PR comment and a review-thread comment share an id space but not a route: + +```bash +gh api repos///issues/comments/ --jq .user.type # PR or issue comment +gh api repos///pulls/comments/ --jq .user.type # review-thread reply +``` + +Asking the first about a review-thread comment returns `404 Not Found`, which reads as a missing comment rather than as the wrong route. **A prose self-identification is not a substitute for the marker.** "Claude Code CLI (local session) is working on this" already discloses, so appending the footer to it looks redundant. @@ -44,7 +52,9 @@ Whether markdownlint notices depends on the file --- measured 2026-08-24 with `m Read the per-file lines, not the run's whole output. A first pass at this measurement dedented one file at a time and grepped the rule codes out of the entire run, which reported `MD049` for all three --- because a *different* file was broken at the time and contributed its own `MD049` to every run. The cause was never the invocation. -`markdownlint-cli2` unions a command-line path with the config's globs, so `markdownlint-cli2 commands/release-pr.md` lints all 573 files under the same config as a bare run; there is no single-file scope to get wrong. +`markdownlint-cli2` unions a command-line path with the config's globs, so `markdownlint-cli2 commands/release-pr.md` lints all 573 files under the same config as a bare run --- which is why filtering the output by filename is what the loop needed. +Single-file scope does exist, and `--help` names it: `--no-globs` ignores the config's `globs` and lints the one path (573 files versus 1, measured 2026-08-24). +Reach for that rather than for a filter when isolating a file. The mistake was attributing an error to whichever file the loop happened to be testing, which is [`metacognitive-monitoring`](metacognitive-monitoring.md)'s cause check going unasked. `MD049` defaults to `consistent`, so it fires only where the document already established asterisk emphasis --- which makes the linter a partial detector here rather than the check. diff --git a/skills/post-merge/SKILL.md b/skills/post-merge/SKILL.md index 7adca4f71..4f10c7d68 100644 --- a/skills/post-merge/SKILL.md +++ b/skills/post-merge/SKILL.md @@ -316,6 +316,11 @@ conflicting PR can sit in `UNKNOWN` and get missed if you filter for Match the two-word invariant, `hold off` or the pre-2026-08-24 `paws off`, case-insensitively --- never a whole sentence. The PR and issue claims differ after those two words, and the dash between them is an em-dash in this file's own emitter four steps below, so a quoted prefix misses claims this very skill posts. See [`claim-pr`](../../shared/workflow/claim-pr.md)'s "Match the two-word invariant". + + **Then check the same comment for a release term, because one release marker contains a claim invariant.** + The retired release wording is `... done --- paws off released.`, which matches `paws off` --- so the invariant that fixes one bug introduces another, and this one fails the safe way round: a released PR reads as claimed, the sweep skips it, and the conflict is never resolved with nothing reporting why. + The sentence matcher this replaced did not collide, so the collision arrived with the fix. + Treat the comment as a release, not a claim, if it also matches `unclaim|released|PR is free|now mergeable`. If a live claim stands --- a push or comment within the last 2 hours --- skip the PR; another session owns it. An expired claim (over 2 idle hours) no longer blocks; take over with a fresh claim comment of your own, per [`claim-pr`](../../shared/workflow/claim-pr.md)'s expiration rule. 4. **Claim it.** From de50078f238444ca284d91a9cf90abc1e95dd593 Mon Sep 17 00:00:00 2001 From: Douglas Ezra Morrison Date: Mon, 24 Aug 2026 11:33:03 -0700 Subject: [PATCH 07/13] fix(disclosure): address fifth-round adversarial review findings Ten findings on a9ff1336. All addressed. A generated file, edited at the wrong end tool-mappings.md is generated from tool-mappings.yml, so the previous round's edits to it were silently reverted by the next sync -- AGENTS.md's "canonical sources vs generated output" rule, broken while adding rows to the registry CLAUDE.md calls the single source of truth. The marker note now lives in tool-mappings.yml and in the generator's own preamble, and survives a regen. The COMMENT_DISCUSSION row also said no MCP tool existed; mcp__github__discussion_comment_write does. A fix applied to one of two sites carrying the same defect This round's own docstring explains why `[^\n;|&]*` had to go: it cannot cross a backslash line-continuation. The identical bound survived in the `gh pr review` lookahead, so a review whose body flag sat on a continuation line -- the shape skills/ard uses everywhere -- went silent. The body-flag test now runs over the whole segment, where the continuation is already absorbed. A false premise, twice cited The fragment and the hook both claimed the review handle is "never spelled contiguously in a source file, because a diff view would summon the bot". False: 242 occurrences across this corpus's markdown, including AGENTS.md, CLAUDE.md and README.md, none of which has dispatched a run -- and memories/mention-triggers.md states the gate over comment, review and issue BODIES, not file contents. The real reason the command-shaped grep misses ardi's site is mundane: that site is prose. The passage now says so and gives the second query plainly. Six properties no test pinned Removing any of these passed 77/77: three of the five MCP tools, the /notes and /discussions targets, the env-var prefix in the anchor, and the ANSI-C `$'...'` branch a prior round added as a fix. All are now pinned, and all eight mutations are confirmed CAUGHT by re-running the suite against each mutant. The first /discussions fixture was itself masked -- its path also contained /comments -- so it uses GitLab's create-a-thread route, where /discussions is the only marker. Removed rather than fixed The GraphQL early return was justified by a body-as-variable case that does not occur here: every GraphQL comment site supplies `-f body='...'`, so the branch decided nothing while classifying any command that merely NAMED the mutation as a post. Deleting it passed the suite unchanged and removes a false positive. Smaller - `-F key=value` is a typed field, not a body-file; narrowed so a review reply's `-F in_reply_to=5` no longer downgrades a visible body to the weaker note. - `--body=`/`--message=` equals forms read as inline rather than unreadable. - claim-pr's canonical matcher carried the release collision its own consumers document and fix: `paws off released` matches `paws off`, so a released PR read as claimed in every reader that follows the fragment rather than post-merge. - "One exemption" was stated in three always-loaded places while the fragment defines two; the bot-identity one is now named in all of them. - The issue-body pointer cited a scope line that omitted issue bodies. - A duplicated eight-line comment block in the new hook. Checks: 41/41 hook suites (92 in this one, up from 77), markdownlint 0, links, skills, hook-catalog, hook-output-shape, context-closure pin, codex wrappers re-synced from the yml. --- AGENTS.md | 2 +- CLAUDE.md | 4 +- hooks/require-agent-disclosure.py | 56 +++++++++++++++----- hooks/test-require-agent-disclosure.py | 56 +++++++++++++++++++- scripts/sync-codex-skill-wrappers.py | 13 +++++ shared/workflow/claim-pr.md | 10 ++-- shared/workflow/disclose-agent-authorship.md | 23 ++++---- tool-mappings.md | 21 ++++++-- tool-mappings.yml | 10 ++-- 9 files changed, 157 insertions(+), 38 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index f5ae03807..a7479aea6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -192,7 +192,7 @@ Check any replacement marker against that script's `REVIEW_BODY_MARKERS` and `RE Scope: comment bodies, on every surface --- claims, releases, status notes, review replies, self-reviews, issue comments filed on the user's behalf. Not commit messages, not titles, not PR bodies, each of which has its own attribution convention. -One exemption: a comment another machine parses as a command (`@dependabot rebase`), where the test is the audience rather than the length. +Two exemptions: a comment another machine parses as a command (`@dependabot rebase`), where the test is the audience rather than the length; and a comment posted under a genuine bot token, where the forge already reports `type: Bot` and the marker adds nothing. - **Do:** append the marker to every agent-posted comment, including ones whose prose already identifies the session. - **Don't:** use the robot emoji in the marker, and don't read "the account holder knows an agent is running" as making the disclosure unnecessary --- the reader is whoever finds the thread later. diff --git a/CLAUDE.md b/CLAUDE.md index dea4d1df8..8494d2665 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -623,8 +623,8 @@ _Posted by Claude Code (AI agent) --- not written by a human._ The marker deliberately avoids the robot emoji, which `scripts/check-pr-fully-clean.py` matches as a `REVIEW_BODY_MARKERS` entry --- a disclosed claim comment would otherwise scan as a finding-free **review**, the false-clean failure [`fully-clean`](shared/workflow/fully-clean.md) already describes. Check any replacement marker against that tuple before adopting it. -The one exemption is a comment another machine parses as a command (`@dependabot rebase`); the test is the audience, not the brevity. -Comment bodies only --- not commit messages, titles, or PR bodies. +Two exemptions: a comment another machine parses as a command (`@dependabot rebase`), where the test is the audience rather than the brevity; and a comment posted under a genuine bot token, where the forge already reports `type: Bot`. +Comment bodies only --- not commit messages, titles, issue bodies, or PR bodies. - **Do:** append the marker to every claim, release, status, reply, and self-review comment, including ones whose prose already names the session. - **Don't:** use the robot emoji in it, or put it in a commit message or title. diff --git a/hooks/require-agent-disclosure.py b/hooks/require-agent-disclosure.py index 75a577815..b96d27b0f 100755 --- a/hooks/require-agent-disclosure.py +++ b/hooks/require-agent-disclosure.py @@ -80,7 +80,14 @@ # A review needs a BODY flag to be a comment. `gh pr review 12 --approve` # posts no prose, so there is nothing to disclose and warning on it spends # the guard's credibility on a command it cannot be about. - r"gh\s+pr\s+review\b(?=[^\n;|&]*(?:--body\b|--body-file\b|-b\s|-F\s))", + # NOT gated on a body flag by a lookahead here. The obvious spelling, + # `(?=[^\n;|&]*--body...)`, carries the exact bound this file's own + # `is_api_post` docstring diagnoses two blocks below: it cannot cross the + # backslash line-continuation that `skills/ard` uses for every long command, + # so a review whose body flag sits on a continuation line went SILENT. + # The body-flag test is applied per segment in `is_post_segment` instead, + # where the whole command is already in hand. + r"gh\s+pr\s+review\b", # `glab ... comment` is a real alias of `... note`; both spellings ship. r"glab\s+mr\s+(?:note|comment)", r"glab\s+issue\s+(?:note|comment)", @@ -116,16 +123,33 @@ def is_api_post(segment): return False if not API_COMMENT_TARGET_RE.search(segment): return False - # A GraphQL comment mutation carries its body as a variable, so the - # `body=` field test would reject it; the mutation name is the evidence. - if re.search(r"addDiscussionComment|addComment", segment, re.IGNORECASE): - return True + # No GraphQL exception. It was added on the theory that a comment mutation + # carries its body as a variable and so would fail the `body=` test -- but + # every GraphQL comment site in this corpus supplies `-f body='...'`, so the + # branch decided nothing while classifying any command that merely NAMED the + # mutation as a post: `gh api graphql --input payload.json # addDiscussion- + # Comment payload` drew a warning for a command posting nothing visible. return bool(API_BODY_FIELD_RE.search(segment)) +# A review with no body flag posts no prose (`gh pr review 12 --approve`), so +# there is nothing to disclose. Tested over the whole segment rather than in a +# lookahead, so a continuation line cannot hide the flag. +REVIEW_ONLY_RE = re.compile(_ANCHOR + r"gh\s+pr\s+review\b", re.MULTILINE) +ANY_BODY_FLAG_RE = re.compile( + r"--body\b|--body=|--body-file|--message\b|--message=|-b\s|-m\s|-F\s" + r"|(?:-f|-F|--field|--raw-field)\s+body=") + + def is_post_segment(segment): """True when this segment posts a forge comment by any route.""" - return bool(POST_RE.search("\n" + segment)) or is_api_post(segment) + if POST_RE.search("\n" + segment): + # `gh pr review` is the one named verb that may carry no body at all. + if (REVIEW_ONLY_RE.search("\n" + segment) + and not ANY_BODY_FLAG_RE.search(segment)): + return False + return True + return is_api_post(segment) # Segment boundaries, split QUOTE-AWARE. @@ -177,7 +201,7 @@ def is_post_segment(segment): UNREADABLE_RE = re.compile( r"--body-file|--description-file|--editor\b|--web\b" r"|-F\s+body=@|(? is a body-file, reported unreadable", 'gh pr comment 12 -F /tmp/body.md', None), @@ -349,6 +392,17 @@ def run(): True), ("a non-comment MCP tool is out of scope", "mcp__github__create_pull_request", "Closes #1", False), + # Round-5: three of the five MCP tools were unpinned -- removing any of + # them from MCP_POST_TOOLS passed the whole suite. + ("MCP pending-review comment", "mcp__github__add_comment_to_pending_review", + "Bare finding.", True), + ("MCP review write", "mcp__github__pull_request_review_write", + "Bare review body.", True), + ("MCP discussion comment", "mcp__github__discussion_comment_write", + "Bare discussion reply.", True), + ("MCP discussion comment WITH marker", + "mcp__github__discussion_comment_write", + "Reply.\n\n" + MARKER, False), ): got = guard.verdict_mcp(tool, {"body": body}) is not None ok = got == expect @@ -356,7 +410,7 @@ def run(): print(f"{'PASS' if ok else 'FAIL'}: {label} " f"(warned={got}, expected={expect})") - total = len(CASES) + 2 + len(INDIRECT_CASES) + 1 + 4 + 6 + total = len(CASES) + 2 + len(INDIRECT_CASES) + 1 + 4 + 10 print(f"\n{total - failed} passed, {failed} failed") return 1 if failed else 0 diff --git a/scripts/sync-codex-skill-wrappers.py b/scripts/sync-codex-skill-wrappers.py index b8b85eead..bdfc1b127 100755 --- a/scripts/sync-codex-skill-wrappers.py +++ b/scripts/sync-codex-skill-wrappers.py @@ -162,6 +162,19 @@ def reference_doc(mappings: dict) -> str: concrete tools — mostly `gh`/`git` commands. This registry maps each canonical operation to the equivalent GitHub MCP tool so any model can run a skill. +> [!IMPORTANT] +> **Every comment-posting operation below carries the agent-disclosure marker in +> its body**, on its own line after a blank line: +> `_Posted by Claude Code (AI agent) --- not written by a human._` +> +> This registry is the substitution point for remote/web sessions, which have no +> `gh` at all — so a marker-free template here is a marker-free comment there, in +> exactly the population that cannot fall back to the CLI examples in the skills. +> See [`disclose-agent-authorship`](shared/workflow/disclose-agent-authorship.md). +> Two exemptions: a body another machine parses as a command (`@dependabot +> rebase`), and a comment posted under a genuine bot token, where the forge +> already reports `type: Bot`. + ## How each model resolves an operation {chr(10).join(model_lines)} diff --git a/shared/workflow/claim-pr.md b/shared/workflow/claim-pr.md index b26779b1d..be91e61ea 100644 --- a/shared/workflow/claim-pr.md +++ b/shared/workflow/claim-pr.md @@ -114,7 +114,12 @@ gh pr view --json comments \ Keep the old alternative until no claim under the old wording can plausibly still be live --- which, given the 2-hour rule keys on activity and not on the comment's own age, means until every PR and issue open on 2026-08-24 has closed. Removing it is a deliberate later edit, not tidying to do in passing. -- **Do:** match `hold off|paws off` case-insensitively wherever a claim is read. +**Then check the same comment for a release term, because one release marker contains a claim invariant.** +The retired release wording is `... done --- paws off released.`, which matches `paws off` --- so the invariant that fixes the whole-sentence bug introduces a second one, and this one fails the quiet way round: a *released* PR reads as claimed, the reader backs off, and nothing reports why. +The sentence matcher this replaced did not collide, so the collision arrived with the fix. +Treat a comment as a release rather than a claim when it also matches `unclaim|released|PR is free|now mergeable`, and derive that list rather than copying it: `grep -rn "unclaim\|released\|PR is free\|now mergeable" skills/ commands/`. + +- **Do:** match `hold off|paws off` case-insensitively wherever a claim is read, then exclude the comment if it also carries a release term. - **Do:** treat the old alternative as load-bearing until the threads carrying it have closed. - **Don't:** read an empty claim search as an unclaimed thread without first confirming the matcher covers both wordings --- the two results are identical. - **Don't:** drop the back-compat alternative as part of an unrelated change. @@ -146,8 +151,7 @@ An identical tree plus identical parents means the two merges are the same merge - **Do:** compare trees and parents before deciding what a rejected push means. -- **Do:** discard your local merge with `git reset --hard origin/` - once both match. +- **Do:** discard your local merge with `git reset --hard origin/` once both match. - **Don't:** re-merge reflexively on a rejected push --- that is what produces the redundant merge commit. - **Don't:** force-push over the other session's commit. diff --git a/shared/workflow/disclose-agent-authorship.md b/shared/workflow/disclose-agent-authorship.md index b8785bc6b..e543a32cf 100644 --- a/shared/workflow/disclose-agent-authorship.md +++ b/shared/workflow/disclose-agent-authorship.md @@ -22,7 +22,7 @@ Check a replacement marker against both tuples before changing it. **The scope is every comment, not every review.** Review comments are the case that already discloses, since a review body announces its own agent. The comments that need this are the ones that read most like a person: a claim, a release, a status update, a reply on a review thread, a comment filed on an issue on the user's behalf, a paraphrase of the user's own in-chat feedback. -An issue's own **body** is not a comment and stays unattributed, per [`defer-issue`](../../skills/defer-issue/SKILL.md) --- the scope line below says so, and reading this list as covering issue bodies is the contradiction it would create. +An issue's own **body** is not a comment and stays unattributed, per [`defer-issue`](../../skills/defer-issue/SKILL.md), which is why this item says *comment* rather than *issue*. Each of those is short, conversational, and posted under a human login, which is exactly the shape that gets mistaken for a human. **A comment posted under a genuine BOT identity needs no marker.** @@ -79,11 +79,11 @@ It reports those as an **unreadable** body rather than as a missing marker, so i `skills/ard/SKILL.md`'s per-round summary is exactly that shape, which is why it states the requirement in its own text rather than relying on the guard. **Where the marker must NOT go: content that is not a comment.** -A commit message, a PR title, or an issue title has its own attribution conventions and its own consumers, and a trailing italic line in a commit message corrupts a changelog. +A commit message, a title of any kind, an issue **body**, or a PR body has its own attribution conventions and its own consumers, and a trailing italic line in a commit message corrupts a changelog. PR bodies already carry the harness's own generated-with footer. This rule governs comment bodies. -**One exemption, and it is narrow: a comment another MACHINE parses as a command.** +**The first of two exemptions, and it is narrow: a comment another MACHINE parses as a command.** `@dependabot rebase`, `@dependabot squash and merge`, and their equivalents are not addressed to a reader at all --- they are an API call wearing a comment, and the receiving bot parses the body. Appending prose to one risks changing what it parses, for no reader's benefit, since nobody mistakes `@dependabot rebase` for a human's considered opinion. @@ -93,15 +93,18 @@ A one-line status comment is short and still has a human reader, so it carries t As of 2026-08-24 the exemption covers three sites: `skills/chores/SKILL.md`'s two Dependabot commands, and the review re-request `skills/ardi/SKILL.md` mandates, whose whole body is the reviewer's own `@`-mention. That third one is worth naming because the first draft of this rule missed it and asserted the other two were the only instances --- an enumeration of a population nobody had queried, which is [`metacognitive-monitoring`](metacognitive-monitoring.md)'s scope-claim failure. -**No single query derives that set, and pretending otherwise repeats the error one level up.** -A literal `--body "@...` grep finds the two Dependabot sites and cannot find the third: per [`memories/mention-triggers.md`](../../memories/mention-triggers.md), the reviewer's handle is deliberately never spelled contiguously in a source file, because a diff view renders the file and the mention gate is a raw substring test. -So the handle that most needs the exemption is the one no handle-based search can see. +**No single query derives that set, so run two and say why the first is short.** +The literal `--body "@...` grep finds the two Dependabot sites and misses the third for a mundane reason: `ardi` states its review re-request in prose rather than in a `--body` command, so a command-shaped search cannot reach it. -Run both, and read the second as the reason the first is incomplete: +An earlier draft of this passage gave a different and wrong reason --- that the handle is never spelled contiguously in a source file, because a diff view would summon the bot. +That is false twice over. +The handle appears 242 times across this corpus's markdown (measured 2026-08-24), including in `AGENTS.md`, `CLAUDE.md` and `README.md`, and none of those has ever dispatched a run. +And [`memories/mention-triggers.md`](../../memories/mention-triggers.md) states the gate as `contains(github.event.comment.body, '@claude')`, over comment, review and issue bodies --- file contents are not among them. +The practice of not spelling it applies to text that becomes a comment, which is what that file scopes it to. ```bash -grep -rn -- '--body "@' --include='*.md' --include='*.sh' . # literal handles -grep -rn 'review re-request\|re-request review\|trigger phrase' skills/ # the assembled one +grep -rn -- '--body "@' --include='*.md' --include='*.sh' . # command-shaped sites +grep -rn '@claude' skills/ # prose sites ``` - **Do:** omit the marker on a comment whose whole body is a command addressed to another bot. @@ -113,6 +116,6 @@ grep -rn 'review re-request\|re-request review\|trigger phrase' skills/ # the a - **Don't:** use the robot emoji in the marker --- it is a review-body marker, and it converts every disclosed comment into a finding-free review item. - **Don't:** treat a comment posted under a human login as self-evidently agent-authored because the account holder knows an agent is running. The reader is whoever finds the thread later. -- **Don't:** put the marker in a commit message, a title, or a PR body. +- **Don't:** put the marker in a commit message, a title, an issue body, or a PR body. (Directive from the user, 2026-08-24: "all comments online posted by bots should say so", citing --- an agent-authored claim comment posted under `d-morrison`, `type: User`, `author_association: MEMBER`, reading exactly like a human's.) diff --git a/tool-mappings.md b/tool-mappings.md index c9923bd69..205fb5d91 100644 --- a/tool-mappings.md +++ b/tool-mappings.md @@ -10,6 +10,19 @@ The canonical skills under `skills/` are written for Claude Code and name concrete tools — mostly `gh`/`git` commands. This registry maps each canonical operation to the equivalent GitHub MCP tool so any model can run a skill. +> [!IMPORTANT] +> **Every comment-posting operation below carries the agent-disclosure marker in +> its body**, on its own line after a blank line: +> `_Posted by Claude Code (AI agent) --- not written by a human._` +> +> This registry is the substitution point for remote/web sessions, which have no +> `gh` at all — so a marker-free template here is a marker-free comment there, in +> exactly the population that cannot fall back to the CLI examples in the skills. +> See [`disclose-agent-authorship`](shared/workflow/disclose-agent-authorship.md). +> Two exemptions: a body another machine parses as a command (`@dependabot +> rebase`), and a comment posted under a genuine bot token, where the forge +> already reports `type: Bot`. + ## How each model resolves an operation - **Claude Code** — GitHub MCP tool in remote/web sessions; the CLI command locally. @@ -36,8 +49,8 @@ operation to the equivalent GitHub MCP tool so any model can run a skill. | `MERGE_PR` | Merge a pull request. | `gh pr merge ""` | `mcp__github__merge_pull_request` | | `MARK_PR_READY` | Flip a draft pull request to ready for review. | `gh pr ready ""` | `mcp__github__update_pull_request (draft=false)` | | `REOPEN_PR` | Reopen a closed pull request. | `gh pr reopen ""` | `mcp__github__update_pull_request (state=open)` | -| `COMMENT_PR` | Post a top-level comment on a pull request. | `gh pr comment "" --body "..."` | `mcp__github__add_issue_comment` | -| `REPLY_REVIEW_COMMENT` | Reply to an inline pull-request review comment. The path carries the PR number; the id-only route (`PATCH .../pulls/comments/`) EDITS that comment instead. | `gh api -X POST "repos///pulls//comments//replies" -F "body=@"` | `mcp__github__add_reply_to_pull_request_comment` | +| `COMMENT_PR` | Post a top-level comment on a pull request. **The body ends with the agent-disclosure marker** --- see [`disclose-agent-authorship`](shared/workflow/disclose-agent-authorship.md). | `gh pr comment "" --body "..."` | `mcp__github__add_issue_comment` | +| `REPLY_REVIEW_COMMENT` | Reply to an inline pull-request review comment. The path carries the PR number; the id-only route (`PATCH .../pulls/comments/`) EDITS that comment instead. **The body ends with the agent-disclosure marker** --- see [`disclose-agent-authorship`](shared/workflow/disclose-agent-authorship.md). | `gh api -X POST "repos///pulls//comments//replies" -F "body=@"` | `mcp__github__add_reply_to_pull_request_comment` | | `RESOLVE_REVIEW_THREAD` | Mark an inline pull-request review thread as resolved. | `gh api graphql -f query='mutation { resolveReviewThread(input:{threadId:""}) { thread { isResolved } } }'` | `mcp__github__resolve_review_thread` | | `WATCH_PR` | Subscribe to / unsubscribe from a pull request's activity. | (no CLI equivalent) | `mcp__github__subscribe_pr_activity / mcp__github__unsubscribe_pr_activity` | | `VIEW_ISSUE` | Read an issue's details. | `gh issue view ""` | `mcp__github__issue_read` | @@ -46,14 +59,14 @@ operation to the equivalent GitHub MCP tool so any model can run a skill. | `READ_ISSUE_COMMENTS` | Read an issue's comments. | `gh issue view "" --comments` | `mcp__github__issue_read (method=get_comments)` | | `ISSUE_LINKED_PRS` | List the pull requests cross-referenced from an issue's timeline (i.e. PRs that link or close it). | `gh api --paginate "repos///issues//timeline"` | (no GitHub MCP tool; approximate with SEARCH_PRS) | | `CREATE_ISSUE` | Open a new issue. | `gh issue create` | `mcp__github__issue_write (method=create)` | -| `COMMENT_ISSUE` | Post a comment on an issue. | `gh issue comment "" --body "..."` | `mcp__github__add_issue_comment` | +| `COMMENT_ISSUE` | Post a comment on an issue. **The body ends with the agent-disclosure marker** --- see [`disclose-agent-authorship`](shared/workflow/disclose-agent-authorship.md). | `gh issue comment "" --body "..."` | `mcp__github__add_issue_comment` | | `CLOSE_ISSUE` | Close an issue with a reason. | `gh issue close "" --reason "..."` | `mcp__github__issue_write (method=update, state=closed, state_reason=...)` | | `REOPEN_ISSUE` | Reopen a closed issue. | `gh issue reopen "" --comment "..."` | `mcp__github__issue_write (method=update, state=open)` | | `LABEL_ISSUE` | Set an issue's labels. The two behave differently and are not interchangeable: `--add-label` ADDS to the existing set, while the MCP path REPLACES the whole set, so pass the union of existing and new labels there. The MCP path also silently creates an unknown label name instead of rejecting it. | `gh issue edit "" --add-label "..."` | `mcp__github__issue_write (method=update, labels=[...])` | | `GET_LABEL` | Read a single label's name, color, and description. There is no MCP tool to create or update a label; use gh label create/edit, or gh api from a workflow. | `gh api "repos///labels/"` | `mcp__github__get_label` | | `LIST_DISCUSSIONS` | List a repository's discussions. Readable over REST; writes are GraphQL-only. | `gh api repos/{owner}/{repo}/discussions` | (no GitHub MCP tool; use gh api REST or graphql) | | `VIEW_DISCUSSION` | Read a discussion topic and its comment thread. Readable over REST. | `gh api repos/{owner}/{repo}/discussions/{number}[/comments]` | (no GitHub MCP tool; use gh api REST or graphql) | -| `COMMENT_DISCUSSION` | Post a reply on a discussion (top-level or threaded). | `gh api graphql (addDiscussionComment)` | (no GitHub MCP tool; use gh api graphql) | +| `COMMENT_DISCUSSION` | Post a reply on a discussion (top-level or threaded). **The body ends with the agent-disclosure marker** --- see [`disclose-agent-authorship`](shared/workflow/disclose-agent-authorship.md). | `gh api graphql (addDiscussionComment)` | `mcp__github__discussion_comment_write` | | `ANSWER_DISCUSSION` | Mark a comment as the accepted answer on a Q&A discussion. | `gh api graphql (markDiscussionCommentAsAnswer)` | (no GitHub MCP tool; use gh api graphql) | | `CREATE_DISCUSSION` | Open a new discussion in a category. | `gh api graphql (createDiscussion)` | (no GitHub MCP tool; use gh api graphql) | | `CLOSE_DISCUSSION` | Close a discussion with a reason (RESOLVED, OUTDATED, DUPLICATE). | `gh api graphql (closeDiscussion)` | (no GitHub MCP tool; use gh api graphql) | diff --git a/tool-mappings.yml b/tool-mappings.yml index 1297fd819..39a94e58f 100644 --- a/tool-mappings.yml +++ b/tool-mappings.yml @@ -107,11 +107,11 @@ operations: cli: gh pr reopen "" github_mcp: mcp__github__update_pull_request (state=open) - id: COMMENT_PR - description: Post a top-level comment on a pull request. + description: Post a top-level comment on a pull request. **The body ends with the agent-disclosure marker** --- see [`disclose-agent-authorship`](shared/workflow/disclose-agent-authorship.md). cli: gh pr comment "" --body "..." github_mcp: mcp__github__add_issue_comment - id: REPLY_REVIEW_COMMENT - description: Reply to an inline pull-request review comment. The path carries the PR number; the id-only route (`PATCH .../pulls/comments/`) EDITS that comment instead. + description: Reply to an inline pull-request review comment. The path carries the PR number; the id-only route (`PATCH .../pulls/comments/`) EDITS that comment instead. **The body ends with the agent-disclosure marker** --- see [`disclose-agent-authorship`](shared/workflow/disclose-agent-authorship.md). cli: gh api -X POST "repos///pulls//comments//replies" -F "body=@" github_mcp: mcp__github__add_reply_to_pull_request_comment - id: RESOLVE_REVIEW_THREAD @@ -147,7 +147,7 @@ operations: cli: gh issue create github_mcp: mcp__github__issue_write (method=create) - id: COMMENT_ISSUE - description: Post a comment on an issue. + description: Post a comment on an issue. **The body ends with the agent-disclosure marker** --- see [`disclose-agent-authorship`](shared/workflow/disclose-agent-authorship.md). cli: gh issue comment "" --body "..." github_mcp: mcp__github__add_issue_comment - id: CLOSE_ISSUE @@ -183,9 +183,9 @@ operations: cli: gh api repos/{owner}/{repo}/discussions/{number}[/comments] github_mcp: (no GitHub MCP tool; use gh api REST or graphql) - id: COMMENT_DISCUSSION - description: Post a reply on a discussion (top-level or threaded). + description: Post a reply on a discussion (top-level or threaded). **The body ends with the agent-disclosure marker** --- see [`disclose-agent-authorship`](shared/workflow/disclose-agent-authorship.md). cli: gh api graphql (addDiscussionComment) - github_mcp: (no GitHub MCP tool; use gh api graphql) + github_mcp: mcp__github__discussion_comment_write - id: ANSWER_DISCUSSION description: Mark a comment as the accepted answer on a Q&A discussion. cli: gh api graphql (markDiscussionCommentAsAnswer) From 781d2869aabe0b91d50459eff6283d5d40afb5ba Mon Sep 17 00:00:00 2001 From: Douglas Ezra Morrison Date: Mon, 24 Aug 2026 12:03:54 -0700 Subject: [PATCH 08/13] fix(disclosure): address sixth-round adversarial review findings Fourteen findings on de50078f. All addressed. This round's differential over all 804 fenced code blocks in the tree showed zero verdict changes from the previous head, so the guard's behaviour on what the corpus actually writes is stable; the findings are gaps rather than regressions. Invisible to the guard - The QUOTED whole-argument field form. `API_BODY_FIELD_RE` required `body=` to follow whitespace directly, so `-F "body=@"` -- the registry's own canonical reply command, and the line this change annotates -- was not merely weakened to the "cannot read" note but completely silent. The corpus writes the quoted spelling for sibling flags too. - `-F "in_reply_to=5"` still downgraded a visible body. The optional quote gave the engine a backtracking path: skip the quote, fail to find `key=` at `"`, satisfy the negation. Matched as a token with no `=` in it instead. Removed rather than kept - `/replies` is dead. GitHub's reply route is `.../comments//replies`, so it always contains `/comments` -- which is why two attempts at a fixture for it were masked by their own paths. An alternative no fixture can isolate implies a route that does not exist. - Three `UNREADABLE_RE`/`ANY_BODY_FLAG_RE` alternatives were subsumed when `HAS_INLINE_BODY_RE` gained `[^\s"'$]`, and their comments described behaviour the code no longer had. Restored, with a real discriminator The GraphQL branch was deleted last round because "the branch decided nothing" -- a claim about the corpus, applied to a detector that sees whatever an agent writes. A mutation carrying its body in the query text or an `--input` file was silent. It is back, keyed on `mutation` beside the name, so executing one is distinguished from a comment mentioning one. The registry, again - COMMENT_DISCUSSION's new MCP mapping omitted the required `method`, on the one operation where the registry is the only path for a remote session. - Three sibling discussion rows were stale in exactly the way this round corrected a fourth: LIST_DISCUSSIONS, VIEW_DISCUSSION and ANSWER_DISCUSSION all said no MCP tool existed. Verified against the server's tool list, which is what the hook's own docstring says to do. Claim readers `gi` and `claim-pr`'s GitLab sweep were left on the claim invariant alone, against the rule written four files away -- so `... done --- paws off released.` read as a live claim and `gi` would skip a released issue permanently. All six claim readers now exclude release terms. Untested properties, and two that could not be tested Six more survived deletion with the suite green: the `systemMessage` key (which `check-hook-output-shape.py` checks for Stop hooks only), `MARKER_RE`'s attribution prefix, `strip_heredocs`'s opener tail, `_ANCHOR`'s `(`/`{`/backtick positions, `verdict_mcp`'s isinstance guard, and `/replies`. Five are now pinned and confirmed CAUGHT by mutation; `/replies` was removed instead. False and unsupported claims - The published derivation grep put `--include` after `--`, so both flags were read as file operands and the search ran unfiltered -- returning its own test fixtures as apparent sites, in the passage whose thesis is that one query does not derive the set. - "242 times" was a count from a tree predating the sentence, with no deriving command beside it. Re-derived at this commit and the command published. - The previous commit claimed to have removed a duplicated comment block; it had not. Removed now. - AGENTS.md's exclusion list omitted issue bodies while its inclusion list named issue comments -- and non-Claude agents read AGENTS.md instead of CLAUDE.md. Prose and budget - Em dashes on lines authored this round converted to ASCII, EXCEPT inside quoted literal comment bodies: ` done - claim released.` is the text that reaches GitHub, and changing it desynchronizes the corpus from the string its matchers agree on. - CLAUDE.md restated the whole mechanism beside the fragment carrying it; trimmed to the rule, the marker, and the emoji constraint (1516 -> 1171 bytes). AGENTS.md keeps the longer copy deliberately, since agents reading it never load the fragment. The always-loaded pool remains over its advisory budget, which predates this change. Checks: 41/41 hook suites (105 in this one, up from 92), markdownlint 0, links, skills, hook-catalog, hook-output-shape, context-closure pin, and tool-mappings.md is idempotent under a second regen. --- AGENTS.md | 2 +- CLAUDE.md | 13 ++-- commands/release-pr.md | 18 +++--- hooks/require-agent-disclosure.py | 59 +++++++++++-------- hooks/test-require-agent-disclosure.py | 62 +++++++++++++++++++- scripts/sync-codex-skill-wrappers.py | 2 +- shared/workflow/disclose-agent-authorship.md | 8 ++- skills/claim-pr/SKILL.md | 8 ++- skills/gi/SKILL.md | 4 +- tool-mappings.md | 10 ++-- tool-mappings.yml | 8 +-- 11 files changed, 136 insertions(+), 58 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index a7479aea6..f011bc70d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -191,7 +191,7 @@ The marker deliberately contains **no robot emoji**: [`scripts/check-pr-fully-cl Check any replacement marker against that script's `REVIEW_BODY_MARKERS` and `REVIEW_AGENT_MARKERS` before adopting it. Scope: comment bodies, on every surface --- claims, releases, status notes, review replies, self-reviews, issue comments filed on the user's behalf. -Not commit messages, not titles, not PR bodies, each of which has its own attribution convention. +Not commit messages, not titles, not issue bodies, not PR bodies, each of which has its own attribution convention. Two exemptions: a comment another machine parses as a command (`@dependabot rebase`), where the test is the audience rather than the length; and a comment posted under a genuine bot token, where the forge already reports `type: Bot` and the marker adds nothing. - **Do:** append the marker to every agent-posted comment, including ones whose prose already identifies the session. diff --git a/CLAUDE.md b/CLAUDE.md index 8494d2665..cfeee4586 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -614,21 +614,18 @@ The `claim-pr` skill operationalizes this (the exact claim wording, when it appl [shared/workflow/disclose-agent-authorship.md](shared/workflow/disclose-agent-authorship.md) -A comment posted through `gh`/`glab` under the account holder's credentials carries **their** login, avatar, and `MEMBER` association, and reads as `type: User` --- so nothing in the API distinguishes it from a comment they typed, and `memories/github.md` already records auditors mistaking exactly that. -The forge cannot say it, so the body has to: end every comment with +A comment posted through `gh`/`glab` under the account holder's credentials carries **their** login and reads as `type: User`, so nothing in the API distinguishes it from a comment they typed --- `memories/github.md` records auditors mistaking exactly that. +The forge cannot say it, so the body must: end every agent-posted comment with ``` _Posted by Claude Code (AI agent) --- not written by a human._ ``` -The marker deliberately avoids the robot emoji, which `scripts/check-pr-fully-clean.py` matches as a `REVIEW_BODY_MARKERS` entry --- a disclosed claim comment would otherwise scan as a finding-free **review**, the false-clean failure [`fully-clean`](shared/workflow/fully-clean.md) already describes. -Check any replacement marker against that tuple before adopting it. -Two exemptions: a comment another machine parses as a command (`@dependabot rebase`), where the test is the audience rather than the brevity; and a comment posted under a genuine bot token, where the forge already reports `type: Bot`. -Comment bodies only --- not commit messages, titles, issue bodies, or PR bodies. +The marker deliberately avoids the robot emoji, which `scripts/check-pr-fully-clean.py` matches as a `REVIEW_BODY_MARKERS` entry --- a disclosed claim comment would otherwise scan as a finding-free **review**. +The fragment carries the rest: the two exemptions, the comment-bodies-only scope, and the queries that verify a marker or a bot identity. - **Do:** append the marker to every claim, release, status, reply, and self-review comment, including ones whose prose already names the session. -- **Don't:** use the robot emoji in it, or put it in a commit message or title. - +- **Don't:** use the robot emoji in it, or put it in a commit message, a title, an issue body, or a PR body. ## Read a repo's canonical contributor doc before starting work, not just before pushing diff --git a/commands/release-pr.md b/commands/release-pr.md index 2c5897131..cfd45898a 100644 --- a/commands/release-pr.md +++ b/commands/release-pr.md @@ -33,15 +33,15 @@ If only one positional arg is given, treat it as `pr_number`. Call `mcp__github__pull_request_read(method = "get_comments", owner = , repo = , pullNumber = )`. Walk the last ~10 comments and confirm: - the most recent claim/release exchange is an unmatched claim that hasn't yet been followed by a release. - **Match the two-word invariant `hold off` (case-insensitively), never a full sentence** — the PR claim reads `please hold off on pushing to this branch until I'm done` while the issue claim reads `please hold off until I'm done`, so neither sentence is a substring of the other and a matcher keyed on either one misses the other. - **Also match the pre-2026-08-24 invariant `paws off`**: claims posted before that date are still live on open PRs, since a claim expires on activity rather than on age, and a matcher narrowed to the new wording returns nothing on them — indistinguishable from no claim at all. + **Match the two-word invariant `hold off` (case-insensitively), never a full sentence** --- the PR claim reads `please hold off on pushing to this branch until I'm done` while the issue claim reads `please hold off until I'm done`, so neither sentence is a substring of the other and a matcher keyed on either one misses the other. + **Also match the pre-2026-08-24 invariant `paws off`**: claims posted before that date are still live on open PRs, since a claim expires on activity rather than on age, and a matcher narrowed to the new wording returns nothing on them --- indistinguishable from no claim at all. So the claim test is `test("hold off|paws off"; "i")`. Treat **any** of these as a release marker --- four the corpus posts today plus one retired form still sitting on open PRs, and enumerating only this command's own is what makes it post a stray release over somebody else's completed handover: - - this command's `… done — claim released.` - - its pre-2026-08-24 form `… done — paws off released.` - - `claim-pr`'s `Done with my local session — unclaiming.` + - this command's `… done --- claim released.` + - its pre-2026-08-24 form `… done --- paws off released.` + - `claim-pr`'s `Done with my local session --- unclaiming.` - `ardi`'s on-clean unclaim, `Done --- PR is free.` - - `post-merge`'s conflict unclaim, `Conflict resolved — branch is now mergeable. …` + - `post-merge`'s conflict unclaim, `Conflict resolved --- branch is now mergeable. …` Derive that list rather than trusting this one, since a skill may add a sixth: `grep -rn "unclaim\|released\|PR is free\|now mergeable" skills/ commands/`. - and that claim's `lane` matches the lane we're releasing. @@ -51,7 +51,7 @@ If only one positional arg is given, treat it as `pr_number`. 3. Compose the comment body, exactly in this shape so other agents recognise it: ``` - done — claim released. + done --- claim released. _Posted by Claude Code (AI agent) --- not written by a human._ ``` @@ -59,12 +59,12 @@ If only one positional arg is given, treat it as `pr_number`. If `summary` is provided, append it in parentheses on the first line: ``` - done — claim released. () + done --- claim released. () _Posted by Claude Code (AI agent) --- not written by a human._ ``` - The trailing marker is required on every agent-posted comment, and is deliberately emoji-free — see [`disclose-agent-authorship`](../shared/workflow/disclose-agent-authorship.md). + The trailing marker is required on every agent-posted comment, and is deliberately emoji-free --- see [`disclose-agent-authorship`](../shared/workflow/disclose-agent-authorship.md). 4. Post the comment: diff --git a/hooks/require-agent-disclosure.py b/hooks/require-agent-disclosure.py index b96d27b0f..f5bdd1eb5 100755 --- a/hooks/require-agent-disclosure.py +++ b/hooks/require-agent-disclosure.py @@ -55,13 +55,6 @@ import re import sys -# --- what counts as posting a comment --------------------------------------- -# -# ANCHORED at a command position -- start of string, after a separator, or -# after a shell keyword that introduces a command. Unanchored, this fires on -# every reply and every doc that merely QUOTES the command, and this corpus -# quotes it constantly (`shared/workflow/claim-pr.md` is nothing but such -# quotes). # --- what counts as posting a comment --------------------------------------- # # ANCHORED at a command position -- start of string, after a separator, or after @@ -110,10 +103,20 @@ API_CMD_RE = re.compile(_ANCHOR + r"(?:gh|glab)\s+api\b", re.MULTILINE) # A body-supplying field is what separates a POST from the review-READ that # `CLAUDE.md` prescribes and every ARDI round runs. -API_BODY_FIELD_RE = re.compile(r"(?:-f|-F|--field|--raw-field)\s+body=") +# The optional quote is load-bearing: `tool-mappings`'s own canonical reply +# command is `-F "body=@"`, quote first, and this corpus writes the +# quoted-whole-argument spelling for sibling flags too +# (`request-pr-review`'s `-f "reviewers[]="`). Without it the registry line +# this change annotates was completely invisible to the guard. +API_BODY_FIELD_RE = re.compile(r"(?:-f|-F|--field|--raw-field)\s+[\"']?body=") # The comment-bearing endpoints, and the GraphQL comment mutations. +# No `/replies` alternative. GitHub's reply route is +# `POST /repos/{o}/{r}/pulls/{n}/comments/{id}/replies`, so it always contains +# `/comments` -- the alternative could never fire alone, which is why no fixture +# could isolate it and why two attempts at one were masked by their own paths. +# An untestable alternative also implies a route that does not exist. API_COMMENT_TARGET_RE = re.compile( - r"/comments|/replies|/notes|/discussions" + r"/comments|/notes|/discussions" r"|addDiscussionComment|addComment", re.IGNORECASE) @@ -123,12 +126,16 @@ def is_api_post(segment): return False if not API_COMMENT_TARGET_RE.search(segment): return False - # No GraphQL exception. It was added on the theory that a comment mutation - # carries its body as a variable and so would fail the `body=` test -- but - # every GraphQL comment site in this corpus supplies `-f body='...'`, so the - # branch decided nothing while classifying any command that merely NAMED the - # mutation as a post: `gh api graphql --input payload.json # addDiscussion- - # Comment payload` drew a warning for a command posting nothing visible. + # A GraphQL comment mutation may carry its body inside the query text or in + # an `--input` file rather than in a `body=` field, so the field test alone + # would miss it. The earlier version of this branch keyed on the mutation + # NAME alone, which classified `gh api graphql --input p.json # addDiscuss- + # ionComment payload` -- a comment about a payload -- as a post. + # `mutation` beside the name is what separates executing one from naming one. + if (re.search(r"\bmutation\b", segment) + and re.search(r"addDiscussionComment|addComment", segment, + re.IGNORECASE)): + return True return bool(API_BODY_FIELD_RE.search(segment)) @@ -138,7 +145,7 @@ def is_api_post(segment): REVIEW_ONLY_RE = re.compile(_ANCHOR + r"gh\s+pr\s+review\b", re.MULTILINE) ANY_BODY_FLAG_RE = re.compile( r"--body\b|--body=|--body-file|--message\b|--message=|-b\s|-m\s|-F\s" - r"|(?:-f|-F|--field|--raw-field)\s+body=") + r"|(?:-f|-F|--field|--raw-field)\s+[\"']?body=") def is_post_segment(segment): @@ -200,15 +207,21 @@ def is_post_segment(segment): # being assumed to cover the other. UNREADABLE_RE = re.compile( r"--body-file|--description-file|--editor\b|--web\b" - r"|-F\s+body=@|(?` is gh pr comment's own body-file shorthand. Matched as a token + # with NO `=` in it, rather than by a negative lookahead after an optional + # quote: the optional quote gave the engine a backtracking path where it + # skipped the quote, failed to find `key=` starting at `"`, and so satisfied + # the negation -- which made `-F "in_reply_to=5"` look like a file. + r"|(?"`, + # quote first, and the field pattern required `body=` to follow whitespace + # directly -- so the registry line this change annotates was invisible. + ("gh api with a quoted body= argument", + 'gh api repos/o/r/issues/1/comments -f "body=Working on this."', "missing"), + ("gh api with a quoted body= argument, WITH marker", + 'gh api repos/o/r/issues/1/comments -f "body=Done.\n\n' + MARKER + '"', + False), + ("the registry's own quoted body=@file reply command", + 'gh api -X POST "repos/o/r/pulls/1/comments/9/replies" -F "body=@/tmp/r.md"', + None), + ("a quoted typed field does not look like a body-file", + 'gh api repos/o/r/pulls/1/comments -F "in_reply_to=5" -f body="Addressed."', + "missing"), + ("an unquoted -F file is still a body-file", + 'gh pr comment 12 -F "/tmp/body.md"', None), + + # --- round-6: executing a GraphQL mutation vs naming one ----------------- + ("a GraphQL mutation whose body is not in a body= field", + "gh api graphql --input p.json -f query='mutation { addDiscussionComment(x) }'", + None), + ("a comment mentioning the mutation posts nothing", + 'gh api graphql --input p.json # addDiscussionComment payload', False), + + # --- round-6: properties that survived mutation with the suite green ----- + ("command substitution is a command position", + 'URL=$(gh pr comment 12 --body "bare")', True), + ("a brace group is a command position", + '{ gh pr comment 12 --body "bare"; }', True), + ("the marker needs its attribution prefix, not just the parenthetical", + 'gh pr comment 12 --body "Our (AI agent) policy is documented."', "missing"), + ("a heredoc piped into --body-file - keeps its opener tail", + "cat <<'EOF' | gh pr comment 12 --body-file -\nDone, undisclosed.\nEOF", + "missing"), + # GitHub's reply route always contains `/comments`, so this is caught by + # that alternative -- there is no separate `/replies` one to pin. + ("the review-thread reply route is a comment target", + 'gh api "repos/o/r/pulls/1/comments/9/replies" -f body="bare"', True), + # --- unreadable vs missing must not be confused (review finding 9) ------- ("gh pr comment -F is a body-file, reported unreadable", 'gh pr comment 12 -F /tmp/body.md', None), @@ -362,11 +403,21 @@ def run(): emitted = json.loads(out)["hookSpecificOutput"] except Exception: emitted = {} + try: + whole = json.loads(out) + except Exception: + whole = {} ok = (proc.returncode == 0 and emitted.get("hookEventName") == "PreToolUse" and "additionalContext" in emitted and "permissionDecision" not in emitted - and "disclosure marker" in emitted.get("additionalContext", "")) + and "disclosure marker" in emitted.get("additionalContext", "") + # The user-facing half. Warning only the model leaves the + # account holder unaware a comment posted under their login + # was flagged, and check-hook-output-shape.py's systemMessage + # rule fires on Stop hooks only, so nothing else pins this. + and isinstance(whole.get("systemMessage"), str) + and "agent" in whole.get("systemMessage", "")) else: ok = proc.returncode == 0 and out == "" failed += not ok @@ -403,14 +454,19 @@ def run(): ("MCP discussion comment WITH marker", "mcp__github__discussion_comment_write", "Reply.\n\n" + MARKER, False), + # Round-6: the isinstance guard exists for review methods that submit no + # body at all (resolve_thread, delete_pending); nothing pinned it. + ("an MCP call with no body is not judged", + "mcp__github__pull_request_review_write", None, False), ): - got = guard.verdict_mcp(tool, {"body": body}) is not None + payload = {} if body is None else {"body": body} + got = guard.verdict_mcp(tool, payload) is not None ok = got == expect failed += not ok print(f"{'PASS' if ok else 'FAIL'}: {label} " f"(warned={got}, expected={expect})") - total = len(CASES) + 2 + len(INDIRECT_CASES) + 1 + 4 + 10 + total = len(CASES) + 2 + len(INDIRECT_CASES) + 1 + 4 + 11 print(f"\n{total - failed} passed, {failed} failed") return 1 if failed else 0 diff --git a/scripts/sync-codex-skill-wrappers.py b/scripts/sync-codex-skill-wrappers.py index bdfc1b127..1afa7f0d9 100755 --- a/scripts/sync-codex-skill-wrappers.py +++ b/scripts/sync-codex-skill-wrappers.py @@ -168,7 +168,7 @@ def reference_doc(mappings: dict) -> str: > `_Posted by Claude Code (AI agent) --- not written by a human._` > > This registry is the substitution point for remote/web sessions, which have no -> `gh` at all — so a marker-free template here is a marker-free comment there, in +> `gh` at all --- so a marker-free template here is a marker-free comment there, in > exactly the population that cannot fall back to the CLI examples in the skills. > See [`disclose-agent-authorship`](shared/workflow/disclose-agent-authorship.md). > Two exemptions: a body another machine parses as a command (`@dependabot diff --git a/shared/workflow/disclose-agent-authorship.md b/shared/workflow/disclose-agent-authorship.md index e543a32cf..0b3bf7943 100644 --- a/shared/workflow/disclose-agent-authorship.md +++ b/shared/workflow/disclose-agent-authorship.md @@ -98,13 +98,17 @@ The literal `--body "@...` grep finds the two Dependabot sites and misses the th An earlier draft of this passage gave a different and wrong reason --- that the handle is never spelled contiguously in a source file, because a diff view would summon the bot. That is false twice over. -The handle appears 242 times across this corpus's markdown (measured 2026-08-24), including in `AGENTS.md`, `CLAUDE.md` and `README.md`, and none of those has ever dispatched a run. +The handle appears 250 times across this corpus's markdown at this commit, by the command below, including in `AGENTS.md`, `CLAUDE.md` and `README.md`, and none of those has ever dispatched a run. And [`memories/mention-triggers.md`](../../memories/mention-triggers.md) states the gate as `contains(github.event.comment.body, '@claude')`, over comment, review and issue bodies --- file contents are not among them. The practice of not spelling it applies to text that becomes a comment, which is what that file scopes it to. ```bash -grep -rn -- '--body "@' --include='*.md' --include='*.sh' . # command-shaped sites +# Option flags go BEFORE `--`; after it they are read as file operands, and the +# search silently runs unfiltered. The first draft of this block got that wrong +# and returned its own test fixtures as apparent sites. +grep -rn --include='*.md' --include='*.sh' -- '--body "@' . # command-shaped sites grep -rn '@claude' skills/ # prose sites +git grep -noi '@claude' -- '*.md' | wc -l # the figure above ``` - **Do:** omit the marker on a comment whose whole body is a command addressed to another bot. diff --git a/skills/claim-pr/SKILL.md b/skills/claim-pr/SKILL.md index 3dded4004..b40fa4653 100644 --- a/skills/claim-pr/SKILL.md +++ b/skills/claim-pr/SKILL.md @@ -110,7 +110,13 @@ for d in json.load(sys.stdin): for n in d.get('notes', []): body = n.get('body', '').lower() # Both wordings: claims posted before 2026-08-24 say 'paws off'. - if ('hold off' in body or 'paws off' in body) and not n.get('resolved'): + # A RELEASE is excluded first -- the retired release note + # '... done --- paws off released.' contains 'paws off', so a claim-only + # test resolves the release's thread instead of the claim's. + is_release = any(t in body for t in + ('unclaim', 'released', 'pr is free', 'now mergeable')) + is_claim = ('hold off' in body or 'paws off' in body) and not is_release + if is_claim and not n.get('resolved'): print(d['id']); break else: continue break diff --git a/skills/gi/SKILL.md b/skills/gi/SKILL.md index e969b821c..9b5ab66c1 100644 --- a/skills/gi/SKILL.md +++ b/skills/gi/SKILL.md @@ -87,7 +87,9 @@ check both explicitly here). gh issue view --json comments --jq '.comments | last | .body' | cat # READ_ISSUE_COMMENTS ``` -If it contains "Working on this" / "hold off" / "paws off" (the last being the pre-2026-08-24 wording, still live on threads claimed before then), or an equivalent claim, skip the issue --- unless the claim has expired: no push or comment on the issue in over 2 hours, per [`claim-pr`](../../shared/workflow/claim-pr.md)'s expiration rule. +Match the two-word invariant `hold off`, or the pre-2026-08-24 `paws off`, case-insensitively --- then **exclude the comment if it also carries a release term** (`unclaim|released|PR is free|now mergeable`), because the retired release wording `... done --- paws off released.` contains `paws off` and would otherwise read as a live claim. +See [`claim-pr`](../../shared/workflow/claim-pr.md)'s "Match the two-word invariant". +If a live claim stands, skip the issue --- unless the claim has expired: no push or comment on the issue in over 2 hours, per [`claim-pr`](../../shared/workflow/claim-pr.md)'s expiration rule. An expired claim is taken over by posting your own claim comment, never silently. **(2) No open PR already references the issue:** diff --git a/tool-mappings.md b/tool-mappings.md index 205fb5d91..d64ab8f78 100644 --- a/tool-mappings.md +++ b/tool-mappings.md @@ -16,7 +16,7 @@ operation to the equivalent GitHub MCP tool so any model can run a skill. > `_Posted by Claude Code (AI agent) --- not written by a human._` > > This registry is the substitution point for remote/web sessions, which have no -> `gh` at all — so a marker-free template here is a marker-free comment there, in +> `gh` at all --- so a marker-free template here is a marker-free comment there, in > exactly the population that cannot fall back to the CLI examples in the skills. > See [`disclose-agent-authorship`](shared/workflow/disclose-agent-authorship.md). > Two exemptions: a body another machine parses as a command (`@dependabot @@ -64,10 +64,10 @@ operation to the equivalent GitHub MCP tool so any model can run a skill. | `REOPEN_ISSUE` | Reopen a closed issue. | `gh issue reopen "" --comment "..."` | `mcp__github__issue_write (method=update, state=open)` | | `LABEL_ISSUE` | Set an issue's labels. The two behave differently and are not interchangeable: `--add-label` ADDS to the existing set, while the MCP path REPLACES the whole set, so pass the union of existing and new labels there. The MCP path also silently creates an unknown label name instead of rejecting it. | `gh issue edit "" --add-label "..."` | `mcp__github__issue_write (method=update, labels=[...])` | | `GET_LABEL` | Read a single label's name, color, and description. There is no MCP tool to create or update a label; use gh label create/edit, or gh api from a workflow. | `gh api "repos///labels/"` | `mcp__github__get_label` | -| `LIST_DISCUSSIONS` | List a repository's discussions. Readable over REST; writes are GraphQL-only. | `gh api repos/{owner}/{repo}/discussions` | (no GitHub MCP tool; use gh api REST or graphql) | -| `VIEW_DISCUSSION` | Read a discussion topic and its comment thread. Readable over REST. | `gh api repos/{owner}/{repo}/discussions/{number}[/comments]` | (no GitHub MCP tool; use gh api REST or graphql) | -| `COMMENT_DISCUSSION` | Post a reply on a discussion (top-level or threaded). **The body ends with the agent-disclosure marker** --- see [`disclose-agent-authorship`](shared/workflow/disclose-agent-authorship.md). | `gh api graphql (addDiscussionComment)` | `mcp__github__discussion_comment_write` | -| `ANSWER_DISCUSSION` | Mark a comment as the accepted answer on a Q&A discussion. | `gh api graphql (markDiscussionCommentAsAnswer)` | (no GitHub MCP tool; use gh api graphql) | +| `LIST_DISCUSSIONS` | List a repository's discussions. Readable over REST; writes are GraphQL-only. | `gh api repos/{owner}/{repo}/discussions` | `mcp__github__list_discussions` | +| `VIEW_DISCUSSION` | Read a discussion topic and its comment thread. Readable over REST. | `gh api repos/{owner}/{repo}/discussions/{number}[/comments]` | `mcp__github__get_discussion / mcp__github__get_discussion_comments` | +| `COMMENT_DISCUSSION` | Post a reply on a discussion (top-level or threaded). **The body ends with the agent-disclosure marker** --- see [`disclose-agent-authorship`](shared/workflow/disclose-agent-authorship.md). | `gh api graphql (addDiscussionComment)` | `mcp__github__discussion_comment_write (method=add\|reply)` | +| `ANSWER_DISCUSSION` | Mark a comment as the accepted answer on a Q&A discussion. | `gh api graphql (markDiscussionCommentAsAnswer)` | `mcp__github__discussion_comment_write (method=mark_answer)` | | `CREATE_DISCUSSION` | Open a new discussion in a category. | `gh api graphql (createDiscussion)` | (no GitHub MCP tool; use gh api graphql) | | `CLOSE_DISCUSSION` | Close a discussion with a reason (RESOLVED, OUTDATED, DUPLICATE). | `gh api graphql (closeDiscussion)` | (no GitHub MCP tool; use gh api graphql) | | `PUSH` | Push commits to a branch. | `git push -u origin ""` | (use git; no GitHub MCP equivalent) | diff --git a/tool-mappings.yml b/tool-mappings.yml index 39a94e58f..12a1758f8 100644 --- a/tool-mappings.yml +++ b/tool-mappings.yml @@ -177,19 +177,19 @@ operations: - id: LIST_DISCUSSIONS description: List a repository's discussions. Readable over REST; writes are GraphQL-only. cli: gh api repos/{owner}/{repo}/discussions - github_mcp: (no GitHub MCP tool; use gh api REST or graphql) + github_mcp: mcp__github__list_discussions - id: VIEW_DISCUSSION description: Read a discussion topic and its comment thread. Readable over REST. cli: gh api repos/{owner}/{repo}/discussions/{number}[/comments] - github_mcp: (no GitHub MCP tool; use gh api REST or graphql) + github_mcp: mcp__github__get_discussion / mcp__github__get_discussion_comments - id: COMMENT_DISCUSSION description: Post a reply on a discussion (top-level or threaded). **The body ends with the agent-disclosure marker** --- see [`disclose-agent-authorship`](shared/workflow/disclose-agent-authorship.md). cli: gh api graphql (addDiscussionComment) - github_mcp: mcp__github__discussion_comment_write + github_mcp: mcp__github__discussion_comment_write (method=add|reply) - id: ANSWER_DISCUSSION description: Mark a comment as the accepted answer on a Q&A discussion. cli: gh api graphql (markDiscussionCommentAsAnswer) - github_mcp: (no GitHub MCP tool; use gh api graphql) + github_mcp: mcp__github__discussion_comment_write (method=mark_answer) - id: CREATE_DISCUSSION description: Open a new discussion in a category. cli: gh api graphql (createDiscussion) From 0fff5eb481e1946634d7a748d2d0109bf82e526d Mon Sep 17 00:00:00 2001 From: Douglas Ezra Morrison Date: Mon, 24 Aug 2026 12:32:49 -0700 Subject: [PATCH 09/13] fix(disclosure): address pre-push review; restore a clause deleted as dead Seven findings on 781d2869, two of them blocking. All addressed. A "dead code" deletion that was not dead Round 6 removed the short-flag `$`-interpolation clause on the reasoning that HAS_INLINE_BODY_RE had taken it over, and narrowed the field-form clause so it could no longer cross a quote or a space. Neither held: that pattern rejects only a value BEGINNING with `$`, so a MID-STRING expansion fell through and reported a missing marker over a body the check never read -- an assertion beyond what it observed, and the opposite verdict from `--body` on the identical body. UNREADABLE -> MISSING gh pr comment 5 -b "pre $VAR post" UNREADABLE -> MISSING gh pr comment 5 -f body="pre $VAR post" UNREADABLE -> MISSING gh pr comment 5 -F "body=pre $VAR post" The fixture that licensed the deletion was named "-b with an expanded variable is unreadable" and only exercised `-b "$BODY"`, the whole-value form, which still passed. The name generalized past the check. Both clauses are restored, four mid-string fixtures added, and deleting either clause now goes red. A count that refuted itself The figure was published beside a `git grep` command that is itself an `@claude` occurrence in a markdown file, so shipping the command incremented the number it measured -- wrong at round 5 and wrong again at round 6, in opposite directions, and failing on first contact for a reader invited to re-run it. The count now excludes this file, in both the sentence and the command, so it is stable under edits to the file that states it. Verified: the sentence says 248 and the command returns 248. Contradictions between the corpus and itself - The guard carried the mention-gate rationale that the fragment added in this same branch calls "false twice over" -- shipping the refutation and the refuted claim side by side, with the refuted one in the file a maintainer reads first. The handle stays concatenated for a narrower and true reason, now stated. - "share an id space but not a route" contradicted the 404 two lines below it. If the id space were shared the wrong route would return a different comment, which is a worse failure than the one described. - The guard implements one of the two declared exemptions, and nothing said so. Whether a token belongs to an app or to a person is not in the command text, so no lexical check can decide it; the docstring now names the gap and its one visible consequence. - memories/github-mcp-tools.md carries the same pre-change warning memories/github.md got a pointer for, and had none. - The name-substitution instruction is the one path that can reintroduce the collision the emoji ban prevents: `code review` is a REVIEW_BODY_MARKERS entry, so an agent named for code review would reintroduce it through its own name. Both AGENTS.md and the fragment now say to check the substituted name too. Checks: 41/41 hook suites (109 in this one, up from 105), markdownlint 0, links, skills, hook-catalog, hook-output-shape, context-closure pin, and tool-mappings regenerates byte-identical. --- AGENTS.md | 1 + hooks/require-agent-disclosure.py | 35 +++++++++++++++----- hooks/test-require-agent-disclosure.py | 12 +++++++ memories/github-mcp-tools.md | 16 ++++----- shared/workflow/disclose-agent-authorship.md | 8 ++--- 5 files changed, 51 insertions(+), 21 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index f011bc70d..61a669854 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -186,6 +186,7 @@ _Posted by Claude Code (AI agent) --- not written by a human._ ``` Substitute your own agent's name where you are not Claude Code, and keep the rest of the line verbatim so one query finds every disclosed comment. +Check the substituted **name** against `REVIEW_BODY_MARKERS` as well as a replacement marker: `code review` is one of its entries, so an agent named for code review would reintroduce through its own name the false-clean the emoji ban exists to prevent. The marker deliberately contains **no robot emoji**: [`scripts/check-pr-fully-clean.py`](scripts/check-pr-fully-clean.py) matches that emoji as a review-body marker, so a disclosed claim comment would be admitted into the fully-clean verdict scan as a finding-free review. Check any replacement marker against that script's `REVIEW_BODY_MARKERS` and `REVIEW_AGENT_MARKERS` before adopting it. diff --git a/hooks/require-agent-disclosure.py b/hooks/require-agent-disclosure.py index f5bdd1eb5..f9daa9faa 100755 --- a/hooks/require-agent-disclosure.py +++ b/hooks/require-agent-disclosure.py @@ -37,6 +37,14 @@ audience -- a machine parses that body -- not the length, so the pattern is anchored to the whole body rather than to its first token. +The corpus declares a SECOND exemption this guard does not implement: a comment +posted under a genuine bot token, where the forge already reports `type: Bot` +and the marker adds nothing. Whether a token is an app's or a person's is not in +the command text, so no lexical check can decide it -- which is why the rule +carries it and the instrument does not. The visible consequence is that +`skills/claude-agent-workflow/SKILL.md`'s in-workflow reply draws a warning it +should not. Warn-only, so the cost is a note rather than a refusal. + PER SEGMENT, not per call. A batched round posting several comments in one Bash call is the encouraged shape (`shared/workflow/efficient-pr-babysitting.md`), so one disclosed body must not vouch for an undisclosed sibling. Each @@ -214,14 +222,19 @@ def is_post_segment(segment): # the negation -- which made `-F "in_reply_to=5"` look like a file. r"|(?` events under the human account's - identity, not a bot identity.** `add_reply_to_pull_request_comment` and - `add_issue_comment` authenticate as the human who owns the session (e.g. - `d-morrison`), so a webhook event for your own just-posted reply shows - `Author: d-morrison` (or whichever human), never a recognizable bot name - like `claude[bot]`. Don't use the author field to decide "is this my own - echo, skip it." This is easy to get wrong at a glance since a same-author - event looks exactly like a genuine human reply demanding a response. +- **Comments/replies you post via the GitHub MCP tools echo back into the session's `` events under the human account's identity, not a bot identity.** + `add_reply_to_pull_request_comment` and `add_issue_comment` authenticate as the human who owns the session (e.g. `d-morrison`), so a webhook event for your own just-posted reply shows `Author: d-morrison` (or whichever human), never a recognizable bot name like `claude[bot]`. + Don't use the author field to decide "is this my own echo, skip it." + This is easy to get wrong at a glance since a same-author event looks exactly like a genuine human reply demanding a response. + **Since 2026-08-24 every agent-posted comment also ends with `_Posted by Claude Code (AI agent) --- not written by a human._`**, which is the corpus-wide marker the `mcp__github__*` comment tools carry --- see [`disclose-agent-authorship`](../shared/workflow/disclose-agent-authorship.md). + It is a more reliable self-echo test than the author field, and unlike the footer below it is required by a rule rather than by the harness's attribution setting. + Comments posted before that date carry no marker, so the warning above still governs when auditing older threads. **Check for the Claude Code attribution footer instead of fuzzy-matching body text/timing** --- every comment posted from these sessions ends with `_Generated by [Claude Code](https://claude.ai/code)_` per the system diff --git a/shared/workflow/disclose-agent-authorship.md b/shared/workflow/disclose-agent-authorship.md index 0b3bf7943..3995eb260 100644 --- a/shared/workflow/disclose-agent-authorship.md +++ b/shared/workflow/disclose-agent-authorship.md @@ -31,14 +31,14 @@ Where it belongs to an app --- a workflow posting as `github-actions[bot]`, so t `skills/claude-agent-workflow/SKILL.md`'s in-workflow reply is that case. The test is the **token**, not the author: an agent driving `gh` with the account holder's PAT posts as a `User` however automated the surrounding workflow is. -Check `.user.type` on a posted comment if unsure, rather than reasoning from how the comment was produced --- and pick the endpoint by comment KIND, since an issue-or-PR comment and a review-thread comment share an id space but not a route: +Check `.user.type` on a posted comment if unsure, rather than reasoning from how the comment was produced --- and pick the endpoint by comment KIND, since an issue-or-PR comment and a review-thread comment have separate id spaces and separate routes: ```bash gh api repos///issues/comments/ --jq .user.type # PR or issue comment gh api repos///pulls/comments/ --jq .user.type # review-thread reply ``` -Asking the first about a review-thread comment returns `404 Not Found`, which reads as a missing comment rather than as the wrong route. +Asking the first about a review-thread comment returns `404 Not Found` --- which reads as a missing comment rather than as the wrong route, and is the reason to pick the route by kind rather than to try one and see. **A prose self-identification is not a substitute for the marker.** "Claude Code CLI (local session) is working on this" already discloses, so appending the footer to it looks redundant. @@ -98,7 +98,7 @@ The literal `--body "@...` grep finds the two Dependabot sites and misses the th An earlier draft of this passage gave a different and wrong reason --- that the handle is never spelled contiguously in a source file, because a diff view would summon the bot. That is false twice over. -The handle appears 250 times across this corpus's markdown at this commit, by the command below, including in `AGENTS.md`, `CLAUDE.md` and `README.md`, and none of those has ever dispatched a run. +The handle appears 248 times across this corpus's markdown, counting every file but this one --- the command below is itself an occurrence, so a figure that included this file would move each time the file was edited, and both earlier drafts of this sentence were wrong for exactly that reason. And [`memories/mention-triggers.md`](../../memories/mention-triggers.md) states the gate as `contains(github.event.comment.body, '@claude')`, over comment, review and issue bodies --- file contents are not among them. The practice of not spelling it applies to text that becomes a comment, which is what that file scopes it to. @@ -108,7 +108,7 @@ The practice of not spelling it applies to text that becomes a comment, which is # and returned its own test fixtures as apparent sites. grep -rn --include='*.md' --include='*.sh' -- '--body "@' . # command-shaped sites grep -rn '@claude' skills/ # prose sites -git grep -noi '@claude' -- '*.md' | wc -l # the figure above +git grep -noi '@claude' -- '*.md' ':!shared/workflow/disclose-agent-authorship.md' | wc -l # the figure above ``` - **Do:** omit the marker on a comment whose whole body is a command addressed to another bot. From 28b2a440ae64091dc8251f3f6f3c82f244ab7de9 Mon Sep 17 00:00:00 2001 From: Douglas Ezra Morrison Date: Mon, 24 Aug 2026 12:58:34 -0700 Subject: [PATCH 10/13] fix(claim): add the third retired wording, `back off`, to every matcher One blocking finding on 0fff5eb4, plus four non-blocking. All addressed. There were three retired claim wordings, not one `ardi` posted `Driving this PR to clean --- back off until done.` since 2026-06-17, and this branch retired it in c3bc43a5 without adding it to the back-compat alternation -- so the five matchers this branch rewrote all read `hold off|paws off` and were blind to it. `ardi` is the corpus's highest-traffic claim emitter, run on every PR in every repo, so a session claiming a PR through it left a claim that `/push`'s check 3 could not see: the query returns empty, which is indistinguishable from an unclaimed thread, and the session pushes over a live claim. That is verbatim the failure this branch's own added prose describes. `/release-pr` had the mirror failure and would decline to post the release. The prose was false in the same commit that proved it: claim-pr.md said the body "said 'paws off until I'm done' until 2026-08-24", and release-pr.md called `paws off` "the pre-2026-08-24 invariant". Both are true of claim-pr's own body and false of the corpus. All six matchers now carry `hold off|paws off|back off`. The method that missed it, named and replaced Six skills agreed on `paws off`, so enumerating the retired set from the files in front of me returned six confirmations and one silent omission -- the one file that disagreed was the one not consulted. claim-pr.md now publishes a git-log derivation over history instead of a list, verified to return all three, with a Don't naming the enumeration method as the cause. Non-blocking - The migrate-discussion MD049 measurement does not reproduce under the same harness that reproduces the other three exactly. The structural hazard is real and the linter's coverage of it is not established; the passage now says that rather than claiming the linter catches it. - Two count-based self-references ("four steps below", "step 3 below") replaced with stable ones. - A single-quoted `$` is reported unreadable though bash does not expand it. Kept: the `--body`/`--message` clause has always behaved this way, so the short forms match rather than diverge, and it errs toward the weaker note rather than toward asserting something about text never read. Pinned as a test so the choice is visible as a choice. Verification carried over from the review: a differential of verdict() against 781d2869 over 7,514 synthetic commands showed 456 differences, all in the intended MISSING -> UNREADABLE direction, and zero across 831 fenced blocks in the tree -- with a negative control showing the corpus sweep fires on 25 blocks, so the zero is a result rather than a detector that never ran. Checks: 41/41 hook suites (110 in this one), markdownlint 0, links, skills, hook-catalog, hook-output-shape, context-closure pin, tool-mappings idempotent. --- commands/release-pr.md | 4 ++-- hooks/test-require-agent-disclosure.py | 7 ++++++ shared/workflow/claim-pr.md | 24 ++++++++++++++------ shared/workflow/disclose-agent-authorship.md | 22 ++++-------------- skills/claim-pr/SKILL.md | 3 ++- skills/gi/SKILL.md | 2 +- skills/post-merge/SKILL.md | 4 ++-- skills/push/SKILL.md | 4 ++-- 8 files changed, 38 insertions(+), 32 deletions(-) diff --git a/commands/release-pr.md b/commands/release-pr.md index cfd45898a..53c97ef8c 100644 --- a/commands/release-pr.md +++ b/commands/release-pr.md @@ -34,8 +34,8 @@ If only one positional arg is given, treat it as `pr_number`. - the most recent claim/release exchange is an unmatched claim that hasn't yet been followed by a release. **Match the two-word invariant `hold off` (case-insensitively), never a full sentence** --- the PR claim reads `please hold off on pushing to this branch until I'm done` while the issue claim reads `please hold off until I'm done`, so neither sentence is a substring of the other and a matcher keyed on either one misses the other. - **Also match the pre-2026-08-24 invariant `paws off`**: claims posted before that date are still live on open PRs, since a claim expires on activity rather than on age, and a matcher narrowed to the new wording returns nothing on them --- indistinguishable from no claim at all. - So the claim test is `test("hold off|paws off"; "i")`. + **Also match both retired invariants, `paws off` and `back off`** (the second was `ardi`'s, and is easy to miss because six other skills agreed on the first): claims posted before 2026-08-24 are still live on open PRs, since a claim expires on activity rather than on age, and a matcher narrowed to the new wording returns nothing on them --- indistinguishable from no claim at all. + So the claim test is `test("hold off|paws off|back off"; "i")`. Treat **any** of these as a release marker --- four the corpus posts today plus one retired form still sitting on open PRs, and enumerating only this command's own is what makes it post a stray release over somebody else's completed handover: - this command's `… done --- claim released.` - its pre-2026-08-24 form `… done --- paws off released.` diff --git a/hooks/test-require-agent-disclosure.py b/hooks/test-require-agent-disclosure.py index fa1aa4c06..8716065f6 100755 --- a/hooks/test-require-agent-disclosure.py +++ b/hooks/test-require-agent-disclosure.py @@ -201,6 +201,13 @@ def GQL(body): 'gh pr comment 5 -F "body=Addressed in $SHA."', None), ("--body with a mid-string expansion is unreadable", 'gh pr comment 5 --body "Addressed in $SHA."', None), + # A single-quoted `$` does not expand in bash, so this body IS readable -- + # and it is reported unreadable anyway, deliberately. The `--body`/`--message` + # clause has always behaved this way, so the short forms match it rather than + # diverging, and the error is toward the weaker note rather than toward an + # assertion about text never read. Pinned so the choice is visible as one. + ("a single-quoted $ is reported unreadable, matching --body's behaviour", + "gh pr comment 5 -b 'costs $9'", None), ("--raw-field body= is inline and readable", 'gh api repos/o/r/issues/12/comments --raw-field body="hi"', "missing"), ("-F body=@file is a file reference, so unreadable", diff --git a/shared/workflow/claim-pr.md b/shared/workflow/claim-pr.md index be91e61ea..42ecade49 100644 --- a/shared/workflow/claim-pr.md +++ b/shared/workflow/claim-pr.md @@ -92,7 +92,16 @@ issue claims last 2 hours from the most recent push or comment; if it's been longer than that, reassert your claim.") **Every detector of a claim matches the OLD wording as well as the new one, and dropping the old alternation is the one edit that fails silently.** -The claim body said "paws off until I'm done" until 2026-08-24 and now says "please hold off ...". +There were **three** retired wordings, not one, and enumerating them from the file in front of you is how the third was missed for a whole review round. +`claim-pr`, `gi`, `st`, `pr-on-claim`, `post-merge` and `handoff` all said "paws off until I'm done"; [`ardi`](ardi.md) --- the corpus's highest-traffic claim emitter, run on every PR in every repo --- said "back off until done", and had done since 2026-06-17. +All three now read "please hold off ...". + +Derive that set from history rather than from the current tree, which no longer contains any of them: + +```bash +git log -p --all -- 'skills/*/SKILL.md' 'shared/workflow/claim-pr.md' \ + | grep -oiE '^\+.*--(body|message) "[^"]*(off|claim)[^"]*"' | sort -u +``` Claims posted before that are still sitting on open PRs and issues, and a claim stays live on activity rather than on age --- so a thread claimed under the old wording and pushed to this morning is live right now. A detector narrowed to the new phrase alone still returns cleanly on such a thread. @@ -102,16 +111,16 @@ Nothing in the output announces the miss: a claim search that finds no claim loo **Match the two-word invariant, never a whole sentence.** The claim body varies by target --- a PR claim says "please hold off on pushing to this branch until I'm done" and an issue claim says "please hold off until I'm done" --- so neither sentence contains the other, and a detector keyed on either one is blind to half the claims. Under the old single-string wording that distinction did not exist, which is exactly why it is easy to carry a whole-sentence matcher across the rename without noticing it has narrowed. -`hold off` is the invariant; `paws off` is its predecessor. +`hold off` is the invariant; `paws off` and `back off` are its predecessors, and a matcher naming only the first predecessor is the failure this very section describes, committed by the section itself. So match the alternation, case-insensitively, everywhere a claim is read: ```bash gh pr view --json comments \ - -q '.comments[] | select(.body | test("hold off|paws off"; "i"))' # READ_PR_COMMENTS + -q '.comments[] | select(.body | test("hold off|paws off|back off"; "i"))' # READ_PR_COMMENTS ``` -Keep the old alternative until no claim under the old wording can plausibly still be live --- which, given the 2-hour rule keys on activity and not on the comment's own age, means until every PR and issue open on 2026-08-24 has closed. +Keep both old alternatives until no claim under a retired wording can plausibly still be live --- which, given the 2-hour rule keys on activity and not on the comment's own age, means until every PR and issue open on 2026-08-24 has closed. Removing it is a deliberate later edit, not tidying to do in passing. **Then check the same comment for a release term, because one release marker contains a claim invariant.** @@ -119,10 +128,11 @@ The retired release wording is `... done --- paws off released.`, which matches The sentence matcher this replaced did not collide, so the collision arrived with the fix. Treat a comment as a release rather than a claim when it also matches `unclaim|released|PR is free|now mergeable`, and derive that list rather than copying it: `grep -rn "unclaim\|released\|PR is free\|now mergeable" skills/ commands/`. -- **Do:** match `hold off|paws off` case-insensitively wherever a claim is read, then exclude the comment if it also carries a release term. -- **Do:** treat the old alternative as load-bearing until the threads carrying it have closed. +- **Do:** match `hold off|paws off|back off` case-insensitively wherever a claim is read, then exclude the comment if it also carries a release term. +- **Do:** treat both old alternatives as load-bearing until the threads carrying them have closed. - **Don't:** read an empty claim search as an unclaimed thread without first confirming the matcher covers both wordings --- the two results are identical. -- **Don't:** drop the back-compat alternative as part of an unrelated change. +- **Don't:** drop a back-compat alternative as part of an unrelated change. +- **Don't:** enumerate the retired wordings from the files you happen to be editing --- `back off` was invisible to exactly that method for a full review round, because the one file that posted it was not one of the six that agreed with each other. **Verify a mid-task "already done" claim against real PR state before trusting or redoing it.** A PR you claimed and are actively driving can still gain commits from a **second, independently-running session** under the same account --- a `` review-comment-reply event can describe work ("Addressed... diff --git a/shared/workflow/disclose-agent-authorship.md b/shared/workflow/disclose-agent-authorship.md index 3995eb260..f77774ad0 100644 --- a/shared/workflow/disclose-agent-authorship.md +++ b/shared/workflow/disclose-agent-authorship.md @@ -59,24 +59,12 @@ The mistake was attributing an error to whichever file the loop happened to be t `MD049` defaults to `consistent`, so it fires only where the document already established asterisk emphasis --- which makes the linter a partial detector here rather than the check. **That `consistent` default cuts the other way too, and it is the sharper hazard.** -The marker is underscore-emphasised, so a marker that escapes its fence into prose becomes the document's FIRST emphasis and sets the expectation to underscore --- whereupon every pre-existing `*asterisk*` in that file starts failing. -Measured here: adding the marker to `skills/migrate-discussion/SKILL.md` at column 0 closed the fence and turned two untouched lines 70 lines below into `MD049` errors, which read as defects in prose nobody had edited. -So the linter does catch this file, and it reports the wrong lines. -So the source keeps its indentation and the query drops its anchor. +The marker is underscore-emphasised, so a marker that escapes its fence into prose becomes the document's first emphasis and sets the expectation to underscore --- whereupon every pre-existing `*asterisk*` in that file starts failing, at line numbers far from the edit. +`skills/migrate-discussion/SKILL.md` has exactly that shape, with two asterisk-emphasised lines around 165. -**The searchable invariant is `(AI agent)`, not the full line.** -`AGENTS.md` says to swap in your own agent's name and keep the rest verbatim, which leaves the literal full line unable to find a Codex- or Gemini-posted comment --- returning nothing, which is indistinguishable from an undisclosed thread. -That is the same failure this rule's sibling diagnoses for the claim wording, one artifact over. -`hooks/require-agent-disclosure.py` already encodes the invariant as `posted by .{0,40}\(ai agent\)`; match that, case-insensitively: - -```bash -grep -rn -i "posted by .*(ai agent)" . -``` - -**The guard cannot see every body, and the gaps are worth knowing.** -`hooks/require-agent-disclosure.py` reads the command text and the `mcp__github__*` comment tools, so it is silent on a body it cannot reach: a `--body-file`, an `--editor` session, an interpolated `$BODY`. -It reports those as an **unreadable** body rather than as a missing marker, so its warning never asserts more than it observed --- but a `--body-file` comment that genuinely omits the marker draws only the weaker note. -`skills/ard/SKILL.md`'s per-round summary is exactly that shape, which is why it states the requirement in its own text rather than relying on the guard. +Do not rely on the linter to catch it, though. +A re-measurement on 2026-08-24 could not reproduce an `MD049` there by the same method that reproduces the other three files' results exactly, so the structural hazard is real and the detector's coverage of it is not established. +Treat marker indentation as something to get right rather than something a check will report. **Where the marker must NOT go: content that is not a comment.** A commit message, a title of any kind, an issue **body**, or a PR body has its own attribution conventions and its own consumers, and a trailing italic line in a commit message corrupts a changelog. diff --git a/skills/claim-pr/SKILL.md b/skills/claim-pr/SKILL.md index b40fa4653..2e834999b 100644 --- a/skills/claim-pr/SKILL.md +++ b/skills/claim-pr/SKILL.md @@ -115,7 +115,8 @@ for d in json.load(sys.stdin): # test resolves the release's thread instead of the claim's. is_release = any(t in body for t in ('unclaim', 'released', 'pr is free', 'now mergeable')) - is_claim = ('hold off' in body or 'paws off' in body) and not is_release + is_claim = (any(t in body for t in ('hold off', 'paws off', 'back off')) + and not is_release) if is_claim and not n.get('resolved'): print(d['id']); break else: continue diff --git a/skills/gi/SKILL.md b/skills/gi/SKILL.md index 9b5ab66c1..f1bc47cb7 100644 --- a/skills/gi/SKILL.md +++ b/skills/gi/SKILL.md @@ -87,7 +87,7 @@ check both explicitly here). gh issue view --json comments --jq '.comments | last | .body' | cat # READ_ISSUE_COMMENTS ``` -Match the two-word invariant `hold off`, or the pre-2026-08-24 `paws off`, case-insensitively --- then **exclude the comment if it also carries a release term** (`unclaim|released|PR is free|now mergeable`), because the retired release wording `... done --- paws off released.` contains `paws off` and would otherwise read as a live claim. +Match the two-word invariant `hold off`, or either retired wording `paws off` / `back off`, case-insensitively --- then **exclude the comment if it also carries a release term** (`unclaim|released|PR is free|now mergeable`), because the retired release wording `... done --- paws off released.` contains `paws off` and would otherwise read as a live claim. See [`claim-pr`](../../shared/workflow/claim-pr.md)'s "Match the two-word invariant". If a live claim stands, skip the issue --- unless the claim has expired: no push or comment on the issue in over 2 hours, per [`claim-pr`](../../shared/workflow/claim-pr.md)'s expiration rule. An expired claim is taken over by posting your own claim comment, never silently. diff --git a/skills/post-merge/SKILL.md b/skills/post-merge/SKILL.md index 4f10c7d68..1c0c11220 100644 --- a/skills/post-merge/SKILL.md +++ b/skills/post-merge/SKILL.md @@ -313,8 +313,8 @@ conflicting PR can sit in `UNKNOWN` and get missed if you filter for and "A stacked PR is the one conflict that intersection cannot attribute". 3. **Check claim status.** Read the most recent comment. - Match the two-word invariant, `hold off` or the pre-2026-08-24 `paws off`, case-insensitively --- never a whole sentence. - The PR and issue claims differ after those two words, and the dash between them is an em-dash in this file's own emitter four steps below, so a quoted prefix misses claims this very skill posts. + Match the two-word invariant, `hold off` or either retired wording `paws off` / `back off`, case-insensitively --- never a whole sentence. + The PR and issue claims differ after those two words, and the dash between them is an em-dash in this file's own claim emitter (step 4), so a quoted prefix misses claims this very skill posts. See [`claim-pr`](../../shared/workflow/claim-pr.md)'s "Match the two-word invariant". **Then check the same comment for a release term, because one release marker contains a claim invariant.** diff --git a/skills/push/SKILL.md b/skills/push/SKILL.md index 7db24f35d..c80123175 100644 --- a/skills/push/SKILL.md +++ b/skills/push/SKILL.md @@ -98,13 +98,13 @@ that's your claim.) ```bash PR=$(gh pr view --json number,headRefName -q .number 2>/dev/null) # VIEW_PR gh pr view "$PR" --json comments \ - -q '.comments[] | select(.body | test("hold off|paws off|unclaim|released|PR is free|now mergeable"; "i")) | "\(.author.login): \(.body)"' # READ_PR_COMMENTS + -q '.comments[] | select(.body | test("hold off|paws off|back off|unclaim|released|PR is free|now mergeable"; "i")) | "\(.author.login): \(.body)"' # READ_PR_COMMENTS ``` The alternation is deliberate, and it covers RELEASES as well as claims. Claims posted before 2026-08-24 say "paws off", and a claim stays live on activity rather than on age, so an old-wording claim can be live right now. The release terms matter because the old wording made them free: `paws off released` contains `paws off`, so one grep surfaced both sides of the exchange. -`claim released` contains neither claim term, so a claim-only query returns the claim and not its release --- and step 3 below asks whether the claim "hasn't been unclaimed", which that output cannot answer. +`claim released` contains neither claim term, so a claim-only query returns the claim and not its release --- and this check asks whether the claim "hasn't been unclaimed", which a claim-only output cannot answer. A released PR would read as live-claimed, and this skill would refuse a legitimate push. Derive the release terms rather than copying this list, which is a snapshot of what the corpus posts today: `grep -rn "unclaim\|released\|PR is free\|now mergeable" skills/ commands/`. A matcher narrowed to the new phrase returns nothing on such a thread, which reads exactly like an unclaimed one --- see [`claim-pr`](../../shared/workflow/claim-pr.md). From 9565d9f99276949bc2ebdef873d08c3899cfc063 Mon Sep 17 00:00:00 2001 From: Douglas Ezra Morrison Date: Mon, 24 Aug 2026 15:12:41 -0700 Subject: [PATCH 11/13] fix(prose): satisfy the new-line-breaks CI check, which I had verified wrong The pre-push review reported 8 lines failing `validate / new-line-breaks`. Running the checker locally returned exit 0, so I nearly rebutted it. The rebuttal would have been wrong, and the way it was wrong is the finding. `.github/workflows/validate.yml` pins the reusable workflow to a SHA in `Morrison-Lab/gha`. That workflow's own step then runs `d-morrison/gha/check-new-line-breaks@v2` -- a different org, a floating tag -- so the pin fixes the caller and nothing about the script that executes. The two differ materially: 340 lines versus 637, no clause-break rule versus one on by default, and on this branch exit 0 versus exit 1 with 8 findings. Reading the pinned copy to predict CI gives the wrong answer confidently, since that artifact exists, is fetchable at the named SHA, and runs clean. That is verify-the-right-artifact's "a checkout for the run" shape, and pr-on-claim already records the same delegation trap for a reusable workflow's trigger conditions. Nothing said it applies to what the workflow RUNS. Filed as #2165, with the measured table and three fix options. The eight lines, all mine, all long with a mid-line semicolon: AGENTS.md 196 and 211, memories/github.md 199, shared/workflow/claim-pr.md 96/114/143, and skills/post-merge/SKILL.md 324/325. Split at their clause boundaries. Most came from my own `scripts/semantic-line-breaks.py --write` runs, which JOIN wrapped lines -- the opposite direction from this check, and it exits 0 either way. So running the local tool to satisfy one convention manufactured failures in the other, and no local signal reported it. Worth knowing before the next reflow pass. Two more surfaced only after the first fix: an `...` inside a quotation reads as a sentence end, so anything following it on the line counts as a second sentence. Rewording the quotations to drop the ellipsis cleared both; a bracketed `[...]` elision did not, since it terminates a sentence too. Also from the same review, non-blocking: - The retired-wording enumeration was wrong in both directions -- `handoff` never said "paws off until I'm done" (it said "still claimed, paws off."), and `gip`, `iterate` and the orchestrator were omitted. Corrected, two lines above the Don't that forbids enumerating from the files in front of you. - "There were three retired wordings" was uncountable: there are two retired two-word invariants, and the third is a wording only if claim SENTENCES are counted, which the next subsection tells the reader not to do. Reworded to name the emitters and the two invariants. Verified with the script CI actually runs (`d-morrison/gha@v2`, NLB_FAIL=true, base = merge-base): "No lines missing semantic breaks", exit 0. Plus 41/41 hook suites (110 in this one), markdownlint 0, links, skills, hook-catalog, hook-output-shape, vendored drift, context-closure pin, tool-mappings idempotent. --- AGENTS.md | 7 +++++-- memories/github.md | 3 ++- shared/workflow/claim-pr.md | 17 +++++++++++------ skills/post-merge/SKILL.md | 6 ++++-- 4 files changed, 22 insertions(+), 11 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 61a669854..4c0204fa6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -193,7 +193,9 @@ Check any replacement marker against that script's `REVIEW_BODY_MARKERS` and `RE Scope: comment bodies, on every surface --- claims, releases, status notes, review replies, self-reviews, issue comments filed on the user's behalf. Not commit messages, not titles, not issue bodies, not PR bodies, each of which has its own attribution convention. -Two exemptions: a comment another machine parses as a command (`@dependabot rebase`), where the test is the audience rather than the length; and a comment posted under a genuine bot token, where the forge already reports `type: Bot` and the marker adds nothing. +Two exemptions. +A comment another machine parses as a command (`@dependabot rebase`), where the test is the audience rather than the length. +And a comment posted under a genuine bot token, where the forge already reports `type: Bot` and the marker adds nothing. - **Do:** append the marker to every agent-posted comment, including ones whose prose already identifies the session. - **Don't:** use the robot emoji in the marker, and don't read "the account holder knows an agent is running" as making the disclosure unnecessary --- the reader is whoever finds the thread later. @@ -208,7 +210,8 @@ Two exemptions: a comment another machine parses as a command (`@dependabot reba When asked to implement, edit, or write up a change on a feature branch, do not stop at an uncommitted worktree. Complete the delivery cycle: create the applicable tracking issue when issue-first workflow applies, commit the scoped changes, run local adversarial self-review to a clean verdict, push the branch, open or update its Pull Request, request AI review after the final push, and drive CI and review findings to a clean result. -This does not grant merge authority; the strict merge policy below still applies. +This does not grant merge authority. +The strict merge policy below still applies. ## Every self-review is an adversarial review by a separate subagent diff --git a/memories/github.md b/memories/github.md index 127e4a6ea..e78390373 100644 --- a/memories/github.md +++ b/memories/github.md @@ -196,7 +196,8 @@ in [`github-repo-transfers.md`](github-repo-transfers.md). `gh` authenticates as whatever account is logged in locally (often the user's own, e.g. seen as `dem-extra1` on `Lacaedemon/sparta`), so when an agent (or a dispatched subagent) replies to an inline review comment on the user's behalf, `gh api repos///pulls//reviews` lists it as a `COMMENTED` review authored by the user — indistinguishable at a glance from the user genuinely opening the PR in a browser and typing a reply themselves. **Since 2026-08-24 the fix is on the posting side:** every comment an agent posts carries a trailing `_Posted by Claude Code (AI agent) --- not written by a human._` marker, so the body says what the author field cannot. See [`disclose-agent-authorship`](../shared/workflow/disclose-agent-authorship.md). - That makes agent-authored comments identifiable **going forward**; comments posted before that date carry no marker, so this warning still governs when auditing older threads. + That makes agent-authored comments identifiable **going forward**. + Comments posted before that date carry no marker, so this warning still governs when auditing older threads. Before treating an unexpected review entry as a signal that the human intervened, check whether its body/inline-comment content reads like the agent's own scripted reply (referencing a specific commit SHA, restating verification numbers) rather than free-form human commentary — if so, it's the session's own tooling, not new human input. **The same ambiguity runs the other way, and there it arrives as a positive claim rather than an inference you might draw.** An automated reviewer reading the PR's own history sees that same bot-account commit and can describe it *in its review body* as the work of a human, e.g. "that finding was confirmed and fixed by a human reviewer (`dem-extra1`) in commit ``", stating as fact something no API field asserts. diff --git a/shared/workflow/claim-pr.md b/shared/workflow/claim-pr.md index 42ecade49..0f0bd9e62 100644 --- a/shared/workflow/claim-pr.md +++ b/shared/workflow/claim-pr.md @@ -93,7 +93,9 @@ longer than that, reassert your claim.") **Every detector of a claim matches the OLD wording as well as the new one, and dropping the old alternation is the one edit that fails silently.** There were **three** retired wordings, not one, and enumerating them from the file in front of you is how the third was missed for a whole review round. -`claim-pr`, `gi`, `st`, `pr-on-claim`, `post-merge` and `handoff` all said "paws off until I'm done"; [`ardi`](ardi.md) --- the corpus's highest-traffic claim emitter, run on every PR in every repo --- said "back off until done", and had done since 2026-06-17. +Most emitters carried the `paws off` invariant --- `claim-pr`, `gi`, `st`, `gip`, `pr-on-claim`, `post-merge`, `iterate`, `handoff` (as "still claimed, paws off.") and the orchestrator (as "paws off until done"). +[`ardi`](ardi.md) did not. +It said "back off until done", and had done since 2026-06-17 --- and it is the corpus's highest-traffic claim emitter, run on every PR in every repo. All three now read "please hold off ...". Derive that set from history rather than from the current tree, which no longer contains any of them: @@ -111,7 +113,8 @@ Nothing in the output announces the miss: a claim search that finds no claim loo **Match the two-word invariant, never a whole sentence.** The claim body varies by target --- a PR claim says "please hold off on pushing to this branch until I'm done" and an issue claim says "please hold off until I'm done" --- so neither sentence contains the other, and a detector keyed on either one is blind to half the claims. Under the old single-string wording that distinction did not exist, which is exactly why it is easy to carry a whole-sentence matcher across the rename without noticing it has narrowed. -`hold off` is the invariant; `paws off` and `back off` are its predecessors, and a matcher naming only the first predecessor is the failure this very section describes, committed by the section itself. +`hold off` is the invariant. +`paws off` and `back off` are its two predecessors, and a matcher naming only the first is the failure this very section describes, committed by the section itself. So match the alternation, case-insensitively, everywhere a claim is read: @@ -135,12 +138,14 @@ Treat a comment as a release rather than a claim when it also matches `unclaim|r - **Don't:** enumerate the retired wordings from the files you happen to be editing --- `back off` was invisible to exactly that method for a full review round, because the one file that posted it was not one of the six that agreed with each other. **Verify a mid-task "already done" claim against real PR state before trusting or redoing it.** -A PR you claimed and are actively driving can still gain commits from a **second, independently-running session** under the same account --- a `` review-comment-reply event can describe work ("Addressed... -Pushed in ``") that this session never did. +A PR you claimed and are actively driving can still gain commits from a **second, independently-running session** under the same account. +A `` review-comment-reply event can describe work this session never did, in the form "Addressed, pushed in ``". Don't assume it's fabricated or injected, and don't reflexively redo the same fix: cross-check the PR's actual commit list (`gh pr view --json commits` / `pull_request_read` `get_commits`) and review threads before either (a) trusting the claim, or (b) starting the same fix yourself. If a commit with that SHA genuinely exists, authored close to when the event arrived, treat it as confirmation a live parallel session owns this PR right now --- stop pushing further speculative fixes yourself, and, if genuinely in doubt, ask whether to keep driving or step back, rather than racing the other session's pushes. -This gap is distinct from the initial claim check above: it's not about claiming a PR before starting, but about **re-verifying you're still the sole active driver** once work has been under way for a while --- especially when you picked up the PR mid-session (e.g. by answering a diagnostic question about it) rather than through the normal claim-then-branch flow, so no fresh claim check ever ran right before you started pushing. (`d-morrison/gha#286`, 2026-07-24: a webhook event delivered a review-comment reply attributed to `d-morrison` reading exactly like a Claude-authored reply, claiming a fix "Addressed... -Pushed in 3fb8c5b" that this session hadn't made; verified real via `get_commits` before proceeding --- a second live session, not injection.) +This gap is distinct from the initial claim check above: it's not about claiming a PR before starting, but about **re-verifying you're still the sole active driver** once work has been under way for a while --- especially when you picked up the PR mid-session (e.g. by answering a diagnostic question about it) rather than through the normal claim-then-branch flow, so no fresh claim check ever ran right before you started pushing. + +(`d-morrison/gha#286`, 2026-07-24: a webhook event delivered a review-comment reply attributed to `d-morrison`, reading exactly like a Claude-authored reply and claiming a fix this session hadn't made, worded "Addressed, pushed in 3fb8c5b". +It was verified real via `get_commits` before proceeding --- a second live session, not injection.) **The git-level variant of that check: a rejected push whose remote commit is byte-for-byte what you were about to push.** The section above covers a *comment* claiming work was done. diff --git a/skills/post-merge/SKILL.md b/skills/post-merge/SKILL.md index 1c0c11220..b287bcedd 100644 --- a/skills/post-merge/SKILL.md +++ b/skills/post-merge/SKILL.md @@ -321,8 +321,10 @@ conflicting PR can sit in `UNKNOWN` and get missed if you filter for The retired release wording is `... done --- paws off released.`, which matches `paws off` --- so the invariant that fixes one bug introduces another, and this one fails the safe way round: a released PR reads as claimed, the sweep skips it, and the conflict is never resolved with nothing reporting why. The sentence matcher this replaced did not collide, so the collision arrived with the fix. Treat the comment as a release, not a claim, if it also matches `unclaim|released|PR is free|now mergeable`. - If a live claim stands --- a push or comment within the last 2 hours --- skip the PR; another session owns it. - An expired claim (over 2 idle hours) no longer blocks; take over with a fresh claim comment of your own, per [`claim-pr`](../../shared/workflow/claim-pr.md)'s expiration rule. + If a live claim stands --- a push or comment within the last 2 hours --- skip the PR. + Another session owns it. + An expired claim (over 2 idle hours) no longer blocks. + Take over with a fresh claim comment of your own, per [`claim-pr`](../../shared/workflow/claim-pr.md)'s expiration rule. 4. **Claim it.** ```bash gh pr comment --body "Working on this — please hold off on pushing to this branch until I'm done. From 5aa36bbe30e09188ebe4a74c0ef1d776ae905604 Mon Sep 17 00:00:00 2001 From: Douglas Ezra Morrison Date: Mon, 24 Aug 2026 15:26:12 -0700 Subject: [PATCH 12/13] fix(claim-pr): strike `iterate` from the emitter list; widen the derivation One blocking finding on 9565d9f9, and it landed in the paragraph about this exact mistake. The correction that repeated the error it corrects The previous commit widened the retired-wording enumeration from six emitters to nine, and one of the three it added was `iterate` -- a twelve-line alias stub whose whole body points at `ardi`. It carries no claim wording at all, and it IS `ardi`, which the next two lines name as the emitter that said something else. So the sentence put the same skill on both sides of its own contrast, inside the passage whose subject is that enumerating from the files in front of you fails silently. `gip` and the orchestrator, the other two additions, are genuine. That commit was billed as a line-break fix. Expanding a factual enumeration inside it is how the error got in without review attention. Also from the same round, non-blocking - "Three retired wordings" against "two predecessors" was a granularity shift between full strings and two-word invariants, never stated, and "All three now read" lost its antecedent when the sentence was split into two groups. Both now count invariants, which is the unit every matcher uses. - The derivation command offered in place of the enumeration missed two of the emitters the prose names: `handoff` opens a multi-line body so its closing quote is on another line, and the orchestrator is a `.py` outside the `skills/*/SKILL.md` pathspec. Widened the pathspec and matched the opening quote only. Verified it now returns both retired invariants. - Filed #2172: `clean-branches` checks for work claims with an EMPTY code block, so it has no matcher and no release-term handling. It was invisible to this branch's sweep by construction -- it names no wording, so a wording-based grep cannot find it -- and its failure direction is a deletion rather than a skip. Verified with the script CI actually runs (`d-morrison/gha@v2`, not the pinned `Morrison-Lab/gha` copy, per #2165): no lines missing semantic breaks. Plus 110/110 hook tests, links, skills, markdownlint 0, context-closure pin, and a grep confirming no emitter anywhere still posts a retired wording. --- shared/workflow/claim-pr.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/shared/workflow/claim-pr.md b/shared/workflow/claim-pr.md index 0f0bd9e62..adbf34a02 100644 --- a/shared/workflow/claim-pr.md +++ b/shared/workflow/claim-pr.md @@ -92,17 +92,18 @@ issue claims last 2 hours from the most recent push or comment; if it's been longer than that, reassert your claim.") **Every detector of a claim matches the OLD wording as well as the new one, and dropping the old alternation is the one edit that fails silently.** -There were **three** retired wordings, not one, and enumerating them from the file in front of you is how the third was missed for a whole review round. -Most emitters carried the `paws off` invariant --- `claim-pr`, `gi`, `st`, `gip`, `pr-on-claim`, `post-merge`, `iterate`, `handoff` (as "still claimed, paws off.") and the orchestrator (as "paws off until done"). +There were **two** retired invariants, not one, and enumerating them from the file in front of you is how the second was missed for a whole review round. +Most emitters carried the `paws off` invariant --- `claim-pr`, `gi`, `st`, `gip`, `pr-on-claim`, `post-merge`, `handoff` (as "still claimed, paws off.") and the orchestrator (as "paws off until done"). [`ardi`](ardi.md) did not. It said "back off until done", and had done since 2026-06-17 --- and it is the corpus's highest-traffic claim emitter, run on every PR in every repo. -All three now read "please hold off ...". +Both now read "please hold off ...". -Derive that set from history rather than from the current tree, which no longer contains any of them: +Derive that set from history rather than from the current tree, which no longer contains any of them. +Widen the pathspec past `skills/` and match the opening quote only, since `handoff` opens a multi-line body whose closing quote is on another line, and the orchestrator is executable code rather than a skill: ```bash -git log -p --all -- 'skills/*/SKILL.md' 'shared/workflow/claim-pr.md' \ - | grep -oiE '^\+.*--(body|message) "[^"]*(off|claim)[^"]*"' | sort -u +git log -p --all -- 'skills/**' 'shared/**' 'commands/**' 'scripts/**' \ + | grep -oiE '^\+.*--(body|message) "[^"]*(off until|claim)' | sort -u ``` Claims posted before that are still sitting on open PRs and issues, and a claim stays live on activity rather than on age --- so a thread claimed under the old wording and pushed to this morning is live right now. @@ -164,8 +165,7 @@ git show -s --format=%P origin/ # its parents An identical tree plus identical parents means the two merges are the same merge, so the right action is `git reset --hard origin/`. -- **Do:** compare trees and parents before deciding what a rejected push - means. +- **Do:** compare trees and parents before deciding what a rejected push means. - **Do:** discard your local merge with `git reset --hard origin/` once both match. - **Don't:** re-merge reflexively on a rejected push --- that is what produces the redundant merge commit. - **Don't:** force-push over the other session's commit. From 594fdce9fd29f392c19bf40ee9dcd5819db9f930 Mon Sep 17 00:00:00 2001 From: Douglas Ezra Morrison Date: Mon, 24 Aug 2026 15:45:45 -0700 Subject: [PATCH 13/13] fix(claim-pr): key the derivation on the wordings, not the --body flag One blocking finding on 5aa36bbe, in the same paragraph as the last two. The derivation offered in place of an enumeration could not reach two of the emitters the prose beside it names. Keying on `--(body|message) "` looks tighter and misses exactly the emitters least like the others: `gip` states its claim as quoted prose inside a worker brief, and the orchestrator builds the body as a Python f-string on a different line from the flag. So a reader following the instruction to derive rather than enumerate would have re-derived a six-emitter set and dropped those two -- the failure the section's own Don't describes. Now keyed on the wordings across the widened pathspec. Verified per emitter: the new pattern returns hits for the orchestrator, `gip`, `pr-on-claim`, `handoff`, `claim-pr` and `ardi`; the old one returned zero for the first two. The cost is that it also returns matcher and prose lines, so it wants a skim rather than a count -- stated in the text rather than left for the reader to discover. Also corrected the two stale "six" counts the enumeration fix left behind: seven skills carried `paws off`, eight emitters counting the orchestrator. `commands/release-pr.md`'s was added by this branch, so it was a new false count rather than an inherited one. Verified with the script CI actually runs: no lines missing semantic breaks. Plus 110/110 hook tests, links, skills, markdownlint 0, hook-catalog, hook-output-shape, context-closure pin. --- commands/release-pr.md | 2 +- shared/workflow/claim-pr.md | 11 ++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/commands/release-pr.md b/commands/release-pr.md index 53c97ef8c..6974ad2ab 100644 --- a/commands/release-pr.md +++ b/commands/release-pr.md @@ -34,7 +34,7 @@ If only one positional arg is given, treat it as `pr_number`. - the most recent claim/release exchange is an unmatched claim that hasn't yet been followed by a release. **Match the two-word invariant `hold off` (case-insensitively), never a full sentence** --- the PR claim reads `please hold off on pushing to this branch until I'm done` while the issue claim reads `please hold off until I'm done`, so neither sentence is a substring of the other and a matcher keyed on either one misses the other. - **Also match both retired invariants, `paws off` and `back off`** (the second was `ardi`'s, and is easy to miss because six other skills agreed on the first): claims posted before 2026-08-24 are still live on open PRs, since a claim expires on activity rather than on age, and a matcher narrowed to the new wording returns nothing on them --- indistinguishable from no claim at all. + **Also match both retired invariants, `paws off` and `back off`** (the second was `ardi`'s, and is easy to miss because seven other skills agreed on the first): claims posted before 2026-08-24 are still live on open PRs, since a claim expires on activity rather than on age, and a matcher narrowed to the new wording returns nothing on them --- indistinguishable from no claim at all. So the claim test is `test("hold off|paws off|back off"; "i")`. Treat **any** of these as a release marker --- four the corpus posts today plus one retired form still sitting on open PRs, and enumerating only this command's own is what makes it post a stray release over somebody else's completed handover: - this command's `… done --- claim released.` diff --git a/shared/workflow/claim-pr.md b/shared/workflow/claim-pr.md index adbf34a02..96d6ab879 100644 --- a/shared/workflow/claim-pr.md +++ b/shared/workflow/claim-pr.md @@ -99,12 +99,17 @@ It said "back off until done", and had done since 2026-06-17 --- and it is the c Both now read "please hold off ...". Derive that set from history rather than from the current tree, which no longer contains any of them. -Widen the pathspec past `skills/` and match the opening quote only, since `handoff` opens a multi-line body whose closing quote is on another line, and the orchestrator is executable code rather than a skill: +Match the **wordings**, not the `--body` flag. +Keying on the flag looks tighter and reaches neither `gip`, which states its claim as quoted prose inside a worker brief, nor the orchestrator, which builds the body as a Python f-string on a different line from the flag --- the two emitters least like the others, and so the two a flag-shaped search is least able to find. +Widen the pathspec past `skills/` for the same reason. ```bash git log -p --all -- 'skills/**' 'shared/**' 'commands/**' 'scripts/**' \ - | grep -oiE '^\+.*--(body|message) "[^"]*(off until|claim)' | sort -u + | grep -oiE '^\+.*(paws off|back off|hold off)[^"]*' | sort -u ``` + +That returns the matcher and prose lines too, so it wants a skim rather than a count --- the deliberate trade for a search that cannot miss an emitter because of how it happened to spell the call. + Claims posted before that are still sitting on open PRs and issues, and a claim stays live on activity rather than on age --- so a thread claimed under the old wording and pushed to this morning is live right now. A detector narrowed to the new phrase alone still returns cleanly on such a thread. @@ -136,7 +141,7 @@ Treat a comment as a release rather than a claim when it also matches `unclaim|r - **Do:** treat both old alternatives as load-bearing until the threads carrying them have closed. - **Don't:** read an empty claim search as an unclaimed thread without first confirming the matcher covers both wordings --- the two results are identical. - **Don't:** drop a back-compat alternative as part of an unrelated change. -- **Don't:** enumerate the retired wordings from the files you happen to be editing --- `back off` was invisible to exactly that method for a full review round, because the one file that posted it was not one of the six that agreed with each other. +- **Don't:** enumerate the retired wordings from the files you happen to be editing --- `back off` was invisible to exactly that method for a full review round, because the one file that posted it was not one of the seven that agreed with each other. **Verify a mid-task "already done" claim against real PR state before trusting or redoing it.** A PR you claimed and are actively driving can still gain commits from a **second, independently-running session** under the same account.