Two async-timing tests are flaky on CI: identical trees give different failures

[iceteaSA]

Two async-timing tests fail intermittently on CI. Same bytes in, different outcome each run — so a red check on a PR currently carries no information about the PR.

Evidence

Three CI runs of the same tree (commits differ only by amend; git diff between them is empty), PR #218:

run	commit	result
1	b90645b	FAIL — packages/e2e-tests › bridges back to a stale Opus cache after more than 20 Fable blocks
2	4dfbf7f	FAIL — packages/opencode › writes the manifest before clearing a migrated legacy handle
3	e2af7f6	PASS

Two different tests, then green, from identical source.

The two failures

bridges back to a stale Opus cache… — waitForSessionText gives up after ~36–42 s waiting for the Fable recovery window complete. Returning to Fable 5. notice (packages/opencode/src/index.ts:783). It passed on another CI run in 16 s, so the wait is racing notice delivery rather than observing a stuck one.

writes the manifest before clearing a migrated legacy handle (packages/opencode/src/tests/index.test.ts:2451) — asserts on stateAtManifestWrite, captured inside an fs.rename spy. It failed with Received value must be an array type…, i.e. the capture was still undefined: the startup migration had not reached its manifest write by the time getPlugin returned and the assertion ran.

Both are the same shape — an assertion racing detached async work, with no barrier tying the two together.

Not reproducible locally

writes the manifest… passes 5/5 in isolation and inside full-suite runs (1874/1874, repeatedly). The e2e test fails consistently on my machine — but it fails identically on a clean detached worktree at main, so that is a local environment issue, not a signal about either PR.

Suggested fix

Same treatment as #176: replace the implicit timing assumption with an explicit barrier. For the migration test, expose or await the startup migration's completion rather than relying on it finishing before getPlugin resolves. For the e2e test, wait on the notice being queued rather than on its rendered text, or raise the bound with a named timeout that says what it was waiting for.

Happy to take either if useful — filing first since both are pre-existing and neither is related to what I am working on.

[iceteaSA]

Follow-up with a firmer diagnosis on the e2e half — it is not a timeout that needs raising.

Raising waitForSessionText from 30s to 120s on bridges back to a stale Opus cache after more than 20 Fable blocks:

(fail) ... [127342.46ms]

The Fable recovery window complete. Returning to Fable 5. notice never arrives at all. The test is observing a real absence, not losing a race.

Version gap worth noting: CI pins opencode-ai@1.18.18; this host runs 1.18.29. The notice fails to arrive here deterministically (4/4 including the 120s run) and fails on CI in 3 of 4 observed runs, passing once in 16s. So the delivery is unreliable on the pinned version and appears absent on the newer one.

That points at the no-TUI desktop-notice path — the one that schedules delivery off session.idle and was last reworked for OpenCode 1.18 event changes in #162. If it has regressed again on newer hosts, the user-visible consequence is larger than a red check: a recovery-window restoration that silently never notifies anyone running without a TUI attached.

I have not chased it further because it is unrelated to what I am working on, but the fix is probably in that delivery path rather than in the test. Two things worth deciding separately: whether to bump the CI pin (11 patch versions behind), and whether the notice path needs the same treatment #162 got.

The other half of this issue — writes the manifest before clearing a migrated legacy handle — turned out to have a different cause and is fixed in #218: a mock.module in a new test leaked process-wide (mock.restore() does not undo it), so a stub answered another suite's OAuth exchange. That one is no longer flaky.

[iceteaSA]

Narrowed further. The delivery path is not broken in general — only the restoration notice is lost.

Instrumented the e2e to dump every notice that reaches the session:

"Fable content filter detected. Switched to Opus 4.8 for a 10-response recovery window…"   <- arrives
"Fable recovery window complete. Returning to Fable 5."                                    <- never arrives

And the plugin log shows the recovery window genuinely completing — remaining counts 9→0 with a Fable cache warmed line at each step, ending at remaining: 0. So the window closes, the warm settles, and notifyRestored should publish.

The asymmetry is the clue. The switch notice is published while more turns are still coming, so another session.idle follows it. The restoration notice is published ~2s after the final Opus turn — from inside warm.then(...) — by which time the session has already gone idle and nothing further will happen. Delivery is gated on desktopNoticeSafeSessions plus a probe, and something in that window leaves the notice pending with no later idle event to release it.

If that reading is right the user-visible bug is: after the last recovery turn, nobody is told the model was restored unless they happen to send another message. The test is the only thing that ever noticed, because a human would send another turn and see the state resolve anyway.

I stopped short of a fix. Two reasons: it is unrelated to the PRs I have open, and that path carries real hazards (an ignored notice becoming a retry parent and duplicating a billed provider turn — the #162 class), so it deserves its own change with its own review rather than being folded into a custody PR.

One cheap thing that would make the next attempt much easier: the desktop-notice path has no logging at all — grep -i desktop over a full failing run returns nothing. Tracing it requires instrumenting production code by hand. A couple of debug lines at queue and at flush-decision would have made this a five-minute diagnosis.

[iceteaSA]

A third flake class on this machine, distinct from the two already filed — manifest-lock and legacy-migration tests in packages/opencode, load-sensitive:

withCustodyManifestLock > reports a held lock as lock_busy
fallback Claustrum credential resolution > writes the manifest before clearing a migrated legacy handle
fallback Claustrum credential resolution > migrates a legacy handle into an empty manifest at startup
fallback Claustrum credential resolution > records manifest lock ownership during a migration write
fallback Claustrum credential resolution > re-migrates a legacy handle after a crash when the mode is already claustrum
fallback Claustrum credential resolution > preserves two concurrent legacy migrations in one manifest

Same-tree evidence, which is what separates this from a regression: one commit produced 1869 pass / 6 fail at load average 11.6, then 1875 pass / 0 fail on a quiet machine minutes later. All six pass in isolation. The failing subset varies run to run — 1, 2, 4 or 6 — which is the signature of a shared deadline rather than a shared bug.

They are lock-acquisition tests with ~100ms budgets, so under CPU contention the acquisition simply misses its window. Same family as the closed #176 (unbounded mf.ready under suite load) and the migration-state race already in this issue — a fixed timeout used as a synchronisation barrier.

Worth noting for anyone reading a red check here: a full-suite failure count on this repo is only meaningful alongside the machine's load. I nearly attributed these to my own change before running the parent commit, which failed more.

[ualtinok]

Resolved on main. The three independent failures were fixed by isolating Pi tests from host config (#219), making custody lock tests independent of wall-clock timing (#228), and fixing the real desktop-notice supersession race in 37b391b1. Final verification under Bun 1.3.14 passed Core (199), OpenCode (1,876), Pi (114), and all 36 E2E tests; the notice regression also passed repeated runs against OpenCode 1.18.18 and 1.18.30.