mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-10-06 02:47:56 +00:00
Fix #4888: Finalize pending api_req_started messages on task completion
- Add finish() method to Task class that finds all api_req_started messages without cost or cancelReason and sets their cost to 0 - Call finish() before emitting taskCompleted in attemptCompletionTool - Add comprehensive tests for the finish() method - Prevents UI spinners from persisting in completed task history
This commit is contained in:
parent
3e28921ef0
commit
5233f617d0
3 changed files with 214 additions and 0 deletions
|
|
@ -1908,6 +1908,39 @@ export class Task extends EventEmitter<ClineEvents> {
|
|||
}
|
||||
}
|
||||
|
||||
// Task completion
|
||||
|
||||
public async finish() {
|
||||
// Find any api_req_started messages that are still pending (no cost and no cancelReason)
|
||||
// While theoretically only the last one should be unfinished, the UI checks all messages
|
||||
const messages = [...this.clineMessages]
|
||||
let hasUpdates = false
|
||||
|
||||
for (let i = 0; i < messages.length; i++) {
|
||||
const message = messages[i]
|
||||
if (message.type === "say" && message.say === "api_req_started") {
|
||||
const apiReqInfo: ClineApiReqInfo = JSON.parse(message.text || "{}")
|
||||
|
||||
// Check if this message is still pending (no cost and no cancelReason)
|
||||
if (apiReqInfo.cost === undefined && apiReqInfo.cancelReason === undefined) {
|
||||
// Update the message to have a cost of 0 to indicate completion
|
||||
apiReqInfo.cost = 0
|
||||
messages[i] = {
|
||||
...message,
|
||||
text: JSON.stringify(apiReqInfo),
|
||||
}
|
||||
hasUpdates = true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Save the updated messages if any were modified
|
||||
if (hasUpdates) {
|
||||
this.clineMessages = messages
|
||||
await this.saveClineMessages()
|
||||
}
|
||||
}
|
||||
|
||||
// Getters
|
||||
|
||||
public get cwd() {
|
||||
|
|
|
|||
|
|
@ -1334,5 +1334,184 @@ describe("Cline", () => {
|
|||
expect(task.diffStrategy).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
describe("finish method", () => {
|
||||
it("should finalize all pending api_req_started messages with cost 0", async () => {
|
||||
const [cline, task] = Task.create({
|
||||
provider: mockProvider,
|
||||
apiConfiguration: mockApiConfig,
|
||||
task: "test task",
|
||||
})
|
||||
|
||||
// Mock saveClineMessages
|
||||
const saveSpy = vi.spyOn(cline as any, "saveClineMessages").mockResolvedValue(undefined)
|
||||
|
||||
// Set up messages with multiple api_req_started messages
|
||||
cline.clineMessages = [
|
||||
{
|
||||
ts: Date.now(),
|
||||
type: "say",
|
||||
say: "text",
|
||||
text: "Regular message",
|
||||
},
|
||||
{
|
||||
ts: Date.now(),
|
||||
type: "say",
|
||||
say: "api_req_started",
|
||||
text: JSON.stringify({
|
||||
request: "test request 1",
|
||||
tokensIn: 100,
|
||||
tokensOut: 50,
|
||||
// No cost - should be updated
|
||||
}),
|
||||
},
|
||||
{
|
||||
ts: Date.now(),
|
||||
type: "say",
|
||||
say: "api_req_started",
|
||||
text: JSON.stringify({
|
||||
request: "test request 2",
|
||||
tokensIn: 200,
|
||||
tokensOut: 100,
|
||||
cost: 0.001, // Already has cost
|
||||
}),
|
||||
},
|
||||
{
|
||||
ts: Date.now(),
|
||||
type: "say",
|
||||
say: "text",
|
||||
text: "Another regular message",
|
||||
},
|
||||
{
|
||||
ts: Date.now(),
|
||||
type: "say",
|
||||
say: "api_req_started",
|
||||
text: JSON.stringify({
|
||||
request: "test request 3",
|
||||
tokensIn: 300,
|
||||
tokensOut: 150,
|
||||
// No cost or cancelReason - should be updated
|
||||
}),
|
||||
},
|
||||
]
|
||||
|
||||
// Call finish
|
||||
await cline.finish()
|
||||
|
||||
// Verify saveClineMessages was called
|
||||
expect(saveSpy).toHaveBeenCalled()
|
||||
|
||||
// Verify the messages were updated correctly
|
||||
const messages = cline.clineMessages
|
||||
|
||||
// First api_req_started (index 1) had no cost, should now have cost 0
|
||||
const msg1 = JSON.parse(messages[1].text || "{}")
|
||||
expect(msg1.cost).toBe(0)
|
||||
expect(msg1.request).toBe("test request 1")
|
||||
|
||||
// Second api_req_started (index 2) already had cost, should be unchanged
|
||||
const msg2 = JSON.parse(messages[2].text || "{}")
|
||||
expect(msg2.cost).toBe(0.001)
|
||||
|
||||
// Last api_req_started (index 4) had no cost/cancelReason, should now have cost 0
|
||||
const msg3 = JSON.parse(messages[4].text || "{}")
|
||||
expect(msg3.cost).toBe(0)
|
||||
expect(msg3.request).toBe("test request 3")
|
||||
|
||||
// Verify that ALL api_req_started messages now have either cost or cancelReason
|
||||
const apiReqMessages = messages.filter((m) => m.type === "say" && m.say === "api_req_started")
|
||||
for (const msg of apiReqMessages) {
|
||||
const apiReqInfo = JSON.parse(msg.text || "{}")
|
||||
const hasCost = apiReqInfo.cost !== undefined
|
||||
const hasCancelReason = apiReqInfo.cancelReason !== undefined
|
||||
expect(hasCost || hasCancelReason).toBe(true)
|
||||
}
|
||||
|
||||
await cline.abortTask(true)
|
||||
await task.catch(() => {})
|
||||
})
|
||||
|
||||
it("should not save if no api_req_started messages need updating", async () => {
|
||||
const [cline, task] = Task.create({
|
||||
provider: mockProvider,
|
||||
apiConfiguration: mockApiConfig,
|
||||
task: "test task",
|
||||
})
|
||||
|
||||
// Mock saveClineMessages
|
||||
const saveSpy = vi.spyOn(cline as any, "saveClineMessages").mockResolvedValue(undefined)
|
||||
|
||||
// Set up messages where all api_req_started already have cost or cancelReason
|
||||
cline.clineMessages = [
|
||||
{
|
||||
ts: Date.now(),
|
||||
type: "say",
|
||||
say: "api_req_started",
|
||||
text: JSON.stringify({
|
||||
request: "test request 1",
|
||||
tokensIn: 100,
|
||||
tokensOut: 50,
|
||||
cost: 0.001, // Has cost
|
||||
}),
|
||||
},
|
||||
{
|
||||
ts: Date.now(),
|
||||
type: "say",
|
||||
say: "api_req_started",
|
||||
text: JSON.stringify({
|
||||
request: "test request 2",
|
||||
tokensIn: 200,
|
||||
tokensOut: 100,
|
||||
cancelReason: "User cancelled", // Has cancelReason
|
||||
}),
|
||||
},
|
||||
]
|
||||
|
||||
// Clear any calls from setup
|
||||
saveSpy.mockClear()
|
||||
|
||||
// Call finish
|
||||
await cline.finish()
|
||||
|
||||
// Verify saveClineMessages was NOT called since no updates were needed
|
||||
expect(saveSpy).not.toHaveBeenCalled()
|
||||
|
||||
await cline.abortTask(true)
|
||||
await task.catch(() => {})
|
||||
})
|
||||
|
||||
it("should handle case with no api_req_started messages", async () => {
|
||||
const [cline, task] = Task.create({
|
||||
provider: mockProvider,
|
||||
apiConfiguration: mockApiConfig,
|
||||
task: "test task",
|
||||
})
|
||||
|
||||
// Mock saveClineMessages
|
||||
const saveSpy = vi.spyOn(cline as any, "saveClineMessages").mockResolvedValue(undefined)
|
||||
|
||||
// Set up messages with no api_req_started
|
||||
cline.clineMessages = [
|
||||
{
|
||||
ts: Date.now(),
|
||||
type: "say",
|
||||
say: "text",
|
||||
text: "Just a regular message",
|
||||
},
|
||||
]
|
||||
|
||||
// Clear any calls from setup
|
||||
saveSpy.mockClear()
|
||||
|
||||
// Call finish
|
||||
await cline.finish()
|
||||
|
||||
// Verify saveClineMessages was NOT called
|
||||
expect(saveSpy).not.toHaveBeenCalled()
|
||||
|
||||
await cline.abortTask(true)
|
||||
await task.catch(() => {})
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -45,6 +45,7 @@ export async function attemptCompletionTool(
|
|||
// we have command string, which means we have the result as well, so finish it (doesnt have to exist yet)
|
||||
await cline.say("completion_result", removeClosingTag("result", result), undefined, false)
|
||||
|
||||
await cline.finish()
|
||||
TelemetryService.instance.captureTaskCompleted(cline.taskId)
|
||||
cline.emit("taskCompleted", cline.taskId, cline.getTokenUsage(), cline.toolUsage)
|
||||
|
||||
|
|
@ -68,6 +69,7 @@ export async function attemptCompletionTool(
|
|||
// Command execution is permanently disabled in attempt_completion
|
||||
// Users must use execute_command tool separately before attempt_completion
|
||||
await cline.say("completion_result", result, undefined, false)
|
||||
await cline.finish()
|
||||
TelemetryService.instance.captureTaskCompleted(cline.taskId)
|
||||
cline.emit("taskCompleted", cline.taskId, cline.getTokenUsage(), cline.toolUsage)
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue