fix(app): land on the next session when the open one is deleted

CHANGELOG.md

@@ -193,6 +193,15 @@ already in use` and no way forward short of finding the file in a temporary
   the sandbox rather than in the project on screen. The hold that keeps the
   strip still while a sandbox restarts no longer applies after you switch
   sides.
+- **Deleting the session you are reading takes you to the next one** — the
+  screen stayed on the session that had just been deleted, saying "This session
+  cannot be found" and offering nothing but **Close Tab**. Deleting the session
+  on screen now moves you to the row below it in the sidebar, and from the
+  bottom of the list back to the top. If the delete emptied the list, you land
+  on a new session in the same project. The deleted session leaves the tab strip
+  with it — no leftover **Unknown Session** tab to close by hand. Deleting a
+  session you are not currently in still just removes it and its tab, leaving
+  you where you were. Both **Code** and **Work**.
 - **A folder on Work is unmounted from its own menu** — the menu on a **Work**
   project offered **Close**, which only took the row out of the sidebar while
   the sandbox went on holding the folder: the folder was still mounted, still

packages/app/e2e/regression/session-delete-navigation.spec.ts

@@ -0,0 +1,104 @@
+import { expect, test, type Locator, type Page } from "@playwright/test"
+import { fixture, pageMessages } from "../smoke/session-timeline.fixture"
+import { mockOpenCodeServer } from "../utils/mock-server"
+import { expectAppVisible, expectSessionTitle } from "../utils/waits"
+
+// Deleting the session you are looking at used to leave the route pointing at
+// it: the tab went, the screen stayed, and what it said was "This session
+// cannot be found". It now lands on the row below the deleted one in the
+// sidebar, wrapping from the bottom of the list back to the top.
+//
+// The fixture lists two sessions, newest first, so the rows read
+// [target, source] and the two directions are one row apart.
+
+const sidebar = (page: Page) => page.locator('[data-component="app-sidebar"]')
+const rows = (page: Page) => sidebar(page).locator('[data-component="sidebar-session-row"]')
+// The menu is a sibling of the row button, not a child — both sit in the
+// wrapper that owns the hover state.
+const rowMenu = (row: Locator) => row.locator("xpath=..").locator('[data-action="session-menu"]')
+const dialog = (page: Page) => page.locator('[data-slot="dialog-content"]')
+const tabs = (page: Page) => page.locator("[data-titlebar-tab-slot]")
+
+// Hover first: the trigger is pointer-events-none until its row is hovered, and
+// Playwright hit-tests before it moves the mouse, so a bare click never becomes
+// actionable.
+async function deleteRow(page: Page, row: Locator) {
+  await row.hover()
+  await rowMenu(row).click()
+  await expect(rowMenu(row)).toHaveAttribute("aria-expanded", "true")
+  await page.getByRole("menuitem", { name: /^Delete/ }).click()
+  await expectAppVisible(dialog(page))
+  await dialog(page).getByRole("button", { name: "Delete session" }).click()
+  await expect(dialog(page)).toHaveCount(0)
+}
+
+async function boot(page: Page) {
+  await mockOpenCodeServer(page, {
+    sessions: fixture.sessions,
+    provider: fixture.provider,
+    directory: fixture.directory,
+    project: fixture.project,
+    pageMessages,
+  })
+  await page.addInitScript((directory) => {
+    localStorage.setItem(
+      "opencode.global.dat:server",
+      JSON.stringify({
+        projects: { local: [{ worktree: directory, expanded: true }] },
+        lastProject: { local: directory },
+      }),
+    )
+  }, fixture.directory)
+  await page.goto("/")
+  await expectAppVisible(sidebar(page))
+  await expectAppVisible(rows(page).first())
+}
+
+test("lands on the row below when the open session is deleted", async ({ page }) => {
+  await boot(page)
+
+  await rows(page).first().click()
+  await expectSessionTitle(page, fixture.expected.targetTitle)
+
+  await deleteRow(page, rows(page).first())
+
+  await expectSessionTitle(page, fixture.expected.sourceTitle)
+  await expect.poll(() => new URL(page.url()).pathname).toContain(`/session/${fixture.sourceID}`)
+  // The deleted session's tab goes with it, leaving only the one landed on.
+  // Removing the tab and then navigating away re-added it from the route the
+  // instant it had no tab, and it came back as an "Unknown Session" ghost —
+  // titleless, because the removal took its remembered title too. The mock
+  // still answers for a deleted session, so here the ghost keeps its title;
+  // what gives it away either way is a tab that should not be in the strip.
+  await expect(tabs(page)).toHaveCount(1)
+  await expect(tabs(page).locator("a").first()).toHaveAttribute("href", new RegExp(`/session/${fixture.sourceID}$`))
+})
+
+// The bottom row has nothing below it, and stopping there is what left the
+// route on a deleted session in the first place.
+test("wraps to the top row when the bottom one is deleted", async ({ page }) => {
+  await boot(page)
+
+  await rows(page).nth(1).click()
+  await expectSessionTitle(page, fixture.expected.sourceTitle)
+
+  await deleteRow(page, rows(page).nth(1))
+
+  await expectSessionTitle(page, fixture.expected.targetTitle)
+  await expect.poll(() => new URL(page.url()).pathname).toContain(`/session/${fixture.targetID}`)
+})
+
+// Only the session on screen moves anyone. Deleting a row you are not in takes
+// its tab and leaves you where you were.
+test("stays put when the deleted session is not the one on screen", async ({ page }) => {
+  await boot(page)
+
+  await rows(page).nth(1).click()
+  await expectSessionTitle(page, fixture.expected.sourceTitle)
+  const before = new URL(page.url()).pathname
+
+  await deleteRow(page, rows(page).first())
+
+  await expectSessionTitle(page, fixture.expected.sourceTitle)
+  expect(new URL(page.url()).pathname).toBe(before)
+})

packages/app/src/pages/home-model.tsx

@@ -243,6 +243,10 @@ export function createHomeModel() {
   })
   const isPinned = (session: Session) => pinnedIDs().includes(session.id)
   const togglePin = (session: Session) => pinned?.toggle(selection().server, session.id)
+  // Fork: the sidebar's sessions in the order it renders them — Pinned above
+  // Recents — which is the list "the one below this" refers to when a delete
+  // has to pick where to land.
+  const listedSessions = () => [...pinnedRecords(), ...recentRecords()].map((record) => record.session)
 
   function setSelection(next: HomeProjectSelection) {
     layout.home.setSelection(next)
@@ -325,7 +329,8 @@ export function createHomeModel() {
       tabs.select(existing)
       return
     }
-    tabs.newDraft({ server, directory })
+    // Fork: returned for the same reason openSession's transition is.
+    return tabs.newDraft({ server, directory })
   }
 
   function editProject(conn: ServerConnection.Any, project: LocalProject) {
@@ -374,7 +379,9 @@ export function createHomeModel() {
       return
     }
     ctx.projects.touch(directory)
-    startTransition(() => {
+    // Fork: returned so a caller that must not act until the route has moved
+    // can await it — deleteSession drops the old tabs only once this lands.
+    return startTransition(() => {
       const tab = tabs.addSessionTab({ server: ServerConnection.key(conn), sessionId: session.id, directory })
       tabs.select(tab)
     })
@@ -438,6 +445,10 @@ export function createHomeModel() {
     const conn = focusedServer()
     const ctx = focusedServerCtx()
     if (!conn || !ctx) return
+    // Fork: snapshotted before the request, so where the delete lands is
+    // decided from the list the user was looking at when they confirmed it —
+    // the delete itself refreshes the index this is derived from.
+    const listed = listedSessions()
     const [store, setStore] = ctx.sync.child(session.directory)
     const deleted = await ctx.sdk.client.session
       .delete({ directory: session.directory, sessionID: session.id })
@@ -459,6 +470,28 @@ export function createHomeModel() {
     // be pinned too.
     removed.forEach((id) => pinned?.remove(ServerConnection.key(conn), id))
     refreshIndex()
+    // Fork: the deleted session's tab goes with it, but the route stayed where
+    // it was — which left the window on a session that no longer exists,
+    // reading "This session cannot be found" with nothing to do but close the
+    // tab. Land on the row below it in the sidebar instead, and on a new
+    // session in the same project when the delete emptied the list. Deleting a
+    // session that was not on screen, or one belonging to a server the route is
+    // not on, moves nobody.
+    //
+    // Ahead of dropping the tabs, and awaited, because the titlebar keeps a tab
+    // for whatever session the route names: leaving first and removing second
+    // means the route never names a session with no tab, which is the state
+    // that had it mint the deleted session a fresh, titleless tab — the
+    // "Unknown Session" left behind in the strip.
+    const route = layout.route()
+    const leaving =
+      route.type === "session" && route.server === ServerConnection.key(conn) && removed.has(route.sessionId)
+    if (leaving) {
+      const next = sessionAfterRemoval(listed, session.id, removed)
+      if (next) await openSession(next)
+      else await openProjectNewSession(conn, session.directory)
+    }
+
     notifySessionTabsRemoved({
       server: ServerConnection.key(conn),
       directory: session.directory,
@@ -516,6 +549,29 @@ export function createHomeModel() {
   }
 }
 
+// Fork: where deleting the session on screen lands — the row below it in the
+// sidebar's own list. The scan wraps, which is what turns deleting the bottom
+// row into the top one rather than into nothing.
+//
+// Skipping the ids the delete took is what stops the wrap from coming all the
+// way back around to the deleted session itself, which is the only candidate
+// left when it was the only row. That is the caller's cue to open a new session
+// instead, and the one case this answers undefined for. The set covers the
+// whole subtree rather than the one id, which costs nothing and does not depend
+// on child sessions staying out of the index the list is built from.
+//
+// Indexed by the deleted session rather than by the session on screen, because
+// those differ whenever a child session is open and its parent is deleted, and
+// a child is not a row. A deleted session the list does not hold — one past the
+// rendered cap — starts the scan at the top for the same reason: it has no row
+// below it, and the top of the list is the honest answer.
+export function sessionAfterRemoval(listed: Session[], deletedID: string, removed: ReadonlySet<string>) {
+  const index = listed.findIndex((session) => session.id === deletedID)
+  return listed
+    .map((_, step) => listed[(index + 1 + step) % listed.length]!)
+    .find((session) => !removed.has(session.id))
+}
+
 // Exported for the same reason buildHomeSessionRecords is: it is the rule that
 // decides how much a delete takes with it, and it is worth testing on its own.
 export function descendantIDs(sessions: Session[], rootID: string) {

packages/app/test-browser/home-session-records.test.ts

@@ -1,7 +1,7 @@
 import { describe, expect, test } from "bun:test"
 import type { Session } from "@opencode-ai/sdk/v2/client"
 import type { LocalProject } from "@/context/layout"
-import { buildHomeSessionRecords, descendantIDs } from "@/pages/home-model"
+import { buildHomeSessionRecords, descendantIDs, sessionAfterRemoval } from "@/pages/home-model"
 
 // The recents list the persistent sidebar and the home screen both render comes
 // out of buildHomeSessionRecords, and the subtree a delete evicts comes out of
@@ -206,3 +206,54 @@ describe("descendantIDs", () => {
     expect([...descendantIDs(cyclic, "a")].sort()).toEqual(["a", "b"])
   })
 })
+
+describe("sessionAfterRemoval", () => {
+  // The sidebar's own order: Pinned above Recents, each newest first. The
+  // function is handed one flat list because that is what the user reads.
+  const listed = ["pinned_a", "recent_a", "recent_b", "recent_c"].map((id) => ({ id })) as unknown as Session[]
+  const after = (deleted: string, removed = [deleted]) => sessionAfterRemoval(listed, deleted, new Set(removed))?.id
+
+  test("lands on the row below the deleted one", () => {
+    expect(after("recent_a")).toBe("recent_b")
+  })
+
+  test("crosses from Pinned into Recents", () => {
+    expect(after("pinned_a")).toBe("recent_a")
+  })
+
+  test("wraps to the top when the bottom row is deleted", () => {
+    expect(after("recent_c")).toBe("pinned_a")
+  })
+
+  // The removed set is the whole deleted subtree, not just the one id. Children
+  // are not rows today, so this is the defensive half of the skip.
+  test("skips rows the same delete took", () => {
+    expect(after("recent_a", ["recent_a", "recent_b"])).toBe("recent_c")
+  })
+
+  test("wraps past a removed run to the first survivor", () => {
+    expect(after("recent_b", ["recent_b", "recent_c", "pinned_a"])).toBe("recent_a")
+  })
+
+  // The wrap would otherwise come back around to the deleted session and hand
+  // the caller the very row it just removed. Undefined is what sends the user
+  // to a new session instead.
+  test("is undefined when the delete emptied the list", () => {
+    expect(after("recent_a", ["pinned_a", "recent_a", "recent_b", "recent_c"])).toBeUndefined()
+  })
+
+  test("is undefined for an empty list", () => {
+    expect(sessionAfterRemoval([], "recent_a", new Set(["recent_a"]))).toBeUndefined()
+  })
+
+  // Past the rendered cap, so it has no row below it: the top of the list is
+  // the only answer that is not arbitrary.
+  test("starts at the top for a session the list does not hold", () => {
+    expect(after("ses_offscreen")).toBe("pinned_a")
+  })
+
+  test("keeps the caller's order rather than re-sorting", () => {
+    const reversed = ["recent_c", "recent_b", "recent_a"].map((id) => ({ id })) as unknown as Session[]
+    expect(sessionAfterRemoval(reversed, "recent_c", new Set(["recent_c"]))?.id).toBe("recent_b")
+  })
+})