ci(coverage): ratchet runtime boundaries

This commit is contained in:
4gray committed 2026-07-26 02:30:40 +02:00
1 parent 6eb58760fa
commit 5fdf177643
5 files changed
+177 -35

No files matched your search

+7
View File
@@ -0,0 +1,7 @@
---
type: fix
area: electron
---
The desktop remote-control server now blocks crafted static paths from escaping
bundled web files on Windows.
+20 -5
View File
@@ -55,11 +55,25 @@ Docs-only changes (Markdown, `docs/`, `.plans/`, `.codex/`, `.claude/`) and
PRs no E2E validation runs in CI, which is intentional: they cannot affect app
behavior.
| Tier | Rule | Validation |
| --- | --- | --- |
| A | Product/runtime Angular, Electron, backend, data-access, portal, playlist, workspace, playback, EPG, and shared UI code collects source coverage. | `pnpm run coverage:ci` |
| B | Validate behavior without percentage coverage, such as `website`, `packaging`, and Playwright E2E projects. | `pnpm nx test website`, `pnpm nx test packaging`, or the closest E2E target |
| C | Excluded from the source coverage baseline, such as mock servers, test helper libraries, and untested feature shells. | Validate through dependent flows, or add focused tests when changing behavior directly |
Tier A coverage is fail-closed. `coverage:unit:ci` relays Jest output but exits
nonzero on a `Failed to collect coverage` marker, a missing or invalid project
report, or a runtime-owning production TypeScript file absent from that report.
`coverage:merge` requires every configured Tier A report before replacing the
merged output. `coverage:health --require-report` independently repeats report
completeness checks plus aggregate and selected critical-file ratchets.
Runtime-owning files are discovered from the TypeScript AST. Specs,
declarations, test setup and stubs, generated and environment files, `index.ts`,
type-only files, and pure re-export shims are excluded. Ratchets live under
`reporting.coverageRatchet` in `tools/coverage/coverage-policy.json`; update
them only from a fresh full `coverage:ci` report when every value stays level
or rises, and never lower one to accept a regression.
| Tier | Rule | Validation |
| ---- | ------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------- |
| A | Product/runtime Angular, Electron, backend, data-access, portal, playlist, workspace, playback, EPG, and shared UI code collects source coverage. | `pnpm run coverage:ci` |
| B | Validate behavior without percentage coverage, such as `website`, `packaging`, and Playwright E2E projects. | `pnpm nx test website`, `pnpm nx test packaging`, or the closest E2E target |
| C | Excluded from the source coverage baseline, such as mock servers, test helper libraries, and untested feature shells. | Validate through dependent flows, or add focused tests when changing behavior directly |
`apps/website` is an Astro marketing site. Its useful signal is a successful
static build plus targeted output checks, not a merged code coverage percentage.
@@ -69,6 +83,7 @@ Projects with a test target but no specs, such as `remote-control-web` and
For local coverage inspection:
```bash
pnpm run coverage:tools:test
pnpm run coverage:unit:ci
pnpm run coverage:merge
pnpm run coverage:health
@@ -4,10 +4,16 @@
**Goal:** Make Tier A coverage fail closed and add meaningful regression coverage for the HTTP server, remote control, settings, and download-file Electron boundaries.
**Architecture:** A shared pure Node module classifies runtime-owning TypeScript, validates per-project Istanbul reports, scans child output for collection failures, and evaluates aggregate/per-file ratchets. Existing Electron modules keep their public behavior; tests use real loopback/filesystem behavior where practical and mock only process, persistence, or native-shell boundaries.
**Architecture:** A shared pure Node module classifies runtime-owning TypeScript, validates per-project Istanbul reports, scans child output for collection failures, and evaluates aggregate/per-file ratchets. Existing Electron contracts stay unchanged except for the TDD-backed rejection of a discovered static-path traversal escape; tests use real loopback/filesystem behavior where practical and mock only process, persistence, or native-shell boundaries.
**Tech Stack:** Node.js ESM and `node:test`, TypeScript compiler API, Istanbul coverage maps, Jest/ts-jest, Electron main-process IPC, Node HTTP streams, Angular Jest configuration, Nx, Playwright.
**Execution note (2026-07-26):** The branch was rebased onto
`origin/master` commit `a5bb8dc25ca1284320d93f14e17de58687984602`
before the final ratchet run. A real-loopback HTTP regression exposed a Windows
double-leading-slash traversal escape; the implementation adds a tested pure
resolver and a user-facing Electron fix note.
---
## Scope And File Map
@@ -37,7 +43,8 @@ Modify:
- `libs/m3u-state/tsconfig.spec.json` — use bundler resolution and include the
shared Electron window declaration so `effects.ts` instruments.
- `apps/electron-backend/src/app/server/http-server.ts` — optional static-root
and server-factory seam; accept port `0` for loopback tests.
and server-factory seam, port `0` support for loopback tests, and a tested
static-root containment resolver.
- `apps/electron-backend/src/app/events/remote-control.events.ts` — export the
class for isolated instances while preserving the default singleton.
- `apps/electron-backend/src/app/events/settings.events.spec.ts` — persistence,
@@ -1615,6 +1622,10 @@ Expected: the provisional policy remains the only unstaged source change.
`docs/architecture/validation-map.md`
- Modify:
`docs/superpowers/specs/2026-07-25-coverage-integrity-runtime-boundaries-design.md`
- Modify:
`docs/superpowers/plans/2026-07-25-coverage-integrity-runtime-boundaries.md`
- Create:
`.changes/electron-remote-static-paths.md`
- [ ] **Step 1: Generate a complete fresh candidate report**
@@ -1728,19 +1739,32 @@ Also include `pnpm run coverage:tools:test` in the local coverage command block.
- [ ] **Step 5: Confirm documentation and release-note decisions**
Because the real-loopback regression exposed and fixed an observable Windows
static-path traversal escape, add `.changes/electron-remote-static-paths.md`:
```markdown
---
type: fix
area: electron
---
The desktop remote-control server now blocks crafted static paths from escaping
bundled web files on Windows.
```
Run:
```bash
pnpm exec prettier --check \
docs/architecture/validation-map.md \
docs/superpowers/specs/2026-07-25-coverage-integrity-runtime-boundaries-design.md
docs/superpowers/specs/2026-07-25-coverage-integrity-runtime-boundaries-design.md \
docs/superpowers/plans/2026-07-25-coverage-integrity-runtime-boundaries.md \
.changes/electron-remote-static-paths.md
pnpm run release:notes:validate
```
Expected: both pass. Do not add `.changes/*.md`: this PR changes tests, CI
integrity, and behavior-preserving testability seams, not user-visible
behavior. Because it touches runtime `.ts`, apply `no-release-note` to the PR
in Task 10.
Expected: both pass. The release note records the narrow user-visible security
fix; do not apply `no-release-note` in Task 10.
- [ ] **Step 6: Commit ratchets and docs**
@@ -1750,12 +1774,14 @@ Run:
git add \
tools/coverage/coverage-policy.json \
docs/architecture/validation-map.md \
docs/superpowers/specs/2026-07-25-coverage-integrity-runtime-boundaries-design.md
docs/superpowers/specs/2026-07-25-coverage-integrity-runtime-boundaries-design.md \
docs/superpowers/plans/2026-07-25-coverage-integrity-runtime-boundaries.md \
.changes/electron-remote-static-paths.md
git commit -m "ci(coverage): ratchet runtime boundaries"
```
Expected: the provisional policy is replaced by achieved values and the
worktree becomes clean.
Expected: the provisional policy is replaced by achieved values, the security
fix has a valid release note, and the worktree becomes clean.
### Task 9: Run Fresh Validation And Audit The Final Diff
@@ -1921,7 +1947,7 @@ The body must contain:
- runner, merge, health, and ratchet behavior;
- exact validation commands/results;
- docs update;
- no release note because behavior is unchanged; and
- the Electron fix release note for Windows static-path containment; and
- deferred `database.worker.ts`, dashboard rails, and Stalker search.
Use:
@@ -1938,17 +1964,17 @@ gh pr create \
Provide the fully populated body on standard input; do not leave metric or
validation placeholders.
- [ ] **Step 5: Apply the internal-only release-note label**
- [ ] **Step 5: Verify release-note metadata**
Run:
```bash
gh pr edit --add-label no-release-note
gh pr view --json url,isDraft,headRefName,baseRefName,labels,commits,statusCheckRollup
gh pr view --json url,isDraft,headRefName,baseRefName,labels,files,commits,statusCheckRollup
```
Expected: the PR is draft, head/base are correct, `no-release-note` is present,
and the command prints the PR URL.
Expected: the PR is draft, head/base are correct,
`.changes/electron-remote-static-paths.md` is included, `no-release-note` is
absent, and the command prints the PR URL.
- [ ] **Step 6: Return the exact handoff**
@@ -1960,5 +1986,6 @@ Return:
- aggregate and selected-file before/after metrics;
- exact command results and any explicit environment-only skip;
- docs updated (`docs/architecture/validation-map.md`);
- release note skipped with `no-release-note`;
- release note added (`.changes/electron-remote-static-paths.md`) with no
`no-release-note` label;
- deferred zero-coverage follow-ups.
@@ -4,8 +4,9 @@
Approved in the delegated coverage-audit task on 2026-07-25.
The implementation branch starts from fresh `origin/master` commit
`4e5132cb` (`ci(release): gate PRs on an authored release note (#1257)`).
Before recording the final ratchets, the implementation branch was rebased
onto fresh `origin/master` commit
`a5bb8dc25ca1284320d93f14e17de58687984602`.
## Goal
@@ -18,31 +19,34 @@ percentage by testing type-only modules, re-export shims, or low-risk code.
## Fresh Baseline
`pnpm run coverage:ci` on `4e5132cb` completed successfully and reported:
An uncached `pnpm run coverage:ci` on
`a5bb8dc25ca1284320d93f14e17de58687984602` completed successfully and reported:
| Metric | Covered / total | Percent |
| ---------- | --------------: | ------: |
| Statements | 24,684 / 35,843 | 68.86% |
| Branches | 17,290 / 29,471 | 58.66% |
| Functions | 5,728 / 8,524 | 67.19% |
| Lines | 24,051 / 34,754 | 69.20% |
| Statements | 24,974 / 36,062 | 69.25% |
| Branches | 17,461 / 29,637 | 58.91% |
| Functions | 5,784 / 8,577 | 67.43% |
| Lines | 24,315 / 34,959 | 69.55% |
The successful command nevertheless printed `Failed to collect coverage` for
`libs/m3u-state/src/lib/effects.ts`. TypeScript could not resolve
`@angular/material/snack-bar`, inferred the injected snack bar as `unknown`,
and did not see the shared `window.electron` declaration under the project's
spec compiler configuration. The file was absent from both the project report
and `coverage/merged/coverage-final.json`.
spec compiler configuration (`TS2307` once, `TS2571` twice, and `TS2551`
once). The file was absent from both the project report and
`coverage/merged/coverage-final.json`.
All 30 Tier A projects produced a `coverage-final.json`, so the current merge
included 30 reports and still missed an executable file. This demonstrates
All 30 Tier A projects produced a `coverage-final.json`, so 31 report files
including the merged report existed. The current merge included all 30 project
reports and 709 files while still missing an executable file. This demonstrates
that project-level report existence is necessary but not sufficient.
The audit file status on the same baseline is:
| File | Statement coverage |
| --------------------------------------------------------------------------------------- | -----------------: |
| `apps/electron-backend/src/app/workers/database.worker.ts` | 0 / 205 (0%) |
| `apps/electron-backend/src/app/workers/database.worker.ts` | 0 / 207 (0%) |
| `apps/electron-backend/src/app/events/remote-control.events.ts` | 0 / 115 (0%) |
| `apps/electron-backend/src/app/server/http-server.ts` | 0 / 73 (0%) |
| `apps/electron-backend/src/app/events/database/downloads.events.ts` | 69 / 147 (46.93%) |
@@ -333,6 +337,57 @@ They must not depend on a previously generated workspace `coverage/` tree.
- Deferred database worker, dashboard, and Stalker search coverage is listed
in the draft PR.
## Implementation Outcome
The fresh uncached implementation run merged all 30 required Tier A reports
into the 31st `coverage-final.json` and included 710 files, with no
collection-failure marker or missing runtime-owning source. The achieved
ratchets are:
| Metric | Covered / total | Percent |
| ---------- | --------------: | ------: |
| Statements | 25,196 / 36,241 | 69.52% |
| Branches | 17,569 / 29,748 | 59.05% |
| Functions | 5,823 / 8,635 | 67.43% |
| Lines | 24,535 / 35,136 | 69.82% |
The selected Electron boundary statement coverage changed as follows:
| File | Baseline | After |
| -------------------------- | ----------------: | -----------------: |
| `http-server.ts` | 0 / 73 (0%) | 83 / 92 (90.21%) |
| `remote-control.events.ts` | 0 / 115 (0%) | 112 / 116 (96.55%) |
| `settings.events.ts` | 16 / 27 (59.25%) | 26 / 27 (96.29%) |
| `downloads.events.ts` | 69 / 147 (46.93%) | 86 / 147 (58.50%) |
The integrity correction makes the denominator honest:
`libs/m3u-state/src/lib/effects.ts` is now present at 0 / 159 statements
instead of silently disappearing. The aggregate improvement therefore holds
even after surfacing those 159 previously unreported uncovered statements.
Two production seams were sufficient for isolated contract tests:
`HttpServer` accepts an optional server factory and static distribution path,
and `RemoteControlEvents` is a named export while the production singleton is
unchanged. Real loopback HTTP tests also exposed a Windows
double-leading-slash traversal escape in the prior static-path normalization.
A direct failing regression preceded the narrow fix: production now uses the
pure, platform-testable `resolveStaticFilePath` resolver to decode once, reject
malformed or NUL paths, strip leading separators, resolve against the static
root, and enforce containment. This is a tested behavior and security fix, not
a behavior-preserving refactor.
The release decision follows that observable fix:
`.changes/electron-remote-static-paths.md` records it as an Electron bug fix,
so the draft PR must not carry `no-release-note`. The canonical coverage
workflow is documented in `docs/architecture/validation-map.md`; `AGENTS.md`
and `CLAUDE.md` do not describe the changed coverage contract or HTTP path
logic and require no update.
The deferred production files remain uncovered in this change:
`database.worker.ts` at 0 / 207, `workspace-dashboard-rails.component.ts` at
0 / 206, and `stalker-search.component.ts` at 0 / 166. They retain the
separate follow-up scopes described above.
## Validation Ladder
Run fresh, uncached validation in this order:
+39 -1
View File
@@ -3,7 +3,45 @@
"reporting": {
"mergedCoverageDir": "coverage/merged",
"e2eSummaryDir": "coverage/e2e",
"unitCodecovFlag": "unit"
"unitCodecovFlag": "unit",
"coverageRatchet": {
"merged": {
"statements": 69.52,
"branches": 59.05,
"functions": 67.43,
"lines": 69.82
},
"criticalFiles": [
{
"path": "apps/electron-backend/src/app/server/http-server.ts",
"statements": {
"minimumCovered": 83,
"minimumPercent": 90.21
}
},
{
"path": "apps/electron-backend/src/app/events/remote-control.events.ts",
"statements": {
"minimumCovered": 112,
"minimumPercent": 96.55
}
},
{
"path": "apps/electron-backend/src/app/events/settings.events.ts",
"statements": {
"minimumCovered": 26,
"minimumPercent": 96.29
}
},
{
"path": "apps/electron-backend/src/app/events/database/downloads.events.ts",
"statements": {
"minimumCovered": 86,
"minimumPercent": 58.5
}
}
]
}
},
"unitCoverage": {
"description": "Tier A projects collect and trend source coverage. Tier B projects are validated without percentage coverage. Tier C projects are excluded from the source coverage baseline.",