Repository navigation
fix(desktop,host-service): make workspace-close terminal disposal reliable - #5752
Merged
Merged
Conversation
|
Ready to review this PR? Stage has broken it down into 6 individual chapters for you: 
Chapters generated by Stage for commit 1061f5a on Jul 18, 2026 8:56pm UTC. |
Contributor
📝 WalkthroughWalkthroughDesktop workspace and worktree disposal now returns aggregated host outcomes, displays failures with retry actions, and awaits disposal calls. Host services persist disposal intent in ChangesDisposal reliability
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant WorkspaceAction
participant HostService
participant TerminalDatabase
participant TerminalReaper
WorkspaceAction->>HostService: request session disposal
HostService->>TerminalDatabase: stamp disposeRequestedAt
HostService-->>WorkspaceAction: return disposal outcome
TerminalReaper->>TerminalDatabase: load requested sessions
TerminalReaper->>HostService: reap stamped session
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
Greptile SummaryThis PR closes the two main terminal-session leak vectors on workspace close/delete: the renderer previously fire-and-forgot disposal broadcasts (swallowing all errors), and a failed kill left an
Confidence Score: 4/5Safe to merge; the core durability logic (stamp + reaper retry + renderer await + toast) is sound, and the test coverage for shouldReapRow and the integration test are solid. The one finding is in toastDisposeFailures: when a multi-host run produces both unreachableHosts > 0 and failed > 0, the early return shows only the unreachable message and drops context about stamped-but-failed sessions that the reaper is already retrying. This is a UX information-loss issue in a multi-org scenario rather than a data-loss or crash path, but the toast copy becomes misleading in that edge case. apps/desktop/src/renderer/lib/dispose-host-sessions.ts — the combined-error branch in toastDisposeFailures.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/lib/dispose-host-sessions.ts | Refactored from fire-and-forget to awaited disposal with structured result and retry toast; minor UX issue when both unreachableHosts and failed are non-zero simultaneously. |
| apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts | Awaits dispose result and surfaces failures via toastDisposeFailures; retryDispose closure correctly captures variables.id. |
| apps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.ts | Mirrors useCloseWorkspace change; awaits dispose and surfaces failures. Correct. |
| apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/DeleteWorktreeDialog.tsx | Uses void+.then() pattern (fire-and-toast) for the dispose rather than await; intentional since onSuccess isn't awaiting dispose-completion before dialog teardown, and no subsequent ops depend on it. |
| packages/host-service/src/terminal/reaper/reaper.ts | Extracts shouldReapRow predicate, adds disposeRequestedAt to TerminalRow, widens reap condition to retry stamped rows; logic is correct and the predicate is unit-tested. |
| packages/host-service/src/terminal/terminal.ts | disposeSessionAndWait stamps disposeRequestedAt on entry using an isNull guard (first-request-wins); synchronous SQLite .run() call is appropriate for better-sqlite3. |
| packages/host-service/drizzle/0010_reflective_cerebro.sql | Single nullable integer column added with ALTER TABLE; backward-compatible, no data migration needed. |
| packages/host-service/src/terminal/reaper/reaper.test.ts | New shouldReapRow unit tests cover stamped-active, live-unstamped, dead-status, and workspace-less cases correctly. |
| packages/host-service/test/integration/terminal.integration.test.ts | Timeout bumped from 5s to 20s for the two-kill-sequence test; justified by comment explaining the ~1s SIGKILL escalation latency per sequence. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Workspace close / delete / worktree delete] --> B[disposeHostSessionsForWorkspace\nor ForWorktreePath]
B --> C[localHostClients: one per org]
C --> D{For each host}
D -- success --> E[terminated++]
D -- kill failed --> F[failed++\nstamped disposeRequestedAt]
D -- host unreachable --> G[unreachableHosts++]
E & F & G --> H[DisposeHostSessionsResult]
H --> I{toastDisposeFailures}
I -- all zero --> J[no-op]
I -- unreachableHosts > 0 --> K[toast: processes may keep running\nRetry action]
I -- failed > 0 only --> L[toast: N sessions failed\nhost will keep retrying]
K & L --> M{User clicks Retry?}
M -- yes --> B
F --> N[Reaper next pass\nup to 5 min]
N --> O{shouldReapRow?\ndisposeRequestedAt != null}
O -- yes --> P[disposeSessionAndWait again]
P -- success --> Q[status = disposed]
P -- fail --> N
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[Workspace close / delete / worktree delete] --> B[disposeHostSessionsForWorkspace\nor ForWorktreePath]
B --> C[localHostClients: one per org]
C --> D{For each host}
D -- success --> E[terminated++]
D -- kill failed --> F[failed++\nstamped disposeRequestedAt]
D -- host unreachable --> G[unreachableHosts++]
E & F & G --> H[DisposeHostSessionsResult]
H --> I{toastDisposeFailures}
I -- all zero --> J[no-op]
I -- unreachableHosts > 0 --> K[toast: processes may keep running\nRetry action]
I -- failed > 0 only --> L[toast: N sessions failed\nhost will keep retrying]
K & L --> M{User clicks Retry?}
M -- yes --> B
F --> N[Reaper next pass\nup to 5 min]
N --> O{shouldReapRow?\ndisposeRequestedAt != null}
O -- yes --> P[disposeSessionAndWait again]
P -- success --> Q[status = disposed]
P -- fail --> N
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
apps/desktop/src/renderer/lib/dispose-host-sessions.ts:119-132
When both `unreachableHosts > 0` and `failed > 0` are true simultaneously (e.g., two orgs running host-services, one unreachable and one with failed kills), the early `return` after the unreachable toast discards the `failed` count entirely. The user sees "processes may keep running" for everything, missing the fact that the stamped-but-failed sessions on the reachable host are already queued for reaper retry. At minimum, the description could mention both conditions, or the `failed` branch should be allowed to fire as a second toast.
```suggestion
if (result.unreachableHosts > 0) {
const description =
result.failed > 0
? "Its terminal processes may keep running. The reachable host will keep retrying failed sessions."
: "Its terminal processes may keep running.";
toast.error("Couldn't reach the host to close terminal sessions", {
description,
action: retryAction,
});
return;
}
toast.error(
`Failed to close ${result.failed} terminal session${result.failed === 1 ? "" : "s"}`,
{
description: "The host will keep retrying in the background.",
action: retryAction,
},
);
```
Reviews (1): Last reviewed commit: "fix(desktop,host-service): make workspac..." | Re-trigger Greptile
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+119
to
+132
| if (result.unreachableHosts > 0) { | ||
| toast.error("Couldn't reach the host to close terminal sessions", { | ||
| description: "Its terminal processes may keep running.", | ||
| action: retryAction, | ||
| }); | ||
| return; | ||
| } | ||
| toast.error( | ||
| `Failed to close ${result.failed} terminal session${result.failed === 1 ? "" : "s"}`, | ||
| { | ||
| description: "The host will keep retrying in the background.", | ||
| action: retryAction, | ||
| }, | ||
| ); |
Contributor
There was a problem hiding this comment.
unreachableHosts > 0 and failed > 0 are true simultaneously (e.g., two orgs running host-services, one unreachable and one with failed kills), the early return after the unreachable toast discards the failed count entirely. The user sees "processes may keep running" for everything, missing the fact that the stamped-but-failed sessions on the reachable host are already queued for reaper retry. At minimum, the description could mention both conditions, or the failed branch should be allowed to fire as a second toast.
Suggested change
| if (result.unreachableHosts > 0) { | |
| toast.error("Couldn't reach the host to close terminal sessions", { | |
| description: "Its terminal processes may keep running.", | |
| action: retryAction, | |
| }); | |
| return; | |
| } | |
| toast.error( | |
| `Failed to close ${result.failed} terminal session${result.failed === 1 ? "" : "s"}`, | |
| { | |
| description: "The host will keep retrying in the background.", | |
| action: retryAction, | |
| }, | |
| ); | |
| if (result.unreachableHosts > 0) { | |
| const description = | |
| result.failed > 0 | |
| ? "Its terminal processes may keep running. The reachable host will keep retrying failed sessions." | |
| : "Its terminal processes may keep running."; | |
| toast.error("Couldn't reach the host to close terminal sessions", { | |
| description, | |
| action: retryAction, | |
| }); | |
| return; | |
| } | |
| toast.error( | |
| `Failed to close ${result.failed} terminal session${result.failed === 1 ? "" : "s"}`, | |
| { | |
| description: "The host will keep retrying in the background.", | |
| action: retryAction, | |
| }, | |
| ); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/lib/dispose-host-sessions.ts
Line: 119-132
Comment:
When both `unreachableHosts > 0` and `failed > 0` are true simultaneously (e.g., two orgs running host-services, one unreachable and one with failed kills), the early `return` after the unreachable toast discards the `failed` count entirely. The user sees "processes may keep running" for everything, missing the fact that the stamped-but-failed sessions on the reachable host are already queued for reaper retry. At minimum, the description could mention both conditions, or the `failed` branch should be allowed to fire as a second toast.
```suggestion
if (result.unreachableHosts > 0) {
const description =
result.failed > 0
? "Its terminal processes may keep running. The reachable host will keep retrying failed sessions."
: "Its terminal processes may keep running.";
toast.error("Couldn't reach the host to close terminal sessions", {
description,
action: retryAction,
});
return;
}
toast.error(
`Failed to close ${result.failed} terminal session${result.failed === 1 ? "" : "s"}`,
{
description: "The host will keep retrying in the background.",
action: retryAction,
},
);
```
How can I resolve this? If you propose a fix, please make it concise.![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 13 files
You’re at about 94% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Architecture diagram
sequenceDiagram
participant Renderer as Renderer (React)
participant Dispose as dispose-host-sessions
participant WorkspaceAPI as Workspace API (tRPC)
participant HostService as Host Service (per org)
participant DB as SQLite (terminal_sessions)
participant Reaper as Background Reaper
participant Daemon as PTY Daemon
Note over Renderer,Daemon: Workspace Close/Delete Flow
Renderer->>WorkspaceAPI: closeWorkspace(id) or deleteWorkspace(id)
WorkspaceAPI-->>Renderer: success
Renderer->>Dispose: disposeHostSessionsForWorkspace(id)
Dispose->>Dispose: getLocalHostClients()
loop For each host service
Dispose->>HostService: disposeWorkspaceSessions({ workspaceId })
alt Host reachable
HostService->>DB: UPDATE terminal_sessions SET dispose_requestedAt = now
HostService->>Daemon: kill sessions (SIGTERM → SIGKILL)
alt Kill succeeds
HostService->>DB: DELETE or mark sessions disposed
HostService-->>Dispose: { terminated: N, failed: 0 }
else Kill fails (daemon hiccup)
HostService-->>Dispose: { terminated: 0, failed: N }
Note over DB,Reaper: Row remains active with dispose_requestedAt stamp
end
else Host unreachable
HostService-->>Dispose: null
Note over Dispose: No stamp written — sessions unreachable
end
end
Dispose-->>Renderer: { terminated, failed, unreachableHosts }
alt Any failures
Renderer->>Renderer: toastDisposeFailures()
Renderer->>Renderer: Show toast with Retry button
opt User clicks Retry
Renderer->>Dispose: disposeHostSessionsForWorkspace(id)
end
else All clean
Note over Renderer: No toast
end
Note over DB,Reaper: Background Recovery (independent loop)
loop Every REAP_INTERVAL_MS
Reaper->>DB: SELECT sessions with shouldReapRow()
alt disposeRequestedAt != null (stamped, active)
Reaper->>Daemon: kill session
Reaper->>DB: DELETE or mark disposed
else Dead status or no workspace
Reaper->>Daemon: cleanup
else Live, unstamped, has workspace
Note over Reaper: Skip — still active
end
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/host-service/test/integration/terminal.integration.test.ts`:
- Around line 323-324: Restore SUPERSET_PTY_DAEMON_SOCKET and SUPERSET_HOME_DIR
to their original values during the integration test cleanup, rather than
leaving the temporary socket and home directory assignments in place. Update the
setup/teardown flow around these environment variables so subsequent tests
cannot inherit stale paths, while preserving the existing cleanup behavior.
🪄 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: dffba2a3-9e3f-419d-a465-07da6dd9813d
📒 Files selected for processing (1)
packages/host-service/test/integration/terminal.integration.test.ts
Comment on lines
+323
to
+324
| process.env.SUPERSET_PTY_DAEMON_SOCKET = socketPath; | ||
| process.env.SUPERSET_HOME_DIR = tmp; |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Restore the process environment during cleanup.
Lines 323-324 leave SUPERSET_PTY_DAEMON_SOCKET and SUPERSET_HOME_DIR pointing at a directory removed on Line 385. Subsequent tests can therefore connect through stale paths and become order-dependent.
Proposed fix
+ const previousSocket = process.env.SUPERSET_PTY_DAEMON_SOCKET;
+ const previousHome = process.env.SUPERSET_HOME_DIR;
try {
// ...
} finally {
stopReaper?.();
await scenario.host.trpc.terminal.killSession
.mutate({ workspaceId: scenario.workspaceId, terminalId: sparedId })
.catch(() => {});
await disposeDaemonClient();
await stopDaemonProcess(daemonProcess);
rmSync(tmp, { recursive: true, force: true });
+ if (previousSocket === undefined) {
+ delete process.env.SUPERSET_PTY_DAEMON_SOCKET;
+ } else {
+ process.env.SUPERSET_PTY_DAEMON_SOCKET = previousSocket;
+ }
+ if (previousHome === undefined) {
+ delete process.env.SUPERSET_HOME_DIR;
+ } else {
+ process.env.SUPERSET_HOME_DIR = previousHome;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| process.env.SUPERSET_PTY_DAEMON_SOCKET = socketPath; | |
| process.env.SUPERSET_HOME_DIR = tmp; | |
| const previousSocket = process.env.SUPERSET_PTY_DAEMON_SOCKET; | |
| const previousHome = process.env.SUPERSET_HOME_DIR; | |
| try { | |
| process.env.SUPERSET_PTY_DAEMON_SOCKET = socketPath; | |
| process.env.SUPERSET_HOME_DIR = tmp; | |
| // ... | |
| } finally { | |
| stopReaper?.(); | |
| await scenario.host.trpc.terminal.killSession | |
| .mutate({ workspaceId: scenario.workspaceId, terminalId: sparedId }) | |
| .catch(() => {}); | |
| await disposeDaemonClient(); | |
| await stopDaemonProcess(daemonProcess); | |
| rmSync(tmp, { recursive: true, force: true }); | |
| if (previousSocket === undefined) { | |
| delete process.env.SUPERSET_PTY_DAEMON_SOCKET; | |
| } else { | |
| process.env.SUPERSET_PTY_DAEMON_SOCKET = previousSocket; | |
| } | |
| if (previousHome === undefined) { | |
| delete process.env.SUPERSET_HOME_DIR; | |
| } else { | |
| process.env.SUPERSET_HOME_DIR = previousHome; | |
| } | |
| } |
🤖 Prompt for 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.
In `@packages/host-service/test/integration/terminal.integration.test.ts` around
lines 323 - 324, Restore SUPERSET_PTY_DAEMON_SOCKET and SUPERSET_HOME_DIR to
their original values during the integration test cleanup, rather than leaving
the temporary socket and home directory assignments in place. Update the
setup/teardown flow around these environment variables so subsequent tests
cannot inherit stale paths, while preserving the existing cleanup behavior.
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
You’re at about 95% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/host-service/test/integration/terminal.integration.test.ts">
<violation number="1" location="packages/host-service/test/integration/terminal.integration.test.ts:379">
P3: The test cleans up `sparedId` via killSession but never cleans up the DB-stamped `stampedId`. After the reaper kills the daemon session, the row still exists in the DB (stamped, possibly still `active` if reaper's `disposeSessionAndWait` succeeded). Since the test uses `scenario.host.db` which is an in-memory test DB cleaned up via `host.dispose()`, this is safe in practice — but the asymmetry with `sparedId` (which gets an explicit killSession) is inconsistent. Consider deleting the stamped row after the reaper assertion or adding a brief comment explaining why it's unnecessary.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ); | ||
| expect((await aliveIds()).has(sparedId)).toBe(true); | ||
| } finally { | ||
| stopReaper?.(); |
Contributor
There was a problem hiding this comment.
P3: The test cleans up sparedId via killSession but never cleans up the DB-stamped stampedId. After the reaper kills the daemon session, the row still exists in the DB (stamped, possibly still active if reaper's disposeSessionAndWait succeeded). Since the test uses scenario.host.db which is an in-memory test DB cleaned up via host.dispose(), this is safe in practice — but the asymmetry with sparedId (which gets an explicit killSession) is inconsistent. Consider deleting the stamped row after the reaper assertion or adding a brief comment explaining why it's unnecessary.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/test/integration/terminal.integration.test.ts, line 379:
<comment>The test cleans up `sparedId` via killSession but never cleans up the DB-stamped `stampedId`. After the reaper kills the daemon session, the row still exists in the DB (stamped, possibly still `active` if reaper's `disposeSessionAndWait` succeeded). Since the test uses `scenario.host.db` which is an in-memory test DB cleaned up via `host.dispose()`, this is safe in practice — but the asymmetry with `sparedId` (which gets an explicit killSession) is inconsistent. Consider deleting the stamped row after the reaper assertion or adding a brief comment explaining why it's unnecessary.</comment>
<file context>
@@ -296,6 +299,93 @@ describe("terminal router integration", () => {
+ );
+ expect((await aliveIds()).has(sparedId)).toBe(true);
+ } finally {
+ stopReaper?.();
+ await scenario.host.trpc.terminal.killSession
+ .mutate({ workspaceId: scenario.workspaceId, terminalId: sparedId })
</file context>
Member
Author
|
Addressed the substantive review findings in 1061f5a:
Not changed: test env-var restore (coderabbit) — Re-verified: host-service suite 902/902; typecheck + lint clean; CDP re-verification of the two new/changed toast branches against the running app — coordinator-unavailable and combined unreachable+failed both render correct copy with a working Retry, success still adds no toast. |
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR 2 of the PTY-accumulation cleanup (
plans/pty-lifecycle-cleanup.md; PR 1 was #5748). Workspace close/delete disposal was fire-and-forget: the renderer broadcast swallowed every error, so a transient host-service failure silently leaked all of a workspace's sessions — and a failed kill left anactiverow the reaper's predicate never matched while the workspace row existed, making the session immortal.Host-service — durable intent-to-kill:
terminal_sessions.dispose_requested_atcolumn (migration 0010, single ALTER).disposeSessionAndWaitstamps it on entry (first request time wins).shouldReapRowand widened: any stamped row is retried regardless of workspace liveness. Success still deletes the row / marks it disposed, so retries converge; the reaper only acts on daemon-live sessions, so there's no perpetual no-op loop.Renderer — await and surface:
disposeHostSessionsForWorkspace/ForWorktreePathnow await every host and return{terminated, failed, unreachableHosts}instead of swallowing results.useCloseWorkspace,useDeleteWorkspace,DeleteWorktreeDialog) await the result; failures surface as a toast with a Retry action. Copy reflects reality: a host-confirmed failure says the host keeps retrying (it's stamped); an unreachable host warns processes may keep running (nothing was stamped — the renderer Retry is the only recovery there).Also: bumped the marginal 5s budget on the two-kill-sequence daemon integration test (flaky on main under load, each sequence blocks on the ~1s SIGKILL escalation).
The remaining uncovered class — renderer crashes before the broadcast ever fires — is the liveness-reaping backstop deliberately deferred in the plan.
Test Plan
shouldReapRowunit tests: stamped-active rows reaped; live unstamped sessions kept; dead-status/workspace-less rows still reapedSummary by cubic
Makes terminal disposal reliable when closing or deleting a workspace. Prevents leaked sessions and shows a single retryable error when a kill fails, a host is unreachable, or the coordinator is down.
Bug Fixes
host-service: Added durable intent-to-kill stampdispose_requested_atand widened the reaper viashouldReapRowto retry stamped sessions regardless of workspace liveness.Migration
terminal_sessions.dispose_requested_at(migration0010_reflective_cerebro).Written for commit 1061f5a. Summary will update on new commits.
Summary by CodeRabbit