mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 21:07:01 -06:00
fix(pi-runner): flush blocks after compaction retry (#85288) (thanks @spacegeologist)
Behavior addressed: Embedded PI compaction retry now drains block replies again after the retry wait resolves, so retry-generated replies are not left behind while preserving aggregate-timeout fallback behavior. Real environment tested: local OpenClaw focused Pi runner test shard plus contributor local live-output proof in the PR body. Exact steps or command run after this patch: pnpm test src/agents/pi-embedded-runner/run/attempt.spawn-workspace.context-engine.test.ts src/agents/pi-embedded-runner/run/compaction-retry-aggregate-timeout.test.ts; .agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main Evidence after fix: 2 test files passed, 55 tests passed; final autoreview clean with no accepted/actionable findings. Observed result after fix: the runner flushes before the compaction wait, waits for compaction retry, then performs a second idempotent flush when the wait resolves without timing out. What was not tested: fresh external-channel live retry by this agent; PR retains contributor live-output proof for the delayed channel adapter path. Thanks @spacegeologist. Co-authored-by: zhengzuo0-ai <zheng.zuo0@gmail.com>
This commit is contained in:
@@ -211,6 +211,32 @@ describe("runEmbeddedAttempt context engine sessionKey forwarding", () => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("flushes block replies again after compaction retry wait resolves", async () => {
|
||||
const order: string[] = [];
|
||||
let flushCount = 0;
|
||||
const onBlockReplyFlush = vi.fn(async () => {
|
||||
flushCount += 1;
|
||||
order.push(`flush-${flushCount}`);
|
||||
});
|
||||
hoisted.waitForCompactionRetryWithAggregateTimeoutMock.mockImplementation(async () => {
|
||||
order.push("retry-wait");
|
||||
return { timedOut: false };
|
||||
});
|
||||
|
||||
await createContextEngineAttemptRunner({
|
||||
contextEngine: createContextEngineBootstrapAndAssemble(),
|
||||
sessionKey,
|
||||
tempPaths,
|
||||
attemptOverrides: {
|
||||
onBlockReplyFlush,
|
||||
},
|
||||
});
|
||||
|
||||
expect(onBlockReplyFlush).toHaveBeenCalledTimes(2);
|
||||
expect(hoisted.waitForCompactionRetryWithAggregateTimeoutMock).toHaveBeenCalledTimes(1);
|
||||
expect(order).toEqual(["flush-1", "retry-wait", "flush-2"]);
|
||||
});
|
||||
|
||||
it("enables Tool Search controls for embedded PI runs when configured", async () => {
|
||||
await createContextEngineAttemptRunner({
|
||||
contextEngine: {
|
||||
|
||||
@@ -32,6 +32,8 @@ type AcquireSessionWriteLockFn =
|
||||
typeof import("../../session-write-lock.js").acquireSessionWriteLock;
|
||||
type ShouldPreemptivelyCompactBeforePromptFn =
|
||||
typeof import("./preemptive-compaction.js").shouldPreemptivelyCompactBeforePrompt;
|
||||
type WaitForCompactionRetryWithAggregateTimeoutFn =
|
||||
typeof import("./compaction-retry-aggregate-timeout.js").waitForCompactionRetryWithAggregateTimeout;
|
||||
|
||||
type SubscriptionMock = ReturnType<SubscribeEmbeddedPiSessionFn>;
|
||||
type UnknownMock = Mock<(...args: unknown[]) => unknown>;
|
||||
@@ -92,6 +94,7 @@ type AttemptSpawnWorkspaceHoisted = {
|
||||
(sessionKey: string | undefined, config: unknown) => number | undefined
|
||||
>;
|
||||
limitHistoryTurnsMock: Mock<<T>(messages: T, limit: number | undefined) => T>;
|
||||
waitForCompactionRetryWithAggregateTimeoutMock: Mock<WaitForCompactionRetryWithAggregateTimeoutFn>;
|
||||
preemptiveCompactionCalls: Parameters<ShouldPreemptivelyCompactBeforePromptFn>[0][];
|
||||
systemPromptOverrideTexts: string[];
|
||||
sessionManager: SessionManagerMocks;
|
||||
@@ -191,6 +194,10 @@ const hoisted = vi.hoisted((): AttemptSpawnWorkspaceHoisted => {
|
||||
const limitHistoryTurnsMock = vi.fn<<T>(messages: T, limit: number | undefined) => T>(
|
||||
(messages) => messages,
|
||||
);
|
||||
const waitForCompactionRetryWithAggregateTimeoutMock =
|
||||
vi.fn<WaitForCompactionRetryWithAggregateTimeoutFn>(async () => ({
|
||||
timedOut: false,
|
||||
}));
|
||||
const preemptiveCompactionCalls: Parameters<ShouldPreemptivelyCompactBeforePromptFn>[0][] = [];
|
||||
const systemPromptOverrideTexts: string[] = [];
|
||||
const sessionManager = {
|
||||
@@ -234,6 +241,7 @@ const hoisted = vi.hoisted((): AttemptSpawnWorkspaceHoisted => {
|
||||
detectAndLoadPromptImagesMock,
|
||||
getHistoryLimitFromSessionKeyMock,
|
||||
limitHistoryTurnsMock,
|
||||
waitForCompactionRetryWithAggregateTimeoutMock,
|
||||
preemptiveCompactionCalls,
|
||||
systemPromptOverrideTexts,
|
||||
sessionManager,
|
||||
@@ -790,10 +798,9 @@ vi.mock("../utils.js", () => ({
|
||||
}));
|
||||
|
||||
vi.mock("./compaction-retry-aggregate-timeout.js", () => ({
|
||||
waitForCompactionRetryWithAggregateTimeout: async () => ({
|
||||
timedOut: false,
|
||||
aborted: false,
|
||||
}),
|
||||
waitForCompactionRetryWithAggregateTimeout: (
|
||||
...args: Parameters<WaitForCompactionRetryWithAggregateTimeoutFn>
|
||||
) => hoisted.waitForCompactionRetryWithAggregateTimeoutMock(...args),
|
||||
}));
|
||||
|
||||
vi.mock("./compaction-timeout.js", () => ({
|
||||
@@ -975,6 +982,9 @@ export function resetEmbeddedAttemptHarness(
|
||||
hoisted.runContextEngineMaintenanceMock.mockReset().mockResolvedValue(undefined);
|
||||
hoisted.getHistoryLimitFromSessionKeyMock.mockReset().mockReturnValue(undefined);
|
||||
hoisted.limitHistoryTurnsMock.mockReset().mockImplementation((messages) => messages);
|
||||
hoisted.waitForCompactionRetryWithAggregateTimeoutMock
|
||||
.mockReset()
|
||||
.mockResolvedValue({ timedOut: false });
|
||||
hoisted.preemptiveCompactionCalls.length = 0;
|
||||
hoisted.systemPromptOverrideTexts.length = 0;
|
||||
hoisted.sessionManager.getLeafEntry.mockReset().mockReturnValue(null);
|
||||
|
||||
@@ -4483,6 +4483,11 @@ export async function runEmbeddedAttempt(
|
||||
`proceeding with pre-compaction state runId=${params.runId} sessionId=${params.sessionId}`,
|
||||
);
|
||||
}
|
||||
} else if (onBlockReplyFlush) {
|
||||
// Retry-generated blocks can still be draining when the compaction
|
||||
// retry wait resolves; this second drain is idempotent when no new
|
||||
// blocks were produced.
|
||||
await onBlockReplyFlush();
|
||||
}
|
||||
} catch (err) {
|
||||
if (isRunnerAbortError(err)) {
|
||||
|
||||
Reference in New Issue
Block a user