fix(run): prevent premature exit on idle before meaningful work (#1263)
The run command's completion check had a race condition: when a session transitions busy->idle before the LLM generates any output (empty response or API delay), checkCompletionConditions() returns true because 0 incomplete todos + 0 busy children = complete. This caused the runner to exit with 'All tasks completed' before any work was done. Fix: - Add hasReceivedMeaningfulWork flag to EventState - Set flag on: assistant text content, tool execution, or message update with actual content (all scoped to main session only) - Guard completion check in runner poll loop: skip if no meaningful work has been observed yet This ensures the runner waits until the session has produced at least one observable output before considering completion conditions. Adds 6 new test cases covering the race condition scenarios.
This commit is contained in:
@@ -82,6 +82,7 @@ describe("createEventState", () => {
|
||||
expect(state.lastOutput).toBe("")
|
||||
expect(state.lastPartText).toBe("")
|
||||
expect(state.currentTool).toBe(null)
|
||||
expect(state.hasReceivedMeaningfulWork).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -126,6 +127,121 @@ describe("event handling", () => {
|
||||
expect(state.mainSessionIdle).toBe(false)
|
||||
})
|
||||
|
||||
it("hasReceivedMeaningfulWork is false initially after session.idle", async () => {
|
||||
// #given - session goes idle without any assistant output (race condition scenario)
|
||||
const ctx = createMockContext("my-session")
|
||||
const state = createEventState()
|
||||
|
||||
const payload: EventPayload = {
|
||||
type: "session.idle",
|
||||
properties: { sessionID: "my-session" },
|
||||
}
|
||||
|
||||
const events = toAsyncIterable([payload])
|
||||
const { processEvents } = await import("./events")
|
||||
|
||||
// #when
|
||||
await processEvents(ctx, events, state)
|
||||
|
||||
// #then - idle but no meaningful work yet
|
||||
expect(state.mainSessionIdle).toBe(true)
|
||||
expect(state.hasReceivedMeaningfulWork).toBe(false)
|
||||
})
|
||||
|
||||
it("message.updated with assistant content sets hasReceivedMeaningfulWork", async () => {
|
||||
// #given
|
||||
const ctx = createMockContext("my-session")
|
||||
const state = createEventState()
|
||||
|
||||
const payload: EventPayload = {
|
||||
type: "message.updated",
|
||||
properties: {
|
||||
info: { sessionID: "my-session", role: "assistant" },
|
||||
content: "Hello, I will fix this bug.",
|
||||
},
|
||||
}
|
||||
|
||||
const events = toAsyncIterable([payload])
|
||||
const { processEvents } = await import("./events")
|
||||
|
||||
// #when
|
||||
await processEvents(ctx, events, state)
|
||||
|
||||
// #then
|
||||
expect(state.hasReceivedMeaningfulWork).toBe(true)
|
||||
})
|
||||
|
||||
it("message.updated with empty assistant content does not set hasReceivedMeaningfulWork", async () => {
|
||||
// #given - empty assistant message (race condition: message created but no content yet)
|
||||
const ctx = createMockContext("my-session")
|
||||
const state = createEventState()
|
||||
|
||||
const payload: EventPayload = {
|
||||
type: "message.updated",
|
||||
properties: {
|
||||
info: { sessionID: "my-session", role: "assistant" },
|
||||
content: "",
|
||||
},
|
||||
}
|
||||
|
||||
const events = toAsyncIterable([payload])
|
||||
const { processEvents } = await import("./events")
|
||||
|
||||
// #when
|
||||
await processEvents(ctx, events, state)
|
||||
|
||||
// #then - empty content should not count as meaningful work
|
||||
expect(state.hasReceivedMeaningfulWork).toBe(false)
|
||||
})
|
||||
|
||||
it("tool.execute sets hasReceivedMeaningfulWork", async () => {
|
||||
// #given
|
||||
const ctx = createMockContext("my-session")
|
||||
const state = createEventState()
|
||||
|
||||
const payload: EventPayload = {
|
||||
type: "tool.execute",
|
||||
properties: {
|
||||
sessionID: "my-session",
|
||||
name: "read_file",
|
||||
input: { filePath: "/src/index.ts" },
|
||||
},
|
||||
}
|
||||
|
||||
const events = toAsyncIterable([payload])
|
||||
const { processEvents } = await import("./events")
|
||||
|
||||
// #when
|
||||
await processEvents(ctx, events, state)
|
||||
|
||||
// #then
|
||||
expect(state.hasReceivedMeaningfulWork).toBe(true)
|
||||
})
|
||||
|
||||
it("tool.execute from different session does not set hasReceivedMeaningfulWork", async () => {
|
||||
// #given
|
||||
const ctx = createMockContext("my-session")
|
||||
const state = createEventState()
|
||||
|
||||
const payload: EventPayload = {
|
||||
type: "tool.execute",
|
||||
properties: {
|
||||
sessionID: "other-session",
|
||||
name: "read_file",
|
||||
input: { filePath: "/src/index.ts" },
|
||||
},
|
||||
}
|
||||
|
||||
const events = toAsyncIterable([payload])
|
||||
const { processEvents } = await import("./events")
|
||||
|
||||
// #when
|
||||
await processEvents(ctx, events, state)
|
||||
|
||||
// #then - different session's tool call shouldn't count
|
||||
expect(state.hasReceivedMeaningfulWork).toBe(false)
|
||||
})
|
||||
|
||||
it("session.status with busy type sets mainSessionIdle to false", async () => {
|
||||
// #given
|
||||
const ctx = createMockContext("my-session")
|
||||
@@ -136,6 +252,7 @@ describe("event handling", () => {
|
||||
lastOutput: "",
|
||||
lastPartText: "",
|
||||
currentTool: null,
|
||||
hasReceivedMeaningfulWork: false,
|
||||
}
|
||||
|
||||
const payload: EventPayload = {
|
||||
|
||||
Reference in New Issue
Block a user