diff --git a/packages/core/src/loop/turn.ts b/packages/core/src/loop/turn.ts index 55361b3a..ebffa93f 100644 --- a/packages/core/src/loop/turn.ts +++ b/packages/core/src/loop/turn.ts @@ -1494,11 +1494,29 @@ export async function tryExpertRescue( // derivation only when no sticky identity exists (nothing has been recorded then, so // there's no inconsistency). If R4 is already recorded for this block, the expert has // already tried and failed on this exact block; escalate to R5 instead. - const block = + let block = (state.blockFingerprint ?? "") !== "" ? (state.blockFingerprint ?? "") : fingerprintFor(state, gateErrors); + // The expert is the LAST resort before parking. Callers OUTSIDE checkStuck reach here without + // its `escalation-N` guard — notably the read-only-spin park (session.ts) — so BOTH identity + // sources can be empty at once (observed live, build12: a near-green rollback reset errorAge so + // fingerprintFor derives nothing, and blockFingerprint was never set on this path). The old code + // then SKIPPED the expert and parked, wasting the rung. Derive a per-error-set identity from the + // failing error KEYS (the dialect fingerprintFor/samePersist use — NOT e.file, which would + // collapse distinct errors in one file). This local id is used for BOTH the R4 lookup below and + // the record-on-success — so the SAME error set skips (fires once) while a DIFFERENT set gets a + // distinct id and its own shot. It is deliberately NOT written to state.blockFingerprint: this + // fallback only fires on the settleGate-less callers (settleGate always has escalation-N), so + // persisting would only leave a stale id that makes a later, different stall skip the expert. + if (block === "") { + block = gateErrors + .map((e) => e.key) + .sort() + .join("|"); + } + if (block === "") { return skip("no block fingerprint computed"); } diff --git a/packages/core/tests/expert-rescue.test.ts b/packages/core/tests/expert-rescue.test.ts index c5b83567..a9b16881 100644 --- a/packages/core/tests/expert-rescue.test.ts +++ b/packages/core/tests/expert-rescue.test.ts @@ -289,6 +289,88 @@ describe("tryExpertRescue", () => { } }); + test("EMPTY fingerprint at exhaustion (post-rollback errorAge reset) → expert STILL fires, not skip+park", async () => { + // build12: a near-green rollback reset errorAge (so fingerprintFor derives nothing) and a + // frontier-advance had cleared blockFingerprint — so at ladder exhaustion the expert was + // skipped ("no block fingerprint computed; parking"), wasting the last-resort rung with + // rollback budget spent. The expert must fall back to a stable per-error-set identity and fire. + const dir = await mkdtemp(join(tmpdir(), "expert-rescue-empty-fp-")); + + try { + await Bun.write(join(dir, "x.ts"), "export const s = v as any;\n"); + const events: ILoopEvent[] = []; + const ctx = makeCtx(events, dir); + const state = freshState(); + + // The exact empty-fingerprint state: NO blockFingerprint, NO aged errors, NO history. + state.blockFingerprint = ""; + state.errorAge = new Map(); + state.recentGateFingerprints = []; + state.triedLeversByBlock = new Map(); + + const ask: ExpertAsk = async () => + "```ts\nexport const s = String(v);\n```"; + + const rescued = await tryExpertRescue( + ctx, + state, + [fileErr("x.ts")], + async () => ask + ); + + // The last-resort expert fired instead of skip+park. + expect(rescued).toBe(true); + expect(await Bun.file(join(dir, "x.ts")).text()).toBe( + "export const s = String(v);\n" + ); + // It did NOT bail with the empty-fingerprint skip. + expect( + toolMsgs(events).some((m) => m.includes("no block fingerprint")) + ).toBe(false); + // The fallback id is derived from the error KEYS and used for recording — but is NOT + // persisted to state (persisting would leave a stale id that skips a later DIFFERENT stall). + const derivedId = [fileErr("x.ts").key].sort().join("|"); + + expect(state.blockFingerprint ?? "").toBe(""); + expect(state.triedLeversByBlock.get(derivedId)?.has("R4")).toBe(true); + + // ONE-SHOT for the SAME error set: a second exhaustion (file reverted) SKIPS as already-tried. + await Bun.write(join(dir, "x.ts"), "export const s = v as any;\n"); + const rescued2 = await tryExpertRescue( + ctx, + state, + [fileErr("x.ts")], + async () => ask + ); + + expect(rescued2).toBe(false); + expect( + toolMsgs(events).some((m) => m.includes("already tried for this block")) + ).toBe(true); + + // But a DIFFERENT error set is NOT stale-skipped — it gets its own shot (the bug a sticky + // write would cause: the old id lingers and a new stall finds R4 under it and never fires). + await Bun.write(join(dir, "y.ts"), "export const t = w as any;\n"); + const askY: ExpertAsk = async () => + "```ts\nexport const t = String(w);\n```"; + const rescuedY = await tryExpertRescue( + ctx, + state, + [fileErr("y.ts")], + async () => askY + ); + + expect(rescuedY).toBe(true); + expect( + state.triedLeversByBlock + .get([fileErr("y.ts").key].sort().join("|")) + ?.has("R4") + ).toBe(true); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + test("no file-scoped error → skips with a visible reason", async () => { const events: ILoopEvent[] = []; const ctx = makeCtx(events, "/tmp");