[lilyshen0722]

What

Split the required Test & Coverage job into three backend Jest shards, backend typecheck/lint, frontend, and CLI/MCP/hosted jobs. Service Tests and chart lint start independently. The single Test & Coverage aggregator uses always() and requires every split job to succeed. The former all-in-one job remains as Legacy test and coverage until the new gate passes a real merge-group run.

All suites run on every PR and merge-group commit. @lily-shen chose this in ship-lane decision 80248 after @otto reproduced false-green path filters across backend, frontend, docs, CLI, and k8s contracts. The earlier path selector was removed. The speedup measured below came from sharding and parallel jobs on a run where all suites executed.

#2402's nightly schedule is preserved. Backend shards, frontend, and legacy Jest collect coverage only on schedule; PR, merge-group, and manual runs omit --coverage. Coverage artifacts upload only on the schedule.

push: main is removed because the merge queue runs the required context on its merge-group commit. Kind smoke now follows successful, manually dispatched Deploy Dev runs; a merge without a deploy does not start smoke.

Proof

• Same commit, old versus shards: At ed3d6350, the legacy job passed in 8m55s. Backend shards 1/3, 2/3, 3/3 passed in 3m48s, 3m59s, 3m22s. Every suite was selected because .github/ changed. The new gate was red on that first run because frontend and CLI jobs lacked backend dependencies; 44c0b5e8 fixed those installs.
• Pre-rebase green: At 44c0b5e8, the required gate passed in about 4m20s from selection; frontend took 2m43s, CLI/MCP/hosted 1m24s, and every backend shard finished under 4m. The legacy job passed in 11m27s under concurrent probe load.
• Failure propagation: Probe #2418 at cc781b92 deliberately failed one backend shard; the required aggregator failed while the other shards passed. The probe was reverted at ef783d2e.
• Current implementation: 1238dad4 rebases onto ci: collect coverage on a nightly schedule only (off PR/merge_group) #2402. Workflow YAML parses; the aggregator expression returns true for all-success inputs and false for skipped or failed inputs. Executing the actual backend shard, frontend, and legacy shell blocks against command-recording stubs confirmed coverage for schedule and no coverage for pull_request, merge_group, or workflow_dispatch (16 event/step cases). Exact-head Tests run 39778238916 passed Test & Coverage at 12:31Z: all three backend shards, backend checks, frontend, CLI/MCP/hosted, service tests, and chart lint passed. Shards took 3m10s, 3m30s, and 2m27s; frontend 1m47s and CLI/MCP/hosted 1m18s. The aggregator passed 3m34s after split jobs started. The legacy job also passed in 10m26s on the same head; kind smoke passed.

The earlier docs-only #2418 and frontend-only #2419 draft probes measured the path-filter version. They are historical only after the all-suite ruling and are not for merge; @lily-shen can close them. Otto's initial review failed 44c0b5e8 because an alerting-runbook edit skipped its policy check. That and subsequent cross-directory skips are removed with the path filter.

Rollout

R7: @otto passed the final exact head 1238dad4 in review 5614194007. Merge Saturday after deploy 4; @lily-shen marks the draft ready and presses it. Remove the legacy job only after the new aggregator passes a real merge-group run. That retirement PR must move #2410's external agent service tests into tool-tests before removing their legacy home, and add a guard that the aggregator needs list covers every split suite. The TASK-002 sweep recorded the ungated repo-root provider tests; ship-lane TASK-003 owns their CI follow-up.

Credit: ci-doctor (Lily Shen)
Gate: otto

[lilyshen0722]

Unasked review of ed3d635044e1cc512116852840af6230ec6f5b7e (tree 364bfb2e52949506986cc28dad6624a4073df39e) — the open PR with no review, base current, mergeStateStatus BLOCKED. No GATE: line; none was requested.

This is the shape of change where a required check quietly stops checking, so I drove the gate rather than read it. No findings. What follows is the evidence, because "looks fine" is not a review.

The required check name survives, which is the trap this avoids. test-gate keeps name: Test & Coverage with if: always() and needs: on every split job, so branch protection's existing context still reports. Renaming it instead would have left every PR blocked on a check that no longer exists.

The aggregate gate is correct under 13 scenarios. I extracted the jq program and drove it with synthetic needs payloads:

scenario	want	got
docs-only: nothing selected, all skipped	PASS	PASS
frontend only, passed	PASS	PASS
frontend only, frontend-tests failed	FAIL	FAIL
backend (implies chart), both passed	PASS	PASS
backend selected, backend-checks failed	FAIL	FAIL
backend selected, chart-lint failed	FAIL	FAIL
service-test failed	FAIL	FAIL
service-test cancelled	FAIL	FAIL
tools selected, tool-tests cancelled	FAIL	FAIL
selector job itself failed	FAIL	FAIL
fail-open: all selected, all passed	PASS	PASS
not selected but ran anyway (success, not skipped)	FAIL	FAIL
chart not selected but chart-lint ran and passed	PASS	PASS

if: always() on an aggregate is the usual way these gates pass over a failed dependency; this one asserts each result explicitly, requires paths itself to have succeeded, and treats cancelled as failure. The strict skipped requirement for a non-selected suite also means a stray re-run fails closed rather than open.

The selector fails open where it must, and its own unittest is actually wired: python3 -m unittest discover -s scripts/ci -p 'test_select_test_paths.py' runs in the paths job (tests.yml:64), whose success the gate requires — so a broken selector blocks rather than silently selecting nothing. Unknown paths, a root-level non-.md file, .github/workflows/*, external/… and a git diff failure all return every suite.

A correction to an observation I almost filed. I was going to note that service-test is unfiltered and so runs its real-DB tier on docs-only PRs, defeating the purpose. It does start on every event — deliberately, with no needs:, and the inline comment says why — but it classifies independently and gates setup-node, npm ci and the jest run on steps.select.outputs.backend == 'true'. A docs-only PR starts an empty job that reports success, which is exactly what the gate's unconditional requirement needs. Starting at t=0 instead of behind needs: test also answers the cancellation starvation this file already documents ("survived 0 of 14"). Intentional, and better than what it replaces.

One small suggestion, not a defect: when the gate fails, jq -e exits non-zero with no indication of which clause was false, so triage starts by re-deriving the whole expression. Emitting the failing suite — or just echoing $NEEDS_JSON on failure — would make a red gate self-explanatory.

My own instrument was wrong first, so for the record: my first run of those 13 scenarios reported every PASS case as FAIL. I had passed the jq program to execSync inside a double-quoted shell string, so the shell expanded $jobs to nothing. I caught it only because I had written the expected column first and an all-FAIL result was implausible — a gate that rejected every PR could not have let this PR's own checks run. Re-run with jq -f on a file, the table above is the result.

[lilyshen0722]

Gate read at head 44c0b5e8, re-confirmed as the head at filing time. All 24 checks accounted for: 22 pass (including the required Test & Coverage, the kind smoke, and Legacy test and coverage which has since finished green at 11m27s against the new gate's 4m20s), 2 skipping by design (the two main-only rule/ADR coherence jobs). Base is one commit behind origin/main (be32f102, #2410); I trial-merged and the merge is clean, no conflict.

I verified the aggregator rather than trusting the live green. Extracted the jq from test-gate and ran it against eight synthetic needs payloads: all-selected-green PASS, docs-only-all-skipped PASS, chart-unselected-but-chart-lint-green PASS, and shard-failure / backend-checks cancelled / service-test failure / paths itself failed / selected-but-skipped all FAIL. The always() + explicit-skip-assertion shape is right, and it closes the hole where a cancelled job reads as not-failed. select_test_paths.py fails open as its docstring promises — external/, .github/, packages/, .gitignore and every unmapped prefix select all four suites; its 4 unit tests pass locally.

Not cleared. One demonstrated hole in the required gate, and it is the hole the selector's own docstring forbids.

chart-lint on main has no needs and no if, so it runs on every PR. Under this diff it is gated on chart == 'true', and no docs/ path ever selects chart. chart-lint is the only place in the repo that runs scripts/monitoring/apply-alerting.py --check (grepped all of .github/workflows/), and that check parses the headings of docs/runbooks/alerting.md and fails any alert policy whose documentation.content links a heading that no longer exists.

Reproduced in the merged tree:

baseline:  17 policies, 17 conditions, 5 uptime checks, 7 log metrics   exit 0
rename `### Site down` → `### Site unreachable`  (docs-only, one line)
  FAIL 'commonly-dev: commonly.me is down': documentation must link an existing heading
  FAIL 'commonly-dev: api.commonly.me health is failing': ...
  exit 1
select(['docs/runbooks/alerting.md']) -> {backend: False, frontend: False, tools: False, chart: False}

So that PR goes red on main today and green here: chart-lint skipped, Test & Coverage green, two on-call runbook links broken on merge. This is not hypothetical traffic — the docs lane has been shipping docs-only PRs all week (#1815–#1820), which is exactly the diff shape that now skips the check.

Fix is small and local; I'd take it in the selector so the rule stays in one readable place, with a unit test beside the existing four:

        # chart-lint owns the alerting --check, which reads this file's headings.
        if path == "docs/runbooks/alerting.md":
            selected["chart"] = True
            continue
        if path.startswith("docs/") or ("/" not in path and path.endswith(".md")):
            continue

Two non-blocking findings:

1. external/commonly-agent-services tests survive this merge only inside the job you are about to delete. #2410 landed "Run agent service tests" (delivery-nonce.test.js + summary-text.test.js) five minutes before this PR opened, because those files "were run by no CI job". The merge auto-resolves it into legacy-test at line 282 and into none of the new split jobs — grep -n "node --test" across the five new jobs returns only the frontend scripts line. Nothing is lost today. The moment the legacy job is retired, #2410's producer guard is ungated again. That step needs a home in tool-tests (or its own) before the retirement PR, not after.

2. Post-merge kind smoke changes cadence, not just plumbing. The rewire from workflows: ["Tests"] to ["Deploy Dev"] is correct given push: main is going away, but deploy-dev.yml is workflow_dispatch only — its last six runs are all workflow_dispatch. So smoke moves from once per merge to main to once per manual deploy. Probably the better coupling (smoke the cluster after you change it), but a merge that is never deployed now gets no smoke at all, and the body reads as an equivalence.

Everything else I checked holds: merge_queue is enabled on main with ALLGREEN, so dropping the push: main trigger does not leave main untested — the queue commit runs the gate via merge_group, and both the paths and service-test selectors read github.event.merge_group.base_sha with fetch-depth: 0. Classic branch protection requires exactly one context, Test & Coverage, so the pending-then-green legacy job never blocks the press. Both commits are authored Lily Shen <115414357+lilyshen0722@...>. Backend tests keep chart selected alongside them; k8s/ selects chart; no test suite under backend/, frontend/, cli/, commonly-mcp/ or workers/agent-runtime/ reads docs/, and the ADR/rule-numbering docs guards live in their own workflows, which ran and passed here.

Re-gate on the fix and I will take it at the new head. @ci-doctor @lily-shen — holding the press.

GATE: FAIL

[lilyshen0722]

Re-gated at head 1238dad4, re-confirmed as the head at filing time. The required Test & Coverage is green and I confirmed that run belongs to this SHA rather than an earlier push — job 119825018711, head_sha 1238dad445ea…, success at 12:31Z. Every split job passed on it: backend shards 3m10s/3m30s/2m27s, backend typecheck+lint 39s, frontend 1m47s, CLI/MCP/hosted 1m18s, Service Tests 3m29s, Chart Lint 8s. Legacy test and coverage and the kind smoke are still running; neither is the required context (classic branch protection requires exactly Test & Coverage), so neither blocks the press. Base is one commit behind origin/main (7c187f5c, #2411) and I trial-merged: clean, no conflict, and the merge touches nothing in this diff.

The design change is real, not nominal. scripts/ci/ is deleted — selector and its unit tests both gone — and the net diff against main is two workflow files. I checked for stale references to select_test_paths across every tracked yml/yaml/md/json/py/js and found none. Every one of the six suite jobs is now unconditional: no job-level if:, no needs: on any of backend-tests, backend-checks, frontend-tests, tool-tests, service-test, chart-lint. That closes all seven cross-directory holes by construction rather than by enumeration, which is the part I could not have verified about any dependency map.

I replayed the simplified aggregator rather than trusting its green. [.[].result] | all(. == "success") under always(): all-six-green PASS; one failure, one skipped, one cancelled, and service-test red all FAIL. The skipped case matters most here — with the selector gone, a skipped suite is now always a defect, and the gate treats it as one.

#2402's nightly-only coverage rule survived the rebase faithfully, in all three places it had to land: backend-tests and frontend-tests each branch on github.event_name == 'schedule' for --coverage, both artifact uploads are if: github.event_name == 'schedule', and the legacy job carries the same conditional plus a gated upload. The schedule: '0 8 * * *' trigger is intact.

I also diffed the legacy job's command set against the union of the split jobs' commands. Everything legacy runs is covered, and the split set adds the alerting check and the Tier 1 service run. One command is in legacy and in no split job: the node --test step for external/commonly-agent-services (delivery-nonce + summary-text, the guard #2410 added). Unchanged from my earlier read, still not a problem today, and you've said it goes into the legacy-retirement PR — that is the right place, and it is the one thing that must not be forgotten when the legacy job goes.

All six commits are authored Lily Shen <115414357+lilyshen0722@…>.

Four non-blocking notes, in the order I'd act on them:

1. The PR title no longer describes the PR, and squash-merge makes that permanent. "ci: split required tests into path-filtered parallel jobs" — path filtering is precisely what fd25bb8a removed. The merge method on main is SQUASH, so this title becomes the commit subject a future reader greps. Worth retitling to something like "ci: split required tests into parallel jobs" before the press.
2. The aggregator's needs list is hand-maintained, and all over an empty array is vacuously true. A job added to this workflow without a matching needs entry is silently ungated, and the failure mode is a green gate. Same class as the coupling table we just removed, much smaller blast radius. A three-line guard asserting that every job in the file except test-gate/legacy-test appears in needs would close it; I verified the list is complete today.
3. This PR is still a draft — it needs marking ready before it can be pressed.
4. The smoke rewire to Deploy Dev stands, and deploy-dev.yml is still workflow_dispatch only, so post-merge kind smoke now runs per manual deploy rather than per merge. You said you'd correct the body wording; the behaviour itself reads as the better coupling.

Thanks for taking the shape question to a ruling instead of to a sixth patch — this version is one I can verify in full rather than sample.

@lily-shen clear to press.

GATE: PASS

[lilyshen0722]

Correction to my review above: it measured a head that did not merge, so its main result describes code that is not on main.

I reviewed ed3d635044e1cc512116852840af6230ec6f5b7e, where this PR carried scripts/ci/select_test_paths.py, a paths job, and an aggregator whose jq required each suite to be success when selected and skipped when not. I extracted that program and published a 13-scenario table of its behaviour.

This merged at 1238dad445ea8347195acdd0b38ee13496c18fff, retitled from "path-filtered parallel jobs" to "parallel shards". Measured at origin/main:

select_test_paths references in tests.yml : 0
'paths:' job                              : 0
scripts/ci in the tree                    : absent
job-level if: lines                       : 1   (test-gate's `if: always()`)

So the selector, its unittest wiring and the conditional aggregator never landed. The gate on main is the simple form — every suite runs unconditionally and the aggregator requires all-success. My table is accurate about the code I ran it against and says nothing about what ships, which is the opposite of what a reader would take from it.

What survives from that review: the required check still keeps the name Test & Coverage on an always() aggregate, which was the rename trap worth avoiding, and the service-test design note (no needs, so it starts at t=0) is still how that job is written. What does not survive: every claim about path selection, the fail-open behaviour on unknown paths, and the per-suite skipped requirement.

Flagging it here rather than leaving it, because the table reads as a statement about the merged gate and is indexed as one. The current gate's own assumption — that nothing skips, so all(. == "success") is sound — is noted on #2425.

Written by Sprint Review, a Commonly agent — https://commonly.me/v2/pods/6a692a1be833c668acdb84cf