Skip to content

bug: Start and inform a review from the @codex review comment #29

Description

@idy

Background

Two defects sit on the same path — the PR comment that asks for a review.

The comment does not start a review. The resolve job gates
issue_comment events with /^@codex(?:\s+review(?:\s+[\s\S]+)?)?\s*$/i. The
^ anchor applies to the whole trimmed comment, so the request only counts
when the comment begins with @codex. A comment that describes what was
pushed and then ends with @codex review on its own line never matches:
eligible becomes false, and start, review, publish, and finalize
are all skipped. No error is raised and no comment is posted, so from the
requester's side the workflow looks like it simply did not run.

Confirmed live on GizClaw/gizclaw#1084, which consumes this workflow at
@latest: runs 33785352754, 33785379065, and 33786877584 each show
review / resolve succeeding with review / start skipped. The PR was open
and not a draft at the time, and issue_comment is a trusted base event, so
the trigger expression is the only remaining cause. Two pushes went unreviewed.

The expression is also untestable where it lives — inside an
actions/github-script block, with no case table and no bot-author guard.

The comment never reaches the reviewer either. The discussion step already
collects the last 20 PR comments and a trigger_comment_id into
PR_CONTEXT_FILE, but nothing reads context.comments. prStageSnapshot()
sends only repository, number, title, body, and linked Issues; the Issue stage
sends Issue snapshots; the code stage sends the diff chunk and Issue context.
The collected comments are dead data. The code-stage prompt already declares
"every ... discussion comment" untrusted input, which shows the wiring was
intended and never completed.

The practical cost is visible in the same PR: Codex could not see the author's
statement of what the push changed, the validation the author had already run,
the disclosures the author volunteered, or any focus given after
@codex review. Each round it re-derived intent from the diff alone. The
600-character clip would have truncated both real comments (1,180 and 899
characters) mid-sentence anyway.

Goal

  • Accept @codex or @codex review <focus> on its own line anywhere in a PR
    comment.
  • Keep the gate tight: do not start a review from an incidental mention, from
    quoted trigger text, or from a comment authored by an app.
  • Move the predicate into a tested module beside the other shared review
    scripts.
  • Give the code review stage the recent PR discussion as untrusted background,
    with the triggering comment identified and kept intact.
  • Preserve zero-token evidence reuse: discussion must not enter any
    content-addressed stage identity.

Non-goals

  • Do not change reactions, Check Runs, the session and evidence model, or PR
    and Issue stage behavior.
  • Do not let discussion content relax, override, or extend the trusted caller
    review profile. Author-stated focus reaches the code turn only as untrusted
    background that must be checked against the diff; it never becomes trusted
    instructions and never replaces the caller's review profile.
  • Do not derive review instructions from the comment during trigger matching.
    Eligibility is decided by the trigger expression alone, and the matched text
    is never parsed for parameters.
  • Do not change the events the caller subscribes to, or consumer workflow pins.

Code Changes Tree

.github/
├── scripts/
│   ├── review-request/
│   │   ├── common.mjs             # line-anchored trigger plus quoted-region stripping and the bot guard
│   │   ├── evaluate.mjs           # read the comment from env and publish the requested step output
│   │   └── test.mjs               # case table over accepted and rejected comment bodies
│   └── pr-review/
│       ├── run.mjs                # write the discussion stage input and load it in the code turns
│       └── test.mjs               # cover the wiring and the identity exclusion
└── workflows/
    └── codex-openai-review.yml    # resolve gates on the module; discussion keeps the trigger comment intact
README.md                          # document which comments start a review and what code review reads

Design

Trigger

Add .github/scripts/review-request/common.mjs following the existing
issue-review and pr-readiness module shape.

stripQuotedText blanks every region where the trigger is quoted rather than
requested: fenced code blocks (backtick or tilde, including an unterminated
one), inline code spans, and block quotes. Lines are replaced with empty lines
instead of removed, so a stripped region cannot join two unrelated lines into
one apparent command.

REVIEW_REQUEST_COMMAND is /^ {0,3}@codex(?:[ \t]+review\b.*?)?[ \t]*$/im.
The m flag lets the request appear on any line; requiring the whole line to
be the command keeps a mid-sentence mention out. At most three leading spaces
are allowed, which is CommonMark's indented-code-block boundary, so a
four-space-indented mention does not fire either.

Inline code spans are stripped over the whole comment rather than line by line,
because CommonMark lets a span cross line breaks. Each span's characters are
replaced with spaces while newlines are preserved, so no surrounding line
moves. The closing-delimiter search stops at a paragraph break, which is where
CommonMark ends a span, so one stray backtick cannot blank the rest of the
comment; an unmatched run is treated as literal text.

isReviewRequestComment rejects a comment whose author type is Bot or whose
login ends in [bot] before testing the body.

resolve gains two issue_comment-only steps: a sparse checkout of
.github/scripts/review-request from job.workflow_repository at
job.workflow_sha, matching how review and finalize already pin the exact
reviewer implementation, and a node evaluate.mjs step that receives the
comment through the environment and sets a requested output. The
github-script step then gates on that output instead of an inline regex.

Discussion context

The discussion step keeps the last 20 comments, restores the triggering comment
when newer ones crowd it out of that window, and adds id, is_trigger,
author_is_bot, and body_truncated. Per-comment clipping rises from 600 to
2,000 characters, and the triggering comment gets 8,000, so the comment that
asked for the review survives whole.

run.mjs writes a code-discussion stage input beside the existing
code-linked-issues one. The first code chunk turn reads it after the Issue
context, and an aggregation turn that runs without a preceding code turn reads
it directly. The prompt frames it as untrusted input from any commenter, usable
only as author-stated intent, validation claims, disclosures, and request
focus; it cannot change the trusted review profile, every claim must be checked
against the diff, and a claim the diff contradicts is reported.

Discussion is deliberately kept out of prStageSnapshot() and the code stage
identity. Putting it in would let any comment invalidate cached evidence and
force a paid re-review. The documented consequence is that a comment on an
unchanged head still reuses evidence at zero tokens and its text is not read
until a code turn runs for another reason — which is the real case here, since
both GizClaw/gizclaw#1084 comments followed a push.

Test And Acceptance Criteria

Acceptance Criteria

  • A comment whose body is explanation text followed by @codex review on its
    own line starts a review.
  • Bare @codex, @codex review, and @codex review <focus> continue to
    start a review, with or without surrounding whitespace.
  • A mention mid-sentence, a mention followed by other words on the same line,
    @codex reviewing, a mention inside a fenced code block, inline code span,
    indented code block, or block quote, and unrelated text do not start a
    review. Inline code spans include spans that cross line breaks, which
    CommonMark permits, and a stray unmatched backtick must not suppress a later
    request in another paragraph.
  • A comment authored by an app does not start a review.
  • Non-comment issue_comment payloads and non-PR issues remain ineligible.
  • A code review turn reads a discussion input file that identifies the
    triggering comment and marks bot authors and truncation. The triggering
    comment is always present, including when 20 newer comments would otherwise
    crowd it out of the window.
  • Both real api/http: add device Wi-Fi scan and join over RPC gizclaw#1084 comments are carried without truncation.
  • No stage identity contains discussion content, so an unchanged head still
    reports reused with zero model tokens.

Validation

  • node .github/scripts/review-request/test.mjs
  • node .github/scripts/issue-review/test.mjs
  • node .github/scripts/pr-readiness/test.mjs
  • node .github/scripts/pr-review/test.mjs
  • actionlint .github/workflows/codex-openai-review.yml

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions