Skip to content

Commit aced300

Browse files
mchwangclaude
andauthored
Read classic required checks for a token without admin rights (#186)
A token without admin rights got 404 "Not Found" from branches/<b>/protection on every protected branch checked, so its merge rules read as unknown and merging was blocked. The branch read (branches/<b>) showed such a token protection.enabled and protection.required_status_checks (enforcement_level, contexts, checks) on the public repositories checked. Use it on a "Not Found" answer: enabled false with enforcement off and no checks is no classic protection; enabled true takes its checks, held to the protection read's rules. It has no `strict`, so it never gives the atomic base guard. Every other answer still fails closed. Classic protection off whose required-checks object is missing or contradicts itself (enforced, or listing checks) was read as no classic protection and now fails closed too. Recorded with gh 2.102.0 from public repositories read without admin rights: vitejs/vite (ruleset only), cli/cli (classic only) and sveltejs/svelte (both). Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent c0b8bf0 commit aced300

10 files changed

Lines changed: 338 additions & 33 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ An existing-store configuration may add a trusted GitHub binding:
3131
}
3232
```
3333

34-
The issue must match the stored plan. `pullRequest` is required unless the configuration has a `runner` block (see below). The authenticated `gh` account must be able to read the pull request, issue timeline, applicable rulesets, and classic branch protection, and to merge the PR. Codeboost unions required checks from both rule sources, requires strict server-enforced current-base checks, rechecks the base and head immediately before merging, and passes the reviewed head to `gh pr merge --match-head-commit`. Missing permissions and ambiguous rule responses block the merge. When the branch uses a merge queue, codeboost adds the exact reviewed head to the queue and treats the merge as done only when GitHub confirms it merged; a queued pull request is not merged. If GitHub removes it from the queue while the plan, snapshot, review and head are unchanged, Merge offers a retry after the status refresh; if the head changed, it goes back to review first. With the opt-in runner, `prepare-merge` handles a moved base and runs every allowed `cmd:` acceptance check on the resulting exact head; merge remains blocked until that evidence is current.
34+
The issue must match the stored plan. `pullRequest` is required unless the configuration has a `runner` block (see below). The authenticated `gh` account must be able to read the pull request, issue timeline, applicable rulesets and branch, and to merge the PR. Without admin rights, classic required checks are read from the branch (observed with read-only access to public repositories; write access was not checked), but classic protection's strict setting cannot be seen, so codeboost's Merge then needs a strict ruleset or a merge queue ([guarded-merge.md](docs/implementation/guarded-merge.md)). Codeboost unions required checks from both rule sources, requires strict server-enforced current-base checks, rechecks the base and head immediately before merging, and passes the reviewed head to `gh pr merge --match-head-commit`. Missing permissions and ambiguous rule responses block the merge. When the branch uses a merge queue, codeboost adds the exact reviewed head to the queue and treats the merge as done only when GitHub confirms it merged; a queued pull request is not merged. If GitHub removes it from the queue while the plan, snapshot, review and head are unchanged, Merge offers a retry after the status refresh; if the head changed, it goes back to review first. With the opt-in runner, `prepare-merge` handles a moved base and runs every allowed `cmd:` acceptance check on the resulting exact head; merge remains blocked until that evidence is current.
3535

3636
## Runner (opt-in)
3737

‎docs/architecture.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,7 @@ These are the decisions that shape the structure. The full list, with the reason
410410
| C, K | Guarded merge, merge queue | Done |
411411
| D | Agent isolation boundary | Done. Since the isolation gate: bounded diff export (#76), task-change inspection and the runner's commit as a bounded `git bundle` (#66, PRs #85 and #90), owner-scoped recovery for Ask (#95), the plan schema for Claude (#97), gitlink mounts and the pre-launch tree check (#81, #99, PR #100), and Codex refused in every phase (#93, PR #106). The runner-contract follow-ups (#51) are closed: the last one tests export and removal through a recovery handle after a real restart (PR #109). |
412412
| E | Planning library and suggestions | Done. E4 (PR #45) added the acceptance fixtures and recorded Claude output that replays through validation and the Store. |
413-
| F | Runner | Merged: F1 (lifecycle store, coordinator, shutdown wiring, startup recovery and the lock, planning API, feedback events, and stops that wait for their commit: #53, #74, #57, #59, #60, #96); F2a–F2b (execute prompts, audit, per-item execution, real workspace, durable safety findings: #67, #68, #92, #94, #128); F2d (already-fixed check, PR opening, branch pusher, production publishing, closing PRs on cancel: #84, #101, #110, #115, #116, #119, #123); production wiring and `start`/`resume` (#91, PRs #102, #105); planning through lane D (#117, #124, PRs #118, #120, #125); merging the task's own PR (#121, PR #126); start/resume approval, review-version, recovery and completed-prefix guards (#107, PR #130); continuation after a scope pause (#88, PR #134); the F3 durable trusted local-rebase foundation (#22, PR #136); the F4 foreign-conflict engine plus production sandbox adapter (#22, PRs #138 and #140); and sandboxed owned-commit conflict resolution (#22, PR #144). F5 (PR #149) added current base/head refresh, rebase admission, post-rebase attribution and approval refresh, and head-bound `cmd:` execution. F6a (PR #153) added the durable push of the rewritten head. F6b (PR #161) added the one-click merge job (preparation, the required-check wait, the exact-pair guarded merge) and an already-fixed scan covering every base commit after the task's original base. F6c (PR #177) runs every T9 suite in CI and adds the nightly live Claude probes; T9 is complete (PR #178) and #22 is closed; its follow-ups are #174 and #176. T6 and T12 are verified, and PR #183 also fixed reading required checks that come only from a ruleset, for a token with admin rights. Startup now refuses a `pathIdentity` that disagrees with a probe of task storage (#162, PR #180); follow-ups #179 and #181. |
413+
| F | Runner | Merged: F1 (lifecycle store, coordinator, shutdown wiring, startup recovery and the lock, planning API, feedback events, and stops that wait for their commit: #53, #74, #57, #59, #60, #96); F2a–F2b (execute prompts, audit, per-item execution, real workspace, durable safety findings: #67, #68, #92, #94, #128); F2d (already-fixed check, PR opening, branch pusher, production publishing, closing PRs on cancel: #84, #101, #110, #115, #116, #119, #123); production wiring and `start`/`resume` (#91, PRs #102, #105); planning through lane D (#117, #124, PRs #118, #120, #125); merging the task's own PR (#121, PR #126); start/resume approval, review-version, recovery and completed-prefix guards (#107, PR #130); continuation after a scope pause (#88, PR #134); the F3 durable trusted local-rebase foundation (#22, PR #136); the F4 foreign-conflict engine plus production sandbox adapter (#22, PRs #138 and #140); and sandboxed owned-commit conflict resolution (#22, PR #144). F5 (PR #149) added current base/head refresh, rebase admission, post-rebase attribution and approval refresh, and head-bound `cmd:` execution. F6a (PR #153) added the durable push of the rewritten head. F6b (PR #161) added the one-click merge job (preparation, the required-check wait, the exact-pair guarded merge) and an already-fixed scan covering every base commit after the task's original base. F6c (PR #177) runs every T9 suite in CI and adds the nightly live Claude probes; T9 is complete (PR #178) and #22 is closed; its follow-ups are #174 and #176. T6 and T12 are verified, and PR #183 also fixed reading required checks that come only from a ruleset, for a token with admin rights. Required checks are now also read for a token without admin rights (observed with read-only access to public repositories): classic checks then come from the branch read, which has no `strict`, so classic protection gives such a token no atomic base guard. Startup now refuses a `pathIdentity` that disagrees with a probe of task storage (#162, PR #180); follow-ups #179 and #181. |
414414
| Hardening | Allowlisted subprocess environments | Done: every Git command (#82, #83) and every `gh` runner (#84, #86). |
415415
| Tests | Test suite reliability | Done. The full suite runs ordinary tests first and Docker-backed files serially; CI and the real-Docker gate passed on PR #132 (#129). |
416416
| G | Planning screen | Done (#145). The screen displays/imports the current plan, requests and renders a next-revision draft or suggestion cards, and applies results with revision-bound exact ambiguous replay. G4 composes the browser with the production planning setup, D-backed provider boundary, F endpoints and SQLite persistence; focused adapter, recorded-output and exact-head real-Docker planning suites cover the lower boundary. |

‎docs/designs/codeboost-plan-indexed-review.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2043,7 +2043,7 @@ These `T` IDs are requirement identifiers, not the build-order numbers. Build st
20432043
- Surfaced by: R6 (D7: A)
20442044
- Files: github/checks
20452045
- Verify: recorded-output tests for zero checks, pending then passing, and timeout
2046-
- Evidence (2026-10-20): `test/merge-recorded.test.ts` replays GitHub's real answers, recorded with gh 2.102.0 from this repository by `scripts/record-merge-gh.ts` (`test/fixtures/gh/`). Zero checks: pull request 104, into a branch no rule covers, reads as an empty required set with the rules known. Pending then passing: pull request 183 at one head, read while `test` and `real-docker` ran and after they passed. Through the merge job, the wait sees both checks pending, then merges once they pass; at the time limit with the checks still pending, the job ends `blocked`. For the job, the recorded ruleset is changed in one field, its strict policy, because the repository's ruleset is not strict, so a job against it stops at once for the missing atomic base guard (T7), which is also tested ("ends the job for a person, before waiting, against the repository's real non-strict ruleset"). Zero checks pass a job only with an atomic base guard, and with no required checks only a merge queue gives one (a strict policy guards only a non-empty set). That job case is tested with injected states: a merge queue and no required checks (`test/merge-job.test.ts`, "ends queued when the merge queue accepted the head"). The recordings found two bugs, fixed in PR #183: a ruleset check with no `integration_id` and a ruleset-only branch's 404 "Branch not protected" both made the rules unknown, so a branch whose required checks come only from a ruleset (this repository's `main` among them) blocked merging. The 404 was recorded with a token with admin rights; a token without them may get "Not Found" instead, which still blocks merging as unknown rules. Reviews are not checks: required approving reviews in a ruleset or classic protection are ignored, and only their required checks count (`test/merge.test.ts`, "ignores required reviews in both rule sources and reads only their required checks"). #176 stays open and is tracked under build step 4, not here: a required check pinned to an app always reads as missing, which fails closed (the job blocks at its time limit) rather than merging without the check (`test/merge.test.ts`, "reads a requirement pinned to an app as missing against gh's app-less check runs"; a missing check blocks the job at its time limit: `test/merge-job.test.ts`, "ends approved, merge blocked when required checks do not pass in time"). No recording has an app-pinned check
2046+
- Evidence (2026-10-20): `test/merge-recorded.test.ts` replays GitHub's real answers, recorded with gh 2.102.0 from this repository by `scripts/record-merge-gh.ts` (`test/fixtures/gh/`). Zero checks: pull request 104, into a branch no rule covers, reads as an empty required set with the rules known. Pending then passing: pull request 183 at one head, read while `test` and `real-docker` ran and after they passed. Through the merge job, the wait sees both checks pending, then merges once they pass; at the time limit with the checks still pending, the job ends `blocked`. For the job, the recorded ruleset is changed in one field, its strict policy, because the repository's ruleset is not strict, so a job against it stops at once for the missing atomic base guard (T7), which is also tested ("ends the job for a person, before waiting, against the repository's real non-strict ruleset"). Zero checks pass a job only with an atomic base guard, and with no required checks only a merge queue gives one (a strict policy guards only a non-empty set). That job case is tested with injected states: a merge queue and no required checks (`test/merge-job.test.ts`, "ends queued when the merge queue accepted the head"). The recordings found two bugs, fixed in PR #183: a ruleset check with no `integration_id` and a ruleset-only branch's 404 "Branch not protected" both made the rules unknown, so a branch whose required checks come only from a ruleset (this repository's `main` among them) blocked merging. The 404 was recorded with a token with admin rights; a token without them may get "Not Found" instead, which still blocks merging as unknown rules. Reviews are not checks: required approving reviews in a ruleset or classic protection are ignored, and only their required checks count (`test/merge.test.ts`, "ignores required reviews in both rule sources and reads only their required checks"). #176 stays open and is tracked under build step 4, not here: a required check pinned to an app always reads as missing, which fails closed (the job blocks at its time limit) rather than merging without the check (`test/merge.test.ts`, "reads a requirement pinned to an app as missing against gh's app-less check runs"; a missing check blocks the job at its time limit: `test/merge-job.test.ts`, "ends approved, merge blocked when required checks do not pass in time"). The PR #183 recordings have no app-pinned check. *Later:* a token without admin rights does get "Not Found", on ruleset-only and classic-protected branches alike; the rules are now read for it, with classic checks from the branch read, replayed from recordings of vitejs/vite, cli/cli and sveltejs/svelte (`guarded-merge.md`). The vite and svelte recordings have app-pinned checks, which read as missing (#176).
20472047
- [x] **T13 (P2, human: ~1 hour / CC: ~5 min)** — runner — Use `node:sqlite` only; enforce the minimum Node version; add a CI warning check
20482048
- Surfaced by: R7 (D8: A)
20492049
- Files: runner/store, web/cli

0 commit comments

Comments
 (0)