fix: make permanent workspace delete remove everything it owns - #347
Open
ProfSynapse wants to merge 2 commits into
Open
fix: make permanent workspace delete remove everything it owns#347ProfSynapse wants to merge 2 commits into
ProfSynapse wants to merge 2 commits into
Conversation
Deleting a workspace ran one `DELETE FROM workspaces WHERE id = ?` and left every child row and both of its JSONL streams behind. Measured in a real vault, deleting a workspace with 2 sessions / 2 states / 2 traces / 1 project / 2 tasks left: sessions 2, states 2, memory_traces 2, projects 1, tasks 2 in SQLite, plus `workspaces/ws_<id>/` (8 events) and `tasks/tasks_<id>/` (3 events) on disk. Running "Rebuild cache" reproduced that orphan set exactly, because SQLite is dropped and replayed from those two surviving streams. Two independent causes: - The schema declares `ON DELETE CASCADE` on sessions/states/memory_traces/ projects/tasks, but FK enforcement is off on the shared connection (SQLite's per-connection default — schema.ts already says so for `note_properties`). Nothing ever cascaded, in the live delete or in `applyWorkspaceDeleted`. - Nothing removed the event streams. The tombstone made `reconcileMissingWorkspaces` skip the workspace stream, but `fullRebuild` replays every stream it lists, and the tasks stream is a SEPARATE file that carries no tombstone at all. `workspaceOwnership.ts` now states once what a workspace owns — the ordered row purge and both stream paths — and both the live delete and the replay of that delete go through it, so they cannot drift apart. Conversations are deliberately excluded: `workspaceId` there is a nullable back-reference, not ownership. Ordering is deliberate, because the two halves are not symmetric. Tombstone, then streams, then SQLite. If stream removal fails we throw before touching the cache, so nothing is destroyed, the workspace still lists, and the delete is a retryable no-op; `WorkspaceService` already skips its 'deleted' notification when the write throws. If the (local, transactional) SQLite purge fails, the rows are stale cache over an event store that no longer has the workspace, and the next rebuild clears them. Every partial failure converges toward deleted. The opposite order converges on the workspace coming back with all its states at the next rebuild, which is the shape of #333. This stays UI-only. No tool is added and none becomes reachable — the memory agent still exposes only the reversible `archiveWorkspace`. Verified in a real headless Obsidian on a cold start: delete now leaves 0 rows in every workspace-keyed table and removes both stream directories, and `rebuildCache()` brings nothing back. A stream that survives with a tombstone (sync from another device, or a removal we failed and threw on) replays to nothing too. Simulating a locked vault leaves the workspace fully intact and the retry completes it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C6aSoCAS5gNoew6n9qv6DJ
The nexus-storage "Data reappeared after a rebuild" entry blamed a missing deletion event. Workspace deletion appended one and the data still came back, because three other conditions were unmet: the applier handling the tombstone deleted less than the live delete did, no declared ON DELETE CASCADE fires (FK enforcement is off and nothing turns it on), and a workspace owns two streams so a tombstone in one says nothing about the other. Names the check that would have caught it: rebuildCache() plus a per-table count in the running app, not "the row is gone from the UI". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C6aSoCAS5gNoew6n9qv6DJ
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.
Permanent workspace deletion removed the
workspacesrow and left everything else behind.Stays UI-only: no delete tool is added and none becomes reachable. The AI still gets
archiveWorkspace, which is reversible.Measured before fixing
Seeded a workspace with 2 sessions / 2 states / 2 traces / 1 project / 2 tasks in a real headless Obsidian and deleted it through the exact
WorkspacesTabcall. Only theworkspacesrow and its FTS entry went.Survivors: sessions 2, states 2, memory_traces 2, projects 1, tasks 2, plus
workspaces/ws_<id>/(8 events, tombstone appended) andtasks/tasks_<id>/(3 events, untouched).rebuildCache()reproduced that orphan set identically, so this was never cache-only residue. The original report of "orphan states rows" understated it — every child table survived.Two independent causes, both proven
ON DELETE CASCADEs never fire. SQLite foreign-key enforcement is off and nothing turns it on —grep -rn "foreign_keys" src/returns nothing.schema.tsalready notes this fornote_properties; it applies file-wide.StorageRouter.deleteFileonly reached a legacy flat file that a vault-root install never wrote, and the tasks stream carries no tombstone at all.What a workspace actually owns
SQLite: sessions, states, memory_traces, projects, tasks (and
task_dependencies/task_note_linksthrough them), plustrace_embedding_metadataand its vec0 rows — which nothing in the system would ever have collected, sinceclearAllDataskips them too.workspace_ftswas already handled by a delete trigger.JSONL: two streams, not one —
workspaces/ws_<id>andtasks/tasks_<id>.Conversations are deliberately not owned — nullable back-reference, own stream, outlives the workspace. A test pins that boundary so a future "tidy up" does not quietly widen the blast radius.
Ownership is declared once
New
src/database/workspaceOwnership.ts. Both the live delete andapplyWorkspaceDeleted(the replay path) go through it, so they cannot drift — and that drift is precisely what made the rebuild resurrect the children.Stream removal is new and idempotent, from
ShardedJsonlStreamStoreup throughJSONLWriter. It removes cloud-sync conflict siblings too; leaving one turns a delete into a partial truncation.Partial failure, chosen rather than incidental
Order: tombstone → streams → SQLite. Throw on any failure. Purge the cache only if the JSONL side finished.
The halves are not symmetric, so the choice is by direction of convergence:
Both converge on deleted. SQLite-first converges on the workspace coming back with all its states, which is the #333 shape.
The tasks stream goes first so the tombstone-bearing stream survives longest: once the tombstone lands, every later failure still resolves to deleted, leaving only disk bytes.
Evidence
12 regression tests; 11 fail against pre-fix code (verified by checking out the base commit's
src/). The 12th is the conversations-boundary guard, which correctly passes both ways.Live on a cold start: after delete every table 0 and both stream directories gone; after
rebuildCache(), identical — nothing returned. A stream left on disk with a tombstone replays to 0/0/0/0. Full process restart: no resurrection,dev:errorsclean. A simulatedEBUSYvault throws with the workspace fully intact, and a retry completes it.npx jest tests/unit tests/integration— 4378 passing.npm run buildgreen.Neighbouring gaps found, measured, not fixed
None qualified as small-and-obviously-correct:
deleteSessionwrites no event at all, so a deleted session comes back on rebuild (0 → 1) and its states are never removed. Same family, and the most serious one left.🤖 Generated with Claude Code
https://claude.ai/code/session_01C6aSoCAS5gNoew6n9qv6DJ
Generated by Claude Code