Skip to content

Fix cross-window tab drag: undo-close resurrecting content-transferred windows (APP-5285) - #14938

Draft
warp-agent-staging[bot] wants to merge 3 commits into
masterfrom
factory/app-5285-cross-window-tab-drag-undo-close
Draft

Fix cross-window tab drag: undo-close resurrecting content-transferred windows (APP-5285)#14938
warp-agent-staging[bot] wants to merge 3 commits into
masterfrom
factory/app-5285-cross-window-tab-drag-undo-close

Conversation

@warp-agent-staging

@warp-agent-staging warp-agent-staging Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes APP-5285: dragging a tab across windows could leave the dragged tab's pane group with two owners, which showed up to the reporter as a tab disappearing during the drag, a duplicate tab after Cmd+Shift+T, and a "crash" that was actually a repeating terminal_panes.uuid UNIQUE-constraint error storm from save_app_state.

Root cause. The existing guards in CrossWindowTabDrag (pending_source_window_closes, source_placeholder_consumed, ClosePreviewOnly) all protect the transient async gap between a transfer-driven window close being requested and Workspace::on_window_closed actually running. They do not cover a separate, generic path: when a window closes with TerminationMode::ContentTransferred (its content was handed off, not destroyed), the closing Workspace's own tabs list is never cleared, so it still references the PaneGroup now owned by the target window. The top-level on_window_will_close app callback (app/src/lib.rs) has no notion of why a window closed, so it unconditionally pushed every closed window onto UndoCloseStack. Pressing Cmd+Shift+T shortly after a cross-window drag could pop that stale Workspace back onto the screen, permanently duplicating ownership of the same pane group across two windows (not just for a few frames) — hence the duplicate tab, the confirmation-dialog-on-close warning about killing processes, and a terminal_panes.uuid UNIQUE violation on every subsequent save_app (window move/resize/focus) until the app restarted.

Fix.

  • Four exact call sites issue close_window(_, TerminationMode::ContentTransferred): Workspace::close_window_for_content_transfer, the RemoveSourceTabAndClosePreview / ClosePreviewOnly arms of handle_drop_result, and CrossWindowTabDrag::finalize_handoff's put-back preview close. Each now calls CrossWindowTabDrag::mark_content_transferred_window_close at the exact point the close is issued.
  • The top-level on_window_will_close callback (app/src/lib.rs) consumes that marker via take_content_transferred_window_close and skips the UndoCloseStack registration when set, so a content-transferred close can never be resurrected via undo-close. (An earlier revision of this fix keyed the marker off Workspace::suppress_detach_panes_on_window_close instead — review caught that this flag is also set, and not reliably cleared, on windows that stay open after a handoff/reverse-handoff, which would have mis-marked a later ordinary close of such a window. Moving the marking to the actual close call sites fixed that.)
  • Defense in depth: report_db_error now classifies a terminal_panes.uuid UNIQUE violation specifically (not just any terminal_panes UNIQUE violation, which could mask a distinct corruption signal like a primary-key collision) and reports it with ReportErrorLogMode::OncePerRun and a message naming the known cause, instead of the generic "SQLite error" every time.
  • A new debug-time invariant check (assert_no_duplicate_pane_group_ownership in get_app_state) catches dual pane-group ownership at the moment of formation: debug_assert! panics loudly in dev/dogfood builds with a backtrace into the offending mutation; release builds report once per run instead, so a regression can't crash a user's session or storm Sentry the way the DB-level symptom did.

Updated specs/pei/cross-window-tab-drag/TECH.md with the new mitigation.

On the reporter's history question

The reporter followed up: general command/prompt history came back after restart, but the one AI conversation that was on the dragged tab is missing from it. I traced this and it does not come from a genuine deletion:

  • agent_conversations (the table backing conversation content) is a table independent of terminal_panes/windows/tabs, keyed by conversation_id, and is written via its own ModelEvents on transactions independent of save_app_state. Startup loads all rows from it unconditionally (read_agent_conversation_metadata), regardless of whether any tab currently references them — that's what backs the conversation list view.
  • Nothing on the paths this incident actually exercised deletes from that table. TerminalPane::detach's DetachType::Closed branch (the only path that clears anything AI-related on close) only clears in-memory tracking (clear_conversations_for_terminal_surface) and deletes terminal blocks (delete_blocks, a different table); it never calls delete_conversation, which is the one path that actually issues ModelEvent::DeleteAIConversation. The reporter also said he never touched (closed) either duplicate, so this path didn't run anyway.
  • What actually happened: save_app_state is a single full-snapshot rebuild (delete-then-reinsert of every session table in one transaction), and it started failing on the terminal_panes.uuid UNIQUE violation the moment the duplicate ownership existed. That snapshot is also what captures terminal_panes.conversation_ids/active_conversation_id — the linkage of which conversation belongs to which pane. Once saves started failing, that linkage froze at whatever was last captured, which predates the newest conversation on the dragged tab. On restart, the app rehydrated that stale linkage, so the terminal pane it restored doesn't point at the newest conversation anymore.
  • Net effect: the conversation is very likely orphaned, not deleted — its data should still be in the agent_conversations table and reachable through the global conversation list/history surface, just no longer auto-surfaced in that specific tab.
  • This does not recur from this root cause after the fix: save_app failures are now bounded to the brief async close gap instead of persisting indefinitely, so the linkage-freeze window that caused this is gone.

