fix(todo): skip unsafe continuation tails
This commit is contained in:
@@ -40,7 +40,18 @@ function createStateStore(): {
|
|||||||
resetContinuationProgress: (sessionID: string) => {
|
resetContinuationProgress: (sessionID: string) => {
|
||||||
resetCalls.push(sessionID)
|
resetCalls.push(sessionID)
|
||||||
},
|
},
|
||||||
cancelCountdown: () => {},
|
cancelCountdown: () => {
|
||||||
|
if (state.countdownTimer) {
|
||||||
|
clearTimeout(state.countdownTimer)
|
||||||
|
state.countdownTimer = undefined
|
||||||
|
}
|
||||||
|
if (state.countdownInterval) {
|
||||||
|
clearInterval(state.countdownInterval)
|
||||||
|
state.countdownInterval = undefined
|
||||||
|
}
|
||||||
|
state.countdownStartedAt = undefined
|
||||||
|
state.inFlight = false
|
||||||
|
},
|
||||||
cleanup: () => {},
|
cleanup: () => {},
|
||||||
cancelAllCountdowns: () => {},
|
cancelAllCountdowns: () => {},
|
||||||
shutdown: () => {},
|
shutdown: () => {},
|
||||||
@@ -136,4 +147,85 @@ describe("handleSessionIdle", () => {
|
|||||||
// reset is still called only once (from the first idle)
|
// reset is still called only once (from the first idle)
|
||||||
expect(resetCalls).toHaveLength(1)
|
expect(resetCalls).toHaveLength(1)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it("skips todo continuation when the previous internal continuation has only an empty unknown assistant turn", async () => {
|
||||||
|
// given
|
||||||
|
const sessionID = "ses_internal_noop_tail"
|
||||||
|
const { store, trackCalls, state } = createStateStore()
|
||||||
|
const ctx = {
|
||||||
|
client: {
|
||||||
|
session: {
|
||||||
|
messages: async () => ({
|
||||||
|
data: [
|
||||||
|
{
|
||||||
|
info: { role: "user" },
|
||||||
|
parts: [{ type: "text", text: "continue\n<!-- OMO_INTERNAL_INITIATOR -->", synthetic: true }],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
info: { role: "assistant", finish: "unknown", time: { completed: Date.now() } },
|
||||||
|
parts: [{ type: "step-start" }, { type: "step-finish", reason: "unknown" }],
|
||||||
|
},
|
||||||
|
],
|
||||||
|
}),
|
||||||
|
todo: async () => ({
|
||||||
|
data: [
|
||||||
|
{ id: "todo-1", content: "Finish init-deep", status: "pending", priority: "high" },
|
||||||
|
],
|
||||||
|
}),
|
||||||
|
},
|
||||||
|
},
|
||||||
|
directory: "/tmp/test",
|
||||||
|
}
|
||||||
|
|
||||||
|
try {
|
||||||
|
// when
|
||||||
|
await handleSessionIdle({
|
||||||
|
ctx: ctx as never,
|
||||||
|
sessionID,
|
||||||
|
sessionStateStore: store,
|
||||||
|
})
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(trackCalls).toEqual([])
|
||||||
|
expect(state.countdownStartedAt).toBeUndefined()
|
||||||
|
} finally {
|
||||||
|
store.cancelCountdown(sessionID)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
it("skips todo continuation when session messages cannot be inspected", async () => {
|
||||||
|
// given
|
||||||
|
const sessionID = "ses_messages_fetch_fails"
|
||||||
|
const { store, trackCalls, state } = createStateStore()
|
||||||
|
const ctx = {
|
||||||
|
client: {
|
||||||
|
session: {
|
||||||
|
messages: async () => {
|
||||||
|
throw new Error("message endpoint failed")
|
||||||
|
},
|
||||||
|
todo: async () => ({
|
||||||
|
data: [
|
||||||
|
{ id: "todo-1", content: "Finish init-deep", status: "pending", priority: "high" },
|
||||||
|
],
|
||||||
|
}),
|
||||||
|
},
|
||||||
|
},
|
||||||
|
directory: "/tmp/test",
|
||||||
|
}
|
||||||
|
|
||||||
|
try {
|
||||||
|
// when
|
||||||
|
await handleSessionIdle({
|
||||||
|
ctx: ctx as never,
|
||||||
|
sessionID,
|
||||||
|
sessionStateStore: store,
|
||||||
|
})
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(trackCalls).toEqual([])
|
||||||
|
expect(state.countdownStartedAt).toBeUndefined()
|
||||||
|
} finally {
|
||||||
|
store.cancelCountdown(sessionID)
|
||||||
|
}
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import { getSessionAgent } from "../../features/claude-code-session-state"
|
|||||||
import { normalizeSDKResponse } from "../../shared"
|
import { normalizeSDKResponse } from "../../shared"
|
||||||
import { getAgentConfigKey } from "../../shared/agent-display-names"
|
import { getAgentConfigKey } from "../../shared/agent-display-names"
|
||||||
import { log } from "../../shared/logger"
|
import { log } from "../../shared/logger"
|
||||||
|
import { latestAssistantTurnBlocksInternalPrompt } from "../../shared/prompt-async-gate/pending-tool-turn"
|
||||||
|
|
||||||
import { isLastAssistantMessageAborted } from "./abort-detection"
|
import { isLastAssistantMessageAborted } from "./abort-detection"
|
||||||
import { acknowledgeCompactionGuard, isCompactionGuardActive } from "./compaction-guard"
|
import { acknowledgeCompactionGuard, isCompactionGuardActive } from "./compaction-guard"
|
||||||
@@ -92,8 +93,13 @@ export async function handleSessionIdle(args: {
|
|||||||
log(`[${HOOK_NAME}] Skipped: pending question awaiting user response`, { sessionID })
|
log(`[${HOOK_NAME}] Skipped: pending question awaiting user response`, { sessionID })
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
if (latestAssistantTurnBlocksInternalPrompt(prefetchedMessages)) {
|
||||||
|
log(`[${HOOK_NAME}] Skipped: pending internal continuation response`, { sessionID })
|
||||||
|
return
|
||||||
|
}
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
log(`[${HOOK_NAME}] Messages fetch failed, continuing`, { sessionID, error: String(error) })
|
log(`[${HOOK_NAME}] Messages fetch failed, skipping continuation`, { sessionID, error: String(error) })
|
||||||
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
let todos: Todo[] = []
|
let todos: Todo[] = []
|
||||||
|
|||||||
@@ -39,6 +39,44 @@ describe("hasUnansweredQuestion", () => {
|
|||||||
expect(hasUnansweredQuestion(messages)).toBe(true)
|
expect(hasUnansweredQuestion(messages)).toBe(true)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test("#given last assistant message with OpenCode question tool field #when checking pending question #then returns true", () => {
|
||||||
|
const messages = [
|
||||||
|
{ info: { role: "user" } },
|
||||||
|
{
|
||||||
|
info: { role: "assistant" },
|
||||||
|
parts: [
|
||||||
|
{ type: "tool", tool: "question" },
|
||||||
|
],
|
||||||
|
},
|
||||||
|
]
|
||||||
|
expect(hasUnansweredQuestion(messages)).toBe(true)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("#given last assistant message with OpenCode ask_user_question tool field #when checking pending question #then returns true", () => {
|
||||||
|
const messages = [
|
||||||
|
{ info: { role: "user" } },
|
||||||
|
{
|
||||||
|
info: { role: "assistant" },
|
||||||
|
parts: [
|
||||||
|
{ type: "tool", tool: "ask_user_question" },
|
||||||
|
],
|
||||||
|
},
|
||||||
|
]
|
||||||
|
expect(hasUnansweredQuestion(messages)).toBe(true)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("#given completed OpenCode question tool #when checking pending question #then returns false", () => {
|
||||||
|
const messages = [
|
||||||
|
{
|
||||||
|
info: { role: "assistant" },
|
||||||
|
parts: [
|
||||||
|
{ type: "tool", tool: "question", state: { status: "completed" } },
|
||||||
|
],
|
||||||
|
},
|
||||||
|
]
|
||||||
|
expect(hasUnansweredQuestion(messages)).toBe(false)
|
||||||
|
})
|
||||||
|
|
||||||
test("given user message after question (answered), returns false", () => {
|
test("given user message after question (answered), returns false", () => {
|
||||||
const messages = [
|
const messages = [
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -5,7 +5,9 @@ import { HOOK_NAME } from "./constants"
|
|||||||
interface MessagePart {
|
interface MessagePart {
|
||||||
type?: string
|
type?: string
|
||||||
name?: string
|
name?: string
|
||||||
|
tool?: string
|
||||||
toolName?: string
|
toolName?: string
|
||||||
|
state?: { status?: string }
|
||||||
text?: string
|
text?: string
|
||||||
synthetic?: boolean
|
synthetic?: boolean
|
||||||
}
|
}
|
||||||
@@ -16,6 +18,20 @@ interface Message {
|
|||||||
parts?: MessagePart[]
|
parts?: MessagePart[]
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const QUESTION_TOOL_NAMES = new Set(["question", "ask_user_question", "askuserquestion"])
|
||||||
|
|
||||||
|
function getToolName(part: MessagePart): string | undefined {
|
||||||
|
return part.name ?? part.tool ?? part.toolName
|
||||||
|
}
|
||||||
|
|
||||||
|
function isUnansweredQuestionTool(part: MessagePart): boolean {
|
||||||
|
const toolName = getToolName(part)
|
||||||
|
if (!QUESTION_TOOL_NAMES.has(toolName?.toLowerCase() ?? "")) {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
return part.state?.status !== "completed"
|
||||||
|
}
|
||||||
|
|
||||||
export function hasUnansweredQuestion(messages: Message[]): boolean {
|
export function hasUnansweredQuestion(messages: Message[]): boolean {
|
||||||
if (!messages || messages.length === 0) return false
|
if (!messages || messages.length === 0) return false
|
||||||
|
|
||||||
@@ -33,8 +49,8 @@ export function hasUnansweredQuestion(messages: Message[]): boolean {
|
|||||||
if (role === "assistant" && msg.parts) {
|
if (role === "assistant" && msg.parts) {
|
||||||
const hasQuestion = msg.parts.some(
|
const hasQuestion = msg.parts.some(
|
||||||
(part) =>
|
(part) =>
|
||||||
(part.type === "tool_use" || part.type === "tool-invocation") &&
|
(part.type === "tool" || part.type === "tool_use" || part.type === "tool-invocation") &&
|
||||||
(part.name === "question" || part.toolName === "question"),
|
isUnansweredQuestionTool(part),
|
||||||
)
|
)
|
||||||
if (hasQuestion) {
|
if (hasQuestion) {
|
||||||
log(`[${HOOK_NAME}] Detected pending question tool in last assistant message`)
|
log(`[${HOOK_NAME}] Detected pending question tool in last assistant message`)
|
||||||
|
|||||||
Reference in New Issue
Block a user