Repository navigation
chore(deps): bump claude-code-action to v1.0.250, pin --permission-mode default - #302
Merged
Merged
Conversation
…de default Moves claude-review, claude-adversarial-review and verifier-on-high-risk from a4f54ef (v1.0.229) to a26b07d (v1.0.250), the bump #282 proposes. v1.0.250 bundles claude-agent-sdk 0.3.300. 0.3.276 sent `--permission-mode default` whenever the caller set no mode (`permissionMode ?? "default"`); 0.3.300 sends none, so the CLI would take `permissions.defaultMode` from the caller's .claude/settings.json, which the action restores from the PR's base branch. Each lane now passes `--permission-mode default` in claude_args (not `auto`, which runs classifier-approved tools outside --allowedTools), and selftest/test_claude_lanes_permission_mode.py pins it with hardcoded lanes and 13 mutant negative controls. Also moves the ran-check selftest's pin and rewrites the stale pin comment in claude-review.yml: every lane runs its own pinned CLI through path_to_claude_code_executable, so the bundled CLI no longer reaches any lane; the comment now lists what a bump must re-check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t only the looser modes Review follow-ups on #302 (comment-only): the fallback was dropped in Agent SDK 0.3.286 (first bundled in v1.0.238), not 0.3.300; and plan/dontAsk are stricter than default, so the docstring now names only the modes that let tools outside --allowedTools run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
topcoder1
added a commit
that referenced
this pull request
Oct 17, 2026
…ranch and file names (#303) ## Summary This closes two gaps in this repo's copy of `scripts/validate-required-checks.sh`. topcoder1/dotclaude#525 (merged 2026-10-16) closed the same two in dotclaude's copy. This is the copy that the weekly "Audit required checks" workflow runs across the fleet, so each gap was a silent pass that could hide a phantom required check in any fleet repo. 1. **The rules call read only one page.** `GET /repos/<repo>/rules/branches/<branch>` returns its rules 30 to a page, and the call had no `--paginate`. A `required_status_checks` rule after rule 30 was never read. The branch then passed with `no required_status_checks rule on main` and exit 0, even when a required context had no producer. 2. **`--branch` and workflow file names went into the URL unencoded.** gh parses the endpoint as a URL: - it never sends anything after a `#`; - a `?` starts the query; - a `%` that starts no `%XX` escape fails inside gh; - GitHub decodes any `%XX` that is left. As a result, `--branch 'release#2'` read `release`'s rules, and a workflow named `ci#2.yml` was read as `ci`. This copy has no `--ref`. ## What changed - **Paging:** the rules call runs `gh api --paginate`. - Every page gh prints must be a JSON array. `add` would treat a `null` page as nothing at all, so a `null` page would be merged away unseen. - The pages are then merged (`jq -cs 'if all(.[]; type == "array") then add else error(…) end'`), and the existing shape check runs on the merged list. - Both output forms are handled: gh 2.97.0 prints the pages as one array, and gh's help describes one array per page. - The shape check needs the merged list because jq goes by the last of several values. That holds for `jq -e`, and also for jq's exit status: jq 1.7.1, measured 2026-10-17, still exits 0 after an error on an earlier input. Without the merge, a bad rule on page 1 of 2, printed as an array per page, read as a clean pass. - **Encoding:** a new `uri()` (jq `@uri`, with `/` left as it is) encodes `--branch` in the rules path and each workflow file name in the per-file path. - Values made only of letters, digits, `-._~` and `/` send exactly the bytes they did before. Any other character is now sent percent-encoded, and GitHub decodes it once, back to the same value. - Both call sites are assignments. A `uri` failure there exits 4; inside a command's arguments it would not stop the command. - **Error messages** name the endpoint as sent. This covers the rules shape error and the per-file "did not answer base64 file content" error. `api_get`'s own messages already did. - **Unchanged:** - #299's exit codes: 0 clean, 1 FINDING only, 2 usage, 3 API error naming the endpoint, 4 internal error. - #299's rule that no message quotes the response, because the digest is a PUBLIC issue. dotclaude's copy quotes up to 200 bytes of a bad rules answer; this one names only the endpoint. The merge therefore reads `$API_BODY` directly, and no raw answer is kept for a message. ## Measured live (gh 2.97.0) I re-measured these on 2026-10-17 with read-only GETs on this repo. The selftest shim copies them: - `rules/branches/main#x` read main's rules. - `rules/branches/ma%69n` read main's rules: GitHub decodes `%XX`. - `contents/a%zz.md` failed inside gh with `invalid URL escape "%zz"`, rc 1, and sent nothing. - `--paginate` with `?per_page=1` printed this repo's 4 rules as one array (gh followed `Link` 4 times). The local-server results come from dotclaude#525 (2026-10-16): - a `null` page 2 printed `[{"type":"deletion"}null`, rc 0; - a 403 on page 2 printed page 1 and then the error body, rc 1. ## Old against new, live Old is origin/main at 71322c5; new is this branch. Both ran on this repo, topcoder1/ci-workflows, which is itself in the fleet list, through a `gh` wrapper that logged every call's argv. | Run | Old | New | |---|---|---| | default | exit 0, 6 contexts ✓ | exit 0. Stdout is byte-identical (23 lines). The 41 gh calls are the same, except that the rules call adds `--paginate` | | `--strict` | exit 0 | the same | | `--branch 'main#x'` | **exit 0: validated `main`'s 6 contexts** | exit 0: `no required_status_checks rule on main#x`. One call, to `rules/branches/main%23x` | This repo has 4 rule entries, so paging changes nothing here live. The selftest covers paging. A run on a private fleet repo gave the same verdicts. ## Tests All tests are in `selftest/test_audit_required_checks.sh`, which CI runs through `selftest/test_workflow_guards.py`. Every expected value is hardcoded. **The PATH-shim `gh`** reads an endpoint the way gh and GitHub did above: - It first fills gh's placeholders, `{owner}`/`{repo}`/`{branch}` and `:owner`/`:repo`/`:branch`, as for the scheduled audit's checkout of this repo on main. gh fills them from the checkout it runs in: measured 2026-10-17, `contents/{repo}.yml` and `contents/:repo.yml` both requested `contents/ci-workflows.yml`. - It drops the fragment, refuses a bad `%` in the path, and decodes `%XX`. - In the query, it splits on `&`. A parameter with a bad `%` gets HTTP 400 from the rules endpoint (measured 2026-10-17); the contents endpoint drops it instead. - It serves rules per branch (`branch-rules/`). - It pages rules 30 at a time, or 100 under `--paginate`, printed as one array or (`pages_apart`) as one array per page. - It can also serve GitHub's own pages (`rule-pages/`), a failing page included. Without `--paginate` it prints the one page as GitHub sent it. With `--paginate` it joins them as gh 2.97.0's `paginatedArrayReader` does: a page with another after it loses its `]`, and a later page's `[` becomes `,`. **New cases:** - **h (paging):** - the required-checks rule as rule 31 and as rule 101 (past even a 100-rule page), each in both `--paginate` forms; - a bad rule on page 1 of 2, with pages printed apart; - a `null` page 2, in both forms; - a page 2 that fails with 403, in both forms. The bad-rule, `null` and failing-page cases carry canaries, which must not appear in the output. - **i (encoding):** - workflow file names `ci#2.yml`, `ci?2.yml`, `50%.yml`, `{repo}.yml`, `:repo.yml` and `ci 2.yml`; - `--branch` `release#2` and `rel%41se`; - the endpoint each error names: a failed file read, a file answer that is not base64, and a rules answer that is not a list for `rel/main#x`. The last one also pins that `/` is sent as it is. - **s1–s4 (shim controls):** these pin what the shim does with the requests origin/main sends (no `--paginate`, values as given) and with both `--paginate` forms. They keep cases h and i able to tell the fix from the bug. **RED first.** Commit c11a330 is the tests alone: run against origin/main's validator, 15 of its 65 checks fail. The final test file, with the cases the reviews added, fails 18 of 70 there. Both counts are the same under `/bin/bash` 3.2.57 and bash 5.3.15, and each failure is the reported bug: - Paging: 4 of 4 rule-31/101 cases exited 0 with `no required_status_checks rule on main`. The `null` page 2 exited 1 in both forms, a false FINDING from page 1 judged alone. The failing page 2 exited 0 in both forms, a silent pass. - File names: `#` and `?` read `ci` and got a 404. `%.` failed inside gh. `{repo}` and `:repo` read `ci-workflows.yml` and got a 404. - Branches: both read a branch with no rules, exit 0. - Endpoint messages: 3 of 3 named the unencoded endpoint, or exited 0. The 52 that pass on the old code: - the 39 existing checks; - the 11 shim-control checks; - the space in a file name (gh always escaped it itself); - the bad rule on page 1, which the old code also rejected, since it read page 1 only. This case kills a fix that paginates without merging. **GREEN:** 70 of 70 checks pass under both bashes, and the pytest wrapper for this file passes on a349673. The full suite (`uv run pytest -q`) gave `787 passed, 1 skipped` on 0a15680; the commits since then change only this selftest and its README. shellcheck 0.11.0 is clean at CI's `error` severity. At default severity the selftest is clean, and the validator reports only the 3 notes origin/main already has (SC2001, SC2034, SC2086). prettier 3.9.10 is clean on the README. **Mutation check (mine; the independent review repeated it).** Each of the following, applied to a scratch copy, makes a named case fail: - no `--paginate`; - `per_page=100` instead of `--paginate`; - `--paginate` with no merge; - `add` with no array guard; - no `uri` on the branch; - no `uri` on the file name; - `uri` that also encodes `/`; - an encoder that handles only `#`; - an encoder that handles `#`, `?`, `%` and space but not braces, or not `:`; - either message naming the raw value; - the rules message quoting the answer, which the canaries catch; - `api_get` echoing gh's stdout on failure. Only the failing-page-2 canary catches this one. ## Codex pre-review - **Round 1:** native `codex review --base origin/main`: no findings. - **Rounds 2–6:** an adversarial prompt through `codex exec -s read-only`. Each round listed the findings already declined, so that only new issues counted. - **Round 2:** 2 P2 and 1 P3, all declined: - P2: a single page whose body is `[][]` merges to `[]`, exit 0. The old shape check accepted it too, because jq goes by the last value. It needs GitHub to send invalid JSON with HTTP 200, and it cannot be told apart from gh's array-per-page output, which must stay accepted. - P2: NUL bytes in a response are dropped by `api_get`'s command substitution. That capture predates this change (#299), and the input needs a raw NUL inside GitHub's JSON. - P3: a newline in a workflow file name. This predates the change and is listed below. - **Round 3:** 1 P3, and it was false. It claimed the selftest's `${3%]}` strips nothing. It did strip the `]` under both bashes: the page file it wrote had none. Since 49d23c2 that helper strips nothing, because the fixtures now hold GitHub's own pages and the shim joins them. - **Round 4:** 1 P2 and 1 P3, both fixed in db68825, tests only: - P2: the failing-page-2 case had no canary, so an `api_get` that echoed gh's stdout (page 1's rules) would have passed. Page 1 now carries a canary, and this is the only case that catches that echo. - P3: gh fills `{owner}`, `{repo}` and `{branch}` from the checkout it runs in. I measured this, the shim now models it, and a `{repo}.yml` case covers it. - **Round 5:** 1 P2 declined and 1 P3 fixed in d7ee6e5: - P2: gh 2.97.0's `paginatedArrayReader` holds back a page's `]` only when it is the last byte read, so a page body ending in whitespace would merge into invalid JSON. GitHub's rules pages end in `]`: a live 4-page `--paginate` of this repo's rules merged into valid JSON. If a page did not, the validator would exit 3 naming the endpoint, which is the fail-closed path. - P3: gh also fills `:owner`, `:repo` and `:branch` (its `placeholderRE`). I measured this and added a `:repo.yml` case. - **Round 6,** on d7ee6e5: `No findings.` ## Independent review An opus `code-reviewer` lane, separate from the authoring context, reviewed 71322c5..d7ee6e5. **Verdict: APPROVE with nits.** It found no validator defect at any confidence. - **`uri` failure:** it made `uri` fail at each call site, on both bashes. Both sites exit 4, and no request goes out with a truncated endpoint. With `uri` inlined into an argument instead, the request goes out as `.../workflows/` and the run exits 3. That is why both call sites are assignments. - **The merge, against real gh 2.97.0 output** from a throwaway local server: one merged array, `[...null`, page 1 followed by the 403 body, and an empty last page. - **The merge, against real GitHub:** `per_page=100`, the Link-following, and 4 pages at `per_page=1` printed as one array. - **Leaks:** both the array guard and the merge's `2>/dev/null` are needed. jq's `add` type error quotes the input. - **Mutations:** 24, all killed on both bashes. That covers the ten listed above, plus eleven of its own: `any` for `all` in the guard, `%` or `?` kept, the shape check dropped or run on the raw answer, the merge's stderr kept, `uri` inlined, and more. The three that survived earlier commits are the ones db68825 and d7ee6e5 closed: `api_get` echoing gh's stdout, braces left unencoded, and `:` left unencoded. - **RED/GREEN:** it reproduced the numbers at each commit, with identical verdict lines on both bashes. - **Its two LOW findings were both on shim fidelity.** It measured both against real GitHub and gh, and both are fixed in 49d23c2: - A bad `%` in the rules endpoint's query gets HTTP 400; only contents drops the parameter. s1 had asserted the drop. - The page fixtures had held gh's `--paginate` bytes. The old, unpaginated validator therefore saw an unclosed page 1 and looked fail-closed, when it really exited 1 (`null` page) and 0 (failing page 2). `rule-pages/` now holds GitHub's own pages, and the shim joins them as gh's `paginatedArrayReader` does. - **Re-check of 49d23c2: APPROVE.** - Real gh 2.97.0 and the shim, given the same page bodies, printed byte-identical output in 24 of 26 scenario pairs. Those included an empty middle page, an empty first page and a null first page. - The other 2 differ only by a trailing newline after a failing page's error body, which comes from the existing `fail_with` `echo` and affects no outcome. - The 400 is byte-exact against real gh. - Its optional nit, an empty-page join that nothing tested, is pinned in a349673. The shim mutation that joined it as `,` is now caught. - **Left below the bar:** - Next-page Links for an encoded branch. This would matter only past 100 rules, on a branch with special characters, through a manual `--branch`. - Pre-existing: this copy's rules shape check is weaker than dotclaude's. It requires no string `type` and no non-empty string `context`, so a malformed rule of that kind reads as no required checks, exit 0. That is out of scope here; a follow-up session is proposed. ## Preflight `bb-preflight.sh topcoder1/ci-workflows claude/validator-page-encode`: - On 0a15680 it reported **READY TO PUSH**. - On f0fad3b, 49d23c2 and a349673 it reported **NOT READY on the in-flight cap only.** Two other Claude PRs are open: #302, a draft opened 2026-10-17, and #269. Neither touches these three files, and the preflight's file-overlap check is green. - Every other check is green: working tree, policy gate, required checks reachable, and ruff. ## Known limitations, not changed here - A newline inside a workflow file name splits it into two names in the listing loop. This predates the change; dotclaude#525 lists the same limitation. - A failure on page 2 or later is reported against the endpoint the validator called, not the page URL gh followed. gh's stderr never carries that URL. - The fleet wrapper always audits `main`. All 14 fleet repos default to `main` today (checked 2026-10-17), so a repo added later with another default branch would read as clean. - The quadratic `gsub("\\s"; "")` in the per-file base64 check is unchanged. Codex rounds: 6, the cap. Round 1 was the native review; rounds 2–6 used the adversarial lane. Round 6, on d7ee6e5, found nothing; the commits after it (README wording, plus the independent review's test-only fixes) were re-checked by the reviewer, since the loop was at its cap. Round 5 did find something, so the loop stopped at the cap, not on two clean rounds in a row. Of the 8 findings, 3 were fixed (all test gaps, each test failing first), 4 were declined for the reasons above, and 1 was false. Auto-merge rationale: manual click-merge. PRs to topcoder1/ci-workflows are always manual merge, and this validator decides the fleet audit's verdicts, which are posted publicly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
topcoder1
marked this pull request as ready for review
October 17, 2026 21:56
|
Risk class: This PR touches one of the blocked path categories from Auto-merge is refused by (This is a policy notice, not a code-quality failure. The classify job itself does not fail — required CI checks remain authoritative for "is the code green.") |
|
All the logic checks out cleanly:
No issues found. This is a well-structured maintenance bump with solid guard tests. Claude Review · workflow run |
|
The diff is clean. The No issues found. Diff correctly guards the Claude Review · workflow run |
|
The diff is clean and well-structured. Here's my assessment: Action pin bump (all three workflows): SHA updated consistently from v1.0.229 (
New test (
No issues found. Diff is a well-guarded security fix with comprehensive mutation testing. Claude Review · workflow run |
topcoder1
added a commit
that referenced
this pull request
Oct 17, 2026
…rror, not as no required checks (#305) ## Summary This copy of `scripts/validate-required-checks.sh` checked the rules answer (`GET /repos/<repo>/rules/branches/<branch>`, merged across pages since #303) only for being an array of objects. The weekly "Audit required checks" workflow runs this copy across the fleet. The contexts filter keeps only the rules whose `type` is `required_status_checks`, and reads their checks with `[]?`. So a malformed rule read as no required checks: exit 0 with `no required_status_checks rule on main`. The fleet audit then filed the repo as clean and opened no issue, so whatever check that rule required was never validated: a silent pass. topcoder1/dotclaude#521 (merged 2026-10-16) closed the same gap in dotclaude's copy. This PR ports that check. ## What changed - **The shape check** (`scripts/validate-required-checks.sh`) is now dotclaude's jq program, byte for byte: - every rule has a string `type`; - every `required_status_checks` rule lists its checks as an array of objects, each with a non-empty string `context`. Anything else exits 3 and names the endpoint as it was sent. `[]`, when no rule applies, still exits 0. - **#299's contract is unchanged:** - 0 clean, 1 FINDING only, 2 usage, 3 API error, 4 internal error; - no message quotes the response, because the digest is a PUBLIC issue. dotclaude's copy appends `${RULES_ANSWER:0:200}` to this error; this one names only the endpoint. - **The message** is now dotclaude's: `did not answer a valid list of rules`. "an array of rules" was wrong for an array that holds a bad rule. Four existing assertions take the new wording. - **The CONTEXTS filter is unchanged.** Once the check holds, its `[]?` cannot drop anything. - **`selftest/README.md`:** one added passage. ## Tests All tests are in `selftest/test_audit_required_checks.sh`, new case j. CI runs that file through `selftest/test_workflow_guards.py`, and every expected value is hardcoded. **The fixtures.** Each malformed answer is clean-a's rules answer with one defect, and every `ruleset_source` in it is a canary. There are 14: - no `type`, and a number for it; - a `required_status_checks` rule whose checks are missing (`parameters: {}`), a string, or an empty object, and one whose `parameters` is a string; - a check that is a string; - a check whose context is missing, empty, a number, or an object; - a bad check after a good one, and before one, in the same list; - a bad rule beside a good one. **The assertions:** - Each must exit 3, with stdout and stderr byte-equal (`cmp`) to the header and the one API error line. So no verdict line and no byte of the answer can appear, whichever field it comes from. - A fleet run over clean-a and two malformed repos must exit 3 with two `ERROR: not audited (exit 3: GitHub API error)` blocks naming the endpoint, and no canary. The workflow's own digest step must then open the issue. - Two positive controls, which must still exit 0: a full answer (`pull_request` and `non_fast_forward` rules, a check with `integration_id`), and a rule with an empty list of checks. - The fixture builder fails the run if an edit does not build a JSON array that carries the canary. Without that, a typo would serve an empty answer, which exits 3 with the same message and passes having tested nothing. **RED first.** Commit 83da44c is the tests alone. Against origin/main's validator (5b880ca; the validator is unchanged since 67e3588), 21 of 87 checks fail, the same under `/bin/bash` 3.2.57 and bash 5.3.15: - 6 are the new wording. - 15 are case j: - 9 silent passes, exit 0: no type, numeric type, no list, string list, empty-object list, empty context, bad rule beside a good one, bad check after a good one, and the fleet run reading `All 3 repos clean.`; - 4 false findings, exit 1: phantoms named `null` and `5`, a bad check before a good one (`null`), and an object context, which printed the answer's own bytes into the output; - 2 crashes, exit 4: a check that is a string, and string `parameters`. **GREEN.** 87 of 87 pass under both bashes, with identical verdict lines. - The full suite (`uv run pytest -q`) gave `787 passed, 1 skipped`. It ran before the rebase onto #304, which adds only #304's own files. This test's CI wrapper passes on the rebased head. - shellcheck 0.11.0 is clean on the selftest at style severity, with one `SC2016` disable for the jq variable `$c`. - The validator reports only the 3 notes origin/main already has. - prettier 3.9.10 is clean on the README. **Mutation check.** Each of these, applied to a scratch copy, makes a named case fail: - dropping the `type`, array, string-context or non-empty check; - the old check; - checking only the first check of a list, or only the last; - the shape check's stderr kept, or its redirect order swapped; - echoing 200 bytes of the answer; - leaking the rules' `parameters`, which hold no canary; - a newline-only leak; - the old wording; - exit 0 on a bad shape. The one survivor, dropping the per-check `type == "object"`, is equivalent: a non-object check makes jq error or evaluate to false, and both take the same exit-3 branch. The independent review confirmed this over 14 check values. ## Measured live (read-only) - **Fleet:** the real rules answers of all 13 repos the audit covers pass both the old and the new check. - **This repo, end to end:** the old validator (67e3588) and the new one both exit 0, with byte-identical stdout (23 lines), in default and `--strict` mode. ## Codex pre-review - **Rounds 1 and 2:** native `codex review --base`. No findings. - **Rounds 3 to 6:** an adversarial prompt through `codex exec -s read-only`. Each round listed the findings already declined, so that only new ones counted. - **Round 3:** 2 P1 and 2 P2. - The P1s (declined here): a context with leading or trailing whitespace, or a newline, is altered by the newline-delimited `CONTEXTS` transport and Python's `.strip()`. `" test "` validates as `test`, and `"\n"` reads as no rule, exit 0. Both copies behave the same, and the gap predates this change. The right fix is to carry contexts exactly and report a misconfigured context as a finding, not to reject it here as an API error. That is a follow-up, below. - P2: case j had no whitespace or newline fixtures; these go with that follow-up. - P2: the leak check only grepped for a canary. FIXED with the exact-output assertion. - **Round 4:** 1 P2. `$(cat ...)` drops trailing newlines, so a newline-only leak would pass. FIXED with `cmp`. - **Round 5:** 1 P1, a NUL variant of round 3's transport gap. `"\u0000"` reads as no rule, exit 0. Declined here and folded into the same follow-up. - **Round 6:** `No new findings.` ## Independent review An opus `code-reviewer`, separate from the authoring context, reviewed the first draft. **Verdict: APPROVE WITH NITS.** - It found no validator defect. - It re-ran RED and GREEN on both bashes, and every claimed mutant kill. - It confirmed that the jq program and its comment are byte-identical to dotclaude main, apart from the dropped byte echo. - It found 5 issues, all in the tests and docs. All are fixed in the tests above: - MEDIUM: every bad check was alone in its list, so a first-only or last-only check passed. Two mixed-list fixtures now catch both. - LOW: no fixture made the check itself error, so the stderr redirect went untested. A string `parameters` now does. - LOW: a broken fixture edit would pass vacuously. Now guarded. - LOW: the README and the case-j comment missed the empty-context silent pass, and said "object" where only `{}` read as no checks. - Note: an object context printed the answer's bytes under the old code. Now pinned. **Re-check of the final tests: APPROVE.** It reviewed 8857416, whose files are byte-identical to this branch's. - It re-ran GREEN and RED under both bashes, in a clean environment (`env -i`, `LANG=C` and `C.UTF-8`), and through CI's entry point. That is `pytest selftest/test_workflow_guards.py -k audit_required_checks`: 1 passed, and 1 failed on the old validator. - It killed all 20 non-equivalent mutants, including four leaks no canary can see: the first 20 bytes, a rule's `type`, the body length, and a trailing space. - Its one LOW is optional and outside this diff. Cases a, h and i still check their merge-step errors with canary greps, not byte for byte. While that error branch is shared with the shape check, case j guards it. ## Preflight `bb-preflight.sh topcoder1/ci-workflows claude/validator-strict-rules-shape` was run before the push. - **Before the rebase,** it reported NOT READY on two checks: - the in-flight cap; - a `selftest/README.md` overlap with #304, in another entry. A 3-way merge from #304's real merge base gave 0 conflicts in either order. - **After #304 merged,** this branch was rebased onto 5b880ca, and RED and GREEN were re-run on the rebased commits. bb-preflight now reports **NOT READY on the in-flight cap only.** #302 (a draft) and #269 are open, and this branch overlaps neither ("no file overlap with sibling Claude PRs"). - Every other check is green: working tree, policy gate, required checks reachable, and ruff. ## Known limitations, not changed here - **Contexts the text transport cannot carry exactly:** leading or trailing whitespace, newlines, NUL bytes. See the Codex rounds above. Both copies need the same fix, proposed as its own change. - **A non-string `ruleset_source_type`** on a `required_status_checks` rule makes the filter's `ascii_downcase` error, so the run exits 4 ("validator crashed") instead of 3. That is loud, not silent. dotclaude's copy behaves the same. - **The rules endpoint lists rulesets only.** A check required only by classic branch protection is not seen. No fleet repo requires a check that way alone today. Codex rounds: 6, the cap. Rounds 1 and 2 were native reviews; rounds 3 to 6 were adversarial. Round 6 found nothing new; round 5's only finding was a variant of round 3's declined transport gap. So the loop stopped at the cap, not on two clean rounds in a row. Of the 6 findings, 2 were fixed (both test gaps, each now killing a mutant) and 4 were declined into the follow-up: the transport gap and its test coverage. Auto-merge rationale: manual click-merge. PRs to topcoder1/ci-workflows are always manual merge, and this validator decides the fleet audit's verdicts, which are posted publicly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
Bumps
anthropics/claude-code-actionfroma4f54ef(v1.0.229) toa26b07d8c778b0c43f64618e5fd29fd0f8af72e8(v1.0.250) in the three Claude lanes, and pins the reviewer's permission mode, which the new release would otherwise leave to the caller's settings.The one behavioural change in v1.0.250: the bundled Agent SDK moves from 0.3.276 to 0.3.300 (exact versions from
bun.lock). 0.3.276 computedpermissionMode ?? (resolvePermissionModeInCli ? undefined : "default"), so it always sent--permission-mode default. 0.3.300 passes the option through with no fallback, so it sends nothing. The CLI then takespermissions.defaultModefrom the settings it loads (settingSources: user, project, local). The project.claude/is the onerestoreConfigFromBasechecks out fromorigin/<PR base branch>, which is not always the default branch. AbypassPermissionsoracceptEditsthere would let tools outside a lane's--allowedToolsrun. This PR pinsdefault, notauto, becauseautoruns whatever its classifier approves, allowlisted or not.Everything else the review-ran and caller checks rely on is unchanged between the two SHAs:
src/github/token.ts,action.ymlandsrc/entrypoints/post-buffered-inline-comments.tsare byte-identical,base-action/src/parse-sdk-options.tsis identical, andsrc/entrypoints/run.tsdiffers only in the bundled CLI version string (2.1.276 → 2.1.300). No lane uses that CLI:installClaudeCodereturns early whenpath_to_claude_code_executableis set. Agent mode only prepends--mcp-configtoclaude_args, and 0.3.300's extraArgs serializer emits--permission-mode defaultas two argv words, exactly what 0.3.276 sent.Changes
uses:now point toanthropics/claude-code-action@a26b07d8c778b0c43f64618e5fd29fd0f8af72e8 # v1.0.250(SHA checked against the annotated tagv1.0.250).selftest/test_claude_review_ran_check.sh: the header comment, the pin and the pass message all move to v1.0.250 /a26b07d.--permission-mode defaultis added toclaude_argsinclaude-review.yml,claude-adversarial-review.ymlandverifier-on-high-risk.yml. The newselftest/test_claude_lanes_permission_mode.pyhardcodes the expected lanes and the flag rather than reading them from the workflows. It parsesclaude_argsthe way the action does (#lines dropped, then split into shell words) and requires exactly one--permission-mode defaultper claude-code-action step, with no--flag=valueform and no--(allow-)dangerously-skip-permissions. It also fails on any workflow or composite action that uses claude-code-action but isn't listed. It has 13 mutant negative controls: flag dropped,auto,bypassPermissions, no value,=form, misspelled, commented out, given twice, bypass flag added, unlisted new lane, unlisted composite, expected lane removed, step renamed away.claude-review.ymlis rewritten. The old hold reason ("later ones bundle Claude Code 2.1.277+, which the adversarial and verifier lanes would run") is stale, because every lane now runs its own pinned CLI. The comment now lists what a bump must re-check: the caller and review-ran check assumptions, the composite's step order, and the SDK's permission-mode default together with howclaude_argsreaches the CLI.Verification
claude_args carries --permission-mode 0 times; 10 failed, 4 passed) and GREEN after (14 passed). As an on-disk negative control I set the adversarial lane to--permission-mode auto; the test failed with--permission-mode is 'auto', not 'default', and passed again once the file was restored.uv sync && uv run --no-sync pytest -q: 801 passed, 1 skipped (a pre-existing conditional skip), 9m37s. This includes every.shselftest throughtest_workflow_guards.py.actionlint -color -shellcheck=(1.7.12, the CI pin): clean.ruff check testsandruff format --check tests(0.15.10, CI's scope): clean.ruff check .: clean.shellcheck -S errorover all 56.shscripts: clean.Supersedes #282 (left open for the owner to close).
Codex rounds: 2
codex review --uncommitted): 0 findings.codex review --uncommitted): 0 findings.Auto-merge rationale: never — ci-workflows PRs are always manual click-merge; .github/workflows high-risk.
🤖 Generated with Claude Code