fix(ralph-loop): adopt OContinue patterns for better performance and abort handling (#431)
This commit is contained in:
@@ -423,5 +423,162 @@ describe("ralph-loop", () => {
|
|||||||
expect(promptCalls[0].text).toContain("Create a calculator app")
|
expect(promptCalls[0].text).toContain("Create a calculator app")
|
||||||
expect(promptCalls[0].text).toContain("<promise>CALCULATOR_DONE</promise>")
|
expect(promptCalls[0].text).toContain("<promise>CALCULATOR_DONE</promise>")
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test("should clear loop state on user abort (MessageAbortedError)", async () => {
|
||||||
|
// #given - active loop
|
||||||
|
const hook = createRalphLoopHook(createMockPluginInput())
|
||||||
|
hook.startLoop("session-123", "Build something")
|
||||||
|
expect(hook.getState()).not.toBeNull()
|
||||||
|
|
||||||
|
// #when - user aborts (Ctrl+C)
|
||||||
|
await hook.event({
|
||||||
|
event: {
|
||||||
|
type: "session.error",
|
||||||
|
properties: {
|
||||||
|
sessionID: "session-123",
|
||||||
|
error: { name: "MessageAbortedError", message: "User aborted" },
|
||||||
|
},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
// #then - loop state should be cleared immediately
|
||||||
|
expect(hook.getState()).toBeNull()
|
||||||
|
})
|
||||||
|
|
||||||
|
test("should NOT set recovery mode on user abort", async () => {
|
||||||
|
// #given - active loop
|
||||||
|
const hook = createRalphLoopHook(createMockPluginInput())
|
||||||
|
hook.startLoop("session-123", "Build something")
|
||||||
|
|
||||||
|
// #when - user aborts (Ctrl+C)
|
||||||
|
await hook.event({
|
||||||
|
event: {
|
||||||
|
type: "session.error",
|
||||||
|
properties: {
|
||||||
|
sessionID: "session-123",
|
||||||
|
error: { name: "MessageAbortedError" },
|
||||||
|
},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
// Start a new loop
|
||||||
|
hook.startLoop("session-123", "New task")
|
||||||
|
|
||||||
|
// #when - session goes idle immediately (should work, no recovery mode)
|
||||||
|
await hook.event({
|
||||||
|
event: { type: "session.idle", properties: { sessionID: "session-123" } },
|
||||||
|
})
|
||||||
|
|
||||||
|
// #then - continuation should be injected (not blocked by recovery)
|
||||||
|
expect(promptCalls.length).toBe(1)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("should only check LAST assistant message for completion", async () => {
|
||||||
|
// #given - multiple assistant messages, only first has completion promise
|
||||||
|
mockSessionMessages = [
|
||||||
|
{ info: { role: "user" }, parts: [{ type: "text", text: "Start task" }] },
|
||||||
|
{ info: { role: "assistant" }, parts: [{ type: "text", text: "I'll work on it. <promise>DONE</promise>" }] },
|
||||||
|
{ info: { role: "user" }, parts: [{ type: "text", text: "Continue" }] },
|
||||||
|
{ info: { role: "assistant" }, parts: [{ type: "text", text: "Working on more features..." }] },
|
||||||
|
]
|
||||||
|
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 continue (last message has no completion promise)
|
||||||
|
expect(promptCalls.length).toBe(1)
|
||||||
|
expect(hook.getState()?.iteration).toBe(2)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("should detect completion only in LAST assistant message", async () => {
|
||||||
|
// #given - last assistant message has completion promise
|
||||||
|
mockSessionMessages = [
|
||||||
|
{ info: { role: "user" }, parts: [{ type: "text", text: "Start task" }] },
|
||||||
|
{ info: { role: "assistant" }, parts: [{ type: "text", text: "Starting work..." }] },
|
||||||
|
{ info: { role: "user" }, parts: [{ type: "text", text: "Continue" }] },
|
||||||
|
{ info: { role: "assistant" }, parts: [{ type: "text", text: "Task complete! <promise>DONE</promise>" }] },
|
||||||
|
]
|
||||||
|
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 (last message has completion promise)
|
||||||
|
expect(promptCalls.length).toBe(0)
|
||||||
|
expect(toastCalls.some((t) => t.title === "Ralph Loop Complete!")).toBe(true)
|
||||||
|
expect(hook.getState()).toBeNull()
|
||||||
|
})
|
||||||
|
|
||||||
|
test("should check transcript BEFORE API to optimize performance", async () => {
|
||||||
|
// #given - transcript has completion promise
|
||||||
|
const transcriptPath = join(TEST_DIR, "transcript.jsonl")
|
||||||
|
writeFileSync(transcriptPath, JSON.stringify({ content: "<promise>DONE</promise>" }))
|
||||||
|
mockSessionMessages = [
|
||||||
|
{ info: { role: "assistant" }, parts: [{ type: "text", text: "No promise here" }] },
|
||||||
|
]
|
||||||
|
const hook = createRalphLoopHook(createMockPluginInput(), {
|
||||||
|
getTranscriptPath: () => transcriptPath,
|
||||||
|
})
|
||||||
|
hook.startLoop("session-123", "Build something", { completionPromise: "DONE" })
|
||||||
|
|
||||||
|
// #when - session goes idle
|
||||||
|
await hook.event({
|
||||||
|
event: { type: "session.idle", properties: { sessionID: "session-123" } },
|
||||||
|
})
|
||||||
|
|
||||||
|
// #then - should complete via transcript (API not called when transcript succeeds)
|
||||||
|
expect(promptCalls.length).toBe(0)
|
||||||
|
expect(hook.getState()).toBeNull()
|
||||||
|
// API should NOT be called since transcript found completion
|
||||||
|
expect(messagesCalls.length).toBe(0)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
describe("API timeout protection", () => {
|
||||||
|
test("should not hang when session.messages() times out", async () => {
|
||||||
|
// #given - slow API that takes longer than timeout
|
||||||
|
const slowMock = {
|
||||||
|
...createMockPluginInput(),
|
||||||
|
client: {
|
||||||
|
...createMockPluginInput().client,
|
||||||
|
session: {
|
||||||
|
...createMockPluginInput().client.session,
|
||||||
|
messages: async () => {
|
||||||
|
// Simulate slow API (would hang without timeout)
|
||||||
|
await new Promise((resolve) => setTimeout(resolve, 10000))
|
||||||
|
return { data: [] }
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
const hook = createRalphLoopHook(slowMock as any, {
|
||||||
|
getTranscriptPath: () => join(TEST_DIR, "nonexistent.jsonl"),
|
||||||
|
apiTimeout: 100, // 100ms timeout for test
|
||||||
|
})
|
||||||
|
hook.startLoop("session-123", "Build something")
|
||||||
|
|
||||||
|
// #when - session goes idle (API will timeout)
|
||||||
|
const startTime = Date.now()
|
||||||
|
await hook.event({
|
||||||
|
event: { type: "session.idle", properties: { sessionID: "session-123" } },
|
||||||
|
})
|
||||||
|
const elapsed = Date.now() - startTime
|
||||||
|
|
||||||
|
// #then - should complete within timeout + buffer (not hang for 10s)
|
||||||
|
expect(elapsed).toBeLessThan(500)
|
||||||
|
// #then - loop should continue (API timeout = no completion detected)
|
||||||
|
expect(promptCalls.length).toBe(1)
|
||||||
|
})
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -53,6 +53,8 @@ export interface RalphLoopHook {
|
|||||||
getState: () => RalphLoopState | null
|
getState: () => RalphLoopState | null
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const DEFAULT_API_TIMEOUT = 3000
|
||||||
|
|
||||||
export function createRalphLoopHook(
|
export function createRalphLoopHook(
|
||||||
ctx: PluginInput,
|
ctx: PluginInput,
|
||||||
options?: RalphLoopOptions
|
options?: RalphLoopOptions
|
||||||
@@ -61,6 +63,7 @@ export function createRalphLoopHook(
|
|||||||
const config = options?.config
|
const config = options?.config
|
||||||
const stateDir = config?.state_dir
|
const stateDir = config?.state_dir
|
||||||
const getTranscriptPath = options?.getTranscriptPath ?? getDefaultTranscriptPath
|
const getTranscriptPath = options?.getTranscriptPath ?? getDefaultTranscriptPath
|
||||||
|
const apiTimeout = options?.apiTimeout ?? DEFAULT_API_TIMEOUT
|
||||||
|
|
||||||
function getSessionState(sessionID: string): SessionState {
|
function getSessionState(sessionID: string): SessionState {
|
||||||
let state = sessions.get(sessionID)
|
let state = sessions.get(sessionID)
|
||||||
@@ -97,32 +100,34 @@ export function createRalphLoopHook(
|
|||||||
promise: string
|
promise: string
|
||||||
): Promise<boolean> {
|
): Promise<boolean> {
|
||||||
try {
|
try {
|
||||||
const response = await ctx.client.session.messages({
|
const response = await Promise.race([
|
||||||
path: { id: sessionID },
|
ctx.client.session.messages({
|
||||||
query: { directory: ctx.directory },
|
path: { id: sessionID },
|
||||||
})
|
query: { directory: ctx.directory },
|
||||||
|
}),
|
||||||
|
new Promise<never>((_, reject) =>
|
||||||
|
setTimeout(() => reject(new Error("API timeout")), apiTimeout)
|
||||||
|
),
|
||||||
|
])
|
||||||
|
|
||||||
const messages = (response as { data?: unknown[] }).data ?? []
|
const messages = (response as { data?: unknown[] }).data ?? []
|
||||||
|
|
||||||
if (!Array.isArray(messages)) return false
|
if (!Array.isArray(messages)) return false
|
||||||
|
|
||||||
|
const assistantMessages = (messages as OpenCodeSessionMessage[]).filter(
|
||||||
|
(msg) => msg.info?.role === "assistant"
|
||||||
|
)
|
||||||
|
const lastAssistant = assistantMessages[assistantMessages.length - 1]
|
||||||
|
if (!lastAssistant?.parts) return false
|
||||||
|
|
||||||
const pattern = new RegExp(`<promise>\\s*${escapeRegex(promise)}\\s*</promise>`, "is")
|
const pattern = new RegExp(`<promise>\\s*${escapeRegex(promise)}\\s*</promise>`, "is")
|
||||||
|
const responseText = lastAssistant.parts
|
||||||
|
.filter((p) => p.type === "text")
|
||||||
|
.map((p) => p.text ?? "")
|
||||||
|
.join("\n")
|
||||||
|
|
||||||
for (const msg of messages as OpenCodeSessionMessage[]) {
|
return pattern.test(responseText)
|
||||||
if (msg.info?.role !== "assistant") continue
|
|
||||||
|
|
||||||
for (const part of msg.parts || []) {
|
|
||||||
if (part.type === "text" && part.text) {
|
|
||||||
if (pattern.test(part.text)) {
|
|
||||||
return true
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
return false
|
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
log(`[${HOOK_NAME}] Failed to fetch session messages`, { sessionID, error: String(err) })
|
log(`[${HOOK_NAME}] Session messages check failed`, { sessionID, error: String(err) })
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -197,20 +202,19 @@ export function createRalphLoopHook(
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
const completionDetectedViaApi = await detectCompletionInSessionMessages(
|
|
||||||
sessionID,
|
|
||||||
state.completion_promise
|
|
||||||
)
|
|
||||||
|
|
||||||
const transcriptPath = getTranscriptPath(sessionID)
|
const transcriptPath = getTranscriptPath(sessionID)
|
||||||
const completionDetectedViaTranscript = detectCompletionPromise(transcriptPath, state.completion_promise)
|
const completionDetectedViaTranscript = detectCompletionPromise(transcriptPath, state.completion_promise)
|
||||||
|
|
||||||
if (completionDetectedViaApi || completionDetectedViaTranscript) {
|
const completionDetectedViaApi = completionDetectedViaTranscript
|
||||||
|
? false
|
||||||
|
: await detectCompletionInSessionMessages(sessionID, state.completion_promise)
|
||||||
|
|
||||||
|
if (completionDetectedViaTranscript || completionDetectedViaApi) {
|
||||||
log(`[${HOOK_NAME}] Completion detected!`, {
|
log(`[${HOOK_NAME}] Completion detected!`, {
|
||||||
sessionID,
|
sessionID,
|
||||||
iteration: state.iteration,
|
iteration: state.iteration,
|
||||||
promise: state.completion_promise,
|
promise: state.completion_promise,
|
||||||
detectedVia: completionDetectedViaApi ? "session_messages_api" : "transcript_file",
|
detectedVia: completionDetectedViaTranscript ? "transcript_file" : "session_messages_api",
|
||||||
})
|
})
|
||||||
clearState(ctx.directory, stateDir)
|
clearState(ctx.directory, stateDir)
|
||||||
|
|
||||||
@@ -308,6 +312,20 @@ export function createRalphLoopHook(
|
|||||||
|
|
||||||
if (event.type === "session.error") {
|
if (event.type === "session.error") {
|
||||||
const sessionID = props?.sessionID as string | undefined
|
const sessionID = props?.sessionID as string | undefined
|
||||||
|
const error = props?.error as { name?: string } | undefined
|
||||||
|
|
||||||
|
if (error?.name === "MessageAbortedError") {
|
||||||
|
if (sessionID) {
|
||||||
|
const state = readState(ctx.directory, stateDir)
|
||||||
|
if (state?.session_id === sessionID) {
|
||||||
|
clearState(ctx.directory, stateDir)
|
||||||
|
log(`[${HOOK_NAME}] User aborted, loop cleared`, { sessionID })
|
||||||
|
}
|
||||||
|
sessions.delete(sessionID)
|
||||||
|
}
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
if (sessionID) {
|
if (sessionID) {
|
||||||
const sessionState = getSessionState(sessionID)
|
const sessionState = getSessionState(sessionID)
|
||||||
sessionState.isRecovering = true
|
sessionState.isRecovering = true
|
||||||
|
|||||||
@@ -13,4 +13,5 @@ export interface RalphLoopState {
|
|||||||
export interface RalphLoopOptions {
|
export interface RalphLoopOptions {
|
||||||
config?: RalphLoopConfig
|
config?: RalphLoopConfig
|
||||||
getTranscriptPath?: (sessionId: string) => string
|
getTranscriptPath?: (sessionId: string) => string
|
||||||
|
apiTimeout?: number
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user