Shard the CI test suite - #286
Conversation
scripts/test/run-all-sharding.check.sh proves the shard partition independently of the runner. Run against the unchanged run-all.sh, it correctly refuses: "error: run-all.sh does not implement --shard/--list yet", exit 2, no suite header. Part of ci-test-shards (#269).
Adds --shard <index>/<count> and --list to the test runner, plus YSTACK_TEST_SHARD as a lower-precedence environment fallback (the flag never reads or validates it when given). Discovery, the GIT_* defaults, and the argument-less run stay byte-identical. Malformed selectors print the single usage line to stderr and exit 2 before any suite runs. Verified: run-all-sharding.check.sh passes 332 assertions; --list matches raw discovery (62 suites); the three discovery-string pins still pass; shellcheck -x -S style clean at 0.11.0; schema guard and rename gate clean. Part of ci-test-shards (#269).
\s is a GNU sed -E extension; BSD/macOS sed -E does not support it, so the matrix-list extraction silently no-op'd and the assertion compared against the wrong string. Use [[:space:]], which both sed implementations support, verified by applying the operator's patch to a scratch copy and confirming assertion 8 passes there (and correctly fails when the matrix or run line is deliberately mismatched). Part of ci-test-shards (#269).
Appends scripts/test/run-all-sharding.check.sh as the manifest's last line, so the checks job's structure check also enforces that the file exists and is executable. Part of ci-test-shards (#269).
Names the three jobs the restored CI becomes: checks, six parallel test shards, and the aggregate ci job that stays the hard merge gate. RESTORE.md is not a constitution path, so this is an agent commit; the matching AGENTS.md sentence is proposed text only (proposals/). Part of ci-test-shards (#269).
One unified diff, applyable with git apply, holding both
operator-owned edits: the .github/workflows/ci.yml split into checks
(existing gate steps plus a new Sharding proof step), six parallel
test shards (matrix.shard 1..6, fail-fast: false, each running
scripts/test/run-all.sh --shard ${{ matrix.shard }}/6), and an
aggregate ci job (needs: [checks, test], if: always(), explicit
needs.*.result == 'success' checks); and the AGENTS.md CI bullet
sentence naming the three jobs and the shard flag.
This is proposed text, not an applied change (proposals/README.md).
Verified: git apply --check passes against this branch; applying it
to a scratch copy reproduces the operator's intended files exactly
and the sharding proof's assertion 8 goes from "workflow is still
serial" to a passing shard-count-equals-matrix check.
Part of ci-test-shards (#269).
Removes a per-shard "path is in the raw list" check that duplicated what the exact-membership cmp already proves (not asked for by the plan), and folds the repeated timeout+capture boilerplate into two small helpers (run, bounded_list). Same 196 assertions still run and pass; net diff for the initiative is now 363 lines, inside the 300-400 review_size:standard budget. Part of ci-test-shards (#269).
Codex reviewer (cross-vendor, read-only)Reviewed-head: c38d0e0 Posted verbatim by The patch does not actually activate the sharded CI workflow it documents, so the primary behavior remains unchanged. There is also a selector validation edge case that violates the stated refusal contract. Full review comments:
|
Fix claim
yshifu diagnosis of the round-0 review (one P2, one P3):
|
Cap the shard selector's numeric shape to at most two digits per side (^[1-9][0-9]?/[1-9][0-9]?$) before any [ -gt ]/[ -le ] comparison. Count is bounded at 16 by the spec, so two digits always suffice, and a value this short can never overflow bash's `[` arithmetic — unlike the prior unbounded [0-9]*, which let a value like --shard 1/999999999999999999999999 reach the range check and fail there with "integer expression expected" instead of refusing cleanly with the usage line and exit 2. Applies to both the --shard flag and YSTACK_TEST_SHARD, since both funnel through the same shard_value check. Add the oversized selector (both operand positions) and a leading-zero selector to run-all-sharding.check.sh's refusal list, exercised as both the flag and the environment variable per the existing loop.
Deploying ystack with
|
| Latest commit: |
0a4e5ee
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://753868a7.fabrica-6yx.pages.dev |
| Branch Preview URL: | https://ystack-impl-ci-test-shards.fabrica-6yx.pages.dev |
Reply to round-0 [P2] "Activate the sharded workflow instead of only proposing it"No code change for this finding — it's expected at this head by design, not an oversight. This initiative is
And Step 5 itself:
The spec's Order-of-work step 5 (
So at this head, The final review at |
Fix-round summary (round-0 → round-1)Implemented:
Pushed back on (no code change):
New head: Proof at the new head (full outputs in the updated PR body):
Note: because the fix touches Labels: |
Codex reviewer (cross-vendor, read-only)Reviewed-head: 0a4e5ee Posted verbatim by The branch does not apply the workflow change required to actually shard CI, so the central feature is absent and the documentation is inaccurate. Review comment:
|
Step 5 evidence recorded; Step 6 is yours@yihanzhu — operator step (plan Step 5 record (the no-argument run under the unchanged workflow):
What you do (the two constitution files; nothing else changes): cd ~/git/ystack && git fetch origin && git checkout ystack/impl/ci-test-shards && git pull --ff-only origin ystack/impl/ci-test-shards
git apply --check proposals/ci-test-shards-shard-ci.patch && git apply proposals/ci-test-shards-shard-ci.patch
git diff --stat # expect exactly .github/workflows/ci.yml and AGENTS.md
git add .github/workflows/ci.yml AGENTS.md
git commit -m "ci: shard the test suite into six jobs behind one aggregate ci gate (operator commit, plan Step 6)"
git push origin ystack/impl/ci-test-shardsThen reply here with the commit SHA (or just "done"). I take it from there: the red-shard proof commits (add, record, delete), the identity records (HR/HD/HF), the final review at HF, and the merge. Do not merge this PR yourself yet — the proof commits come after your commit. |
Closes #269
Claim / tuple
claim-ci-test-shards-20260911T035451Z-0c87a3c49827a9b8ed7eab7c3247699d31bf2a586914b45b(frontmatterspec-blob: 1d419e187315879e2bc52366b2be11a43b511ad4)1d419e187315879e2bc52366b2be11a43b511ad4(frontmatterintent-blob: e56b92429ca56a638827ebf7d0e270ffa2bd1089,risk: high)e56b92429ca56a638827ebf7d0e270ffa2bd1089e62b90ba07c905e2d31a8a0f47eb5c63431b9d19review_size: standard— measured net line count (git diff --shortstat e62b90b..HEAD, current head0a4e5ee): 376 insertions(+), 7 deletions(-) = 369 net lines, inside the 300-400 budget the plan claims (no exception).What changed
Adds
--shard <index>/<count>and--listtoscripts/test/run-all.sh, a newscripts/test/run-all-sharding.check.shthat proves the partition independentlyof the runner, a manifest line, a
RESTORE.mddocs update, andproposals/ci-test-shards-shard-ci.patch— the proposed (not applied) unifieddiff for the two operator-owned constitution paths (
.github/workflows/ci.yml,AGENTS.md). Per the spec, this initiative isrisk: highand agents neverwrite those two files directly; see "Operator step (Step 6)" below.
Round 1 — fix for round-0 review
The round-0 review (gpt-5.5) raised one P2 and one P3 finding.
change — replied on the PR
(Shard the CI test suite #286 (comment)) with the
rationale: this is the accepted plan's Step 5/6 sequence by design (agent-authored
files open the PR first so the still-serial workflow produces the required
no-argument
H0run; the operator applies the sharded workflow patch as the lastpermanent change before the final review at
HF).0a4e5ee: the selector regex was^[1-9][0-9]*/[1-9][0-9]*$, unbounded in digitcount, so a value like
--shard 1/999999999999999999999999passed the shape checkand then hit
[ "$count" -gt 16 ], which errors with "integer expression expected"on a number too large for
[to parse instead of refusing cleanly. Fixed by cappingthe shape to at most two digits per side (
^[1-9][0-9]?/[1-9][0-9]?$) before anyarithmetic — count is bounded at 16 by the spec, so two digits always suffice, and a
value that short can never overflow the subsequent range checks. Applies identically
to the
--shardflag andYSTACK_TEST_SHARD, since both funnel through the sameshard_valuecheck. Added the oversized selector (both operand positions) and aleading-zero selector (
01/6) torun-all-sharding.check.sh's refusal list,exercised as both the flag and the environment variable.
Because the fix touches
scripts/test/run-all.shandscripts/test/run-all-sharding.check.sh— both underscripts/test— theH0tree-identity binding from Step 5 no longer holds against the original
H0(
c38d0e0). Per plan Step 6's second precondition,H0moves to this round's newhead; see the updated Proof section and Identity records below. The workflow is still
unchanged (serial) at this head, so the new
H0's no-argument CI run is stillproducible — the manager will fill it in once this round's CI run completes.
Step 0 — proof script, pre-fix and post-fix
Pre-fix, against the unchanged runner (before Step 1's edit), commit
0c1828a:Exit
2, under a second, no==>header (stdout was empty) — the refusal thatproves the script tests something.
Post-fix, at the current head
0a4e5ee:Exit
0.GITHUB_ACTIONSis unset locally, so assertion 8 takes the "still serial"branch and passes — this is the local serial line referenced in Proof item 13 below,
to be paired there with CI's post-Step-6 equality line. (196 → 202 assertions: round 1
added the oversized-selector and leading-zero refusal cases, each run as both the flag
and
YSTACK_TEST_SHARD.)Proof (all run at commit
0a4e5ee, repo root, onystack/impl/ci-test-shards)0/4 5/4 9/6 a/b 1/0 1/17 1 /4 4/ '' 01/6 1/999999999999999999999999 999999999999999999999999/6(and a bare
--shard) refuse with exit2, empty stdout, and the exact usage lineon stderr — as the flag and as
YSTACK_TEST_SHARD. Confirmed both directions:assertions).
bash scripts/test/portable-core-schema.test.sh→failures: 0, exit0.bash scripts/check-rename.sh→ clean, exit0.ci.yml/AGENTS.mdcontent exactly, and re-running the sharding proofagainst that scratch copy moves assertion 8 from "still serial" to a
passing shard-count-equals-matrix check — and correctly fails it when the
matrix or run line is deliberately mismatched.)
git diff --stat e62b90blists exactly five paths:scripts/test/run-all.sh,scripts/test/run-all-sharding.check.sh,ci/required-files.txt,RESTORE.md,proposals/ci-test-shards-shard-ci.patch.H0moved to thisround's head because the fix touches
scripts/test. Filled in by the manageronce this round's PR CI run (under the still-unchanged workflow) completes:
H0):0a4e5eecdf0a12fe61db0021eea17fdffcfef114cijob status:success(62==>headers,all 62 test scripts passed)git rev-parse H0:scripts/test=fb6410afbda9e7b41c863db7ae1943142cd569eegit rev-parse HF:scripts/test=<TODO, recorded once HF is known>Filled in after Step 6 (operator's commit) lands and a sharded CI run
completes: check list (
ci,checks,test (1)..test (6)),cistatus, wall-clock duration, and the CI equality line from assertion 8 —
to be pasted beside item 1's local "still serial" line above.
cired, then removed again (R11). Filled inafter Step 6, per the plan's red-shard procedure —
HR/HDcommits, runURL, job list, and the identity records (
git diff --name-status HR HD,HD:scripts/testvsHF:scripts/test,HR:.github/workflows/ci.ymlvsHF:.github/workflows/ci.yml).Operator step (Step 6)
This PR's agent-authored commits are complete and proved above. Per the
spec (
risk: high), the two constitution paths —.github/workflows/ci.ymland
AGENTS.md— are the operator's to apply and commit, as the lastpermanent file change on this branch, only after proof item 12 above is
filled in (the no-argument run must happen while the workflow here is still
serial):
After that lands and a sharded run goes green (target: under 25 minutes,
recorded in item 13), the red-shard proof of R11 follows: add
scripts/test/zz-red.test.sh(#!/usr/bin/env bash/exit 1), record thered run (
test (3)red, the other five green,checksgreen,cired — notskipped), then push a second commit deleting it and wait for green again. Full
procedure and exact identity records in
work/ci-test-shards/plan.mdStep 6.Identity records (placeholders — manager fills in as heads accumulate)
H0(this PR's first CI run under the current head, old serial workflow):0a4e5eecdf0a12fe61db0021eea17fdffcfef114HF(final head):<TODO>HR(red-shard add commit):<TODO>HD(red-shard delete commit):<TODO>