ci: shard the dev lane on Linux and move Windows to the shipping boundary - #899
ci: shard the dev lane on Linux and move Windows to the shipping boundary#899lidge-jun wants to merge 5 commits into
Conversation
github-actions
Bot
added
the
chore
|
Warning Review limit reached
Next review available in: 5 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)
📝 WalkthroughWalkthroughCI now runs four Linux test shards, centralized checks, unconditional macOS validation, conditional Windows validation, affected-path GUI and packaging checks, and a final aggregate gate. Workflow tests and development plans define the job structure, filters, timeouts, dependencies, and validation criteria. ChangesCI development lane
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequestOrPush
participant Changes
participant LinuxShards
participant Gates
participant PlatformJobs
participant CI
PullRequestOrPush->>Changes: Detect GUI and packaging changes
PullRequestOrPush->>LinuxShards: Start four Linux test shards
PullRequestOrPush->>Gates: Run shared checks
PullRequestOrPush->>PlatformJobs: Run macOS, or eligible Windows, validation
Changes->>Gates: Control GUI checks
Changes->>PlatformJobs: Control packaging smoke checks
LinuxShards->>CI: Report shard results
Gates->>CI: Report shared-check results
PlatformJobs->>CI: Report platform results
CI->>CI: Accept success or intentional skip only
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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 @.github/workflows/ci.yml:
- Around line 143-144: Add persist-credentials: false to every checkout step in
.github/workflows/ci.yml at line ranges 143-144, 202-203, 233-234, 290-291,
355-356, and 390-391, ensuring none persist the GITHUB_TOKEN in local Git
configuration.
In `@devlog/_plan/260803_ci_dev_lane_sharding/000_plan.md`:
- Line 3: Replace the future-dated audit entries with the actual August 2, 2026
date: update the plan-open date in
devlog/_plan/260803_ci_dev_lane_sharding/000_plan.md lines 3-3, the
evidence-collection date in
devlog/_plan/260803_ci_dev_lane_sharding/001_shard_evidence.md lines 3-5, and
the audit date in
devlog/_plan/260803_ci_dev_lane_sharding/002_audit_synthesis.md lines 5-6.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0095ce35-ad1a-457c-81f8-8e7f4029cb61
📒 Files selected for processing (9)
.github/workflows/ci.ymldevlog/_plan/260803_ci_dev_lane_sharding/000_plan.mddevlog/_plan/260803_ci_dev_lane_sharding/001_shard_evidence.mddevlog/_plan/260803_ci_dev_lane_sharding/002_audit_synthesis.mddevlog/_plan/260803_ci_dev_lane_sharding/010_linux_shard_matrix.mddevlog/_plan/260803_ci_dev_lane_sharding/011_platform_legs.mddevlog/_plan/260803_ci_dev_lane_sharding/030_affected_scoping.mddevlog/_plan/260803_ci_dev_lane_sharding/040_ship_and_review.mdtests/ci-workflows.test.ts
| @@ -0,0 +1,230 @@ | |||
| # dev CI lane: shard on Linux, move Windows off the PR path | |||
|
|
|||
| Opened 2026-08-03 against `dev@f9b9440c551e3d7f3e2041098caa2ee4de57698e` (v2.10.0). | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the future-dated audit records.
The current date is August 2, 2026. These records state that work occurred on August 3, 2026. Future-dated plans, evidence, and audit results undermine the traceability that these documents provide.
devlog/_plan/260803_ci_dev_lane_sharding/000_plan.md#L3-L3: Use the actual plan-open date.devlog/_plan/260803_ci_dev_lane_sharding/001_shard_evidence.md#L3-L5: Use the actual evidence-collection date.devlog/_plan/260803_ci_dev_lane_sharding/002_audit_synthesis.md#L5-L6: Use the actual audit date.
📍 Affects 3 files
devlog/_plan/260803_ci_dev_lane_sharding/000_plan.md#L3-L3(this comment)devlog/_plan/260803_ci_dev_lane_sharding/001_shard_evidence.md#L3-L5devlog/_plan/260803_ci_dev_lane_sharding/002_audit_synthesis.md#L5-L6
🤖 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 `@devlog/_plan/260803_ci_dev_lane_sharding/000_plan.md` at line 3, Replace the
future-dated audit entries with the actual August 2, 2026 date: update the
plan-open date in devlog/_plan/260803_ci_dev_lane_sharding/000_plan.md lines
3-3, the evidence-collection date in
devlog/_plan/260803_ci_dev_lane_sharding/001_shard_evidence.md lines 3-5, and
the audit date in
devlog/_plan/260803_ci_dev_lane_sharding/002_audit_synthesis.md lines 5-6.
…flow The Windows leg decides when a pull request turns green. On the last all-green run the suite took 5m49s on ubuntu, 5m23s on macos, and 16m23s on windows, so a contributor waits roughly three times longer than the verdict actually took to produce. The 30-minute ceiling on that job, and the two increases before it, are the same problem being paid for rather than fixed. This unit plans the change: four Linux shards for the suite, one job for the platform-independent gates, macOS kept whole and unsharded as the control that would notice if the shards stopped being independent, and Windows moved to promotion and dispatch where a maintainer consumes it. One aggregate check named `ci` covers whatever the job graph happens to be, so shard counts and platform legs can move without a required check name moving with them. No workflow is edited here. The docs carry the diff-level specification and the evidence behind it: shard tiling proven on this suite (four shards, 120 files each, union 480, no overlap), the runner and required-check behavior quoted from the official documentation, and the measured baseline the result will be compared against. An independent review ran four rounds against this plan and failed it three times. Every blocker was a local fact asserted without being checked: tests said not to need React that import JSX, a phase boundary that would have deleted two platforms in a commit claiming it did not, a duplicated YAML key, and a changed-files action left on its default base that would have diffed dev against main and saved nothing. The rounds and their dispositions are recorded with the plan, since the corrections are more useful to the next person than a clean-looking document.
One matrix job ran the whole gate on three platforms, and the Windows leg took 16m23s against ubuntu's 5m49s and macos's 5m23s on the last all-green run. It decided when every pull request turned green. The comment above its timeout records the ceiling being raised twice rather than the gap being closed, most recently to 30 minutes. The suite now runs as four Linux shards. `bun test --shard=i/4` sorts files by path and deals them round-robin, so the split is deterministic and the four shards cover the suite exactly once: 120 files each, union 480, no file in two shards. Everything that is not the suite — typecheck, privacy scan, GUI tests, lint, build, release-helper syntax, CLI smoke — moves to a single `gates` job instead of being repeated per platform. macOS keeps running the whole suite, unsharded. That is deliberate: the shards each cover a quarter of the files, which assumes no test depends on a sibling having run in the same pool, and this leg is the control that would notice if that assumption broke. It is also the fastest leg on the board, so there was never a reason to trim it. Windows moves to promotion and dispatch. Its coverage is not dropped — release.yml already refuses to publish a commit without a successful run of this workflow, so main and preview still gate on it, and the Windows npm-global smoke still runs on every push. The trade is that a Windows-only regression can now reach dev and be caught at promotion rather than at review. dev is not published; that is what makes the trade acceptable rather than free. A new `ci` job is the one check that means "CI passed". Shard names move with the shard count and the platform legs come and go by trigger, so neither is a stable thing to require in branch protection. It depends on every other job and asserts each result with an allowlist: success and skipped pass, anything else fails, so a result type added later cannot slip through. `if: always()` is what keeps it from being skipped along with a failed dependency and reporting success by omission. The suite's workflow pins move with it. The exact `timeout-minutes:` occurrence count is replaced by iterating the parsed jobs, which is what its comment always claimed to check; the shard matrix and the `--shard` divisor are pinned to each other, because drift between them would stop running files while CI stayed green; and the gate's `needs` is compared against the workflow's own job list rather than a hardcoded one, so adding a job without gating it fails immediately. Both new pins were driven red before being kept.
The sharded lane still paid for three npm-global smoke jobs and a GUI
lint and build on every run, including pushes that touched neither. This
scopes them to the areas that can actually break them.
The filtering is per job, never on the workflow trigger. A workflow
skipped by path filtering leaves its checks Pending forever, so a pull
request requiring them can never merge, while a job skipped by a
condition reports success. That asymmetry is why the coarse `paths:`
list in the `on:` block stays coarse and a `changes` job does the real
work.
`base: ${{ github.ref }}` is doing more than it looks. Left unset, the
action compares against the repository default branch, which is `main` —
so a push to `dev` would be diffed against `main` and every area touched
since the last promotion would keep reading as changed. The scoped jobs
would run on nearly every push, the saving would not happen, and nothing
would look wrong.
The packaging filter covers `src/**` deliberately. package.json ships
`src` and bin/ocx.mjs executes it, so a source change is a packaging
change; without that entry an ordinary source pull request would get no
Windows verification at all now that the Windows suite runs only at the
shipping boundary. That keeps the 1m53s Windows smoke on the common path
while the 16-minute suite stays off it.
Three tarball inputs — assets, README, LICENSE — were named by the
filter but absent from the trigger's own path list, which meant a change
to one of them never started this workflow and the filter never got to
see it. The trigger list grows from 14 entries to 17 to match, on both
push and pull_request, since checking one and not the other is how a
change lands on dev having been verified on the wrong trigger.
The pins move with the behavior. The GUI lint and build assertions
became substring checks that a permanently-false condition would still
satisfy, so they now assert the condition itself: a gate that cannot
fire is dropped in every sense that matters, which is the outcome PR #97
already hit once by a different route. The packaging filter is compared
as a whole list rather than sampled, and every pattern in it is checked
against the trigger's path list so an unreachable entry fails the suite.
Each new pin was driven red before being kept.
![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: d535636a3c
ℹ️ 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".
| if: >- | ||
| github.event_name == 'workflow_dispatch' || | ||
| (github.event_name == 'push' && | ||
| (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/preview')) |
There was a problem hiding this comment.
When a commit already has a successful PR or dev-push CI run, a release can be dispatched immediately after that SHA is promoted to main or preview; .github/workflows/release.yml lines 140-147 accepts any successful ci.yml run for the SHA without constraining its event or branch. Because this condition skips platform-windows in those earlier runs, publishing can proceed while the new promotion run—and its Windows coverage—is still pending or even fails. Make the release gate select the successful main/preview promotion run or explicitly verify that its Windows job succeeded.
AGENTS.md reference: .github/AGENTS.md:L17-L18
Useful? React with 👍 / 👎.
| packaging: | ||
| - 'package.json' | ||
| - 'bun.lock' | ||
| - 'src/**' | ||
| - 'bin/**' |
There was a problem hiding this comment.
A PR that changes only .gitattributes still triggers this workflow, but this filter leaves packaging false and skips all three npm-global-smoke jobs. Because .gitattributes controls how tracked package inputs such as bin/ocx.mjs are materialized on each runner, an attribute change can introduce CRLF shebangs or other checkout transformations into the tarball without exercising npm pack, global installation, or ocx help on any platform. Add .gitattributes to the packaging filter and its asserted allowlist.
AGENTS.md reference: .github/AGENTS.md:L18-L18
Useful? React with 👍 / 👎.
The first sharded run went red on shard 1/4 and macos, and the failure was mine rather than flaky: tests that fetch the served dashboard read their session bootstrap out of gui/dist/index.html, and nothing in those jobs built it. The old three-platform job satisfied this by accident. It ran the suite and the GUI build in the same job, so gui/dist always existed by the time the tests ran. Splitting the suite away from the gates removed the coincidence without removing the dependency, and area-scoping the gates' build behind `gui/**` meant even that copy was conditional. Reproduced on an untouched checkout of dev to be sure the split was the cause: the same test fails there with no gui/dist present and passes after `bun run build`. So the dependency is real and pre-existing; only its accidental satisfaction disappeared. The suite pin that would have caught this now exists: every job running the root suite must have an unconditional GUI build step. It was driven red before being kept.
d535636 to
32c56a8
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 @.github/workflows/ci.yml:
- Around line 367-370: Update the “Clean workspace (self-hosted only)” step so
git clean failures are not suppressed for existing worktrees. Preserve tolerance
only when no repository exists initially, but propagate any cleanup failure
after a repository is present so the workflow stops instead of continuing with
stale content.
In `@devlog/_plan/260803_ci_dev_lane_sharding/010_linux_shard_matrix.md`:
- Around line 98-105: Update all documented root-suite jobs to build the GUI in
their own workspace before tests: in
devlog/_plan/260803_ci_dev_lane_sharding/010_linux_shard_matrix.md lines 98-105,
add an unconditional Build GUI step after dependency installation and before
shard tests; in devlog/_plan/260803_ci_dev_lane_sharding/011_platform_legs.md
lines 105-110, remove the claim that gates supplies the build and add the
unconditional step before macOS tests; and at lines 152-159, add the same step
before Windows tests.
In `@devlog/_plan/260803_ci_dev_lane_sharding/040_ship_and_review.md`:
- Around line 11-20: Update the shell command block in “Run in this order” to
enable fail-fast behavior by adding “set -euo pipefail” before the listed
commands, preserving the existing sequence and prose.
- Around line 45-53: Update the two public-artifact scans in the ship-and-review
instructions so the diff scan includes devlog alongside .github and tests, and
both scans use the same declared sensitive patterns: /Users/, worktree, macmini,
and session. Keep the existing clean-result behavior unchanged.
- Around line 29-37: Replace the shard-validation loop in the documented plan
with a method that does not rely on unmatched test-name output from Bun. Use a
supported file-list/dry-run interface or reproduce Bun’s documented sharding
algorithm from the sorted test-file list, then validate both that no test file
appears in multiple shards and that the union of shard assignments equals all
files found by find tests -name '*.test.ts'.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 05c42f3a-7d70-4a08-ab6e-a845dbac0343
📒 Files selected for processing (9)
.github/workflows/ci.ymldevlog/_plan/260803_ci_dev_lane_sharding/000_plan.mddevlog/_plan/260803_ci_dev_lane_sharding/001_shard_evidence.mddevlog/_plan/260803_ci_dev_lane_sharding/002_audit_synthesis.mddevlog/_plan/260803_ci_dev_lane_sharding/010_linux_shard_matrix.mddevlog/_plan/260803_ci_dev_lane_sharding/011_platform_legs.mddevlog/_plan/260803_ci_dev_lane_sharding/030_affected_scoping.mddevlog/_plan/260803_ci_dev_lane_sharding/040_ship_and_review.mdtests/ci-workflows.test.ts
| ```bash | ||
| for s in 1 2 3 4; do | ||
| bun test --shard=$s/4 --test-name-pattern '____never_match____' 2>&1 \ | ||
| | grep -oE '^tests/[^:]+\.test\.ts' | sort -u | ||
| done | sort | uniq -d | ||
| ``` | ||
|
|
||
| Empty output means no file is in two shards. Compare the union count against | ||
| `find tests -name '*.test.ts' | wc -l` for the other direction. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
set -eu
printf '%s\n' '--- target file ---'
cat -n devlog/_plan/260803_ci_dev_lane_sharding/040_ship_and_review.md | sed -n '20,45p'
printf '%s\n' '--- Bun/version references ---'
rg -n -i 'bun|shard|test-name-pattern|never_match' \
--glob '!node_modules/**' \
--glob '!dist/**' \
--glob '!build/**' \
. | head -250
printf '%s\n' '--- version files ---'
git ls-files | rg '(^|/)(package\.json|bun\.lockb?|\.bun-version|mise\.toml|package-lock\.json|yarn\.lock|pnpm-lock\.yaml|Dockerfile|.*\.yml|.*\.yaml)$' | head -100Repository: lidge-jun/opencodex
Length of output: 28242
🌐 Web query:
Bun 1.3.14 bun test --shard --test-name-pattern no matching tests prints test file paths output behavior
💡 Result:
In Bun 1.3.14, when using bun test with both --shard and --test-name-pattern, the execution behavior is designed to filter the test suite while respecting sharding logic [1][2]. Specifically, the sharding mechanism in Bun (introduced to split test files across CI runners) operates by partitioning the list of discovered test files [1][2]. The filtering process works as follows: 1. Sharding is applied to the pool of test files first. Discovered test files are sorted by path, and then distributed using a round-robin assignment based on the provided shard index (e.g., --shard=M/N) [2]. 2. The --test-name-pattern (or -t) flag filters individual tests within those selected files by matching test names (prefixed by describe blocks) against a regex pattern [3][4][5]. Regarding "no matching tests": If the --test-name-pattern filter results in zero tests being matched within the files assigned to a specific shard, Bun typically reports the filtered status [5]. However, regarding the shard itself, if a shard is valid but ends up with zero files assigned to it (e.g., running --shard=5/5 when only 2 test files exist), Bun exits with a status code of 0 and prints a summary line instead of treating it as an error or "No tests found!" [2]. If you pass a --test-name-pattern that matches nothing in your project, the test runner will report that tests were filtered out [5]. Because sharding happens at the file discovery level and test name pattern matching happens at the test execution level, you will see output indicating the number of tests run and the number of tests filtered out [5]. It does not print raw test file paths as a list of "no matching tests"; rather, it provides a summary of the test execution results [2][5].
Citations:
- 1: Add
bun test --shard=M/Nfor splitting tests across CI jobs oven-sh/bun#29366 - 2: oven-sh/bun@d7a66f5
- 3: https://bun.com/docs/test/discovery
- 4: https://bun.sh/docs/test/discovery
- 5: https://bun.com/docs/guides/test/run-tests
🏁 Script executed:
set -eu
printf '%s\n' '--- package.json test configuration ---'
cat -n package.json | sed -n '1,75p'
printf '%s\n' '--- bunfig.toml ---'
cat -n bunfig.toml
printf '%s\n' '--- test wrapper outline and relevant source ---'
ast-grep outline scripts/test.ts --match '$_' --view compact 2>/dev/null || true
wc -l scripts/test.ts
cat -n scripts/test.ts | sed -n '1,220p'
printf '%s\n' '--- test file paths ---'
find tests -name '*.test.ts' -print | sort | sed -n '1,120p'Repository: lidge-jun/opencodex
Length of output: 14571
🏁 Script executed:
set -eu
printf '%s\n' '--- Bun documentation references ---'
for url in \
'https://bun.com/docs/test/filtering' \
'https://bun.com/docs/test/discovery' \
'https://bun.com/docs/guides/test/run-tests' \
'https://github.com/oven-sh/bun/commit/d7a66f504135418c40c8cbec3931f4f3520b7345d.patch'
do
printf '\nURL: %s\n' "$url"
curl -LfsS --max-time 20 "$url" | rg -in -C 3 'test-name-pattern|filtered|shard|file|no tests|0 pass|summary' | head -100 || true
done
printf '%s\n' '--- local Bun availability only ---'
if command -v bun >/dev/null 2>&1; then bun --version; else echo 'bun not installed'; fi
printf '%s\n' '--- pipeline status probe ---'
set +e
printf '%s\n' 'Bun summary: 0 pass, 0 fail, 0 files' \
| grep -oE '^tests/[^:]+\.test\.ts' \
| sort -u
printf 'without_pipefail_pipeline_status=%s\n' "${PIPESTATUS[*]}"
set -o pipefail
printf '%s\n' 'Bun summary: 0 pass, 0 fail, 0 files' \
| grep -oE '^tests/[^:]+\.test\.ts' \
| sort -u
printf 'with_pipefail_pipeline_status=%s\n' "${PIPESTATUS[*]}"Repository: lidge-jun/opencodex
Length of output: 50376
🏁 Script executed:
set -eu
log=/tmp/coderabbit-shell-logs/shell-output-2zZTZD
printf '%s\n' '--- concise prior probe output ---'
rg -n -C 2 'local Bun availability|bun not installed|^[0-9]+\.[0-9]+\.[0-9]+$|without_pipefail|with_pipefail|Ran .* files|filtered out' "$log" | tail -80 || true
printf '%s\n' '--- direct pipeline-status probe ---'
set +e
printf '%s\n' 'Bun summary: 0 pass, 0 fail, 0 files' | grep -oE '^tests/[^:]+\.test\.ts' | sort -u >/dev/null
printf 'without_pipefail_pipeline_status=%s\n' "${PIPESTATUS[*]}"
set -o pipefail
printf '%s\n' 'Bun summary: 0 pass, 0 fail, 0 files' | grep -oE '^tests/[^:]+\.test\.ts' | sort -u >/dev/null
printf 'with_pipefail_pipeline_status=%s\n' "${PIPESTATUS[*]}"
printf '%s\n' '--- relevant commit patch lines ---'
curl -LfsS --max-time 20 \
'https://github.com/oven-sh/bun/commit/d7a66f504135418c40c8cbec3931f4f3520b7345d.patch' \
| rg -n -C 3 'shard|test-name-pattern|filtered|Ran .* files|file path|test file' \
| head -160 || trueRepository: lidge-jun/opencodex
Length of output: 50376
Replace the shard-validation method.
At lines 29-37, the command assumes that Bun 1.3.14 emits selected test-file paths for an unmatched --test-name-pattern. Bun emits a summary instead, so grep produces no files for every shard and empty output does not prove non-overlap.
Do not add set -o pipefail to this pipeline without changing the extraction method. grep returns status 1 when no path matches, so pipefail would fail every shard. Use a supported file-list or dry-run interface, or reproduce Bun’s documented sharding algorithm from the sorted test-file list. Enforce both duplicate detection and union coverage.
🤖 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 `@devlog/_plan/260803_ci_dev_lane_sharding/040_ship_and_review.md` around lines
29 - 37, Replace the shard-validation loop in the documented plan with a method
that does not rely on unmatched test-name output from Bun. Use a supported
file-list/dry-run interface or reproduce Bun’s documented sharding algorithm
from the sorted test-file list, then validate both that no test file appears in
multiple shards and that the union of shard assignments equals all files found
by find tests -name '*.test.ts'.
Review found a hole this change opened outside the file it edited. release.yml accepts any successful ci.yml run for the release SHA. After a promotion, the pull-request run for that same commit is still there and still green — and now deliberately Windows-free, since the Windows leg is scoped to promotion and dispatch. So a publish could be satisfied by a run that proved nothing about Windows while the promotion run carrying it was still in flight, or had failed. The gate now selects a `push` run on the release branch specifically. A pull-request run for the same SHA no longer qualifies, and the error says why so the next person does not read it as flakiness. Also from review: - `.gitattributes` joins the packaging filter. It decides how tracked package inputs are materialized on each runner, so an attribute change can put CRLF shebangs into the tarball without any source file moving. - The self-hosted workspace wipe stops swallowing its own failure. A clean that fails on permissions left deleted files on disk, and the checkout after it then validated a tree that no longer exists in git. Only the no-repository case — first run on a fresh box — is tolerated. - Every checkout sets `persist-credentials: false`, matching what the other workflows here already do. No job in this one pushes, and the self-hosted box keeps its checkout between jobs. The plan documents carried the same GUI-build omission the first live run found, so they are corrected too rather than left describing a workflow that fails. One review point is rebutted rather than applied: the devlog dates are not future-dated. They read 2026-08-03 because that is the date in the authoring timezone; the same moment is 2026-08-02 in UTC, which is what the CI timestamps show. The reasoning is recorded with the audit trail.
|
Thanks both — nine comments, seven fixed in Codex P1 (release gate) was the best find in this review, planning rounds included. It is a hole this PR opened outside the file it edits:
Each fix is pinned in Rebutted: the "future-dated" devlog records. The dates read 2026-08-03 because that is the date in the authoring timezone. The same moment is 2026-08-02 in UTC, which is what the CI timestamps you compared against show: The unit slug matches. Rewriting them to a UTC reading would make the record less accurate. Worth noting separately: the first sharded run went red on shard 1/4 and macOS, and it was a real defect in this PR rather than flake. Tests that fetch the served dashboard read their session bootstrap out of That one is the reason the audit trail in Current run: 4m26s wall clock against the 16m39s baseline, all shards green, Windows correctly skipped, |
1 participant
What this changes
The dev/PR lane becomes a Linux lane, split four ways. Windows moves to the
shipping boundary. macOS keeps running the whole suite.
bun test --shard=i/4jobs onubuntu-latest.build, release-helper syntax, CLI smoke — moves into one
gatesjob insteadof being repeated per platform.
platform-macosruns the whole suite, unsharded, on every pull request.platform-windowsruns on promotion tomain/previewand onworkflow_dispatch, not on pull requests.cijob aggregates every other job into one stable check name.npm-global-smokejobs run only when theirown inputs changed.
Why
Measured on run 30748690567,
a fully green run of #880:
The Windows leg decided when every pull request turned green. The comment
above its
timeout-minutesrecords that ceiling being raised twice —after #711 passed at 11.8min while #653 was killed at 12.0min (#717), then
again after #827 hit the 20-minute kill — rather than the gap being closed.
This is not a cost argument. Standard runners are free on public
repositories, so no minutes are saved. What changes is feedback latency and
the concurrency budget (20 concurrent jobs on the Free plan, 5 for macOS)
against roughly twenty open pull requests.
The cost, stated plainly
A Windows-only regression can now land on
devand be caught at promotioninstead of at review time. That is a real reduction in when Windows is
verified, and it is the price of the change.
What keeps it acceptable:
devis not published.mainandprevieware, andrelease.ymlalreadyrefuses to publish a commit without a successful run of this workflow — so
the Windows leg still gates everything that ships.
npm-global-smokeleg (1m 53s) still runs on any change to apackaged input, which includes
src/**. An ordinary source PR keeps aWindows signal.
If Windows-only regressions turn out to be frequent on
dev, the rightresponse is to put Windows back on the PR lane sharded, not to defend this
arrangement.
Concurrency: this trades more jobs for a shorter path
"Faster CI" usually implies "less CI"; here it does not, and the peak goes up.
Required checks
devhas no branch protection configured today, so nothing needs re-pointing.If it is enabled later,
ciis the check to require — shard names changewith the shard count and platform legs come and go by trigger, so neither is
stable.
cialso means removing the workflow-levelpaths:filter, ormoving the gate to an always-triggered workflow. A docs-only PR does not
trigger this workflow, so no
cicheck would be created and the PR would sitpending forever. Harmless now, a trap afterwards.
Two details worth a reviewer's attention
base: ${{ github.ref }}on the filter step. Left unset,paths-filtercompares against the repository default branch (
main), so a push todevwould be diffed against
mainand every area touched since the last promotionwould keep reading as changed. The scoped jobs would run anyway, the saving
would not happen, and nothing would look wrong.
The gate treats
skippedas a pass. That is required, not sloppy: atrigger-scoped or area-filtered job reports
skipped, and GitHub reportsskipped jobs as success regardless. The gate uses an allowlist (
successorskippedpass, everything else fails) so a result type added later cannot slipthrough, and
if: always()keeps the gate from being skipped along with afailed dependency and reporting success by omission.
Security review requested
Per
MAINTAINERS.md, GitHub Actions changes require explicit security review.Specific points:
dorny/paths-filteris a new third-party dependency, pinned to the fullcommit SHA
de90cc6f(verified to equal tag v3.0.2). A no-dependencyfallback using
git diff --name-onlyis documented in the devlog if thedependency is unwanted.
changesjob takespull-requests: read, a permission this workflow didnot previously hold. It is scoped to that one job; the action cannot read a
PR's file list without it.
select-windows-runnerand its security commentary are unchanged. Theself-hosted routing, and the note that it is a cost control rather than a
security boundary, carry forward verbatim.
contents: write. No secret is newly exposed.Verification
actionlint: clean.bun x tsc --noEmit: exit 0.bun run test: 6960 pass, 8 skip, 0 fail, 33933 assertions across 480 files.bun run privacy:scan: passed.in two shards, none missed.
tests/ci-workflows.test.ts: 66 pass, 733 assertions (692 before).Every new pin was driven red before being kept — shard/divisor drift, a job
missing from the gate's
needs, a deletedbase:, a droppedpull-requests: read, and a path present in one trigger list but not the othereach fail the suite.
Planning record
devlog/_plan/260803_ci_dev_lane_sharding/carries the diff-level plan and theevidence. It includes the audit trail: an independent review ran four rounds
and failed the plan three times. Every blocker was a local fact asserted
without being checked — tests said not to need React that import JSX, a phase
boundary that would have deleted two platforms in a commit claiming otherwise,
a duplicated YAML key, and the
base:default above. The corrections arerecorded rather than tidied away.
Summary by CodeRabbit