Repository navigation
Conversation
…st-debt entry Prerequisite B for #1329 AC13. Parent is 2c310e7, the formatter-only and cross-process-collision commit; this is a single fresh commit, not an amend and not a reuse of the wip/1343-s1-scaffold preservation commit. Scope note: the certified R10 delivery plan was cut back to the issue objective by the user. The TCB machinery, the attestation records, PreimplementationTrustAnchorV2, the systemd quarantine manager, the root views, the fourteen roles and scripts/Invoke-BoundOnce.ps1 are discarded, not deferred, and none of them appears here in any form. Rationale: #1343 (comment) Suite guard and Windows matrix (src-tauri/tests/cli_test_reset.rs): - ResetTestCoordinator { handle, owned, handle_open, wait_count, release_count, _not_send: PhantomData<Rc<()>> }. The Rc marker makes the guard !Send + !Sync, so ReleaseMutex can only run on the owning thread by construction. - One parameterised constructor: exactly one CreateMutexW, GetLastError captured in the immediately following statement, the guard built with owned=false and handle_open=true before any wait, then exactly one WaitForSingleObject with a finite 120000 ms timeout. Ownership only for WAIT_OBJECT_0 or WAIT_ABANDONED; any other result closes the handle. - complete() performs at most one ReleaseMutex, closes, preserves the primary and secondary errors and leaves both flags false, so Drop is a no-op on the normal path. Drop repeats best effort without panicking and emits AC1343_SUITE_CLEANUP_FAILED on failure. - The ten active Windows tests acquire the suite first and complete last. - Zero-depth probe with both subcases differing only in the initial-owner flag. The second acquirer runs on a fresh thread: a Windows mutex is reentrant for its owning thread, so without that the negative subcase could never produce WAIT_TIMEOUT and would have passed green while testing nothing. Private mutex, helper and protocol (src-tauri/tests/cli_test_reset.rs): - reset_test_coordinator_private_handoff creates the nonce-named probe owned, runs the timeout child, validates and reaps it, releases without closing, runs the acquire child, validates and reaps it, and only then closes. - Ignored helper windows::reset_test_coordinator_child, launched through canonicalize(current_exe()) with the exact argv --ignored --exact windows::reset_test_coordinator_child --nocapture --test-threads=1, env_clear and only the canonical tuple. - Closed environment parser running before any Win32 call, rejecting case aliases, duplicates, unknown names, NUL, non-ASCII, absence and bad values. - Child stdout asserted as the closed libtest envelope; child stderr reserved to exactly two AC1343_COORD_V1 records over a closed seventeen-key set. Unsafe proofs (src-tauri/src/testability/reset.rs, src-tauri/tests/cli_test_reset.rs): - Every unsafe site now carries a // SAFETY: proof: 8 in reset.rs, including the unsafe extern "system" block, and 30 in cli_test_reset.rs. The release and drop proofs state the same-thread ownership argument explicitly, since the !Send marker is what makes it hold. - Each file also carries #![deny(clippy::undocumented_unsafe_blocks)]. Without it the criterion would be satisfied once and unenforced afterwards: the lint is allow-by-default, so unsafe site 39 could land uncommented with clippy -D warnings still green. The attribute is module-scoped and measured to affect only these two files. Test debt (test-debt.allowlist.json): - Allowlists windows::reset_test_coordinator_child. It is not deferred debt: it runs on every run as a spawned process and carries #[ignore] only so libtest does not also run it in-process, where it would contend for the same named mutex as its own parent. src-tauri/Cargo.toml adds only "Win32_System_Pipes" to the existing windows-sys 0.59 feature array. No new dependency and no lockfile delta.
Resolves the single conflict introduced by 9c7c77e (#1154), which replaced tauri::Builder::default().any_thread() with agentscommander_lib::test_support::test_builder() in the same region of src-tauri/tests/pty_powershell_managed_native.rs that the #1343 formatter-only commit reformatted. Resolution takes main's test_builder() call and leaves .manage(MasterToken::new(...)) on one line, which is what rustfmt produces now that the shorter receiver fits. Nothing else in that file, and no other file, was resolved by hand: everything else auto-merged. 8415854 remains the reviewed prerequisite commit for #1329 AC13. This merge exists only to make the branch mergeable and does not change what was reviewed.
`RESET_PROCESS_MUTEX_NAME` was declared unconditionally but read only by `platform_reset_mutex_wait_adapter`, which is `#[cfg(target_os = "windows")]`. On any other target the constant is dead code, so `cargo clippy --all-targets -- -D warnings` promotes the `dead_code` warning to an error and the job exits 101. Introduced by 2c310e7, which added the reset mutex. It stayed invisible because `rust-regression-linux` and `rust-regression-macos` did not exist in `pr-regression-gates.yml` until #1154 added them; this branch only started running them after merging main. One attribute, no logic change. The Windows path that actually reads the constant is unchanged.
|
Owner
Author
|
Heads-up: the required checks on The
This branch still runs the old workflow, so those checks never report here and the PR cannot merge as it is. What to do when this PR is ready to land: wait until PR #3185 is merged, then bring this branch up to date with |
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 the prerequisite for #1329 AC13.
What this implements
Issue #1343 asks for two things: deterministic parallel Rust workspace tests, and the repository-wide rustfmt baseline. Both are delivered.
Deterministic isolation for
cli_test_reset. The cross-process shared-state collision is fixed by a suite guard rather than by serialising the runner.ResetTestCoordinatorholds{ handle, owned, handle_open, wait_count, release_count, _not_send: PhantomData<Rc<()>> }; theRcmarker makes it!Send + !Sync, soReleaseMutexcan only run on the thread that owns the mutex, by construction rather than by convention. One parameterised constructor performs exactly oneCreateMutexW, capturesGetLastErrorin the immediately following statement, builds the guard withowned=falseandhandle_open=truebefore any wait, then performs exactly oneWaitForSingleObjectwith a finite 120000 ms timeout. Ownership is taken only forWAIT_OBJECT_0orWAIT_ABANDONED; every other result closes the handle.complete()releases at most once, closes, preserves both the primary and the secondary error, and leaves both flags false soDropis a no-op on the normal path.Droprepeats the same cleanup best effort without panicking and emitsAC1343_SUITE_CLEANUP_FAILEDon failure. The ten active Windows tests acquire the suite first and complete last.Deterministic Windows regression coverage for the five behaviours the issue names, against the real Win32 primitives and not only against injected adapters:
reset_mutex_cross_process,reset_test_coordinator_private_handoffreset_test_coordinator_private_handoff,reset_mutex_timeout_closes_unowned_handlereset_mutex_cross_processkills the holder and asserts the contender observesWAIT_ABANDONEDcomplete()paths,reset_mutex_complete_preserves_primary_cleanup_errorThe zero-depth probe runs its second acquirer on a fresh thread. A Windows mutex is reentrant for its owning thread, so without that the negative subcase could never produce
WAIT_TIMEOUT: it would have passed green while testing nothing.Repository-wide rustfmt baseline, restored in the parent commit
2c310e7eacross exactly the five paths the issue lists, and verified here to hold across the whole workspace.// SAFETY:proofs on every unsafe site: 8 insrc-tauri/src/testability/reset.rs, including theunsafe extern "system"block, and 30 insrc-tauri/tests/cli_test_reset.rs. The release and drop proofs state the same-thread ownership argument explicitly, because the!Sendmarker is what makes it hold. Both files also carry#![deny(clippy::undocumented_unsafe_blocks)]: the lint is allow-by-default, so without it the criterion would be satisfied once and unenforced afterwards, and unsafe site 39 could land uncommented withclippy -D warningsstill green. The attribute is module-scoped, and it was measured before adoption: it produces findings only inside these two files and drags nothing else in the workspace red.Test debt.
windows::reset_test_coordinator_childis allowlisted. It is not deferred debt: it runs on every run as a spawned process, and it carries#[ignore]only so libtest does not also run it in-process, where it would contend for the same named mutex as its own parent.src-tauri/Cargo.tomladds one line,"Win32_System_Pipes", to the existingwindows-sys 0.59feature array. No new dependency, no lockfile delta.Scope: the delivery plan was cut back to the issue objective
The certified R10 delivery plan accumulated a large verification apparatus around this objective. The user cut it back to what the issue actually asks for. The TCB machinery, the attestation records,
PreimplementationTrustAnchorV2, the systemd quarantine manager, the root-view owner/verifier/quarantine chain and the fourteen-rolescripts/Invoke-BoundOnce.ps1are discarded, not deferred, and none of them appears here in any form.The decision and its three verified findings are recorded on the issue: #1343 (comment)
Verification
Every gate below was executed, not estimated. Toolchain
cargo 1.97.1 (c980f4866 2026-06-30), Windows 11, ordinary parallel execution with no--test-threadsoverride anywhere.At this commit
cargo fmt --manifest-path src-tauri/Cargo.toml --all -- --checkcargo check --manifest-path src-tauri/Cargo.toml --workspacecargo clippy --manifest-path src-tauri/Cargo.toml --workspace --all-targets -- -D warningscargo test --manifest-path src-tauri/Cargo.toml --workspacenpm run test:debtB-testproduced 57 target results, allok, zero failures:src/lib.rs3462 passed / 0 failed / 23 ignored,cli_test_reset10 passed / 0 failed / 1 ignored, and bothcrates/members green.B-debtreports 27 ignored Rust tests discovered, 27 allowlisted, 0 unallowlisted, with no stale entries.The
cargo fmtcommand above spans the whole root workspace, not only the five paths the issue enumerates. Verified with-v: it visits every crate root of all three members,src-tauri/build.rsand bothsession-bridgebinaries included. A separate per-file sweep of all 228 tracked.rsfiles agrees.At the unmodified A cut
4f69e6b3Recorded as observed, in a clean dedicated worktree with a cold target directory.
ok;src/lib.rs3455 passed / 0 failed / 23 ignoredA-test passing is recorded as passing. The issue anticipates it: "Exact reruns at the A cut may pass depending on scheduling; no green run disproves the collision and no later-head failure may be relabelled as A evidence." No rerun was attempted to chase a red, and none is claimed. The historical
cli_test_resetcollision stays diagnostic evidence from a later head, never cut A.The lint was measured before it was adopted
Adding
#![deny(clippy::undocumented_unsafe_blocks)]produces findings only inside the two files that carry it: the first workspace-wide clippy run with the attribute in place emitted exactly 7 errors, all inreset.rs, and cited no other file. A negative control confirms it is live rather than decorative: removing one// SAFETY:comment fromcli_test_reset.rsfails clippy with exactly one error naming line 1 as the lint level source.The merge commit
mainmoved 66 commits since the A cut, and9c7c77e5(#1154) replacedtauri::Builder::default().any_thread()withagentscommander_lib::test_support::test_builder()in the same region ofsrc-tauri/tests/pty_powershell_managed_native.rsthat the formatter-only commit reformatted.mainwas merged in to resolve it, deliberately rather than rebased, so that8415854estays an ancestor and remains the reviewed prerequisite commit. The merge resolved exactly one conflict block by takingmain's call and keeping the rustfmt-correct wrapping of the following.manage(...); every other file auto-merged.Gate set re-run on the merged tree:
check0,clippy -D warnings0,test --workspace0 in 245s (57 target results allok,src/lib.rs3490 passed / 0 failed / 23 ignored,cli_test_reset10 / 0 / 1),npm run test:debt0.cargo fmt --checkon the merged tree exits 1, with 145 hunks across 26 files and zero in the five paths this issue covers. Every one of them is pre-existing drift inherited frommain, and none is authored by this branch. The number reconciles exactly with an independent measurement atmain8f272a76: 221 hunks over 31 files, minus the 76 in the five baseline paths that this PR fixes. Clearing the other 145 is explicitly outside this issue's Scope, which says to apply rustfmt to the baseline files only, and it is recorded as input to #1401 instead. See below.Not included
cargo fmt --checkgate. The repository has none today, so the restored baseline is currently protected by convention only. That is tracked separately rather than turned on inside this PR, because enabling it can redden branches already in flight.crates/terminal-snapshot-renderer/tests/fixtures/diagnostic-failure-harness/failures.rs, a nested fixture crate that declares its own[workspace]and is therefore unreachable bycargo fmt --all. Its blob is unchanged since the A cut, so it is out of scope here.auth::write_registryreplaces its registry throughReplaceFileWand fails deterministically when the working-directory path is long enough, which makes the terminal-snapshot acceptance fixture fail to construct. Pre-existing, unrelated to this change, no CI reproduction, tracked separately.