fix(task): guard saveClineMessages against abandoned tasks (fixes #1021) by easonLiangWorldedtech · Pull Request #1496 · Zoo-Code-Org/Zoo-Code

easonLiangWorldedtech · 2026-09-03T01:26:53Z

Tracking issue: #1387

Supersedes #1382 (same branch). #1382 was opened on 2026-08-27, before the org's .coderabbit.yaml (introduced in #1433, 2026-08-29) existed on main; 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 execute updateTaskHistory() after abandonSubtask's atomicUpdatePair() had already cleared parentTaskId/'rootTaskId', silently reattaching the severed parent-child link.

Changes

src/core/task/Task.ts — saveClineMessages() now checks this.abandoned before updateTaskHistory() and returns false when the task was abandoned. This catches both the explicit abort save and any in-flight fire-and-forget saves (the two un-awaited saveClineMessages().catch(...) call sites).
Per-task message persistence is unaffected: 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 upstream main so 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-stats branch (commit 1d1eb915e, "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 in main (the dashboard feature ships separately in #1225), so those hunks are deliberately excluded here — the behavioral fix is the guard itself.

Test

Regression test in Task.spec.ts (saveClineMessages abandoned guard (#1021)): an abandoned task's saveClineMessages() persists messages (saveTaskMessages called once) but never calls updateTaskHistory.
Non-abandoned control and in-flight deferred-save cases covered; all 9 guard lines covered per lcov.
pnpm --dir src exec vitest run core/task/__tests__/Task.spec.ts → 101/101 passing.
ESLint clean; no suppression count changes.

…-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.

coderabbitai · 2026-09-03T01:27:01Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: 9c775915-d56b-4c03-9906-0dc283592350

📥 Commits

Reviewing files that changed from the base of the PR and between 0d937c0 and d48f46d.

📒 Files selected for processing (2)

src/core/task/Task.ts
src/core/task/__tests__/Task.spec.ts

Included review availability: Your plan provides up to 4 included reviews per hour; 1 remains after this review.

📜 Recent review details

🧰 Additional context used

📓 Path-based instructions (5)

Check persistence and lifecycle invariants: awaited atomic writes, rollback or explicit partial-failure behavior, cross-window state consistency, stale listeners/watchers, cancellation, idempotency, and safe restart/resume without lost or d...