fix: make npm run ci a true superset of GitHub CI (coverage gate)

[cliffhall]

Closes #1619

The local npm run ci script was not at parity with GitHub CI. The workflow .github/workflows/main.yml runs validate → coverage → smoke → test:storybook, but the root ci script ran validate → ci:integration → smoke → ci:storybook — omitting npm run coverage, the per-file ≥90% (lines/statements/functions/branches) coverage gate that CI enforces. As a result a push could pass every documented local pre-push command and still fail CI on the coverage gate. This exact gap broke PR #1601 (HistoryEntry.tsx at 85.71% function coverage).

Changes

1. Root package.json ci script now chains npm run coverage in place of ci:integration: npm run validate && npm run coverage && npm run smoke && npm run ci:storybook. Since web's coverage:web (test:coverage) already runs the integration project under v8 instrumentation, coverage supersedes the old integration-only step and makes npm run ci a genuine superset of GitHub CI. The ci:integration / ci:storybook script definitions are left intact for any other references.

2. AGENTS.md — renamed the pre-push section to "Mandatory pre-push gate", removed the "or at minimum npm run validate" loophole and the soft "optionally also run coverage" phrasing (validate runs test, not test:coverage, so it does zero coverage gating), and corrected the parity description so npm run ci is documented as validate → coverage → smoke → storybook.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 52s —— View job

---

Review: npm run ci coverage-gate parity

I verified the change against .github/workflows/main.yml. This is a small, correct, and well-motivated fix. ✅

Parity is now exact. The CI workflow runs, in order:

CI step (main.yml)	Local ci after this PR
npm run validate (L34)	npm run validate
npm run coverage (L48)	npm run coverage ⬅️ new
npm run smoke (L57)	npm run smoke
Storybook play tests (L69–71)	npm run ci:storybook

ci:storybook runs npx playwright install chromium && npm run test:storybook, matching the workflow's two Playwright steps. So npm run ci is now a genuine superset of GitHub CI, and the coverage step (via coverage:web → test:coverage, which runs --project=unit --project=integration) subsumes the old integration-only step — confirming the claim that dropping ci:integration from the chain loses no coverage. Good catch on the root cause; this closes the exact gap that let PR #1601 pass locally and fail CI.

Minor observations (non-blocking):

1. ci:integration (package.json:41) is now dead. After this change nothing references it — I grepped the repo and its only occurrence is its own definition. The PR body says it's "left intact for any other references," but there are none. Keeping it is harmless, but a truly-orphaned script tends to rot (e.g. it still points at test:integration, which is no longer part of any CI/ci path). Consider either removing it, or leaving a one-line comment noting it's a manual-only convenience alias so a future reader doesn't assume it's wired into ci. Fix this →

2. AGENTS.md is accurate and internally consistent with the new chain — the "Mandatory pre-push gate" rename, the removal of the "or at minimum npm run validate" loophole, and the "validate runs test, not test:coverage" clarification all correctly describe the behavior. One small redundancy: the "never treat it as optional before a push" bullet (the nested coverage line) now largely repeats the point already made in the top-level ci bullet, but that's stylistic, not wrong.

Verdict: LGTM. The functional change (package.json:40) is correct and the docs match. The only thing worth cleaning up is the now-dead ci:integration alias, and that's optional.

---

• Read the PR diff
• Analyze package.json script changes vs. .github/workflows/main.yml
• Review AGENTS.md doc changes for accuracy
• Post review feedback
  • 1619-ci-coverage-parity

[cliffhall]

Thanks for the review! Addressed the actionable item:

1. Orphaned ci:integration (package.json:41) — Removed it (812bdb0). Confirmed via grep that its only occurrence was its own definition. package.json can't carry a clarifying comment (JSON), so removal is the cleaner of the two options you offered. The underlying clients/web test:integration script is untouched and still available for direct manual integration runs; it's just no longer wrapped by a root alias that nothing calls.

2. AGENTS.md "coverage" bullet redundancy — Leaving as-is. The top-level ci bullet frames coverage as part of the mandatory gate; the nested bullet under the validate-decomposition explains why validate alone is insufficient (it runs test, not test:coverage). Slight overlap, but they serve different readers at different points in the doc, and you flagged it as stylistic/non-blocking.

Also confirmed the earlier npm run ci failure I hit was a pre-existing flaky TUI OAuth waitForFrame test on v2/main (passes without --coverage and on retry), not related to this change — full ci went green on re-run.

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 36s —— View job

---

Re-review: npm run ci coverage-gate parity

I re-verified the current branch state against .github/workflows/main.yml. The follow-up commit (812bdb0c) that removed the orphaned ci:integration alias is in place, and the fix is correct. ✅

Parity is exact. The workflow runs four steps in order; local ci now mirrors them:

CI step (main.yml)	Local ci (package.json:40)
npm run validate (fast: format/lint/build/unit)	npm run validate
npm run coverage (≥90 per-file gate; runs --project=unit --project=integration)	npm run coverage ⬅️ new
npm run smoke	npm run smoke
Storybook play tests (playwright install chromium → test:storybook)	npm run ci:storybook

Confirmed coverage (package.json:47) chains all four clients' test:coverage, and web's includes the integration project — so dropping the old ci:integration step loses no coverage. This closes the exact gap that let PR #1601 pass locally and fail CI on a function-coverage regression.

Orphaned alias cleanup — resolved. grep -rn "ci:integration" across *.json/*.md/*.yml (excluding node_modules) now returns zero hits. The dead alias flagged in my prior review is gone, and clients/web's underlying test:integration script is untouched and still available for direct manual runs. Removal was the right call — JSON can't carry a clarifying comment.

AGENTS.md — the "Mandatory pre-push gate" rewrite is accurate and internally consistent with the new chain: the removed validate-as-substitute loophole, the "validate runs test, not test:coverage → zero coverage gating" clarification, and the validate → coverage → smoke → storybook description all match the actual scripts. The mild redundancy in the nested coverage bullet is stylistic only, as noted before.

Verdict: LGTM. Functional change is correct, the follow-up cleanup addressed the only actionable item from the prior pass, and docs match behavior. Nothing blocking.

---

• Fetch and diff branch against origin/v2/main
• Verify ci chain vs. .github/workflows/main.yml (all four steps)
• Confirm ci:integration fully removed (grep)
• Check AGENTS.md accuracy vs. scripts
• Post review
  • 1619-ci-coverage-parity

[cliffhall]

Thanks for the re-review — glad the cleanup landed cleanly and parity is confirmed exact. No further changes on the stylistic AGENTS.md redundancy, per your non-blocking note. 🎉