Skip to content

fix(web): deleting an archived thread also clears its draft, staged uploads and terminal state - #29164

Open
saphid wants to merge 1 commit into
pingdotgg:mainfrom
saphid:fix/archived-thread-delete-cleanup
Open

saphid wants to merge 1 commit into
pingdotgg:mainfrom
saphid:fix/archived-thread-delete-cleanup

Conversation

@saphid

@saphid saphid commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Problem

If you archive a thread that has unsent composer text, a staged attachment, or an opened terminal drawer, then delete it from Settings → Archived threads, that state is never cleaned up. The composer draft and terminal UI state stay in local storage for a thread that no longer exists, and the staged uploads are never released, so their pending uploads also stay on the server.

Archived threads are not in the live thread shell, so deleteThread takes its early if (!resolved) branch. That branch only expires undos, refreshes the archive list, and clears panel state. The live-thread branch also releases draft uploads, clears the draft, and clears terminal UI state.

Change

The archived branch now calls releaseComposerDraftUploads, clearComposerDraftForThread and clearTerminalUiState with the thread ref after a successful delete. These are the same calls, in the same order, as the live-thread branch (uploads are released before the draft is cleared, because the release reads the draft's attachments). It follows #28386, which added clearDeletedThreadPanelState to both branches.

clearProjectDraftThreadById is left out. It needs the thread's project id, which is not available without a shell, and it only maps project draft threads, which are never archived server threads.

Scope and approval

Very small, focused fix for an obvious bug, so no prior issue: the two branches of one function disagree about cleanup after the same delete, and the fix copies three existing calls into the branch that was missing them (3 source lines, one added assertion in the existing test case).

Overlap search: #28386 (merged) made the same kind of fix for panel state but did not touch drafts, uploads or terminal state. #22160 (open) only changes the archive page's delete confirmation. #25444 (merged, undo expiry) and #25832 (open, server device release) are different concerns. GitHub PR searches for this branch's cleanup found nothing else.

Verification

  • Extended the existing case "deletes an archived thread only with its own environment's grant" in apps/web/src/hooks/useThreadActions.permissions.test.ts to assert the local cleanup effects.
  • On main it fails: AssertionError: expected [ 'refresh-archive' ] to deeply equal [ 'refresh-archive', …(3) ].
  • With the fix: vp test run src/hooks/useThreadActions.permissions.test.ts → 36 passed (36).
  • vp run check:changed → format, lint, knip, typecheck (2 packages), tests (3 files) and React compiler all pass.
  • Not checked: a manual run in a real client. The change has no visible surface. The leftover state lives in local storage and in the server's pending uploads.

Reviewed with sol-loop: 1 round with GPT-6.1 Sol on high. Sol verdict: approved.

Created with Claude Opus 5.5 in Claude Code (T3 Code).

🤖 Generated with Claude Code

…ploads and terminal state

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Nov 2, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at d975ea1

Macroscope's review found this PR approvable — Deleting an archived thread now also clears its draft, staged uploads, and terminal UI state. Cleanup runs only after successful deletion and matches the existing cleanup for loaded threads.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between d01b92c and d975ea1.

Files selected for processing: 2
  • apps/web/src/hooks/useThreadActions.permissions.test.ts
  • apps/web/src/hooks/useThreadActions.ts

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

Walkthrough

When deletion succeeds for a thread whose shell cannot be resolved, deleteThread now releases composer draft uploads and clears the composer draft and terminal UI state. The archived-thread deletion test expects the terminal UI cleanup effect.

Changes: Archived thread deletion

Layer / File(s) Summary
Deletion cleanup and test
apps/web/src/hooks/useThreadActions.ts, apps/web/src/hooks/useThreadActions.permissions.test.ts
When deletion succeeds without a resolvable thread shell, the hook releases composer draft uploads and clears the composer draft and terminal UI state. The test expects the terminal UI cleanup effect.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix

Suggested reviewers: only21mil, t3dotgg

Merge Risk: Minimal · up to d975e

The archived-thread cleanup ordering is correct and the change is ready to merge.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title names the change: deleting an archived thread now also clears its composer draft, staged uploads, and terminal state. The raw summary and objectives confirm the change adds these three clean…
Description check Passed The description follows the template sections: Problem, Change, Scope and approval, and Verification. The Problem section gives the expected behavior and the cause, with the early if (!resolved) branc…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the change: deleting an archived thread now also clears its composer draft, staged uploads, and terminal state. The raw summary and objectives confirm the change adds these three cleanup calls to the archived-thread deletion branch of deleteThread. The title is specific and follows the conventional commit format. It is slightly long at 94 characters, but this does not affect clarity.

Full details: Description check

Explanation

The description follows the template sections: Problem, Change, Scope and approval, and Verification. The Problem section gives the expected behavior and the cause, with the early if (!resolved) branch named. The Change section explains the three calls, their order, and why clearProjectDraftThreadById is omitted. The Scope section argues why the fix is small and gives an overlap search. The Verification section lists the focused test, the failing result on main, the passing result with the fix, and the checks that were not run. The description also states the manual run was not performed. The template asks for a conventional commit title, and the PR title meets that. The description does not include screenshots. This is correct because the change has no visible UI surface, and the author explains this. The description also names the model and harness, as the template asks.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

@maria-rcks

maria-rcks commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified unchanged head d975ea11e7e879f29b8057b5d1f8aa08fc13f28a on linux with the real dev web client, repository playwright, /usr/bin/google-chrome, and a successful codex gpt-6.1-sol turn. projects, threads, drafts, attachments, and terminal state were created through the app. only one dev server ran at a time, using isolated data.

reproduced on github/main at 6f41364805: after archiving a thread with unsent text, staged README.md, and an open terminal, deleting it through settings > archive removed the conversation but left the draft, terminal state, and pending attachment file behind.

on this pr head, repeated the flow, reloaded the browser with the archived draft persisted, then deleted through settings > archive. the thread-specific composer and terminal entries disappeared. the client sent attachments.delete, the server returned success, and the pending file disappeared from disk.

main: after archive/delete: draft=true terminal=true pendingFileOnServer=true attachments.delete=false
pr: after archive/delete: draft=false terminal=false pendingFileOnServer=false attachments.delete=true
pr: attachments.delete RPC returned Success

nearby behavior passed: unarchiving restored the draft, attachment, and open terminal; canceling archive deletion preserved them; deleting a live thread cleared its draft, file/image uploads, and terminal state. the existing permission suite passed on blacksmith: 36 tests. ci reports 31 successful checks. no source changes were needed.

read-only storage, websocket, and filesystem verification transcript

real codex response, unsent draft, staged file, and open terminal before archiving

archive is empty after confirmed deletion

archive and confirmed deletion in the real app after reload

Not verified

native ios/android clients, the electron shell, macos/windows, and remote/relay or multiple-environment browser sessions were not exercised on this linux vm. permission isolation was covered by the existing tests rather than a second running environment. server deletion failures were not injected.

This branch has not been deployed

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

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants