fix(machines): a view of terminals you can actually remove by 2witstudios · Pull Request #2091 · 2witstudios/PageSpace

2witstudios · 2026-07-16T22:48:45Z

The bug

A machine kept at least one workspace, always. That floor is the whole bug, and it produced both reported symptoms:

removeWorkspace (workspace-reducer.ts:443) no-op'd on the last workspace, so WorkspaceLeaves compensated by emptying its panes in place — the row survived every removal attempt. An unremovable listing, no matter how many times you confirmed.
+ New terminal (NodeActionPalette.tsx:280) then called createWorkspace unconditionally, minting a second workspace and activating it. The middle view swapped ("sorta replaces it"), but the sidebar now showed the zombie row and the new one. Two listings.

Both symptoms, one cause.

A second, independent unremovable path: a shell/claude/codex orphan rendered its remove button only when !launchable (:288), so it had no remove button at all — and the only stop path was "remove the workspace holding it", which is precisely what an unclaimed session lacks.

The fix

A workspace is a view of terminals. A view you cannot destroy is not a view. Zero workspaces is now a legal, converged state.

Reducer — drop the >=1 floor. activeWorkspaceId: '' is the no-active sentinel. mergeServerWorkspaces applies an empty server list instead of falling back to local — "server has zero" could never converge before, since hydrate runs once per mount and nothing prunes after. applyServerWorkspaceDeleted now applies to the last row (it was silently dropped, so a teammate removing the last view left you a permanent phantom).
Store — ensureMachine creates the machine's entry and nothing else. It still repairs a dangling activeWorkspaceId, by re-targeting rather than minting.
Empty state — "No terminals open" + New Terminal, in TerminalPanes. Keyed on "no active workspace resolves", not on the entry existing (sanitizeMachines drops empty entries on rehydrate).
Close means close — closing a pane kills its PTY unless another pane shows that same session; closing the last pane removes the workspace. Detach is what manufactured the orphan rows.
Every unclaimed row is removable, not just !launchable ones.

NodeActionPalette needed no change — once removal genuinely removes, an unconditional createWorkspace is correct.

Five traps this had to step around

Resurrection. pushWorkspaceUpdate PATCHes and falls back to POST-create on a 404 (useMachineWorkspaceSync.ts:294-311). A layout push for a workspace the close just removed would re-create it server-side and broadcast the resurrected row back — including to the browser whose user just closed it. Last-pane close routes to pushRemoval; a test asserts PATCH is not called.
order[-1]. Dropping the floor makes the neighbour lookup order[Math.min(n, -1)] → undefined in a field typed string. The repo has strict but not noUncheckedIndexedAccess, so it compiles and ships, and downstream reads it as "not mounted yet" rather than "nothing active". Guarded with ?? ''; a test asserts === '' exactly (not just falsy).
Empty bootstrap claims. Now that ensureMachine mints nothing, the payload is usually empty — and DevelopmentSidebar mounts the sync hook per machine row, so merely rendering it would fire an empty first-writer-wins claim at every machine in the drive, permanently discarding other browsers' un-migrated history. Guarded — and it hydrates rather than merely skipping the POST (the normal hydrate is gated on bootstrapped). That hydrate is provisional (Codex P2): it records the bootstrap decision (bootstrapAttempted) but not hydratedOnce, so when another browser with real history later wins the claim, the bootstrapped broadcast — the ONLY way its seeded rows reach this browser — still applies instead of being gated out until remount. And the decision is cross-instance (a module-level registry): the hook is mounted more than once per machine (sidebar row + MachineView), and instance 1's provisional hydrate writes server rows into the shared store — a per-instance ref would let instance 2 read that non-empty list and burn the claim with an echo of the server's own rows.
Kill by the pane's own scope (Codex P2). Close-kill originally routed through the workspace-scoped useAgentTerminals instance — but a pane's (project, branch) can differ from its workspace's (restored server layouts), and a DELETE under the workspace scope would kill a different same-named terminal (or nothing) while the closed one lived on unclaimed. killAgentTerminal(machineId, scope) addresses the DELETE by the pane's own triple and revalidates that scope's SWR list. The sweep for this class also caught WorkspaceLeaves' remove-workspace kill: it now uses pane scopes too, spares sessions another workspace's pane still shows (the invariant close-and-kill enforces), and dedupes two panes showing one session; the unclaimed-row check compares full session identity machine-wide instead of pane names at the node. A kill that 404s counts as success — the session (or its checkout) is already gone, which is the call's goal state; treating it as failure made a workspace holding a stale pane permanently unremovable (kill rejects → dialog stays open → retry hits the same 404 forever). Real failures (5xx, network) still throw: a running, billing agent's only row must not be dropped silently. The closing pane is resolved within its own workspace (CodeRabbit): pane ids are only grid-unique (server layouts carry ids other clients minted), so a machine-wide id lookup could kill a same-id pane's session in another workspace; boundElsewhere compares (workspace, pane) tuples for the same reason.
Removal has no "next push". Every push here is optimistic and swallows transient failures because the next push reconciles — but nothing follows a removal, so a swallowed DELETE failure left the server row alive to resurrect the removed workspace on the next hydrate (CodeRabbit). pushRemoval now retries (bounded, backoff), and each retry checks the workspace is still locally removed first — session-derived workspace ids are deterministic, so a stale retry could otherwise delete a re-opened session's new incarnation. Deliberately no rollback-on-failure: restoring a grid whose PTYs were already killed is a strictly worse dead-pane state; the resurrected row is visible and removable again.

