Repository navigation
perf(ci): cut the E2E critical path by 51% and the macOS unit leg by 15% - #1283
Conversation
Tencent#1210 pinned the POSIX `bash -lc` rendering in codex-team-hooks.test.ts, and Tencent#1221 put that file on the Windows test list. On Windows the command renders the Git Bash path, and the Windows list guard rejects a pinned POSIX rendering in a file that does not pin the platform. Every Lint & Test leg and the Windows job on main fail.
14abcf5 to
04aae97
Compare
The full-surface E2E job ran every E2E file on one runner under the fixture-repo lock, so it set the pace of every main push and internal PR. Only e2e.test.ts uses the fixture repo: run that file alone under the lock, and run the rest in the three credential-free shards on every event.
04aae97 to
2df819d
Compare
c44449a to
2df819d
Compare
Unit tests that hold the exclude lock wait out updateFileLocked's whole retry budget, and the workflow driver tests wait out its 2 s poll. Production keeps both values; the test files that pay for them shorten them through _setExcludeLockRetryMs and _setDriverPollMs.
|
No confident findings in the specified PR diff. The description documents sufficient testing: it records two credential-free E2E shard runs and links the CI run, alongside unit-test results. The explicitly deferred credentialed E2E run is not blocking; the review rules require one representative real-CLI run, not the full provider × agent matrix. Review was read-only; I did not run, build, or install PR code. |
findMcpGitExcludes asks where each agent's config directory is excluded, including directories not created yet (.codex, .kiro/settings, .trae). For each one locateExclude started git in a missing cwd, which always failed, and the null answer is not cached, so every call paid for it again. The answer is the same null; it now comes without the process. mcp-reconcile.test.ts and uninstall.test.ts start 35% fewer git processes (10918 to 7115) and run 13% and 9% faster locally.
Five of the six unit legs ran with --coverage and discarded the report: only Ubuntu on Node 20 uploads it, and no threshold reads it. In a paired run on one macOS runner, coverage cost about 67 s of a 550-810 s leg.
|
No confident findings in the specified PR diff. The earlier review raised no findings, so none require a resolution update. The description documents sufficient testing: two credential-free E2E shard runs, a linked CI run, and unit-test results. The explicitly deferred credentialed E2E run is not blocking; one representative real-CLI run suffices under the review rules. Review was read-only. I did not run, build, or install PR code. |
jeff-r2026
left a comment
There was a problem hiding this comment.
LGTM. CI/test-only change; approving per maintainer decision.
Summary
CI is slow from work, not from queueing: runners start in 3–13 s. Two jobs set the wall time. On main and internal PRs it was the credentialed E2E job, which ran every E2E file on one runner under the fixture-repo lock. On every PR it is the macOS unit leg, whose suite spends 43% of its test time in 44.6k
gitprocesses.Work the unit suite did for nothing:
Production keeps the lock retry and the poll interval.
Evidence
E2E (GitHub provider, full surface): 913 s median over 40 main pushes.After: slowest credential-free shard: 449 s, 398 s and 419 s in this PR's runs (-51%).
After: 713 s and 729 s (Vitest 675–698 s), -15%. Run.
mcp-reconcile.test.ts136–145 s → 95–102 s;workflow-workspace.test.ts, touched only by the coverage change, 160–172 s → 123–133 s.mcp-reconcile.test.ts+uninstall.test.tsstart 10918 → 7115gitprocesses; 51–52 s → 44–45 s and 17–18 s → 16 s, three runs each. Unit suite 12180 passed;tscand lint clean.npm run build, thennpm run test:e2e):git-exclude-worktrees-915,git-exclude-delivered-915,claude-mcp-local-915: 73 passed.Not measured: the credentialed
e2ejob, which fork PRs skip, so its first run is the merge to main. macOS runners vary: the same commit takes 300 s on some and 700–850 s on others; the numbers above compare the slow ones.Ruled out: raising Vitest workers on the 3-vCPU macOS runner. 6 workers halved the leg, but 8 timed out submodule tests in
workflow-workspace.test.ts, so the headroom is too thin to rely on.Merge Danger
Door: two-way
Blast Radius: CI, and
teamai pull/teamai uninstallwhen they look up a git exclude file for an agent directory that does not exist (same answer, without starting git).If the narrowed
e2ejob fails on main, a revert restores the single-runner job.