[lilyshen0722]

What

Retire the Legacy test and coverage job after #2417's replacement Test & Coverage passed a real merge-group run. Move the two external/commonly-agent-services node:test files from the legacy job into tool-tests, preserving their CI guard. Keep the required check's exact name and independent Service Tests job.

The aggregator now compares every top-level suite job in tests.yml with its needs keys before checking all results. A future suite added without an aggregator dependency fails the required check.

Proof

• #2417 merge-group run 39783207076 reported Test & Coverage success at 2026-10-22 13:04:04 UTC on merge commit 890171a7. This satisfies the legacy-retirement condition.
• Local node --test of the two moved agent-service files passed all 13 tests.
• Workflow YAML parses; legacy-test is absent, Test & Coverage keeps its exact name, and Service Tests still has no needs.
• Executed the guard shell block against the current workflow and a temporary variant with an omitted future-suite job: current job set passed; the omitted dependency failed with future-suite in the diff.
• Exact-head Tests run 39784288048 passed Test & Coverage at fed37d26: all three backend shards, backend checks, frontend, CLI/MCP/hosted (including the moved agent-service tests), service tier, and chart lint passed. The required gate's checkout/completeness step also passed. Kind smoke also passed at this head.

This is follow-up 1 of Lily's ordered list. The strict render-parse step and any future path filtering each get their own PR. @otto gated exact head fed37d26 PASS in review 5614805051; @lily-shen handles the merge.

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

[lilyshen0722]

Gated at head fed37d26, re-confirmed as the head at filing time. The required Test & Coverage is green and belongs to this SHA (head_sha fed37d26f75a…, success). Every suite passed on it — backend shards 1m52s/2m13s/2m48s, backend typecheck+lint 50s, frontend 1m52s, CLI/MCP/hosted 1m59s, Service Tests 3m17s, Chart Lint 10s — and I held for the kind smoke rather than filing while it ran: it passed at 13:15Z. No Legacy test and coverage context is reported any more, which is the visible half of the retirement. Base trial-merged against origin/main (890171a7): clean. Single commit, authored Lily Shen.

The precondition was a claim, so I measured it rather than accepting it. Retiring the legacy job was conditioned on the new aggregator passing a real merge-group run, and that has now actually happened: gh run list --workflow=tests.yml --event=merge_group shows a success on 890171a7 at 13:01Z — the queue commit for #2417, which merged at 13:04Z. Every earlier merge_group run in the list was cancelled, so this is the first genuine pass, and it is the right one. The condition is met in fact, not in prospect.

All three carry-forwards from the #2417 gate landed, and I verified each by running it:

1. The external/commonly-agent-services step is relocated, not dropped. It sits in tool-tests as its own run: block, which matters: those suites need repo-root cwd and a step's shell starts at the workspace root regardless of earlier cds. Ran both files from root exactly as the job will — 13 tests, 13 pass. This was the one command that existed only in the job being deleted, so it was the thing most likely to be lost, and it wasn't.

2. The needs-completeness guard bites in both directions. I ran its awk against the head: six job ids, matching the needs list exactly, exit 0. Then I perturbed it twice — dropped chart-lint from needs → exit 1; added a brand-new-suite: job to the file without a needs entry → exit 1; restored → exit 0. It also closes the vacuous-pass hole I flagged on #2417: emptying needs now fails this step before [.[].result] | all(...) can return true over an empty array. And it fails closed by construction — if the /^jobs:$/ anchor ever stops matching, the awk side is empty, the needs side is not, and the diff fails.

3. The smoke wording was a PR-body item and was already corrected on #2417. smoke-test.yml correctly needs nothing here; the behaviour (kind smoke following a manually dispatched Deploy Dev) is unchanged and accurately described.

I also re-ran the orphan check that found carry-forward 1 in the first place, this time comparing main's now-deleted legacy-test command set against the union of every remaining job: nothing orphaned. The only non-match is a line of comment prose. So the deletion removes a duplicate, not coverage.

Two non-blocking notes:

1. The guard makes "every job gates" a policy, not a convenience. It excludes only test-gate, so any future job added to this workflow must appear in needs or CI goes red. That is almost certainly the right default — it is exactly what stopped legacy-test from being forgotten in reverse. Worth knowing that a deliberately advisory job (an experiment, a soft-fail probe) now requires editing the guard's exclusion rather than just being added, and the error it produces will read as a missing-coverage failure rather than "this job is intentionally not gating."
2. The guard depends on bash process substitution. The workflow sets no shell: on the step and no defaults: run: shell:, so it gets Actions' default bash -e and works. If anyone ever adds a workflow-level shell: sh default, this step breaks with a syntax error rather than a coverage message.

Still a draft — it needs marking ready before it can be pressed.

@lily-shen clear to press.

GATE: PASS

[lilyshen0722]

Unasked review of fed37d26f75a4e42f904766793b5c6961a0b22c7 (tree 6eccd41421290a711fc85155f6458bcfb7e006de) — unreviewed, base current, mergeStateStatus UNSTABLE. No GATE: line; none was requested.

The stated proof holds, to the second. #2417's comment made retirement conditional on a real merge-group run, so I checked the run rather than the claim:

run 39783207076: event=merge_group  conclusion=success  head_sha=890171a7417af05ca296a92d4142fe3c7b12b5a1
job "Test & Coverage": conclusion=success  completed=2026-10-22T13:04:04Z
890171a74 = "ci: split required tests into parallel shards (#2417)", ancestor of main

Event, conclusion, job, sha and timestamp all match the body. The precondition is genuinely met — worth saying because two other PRs today state preconditions that are not.

The self-audit works. I ran the gate's own awk over this head's workflow and diffed it against the needs list by hand: six ids on each side, diff -u clean. So a suite added without an aggregator dependency does fail the required check, as claimed.

The moved tests are preserved exactly — both external/commonly-agent-services/shared/{delivery-nonce,summary-text}.test.js move from legacy-test into tool-tests with the same node --test invocation.

One finding, and it is about a coupling the body does not state.

[.[].result] | all(. == "success") is correct only because no job in this workflow skips. I checked: at this head there is exactly one job-level if: in the file (test-gate's if: always()), and grep -c steps.select.outputs is 0 — nothing is conditional, so every need always reports success and requiring all-success is both right and simpler than what it replaces.

That makes the gate brittle in a specific, predictable direction. The moment a suite becomes conditional — a job-level if: for a docs-only skip, which is the obvious next move for a workflow whose predecessor was titled "path-filtered" — its result becomes "skipped", all(. == "success") is false, and the required check fails on exactly the PRs the filtering was meant to speed up. The failure is loud and fail-closed rather than silent, which is the right direction, but the next person to add filtering will spend the debugging time.

Two lines of comment above the jq saying "every job here runs unconditionally; if a suite becomes conditional, this must accept skipped for it" would carry the constraint to whoever touches it. The coverage check above it already encodes the set of suites; this encodes the assumption about their results, and only one of the two is written down.

Separately, for the record, and it corrects my own earlier work rather than this PR: my review of #2417 was taken at ed3d635044, where that PR carried scripts/ci/select_test_paths.py, a paths job, and a per-suite conditional aggregator — and I published a 13-scenario table of that gate's behaviour. #2417 merged at 1238dad445 under the retitled "parallel shards", and main has none of it: select_test_paths 0 references, no paths job, scripts/ci absent from the tree. So that table describes code that never landed, and I have said so on #2417. It also means this PR is not removing path filtering — there was none on main to remove, which is what I initially suspected and had to drop.