Skip to content

chore(stryker): register 3 covering unit tests missing from tap.testFiles - #13357

Merged
diegosouzapw merged 4 commits into
cheaper-inference:release/v3.8.51from
patrykkopycinski:pr-fix-test-script-hang
Sep 18, 2026
Merged

diegosouzapw merged 4 commits into
cheaper-inference:release/v3.8.51from
patrykkopycinski:pr-fix-test-script-hang

Conversation

@patrykkopycinski

@patrykkopycinski patrykkopycinski commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

Registers the three unit tests that cover the mutated sources but were absent from
tap.testFiles, which is what Fast Quality Gates was failing on.

$ node scripts/check/check-mutation-test-coverage.mjs --strict   # pristine base f1e7148c1, zero diff
✗ mutation coverage: 3 test files cover mutated sources but are not registered

Added, at their correct sorted positions:

  • tests/unit/combo-identical-error-streak.test.ts
  • tests/unit/13601-header-drop-count-surfaced.test.ts
  • tests/unit/semantic-cache-no-truncated-writes.test.ts

Verified individually green (8 + 3 + 3 = 14/14), and mutation-tested: removing the
three entries turns the gate red again, restoring them turns it green.

Scope reduction — both original fixes were already landed on base

This PR was opened against an older base. Rebasing onto release/v3.8.51 (f1e7148c1)
showed both defects had already been fixed upstream, so both commits were dropped:

  1. npm run test terminating — already fixed by fix(test): cap local unit-test concurrency at 4 to avoid exhausting commit charge #13187 (178d25250). Base's test
    script carries --test-force-exit on both node invocations and already ends with
    && npm run test:unit:serial. Nothing left to fix, and the delegation this PR added
    additionally reversed fix(test): cap local unit-test concurrency at 4 to avoid exhausting commit charge #13187's deliberate --test-concurrency=4 cap to 20 —
    measured on the same tree and suite, 20 produced 356 cancelled tests via
    'Promise resolution is still pending but the event loop has already resolved'
    against 0 at concurrency 4. It also tripped
    tests/unit/test-serial-quarantine.test.ts, which asserts every parallel runner
    script ends with the serial step (base 4/4 pass → branch 3/4).
  2. RAYCAST env-doc sync — moot. ENVIRONMENT.md no longer exists at the repo root
    in base, and check-env-doc-sync reports in-sync on pristine f1e7148c1.

package.json is therefore byte-identical to base; stryker.conf.json +3 is the entire diff.

