Skip to content

feat(chat): add mass deletion of conversations - #680

Merged
julien-nc merged 3 commits into
nextcloud:mainfrom
WSHAPER:feat/mass-delete-conversations
Oct 6, 2026
Merged

julien-nc merged 3 commits into
nextcloud:mainfrom
WSHAPER:feat/mass-delete-conversations

Conversation

@WSHAPER

@WSHAPER WSHAPER commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Fixes #639

Summary

Adds mass deletion of chat conversations:

  • New bulk delete endpoint taking a list of session IDs, available both as DELETE /ocs/v2.php/apps/assistant/chat/sessions (RESTful) and DELETE /ocs/v2.php/apps/assistant/chat/delete_sessions (legacy). It deletes the sessions and their messages in a single transaction, restricted to sessions owned by the current user. Non-existent or foreign session IDs are ignored, like the single deletion endpoint ignores unknown IDs. openapi.json regenerated.
  • A deletion mode in the conversation list: a "Delete multiple conversations" button above the list enters the mode, rows toggle selection on click (active highlight as state indicator), a Select all/Deselect all toggle and a full-width error-variant delete button act on the selection, and a confirmation dialog (error variant for the destructive action, Escape closes the dialog first and exits the mode on the second press) guards the deletion. The single delete per row stays as an inline action and now reuses the bulk endpoint and dialog. Not available in the scheduled tasks (assignments) view.
  • Per the review feedback, the per-item "delete multiple" context action was dropped (the global button covers it) and the single delete is always inline, so no NcAppNavigationItem action menu is rendered at all. That menu cannot open when the assistant is displayed on top of the viewer, because NcAppNavigationItem hardcodes its popover container to #app-navigation-vue and floating-vue then resolves to the Files instance underneath the viewer. Tracked for upstream in nextcloud-vue (no prop exists as of 9.9.0 / latest / master).

Implementation notes

  • The confirmation dialog is bound with v-model:open on a writable computed and remounted via a key on every open: closing it with Escape leaves a 300ms delayed internal teardown in NcDialog which, with a one-way open binding, left the open prop stuck true when the dialog was quickly reopened, making the delete button unresponsive until a page reload.
  • Unit/integration tests: ChatServiceTest covers the service-level edge cases (happy path, ownership/unknown/duplicate IDs, empty list, null user). Full suite passes (45 tests).

Manual test plan

  1. Chat with a few conversations → "Delete multiple conversations" → select rows (click), Select all/Deselect all, "Delete N conversation(s)" with confirm dialog; Cancel keeps the selection, Escape closes the dialog first and exits the mode when pressed again.
  2. Single delete via the inline row action still works.
  3. API: DELETE .../chat/delete_sessions?sessionIds[]=1&sessionIds[]=2 → 200, sessions and messages gone, other users' data untouched; empty list → 400.

Detailed testing notes are in the issue.

AI disclosure

This change was developed with AI assistance (opencode / glm-5.3); the code was reviewed, tested manually and adjusted by the contributor.

@WSHAPER
WSHAPER requested a review from julien-nc October 5, 2026 13:08
Add a bulk delete endpoint that takes a list of session IDs and deletes
the sessions and their messages in a single transaction, restricted to
sessions owned by the current user. Non-existent or foreign session IDs
are ignored, like the single deletion endpoint ignores unknown IDs.

In the conversation list, add a deletion mode with a "Delete multiple
conversations" entry button matching the "New conversation" button
style, per-conversation selection, a select-all/deselect-all toggle and
a confirmation dialog. Single deletion reuses the bulk endpoint and
dialog. Destructive buttons use the error variant like the confirmation
dialogs of the server apps, and the deletion mode is left with Escape.

The dialog is bound with v-model on a writable computed and remounted
via a key on every open: closing it with Escape leaves a 300ms delayed
internal teardown in NcDialog which, with a one-way open binding, kept
the open prop stuck true when the dialog was quickly reopened, making
the delete button unresponsive until a page reload.

Fixes nextcloud#639

Assisted-by: opencode:glm-5.3
Signed-off-by: WSHAPER <42714629+WSHAPER@users.noreply.github.com>
…inline

Address review feedback: having one action inline and one in the action
menu looked inconsistent, and NcAppNavigationItem's action menu does not
open when the assistant is used inside the viewer because its popover
container is hardcoded to "#app-navigation-vue", of which a second
instance exists beneath the viewer. The global "Delete multiple
conversations" button at the top of the list is enough to enter the
deletion mode; the single-delete action is now the only per-item action
and is always inline, so no action menu is rendered at all.

Assisted-by: opencode:glm-5.3
Signed-off-by: WSHAPER <42714629+WSHAPER@users.noreply.github.com>
@WSHAPER
WSHAPER force-pushed the feat/mass-delete-conversations branch from 69b474a to 69c41e8 Compare October 5, 2026 13:11
@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 4 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b199dd6e-de66-4b22-a44f-d22d673e0342
📥 Commits

Reviewing files that changed from the base of the PR and between 69c41e8 and 2df8538.

📒 Files selected for processing (4)
  • lib/Db/ChattyLLM/MessageMapper.php
  • lib/Db/ChattyLLM/SessionMapper.php
  • src/components/ChattyLLM/ChattyLLMInputForm.vue
  • tests/unit/Service/ChatServiceTest.php
📝 Walkthrough

Walkthrough

The change adds API endpoints for deleting multiple chat sessions. The service removes matching sessions owned by the current user and their messages in a transaction. The chat interface adds a deletion mode with individual or select-all selection, confirmation, and local state updates after successful deletion. Service tests cover deletion, ownership, duplicate and unknown IDs, empty input, and a missing user ID.

Priority: ➖ Normal

Merge Risk: 🟡 Moderate · up to 69c41

Large conversation selections may fail to delete, and running the new tests against a shared database could remove an existing account. Address those risks before merging; the dialog also needs a small rendering safeguard.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 69c41

The bulk API preserves account ownership and deletes sessions and messages transactionally. The identified new risk is limited to database-backed tests: cleanup can delete pre-existing accounts when the tests run against a shared installation.

Retained concerns

  • Medium · security · inferred: The new database-backed fixture crosses its cleanup ownership boundary: setup reuses either fixed-name account when it already exists, but teardown deletes that account and all its chat data. Running the suite against a persistent or shared installation can therefore destroy identities the fixture did not create. The fresh-install CI workflow limits this exposure but does not make cleanup ownership explicit.
Security review details

Security Blast Radius

  • inferred — The new API can delete multiple conversations in one request, but its application-level deletion scope remains the authenticated user’s matching sessions and messages. The fixture concern has a separate privileged execution scope: the connected test installation and the two fixed-name accounts, not remote access through the chat endpoint.

Security Findings and Attack Paths

  • inferred — If either chat_service_test_user or chat_service_other_user already exists, fixture setup adopts it and teardown deletes its chat data and account. Triggering this outcome requires executing the database-backed suite against that installation. The normal CI workflow creates a fresh installation, so a pre-existing-account collision there was not established.

Trust Boundaries and Controls

  • observed — Both aliases delegate the controller’s user identity to the same service, which rejects null identity. The destructive handler has NoAdminRequired but no NoCSRFRequired exemption. Ownership filtering precedes the message sink, and the mixed-ID test verifies preservation of another user’s session and messages. Runtime CSRF middleware was not inspected.

Resilience and Maintainability Implications

  • inferred — Transactional deletion does not establish permanent quiescence of background writers. Tasks are deliberately retained, and the unchanged completion listener inserts a message before checking session existence. The repository-defined message table has no foreign key to the session table, so later completion can leave orphaned content. This lifecycle gap also existed with individual deletion and is not retained as an introduced PR concern.

Hardening Proposals

  • proposed — Give database-backed fixtures explicit creation ownership: use isolated identities, fail rather than adopt an existing account, and restrict teardown to resources actually created by the fixture.
  • proposed — As separate lifecycle hardening, coordinate deletion with task completion so a deleted session cannot receive new persisted content, and route legacy individual deletion through the owner-filtered cleanup flow.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 6 files. (2 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding bulk deletion of chat conversations.
Description check ✅ Passed The description explains the bulk deletion endpoints, ownership rules, user interface changes, and testing. It is directly related to the changeset.
Linked Issues check ✅ Passed Issue #639 asks for a bulk-delete endpoint, a control to enter deletion mode, selectable conversations, and optionally select-all/none. The PR adds RESTful and legacy endpoints, restricts deletion to …
Out of Scope Changes check ✅ Passed The changed endpoints, service and mapper methods, OpenAPI definitions, UI controls, and tests support issue #639's bulk conversation deletion. The reported omission of context-menu actions does not e…
Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 6 files. (2 skipped: 2 unsupported.)

  • Autopilot · 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

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a6db6ef8-f622-490c-bae0-0ba70ab36c70
📥 Commits

Reviewing files that changed from the base of the PR and between 5e8d1ab and 69c41e8.

📒 Files selected for processing (8)
  • appinfo/routes.php
  • lib/Controller/ChattyLLMController.php
  • lib/Db/ChattyLLM/MessageMapper.php
  • lib/Db/ChattyLLM/SessionMapper.php
  • lib/Service/ChatService.php
  • openapi.json
  • src/components/ChattyLLM/ChattyLLMInputForm.vue
  • tests/unit/Service/ChatServiceTest.php

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Comment thread lib/Service/ChatService.php
Comment thread src/components/ChattyLLM/ChattyLLMInputForm.vue Outdated
Comment thread tests/unit/Service/ChatServiceTest.php

@julien-nc julien-nc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice! Works just fine.

  • Let's put the button at the bottom of the conversation list for now.

Comment thread tests/unit/Service/ChatServiceTest.php
Comment thread lib/Db/ChattyLLM/SessionMapper.php Outdated
Comment thread lib/Db/ChattyLLM/SessionMapper.php
Comment thread src/components/ChattyLLM/ChattyLLMInputForm.vue Outdated
Comment thread src/components/ChattyLLM/ChattyLLMInputForm.vue Outdated
Chunk the IN queries of the bulk session operations with
IQueryBuilder::MAX_IN_PARAMETERS so large ID lists stay within the
database bind limits, guard the confirmation message and title getter
against missing sessions, and make the ChatServiceTest only remove the
users and sessions it created so tests don't alter instance state in CI.

Assisted-by: opencode:glm-5.3
Signed-off-by: WSHAPER <42714629+WSHAPER@users.noreply.github.com>

@julien-nc julien-nc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

@julien-nc
julien-nc merged commit 9e947e0 into nextcloud:main Oct 6, 2026
16 checks passed
@WSHAPER
WSHAPER deleted the feat/mass-delete-conversations branch October 6, 2026 12:00
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.

Mass deletion of chat conversations

2 participants