Skip to content

fix(web): removing a queued message's attachment counts as an edit when its run starts - #29601

Open
saphid wants to merge 2 commits into
pingdotgg:mainfrom
saphid:fix/web-queued-edit-removed-attachment
Open

saphid wants to merge 2 commits into
pingdotgg:mainfrom
saphid:fix/web-queued-edit-removed-attachment

Conversation

@saphid

@saphid saphid commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Problem

In the web composer, you can edit a queued message and remove one of its attachments. That removal didn't count as a change. If the run then left the queue before you saved (another client started or cancelled it, or the turn ended), the edit closed with no notice. The message ran with the attachment you had removed, and nothing told you your edit was gone.

Change

The edit now records that an existing attachment was removed, and recoverQueuedMessageEdit treats that as a change. It also keeps an edit only when there is something to put back in the composer. Otherwise, an attachment-only message with one attachment removed would say "kept in the composer" while leaving the composer empty. In that case the edit is reported as discarded.

This copies the mobile sibling, exitQueuedRunEdit in apps/mobile/src/state/queued-run-edit.ts. Since #25468, mobile tracks removedExistingAttachment ("Removing a server attachment changes the message even when the draft looks untouched") and keeps an edit only when it has restorable content.

Scope and approval

Very small, focused fix for an obvious bug, so no prior issue. Mobile already treats removing a queued attachment as an edit. Web dropped that edit with no notice. Web only, 3 files.

Overlap search: the local PR index and GitHub (queued edit removed attachment) found nothing. #29570 releases staged uploads when a queued edit is discarded. It touches the same function's discard branch but not how a change is detected. #28086 and #18631 hold a queued run while it is being edited. Neither changes how web detects a change, and an exit can still happen without a hold (older servers, a cancel from another client).

Verification

  • Red on main: I added a case to queuedMessageEdit.test.ts ("reports a removed queued attachment as an edit when the run leaves"). It fails with AssertionError: expected 'clean' to be 'kept'.
  • Green: vp test run src/components/chat/queuedMessageEdit.test.ts passes, 6 tests. I reran it after rebasing on the latest main.
  • vp run check:changed passes: format, lint, knip, typecheck (2 packages), tests (2 files), and the React compiler check.
  • Verified during takeover: Chrome with two real clients against an isolated dev server and the free replay provider. Removed a stored file attachment while editing its queued message; releasing the active run from the second client started the queued run and recovered the unsaved text with the expected notification.

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

Takeover verification

Merged current main without conflicts in dc055f44dbae3d529764c4c27edc9dcf2a21b3ab. No implementation changes beyond the original three-file fix. All 31 CI checks are green. vp run check:changed github/main passed through blacksmith-check, including lint, typecheck and 13 focused tests. Two independent GPT-6.1 Sol reviewers approved this head.

The separate timeline-heading test failed identically on this head and main (expected 577 to be close to 473); left unchanged. The PDF preview test passed on rerun. Native desktop and mobile were not exercised; this change is web-only.

recording of queued attachment edit recovery

recovered composer text and kept-edit notification

Takeover verification by gpt-6.1-sol in the Codex harness through T3 Code.

…en its run starts

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:S 10-29 changed lines (additions + deletions). labels Nov 2, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at fa86510

Macroscope's review found this PR approvable — Attachment removal now counts as an unsaved queued-message edit when its run starts, and cleared edits are reported as discarded. The change is limited to local draft recovery and notifications, while preserving existing thread drafts.

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 →

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 60aec28 and fa86510.

Files selected for processing: 3
  • apps/web/src/components/chat/queuedMessageEdit.test.ts
  • apps/web/src/components/chat/queuedMessageEdit.ts
  • apps/web/src/components/chat/useQueuedRunEdit.ts

Limit details: You’ve used all 10 included reviews currently available.

Walkthrough

Queued edit recovery now tracks when an existing attachment is removed. Recovery uses that state and the edit draft’s content to decide whether to retain the prompt or leave the composer draft empty.

Changes: Queued Edit Recovery

Layer / File(s) Summary
Track attachment removal during recovery
apps/web/src/components/chat/useQueuedRunEdit.ts, apps/web/src/components/chat/queuedMessageEdit.ts, apps/web/src/components/chat/queuedMessageEdit.test.ts
The queued-run edit state records attachment removal and passes it to recovery. Recovery considers the removal when deciding whether an edit changed and whether the thread draft is empty. A test covers nonempty original text and an attachment-only edit.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related issues

  • pingdotgg/t3code#15611 — Both concern queued-edit behavior when a run starts. This change handles recovery when a run leaves the queue; it does not add the dispatch guard described in the issue.

Suggested reviewers: juliusmarminge

Merge Risk: Low · up to fa865

Recovered drafts can omit retained attachments. This is a bounded recovery risk; users should check attachments before resending.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main web fix: treating removal of a queued message attachment as an edit when the run starts.
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.
Description check Passed The description is complete and follows the required template. It explains the problem, change, scope and approval rationale, focused verification results, UI evidence, known limitations, and the agen…
Full details: Description check

Explanation

The description is complete and follows the required template. It explains the problem, change, scope and approval rationale, focused verification results, UI evidence, known limitations, and the agent and harness used.

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

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.

@coderabbitai coderabbitai Bot 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.

  • Fix comments: Fix CodeRabbit comments on this PR

@maria-rcks

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

Merged main without conflicts and preserved the three-file fix. All 31 checks are green on dc055f44db. Blacksmith check:changed passed, including 13 focused tests. Two independent reviewers found no issues. Verified attachment removal and recovery through two real browser clients; evidence is in the PR description. The withdrawn attachment finding needs no change. The separate timeline-heading failure reproduces identically on main, so it remains outside this PR.

@maria-rcks

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 dc055f44dbae3d529764c4c27edc9dcf2a21b3ab on this Linux VM. Installed dependencies with vp i, ran one background dev server at a time, and drove two Chrome clients using the repository's Playwright dependency and /usr/bin/google-chrome. Created the project, thread, queued messages and attachments through the app. Provider turns used real Codex gpt-6.1-sol through the local proxy, without replay or mocks.

  • Reproduced on github/main (61c3794c32) in a separate worktree: remove an existing file attachment while editing a queued message, then click Send now in the second client. The editor closes with an empty composer and no notification.
  • Repeated on this PR: the unsaved text returns to the composer and the "kept in the composer" notification appears. The dispatched original message retains its attachment.
  • Removing the only image from an attachment-only queued message, then cancelling that message from the other client, shows "was discarded" and leaves the composer empty.
  • Saving an attachment removal persists it. Reopening the queued editor shows no stored attachment. Cancelling an unsaved removal preserves the original attachment when reopened.
  • A subsequent real provider turn read the uploaded file and returned its exact contents, attachment-verification-29601.

Read back the running server's bounded thread projection using its protocol header:

GET /api/orchestration/threads/<thread>/bounded
x-t3-orchestration-protocol: 2
200
provider/model: codex / gpt-6.1-sol
dispatched original message: attachments = [queued-note.txt]
saved removal message: attachments = []
provider response: SAVED_WITHOUT_ATTACHMENT
later attachment read: attachment-verification-29601

vp run check:changed github/main passed through blacksmith-check: format, lint, knip, two-package typecheck, all 13 focused tests, and React compiler checks. The worker initially lacked Chromium; installed it and reran successfully. All 31 PR CI checks are green. No source changes or pushes were needed.

Main: the edit closes silently and the composer is empty.

main loses the unsaved queued attachment removal

PR: the text is recovered and the notification explains what happened.

pr recovers the unsaved text and shows the kept-edit notification

Recording: remove the attachment, dispatch from the second client, and observe recovery in the first client.

real codex queued attachment removal and recovery recording

empty attachment-only edit reports discarded instead of kept

Not verified

Native Electron, iOS and Android clients, macOS/Windows, remote relay/tunnel connections, and providers other than Codex were not exercised on this Linux VM. The recorded exit uses the second client's Send now action; automatic queue advancement with the unsaved edit still open was not separately verified. This PR changes web recovery logic.

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:S 10-29 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