Merge pull request #3672 from code-yeongyu/fix/ralph-loop-bg-task-guard
fix(ralph-loop): skip continuation when background tasks are pending (fixes #3526)
This commit is contained in:
@@ -59,6 +59,7 @@ export function createHooks(args: {
|
|||||||
ctx,
|
ctx,
|
||||||
pluginConfig,
|
pluginConfig,
|
||||||
modelCacheState,
|
modelCacheState,
|
||||||
|
backgroundManager,
|
||||||
modelFallbackControllerAccessor,
|
modelFallbackControllerAccessor,
|
||||||
isHookEnabled,
|
isHookEnabled,
|
||||||
safeHookEnabled,
|
safeHookEnabled,
|
||||||
|
|||||||
@@ -17,7 +17,7 @@ describe("ralph-loop", () => {
|
|||||||
let mockSessionMessages: Array<{ info?: { role?: string }; parts?: Array<{ type: string; text?: string }> }>
|
let mockSessionMessages: Array<{ info?: { role?: string }; parts?: Array<{ type: string; text?: string }> }>
|
||||||
let mockMessagesApiResponseShape: "data" | "array"
|
let mockMessagesApiResponseShape: "data" | "array"
|
||||||
|
|
||||||
function createMockPluginInput() {
|
function createMockPluginInput(): Parameters<typeof createRalphLoopHook>[0] {
|
||||||
return {
|
return {
|
||||||
client: {
|
client: {
|
||||||
session: {
|
session: {
|
||||||
@@ -63,7 +63,7 @@ describe("ralph-loop", () => {
|
|||||||
},
|
},
|
||||||
},
|
},
|
||||||
directory: TEST_DIR,
|
directory: TEST_DIR,
|
||||||
} as unknown as Parameters<typeof createRalphLoopHook>[0]
|
} as Parameters<typeof createRalphLoopHook>[0]
|
||||||
}
|
}
|
||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
@@ -304,6 +304,33 @@ describe("ralph-loop", () => {
|
|||||||
expect(state?.iteration).toBe(2)
|
expect(state?.iteration).toBe(2)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test("should skip continuation when background task is running", async () => {
|
||||||
|
// given - active loop state with a running background task
|
||||||
|
const hook = createRalphLoopHook(createMockPluginInput(), {
|
||||||
|
backgroundManager: {
|
||||||
|
getTasksByParentSession: (sessionID: string) => sessionID === "session-123"
|
||||||
|
? [{ status: "running" }]
|
||||||
|
: [],
|
||||||
|
},
|
||||||
|
})
|
||||||
|
hook.startLoop("session-123", "Build a feature", { maxIterations: 10 })
|
||||||
|
|
||||||
|
// when - session goes idle
|
||||||
|
await hook.event({
|
||||||
|
event: {
|
||||||
|
type: "session.idle",
|
||||||
|
properties: { sessionID: "session-123" },
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
// then - no continuation should be injected
|
||||||
|
expect(promptCalls.length).toBe(0)
|
||||||
|
|
||||||
|
// then - iteration should not be incremented
|
||||||
|
const state = hook.getState()
|
||||||
|
expect(state?.iteration).toBe(1)
|
||||||
|
})
|
||||||
|
|
||||||
test("should stop loop when max iterations reached", async () => {
|
test("should stop loop when max iterations reached", async () => {
|
||||||
// given - loop at max iteration
|
// given - loop at max iteration
|
||||||
const hook = createRalphLoopHook(createMockPluginInput())
|
const hook = createRalphLoopHook(createMockPluginInput())
|
||||||
@@ -1144,20 +1171,14 @@ Original task: Build something`
|
|||||||
test("should not hang when session.messages() throws", async () => {
|
test("should not hang when session.messages() throws", async () => {
|
||||||
// given - API that throws (simulates timeout error)
|
// given - API that throws (simulates timeout error)
|
||||||
let apiCallCount = 0
|
let apiCallCount = 0
|
||||||
const errorMock = {
|
const errorMock = createMockPluginInput()
|
||||||
...createMockPluginInput(),
|
Object.defineProperty(errorMock.client.session, "messages", {
|
||||||
client: {
|
value: async () => {
|
||||||
...createMockPluginInput().client,
|
apiCallCount++
|
||||||
session: {
|
throw new Error("API timeout")
|
||||||
...createMockPluginInput().client.session,
|
|
||||||
messages: async () => {
|
|
||||||
apiCallCount++
|
|
||||||
throw new Error("API timeout")
|
|
||||||
},
|
|
||||||
},
|
|
||||||
},
|
},
|
||||||
}
|
})
|
||||||
const hook = createRalphLoopHook(errorMock as any, {
|
const hook = createRalphLoopHook(errorMock, {
|
||||||
getTranscriptPath: () => join(TEST_DIR, "nonexistent.jsonl"),
|
getTranscriptPath: () => join(TEST_DIR, "nonexistent.jsonl"),
|
||||||
apiTimeout: 100,
|
apiTimeout: 100,
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -25,7 +25,7 @@ type LoopStateController = {
|
|||||||
setVerificationSessionID: (sessionID: string, verificationSessionID: string) => RalphLoopState | null
|
setVerificationSessionID: (sessionID: string, verificationSessionID: string) => RalphLoopState | null
|
||||||
restartAfterFailedVerification: (sessionID: string, messageCountAtStart?: number) => RalphLoopState | null
|
restartAfterFailedVerification: (sessionID: string, messageCountAtStart?: number) => RalphLoopState | null
|
||||||
}
|
}
|
||||||
type RalphLoopEventHandlerOptions = { directory: string; apiTimeoutMs: number; getTranscriptPath: (sessionID: string) => string | undefined; checkSessionExists?: RalphLoopOptions["checkSessionExists"]; sessionRecovery: SessionRecovery; loopState: LoopStateController }
|
type RalphLoopEventHandlerOptions = { directory: string; apiTimeoutMs: number; getTranscriptPath: (sessionID: string) => string | undefined; checkSessionExists?: RalphLoopOptions["checkSessionExists"]; backgroundManager?: RalphLoopOptions["backgroundManager"]; sessionRecovery: SessionRecovery; loopState: LoopStateController }
|
||||||
|
|
||||||
export function createRalphLoopEventHandler(
|
export function createRalphLoopEventHandler(
|
||||||
ctx: PluginInput,
|
ctx: PluginInput,
|
||||||
@@ -59,6 +59,15 @@ export function createRalphLoopEventHandler(
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const hasRunningBackgroundTasks = options.backgroundManager
|
||||||
|
? options.backgroundManager.getTasksByParentSession(sessionID).some((task: { status: string }) => task.status === "running")
|
||||||
|
: false
|
||||||
|
|
||||||
|
if (hasRunningBackgroundTasks) {
|
||||||
|
log(`[${HOOK_NAME}] Skipped: background tasks running`, { sessionID })
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
const verificationSessionID = state.verification_pending
|
const verificationSessionID = state.verification_pending
|
||||||
? state.verification_session_id
|
? state.verification_session_id
|
||||||
: undefined
|
: undefined
|
||||||
|
|||||||
@@ -46,6 +46,7 @@ export function createRalphLoopHook(
|
|||||||
const getTranscriptPath = options?.getTranscriptPath ?? getDefaultTranscriptPath
|
const getTranscriptPath = options?.getTranscriptPath ?? getDefaultTranscriptPath
|
||||||
const apiTimeout = options?.apiTimeout ?? DEFAULT_API_TIMEOUT
|
const apiTimeout = options?.apiTimeout ?? DEFAULT_API_TIMEOUT
|
||||||
const checkSessionExists = options?.checkSessionExists
|
const checkSessionExists = options?.checkSessionExists
|
||||||
|
const backgroundManager = options?.backgroundManager
|
||||||
|
|
||||||
const loopState = createLoopStateController({
|
const loopState = createLoopStateController({
|
||||||
directory: ctx.directory,
|
directory: ctx.directory,
|
||||||
@@ -59,6 +60,7 @@ export function createRalphLoopHook(
|
|||||||
apiTimeoutMs: apiTimeout,
|
apiTimeoutMs: apiTimeout,
|
||||||
getTranscriptPath,
|
getTranscriptPath,
|
||||||
checkSessionExists,
|
checkSessionExists,
|
||||||
|
backgroundManager,
|
||||||
sessionRecovery,
|
sessionRecovery,
|
||||||
loopState,
|
loopState,
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -22,4 +22,5 @@ export interface RalphLoopOptions {
|
|||||||
getTranscriptPath?: (sessionId: string) => string
|
getTranscriptPath?: (sessionId: string) => string
|
||||||
apiTimeout?: number
|
apiTimeout?: number
|
||||||
checkSessionExists?: (sessionId: string) => Promise<boolean>
|
checkSessionExists?: (sessionId: string) => Promise<boolean>
|
||||||
|
backgroundManager?: { getTasksByParentSession: (sessionId: string) => Array<{ status: string }> }
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
import type { HookName, OhMyOpenCodeConfig } from "../../config"
|
import type { HookName, OhMyOpenCodeConfig } from "../../config"
|
||||||
|
import type { BackgroundManager } from "../../features/background-agent"
|
||||||
import type { ModelFallbackControllerAccessor } from "../../hooks/model-fallback"
|
import type { ModelFallbackControllerAccessor } from "../../hooks/model-fallback"
|
||||||
import type { PluginContext } from "../types"
|
import type { PluginContext } from "../types"
|
||||||
import type { ModelCacheState } from "../../plugin-state"
|
import type { ModelCacheState } from "../../plugin-state"
|
||||||
@@ -11,16 +12,18 @@ export function createCoreHooks(args: {
|
|||||||
ctx: PluginContext
|
ctx: PluginContext
|
||||||
pluginConfig: OhMyOpenCodeConfig
|
pluginConfig: OhMyOpenCodeConfig
|
||||||
modelCacheState: ModelCacheState
|
modelCacheState: ModelCacheState
|
||||||
|
backgroundManager: BackgroundManager
|
||||||
modelFallbackControllerAccessor?: ModelFallbackControllerAccessor
|
modelFallbackControllerAccessor?: ModelFallbackControllerAccessor
|
||||||
isHookEnabled: (hookName: HookName) => boolean
|
isHookEnabled: (hookName: HookName) => boolean
|
||||||
safeHookEnabled: boolean
|
safeHookEnabled: boolean
|
||||||
}) {
|
}) {
|
||||||
const { ctx, pluginConfig, modelCacheState, modelFallbackControllerAccessor, isHookEnabled, safeHookEnabled } = args
|
const { ctx, pluginConfig, modelCacheState, backgroundManager, modelFallbackControllerAccessor, isHookEnabled, safeHookEnabled } = args
|
||||||
|
|
||||||
const session = createSessionHooks({
|
const session = createSessionHooks({
|
||||||
ctx,
|
ctx,
|
||||||
pluginConfig,
|
pluginConfig,
|
||||||
modelCacheState,
|
modelCacheState,
|
||||||
|
backgroundManager,
|
||||||
modelFallbackControllerAccessor,
|
modelFallbackControllerAccessor,
|
||||||
isHookEnabled,
|
isHookEnabled,
|
||||||
safeHookEnabled,
|
safeHookEnabled,
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
import type { OhMyOpenCodeConfig, HookName } from "../../config"
|
import type { OhMyOpenCodeConfig, HookName } from "../../config"
|
||||||
|
import type { BackgroundManager } from "../../features/background-agent"
|
||||||
import type { ModelFallbackControllerAccessor } from "../../hooks/model-fallback"
|
import type { ModelFallbackControllerAccessor } from "../../hooks/model-fallback"
|
||||||
import type { ModelCacheState } from "../../plugin-state"
|
import type { ModelCacheState } from "../../plugin-state"
|
||||||
import type { PluginContext } from "../types"
|
import type { PluginContext } from "../types"
|
||||||
@@ -70,11 +71,12 @@ export function createSessionHooks(args: {
|
|||||||
ctx: PluginContext
|
ctx: PluginContext
|
||||||
pluginConfig: OhMyOpenCodeConfig
|
pluginConfig: OhMyOpenCodeConfig
|
||||||
modelCacheState: ModelCacheState
|
modelCacheState: ModelCacheState
|
||||||
|
backgroundManager: BackgroundManager
|
||||||
modelFallbackControllerAccessor?: ModelFallbackControllerAccessor
|
modelFallbackControllerAccessor?: ModelFallbackControllerAccessor
|
||||||
isHookEnabled: (hookName: HookName) => boolean
|
isHookEnabled: (hookName: HookName) => boolean
|
||||||
safeHookEnabled: boolean
|
safeHookEnabled: boolean
|
||||||
}): SessionHooks {
|
}): SessionHooks {
|
||||||
const { ctx, pluginConfig, modelCacheState, modelFallbackControllerAccessor, isHookEnabled, safeHookEnabled } = args
|
const { ctx, pluginConfig, modelCacheState, backgroundManager, modelFallbackControllerAccessor, isHookEnabled, safeHookEnabled } = args
|
||||||
const safeHook = <T>(hookName: HookName, factory: () => T): T | null =>
|
const safeHook = <T>(hookName: HookName, factory: () => T): T | null =>
|
||||||
safeCreateHook(hookName, factory, { enabled: safeHookEnabled })
|
safeCreateHook(hookName, factory, { enabled: safeHookEnabled })
|
||||||
|
|
||||||
@@ -211,6 +213,7 @@ export function createSessionHooks(args: {
|
|||||||
createRalphLoopHook(ctx, {
|
createRalphLoopHook(ctx, {
|
||||||
config: pluginConfig.ralph_loop,
|
config: pluginConfig.ralph_loop,
|
||||||
checkSessionExists: async (sessionId) => await sessionExists(sessionId),
|
checkSessionExists: async (sessionId) => await sessionExists(sessionId),
|
||||||
|
backgroundManager,
|
||||||
}))
|
}))
|
||||||
: null
|
: null
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user