ci: run the TypeScript suite — 401 tests that no workflow executed

[mdheller]

The gap

The TypeScript suite was never executed by any workflow.

• .github/workflows/ci.yml:28 runs cargo test --workspace — Rust only.
• .github/workflows/ts-ci.yml ran npm run typecheck, npm run build, and a ts/dist staleness check. It contained zero references to npm test.

package.json has defined "test": "node --import tsx --test ts/src/*.test.ts" the whole time, and that suite is currently 401 passing. So a TypeScript regression could merge with every required check green.

Typecheck proves the code compiles. It does not prove it works. The 12 new regression tests guarding the two CodeQL fixes in #34 would have been unenforced the moment they merged — tests that exist to stop a security regression, watched by nothing.

The change

One step, in the job that already has node and npm install, under the same if: steps.changed.outputs.ts == 'true' guard as every other step:

- name: Test
  if: steps.changed.outputs.ts == 'true'
  run: npm test

Path-filter behaviour is unchanged — a PR touching no package.json or ts/** still short-circuits to "required check trivially satisfied" without ever setting up node, so this stays a no-op for non-TS PRs. Cost on TS PRs is the ~5s suite on a runner that is already warm.

Placed before Rebuild dist deliberately: a logic failure is then reported as a test failure, rather than surfacing later as a confusing ts/dist diff.

Teeth proven, both directions

I did not want to ship a step that might be decoration, so this branch first carried a deliberately failing assertion:

commit	CI
4ef89e35 — deliberate failing test present	completed failure ✅ red as required
2afaeb17 — failing test removed	completed success ✅ green

If CI had stayed green with that test present, the step would not have been wired and the whole change would have been theatre. The temporary test is gone; only the workflow line remains.

Why this matters beyond the one line

This is the eighth instance this week of the same shape in this estate: a correct artifact with nothing connected to it. A lifecycle FSM with no scheduler, a governance library with no callers, an engine-version guard invoked by nothing (and cited as authority while it had never run), five engine releases merged but never shipped, a tsc red under a green suite, a daily backup that never ran once, a rollback executor with no CLI behind a || true — and now 401 tests no job executed.

In each case the artifact was right, the connection was missing, and every gate that inspected the artifact reported green. The question worth asking before building anything here is "what calls this?"

[mdheller]

Review pass — one real finding, fixed in this branch.

The Test step was added to the right job. build-and-verify-dist is one of only two required contexts on main (gh api repos/SocioProphet/hellgraph/branches/main/protection → ["build-and-verify-dist", "CodeQL"]), so a failing suite genuinely blocks. Putting the tests in a new job would have produced a gate that reports red without gating.

The teeth proof in this branch's history is genuine — verified independently rather than taken on trust. Commit 4ef89e3 (deliberate failing test) → build-and-verify-dist -> failure via the check-runs API. 2afaeb1 removed it.

Finding: the step was skipped on this PR's own head commit. The Detect TS/dist-relevant changes step grepped ^(package\.json|ts/). This PR touches .github/workflows/ts-ci.yml and package-lock.json, neither of which matches, so job 90721215994 reported:

3. Detect TS/dist-relevant changes -> success
4-9. setup-node / Install / Typecheck / Test / Rebuild dist / dist-stale -> skipped
10. No TS/dist changes — required check trivially satisfied -> success

The commit that wires up the test suite had that step skipped, and the PR was green without ever running the tests it adds. Two real holes behind it:

• a dependency bump can land entirely in package-lock.json — the change most likely to break the suite;
• a PR editing only ts-ci.yml skipped everything, so the workflow defining this gate was the one change the gate never ran on.

Widened to ^(package(-lock)?\.json|ts/|\.github/workflows/ts-ci\.yml). The escape hatch the file header describes is preserved deliberately — verified by truth table that Rust-only, README-only and ci.yml-only PRs still resolve to ts=false, so the required context still reports instead of hanging "expected".

Also: the npm test glob ts/src/*.test.ts was checked against the tree — 67 of 67 *.test.ts files match, none live outside ts/src/. And package-lock.json was resynced to the current version (metadata only; no dependency added, removed or re-resolved).

Rebased onto post-#35 main, so this head carries the kko-provenance job.

[mdheller]

@copilot 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.

Adds TypeScript test execution to the existing TS CI workflow so the currently-defined npm test suite (401 tests) is enforced on relevant PRs, closing a CI gap where only typecheck/build ran.

Changes:

• Expand the path filter to include package-lock.json and the workflow file itself so TS CI runs when dependencies or its own gating logic changes.
• Add a npm test step (guarded by the existing steps.changed.outputs.ts == 'true' condition) before rebuilding dist.
• Update the “trivially satisfied” message to reflect the widened path filter.

---

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

[mdheller]

Rejecting — there is nothing to centralize, and the indirection would make the gate harder to audit.

The comment's premise is duplication ("easy to desync from other workflows if similar filters exist"). Checked it:

$ grep -rl "git diff --name-only" .github/workflows/
.github/workflows/ts-ci.yml
(count: 1)

One workflow, one filter, one call site. ci.yml and docker-publish.yml have no changed-files logic at all. There is no second copy to drift from, so extracting it would create the coupling it is meant to prevent rather than remove one.

Two further reasons to keep it inline:

• Auditability. This filter decides whether the required check actually runs. Today that decision is visible in the same file as the steps it gates — which is what let this PR's own defect be spotted (the head commit reported Test -> skipped). Behind a script indirection, reviewing the workflow no longer shows you what the workflow does.
• It is 2 lines, not a subsystem. A checked-in script would need its own tests and its own review discipline to be trustworthy, and would still have to stay in step with the on: push: paths: list directly above it — which a script cannot own, because that list is workflow syntax.

If a second workflow ever needs the same filter, that is the point to extract it, and the second use case will show what the right interface is. Extracting on a sample of one usually guesses wrong.

I did widen the regex in this PR (package(-lock)?\.json|ts/|\.github/workflows/ts-ci\.yml) and verified by truth table that Rust-only, README-only and ci.yml-only PRs still resolve to ts=false, so the required context keeps reporting instead of hanging "expected".

[Copilot]

@copilot review

Reviewed the current head. No issues found.

Test step — placed between Typecheck and Rebuild dist (lines 83–85), gated with steps.changed.outputs.ts == 'true' matching every other step in the job. Ordering is correct: a logic failure surfaces as a test failure rather than a dist diff.

Path filter — ^(package(-lock)?\.json|ts/|\.github/workflows/ts-ci\.yml) covers the two previously open holes: lockfile-only bumps and workflow-self edits. Truth table holds: a Rust-only, README-only, or ci.yml-only PR still resolves to ts=false, so the required context reports without hanging on "expected".

Test file coverage — ts/src/*.test.ts in npm test matches all test files: find ts/src -name "*.test.ts" returns 68, all flat in ts/src/ (zero in subdirectories). The comment says "67 of 67" — one file appears to have been added to the tree since the review was written. Not a code issue; the glob still covers them.

package-lock.json — version field only (0.4.45 → 0.4.46); two lines changed, no dependencies added, removed, or re-resolved.

LGTM.