stack 3/5: add first-contributor trust lane - #902
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Stack navigation
Review and merge bottom-up. Each PR targets the preceding stack branch, so its Files changed view contains only that layer. |
…arded dispatch, reuse permission helper
[shipping-github] Verdict: approve-commentPR: Semantic propagation
UsefulnessHigh: a concrete, metadata-based trust lane (one active implementation PR, 500-line cap, sponsorship for workflow/auth/release/dependency surfaces) that closes the "first PR is free" gap in the stack without AI-origin guessing. Without it the admission gate alone cannot protect security-sensitive surfaces from first-time contributors. Bugs / correctness
Security
Spec / standards
Reviews
Base / CI
GateNone: draft cleared, PR is ready for review. Simplification (approved S1)
Bottom lineSubstantively clean and approve-worthy after the fix batch: the gate now actually runs, renames and closed issues cannot bypass it, |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Same read as #900: the implementation is fine, the policy is the open question.
The size and concurrency caps are where I hesitate. 500 lines rejects, for example, a provider preset with its registry rows, adapter wiring, tests, and five-locale docs — which is exactly the shape of a good first contribution here, and several have landed at more than that. The one-PR limit also penalizes someone who finds two unrelated bugs in an afternoon. What I would merge without further discussion: the sponsorship requirement for sensitive surfaces, as its own check. What needs a decision from me first: the caps. Leaving this open rather than closing it — if you feel strongly that the caps carry their weight, make that case and I will read it properly. My prior is that the deterministic hygiene gate in #903 catches most of what the size cap is proxying for, without telling a newcomer their fix is too big. |
|
Taking half of this, and closing the PR rather than leaving it open on a policy question I have now answered. Kept — the sponsorship requirement, carried into #920 with your co-authorship: authentication, credential handling, workflows, release automation, and dependency installation need Dropped — 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. Someone who finds two unrelated bugs in an afternoon should be able to send both. Telling a newcomer their fix is too big is a worse failure than reviewing a large diff. The restricted-path list in |
…n the docs Consolidates the parts of @Wibias's five-PR governance stack (lidge-jun#900, lidge-jun#902, lidge-jun#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 lidge-jun#899 was the actual bottleneck and everything here is judged by whether it makes the lane worse. Kept, from lidge-jun#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 lidge-jun#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 lidge-jun#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: lidge-jun#901's readiness gate. It makes CodeRabbit's judgment blocking and triggers per check_run, which scales with the job count lidge-jun#899 just raised. Co-authored-by: Wibias <37517432+Wibias@users.noreply.github.com>
Stack
3/5 — first-contributor trust lane
Base:
agent/pr-readiness-gate(#901)Next: deterministic anti-slop CI
Summary
large-change-approvedmaintainer-sponsoredfor workflow, OAuth/authentication, release, and dependency surfacesVerification
node --test .github/scripts/pr-trust-lane.test.cjs— 8 passed, 0 failedRollout
Tune the 500-line threshold and restricted path list during draft review before promoting the trusted workflow to the default branch.