Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
0575a35
fix: prevent agent loop stall from WriteToFileTool filesystem errors …
awschmeder Jun 26, 2026
75b52e3
fix: clear stuck UI spinner and duplicate/repeated errors on write_to…
awschmeder Jun 26, 2026
0966556
fix(write-to-file): address partial filesystem error review
easonliang28 Jul 30, 2026
16c4d48
fix(write-to-file): guard diff reset cleanup
easonliang28 Jul 30, 2026
be0e154
fix(write-to-file): guard partial ask finalization
easonliang28 Jul 30, 2026
224690b
fix(write-to-file): clean partial state on task abort
easonliang28 Jul 30, 2026
dd31f44
Merge branch 'main' into fix/pr-727-review
easonLiangWorldedtech Aug 12, 2026
c17b15b
fix(write-to-file): address remaining review cleanup
easonliang28 Aug 12, 2026
c011322
Merge branch 'main' into fix/pr-727-review
easonLiangWorldedtech Aug 12, 2026
44f9aa9
Merge branch 'main' into fix/pr-727-review
easonLiangWorldedtech Aug 15, 2026
6f99772
fix(write-to-file): clean up partial state on rooignore denial
easonliang28 Aug 19, 2026
4189f88
test(task): cover finalizePartialToolAsk updateClineMessage rejection…
easonliang28 Aug 23, 2026
f3e4d40
fix(tools): finalize open partial tool ask on args parse failure
easonliang28 Aug 29, 2026
c47e3c7
fix(write-to-file): address review findings on partial-stream cleanup
easonliang28 Aug 29, 2026
a03ac8c
Merge branch 'main' into fix/pr-727-review
easonLiangWorldedtech Sep 2, 2026
d7f8038
test(write-to-file): cover partial-ask finalize rejection, revert fai…
easonliang28 Sep 2, 2026
26ad729
Merge branch 'main' into fix/pr-727-review
edelauna Sep 3, 2026
f8b6b35
fix(mutation): match direct spec files case-insensitively
easonliang28 Sep 4, 2026
5d32001
test(task): cover finalizePartialToolAsk predicate clause mismatches
easonliang28 Sep 4, 2026
26dda51
test(tools): cover write_to_file stabilization and cleanup branches
easonliang28 Sep 4, 2026
4bd7d88
Merge branch 'main' into fix/pr-727-review
easonLiangWorldedtech Sep 4, 2026
4fd1a9f
fix(core): address CodeRabbit review findings on write_to_file and ta…
easonliang28 Sep 4, 2026
3228643
fix(task): report save stages independently in saveClineMessages
easonliang28 Sep 4, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions scripts/stryker-diff.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -293,12 +293,14 @@ export function parseVitestTestFiles(report, runRoot) {
}

export function preferDirectTestFiles(testFiles, sourceFiles) {
// Spec files follow the lowerCamel source-name convention (e.g.
// writeToFileTool.spec.ts for WriteToFileTool.ts), so match case-insensitively.
const sourceNames = sourceFiles.map((sourceFile) => path.posix.basename(sourceFile, path.posix.extname(sourceFile)))
const direct = testFiles.filter((testFile) => {
const testName = path.posix.basename(testFile)
const testName = path.posix.basename(testFile).toLowerCase()
return sourceNames.some(
(sourceName) =>
testName.startsWith(`${sourceName}.`) && /\.(?:test|spec)(?:\.[^.]+)?\.[cm]?[jt]sx?$/.test(testName),
testName.startsWith(`${sourceName.toLowerCase()}.`) && /\.(?:test|spec)(?:\.[^.]+)?\.[cm]?[jt]sx?$/.test(testName),
)
})
return direct.length > 0 ? direct : testFiles
Expand Down
14 changes: 14 additions & 0 deletions scripts/stryker-diff.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,20 @@ describe("preferDirectTestFiles", () => {
])
assert.deepEqual(preferDirectTestFiles(related, ["webview-ui/src/utils/unmatched.ts"]), related)
})

it("matches lowerCamel spec names against PascalCase sources case-insensitively", () => {
const related = [
"src/core/tools/__tests__/writeToFileTool.spec.ts",
"src/core/task/__tests__/Task.spec.ts",
"src/core/tools/__tests__/presentAssistantMessage-custom-tool.spec.ts",
]
assert.deepEqual(
preferDirectTestFiles(related, ["src/core/tools/WriteToFileTool.ts", "src/core/task/Task.ts"]),
["src/core/tools/__tests__/writeToFileTool.spec.ts", "src/core/task/__tests__/Task.spec.ts"],
)
// No source with a matching spec name: fall back to all related tests.
assert.deepEqual(preferDirectTestFiles(related, ["src/core/tools/ReadFileTool.ts"]), related)
})
})

