Skip to content

fix(sessions): confirm permanent deletion in a modal - #135

Merged
miyaontherelay merged 1 commit into
mainfrom
fix/session-delete-confirm-modal
Nov 11, 2026
Merged

miyaontherelay merged 1 commit into
mainfrom
fix/session-delete-confirm-modal

Conversation

@miyaontherelay

@miyaontherelay miyaontherelay commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

Clicking Delete session now opens a modal confirmation instead of expanding the warning beside Details/Resume. The warning, button copy, role gating, and DELETE API contract are unchanged.

The dialog follows the existing invite modal's portal, Card styling, background isolation and focus behavior (this webapp has no shared Dialog primitive). Cancel is focused initially; Tab stays inside; Escape and Cancel close it and restore focus. Pending, server-in-progress and error messages stay inside the modal. A synchronous in-flight guard prevents duplicate DELETE calls across rapid clicks or closing/reopening while the request is pending.

Validation: npm run check passes (lint, 58 test files / 910 tests, TypeScript and production build). The 10 session-delete tests cover modal open, Cancel/Escape and restored focus, focus trapping and inert background, one DELETE across repeated clicks/reopening, pending/error placement, member gating and unchanged request encoding. git diff --check passes.

Owner review: adversarial pass checked duplicate submits, pending dismissal, unmount/navigation, focus escape and background restoration. Contextual pass checked the existing invite dialog pattern, unchanged copy/API/authorization and exact two-file scope. Independent review by selfhost-usage-page passed on exact head b926ab64 (review). PR CI check, linux/amd64 container smoke and Greptile review all pass; no unresolved threads. Based on main 7259485 (includes the downloads move); no downloads files change.

For the 2026.11.1 release. This PR does not publish an image, deploy, apply migrations or activate erasure flags.

Adversarial review: checked pending close/reopen and duplicate submission, unmount navigation, keyboard and programmatic focus escape, initial safe focus and background restoration. Synchronous in-flight guard prevents parallel DELETE calls.

Contextual review: follows existing invite modal portal/Card/focus pattern; preserves warning, button copy, role gating and API. Two session-delete files only, based on main7259485 including downloads move.

Validation: npm run check passes lint, 58 files/910 tests, typecheck and production build. Focused DeleteSession suite10/10; diff --check passes. Independent final-head review requested before merge.
@coderabbitai

coderabbitai Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Pre-merge checks: Inconclusive 2

Failed Failed checks: 2 inconclusive

Check name Status Explanation Resolution
Title check Pending This assessment has not completed. The check will be retried on the next pre-merge check run.
Description check Pending This assessment has not completed. The check will be retried on the next pre-merge check run.
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

@greptile-apps

greptile-apps Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge.

Summary

Moves permanent session deletion into a modal confirmation.

  • Session deletion now asks for confirmation in a modal above the page.
  • A session deletion sends only one request while an earlier request is pending.

No actionable issues were found.

Reviews (1) · Last reviewed commit: "fix(sessions): confirm permanent deletio..." · Reviewed by Greptile

@khaliqgant

Copy link
Copy Markdown
Member

Independent review at exact head b926ab64d7ed27e047f8c75f6cad698598765626: looks good to merge. (Posted by the selfhost-usage-page agent at hard-delete-owner's request; same GitHub account, so this is a comment, not an approval.)

Scope: only src/sessions/DeleteSession.tsx and its test. No overlap with the downloads (#133) or Usage changes.

Checked:

  • Hooks: all hooks run before the if (!organization) return null early return; erase and close are plain functions after it, so the hook order is stable.
  • Single DELETE: the inFlight ref and the started guard block a second request on a double click, or on close-and-reopen while pending. The destructive button is disabled while busy or started, and the test covers repeated clicks plus reopening.
  • Modal: it's portalled to body with role="alertdialog", aria-modal, and labelled-by/described-by. Background body children get aria-hidden + inert, and the cleanup restores their previous values. Cancel gets initial focus; Tab and Shift+Tab wrap inside the panel; focusin outside goes back to Cancel; Escape closes. Cleanup refocuses the trigger only if it's still connected.
  • Outcomes: completed closes and calls onDeleted only while mounted; failed keeps a retry; an in-progress erase is reported separately from erased (hidden versus 48h erase copy); SignedOut goes to onSignedOut.
  • Close handler: the onClose ref is updated per render while the effect depends only on opener, so listeners aren't re-bound on every render.

Minor, non-blocking: if Escape closes the dialog while a request is pending and the request then fails, close() has already cleared error. The later setError is only visible after reopening, which is acceptable.

CI: check, amd64 image smoke and Greptile all pass; 0 unresolved threads.

@miyaontherelay
miyaontherelay merged commit 67ee518 into main Nov 11, 2026
6 checks passed
@miyaontherelay
miyaontherelay deleted the fix/session-delete-confirm-modal branch November 11, 2026 22:41
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.

2 participants