runtime-broker: remaining test-oracle gaps and the reconcile=true fan-out decision from PR #13214
Why this issue exists
PR #13214 (issue #13183 hardening) went through three bot review rounds and five self-audit rounds. Under this repo's ~5-review-round rule the last wave landed Critical fixes only, so four items were deferred. None is a known production defect: three are test oracles that cannot currently fail for the property they are named after, and one is a design decision that needs a client-side change.
1. stalledRenewalTickDoesNotDeadlockClose's 5 s bound cannot fail
packages/sdk-java/runtime-broker/src/test/java/com/alibaba/qwen/code/runtimebroker/Issue13183AdversarialTest.java
The test measures closeElapsed < 5s, but RuntimeBrokerService.close() never joins: it is closed.compareAndSet → four cancel(false) map sweeps → scheduler.shutdownNow() → renewalScheduler.shutdownNow() → provisioner.close(). awaitTermination has no call site in the module's main sources, so the measured time is bounded by two non-blocking calls no matter where the renewal thread is parked. The fake stall is also interruptible — shutdownNow() interrupts it and the stub's catch only re-sets the flag — so adding a natural renewalScheduler.awaitTermination(...) to close() would leave this test green at ~12 ms. Unlike its two siblings, it also never records which pool the parked thread came from, so nothing ties the stall to the renewal pool its name claims.
Fix: make the park survive interruption (model a wedged JDBC read), and record the thread name, asserting qwen-runtime-broker-lease-renewal.
2. The LOST drain's per-pass claim renewal is not pinned
Issue13183RegressionTest.lostReclaimDrainsAWholeGeneration
The test pins the "loops past three bounded passes" half of its javadoc but not the "under a renewed claim" half. reclaimService(...) builds the service with a 3 s operation lease and the drain is 303 ACQUIRING sessions against LIMIT 100 … FOR UPDATE per pass — four passes costing tens of milliseconds in H2, so hoisting renewRecoveryClaim(claimed) out of the loop leaves the test green. In production (30 s lease, MySQL over the network, 1600 sessions = 16 locked passes) the claim lapses mid-drain and recoverLostDrained throws runtime_provision_fenced: the warm answers 503 and the LOST generation is not reclaimed, which is what the loop exists to prevent.
Fix: wrap the binding repository in the file's own DelegatingBindingRepository, record expected.getOperationLeaseUntil() for each recoverLost call on the LOST binding, and assert the sequence advances (more than one distinct deadline).
3. startResponsesNeverServeTheCooldownCache asserts only the bypass half
RuntimeBrokerHttpServerTest
Nothing pins that the preceding cooled GET actually armed the cooldown, so the test cannot tell "armed and bypassed" from "never armed". Narrowing stampUnknownLookupCooldown to lookups that leave the record UNKNOWN keeps it green, while every plain GET over an executing execution fans through to the worker again — the poll shape #13183 was filed for. The oracle is also time-bounded: a stall longer than the 1 s cooldown between the GET and the :start retry silently disarms it.
Fix: assert the worker status-call count does not grow across the cooled GET (or read the cooldown map through a package-visible accessor), so "never armed" fails independently of the bypass assertion.
4. The observation cooldown cannot reach the shipped repeated-UNKNOWN poller
Decision needed, not just a patch. The only in-tree caller that tolerates repeated UNKNOWN observations is packages/cli/src/serve/hosted-workspace-broker.ts, which builds waitForUnknown ? path + '?reconcile=true' : path and loops every 250 ms. reconcile=true bypasses the cooldown by design — that is what makes an explicit reconcile authoritative — so the fan-out on that path is unchanged. Measured over a real MCP turn with an execution made UNKNOWN for real: ≈1.4 worker status calls/s on both the patched and the pre-patch broker, about 900 status calls per execution over a 630 s window.
Options:
- apply a shorter cooldown to
reconcile=true as well, and pin the trade-off (an explicit reconcile can then answer up to that much stale);
- back off in the client, which keeps the broker's contract intact;
- leave it and keep the design doc's statement that the cooldown covers automatic observation only (currently the case).
Context
Found by the bot's round-3 review (R3-10, R3-11, R3-12, R3-3) and confirmed by a three-arm verification rig on PR #13214. Deferred there under the round cap; recorded in the PR thread so nothing is silently dropped.
runtime-broker: remaining test-oracle gaps and the reconcile=true fan-out decision from PR #13214
Why this issue exists
PR #13214 (issue #13183 hardening) went through three bot review rounds and five self-audit rounds. Under this repo's ~5-review-round rule the last wave landed Critical fixes only, so four items were deferred. None is a known production defect: three are test oracles that cannot currently fail for the property they are named after, and one is a design decision that needs a client-side change.
1.
stalledRenewalTickDoesNotDeadlockClose's 5 s bound cannot failpackages/sdk-java/runtime-broker/src/test/java/com/alibaba/qwen/code/runtimebroker/Issue13183AdversarialTest.javaThe test measures
closeElapsed < 5s, butRuntimeBrokerService.close()never joins: it isclosed.compareAndSet→ fourcancel(false)map sweeps →scheduler.shutdownNow()→renewalScheduler.shutdownNow()→provisioner.close().awaitTerminationhas no call site in the module's main sources, so the measured time is bounded by two non-blocking calls no matter where the renewal thread is parked. The fake stall is also interruptible —shutdownNow()interrupts it and the stub's catch only re-sets the flag — so adding a naturalrenewalScheduler.awaitTermination(...)toclose()would leave this test green at ~12 ms. Unlike its two siblings, it also never records which pool the parked thread came from, so nothing ties the stall to the renewal pool its name claims.Fix: make the park survive interruption (model a wedged JDBC read), and record the thread name, asserting
qwen-runtime-broker-lease-renewal.2. The LOST drain's per-pass claim renewal is not pinned
Issue13183RegressionTest.lostReclaimDrainsAWholeGenerationThe test pins the "loops past three bounded passes" half of its javadoc but not the "under a renewed claim" half.
reclaimService(...)builds the service with a 3 s operation lease and the drain is 303 ACQUIRING sessions againstLIMIT 100 … FOR UPDATEper pass — four passes costing tens of milliseconds in H2, so hoistingrenewRecoveryClaim(claimed)out of the loop leaves the test green. In production (30 s lease, MySQL over the network, 1600 sessions = 16 locked passes) the claim lapses mid-drain andrecoverLostDrainedthrowsruntime_provision_fenced: the warm answers 503 and the LOST generation is not reclaimed, which is what the loop exists to prevent.Fix: wrap the binding repository in the file's own
DelegatingBindingRepository, recordexpected.getOperationLeaseUntil()for eachrecoverLostcall on the LOST binding, and assert the sequence advances (more than one distinct deadline).3.
startResponsesNeverServeTheCooldownCacheasserts only the bypass halfRuntimeBrokerHttpServerTestNothing pins that the preceding cooled
GETactually armed the cooldown, so the test cannot tell "armed and bypassed" from "never armed". NarrowingstampUnknownLookupCooldownto lookups that leave the recordUNKNOWNkeeps it green, while every plain GET over anexecutingexecution fans through to the worker again — the poll shape #13183 was filed for. The oracle is also time-bounded: a stall longer than the 1 s cooldown between the GET and the:startretry silently disarms it.Fix: assert the worker status-call count does not grow across the cooled GET (or read the cooldown map through a package-visible accessor), so "never armed" fails independently of the bypass assertion.
4. The observation cooldown cannot reach the shipped repeated-UNKNOWN poller
Decision needed, not just a patch. The only in-tree caller that tolerates repeated UNKNOWN observations is
packages/cli/src/serve/hosted-workspace-broker.ts, which buildswaitForUnknown ? path + '?reconcile=true' : pathand loops every 250 ms.reconcile=truebypasses the cooldown by design — that is what makes an explicit reconcile authoritative — so the fan-out on that path is unchanged. Measured over a real MCP turn with an execution made UNKNOWN for real: ≈1.4 workerstatuscalls/s on both the patched and the pre-patch broker, about 900 status calls per execution over a 630 s window.Options:
reconcile=trueas well, and pin the trade-off (an explicit reconcile can then answer up to that much stale);Context
Found by the bot's round-3 review (R3-10, R3-11, R3-12, R3-3) and confirmed by a three-arm verification rig on PR #13214. Deferred there under the round cap; recorded in the PR thread so nothing is silently dropped.