luna/oh-my-opencode
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #4142 from code-yeongyu/fix/prompt-dispatch-gate

Browse Source 
Fix duplicate internal prompt dispatch gating
This commit is contained in:
YeonGyu-Kim
2026-05-18 14:28:16 +09:00
committed by GitHub
parent d44cd1c1a0 428a18c748
commit ed5a3f90a4
6 changed files with 341 additions and 148 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/hooks/ralph-loop/completion-handler.ts
+4
View File
@@ -4,6 +4,7 @@ import { buildContinuationPrompt } from "./continuation-prompt-builder"
import { HOOK_NAME } from "./constants"
import { injectContinuationPrompt } from "./continuation-prompt-injector"
import type { RalphLoopState } from "./types"
import { releasePromptAsyncReservation } from "../shared/prompt-async-gate"
type LoopStateController = {
clear: () => boolean
@@ -45,6 +46,9 @@ export async function handleDetectedCompletion(
return
}
releasePromptAsyncReservation(sessionID, "ralph-loop:completion-detected", {
reservedBy: HOOK_NAME,
})
const promptResult = await injectContinuationPrompt(ctx, {
sessionID,
prompt: buildContinuationPrompt(verificationState),
src/hooks/ralph-loop/index.test.ts
+119 -65
View File
@@ -7,6 +7,7 @@ import { createRalphLoopHook } from "./index"
import { readState, writeState, clearState } from "./storage"
import type { RalphLoopState } from "./types"
import { parseRalphLoopArguments } from "./command-arguments"
import { DEFAULT_PROMPT_ASYNC_POST_DISPATCH_HOLD_MS } from "../shared/prompt-async-gate"
describe("ralph-loop", () => {
const TEST_DIR = join(tmpdir(), "ralph-loop-test-" + Date.now())
@@ -331,33 +332,41 @@ describe("ralph-loop", () => {
test("#given new activity after an idle continuation #when session idles again #then next iteration can continue", async () => {
// given
const originalDateNow = Date.now
let currentNow = originalDateNow()
Date.now = () => currentNow
const hook = createRalphLoopHook(createMockPluginInput(), { idleSettleMs: 0 })
hook.startLoop("session-123", "Build a feature", { maxIterations: 10 })
try {
hook.startLoop("session-123", "Build a feature", { maxIterations: 10 })
await hook.event({
event: {
type: "session.idle",
properties: { sessionID: "session-123" },
},
})
await hook.event({
event: {
type: "session.idle",
properties: { sessionID: "session-123" },
},
})
// when
await hook.event({
event: {
type: "message.part.updated",
properties: { sessionID: "session-123" },
},
})
await hook.event({
event: {
type: "session.idle",
properties: { sessionID: "session-123" },
},
})
// when
await hook.event({
event: {
type: "message.part.updated",
properties: { sessionID: "session-123" },
},
})
currentNow += DEFAULT_PROMPT_ASYNC_POST_DISPATCH_HOLD_MS + 1
await hook.event({
event: {
type: "session.idle",
properties: { sessionID: "session-123" },
},
})
// then
expect(promptCalls.length).toBe(2)
expect(hook.getState()?.iteration).toBe(3)
// then
expect(promptCalls.length).toBe(2)
expect(hook.getState()?.iteration).toBe(3)
} finally {
Date.now = originalDateNow
}
})
test("should inject continuation when idle event carries session id in info", async () => {
@@ -889,25 +898,62 @@ describe("ralph-loop", () => {
expect(promptCalls.length).toBe(1)
})
test("#given assistant activity follows an idle continuation #when stale idle arrives before dispatch hold expires #then no duplicate prompt is sent", async () => {
// given - active loop with deterministic dispatch hold time
const originalDateNow = Date.now
let currentNow = originalDateNow()
Date.now = () => currentNow
const hook = createRalphLoopHook(createMockPluginInput(), { idleSettleMs: 0 })
hook.startLoop("session-123", "Build feature", { maxIterations: 5 })
try {
await hook.event({
event: { type: "session.idle", properties: { sessionID: "session-123" } },
})
// when - assistant activity arrives, followed by an immediate stale idle
await hook.event({
event: { type: "message.part.updated", properties: { sessionID: "session-123" } },
})
await hook.event({
event: { type: "session.idle", properties: { sessionID: "session-123" } },
})
// then - activity did not release the prompt gate reservation
expect(hook.getState()?.iteration).toBe(2)
expect(promptCalls.length).toBe(1)
} finally {
Date.now = originalDateNow
}
})
test("should handle multiple iterations correctly", async () => {
// given - active loop
const originalDateNow = Date.now
let currentNow = originalDateNow()
Date.now = () => currentNow
const hook = createRalphLoopHook(createMockPluginInput())
hook.startLoop("session-123", "Build feature", { maxIterations: 5 })
// when - multiple idle events
await hook.event({
event: { type: "session.idle", properties: { sessionID: "session-123" } },
})
await hook.event({
event: { type: "message.part.updated", properties: { sessionID: "session-123" } },
})
await hook.event({
event: { type: "session.idle", properties: { sessionID: "session-123" } },
})
try {
// when - multiple idle events separated by the dispatch hold expiring
await hook.event({
event: { type: "session.idle", properties: { sessionID: "session-123" } },
})
await hook.event({
event: { type: "message.part.updated", properties: { sessionID: "session-123" } },
})
currentNow += DEFAULT_PROMPT_ASYNC_POST_DISPATCH_HOLD_MS + 1
await hook.event({
event: { type: "session.idle", properties: { sessionID: "session-123" } },
})
// then - iteration incremented correctly
expect(hook.getState()?.iteration).toBe(3)
expect(promptCalls.length).toBe(2)
// then - iteration incremented correctly
expect(hook.getState()?.iteration).toBe(3)
expect(promptCalls.length).toBe(2)
} finally {
Date.now = originalDateNow
}
})
test("should include prompt and promise in continuation message", async () => {
@@ -1141,41 +1187,49 @@ describe("ralph-loop", () => {
test("should allow starting new loop in same session (restart)", async () => {
// given - active loop in session A at iteration 5
const originalDateNow = Date.now
let currentNow = originalDateNow()
Date.now = () => currentNow
const hook = createRalphLoopHook(createMockPluginInput())
hook.startLoop("session-A", "First task", { maxIterations: 10 })
try {
hook.startLoop("session-A", "First task", { maxIterations: 10 })
// Simulate some iterations
await hook.event({
event: { type: "session.idle", properties: { sessionID: "session-A" } },
})
await hook.event({
event: { type: "message.part.updated", properties: { sessionID: "session-A" } },
})
await hook.event({
event: { type: "session.idle", properties: { sessionID: "session-A" } },
})
expect(hook.getState()?.iteration).toBe(3)
expect(promptCalls.length).toBe(2)
// Simulate some iterations
await hook.event({
event: { type: "session.idle", properties: { sessionID: "session-A" } },
})
await hook.event({
event: { type: "message.part.updated", properties: { sessionID: "session-A" } },
})
currentNow += DEFAULT_PROMPT_ASYNC_POST_DISPATCH_HOLD_MS + 1
await hook.event({
event: { type: "session.idle", properties: { sessionID: "session-A" } },
})
expect(hook.getState()?.iteration).toBe(3)
expect(promptCalls.length).toBe(2)
// when - start NEW loop in same session (restart)
hook.startLoop("session-A", "Restarted task", { maxIterations: 50 })
// when - start NEW loop in same session (restart)
hook.startLoop("session-A", "Restarted task", { maxIterations: 50 })
// then - state should be reset to iteration 1 with new prompt
expect(hook.getState()?.session_id).toBe("session-A")
expect(hook.getState()?.prompt).toBe("Restarted task")
expect(hook.getState()?.max_iterations).toBe(50)
expect(hook.getState()?.iteration).toBe(1)
// then - state should be reset to iteration 1 with new prompt
expect(hook.getState()?.session_id).toBe("session-A")
expect(hook.getState()?.prompt).toBe("Restarted task")
expect(hook.getState()?.max_iterations).toBe(50)
expect(hook.getState()?.iteration).toBe(1)
// when - session goes idle
promptCalls = [] // Reset to check new continuation
await hook.event({
event: { type: "session.idle", properties: { sessionID: "session-A" } },
})
// when - session goes idle
promptCalls = [] // Reset to check new continuation
await hook.event({
event: { type: "session.idle", properties: { sessionID: "session-A" } },
})
// then - continuation should use new task
expect(promptCalls.length).toBe(1)
expect(promptCalls[0].text).toContain("Restarted task")
expect(promptCalls[0].text).toContain("2/50")
// then - continuation should use new task
expect(promptCalls.length).toBe(1)
expect(promptCalls[0].text).toContain("Restarted task")
expect(promptCalls[0].text).toContain("2/50")
} finally {
Date.now = originalDateNow
}
})
test("should NOT detect completion from user message in transcript (issue #622)", async () => {
src/hooks/ralph-loop/non-abort-error-continuation.test.ts
+169 -77
View File
@@ -5,26 +5,35 @@ import { tmpdir } from "node:os"
import { join } from "node:path"
import { createRalphLoopHook } from "./index"
import { clearState } from "./storage"
import { DEFAULT_PROMPT_ASYNC_POST_DISPATCH_HOLD_MS } from "../shared/prompt-async-gate"
describe("ralph-loop non-abort error continuation", () => {
const testDirectory = join(tmpdir(), `ralph-loop-non-abort-error-${Date.now()}`)
let promptCalls: Array<{ sessionID: string; text: string }>
let messagesCalls: Array<{ sessionID: string }>
let syncPromptCalls: number
beforeEach(() => {
promptCalls = []
messagesCalls = []
syncPromptCalls = 0
mkdirSync(testDirectory, { recursive: true })
clearState(testDirectory)
})
afterEach(() => {
expect(syncPromptCalls).toBe(0)
clearState(testDirectory)
if (existsSync(testDirectory)) {
rmSync(testDirectory, { recursive: true, force: true })
}
})
async function failSyncPrompt(): Promise<never> {
syncPromptCalls += 1
throw new Error("Ralph Loop runtime-error continuation must use promptAsync")
}
test("continues immediately after non-abort session error", async () => {
// given - an active Ralph Loop receives a recoverable command error
const hook = createRalphLoopHook({
@@ -49,16 +58,7 @@ describe("ralph-loop non-abort error continuation", () => {
})
return {}
},
prompt: async (options: {
path: { id: string }
body: { parts: Array<{ type: string; text: string }> }
}) => {
promptCalls.push({
sessionID: options.path.id,
text: options.body.parts[0]?.text ?? "",
})
return {}
},
prompt: failSyncPrompt,
},
tui: {
showToast: async () => ({}),
@@ -112,7 +112,7 @@ describe("ralph-loop non-abort error continuation", () => {
})
return {}
},
prompt: async () => ({}),
prompt: failSyncPrompt,
},
tui: {
showToast: async () => ({}),
@@ -144,8 +144,11 @@ describe("ralph-loop non-abort error continuation", () => {
expect(hook.getState()?.iteration).toBe(2)
})
test("continues after retry run activity when no stale idle arrived", async () => {
test("keeps runtime retry dispatch reserved when activity is followed by immediate stale idle", async () => {
// given - an active loop retries a recoverable runtime error
const originalDateNow = Date.now
let currentNow = originalDateNow()
Date.now = () => currentNow
const hook = createRalphLoopHook({
directory: testDirectory,
project: testDirectory,
@@ -168,7 +171,7 @@ describe("ralph-loop non-abort error continuation", () => {
})
return {}
},
prompt: async () => ({}),
prompt: failSyncPrompt,
},
tui: {
showToast: async () => ({}),
@@ -176,45 +179,129 @@ describe("ralph-loop non-abort error continuation", () => {
},
} as never)
hook.startLoop("session-123", "Keep working", {
messageCountAtStart: 0,
maxIterations: 5,
})
try {
hook.startLoop("session-123", "Keep working", {
messageCountAtStart: 0,
maxIterations: 5,
})
await hook.event({
event: {
type: "session.error",
properties: {
sessionID: "session-123",
error: { name: "RuntimeError" },
await hook.event({
event: {
type: "session.error",
properties: {
sessionID: "session-123",
error: { name: "RuntimeError" },
},
},
})
// when - retried-run activity is followed by a stale idle before the hold expires
await hook.event({
event: {
type: "message.part.delta",
properties: {
sessionID: "session-123",
messageID: "msg-1",
partID: "part-1",
field: "text",
delta: "working",
},
},
})
await hook.event({
event: { type: "session.idle", properties: { sessionID: "session-123" } },
})
// then - the immediate stale idle is deferred by the prompt gate reservation
expect(promptCalls).toHaveLength(1)
expect(hook.getState()?.iteration).toBe(2)
} finally {
Date.now = originalDateNow
}
})
test("continues after retry run activity when no stale idle arrived", async () => {
// given - an active loop retries a recoverable runtime error
const originalDateNow = Date.now
let currentNow = originalDateNow()
Date.now = () => currentNow
const hook = createRalphLoopHook({
directory: testDirectory,
project: testDirectory,
worktree: testDirectory,
serverUrl: "http://localhost:4096",
$: async () => ({}),
client: {
session: {
messages: async (options: { path: { id: string } }) => {
messagesCalls.push({ sessionID: options.path.id })
return { data: [] }
},
promptAsync: async (options: {
path: { id: string }
body: { parts: Array<{ type: string; text: string }> }
}) => {
promptCalls.push({
sessionID: options.path.id,
text: options.body.parts[0]?.text ?? "",
})
return {}
},
prompt: failSyncPrompt,
},
tui: {
showToast: async () => ({}),
},
},
})
} as never)
// when - the retried run emits real assistant activity before any stale idle
await hook.event({
event: {
type: "message.part.delta",
properties: {
sessionID: "session-123",
messageID: "msg-1",
partID: "part-1",
field: "text",
delta: "working",
try {
hook.startLoop("session-123", "Keep working", {
messageCountAtStart: 0,
maxIterations: 5,
})
await hook.event({
event: {
type: "session.error",
properties: {
sessionID: "session-123",
error: { name: "RuntimeError" },
},
},
},
})
await hook.event({
event: { type: "session.idle", properties: { sessionID: "session-123" } },
})
})
// then - the real idle is allowed to continue the loop
expect(promptCalls).toHaveLength(2)
expect(hook.getState()?.iteration).toBe(3)
// when - retried-run activity is followed by an idle after the hold expires
await hook.event({
event: {
type: "message.part.delta",
properties: {
sessionID: "session-123",
messageID: "msg-1",
partID: "part-1",
field: "text",
delta: "working",
},
},
})
currentNow += DEFAULT_PROMPT_ASYNC_POST_DISPATCH_HOLD_MS + 1
await hook.event({
event: { type: "session.idle", properties: { sessionID: "session-123" } },
})
// then - the later real idle is allowed to continue the loop
expect(promptCalls).toHaveLength(2)
expect(hook.getState()?.iteration).toBe(3)
} finally {
Date.now = originalDateNow
}
})
test("continues after retry run activity from legacy message.part.updated part session id", async () => {
// given - an active loop retries a recoverable runtime error
const originalDateNow = Date.now
let currentNow = originalDateNow()
Date.now = () => currentNow
const hook = createRalphLoopHook({
directory: testDirectory,
project: testDirectory,
@@ -237,7 +324,7 @@ describe("ralph-loop non-abort error continuation", () => {
})
return {}
},
prompt: async () => ({}),
prompt: failSyncPrompt,
},
tui: {
showToast: async () => ({}),
@@ -245,43 +332,48 @@ describe("ralph-loop non-abort error continuation", () => {
},
} as never)
hook.startLoop("session-123", "Keep working", {
messageCountAtStart: 0,
maxIterations: 5,
})
try {
hook.startLoop("session-123", "Keep working", {
messageCountAtStart: 0,
maxIterations: 5,
})
await hook.event({
event: {
type: "session.error",
properties: {
sessionID: "session-123",
error: { name: "RuntimeError" },
},
},
})
// when - the retried run emits legacy assistant activity before any stale idle
await hook.event({
event: {
type: "message.part.updated",
properties: {
part: {
id: "part-1",
messageID: "msg-1",
await hook.event({
event: {
type: "session.error",
properties: {
sessionID: "session-123",
type: "text",
text: "working",
error: { name: "RuntimeError" },
},
},
},
})
await hook.event({
event: { type: "session.idle", properties: { sessionID: "session-123" } },
})
})
// then - the real idle is allowed to continue the loop
expect(promptCalls).toHaveLength(2)
expect(hook.getState()?.iteration).toBe(3)
// when - legacy retried-run activity is followed by an idle after the hold expires
await hook.event({
event: {
type: "message.part.updated",
properties: {
part: {
id: "part-1",
messageID: "msg-1",
sessionID: "session-123",
type: "text",
text: "working",
},
},
},
})
currentNow += DEFAULT_PROMPT_ASYNC_POST_DISPATCH_HOLD_MS + 1
await hook.event({
event: { type: "session.idle", properties: { sessionID: "session-123" } },
})
// then - the later real idle is allowed to continue the loop
expect(promptCalls).toHaveLength(2)
expect(hook.getState()?.iteration).toBe(3)
} finally {
Date.now = originalDateNow
}
})
test("skips immediate runtime retry while background tasks are running", async () => {
@@ -308,7 +400,7 @@ describe("ralph-loop non-abort error continuation", () => {
})
return {}
},
prompt: async () => ({}),
prompt: failSyncPrompt,
},
tui: {
showToast: async () => ({}),
@@ -367,7 +459,7 @@ describe("ralph-loop non-abort error continuation", () => {
})
return {}
},
prompt: async () => ({}),
prompt: failSyncPrompt,
},
tui: {
showToast: async () => ({}),
src/hooks/ralph-loop/ralph-loop-event-handler.ts
-4
View File
@@ -2,7 +2,6 @@ import type { PluginInput } from "@opencode-ai/plugin"
import { log } from "../../shared/logger"
import { resolveMessageEventSessionID, resolveSessionEventID } from "../../shared/event-session-id"
import { isSessionActive } from "../shared/session-idle-settle"
import { releasePromptAsyncReservation } from "../shared/prompt-async-gate"
import type { IterationCommitExpectation, RalphLoopOptions, RalphLoopState } from "./types"
import { HOOK_NAME } from "./constants"
import { handleDetectedCompletion } from "./completion-handler"
@@ -197,9 +196,6 @@ export function createRalphLoopEventHandler(
const props = event.properties as Record<string, unknown> | undefined
const runtimeRetryActivitySessionID = getRuntimeRetryActivitySessionID(event.type, props)
if (runtimeRetryActivitySessionID) {
releasePromptAsyncReservation(runtimeRetryActivitySessionID, "ralph-loop:activity", {
reservedBy: HOOK_NAME,
})
runtimeErrorRetriedSessions.delete(runtimeRetryActivitySessionID)
recentHandledSyntheticIdleAt.delete(runtimeRetryActivitySessionID)
}
src/hooks/shared/prompt-async-gate.test.ts
+46
View File
@@ -10,6 +10,7 @@ import {
describe("dispatchInternalPrompt", () => {
afterEach(() => {
// then
_setPromptGateMessagesFetchTimeoutMsForTesting(undefined)
releaseAllPromptAsyncReservationsForTesting()
})
@@ -126,6 +127,7 @@ describe("dispatchInternalPrompt", () => {
describe("dispatchInternalPrompt shared gate behavior", () => {
afterEach(() => {
// then
_setPromptGateMessagesFetchTimeoutMsForTesting(undefined)
releaseAllPromptAsyncReservationsForTesting()
})
@@ -242,6 +244,50 @@ describe("dispatchInternalPrompt shared gate behavior", () => {
})
})
test("#given SDK messages depends on its session receiver #when latest assistant waits on tools #then method binding is preserved and no prompt is sent", async () => {
// given
let promptCalls = 0
const session = {
_client: {
messages: [
{
info: { id: "msg_user", role: "user" },
parts: [{ type: "text", text: "run work" }],
},
{
info: { id: "msg_assistant", role: "assistant", finish: "tool-calls" },
parts: [{ type: "tool_use", id: "toolu_pending", state: { status: "running" } }],
},
],
},
async messages(
this: { _client: { messages: unknown[] } },
_input: { path: { id: string }; query: { directory: string } },
) {
return { data: this._client.messages }
},
async promptAsync() {
promptCalls += 1
},
}
const client = { session }
// when
const result = await dispatchInternalPrompt({
mode: "async",
client,
sessionID: "ses_bound_messages",
input: { path: { id: "ses_bound_messages" }, body: { parts: [] } },
source: "test:bound-messages",
settleMs: 0,
postDispatchHoldMs: 0,
})
// then
expect(result.status).toBe("active")
expect(promptCalls).toBe(0)
})
test("#given session.status reports busy #when an internal promptAsync is requested #then no prompt is sent", async () => {
// given
let promptCalls = 0
src/shared/prompt-async-gate.ts
+3 -2
View File
@@ -238,10 +238,11 @@ async function sessionLatestAssistantIsWaitingOnTools<TInput>(args: {
source: string
timeoutMs: number
}): Promise<boolean> {
const messages = args.client.session?.messages
if (typeof messages !== "function") {
const session = args.client.session
if (typeof session?.messages !== "function") {
return false
}
const messages = session.messages.bind(session)
try {
const response = await withDispatchTimeout(