ci: sponsor high-blast-radius surfaces, consolidate the governance stack - #920
Conversation
…n the docs Consolidates the parts of @Wibias's five-PR governance stack (#900, #902, #905) that reduce risk without adding friction, and drops the parts that do the opposite. The measurement behind the ordering: Windows was the last job to finish in 23 of 23 recent CI runs at a 17m41s median, so #899 was the actual bottleneck and everything here is judged by whether it makes the lane worse. Kept, from #902's trust lane: authentication, credential handling, GitHub Actions workflows, release automation, and dependency installation need a maintainer to sponsor the change before it merges. MAINTAINERS.md already requires security review for exactly these; this makes the requirement visible on the pull request instead of relying on a reviewer noticing. It runs inside the existing hygiene job rather than adding a workflow, and it applies to every contributor — blast radius does not depend on how many PRs someone has merged, which is why the upstream first-timer exemption is gone. Dropped, from the same PR: the 500-line cap and the one-open-PR limit. A provider preset with its registry rows, adapter wiring, tests, and five locales clears 500 lines by itself, and several good first contributions here have. Telling a newcomer their fix is too big is a worse failure than reviewing a large diff. Dropped, from #900: the admission gate requiring a pre-approved issue, and the five-day auto-close. The DeepSeek reasoning replay, the Cursor Grok parameters, the AgentRouter EOF tolerance, the tool-result image forwarding — every one arrived as an unplanned PR from someone who hit the bug. A gate that required a planning discussion first would have lost all of them. Kept, from #905: CODEOWNERS entries for the high-impact runtime directories, and the contributor documentation — rewritten to describe what is actually enforced. The submitted version documented the approved-for-work gate, the size caps, and the automatic closure timers, none of which exist here, and publishing rules the repository does not enforce is worse than publishing none. Not included: #901's readiness gate. It makes CodeRabbit's judgment blocking and triggers per check_run, which scales with the job count #899 just raised. Co-authored-by: Wibias <37517432+Wibias@users.noreply.github.com>
|
Warning Review limit reached
Next review available in: 14 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
Comment |
github-actions
Bot
added
the
chore
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04ccb7939a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Sponsorship is head-independent: it is about which surfaces the | ||
| // change touches, not about the state of a particular revision, so | ||
| // it is NOT cleared by the synchronize sweep above. |
There was a problem hiding this comment.
When a sponsored PR receives a synchronize event, this deliberately preserves maintainer-sponsored, so an author can obtain sponsorship for one revision and then push unreviewed authentication, release, or workflow changes while the gate remains green. Clear the label alongside the other approvals or bind sponsorship to the reviewed head SHA so each security-sensitive revision receives explicit review.
AGENTS.md reference: .github/AGENTS.md:L5-L8
Useful? React with 👍 / 👎.
| authorHasPushPermission: ["OWNER", "MEMBER", "COLLABORATOR"].includes( | ||
| pr.author_association, | ||
| ), | ||
| changedFiles: files.map((file) => file.filename), |
There was a problem hiding this comment.
The listFiles response places the old side of a rename in previous_filename, but only the new filename is passed to the sponsorship classifier. Renaming src/oauth/store.ts to an unrestricted path therefore bypasses sponsorship even when the PR also rewrites that authentication code; collect both names, as assessHygiene already does for rename-sensitive checks.
AGENTS.md reference: AGENTS.md:L199-L205
Useful? React with 👍 / 👎.
| const RESTRICTED_PREFIXES = [ | ||
| ".github/workflows/", | ||
| "src/oauth/", | ||
| ]; |
There was a problem hiding this comment.
This allowlist does not cover the credential-handling scope advertised by the check and documentation: for example, src/providers/api-keys.ts persists provider secrets and src/codex/account-store.ts persists and refreshes access/refresh tokens, yet neither matches these prefixes nor appears in RESTRICTED_FILES. A contributor can change either file without sponsorship, so the classifier needs to include the actual credential stores and handlers rather than only selected auth filenames.
AGENTS.md reference: AGENTS.md:L199-L205
Useful? React with 👍 / 👎.
| labels = [], | ||
| }) { | ||
| // A maintainer's own change carries its own sponsorship. | ||
| if (authorHasPushPermission) return []; |
There was a problem hiding this comment.
Returning here exempts every push-capable author's PR before its restricted paths are inspected, treating the author's own review as sponsorship. MAINTAINERS.md:30-34 explicitly disallows authors approving their own PRs and separately requires security review for these surfaces, so maintainer-authored authentication or workflow changes receive no visible security-review gate; require sponsorship from a distinct maintainer instead.
AGENTS.md reference: .github/AGENTS.md:L5-L8
Useful? React with 👍 / 👎.
| function hasSponsorship(labels) { | ||
| return (labels || []).some( | ||
| (label) => (typeof label === "string" ? label : label?.name) === "maintainer-sponsored", | ||
| ); |
There was a problem hiding this comment.
hasSponsorship trusts only the label name and discards who applied it, so any repository user with label permission can satisfy the security-review gate. This includes writers outside the deliberately narrow security ownership listed in MAINTAINERS.md:98-101, allowing them to sponsor another contributor's authentication, Actions, or release change; validate and persist an authorized labeling actor or derive the verdict from an eligible review instead.
AGENTS.md reference: .github/AGENTS.md:L5-L8
Useful? React with 👍 / 👎.
| - **Cross-platform CI.** The suite runs sharded on Linux and in full on macOS for | ||
| every pull request. Windows runs at the shipping boundary — on promotion to | ||
| `main` or `preview` — so a slow or flaky Windows runner cannot decide when your | ||
| pull request turns green. |
There was a problem hiding this comment.
This new page says Windows runs only at the shipping boundary, while docs-site/src/content/docs/contributing.md:66-70 still tells contributors that pull-request CI has Linux, Windows, and macOS coverage plus a second three-OS lane. The contradictory canonical English pages leave authors unsure whether a PR was tested on Windows; update the existing contributing page alongside this new description so both match ci.yml.
AGENTS.md reference: docs-site/AGENTS.md:L7-L10
Useful? React with 👍 / 👎.
1 participant
Summary
Consolidates the parts of @Wibias's five-PR governance stack worth keeping into
one PR, and drops the parts that work against the goal the stack was opened for.
The ordering came from measurement, not preference. Over 33 recent Cross-platform
CI runs, Windows was the last job to finish in 23 of 23 current-schema runs
(median 17m41s against ubuntu 5m58s), which is why #899 went first. Everything
here is judged by whether it makes that lane worse.
What this adds
Sponsored surfaces (from #902). Authentication, credential handling, GitHub
Actions workflows, release automation, and dependency installation need a
maintainer to apply
maintainer-sponsoredbefore merge.MAINTAINERS.mdalreadyrequires security review for exactly these — this makes the requirement visible
on the PR instead of relying on a reviewer noticing.
It runs inside the existing hygiene job rather than adding a workflow, and it
applies to every contributor. Blast radius does not depend on how many PRs
someone has merged, so the upstream first-timer exemption is gone. An author with
push permission is exempt because their own review is the sponsorship.
CODEOWNERS for the high-impact runtime directories (from #905).
Contributor documentation (from #905), rewritten. The submitted version
documented an approved-for-work gate, size caps, and automatic closure timers —
none of which exist here. Publishing rules the repository does not enforce is
worse than publishing none.
What this drops, and why
check_run, which scales with the job count #899 just raised — up to 18–24 invocations per update.Verification
node --test .github/scripts/pr-sponsored-surface.test.cjs: 7 pass, 0 failnode --test .github/scripts/pr-hygiene.test.cjs: 21 pass, 0 failbun test tests/ci-workflows.test.ts: 66 pass, 0 failissue-quality-tests.yml, which is where thisrepository already runs its policy-script tests
Credit
The sponsorship rule, the CODEOWNERS entries, and the documentation skeleton are
@Wibias's work from #900/#902/#905; the commit carries his co-authorship. The
narrowing, the rewrite, and the dropped policies are mine, and the reasons are
recorded on each original PR rather than only here.