Session identity is the (project, branch, name) triple — machine_agent_terminals' unique index — so both the "bound elsewhere?" check and the kill itself use the full triple, not the name alone.

Considered and rejected

Session-first sidebar (sessions = only server truth, layout = pure local view state). This is PR feat(machines): wire shared MachineTree to multi-workspace store #2031, which Server-synced multiplayer workspaces (machine workspace list/layout/active-selection) #2048/feat(machines): server-authoritative workspace list + layout sync #2058 deliberately replaced (machine-workspaces.ts:36-38 names the gap). It also can't work: name is the session's DB key, there's no update-name method, and the list route doesn't return id — so it loses rename, active-highlight and pane grouping. And a view holds multiple panes by design, so it can't collapse into a session.
Deleting the bootstrap claim table. Not just migration machinery: workspace ids are crypto.randomUUID(), so per-id upsert can't dedupe — without first-writer-wins, three devices POSTing disjoint local lists yield 3x the workspaces. Structural.

Deliberately not fixed

#2048 required the unclaimed-session row become redundant once workspaces were shared truth; #2058 shipped without it. Two server truths (machine_agent_terminals, machine_workspaces) can still disagree, so orphans remain possible via other browsers and agent-spawned sessions — kill-on-close just stops this path feeding it. Closing it means giving every spawn path an owning workspace server-side: its own PR.

Test plan

bun run typecheck — clean
eslint on all changed files — clean
14,585 tests pass. The 4 failing files (activity-tools, grouping, admin-role-version, version-sdk-contract) fail identically on clean master — verified by stashing, not assumed.
Machine surface: 26 files / 388 tests green, across 3 repeat runs.
Inverted the ~8 tests that pinned the floor, including DevelopmentSidebar.test.tsx's expect(...workspaces.length).toBe(2) — that assertion was the bug report.
Added: the end-to-end repro (remove → New terminal → exactly one row); activeWorkspaceId === '' exactly; pushRemoval-not-PATCH on last-pane close; same-name-different-branch is a different session; empty-payload bootstrap does not claim but does hydrate; a later bootstrapped broadcast still applies after an empty non-claim hydrate; close-kill carries the pane's own scope (zero calls through the workspace-scoped hook); workspace removal kills at pane scope / spares sessions shown elsewhere / dedupes; a 404'd kill still removes the workspace; dual-mounted sync hooks never claim the bootstrap with an echo of server rows; a same-id pane in another workspace is never mistaken for the closing pane; a failed removal DELETE retries but a retry is abandoned if the same workspace id was re-materialized.

⚠️ Not verified in a real browser — machines need CODE_EXECUTION_ENABLED + live Sprites credentials. The empty state and kill-on-close are covered by component tests, not by clicking.

Behaviour change worth flagging

Closing a pane now kills its agent, with no confirm step — a stray click ends a running agent. Chosen deliberately (close means close, as in every terminal emulator; detach is what creates the unremovable orphans). Note this does not make machines go cold any sooner: there is no idle reaper (sprites.ts:468), Sprites hibernate on their own, and session count is not an input.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LHzgVXZg69UoSYkigvAT1k

A machine kept at least one workspace, always. That floor is the bug: - `removeWorkspace` no-op'd on the last workspace, so `WorkspaceLeaves` compensated by emptying its panes in place — the row survived every removal attempt, unremovable no matter how many times you confirmed. - `+ New terminal` then called `createWorkspace` unconditionally, minting a SECOND workspace and activating it: the middle view swapped, but the sidebar now showed the zombie row AND the new one. Both symptoms, one cause. A workspace is a VIEW of terminals, and a view you cannot destroy is not a view. Zero workspaces is now a legal, converged state. - reducer: drop the >=1 floor; `activeWorkspaceId: ''` is the no-active sentinel. `mergeServerWorkspaces` applies an empty server list instead of falling back to local — "server has zero" could never converge before, since hydrate runs once per mount and nothing prunes after. `applyServerWorkspaceDeleted` now applies to the last row (it was silently dropped, leaving a phantom). - store: `ensureMachine` creates the machine's ENTRY and nothing else. It still repairs a dangling `activeWorkspaceId`, by re-targeting rather than minting. - TerminalPanes: "No terminals open" + New Terminal. Keyed on "no active workspace resolves", not on the entry existing (`sanitizeMachines` drops empty entries on rehydrate). - close means close: closing a pane kills its PTY unless another pane shows that same session; closing the last pane removes the workspace. Detach is what MANUFACTURED the orphan rows in the first place. - every unclaimed row is now removable, not just `!launchable` ones — a shell/claude/codex orphan had no remove button at all, and the only stop path was "remove the workspace holding it", which is precisely what it lacks. Three traps this had to step around: 1. `pushWorkspaceUpdate` PATCHes and falls back to POST-create on a 404, so a layout push for a workspace the close just removed would RE-CREATE it server-side and broadcast the resurrected row back. Last-pane close routes to `pushRemoval`; a test asserts PATCH is not called. 2. The neighbour lookup becomes `order[-1]` -> `undefined` in a field typed `string` (`noUncheckedIndexedAccess` is off, so it compiles and ships), which downstream reads as "not mounted yet" rather than "nothing active". Guarded with `?? ''`; a test asserts `=== ''` exactly. 3. Now that `ensureMachine` mints nothing, the bootstrap payload is usually empty — and DevelopmentSidebar mounts the sync hook per machine row, so rendering it would fire an empty first-writer-wins claim at every machine in the drive, discarding other browsers' un-migrated history. Guarded, and it hydrates rather than merely skipping the POST. Session identity is the (project, branch, name) triple — `machine_agent_terminals`' unique index — so the "bound elsewhere?" check compares `sessionWorkspaceId`, not the name alone. Deliberately NOT fixed: #2048 required the unclaimed-session row become redundant once workspaces were shared truth; #2058 shipped without it, so two server truths can still disagree. Orphans remain possible via other browsers and agent-spawned sessions. Closing that means giving every spawn path an owning workspace server-side — its own PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LHzgVXZg69UoSYkigvAT1k

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 71429adae4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-07-16T22:53:44Z

+    // `data.bootstrapped`, so returning without one would leave this browser
+    // showing an empty machine while the server holds real rows.
+    if (payload.length === 0) {
+      hydratedOnce.current = true;


Keep empty non-claims eligible for bootstrap broadcasts

When an unbootstrapped machine is first opened by a browser with no local workspace history, this branch hydrates the current empty/non-claimed list and marks the hook as fully hydrated. If another browser with local history then wins /bootstrap, the only notification for those seeded workspaces is machine-workspace:bootstrapped, but onBootstrapped ignores it when hydratedOnce.current is already true, so the first browser stays on an empty workspace list until remount. This path should not suppress the later bootstrap broadcast for machines that were explicitly not claimed.

Useful? React with 👍 / 👎.

Fixed in 984bf19. The empty non-claim hydrate is now provisional: it records the bootstrap decision by setting bootstrapAttempted (this browser has nothing to migrate, ever) but leaves hydratedOnce unset — so onBootstrapped still applies a later claim's broadcast, and a bootstrapped: true SWR revalidation (a missed broadcast) re-enters the hydrate instead of being gated out. Leaving hydratedOnce unset there is safe because that branch only runs when the local list is empty, so the full-list replace cannot wipe anything. Regression test added: given an empty non-claim hydrate, a LATER bootstrapped broadcast (another browser winning the claim) still applies.

Hardened further in 7382c3c after an adversarial pass over the fix: the "nothing to migrate" decision now lives in a module-level registry rather than a per-instance ref. The hook is mounted more than once per machine (DevelopmentSidebar per machine row + MachineView), and on an unclaimed machine with server rows, instance 1's provisional hydrate writes those rows into the shared store — so instance 2 would read a non-empty local list and POST a claim echoing the server's own rows, burning first-writer-wins on nothing. Only the first decliner applies the provisional hydrate (a later instance re-applying SWR's possibly stale cached list would wipe workspaces that arrived over the socket since). Regression test: given TWO mounted instances and an unclaimed machine with server rows, neither claims the bootstrap with an echo of those rows.

chatgpt-codex-connector · 2026-07-16T22:53:44Z

+
+      closePane(workspaceId, paneId);
+      if (closing !== null && !boundElsewhere) {
+        void removeAgentTerminal(closing.name).catch(() => {


Kill panes using the pane's own scope

Here removeAgentTerminal is the hook instance scoped to the active workspace's scope, not to closing.scope. In a restored/server layout where a pane belongs to a different project/branch than its workspace (the code above already treats (project, branch, name) as the session identity), closing that pane DELETEs the same name under the workspace scope, which can kill a different visible terminal and leave the intended one running as an unclaimed session. The kill call needs to target closing.projectName/closing.branchName, not just closing.name through the workspace-scoped hook.

Useful? React with 👍 / 👎.

Fixed in 984bf19. Added a module-level killAgentTerminal(machineId, scope) in useAgentTerminals.ts that DELETEs by the pane's own (projectName, branchName, name) triple and revalidates that scope's SWR list key (so sidebar rows on that scope drop the dead row). closePaneAndKill now calls it with closing (the pane's own scope) instead of routing through the workspace-scoped hook instance. The existing close-kill tests now assert the full scope — the SOLO_WORKSPACE fixture (machine-scoped pane inside an app/main workspace) is exactly the mismatch case, and the test verifies the kill carries { name: 'solo' } with zero calls through the workspace-scoped hook.

Follow-up in d60f63d: swept the rest of the PR for the same defect class and found it in WorkspaceLeaves' remove-workspace confirm, which killed each pane's session by name through the node-scoped hook. That path now (1) kills via killAgentTerminal at each pane's own scope, (2) spares sessions another workspace's pane still shows (the same invariant closePaneAndKill enforces), and (3) dedupes two panes showing one session so the DELETE fires once instead of 404ing on the second call. The unclaimed-row check now also compares full session identity (sessionWorkspaceId) machine-wide instead of pane names at the node. Three regression tests added.

…ootstrap-receptive Two Codex review findings, both real: - closePaneAndKill routed the kill through the workspace-scoped useAgentTerminals instance, so a pane whose (project, branch) differs from its workspace's — a shape a restored server layout can hold — would DELETE a same-named session at the WRONG checkout and leave the intended one running as an unclaimed row. New killAgentTerminal addresses the DELETE by the pane's own (project, branch, name) triple and revalidates that scope's SWR list. - The empty-payload non-claim hydrate set hydratedOnce, which made onBootstrapped ignore the broadcast fired when another browser (with real un-migrated history) later wins the claim — stranding the empty browser on the unclaimed list until remount. The hydrate is now provisional: it records the bootstrap decision (bootstrapAttempted, nothing to migrate) but leaves hydratedOnce unset, so the later bootstrapped broadcast — or a bootstrapped revalidation after a missed one — still applies. Safe because that branch only runs when the local list is empty, so a full-list replace cannot wipe anything. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015c9exjsZrv5aJa4Rcnrcs6

…essions, dedupes Sweep for the same defect class as the TerminalPanes close-kill review finding: WorkspaceLeaves' remove-workspace confirm killed each pane's session BY NAME through the node-scoped hook. Three gaps, one path: - a pane bound at a different checkout than its workspace would be killed under the node's scope — a different same-named terminal, or nothing, while the real one lives on as an unclaimed row; - a session also shown in ANOTHER workspace's pane was killed anyway, pulling the PTY out from under a pane still showing it (TerminalPanes' close-and-kill already spares these); - two panes of the removed workspace showing one session DELETEd it twice — the second call 404s and surfaces a spurious failure toast. Now: dedupe by sessionWorkspaceId, exclude sessions shown elsewhere, kill via killAgentTerminal at each pane's own scope; the confirm dialog counts the sessions actually stopped. The unclaimed-row check also compares full session identity machine-wide instead of pane names at this node, so a same-named session at another checkout can't mask a genuinely unclaimed one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015c9exjsZrv5aJa4Rcnrcs6

coderabbitai · 2026-07-16T23:44:14Z

📝 Walkthrough

Walkthrough

The PR allows machines to have zero workspaces, updates pane and workspace removal semantics, adds scope-aware agent termination, changes bootstrap synchronization, and updates sidebar, terminal, and workspace tests for the new lifecycle.

Changes

Workspace and session lifecycle

Layer / File(s)	Summary
Empty workspace reducer semantics `apps/web/src/stores/machine-workspace/workspace-reducer.ts`, `apps/web/src/stores/machine-workspace/__tests__/workspace-reducer.test.ts`	Reducer state now supports zero workspaces, final-workspace removal, empty active-workspace state, and separate pane-level closure handling.
Store initialization and synchronization `apps/web/src/stores/machine-workspace/useMachineWorkspaceStore.ts`, `apps/web/src/hooks/useMachineWorkspaceSync.ts`, `apps/web/src/stores/machine-workspace/__tests__/*`, `apps/web/src/hooks/__tests__/useMachineWorkspaceSync.test.ts`	`ensureMachine` no longer creates a workspace, `closePane` reports workspace deletion, empty bootstrap claims are declined, and synchronization chooses DELETE or PATCH based on the resulting state.
Scope-aware session removal `apps/web/src/hooks/useAgentTerminals.ts`, `apps/web/src/components/layout/middle-content/page-views/machine/workspace/WorkspaceLeaves.tsx`, `apps/web/src/components/layout/middle-content/page-views/machine/workspace/WorkspaceLeaves.test.tsx`	Agent sessions are deleted by pane scope with deduplication and shared-session checks; workspaces are removed afterward, including the final workspace.
Terminal pane close and empty-state flow `apps/web/src/components/layout/middle-content/page-views/machine/workspace/TerminalPanes.tsx`, `apps/web/src/components/layout/middle-content/page-views/machine/workspace/TerminalPanes.test.tsx`, `apps/web/src/components/layout/left-sidebar/`, `apps/web/src/components/layout/middle-content/page-views/machine/tabs/TerminalTab.test.tsx`, `apps/web/src/lib/development/`	Terminal panes distinguish uninitialized machines from empty machines, create machine-scoped workspaces from the empty state, and allow pane closure with conditional session termination.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant TerminalPanes
  participant useMachineWorkspaceStore
  participant killAgentTerminal
  participant AgentTerminalAPI
  TerminalPanes->>useMachineWorkspaceStore: closePaneAndKill
  TerminalPanes->>killAgentTerminal: terminate unshared pane session
  killAgentTerminal->>AgentTerminalAPI: DELETE terminal by scope
  TerminalPanes->>useMachineWorkspaceStore: remove pane or workspace

Possibly related PRs

2witstudios/PageSpace#1965: Overlaps with the pane close and control behavior in TerminalPanes.tsx.
2witstudios/PageSpace#2006: Overlaps with narrow-viewport pane controls and canClose behavior.
2witstudios/PageSpace#2058: Related to the server-authoritative workspace synchronization and empty-workspace contract.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 84.21% which is sufficient. The required threshold is 80.00%.
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	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and captures the main theme of making terminal rows/workspaces removable.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch pu/always-a-terminal

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.}

…egistry Two holes found by adversarially reviewing the previous convergence commits, both defeating this PR's own mission: - killAgentTerminal treated a 404 as failure, so a workspace holding a stale pane (session already gone server-side) could NEVER be removed: the kill rejected, the confirm dialog stayed open, and every retry hit the same 404 — an unremovable listing, the exact bug class this PR exists to kill. A 404 now counts as success (the session, or the checkout that held it, is already gone — the call's goal state); real failures (5xx, network) still throw, since a running agent's only row must not be dropped silently. Reads the raw status via fetchWithAuth — del() throws a plain Error with no status attached. - The empty-payload bootstrap decline was tracked per hook INSTANCE, but the hook is mounted more than once per machine (DevelopmentSidebar per machine row + MachineView) sharing one store: on an unclaimed machine with server rows, instance 1's provisional hydrate wrote those rows into the store, so instance 2 read a non-empty local list and POSTed a bootstrap claim echoing the server's own rows — burning first-writer-wins on nothing and permanently foreclosing a legacy browser's real history migration. The decision now lives in a module-level registry visible to every instance, and only the first decliner applies the provisional hydrate (a later instance re-applying SWR's possibly stale cached list would wipe workspaces that arrived over the socket since). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015c9exjsZrv5aJa4Rcnrcs6

coderabbitai

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@apps/web/src/components/layout/middle-content/page-views/machine/workspace/TerminalPanes.tsx`:
- Around line 151-165: Update the pane lookup in the machine-wide close flow so
paneId is matched within its owning workspace, not treated as globally unique.
Scope closing and boundElsewhere comparisons to that workspace before deciding
whether to kill the session, while preserving sessionWorkspaceId identity
checks. Add a regression test covering two workspaces with the same pane ID but
different session scopes.

In `@apps/web/src/hooks/useAgentTerminals.ts`:
- Around line 70-74: Update killAgentTerminal to avoid propagating errors from
the post-delete SWR mutate call: trigger mutate for revalidation without
awaiting it, or otherwise swallow its rejection. Preserve the existing
DELETE/404 handling and ensure successful terminal removal remains resolved even
if revalidation fails.

In `@apps/web/src/hooks/useMachineWorkspaceSync.ts`:
- Around line 430-433: Update closePane in
apps/web/src/hooks/useMachineWorkspaceSync.ts:430-433 so workspace removal
awaits a propagated or reliably retried DELETE, and rollback or reconcile local
state when it fails instead of treating pushRemoval as successful. Update
WorkspaceLeaves.tsx:339-351 to await synchronized removal and preserve the
failure after terminal cleanup completes, keeping the error visible to the user.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: aecb7102-324b-413e-ad0d-c993cef6fc27

📥 Commits

Reviewing files that changed from the base of the PR and between 4a56307 and 7382c3c.

📒 Files selected for processing (16)

apps/web/src/components/layout/left-sidebar/DevelopmentSidebar.tsx
apps/web/src/components/layout/left-sidebar/__tests__/DevelopmentSidebar.test.tsx
apps/web/src/components/layout/middle-content/page-views/machine/tabs/TerminalTab.test.tsx
apps/web/src/components/layout/middle-content/page-views/machine/workspace/TerminalPanes.test.tsx
apps/web/src/components/layout/middle-content/page-views/machine/workspace/TerminalPanes.tsx
apps/web/src/components/layout/middle-content/page-views/machine/workspace/WorkspaceLeaves.test.tsx
apps/web/src/components/layout/middle-content/page-views/machine/workspace/WorkspaceLeaves.tsx
apps/web/src/hooks/__tests__/useMachineWorkspaceSync.test.ts
apps/web/src/hooks/useAgentTerminals.ts
apps/web/src/hooks/useMachineWorkspaceSync.ts
apps/web/src/lib/development/pending-workspace.ts
apps/web/src/lib/development/use-drain-pending-workspace.ts
apps/web/src/stores/machine-workspace/__tests__/useMachineWorkspaceStore.test.ts
apps/web/src/stores/machine-workspace/__tests__/workspace-reducer.test.ts
apps/web/src/stores/machine-workspace/useMachineWorkspaceStore.ts
apps/web/src/stores/machine-workspace/workspace-reducer.ts

…ed removal retry Three CodeRabbit findings on the convergence commits: - closePaneAndKill resolved the closing pane by a MACHINE-wide id lookup, but pane ids are only unique within their own grid (server layouts carry ids other clients minted) — a same-id pane in a workspace earlier in the machine order would be found first and ITS session killed instead. The lookup is now scoped to the closing pane's own workspace, and boundElsewhere compares (workspace, pane) tuples. - killAgentTerminal awaited the post-kill SWR revalidation, so a transient list-refetch failure rejected a teardown that had already succeeded — keeping the remove-workspace dialog open over a dead session. The revalidation is now fire-and-forget. - pushRemoval swallowed a failed DELETE outright; the server row survived and the next hydrate resurrected the locally removed workspace. Removal has no "next push" to reconcile through, so it now retries (bounded, backoff), and each retry first checks the workspace is still locally removed — session-derived workspace ids are deterministic, so a stale retry could otherwise delete a re-opened session's new incarnation out from under the user. Deliberately no rollback-on-failure: restoring a grid whose PTYs were already killed is a strictly worse dead-pane state, and the resurrected row is visible and removable again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015c9exjsZrv5aJa4Rcnrcs6

chatgpt-codex-connector Bot reviewed Jul 16, 2026

View reviewed changes

2witstudios mentioned this pull request Jul 16, 2026

feat(machines): broadcast terminal output to every viewer — attach is currently a takeover, not a join #2093

Closed

6 tasks

2witstudios and others added 2 commits July 16, 2026 18:31

coderabbitai Bot reviewed Jul 17, 2026

View reviewed changes

Comment thread apps/web/src/components/layout/middle-content/page-views/machine/workspace/TerminalPanes.tsx

Comment thread apps/web/src/hooks/useAgentTerminals.ts Outdated

Comment thread apps/web/src/hooks/useMachineWorkspaceSync.ts

2witstudios merged commit 624f3ca into master Jul 17, 2026
3 checks passed

2witstudios deleted the pu/always-a-terminal branch July 17, 2026 02:44

This was referenced Jul 22, 2026

Node Sandboxes: node-uniform sandboxes, downward cascade, session-family orchestration tools, lazy project-Sprite promotion #2204

Merged

Entity promotion: machine workspaces/panes as relational entities with verb APIs #2229

Merged

Navigation Menu

Repository navigation

fix(machines): a view of terminals you can actually remove - #2091

fix(machines): a view of terminals you can actually remove#2091
2witstudios merged 5 commits into
masterfrom
pu/always-a-terminal

2witstudios commented Jul 16, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jul 16, 2026

Uh oh!

2witstudios Jul 16, 2026

Uh oh!

2witstudios Jul 17, 2026

Uh oh!

chatgpt-codex-connector Bot Jul 16, 2026

Uh oh!

2witstudios Jul 16, 2026

Uh oh!

2witstudios Jul 16, 2026

Uh oh!

coderabbitai Bot commented Jul 16, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

2witstudios commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The bug

The fix

Five traps this had to step around

Considered and rejected

Deliberately not fixed

Test plan

Behaviour change worth flagging

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

2witstudios Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

2witstudios Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

2witstudios Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

2witstudios Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

2witstudios commented Jul 16, 2026 •

edited

Loading

coderabbitai Bot commented Jul 16, 2026 •

edited

Loading