style: rustfmt the moved-doc-path assertion

Merged
BN
6d ago
+57-9

cargo fmt --all --check (a rust-lint step) fails on main: one hunk in crates/preloop-cli/src/init.rs. The assertion #647 added for the relocated setup-reference page fits on one line, so rustfmt wants it wrapped. #647 landed behind [skip ci], which is why the lint step never saw it.

$ cargo fmt --all --check      # at a0409afdf
Diff in crates/preloop-cli/src/init.rs:4243:
-        assert!(text.contains("docs/control-plane/operate/deploy.mdx"), "{text}");
+        assert!(
+            text.contains("docs/control-plane/operate/deploy.mdx"),
+            "{text}"
+        );

Whitespace only, produced by cargo fmt --all. With it applied, cargo fmt --all --check is clean and cargo check --locked -p preloop-cli --all-targets passes.

Found while merging main into #634: main carried three such breakages — this hunk, the same cargo fmt --all --check failure in github_setup.rs, and two tests in github_setup.rs that no longer compiled because save_app_credentials takes &SecretString. The other two landed in #634's squash; this one was pushed three minutes after #634 was merged, so it stranded on the merged branch and is opened here on its own.


Also fixes the agentenv_provider shard-2 flake

preloop-vm::agentenv_provider::different_trees_populate_without_waiting_for_each_other was failing rust shard 2. It is a race in the test's fake aenv, which keeps its volume and sandbox tables in shared files that concurrent invocations mutate — the engine behaves correctly:

  • volume create appended to the volumes table while volume delete rewrote it (grep -v … > tmp, then mv). A rewrite that read the table before another flow's append wrote back a table without that volume, so the flow's next volume inspect answered not found and its start failed with read-only volume … is not ready after populate. Widening the window between that read and write reproduces it 2/2 deterministically, and the fixture's own call log shows no read-only volume was ever deleted.
  • The sandbox delete rewrite used one fixed aenv.sandboxes.tmp for every invocation, so two concurrent deletes collided: mv: … aenv.sandboxes.tmp: No such file or directory (operation delete). That is what remained after locking only the volumes table.

Both writers now take a lock directory — a new $0.volume-lock for the volumes table and the existing $0.lock for the sandbox and mount tables. The overlap probe stays outside the lock, so the test still proves two trees populate concurrently; one global populate lock would never overlap and the test would fail on its own assertion instead.

The third flake was the probe itself: <exe>.overlap was written only when the second create's mkdir landed inside the first's sleep 1 hold, so a stagger over a second recorded nothing although both populates were in flight (that is the agentenv_provider.rs:1116 panic, the overlap assertion — not the lost-row path). Each create now publishes an arrival marker and waits (bounded at 20 s) for a second create, so the marker records a fact rather than a window; the marker is dropped under an EXIT INT TERM trap so no exit path leaves one behind for a later create to count.

One more window, from CodeRabbit's review: volume delete rewrote the table under $0.volume-lock alone, so a concurrent start could validate a volume under $0.lock and then lose it before the mount row was written. volume delete now takes $0.lock first — the same order start takes it — and releases both on every exit path. Acquisition order is fixed by construction: start and sandbox delete take only $0.lock, volume create only $0.volume-lock, and volume delete is the sole path taking both, always $0.lock then $0.volume-lock, so no cycle exists.

Verified on c9ccd933e: cargo nextest run --locked --workspace --partition hash:2/4 802/802, the target test 20/20 in isolation (0.85–1.44 s each, against ~2.5 s before, since the fixed sleeps are gone), preloop-vm --test agentenv_provider 37/37, --test agentenv_volume_gc 13/13, cargo fmt --all --check clean, cargo clippy -p preloop-vm --all-targets -- -D warnings clean. Negative control: holding one lock across the probe — what a global populate lock would do — still fails the same strict assertion deterministically, 116.7 s, so the rendezvous property is intact.

Verified independently by AgentenvDeterminism against these heads: base a98e769e9 lost or duplicated volume rows in 6/40 rounds and broke 40/40 concurrent sandbox deletes; with the locks, 0/40 on both, no deadlock, 160 concurrent invocations clean.

Left out deliberately: wip-agentenv-det.patch also replaces agentenv_volume_gc.rs's sleep 3 .slow-exec parking with a test-written release file. Same class, different test file, not implicated in the shard-2 failure — happy to take it as a follow-up if wanted rather than widen this pull request.


Devin Review


Summary by cubic

Fixes cargo fmt --all --check failing on main and a race in the AgentENV test fixture that made a parallel populate test flaky.

  • Reformats the moved-doc-path assertion in crates/preloop-cli/src/init.rs; whitespace only, no behavior change.
  • Serializes the fixture's volume table create/delete operations behind a lock directory so a concurrent rewrite can't drop an appended volume; the overlap probe stays outside the lock, so the test still proves concurrent population.
  • Includes an empty commit to re-dispatch checks after the original head was pushed while the engine was not dispatching.

Summary by CodeRabbit

  • Tests
    • Reformatted a test assertion; expected output and behavior are unchanged.