CodeQL alert #121 flags the pr-head checkout in the PR review job. The
checkout is deliberate and nothing in the job executes it, but the job still
carried `id-token: write`, and so did the issue analyst. claude-code-action
only uses OIDC to trade for a Claude GitHub App token when no github_token is
passed - that app token asks for contents, pull_requests and issues write -
or for workload identity, which neither workflow uses. Both pass
github_token, so the permission did nothing for them, yet any code running
in either job could have minted that write token, undoing the review's
read-only split.
Three reviews posted on one morning (#6666, #6675, #6752) also showed rubric
gaps. REVIEW.md said both "open with the verdict" and "lead with No blocking
issues", so the comments opened three ways and two tallies dropped
severities; the opening is now a fixed order - tag line only when blocking,
verdict paragraph, one status line whose tally names all three severities.
#6675 filed a spoofable X-Forwarded-For IP, which that PR first shows to an
admin approving a login, under Design; Design now holds approaches and
trade-offs only, and a concrete consequence the change makes reachable or
worse is a finding. The coverage list ran to some 25 nested lines on #6752;
it is now ten lines at most, one per area, no nested bullets.
Run 38187893539 reviewed PR #6784 in full but posted nothing: the model's
Write to /tmp/review.md was refused. Claude Code checks file permissions
against Edit(path) rules only and never consults a Write(path) rule, so the
allow rule was dead - and so were the Write(//home/**) and Write(//opt/**)
deny rules meant to keep $GITHUB_ENV out of reach. All three are Edit rules
now; a local headless run confirms Write(path) is refused, Edit(path) lets
the file through, and an Edit deny still blocks a Write under it.
allowed_non_write_users="*" skipped the action's write-permission check for
every actor and warned each run. Only a pull_request_target from a fork
needs it; an `@claude review` requester is already OWNER/MEMBER/COLLABORATOR
and passes the action's own check.
The review only hunted defects: a correct fix got no word on whether the
reported problem was real or actually solved, and a feature got no design
feedback. The comment now opens with a verdict for every kind of change -
for a fix, whether the failure exists on base, whether the head removes its
root cause (checked against the issue-analyst's analysis) and whether the
test proves it; for a feature, whether it overlaps what the panel already
has, which deployment shapes it misses, and a prose-only Design section;
for refactor, perf, dependency bumps, docs and CI, a question of their own.
The trace scales to the change, so a typo is not traced like a runtime
change. It also searches for duplicate merged or in-flight work, and runs at
high effort since settling whether a bug is real takes tracing base and head.
No code may appear anywhere in the comment, and the model no longer holds a
write token. The workflow is three jobs: `prepare` (ack, pin the head, skip a
reviewed PR), `review` (read-only token; the model writes /tmp/review.md) and
`post` (no model, no checkout), which strips any fenced block, keeps the
Reviewed head marker the skip relies on, and only then posts one plain
comment. Before, `gh api` with a pull-requests: write token could touch any
PR in the repository, `sed -i` could write $GITHUB_ENV, and the inline-comment
tool told the model to use suggestion blocks. Concurrency stays on the review
job: at workflow level any unrelated comment on the PR would replace a
pending review in the queue.
Restores three guards the earlier claude-bot review had: all author-controlled
input, including text inside the checked-out code, is data and a diff adding
text aimed at a reviewer or bot is a HIGH finding; a blocking finding tags the
repository owner; and a diff too large to read whole has a reading order and
must name what went unread.
The bot never read the replies under its own findings, so a finding a
maintainer had already declined came back on the next `@claude review`.
It now reads every comment and inline thread first: a maintainer's answer
settles a finding for good, anyone else's is a claim checked against the
code, and the summary gives each earlier finding a disposition. It also
reads the issue the PR claims to fix and reports a partial fix.
REVIEW.md asks for an upstream symbol behind every wire-format claim, but
the job had no xray-core source (#6718's review said so). The module the
base go.mod pins is now unpacked into a hidden dir in the base workspace;
nothing from pr-head runs.
REVIEW.md gains the rules only /senior-review carried: keep read,
reproduced and inferred claims apart, evidence for performance findings,
a traced trust boundary for security ones, and duplicated logic as a
finding. The summary now says each inline finding in one line.
65b9bfed narrowed the fix carve-out to "one clause naming WHERE the fix
belongs", but the bot still closes every finding with that clause, and set
beside the defect it already named, the location is the fix. On #6718 it
listed the two capabilities the bounding set lacks, then wrote "The fix
belongs in the capability bounding set". Drop the carve-out from REVIEW.md
and the workflow prompt: the finding's file:line already says where.
Zod 4: use the `error` param instead of the deprecated `message`.
lint:deprecated missed these because tsgolint's no-deprecated does not
resolve object-literal properties on a `string | Params` union.
Geodata: key geo entry rows by page position. antd deprecates rowKey's
index argument, and kind:value repeats within a page because the reader
drops domain attributes (22 pairs in geosite_IR.dat, 108 in geosite_RU).
Nord/PIA: the "All cities/regions" option used a null value, which antd
warns on. Map it through a sentinel at the Select boundary so form state
stays null, with tests that fail when the sentinel is not mapped back.
Tests:
- Run the oxlint guard through node; .bin/oxlint is a sh shim Windows
cannot spawn, and the swallowed error left both guard cases vacuous.
- Start unit workers with --no-experimental-webstorage; msw's localStorage
probe made Node 25+ warn once per forked worker.
- Set IS_REACT_ACT_ENVIRONMENT, which RTL never sets with globals: false,
and settle the async updates it exposed inside act(). The row-cells
memo test now fails when memo is removed.
- Disable antd's click wave in Storybook; it re-rendered inside the next
story's act() and tripped "not configured to support act".
- Assert InboundFormModal's validation log instead of leaking it, and
give the rule-form test a well-formed clients/list response.
* fix(ci): keep a refused Claude credential from reddening a PR
An expired subscription ends the claude-code-action step with exit 0, so the
classifier that exists for "the API refused this run" never sees it -- its
condition is a failed step -- and the final "posted nothing" step reddens the
pull request although nothing is wrong with the repository.
Verified against five real runs (35159059540, 35184688775, 35185722358,
35186543654, 35187380192): step 8 success, step 10 found no cause, step 11
failure, transcript {"error":"oauth_org_not_allowed"} plus a result entry with
api_error_status 403. A usage-limited run carries 429 and a rejected
rate_limit_event, and a real review carries is_error false with no status, so
the 401/403 test fires on the refused credential alone.
* fix(ci): stop a refused credential reddening the issue analysis
The same exit-0 refusal reaches this workflow's "posted no reply" check, which
fails for the same reason and shows up as seven failed runs in a day. It never
attaches to a pull request -- the trigger excludes them -- so this is the same
step and the same 401/403 transcript test applied where the refusal lands.
Reported only as a warning annotation: nothing was analysed, and there is no
comment worth posting about a credential the maintainer has to renew.
The review job ran the official code-review plugin, which fans a pull
request out to five Sonnet reviewers plus a Haiku scorer per finding and
drops everything scored under 80, and the briefing file spent most of its
lines overriding that plugin. Both are gone: the job hands one Senior
Software Engineer prompt to the action inline, the way the issue analyst
does, and denies the Agent tool so the single role is mechanical rather
than a request.
REVIEW.md moves from the emoji markers to CRITICAL/HIGH/MEDIUM/LOW with a
pre-existing qualifier. The uncapped rule is scoped to findings the pull
request introduced or worsened so it cannot collide with the cap of three
pre-existing ones. "A finding is a report, not a patch" stays as it was.
Workflow housekeeping: GH_TOKEN, REPO and PR live in the job env instead
of six step copies; the skip gate is per pull request, so a head pushed
after the automatic review is reviewed only on @claude review; the comment
counters sum gh's per-page jq output, which read "0\n0" as a review on a
pull request with more than 100 comments; --max-turns rises to 300 because
every read now costs the single agent a turn instead of a subagent.
claude-bot.yml ran three jobs: the pull-request review, an @claude mention
responder, and a conflict resolver that committed and pushed to contributor
branches. Only the review is wanted, so the other two are gone and the file
is renamed to say what is left.
Consequences worth knowing:
- secrets.CLAUDE_BOT_PAT is no longer referenced by any workflow. It was the
only push credential handed to an agent in this repository and can now be
deleted from the repository settings.
- @claude goes unanswered everywhere. claude-issue-analyst.yml deliberately
excludes mentions (!contains(body, '@claude')) so the two jobs would not
both reply; with the mention job gone, only `@claude review` on a pull
request still reaches anything. Dropping that clause from the analyst would
restore mention answering on issues.
- The workflow display name changes, so a branch protection rule keyed on
"Claude Bot / review" has to become "Claude PR Review / review". The job
name, which is what statusCheckRollup reports, is unchanged.
The review job itself is byte-identical. The workflow-level permission drops
to issues: read, which is all the remaining job needs - it already declares
its own.
MHSanaei