fix(task): guard saveClineMessages against abandoned tasks (fixes #1021) - #1496
Open
easonLiangWorldedtech wants to merge 5 commits into
Open
Conversation
…-Code-Org#1021) Fire-and-forget saveClineMessages() calls could execute updateTaskHistory() after abandonSubtask's atomicUpdatePair() had already cleared parentTaskId/rootTaskId, silently reattaching the severed parent-child link. Check this.abandoned before updateTaskHistory() to catch both the explicit abort save and any in-flight fire-and-forget saves. Per-task message persistence is unaffected: saveTaskMessages still runs, only the (stale) history-item update is skipped. This is the minimal upstream-main form of the fix developed on the local-usage-stats branch (commit 1d1eb91); that commit's surrounding usage-stats changes are not part of main and are excluded. Regression test in Task.spec.ts: an abandoned task's saveClineMessages() persists messages but never calls updateTaskHistory().
Per CodeRabbit review: document why the provider test double uses the as unknown as MockedClineProvider double assertion (Task receives a full ClineProvider at runtime; this focused unit test only exercises a few methods) — same pattern and rationale as the existing Subtask Rate Limiting block.
…ad review gate (no code change)
Per CodeRabbit review on PR Zoo-Code-Org#1382, the focused task-history tests used a provider double without taskHistoryStore, so provider?.taskHistoryStore.get() threw before the guard was evaluated and the catch returned false - the assertions passed without ever exercising updateTaskHistory. Stub taskHistoryStore.get() on the provider double (shared makeMockProvider helper) so execution reaches updateTaskHistory; add a non-abandoned control asserting updateTaskHistory() runs and the item is written as-is (covers the guard false branch); add an in-flight save test where saveTaskMessages() is deferred, the task is abandoned mid-save, and the guard must still skip the history update when the save resolves. All PR-changed lines in Task.ts (guard at 1138-1140) are now 100% covered on lines and branches: true branch hit twice (abandoned + in-flight), false branch once (control).
Branch base 78c712a predates Zoo-Code-Org#1433 (efc30cf, 2026-08-29), which added the CodeRabbit config to main. Without it, CodeRabbit reviews this head with defaults (request-changes workflow disabled) and can only submit COMMENTED reviews; it never submits the APPROVED review that the PR review gate requires. Sync the config (incl. Zoo-Code-Org#1490) so reviews on this head use the org adversarial review profile and the formal review workflow, letting the gate advance.
easonLiangWorldedtech
requested review from
JamesRobert20,
edelauna,
hannesrudolph,
navedmerchant and
taltas
as code owners
September 3, 2026 01:26
Contributor
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
Contributor
Review processThanks for contributing. This comment tracks the review sequence and the next action.
Current step: Ready for human maintainer review and approval. |
Contributor
Author
|
@coderabbitai approve |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
✅ Action performedComments resolved and changes approved. |
github-actions
Bot
added
the
awaiting-maintainer
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #1382 (same branch). #1382 was opened on 2026-08-27, before the org's
.coderabbit.yaml(introduced in #1433, 2026-08-29) existed onmain; its CodeRabbit review session therefore started on the default configuration and cannot submit the formal APPROVED review the review gate requires on a PR head. This PR re-opens the identical change under a fresh review session (head already synced with the org config file).Summary
Fixes #1021 (part of epic #1375).
Fire-and-forget
saveClineMessages()calls could executeupdateTaskHistory()afterabandonSubtask'satomicUpdatePair()had already clearedparentTaskId/'rootTaskId', silently reattaching the severed parent-child link.Changes
src/core/task/Task.ts—saveClineMessages()now checksthis.abandonedbeforeupdateTaskHistory()and returnsfalsewhen the task was abandoned. This catches both the explicit abort save and any in-flight fire-and-forget saves (the two un-awaitedsaveClineMessages().catch(...)call sites).saveTaskMessages()still runs before the guard; only the (stale) history-item update is skipped, which is exactly the value that would reattach the severed link..coderabbit.yaml— synced from upstreammainso the review session picks up the org's CodeRabbit configuration (reviews.request_changes_workflow).Provenance
This is the minimal upstream-main form of the fix developed on the
feature/local-usage-statsbranch (commit1d1eb915e, "fix(task): guard saveClineMessages against abandoned tasks to prevent race in abandonSubtask"). That commit also carried formatting changes for usage-stats code that is not inmain(the dashboard feature ships separately in #1225), so those hunks are deliberately excluded here — the behavioral fix is the guard itself.Test
Task.spec.ts(saveClineMessages abandoned guard (#1021)): an abandoned task'ssaveClineMessages()persists messages (saveTaskMessagescalled once) but never callsupdateTaskHistory.pnpm --dir src exec vitest run core/task/__tests__/Task.spec.ts→ 101/101 passing.