Repository navigation
feat(desktop,host-service): resume killed agent sessions by id - #6253
Conversation
- per-agent resume args (claude --resume, codex resume, etc.) on configs, presets, and the Settings UI; agents.run accepts resumeSessionId - retain terminal agent bindings with endedAt/endReason instead of deleting so agentSessionId survives kills, daemon death, and reboots - codex wrapper reports SessionEnd on clean exit only (signal deaths stay resumable); claude already registers SessionEnd - terminalAgents.resumeCandidate query + pane banner to resume in place
CDP-verified fixes from end-to-end testing: - backfill resume args for pre-existing agent configs (0017 custom migration) - upgrade a 'death gasp' detach to terminal-exited: claude runs SessionEnd hooks on SIGHUP, so a daemon kill looked like a clean quit and blocked resume; a detach within 30s of terminal death is not a user quit - terminal-exited is sticky (a late goodbye never downgrades a candidate) - daemon-disconnect handler marks all live sessions' bindings ended so resume candidates surface even before panes reattach
…ions after main's 0015/0016)
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis change adds terminal-agent session resumption. It persists resume arguments and terminal end states, exposes resume APIs, updates CLI and agent clients, adds desktop resume controls, and emits clean-exit Codex notifications. ChangesAgent session resumption
Estimated code review effort: 5 (Critical) | ~90 minutes Sequence Diagram(s)sequenceDiagram
participant TerminalPane
participant ResumeBanner
participant HostService
participant TerminalAgentBinding
participant AgentProcess
TerminalPane->>ResumeBanner: display terminal
ResumeBanner->>HostService: request resumeCandidate
HostService->>TerminalAgentBinding: find ended binding
TerminalAgentBinding-->>HostService: return session metadata
ResumeBanner->>HostService: run agent with resumeSessionId
HostService->>AgentProcess: start resume command
AgentProcess-->>HostService: create replacement terminal
HostService->>TerminalAgentBinding: mark old binding ended
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
Greptile SummaryThe PR adds session-by-ID resume support across agent configuration, host-service persistence, CLI/MCP/SDK entry points, and the desktop terminal banner. It also retains ended terminal-agent bindings and classifies terminal death versus clean agent detachment.
Confidence Score: 4/5The PR should not merge until delayed lifecycle events can no longer erase a valid interrupted-session resume candidate. A late non-ending hook for an exited PTY reaches the binding upsert and clears the persisted end markers required by the new resume flow, preventing users from restoring the interrupted conversation. Files Needing Attention: packages/host-service/src/terminal-agents/persistence.ts, packages/host-service/src/terminal-agents/store.ts, packages/host-service/src/trpc/router/notifications/notifications.ts
|
| Filename | Overview |
|---|---|
| packages/host-service/src/terminal-agents/persistence.ts | Retains ended bindings and implements resume lookup, but a delayed non-ending hook can clear the terminal-exited marker. |
| packages/host-service/src/terminal-agents/store.ts | Maps lifecycle events to retained end states while routing all other events through the unconditional revival upsert. |
| packages/host-service/src/terminal/terminal.ts | Marks bindings across PTY exit, daemon disconnect, and cold-respawn paths. |
| packages/host-service/src/trpc/router/agents/agents.ts | Validates resume capability and builds configured session-resume commands. |
| packages/host-service/src/trpc/router/terminal-agents/terminal-agents.ts | Exposes resumable binding metadata and resolves the corresponding agent configuration. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalAgentResumeBanner/TerminalAgentResumeBanner.tsx | Presents the resume action, repoints the pane, and disposes the replaced terminal. |
| packages/host-service/drizzle/0017_nappy_satana.sql | Adds resume arguments and terminal-binding end metadata. |
| packages/host-service/drizzle/0018_backfill_agent_resume_args.sql | Backfills built-in resume arguments for existing untouched configurations. |
Sequence Diagram
sequenceDiagram
participant Agent
participant Hook as Lifecycle hook
participant Host as Host service
participant DB as Binding database
participant UI as Terminal pane
Agent-->>Host: PTY exits unexpectedly
Host->>DB: mark terminal-exited
UI->>Host: query resumeCandidate
Host-->>UI: session ID + agent config
UI->>Host: agents.run(resumeSessionId)
Host->>Agent: launch configured resume command
Host-->>UI: replacement terminal ID
Prompt To Fix All With AI
### Issue 1
packages/host-service/src/terminal-agents/persistence.ts:350-351
**Late hooks erase resume state**
When a delayed `Stop`, `Start`, or other non-ending hook arrives after PTY exit, `recordEvent` reaches this upsert and clears `endedAt` and `endReason`, causing the interrupted session to stop satisfying `findResumeCandidateBinding` and preventing the user from resuming the conversation.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "Merge origin/main into add-agent-resume ..." | Re-trigger Greptile
| endedAt: null, | ||
| endReason: null, |
There was a problem hiding this comment.
When a delayed Stop, Start, or other non-ending hook arrives after PTY exit, recordEvent reaches this upsert and clears endedAt and endReason, causing the interrupted session to stop satisfying findResumeCandidateBinding and preventing the user from resuming the conversation.
Knowledge Base Used: pty-daemon
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/terminal-agents/persistence.ts
Line: 350-351
Comment:
**Late hooks erase resume state**
When a delayed `Stop`, `Start`, or other non-ending hook arrives after PTY exit, `recordEvent` reaches this upsert and clears `endedAt` and `endReason`, causing the interrupted session to stop satisfying `findResumeCandidateBinding` and preventing the user from resuming the conversation.
**Knowledge Base Used:** [pty-daemon](https://app.greptile.com/personal-org-6810/-/custom-context/knowledge-base/superset-sh/superset/-/docs/pty-daemon.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Fixed in bb1eb82 — an ended row now revives only on an explicit Attached (SessionStart), a different agentSessionId, or an event past a 30s straggler window; late Stop/Start from the dead session are dropped. Covered by new store/persistence tests.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/host-service/src/terminal-agents/persistence.ts (1)
340-351: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftKeep terminal-exited state for delayed events from the same agent session.
TerminalAgentStore.recordEventcallsupsertfor every non-end event. A delayedStopevent for the old session can reach this conflict path aftermarkTerminalAgentBindingEndedstoresterminal-exited.Lines 350-351 then clear the end state without checking
agentSessionId.findResumeCandidateBindingwill no longer return the killed session.Only revive an ended binding after an explicit new-session transition, such as a distinct non-empty
agentSessionId. Preserve the end state for an absent or unchanged session ID. Add a regression test for a delayed non-end event after terminal exit.🤖 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/src/terminal-agents/persistence.ts` around lines 340 - 351, Update the onConflictDoUpdate logic in TerminalAgentStore.recordEvent so ended bindings are revived only when binding.agentSessionId is non-empty and differs from the stored session ID; preserve endedAt and endReason for absent or unchanged session IDs. Add a regression test covering a delayed non-end event after markTerminalAgentBindingEnded.apps/desktop/src/renderer/routes/_authenticated/settings/agents/components/V2AgentsSettings/components/AgentFormControls/AgentFormControls.tsx (1)
40-75: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftSplit the components into separate files.
AgentFormControls.tsxdeclares multiple React components. This change extendsAgentLaunchFieldsin that module. Move each component toComponentName/ComponentName.tsxand export it through anindex.tsbarrel.As per coding guidelines, “Use one component per file and organize components as
ComponentName/ComponentName.tsxwith anindex.tsbarrel export.”Also applies to: 113-132
🤖 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 `@apps/desktop/src/renderer/routes/_authenticated/settings/agents/components/V2AgentsSettings/components/AgentFormControls/AgentFormControls.tsx` around lines 40 - 75, Split each React component currently declared in AgentFormControls.tsx, including AgentLaunchFields, into its own ComponentName/ComponentName.tsx file. Add an index.ts barrel in each component directory, export the components through those barrels, and update imports to use the new module structure while preserving existing props and behavior.Source: Coding guidelines
🧹 Nitpick comments (2)
packages/shared/src/host-agent-presets.ts (1)
20-28: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winGuard the shared-prefix assumption in
deriveSuffixArgs.
deriveSuffixArgsassumesvariantCommandstarts with the same tokens ascommand. It does not check that assumption. If a definition supplies aresumeCommandthat uses a different base command, or a shorter one, the slice silently returns the wrong args or an empty list. The result is a broken resume command line at launch time, with no error at build or seed time.The built-in agents all satisfy the invariant today. Custom agent definitions can set
resumeCommandfreely, so a runtime guard adds value.♻️ Proposed guard
function deriveSuffixArgs( commandTokens: string[], variantCommand: string | undefined, ): string[] { if (!variantCommand) return []; - return tokenize(variantCommand).slice(commandTokens.length); + const variantTokens = tokenize(variantCommand); + const sharesPrefix = commandTokens.every( + (token, index) => variantTokens[index] === token, + ); + if (!sharesPrefix) { + throw new Error( + `Variant command "${variantCommand}" must start with base command "${commandTokens.join(" ")}"`, + ); + } + return variantTokens.slice(commandTokens.length); }If a throw is too strict for user-supplied definitions, log a warning and return the full
variantTokensslice from the first divergent index instead.🤖 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/shared/src/host-agent-presets.ts` around lines 20 - 28, Update deriveSuffixArgs to tokenize variantCommand and verify it has commandTokens as an identical shared prefix before slicing. When the prefix is missing, shorter, or divergent, guard the result by logging a warning and returning variant tokens from the first divergent index (or otherwise follow the project’s established non-throwing behavior); preserve the existing suffix-only result for valid prefixes.apps/desktop/src/renderer/hooks/host-service/useTerminalResumeCandidate/useTerminalResumeCandidate.ts (1)
33-49: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winInclude the host URL in the resume-candidate query key.
useTerminalResumeCandidatequeriesgetHostServiceClientByUrl(hostUrl).terminalAgents.resumeCandidate..., butgetTerminalResumeCandidateQueryKeyonly hashesworkspaceIdandterminalId. IfuseWorkspaceHostUrlchanges from local to relay URL for the same terminal, React Query can reuse the prior host’s cache entry for up tostaleTime. AddhostUrlto the query key, or invalidate/drop the old query when the host URL changes.🤖 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 `@apps/desktop/src/renderer/hooks/host-service/useTerminalResumeCandidate/useTerminalResumeCandidate.ts` around lines 33 - 49, Update the queryKey construction in useTerminalResumeCandidate to include hostUrl alongside workspaceId and terminalId, and update getTerminalResumeCandidateQueryKey accordingly. Ensure the memo dependencies include hostUrl so changing hosts produces a distinct React Query cache entry.
🤖 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/shared/src/builtin-terminal-agents.ts`:
- Line 66: Update the Codex resumeCommand configuration to place the resume
subcommand before the session-id placeholder, preserving any global flags before
resume so the resulting command follows the documented `codex --global-flags
resume <session-id>` structure.
---
Outside diff comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/agents/components/V2AgentsSettings/components/AgentFormControls/AgentFormControls.tsx`:
- Around line 40-75: Split each React component currently declared in
AgentFormControls.tsx, including AgentLaunchFields, into its own
ComponentName/ComponentName.tsx file. Add an index.ts barrel in each component
directory, export the components through those barrels, and update imports to
use the new module structure while preserving existing props and behavior.
In `@packages/host-service/src/terminal-agents/persistence.ts`:
- Around line 340-351: Update the onConflictDoUpdate logic in
TerminalAgentStore.recordEvent so ended bindings are revived only when
binding.agentSessionId is non-empty and differs from the stored session ID;
preserve endedAt and endReason for absent or unchanged session IDs. Add a
regression test covering a delayed non-end event after
markTerminalAgentBindingEnded.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/hooks/host-service/useTerminalResumeCandidate/useTerminalResumeCandidate.ts`:
- Around line 33-49: Update the queryKey construction in
useTerminalResumeCandidate to include hostUrl alongside workspaceId and
terminalId, and update getTerminalResumeCandidateQueryKey accordingly. Ensure
the memo dependencies include hostUrl so changing hosts produces a distinct
React Query cache entry.
In `@packages/shared/src/host-agent-presets.ts`:
- Around line 20-28: Update deriveSuffixArgs to tokenize variantCommand and
verify it has commandTokens as an identical shared prefix before slicing. When
the prefix is missing, shorter, or divergent, guard the result by logging a
warning and returning variant tokens from the first divergent index (or
otherwise follow the project’s established non-throwing behavior); preserve the
existing suffix-only result for valid prefixes.
🪄 Autofix
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 Plus
Run ID: 86853bba-432c-46b4-a9e6-0448957722da
📒 Files selected for processing (40)
apps/desktop/src/main/lib/agent-setup/agent-wrappers.test.tsapps/desktop/src/main/lib/agent-setup/templates/codex-wrapper-exec.template.shapps/desktop/src/renderer/hooks/host-service/useTerminalResumeCandidate/index.tsapps/desktop/src/renderer/hooks/host-service/useTerminalResumeCandidate/useTerminalResumeCandidate.tsapps/desktop/src/renderer/lib/preset-icon-key.test.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/TerminalPane.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalAgentResumeBanner/TerminalAgentResumeBanner.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalAgentResumeBanner/index.tsapps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useDefaultV2TerminalPresets/default-v2-terminal-presets.test.tsapps/desktop/src/renderer/routes/_authenticated/settings/agents/components/V2AgentsSettings/V2AgentsSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/agents/components/V2AgentsSettings/components/AgentDetail/AgentDetail.tsxapps/desktop/src/renderer/routes/_authenticated/settings/agents/components/V2AgentsSettings/components/AgentFormControls/AgentFormControls.tsxapps/desktop/src/renderer/routes/_authenticated/settings/agents/components/V2AgentsSettings/components/NewCustomAgentDetail/NewCustomAgentDetail.tsxapps/web/src/trpc/host-client.tspackages/cli/src/commands/agents/create/command.test.tspackages/cli/src/commands/agents/create/command.tspackages/host-service/drizzle/0017_nappy_satana.sqlpackages/host-service/drizzle/0018_backfill_agent_resume_args.sqlpackages/host-service/drizzle/meta/0017_snapshot.jsonpackages/host-service/drizzle/meta/0018_snapshot.jsonpackages/host-service/drizzle/meta/_journal.jsonpackages/host-service/src/app.tspackages/host-service/src/db/schema.tspackages/host-service/src/terminal-agents/persistence.test.tspackages/host-service/src/terminal-agents/persistence.tspackages/host-service/src/terminal-agents/store.test.tspackages/host-service/src/terminal-agents/store.tspackages/host-service/src/terminal-agents/types.tspackages/host-service/src/terminal/terminal.tspackages/host-service/src/trpc/router/agents/agents.test.tspackages/host-service/src/trpc/router/agents/agents.tspackages/host-service/src/trpc/router/settings/agent-configs.test.tspackages/host-service/src/trpc/router/settings/agent-configs.tspackages/host-service/src/trpc/router/terminal-agents/terminal-agents.tspackages/mcp/src/tools/agents/create.tspackages/mcp/src/tools/agents/list.tspackages/sdk/src/resources/agents.tspackages/shared/src/agent-definition.tspackages/shared/src/builtin-terminal-agents.tspackages/shared/src/host-agent-presets.ts
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
A delayed Stop/Start from a dead session (async notify callbacks, in-flight curls) reached the upsert path and cleared endedAt/endReason, destroying the resume candidate. An ended row now revives only on an explicit session start, a different agent session id, or an event past the 30s straggler window. Addresses greptile P1 and coderabbit Major on PR #6253.
There was a problem hiding this comment.
4 issues found and verified against the latest diff
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/cli/src/commands/agents/create/command.ts">
<violation number="1" location="packages/cli/src/commands/agents/create/command.ts:21">
P2: The new resume flow is not discoverable from the checked-in CLI reference: the documented command still says `--prompt` is required and omits `--resume-session`. Updating the reference and adding a resume example would prevent users from missing this public CLI capability.</violation>
</file>
<file name="packages/mcp/src/tools/agents/create.ts">
<violation number="1" location="packages/mcp/src/tools/agents/create.ts:33">
P1: MCP callers cannot use this new resume input with the ID returned by `agents_create`: that value is a terminal UUID, not the agent session ID required by `agents.run`, and no MCP tool exposes the latter. Expose the agent session/resume candidate through MCP or return it from create, then clarify this field so the normal create→resume flow can actually restore the conversation.</violation>
</file>
<file name="packages/sdk/src/resources/agents.ts">
<violation number="1" location="packages/sdk/src/resources/agents.ts:99">
P3: The published SDK reference omits `resumeSessionId` and still presents `prompt` as required, so users following `api.md` cannot discover or correctly document resume launches; update the generated API reference with `prompt?` and `resumeSessionId?`.</violation>
</file>
<file name="packages/host-service/src/terminal-agents/persistence.ts">
<violation number="1" location="packages/host-service/src/terminal-agents/persistence.ts:96">
P2: The death-gasp upgrade clashes with the codex wrapper change added in this same PR. `markTerminalAgentBindingEnded` treats any `terminal-exited` mark landing within 30s of a prior `detached` as a death gasp and re-stamps `endReason` to `terminal-exited`, making it a resume candidate. But the new codex wrapper emits `SessionEnd` (maps to `Detached` -> `detached`, explicitly "not resumable") on every *normal* exit, immediately before the pty dies. So a clean codex quit is recorded as `detached` then, milliseconds later, upgraded back to `terminal-exited` by the pty-exit handler — turning an intentional quit into a resume candidate. Because `terminal-exited` is sticky and the upgrade fires in either event ordering, a codex session the user deliberately closed will surface the resume banner, defeating the purpose of the newly added codex `SessionEnd` reporting. Consider narrowing the upgrade to only the daemon-disconnect/SIGHUP path (or skipping it when a clean `SessionEnd` was already recorded) so legitimate `detached` quits stay final.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| .describe( | ||
| "Prompt sent to the agent. Required unless resumeSessionId is provided.", | ||
| ), | ||
| resumeSessionId: z |
There was a problem hiding this comment.
P1: MCP callers cannot use this new resume input with the ID returned by agents_create: that value is a terminal UUID, not the agent session ID required by agents.run, and no MCP tool exposes the latter. Expose the agent session/resume candidate through MCP or return it from create, then clarify this field so the normal create→resume flow can actually restore the conversation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/mcp/src/tools/agents/create.ts, line 33:
<comment>MCP callers cannot use this new resume input with the ID returned by `agents_create`: that value is a terminal UUID, not the agent session ID required by `agents.run`, and no MCP tool exposes the latter. Expose the agent session/resume candidate through MCP or return it from create, then clarify this field so the normal create→resume flow can actually restore the conversation.</comment>
<file context>
@@ -24,7 +24,19 @@ export function register(server: McpServer): void {
+ .describe(
+ "Prompt sent to the agent. Required unless resumeSessionId is provided.",
+ ),
+ resumeSessionId: z
+ .string()
+ .min(1)
</file context>
There was a problem hiding this comment.
Clarified in 9fda4af: the description now states this is the agent CLI's own session id, not the terminal id this tool returns. Exposing resume candidates through an MCP tool (so automations can do create→kill→resume without desktop help) is a fair follow-up — the data is already queryable via terminalAgents.resumeCandidate.
| prompt: string().desc( | ||
| "Prompt sent to the agent (required unless --resume-session is set)", | ||
| ), | ||
| resumeSession: string().desc( |
There was a problem hiding this comment.
P2: The new resume flow is not discoverable from the checked-in CLI reference: the documented command still says --prompt is required and omits --resume-session. Updating the reference and adding a resume example would prevent users from missing this public CLI capability.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/commands/agents/create/command.ts, line 21:
<comment>The new resume flow is not discoverable from the checked-in CLI reference: the documented command still says `--prompt` is required and omits `--resume-session`. Updating the reference and adding a resume example would prevent users from missing this public CLI capability.</comment>
<file context>
@@ -15,7 +15,12 @@ export default command({
+ prompt: string().desc(
+ "Prompt sent to the agent (required unless --resume-session is set)",
+ ),
+ resumeSession: string().desc(
+ "Session id of a previous run of this agent to restore instead of starting fresh",
+ ),
</file context>
There was a problem hiding this comment.
Docs updated in 9fda4af: --prompt is now documented as required-unless-resuming, --resume-session is listed, and there's a resume example in the CLI reference.
| const isDeathGaspDetach = | ||
| reason === "terminal-exited" && | ||
| row.endReason === "detached" && | ||
| endedAt - row.endedAt <= DEATH_GASP_DETACH_WINDOW_MS; |
There was a problem hiding this comment.
P2: The death-gasp upgrade clashes with the codex wrapper change added in this same PR. markTerminalAgentBindingEnded treats any terminal-exited mark landing within 30s of a prior detached as a death gasp and re-stamps endReason to terminal-exited, making it a resume candidate. But the new codex wrapper emits SessionEnd (maps to Detached -> detached, explicitly "not resumable") on every normal exit, immediately before the pty dies. So a clean codex quit is recorded as detached then, milliseconds later, upgraded back to terminal-exited by the pty-exit handler — turning an intentional quit into a resume candidate. Because terminal-exited is sticky and the upgrade fires in either event ordering, a codex session the user deliberately closed will surface the resume banner, defeating the purpose of the newly added codex SessionEnd reporting. Consider narrowing the upgrade to only the daemon-disconnect/SIGHUP path (or skipping it when a clean SessionEnd was already recorded) so legitimate detached quits stay final.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/src/terminal-agents/persistence.ts, line 96:
<comment>The death-gasp upgrade clashes with the codex wrapper change added in this same PR. `markTerminalAgentBindingEnded` treats any `terminal-exited` mark landing within 30s of a prior `detached` as a death gasp and re-stamps `endReason` to `terminal-exited`, making it a resume candidate. But the new codex wrapper emits `SessionEnd` (maps to `Detached` -> `detached`, explicitly "not resumable") on every *normal* exit, immediately before the pty dies. So a clean codex quit is recorded as `detached` then, milliseconds later, upgraded back to `terminal-exited` by the pty-exit handler — turning an intentional quit into a resume candidate. Because `terminal-exited` is sticky and the upgrade fires in either event ordering, a codex session the user deliberately closed will surface the resume banner, defeating the purpose of the newly added codex `SessionEnd` reporting. Consider narrowing the upgrade to only the daemon-disconnect/SIGHUP path (or skipping it when a clean `SessionEnd` was already recorded) so legitimate `detached` quits stay final.</comment>
<file context>
@@ -40,9 +48,93 @@ function rowToBinding(row: BindingRow): TerminalAgentBinding {
+ const isDeathGaspDetach =
+ reason === "terminal-exited" &&
+ row.endReason === "detached" &&
+ endedAt - row.endedAt <= DEATH_GASP_DETACH_WINDOW_MS;
+ if (!isDeathGaspDetach) return undefined;
+ db.update(terminalAgentBindings)
</file context>
There was a problem hiding this comment.
Intentional trade-off: if the terminal dies within 30s of a clean codex quit, the worst case is a dismissible banner offering to resume a session the user had just ended — and resuming it works. The alternative (no upgrade) breaks the primary case, since claude/codex goodbyes on SIGHUP are indistinguishable from user quits at the data layer. Chose false-positive-dismissible over false-negative-lost-session.
| /** Prompt sent to the agent. Optional when `resumeSessionId` is provided. */ | ||
| prompt?: string; | ||
| /** Session id of a previous run of this agent to restore instead of starting fresh (e.g. `claude --resume <id>`). */ | ||
| resumeSessionId?: string; |
There was a problem hiding this comment.
P3: The published SDK reference omits resumeSessionId and still presents prompt as required, so users following api.md cannot discover or correctly document resume launches; update the generated API reference with prompt? and resumeSessionId?.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/sdk/src/resources/agents.ts, line 99:
<comment>The published SDK reference omits `resumeSessionId` and still presents `prompt` as required, so users following `api.md` cannot discover or correctly document resume launches; update the generated API reference with `prompt?` and `resumeSessionId?`.</comment>
<file context>
@@ -90,8 +93,10 @@ export interface AgentCreateParams {
+ /** Prompt sent to the agent. Optional when `resumeSessionId` is provided. */
+ prompt?: string;
+ /** Session id of a previous run of this agent to restore instead of starting fresh (e.g. `claude --resume <id>`). */
+ resumeSessionId?: string;
/** Reasoning effort for this launch. Supported values depend on the agent; omit to use its default. */
effort?: string;
</file context>
There was a problem hiding this comment.
Fixed in 9fda4af: api.md now shows prompt? and resumeSessionId?.
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/src/terminal-agents/store.test.ts`:
- Around line 487-510: Reset the in-memory store before each independent
straggler case, not just the persisted row. Recreate TerminalAgentStore with the
existing mock persistence before the different-session and expired-window
recordEvent calls, so both cases exercise the no-binding path and verify revival
of the ended row.
🪄 Autofix
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 Plus
Run ID: e23ef274-266e-4327-bb61-1d4126089fdc
📒 Files selected for processing (4)
packages/host-service/src/terminal-agents/persistence.test.tspackages/host-service/src/terminal-agents/persistence.tspackages/host-service/src/terminal-agents/store.test.tspackages/host-service/src/terminal-agents/store.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/host-service/src/terminal-agents/store.ts
- packages/host-service/src/terminal-agents/persistence.test.ts
- packages/host-service/src/terminal-agents/persistence.ts
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…dings ended Cubic review fixes on #6253: - daemon disconnect no longer blanket-marks bindings: the sweep waits for a daemon to answer and marks only sessions it no longer owns (upgrade handoffs and socket blips keep live agents live); no answer after retries means the ptys died with the daemon — mark all - straggler store test scenarios now use fresh stores so the guard is actually exercised - 0018 backfill also requires command = preset_id so repointed/custom rows reusing builtin tags don't get flags their CLI doesn't support - document --resume-session in the CLI reference and resumeSessionId in the SDK api.md; clarify the MCP field is the agent's own session id
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…t failed A failed capture stored undefined, which a replacement binding without a session id yet would match — skip the candidate instead of risking ending it.
…aseline Only candidates whose captured binding had a real agent session id are marked — bindings without one are never resume candidates, and an undefined baseline could match a replacement binding whose session id hadn't been reported yet. Snapshot failures now log terminal id + error.
Agent CLIs only persist a conversation once it has a message; a session killed before its first prompt resumes to 'No conversation found'. Exclude bindings whose lastEventType never progressed past Attached. Found in final CDP verification.
…tion (#6265) * feat(desktop,host-service): resume v1 pane agent sessions after migration Migrated v1 terminal panes came back as bare shells: only {paneId, cwd} crossed the v1->v2 boundary, so an agent running in a v1 pane was simply lost. Now the migrated pane surfaces the same resume banner and detection as a killed v2 session (#6253) and resumes via the same agents.run({resumeSessionId}) path. Three stages: - Capture: hook scripts' v1 dispatch now carries agentId and the un-collapsed rawEventType (the legacy param folds SessionStart/SessionEnd into Start/Stop, hiding clean quits). The Electron hook server records per-pane {agentId, agentSessionId, prompted, endedAt} into app-state.json with host-service detection semantics: never-prompted excluded, own SessionEnd = clean quit, 30s death-gasp upgrade on terminal exit, 30s straggler guard. Entries prune to live pane ids at boot. - Carry: pendingMigratedTerminals entries record their v1PaneId, and the capture is read at v2 pane-creation time (migration.readV1PaneAgentSessions) rather than frozen into the plan - the migration ledgers panes on every boot while the user is still on v1, and the agent keeps running and reporting long after that. - Seed: new terminalAgents.seedResumeCandidate mutation inserts an already-ended binding (terminal-exited, lastEventType Stop) for the recreated terminal after ownership checks, never overwriting a real binding. The existing resumeCandidate query and TerminalAgentResumeBanner take over unchanged. * fix(desktop): harden migrated-pane resume seeding against transient failures Review fixes on #6265 (coderabbit Major, cubic P1/P2): - a pending entry now leaves the queue only after its resume seeding resolves, and a failed capture read aborts the pass — a transient electron/host hiccup retries on the next workspace open instead of permanently dropping the resume candidate (creates are idempotent) - filter v1PaneId by type: healed pre-field queue rows surface it as undefined, which passed the !== null check and broke the batch read - ensureValidShape tolerates a legacy null panes map instead of throwing during initAppState - bump cursor/gemini/copilot hook markers (v6/v5/v4) for the new v1 dispatch params, matching the notify v7 bump * fix(desktop): never inherit a v1 capture session id across an agent change Mirrors the v2 store's identity guard: a codex event landing on an ended claude record (codex's v1 notify payload carries no session id) revived it as agentId codex with claude's session id — migration would then offer a codex resume pointing at a claude session. Agent/session changes now require the event's own session id to mint a record, and goodbyes from a different agent are ignored. Also skip persistence when nothing material changes: hook events stream steadily while an agent works, and each capture write rewrote the whole app-state.json just to bump the unused updatedAt. * feat(desktop): extract codex thread-id as the hook session id Codex's legacy notify callback (agent-turn-complete) carries the resumable id as kebab-case thread-id — the same id `codex resume` takes — but the notify hook only extracted session_id/resourceId, so id-less codex modes (exec launches, pre-hooks codex builds) produced bindings and v1 captures without a session id. Fall back to thread-id when nothing else matched; it fires on every completed turn, benefiting v2 bindings and the v1 migration capture alike. Payload shape captured empirically from codex-cli 0.146.
* docs: weekly update for PRs merged 2026-08-05 to 2026-08-12 - workspaces: sessions (project-less workspaces, #6274), pin + drag-and-drop pinning (#6364), list/board views with Merged & Deleted history columns (#6296), per-project naming instructions (#6250), multi-PR links per branch with remove/restore (#5924) - automations: session automations (#6280), list health stats + Up next grouping + read-only team automations (#6291, #6276), detail page Settings/Run History tabs, inline run errors, would-run preview, RRule validation (#6305) - editor: image pane pan/pinch/zoom (#6285) - tasks: multi-repo filters, expanded task states, PR author/review filters, PR checks (#6201) - monorepos: per-project sparse checkout (#6034) - customization: project icon + color picker, v1 color carry-over (#6249) - terminal presets: built-in Superset CLI preset (#6270) - agents: resume interrupted sessions + Resume Args setting (#6253, #6265), Superset hooks toggle + scoping (#6072) - cli: interactive command browser + branded help (#6273); reference updated for session mode, --task, --skip-branch-prefix, --session (verified against packages/cli/src/commands) * docs: tighten wording and drop em dashes Shorten this week's additions per the writing guide: one thought per sentence, plainest word, no em dashes as prose punctuation. Also sweeps the three pre-existing em dashes in recipes. * docs: add screenshots from the 08-09 changelog batch cli command browser, agent resume banner, PR view with checks, and the project icon/color picker. Copied into apps/docs/public/images per the SOP rather than referenced across apps.
…8691) A host-service that predates resume support (#6253) returns agent configs without resumeArgs. The desktop can adopt such a host, and AgentDetail then crashed on render. Treat a missing resumeArgs as empty, as forkArgs already is. Refs DESKTOP-QC Co-authored-by: superset-home <338338640+superset-home@users.noreply.github.com>
Summary
Killed agent sessions are now resumable. When an agent terminal dies without the agent's own goodbye — laptop reset, pty-daemon killed, crashed pty — the pane shows a " was interrupted [Resume]" banner that relaunches the agent with its resume command (e.g.
claude --resume <session-id>) in the same pane, restoring the full conversation.Resume command plumbing (the
promptArgspattern)resumeCommandon the builtin agent catalog, derived toresumeArgson presets and stored onhost_agent_configs(resume_args_json). All defaults verified against the installed CLIs:claude --resume,codex resume(root flags before subcommand confirmed),amp threads continue,opencode/pi/kimi --session,mastracode --thread,--resumefor gemini/copilot/vibe/grok/cursor-agent/droid.agents.runacceptsresumeSessionId(prompt optional when resuming); validated against the config's resume args. Exposed via CLI (superset agents create --resume-session), MCPagents_create, and the SDK.Retention: bindings survive death
terminal_agent_bindingsrows are retained withended_at/end_reasoninstead of hard-deleted, soagent_session_idsurvives:"detached"= the agent reported its own end (SessionEnd hook) → not resumable"terminal-exited"= the terminal died under it → resume candidateMarked at every death path: SessionEnd hooks,
killSession, pty exit, daemon disconnect (sweeps all live sessions), cold respawn, and a boot backfill. Live reads exclude ended rows; a new session in the same terminal revives the row.Death-gasp rule: claude runs SessionEnd hooks on SIGHUP, so a daemon kill initially looked like a clean quit and blocked resume. A detach recorded within 30s of the terminal's death now upgrades to a resume candidate, and
terminal-exitedis sticky against late goodbyes.Kill hooks
Pane banner
New
terminalAgents.resumeCandidatequery +TerminalAgentResumeBannerin the terminal pane. Resume re-points the same pane at the new terminal (setPaneData, the session-dropdown pattern) and silently disposes the dead session.Testing
Unit: ~1550 tests across host-service/shared/agent-setup/CLI, including resume arg building, retention truth table (death-gasp upgrade, sticky terminal-exited, upsert revival), config CRUD, and codex wrapper exit-reporting (clean exit reports, SIGHUP death doesn't).
CDP end-to-end in the real dev desktop app (real clicks, screenshots + DB checks at each step), re-run after merging main:
kill -9claude+shell): banner appears on dead pane → Resume → conversation restored (codeword check)./exitcontrol: binding markeddetached, no banner.Host-service migrations were regenerated to land after main's 0015/0016 (0017 schema + 0018 backfill).
Summary by cubic
Resume killed agent sessions by session id. The terminal shows a Resume banner and relaunches the agent in the same pane, with stricter daemon-loss handling to avoid false resumes.
New Features
claude --resume <id>,codex resume,amp threads continue).agents.runsupportsresumeSessionId; CLI adds--resume-sessionand makes--promptoptional when resuming; MCPagents_createand the SDK accept it. Docs clarify the id is the agent’s own.ended_at/end_reasonand exposesterminalAgents.resumeCandidateto power the banner; Codex wrapper reportsSessionEndonly on clean exit.Reliability
terminal-exited, stays sticky against late goodbyes, and blocks straggler hooks from clearing resume state.Written for commit d6e7af5. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes