Merge pull request #4108 from code-yeongyu/code-yeongyu/fix-idle-recovery-fanout
fix: prevent idle recovery fanout and slash duplication
This commit is contained in:
@@ -56,6 +56,16 @@ function getCommandExecutionEventID(input: CommandExecuteBeforeInput): string |
|
||||
return null
|
||||
}
|
||||
|
||||
function partsContainAutoSlashCommandTags(parts: Array<{ text?: string }>): boolean {
|
||||
return parts.some((part) =>
|
||||
typeof part.text === "string"
|
||||
&& (
|
||||
part.text.includes(AUTO_SLASH_COMMAND_TAG_OPEN)
|
||||
|| part.text.includes(AUTO_SLASH_COMMAND_TAG_CLOSE)
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
export interface AutoSlashCommandHookOptions {
|
||||
skills?: LoadedSkill[]
|
||||
pluginsEnabled?: boolean
|
||||
@@ -153,6 +163,10 @@ export function createAutoSlashCommandHook(options?: AutoSlashCommandHookOptions
|
||||
input: CommandExecuteBeforeInput,
|
||||
output: CommandExecuteBeforeOutput
|
||||
): Promise<void> => {
|
||||
if (partsContainAutoSlashCommandTags(output.parts)) {
|
||||
return
|
||||
}
|
||||
|
||||
const eventID = getCommandExecutionEventID(input)
|
||||
const commandKey = eventID
|
||||
? `${input.sessionID}:event:${eventID}`
|
||||
|
||||
@@ -355,6 +355,22 @@ describe("createAutoSlashCommandHook", () => {
|
||||
expect(output.parts[0].text).toContain("/ralph-loop Command")
|
||||
})
|
||||
|
||||
it("should not duplicate injection when command output is already tagged", async () => {
|
||||
//#given
|
||||
const hook = createAutoSlashCommandHook()
|
||||
const input = createCommandInput("ralph-loop")
|
||||
const taggedContent = "<auto-slash-command>\n/ralph-loop Command\n</auto-slash-command>"
|
||||
const output = createCommandOutput(taggedContent)
|
||||
|
||||
//#when
|
||||
await hook["command.execute.before"](input, output)
|
||||
|
||||
//#then
|
||||
expect(output.parts).toHaveLength(1)
|
||||
expect(output.parts[0]?.text).toBe(taggedContent)
|
||||
expect(output.parts[0]?.text?.split("<auto-slash-command>").length).toBe(2)
|
||||
})
|
||||
|
||||
it("should inject template for known builtin commands like ulw-loop", async () => {
|
||||
//#given
|
||||
const hook = createAutoSlashCommandHook()
|
||||
|
||||
@@ -210,4 +210,56 @@ describe("session-recovery hook interrupted idle recovery", () => {
|
||||
// then
|
||||
expect(result).toBe(false)
|
||||
})
|
||||
|
||||
test("#given a newer user turn follows an unfinished assistant turn #when idle recovery runs #then it does not recover the stale assistant", async () => {
|
||||
// given
|
||||
const promptAsyncCalls: PromptAsyncCall[] = []
|
||||
const ctx = {
|
||||
client: {
|
||||
session: {
|
||||
messages: async () => ({
|
||||
data: [
|
||||
{
|
||||
info: {
|
||||
id: "msg_stale_assistant",
|
||||
role: "assistant",
|
||||
sessionID: "ses_stale_after_user",
|
||||
finish: "tool-calls",
|
||||
},
|
||||
parts: [
|
||||
{
|
||||
type: "tool_use",
|
||||
id: "toolu_stale_pending",
|
||||
name: "bash",
|
||||
input: {},
|
||||
state: { status: "pending" },
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
info: {
|
||||
id: "msg_newer_user",
|
||||
role: "user",
|
||||
},
|
||||
parts: [{ type: "text", text: "new prompt after interrupted turn" }],
|
||||
},
|
||||
],
|
||||
}),
|
||||
promptAsync: async (call: PromptAsyncCall) => {
|
||||
promptAsyncCalls.push(call)
|
||||
return {}
|
||||
},
|
||||
},
|
||||
},
|
||||
directory: "/tmp/session-recovery-newer-user-test",
|
||||
}
|
||||
const hook = createSessionRecoveryHook(ctx as never)
|
||||
|
||||
// when
|
||||
const result = await hook.handleInterruptedToolResultsOnIdle("ses_stale_after_user")
|
||||
|
||||
// then
|
||||
expect(result).toBe(false)
|
||||
expect(promptAsyncCalls).toHaveLength(0)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -95,7 +95,11 @@ export function createSessionRecoveryHook(ctx: PluginInput, options?: SessionRec
|
||||
const findLatestAssistantMessage = (messages: MessageData[]): MessageData | undefined => {
|
||||
for (let index = messages.length - 1; index >= 0; index--) {
|
||||
const message = messages[index]
|
||||
if (message?.info?.role === "assistant") {
|
||||
const role = message?.info?.role
|
||||
if (role === "user") {
|
||||
return undefined
|
||||
}
|
||||
if (role === "assistant") {
|
||||
return message
|
||||
}
|
||||
}
|
||||
|
||||
@@ -425,6 +425,11 @@ describe("createEventHandler - idle deduplication", () => {
|
||||
return true
|
||||
},
|
||||
},
|
||||
backgroundNotificationHook: {
|
||||
event: async () => {
|
||||
callOrder.push("backgroundNotificationHook")
|
||||
},
|
||||
},
|
||||
todoContinuationEnforcer: {
|
||||
handler: async (input: EventInput) => {
|
||||
if (input.event.type === "session.idle") {
|
||||
@@ -448,6 +453,123 @@ describe("createEventHandler - idle deduplication", () => {
|
||||
expect(callOrder).toEqual(["sessionRecovery"])
|
||||
})
|
||||
|
||||
it("#given idle recovery handles a real idle #when another real idle arrives immediately #then dedup state does not suppress the later idle", async () => {
|
||||
//#given
|
||||
const originalDateNow = Date.now
|
||||
Date.now = () => 40_000
|
||||
const dispatchCalls: EventInput[] = []
|
||||
let recoveryCalls = 0
|
||||
const eventHandler = createEventHandler({
|
||||
ctx: asEventHandlerContext({ directory: "/tmp" }),
|
||||
pluginConfig: asPluginConfig({}),
|
||||
firstMessageVariantGate: {
|
||||
markSessionCreated: () => {},
|
||||
clear: () => {},
|
||||
},
|
||||
managers: createEventHandlerManagers(),
|
||||
hooks: createEventHandlerHooks({
|
||||
sessionRecovery: {
|
||||
handleInterruptedToolResultsOnIdle: async () => {
|
||||
recoveryCalls += 1
|
||||
return recoveryCalls === 1
|
||||
},
|
||||
},
|
||||
autoUpdateChecker: {
|
||||
event: async (input: EventInput) => {
|
||||
if (input.event.type === "session.idle") {
|
||||
dispatchCalls.push(input)
|
||||
}
|
||||
},
|
||||
},
|
||||
}),
|
||||
})
|
||||
|
||||
try {
|
||||
//#when
|
||||
await eventHandler(asEventHandlerInput({
|
||||
event: {
|
||||
type: "session.idle",
|
||||
properties: { sessionID: "ses_recovered_then_real" },
|
||||
},
|
||||
}))
|
||||
await eventHandler(asEventHandlerInput({
|
||||
event: {
|
||||
type: "session.idle",
|
||||
properties: { sessionID: "ses_recovered_then_real" },
|
||||
},
|
||||
}))
|
||||
|
||||
//#then
|
||||
expect(recoveryCalls).toBe(2)
|
||||
expect(dispatchCalls).toHaveLength(1)
|
||||
expect((dispatchCalls[0]?.event.properties as { sessionID?: string } | undefined)?.sessionID).toBe(
|
||||
"ses_recovered_then_real",
|
||||
)
|
||||
} finally {
|
||||
Date.now = originalDateNow
|
||||
}
|
||||
})
|
||||
|
||||
it("#given idle recovery handles a real idle #when a synthetic idle arrives immediately #then dedup state does not suppress the synthetic idle", async () => {
|
||||
//#given
|
||||
const originalDateNow = Date.now
|
||||
Date.now = () => 50_000
|
||||
const dispatchCalls: EventInput[] = []
|
||||
let recoveryCalls = 0
|
||||
const eventHandler = createEventHandler({
|
||||
ctx: asEventHandlerContext({ directory: "/tmp" }),
|
||||
pluginConfig: asPluginConfig({}),
|
||||
firstMessageVariantGate: {
|
||||
markSessionCreated: () => {},
|
||||
clear: () => {},
|
||||
},
|
||||
managers: createEventHandlerManagers(),
|
||||
hooks: createEventHandlerHooks({
|
||||
sessionRecovery: {
|
||||
handleInterruptedToolResultsOnIdle: async () => {
|
||||
recoveryCalls += 1
|
||||
return recoveryCalls === 1
|
||||
},
|
||||
},
|
||||
autoUpdateChecker: {
|
||||
event: async (input: EventInput) => {
|
||||
if (input.event.type === "session.idle") {
|
||||
dispatchCalls.push(input)
|
||||
}
|
||||
},
|
||||
},
|
||||
}),
|
||||
})
|
||||
|
||||
try {
|
||||
//#when
|
||||
await eventHandler(asEventHandlerInput({
|
||||
event: {
|
||||
type: "session.idle",
|
||||
properties: { sessionID: "ses_recovered_then_synthetic" },
|
||||
},
|
||||
}))
|
||||
await eventHandler(asEventHandlerInput({
|
||||
event: {
|
||||
type: "session.status",
|
||||
properties: {
|
||||
sessionID: "ses_recovered_then_synthetic",
|
||||
status: { type: "idle" },
|
||||
},
|
||||
},
|
||||
}))
|
||||
|
||||
//#then
|
||||
expect(recoveryCalls).toBe(2)
|
||||
expect(dispatchCalls).toHaveLength(1)
|
||||
expect((dispatchCalls[0]?.event.properties as { sessionID?: string } | undefined)?.sessionID).toBe(
|
||||
"ses_recovered_then_synthetic",
|
||||
)
|
||||
} finally {
|
||||
Date.now = originalDateNow
|
||||
}
|
||||
})
|
||||
|
||||
it("keeps other session dedup state untouched when bypassing synthetic-idle for current session", async () => {
|
||||
//#given
|
||||
const originalDateNow = Date.now
|
||||
|
||||
+21
-19
@@ -562,6 +562,7 @@ export function createEventHandler(args: {
|
||||
now: Date.now(),
|
||||
dedupWindowMs: DEDUP_WINDOW_MS,
|
||||
});
|
||||
const syntheticIdle = normalizeSessionStatusToIdle(input);
|
||||
|
||||
if (input.event.type === "session.idle") {
|
||||
const sessionID = getEventSessionID(input);
|
||||
@@ -577,15 +578,20 @@ export function createEventHandler(args: {
|
||||
recentAnyIdles.delete(sessionID);
|
||||
}
|
||||
}
|
||||
}
|
||||
const recovered = await recoverInterruptedToolResultsOnIdleEvent(input);
|
||||
if (recovered) {
|
||||
return;
|
||||
}
|
||||
if (sessionID) {
|
||||
const now = Date.now();
|
||||
recentRealIdles.set(sessionID, now);
|
||||
if (!shouldDispatchIdleEvent(sessionID, now)) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (input.event.type === "session.idle") {
|
||||
const recovered = await recoverInterruptedToolResultsOnIdleEvent(input);
|
||||
} else if (syntheticIdle) {
|
||||
const recovered = await recoverInterruptedToolResultsOnIdleEvent(syntheticIdle as EventInput);
|
||||
if (recovered) {
|
||||
return;
|
||||
}
|
||||
@@ -593,7 +599,6 @@ export function createEventHandler(args: {
|
||||
|
||||
await dispatchToHooks(input);
|
||||
|
||||
const syntheticIdle = normalizeSessionStatusToIdle(input);
|
||||
if (syntheticIdle) {
|
||||
const sessionID = (syntheticIdle.event.properties as Record<string, unknown>)?.sessionID as string;
|
||||
const now = Date.now();
|
||||
@@ -606,20 +611,17 @@ export function createEventHandler(args: {
|
||||
if (!shouldDispatchIdleEvent(sessionID, now)) {
|
||||
return;
|
||||
}
|
||||
const recovered = await recoverInterruptedToolResultsOnIdleEvent(syntheticIdle as EventInput);
|
||||
if (!recovered) {
|
||||
await dispatchToHooks(syntheticIdle as EventInput);
|
||||
if (pluginConfig.openclaw) {
|
||||
await dispatchOpenClawEvent({
|
||||
config: pluginConfig.openclaw,
|
||||
rawEvent: "session.idle",
|
||||
context: {
|
||||
sessionId: sessionID,
|
||||
projectPath: pluginContext.directory,
|
||||
tmuxPaneId: managers.tmuxSessionManager.getTrackedPaneId?.(sessionID) ?? process.env.TMUX_PANE,
|
||||
},
|
||||
});
|
||||
}
|
||||
await dispatchToHooks(syntheticIdle as EventInput);
|
||||
if (pluginConfig.openclaw) {
|
||||
await dispatchOpenClawEvent({
|
||||
config: pluginConfig.openclaw,
|
||||
rawEvent: "session.idle",
|
||||
context: {
|
||||
sessionId: sessionID,
|
||||
projectPath: pluginContext.directory,
|
||||
tmuxPaneId: managers.tmuxSessionManager.getTrackedPaneId?.(sessionID) ?? process.env.TMUX_PANE,
|
||||
},
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user