Repository navigation
test(windows): serialize credential tests and retry executable removal - #460
Merged
Merged
Conversation
Two Windows jobs flaked on #443 (#446). Both have a cause outside the code under test. Credential cleanup "left an entry": the RPC process deletes every account in the grant, verifies each is absent while holding the mutation lock, and only then answers ok. The test process then read one back. Windows Credential Manager loses updates when CredWriteW, CredReadW and CredDeleteW run concurrently across threads or processes: "a credential can survive a CredDeleteW that returned success", and some "only reappeared after the processes had exited" (windows-native-keyring-store #20, measured at 51-99 of 400 credentials surviving across 10 processes, none single-threaded). The native e2e binary ran its twelve tests on parallel threads, each with its own RPC child process, so our own suite produced exactly that concurrency. test:native-e2e now runs with --test-threads=1 (the twelve tests finish in under a second on windows-2025), and the assertion names the account that survived. The upstream fix (#21, a per-user named mutex around every call) is not in the 1.1.0 release this crate is pinned to; picking it up is a Cargo.lock repin and stays separate. ordered.exe "stayed locked": the test body had already passed; the failure came from cleanupRoot removing a node.exe copy that a supervised target had just finished executing. Windows keeps a process image locked until its section is released after the last handle closes, and Defender opens freshly copied executables. rmSync retries EPERM and EBUSY only when maxRetries is set, so cleanupRoot now retries for up to two seconds. Coven-Harness: Claude Code / claude-fable-5-1
Coven-Harness: Claude Code / claude-fable-5-1
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #446
Two Windows jobs flaked on #443. Both causes sit outside the code under test; this hardens the tests with evidence.
1. Credential cleanup "reported success but left an entry"
Job 115509182385:
rpc.cleanup(&grant)["ok"]wastrue, thenentry_presentfound one of the three accounts still in Credential Manager.The RPC side cannot have skipped it:
run_cleanup_transaction(src-tauri/src/keyring.rs) deletes every account in the grant's scope, then re-reads each one while still holding the mutation lock, and answersokonly when all read back absent. So the RPC process observed the entry gone; the test process read it back milliseconds later.That is a documented Windows Credential Manager behaviour. windows-native-keyring-store #20 (the crate this repo pins at
=1.1.0) reports that concurrentCredWriteW/CredReadW/CredDeleteWcalls across threads or processes lose updates: "a credential can survive aCredDeleteWthat returned success", and some "only reappeared after the processes had exited". Measured on Windows 11 25H2 with 400 credentials: 38–54 survived across 10 threads, 51–99 across 10 processes, none single-threaded, none with one named mutex around every call. Upstream fixed it in PR #21 (commit52af6ec, a per-userGlobal\named mutex), closed 2026-10-13, with no release shown yet.Our native e2e binary ran its twelve tests on parallel threads, each with its own
phase1-native-rpcchild process writing, reading and deleting credentials, which is exactly that concurrency.Changes:
test:native-e2eruns with--test-threads=1. The twelve tests finished in 0.62 s on windows-2025 in the failing job, so serializing costs nothing measurable.Picking up the upstream mutex is a
Cargo.lockchange (lock-governed) and stays a separate follow-up once a release carries it.2.
ordered.exe"stayed locked"Job 115518915539: the failure is
EPERM … syscall: 'rm'thrown fromcleanupRoot(src/phase1-windows-supervisor.test.ts:155) in the test'sfinally, after bothpe-okruns. The test body had passed; the removal of anode.execopy that a supervised target had just finished executing failed.runCommandresolves on the supervisor'sclose, so the supervisor and its target have exited. Windows still keeps a process image locked until the image section is released after the last handle closes, and Defender opens freshly copied executables for scanning. Node'sfs.rmSyncretriesEBUSY/EPERMonly whenmaxRetriesis set (default 0).cleanupRootnow retries for up to two seconds (maxRetries: 20, retryDelay: 100).Verification
cargo check --target x86_64-pc-windows-gnu --features phase1-conformance --test phase1_native_rpcon the pinned 1.95 toolchain: compiles.vitest run --config vitest.heavy.config.ts src/phase1-windows-supervisor.test.tsloads (skipped on macOS); biome clean.ci:fullon this PR runs both Windows jobs (Windows supervisor behaviorrunstest:native-e2eand the supervisor vitest file).Coven-Harness: Claude Code / claude-fable-5-1