Review summary: buildkite/buildkite-gha PR #99

Change reviewed

PR #99 adds a fourth user-selectable example, nodejs-concurrency, to both .buildkite/examples.yml and .buildkite/upload-examples.sh. The new .github/workflows/example-nodejs-concurrency.yml is manually dispatchable, grants contents: read, applies a repository/ref-based workflow concurrency group with cancel-in-progress: true, checks out the repository, and invokes pinned actions/setup-node v7 for Node 22 with npm caching enabled.

The PR description is empty, and no Linear issue identifier appears in the branch name or PR title, so there were no additional acceptance criteria to inspect. The review used the PR title and implementation as the source of intent. The reviewed head was efb43cede52044de9265d3acb966899a04c94f9d on base main at 39dfeeb573ed360915e7379102dfa0ba690a6c3b.

What I examined

I read all three changed files and traced the example-selection paths through the repository. In particular, I inspected:

• internal/buildkite/pipeline_test.go, including the exact selector contract in TestExamplesPipelineSelectsOneCanonicalWorkflow and the upload-script tests.
• scripts/compare-example, which provides the documented paired GitHub Actions/Buildkite launcher for the existing examples.
• The smoke-manifest inventory and production-policy preflight paths.
• The current compiler handling for action input defaults and GitHub token provisioning, including the fixes merged through PRs #102 and #103 before this branch was rebased.
• The pinned actions/setup-node v7 action.yml, src/cache-restore.ts, and src/cache-utils.ts. Its explicit npm cache path searches the workspace for package-lock.json, npm-shrinkwrap.json, or yarn.lock and throws when none exists. This repository contains none of those lockfiles and no Node package manifest.

Checks and evidence

Checks run locally:

• go test ./... failed in TestExamplesPipelineSelectsOneCanonicalWorkflow: the expected picker options still contain only basic, artifacts, and advanced, while the pipeline now supplies nodejs-concurrency as a fourth option.
• go run ./cmd/buildkite-gha validate --profile hosted-tokenless --format json --event-path testdata/smoke/events/push.json .github/workflows/example-nodejs-concurrency.yml succeeded with an admitted result. It emitted the expected warnings that workflow cancel-in-progress is not enforced by Buildkite and that arbitrary action runtime independence cannot be proven.
• shellcheck -x .buildkite/upload-examples.sh passed.

I did not run the full mise run check aggregate after the complete Go test suite had already reproduced the blocking test failure. GitHub reported the branch's Buildkite build #530 as failed; the configured Buildkite API token could not access that organization/pipeline and returned 404, so I could not inspect its individual job logs. The GitHub Actions differential-oracle jobs and Socket checks visible on the PR passed.

Review findings

Two previously raised blocking concerns remain open:

1. Selector contract failure: the new option was not added to the exact options expected by TestExamplesPipelineSelectsOneCanonicalWorkflow, so the repository's required Go test gate fails.
2. Unusable npm cache configuration: the workflow explicitly enables cache: npm, but the checked-out repository has no supported npm lockfile. The pinned setup-node implementation therefore fails while locating its cache dependency file, before the example can complete.

One prior blocker is now resolved by the rebase rather than by a workflow edit. The current base supports setup-node's conditional token default and supplies the narrow default contents: read token authority; the hosted-tokenless validation now admits this workflow. I acknowledged that in the existing thread and resolved it.

I also left one new non-blocking inline finding: scripts/compare-example still rejects nodejs-concurrency. That script mirrors all three existing example selectors and is the documented way to launch paired GitHub Actions and Buildkite runs, so the new example is not yet available through the same comparison path.

The option was otherwise wired consistently into both Buildkite selection implementations, and both public actions are pinned to immutable commits.

Verdict

I submitted REQUEST_CHANGES because the selector contract still breaks go test ./... and the new workflow still fails at runtime when setup-node looks for an npm lockfile. The prior token-default concern was closed as resolved, and the paired-launcher gap was posted as non-blocking feedback.

Review: #99 (review)

Trigger source: automatic.