[Fix] Task history disappears when user reopens a task - #1319
[Fix] Task history disappears when user reopens a task#1319zoomote[bot] wants to merge 4 commits into
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
Looks like this might also contribute to fixing some of the things I've experienced in #1231, too |
|
Addressed the retry and merge feedback in 3f5b2d04:
The full Zoo Code test suite passes (7,560 tests), and the previously uncovered Task/ClineProvider branches now have direct regression coverage. All three review threads were replied to and resolved. |
c3492b0 to
8754395
Compare
github-actions
Bot
added
the
awaiting-author
| ) | ||
| return [] | ||
| try { | ||
| await fs.unlink(oldPath) |
There was a problem hiding this comment.
I think we should write a wrdning. Although the data is migrated, a history file gets deleted. worth at least a warning in the log:
console.warn(
`[readApiMessages] Found OLD API conversation history file (claude_messages.json): contained data was migrated to the new history and the file removed.`,
)
There was a problem hiding this comment.
actually, the migration does NOT happen here. Only the file deleted after successful read. So where do they get merged or saved into the new message? - I only ever see saveApiMessages() called from tests, but not from live code. It's probably me, not seeing it ... ;-)
There was a problem hiding this comment.
Addressed locally: readApiMessages() now persists parsed legacy claude_messages.json history through the locked safeWriteJson(..., { merge: mergeApiMessageSnapshots }) path before deleting the legacy file. Regression coverage in apiMessages.spec.ts verifies the migrated history is persisted and that cleanup only happens after persistence succeeds.
| function mergeTimestampedSnapshots( | ||
| existing: unknown, | ||
| incoming: unknown, | ||
| mergeMatch: (disk: MessageRecord, next: MessageRecord) => MessageRecord, |
There was a problem hiding this comment.
should this maybe default to undefined?
see comment line 40
There was a problem hiding this comment.
Addressed locally: mergeTimestampedSnapshots() now declares mergeMatch as optional, so API snapshot merging can use the helper without supplying an identity callback.
| const consumed = consumedByTimestamp.get(message.ts) ?? 0 | ||
| consumedByTimestamp.set(message.ts, consumed + 1) | ||
| const diskMessage = existingGroups.get(message.ts)?.[consumed] | ||
| return diskMessage ? mergeMatch(diskMessage, message) : message |
There was a problem hiding this comment.
see line 10
I think this line would be easier to understand if it would read:
if (mergeMatch !== undefined && diskMessage !== undefined) {
return mergeMatch(diskMessage, message)
}
return messagerather then the same functionality being burried in mergeApiMessageSnapshots's (_disk, next) => next on line 84
There was a problem hiding this comment.
Addressed locally: mergeTimestampedSnapshots() now explicitly returns the incoming message when there is no callback or matching disk message. mergeApiMessageSnapshots() therefore calls the helper directly and no longer passes (_disk, next) => next.
github-actions
Bot
removed
the
awaiting-author
3 participants
What changed
Why this change was made
Reopening or quickly leaving a saved task could replace valid history with an empty or shortened message list. Concurrent extension instances could also overwrite a newer history suffix with a stale snapshot. This resolves #1279 and extends the lifecycle and persistence safety work tracked by #355, #208, and #1231.
Impact
Users keep their last saved task content when reopening or resuming tasks. Temporary storage gaps are retried, ordinary concurrent writes preserve newer messages, and stale task instances cannot write after being evicted. Explicit user-driven history rewinds continue to replace history as intended.
Related PRs