describe("related-test discovery", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ describe("presentAssistantMessage - Custom Tool Recording", () => {
},
say: vi.fn().mockResolvedValue(undefined),
ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked" }),
finalizePartialToolAsk: vi.fn().mockResolvedValue(undefined),
}

// Add pushToolResultToUserContent method after mockTask is created so it can reference mockTask
Expand Down
75 changes: 71 additions & 4 deletions src/core/task/Task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ import { ApiStream, GroundingSource } from "../../api/transform/stream"
import { maybeRemoveImageBlocks } from "../../api/transform/image-cleaning"

// shared
import { findLastIndex } from "../../shared/array"
import { findLast, findLastIndex } from "../../shared/array"
import { combineApiRequests } from "../../shared/combineApiRequests"
import { combineCommandSequences } from "../../shared/combineCommandSequences"
import { t } from "../../i18n"
Expand Down Expand Up @@ -1212,14 +1212,28 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
}
}

/**
* Persist the message array, then refresh the derived metadata / task-history entries.
*
* The returned boolean reflects the message write only: `saveTaskMessages` failure
* leaves the on-disk record stale, so callers gating UI updates on durable state must
* skip them. Metadata / task-history stage failures are logged and swallowed — the
* message array is already persisted, and the next save recomputes and re-emits the
* metadata.
*/
private async saveClineMessages(): Promise<boolean> {
try {
await saveTaskMessages({
messages: structuredClone(this.clineMessages),
taskId: this.taskId,
globalStoragePath: this.globalStoragePath,
})
} catch (error) {
console.error("Failed to save Roo messages:", error)
return false
}

try {
if (this._taskApiConfigName === undefined) {
await this.taskApiConfigReady
}
Expand Down Expand Up @@ -1247,11 +1261,14 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
const provider = this.providerRef.deref()
const existingStatus = provider?.taskHistoryStore.get(this.taskId)?.status
await provider?.updateTaskHistory(existingStatus ? { ...historyItem, status: existingStatus } : historyItem)
return true
} catch (error) {
console.error("Failed to save Roo messages:", error)
return false
// The message array was persisted above; a metadata or task-history failure must
// not mask that write (see the method docs). The next saveClineMessages() call
// recomputes and re-emits the metadata update.
console.error("Failed to save task metadata:", error)
}

return true
}

private findMessageByTimestamp(ts: number): ClineMessage | undefined {
Expand Down Expand Up @@ -1955,6 +1972,56 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
return formatResponse.toolError(formatResponse.missingToolParameterError(paramName))
}

/**
* Finalize a partial "tool" ask message without blocking for user input.
* Call this in error paths where a partial tool message was opened during streaming
* but execution failed before the normal approval flow could close it, so the webview
* spinner does not get stuck in a loading state.
*
* The matching partial message may no longer be the final entry if another asynchronous
* message was inserted between the partial ask and the error handler, so search backward
* instead of relying on clineMessages.at(-1).
*
* Any in-progress `progressStatus` on the message is cleared as well: the ask is being
* finalized because it will NOT complete, so a stale "in progress" indicator would be
* misleading (the normal completion path overwrites it with the final status instead).
*
* `isAnswered` is stamped true because the ask is resolved by the system rather than
* by the user: ChatView only shows ask buttons for unanswered messages, so leaving it
* unset would keep Save/Reject armed for a write that already failed.
*/
async finalizePartialToolAsk(text?: string): Promise<void> {
const partialToolAsk = findLast(
this.clineMessages,
(message) =>
message.partial === true &&
message.type === "ask" &&
message.ask === "tool" &&
(text === undefined || message.text === text),
)

if (!partialToolAsk) {
return
}

partialToolAsk.partial = false
partialToolAsk.progressStatus = undefined
partialToolAsk.isAnswered = true
const saved = await this.saveClineMessages()
if (!saved) {
// The persistence write failed: the on-disk record still carries `partial: true`
// while the in-memory message is finalized. Skip the webview-only update so the
// two views do not diverge (a later state resync or restart reload would flip the
// spinner back on from the stale disk record). The next saveClineMessages() call
// re-persists the full message array and repairs the disk record.
console.error("[Task#finalizePartialToolAsk] saveClineMessages failed; skipping webview update")
return
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
await this.updateClineMessage(partialToolAsk).catch((error) => {
console.error("[Task#finalizePartialToolAsk] updateClineMessage failed:", error)
})
}

// Lifecycle
// Start / Resume / Abort / Dispose

Expand Down
Loading
Loading