I was not able to independently confirm from inside this environment that the reporter's specific conversation is still present and reachable (that requires his account/database), so treat the above as a well-evidenced code-path analysis, not a confirmed recovery. If he still can't locate it via conversation history/search, that would point at something this analysis didn't account for and is worth a fast follow-up.

Linked Issue

  • APP-5285 is triaged and ready to implement (root cause + fix scope confirmed with the reporting engineer).

Testing

  • cargo check -p warp --lib, cargo clippy -p warp --all-targets --tests -- -D warnings, ./script/format — all clean.

  • Unit tests (cargo nextest run -p warp persistence::sqlite cross_window_tab_drag undo_close app_state): 36 passed, including:

    • workspace::cross_window_tab_drag::tests::content_transferred_close_is_marked_and_consumed_once / ..._is_tracked_independently_per_window — the marking API.
    • persistence::sqlite::tests::test_sqlite_save_app_state_rolls_back_on_duplicate_terminal_uuid — reproduces the terminal_panes.uuid UNIQUE violation directly and confirms save_app_state rolls back atomically.
    • persistence::sqlite::tests::test_terminal_panes_unique_violation_classifier_matches_uuid_column_only — negative test proving a UNIQUE violation on a different terminal_panes column (the id primary key) is not folded into the same throttle.
  • GUI integration test test_undo_close_does_not_resurrect_content_transferred_window (crates/integration/src/test/workspace.rs): drags a tab into an existing window (closing the temporary preview via ContentTransferred), triggers undo-close, and asserts the window/tab counts are unchanged. Re-ran the four existing cross-window-drag integration tests plus test_restore_single_closed_pane — all pass.

  • Regression-evidence gap, still open: I could not get this class of regression to fail visibly in the GUI integration harness. I confirmed why, concretely: I temporarily disabled the fix and added instrumentation to workspace:save_app, then ran the full integration test suite — save_app/get_app_state (and therefore the new invariant assert, and the real terminal_panes.uuid SQLite path) were invoked zero times during the entire test lifecycle, independent of whether the fix was present. This is a structural property of the GUI integration harness (persistence/global-action dispatch isn't exercised by it), not a failure to drive the right sequence of drag/undo-close actions. The only place I can demonstrate the actual terminal_panes.uuid violation and its throttling is the SQLite unit test above, which reproduces it directly by constructing the duplicate rows rather than driving the full UI flow.

  • Visual verification, inconclusive. I attempted a computer_use session against a real, from-source cargo run --bin warp build in this sandbox (a display and WARP_API_KEY are both available here). Two blockers prevented a clean repro: (1) this environment's identity is a service account, which staging's auth rejects with "Expected a user account," so the app stays on the onboarding screen rather than reaching a normal authenticated session (I proceeded through onboarding to reach a local, non-cloud-authenticated window instead); (2) synthetic mouse input could not complete a real native cross-window drag-and-drop — every attempt reverted the tab back to the source window instead of dropping it on the target. Because the drag itself never completed, the captured video/screenshots do not demonstrate the actual bug or fix — they show an unrelated side effect of a reverted drag (Ctrl+Shift+T added an extra "New agent conversation" tab to the source window after the failed drag, with no dialog and no new window). I'm including this recording/screenshots for transparency about the attempt, but the automated GUI integration test above (which drives the drag through the test framework's direct APIs rather than synthetic OS-level mouse events) is the actual verification of this fix's behavior.

  • I have manually tested my changes locally with ./script/run (blocked by the same service-account auth limitation above; see the automated integration test coverage instead).

Agent Mode

  • Warp Agent Mode - This PR was created via Warp's AI Agent Mode

Computer-use video recordings

In Warp, open a second tab and a second window, drag a tab from the first window onto the second window's tab bar, then press Cmd+Shift+T (undo-close) and observe the result (APP-5285).
Warp tab drag between windows + undo-close (inconclusive — see Testing section): In Warp, open a second tab and a second window, drag a tab from the first window onto the second window's tab bar, then press Cmd+Shift+T (undo-close) and observe the result (APP-5285).

Computer-use screenshots (3) — see Testing section for why this attempt is inconclusive

