Implement agent session resume and relaunch recovery#20
Merged
Conversation
Capture and persist Claude and Codex session identities so Shellraiser can relaunch active agent sessions instead of falling back to a plain shell. - store resumable session metadata in surface state - resume only active sessions on relaunch - fall back to a normal shell when Claude resume artifacts are missing - preserve resume eligibility during app quit/terminal teardown - fix Codex session discovery when rollout timestamps include fractions - add coverage for resume commands, persistence, quit handling, and wrappers
📝 WalkthroughWalkthroughAdds session lifecycle and resume support for Claude/Codex agents: new agent event phases, per-surface session fields, resume eligibility, workspace termination preparation, centralized child-exit handling, and runtime/launcher changes to resume agent sessions. Changes
Sequence DiagramsequenceDiagram
participant Agent as Agent (Claude/Codex)
participant Bridge as AgentRuntimeBridge
participant Manager as WorkspaceManager
participant Surface as SurfaceModel
participant Runtime as GhosttyRuntime
Agent->>Bridge: emit activity (hook-session / session / exited)
Bridge->>Bridge: parse payload (session_id, transcript_path) / map phases
Bridge->>Manager: handleAgentActivityEvent(phase:.session/ .exited, payload)
Manager->>Surface: setSessionIdentity(sessionId, transcriptPath)
Manager->>Surface: setResumeEligibility(shouldResumeSession)
Note over Manager,Surface: state persisted only on change
Runtime->>Runtime: launchCommand(for: surfaceModel)
Runtime->>Surface: check shouldResumeSession & transcriptPath
alt resumable executable available
Runtime->>Agent: start agent with resume command/args
else fallback
Runtime->>Agent: start shell launch (/bin/zsh)
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
Sources/Shellraiser/Models/SurfaceModel.swift (1)
41-42: Consider providing a default forshouldResumeSession.
transcriptPathhas a default (""), butshouldResumeSessiondoes not. For consistency and to reduce call-site verbosity, consider defaulting tofalse:🧹 Proposed fix
transcriptPath: String = "", - shouldResumeSession: Bool, + shouldResumeSession: Bool = false,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Shellraiser/Models/SurfaceModel.swift` around lines 41 - 42, The initializer parameter shouldResumeSession currently has no default while transcriptPath does; update the SurfaceModel initializer (or the SurfaceModel init signature) to give shouldResumeSession a default value (false) so callers can omit it and maintain consistency with transcriptPath; locate the init or struct declaration that contains the parameters transcriptPath: String = "" and shouldResumeSession: Bool and change shouldResumeSession to have a default of false.Sources/Shellraiser/Services/Workspaces/WorkspaceManager+WorkspaceLifecycle.swift (1)
1-1: Unused import.The
AppKitimport appears unnecessary for this extension—onlyFoundationtypes are used here.🧹 Proposed fix
-import AppKit import Foundation🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Shellraiser/Services/Workspaces/WorkspaceManager`+WorkspaceLifecycle.swift at line 1, Remove the unnecessary AppKit import at the top of WorkspaceManager+WorkspaceLifecycle.swift; this file/extension only uses Foundation types, so delete the "import AppKit" line (keep or add "import Foundation" only if the file needs it) to eliminate the unused dependency in the WorkspaceManager workspace lifecycle extension.Tests/ShellraiserTests/GhosttyRuntimeCommandTests.swift (1)
102-106: Tighten the Claude-resume assertion to avoid false positives.Consider also asserting that the command does not include
/bin/zsh, so the test guarantees resume path selection (not mixed/fallback invocation).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Tests/ShellraiserTests/GhosttyRuntimeCommandTests.swift` around lines 102 - 106, The test currently only checks for "claude" and "--resume" separately which can yield false positives; update the GhosttyRuntime.launchCommand(for: surface, terminalConfig: surface.terminalConfig) assertions to verify that the "--resume" flag is associated with the Claude invocation (e.g., assert the command string contains the substring pattern "claude" followed by "--resume" or use a regex to ensure order), and add an assertion that the command does not contain "/bin/zsh" (XCTAssertFalse(command.contains("/bin/zsh"))) to guarantee the resume path was selected rather than a zsh fallback; keep references to GhosttyRuntime.launchCommand, surface, and terminalConfig when locating the test.Tests/ShellraiserTests/ShellraiserAppTests.swift (1)
11-12: Isolate shared scripting-controller state between tests.Both tests mutate
ShellraiserScriptingController.sharedand leave it installed. Adding teardown/reset logic would prevent cross-test coupling and improve suite reliability.Also applies to: 24-25
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Tests/ShellraiserTests/ShellraiserAppTests.swift` around lines 11 - 12, Tests are leaving global ShellraiserScriptingController.shared installed which causes cross-test coupling; update the test class (ShellraiserAppTests) to isolate state by adding teardown/reset logic that uninstalls or resets ShellraiserScriptingController.shared after each test (e.g., implement tearDown() to call the controller's uninstall/reset method or reassign a fresh instance) and/or ensure setUp() initializes a clean shared controller before calls to ShellraiserScriptingController.shared.install(workspaceManager:) used alongside ShellraiserAppDelegate so the shared singleton is not mutated across tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/Shellraiser/Infrastructure/Agents/AgentRuntimeBridge.swift`:
- Around line 451-475: The current poller picks the last rollout-*.jsonl file by
pathname and only checks cwd and a second-precision timestamp, which can cause
one surface to pick up another surface's session_id; update the logic (around
session_file, first_line, extract_codex_session_timestamp,
timestamp_is_at_or_after, and the block that extracts session_id and calls
"$helper_path" codex "$surface_id" session "$session_id") to also verify the
rollout record belongs to the current surface before accepting it—for example,
parse and compare the surface identifier (or another unique surface-scoped
field) from first_line against the local surface_id (and/or use a
higher-precision timestamp if available); only when both cwd, surface_id (and
timestamp) match should you extract session_id and resume the session.
- Around line 487-503: The background monitor started by monitor_codex_session
(spawned in the subshell that creates stamp_file) can outlive the real Codex
process and append a late session event; capture its PID right after starting it
(e.g., monitor_pid="$!"), then after the real Codex run returns and before
calling "$helper" codex "$surface" exited, remove the stamp_file to signal the
monitor to stop and wait for the monitor to exit (wait "$monitor_pid"),
optionally killing it if it doesn't terminate within a short timeout; reference
the stamp_file and monitor_codex_session invocation and use $! to obtain the
background PID and wait/kill to ensure the monitor has stopped before emitting
the exited event.
In `@Sources/Shellraiser/Services/Workspaces/WorkspaceManager.swift`:
- Around line 125-133: The willTerminateNotification handler creates an async
Task which can let the app exit before prepareForTermination() (and its save)
finishes; change the observer block to call prepareForTermination() directly on
the main thread (synchronously) instead of spawning Task—i.e., in the
NotificationCenter callback (where willTerminateObserver is set) invoke
self?.prepareForTermination() directly (WorkspaceManager is `@MainActor` and the
observer uses queue: .main), and remove the Task { `@MainActor` ... } wrapper to
ensure termination work completes before shutdown.
In
`@Sources/Shellraiser/Services/Workspaces/WorkspaceSurfaceManager`+SurfaceState.swift:
- Around line 16-21: When building resolvedTranscriptPath inside
WorkspaceSurfaceManager+SurfaceState (the block using
normalizedTranscriptPath(for:agentType, transcriptPath:)), ensure that when
transcriptPath is nil you explicitly clear any previously stored Claude
transcript for that surface instead of leaving the old value in place: set
resolvedTranscriptPath to nil and update the surface’s stored transcriptPath
(surface.transcriptPath) to nil whenever the incoming transcriptPath is nil or
when sessionId changes, mirroring the same fix for the second occurrence noted
(lines 37-43).
In `@Tests/ShellraiserTests/AgentRuntimeBridgeTests.swift`:
- Around line 50-61: These tests mutate files produced by
AgentRuntimeBridge.shared, which causes flakiness when tests run in parallel;
change the tests to use a test-local AgentRuntimeBridge rooted in a unique temp
directory instead of the shared singleton: add (or use) an initializer or
factory on AgentRuntimeBridge that accepts a root/bin directory (e.g.,
AgentRuntimeBridge(rootURL:)), construct a temp directory with
FileManager.uniqueTemporaryDirectory() inside each test, instantiate
AgentRuntimeBridge(rootURL: tempDir), call prepareRuntimeSupport() on that
instance and read the wrapper files from instance.binDirectory, and ensure the
temp directory is removed in test cleanup; apply the same change to the other
test block referenced (lines 77-85).
---
Nitpick comments:
In `@Sources/Shellraiser/Models/SurfaceModel.swift`:
- Around line 41-42: The initializer parameter shouldResumeSession currently has
no default while transcriptPath does; update the SurfaceModel initializer (or
the SurfaceModel init signature) to give shouldResumeSession a default value
(false) so callers can omit it and maintain consistency with transcriptPath;
locate the init or struct declaration that contains the parameters
transcriptPath: String = "" and shouldResumeSession: Bool and change
shouldResumeSession to have a default of false.
In
`@Sources/Shellraiser/Services/Workspaces/WorkspaceManager`+WorkspaceLifecycle.swift:
- Line 1: Remove the unnecessary AppKit import at the top of
WorkspaceManager+WorkspaceLifecycle.swift; this file/extension only uses
Foundation types, so delete the "import AppKit" line (keep or add "import
Foundation" only if the file needs it) to eliminate the unused dependency in the
WorkspaceManager workspace lifecycle extension.
In `@Tests/ShellraiserTests/GhosttyRuntimeCommandTests.swift`:
- Around line 102-106: The test currently only checks for "claude" and
"--resume" separately which can yield false positives; update the
GhosttyRuntime.launchCommand(for: surface, terminalConfig:
surface.terminalConfig) assertions to verify that the "--resume" flag is
associated with the Claude invocation (e.g., assert the command string contains
the substring pattern "claude" followed by "--resume" or use a regex to ensure
order), and add an assertion that the command does not contain "/bin/zsh"
(XCTAssertFalse(command.contains("/bin/zsh"))) to guarantee the resume path was
selected rather than a zsh fallback; keep references to
GhosttyRuntime.launchCommand, surface, and terminalConfig when locating the
test.
In `@Tests/ShellraiserTests/ShellraiserAppTests.swift`:
- Around line 11-12: Tests are leaving global
ShellraiserScriptingController.shared installed which causes cross-test
coupling; update the test class (ShellraiserAppTests) to isolate state by adding
teardown/reset logic that uninstalls or resets
ShellraiserScriptingController.shared after each test (e.g., implement
tearDown() to call the controller's uninstall/reset method or reassign a fresh
instance) and/or ensure setUp() initializes a clean shared controller before
calls to ShellraiserScriptingController.shared.install(workspaceManager:) used
alongside ShellraiserAppDelegate so the shared singleton is not mutated across
tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 66f0eb9a-fb55-49b1-bf4d-6d1401af5f81
📒 Files selected for processing (22)
Sources/Shellraiser/App/ShellraiserApp.swiftSources/Shellraiser/Features/WorkspaceDetail/PaneLeafView.swiftSources/Shellraiser/Infrastructure/Agents/AgentRuntimeBridge.swiftSources/Shellraiser/Infrastructure/Agents/CompletionModels.swiftSources/Shellraiser/Infrastructure/AppleScript/ShellraiserScriptingController.swiftSources/Shellraiser/Infrastructure/Ghostty/GhosttyRuntime.swiftSources/Shellraiser/Models/SurfaceModel.swiftSources/Shellraiser/Services/Workspaces/WorkspaceManager+Completions.swiftSources/Shellraiser/Services/Workspaces/WorkspaceManager+SurfaceOperations.swiftSources/Shellraiser/Services/Workspaces/WorkspaceManager+WorkspaceLifecycle.swiftSources/Shellraiser/Services/Workspaces/WorkspaceManager.swiftSources/Shellraiser/Services/Workspaces/WorkspaceSurfaceManager+SurfaceState.swiftTests/ShellraiserTests/AgentCompletionEventTests.swiftTests/ShellraiserTests/AgentRuntimeBridgeTests.swiftTests/ShellraiserTests/AgentTypeAndWorkspaceModelTests.swiftTests/ShellraiserTests/GhosttyRuntimeCommandTests.swiftTests/ShellraiserTests/PaneNodeModelOperationsTests.swiftTests/ShellraiserTests/ShellraiserAppTests.swiftTests/ShellraiserTests/SurfaceModelTests.swiftTests/ShellraiserTests/WorkspaceManagerCompletionTests.swiftTests/ShellraiserTests/WorkspaceSurfaceManagerSurfaceStateTests.swiftTests/ShellraiserTests/WorkspaceTestSupport.swift
Sources/Shellraiser/Infrastructure/Agents/AgentRuntimeBridge.swift
Outdated
Show resolved
Hide resolved
Comment on lines
+16
to
+21
| let resolvedTranscriptPath: String? | ||
| if let transcriptPath { | ||
| resolvedTranscriptPath = normalizedTranscriptPath(for: agentType, transcriptPath: transcriptPath) | ||
| } else { | ||
| resolvedTranscriptPath = nil | ||
| } |
There was a problem hiding this comment.
Clear the stored Claude transcript when a session update doesn't provide one.
If transcriptPath is nil, the previous Claude surface.transcriptPath survives even when sessionId changes. GhosttyRuntime uses that path as the resume gate, so a later claudeCode:session update without a transcript can still trigger a resume attempt instead of correctly falling back to a shell.
🛠️ Suggested fix
- let resolvedTranscriptPath: String?
- if let transcriptPath {
- resolvedTranscriptPath = normalizedTranscriptPath(for: agentType, transcriptPath: transcriptPath)
- } else {
- resolvedTranscriptPath = nil
- }
+ let resolvedTranscriptPath = normalizedTranscriptPath(
+ for: agentType,
+ transcriptPath: transcriptPath ?? ""
+ )
@@
switch agentType {
case .claudeCode:
- if let resolvedTranscriptPath,
- surface.transcriptPath != resolvedTranscriptPath {
+ if surface.transcriptPath != resolvedTranscriptPath {
surface.transcriptPath = resolvedTranscriptPath
didChange = true
}Also applies to: 37-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Sources/Shellraiser/Services/Workspaces/WorkspaceSurfaceManager`+SurfaceState.swift
around lines 16 - 21, When building resolvedTranscriptPath inside
WorkspaceSurfaceManager+SurfaceState (the block using
normalizedTranscriptPath(for:agentType, transcriptPath:)), ensure that when
transcriptPath is nil you explicitly clear any previously stored Claude
transcript for that surface instead of leaving the old value in place: set
resolvedTranscriptPath to nil and update the surface’s stored transcriptPath
(surface.transcriptPath) to nil whenever the incoming transcriptPath is nil or
when sessionId changes, mirroring the same fix for the second occurrence noted
(lines 37-43).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/Shellraiser/Infrastructure/Agents/AgentRuntimeBridge.swift`:
- Around line 202-214: The guard for hook-session is ineffective because payload
is constructed with printf '%s\n%s' (so it's never empty); update the logic that
handles the "hook-session" branch (the hook-session case that defines
hook_payload, compact_payload, session_id, transcript_path, and payload) to
either (a) build payload only when session_id (or transcript_path, as required)
is non-empty, or (b) keep payload as-is but change the later guard to check the
extracted session_id (and/or transcript_path) emptiness instead of payload;
ensure the runtime uses the session_id variable (not the newline-only payload)
to decide whether to exit for phase="session".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 65861813-e475-423f-a2d5-abb42833cb89
📒 Files selected for processing (10)
Sources/Shellraiser/Infrastructure/Agents/AgentRuntimeBridge.swiftSources/Shellraiser/Infrastructure/AppleScript/ShellraiserScriptingController.swiftSources/Shellraiser/Models/SurfaceModel.swiftSources/Shellraiser/Services/Workspaces/WorkspaceManager+WorkspaceLifecycle.swiftSources/Shellraiser/Services/Workspaces/WorkspaceManager.swiftSources/Shellraiser/Services/Workspaces/WorkspaceSurfaceManager+SurfaceState.swiftTests/ShellraiserTests/AgentRuntimeBridgeTests.swiftTests/ShellraiserTests/GhosttyRuntimeCommandTests.swiftTests/ShellraiserTests/ShellraiserAppTests.swiftTests/ShellraiserTests/WorkspaceSurfaceManagerSurfaceStateTests.swift
🚧 Files skipped from review as they are similar to previous changes (3)
- Sources/Shellraiser/Infrastructure/AppleScript/ShellraiserScriptingController.swift
- Sources/Shellraiser/Services/Workspaces/WorkspaceManager.swift
- Tests/ShellraiserTests/ShellraiserAppTests.swift
Comment on lines
+202
to
+214
| claudeCode:hook-session) | ||
| hook_payload="$(cat)" | ||
| compact_payload="$(printf '%s' "$hook_payload" | tr -d '\n')" | ||
| session_id="$(printf '%s' "$compact_payload" | sed -n 's/.*"session_id"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' | tr '[:upper:]' '[:lower:]' | sed -n '1p')" | ||
| transcript_path="$(printf '%s' "$compact_payload" | sed -n 's/.*"transcript_path"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' | sed -n '1p')" | ||
| payload="$(printf '%s\n%s' "$session_id" "$transcript_path")" | ||
| phase="session" | ||
| ;; | ||
| esac | ||
|
|
||
| if [ "$phase" = "session" ] && [ -z "$payload" ]; then | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
Session payload emptiness guard is ineffective for hook-session events.
On Line 207, payload is always at least a newline for hook-session (printf '%s\n%s' ...), so the Line 212 guard won’t suppress missing session_id cases. That can still emit a malformed session event.
💡 Suggested fix
- payload="$(printf '%s\n%s' "$session_id" "$transcript_path")"
- phase="session"
+ [ -n "$session_id" ] || exit 0
+ payload="$(printf '%s\n%s' "$session_id" "$transcript_path")"
+ phase="session"
;;
esac
- if [ "$phase" = "session" ] && [ -z "$payload" ]; then
+ if [ "$phase" = "session" ] && [ -z "$payload" ]; then
exit 0
fi📝 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
| claudeCode:hook-session) | |
| hook_payload="$(cat)" | |
| compact_payload="$(printf '%s' "$hook_payload" | tr -d '\n')" | |
| session_id="$(printf '%s' "$compact_payload" | sed -n 's/.*"session_id"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' | tr '[:upper:]' '[:lower:]' | sed -n '1p')" | |
| transcript_path="$(printf '%s' "$compact_payload" | sed -n 's/.*"transcript_path"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' | sed -n '1p')" | |
| payload="$(printf '%s\n%s' "$session_id" "$transcript_path")" | |
| phase="session" | |
| ;; | |
| esac | |
| if [ "$phase" = "session" ] && [ -z "$payload" ]; then | |
| exit 0 | |
| fi | |
| claudeCode:hook-session) | |
| hook_payload="$(cat)" | |
| compact_payload="$(printf '%s' "$hook_payload" | tr -d '\n')" | |
| session_id="$(printf '%s' "$compact_payload" | sed -n 's/.*"session_id"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' | tr '[:upper:]' '[:lower:]' | sed -n '1p')" | |
| transcript_path="$(printf '%s' "$compact_payload" | sed -n 's/.*"transcript_path"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' | sed -n '1p')" | |
| [ -n "$session_id" ] || exit 0 | |
| payload="$(printf '%s\n%s' "$session_id" "$transcript_path")" | |
| phase="session" | |
| ;; | |
| esac | |
| if [ "$phase" = "session" ] && [ -z "$payload" ]; then | |
| exit 0 | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Shellraiser/Infrastructure/Agents/AgentRuntimeBridge.swift` around
lines 202 - 214, The guard for hook-session is ineffective because payload is
constructed with printf '%s\n%s' (so it's never empty); update the logic that
handles the "hook-session" branch (the hook-session case that defines
hook_payload, compact_payload, session_id, transcript_path, and payload) to
either (a) build payload only when session_id (or transcript_path, as required)
is non-empty, or (b) keep payload as-is but change the later guard to check the
extracted session_id (and/or transcript_path) emptiness instead of payload;
ensure the runtime uses the session_id variable (not the newline-only payload)
to decide whether to exit for phase="session".
This was referenced Mar 14, 2026
Merged
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
1 participant
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.
Capture and persist Claude and Codex session identities so Shellraiser can relaunch active agent sessions instead of falling back to a plain shell.
Summary by CodeRabbit
New Features
Bug Fixes
Tests