Repository navigation
Chat view: resume-binding fallback for session-restored terminals - #5791
lawrencecchen wants to merge 4 commits into
Conversation
…n index is empty Fixes the dogfood blocker where opening the chat view on a session-restored terminal showed 'No agent conversation': the resolver only consulted the live RestorableAgentSessionIndex, which isn't repopulated after an app relaunch until the agent re-announces. It now reconstructs the (kind, sessionId, cwd, env) lookup tuple from the panel's persisted SurfaceResumeBindingSnapshot (checkpointId = the agent session id) when the index misses, and only returns the empty state when neither source yields a transcript-backed session. The presenter passes the panel's resume binding through. Swift Testing coverage: Claude + Codex fallback locate the transcript with an empty index; absent binding and non-transcript kinds return nil. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
Greptile SummaryFixes the "No agent conversation" empty state when the agent chat view is opened on a session-restored terminal after a relaunch, where the live
Confidence Score: 5/5Safe to merge — the fallback is read-only, gated to Claude/Codex kinds only, validates session IDs as safe filename components, and leaves the live-index path completely unchanged. The change adds a well-bounded fallback: No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant P as AgentChatPresenter (@MainActor)
participant W as Workspace (@MainActor)
participant D as Task.detached
participant I as RestorableAgentSessionIndex
participant R as AgentChatTranscriptResolver
participant FS as Filesystem
P->>W: surfaceResumeBinding(panelId:)
W-->>P: SurfaceResumeBindingSnapshot?
P->>D: detach(resumeBinding, workspaceId, panelId)
D->>I: RestorableAgentSessionIndex.load()
I-->>D: index
D->>R: resolve(index, workspaceId, panelId, resumeBinding)
alt Live index has entry
R->>I: index.snapshot(workspaceId, panelId)
I-->>R: AgentSessionSnapshot (kind, sessionId, cwd, env)
R->>FS: locate transcript (.jsonl)
FS-->>R: URL?
R-->>D: Resolution(agentKind, sessionId, transcriptURL)
else Index miss — session-restored terminal
R->>R: resumeBindingInputs(binding)
Note over R: validate checkpointId (safe filename component), gate kind to Claude/Codex only
R->>FS: locate transcript (.jsonl or workflow-container sibling)
FS-->>R: URL?
R-->>D: Resolution(agentKind, sessionId, transcriptURL)
else Neither source yields a session
R-->>D: nil
end
D-->>P: Resolution?
alt Resolution found
P->>P: AgentChatWindowController.present(for:)
else nil
P->>P: presentNoSessionAlert()
end
Reviews (3): Last reviewed commit: "CI: gate CmuxAgentConversation package t..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| /// Maps a resume binding's kind string to a `RestorableAgentKind`, limited | ||
| /// to the transcript-backed kinds the P1 parsers support. | ||
| private func restorableKind(fromBindingKind kind: String?) -> RestorableAgentKind? { | ||
| switch kind?.lowercased() { | ||
| case "claude": return .claude | ||
| case "codex": return .codex | ||
| default: return nil | ||
| } | ||
| } |
There was a problem hiding this comment.
restorableKind and resolve()
restorableKind(fromBindingKind:) re-implements the same string-to-enum mapping that RestorableAgentKind.init(rawValue:) already covers, and it must be kept in sync with the switch inputs.kind cases in resolve(). If a new transcript-backed kind (e.g., .gemini) is ever added to that switch, a developer has to remember to add it here too — there is no compile-time enforcement of the coupling. Because resolve() already has default: return nil for non-transcript-backed kinds, the filtering could live entirely in that switch, letting restorableKind simply delegate to RestorableAgentKind(rawValue: kind?.lowercased() ?? "").
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
…review) The fallback used SurfaceResumeBindingSnapshot.checkpointId directly as a transcript filename component, but resume bindings are a trust boundary (the public resume path does not validate the checkpoint id). A value with a path separator or '..' could escape the selected project/sessions dir into another reachable .jsonl. The fallback now applies the same safe-filename invariant the live index path uses for Claude. Test covers ../, a/b, '.', '..'. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… fallback Autoreview: a Claude workflow/sub-agent resume binding records a *container* directory id, not a transcript id; the real .jsonl is a newer sibling with a different id. The fallback now mirrors the live index path's resolvedClaudeWorkflowRecord: when <id> is a container dir under a project dir, it resolves to the newest sibling transcript. Test covers the container-with-two-siblings case (newest wins). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0961641860
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let id = String(child.dropLast(".jsonl".count)) | ||
| guard id != excluded, !id.isEmpty else { continue } | ||
| let path = (projectDir as NSString).appendingPathComponent(child) | ||
| guard regularFileExists(path) else { continue } |
There was a problem hiding this comment.
For restored Claude workflow panels, this fallback can select the newest empty .jsonl sibling because it only checks that the path is a regular file. The live restore path this mirrors (RestorableAgentSessionIndex.newestClaudeSiblingTranscript) filters with regularNonEmptyFileExists, so when Claude leaves a freshly-created empty sibling next to an older transcript with content, View Chat opens a blank conversation even though the resumable transcript exists. Apply the same non-empty check before comparing modification times.
Useful? React with 👍 / 👎.
The resume-binding fallback added Packages/CmuxAgentConversation with Claude/ Codex transcript parser tests, but the Swift-package-unit-tests CI step's PACKAGES array omitted it, so parser regressions compiled but never gated PRs. Confirmed it resolves standalone (swift test, 14 tests, no GhosttyKit dep) and added it to the array. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
…f PR 5791) When both the live hook index and the workspace's in-memory restored snapshot miss (a terminal restored after an app relaunch whose snapshot was consumed or never captured), the resolver now reconstructs the (kind, sessionId, cwd, env) lookup tuple from the panel's persisted SurfaceResumeBindingSnapshot (checkpointId is the agent session id the resume path uses). Binding checkpoint ids are validated as safe filename components (trust boundary). Also ports the Claude workflow-container fallback: when the recorded id is a container directory, the newest sibling transcript in the same project dir resolves instead. Ported from #5791 (stacked on the closed feat-agent-chat-view base), adapted to the current resolver API; Swift Testing coverage included and wired into the test target. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Ported into #5736 as commit ea36eb8 (the resolver now falls back to the panel's persisted resume binding when the live index misses, adapted to the Go-daemon architecture, with equivalent Swift Testing coverage). This branch's base (feat-agent-chat-view, PR 5576) was closed as superseded, so closing this one too. |
This branch was successfully deployed
1 participant
Stacked on #5576. Fixes the dogfood blocker (Lawrence): opening the agent chat view on a session-restored terminal showed "No agent conversation" even though it had a real agent session.
Root cause
AgentChatTranscriptResolverresolved a panel's transcript only through the liveRestorableAgentSessionIndex, which is not repopulated for a terminal restored after an app relaunch (the agent hasn't re-announced via the hook yet). Soindex.snapshot(...)returned nil → empty state.Fix
The resolver now reconstructs the
(kind, sessionId, cwd, env)lookup tuple from the panel's persistedSurfaceResumeBindingSnapshot(the same binding cmux uses to resume the agent;checkpointIdis the agent session id) when the live index misses. It only returns the empty state when neither source yields a transcript-backed session. The presenter capturesworkspace.surfaceResumeBinding(panelId:)on the main actor and passes it through. Existing index path is unchanged (preferred when present).Tests
Swift Testing in
cmuxTests/AgentChatTranscriptResolverResumeBindingTests.swift(wired into the test target): Claude and Codex fallback locate the transcript with an empty index + a resume binding (temp-home fixtures); absent binding and a non-transcript kind both return nil.Verification
Build-only compile (tagged DerivedData): BUILD SUCCEEDED. pbxproj normalized + checked + test-wiring lint ok. Folds into the
guidogfood tag for Lawrence to confirm on a restored terminal.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Touches filesystem transcript resolution from persisted bindings (path traversal guards added) and changes when chat opens vs shows empty state; scope is localized to Agent Chat resolution.
Overview
Fixes “No agent conversation” on session-restored terminals by letting agent chat resolution use the panel’s persisted resume binding when the live
RestorableAgentSessionIndexhas no entry yet (typical after relaunch before the agent re-announces).AgentChatPresenternow readsworkspace.surfaceResumeBinding(panelId:)on the main actor and passes it intoAgentChatTranscriptResolver.resolve. The resolver still prefers the live index; otherwise it rebuilds kind, session id, cwd, and env fromSurfaceResumeBindingSnapshot(checkpointIdas session id), with filename safety checks on binding ids and Claude/Codex-only kinds.Claude lookup also gains the workflow-container path: when the session id is a container directory, it picks the newest sibling
.jsonlin the project dir, matching the live index behavior. NewcmuxTests/AgentChatTranscriptResolverResumeBindingTestscover fallback, workflow siblings, unsafe ids, and unsupported kinds; CI also addsCmuxAgentConversationto the headless SwiftPM test list.Reviewed by Cursor Bugbot for commit dc82bb6. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes the chat view showing “No agent conversation” on session-restored terminals by falling back to the panel’s persisted resume binding when the live index is empty, with session-id validation and a Claude workflow-container fallback. Restored terminals now load the correct Claude/Codex transcript after relaunch.
Bug Fixes
RestorableAgentSessionIndex; if missing, rebuild(kind, sessionId, cwd, env)fromSurfaceResumeBindingSnapshotand only show the empty state if both miss..jsonlin the same project.AgentChatPresenterpassesworkspace.surfaceResumeBinding(panelId:)to the resolver; tests cover fallback, unsafe ids, unsupported kinds, and the workflow case.CI
CmuxAgentConversationSwift package tests to catch Claude/Codex transcript parser regressions.Written for commit dc82bb6. Summary will update on new commits.