ci: run the Node app test suites in the required diagnostics gate

[mdheller]

What this does

The apps/* suites that use the Node built-in test runner (node --test) were never executed by CI. The only workflow that referenced them is images.yml (it builds their containers), and the Dockerfiles run npm install, never npm test. So the health-twin PHI-path exposure / auth-gate / consult-cap tests — and the sibling apps' suites — ran on nobody's PR.

This wires them into the already-required gate.

The protection finding (why placement was the whole problem)

• Classic branch protection on main: 404 (not protected).
• Ruleset main-required-checks (19558691) requires exactly one context: diagnostics-gate. strict_required_status_checks_policy: false.
• Consequence: a brand-new workflow job reports a check but blocks nothing — only diagnostics-gate (an aggregator over its needs:) gates merges. Adding a new job also means editing diagnostics-gate's needs: line and registering the job in tools/ci_diagnostics_gate.py — the exact lines ci: diagnostics-gate must fail closed on an unexplained skip [DO NOT MERGE until probe runs land] #1080 rewrites and a file ci: diagnostics-gate must fail closed on an unexplained skip [DO NOT MERGE until probe runs land] #1080/DO NOT MERGE BEFORE #1080: fail diagnostics-gate on a job declared in the workflow but never wired into needs: #1082 both edit (and tools/ is off-limits here).

So the only non-colliding, actually-blocking route is a matrix leg inside an already-required job. This adds node-app-suites to the existing app-test-diagnostics job (already in diagnostics-gate.needs). Coverage rides the single required context; no new job; the gate and its needs: are untouched.

merge-tree (this branch × contested PRs): both CLEAN.

• HEAD × #1080 → exit 0, auto-merges validate-target-diagnostics.yml, no conflicts.
• HEAD × #1082 → exit 0, auto-merges, no conflicts.
• ci.yml untouched, so no collision with health-twin: a grant id is not a credential — authenticate the holder on the PHI consent path #1081/health-twin: consult the reconciliation verdict on the request path, seal the anti-clamp audit, and give the app CI at all #1089 (both add a top-level health-twin: key).

One leg, all apps, hard timeout

.github/app-ci/run-node-app-tests.sh (kept out of tools/): discovers every apps/* with a node --test script and runs them in one leg (Actions is spend-capped), each under a hard per-app timeout so a hang is RED (exit 124), never a silently-cancelled job. It fails on zero matches and refuses to pass if everything is quarantined.

Sibling inventory (measured locally, direct exit codes)

app	test script	status
health-twin	node --test	PASS (5) — now runs
hellgraph-service	node --test	PASS (94) — now runs
lifecycle-warden	node --test	PASS (27) — now runs
commons-search	node --test	HANG (124) — quarantined
search-gateway	node --test	HANG (124) — quarantined
socioprophet-web	vitest run	other harness — reported, not run here (covered by build-web; wants its own vitest job)
academy-board / portfolio-agent / synapse-bridge	none	no test script

The two hangs (root-caused, not hidden)

commons-search and search-gateway src/server.test.ts: all assertions pass, then the process can't exit. Several tests await fetch(...) (the DELETE calls, the rate-cap loops) without consuming the response body, so undici keeps the HTTP/1.1 keep-alive sockets open and after(() => server.close()) never completes. hellgraph passes because it consumes every body. One-line fix owned by each app lane: server.closeAllConnections() in the after() hook.

They are quarantined in .github/app-ci/quarantine.json — named, with root cause + fix, and shrink-only: the runner asserts len(quarantined) <= ceiling (currently 2) on every run and fails if it grows; stale entries (app/test gone) also fail, forcing pruning. This is a quarantine, not a silent skip.

Proof (local; not pushed to watch CI)

Runner exit codes, direct capture:

1. GREEN baseline → 0 (3 run + pass, 2 quarantined, socioprophet-web reported).
2. Inject a failing test (throwaway app) → 1 (RED).
3. Inject a hanging test, PER_APP_TIMEOUT=5 → 1 via exit 124, whole run bounded at 9s (proves a hang is RED, never a hang).
4. Remove throwaway → GREEN 0 restored, tree clean.
5. Zero-match glob → 1 (refuses to pass on nothing).
6. Grow quarantine past ceiling → 1 (shrink invariant holds).

Out of scope / not touched

apps/health-twin/src/**, gitops-promote.yml, images.yml, infra/**, tools/, ci.yml, Makefile.

DO NOT MERGE pending review.

[Copilot]

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Integrates previously unrun apps/* Node built-in test runner suites (node --test) into the already-required diagnostics-gate by adding a new matrix leg, plus a shared runner script and quarantine file to handle known hangs with enforced limits.

Changes:

• Add a node-app-suites matrix leg to the existing required diagnostics job.
• Introduce a CI script to discover and run all apps/* using node --test with per-app hard timeouts.
• Add a quarantine allowlist JSON for known hanging suites with a ceiling concept.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
.github/workflows/validate-target-diagnostics.yml	Adds the node-app-suites matrix entry and sets up Node for that leg.
.github/app-ci/run-node-app-tests.sh	Discovers apps/* with node --test, applies quarantine rules, runs each suite with a hard timeout.
.github/app-ci/quarantine.json	Defines quarantined apps and a ceiling value used by the runner script.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[mdheller]

Valid and fixed in 5d78d11. You're right that len(apps) <= ceiling within one file can't stop an add-app and raise-ceiling landing in the same diff. Added Invariant 2b: diff against the origin/main baseline (fetched shallow when the checkout lacks it) — the ceiling may not rise and no new app key may appear; a newly-failing suite must be fixed, not quarantined. Proven locally: baseline {ceiling:1, apps:{a}} vs a change to {ceiling:2, apps:{a,b}} now exits RED (ceiling raised 1 -> 2; new quarantined app(s): b). This leg's introduction (no baseline yet) is printed, never silently skipped.

[mdheller]

Valid and fixed in 5d78d11 — this was precisely the silent omission the leg exists to prevent. read_test_script now exits 3 with a reason on an unreadable/unparseable package.json (a valid manifest that simply declares no test still returns empty, as before), and both discovery and the stale-check fail RED on it. Proven: a broken package.json now exits 1 rather than being dropped as 'no tests'.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

.github/workflows/validate-target-diagnostics.yml:204

• The Create virtualenv step appears to run for the node-app-suites leg where it isn’t needed, and with working-directory: . it will create .venv at the repo root. Consider adding an if: matrix.name != 'node-app-suites' guard (or splitting language-specific setup) to avoid unnecessary work and potential cross-leg side effects in this required CI gate.

      - name: Create virtualenv
        working-directory: ${{ matrix.working-directory }}
        run: python3 -m venv .venv
      - name: Setup Node
        if: matrix.name == 'node-app-suites'
        uses: actions/setup-node@v4
        with:
          node-version: '22'

.github/app-ci/run-node-app-tests.sh:79

• The node --test detection pattern is currently redundant and can miss legitimate variants (e.g., multiple spaces like node --test, or other formatting). This can incorrectly classify node --test suites as OTHER_APPS and silently stop running them in this leg. Use a single, clear match such as a regex on node[[:space:]]+--test (or a more robust parse) to reliably detect the built-in runner.

  [ -z "$script" ] && continue
  if [[ "$script" == *"node "*"--test"* || "$script" == *"node --test"* ]]; then
    NODE_APPS+=("$app")
  else
    OTHER_APPS+=("$app  [${script}]")
  fi

.github/app-ci/run-node-app-tests.sh:182

• Quarantine validation checks that the app exists and has a test script, but doesn’t ensure the quarantined app is actually in the node --test discovery set. If an app switches harness (e.g., to vitest) it could remain quarantined forever even though this leg will never run it anyway, undermining the intended shrink pressure and correctness of the quarantine list. Consider additionally asserting that each quarantined app is a member of NODE_APPS (i.e., still a node --test suite), otherwise fail and force pruning + ceiling lowering.

# Invariant 3: no stale quarantine entries.
for qa in "${Q_APPS[@]}"; do
  qpj="apps/$qa/package.json"
  if [ ! -f "$qpj" ]; then
    echo "FATAL: quarantined app '$qa' no longer exists ($qpj is missing)."
    echo "       Remove it from $QUARANTINE_FILE AND lower the ceiling."
    exit 1
  fi
  qscript="$(read_test_script "$qpj")"; qrc=$?
  if [ "$qrc" -ne 0 ]; then
    echo "FATAL: quarantined app '$qa' has an unreadable/unparseable $qpj."
    echo "       A broken quarantined app must be fixed, not waved through."
    exit 1
  fi
  if [ -z "$qscript" ]; then
    echo "FATAL: quarantined app '$qa' no longer declares a test script."
    echo "       Remove it from $QUARANTINE_FILE AND lower the ceiling."
    exit 1
  fi
done

.github/app-ci/run-node-app-tests.sh:133

• The quarantine ratchet is currently 'best-effort' and fails open: if fetching/reading the baseline file fails, BASELINE_JSON becomes empty and the script skips the shrink-only enforcement. Since this leg is intended to mechanically prevent widening quarantine, it’s safer to fail closed once the baseline quarantine exists (e.g., if the baseline ref can’t be retrieved/read, exit non-zero with a clear error) so the ratchet can’t be bypassed by transient git/checkout constraints.

  # Shallow PR checkouts don't carry origin/main; fetch it best-effort before reading.
  if ! git cat-file -e "$BASELINE_REF:.github/app-ci/quarantine.json" 2>/dev/null; then
    git fetch --no-tags --depth=1 origin main >/dev/null 2>&1 || true
  fi
  BASELINE_JSON="$(git show "$BASELINE_REF:.github/app-ci/quarantine.json" 2>/dev/null || true)"
fi