Remaining red gates are base debt, not this diff

  • API Route Typecheck — src/lib/providerModels/modelDiscovery.ts:355 passes an
    untyped record.metadataProvenance into SyncedAvailableModel, whose
    metadataProvenance requires vertexDocs (required since fix(vertex): discover and route partner models correctly #12471). Reproduced on
    pristine f1e7148c1 with zero diff.
  • Unit Tests fast-path 1/2/3 of 4 — the failing assertions live in 4 test files;
    run on both my head and pristine base: identical 52/55 pass, 3 fail, same test names
    on both trees.

Neither is touched by this diff.

@diegosouzapw

Copy link
Copy Markdown
Collaborator

The npm run test fix is solid and well-measured (the m1max timing comparison is convincing)
— delegating to test:unit also prevents the two from drifting apart again. One thing I
couldn't verify: the diff at your PR head only touches package.json, no changes to
ENVIRONMENT.md/.env.example — was the RAYCAST doc-sync half already fixed elsewhere? Just
flagging so we don't lose track of it if it's still open.

@patrykkopycinski

Copy link
Copy Markdown
Contributor Author

Investigated — the RAYCAST doc-sync half is already fixed elsewhere, no further diff needed in this PR.

Evidence, checked against origin/release/v3.8.51 tip c3945a724:

  • docs/reference/ENVIRONMENT.md (the file moved from root ENVIRONMENT.md, which no longer exists at tip) — grep -in raycast → no matches.
  • .env.example — grep -in raycast → no matches (confirmed the file has no RAYCAST_* rows, commented or otherwise; lines around the old 1263-1266 range are now unrelated VISION_BRIDGE_* docs).

Landing commit: 0ad03316e834717c043177e0399137ecfceded31 — chore(providers): retire GPL-derived Raycast and Hailuo integrations (#11691), merged 2026-08-28T08:16:59Z. That PR's diff includes:

.env.example                    | 11 -
docs/reference/ENVIRONMENT.md   |  4 -
open-sse/services/raycast.ts    | 280 -----
scripts/raycast/usage-benchmark.mjs | 165 -----

i.e. it dropped both the RAYCAST_* env rows and their docs together with the GPL-derived provider code (open-sse/services/raycast.ts, scripts/raycast/usage-benchmark.mjs) this PR's description cited as the reason the doc block was dead.

git merge-base --is-ancestor 0ad03316e origin/release/v3.8.51 → confirms it's an ancestor of the current tip.

So this PR's diff correctly stays package.json-only — the npm run test fix — and no ENVIRONMENT.md/.env.example changes are needed here.

@patrykkopycinski
patrykkopycinski force-pushed the pr-fix-test-script-hang branch 2 times, most recently from 2c52d8c to 9512dfe Compare September 16, 2026 07:48
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Holding this one rather than merging, for two reasons:

  1. The stated defect is gone on the current tip. npm run test and npm run test:unit are already byte-identical in content on release/v3.8.51 — both carry --test-force-exit on each node invocation and the test:unit:serial tail. The RAYCAST half of the body is also already done: .env.example and docs/reference/ENVIRONMENT.md have no RAYCAST references left, and check:env-doc-sync is green on that. So the diff no longer matches the problem it describes.

  2. The diff carries an undeclared change: --test-concurrency goes from 4 to 20 in both node invocations of test:unit, which the commit message and PR body never mention, and which diverges test:unit from test:unit:ci/test:unit:ci:shard (still 4). On this box that is not free — the unit suite already contends with parallel builds, and a 5× concurrency bump is exactly the kind of change that turns timing-sensitive tests into flakes and pushes memory over the edge.

If you still want the concurrency change, open it on its own with a before/after measurement (wall time and peak RSS at a comparable load) and it gets judged on its own merits. Nothing here is wasted — the force-exit work you did is what is already in the tree.

@patrykkopycinski
patrykkopycinski force-pushed the pr-fix-test-script-hang branch 3 times, most recently from 2fe8ef8 to 0d7cfcb Compare September 16, 2026 17:06
Two independent defects, both in the test/dev entrypoint layer.

1. `npm run test` never terminated. It was a hand-maintained copy of
   `test:unit` that had drifted: it omitted `--test-force-exit` on BOTH
   node invocations and dropped the trailing `&& npm run test:unit:serial`.
   Per AGENTS.md ('Database Handles in Tests'), unreleased SQLite handles
   make Node's native runner hang indefinitely — every sibling script
   (`test:unit`, `test:unit:ci`, `test:unit:ci:shard`) already carried the
   flag; only `test` did not. Measured on m1max at 84c6ad7, same suite
   both arms: without the flag the runner was killed at the 420s ceiling
   (exit 137, no summary line, 23 orphaned node processes); with it the
   runner exited on its own in 419s leaving 1. `test` now delegates to
   `test:unit` so the two cannot drift again, which also makes the serial
   suite reachable from `npm run test` for the first time.

2. Removing the RAYCAST_* rows from ENVIRONMENT.md (#9) broke
   check-env-doc-sync. `parseEnvExampleVars` matches `^#?\s*(VAR)=`, so it
   counts COMMENTED-OUT vars: the four entries still sat at
   .env.example:1263-1266 and became `envMissingDoc` drift the moment their
   docs disappeared. The #9 verification only ran the fabricated-docs gate
   and missed this one. The block is dead either way — it documents
   open-sse/services/raycast.ts and scripts/raycast/usage-benchmark.mjs,
   both deleted with the GPL-derived provider in cheaper-inference#11691, and no live code
   reads the vars — so it is removed rather than re-documented.

envMissingDoc is now []. The remaining codeMissingEnv failure
(CURSOR_AGENT_BINARY, CURSOR_MAX_FRAME_BYTES, OMNIROOT) is pre-existing
drift on the base, absent from this diff, and left alone.
…iles

check:mutation-test-coverage --strict fails identically on pristine
release/v3.8.51 (f1e7148) with an empty diff — base debt blocking this PR.

- combo-identical-error-streak.test.ts -> comboPredicates.ts
- 13601-header-drop-count-surfaced.test.ts -> responseHeaders.ts
- semantic-cache-no-truncated-writes.test.ts -> semanticCache.ts
@patrykkopycinski
patrykkopycinski force-pushed the pr-fix-test-script-hang branch 2 times, most recently from 0d7cfcb to 25c999d Compare September 16, 2026 21:21
@patrykkopycinski

Copy link
Copy Markdown
Contributor Author

CI attribution for run 35126101685

Both red gates reproduce on pristine release/v3.8.51 with an empty diff, so neither is caused by this PR. One of them is fixed here.

1. Fast Quality Gates → check:mutation-test-coverage --strict — base debt, fixed in this PR (25c999d83).

Reproduced on pristine base f1e7148c1 (no diff applied):

$ git checkout --detach f1e7148c1
$ node scripts/check/check-mutation-test-coverage.mjs --strict
Scanned 5235 unit test file(s) against 31 mutated module(s).
  open-sse/services/combo/comboPredicates.ts
    + tests/unit/combo-identical-error-streak.test.ts
  open-sse/handlers/chatCore/responseHeaders.ts
    + tests/unit/13601-header-drop-count-surfaced.test.ts
  open-sse/handlers/chatCore/semanticCache.ts
    + tests/unit/semantic-cache-no-truncated-writes.test.ts
✗ 3 covering unit test(s) across 3 module(s) are missing from stryker.conf.json tap.testFiles.

Registering those 3 files in tap.testFiles (3-line diff, insertion only — existing order preserved) turns it green:

✓ No drift — every covering unit test is listed in tap.testFiles.

2. Unit Tests fast-path (1-4/4) — base-side hang, not attributable to this diff.

Those shards run test:unit:ci:shard. This PR's diff does not touch that script — it only changes test and test:unit (verified: git diff origin/release/v3.8.51...HEAD -- package.json | grep -E '^\s*[+-].*test:unit:ci:shard' returns nothing).

The shard failures are the symptom this PR exists to fix. On pristine base f1e7148c1, with no diff applied:

$ node --import tsx/esm ... --test --test-force-exit --test-concurrency=4 \
    tests/unit/source-no-raw-nul-bytes.test.ts tests/unit/gen-budget-card-svg.test.ts
TAP version 13
# Subtest: the budget card prints the catalog's own totals (no regex-parsed subset)
<hangs — killed at 400s, never emits a plan or a result>

gen-budget-card-svg.test.ts never terminates, so the runner is killed and every test in flight in that shard is reported failed. That explains why the ~20 "failures" are an unrelated grab-bag (runAuthzPipeline, NUL-byte scanner, reasoning-effort clamps, getProviderCredentials, budget card, …) rather than a coherent set.

Corroborating evidence that these are load/termination artifacts and not regressions: running the full unit suite (4587 files, ~34.6k tests) locally on pristine base vs. three separate PR branches, the same tests flip pass↔fail in both directions across runs of an identical, zero-diff commit:

tree pass/fail failures unique vs. base
pristine base 34581 / 23 —
#12723 34612 / 22 0
#13355 34588 / 22 0

Reproducing the numbers

git checkout --detach f1e7148c1
node scripts/check/check-mutation-test-coverage.mjs --strict   # gate 1, fails on base

export DISABLE_SQLITE_AUTO_BACKUP=true
node --max-old-space-size=8192 --import tsx/esm \
  --import ./open-sse/utils/setupPolyfill.ts \
  --import ./tests/_setup/isolateDataDir.ts \
  --test --test-force-exit --test-concurrency=4 \
  tests/unit/gen-budget-card-svg.test.ts                        # gate 2, hangs on base

Note: 6 test files (vnc-cdp-bridge-auth, cdp-proxy-auth-gate-13679, adobe-firefly-browser-login, vnc-session, vnc-session-docker-args, wreq-native-manifest) were excluded from the local full-suite runs above — they bind CDP port :9222, which a local Chrome holds. They are unrelated to all diffs discussed here.

…:unit

The dedupe made `test` delegate to `test:unit`, but it also silently
reverted the deliberate local concurrency cap from cheaper-inference#13187 ("cap local
unit-test concurrency at 4 to avoid exhausting commit charge") back to 20.

Measured on m1max (16 cores), same suite and same tree, only the flag differs:
  concurrency=20 -> 356 cancelled, 356 "event loop has already resolved" bailouts
  concurrency=4  -> 0 cancelled, 0 bailouts
So 20 does not just slow the run down, it makes the runner abandon tests and
still print a summary -- a false green. Restore 4; termination is preserved via
delegation to test:unit, which already carries --test-force-exit.
The cheaper-inference#13187 batch commit (178d252) already gave `test` both `--test-force-exit` flags and the trailing `&& npm run test:unit:serial` step, so the delegation fix was redundant. It also broke tests/unit/test-serial-quarantine.test.ts, which asserts every parallel runner script ends with the serial step (base 4/4 -> head 3/4). package.json is now byte-identical to base; this PR is the stryker tap.testFiles fix only.
@patrykkopycinski

Copy link
Copy Markdown
Contributor Author

Scope reduction after rebase onto release/v3.8.51 (f1e7148c1)

Both defects this PR originally fixed are already fixed on base — the PR body is now stale. Verified, not assumed:

1. npm run test termination — already fixed by #13187 (178d25250).

base package.json scripts.test already carries --test-force-exit on both node invocations and already ends with && npm run test:unit:serial:

cross-env ... --test-force-exit --test-concurrency=4 tests/unit/*.test.ts "..." "..." \
  && cross-env ... --test --test-force-exit --test-concurrency=4 "tests/unit/dashboard/**/*.test.ts" \
  && npm run test:unit:serial

So the delegation fix was redundant. It was also actively harmful:

Measured, same tree and suite, on this machine:

test:unit concurrency cancelled "event loop has already resolved" bailouts
20 (this PR as pushed) 356 356
4 (base / #13187) 0 0

At 20, npm run test "terminates" by cancelling 356 tests — a false green, not a fix. Guard test on corresponding trees: base 4/4 pass, this branch pre-revert 3/4 (not ok 2 - todos os runners paralelos terminam com o passo serial).

2. RAYCAST env-doc sync — moot: ENVIRONMENT.md no longer exists on base.

git show origin/release/v3.8.51:ENVIRONMENT.md → path 'ENVIRONMENT.md' does not exist. Removed upstream while this PR was open, so the envMissingDoc it repaired cannot recur. check-env-doc-sync.mjs reports "✓ Env / docs contract is in sync" on pristine base — no repair needed.


Current diff: stryker.conf.json only (+3), which is still genuinely required.

Fast Quality Gates failed on 3 tests missing from tap.testFiles, reproducible on pristine base and absent from this diff's original scope:

 3 covering unit test(s) across 3 module(s) are missing from stryker.conf.json tap.testFiles.
  open-sse/services/combo/comboPredicates.ts          + tests/unit/combo-identical-error-streak.test.ts
  open-sse/handlers/chatCore/responseHeaders.ts        + tests/unit/13601-header-drop-count-surfaced.test.ts
  open-sse/handlers/chatCore/semanticCache.ts          + tests/unit/semantic-cache-no-truncated-writes.test.ts

Registered all three; gate now ✓ No drift — every covering unit test is listed in tap.testFiles. Registered tests verified green individually (8 + 3 + 3 = 14 pass, 0 fail). Mutation-tested: reverting the 3 entries returns the gate to RED, restoring returns it GREEN.

package.json is now byte-identical to base. Ready for re-review; happy to have this closed instead if the maintainer prefers to track the stryker drift separately — the chunk belongs wherever it's easiest to land.

@patrykkopycinski

Copy link
Copy Markdown
Contributor Author

Fast Quality Gates is green — the stryker fix landed

Run 35161135356:

Gate Result
Fast Quality Gates pass (was fail on the 3 missing tap.testFiles entries)
Merge integrity, Docs Gates, No new ESLint warnings, Vitest, semgrep ×2, Change Classification pass
Unit Tests fast-path 4/4 pass
Unit Tests fast-path 1,2,3/4 fail — base debt, see below
API Route Typecheck fail — base debt, see below

Also confirmed resolved: todos os runners paralelos terminam com o passo serial (tests/unit/test-serial-quarantine.test.ts) is gone from the failure list now that package.json is back to base.

Remaining red gates are base debt — reproduced on pristine base with zero diff

Ran the exact 4 test files the failing assertions live in, on my head (7bda01936) and on pristine release/v3.8.51 (f1e7148c1) — identical 52/55 pass, 3 fail, same test names on both trees:

ℹ tests 55 / pass 52 / fail 3      <- head
ℹ tests 55 / pass 52 / fail 3      <- base, zero diff
  ✖ runAuthzPipeline allows dashboard sessions to reach DB health management API
  ✖ #2331 codex.ts still prioritizes modelEffort first in rawEffort chain
  ✖ direct translation calls have English messages

The fourth, OAuthModal renders the structured panel, passes in isolation on both trees — it only fails under CI shard CPU contention, same class as the rest of the shard noise.

API Route Typecheck is likewise base debt: src/lib/providerModels/modelDiscovery.ts:355 passes an untyped record.metadataProvenance into SyncedAvailableModel, whose metadataProvenance requires vertexDocs (made required by #12471). node scripts/check/check-api-typecheck.mjs fails identically on pristine f1e7148c1:

✗ src/lib/providerModels/modelDiscovery.ts TS2345 (baseline 0, live 1)

Neither is in this diff (package.json is byte-identical to base; stryker.conf.json +3 is the only change). Suggest those two be tracked at the base rather than fold unrelated fixes into this PR.

@patrykkopycinski patrykkopycinski changed the title fix(test): make npm run test terminate and restore RAYCAST env-doc sync chore(stryker): register 3 covering unit tests missing from tap.testFiles Sep 16, 2026
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Thanks @patrykkopycinski — merging via the release merge-train. Validated in local merge-train (.claude/worktrees/merge-train-20260918-111718-suite.log) on the devbox @ train tip 7bb373fba5e241964c0ffb17bd03a804700839bb, boarded with 55 sibling PRs: typecheck:core, file-size, complexity, cognitive-complexity, changelog-integrity green; 747/747 changed-area node:test cases + 476/476 vitest green (fast parity mode — the full suite ran today on the release tip via the base-red train and runs again on the 3b train). Merged --admin per merge-gates §7.

@diegosouzapw
diegosouzapw merged commit d073f1b into cheaper-inference:release/v3.8.51 Sep 18, 2026
12 of 16 checks passed
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…iles (cheaper-inference#13357)

* fix(test): make npm run test terminate and restore RAYCAST env-doc sync

Two independent defects, both in the test/dev entrypoint layer.

1. `npm run test` never terminated. It was a hand-maintained copy of
   `test:unit` that had drifted: it omitted `--test-force-exit` on BOTH
   node invocations and dropped the trailing `&& npm run test:unit:serial`.
   Per AGENTS.md ('Database Handles in Tests'), unreleased SQLite handles
   make Node's native runner hang indefinitely — every sibling script
   (`test:unit`, `test:unit:ci`, `test:unit:ci:shard`) already carried the
   flag; only `test` did not. Measured on m1max at 84c6ad7, same suite
   both arms: without the flag the runner was killed at the 420s ceiling
   (exit 137, no summary line, 23 orphaned node processes); with it the
   runner exited on its own in 419s leaving 1. `test` now delegates to
   `test:unit` so the two cannot drift again, which also makes the serial
   suite reachable from `npm run test` for the first time.

2. Removing the RAYCAST_* rows from ENVIRONMENT.md (cheaper-inference#9) broke
   check-env-doc-sync. `parseEnvExampleVars` matches `^#?\s*(VAR)=`, so it
   counts COMMENTED-OUT vars: the four entries still sat at
   .env.example:1263-1266 and became `envMissingDoc` drift the moment their
   docs disappeared. The cheaper-inference#9 verification only ran the fabricated-docs gate
   and missed this one. The block is dead either way — it documents
   open-sse/services/raycast.ts and scripts/raycast/usage-benchmark.mjs,
   both deleted with the GPL-derived provider in cheaper-inference#11691, and no live code
   reads the vars — so it is removed rather than re-documented.

envMissingDoc is now []. The remaining codeMissingEnv failure
(CURSOR_AGENT_BINARY, CURSOR_MAX_FRAME_BYTES, OMNIROOT) is pre-existing
drift on the base, absent from this diff, and left alone.

* chore(stryker): register 3 covering unit tests missing from tap.testFiles

check:mutation-test-coverage --strict fails identically on pristine
release/v3.8.51 (7339a9f) with an empty diff — base debt blocking this PR.

- combo-identical-error-streak.test.ts -> comboPredicates.ts
- 13601-header-drop-count-surfaced.test.ts -> responseHeaders.ts
- semantic-cache-no-truncated-writes.test.ts -> semanticCache.ts

* fix(test): keep the cheaper-inference#13187 concurrency-4 cap in test:unit

The dedupe made `test` delegate to `test:unit`, but it also silently
reverted the deliberate local concurrency cap from cheaper-inference#13187 ("cap local
unit-test concurrency at 4 to avoid exhausting commit charge") back to 20.

Measured on m1max (16 cores), same suite and same tree, only the flag differs:
  concurrency=20 -> 356 cancelled, 356 "event loop has already resolved" bailouts
  concurrency=4  -> 0 cancelled, 0 bailouts
So 20 does not just slow the run down, it makes the runner abandon tests and
still print a summary -- a false green. Restore 4; termination is preserved via
delegation to test:unit, which already carries --test-force-exit.

* revert(test): drop redundant test-script delegation

The cheaper-inference#13187 batch commit (5d3c7d6) already gave `test` both `--test-force-exit` flags and the trailing `&& npm run test:unit:serial` step, so the delegation fix was redundant. It also broke tests/unit/test-serial-quarantine.test.ts, which asserts every parallel runner script ends with the serial step (base 4/4 -> head 3/4). package.json is now byte-identical to base; this PR is the stryker tap.testFiles fix only.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants