fix(task): skip saveClineMessages when history task aborts before messages load

[edelauna]

Related GitHub Issue

Closes: #1180

Description

resumeTaskFromHistory() starts with an async disk read (getSavedClineMessages). Until that read completes, clineMessages is []. evictCurrentTask() calls abortTask(), which called saveClineMessages() → taskMetadata(). With an empty array, taskMetadata() writes the no_messages placeholder as the stored title, permanently overwriting the real one.

On Windows, ui_messages.json files can reach 17–20 MB (read_file payloads stored in clineMessages). With Defender real-time scanning, the read can take hundreds of milliseconds — wide enough for rapid history navigation to hit this window reliably. This is the "Work #1 (no message)" / "工作 #1 (無訊息)" bug reported against v3.76.0.

Fix: guard in abortTask() — if the task is a history task and clineMessages is still empty, skip saveClineMessages(). The on-disk data is already the correct source of truth at that point.

This bug first appeared in v3.74.0 (PR #1014, subtask navigation buttons). v3.72.0 is not affected.

Test Procedure

Unit test (src/core/task/__tests__/Task.resume-eviction-race.spec.ts):

• Constructs a history task with startTask: false
• Holds the readTaskMessages disk read open via a deferred promise
• Calls task.run() fire-and-forget (mirrors createTaskWithHistoryItem)
• Calls task.abortTask(true) while the read is still pending
• Asserts updateTaskHistory was not called with a title containing "no_messages"

E2E smoke test (apps/vscode-e2e/src/suite/resume-eviction-race.test.ts):

• Runs a task to completion, then resumes and immediately evicts it
• Asserts the stored title is unchanged after the round-trip

Note: the e2e test cannot reliably trigger the race on Linux (disk reads complete in < 1 ms). The unit test is the regression anchor.

Pre-Submission Checklist

• Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
• Scope: My changes are focused on the linked issue (one major feature/fix per PR).
• Self-Review: I have performed a thorough self-review of my code.
• Testing: New and/or updated tests have been added to cover my changes (if applicable).
• Visual Snapshot (UI changes only): Not applicable — no UI changes.
• Documentation Impact: No documentation updates required.
• Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Additional Notes

A related open issue (#1021) guards updateTaskHistory against fire-and-forget saves on abandoned tasks (different race, same code path). That fix is pending on feature/local-usage-stats. The two fixes are orthogonal and will not conflict.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Prevented resumed tasks from losing their saved titles when quickly interrupted or removed.
  • Preserved task history during resume and eviction race conditions.

• Tests

  • Added end-to-end and regression coverage for resume, interruption, eviction, and title persistence scenarios.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 49ec9e85-fbd2-4347-bf8f-871092041ae7

📥 Commits

Reviewing files that changed from the base of the PR and between c66ca51 and c293703.

📒 Files selected for processing (1)

• src/core/task/__tests__/Task.resume-eviction-race.spec.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/core/task/tests/Task.resume-eviction-race.spec.ts

---

📝 Walkthrough

Walkthrough

The abort path no longer saves empty messages for history tasks while resume loading is pending. Unit and VS Code E2E tests cover rapid eviction and verify that persisted task titles remain unchanged.

Changes

Resume eviction title preservation

Layer / File(s)	Summary
Abort save guard and unit regression test src/core/task/Task.ts, src/core/task/__tests__/Task.resume-eviction-race.spec.ts	abortTask skips saving unloaded history tasks. The unit test holds message loading open, aborts the task, and verifies that no "no_messages" title update is written.
End-to-end race regression apps/vscode-e2e/fixtures/resume-eviction-race.json, apps/vscode-e2e/src/suite/resume-eviction-race.test.ts	The fixture completes a task. The E2E test resumes and immediately evicts it, then verifies that the original persisted title remains unchanged.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

• fix(task): guard saveClineMessages against abandoned tasks to prevent race in abandonSubtask #1021 — Both changes modify Task.ts abort/save behavior to prevent task-history corruption, but they address different race conditions.

Possibly related PRs

• Zoo-Code-Org/Zoo-Code#1031 — Modifies Task.ts and task resume lifecycle behavior related to this abort fix.
• Zoo-Code-Org/Zoo-Code#1123 — Also modifies Task.ts, although it addresses usage-statistics recording rather than this persistence race.

Suggested labels: awaiting-review

Suggested reviewers: navedmerchant

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main fix: skipping message saves when a history task aborts before messages load.
Description check	✅ Passed	The description includes the linked issue, root cause, implementation details, test procedures, checklist, and relevant reviewer notes.
Linked Issues check	✅ Passed	The guard and regression tests directly address issue #1180 by preserving task titles during resume-and-eviction races.
Out of Scope Changes check	✅ Passed	The implementation, unit test, E2E fixture, and E2E test are directly related to the linked issue and stated fix.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/resume-eviction-title-clobber

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

src/core/task/__tests__/Task.resume-eviction-race.spec.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

src/core/task/__tests__/Task.resume-eviction-race.spec.ts (1)

105-130: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Replace the double assertion with a precise provider test double.

as unknown as ClineProvider bypasses type checking for the mock contract. Define the required public members with Pick<ClineProvider, ...> or document why a double assertion is required. As per coding guidelines, “Avoid as any; use typed APIs, bracket notation for private members, or precise test doubles and type guards. Use double assertions only as a last resort and document them.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/task/__tests__/Task.resume-eviction-race.spec.ts` around lines 105 -
130, Update makeMockProvider to use a precise Pick<ClineProvider, ...>
test-double type covering only the public members this mock supplies and
consumes, eliminating the undocumented as unknown as ClineProvider assertion.
Preserve the existing mocked behavior while allowing TypeScript to validate the
provider contract.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/vscode-e2e/src/suite/resume-eviction-race.test.ts`:
- Around line 89-95: Update the assertion in the resume-eviction race test to
compare afterEviction.task exactly with beforeResume.task, rather than checking
only for the RESUME_EVICTION_RACE_SMOKE substring. Preserve the existing
diagnostic context while verifying the persisted title is unchanged across
resume.

In `@src/core/task/__tests__/Task.resume-eviction-race.spec.ts`:
- Around line 193-198: Update the abort-race assertion in
Task.resume-eviction-race.spec.ts to verify that updateTaskHistory has not been
called at all before readDeferred resolves, rather than matching a specific
"no_messages" task value. Preserve the test’s focus on ensuring abort does not
persist any task metadata.

---

Nitpick comments:
In `@src/core/task/__tests__/Task.resume-eviction-race.spec.ts`:
- Around line 105-130: Update makeMockProvider to use a precise
Pick<ClineProvider, ...> test-double type covering only the public members this
mock supplies and consumes, eliminating the undocumented as unknown as
ClineProvider assertion. Preserve the existing mocked behavior while allowing
TypeScript to validate the provider contract.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5cb2f00b-54f5-4bde-b75d-1db3d07751a6

📥 Commits

Reviewing files that changed from the base of the PR and between f149073 and 0c680cf.

📒 Files selected for processing (4)

• apps/vscode-e2e/fixtures/resume-eviction-race.json
• apps/vscode-e2e/src/suite/resume-eviction-race.test.ts
• src/core/task/Task.ts
• src/core/task/__tests__/Task.resume-eviction-race.spec.ts

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/core/task/__tests__/Task.resume-eviction-race.spec.ts (1)

187-187: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Make the deferred-resume fixture cover every read path.

resumeTaskFromHistory() calls getSavedClineMessages() twice, so the one-time mock value makes the second read resolve undefined. Configure a response for every required read before awaiting resume completion. Keep task.run() failures uncaught so unexpected resume and setup failures remain visible.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/task/__tests__/Task.resume-eviction-race.spec.ts` at line 187,
Update the resumeTaskFromHistory test fixture around mockReadTaskMessages so it
provides deferred responses for both getSavedClineMessages() reads before
awaiting resume completion, instead of configuring only a one-time response.
Preserve uncaught task.run() failures so unexpected resume or setup errors
remain visible.

🧹 Nitpick comments (1)

src/core/task/__tests__/Task.resume-eviction-race.spec.ts (1)

126-127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Keep the updateTaskHistory mock fully typed.

The cast through unknown bypasses ClineProvider["updateTaskHistory"]. If that method changes, this test can still compile with an incompatible mock. Define the mock with the exact method signature and pass that type through makeMockProvider.

As per coding guidelines, use typed APIs and use double assertions only as a last resort with documentation.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/task/__tests__/Task.resume-eviction-race.spec.ts` around lines 126 -
127, Update the updateTaskHistory mock in the resume-eviction race test to use
the exact ClineProvider["updateTaskHistory"] method signature, then pass that
fully typed mock through makeMockProvider. Remove the unknown-based double
assertion so future method signature changes are caught by compilation.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/core/task/__tests__/Task.resume-eviction-race.spec.ts`:
- Line 209: In the race test, wait until the deferred read has started by
asserting that mockReadTaskMessages has been called before invoking
task.abortTask(true). Keep the existing abort behavior and test scenario
unchanged.

---

Outside diff comments:
In `@src/core/task/__tests__/Task.resume-eviction-race.spec.ts`:
- Line 187: Update the resumeTaskFromHistory test fixture around
mockReadTaskMessages so it provides deferred responses for both
getSavedClineMessages() reads before awaiting resume completion, instead of
configuring only a one-time response. Preserve uncaught task.run() failures so
unexpected resume or setup errors remain visible.

---

Nitpick comments:
In `@src/core/task/__tests__/Task.resume-eviction-race.spec.ts`:
- Around line 126-127: Update the updateTaskHistory mock in the resume-eviction
race test to use the exact ClineProvider["updateTaskHistory"] method signature,
then pass that fully typed mock through makeMockProvider. Remove the
unknown-based double assertion so future method signature changes are caught by
compilation.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7c4629c5-c488-4db1-8ec3-ad3ce8433afc

📥 Commits

Reviewing files that changed from the base of the PR and between 0c680cf and c66ca51.

📒 Files selected for processing (2)

• apps/vscode-e2e/src/suite/resume-eviction-race.test.ts
• src/core/task/__tests__/Task.resume-eviction-race.spec.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• apps/vscode-e2e/src/suite/resume-eviction-race.test.ts