fix(server): allow re-creating a thread id after a soft delete#4184
fix(server): allow re-creating a thread id after a soft delete#4184sideeffffect wants to merge 4 commits into
Conversation
When a draft thread's bootstrap turn start fails partway (for example, worktree preparation fails), the server cleans up by deleting the just-created thread. Thread deletion is a soft delete: the thread stays in the read model with a non-null deletedAt. Because requireThreadAbsent only checked for the thread's presence — not whether it was soft-deleted — a client retrying with the same draft thread id then failed with "Thread '<id>' already exists and cannot be created twice.", leaving the user unable to start the conversation at all. Treat soft-deleted threads as absent so the retry succeeds.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
github-actions
Bot
added
size:XS
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Approved Straightforward bug fix allowing thread ID re-creation after soft delete, with a single condition change and cleanup handlers that follow existing patterns. Bulk of additions are comprehensive tests covering both positive and negative cases. You can customize Macroscope's approvability policy. Learn more. |
Address review: thread deletion is a soft delete and the persistent projection keeps child rows (messages, activities, proposed plans, checkpoints, sessions) keyed by threadId. Treating every soft-deleted thread as absent could resurrect an id whose child rows still exist, surfacing stale records on the re-created thread. Restrict reuse to a content-free tombstone — no messages, activities, proposed plans, checkpoints, session, or turn — which is exactly the state a draft thread is left in when a bootstrap turn start fails before the turn runs. A tombstone that owned content stays blocked. Added a negative test covering that case.
github-actions
Bot
added
size:M
Address review: instead of a content-sniffing heuristic in requireThreadAbsent (which the command read model cannot evaluate reliably — getCommandReadModel never loads messages/activities/checkpoints — and which wrongly blocked retries that recorded setup-script activities), allow re-creating any soft-deleted thread id and make the re-creation clean at the source. Each thread-keyed persistent projector (messages, proposed plans, activities, sessions, turns/checkpoints) now purges its own rows for the id on thread.created, matching the in-memory projector which already resets the thread to an empty record on re-create. The purge is a no-op for a brand-new id and only clears leftovers from a prior lifecycle, and because each projector touches only its own table it stays replay-safe under independent reprojection. Added a ProjectionPipeline test asserting a recreated thread inherits no stale messages/activities, and a decider test that a live (non-deleted) id is still rejected.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 28b58ea. Configure here.
Address review: the pending-approvals projector was the one thread-keyed projection left unpurged on thread.created, so a reused thread id could carry a stale pendingApprovalCount into refreshThreadShellSummary. It clears its rows on thread.created too now (listByThreadId + deleteByRequestId, since the row is keyed by requestId). Extended the recreate pipeline test to cover pending approvals.
1 participant
What Changed
requireThreadAbsent(the invariant guardingthread.create) now treats a soft-deleted thread as absent, so a thread id can be created again after its predecessor was deleted. Added an end-to-end decider test: soft-delete a thread, then re-create the same id and assert athread.createdevent is produced and the thread is resurrected.Why
Thread deletion is a soft delete — the projector sets
deletedAtbut keeps the thread in the read model (projector.ts,thread.deleted). The bootstrap turn-start path creates a draft thread and, if a later step fails (e.g. worktree preparation), cleans up by dispatchingthread.delete(ws.ts,cleanupCreatedThread). That leaves a tombstone.The client's retry re-uses the same draft thread id. Because
requireThreadAbsentonly checked presence — not whether the thread was soft-deleted — the retry failed with:The user was then stuck: every retry hit the same tombstone and the conversation could never start. This is exactly the second failure reported after a first bootstrap failure. Treating
deletedAt !== nullas absent lets the retry succeed. A live (non-deleted) thread is still correctly rejected — covered by the existingcommandInvariantstest.This is the natural companion to #4183: that PR removes the first-failure cause (worktree prep on a commit-less repo); this PR ensures a bootstrap failure from any cause can't wedge the draft id.
UI Changes
None.
Checklist
Note
Medium Risk
Touches orchestration invariants and multiple projection handlers on
thread.created; wrong purge logic could drop data on first create or leave stale rows, though tests target the recreate path.Overview
Fixes bootstrap retries that reuse the same draft thread id after a failed turn start left a soft-deleted tombstone.
requireThreadAbsentnow treats threads withdeletedAtset as absent, sothread.createsucceeds on retry while live threads still fail with "already exists."On
thread.created, persistent projectors clear prior-lifecycle rows for thatthreadId(messages, activities, turns, sessions, proposed plans, and pending approvals) so the re-created thread does not surface stale data—aligned with the in-memory projector reset.Decider and projection-pipeline tests cover recreate-after-delete and unchanged behavior for live ids.
Reviewed by Cursor Bugbot for commit 854c0d2. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Allow re-creating a thread after soft delete by purging stale projection rows
requireThreadAbsentin commandInvariants.ts to treat soft-deleted threads (non-nulldeletedAt) as absent, permittingthread.createcommands for those IDs.thread.createdevent, each projector in ProjectionPipeline.ts now purges stale rows for the thread ID: messages, proposed plans, activities, sessions, turns, and pending approvals.requestIdafter alistByThreadIdquery, since no bulk-delete exists for that table.Macroscope summarized 854c0d2.