Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 133 additions & 13 deletions Sources/RestorableAgentSession.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1118,12 +1118,19 @@ struct RestorableAgentSessionIndex: Sendable {
}

for record in state.sessions.values {
let normalizedSessionId = record.sessionId.trimmingCharacters(in: .whitespacesAndNewlines)
let effectiveRecord = kind == .claude
? resolvedClaudeWorkflowRecord(
record,
fileManager: fileManager,
lookup: claudeTranscriptLookup
)
: record
let normalizedSessionId = effectiveRecord.sessionId.trimmingCharacters(in: .whitespacesAndNewlines)
guard !normalizedSessionId.isEmpty,
let workspaceId = UUID(uuidString: record.workspaceId),
let panelId = UUID(uuidString: record.surfaceId),
let workspaceId = UUID(uuidString: effectiveRecord.workspaceId),
let panelId = UUID(uuidString: effectiveRecord.surfaceId),
hookRecordIsRestorable(
record,
effectiveRecord,
kind: kind,
fileManager: fileManager,
claudeTranscriptLookup: claudeTranscriptLookup
Expand All @@ -1135,39 +1142,39 @@ struct RestorableAgentSessionIndex: Sendable {
kind: kind,
sessionId: normalizedSessionId,
workingDirectory: restorableWorkingDirectory(
for: record,
for: effectiveRecord,
kind: kind,
fileManager: fileManager,
lookup: claudeTranscriptLookup
),
launchCommand: record.launchCommand,
launchCommand: effectiveRecord.launchCommand,
registration: registration
)
let key = PanelKey(workspaceId: workspaceId, panelId: panelId)
let sessionKey = SessionKey(kind: kind, sessionId: normalizedSessionId)
let liveProcessID = liveScopedProcessID(
for: record,
for: effectiveRecord,
kind: kind,
workspaceId: workspaceId,
panelId: panelId,
processArgumentsProvider: processArgumentsProvider
)
let entry = Entry(
snapshot: snapshot,
lifecycle: record.agentLifecycle,
updatedAt: record.updatedAt,
lifecycle: effectiveRecord.agentLifecycle,
updatedAt: effectiveRecord.updatedAt,
processIDs: liveProcessID.map { [$0] } ?? []
)
if hookCandidatesByPanel[key]?.updatedAt ?? -Double.infinity <= record.updatedAt {
if hookCandidatesByPanel[key]?.updatedAt ?? -Double.infinity <= effectiveRecord.updatedAt {
hookCandidatesByPanel[key] = entry
}
if hookCandidatesBySession[sessionKey]?.updatedAt ?? -Double.infinity <= record.updatedAt {
if hookCandidatesBySession[sessionKey]?.updatedAt ?? -Double.infinity <= effectiveRecord.updatedAt {
hookCandidatesBySession[sessionKey] = entry
}
guard record.pid == nil || liveProcessID != nil else {
guard effectiveRecord.pid == nil || liveProcessID != nil else {
continue
}
Comment on lines +1174 to 1176

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Stale PID drops resolved workflow sessions from the index

After resolvedClaudeWorkflowRecord remaps the container to a sibling transcript, effectiveRecord.pid still holds the workflow container process's PID. For the common case — the user wants to resume a completed workflow — that process is dead, so liveProcessID is nil. The guard effectiveRecord.pid == nil || liveProcessID != nil then skips the resolved[key] = entry assignment, removing the session from the restorable index even though a valid sibling transcript was found.

The resolved entry still ends up in hookCandidatesByPanel and hookCandidatesBySession (written before this guard), but those are only consulted by the detectedSnapshots loop for process-detected entries, not for hook-only sessions. In practice this means workflow sessions that had an associated process — which is the normal state — will never appear as restorable after the process exits, defeating the purpose of this fix.

The fix is to clear pid in resolvedClaudeWorkflowRecord when the session ID changes: the container's process is irrelevant to the sibling session. The test doesn't exercise this path because hookRecord(...) defaults pid to nil; adding a non-nil PID to the test fixture would reproduce the failure.

if let existing = resolved[key], existing.updatedAt > record.updatedAt {
if let existing = resolved[key], existing.updatedAt > effectiveRecord.updatedAt {
continue
}
resolved[key] = entry
Expand Down Expand Up @@ -1239,6 +1246,119 @@ struct RestorableAgentSessionIndex: Sendable {
return claudeTranscriptExists(for: record, fileManager: fileManager, lookup: claudeTranscriptLookup)
}

private static func resolvedClaudeWorkflowRecord(
_ record: RestorableAgentHookSessionRecord,
fileManager: FileManager,
lookup: ClaudeTranscriptLookupCache
) -> RestorableAgentHookSessionRecord {
guard let sessionId = normalizedNonEmptyValue(record.sessionId),
claudeSessionIdIsSafeFilename(sessionId) else {
return record
}
if let transcriptPath = normalizedNonEmptyValue(record.transcriptPath),
regularNonEmptyFileExists(
atPath: (transcriptPath as NSString).expandingTildeInPath,
fileManager: fileManager
) {
return record
}

let roots = lookup.configRoots(for: record)
guard !roots.isEmpty else { return record }
let candidateProjectDirs = claudeWorkflowProjectDirs(
for: record,
sessionId: sessionId,
roots: roots,
fileManager: fileManager,
lookup: lookup
)
guard let resolved = newestClaudeSiblingTranscript(
in: candidateProjectDirs,
excludingSessionId: sessionId,
fileManager: fileManager
) else {
return record
}

var resolvedRecord = record
resolvedRecord.sessionId = resolved.sessionId
resolvedRecord.transcriptPath = resolved.path
return resolvedRecord
}

private static func claudeWorkflowProjectDirs(
for record: RestorableAgentHookSessionRecord,
sessionId: String,
roots: [String],
fileManager: FileManager,
lookup: ClaudeTranscriptLookupCache
) -> [String] {
var projectDirs: [String] = []
var seen: Set<String> = []

func appendIfWorkflowContainer(projectRoot: String) {
let workflowContainer = (projectRoot as NSString).appendingPathComponent(sessionId)
var isDirectory: ObjCBool = false
guard fileManager.fileExists(atPath: workflowContainer, isDirectory: &isDirectory),
isDirectory.boolValue else {
return
}
let standardized = (projectRoot as NSString).standardizingPath
guard seen.insert(standardized).inserted else { return }
projectDirs.append(standardized)
}

let cwdCandidates = [
normalizedWorkingDirectory(record.launchCommand?.workingDirectory),
normalizedWorkingDirectory(record.cwd),
].compactMap { $0 }
for root in roots {
let projectsRoot = (root as NSString).appendingPathComponent("projects")
for cwd in cwdCandidates {
appendIfWorkflowContainer(
projectRoot: (projectsRoot as NSString).appendingPathComponent(encodeClaudeProjectDir(cwd))
)
}
for projectDir in lookup.projectDirs(configRoot: root) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This full projectDirs walk runs in the load path per Claude record, which can degrade startup as records and known project directories grow. Cache or bound this fallback lookup so the same config-root/session checks are not re-scanned repeatedly.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/RestorableAgentSession.swift, line 1322:

<comment>This full `projectDirs` walk runs in the load path per Claude record, which can degrade startup as records and known project directories grow. Cache or bound this fallback lookup so the same config-root/session checks are not re-scanned repeatedly.</comment>

<file context>
@@ -1239,6 +1246,119 @@ struct RestorableAgentSessionIndex: Sendable {
+                    projectRoot: (projectsRoot as NSString).appendingPathComponent(encodeClaudeProjectDir(cwd))
+                )
+            }
+            for projectDir in lookup.projectDirs(configRoot: root) {
+                appendIfWorkflowContainer(
+                    projectRoot: (projectsRoot as NSString).appendingPathComponent(projectDir)
</file context>

appendIfWorkflowContainer(
projectRoot: (projectsRoot as NSString).appendingPathComponent(projectDir)
)
}
Comment on lines +1322 to +1326

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Per-record full project-dir scan for non-workflow sessions

For every Claude record that lacks an explicit transcriptPath — including ordinary (non-workflow) sessions — the fallback loop calls fileManager.fileExists(atPath:) once per entry returned by lookup.projectDirs(configRoot:). A user who has opened many repositories in Claude can accumulate hundreds of project directories, so for N Claude records × P project dirs this is O(N × P) stat calls on every RestorableAgentSessionIndex.load. A lightweight early-exit would help: if the CWD-derived candidate already matched (i.e., projectDirs is non-empty after the first loop), skip the full-project-dirs scan entirely, since a workflow container is almost certainly under the session's own working directory.

Rule Used: Flag production code that adds nested full-collect... (source)

}
return projectDirs
Comment on lines +1266 to +1328

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Cache or bound the project-directory fallback scan.

Lines 1266-1328 run inside the per-record load loop, but they still iterate every Claude project directory and probe <projectRoot>/<sessionId> for each Claude hook record. On installs with roughly 1000 workspaces/sessions this becomes a startup-time full rescan per target record. Please move this fallback behind a cache/index keyed by config root + session id, or otherwise stop after the cwd-derived candidates fail without walking the full project set. As per coding guidelines, "fail when the diff violates .github/review-bot-rules/algorithmic-complexity.md": nested full-collection scans and per-target rescans in production paths expected to handle about 1000 workspaces or similar user-owned records.

🤖 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 `@Sources/RestorableAgentSession.swift` around lines 1266 - 1328, The fallback
scan in claudeWorkflowProjectDirs is doing a full per-record walk of all
projectDirs for every sessionId; change it to short-circuit and/or memoize:
first attempt only the cwdCandidates and if any are found return immediately
(avoid iterating lookup.projectDirs), and add a simple cache keyed by
(configRoot, sessionId) — e.g., a private static dictionary on
RestorableAgentSession or a method on ClaudeTranscriptLookupCache — to store
previously computed projectDirs so subsequent calls to claudeWorkflowProjectDirs
for the same config root + sessionId return the cached [String] instead of
probing every project; ensure cache lookup/insertion surrounds the loop that
iterates lookup.projectDirs and preserves the existing behavior for misses.

}

private static func newestClaudeSiblingTranscript(
in projectDirs: [String],
excludingSessionId excludedSessionId: String,
fileManager: FileManager
) -> (sessionId: String, path: String)? {
var best: (sessionId: String, path: String, modifiedAt: TimeInterval)?
for projectDir in projectDirs {
guard let children = try? fileManager.contentsOfDirectory(atPath: projectDir) else {
continue
}
for child in children where child.hasSuffix(".jsonl") {
let sessionId = String(child.dropLast(".jsonl".count))
guard sessionId != excludedSessionId,
claudeSessionIdIsSafeFilename(sessionId) else {
continue
}
let path = (projectDir as NSString).appendingPathComponent(child)
guard regularNonEmptyFileExists(atPath: path, fileManager: fileManager) else {
continue
}
let modifiedAt = ((try? fileManager.attributesOfItem(atPath: path)[.modificationDate]) as? Date)?
.timeIntervalSince1970 ?? 0
if best == nil || modifiedAt > best!.modifiedAt {
best = (sessionId, path, modifiedAt)
}
}
}
guard let best else { return nil }
return (best.sessionId, best.path)
Comment on lines +1331 to +1359

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don't auto-resolve to the newest sibling transcript.

Lines 1331-1359 always pick the most recently modified sibling .jsonl, but a Claude project directory can legitimately contain multiple transcripts over time. cmuxTests/RestorableAgentSessionIndexTests.swift Lines 165-184 already model that shape, so a later unrelated chat in the same repo can steal the workflow entry and make resume/fork target the wrong conversation. Please only resolve when you can link the workflow container to a single transcript deterministically; otherwise leave the record non-restorable.

🤖 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 `@Sources/RestorableAgentSession.swift` around lines 1331 - 1359, The current
newestClaudeSiblingTranscript(in:excludingSessionId:fileManager:) automatically
returns the most recently modified sibling .jsonl which can incorrectly
associate an unrelated later transcript; change the logic to only resolve when
there is exactly one eligible transcript across all projectDirs: iterate
projectDirs and collect candidates that pass
claudeSessionIdIsSafeFilename(sessionId) and regularNonEmptyFileExists(atPath:),
using fileManager.contentsOfDirectory(atPath:), and if the candidate count == 1
return that sessionId/path, otherwise return nil (do not pick the newest). Keep
the same helper checks (claudeSessionIdIsSafeFilename,
regularNonEmptyFileExists) and return nil for zero or multiple matches so
records remain non-restorable unless deterministic.

}

private static func claudeTranscriptExists(
for record: RestorableAgentHookSessionRecord,
fileManager: FileManager,
Expand Down
58 changes: 58 additions & 0 deletions cmuxTests/RestorableAgentSessionIndexTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,64 @@ final class RestorableAgentSessionIndexTests: XCTestCase {
XCTAssertEqual(snapshot.workingDirectory, launchCwd.path)
}

func testClaudeWorkflowDirectorySessionUsesSiblingJsonlSessionForResume() throws {
let fm = FileManager.default
let root = fm.temporaryDirectory
.appendingPathComponent("cmux-claude-workflow-directory-\(UUID().uuidString)", isDirectory: true)
defer { try? fm.removeItem(at: root) }

let configDir = root.appendingPathComponent("claude-config", isDirectory: true)
let projectsDir = configDir.appendingPathComponent("projects", isDirectory: true)
let cwd = root.appendingPathComponent("repo", isDirectory: true)
try fm.createDirectory(at: cwd, withIntermediateDirectories: true)
let projectDir = projectsDir.appendingPathComponent(
expectedClaudeProjectDirName(cwd.path),
isDirectory: true
)
let workflowContainerSessionId = "aaaaaaaa-1111-1111-1111-aaaaaaaaaaaa"
let resumableSessionId = "bbbbbbbb-2222-2222-2222-bbbbbbbbbbbb"
let workflowContainerURL = projectDir
.appendingPathComponent(workflowContainerSessionId, isDirectory: true)
try fm.createDirectory(
at: workflowContainerURL
.appendingPathComponent("subagents", isDirectory: true),
withIntermediateDirectories: true
)
let siblingTranscriptURL = projectDir.appendingPathComponent("\(resumableSessionId).jsonl", isDirectory: false)
try writeClaudeTranscript(sessionId: resumableSessionId, transcriptURL: siblingTranscriptURL, cwd: cwd)

let workspaceId = UUID()
let panelId = UUID()
try writeClaudeHookStore(
root: root,
sessions: [
workflowContainerSessionId: hookRecord(
sessionId: workflowContainerSessionId,
workspaceId: workspaceId,
panelId: panelId,
cwd: cwd.path,
configDir: configDir.path,
transcriptPath: workflowContainerURL.path,
isRestorable: true,
updatedAt: 10
),
]
)

let index = RestorableAgentSessionIndex.load(homeDirectory: root.path, fileManager: fm)
let snapshot = try XCTUnwrap(index.snapshot(workspaceId: workspaceId, panelId: panelId))

XCTAssertEqual(snapshot.sessionId, resumableSessionId)
XCTAssertEqual(snapshot.workingDirectory, cwd.path)
XCTAssertTrue(
try XCTUnwrap(snapshot.resumeCommand).contains("'--resume' '\(resumableSessionId)'")
)
XCTAssertFalse(
try XCTUnwrap(snapshot.resumeCommand).contains(workflowContainerSessionId),
"The Workflow container id is not accepted by claude --resume."
)
}

/// Mirrors Claude's external project-directory naming rule ("/" and "." both become "-")
/// independently of the production `encodeClaudeProjectDir`, so these regression tests fail if
/// that helper regresses instead of masking it by sharing the same code path.
Expand Down
Loading