Warp launched from source but is stuck on the "Welcome to Warp" onboarding/login screen instead of an authenticated terminal, because the environment's identity resolves to a service account, which staging rejects with "Expected a user account".
Warp launched from source but is stuck on the "Welcome to Warp" onboarding/login screen instead of an authenticated terminal, because the environment's identity resolves to a service account, which staging rejects with "Expected a user account".

State immediately after the cross-window tab-drag attempt: the dragged tab reverted to the source (left) window, which still has 2 tabs, while the target (right) window still has only 1 tab — the tab did not transfer.
State immediately after the cross-window tab-drag attempt: the dragged tab reverted to the source (left) window, which still has 2 tabs, while the target (right) window still has only 1 tab — the tab did not transfer.

State immediately after pressing Ctrl+Shift+T (undo-close): the focused left window gained a new "New agent conversation" tab (now 3 tabs: ~, ~, New agent conversation), duplicating the type/content of the right window's existing "New agent conversation" tab; no new window and no confirmation dialog appeared.
State immediately after pressing Ctrl+Shift+T (undo-close): the focused left window gained a new "New agent conversation" tab (now 3 tabs: ~, ~, New agent conversation), duplicating the type/content of the right window's existing "New agent conversation" tab; no new window and no confirmation dialog appeared.

Conversation: https://staging.warp.dev/conversation/7c07f701-2d29-404e-bcb7-c07dae0307c0
Run: https://oz.staging.warp.dev/runs/019ff0ef-ab9c-75a6-b8fc-07b43d8b831a

This PR was generated with Oz.

…d windows (APP-5285)

When a cross-window tab drag closes a window because its content was
handed off elsewhere (TerminationMode::ContentTransferred), the closing
Workspace's own `tabs` list still references the PaneGroup that was just
adopted by the target window. The generic on_window_will_close app
callback doesn't know a close was transfer-driven, so it unconditionally
pushed the stale Workspace onto UndoCloseStack. Cmd+Shift+T could then
resurrect that window, giving two windows permanent ownership of the same
pane group -- producing a duplicate tab, a sustained
`terminal_panes.uuid` UNIQUE-constraint storm on every subsequent
save_app, and eventual data loss if the user closed either copy.

Fix:
- Workspace::on_window_closed marks the close via
  CrossWindowTabDrag::mark_content_transferred_window_close whenever
  suppress_detach_panes_on_window_close is true.
- The top-level on_window_will_close callback in lib.rs consumes that
  marker and skips the UndoCloseStack registration for content-transferred
  closes.
- Defense in depth: report_db_error now classifies a terminal_panes.uuid
  UNIQUE violation and reports it with ReportErrorLogMode::OncePerRun, so
  a residual/future instance of this class of bug produces one Sentry
  event instead of a storm.

Adds unit tests for the new CrossWindowTabDrag marking API and a
save_app_state rollback/classification test, plus a GUI integration test
(test_undo_close_does_not_resurrect_content_transferred_window) that
drives a real cross-window attach and confirms undo-close is a no-op
afterward. Updates specs/pei/cross-window-tab-drag/TECH.md to document
the mitigation.

Co-Authored-By: Warp Agent <agent@warp.dev>
@cla-bot cla-bot Bot added the cla-signed label Aug 11, 2026

@warp-agent-staging warp-agent-staging Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Overview

Fixes APP-5285: a cross-window tab drag could leave two windows owning the same pane group, producing a vanished tab, a duplicate on Cmd+Shift+T, and a terminal_panes.uuid UNIQUE error storm. The mechanism holds up under review, but one conclusion in the description needs the reporter's judgment before this is called complete.

Concerns

  • The description concludes that the reporter's emptied command and prompt history was harmless fallout that stops being possible once the root cause is fixed. The transactional rollback argument proves the failing snapshot could not wipe already-saved SQLite state, but it does not establish that the lost tab's session-associated history survived, nor rule out an orphaned session or an earlier successful write of a bad snapshot — APP-5285 records session/prompt association loss as an open risk, and this branch adds no restart or history validation. Only the reporter can settle it: if the history came back after restart, this closes as no-data-loss; if it did not, this needs a tear-off-plus-restart coverage case and a look at the reporter's database before merge.

Verdict

Checks: build pass, tests pass, CI green (core test jobs skipped while draft), visual proof missing (recording in progress)

Found: 0 critical, 1 important, 0 suggestions, 0 nits

Remaining review findings — the marker's trigger condition, the over-broad UNIQUE matcher, the missing pre-fix test failure, and the missing recording — are being addressed in a revision and are not repeated here.

@warp-agent-staging

Copy link
Copy Markdown
Contributor Author

Answer from the reporter on the open history question: the general command and prompt history came back after restart, but the conversation belonging to the dragged tab is missing from it.

