ci: balance test shards and stabilize CI checks - #651
Conversation
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. |
8db6213 to
27c9128
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
6fa83bc to
7be8379
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
91625fe to
7952cf6
Compare
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
CI benchmarkCandidate: Prospective declaration: Verified: 10/10 full attempts passed, in 213, 205, 190, 192, 191, 220, 165, 199, 199, 213 seconds. All 33 jobs passed in each. Fixed-cohort and all-current-head nearest-rank p90 are both 213 seconds, versus 428 seconds originally: 2.009× faster (50.23% less elapsed time). The 220-second sample remains included. The collector stopped after attempt 10; no eleventh attempt was requested. A fresh workflow enumeration confirms these are all completed full attempts on this head. Elapsed time is workflow start through the last job completion, including queueing, setup, dependencies, tests, packaging, report uploads, and aggregates. The frozen original ten runs had 5 passes / 5 failures; the final candidate had 10 passes / 0 failures. This is the observed ten-run result, not a guarantee of the long-run failure rate.
The 220-second attempt's slowest Windows job spent 143 seconds in tests versus 68 seconds in the first attempt. Both workers slowed across several unrelated files; caches hit and runtime/image versions were unchanged. No isolated source bottleneck or underlying host cause was established, and no code change was made. The full duration is retained. All 15 artifacts and 32 XML reports from both the first and tenth attempts were saved and their ZIP digests and extracted bytes checked. Raw identities, outcomes, and assertions match the passing reference and each other exactly, with no normalization, missing cases, or duplicate identities: Ubuntu Node 22 1,932 cases / 23,486 assertions; each Windows version 1,928 cases / 23,288 assertions. These are the lanes with uploaded JUnit reports; other Unix lanes are verified through their actual test steps. The corrected tool-name case passed all 15 assertions on Ubuntu and both Windows versions in both checkpoints. Both prior Windows failure deltas are restored; nothing else changed. Every report is bound to the actual tested merge, with the stated base/head parents and matching tree. Both machine-policy checks actually passed in all ten attempts. The fresh GitHub review also found no major issues. All 71 earlier completed trials remain recorded: 51 passed, 20 failed. Including the accepted candidate, the ledger has 81 completed trials: 61 passed, 20 failed. The first 70 remain in the PR description; the latest prior failed attempt is below.
That attempt includes the full wait for the old-head description run to cancel. All 33 canceled-run job records are retained; it is excluded as canceled, not selected by timing. None of the resulting queue delay is subtracted from the 496-second sample. Both Windows versions failed the same advertised Deep-tool-name test: a timed synchronous subprocess returned null status in about nine milliseconds. The logs omit its error and signal. The pinned Bun Windows cached-timer mechanism fits the observed sequence, but remains an inference rather than a native reproduction. No second attempt was requested on that revision. All 15 artifacts and 32 XML reports were preserved. Every test identity remains present. Only the named case changed from passed/15 assertions to failed/5 assertions on each Windows version; all other outcomes and assertions match the passing reference exactly. The report run/merge properties, parents, and source tree were independently checked. The only new change replaces that fixture's synchronous subprocess call with the existing asynchronous process helper. It still executes the real selected Node and bundled server, sends the same JSON-RPC stdin, checks both worker and reducer layouts, parses the same output, and retains all 15 target assertions and cleanup. The child deadline remains 30 seconds. There is no production change, new helper, retry, test skip, or deadline increase. Local validation passed: four focused tests / 37 assertions; the exact eight-file worker passed 32 tests / 1,360 assertions with both recorded seeds, in 4.539 and 4.586 seconds. Raw identities, outcomes, and assertions exactly match both the unchanged Linux replays and the prior passing hosted Ubuntu subset. Linux did not reproduce the Windows failure; these runs do not establish a Windows fix. Types, formatting, and diff checks passed. No new full local suite or installed-consumer run was performed. The unchanged 277-file archive was rehashed, not repacked; this test is excluded from the npm payload. Three fresh native reviews and a separate independent verifier cleared the exact The full workload remains 33 jobs, 20 test lanes, and eight package-verification lanes, including both Windows machine-policy checks. The first- and last-run artifact checkpoints are complete; the failed prior-head contrast is retained separately. As disclosed in the description, two controlled overlapping imports replace the eight-way unsynchronized import stress scenario; this removes that stress coverage and does not claim to fix a production race. Existing ACL churn and fail-closed checks remain. The redundant export chunk-count assertion was removed while preserving the real large-export and forwarding checks. Production source is unchanged. Progress updates edit this comment, not the PR description. PR checks are green, the branch is mergeable, and there are no unresolved review threads. Exact post-merge main verification remains pending; the PR is still open. Merge authorization has not been granted. |
600a357 to
25ff9db
Compare
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Summary
Make CI faster and more reliable while retaining supported runtime coverage. Acceptance requires ten full passing PR attempts with p90 ≤214 seconds, versus the frozen 428-second original baseline, followed by verification of the exact post-merge main run.
Changes
pnpm testserial.packageManagerpin. An exact hit skips reinstallation; a miss or cache error performs the real install, whose failures still propagate. Keep the resolved pnpm store cache. In full CI, only package-verification jobs restore the npm download cache.RUNNER_TEMPfor the fresh Windows npm consumer and a separate private directory for credential state. Local invocations withoutRUNNER_TEMPretain the normal temp directory; Unix consumer selection is unchanged. Both owned directories are cleaned up on success or failure. Preserve optional binaries, consumer install arguments, and timeouts.There is no production-source change relative to current main.
Testing
New
5f004f6: only the timing table changes. The existing report/shard suite passed seven tests / 48 assertions in 1.615 seconds; raw identities, outcomes, and assertions exactly match the prior hosted Ubuntu subset. Formatting and diff checks passed. Independent assignment checks found all 97 Windows files exactly once across 14 nonempty workers and all 98 Unix files exactly once across four workers, with Unix assignments identical to the prior head. The 6,000-file regression fixture and all other test source are unchanged. The existing 277-file archive was rehashed, not repacked; CI timing tables are excluded from the npm payload. No new full local suite or installed-consumer run was performed. Three fresh native reviews and a separate independent verifier cleared the exacta3b157c/5f004f6publication gate with no actionable findings. Optional native-review credential/API probes encountered filesystem-ownership and fixture failures and are not counted as passing tests; new-head hosted execution is unverified.Prior
d201738: the one-line atomic PID fixture correction passed the 11-test file (326 assertions) and exact 367-case worker (358 passed, nine skipped / 2,346 assertions in 74.96 seconds). A disposable reproduction of the old empty-file race failed before the fix and passed all five assertions afterward. Types, formatting, actionlint, and diff checks passed. Three fresh native reviews and an independent verifier cleared the exact base/head; four optional credential probes blocked by sandbox ownership were not counted as passing tests. Seven complete hosted attempts all passed in 204, 202, 185, 221, 174, 196, and 237 seconds, but the declared cohort failed timing after two samples exceeded 214 seconds. Both slow samples remain included; seven-run p90 is 237 seconds. The collector did not request attempt 8. First and seventh reports preserve exact raw identities, outcomes, and assertion counts on Ubuntu and both Windows versions: 1,932 cases / 23,486 assertions on Ubuntu and 1,928 cases / 23,288 assertions per Windows version. The PID case also passed on the previously failing Ubuntu Node 24 lane and macOS. Independent Node/Bun setup delays and unrelated runtime spikes show host variance remains; this change does not claim to fix that cause.Prior
d3e1514: the exact failed seven-file Python worker passed 89 cases / 581 assertions in 34.08 seconds; types, formatting, actionlint, and diff checks passed. Three fresh native reviews and independent verification found no actionable issues. Fifteen full hosted attempts produced 12 passes, three failures, combined p90 204 seconds; all three declared cohorts failed. First-run Ubuntu/Windows inventories exactly matched the prior passing reports. The modified Python fixtures passed on both Windows versions; the first run did not exercise their larger deadline. Failures were a Windows export exit, a macOS diff-preview timeout, and the Ubuntu Node 24 empty-descendant-PID race addressed by the current correction. The first two causes remain unproven. All reports from attempts 1, 8, 10, and 15 were retained; uploaded reports exclude the macOS and Ubuntu Node 24 failure lanes, whose logs were preserved separately.Prior
25ff9dbintegrates maina3b157cand retains the workflow split and pnpm tool cache. 302 focused checks passed in 28.07 seconds; types, formatting, actionlint, and diff checks passed. Raw comparison with the previous focused run found exactly nine superseded identities removed and eight upstream replacements added, including one renamed stable-context test. No other identities were omitted. All six pnpm-cache and independent package-verification cases remain; upstream file-based CI classification and Markdown formatting replace the superseded event-based checks. The workflow expands to 33 jobs, 20 test lanes, and eight package-verification lanes. Subsequent hosted runs verified exact Windows pnpm cache misses and hits and real Unix package/test overlap. Four complete attempts measured 228 seconds passed, 214 failed, 167 passed, 195 failed; both declared cohorts failed on Python history fixtures. The combined four-attempt p90 was 228 seconds. All report artifacts from attempts 1, 2, and 4 were saved and independently checked; no test identities disappeared. The only failure deltas were the two history cases in attempt 2 and the alias case in attempt 4.Reused the unchanged
2125c0afull local four-worker coverage run: 103.912 seconds; 1,927 cases, 1,899 passed, 28 skipped. The first hosted25ff9dbrun subsequently verified Ubuntu 1,932 = 1,904 passed + 28 skipped and each Windows version 1,928 = 1,866 passed + 62 skipped, with only the declared CI-test replacements and additions. The earlier installed-consumer result and unchanged 277-file archive are reused explicitly; no repack or consumer rerun was performed for these excluded files.Prior
2125c0achanged only the excluded package-smoke harness and testing documentation. The actual installed-consumer check passed in 32.50 seconds, including the native lock fixture and nested worker. A separate injected child failure propagated and removed both owned temporary directories; the success path also removed both. All 22 focused checks, typecheck, formatting, actionlint, and diff checks passed. The first full hosted run also passed in 202 seconds. Windows npm installation took 20.6 and 25.6 seconds, down from 143.4 and 141.2 seconds on the previous revision; both native credential-lock fixtures and installed consumers passed.Prior
ef4aa31correction changes only the Windows verification workflow, its documentation, and the diff-preview Git fixture. 299 focused tests passed; the exact failed 341-case worker passed in 74.57 seconds (337 passed, four skipped), with unchanged raw identities, outcomes, and assertions. Types, formatting, actionlint, and diff checks passed. That correction subsequently passed the full hosted matrix, including both Windows package checks, in 282 seconds.Prior
7952cf6local four-worker coverage run on four CPUs: 104.19 seconds; 1,927 cases, 1,899 passed, 28 expected skips. The preceding run with the old timing estimate passed in 110.14 seconds; every raw identity/outcome is identical between those runs.For
7952cf6, compared with the previous 1,931-case revision, the only inventory changes were six superseded PR cases removed, four upstream cases replaced, and six upstream cases added. No identity normalization or unexpected omission. All 136 upstream runtime test declarations remain across the split files; only the previously reviewed ambient-import fixture differs. Three upstream cases retain their names with updated upstream assertions.Prior
7952cf6fresh package build and actual installed-consumer checks passed, including public imports, NodeNext types, CLI, the new Node SQLite credential-lock fixture, bundled executable, and nested worker without a global install. The archive contains 277 files, including 113 bundled plugin files. Tests, CI runners, timing table, and the checkout-only native fixture are excluded. The archive is unchanged and was not repacked for the excluded harness/documentation change; the new consumer-script execution is reported above.Runtime/API source, README, and native fixture are byte-identical to main
a3b157c. Bun remains 1.3.14; the attempted 1.4.0 upgrade failed unrelated compatibility checks and was not adopted.The exact
a3b157c/25ff9dbpublication gate passed: three fresh native reviews and a separate independent verifier found no actionable issues. Earlier review receipts were not reused.All 15 first-run report artifacts from
2125c0awere captured before retry. Raw identities and outcomes exactly matchef4aa31: Ubuntu 1,927 = 1,899 passed + 28 skipped; each Windows version 1,923 = 1,862 passed + 61 skipped. The tested merge has parents84d0717and2125c0a, and its tree equals that revision. Prior reports and trials remain evidence for their respective revisions.Hosted acceptance protocol
Measure each complete workflow attempt from
run_started_atto its last completed job, including queues, setup, dependencies, tests, packaging, and uploads. Nearest-rank p90 is the ninth of ten sorted durations. The frozen original baseline is the latest ten qualifying full PR attempts before implementation; it includes five successes and five failures.Declare each non-overlapping ten-run cohort before it starts, on one unchanged head. All ten must pass. A code revision starts a new declared cohort; the prior incomplete cohort remains recorded. Stop and diagnose a failure; a cohort may stop for timing futility after two samples exceed 214 seconds. Retain every failed cohort and earlier trial. The retry-cohort rule was amended after the first
6fa83bctiming failure; no slow sample is dropped within a cohort. Require both the fixed-cohort p90 and the combined p90 of every complete current-head attempt to be ≤214 seconds. Report both; ten runs do not guarantee long-run p90.Validation checkpoint before publication:
5f004f6followsd201738on basea3b157cand changes only the large tracked-output fixture's Windows timing estimate. New-head hosted behavior and ten-run acceptance remain unverified. All 70 completed earlier trials remain below. Current hosted results are maintained in the benchmark comment.Frozen original runs
All recorded candidate attempts
410ab2b410ab2be59b593e59b593cbc9733cbc97335f29cb45f29cb44c32cbafa39fe78db62138db62138db621327c912827c91286fa83bc6fa83bc6fa83bc6fa83bc6fa83bc6fa83bc6fa83bc7be83797be8379617db52617db52384d242384d242384d242384d242384d24291625fe91625fe91625fe91625fe91625fe91625fe7952cf6ef4aa312125c0a2125c0a2125c0a2125c0a2125c0a25ff9db25ff9db25ff9db25ff9dbd3e1514d3e1514d3e1514d3e1514d3e1514d3e1514d3e1514d3e1514d3e1514d3e1514d3e1514d3e1514d3e1514d3e1514d3e1514d201738d201738d201738d201738d201738d201738d201738Risk and rollout
No public CLI surface, runtime dependency, supported runtime lane, or protected check name changes. The three scan-history child deadlines increase from 10 to 30 seconds; the existing whole-test and workflow limits remain unchanged. This permits slower functional-fixture completion and may delay failure of a stuck child by up to 20 seconds. Whole-workflow measurements still include that time. The six additional Unix verification jobs repeat setup and use more runner resources; any queue or setup cost remains part of the whole-workflow benchmark. Timing estimates affect scheduling only; per-worker LCOV is diagnostic, not a merged line-coverage guarantee. The upstream classifier precedes heavy jobs; invalid titles and formatting failures still fail required checks.
Upstream's SQLite transaction now serializes owner inspection and cleanup, superseding this PR's earlier same-version lock workaround. Upstream's compatibility limitations remain unchanged. The earlier isolated macOS CLI-output timeout remains unexplained and unchanged. Recurring Windows history Python startup/child failures motivated the disclosed fixture setup and deadline change; their underlying host cause remains unproven. The prior
2125c0aWindows login-readiness failure also remains unproven and retained in the benchmark. The removed eight-way ambient-import stress scenario is not claimed fixed in production.Hosted runner queues and setup outliers remain included in the measurements. In the earlier
91625fecohort, one job waited 305 seconds for a runner; another cached-Node worker spent about 24 seconds collecting tool versions and 21 seconds installing pnpm. Cache hits do not eliminate this observed variance, and the logs do not establish its underlying cause.Do not merge until required checks, an approving review, and the ten-run timing comparison pass. Verify the full main-branch workflow for the exact merge commit afterward; the earlier main runs are not that verification.
Public disclosure review