fix(ralph-loop): add inFlight guard and improve completion detection to prevent infinite loops

Closes #2084
This commit is contained in:
YeonGyu-Kim 2026-02-26 20:58:55 +09:00
parent 0516f2febc
commit 2295161022
4 changed files with 211 additions and 116 deletions

View File

@ -79,8 +79,8 @@ export async function detectCompletionInSessionMessages(
if (assistantMessages.length === 0) return false if (assistantMessages.length === 0) return false
const pattern = buildPromisePattern(options.promise) const pattern = buildPromisePattern(options.promise)
const recentAssistants = assistantMessages.slice(-3) for (let index = assistantMessages.length - 1; index >= 0; index -= 1) {
for (const assistant of recentAssistants) { const assistant = assistantMessages[index]
if (!assistant.parts) continue if (!assistant.parts) continue
let responseText = "" let responseText = ""

View File

@ -494,6 +494,7 @@ describe("ralph-loop", () => {
config: { config: {
enabled: true, enabled: true,
default_max_iterations: 200, default_max_iterations: 200,
default_strategy: "continue",
}, },
}) })
@ -708,6 +709,57 @@ describe("ralph-loop", () => {
expect(promptCalls[0].text).toContain("<promise>CALCULATOR_DONE</promise>") expect(promptCalls[0].text).toContain("<promise>CALCULATOR_DONE</promise>")
}) })
test("should skip concurrent idle events for same session when handler is in flight", async () => {
// given - active loop with delayed prompt injection
let releasePromptAsync: (() => void) | undefined
const promptAsyncBlocked = new Promise<void>((resolve) => {
releasePromptAsync = resolve
})
let firstPromptStartedResolve: (() => void) | undefined
const firstPromptStarted = new Promise<void>((resolve) => {
firstPromptStartedResolve = resolve
})
const mockInput = createMockPluginInput() as {
client: {
session: {
promptAsync: (opts: { path: { id: string }; body: { parts: Array<{ type: string; text: string }> } }) => Promise<unknown>
}
}
}
const originalPromptAsync = mockInput.client.session.promptAsync
let promptAsyncCalls = 0
mockInput.client.session.promptAsync = async (opts) => {
promptAsyncCalls += 1
if (promptAsyncCalls === 1) {
firstPromptStartedResolve?.()
}
await promptAsyncBlocked
return originalPromptAsync(opts)
}
const hook = createRalphLoopHook(mockInput as Parameters<typeof createRalphLoopHook>[0])
hook.startLoop("session-123", "Build feature", { maxIterations: 10 })
// when - second idle arrives while first idle processing is still in flight
const firstIdle = hook.event({
event: { type: "session.idle", properties: { sessionID: "session-123" } },
})
await firstPromptStarted
const secondIdle = hook.event({
event: { type: "session.idle", properties: { sessionID: "session-123" } },
})
releasePromptAsync?.()
await Promise.all([firstIdle, secondIdle])
// then - only one continuation should be injected
expect(promptAsyncCalls).toBe(1)
expect(promptCalls.length).toBe(1)
expect(hook.getState()?.iteration).toBe(2)
})
test("should clear loop state on user abort (MessageAbortedError)", async () => { test("should clear loop state on user abort (MessageAbortedError)", async () => {
// given - active loop // given - active loop
const hook = createRalphLoopHook(createMockPluginInput()) const hook = createRalphLoopHook(createMockPluginInput())
@ -782,8 +834,8 @@ describe("ralph-loop", () => {
expect(hook.getState()).toBeNull() expect(hook.getState()).toBeNull()
}) })
test("should NOT detect completion if promise is older than last 3 assistant messages", async () => { test("should detect completion even when promise is older than previous narrow window", async () => {
// given - promise appears in an assistant message older than last 3 // given - promise appears in an older assistant message with additional assistant output after it
mockSessionMessages = [ mockSessionMessages = [
{ info: { role: "user" }, parts: [{ type: "text", text: "Start task" }] }, { info: { role: "user" }, parts: [{ type: "text", text: "Start task" }] },
{ info: { role: "assistant" }, parts: [{ type: "text", text: "Promise early <promise>DONE</promise>" }] }, { info: { role: "assistant" }, parts: [{ type: "text", text: "Promise early <promise>DONE</promise>" }] },
@ -801,9 +853,40 @@ describe("ralph-loop", () => {
event: { type: "session.idle", properties: { sessionID: "session-123" } }, event: { type: "session.idle", properties: { sessionID: "session-123" } },
}) })
// then - loop should continue (promise is older than last 3 assistant messages) // then - loop should complete because all assistant messages are scanned
expect(promptCalls.length).toBe(1) expect(promptCalls.length).toBe(0)
expect(hook.getState()?.iteration).toBe(2) expect(toastCalls.some((t) => t.title === "Ralph Loop Complete!")).toBe(true)
expect(hook.getState()).toBeNull()
})
test("should detect completion when many assistant messages are emitted after promise", async () => {
// given - completion promise followed by long assistant output sequence
mockSessionMessages = [
{ info: { role: "user" }, parts: [{ type: "text", text: "Start task" }] },
{ info: { role: "assistant" }, parts: [{ type: "text", text: "Done now <promise>DONE</promise>" }] },
]
for (let index = 1; index <= 25; index += 1) {
mockSessionMessages.push({
info: { role: "assistant" },
parts: [{ type: "text", text: `Post-completion assistant output ${index}` }],
})
}
const hook = createRalphLoopHook(createMockPluginInput(), {
getTranscriptPath: () => join(TEST_DIR, "nonexistent.jsonl"),
})
hook.startLoop("session-123", "Build something", { completionPromise: "DONE" })
// when - session goes idle
await hook.event({
event: { type: "session.idle", properties: { sessionID: "session-123" } },
})
// then - loop should complete despite large trailing output
expect(promptCalls.length).toBe(0)
expect(toastCalls.some((t) => t.title === "Ralph Loop Complete!")).toBe(true)
expect(hook.getState()).toBeNull()
}) })
test("should allow starting new loop while previous loop is active (different session)", async () => { test("should allow starting new loop while previous loop is active (different session)", async () => {

View File

@ -25,6 +25,8 @@ export function createRalphLoopEventHandler(
ctx: PluginInput, ctx: PluginInput,
options: RalphLoopEventHandlerOptions, options: RalphLoopEventHandlerOptions,
) { ) {
const inFlightSessions = new Set<string>()
return async ({ event }: { event: { type: string; properties?: unknown } }): Promise<void> => { return async ({ event }: { event: { type: string; properties?: unknown } }): Promise<void> => {
const props = event.properties as Record<string, unknown> | undefined const props = event.properties as Record<string, unknown> | undefined
@ -32,6 +34,15 @@ export function createRalphLoopEventHandler(
const sessionID = props?.sessionID as string | undefined const sessionID = props?.sessionID as string | undefined
if (!sessionID) return if (!sessionID) return
if (inFlightSessions.has(sessionID)) {
log(`[${HOOK_NAME}] Skipped: handler in flight`, { sessionID })
return
}
inFlightSessions.add(sessionID)
try {
if (options.sessionRecovery.isRecovering(sessionID)) { if (options.sessionRecovery.isRecovering(sessionID)) {
log(`[${HOOK_NAME}] Skipped: in recovery`, { sessionID }) log(`[${HOOK_NAME}] Skipped: in recovery`, { sessionID })
return return
@ -141,6 +152,9 @@ export function createRalphLoopEventHandler(
}) })
} }
return return
} finally {
inFlightSessions.delete(sessionID)
}
} }
if (event.type === "session.deleted") { if (event.type === "session.deleted") {

View File

@ -36,7 +36,7 @@ async function waitUntil(condition: () => boolean): Promise<void> {
} }
describe("ralph-loop reset strategy race condition", () => { describe("ralph-loop reset strategy race condition", () => {
test("should continue iteration when old session idle arrives before TUI switch completes", async () => { test("should skip duplicate idle while reset iteration handling is in flight", async () => {
// given - reset strategy loop with blocked TUI session switch // given - reset strategy loop with blocked TUI session switch
const promptCalls: Array<{ sessionID: string; text: string }> = [] const promptCalls: Array<{ sessionID: string; text: string }> = []
const createSessionCalls: Array<{ parentID?: string }> = [] const createSessionCalls: Array<{ parentID?: string }> = []
@ -85,7 +85,7 @@ describe("ralph-loop reset strategy race condition", () => {
}, },
}, },
}, },
} as Parameters<typeof createRalphLoopHook>[0]) } as unknown as Parameters<typeof createRalphLoopHook>[0])
hook.startLoop("session-old", "Build feature", { strategy: "reset" }) hook.startLoop("session-old", "Build feature", { strategy: "reset" })
@ -100,14 +100,12 @@ describe("ralph-loop reset strategy race condition", () => {
event: { type: "session.idle", properties: { sessionID: "session-old" } }, event: { type: "session.idle", properties: { sessionID: "session-old" } },
}) })
await waitUntil(() => selectSessionCalls > 1)
selectSessionDeferred.resolve() selectSessionDeferred.resolve()
await Promise.all([firstIdleEvent, secondIdleEvent]) await Promise.all([firstIdleEvent, secondIdleEvent])
// then - second idle should not be skipped during reset transition // then - duplicate idle should be skipped to prevent concurrent continuation injection
expect(createSessionCalls.length).toBe(2) expect(createSessionCalls.length).toBe(1)
expect(promptCalls.length).toBe(2) expect(promptCalls.length).toBe(1)
expect(hook.getState()?.iteration).toBe(3) expect(hook.getState()?.iteration).toBe(2)
}) })
}) })