test(windows): verify packaged sandbox client cancellation - #3586
Conversation
Generated-by: Codex
Astro-Han
left a comment
There was a problem hiding this comment.
Review at exact head bbbb6b4f19a64f6a1047defba962382321914d47.
APPROVE. For a test-only PR the question that matters is whether the new check can actually fail, and here there is direct evidence that it ran for real rather than passing vacuously.
Release Windows check executed on this head and its log carries [verify-windows-sandbox] packaged client cancellation and recovery verified three times — so verifyPackagedClientCancellation genuinely ran inside the packaged Windows sandbox, not on a skipped platform branch. That is the thing most easily assumed and rarely checked: a Windows-only verification added to a script that never gets invoked would sit green forever.
The design is deliberately resistant to false green in two ways:
- It asserts the pre-abort state before aborting. The probe waits until the sandbox process is actually observable, so an abort that "succeeds" because nothing was ever running cannot pass.
settledEarlyturns a premature settle into a failure, not a pass.throw new Error(\${description} settled before the expected state was observed.`)` — a cancellation that resolves before the expected condition is reached is treated as a broken observation. Most cancellation tests get this backwards and accept early settlement as success.
The recovery half matters as much as the cancellation half: asserting (await listCancellationProcesses(sandboxExecutable)).length === 0 and then confirming the client still works distinguishes cancelled from wedged. An abort that leaves an orphaned sandbox process would otherwise look identical to a clean one from the client's side.
The timing discipline is right too — the waits are condition probes with deadlines rather than bare sleeps, so this shouldn't become a flaky-under-load test.
No P0–P3. One file, scripts/verify-windows-sandbox-e2e.mjs, +227/-1, no production code touched.
Verification: exact-head test and package are completed/success, and Release Windows check (run 32630825228) is green with the cancellation assertion present in its log. I did not attempt to run the Windows AppContainer path locally and am not claiming it.
Summary
Adds explicit client-initiated cancellation evidence to the existing packaged Windows filesystem-worker E2E.
The scenario uses
FilesystemWorkerClient.abortSignalagainst a deterministic long-running AppContainer child provided by the already-packaged broker's--stdio-probe --sleepcommand. It proves the one-shot broker and AppContainer child are active before abort, then requires:FilesystemWorkerClientErrorwithreason: aborted,stage: launch, anddispatched: true;The WMI probe is bounded by the remaining outer deadline. A unique request identity prevents ledger collisions. Failure cleanup aborts the probe and runs one recovery launch; simultaneous evidence and cleanup failures are reported together. Both an immediately clean cancellation and stale-ledger replay are accepted, so the test pins the security outcome rather than the current cleanup mechanism.
This is test/release evidence only. It does not change Runtime behavior, the worker protocol, broker behavior, timeout policy, cancellation classification, ACL semantics, or lifecycle ownership.
Refs #2142
Scope
This is the explicit client-cancel slice left open by Phase 4. It does not claim to close the whole lifecycle checkbox. Runtime Host death mid-launch, sustained concurrency soak, unsettled-state recovery/quarantine, the adversarial matrix, and independent human security review remain separate work.
It is independent of #3558 at the code level: this PR changes only the existing packaged worker E2E consumed by
verify:windows-x64; #3558 promotes the broader existing lifecycle smokes through a different four-file diff.Verification
Exact head
bbbb6b4f19a64f6a1047defba962382321914d47; the branch started frommain9de05e266, and fresh GitHub checks passed against the merged result at basedc9d2f0dd:win-unpacked-shaped temporary directory;npm run rebuildpassed;biome lint .: 2,611 files, no findings;biome format .: 1,600 files, no changes;git diff --checkpassed.Local limitations:
dlltool.exe; the L2 broker was built from the exact current sources by placing the installed LLVMllvm-dlltool.exeunder the expected temporary tool name and adding only that temporary directory to PATH;0xffffffff, and one Git fixture cleanup hitsEBUSY; the changed release ownership and packaged-closure tests pass directly;win-unpackedoutput. The authoritative packaged evidence is below.L3 on the exact head:
win-unpackedapp, the pinned-upgrade result, and the automatic-update result.AI use
Tool(s) and scope: Codex designed and implemented the bounded packaged client-cancel evidence, ran the local quality gate and real-Electron L2, and performed an author-side adversarial review. The commit includes a
Generated-by: Codextrailer. AI review is not independent human security review.Checklist
Does this PR entail a change in behavior?