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
15 changes: 12 additions & 3 deletions .github/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -481,8 +481,16 @@ jobs (`ci.yml`, `deploy.yml`, `test-deploy.yml`, `visual-regression.yml`, `story
`claude-implement` (`implement`), `bestaxbot-reply` (`respond`), `claude-review` (`review`),
`claude-pr-loop` (`fix` and `verify`). Nineteen jobs; the command below is the check.
- **Audit, deliberately, pending a measured allowlist** — none, as of #578. That issue closed the
group by measuring all six of its members instead of guessing for them, and all six landed on
the same eight hosts the Claude-session jobs above already used. Keep the group here rather
group by measuring all six of its members instead of guessing for them. All six now **use** the
same nine-host allowlist, and the gap between that and what the measurement produced is the part
worth keeping: the runs surfaced six application hosts, a strict subset of the eight the
Claude-session jobs above already used, and `nodejs.org` appeared in no run at all. It is on the
list anyway, and that ninth host is why "measured" is not the same as "complete". None
of `ai-scan`/`scan`, `ai-triage`/`triage` or `claude-repro`/`author` runs `actions/setup-node`,
so their eight-host list never needed it; all six of #578's jobs do run it, and setup-node falls
back to `nodejs.org/dist` when the toolcache misses and the `actions/node-versions` lookup fails.
Six audit runs could not surface that, because the toolcache hit every time and an unexercised
path leaves no endpoint in any log. Review caught it, not measurement. Keep the group here rather
than deleting it: audit remains the correct starting point for an **existing** live job whose
egress has never been observed, and the recipe under this rule is what keeps it a short state
rather than an open-ended one. What it is not is a resting place, which is the whole reason
Expand Down Expand Up @@ -564,7 +572,8 @@ Two things about reading its output, both learned assembling the #578 lists:
`productionresultssa<N>.blob.core.windows.net`,
`run-actions-<N>-azure-*.actions.githubusercontent.com` and `hosted-compute-*.githubapp.com`
are the runner talking to its own control plane. The blob host cannot be pinned even in
principle — its name rotates per run (sa3, sa6, sa7, sa9, sa11 and sa13 across six runs). Run
principle — its name rotates per run (sa3, sa6, sa7, sa9, sa11, sa13 and sa17 so far, and the
list keeps growing every time anyone looks). Run
33221210633 is the evidence that omitting them is right: `auto-close-duplicates` at `block`
with the five-host list observed only `api.github.com` and `github.com`, and completed all
fifteen of its API calls under the firewall.
Expand Down
13 changes: 10 additions & 3 deletions .github/workflows/claude-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -531,9 +531,16 @@ jobs:
# usage-limit failure on the very first call). The PR then silently
# gets no review while the run shows green, and the AI loop's
# deep-review gate waits on a review that never comes. Fail loudly.
# An empty/missing execution file is tolerated: the action skips
# without a session when the PR modifies this workflow file (its
# OIDC exchange requires the file to match the default branch).
# An empty/missing execution file is tolerated: the action skips without
# a session whenever the PR HEAD's copy of this file differs from the
# default branch's (its OIDC exchange requires them to match).
#
# Read that as written. It is NOT "when the PR modifies this file" — that
# narrower version is what CLAUDE.md said until #578's own flip changed
# this workflow and every PR still carrying the pre-flip copy started
# getting silently skipped reviews. The branch never touched the file;
# it just had not caught up. Age is not the criterion either: a branch
# that has since merged or rebased the current version is fine.
- name: Fail on silent session error
if: steps.claude.outcome == 'success'
env:
Expand Down
16 changes: 13 additions & 3 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,19 @@ green and every AI review thread is resolved.
lands as a PR review from `claude` marked `<!-- claude-deep-review -->`; it reviewed the
code checked out when its workflow started, which a racing push may have superseded — so
look for that review comment (not the current head's checks) and verify its findings
against current code. **Today a PR that modifies `claude-review.yml` is not deep-reviewed:**
the run logs `Skipping action due to workflow validation` and posts nothing while the job
still goes green — so check for the review comment, never the job's conclusion. That is a
against current code. **Today a PR whose copy of `claude-review.yml` differs from the default
branch's is not deep-reviewed:** the run logs `Skipping action due to workflow validation` and
posts nothing while the job still goes green — so check for the review comment, never the job's
conclusion. Read that condition as written, because the narrower version ("a PR that _modifies_
`claude-review.yml`") is what this line said until it bit. The workflow runs from the **PR
head's** copy, so what matters is only whether that copy still matches the default branch —
Comment thread
allxsmith marked this conversation as resolved.
never the branch's age, and never whether the PR touched the file. An old branch that has since
merged or rebased the current version is fine; a branch opened five minutes ago off a stale
base is not. #578's flip changed the file, so every PR still carrying the pre-flip copy
inherited a skipped review; #605 reproduced it (`"egress_policy":"audit"` read from the branch's
own retained copy, session skipped) and merging `main` in fixed it. Expect this after any edit
to `claude-review.yml`, and confirm the head's copy matches before trusting a green
deep-review job. That is a
consequence of configuration rather than a property of the action: the validation lives on
the OIDC to app-token exchange, and `setupGitHubToken()` returns before reaching it whenever
a `github_token` input is supplied — the same early return `ai-triage.yml` already documents
Expand Down
Loading