docs(ci): add .buildkite/ scaffold + Buildkite setup runbook#486
docs(ci): add .buildkite/ scaffold + Buildkite setup runbook#486mattwilkinsonn wants to merge 15 commits into
docs(ci): add .buildkite/ scaffold + Buildkite setup runbook#486Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Buildkite CI: README and placeholders, an orchestrator pipeline that fans out to downstream pipelines via monorepo-diff, a ChangesBuildkite CI Infrastructure Documentation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has required the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Greptile SummaryEstablishes the Buildkite CI infrastructure: a
Confidence Score: 4/5Safe to merge with the understanding that the open prior-thread issues will be addressed in follow-up commits. The fan-out script calls .buildkite/scripts/fan-out.sh (missing fetch before merge-base), .buildkite/images/Dockerfile.ci (SHA verification gaps), .buildkite/pipelines/orchestrator.yml (mutable graphite-ci plugin ref) Important Files Changed
Reviews (30): Last reviewed commit: "fix(ci): junit artifact copied past the ..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.buildkite/README.md:
- Around line 37-39: Update the phrasing for the `nightly.yml` bullet so it
doesn't claim "non-x64 runners" while listing `macos-26-intel`; instead describe
that `nightly.yml` runs across multiple architectures/hosts — e.g., "coverage
for multiple architectures/hosts (e.g., `ubuntu-24.04-arm`, `macos-latest`,
`macos-26-intel`)" — so the entry for `nightly.yml` accurately reflects
inclusion of both ARM and Intel mac runners.
In `@docs/buildkite-setup.md`:
- Around line 31-34: The text incorrectly treats "pull_request: synchronize" as
a separate event; update the sentence to say Buildkite listens to the GitHub
'pull_request' event (including actions like 'synchronize') and 'push' events,
so rephrase to clarify that 'synchronize' is an action under 'pull_request'
rather than its own event and ensure the identifiers 'pull_request' and
'synchronize' are used precisely in the sentence.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ac2e0551-487e-416a-96f3-7661eec839b2
📒 Files selected for processing (4)
.buildkite/README.md.buildkite/hooks/.gitkeep.buildkite/pipelines/.gitkeepdocs/buildkite-setup.md
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.buildkite/pipelines/orchestrator.yml (1)
169-183:⚠️ Potential issue | 🟡 MinorPrevent duplicate
seal-live-teststriggers when both LLM paths change and thelive-testslabel is present
- Buildkite doesn’t automatically deduplicate multiple
triggersteps that target the same downstream pipeline; each matching trigger step creates its own downstream build..buildkite/pipelines/orchestrator.ymlcontains two separateseal-live-teststriggers (the LLM path-filtered one plus the label-gated onebuild.pull_request.labels includes "live-tests"), with no cancellation/dedup mechanism configured here.- Mitigate by enabling “Cancel Intermediate Builds” on the
seal-live-testspipeline (or document that the double-run is intentional/acceptable).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.buildkite/pipelines/orchestrator.yml around lines 169 - 183, The pipeline has two separate trigger steps that both call the downstream pipeline "seal-live-tests" (the LLM path-filtered trigger plus the label-gated trigger labeled ":satellite: Live API Tests (label-triggered)"), which can cause duplicate downstream builds when a PR both changes LLM paths and has the "live-tests" label; fix by enabling "Cancel Intermediate Builds" (or equivalent automatic dedup/cancellation) on the downstream "seal-live-tests" pipeline OR add documentation/comments near the trigger step explaining the double-run is intentional/acceptable so maintainers know why duplicates occur; ensure the change references the trigger name "seal-live-tests" and the label-gated step to avoid accidental edits elsewhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/buildkite-setup.md`:
- Around line 156-160: The PR objectives claim GRAPHITE_TOKEN is no longer
required but the docs and pipeline still reference GRAPHITE_TOKEN and the
orchestrator injects graphite_token: "$$GRAPHITE_TOKEN"; either update the PR
objectives summary to state that GRAPHITE_TOKEN is required, or remove
references and injection of GRAPHITE_TOKEN from docs/buildkite-setup.md and
.buildkite/pipelines/orchestrator.yml; specifically reconcile and change the PR
objectives text to match the actual state or remove the env var usage
(GRAPHITE_TOKEN and graphite_token) from the orchestrator pipeline and docs so
they are consistent.
In `@Justfile`:
- Around line 159-216: The bk download URL is incorrect; update the bk_url
construction in the bk installation block (where bk_tag is resolved) to use the
published asset name pattern "bk_${bk_tag}_linux_${bk_arch}.tar.gz" instead of
"cli_${bk_tag}_linux_${bk_arch}.tar.gz" so the URL becomes
https://github.com/buildkite/cli/releases/download/v${bk_tag}/bk_${bk_tag}_linux_${bk_arch}.tar.gz;
ensure this change is made in the code that sets bk_url (the section using
bk_tag and bk_arch before fetching and extracting bk).
---
Outside diff comments:
In @.buildkite/pipelines/orchestrator.yml:
- Around line 169-183: The pipeline has two separate trigger steps that both
call the downstream pipeline "seal-live-tests" (the LLM path-filtered trigger
plus the label-gated trigger labeled ":satellite: Live API Tests
(label-triggered)"), which can cause duplicate downstream builds when a PR both
changes LLM paths and has the "live-tests" label; fix by enabling "Cancel
Intermediate Builds" (or equivalent automatic dedup/cancellation) on the
downstream "seal-live-tests" pipeline OR add documentation/comments near the
trigger step explaining the double-run is intentional/acceptable so maintainers
know why duplicates occur; ensure the change references the trigger name
"seal-live-tests" and the label-gated step to avoid accidental edits elsewhere.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b12ca056-d8c2-4579-a0d5-edd3b391488d
📒 Files selected for processing (7)
.buildkite/README.md.buildkite/pipelines/lints.yml.buildkite/pipelines/orchestrator.yml.buildkite/scripts/lints-install-tools.sh.github/workflows/ci.ymlJustfiledocs/buildkite-setup.md
💤 Files with no reviewable changes (1)
- .github/workflows/ci.yml
mattwilkinsonn
force-pushed
the
sea-587-buildkite-migration--poseidon
branch
from
6389736 to
8b87901
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.buildkite/pipelines/orchestrator.yml (1)
39-39:⚠️ Potential issue | 🟠 MajorPin the Graphite Buildkite plugin to an immutable ref (don’t use
#main)In
.buildkite/pipelines/orchestrator.yml(line 39),- withgraphite/graphite-ci#main:should be changed to- withgraphite/graphite-ci#<git-tag>or the most robust- withgraphite/graphite-ci#<full-commit-sha>. Buildkite recommends immutable plugin refs (tags/SHAs) and avoiding branch refs like#main; Graphite’s plugin repo does not appear to provide stable version tags, so pinning to a specific commit SHA is the practical option here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.buildkite/pipelines/orchestrator.yml at line 39, Replace the mutable plugin ref "- withgraphite/graphite-ci#main:" with an immutable ref by pinning to a specific tag or full commit SHA (e.g. "- withgraphite/graphite-ci#<git-tag>" or "- withgraphite/graphite-ci#<full-commit-sha>") so the Buildkite pipeline uses a fixed plugin version; update the occurrence of "- withgraphite/graphite-ci#main:" accordingly in the orchestrator pipeline definition.Justfile (1)
184-207:⚠️ Potential issue | 🟠 Major | ⚡ Quick winVerify downloaded Buildkite binaries before installing them to PATH.
This block installs executables from GitHub release tarballs without checksum/signature validation, which weakens the trust boundary for local developer tooling.
Suggested hardening
- if curl -fsSL -o "$tmp_dir/bk.tar.gz" "$bk_url"; then + checks_url="https://github.com/buildkite/cli/releases/download/v${bk_tag}/checksums.txt" + if curl -fsSL -o "$tmp_dir/checksums.txt" "$checks_url" \ + && curl -fsSL -o "$tmp_dir/bk.tar.gz" "$bk_url" \ + && (cd "$tmp_dir" && grep "bk_${bk_tag}_linux_${bk_arch}.tar.gz" checksums.txt | sha256sum --check --strict); then tar -xzf "$tmp_dir/bk.tar.gz" -C "$tmp_dir" bk install -m 0755 "$tmp_dir/bk" "$bin_dir/bk"#!/usr/bin/env bash set -euo pipefail echo "Agent release assets:" gh api repos/buildkite/agent/releases/latest --jq '.assets[].name' echo echo "CLI release assets:" gh api repos/buildkite/cli/releases/latest --jq '.assets[].name'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Justfile` around lines 184 - 207, The script currently downloads buildkite-agent and bk tarballs using ba_url/bk_url and installs files from tmp_dir without integrity/authenticity checks; update the download-and-install blocks for buildkite-agent (ba_url, tmp_dir, buildkite-agent) and bk (bk_url, tmp_dir, bk) to (1) fetch corresponding release asset checksums or signatures (via the GitHub releases API or provided .sha256/.asc assets), (2) verify the downloaded tarball using sha256sum --check or gpg --verify before extracting, (3) abort and emit a clear error if verification fails, and (4) only run tar/extract and install -m 0755 into $bin_dir once verification succeeds, ensuring tmp_dir cleanup still happens on both success and failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.buildkite/pipelines/orchestrator.yml:
- Line 39: Replace the mutable plugin ref "- withgraphite/graphite-ci#main:"
with an immutable ref by pinning to a specific tag or full commit SHA (e.g. "-
withgraphite/graphite-ci#<git-tag>" or "-
withgraphite/graphite-ci#<full-commit-sha>") so the Buildkite pipeline uses a
fixed plugin version; update the occurrence of "-
withgraphite/graphite-ci#main:" accordingly in the orchestrator pipeline
definition.
In `@Justfile`:
- Around line 184-207: The script currently downloads buildkite-agent and bk
tarballs using ba_url/bk_url and installs files from tmp_dir without
integrity/authenticity checks; update the download-and-install blocks for
buildkite-agent (ba_url, tmp_dir, buildkite-agent) and bk (bk_url, tmp_dir, bk)
to (1) fetch corresponding release asset checksums or signatures (via the GitHub
releases API or provided .sha256/.asc assets), (2) verify the downloaded tarball
using sha256sum --check or gpg --verify before extracting, (3) abort and emit a
clear error if verification fails, and (4) only run tar/extract and install -m
0755 into $bin_dir once verification succeeds, ensuring tmp_dir cleanup still
happens on both success and failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 57ff2619-cbcf-4754-92a7-104b1ad3af35
📒 Files selected for processing (4)
.buildkite/pipelines/lints.yml.buildkite/pipelines/orchestrator.ymlJustfiledocs/buildkite-setup.md
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.buildkite/pipelines/orchestrator.yml (2)
25-42:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPin the Graphite-CI plugin to a stable version or commit SHA.
The plugin reference
withgraphite/graphite-ci#maintracks themainbranch, which means the pipeline can break without warning if the plugin changes. For production CI stability, pin to a specific release tag or commit SHA (e.g.,withgraphite/graphite-ci#v1.2.3orwithgraphite/graphite-ci#abc123).📌 Proposed fix to pin the plugin version
plugins: - - withgraphite/graphite-ci#main: + - withgraphite/graphite-ci#v1.0.0: # Replace with actual stable version/SHA # GRAPHITE_TOKEN injected by Buildkite's pipeline-level🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.buildkite/pipelines/orchestrator.yml around lines 25 - 42, The pipeline step using the Graphite CI plugin (label ":graphite: Graphite CI optimizer", plugin reference withgraphite/graphite-ci#main) should be pinned to a specific release tag or commit SHA instead of tracking `#main`; update the plugin reference `withgraphite/graphite-ci#main` to a fixed identifier like `withgraphite/graphite-ci#vX.Y.Z` or `withgraphite/graphite-ci#<commit-sha>` to ensure CI stability and avoid unexpected breakage.
43-53: 🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoffVerify that synchronous lints trigger aligns with orchestrator performance goals.
The
async: falseconfiguration blocks the orchestrator step until theseal-lintspipeline completes. While this ensures correct commit-status ordering for branch protection, it also means the orchestrator agent remains occupied during the entire lints run (~8min cold-cache budget per lints.yml). If downstream pipelines will eventually run concurrently (seal-test-linux, seal-test-macos, etc.), confirm whether the orchestrator should block on lints or trigger all pipelines asynchronously and rely on GitHub's commit-status aggregation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.buildkite/pipelines/orchestrator.yml around lines 43 - 53, The orchestrator step for the Lints pipeline currently uses "async: false" which blocks the orchestrator agent while "seal-lints" runs; change the step with label ":hammer_and_wrench: Lints" (trigger: "seal-lints") to "async: true" so the orchestrator no longer waits for completion, and add/adjust a brief comment noting we rely on GitHub commit-status aggregation for final gating; if you decide to keep synchronous behavior instead, document the performance tradeoff and leave "async: false" intentionally.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.buildkite/pipelines/orchestrator.yml:
- Around line 25-42: The pipeline step using the Graphite CI plugin (label
":graphite: Graphite CI optimizer", plugin reference
withgraphite/graphite-ci#main) should be pinned to a specific release tag or
commit SHA instead of tracking `#main`; update the plugin reference
`withgraphite/graphite-ci#main` to a fixed identifier like
`withgraphite/graphite-ci#vX.Y.Z` or `withgraphite/graphite-ci#<commit-sha>` to
ensure CI stability and avoid unexpected breakage.
- Around line 43-53: The orchestrator step for the Lints pipeline currently uses
"async: false" which blocks the orchestrator agent while "seal-lints" runs;
change the step with label ":hammer_and_wrench: Lints" (trigger: "seal-lints")
to "async: true" so the orchestrator no longer waits for completion, and
add/adjust a brief comment noting we rely on GitHub commit-status aggregation
for final gating; if you decide to keep synchronous behavior instead, document
the performance tradeoff and leave "async: false" intentionally.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 099b1c85-d833-4290-b824-0e6d6f3a6085
📒 Files selected for processing (2)
.buildkite/pipelines/lints.yml.buildkite/pipelines/orchestrator.yml
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| && if [ "${actionlint_arch}" = "amd64" ]; then \ | ||
| echo "${ACTIONLINT_SHA256} /tmp/${tarball}" | sha256sum --check --strict; \ | ||
| fi \ |
There was a problem hiding this comment.
SHA256 verification is only performed for amd64 architecture but skipped for arm64. This creates a security vulnerability where arm64 builds could download and execute a compromised actionlint binary without integrity verification.
# Fix: Add SHA256 verification for arm64 as well
ARG ACTIONLINT_SHA256_ARM64=<arm64_sha256_here>
...
&& if [ "${actionlint_arch}" = "amd64" ]; then \
echo "${ACTIONLINT_SHA256} /tmp/${tarball}" | sha256sum --check --strict; \
elif [ "${actionlint_arch}" = "arm64" ]; then \
echo "${ACTIONLINT_SHA256_ARM64} /tmp/${tarball}" | sha256sum --check --strict; \
fi \| && if [ "${actionlint_arch}" = "amd64" ]; then \ | |
| echo "${ACTIONLINT_SHA256} /tmp/${tarball}" | sha256sum --check --strict; \ | |
| fi \ | |
| && if [ "${actionlint_arch}" = "amd64" ]; then \ | |
| echo "${ACTIONLINT_SHA256} /tmp/${tarball}" | sha256sum --check --strict; \ | |
| elif [ "${actionlint_arch}" = "arm64" ]; then \ | |
| echo "${ACTIONLINT_SHA256_ARM64} /tmp/${tarball}" | sha256sum --check --strict; \ | |
| fi \ | |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
mattwilkinsonn
force-pushed
the
sea-587-buildkite-migration--poseidon
branch
3 times, most recently
from
d096d99 to
7800fe7
Compare
mattwilkinsonn
force-pushed
the
sea-587-buildkite-migration--poseidon
branch
4 times, most recently
from
daefcdc to
bef29b1
Compare
| # sccache — RUSTC_WRAPPER target for the Test pipelines. | ||
| RUN ARCH="$(uname -m)" \ | ||
| && case "${ARCH}" in \ | ||
| x86_64) sccache_target="x86_64-unknown-linux-musl" ;; \ | ||
| aarch64) sccache_target="aarch64-unknown-linux-musl" ;; \ | ||
| *) echo "unsupported arch ${ARCH}"; exit 1 ;; \ | ||
| esac \ | ||
| && curl -fsSL "https://github.com/mozilla/sccache/releases/download/v${SCCACHE_VERSION}/sccache-v${SCCACHE_VERSION}-${sccache_target}.tar.gz" \ | ||
| | tar -xz -C /tmp \ | ||
| && install -m 0755 "/tmp/sccache-v${SCCACHE_VERSION}-${sccache_target}/sccache" /usr/local/bin/sccache \ | ||
| && rm -rf /tmp/sccache-* | ||
|
|
||
| # just — release binary download. Avoids cargo-installing it inside | ||
| # the image (would pull every cargo dep). | ||
| RUN ARCH="$(uname -m)" \ | ||
| && case "${ARCH}" in \ | ||
| x86_64) just_target="x86_64-unknown-linux-musl" ;; \ | ||
| aarch64) just_target="aarch64-unknown-linux-musl" ;; \ | ||
| *) echo "unsupported arch ${ARCH}"; exit 1 ;; \ | ||
| esac \ | ||
| && curl -fsSL "https://github.com/casey/just/releases/download/${JUST_VERSION}/just-${JUST_VERSION}-${just_target}.tar.gz" \ | ||
| | tar -xz -C /tmp just \ | ||
| && install -m 0755 /tmp/just /usr/local/bin/just \ | ||
| && rm /tmp/just | ||
|
|
||
| # actionlint — SHA-pinned release binary. Matches the version the | ||
| # old install-script used. | ||
| RUN ARCH="$(uname -m)" \ | ||
| && case "${ARCH}" in \ | ||
| x86_64) actionlint_arch="amd64" ;; \ | ||
| aarch64) actionlint_arch="arm64" ;; \ | ||
| *) echo "unsupported arch ${ARCH}"; exit 1 ;; \ | ||
| esac \ | ||
| && tarball="actionlint_${ACTIONLINT_VERSION}_linux_${actionlint_arch}.tar.gz" \ | ||
| && curl -fsSL -o "/tmp/${tarball}" "https://github.com/rhysd/actionlint/releases/download/v${ACTIONLINT_VERSION}/${tarball}" \ | ||
| && if [ "${actionlint_arch}" = "amd64" ]; then \ | ||
| echo "${ACTIONLINT_SHA256} /tmp/${tarball}" | sha256sum --check --strict; \ | ||
| fi \ | ||
| && tar -xzf "/tmp/${tarball}" -C /tmp actionlint \ | ||
| && install -m 0755 /tmp/actionlint /usr/local/bin/actionlint \ | ||
| && rm "/tmp/${tarball}" /tmp/actionlint | ||
|
|
||
| # taplo — TOML formatter, used by hk.pkl pre-push hook + occasional | ||
| # CI use. Release assets are named by plain machine arch: | ||
| # `taplo-linux-x86_64.gz` / `taplo-linux-aarch64.gz`. | ||
| RUN ARCH="$(uname -m)" \ | ||
| && curl -fsSL "https://github.com/tamasfe/taplo/releases/download/${TAPLO_VERSION}/taplo-linux-${ARCH}.gz" \ | ||
| | gunzip > /tmp/taplo \ | ||
| && install -m 0755 /tmp/taplo /usr/local/bin/taplo \ | ||
| && rm /tmp/taplo |
There was a problem hiding this comment.
sccache, just, and taplo are all fetched from GitHub release URLs and installed directly — sccache and just via tar pipe, taplo via gunzip pipe — with no SHA256 verification. actionlint for amd64 correctly verifies its hash (lines 108–110), making the pattern inconsistent: a compromised or tampered release artifact for any of these three tools would be silently baked into seal-ci:latest and then run as root inside every downstream pipeline. The fix is the same pattern already used for actionlint — add ARG SHA256 constants per-arch and sha256sum --check after the download.
mattwilkinsonn
force-pushed
the
sea-587-buildkite-migration--poseidon
branch
2 times, most recently
from
0ab0a21 to
5c0a853
Compare
mattwilkinsonn
force-pushed
the
sea-587-buildkite-migration--poseidon
branch
from
5c0a853 to
d2bab06
Compare
|
Docs preview: https://e0efbd7e.seal-docs.pages.dev |
| - sealedsecurity/secret-env#v1.0.0: | ||
| variables: |
There was a problem hiding this comment.
#main branch
The withgraphite/graphite-ci#main reference resolves to whatever commit is at main in that repo at build time, so any change — including a supply-chain compromise — is silently adopted by every orchestrator build. Every other external reference in this PR is pinned: docker#v5.13.0 uses a version tag, dorny/paths-filter in the GHA workflows uses a full commit SHA, and graphite-ci-action in the deleted ci.yml was pinned to @26bc0dcdeaba101794931d67a464ecbb21a76a8b. Pinning withgraphite/graphite-ci to a specific commit SHA keeps the same security posture the rest of the repo maintains.
| - sealedsecurity/secret-env#v1.0.0: | ||
| variables: | ||
| BUILDKITE_PLUGIN_GRAPHITE_CI_GRAPHITE_TOKEN: GRAPHITE_CI_TOKEN | ||
| - withgraphite/graphite-ci#main: {} |
There was a problem hiding this comment.
withgraphite/graphite-ci plugin to a specific commit SHA to match the security posture used everywhere else in this PR (docker#v5.13.0, the deleted ci.yml's @26bc0dcdeaba101794931d67a464ecbb21a76a8b, and dorny/paths-filter@fbd0ab8f3e69293af611ebaee6363fc25e6d187d). Resolve the current HEAD of the plugin and pin it here.
| - withgraphite/graphite-ci#main: {} | |
| - withgraphite/graphite-ci#<COMMIT_SHA>: {} |
| message: "\$BUILDKITE_MESSAGE" | ||
| EOF |
There was a problem hiding this comment.
origin/main, but runs without git fetch
fan-out.sh calls git merge-base HEAD origin/main to find the diff base, but Buildkite hosted agents check out via the GitHub App and may not have origin/main populated in their shallow clone — the origin/main ref is only guaranteed present after an explicit git fetch origin main. If the ref is absent, git merge-base exits non-zero and the fan-out step aborts, causing the orchestrator build to fail with no downstream pipelines triggered. Consider adding git fetch origin main --no-tags --depth=1 before the merge-base call.
mattwilkinsonn
force-pushed
the
sea-587-buildkite-migration--poseidon
branch
2 times, most recently
from
030d2c1 to
067f534
Compare
| test_patterns_unknown_filter_is_empty() { | ||
| ! filter_patterns ci-image 2>/dev/null | grep -Fqx 'no-such-pattern' | ||
| } |
There was a problem hiding this comment.
Test does not match its intent. The test test_patterns_unknown_filter_is_empty checks whether filter_patterns ci-image output contains 'no-such-pattern', but ci-image is a known/existing filter, not an unknown one.
# Current (incorrect):
test_patterns_unknown_filter_is_empty() {
! filter_patterns ci-image 2>/dev/null | grep -Fqx 'no-such-pattern'
}
# Should be:
test_patterns_unknown_filter_is_empty() {
[[ -z "$(filter_patterns no-such-filter 2>/dev/null)" ]]
}The test should call filter_patterns with a non-existent filter name and verify the output is empty. The current implementation will pass even if unknown filters return data, failing to catch the bug it's supposed to detect.
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
mattwilkinsonn
force-pushed
the
sea-587-buildkite-migration--poseidon
branch
2 times, most recently
from
69ef990 to
fbc1a65
Compare
mattwilkinsonn
force-pushed
the
sea-587-buildkite-migration--poseidon
branch
from
fbc1a65 to
404ee7b
Compare
| RUN ARCH="$(uname -m)" \ | ||
| && case "${ARCH}" in \ | ||
| x86_64) binstall_target="x86_64-unknown-linux-musl" ;; \ | ||
| aarch64) binstall_target="aarch64-unknown-linux-musl" ;; \ | ||
| *) echo "unsupported arch ${ARCH}"; exit 1 ;; \ | ||
| esac \ | ||
| && curl -fsSL "https://github.com/cargo-bins/cargo-binstall/releases/download/v${CARGO_BINSTALL_VERSION}/cargo-binstall-${binstall_target}.tgz" \ | ||
| | tar -xz -C /usr/local/bin cargo-binstall \ | ||
| && cargo binstall --no-confirm \ | ||
| "cargo-nextest@${NEXTEST_VERSION}" \ | ||
| "cargo-edit@${CARGO_EDIT_VERSION}" \ | ||
| "cargo-update@${CARGO_UPDATE_VERSION}" \ | ||
| && chmod -R a+w "${CARGO_HOME}" |
There was a problem hiding this comment.
cargo-binstall bootstrap tarball has no SHA256 check
The cargo-binstall release tarball is downloaded and piped directly into tar with no integrity verification — yet it is then trusted to install cargo-nextest, cargo-edit, and cargo-update. A compromised or tampered release artifact would silently backdoor everything that cargo binstall subsequently installs, since those invocations inherit the compromised binary's behavior. The prior review thread at line 117 flags sccache, just, and taplo for the same missing-SHA pattern, but cargo-binstall is the installer for the others and so carries compounded risk. The fix is the same: add an ARG CARGO_BINSTALL_SHA256_AMD64 / ARG CARGO_BINSTALL_SHA256_ARM64 and sha256sum --check --strict before extracting.
…age-build chaining (SEA-587)
mattwilkinsonn
force-pushed
the
sea-587-buildkite-migration--poseidon
branch
from
b751254 to
7351c3d
Compare
mattwilkinsonn
force-pushed
the
sea-587-buildkite-migration--poseidon
branch
from
e7e1f9a to
f0c7978
Compare
mattwilkinsonn
force-pushed
the
sea-587-buildkite-migration--poseidon
branch
from
3ccc0d1 to
bb0cf67
Compare
mattwilkinsonn
force-pushed
the
sea-587-buildkite-migration--poseidon
branch
from
bb0cf67 to
6b1061a
Compare
…olume du (SEA-587)
1 participant
Pull request
Summary
Establishes the Buildkite CI infrastructure by adding the
.buildkite/directory with an orchestrator pipeline, a lints pipeline, a tool-install script, and a comprehensive setup runbook (docs/buildkite-setup.md). Thelintsjob is removed from.github/workflows/ci.ymland migrated to Buildkite, where it runs as theseal-lintsdownstream pipeline triggered by the orchestrator.Changes
.buildkite/README.mddescribing the pipeline layout, path-filtering strategy, and which workflows intentionally remain on GitHub Actions.buildkite/pipelines/orchestrator.ymlas the single Buildkite GitHub App entrypoint: runs the Graphite-CI optimizer, then usesmonorepo-diff-buildkite-pluginto fan outtrigger:steps to per-concern downstream pipelines based on path filters mirroring.github/path-filters/*.yml.buildkite/pipelines/lints.ymlrunningcheck-fmt,actionlint, and the docs site lint/check suite (biome,markdownlint-cli2,astro check) on alinux-amd64hosted agent with S3-backed bun install cache.buildkite/scripts/lints-install-tools.shto idempotently installrustfmt,actionlint(SHA-pinned),just,nvm/Node 24, andbunon the agentlintsjob from.github/workflows/ci.ymlbuildkite-agentandbkCLI installation tojust install-deps— via Homebrew on macOS and via pinned release tarballs into$CARGO_HOME/binon Linuxdocs/buildkite-setup.mdcovering the full one-time setup: Buildkite org and agent queue configuration, GitHub App installation, required-status-check naming and two-step rollout order, path filtering, Graphite integration, per-pipeline secrets inventory, and S3-backed cache strategyNotes for reviewers
trigger:steps forseal-test-linux,seal-test-macos,seal-live-tests,seal-clients-even-terminal, andseal-docsare wired but those downstream pipeline YAML files are not included in this PR — they land separately.live-testslabel to a PR, independent of path changes, via a dedicatedtrigger:step in the orchestrator.buildkite/seal-lints,buildkite/seal-test-linux, etc.) are posted back to GitHub through the orchestrator's triggered builds.