ci(smart-ci): add the nightly coordinator decision module (CI-10 slice 1) - #2587
Conversation
CI-10 slice 1 (#2334): a pure decision over the last deep-qualified receipt, the main head and tree SHAs, the diff since that receipt, the clock and the weekly UTC slot. Verdicts are no-change, affected, weekly-full and full-sweep; every fail-closed path selects the complete sweep with a stable reason id, and a quiet night is an explicit receipt rather than a skipped workflow. Path-to-group mapping reuses matchGroups() from lib/plan.mjs. The group-to-suite table is a module constant that CI10-2 moves into ci/policy.v1.json. No workflow behaviour changes in this slice.
The issue's head-start test plan (#2334): no-change with both SHAs, backend-only selection without the browser matrix, the weekly slot overriding an empty diff, missing / unreadable / incomplete last receipt, an unreachable diff, duplicate qualification, an unmapped path, a changed control path, determinism, and CLI argv parsing with file output through a temp directory. Adds the enumeration test that asserts every ci/policy.v1.json path group has a deep-suite mapping in both directions, so a new policy group cannot silently select nothing.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
… edges Five ci-nightly.yml jobs declare needs: backend-solution, so a receipt that selected e2e-smoke, e2e-cross-browser, container-images or the k6 suites without backend-solution promised evidence GitHub Actions would skip. Declare the needs edges as SUITE_PREREQUISITES, close every receipt selection under them, and enumerate the workflow edges in the tests so a new edge cannot drift away from the table.
Fix round: one HIGH finding fixed, six non-blocking findings triagedHead Fixed (HIGH): selection was not closed under the workflow
|
|
Review (agent half of the ADR-0066 gate; Codex credits exhausted, SC-9). Two fresh-context reviewer passes (read-only, Opus 5). This PR is CI-control ( Round 1 at f6a3ff6: verdict FIX_FIRST on one HIGH. Round 2 (scoped to the fix diff) at 9293312: verdict SHIP. The closure is monotone (adds only), no suite can be both selected and skipped, the guard is non-vacuous and runs in the Planner Self-Test lane, and the module is imported by no workflow yet, so nothing it computes can select, skip or gate a job in this slice. Confirmed by round 1: two new files only; determinism (no Non-blocking findings, all recorded here and on #2334 for the slice that makes each reachable:
Merge gate: maintainer review (SC-10) plus hosted |
docs(outstanding): v0.3 residual decision batch D-1 to D-11, queue #2587 under SC-10
Summary
CI-10 slice 1 (
CI10-1-coordinator) from the acceleration-bundle issue filedocs/analysis/2026-08-30-acceleration-bundle/issues/2334-ci-10-nightly-coordinator-and-release-qualification.md.Two new files, nothing else touched:
scripts/ci/smart-ci/nightly-coordinator.mjs- a pure decision function plus a thin CLI, in thesame shape as
plan.mjsandrecall-report.mjs.scripts/ci/smart-ci/nightly-coordinator.test.mjs- 24node --testcases covering the issue'shead-start test plan plus the workflow dependency graph.
This slice changes no workflow behaviour. Nothing schedules, skips or gates a job here. Wiring the
verdict into
ci-nightly.ymlandnightly-quality.ymlis CI10-2, the weekly sweep is CI10-3, andthe release-trigger collapse is CI10-4. Consistent with the issue's rollout note, the coordinator is
meant to land computing and logging its verdict while the existing nightlies still run
unconditionally; rollback is deleting two files, since nothing consumes them yet.
The function answers one question: what changed on
mainsince the last complete deepqualification, and which deep suites would produce new evidence tonight. Every input is explicit.
There is no network access, no git invocation and no clock read anywhere in the module, including
the CLI: the workflow will supply the SHAs, the diff, the receipt and
nowUtcas arguments andfiles.
generatedAtUtcechoes thenowUtcinput rather than falling back toDate.now(), so areceipt is reproducible.
Path-to-group matching reuses
matchGroups()fromscripts/ci/smart-ci/lib/plan.mjs. No secondglob matcher was written, and the escalation reason ids
unmapped-pathandcontrol-path-changedeliberately repeat the planner's vocabulary.
Design
Inputs: the parsed policy document and its digest, the last deep-qualified receipt
(
{ headSha, treeSha, completedAtUtc, complete }) or null, the currentmainheadShaandtreeSha, the changed-file list between the last qualified SHA and currentmainor null when thatdiff is unavailable,
nowUtcas an ISO-8601 string, an optional weekly slot as a UTC weekdaynumber, and an optional
forceFullflag for the laterworkflow_dispatchinput.no-changeidentical-tree-sha), or the diff maps only to path groups with no deep suite (no-deep-suite-groups)affectedaffected-groups)needs:edgesweekly-fullweekly-slot)full-sweep--force-fullPrecedence is
full-sweep>weekly-full>no-change(identical tree) >affected.Fail-closed reasons, each a stable id carried in the receipt, matching
docs/ci/SMART_CI.mdinvariant 2 (unknown change equals full escalation):
force-full-requestedlast-receipt-missinglast-receipt-unreadablelast-receipt-incompletecomplete !== true; a partially failed nightly never advances the markercurrent-head-sha-invalid/current-tree-sha-invalidnow-unparseablenowUtcdoes not parseweekly-slot-invalidpolicy-invalidvalidatePolicy()fromlib/plan.mjsreports errors, or the policy is missingdiff-unavailableunmapped-pathcontrol-path-changepolicy.controlPathsgroup-not-in-suite-mapGROUP_DEEP_SUITEScoordinator-errorA
no-changeverdict is an explicit receipt, never a skip. It still lists all twelve suites underskippedSuiteswith the reasonno-change, because invariant 1 forbids a skipped job reporting assuccess to branch protection.
Deep suite vocabulary is the twelve job ids that exist today:
openapi-guardrail,developer-portal,backend-solution,e2e-smoke,load-concurrency-harness,performance-regression-gate,e2e-cross-browser,container-images,sast-scanningfrom.github/workflows/ci-nightly.yml, andbackend-coverage,frontend-coverage,dependency-security-signalsfrom.github/workflows/nightly-quality.yml. Verified against bothfiles; the module reads neither.
The group-to-suite table
GROUP_DEEP_SUITESis an exported module constant with a comment sayingCI10-2 moves it into
ci/policy.v1.jsonnext topathGroupsso the policy digest covers it. It isdeliberately conservative: backend groups select
backend-solution,backend-coverage,load-concurrency-harness,performance-regression-gateandcontainer-images; frontend groupsselect
frontend-coverage,e2e-smokeande2e-cross-browser;containers-deployandbackend-project-filespull independency-security-signals,sast-scanningandcontainer-images; docs, repo metadata, agent tooling, worktree helpers, governance scripts andother scripts select nothing. Note that the dependency lockfiles and manifests named in the issue
(
package-lock.json,Directory.Packages.props,frontend/taskdeck-web/package.jsonand the rest)are already
controlPathsin the policy, so they escalate to the full sweep before this table isconsulted;
backend-project-filescovers the.csprojand.slnmanifests that are not controlpaths.
A selection is not a free set of job ids:
ci-nightly.ymlgivese2e-smoke,load-concurrency-harness,performance-regression-gate,e2e-cross-browserandcontainer-imagesaneeds: backend-solutionedge, and GitHub Actions skips a job whoseneeds:dependency was skipped unless its
if:usesalways()or!cancelled(), which none of these do.A receipt that selected a dependent without its dependency would therefore promise evidence that
never runs, which is the invariant 1 failure mode. Those edges are declared as the exported table
SUITE_PREREQUISITES(nightly-quality.ymldeclares noneeds:at all, so its three suites carrynone), and
closeUnderPrerequisites()closes every selection transitively insidebuildReceipt(),so the closure holds for every verdict and for any future selection path. Closure only adds, so it
can never shrink a full sweep. A group entry therefore names the suites whose evidence that surface
needs and does not have to repeat their dependencies.
nightlyCoordinatorSuiteGraphErrors()checksthe table names only known suites and stays acyclic, and a test parses the
needs:edges straightout of both workflow files and asserts they equal the declared table, so a workflow that gains or
loses an edge fails the suite rather than silently drifting. CI10-2 owns keeping the two in step
when it wires the receipt into the workflows.
nightlyCoordinatorMappingErrors(policy)reconciles the table against the live policy in bothdirections and a test enumerates it, so neither a new policy group with no mapping nor a dropped
policy group can pass unnoticed. At decision time a matched group with no mapping is a
full-sweepwith
group-not-in-suite-map, never an empty selection.detectDuplicateQualification(treeSha, lastReceipt)is exported for CI-12#2336to consume as thenightly half of its duplicate-qualification flag. It reports
tree-sha-already-qualifiedwhen thetree SHAs match, and distinguishes
tree-sha-differs,no-last-receipt,current-tree-sha-invalid,last-receipt-tree-sha-invalidandlast-receipt-incompleteso theconsumer never has to guess why a match did not happen. An incomplete receipt is not a duplicate
because it qualified nothing.
The receipt is JSON with
schemaVersion: 1andkind: "nightly-plan", carrying both current SHAs(or null with a reason), the last-qualified receipt or its unavailability reason, the sorted reason
ids, the duplicate flag and its reason, the weekly slot state, the diff availability and count, the
matched groups, the unmapped and control paths, the selected suites in canonical order, and the
skipped suites each with a reason. Per the task scope there is no
ci/schemasentry in this slice:the shape is specified in the module's JSDoc header, and CI10-2 adds the schema when it moves the
suite table into the policy. A markdown rendering for
$GITHUB_STEP_SUMMARYis derived only fromthe receipt, so it is deterministic too.
Measurement
Duplicate release qualification measured on 2026-09-04 from
gh run list:ci-release.ymlandrelease-security.ymleach ran twice for one tag only,v0.1.0(push 03:13Z and release 03:22Z on2026-08-19, both
startup_failureforci-release);v0.1.1,v0.1.2,v0.2.0andv0.3.0-rc.1each ran once on the push trigger, so the release-published trigger has not fired for a
workflow-created release since. CI Nightly has failed every scheduled night from 2026-08-26 to
2026-09-04 on exactly one job,
E2E Cross-Browser Matrix / E2E (mobile-safari), issue#2180,which the product lane now owns; Nightly Quality Signals succeeded on every one of those nights.
That measurement is the preflight the issue asks for before CI10-4 touches the release triggers. It
narrows the collapse: the double trigger is real in both workflow files, but it has produced a
measured duplicate for exactly one tag, so CI10-4 is a correctness fix against a latent trigger, not
a running cost. It also confirms the nightly failure the issue makes this lane's ownership is a
single job, not a broad nightly problem.
Docs to add after the pending SMART_CI.md changes land
docs/ci/SMART_CI.mdis deliberately untouched here. Two other pending changes, PR #2506 and theunpublished branch
origin/issue-2326/smart-ci-recall-report(tip3b291914a), already add rows toits sections 10 and 11, and a third writer would collide. The rows below are formatted to match the
existing ones and should be added by whichever change lands last.
Section 10, Commands, after the
Action pin inventoryrow:Section 11, File map, after the
action-pins.mjsline:The module's JSDoc header carries the full receipt shape and the CLI usage, so both lines are
derivable from the source if the wording needs to change.
Verification
Run from the worktree
.worktrees/codex-2334-ci10-nightly-coordinator, on Node 24, at head929331247:node --test scripts/ci/smart-ci/nightly-coordinator.test.mjs- 24 tests, 24 pass, 0 fail.node --test scripts/ci/smart-ci/*.test.mjs- 115 tests, 115 pass, 0 fail. The whole Smart CIsuite is green, so the new module does not disturb the planner, gate, recall, estate, action-pin
or merge-ref tests.
node scripts/check-docs-governance.mjs- "Docs governance check passed."git diff --check- clean, exit 0.full-sweep, reasonlast-receipt-missing, alltwelve suites listed as run.
The test file covers exactly the head-start plan: no relevant change gives
no-changenaming bothSHAs; a backend-only change selects the backend suites and asserts
e2e-cross-browseris absent;the weekly slot forces
weekly-fullwith an empty diff and again with an unchanged tree SHA;missing, unreadable and incomplete last receipts each give
full-sweep; an unreachable diff givesfull-sweepwithdiff-unavailable; duplicate qualification is detected including thecase-insensitive SHA form; an unmapped path and a control-path change each give
full-sweep; twocalls with the same input produce byte-identical JSON and markdown, and input ordering does not move
a byte; the policy-group enumeration runs in both directions and proves a synthetic new group fails
closed; and the CLI test drives the real script through a temp directory, asserting the JSON file,
the markdown file, the appended step summary and stdout all agree.
Two cases were added in the fix round for the
needs:closure. One parses theneeds:edges out ofci-nightly.ymlandnightly-quality.ymland asserts they matchSUITE_PREREQUISITESexactly,that every one of the twelve deep suites is a real job in one of the two files, and that the parse
actually found the five documented edges rather than nothing. The other walks seven representative
diffs, including a docs-only no-change and a fail-closed full sweep, and asserts no receipt ever
selects a suite while listing one of its prerequisites as skipped. The guard was mutation-tested:
deleting the
container-imagesedge from the table makes both cases fail, and restoring it makesthem pass, so they are not vacuous.
Not verified
.claude/rules/ci-control.md):the proving check is the hosted run on this exact PR head, specifically the Smart CI / Planner
Self-Test lane. Everything above is a local run and is additive only. The hosted result on this PR
is the R4 proof.
skipping or gating to verify yet. The verdict is not consumed anywhere; CI10-2 is where a real
nightly first reads it.
least one week including one weekend of the coordinator logging its verdict beside the
unconditional nightlies before it gates anything, and that observation has not started.
gh run listand is cited asgiven; it was not re-run in this worktree.
needs:skip semantics behind the closure are GitHub's documented rule, not something thisslice observed at runtime: nothing here schedules a job, so no real nightly has demonstrated a
dependent being skipped behind a skipped
backend-solution. The dependency edges themselves wereread directly from the two workflow files and are asserted by a test.
scripts/ci/smart-ci/changed.Risk notes
of a defect in it is zero until CI10-2 wires it up.
suites would let a real regression through on an affected night. It is mitigated by conservatism
(groups select supersets), by every ambiguous or unrecognised case escalating to the full sweep,
and by the enumeration test that fails when a policy group has no mapping. It is not eliminated:
the table is a judgement about which suites can produce new evidence for a given surface, and
CI10-2 should review it again when it moves into the policy where the digest covers it.
ci/policy.v1.jsonfor now, so a policy change and a table change are twocommits rather than one. The enumeration test is what keeps them in step until CI10-2 merges them.
matchGroups()is reused rather than reimplemented, so the coordinator inherits the planner's globsemantics exactly, including any future fix to them.
and is not merged by the authoring session.
Refs #2334
Refs #2324