From 8f500a553b1ed2016a2c310d2f10270711a8d3c9 Mon Sep 17 00:00:00 2001 From: Shivang Date: Thu, 27 Aug 2026 15:27:08 -0400 Subject: [PATCH] test: stop the voice budget test racing the CI scheduler VoiceToolSafetyTest > "approval spends the tool's budget rather than adding to it" has failed twice in four days, macOS only, never on Ubuntu or Windows: 47b61bc4 2026-08-24 failure (before any of the latency work) cacaba54 success ae378765 2026-08-27 failure ea28e085 success It is a genuine flake, not a regression from either merge, and it fails at the assertFailsWith line - no exception at all rather than the wrong one. Cause. The test sleeps delay(250) inside the approver, against an approval slice of min(APPROVAL_MS, 400) = 400 ms. That leaves ~150 ms of headroom to cover two withContext(Dispatchers.IO) hops and whatever the runner's scheduler is doing. On a loaded macOS box the headroom runs out, so withTimeoutOrNull around the approval returns null, confirmationRefusal answers with the refusal payload, the tool call never runs, nothing throws, and assertFailsWith fails. The test was asserting on the scheduler, not on the executor. Fix. CompositeVoiceToolExecutor already takes an injectable clock, added with the comment "Injected so a slow approval can be exercised without one" - the test just never used it. The approver now returns immediately while REPORTING that it spent 250 ms, which is exactly the input the property is about. The fixture gains a nowMs parameter to pass it through. Why this is now deterministic in the direction that matters: the approval reports 250 ms of a 400 ms budget, so the call runs under withTimeoutOrNull(150) while sleeping 250 ms. delay can overshoot but never finish early, so the timeout always wins. The remaining sleep is 250-vs-150, where overshoot only strengthens the assertion, rather than 250-vs-400 where overshoot inverted the outcome. The test keeps its full discriminating power: reintroducing the bug it exists for (hand the call a fresh budget instead of the remainder) makes it fail, verified by mutating `remainingMs = budgetMs - (nowMs() - startedAtMs)` to `= budgetMs`. Swept the rest of the voice tests for the same shape. The other delays are all delay(60_000) against small budgets, where overshoot can only make the timeout more certain. No other instance of a sleep sized close to the budget it races. Deliberately not verified by local load testing: the flake is macOS-CI-specific and a green loop on a developer machine is what made this look fine for months. CI is the only venue that can confirm it. --- .../compose/voice/VoiceToolSafetyTest.kt | 27 +++++++++++++++++-- .../compose/voice/VoiceToolSourceFixtures.kt | 7 +++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/compose-ui/src/desktopTest/kotlin/ai/rever/bossterm/compose/voice/VoiceToolSafetyTest.kt b/compose-ui/src/desktopTest/kotlin/ai/rever/bossterm/compose/voice/VoiceToolSafetyTest.kt index ff5b41e3..8080c862 100644 --- a/compose-ui/src/desktopTest/kotlin/ai/rever/bossterm/compose/voice/VoiceToolSafetyTest.kt +++ b/compose-ui/src/desktopTest/kotlin/ai/rever/bossterm/compose/voice/VoiceToolSafetyTest.kt @@ -728,17 +728,40 @@ class VoiceToolSafetyTest { */ @Test fun `approval spends the tool's budget rather than adding to it`() { + // Budget spent by the modal is SCRIPTED through the injected clock, not slept away. + // + // The original version slept `delay(250)` inside the approver against an approval + // slice of min(APPROVAL_MS, 400) = 400 ms, leaving ~150 ms of headroom for two + // `Dispatchers.IO` hops and the runner's scheduler. On a loaded macOS CI box that + // headroom ran out: the approval itself timed out, `confirmationRefusal` returned + // the refusal payload instead of the call ever running, no exception was thrown, + // and `assertFailsWith` failed. It went red twice in four days on macOS only, and + // passed everywhere else - a test asserting on the scheduler, not on the executor. + // + // With the clock scripted, the approver returns immediately while REPORTING that it + // spent 250 ms, which is precisely the input the property is about. + var clockMs = 0L val source = FakeToolSource( list = listOf(externalTool("git_discard")), - policy = VoiceToolPolicy(approve = { _, _ -> kotlinx.coroutines.delay(250); true }), + policy = VoiceToolPolicy(approve = { _, _ -> clockMs += 250L; true }), onCall = { _, _ -> kotlinx.coroutines.delay(250); """{"ok":true}""" }, ) - val exec = composite(FakeBaseExecutor(emptyList()), source, callTimeoutMs = 400L) + val exec = composite( + FakeBaseExecutor(emptyList()), + source, + callTimeoutMs = 400L, + nowMs = { clockMs }, + ) val failure = assertFailsWith { runBlocking { exec.execute("git_discard", noArgs, null) } } assertTrue(failure.message!!.contains("timed out"), failure.message!!) + // Why this is now deterministic in the direction that matters: the approval reports + // 250 ms of a 400 ms budget, so the call runs under `withTimeoutOrNull(150)` while + // sleeping 250 ms. `delay` can overshoot but never finish early, so the timeout + // always wins. Reintroduce the bug - hand the call a fresh 400 ms instead of the + // remaining 150 - and the 250 ms call completes, nothing throws, and this fails. } /** diff --git a/compose-ui/src/desktopTest/kotlin/ai/rever/bossterm/compose/voice/VoiceToolSourceFixtures.kt b/compose-ui/src/desktopTest/kotlin/ai/rever/bossterm/compose/voice/VoiceToolSourceFixtures.kt index d83637b6..703ef034 100644 --- a/compose-ui/src/desktopTest/kotlin/ai/rever/bossterm/compose/voice/VoiceToolSourceFixtures.kt +++ b/compose-ui/src/desktopTest/kotlin/ai/rever/bossterm/compose/voice/VoiceToolSourceFixtures.kt @@ -100,10 +100,17 @@ internal fun composite( gate: VoiceConfirmationGate = VoiceConfirmationGate(), enumerateTimeoutMs: Long = 1_000L, callTimeoutMs: Long = 1_000L, + /** + * The executor's clock. Pass a scripted one to make a budget test deterministic: a test + * that spends budget with a real `delay` is asserting on the CI runner's scheduler, not + * on the executor. + */ + nowMs: () -> Long = { System.currentTimeMillis() }, ) = CompositeVoiceToolExecutor( base = base, source = source, confirmations = gate, enumerateTimeoutMs = enumerateTimeoutMs, callTimeoutMs = { callTimeoutMs }, + nowMs = nowMs, )