Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .github/actions/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,15 @@ Questions published before the `<details>` shape existed fall back to a single-l
A question that is already open is never re-asked; the original stays the copy the author
answers.

**Self-authored pull requests are skipped, not reviewed.** When the PR was
opened by the reviewer's own identity (the app behind `github_identity_token`,
e.g. `mega-maxwell[bot]` — release candidates, settle PRs, dependency bumps),
`prepare` routes the round to `skip` before any model step and the status
comment says so ("⏭️ Review skipped … opened by mega-maxwell[bot], the
reviewer's own identity"). This is decided from the PR author, independently
of `allowed_bots`, which still governs which bots may *trigger* a review of
someone else's PR.

### Inspecting a review run

The published review says what the reviewer concluded. These say how it got there, and they
Expand Down
83 changes: 74 additions & 9 deletions .github/actions/claude-pr-review/review_pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,25 @@ def is_bot_login(
return canonical_login(login) in reviewer_logins(publisher_login)


def self_authored_pull(
pull: dict[str, Any],
*,
publisher_login: str | None,
) -> str | None:
"""The PR author's login when it is the reviewer's own identity, else None.

A pull request the reviewer opened itself — release candidates, settle
PRs, dependency bumps — is not a review target: the app would be reviewing
its own work, and claude-code-action refuses bot-initiated events anyway.
Deciding it here, before any model step, turns that refusal into an
announced skip instead of a failed round.
"""
login = str((pull.get("user") or {}).get("login") or "")
if login and is_bot_login(login, publisher_login=publisher_login):
return login
return None


def is_stateful_review(
login: str | None,
body: str | None,
Expand Down Expand Up @@ -1372,7 +1391,14 @@ def prepare(args: argparse.Namespace) -> None:
and comparison.get("status") in {"ahead", "identical"}
and comparison_complete
)
if args.event_name == "issue_comment":
self_author = self_authored_pull(pull, publisher_login=publisher_login)
if self_author:
mode = "skip"
mode_reason = (
"pull request opened by the reviewer's own identity "
f"({self_author})"
)
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
# incremental round when the reviewer is still waiting on something and
Expand Down Expand Up @@ -1492,6 +1518,7 @@ def prepare(args: argparse.Namespace) -> None:
"review_scope": {
"mode": mode,
"reason": mode_reason,
"self_authored_by": self_author,
"previous_reviewed_head": previous_head,
"current_head": current_head,
"changed_paths": scope_paths,
Expand Down Expand Up @@ -1523,6 +1550,18 @@ def prepare(args: argparse.Namespace) -> None:
scope_text=scope_label(input_value),
phase="in_progress",
)
elif self_author:
# A self-authored skip is final for the PR, so announcing it strands
# nothing: the comment says why no round will ever run here.
input_value["sticky_comment_id"] = set_sticky_phase(
repository=args.repository,
pull_request=args.pull_request,
manifest=manifest,
sticky_comment_id=sticky_comment_id,
scope_text=scope_label(input_value),
phase="skipped",
reason=f"opened by {self_author}, the reviewer's own identity",
)
(state_dir / "review-input.json").write_text(
json.dumps(input_value, indent=2, sort_keys=True) + "\n",
encoding="utf-8",
Expand Down Expand Up @@ -2743,6 +2782,15 @@ def render_status_body(
" review below, once the round finishes. Anything listed below is"
" carried over from earlier rounds."
)
elif phase == "skipped":
status_title = "⏭️ Review skipped"
reason = public_text(summary.get("reason"), maximum=200)
progress_line = f"Not reviewing {scope_text} · updated {utc_now()}"
detail_line = (
"This pull request is not a review target"
f"{': ' + reason if reason else ''}. No review round runs for it."
" Anything listed below is from an earlier round."
)
elif phase == "failed":
status_title = "🛠️ Review did not finish"
reason = public_text(summary.get("reason"), maximum=200)
Expand Down Expand Up @@ -3122,6 +3170,28 @@ def render_outcomes(outcomes: dict[str, str]) -> str:
return "\n".join(rows)


def report_status_line(
*,
stale: bool,
mode: str,
published: bool,
scope: dict[str, Any],
) -> str:
"""One-line outcome for the job summary and the log."""
if stale:
return "⚠️ Review output was discarded because the PR head changed."
if mode == "skip" and scope.get("self_authored_by"):
return (
"⏭️ Review skipped: the pull request was opened by "
f"{scope['self_authored_by']}, the reviewer's own identity."
)
if mode == "skip":
return "⏭️ The current PR head was already reviewed."
if published:
return "✅ Review completed and publication succeeded."
return "✅ Review completed."


def close_out_sticky(state_dir: Path, *, reason: str | None = None) -> None:
"""Move an announced-but-unpublished round out of the in-progress phase.

Expand Down Expand Up @@ -3225,14 +3295,9 @@ def report(args: argparse.Namespace) -> None:
else None
),
)
if stale:
status = "⚠️ Review output was discarded because the PR head changed."
elif mode == "skip":
status = "⏭️ The current PR head was already reviewed."
elif published:
status = "✅ Review completed and publication succeeded."
else:
status = "✅ Review completed."
status = report_status_line(
stale=stale, mode=mode, published=published, scope=scope
)
scope_reason = scope.get("reason") or "unknown"
status_summary = payload.get("status_summary") or {}
rows = [
Expand Down
60 changes: 60 additions & 0 deletions .github/actions/claude-pr-review/test_review_pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -1778,6 +1778,66 @@ def test_in_progress_status_keeps_prior_open_items_visible(self):
self.assertIn("Open questions awaiting an answer:", body)
self.assertIn("carried over from earlier rounds", body)

def test_self_authored_pull_is_the_reviewer_identity(self):
pull = {"user": {"login": "mega-maxwell[bot]"}}
self.assertEqual(
pipeline.self_authored_pull(pull, publisher_login="mega-maxwell[bot]"),
"mega-maxwell[bot]",
)
self.assertEqual(
pipeline.self_authored_pull(pull, publisher_login="mega-maxwell"),
"mega-maxwell[bot]",
)
self.assertIsNone(
pipeline.self_authored_pull(
{"user": {"login": "someone"}}, publisher_login="mega-maxwell[bot]"
)
)
self.assertIsNone(
pipeline.self_authored_pull({}, publisher_login="mega-maxwell[bot]")
)

def test_skipped_status_explains_the_self_authored_pull(self):
payload = {
"repository": "megaeth-labs/example",
"pull_request": 7,
"status_summary": {
"scope_text": "head `bbbbbbbb`",
"phase": "skipped",
"reason": "opened by mega-maxwell[bot], the reviewer's own identity",
},
}

body = pipeline.render_status_body(payload, {"findings": {}})

self.assertIn("⏭️ Review skipped", body)
self.assertIn("Not reviewing head `bbbbbbbb`", body)
self.assertIn("opened by mega-maxwell[bot], the reviewer's own identity", body)
self.assertNotIn("did not finish", body)
self.assertNotIn("Re-run the workflow", body)

def test_report_status_line_distinguishes_self_authored_skips(self):
self.assertIn(
"opened by mega-maxwell[bot], the reviewer's own identity",
pipeline.report_status_line(
stale=False,
mode="skip",
published=False,
scope={"self_authored_by": "mega-maxwell[bot]"},
),
)
self.assertEqual(
pipeline.report_status_line(
stale=False, mode="skip", published=False, scope={}
),
"⏭️ The current PR head was already reviewed.",
)
self.assertTrue(
pipeline.report_status_line(
stale=True, mode="full", published=True, scope={}
).startswith("⚠️")
)

def test_failed_status_retires_the_in_progress_phase(self):
payload = {
"repository": "megaeth-labs/example",
Expand Down
Loading