diff --git a/.github/actions/CLAUDE-CI.md b/.github/actions/CLAUDE-CI.md index ba0e858..d8d184d 100644 --- a/.github/actions/CLAUDE-CI.md +++ b/.github/actions/CLAUDE-CI.md @@ -198,18 +198,26 @@ on: pull_request: types: [opened, synchronize, ready_for_review, reopened] issue_comment: - types: [created] + # `edited` matters as much as `created`: an agent that answers review + # feedback posts a placeholder the moment it picks the work up and + # rewrites that same comment into the real answer minutes later, so the + # answer only ever arrives as an edit. Any job that must not re-fire on an + # edit (an `@claude` mention job, say) pins `github.event.action == + # 'created'` in its own `if`. + types: [created, edited] jobs: pr-review: # Any PR comment except the reviewer's own status comment, which is posted - # by the CI app and would otherwise retrigger the review. Other bots are - # allowed — their comments may answer a question or push back on a finding. - # Replace mega-maxwell[bot] with your reviewer app's login. + # by the CI app and rewritten in place every round — it would otherwise + # retrigger the review on both events. Other bots are allowed — their + # comments may answer a question or push back on a finding. Replace + # mega-maxwell[bot] with your reviewer app's login. if: >- github.event_name != 'issue_comment' || (github.event.issue.pull_request != null && - github.event.comment.user.login != 'mega-maxwell[bot]') + github.event.comment.user.login != 'mega-maxwell[bot]' && + !contains(github.event.comment.body, 'claude-review:skip')) concurrency: # issue_comment payloads carry issue.number, not pull_request.number. group: claude-pr-review-${{ github.event.pull_request.number || github.event.issue.number }} @@ -218,7 +226,11 @@ jobs: The action gates the round cheaply so routine chatter does not spend a review: on an `issue_comment` event, `prepare` skips unless the PR still has an **open question or open -finding** in the manifest (something a comment could answer, justify, or invalidate). When it +finding** in the manifest (something a comment could answer, justify, or invalidate). It also +skips any comment whose body contains `claude-review:skip` — the opt-out marker a bot puts on +a placeholder it intends to rewrite, so the round lands on the rewrite that carries the +content rather than on the "working on it" it replaces. The `if` above declines the same +comment one step earlier, before a runner starts; the action enforces it either way. When it does run it is an incremental round that reuses the same sticky-comment manifest — so the reviewer keeps its full prior context, unlike a fresh `@claude` session — and it runs on the cheaper incremental model tier. If the comment turns out not to change anything, the publisher diff --git a/.github/actions/claude-pr-review/action.yml b/.github/actions/claude-pr-review/action.yml index f040880..4b67a25 100644 --- a/.github/actions/claude-pr-review/action.yml +++ b/.github/actions/claude-pr-review/action.yml @@ -104,6 +104,10 @@ runs: REVIEW_DEPTH: ${{ inputs.review_depth }} PREMORTEM: ${{ inputs.premortem }} STATE_DIR: ${{ github.workspace }}/.pr-review + # Passed through the environment, never interpolated into the script: + # the body is attacker-controlled text. prepare only tests it for the + # `claude-review:skip` marker a bot puts on a placeholder comment. + TRIGGER_COMMENT_BODY: ${{ github.event.comment.body }} run: | set -euo pipefail python3 "$GITHUB_ACTION_PATH/review_pipeline.py" prepare \ diff --git a/.github/actions/claude-pr-review/review_pipeline.py b/.github/actions/claude-pr-review/review_pipeline.py index d6f0428..15e2d61 100644 --- a/.github/actions/claude-pr-review/review_pipeline.py +++ b/.github/actions/claude-pr-review/review_pipeline.py @@ -33,6 +33,12 @@ r"" ) QUESTION_DISPOSITIONS = {"open", "answered", "withdrawn"} +# A comment carrying this marker never drives a review round. It exists for +# bots that announce work before they have anything to say — post a +# placeholder, then rewrite it in place once the real answer is ready. The +# placeholder is worth no round; the rewrite replaces the whole body, marker +# included, so the resulting `edited` event is a normal comment again. +COMMENT_SKIP_MARKER = "claude-review:skip" CONVERSATION_MAX_ENTRIES = 120 CONVERSATION_MAX_BODY_CHARS = 6_000 CONVERSATION_MAX_TOTAL_BODY_CHARS = 96_000 @@ -1138,6 +1144,19 @@ def write_github_output(values: dict[str, Any]) -> None: output.write(f"{key}={value}\n") +def comment_round_is_suppressed(body: str | None) -> bool: + """True when a triggering comment opts out of driving a review round. + + An agent that answers review feedback typically posts a placeholder + ("Looking into this...") the moment it picks the work up and edits that + same comment into the real answer minutes later. Without the marker the + reviewer spends a round reconciling the placeholder and never sees the + answer, because the answer arrives as an edit of an already-reviewed + comment. Marking the placeholder moves the round to where the content is. + """ + return COMMENT_SKIP_MARKER in (body or "") + + def manifest_has_open_items(manifest: dict[str, Any]) -> bool: """True when the manifest still has an open question or open finding. @@ -1398,6 +1417,11 @@ def prepare(args: argparse.Namespace) -> None: "pull request opened by the reviewer's own identity " f"({self_author})" ) + elif args.event_name == "issue_comment" and comment_round_is_suppressed( + os.environ.get("TRIGGER_COMMENT_BODY", "") + ): + mode = "skip" + mode_reason = "triggering comment carries the no-review marker" elif args.event_name == "issue_comment": # A comment never adds code to review — it can only answer an open # question or justify/invalidate an open finding. Run a reconcile-only diff --git a/.github/actions/claude-pr-review/test_review_pipeline.py b/.github/actions/claude-pr-review/test_review_pipeline.py index d96381c..627df12 100644 --- a/.github/actions/claude-pr-review/test_review_pipeline.py +++ b/.github/actions/claude-pr-review/test_review_pipeline.py @@ -170,6 +170,35 @@ def test_manifest_has_open_items_gates_comment_rounds(self): ) ) + def test_placeholder_comments_do_not_spend_a_review_round(self): + # An agent that answers feedback posts "looking into this", then edits + # that comment into the answer. The placeholder carries the marker and + # is worth no round; the rewrite replaces the whole body, marker + # included, so the edit that carries the answer does drive one. + self.assertTrue( + pipeline.comment_round_is_suppressed( + ":mag: Looking into this...\n\n" + ) + ) + self.assertFalse( + pipeline.comment_round_is_suppressed( + "I don't have evidence of such an auth failure pattern." + ) + ) + self.assertFalse(pipeline.comment_round_is_suppressed(None)) + self.assertFalse(pipeline.comment_round_is_suppressed("")) + + def test_action_reads_trigger_comment_body_from_the_environment(self): + action = Path(pipeline.__file__).with_name("action.yml").read_text( + encoding="utf-8" + ) + # The body is attacker-controlled text, so it reaches prepare as an + # environment variable and never as shell script interpolation. + self.assertIn( + "TRIGGER_COMMENT_BODY: ${{ github.event.comment.body }}", action + ) + self.assertNotIn("github.event.comment.body }}\" \\", action) + def test_action_wires_comment_trigger_inputs(self): action = Path(pipeline.__file__).with_name("action.yml").read_text( encoding="utf-8"