ci: gate the docs site build on pull requests - #4895
Conversation
The Astro toolchain had no pull-request build gate. ci.yml contained no docs-site reference and built no docs, and deploy-docs.yml triggers only on push to main, which is after promotion. The first machine that could discover a broken docs build was the deploy, so a dependency bump under docs-site/ could only be backed by an author's local run. Adds a docs filter to the existing changes job and one job selected by it. docs-site/** is deliberately not added to the ci filter: a prose edit has no business starting the cross-platform matrix, it only has to build. A separate filter output keeps those two questions apart. The workflow file itself is in the docs filter so an edit to the job verifies itself, without which this change's own pull request would skip the job it adds. One Linux leg. The site is static output from a Node/Bun toolchain with no OS-specific behaviour to promise, so another platform would spend queue time without buying coverage. --frozen-lockfile carries as much of the value as the build does, because it fails on a manifest and lockfile that disagree, which is the shape a hand-edited override introduces. The aggregate ci gate is event-aware, so the job is declared in the four places that have to agree: needs, CHANGES_DOCS, GATED_JOBS, and expected_for. No existing job's sharding, timeout, or runner selection changes, no Windows leg is added, and permissions stay contents: read.
|
Warning Review limit reachedNext included review available in 33 minutes. View limit detailsLimit details: You’ve used all 10 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
리뷰 · 우선순위 77 / 80이 PR은 지금 지금 그 구멍은 이미 #4873(Astro 7.2→7.3 등 deps 감사) 리뷰에서 드러났습니다. 그 헤드가 아무리 초록이어도 문서 사이트가 실제로 빌드되는지는 아무 것도 말하지 않습니다. 이 Unit C(#4895)는 그 리뷰를 정직하게 닫을 수 있게 하려는 선행 작업입니다. 계획 문서도 하는 일은 단순합니다. 집계 게이트(
라인 - 문제 .github/workflows/ci.yml docs 필터에 메인테이너의 판단이 필요한 지점
너의 추천 KEEP. types/config 분할에 무효화되지 않고, 중복도 아니며, 권한·시크릿 확장이 없습니다. 이 헤드에서 이 댓글은 grok-bot이 작성했습니다 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff350f93eb
ℹ️ 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".
| - name: Setup project Bun | ||
| uses: ./.github/actions/setup-project-bun |
There was a problem hiding this comment.
Match the deployment toolchain in the docs gate
The PR gate installs the root-pinned Bun version (1.4.0 in this commit), but .github/workflows/deploy-docs.yml still builds with Node 22 and package-manager: bun@latest. Once bun@latest differs—or if the Node-backed Astro action behaves differently—a docs change can pass this gate and then fail during the post-promotion Pages build, defeating the gate's stated purpose. Pin the deploy workflow to the same toolchain or exercise the deployment action/configuration here.
Useful? React with 👍 / 👎.
|
✅ Deterministic PR hygiene checks passed. |
Summary
The Astro toolchain had no pull-request build gate.
.github/workflows/ci.ymlcontained nodocs-sitereference and built no docs, anddeploy-docs.ymltriggers only onpushtomain, which is after promotion. So the first machine that could discover a broken docs site was the deploy, and a dependency bump underdocs-site/could only be backed by an author's local run.That is not hypothetical. It is why the review of #4873, which moves
astrofrom 7.2.2 to 7.3.3, could not close: a fully green run on that head would still have said nothing about whether the site builds.This adds a
docsfilter to the existingchangesjob and one job selected by it.docs-site/**is deliberately not added to thecifilter. Wideningciwould start the whole cross-platform matrix for a prose edit, which is cost without matching evidence: a docs change has to build, not to pass the runtime suite. A separate filter output keeps the two questions apart..github/workflows/ci.ymlis in thedocsfilter so an edit to the job verifies itself. Without that entry this pull request would skip the job it adds, which is the exact failure mode being removed.One Linux leg. The site is static output from a Node/Bun toolchain with no OS-specific behaviour to promise, so a Windows or macOS leg would spend queue time without buying coverage.
--frozen-lockfilecarries as much of the value as the build does: it fails on a manifest and lockfile that disagree, which is the shape a hand-edited override introduces.The aggregate
cigate is event-aware, and it fails by name on any job with no declared expectation, so the job is declared in the four places that have to agree:needs,CHANGES_DOCS,GATED_JOBS, andexpected_for. Requested when the filter is true, requiredskippedotherwise, exactly like the other scoped jobs.No existing job's sharding, timeout, or runner selection changes. No Windows leg is added. Workflow-level
permissionsstaycontents: readand the new job adds none. Actions stay pinned to immutable SHAs with their version comments.Verification
This pull request is its own first evidence. Because the workflow file is in the
docsfilter,docs site buildis requested on this head, so a green run here means the filter selects correctly, the job installs and buildsdocs-site, and the aggregate accepts it.tests/ci-workflows/ci-workflows.test.tsindependently derives the expectedneedslist from the workflow's own job keys and asserts every job carries a numerictimeout-minutes, so a job added without being gated fails rather than passing quietly. It also asserts the pinned action SHAs this job reuses.Checked statically against the gate's own accounting: the workflow now declares 14 jobs,
ciis the only ungated one, andneeds,GATED_JOBS, andexpected_foreach cover the remaining 13.Verified by reading the workflow and the call graph rather than by running the suite locally, so hosted CI at this head is the verification of record.
structure/ops/docs-and-release.mdownsdocs-site/and is updated in the same change: the deploy workflow is now described as the deploy path rather than a review gate, with the new job named as the pull-request gate, and the workflow map row updated.Checklist
This changes a GitHub Actions workflow, which
MAINTAINERS.mdand.github/AGENTS.mdboth place under explicit security review. The relevant surface: no trigger is broadened (pull_requestalready started this workflow for every PR), no permission is added, no secret is read, and no third-party action is introduced.