Skip to content

feat(pages): add page deletion with safe subpage reparenting - #49

Open
Pioneer113 wants to merge 10 commits into
CopilotKit:mainfrom
Pioneer113:feat/page-deletion
Open

Pioneer113 wants to merge 10 commits into
CopilotKit:mainfrom
Pioneer113:feat/page-deletion

Conversation

@Pioneer113

@Pioneer113 Pioneer113 commented Oct 3, 2026 •

Copy link
Copy Markdown

User workflow

Users can create pages, save AI drafts, organize subpages, and move pages within Spaces, but there was previously no way to delete unwanted or temporary pages.

This PR adds page deletion to the document workspace. When a user deletes a page, any nested subpages are reparented to the deleted page's parent (or moved to the root level if the deleted page was at the top level), so no child documents are lost.

Changes

  • Core (src/server/pages.ts): Pages.delete(spaceId, id) runs in one BEGIN IMMEDIATE transaction. It reparents children (bumping their revision so clients resync the new parent), removes the page's page_threads binding, deletes the page, and validates the Space. It returns false for a missing page. New Pages.exists(spaceId, id) helper.
  • API (src/server/page-routes.ts): DELETE /spaces/:spaceId/pages/:id returns { ok: true } or 404 for a missing page or Space. The review receipt endpoint (GET /conversations/:id/reviewed-page/:toolCallId) now returns { deleted: true, pageId, spaceId, reviewDraft } when the reviewed page was deleted, instead of a 404.
  • Client: Delete page in the document menu with a confirmation dialog and an error notice. On success the page leaves the library immediately: SpaceWorkspace removes it, re-parents its children locally and keeps a tombstone so a stale poll cannot bring it back (mergePageSnapshot otherwise never removes pages). It also drops the pending autosave and the dirty flag before navigating home, so the "Leave your unsaved page draft?" prompt does not appear for a page that no longer exists. PageReviewCard (rebased onto fix: bind reviewed-page retries to saved draft #48's ReviewedPage flow) shows "Saved, then deleted" for such a review, with no "Open page" link, and lets the conversation continue (the agent is told not to link the page). The draft-binding check from fix: bind reviewed-page retries to saved draft #48 also applies to deleted reviews. The delete confirmation now says "This can't be undone."
  • Docs: docs/SETUP.md lists page deletion.

Design decisions

  • page_reviews rows are kept on delete. createReviewed uses them for idempotent retries ("retries recover the same saved page"). Deleting them would let a retried approval silently recreate a deleted page. A retry after deletion now fails with 404 instead.
  • Conversation history is not deleted. Only the local page_threads binding is removed. The page's conversation thread stays in the configured Intelligence project; no Intelligence deletion call is made.
  • Deletion is not exposed to agents as a tool. It is an owner-only UI/API action.

Known limitations

  • A concurrent PageService.conversation() call racing with a delete could leave an orphaned page_threads row (no foreign keys in the schema). The window is small and the row is unreachable because lookups join against pages.
  • A page deleted from another tab, or by an agent, is not removed from an already open tab until that tab reloads: the library merge never treats absence as deletion, and only deletions made in this tab are tombstoned.
  • The delete-menu flow and the dirty-draft behaviour were checked manually (see below). PageDocument's deletion lifecycle and the review card's deleted-receipt handling also have react-test-renderer tests.

Verification

  • npm test — 243/243 passed on top of current main (also under Node 24.21).
  • npm run typecheck, npm run lint, npm run check-format — passed.
  • npm run build — succeeded.
  • New tests: reparenting, thread-binding cleanup, retried-review-does-not-recreate (tests/pages.test.ts); DELETE route and deleted-review receipt (tests/page-routes.test.ts); deleted-review recognition (tests/page-review.test.tsx). The new regression tests fail on the code before the fix.

Manual UI check (dev server, local)

  • Deleting a page that has a subpage: the library goes from 3 to 2 pages and the subpage now shows its new parent.
  • Deleting while the editor shows "Unsaved changes": the only confirmation is the delete one; no "Leave your unsaved page draft?" prompt.
  • 375 px wide viewport: the actions menu fits the screen and the page has no horizontal scroll.
  • Keyboard: ArrowUp in the open menu focuses "Delete page" with a visible focus ring; Escape closes the menu and returns focus to "Page actions".
  • Live review flow (local dev server, Intelligence project plus an OpenRouter model, one review_space_page call): the review card is ready → Approve & save → the saved page opens → Delete page (the confirmation reads "This can't be undone…") → reload the chat. The card now shows "Saved, then deleted", with no "Open page" link and no restore error.

@NathanTarbert NathanTarbert left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Pioneer113, this is a really thorough PR. The description explains each decision: keeping review receipts, leaving the Intelligence thread alone, owner-only, not exposed to agents. It's also upfront about what's still open, which makes it much easier to review. The tombstones are a nice fix for the old assumption in mergePageSnapshot that a missing row means a deleted page. And the test that a retried review can't bring a deleted page back covers the guarantee that matters most.

The server side holds up well. Deleting a middle page moves its children to the grandparent and leaves grandchildren alone. Deleting a root page lifts its children to the top level. Deleting twice, deleting through another Space's id, and saving from a stale tab all end in a clean 404 rather than a partial change.

One thing has changed underneath it: #48 merged today and reworked the same review-card code. The receipt GET now returns the page with its reviewDraft, and PageReviewCard.tsx asks the server for every card, including ones that recorded an approval. So this needs a rebase, mostly to fold the deleted response into #48's ReviewedPage flow. The good news is that because every card now checks the server, the second known limitation in your description, an approved card still offering "Open page" for a deleted page, should go away once it's on top of #48.

The other thing is the confirmation text. Deletion is permanent, and any unsaved edits in the open editor go with it, but the prompt only mentions where subpages will move. Adding "This can't be undone." would make sure nobody is surprised.

@Pioneer113

Copy link
Copy Markdown
Author

Thanks for the review, @NathanTarbert. Both points are addressed in the latest push.

  • Rebased onto fix: bind reviewed-page retries to saved draft #48. I merged current main into the branch (no history rewrite, no conflicts on the last merge). The receipt endpoint now returns { deleted: true, pageId, spaceId, reviewDraft } for a deleted page, and PageReviewCard handles it inside the ReviewedPage flow: it shows "Saved, then deleted", offers no "Open page" link, and the fix: bind reviewed-page retries to saved draft #48 draft-binding check applies to deleted reviews too.
  • Second limitation is gone. Since every card now asks the server, an approved card no longer offers "Open page" for a deleted page, so I removed it from the description.
  • Confirmation text. It now reads: Delete "…"? This can't be undone. Any subpages will move to this page's parent.
  • Checks. 204/204 tests pass on top of current main, under Node 26 and Node 24, plus typecheck, lint, format check and build.
  • Live check. On a local dev server with an Intelligence project and a model (one review_space_page call): review card → Approve & save → Delete page → reload the chat. The card shows "Saved, then deleted", with no "Open page" link and no restore error.

@NathanTarbert

Copy link
Copy Markdown
Contributor

Thanks @Pioneer113, this looks great. "Saved, then deleted" with no "Open page" is a nice way to show it, and good to see #48's draft check covering deleted reviews too. The live check from review card to delete to reload is reassuring.

Looks good to me.

@jerelvelarde jerelvelarde left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Page deletion is useful and the server transaction, Space scope, reparenting, and retained draft-bound receipts look sound. Hold merge for a client race that can lose another page's draft. Both findings were reproduced with React renderer tests. Validation: 204 tests, typecheck, lint, formatting, and the rerun production build passed.

Comment thread src/client/PageDocument.tsx Outdated
// Drop pending autosave so navigation is not blocked by the
// unsaved-draft prompt for a page that no longer exists.
controller.dispose();
onDirty(false);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Ignore stale deletion completion after leaving this document. Delay DELETE for page A, navigate to page B, edit B, then let A's deletion finish. The old onDirty(false) clears B's navigation protection, and onHome() unmounts B, silently discarding its unsaved draft without another confirmation. Guard navigation and dirty cleanup by the active document lifecycle; retain the tombstone update for A.

return;
}
if (isDeletedReview(page)) {
setDeletedReview(page);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Clear savedPage when a refreshed receipt reports deletion. If this card already has a saved receipt and Continue conversation returns deleted:true, setDeletedReview leaves savedPage populated. The card then says Saved, then deleted while still offering Open page with a dead URL. Clear the saved-page state when accepting the tombstone.

@Pioneer113

Copy link
Copy Markdown
Author

Thanks for the careful review, @jerelvelarde. Both findings are fixed in the next push.

  • [P1] Stale deletion. PageDocument now tracks whether it is still the document on screen. When the DELETE finishes after the user moved to another page, it still applies the tombstone and refreshes the list, but it no longer calls onDirty(false) or onHome(), so the other page's unsaved-draft protection and its editor are left alone. The new tests/page-document-delete.test.tsx covers both the normal case and a DELETE that resolves after unmount; the second one fails on the previous code (onHome was called).
  • [P2] Saved receipt vs. deleted receipt. When "Continue conversation" gets deleted: true, the card now clears savedPage before accepting the tombstone, so "Open page" no longer shows next to "Saved, then deleted". The new tests/page-review-card.test.tsx renders the card with a saved receipt, clicks Continue with a deleted response, and fails on the previous code.
  • Checks. 243/243 tests pass (Node 26 and Node 24), plus typecheck, lint, format check and build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants