test(custody): make manifest-lock contention tests independent of wall clock

[iceteaSA]

Six manifest-lock and legacy-migration tests fail under CPU contention and pass on a quiet machine. One of them (reports a held lock as lock_busy) now fails on CI and blocks unrelated PRs; filed as #220.

The coupling. In withCustodyManifestLock, deadline = startedAt + ttlMs is the contender's give-up deadline, while now() - ownerClaimedAtMs >= ttlMs is the holder's staleness threshold. One knob, two opposed roles. The test set ttlMs: 30, so a holder starved past one renewal tick looked stale and the contender evicted it — acquiring the lock instead of reporting busy, which is a semantic failure, not a slow test.

Raising the TTL does not fix it, and I tried: at 2s the contender still evicted under load, and the longer give-up path started tripping bun's 5s per-test timeout. Any fix expressed as a duration is the same defect one level down. These assertions are about ordering, not elapsed time.

The fix is a synthetic held owner plus a scripted injected clock, so "holder is fresh" and "contender exhausted its deadline" stop competing for one wall clock. The migration tests raced a different deadline — the 100ms startup warmup — and the concurrent one used 1s sleeps, now explicit barriers. Production code is untouched: the diff is two test files.

The failure mode changed class, which is the claim worth checking. Under 16 CPU hogs, six runs each:

before   4/2  3/3  0/6  1/5  4/2  1/5    assertion failures in every run (2,2,5,2,2,2)
after    6/0  5/1  6/0  6/0  5/1  6/0    assertion failures: 0 everywhere

After the change, every red is a watchdog kill and its ENOENT cleanup cascade — never an assertion. Quiet: 10/10 green, 1.04–30.42ms.

What this does not fix. Extreme oversubscription still trips bun's 5s watchdog on arbitrary tests, because the OS deschedules the test process. The evidence is a three-syscall lock_busy test reported at 7588ms — that is scheduler starvation, not a timing dependence in the code. It is a harness property and it is why the tally above is 5/6 rather than 6/6 under deliberate load. Relevant to #220: CI runners are 2-core, so a red check there deserves this reading before it is blamed on a diff.

Gates: core 188/0 · opencode 1869/0 · pi 114/0 · typecheck, format, biome clean.

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

---

Summary by cubic

Makes the manifest-lock and legacy-migration tests deterministic by removing wall-clock dependence, and isolates the packages/pi test suite from the host config directory. The old lock tests used the TTL as both the contender's give-up deadline and the holder's staleness threshold, so a starved holder was evicted instead of reporting lock_busy under CPU load (issue #220); the PI suite had no test preload and one test read the operator's live config, passing only when that file happened to exist.

• Replaces elapsed-time waits with a synthetic owner plus injected clock in the lock-busy test, and explicit barriers in the concurrent migration test.
• Adds a packages/pi test preload that points config env vars at a fresh temp dir per test; host values are deliberately not restored.
• Production code is untouched; only test files and packages/pi/bunfig.toml change.
• Does not fix bun's 5s watchdog kills under extreme oversubscription, so a CI red on a 2-core runner with ~7s reported times means scheduler starvation, not a semantic failure.

^{Written for commit 6a00dbb. Summary will update on new commits.}

[iceteaSA]

First CI run confirmed the fix on the real runner — all six target tests passed, including reports a held lock as lock_busy, which is the one that had been failing there.

The red was an unrelated pre-existing failure: Pi API fallback routing helpers > sticky-balanced preserves the strict API fallback gate after OAuth exhaustion. packages/pi has no test preload, so its suite reads the operator's live ~/.config/opencode/anthropic-auth.json and passes on a machine that happens to have one. That is fixed in #219, so I have rebased this branch on top of it. Both this PR and #218 now stack on #219.

Zero assertion failures and zero watchdog kills in that run — the classification this change is built around held on CI, not just under my synthetic load.

core 188/0 · opencode 1869/0 · pi 114/0 · typecheck clean.

[cubic-dev-ai]

2 issues found and verified against the latest diff

Confidence score: 4/5

• packages/opencode/src/tests/custody-handle-manifest.test.ts relies on the exact now() call count and ordering in the production retry loop, so harmless loop changes could make the test fail or obscure the behavior it intends to verify—use a controlled clock or assert outcomes rather than invocation order.
• packages/opencode/src/tests/index.test.ts hard-codes delay 100 in withoutClaustrumWarmupDeadline instead of using CLAUSTRUM_WARMUP_TIMEOUT_MS; changing the production constant could leave the real timeout active and make tests flaky or slower—reference the shared constant.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/opencode/src/tests/custody-handle-manifest.test.ts">

<violation number="1" location="packages/opencode/src/tests/custody-handle-manifest.test.ts:1659">
P2: This test only passes because of the exact count and order of `now()` invocations inside the production retry loop (startedAt, then stale-check before the two deadline checks), and at the decision moment the owner is already stale (30-0 >= ttl 30). A static owner record means the "holder remains fresh" scenario the PR describes (issue #220: fresh holder offset by renewal) is no longer exercised at all, and any refactor that samples `now()` once per loop iteration (e.g. `const current = now()`) makes iteration 2 see stale owner 30 -> evict -> lock acquired -> test fails with 'acquired', recreating exactly the flake class this PR removes. Recommend keeping a live holder: start a real `withCustodyManifestLock` renewal (renewalIntervalMs) on the same scripted clock so `writeOwner(now())` rewrites `claimed_at_ms` to fresh values that stay below the staleness threshold until after the contender's deadline throw, making the busy decision deterministic without depending on `now()` call order.</violation>
</file>

<file name="packages/opencode/src/tests/index.test.ts">

<violation number="1" location="packages/opencode/src/tests/index.test.ts:744">
P2: `withoutClaustrumWarmupDeadline` suppresses a `setTimeout` solely when its delay equals the magic number 100, but that deadline is defined by the production constant `CLAUSTRUM_WARMUP_TIMEOUT_MS` (packages/opencode/src/index.ts:953, used at :2648). The two are coupled only by an undocumented literal in the test. If that constant changes, this mock silently stops suppressing the wall-clock warmup deadline, and the timing flakiness this PR is removing silently returns while all tests still pass. It also no-ops every other 100ms timer in the plugin during the wrapped call, not just the warmup deadline. Export `CLAUSTRUM_WARMUP_TIMEOUT_MS` and compare against it instead of the literal 100, and gate the suppression on the timer being the warmup deadline rather than any 100ms timeout.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: This test only passes because of the exact count and order of now() invocations inside the production retry loop (startedAt, then stale-check before the two deadline checks), and at the decision moment the owner is already stale (30-0 >= ttl 30). A static owner record means the "holder remains fresh" scenario the PR describes (issue #220: fresh holder offset by renewal) is no longer exercised at all, and any refactor that samples now() once per loop iteration (e.g. const current = now()) makes iteration 2 see stale owner 30 -> evict -> lock acquired -> test fails with 'acquired', recreating exactly the flake class this PR removes. Recommend keeping a live holder: start a real withCustodyManifestLock renewal (renewalIntervalMs) on the same scripted clock so writeOwner(now()) rewrites claimed_at_ms to fresh values that stay below the staleness threshold until after the contender's deadline throw, making the busy decision deterministic without depending on now() call order.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/tests/custody-handle-manifest.test.ts, line 1659:

<comment>This test only passes because of the exact count and order of `now()` invocations inside the production retry loop (startedAt, then stale-check before the two deadline checks), and at the decision moment the owner is already stale (30-0 >= ttl 30). A static owner record means the "holder remains fresh" scenario the PR describes (issue #220: fresh holder offset by renewal) is no longer exercised at all, and any refactor that samples `now()` once per loop iteration (e.g. `const current = now()`) makes iteration 2 see stale owner 30 -> evict -> lock acquired -> test fails with 'acquired', recreating exactly the flake class this PR removes. Recommend keeping a live holder: start a real `withCustodyManifestLock` renewal (renewalIntervalMs) on the same scripted clock so `writeOwner(now())` rewrites `claimed_at_ms` to fresh values that stay below the staleness threshold until after the contender's deadline throw, making the busy decision deterministic without depending on `now()` call order.</comment>

<file context>
@@ -1655,27 +1655,30 @@ describe('withCustodyManifestLock', () => {
-      const firstEntered = Promise.withResolvers<void>()
-      const releaseFirst = Promise.withResolvers<void>()
+      const lockPath = `${path}.lock`
+      const times = [0, 29, 30]
+      let timeIndex = 0
+      await fs.mkdir(lockPath, { mode: 0o700 })
</file context>

[cubic-dev-ai]

P2: withoutClaustrumWarmupDeadline suppresses a setTimeout solely when its delay equals the magic number 100, but that deadline is defined by the production constant CLAUSTRUM_WARMUP_TIMEOUT_MS (packages/opencode/src/index.ts:953, used at :2648). The two are coupled only by an undocumented literal in the test. If that constant changes, this mock silently stops suppressing the wall-clock warmup deadline, and the timing flakiness this PR is removing silently returns while all tests still pass. It also no-ops every other 100ms timer in the plugin during the wrapped call, not just the warmup deadline. Export CLAUSTRUM_WARMUP_TIMEOUT_MS and compare against it instead of the literal 100, and gate the suppression on the timer being the warmup deadline rather than any 100ms timeout.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/tests/index.test.ts, line 744:

<comment>`withoutClaustrumWarmupDeadline` suppresses a `setTimeout` solely when its delay equals the magic number 100, but that deadline is defined by the production constant `CLAUSTRUM_WARMUP_TIMEOUT_MS` (packages/opencode/src/index.ts:953, used at :2648). The two are coupled only by an undocumented literal in the test. If that constant changes, this mock silently stops suppressing the wall-clock warmup deadline, and the timing flakiness this PR is removing silently returns while all tests still pass. It also no-ops every other 100ms timer in the plugin during the wrapped call, not just the warmup deadline. Export `CLAUSTRUM_WARMUP_TIMEOUT_MS` and compare against it instead of the literal 100, and gate the suppression on the timer being the warmup deadline rather than any 100ms timeout.</comment>

<file context>
@@ -734,6 +734,26 @@ async function getPlugin(
+  const setTimeoutImpl = ((
+    ...arguments_: Parameters<typeof globalThis.setTimeout>
+  ) =>
+    arguments_[1] === 100
+      ? ({ unref() {} } as ReturnType<typeof globalThis.setTimeout>)
+      : originalSetTimeout(...arguments_)) as typeof globalThis.setTimeout
</file context>

[iceteaSA]

Stack, for merge order. Four PRs came out of chasing three separate CI failures to root cause; each fix is in its own PR rather than folded into the custody change.

#219  fix(pi): isolate the test suite from the host config   <- base
 ├── #228  test(custody): lock tests independent of wall clock
 ├── #229  fix(recovery): deliver the restoration notice
 └── #218  fix(custody): carry the vault credential id        <- stacked on #228

Suggested order: #219 → #228 → #229 → #218. Each rebases cleanly on the previous.

This PR: stacked on #219. Green. Fixes the manifest-lock flake that was blocking #218.

The three CI failures were unrelated to each other: a Pi suite reading the operator's live config (#219), manifest-lock tests racing a wall-clock TTL (#228), and a genuine product bug where the model-restored notice never reaches a session with no TUI attached (#229).