Test/close supervisor mutation survivors

.backlog/tasks/task-100 - Close-what-mutation-testing-surfaced-in-platformos-mcp-supervisor.md

@@ -0,0 +1,64 @@
+---
+id: TASK-100
+title: Close what mutation testing surfaced in platformos-mcp-supervisor
+status: Done
+assignee: []
+created_date: '2026-09-03 06:30'
+updated_date: '2026-09-03 07:48'
+labels:
+  - testing
+  - platformos-mcp-supervisor
+  - mutation-testing
+dependencies: []
+priority: medium
+ordinal: 75000
+---
+
+## Description
+
+<!-- SECTION:DESCRIPTION:BEGIN -->
+A local Stryker 10 run over 11 of the supervisor's 29 source files (551 mutants, 82.4% score) found guards that can be deleted with the whole suite still green, plus two design facts that are correct in behaviour but recorded nowhere.
+
+WHY THIS MATTERS: none of these is a bug today. Each is a promise the suite does not keep — a defence that could be removed or inverted in a future refactor without a single test noticing — or a fact the next reader will have to re-derive from scratch.
+
+CONTEXT A FUTURE IMPLEMENTER NEEDS: Stryker is NOT part of this repository. It was installed locally and its configs (`packages/*/stryker.config.mjs`, `packages/*/vitest.stryker.config.mjs`) are gitignored, so you cannot reproduce these findings by running a project script. You do not need to. Every item below states the exact one-line change that survives today, and CLAUDE.md already requires the same discipline: apply the change by hand, confirm the new test fails, then revert. That is the verification, and it is enough.
+
+If you do want the tool back:
+  yarn add -W -D @stryker-mutator/core@^10 @stryker-mutator/vitest-runner@^10
+  git checkout -- package.json yarn.lock
+
+READ THE REPORT WITH THIS CAVEAT. The mutation run excludes `test/integration/**` and `test/guards/**`, because those specs cannot run inside Stryker's sandbox — `stdio-smoke.spec.ts` shells out to `yarn build`, and the guards scan the repository's own layout. Code covered ONLY by those specs therefore shows as uncovered when it is nothing of the sort. That already produced one false finding: the `validate_code` tool-callback body (`transport/validate-code.ts`, the `content: [{ type: 'text', … }]` envelope) reported four no-coverage mutants, and `test/integration/stdio-smoke.spec.ts` in fact calls the tool over a real stdio transport and asserts that exact envelope. No work is needed there. Check the integration specs before believing any no-coverage result.
+
+Deliberately OUT of scope, reviewed and set aside: `assemble.ts` empty-list assertions; the malformed-request sub-branches at `validate-code.ts:289/290/313`; and roughly 50 string-literal mutants that survive because tests derive expected messages from the code under test rather than pasting them, which is this repository's deliberate anti-rot choice and should stay that way. At least one of those is provably unkillable — `Buffer.byteLength(s, '')` equals `Buffer.byteLength(s, 'utf8')`, measured.
+<!-- SECTION:DESCRIPTION:END -->
+
+## Implementation Notes
+
+<!-- SECTION:NOTES:BEGIN -->
+All three subtasks Done. Files changed, all in `packages/platformos-mcp-supervisor`:
+  M src/result/response-budget.spec.ts   (+1 test, +`bytesOf` helper that `diagnosticBytes` now reuses)
+  A src/validate/batch-bounds.spec.ts    (new, 7 tests)
+  M src/validate/batch-bounds.ts         (comment only, +13 lines, 0 removed)
+  M src/validate/validate-buffers.ts     (comment only, +18 lines, 0 removed)
+No production behaviour changed. `validate-code.spec.ts` was not touched.
+
+The branch also carries two pre-existing uncommitted hygiene changes that are NOT part of this task: `.gitignore` (Stryker artefacts) and `vitest.config.mjs` (excluding `.stryker-tmp` so a leftover sandbox cannot be collected as a second copy of every spec running against mutated code).
+<!-- SECTION:NOTES:END -->
+
+## Final Summary
+
+<!-- SECTION:FINAL_SUMMARY:BEGIN -->
+All three subtasks closed. Two new tests' worth of promises the suite was not keeping, one branch decision settled, and one guard documented — no production behaviour changed anywhere.
+
+**100.1 — the warnings truncation is now pinned.** `response-budget.ts` could return the `warnings` bucket unsliced with the entire suite green. One test in `response-budget.spec.ts` drives 40 warnings against a budget derived from the fixture to pay for exactly 12, and asserts the whole returned result in one equality. The errors and infos slices turned out to be pinned already, by the contiguity test and the severity-order test respectively — confirmed by sabotaging all three, not assumed.
+
+**100.2 — `batch-bounds.ts` has a spec, and the file-count branch has an answer.** New `batch-bounds.spec.ts`, seven tests against `batchTooLarge` directly: both caps at their exact boundaries in both directions, bytes-not-characters, precedence between the two caps, and that the two refusals do not collapse into one message. The file-count branch is unreachable over MCP (`VALIDATE_CODE_INPUT.files` caps the array first, and `runValidateCode` is not on the package's public surface) and is **kept** as defence-in-depth, with that reasoning now written at the branch itself so its zero coverage is not read as a gap.
+
+**100.3 — `worthReading` is documented as a performance guard.** Its six survivors are explained by `warm()` only pre-warming a promise impact awaits itself; inverting it cannot change a verdict.
+
+**Verification.** Every claim was measured rather than carried over from the report: both survivors reproduced before any test was written, the file-count branch proved unreachable by planting a `throw` in it, and ten hand-applied mutations across the three files each killed by the intended test and no other. Package suite 549/549 (542 before), type-check clean, prettier clean, build confirms the new spec does not ship in `dist`.
+
+**One correction to the report itself**, recorded in 100.3's notes: the neighbouring `--no-impact` "costs NOTHING" claim was described as wholly untested. It is untested in half — the impact adapter never being called IS asserted in `validate-code.spec.ts`; the project read being skipped is not. The comment states that split rather than the flattened version.
+
+**Out of scope and untouched, as the task specified:** `assemble.ts` empty-list assertions, the malformed-request sub-branches, and the ~50 string-literal mutants that survive because tests derive expected messages from the code under test.
+<!-- SECTION:FINAL_SUMMARY:END -->

.backlog/tasks/task-100.1 - Assert-that-response-budget-truncates-warnings-not-only-errors-and-infos.md

@@ -0,0 +1,78 @@
+---
+id: TASK-100.1
+title: 'Assert that response-budget truncates warnings, not only errors and infos'
+status: Done
+assignee: []
+created_date: '2026-09-03 06:30'
+updated_date: '2026-09-03 07:37'
+labels:
+  - testing
+  - platformos-mcp-supervisor
+dependencies: []
+references:
+  - packages/platformos-mcp-supervisor/src/result/response-budget.ts
+  - packages/platformos-mcp-supervisor/src/result/response-budget.spec.ts
+parent_task_id: TASK-100
+priority: high
+ordinal: 1000
+---
+
+## Description
+
+<!-- SECTION:DESCRIPTION:BEGIN -->
+`src/result/response-budget.ts` exists to stop one `validate_code` answer eating the agent's context — the module's own comment records an unbounded call measured at ~336,000 tokens. It slices three buckets when a result is over budget: `errors`, `warnings`, `infos`.
+
+Only two of the three are asserted. MEASURED: replacing
+
+    warnings: result.warnings.slice(0, taken.warnings),   // line ~158
+
+with
+
+    warnings: result.warnings,
+
+leaves 289/289 tests passing across `src/result/` and `src/transport/`. The warnings truncation can be deleted and nothing notices.
+
+This is the highest-risk gap of the set because warnings are the COMMON severity — a file with hundreds of them is exactly the tail this module defends against, and losing the slice reopens the unbounded response the budget was written for.
+
+There is an existing test for the `errors` bucket; mirror it rather than inventing a new shape.
+<!-- SECTION:DESCRIPTION:END -->
+
+## Acceptance Criteria
+<!-- AC:BEGIN -->
+- [x] #1 A test drives a result whose `warnings` exceed the allocated budget and asserts the returned `warnings` array is sliced to exactly the allocated count
+- [x] #2 The same test asserts the `truncated` field reports the true pre-truncation total for the warnings bucket, not the returned count
+- [x] #3 Assertions use whole-value equality on the returned result per the repo's test guidelines — not a `length` check or a per-property read
+- [x] #4 SABOTAGE-VERIFIED: replacing `result.warnings.slice(0, taken.warnings)` with `result.warnings` makes the new test fail; the change is reverted afterwards and the suite is green
+- [x] #5 The existing errors and infos truncation tests still pass unchanged
+<!-- AC:END -->
+
+## Implementation Plan
+
+<!-- SECTION:PLAN:BEGIN -->
+1. Reproduce the survivor first: replace `result.warnings.slice(0, taken.warnings)` with `result.warnings` and confirm the spec is still green (it was — 13/13).
+2. Add a `bytesOf` helper to the spec that bills a diagnostic list the same way `costOf` does, and fold the existing `diagnosticBytes` onto it rather than duplicating the arithmetic.
+3. Add one test that derives its budget from the entries (`bytesOf(warnings.slice(0, 12))`) so "twelve fit" is arithmetic, not a tuned constant, and asserts the whole returned shape in one equality.
+4. Sabotage-verify, revert, then run the whole package suite, type-check and prettier.
+<!-- SECTION:PLAN:END -->
+
+## Implementation Notes
+
+<!-- SECTION:NOTES:BEGIN -->
+Why the gap existed, which is worth recording because it is not obvious from the file: the errors slice IS pinned (by 'closes a bucket at the first entry that does not fit', which asserts the returned lines are exactly [1]) and the infos slice IS pinned (by 'spends the budget on errors before any info', which asserts infosReturned is exactly 0). Every warnings assertion in the file read a TOTAL or a PRESENCE — `truncated.warnings.total`, `truncated.warnings !== undefined` — and both of those survive returning the whole unsliced list. Confirmed by sabotage on all three slices, not inferred.
+
+The budget is derived from the fixture (`bytesOf(warnings.slice(0, admitted))`) rather than hand-tuned. `diagnostic()` embeds the line number in the message, so entries on lines 1-9 and 10-40 differ in cost; a literal budget would be a magic number that quietly changes meaning if the fixture is touched. Paying exactly the cost of the first twelve makes the thirteenth fail `spent + cost > budgetBytes` by construction.
+
+The generated `truncated.note` string is deliberately NOT asserted, per the repo's rule against pasting a message the code under test produces.
+<!-- SECTION:NOTES:END -->
+
+## Final Summary
+
+<!-- SECTION:FINAL_SUMMARY:BEGIN -->
+Closes the highest-risk mutation survivor in the supervisor's response budget: the `warnings` bucket could be returned unsliced with the entire suite still green.
+
+**What changed** — one test in `src/result/response-budget.spec.ts` drives a single file with 40 warnings against a budget that pays for exactly 12 of them, and asserts the whole returned result in one equality: the returned `warnings` array is `warnings.slice(0, 12)` element-for-element, `truncated.warnings` is `{ returned: 12, total: 40 }`, and the untouched buckets come back empty. A `bytesOf` helper now bills a diagnostic list the way `costOf` does; the pre-existing `diagnosticBytes` was folded onto it instead of keeping two copies of the same arithmetic.
+
+**Why it matters** — warnings are the common severity, so a file with hundreds of them is exactly the tail the module was written for (its own comment records an unbounded call measured at ~336,000 tokens). Losing that slice reopens the unbounded response.
+
+**Verification** — sabotage-verified both ways: with `warnings: result.warnings` substituted for the slice, exactly the new test fails and the other 13 pass; reverted, all 14 pass. Full package suite 542/542 green, `type-check` clean, prettier clean. No source file changed.
+<!-- SECTION:FINAL_SUMMARY:END -->