That settles it as partial data loss rather than a wipe or a pure display artifact. The snapshot-rehydration explanation holds for the bulk of the history, but the lost tab's conversation is the session-association loss APP-5285 flagged as an open risk, so the description's "harmless fallout, no separate defect" conclusion needs correcting. Being established now: whether that conversation is orphaned in the database or discarded, whether this branch prevents a recurrence or only suppresses the duplicate-tab and save-storm symptoms, and whether the conversation is recoverable.

Comment thread app/src/persistence/sqlite.rs Outdated
Comment thread app/src/lib.rs
@Xavientois
Xavientois requested a review from vorporeal August 11, 2026 15:05
@warp-agent-staging

Copy link
Copy Markdown
Contributor Author

Oz couldn't complete this task because required inputs, permissions, or resources were invalid or unavailable.

Error Details:
Harness 'claude' authentication check failed: login credentials are invalid or expired. Verify that the authentication secret configured for this harness is correct.

oz-agent and others added 2 commits August 11, 2026 15:23
…ndo-close skip

Give the `terminal_panes.uuid` UNIQUE violation its own error context so the
Sentry report states the known cause instead of the generic "SQLite <kind>
error", and document why a content-transferred window close is never recorded
on the undo-close stack rather than filtered when the stack is popped.

Co-Authored-By: Warp <agent@warp.dev>
…ant assert

Fixes four review findings on the original fix:

1. The content-transferred marker was keyed off
   `suppress_detach_panes_on_window_close`, which is also set (and not
   reliably cleared) on windows that stay open after a handoff or
   reverse-handoff. That made a later, ordinary close of such a window get
   mis-marked as content-transferred, silently skipping both its
   undo-close registration and its pane detach. Moved the marking to the
   four exact call sites that issue `close_window(_, ContentTransferred)`
   (`Workspace::close_window_for_content_transfer`, the
   `RemoveSourceTabAndClosePreview` / `ClosePreviewOnly` arms of
   `handle_drop_result`, and `CrossWindowTabDrag::finalize_handoff`'s
   put-back preview close), so the marker reflects the actual close reason
   instead of a proxy signal.

2. `is_terminal_panes_unique_violation` matched any `terminal_panes` UNIQUE
   violation, not just `terminal_panes.uuid` specifically, which would
   have thrown a distinct corruption signal (e.g. a primary-key collision)
   into the same once-per-run throttle. Narrowed the match to the exact
   column and added a negative test.

3. Improved the throttled error's context message to name the specific
   violation and root cause, since `report_db_error` already knows exactly
   what triggered it.

4. Added a debug-time invariant check (`assert_no_duplicate_pane_group_ownership`
   in `get_app_state`) that catches dual pane-group ownership at the
   moment of formation: `debug_assert!` in dev/dogfood builds, throttled
   `report_error!` in release so a regression can't crash a session or
   storm Sentry the way the DB-level symptom did.

Also confirmed (via temporary instrumentation, since removed) that
`workspace:save_app` never fires during the GUI integration test binary's
lifecycle in this environment -- 0 invocations across a full test run,
independent of the fix -- so the new assert and the underlying
`get_app_state`/SQLite path are not reachable from that harness. This is a
structural gap in what the integration harness exercises, not a failure
to trigger the right sequence; the persistence-layer regression coverage
(`test_sqlite_save_app_state_rolls_back_on_duplicate_terminal_uuid`) is
what actually exercises the real `terminal_panes.uuid` violation path.

Co-Authored-By: Warp Agent <agent@warp.dev>
@warp-agent-staging

Copy link
Copy Markdown
Contributor Author

Ready for a merge decision. Posting this here because the Slack thread this came from is currently unreachable.

Since the review, this branch also:

  • marks transfer-driven closes at the four call sites that actually issue them, instead of inferring it from suppress_detach_panes_on_window_close, which outlives the drag and would have made ordinary closed windows un-restorable;
  • narrows the throttle matcher to the exact terminal_panes.uuid constraint, with a negative test for a terminal_panes.id collision;
  • adds the duplicate pane-group-ownership assert in get_app_state, so a future violation surfaces at the moment of duplication rather than as a database error later.

On the missing conversation: orphaned, not deleted. Nothing in the paths this incident exercised writes to agent_conversations; the pane-to-conversation linkage froze when save_app_state began failing, so the restored tab no longer points at it. It should still be reachable from the global conversation list — worth checking, since that determines whether any work was actually lost.

Two caveats to weigh before merging:

  • The GUI integration harness never dispatches save_app/get_app_state at all, so the new integration test could not be shown to fail against pre-fix code. The regression evidence rests on the SQLite-level test and the unit tests.
  • A real native cross-window drag could not be driven in the sandbox, so there is no end-to-end recording. A manual pass — drag a tab out to another window, then Cmd+Shift+T — is worth doing before this lands.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants