mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-21 02:06:43 +00:00
* test: serialize concurrent vitest mock resolution in the non-isolated runner Root-causes the subagent-orphan-recovery flake (main run 29566318167, shard agentic-agents-core-subagents): "includes last human message in resume when available" and "adds config change hint..." received the bare resume template because prod code called a different readSessionMessagesAsync mock instance than the one the test configured. Mechanism: vitest's BareModuleMocker.resolveMocks has no in-flight guard. pendingIds is cleared only after all parallel resolveId RPCs settle, and every registration re-invalidates the mock module node. In a shared isolate:false worker, leaked async work from an earlier file (a real-timer dynamic import) triggers a fetchModule while the next file's vi.mock registrations are still pending, starting a second concurrent resolution pass over the same pendingIds. The slower pass re-registers each manual mock and wipes the already-evaluated mock module mid-import-chain: importers evaluated before the wipe (the test file's own binding, subagent-orphan-recovery.test.ts:8) keep factory instance A while later importers (subagent-orphan-recovery.ts via test line 16) instantiate a fresh instance B. Only inline-factory mocks split; vi.hoisted-stable mocks are immune, matching the CI failure signature exactly. Fix: OpenClawNonIsolatedRunner installs a coalescing wrapper around moduleRunner.mocker.resolveMocks (once per worker) so concurrent callers share one pass and registration happens exactly once before any awaiting import proceeds. Proof: - Deterministic repro (poison file leaving a 1ms recurring dynamic-import timer + resolveId jitter on session-transcript-readers) reproduced the exact CI assertion failures at lines 755/772 on the unfixed runner and passes 5/5 with the fix. - New unit tests cover coalescing, idempotent install, and fresh passes. - Stress: 10x subagent-orphan-recovery.test.ts and 5x the full agentic-agents-core-subagents file set (63 files, 1220 tests) at OPENCLAW_VITEST_MAX_WORKERS=6, all green. * test: requeue mock ids queued during an in-flight resolveMocks pass Review follow-up to the resolveMocks serialization pin: upstream snapshots the pendingIds contents at pass start and reassigns the static to [] at the end, so ids queued during the pass's RPC window land in the abandoned array. Pure coalescing would silently drop those registrations (upstream's racy second pass previously processed them). The wrapper now captures the queue reference before each pass, requeues ids pushed past the processed count, and drains until the queue is empty, so every coalesced caller's vi.mock registration is applied before its fetch proceeds. Proof: new unit test "registers ids queued while a pass is in flight before callers proceed"; deterministic zombie+jitter repro still passes 5/5; full agentic-agents-core-subagents file set green at OPENCLAW_VITEST_MAX_WORKERS=6. * test: chain resolveMocks passes per caller instead of sharing one pass The coalescing pin regressed shard agentic-agents-core-auth (models-config.providers.auth-provenance.test.ts, runs 29570918219 and 29571513381): sharing one in-flight pass broke the mocker's freshness invariant. Upstream gives every resolveMocks caller a pass that starts at or after its call, so ids the caller queued (vi.doUnmock before dynamic imports in that file's beforeAll/beforeEach) are always registered before its fetch proceeds. A coalesced caller could instead ride a pass snapshotted before its ids were queued and import with mock state unresolved. Reproduced locally on a cold transform cache: the file then loaded the real provider-auth warm worker plus a live oauth-manager refresh guarded by a 120s withRefreshCallTimeout (the 105-142s stalls and the "Test timed out in 120000ms" CI failure), and the real resolveProviderSyntheticAuthWithPlugin returned undefined (the mode:"none" assertion failure at line 348). The wrapper now chains each caller onto its own sequential pass. This keeps both invariants: serialization (a pass queued behind an in-flight one sees the cleared queue and no-ops, so a snapshot is never registered or its mock modules invalidated twice - the original subagent-orphan-recovery disease) and freshness (each caller's pass starts at or after its call). Abandoned-id requeue is preserved so ids pushed during a pass's RPC window are registered by the next chained pass instead of dropped. Proof: - Cold-cache auth shard (37 files, 528 tests): failed 105-142s with the coalescing pin (also with a coalescing-only variant), passed 20s with the unfixed runner, passes 3/3 in ~20-30s with the chained pin. - Long-timer instrumentation pinpointed the stall: provider-auth warm worker spawn plus oauth-manager withRefreshCallTimeout delay=120000. - Original zombie+jitter orphan-recovery repro still passes 5/5. - Warm agentic-agents-core-subagents (63 files, 1220 tests) and auth shard green at OPENCLAW_VITEST_MAX_WORKERS=6; wrapper unit tests updated to assert serialization, freshness, requeue, idempotent install.
98 lines
3.6 KiB
TypeScript
98 lines
3.6 KiB
TypeScript
// Guards the resolveMocks serialization pin: passes run sequentially so a
|
|
// drained snapshot is never registered (and its mock modules never
|
|
// invalidated) twice, while every caller's pass starts at or after its call so
|
|
// previously queued ids are registered before the caller's fetch proceeds.
|
|
import { describe, expect, it } from "vitest";
|
|
import { serializeMockerResolveMocks } from "./non-isolated-runner.js";
|
|
|
|
// Mirrors BareModuleMocker.resolveMocks: snapshots the static queue's contents
|
|
// at pass start, awaits its RPCs, then reassigns the static to [] so ids
|
|
// pushed during the await land in the abandoned array.
|
|
class FakeMocker {
|
|
static pendingIds: unknown[] = [];
|
|
passes = 0;
|
|
active = 0;
|
|
maxConcurrentPasses = 0;
|
|
processed: unknown[] = [];
|
|
|
|
async resolveMocks(): Promise<void> {
|
|
if (FakeMocker.pendingIds.length === 0) {
|
|
return;
|
|
}
|
|
this.active += 1;
|
|
this.maxConcurrentPasses = Math.max(this.maxConcurrentPasses, this.active);
|
|
this.passes += 1;
|
|
const snapshot = [...FakeMocker.pendingIds];
|
|
// Simulate the parallel resolveId RPC round-trips inside one pass.
|
|
await new Promise((resolve) => {
|
|
setTimeout(resolve, 1);
|
|
});
|
|
this.processed.push(...snapshot);
|
|
FakeMocker.pendingIds = [];
|
|
this.active -= 1;
|
|
}
|
|
}
|
|
|
|
describe("serializeMockerResolveMocks", () => {
|
|
it("serializes concurrent callers and never re-registers a drained snapshot", async () => {
|
|
FakeMocker.pendingIds = ["mock-a", "mock-b"];
|
|
const mocker = new FakeMocker();
|
|
serializeMockerResolveMocks(mocker);
|
|
|
|
await Promise.all([mocker.resolveMocks(), mocker.resolveMocks(), mocker.resolveMocks()]);
|
|
|
|
expect(mocker.maxConcurrentPasses).toBe(1);
|
|
// Later chained passes see the cleared queue and no-op instead of
|
|
// re-registering (and re-invalidating) the same snapshot.
|
|
expect(mocker.passes).toBe(1);
|
|
expect(mocker.processed).toEqual(["mock-a", "mock-b"]);
|
|
expect(FakeMocker.pendingIds).toEqual([]);
|
|
});
|
|
|
|
it("registers ids queued while a pass is in flight before the later caller resolves", async () => {
|
|
FakeMocker.pendingIds = ["mock-a"];
|
|
const mocker = new FakeMocker();
|
|
serializeMockerResolveMocks(mocker);
|
|
|
|
const first = mocker.resolveMocks();
|
|
// Upstream would abandon this push when it reassigns pendingIds to [];
|
|
// the wrapper must requeue it and the second caller's own chained pass
|
|
// must register it before that caller proceeds with its fetch.
|
|
FakeMocker.pendingIds.push("mock-late");
|
|
const second = mocker.resolveMocks();
|
|
await second;
|
|
|
|
expect(mocker.processed).toEqual(["mock-a", "mock-late"]);
|
|
expect(mocker.maxConcurrentPasses).toBe(1);
|
|
await first;
|
|
expect(FakeMocker.pendingIds).toEqual([]);
|
|
});
|
|
|
|
it("does not double-wrap when installed repeatedly", async () => {
|
|
FakeMocker.pendingIds = ["mock-a"];
|
|
const mocker = new FakeMocker();
|
|
serializeMockerResolveMocks(mocker);
|
|
// Identity check: a second install must keep the first wrapper in place.
|
|
const wrapped: unknown = Reflect.get(mocker, "resolveMocks");
|
|
serializeMockerResolveMocks(mocker);
|
|
|
|
expect(Reflect.get(mocker, "resolveMocks")).toBe(wrapped);
|
|
await mocker.resolveMocks();
|
|
expect(mocker.passes).toBe(1);
|
|
});
|
|
|
|
it("allows a fresh pass after the previous one settles", async () => {
|
|
FakeMocker.pendingIds = ["mock-a"];
|
|
const mocker = new FakeMocker();
|
|
serializeMockerResolveMocks(mocker);
|
|
await mocker.resolveMocks();
|
|
|
|
FakeMocker.pendingIds = ["mock-b"];
|
|
await mocker.resolveMocks();
|
|
|
|
expect(mocker.passes).toBe(2);
|
|
expect(mocker.processed).toEqual(["mock-a", "mock-b"]);
|
|
expect(FakeMocker.pendingIds).toEqual([]);
|
|
});
|
|
});
|