mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-13 12:36:15 -08:00
Adds contract-focused regression coverage for the Electron HTTP server, remote-control events, settings events, and managed download paths, and makes Tier A coverage fail closed when instrumentation fails or a runtime-owning production file disappears from a project or from the merged Istanbul report. The old `coverage:ci` exited 0 despite a `Failed to collect coverage` diagnostic: libs/m3u-state/src/lib/effects.ts was simply absent from the merged map. All 30 Tier A reports are now required, the merged map covers 710 files, and effects.ts is reported as 0/159 instead of silently disappearing. Also fixes remote static-file path containment for encoded, malformed, NUL, POSIX and Win32-style traversal inputs, with behavior-preserving testability seams. Statements 69.27% -> 69.54%; http-server.ts 0% -> 90.21%, remote-control.events.ts 0% -> 96.55%, settings.events.ts 59.25% -> 96.29%.
448 lines
20 KiB
Markdown
448 lines
20 KiB
Markdown
# Coverage Integrity And Electron Runtime Boundaries Design
|
|
|
|
## Status
|
|
|
|
Approved in the delegated coverage-audit task on 2026-07-25.
|
|
|
|
Before recording the final ratchets, the implementation branch was rebased
|
|
onto fresh `origin/master` commit
|
|
`7e8c2ccce16b71d72a64c45f1d96dd090dc8e077`.
|
|
|
|
## Goal
|
|
|
|
Increase confidence in security-sensitive and runtime-sensitive Electron
|
|
behavior while making the merged Tier A coverage report fail closed when Jest
|
|
cannot instrument executable production source.
|
|
|
|
This is a risk-weighted first change. It does not try to maximize a global
|
|
percentage by testing type-only modules, re-export shims, or low-risk code.
|
|
|
|
## Fresh Baseline
|
|
|
|
An uncached `pnpm run coverage:ci` on
|
|
`7e8c2ccce16b71d72a64c45f1d96dd090dc8e077` completed successfully and reported:
|
|
|
|
| Metric | Covered / total | Percent |
|
|
| ---------- | --------------: | ------: |
|
|
| Statements | 24,982 / 36,062 | 69.27% |
|
|
| Branches | 17,465 / 29,637 | 58.92% |
|
|
| Functions | 5,785 / 8,577 | 67.44% |
|
|
| Lines | 24,323 / 34,959 | 69.57% |
|
|
|
|
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 (`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 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 / 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%) |
|
|
| `apps/electron-backend/src/app/events/settings.events.ts` | 16 / 27 (59.25%) |
|
|
| `libs/workspace/dashboard/feature/src/lib/rails/workspace-dashboard-rails.component.ts` | 0 / 206 (0%) |
|
|
| `libs/portal/stalker/feature/src/lib/stalker-search/stalker-search.component.ts` | 0 / 166 (0%) |
|
|
| `libs/m3u-state/src/lib/effects.ts` | absent |
|
|
|
|
The original audit is therefore partly stale: `downloads.events.ts` gained a
|
|
meaningful regression suite on master, and `settings.events.ts` already had
|
|
logging coverage. Both remain in this PR because their uncovered IPC
|
|
contracts are part of the approved Electron boundary, but new tests must cover
|
|
missing behavior rather than repeat existing assertions.
|
|
|
|
## Scope
|
|
|
|
### Included
|
|
|
|
- Add behavioral coverage for `HttpServer` routing, static serving, traversal
|
|
containment, MIME responses, and lifecycle.
|
|
- Add behavioral coverage for the remote-control HTTP and IPC boundary:
|
|
method checks, renderer commands, status, channel-number validation,
|
|
malformed JSON, and the 10 KiB body limit.
|
|
- Extend settings IPC coverage for argument normalization, defined-only
|
|
persistence, frame-copy boolean normalization, remote-control fallback, and
|
|
runtime server reconciliation while preserving redacted logging.
|
|
- Extend downloads IPC coverage around the managed-file authorization boundary
|
|
before revealing or opening local paths. Preserve the existing destructive
|
|
cleanup and pause/resume regression tests.
|
|
- Fix the `m3u-state` spec compiler configuration so `effects.ts` can be
|
|
instrumented and appears in its report.
|
|
- Add a fail-closed integrity gate for instrumentation errors, missing Tier A
|
|
reports, missing executable source files, merged metric regressions, and
|
|
regressions in the selected critical Electron files.
|
|
- Document the resulting coverage workflow in
|
|
`docs/architecture/validation-map.md`.
|
|
|
|
### Deferred
|
|
|
|
- `database.worker.ts`: its 950-line worker-thread dispatcher and database
|
|
operation surface deserve a dedicated worker-contract design and PR.
|
|
- `workspace-dashboard-rails.component.ts`: the existing same-name spec tests
|
|
extracted helpers but never instantiates the 660-line component. Component
|
|
coverage belongs in a focused Angular dashboard change.
|
|
- `stalker-search.component.ts`: the 510-line search surface and portal store
|
|
interactions belong in a focused Stalker search change.
|
|
- Broadly raising every current 0% Tier A file.
|
|
- Adding tests to type-only/shared interfaces or pure re-export modules to
|
|
inflate aggregate metrics.
|
|
- Unrelated production behavior changes.
|
|
|
|
## Coverage Integrity Architecture
|
|
|
|
### Shared integrity module
|
|
|
|
Create a pure Node module under `tools/coverage/` that owns:
|
|
|
|
- source-file exclusion rules;
|
|
- TypeScript AST classification of runtime-owning files;
|
|
- coverage path normalization;
|
|
- project- and merged-report completeness checks;
|
|
- instrumentation-error marker detection;
|
|
- merged and critical-file ratchet evaluation; and
|
|
- diagnostic formatting.
|
|
|
|
The runner, merge script, health script, and Node unit tests use this module so
|
|
the definition of an expected executable file does not drift between stages.
|
|
|
|
### Runtime-owning source classification
|
|
|
|
For each Tier A `sourceRoot`, recursively consider production `.ts` files.
|
|
Exclude:
|
|
|
|
- `*.spec.ts` and `*.test.ts`;
|
|
- `*.d.ts`;
|
|
- `test-setup.ts`, test stubs, and generated files;
|
|
- environment files; and
|
|
- `index.ts`, matching the existing Jest collection exclusions.
|
|
|
|
Parse each candidate with the TypeScript compiler API. A file is expected in
|
|
coverage when it has at least one top-level runtime-owning statement.
|
|
Import declarations, interfaces, type aliases, pure export declarations,
|
|
empty statements, and ambient `declare` statements do not make a file
|
|
runtime-owning by themselves. Classes, functions, variables, enums,
|
|
expressions, control flow, and other emitted statements do.
|
|
|
|
This rule intentionally checks executable ownership rather than all TypeScript
|
|
text. It excludes type-only contracts and re-export shims without maintaining
|
|
a large hand-authored manifest. A discovery prototype over the fresh baseline
|
|
classified 591 of 655 Tier A TypeScript source files as runtime-owning and
|
|
identified exactly one absent file: `libs/m3u-state/src/lib/effects.ts`.
|
|
|
|
Coverage keys and expected source paths are normalized to absolute POSIX-style
|
|
paths before comparison so diagnostics are stable across supported CI
|
|
platforms.
|
|
|
|
### Runner behavior
|
|
|
|
`run-tier-a-coverage.mjs` must continue relaying each child process's stdout and
|
|
stderr as it arrives. It also scans a bounded rolling text window, with ANSI
|
|
escapes removed, for Jest's `Failed to collect coverage` marker.
|
|
|
|
After each project command finishes, the runner must fail if:
|
|
|
|
- the Nx/Jest command exits nonzero or terminates by signal;
|
|
- the output contains an instrumentation failure marker;
|
|
- `coverage/<project-root>/coverage-final.json` is missing or invalid; or
|
|
- a runtime-owning file under that project's source root is absent from the
|
|
report.
|
|
|
|
The implementation should use asynchronous child-process streams rather than
|
|
buffering the full multi-project Jest output in memory.
|
|
|
|
### Merge behavior
|
|
|
|
`merge-coverage.mjs` must require exactly one valid report for every configured
|
|
Tier A project. It must list all missing or invalid reports and exit before
|
|
deleting or rewriting `coverage/merged`.
|
|
|
|
The current `.filter(existsSync)` behavior is removed because it silently
|
|
merges partial input.
|
|
|
|
### Health behavior
|
|
|
|
`coverage-health.mjs --require-report` independently reruns:
|
|
|
|
- per-project report existence and JSON validity;
|
|
- runtime-owning source completeness;
|
|
- merged-report completeness and usable instrumentation for every
|
|
runtime-owning Tier A file;
|
|
- merged metric ratchets; and
|
|
- selected critical-file ratchets.
|
|
|
|
It computes the aggregate summary from that validated merged map before
|
|
checking the serialized summary and ratchets. This defense in depth catches
|
|
stale, incomplete, or manually merged reports even when the runner was not the
|
|
process that generated them. Local health checks without `--require-report`
|
|
retain their current warning behavior when report artifacts are not available,
|
|
but any present invalid report remains an error.
|
|
|
|
### Ratchets
|
|
|
|
The policy stores four merged minimum percentages and per-file statement
|
|
minimums for:
|
|
|
|
- `http-server.ts`;
|
|
- `remote-control.events.ts`;
|
|
- `settings.events.ts`; and
|
|
- `downloads.events.ts`.
|
|
|
|
The implementation begins with the fresh master aggregate baseline shown
|
|
above. After the approved behavioral suites and the `effects.ts`
|
|
instrumentation fix are complete, the policy is updated to the reproducible
|
|
post-change values. Those final values become the ratchet: they may stay level
|
|
or rise in future changes, but must not be lowered merely to make CI green.
|
|
|
|
Per-file minimums are likewise recorded from the achieved behavioral suites,
|
|
not from an arbitrary aspirational percentage. The two currently uncovered
|
|
files must become nonzero; the existing `settings` and `downloads` coverage
|
|
must not regress. A reviewed production source shrink is the sole exception to
|
|
the normally monotonic absolute `minimumCovered`: a PR may lower it only when
|
|
it identifies the removed executable statements and a fresh full report proves
|
|
that the file's `minimumPercent`, all aggregate ratchets, and coverage of the
|
|
remaining behavior stay level or rise.
|
|
|
|
## Behavioral Test Design
|
|
|
|
### HTTP server
|
|
|
|
Use a temporary static directory and a real loopback Node HTTP server. Verify:
|
|
|
|
- `/` serves `index.html`;
|
|
- known static assets return their content and correct MIME type;
|
|
- an unknown client-side route falls back to `index.html`;
|
|
- a missing `index.html` returns a plain-text 404;
|
|
- registered remote-control API routes bypass static serving;
|
|
- unknown remote-control API routes return JSON 404;
|
|
- normalized traversal input cannot escape the configured static root;
|
|
- `start`, duplicate `start`, `stop`, enable/disable, and port-change restart
|
|
preserve the current lifecycle contract.
|
|
|
|
Tests should prefer public behavior over direct calls to private methods. If
|
|
an ephemeral port or temporary static root is not observable through the
|
|
current API, introduce only the smallest constructor dependency seam required
|
|
to use real Node HTTP and filesystem behavior. The production singleton keeps
|
|
identical defaults.
|
|
|
|
### Remote control events
|
|
|
|
Capture the HTTP handlers registered during bootstrap and exercise them with
|
|
real readable request streams plus response-contract assertions. Verify:
|
|
|
|
- bootstrap registers every documented endpoint and starts the server only
|
|
when stored settings enable it;
|
|
- channel up/down and volume commands accept only their documented HTTP
|
|
methods and send the expected renderer event;
|
|
- channel selection accepts finite positive values, floors fractional input,
|
|
and rejects missing, non-finite, or sub-one values;
|
|
- status updates merge partial IPC state and refresh `updatedAt`;
|
|
- status reads return the latest merged state;
|
|
- malformed JSON returns 400 without dispatching a command;
|
|
- payloads over 10 KiB return 413, destroy the request, and do not dispatch;
|
|
and
|
|
- missing BrowserWindows are contained without throwing.
|
|
|
|
The existing Electron E2E remote-control suite remains the end-to-end check for
|
|
real volume commands and renderer status.
|
|
|
|
### Settings events
|
|
|
|
Extend the existing same-name spec. Use the real external-player argument
|
|
normalizer and mocked persistent store/server boundaries to verify:
|
|
|
|
- MPV and VLC arguments are normalized before persistence;
|
|
- undefined values are not written;
|
|
- reuse flags preserve explicit false values;
|
|
- frame-copy persistence coerces to boolean;
|
|
- a partial remote-control update reads the missing half from the store;
|
|
- only explicitly supplied remote-control fields are persisted; and
|
|
- `httpServer.updateSettings` receives the resolved enabled/port pair.
|
|
|
|
Retain the existing assertions that logging redacts TMDB and player-argument
|
|
credentials.
|
|
|
|
### Downloads events
|
|
|
|
Extend the current event suite rather than replacing its destructive cleanup
|
|
coverage. Verify the local-file boundary:
|
|
|
|
- a path not present in the downloads database is never revealed or opened,
|
|
even when it exists on disk;
|
|
- a database-managed path missing on disk is never revealed or opened;
|
|
- only a database-managed path that exists reaches
|
|
`shell.showItemInFolder` or `shell.openPath`; and
|
|
- a database lookup failure returns the existing structured not-found response
|
|
and never reaches the shell.
|
|
|
|
Folder authorization internals remain covered by
|
|
`download-directory-authorization.spec.ts`; this event suite tests only the
|
|
IPC-to-authorizer and managed-path integration where it adds distinct signal.
|
|
|
|
## TDD Sequence
|
|
|
|
1. Add failing Node tests for runtime classification, ANSI instrumentation
|
|
marker detection, missing/invalid reports, partial merge input, aggregate
|
|
ratchets, and critical-file ratchets.
|
|
2. Implement the shared integrity module and wire the runner, merge, and health
|
|
scripts until those tests pass.
|
|
3. Enable the selected critical-file policy; confirm fresh targeted coverage
|
|
fails because `http-server.ts` and `remote-control.events.ts` are 0%.
|
|
4. For each Electron boundary, add one contract test at a time, observe the
|
|
intended failure, and make only the smallest testability refactor needed to
|
|
pass it.
|
|
5. Add a failing integrity assertion for the absent `effects.ts`, align the
|
|
`m3u-state` spec compiler configuration, and confirm the real file appears.
|
|
6. If a behavioral test exposes a production defect, add a direct regression
|
|
assertion first and make a separately justified minimal fix. Otherwise
|
|
preserve production behavior.
|
|
7. Record fresh aggregate and per-file post-change ratchets only after all
|
|
behavioral suites are green.
|
|
|
|
## Error Handling And Diagnostics
|
|
|
|
Integrity failures must identify:
|
|
|
|
- the project;
|
|
- the expected report path;
|
|
- each missing runtime-owning source path;
|
|
- the metric or critical file that fell below its ratchet; and
|
|
- the observed and required values.
|
|
|
|
The runner must preserve the original Nx/Jest output and child exit semantics.
|
|
Integrity diagnostics are additive and must not hide the underlying
|
|
instrumentation error.
|
|
|
|
Tests use temporary directories and deterministic synthetic coverage maps.
|
|
They must not depend on a previously generated workspace `coverage/` tree.
|
|
|
|
## Success Criteria
|
|
|
|
- Fresh `pnpm run coverage:ci` exits nonzero for synthetic or real
|
|
instrumentation collection failures.
|
|
- All 30 Tier A project reports are mandatory merge inputs.
|
|
- Every runtime-owning Tier A TypeScript source file is present in its project
|
|
report.
|
|
- `libs/m3u-state/src/lib/effects.ts` is present in coverage.
|
|
- The four selected Electron boundary files meet their recorded behavioral
|
|
statement ratchets; `http-server.ts` and `remote-control.events.ts` are no
|
|
longer 0%.
|
|
- Final merged statements, branches, functions, and lines are each at least
|
|
the fresh master baseline and become the new ratchet.
|
|
- Assertions verify observable contracts rather than merely invoking mocks for
|
|
line execution.
|
|
- No untested production behavior change or unrelated source change is
|
|
included.
|
|
- 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,204 / 36,241 | 69.54% |
|
|
| Branches | 17,573 / 29,748 | 59.07% |
|
|
| Functions | 5,824 / 8,635 | 67.44% |
|
|
| Lines | 24,543 / 35,136 | 69.85% |
|
|
|
|
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.
|
|
Strict health validation also rechecks the merged map itself, so a
|
|
runtime-owning file cannot disappear between valid project reports and the
|
|
merged artifact while permissive percentages still pass.
|
|
|
|
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:
|
|
|
|
```bash
|
|
node --test tools/coverage/*.test.mjs
|
|
pnpm nx test electron-backend --skip-nx-cache --runInBand
|
|
pnpm nx test m3u-state --skip-nx-cache --runInBand
|
|
pnpm run coverage:unit:ci -- --projects=electron-backend,m3u-state
|
|
pnpm nx lint electron-backend --skip-nx-cache
|
|
pnpm nx lint m3u-state --skip-nx-cache
|
|
pnpm run typecheck:backend
|
|
pnpm nx run electron-backend-e2e:e2e-ci--src/downloads.e2e.ts
|
|
pnpm nx run electron-backend-e2e:e2e-ci--src/remote-control.e2e.ts
|
|
pnpm nx run electron-backend-e2e:e2e-ci--src/settings.e2e.ts
|
|
pnpm nx build electron-backend --configuration=production --skip-nx-cache
|
|
pnpm run coverage:ci
|
|
```
|
|
|
|
The final report records exact command results, aggregate before/after metrics,
|
|
and before/after metrics for the selected critical files. If an environment
|
|
prevents an Electron E2E or production build, report the exact blocker and run
|
|
the strongest available lower-level contract validation; do not silently omit
|
|
it.
|
|
|
|
## Documentation Impact
|
|
|
|
Update `docs/architecture/validation-map.md` to make the fail-closed coverage
|
|
contract and ratchet maintenance workflow canonical.
|
|
|
|
`AGENTS.md` and `CLAUDE.md` do not currently describe coverage commands or
|
|
integrity behavior, so no update is expected unless implementation changes a
|
|
process they do describe.
|
|
|
|
## Draft PR Handoff
|
|
|
|
The draft PR body will include:
|
|
|
|
- fresh master baseline and post-change aggregate metrics;
|
|
- selected-file before/after metrics;
|
|
- behavioral contracts added;
|
|
- any production-only testability refactor and why it preserves behavior;
|
|
- coverage integrity and ratchet changes;
|
|
- all validation commands and results;
|
|
- the documentation decision; and
|
|
- deferred 0%-coverage follow-ups.
|