Conversation
Audit lanes found assertions that cannot fail for a production reason. Each deletion here was measured by planting the bug it claims to catch and proving a surviving test goes red; candidates that no surviving test caught were kept. Removed shapes: - self-comparison: the daemon closure-trait table compared against the derive helper that produced it, the MCP exposure list rebuilt from the same filter, the runtime-fact catalog intersected with the CLI names derived from the same catalog, and the Apple trait helpers looped against the table they read. - fixture restated as expectation: the runner-protocol fixture compared its own keys to the trait table it types against, while runner-requests.json already owns per-command wire coverage. - assertion-free composition probes and `typeof x === 'function'` rows beside the test that calls x, kept only where nothing else reached the seam. - duplicated strictness: the Apple keyboard reader re-pinned the kernel's `readSnapshotKeyboardBandFact` table; the seam now asserts forwarding only. - test-only exports deleted with their callers: `shouldResetDaemonAfterRequestTimeout` keeps one caller in the module that declares it. Kept after measuring, because the mutant survived every other test: the boot keep-hot witness, owner-cleanup composition, `--update` retirement residue, the package gate's probe assertions, the shutdown teardown-mechanic parity, the restricted-by-default runtime policy, and the screenshot flag key arrays where a duplicate key is invisible to a membership scan.
The per-command runtime-execution tests each re-pinned `not.toHaveProperty('capability')`
and `('dispatch')` on their own descriptor. Those pins did not close the hole they
claimed: a retired key written in a descriptor literal fails `tsc` (excess-property
check), while one smuggled through a conditional spread — the shape `ownerFiles` already
uses — typechecks, and no `toEqual` on `platformExecution` can see a sibling key.
Measured with the retired key planted both ways: the literal form failed the build, the
spread form stayed green across every surviving test. One sweep over raw and normalized
descriptors now owns the invariant at the registry, so a new descriptor is covered
without repeating the pair per command.
Size Report
Startup median (7 runs, lower is better):
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 24 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
|
Reviewed at 6456356. The deleted keyboard-band test (packages/platform-apple/src/runner/tests/snapshot-presentation.test.ts:242) was the only case that fed malformed keyboard payloads through The Repo Guards failure is scripts/node-test-tmpdir.test.ts:373, a process-kill-timing race in the tmpdir wrapper's owner/child cleanup assertion; the diff here only touches packages/command-registry, packages/platform-apple/src/runner, src/daemon, src/daemon-client, src/commands, src/tests, and one line of daemon-client-timeout.ts, none of which touch scripts/ or tmpdir/process-lifecycle code, so this looks like a pre-existing flake unrelated to the deletions. Smoke Tests was still running when I looked, so I can't say anything about it yet. I read source and checked for sibling coverage on roughly ten of the ~forty deleted tests, not all of them, and I did not run the mutation-plant/measure-red procedure myself for any deletion, so I can't confirm the PR's stated unit/apple-runner pass counts or the "8 candidates restored" claim beyond what's in the diff. Restoring the routing-proof test at snapshot-presentation.test.ts is the one thing I'd want before merge. |
Review finding: `readAppleSnapshotResult` is the only place the Apple runner's wire keyboard fact becomes a daemon fact, and on the same-process capture path it feeds the tap/click occlusion guard before any serialization. After the table dedup, every remaining case used well-formed payloads, so a seam forwarding `result.keyboard` raw survived all 618 runner tests and typechecks. Restored one malformed-payload case per kernel reason code as the routing proof: those restated shapes exist only if the kernel reader ran. Measured: the raw-forwarding mutant now fails here. Titles now match the input after the retired-projection sweep moved that claim to retired-descriptor-projection.test.ts: eight per-command runtime-execution tests dropped "with no legacy projection"/"capability bucket", and the runtime-policy default test names the flag it asserts, pointing at contact-sheet.test.ts for the refusal seam it does not exercise.
|
Fixed at I ran your mutant exactly as written. On your question about Also took the two bot P3s, both valid:
Correction to my previous comment, which got the Repo Guards mechanism backwards and asserted a race it had not proven. The failure is Verified mechanism, reproduced locally: On Smoke Tests: fleet-wide today, not this PR. 22 failure / 22 success on the iOS workflow over the last ~3 days, failing at different steps each time ( Also correcting my own process, since it cost a green signal: I reran the stale run ( |
|
Reviewed at 1d546a3. The delta restores the malformed-keyboard-payload cases in I checked the restored cases by tracing the reader's branches. I did not run the mutation procedure. Smoke Tests was still running when I looked; this test-only delta does not touch a device route. |
thymikee
added
the
ready-for-human
|
Green: 18/18 checks pass at For whoever owns the iOS lane — a correlation, not a claim: hour-by-hour, this workflow has gone from scattered failures to every hour fully failing starting 12:00-13:00 UTC today (23 fail / 34 pass today vs 10 / 54 yesterday, and 12:00Z-16:00Z is 15 fail / 4 pass). The window opens around Nothing further from me here. Two follow-ups this audit surfaced that are worth their own issues, both pre-existing and independent of this diff:
|
This branch has not been deployed
1 participant
Summary
Follows the
docs/agents/test-audit.mdprocedure over 10 read-only lanes. Deletes 505 test lines / adds 59 across 24 files; the only production change is un-exportingshouldResetDaemonAfterRequestTimeout, whose only caller is in-module.Removed shapes: self-comparison (daemon closure traits vs the derive helper that produced them, MCP exposure rebuilt from the same filter, runtime facts intersected with CLI names from the same catalog, runner trait helpers looped against the table they read), a fixture restated as its own expectation, assertion-free composition probes,
typeofrows beside the sibling that callsx, and the keyboard reader re-pinning the kernel'sreadSnapshotKeyboardBandFacttable.Each deletion was measured: plant the bug, prove a surviving test goes red. 8 candidate deletions were rejected because the mutant survived everything else — boot keep-hot, owner-cleanup composition,
--updateresidue, package-gate probes, shutdown teardown parity, the restricted-by-default policy, and the screenshot flag arrays where a duplicate key is invisible to a membership scan. Six restored whole; two keep only their measured assertion.Review restored the keyboard-seam routing proof (the reviewer's raw-forwarding mutant survived all 618 runner tests and
tsc) and retitled 11 tests that promised more than their input exercises.Validation
Head
1d546a3: 18/18 checks pass (36164799265). Locallycheck:affected --rungreen,test:unit11388,apple-runner618.Both review-time reds are pre-existing, reproduced off this branch, and need separate fixes: a test-isolation defect in
check:tmpdir-leaks:test, and fleet-wide iOS Smokesimctl openurltimeouts that also fail onmainat this merge base (green here on attempt 3). Both explained in review comments.