Repository navigation
Add Sessions panel to right sidebar - #2936
Merged
Merged
Conversation
Adds a Sessions mode to the right sidebar (cmd-option-b) alongside the existing file tree. A two-button toggle at the top of the panel switches between Files and Sessions. Sessions lists recent agent runs from: - Claude Code (~/.claude/projects/*/*.jsonl) - Codex (~/.codex/sessions/YYYY/MM/DD/rollout-*.jsonl) - OpenCode (~/.local/share/opencode/opencode.db, snapshotted before read) Each row shows the first user prompt as a title, the working directory, and a relative timestamp. Right-click for Open / Reveal in Finder / Copy File Path / Open Working Directory. A "This folder only" filter scopes the list to the current FileExplorer root. Mode selection persists in UserDefaults under "rightSidebar.mode".
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Greptile SummaryThis PR adds a Sessions panel to the right sidebar (
Confidence Score: 4/5Safe to merge after fixing the double-reload bug; all other findings are P2 quality improvements. One P1 issue (double reload cancels the first scan on every initial sessions view) should be addressed before merging. The remaining findings are P2 style/quality concerns that don't block correctness. Sources/SessionIndexView.swift (double-reload onAppear guard) and Sources/SessionIndexStore.swift (early break in extractClaudeMetadata) Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Sidebar as RightSidebarPanelView
participant View as SessionIndexView
participant Store as SessionIndexStore
participant FS as FileSystem/SQLite
User->>Sidebar: "Click Sessions"
Sidebar->>Store: "currentDirectory = rootPath"
Sidebar->>Store: "reload() — entries empty"
Note over Store: "isLoading = true, Task.detached started"
Sidebar->>View: "render (mode = .sessions)"
View->>Store: "onAppear: reload() — entries still empty (double-reload bug)"
Note over Store: "Cancels first task, restarts scan"
Store->>FS: "scanClaude() + scanCodex() + scanOpenCode()"
FS-->>Store: "SessionEntry arrays"
Note over Store: "MainActor.run: entries = scanned, isLoading = false"
Store-->>View: "published entries -> re-render list"
User->>View: "Right-click row"
View-->>User: "Open / Reveal in Finder / Copy Path / Open Working Dir"
Reviews (1): Last reviewed commit: "Add Sessions panel to right sidebar" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+26
to
+30
| .onAppear { | ||
| if store.entries.isEmpty { | ||
| store.reload() | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
When the user switches to sessions for the first time, modeBar's button action already calls sessionIndexStore.reload() (because entries.isEmpty), setting isLoading = true. Then SessionIndexView appears and this .onAppear fires while entries is still empty — triggering a second reload(), which cancels the first task and restarts the full scan. The first scan is always wasted on initial view.
Suggested change
| .onAppear { | |
| if store.entries.isEmpty { | |
| store.reload() | |
| } | |
| } | |
| .onAppear { | |
| if store.entries.isEmpty && !store.isLoading { | |
| store.reload() | |
| } | |
| } |
Contributor
Author
There was a problem hiding this comment.
Fixed: SessionIndexView's onAppear now also guards on !store.isLoading so the mode-toggle's reload isn't immediately cancelled and restarted.
Comment on lines
+193
to
+198
| } | ||
| } | ||
| } | ||
| if firstUserText != nil { break } | ||
| } | ||
| return (firstUserText ?? "", cwd) |
Contributor
There was a problem hiding this comment.
cwd field
The loop breaks as soon as firstUserText is set (line 196). In Claude Code's JSONL format the cwd field could appear in a line after the first user message — if that happens, the break discards it and the fallback decodeClaudeProjectDir (which is lossy, replacing all - with /) is used instead. For users with hyphens in directory names this silently produces an incorrect cwd, which also affects the "This folder only" filter.
Consider continuing the scan for cwd until both are found, mirroring the extractCodexMetadata pattern:
if firstUserText != nil && cwd != nil { break }
Comment on lines
+283
to
+288
| private func absoluteTime(_ date: Date) -> String { | ||
| let f = DateFormatter() | ||
| f.dateStyle = .medium | ||
| f.timeStyle = .short | ||
| return f.string(from: date) | ||
| } |
Contributor
There was a problem hiding this comment.
DateFormatter allocated on every tooltip access
absoluteTime creates a new DateFormatter each call. DateFormatter initialisation is notoriously expensive (locale/calendar lookups), and helpText — which calls this — is a computed property accessed on every tooltip hover. Cache it the same way relativeFormatter is cached:
Suggested change
| private func absoluteTime(_ date: Date) -> String { | |
| let f = DateFormatter() | |
| f.dateStyle = .medium | |
| f.timeStyle = .short | |
| return f.string(from: date) | |
| } | |
| private static let absoluteFormatter: DateFormatter = { | |
| let f = DateFormatter() | |
| f.dateStyle = .medium | |
| f.timeStyle = .short | |
| return f | |
| }() | |
| private func absoluteTime(_ date: Date) -> String { | |
| SessionRow.absoluteFormatter.string(from: date) | |
| } |
Contributor
Author
There was a problem hiding this comment.
Fixed: cached absoluteFormatter as a static on SessionIndexView, and absoluteTime now reuses it instead of allocating per call.
Comment on lines
+4
to
+22
| /// Mode shown in the right sidebar (the panel toggled by ⌘⌥B). | ||
| enum RightSidebarMode: String, CaseIterable { | ||
| case files | ||
| case sessions | ||
|
|
||
| var label: String { | ||
| switch self { | ||
| case .files: return String(localized: "rightSidebar.mode.files", defaultValue: "Files") | ||
| case .sessions: return String(localized: "rightSidebar.mode.sessions", defaultValue: "Sessions") | ||
| } | ||
| } | ||
|
|
||
| var symbolName: String { | ||
| switch self { | ||
| case .files: return "folder" | ||
| case .sessions: return "bubble.left.and.text.bubble.right" | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
RightSidebarMode is declared in the view file RightSidebarPanelView.swift but is consumed by FileExplorerState in FileExplorerStore.swift — a pure model type. This creates an unusual dependency where a model references a type from a view file. Consider moving RightSidebarMode to FileExplorerStore.swift (alongside FileExplorerState) or a dedicated RightSidebarMode.swift.
Contributor
Author
There was a problem hiding this comment.
Acknowledged. RightSidebarMode is shaped a bit awkwardly (model uses the rawValue, view uses label/symbolName) but moving it to its own file is a no-op refactor; tracking separately rather than bundling into this PR.
|
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:
📝 WalkthroughWalkthroughAdds a session index feature and a right-sidebar mode switch: new SessionIndexStore and SessionIndexView, a RightSidebarPanelView with mode controls, ContentView/FileExplorerState wiring to persist sidebar mode, new localization keys, and agent icon assets. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as RightSidebarPanelView / ContentView
participant Store as SessionIndexStore
participant FS as Filesystem (JSONL files)
participant DB as OpenCode SQLite
UI->>Store: set mode = .sessions
Note right of UI: UI sets currentDirectory from FileExplorerStore
UI->>Store: reload()
Store->>FS: scan Claude & Codex JSONL (concurrent)
Store->>DB: snapshot & query OpenCode DB
FS-->>Store: return parsed session metadata
DB-->>Store: return session rows
Store->>Store: normalize, dedupe, build entries & sections
Store-->>UI: publish entries (isLoading = false)
UI->>UI: render SessionIndexView sections and rows
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 7 files
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="Sources/SessionIndexView.swift">
<violation number="1" location="Sources/SessionIndexView.swift:188">
P2: The working-directory label is reduced to only the final folder name, which hides path context and can make session rows ambiguous.</violation>
</file>
<file name="Sources/SessionIndexStore.swift">
<violation number="1" location="Sources/SessionIndexStore.swift:100">
P2: Root directory filtering is broken because `dir + "/"` becomes `"//"` when `dir` is `/`, excluding valid child paths.</violation>
<violation number="2" location="Sources/SessionIndexStore.swift:150">
P2: `perAgentLimit` is applied only after full file enumeration/sort, so scan cost remains unbounded by the limit for large histories.</violation>
<violation number="3" location="Sources/SessionIndexStore.swift:207">
P2: `decodeClaudeProjectDir` replaces *all* hyphens with `/`, so any directory whose name contains a hyphen (e.g. `my-cool-project`) is decoded to an incorrect path (`my/cool/project`). This corrupts the displayed `cwdLabel` and breaks the "this folder only" filter for those sessions.
Consider validating the decoded path against the filesystem, or skipping the decode entirely when the JSONL `cwd` field is available.</violation>
</file>
<file name="Sources/ContentView.swift">
<violation number="1" location="Sources/ContentView.swift:2982">
P2: Session folder scope can become stale because `currentDirectory` is not cleared/updated when `tab.currentDirectory` is empty due to an early return before the new session sync block.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
| } | ||
|
|
||
| candidates.sort { $0.1 > $1.1 } |
There was a problem hiding this comment.
P2: perAgentLimit is applied only after full file enumeration/sort, so scan cost remains unbounded by the limit for large histories.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/SessionIndexStore.swift, line 150:
<comment>`perAgentLimit` is applied only after full file enumeration/sort, so scan cost remains unbounded by the limit for large histories.</comment>
<file context>
@@ -0,0 +1,359 @@
+ }
+ }
+
+ candidates.sort { $0.1 > $1.1 }
+ let limited = candidates.prefix(perAgentLimit)
+
</file context>
| } | ||
| return base.filter { entry in | ||
| guard let cwd = normalizedDirectory(entry.cwd) else { return false } | ||
| return cwd == dir || cwd.hasPrefix(dir + "/") |
There was a problem hiding this comment.
P2: Root directory filtering is broken because dir + "/" becomes "//" when dir is /, excluding valid child paths.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/SessionIndexStore.swift, line 100:
<comment>Root directory filtering is broken because `dir + "/"` becomes `"//"` when `dir` is `/`, excluding valid child paths.</comment>
<file context>
@@ -0,0 +1,359 @@
+ }
+ return base.filter { entry in
+ guard let cwd = normalizedDirectory(entry.cwd) else { return false }
+ return cwd == dir || cwd.hasPrefix(dir + "/")
+ }
+ }
</file context>
| // This is lossy (cannot distinguish original "-" from "/"), so try as a hint only. | ||
| guard !raw.isEmpty else { return nil } | ||
| let stripped = raw.hasPrefix("-") ? String(raw.dropFirst()) : raw | ||
| let candidate = "/" + stripped.replacingOccurrences(of: "-", with: "/") |
There was a problem hiding this comment.
P2: decodeClaudeProjectDir replaces all hyphens with /, so any directory whose name contains a hyphen (e.g. my-cool-project) is decoded to an incorrect path (my/cool/project). This corrupts the displayed cwdLabel and breaks the "this folder only" filter for those sessions.
Consider validating the decoded path against the filesystem, or skipping the decode entirely when the JSONL cwd field is available.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/SessionIndexStore.swift, line 207:
<comment>`decodeClaudeProjectDir` replaces *all* hyphens with `/`, so any directory whose name contains a hyphen (e.g. `my-cool-project`) is decoded to an incorrect path (`my/cool/project`). This corrupts the displayed `cwdLabel` and breaks the "this folder only" filter for those sessions.
Consider validating the decoded path against the filesystem, or skipping the decode entirely when the JSONL `cwd` field is available.</comment>
<file context>
@@ -0,0 +1,359 @@
+ // This is lossy (cannot distinguish original "-" from "/"), so try as a hint only.
+ guard !raw.isEmpty else { return nil }
+ let stripped = raw.hasPrefix("-") ? String(raw.dropFirst()) : raw
+ let candidate = "/" + stripped.replacingOccurrences(of: "-", with: "/")
+ return candidate
+ }
</file context>
Contributor
Author
There was a problem hiding this comment.
Fixed: decodeClaudeProjectDir now validates the candidate against the filesystem and returns nil on miss, so callers fall back to the JSONL cwd field for directories with real - segments.
There was a problem hiding this comment.
Thanks for implementing that—validating the decoded path and falling back to the JSONL cwd should prevent the hyphen-path issue.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
Sources/SessionIndexView.swift (1)
283-287: Cache the absolute-time formatter.
absoluteTime(_:)creates a newDateFormatteron every call; making it static avoids repeated allocations during list/tooltip recomposition.♻️ Proposed refactor
private struct SessionRow: View { + private static let absoluteFormatter: DateFormatter = { + let f = DateFormatter() + f.dateStyle = .medium + f.timeStyle = .short + return f + }() @@ private func absoluteTime(_ date: Date) -> String { - let f = DateFormatter() - f.dateStyle = .medium - f.timeStyle = .short - return f.string(from: date) + Self.absoluteFormatter.string(from: date) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/SessionIndexView.swift` around lines 283 - 287, The absoluteTime(_:) function currently allocates a new DateFormatter on every call; change it to reuse a single cached formatter by creating a static/shared DateFormatter (e.g. private static let absoluteTimeFormatter) configured once with dateStyle = .medium and timeStyle = .short, and have absoluteTime(_:) call that formatter's string(from:). Update references to use that static formatter (absoluteTime(_:), absoluteTimeFormatter) to avoid repeated allocations during list/tooltip recomposition.Sources/SessionIndexStore.swift (1)
79-90: Add cooperative cancellation inside scan loops.Cancellation currently prevents stale UI assignment, but the detached task can still continue expensive file/SQLite work after a newer reload starts.
⚡ Proposed refactor
func reload() { loadTask?.cancel() isLoading = true loadTask = Task.detached(priority: .userInitiated) { [weak self] in + if Task.isCancelled { return } let scanned = await Self.scanAll() await MainActor.run { guard let self else { return } if Task.isCancelled { return } self.entries = scanned self.isLoading = false } } } @@ for (url, mtime, dirName) in limited { + if Task.isCancelled { break } let preview = readFileHead(url: url, byteCap: titlePreviewByteCap) @@ for (url, mtime) in limited { + if Task.isCancelled { break } let preview = readFileHead(url: url, byteCap: titlePreviewByteCap) @@ - while sqlite3_step(stmt) == SQLITE_ROW { + while !Task.isCancelled, sqlite3_step(stmt) == SQLITE_ROW { let sid = sqliteText(stmt, 0) ?? "" ... }Also applies to: 118-124, 153-168, 236-252, 321-338
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/SessionIndexStore.swift` around lines 79 - 90, The detached reload Task cancels only UI assignment but lets the expensive scanning (Self.scanAll) continue; add cooperative cancellation checks inside the long-running scan loops so work stops when the reload Task is cancelled. Modify the scanning code (e.g., Self.scanAll and any helper loop functions used by it) to periodically check Task.isCancelled or call try Task.checkCancellation() inside each file/row loop and return early (or throw) so the detached Task's cancellation aborts expensive file/SQLite work; ensure reload/loadTask logic still handles the returned/throwing result on the MainActor as before.
🤖 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/ContentView.swift`:
- Around line 2982-2986: The new assignment to
sessionIndexStore.currentDirectory can be skipped by earlier returns, leaving a
stale local directory when the active workspace is remote/empty; update the
control flow so sessionIndexStore.currentDirectory is explicitly cleared (set to
nil) before each early return path that indicates a remote or empty workspace
(referencing tab.isRemoteWorkspace and the early-return conditions around Line
2975/2979), or move the clearing logic to execute unconditionally at the start
of the surrounding method so it always runs regardless of those returns.
In `@Sources/SessionIndexView.swift`:
- Around line 26-30: SessionIndexView currently calls store.reload() in its
.onAppear, which duplicates the reload already triggered by
RightSidebarPanelView and causes a cancel/restart cycle; update the .onAppear in
SessionIndexView to only call store.reload() when entries are empty AND the
store is not already reloading/hasn't performed the initial load (e.g. check a
flag like store.isLoading or store.didInitialLoad) so the call to store.reload()
is skipped if RightSidebarPanelView already triggered it; reference the
SessionIndexView .onAppear block and the store.reload() call and use
store.isLoading / didInitialLoad (or add such a small boolean) to gate the
reload.
---
Nitpick comments:
In `@Sources/SessionIndexStore.swift`:
- Around line 79-90: The detached reload Task cancels only UI assignment but
lets the expensive scanning (Self.scanAll) continue; add cooperative
cancellation checks inside the long-running scan loops so work stops when the
reload Task is cancelled. Modify the scanning code (e.g., Self.scanAll and any
helper loop functions used by it) to periodically check Task.isCancelled or call
try Task.checkCancellation() inside each file/row loop and return early (or
throw) so the detached Task's cancellation aborts expensive file/SQLite work;
ensure reload/loadTask logic still handles the returned/throwing result on the
MainActor as before.
In `@Sources/SessionIndexView.swift`:
- Around line 283-287: The absoluteTime(_:) function currently allocates a new
DateFormatter on every call; change it to reuse a single cached formatter by
creating a static/shared DateFormatter (e.g. private static let
absoluteTimeFormatter) configured once with dateStyle = .medium and timeStyle =
.short, and have absoluteTime(_:) call that formatter's string(from:). Update
references to use that static formatter (absoluteTime(_:),
absoluteTimeFormatter) to avoid repeated allocations during list/tooltip
recomposition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3e975483-fb4a-433e-9acb-3b9bd512c2fb
📒 Files selected for processing (7)
GhosttyTabs.xcodeproj/project.pbxprojResources/Localizable.xcstringsSources/ContentView.swiftSources/FileExplorerStore.swiftSources/RightSidebarPanelView.swiftSources/SessionIndexStore.swiftSources/SessionIndexView.swift
- Replace SF Symbols in agent section headers with the official Claude, OpenAI (Codex), and OpenCode brand marks. Icons rendered from upstream SVGs (simpleicons.org and opencode.ai/favicon.svg) at @1x/@2x/@3x and bundled under Assets.xcassets/AgentIcons. - Make agent sections drag-reorderable. Drag a section header onto another to put it first; the order persists in UserDefaults under "sessionIndex.agentOrder". - Right-sidebar mode bar: shift the toggle 2px left and bump header height from 30 to 31.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Sources/SessionIndexView.swift (1)
26-30:⚠️ Potential issue | 🟡 MinorAvoid double reload on first Sessions open.
SessionIndexView.onAppearcan triggerreload()whenRightSidebarPanelViewhas already initiated one during the mode switch (lines 52-54 in RightSidebarPanelView.swift). This causes a cancel/restart cycle on initial open.💡 Proposed fix
.onAppear { - if store.entries.isEmpty { + if store.entries.isEmpty && !store.isLoading { store.reload() } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/SessionIndexView.swift` around lines 26 - 30, SessionIndexView currently calls store.reload() in onAppear which can duplicate a reload already started by RightSidebarPanelView; update the guard so reload only runs when truly needed by checking the store's state (e.g., if store.entries.isEmpty && !store.isLoading && !store.didLoad) or add a store method like reloadIfNeeded() that encapsulates this logic and call that from SessionIndexView.onAppear instead of directly calling store.reload(). Ensure you reference SessionIndexView.onAppear and store.reload()/store.isLoading or the new reloadIfNeeded() helper when making the change.
🧹 Nitpick comments (3)
Sources/SessionIndexView.swift (1)
353-358: Consider caching theDateFormatterfor tooltip generation.
absoluteTime(_:)allocates a newDateFormatteron each call. While this is only used for tooltips (not high-frequency), you could make it a static property likerelativeFormatterfor consistency and minor efficiency gains.♻️ Suggested refactor
+ static let absoluteFormatter: DateFormatter = { + let f = DateFormatter() + f.dateStyle = .medium + f.timeStyle = .short + return f + }() + private func absoluteTime(_ date: Date) -> String { - let f = DateFormatter() - f.dateStyle = .medium - f.timeStyle = .short - return f.string(from: date) + Self.absoluteFormatter.string(from: date) }Note: If locale changes at runtime are a concern, the current approach is actually safer. This is optional.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/SessionIndexView.swift` around lines 353 - 358, The absoluteTime(_:) function currently creates a new DateFormatter on each call; cache a DateFormatter as a static/shared instance (similar to relativeFormatter) and reuse it in absoluteTime(_:) to avoid repeated allocations—e.g., add a static let absoluteFormatter = DateFormatter() configured with dateStyle = .medium and timeStyle = .short, then return absoluteFormatter.string(from: date) from absoluteTime(_:); if you need to handle runtime locale changes, consider resetting or recreating the formatter when locale updates occur.Sources/RightSidebarPanelView.swift (1)
67-80: Minor: RedundantcurrentDirectoryassignment on.sessionsappear.When switching to
.sessionsvia the mode bar (lines 49-51),currentDirectoryis already set. The.onAppearat lines 75-77 sets it again. This is harmless but redundant for the initial switch—though it does help if the view reappears after being removed from the hierarchy.Consider whether you want to keep both assignments for resilience or consolidate. Current behavior is safe.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/RightSidebarPanelView.swift` around lines 67 - 80, The .onAppear block in contentForMode redundantly reassigns sessionIndexStore.currentDirectory from fileExplorerStore.rootPath when switching to .sessions; remove the .onAppear closure (the lines that set sessionIndexStore.currentDirectory) and ensure the mode-switch logic that already sets currentDirectory (where the mode bar switches to .sessions) remains the single place that initializes sessionIndexStore.currentDirectory; if you want reappearance resilience instead, add a short code comment explaining why the duplicate .onAppear assignment is intentionally kept.Sources/SessionIndexStore.swift (1)
384-390: Consider using failableStringinitializer per static analysis hint.SwiftLint flags line 389 for preferring
String(bytes:encoding:). However, the current fallback toString(decoding:as:)for lossy UTF-8 decoding is intentional—it handles malformed UTF-8 gracefully by replacing invalid sequences. This is appropriate for reading arbitrary session files.You can suppress the warning if intentional, or restructure slightly:
♻️ Optional: Restructure to satisfy linter while preserving behavior
- return String(data: data, encoding: .utf8) ?? String(decoding: data, as: UTF8.self) + // Prefer failable init; fall back to lossy decoding for malformed UTF-8 + if let str = String(data: data, encoding: .utf8) { + return str + } + return String(decoding: data, as: UTF8.self)This is functionally identical but may satisfy the linter depending on its heuristics.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/SessionIndexStore.swift` around lines 384 - 390, The linter prefers String(bytes:encoding:) but the current fallback to lossy decoding is intentional; either suppress the SwiftLint rule here or rewrite to use the failable initializer while preserving lossy fallback. Locate the expression "String(data: data, encoding: .utf8) ?? String(decoding: data, as: UTF8.self)" and either (A) replace it with "String(bytes: data, encoding: .utf8) ?? String(decoding: data, as: UTF8.self)" to satisfy the rule while keeping the same behavior, or (B) add a local SwiftLint suppression (// swiftlint:disable:next preferred_string_init) immediately above that line to silence the warning if you want to keep the original form.
🤖 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/SessionIndexStore.swift`:
- Around line 15-21: The displayName computed property is incorrectly wrapping
brand/product names in String(localized:...), so update the displayName logic in
the enum (the displayName var handling the .claude, .codex, .opencode cases) to
return the literal brand names directly (e.g., "Claude Code", "Codex",
"OpenCode") instead of using String(localized:...), removing localization for
these non-translatable brand names.
---
Duplicate comments:
In `@Sources/SessionIndexView.swift`:
- Around line 26-30: SessionIndexView currently calls store.reload() in onAppear
which can duplicate a reload already started by RightSidebarPanelView; update
the guard so reload only runs when truly needed by checking the store's state
(e.g., if store.entries.isEmpty && !store.isLoading && !store.didLoad) or add a
store method like reloadIfNeeded() that encapsulates this logic and call that
from SessionIndexView.onAppear instead of directly calling store.reload().
Ensure you reference SessionIndexView.onAppear and
store.reload()/store.isLoading or the new reloadIfNeeded() helper when making
the change.
---
Nitpick comments:
In `@Sources/RightSidebarPanelView.swift`:
- Around line 67-80: The .onAppear block in contentForMode redundantly reassigns
sessionIndexStore.currentDirectory from fileExplorerStore.rootPath when
switching to .sessions; remove the .onAppear closure (the lines that set
sessionIndexStore.currentDirectory) and ensure the mode-switch logic that
already sets currentDirectory (where the mode bar switches to .sessions) remains
the single place that initializes sessionIndexStore.currentDirectory; if you
want reappearance resilience instead, add a short code comment explaining why
the duplicate .onAppear assignment is intentionally kept.
In `@Sources/SessionIndexStore.swift`:
- Around line 384-390: The linter prefers String(bytes:encoding:) but the
current fallback to lossy decoding is intentional; either suppress the SwiftLint
rule here or rewrite to use the failable initializer while preserving lossy
fallback. Locate the expression "String(data: data, encoding: .utf8) ??
String(decoding: data, as: UTF8.self)" and either (A) replace it with
"String(bytes: data, encoding: .utf8) ?? String(decoding: data, as: UTF8.self)"
to satisfy the rule while keeping the same behavior, or (B) add a local
SwiftLint suppression (// swiftlint:disable:next preferred_string_init)
immediately above that line to silence the warning if you want to keep the
original form.
In `@Sources/SessionIndexView.swift`:
- Around line 353-358: The absoluteTime(_:) function currently creates a new
DateFormatter on each call; cache a DateFormatter as a static/shared instance
(similar to relativeFormatter) and reuse it in absoluteTime(_:) to avoid
repeated allocations—e.g., add a static let absoluteFormatter = DateFormatter()
configured with dateStyle = .medium and timeStyle = .short, then return
absoluteFormatter.string(from: date) from absoluteTime(_:); if you need to
handle runtime locale changes, consider resetting or recreating the formatter
when locale updates occur.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 82ed0b9e-ca01-47e3-b6de-4bb2842dbd5e
⛔ Files ignored due to path filters (9)
Assets.xcassets/AgentIcons/Claude.imageset/Claude.pngis excluded by!**/*.pngAssets.xcassets/AgentIcons/Claude.imageset/Claude@2x.pngis excluded by!**/*.pngAssets.xcassets/AgentIcons/Claude.imageset/Claude@3x.pngis excluded by!**/*.pngAssets.xcassets/AgentIcons/Codex.imageset/Codex.pngis excluded by!**/*.pngAssets.xcassets/AgentIcons/Codex.imageset/Codex@2x.pngis excluded by!**/*.pngAssets.xcassets/AgentIcons/Codex.imageset/Codex@3x.pngis excluded by!**/*.pngAssets.xcassets/AgentIcons/OpenCode.imageset/OpenCode.pngis excluded by!**/*.pngAssets.xcassets/AgentIcons/OpenCode.imageset/OpenCode@2x.pngis excluded by!**/*.pngAssets.xcassets/AgentIcons/OpenCode.imageset/OpenCode@3x.pngis excluded by!**/*.png
📒 Files selected for processing (7)
Assets.xcassets/AgentIcons/Claude.imageset/Contents.jsonAssets.xcassets/AgentIcons/Codex.imageset/Contents.jsonAssets.xcassets/AgentIcons/Contents.jsonAssets.xcassets/AgentIcons/OpenCode.imageset/Contents.jsonSources/RightSidebarPanelView.swiftSources/SessionIndexStore.swiftSources/SessionIndexView.swift
✅ Files skipped from review due to trivial changes (4)
- Assets.xcassets/AgentIcons/Contents.json
- Assets.xcassets/AgentIcons/Codex.imageset/Contents.json
- Assets.xcassets/AgentIcons/OpenCode.imageset/Contents.json
- Assets.xcassets/AgentIcons/Claude.imageset/Contents.json
- Replace per-section drop zones with insertion-gap slots between sections (and one after the last). Gaps adjacent to the dragged section are hidden, so dropping right next to itself is no longer a valid target. Gives a real "drop after the last" affordance too. - Show cwd basename, git branch, and PR number as small chips under each session title. Branch comes from per-entry gitBranch (Claude) or session_meta.payload.git.branch (Codex). PR badge taps open the URL. Adds "Open Pull Request" to the row context menu when present. - Reads file head + tail (64KB / 32KB) so late-arriving pr-link events near the end of long Claude transcripts are picked up cheaply.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
Sources/SessionIndexView.swift (1)
26-30:⚠️ Potential issue | 🟡 MinorSkip the second initial reload.
Sources/RightSidebarPanelView.swift:46-56already kicks offsessionIndexStore.reload()when the sidebar switches to.sessions. Hittingreload()again here while the first scan is still starting just cancels and restarts the same work on first open.💡 Proposed fix
.onAppear { - if store.entries.isEmpty { + if store.entries.isEmpty && !store.isLoading { store.reload() } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/SessionIndexView.swift` around lines 26 - 30, The .onAppear block in SessionIndexView calling store.reload() causes a duplicate initial scan because RightSidebarPanelView already triggers sessionIndexStore.reload() when switching to .sessions; remove the redundant reload in SessionIndexView (the block that checks store.entries.isEmpty and calls store.reload()) so the initial scan is only started by RightSidebarPanelView, or alternatively replace the call with a guard against an existing load (e.g., check a store.isLoading/hasStarted flag before calling store.reload()).
🤖 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/SessionIndexStore.swift`:
- Around line 455-475: In readFileTail(_:byteCap:) the UTF-8 conversion can fail
when the slice starts mid-multibyte character; change the String(data:...,
encoding: .utf8) calls to a lossy decoder so the tail isn’t dropped — e.g.
replace the two returns that use String(data: ..., encoding: .utf8) with
String(decoding: <slicedData>, as: UTF8.self) (use the same sliced range when
trimming with nl and the full data for the final return) so invalid sequences
are replaced and the tail content is preserved.
- Around line 307-313: The loop that collects candidates currently accepts any
.jsonl file; narrow it to only rollout files by adding a filename filter (e.g.
check url.lastPathComponent starts with "rollout-" or match the regex
"^rollout-.*\\.jsonl$") in the same guard that checks pathExtension, so inside
the for-case-let URL loop (the enumerator iteration that appends to candidates)
only URLs whose lastPathComponent matches "rollout-*.jsonl" are considered
before querying resourceValues and appending to candidates.
In `@Sources/SessionIndexView.swift`:
- Around line 398-405: The pull request badge is force-unwrapping pr.url in the
onTap closure which can crash if session metadata contains a malformed URL;
update the onTap for the MetadataChip in SessionIndexView to safely construct
the URL (e.g., guard/if let url = URL(string: pr.url)) and only call
NSWorkspace.shared.open(url) when the URL is valid, otherwise ignore or log the
invalid value—change the closure attached to entry.pullRequest's MetadataChip to
perform this safe optional URL handling instead of using URL(string: pr.url)!.
- Around line 174-176: The drag source sets store.draggedAgent in the .onDrag
closure but never clears it on cancel, leaving sections dimmed; update the view
that calls .onDrag to also clear store.draggedAgent when the drag ends or is
aborted (for example by adding a simultaneous DragGesture with .onEnded {
store.draggedAgent = nil } or equivalent end/cancel handler attached to the same
view), keeping the existing reset in AgentGapDropDelegate.performDrop(...) for
successful drops.
---
Duplicate comments:
In `@Sources/SessionIndexView.swift`:
- Around line 26-30: The .onAppear block in SessionIndexView calling
store.reload() causes a duplicate initial scan because RightSidebarPanelView
already triggers sessionIndexStore.reload() when switching to .sessions; remove
the redundant reload in SessionIndexView (the block that checks
store.entries.isEmpty and calls store.reload()) so the initial scan is only
started by RightSidebarPanelView, or alternatively replace the call with a guard
against an existing load (e.g., check a store.isLoading/hasStarted flag before
calling store.reload()).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f966e742-0853-420c-b2c8-00cfc4d33b05
📒 Files selected for processing (3)
Resources/Localizable.xcstringsSources/SessionIndexStore.swiftSources/SessionIndexView.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- Resources/Localizable.xcstrings
Comment on lines
+307
to
+313
| for case let url as URL in enumerator { | ||
| guard url.pathExtension == "jsonl" else { continue } | ||
| let values = try? url.resourceValues(forKeys: [.contentModificationDateKey, .isRegularFileKey]) | ||
| guard values?.isRegularFile == true, | ||
| let mtime = values?.contentModificationDate else { continue } | ||
| candidates.append((url, mtime)) | ||
| } |
There was a problem hiding this comment.
Restrict Codex indexing to rollout-*.jsonl.
The PR scope says Codex sessions come from ~/.codex/sessions/.../rollout-*.jsonl, but this loop accepts every .jsonl in that tree. If Codex drops sidecar JSONL files alongside rollouts, they'll be parsed as sessions and pollute the list.
💡 Proposed fix
for case let url as URL in enumerator {
- guard url.pathExtension == "jsonl" else { continue }
+ guard url.pathExtension == "jsonl",
+ url.lastPathComponent.hasPrefix("rollout-") else { continue }
let values = try? url.resourceValues(forKeys: [.contentModificationDateKey, .isRegularFileKey])
guard values?.isRegularFile == true,
let mtime = values?.contentModificationDate else { continue }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/SessionIndexStore.swift` around lines 307 - 313, The loop that
collects candidates currently accepts any .jsonl file; narrow it to only rollout
files by adding a filename filter (e.g. check url.lastPathComponent starts with
"rollout-" or match the regex "^rollout-.*\\.jsonl$") in the same guard that
checks pathExtension, so inside the for-case-let URL loop (the enumerator
iteration that appends to candidates) only URLs whose lastPathComponent matches
"rollout-*.jsonl" are considered before querying resourceValues and appending to
candidates.
- Adds a grouping toggle at the top of the Sessions panel: "By folder"
(default) or "By agent". Both modes use the same collapsible sections
and gap-slot drag-to-reorder. Order persists per-mode in UserDefaults
("sessionIndex.directoryOrder" / "sessionIndex.agentOrder"). Newly
discovered folders append by most-recent activity.
- "By folder" mode shows the agent as a small chip on each row so you
can still tell Claude/Codex/OpenCode runs apart at a glance.
- Double-click a row to open a new cmux tab in the session's working
directory and run the bare resume command. Per Lawrence: no override
flags, no MCP handling — let each CLI restore its own settings.
- Claude: claude --resume <uuid>
- Codex: codex resume <uuid>
- OpenCode: opencode --session <id>
- Adds "Resume in New Tab" and "Copy Resume Command" to the row context
menu.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
Sources/SessionIndexView.swift (2)
27-30:⚠️ Potential issue | 🟡 MinorAvoid the initial cancel/restart reload cycle.
RightSidebarPanelViewalready kicks off the first load for this mode. ThisonAppearstill callsstore.reload()wheneverentriesis empty, so the first open can cancel and restart an in-flight scan. Gate it on!store.isLoading(or a one-time initial-load flag).💡 Suggested fix
.onAppear { - if store.entries.isEmpty { + if store.entries.isEmpty && !store.isLoading { store.reload() } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/SessionIndexView.swift` around lines 27 - 30, The onAppear handler in SessionIndexView calls store.reload() whenever store.entries.isEmpty which can cancel an in-flight scan kicked off by RightSidebarPanelView; change the guard so you only call store.reload() when entries is empty AND the store is not already loading (use store.isLoading) or use a one-time initial-load flag; update the onAppear block that references store.entries and store.reload() (or add a private Bool like didPerformInitialLoad checked/set there) to prevent triggering a reload while store.isLoading is true.
216-218:⚠️ Potential issue | 🟡 MinorClear
draggedKeywhen the drag is aborted.The only reset path is
performDrop(info:). If the drag is cancelled or dropped outside a valid gap, the section stays dimmed becausestore.draggedKeynever returns tonil.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/SessionIndexView.swift` around lines 216 - 218, The view sets store.draggedKey inside the .onDrag closure but never clears it if the drag is cancelled, so add a gesture-based cleanup to reset it: attach a DragGesture().onEnded { _ in store.draggedKey = nil } (or equivalent .gesture modifier) to the same view that has .onDrag so that store.draggedKey is cleared when the drag ends/aborts, keeping performDrop(info:) as the successful-drop reset path; update the view that contains .onDrag { DispatchQueue.main.async { store.draggedKey = section.key } ... } to also include this .gesture cleanup.Sources/SessionIndexStore.swift (1)
465-470:⚠️ Potential issue | 🟠 MajorRestrict Codex indexing to rollout files.
The PR contract says Codex sessions come from
rollout-*.jsonl, but this loop still accepts every.jsonlunder~/.codex/sessions. If Codex writes sidecar JSONL files there, they will be indexed as sessions and pollute the sidebar.💡 Suggested fix
for case let url as URL in enumerator { - guard url.pathExtension == "jsonl" else { continue } + guard url.pathExtension == "jsonl", + url.lastPathComponent.hasPrefix("rollout-") else { continue } let values = try? url.resourceValues(forKeys: [.contentModificationDateKey, .isRegularFileKey]) guard values?.isRegularFile == true, let mtime = values?.contentModificationDate else { continue }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/SessionIndexStore.swift` around lines 465 - 470, The loop that collects session files currently accepts any ".jsonl" under the sessions directory (the enumerator loop that appends to candidates), but the PR contract requires only rollout-*.jsonl be indexed; change the guard so you only accept files whose filename matches the rollout pattern (e.g., check url.lastPathComponent.hasPrefix("rollout-") in addition to url.pathExtension == "jsonl") before adding to candidates. Ensure the check occurs inside the same enumerator loop (the block that obtains resourceValues and appends to candidates) so only rollout-*.jsonl files are considered.
🤖 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/SessionIndexStore.swift`:
- Around line 74-80: The cwdLabel computed property currently collapses the home
directory using cwd.hasPrefix(home), which can falsely match on path segments;
change the check to only collapse when the cwd is exactly the home directory
(cwd == home) or when it begins with the home directory plus a path separator
(cwd.hasPrefix(home + "/")). In the cwdLabel getter (the var cwdLabel, the
guarded cwd and the use of NSHomeDirectory()), apply that
exact-or-prefix-with-slash logic and handle the exact-equality case (return "~")
and the prefix-with-slash case (return "~" + cwd.dropFirst(home.count)) to avoid
producing incorrect labels.
- Around line 55-63: The resumeCommand computed property interpolates external
sessionId directly into shell commands (see resumeCommand and its agent cases),
which allows spaces or shell metacharacters to change execution; fix by avoiding
raw string interpolation—either use an argv-based launch API if available or
implement proper shell-quoting for sessionId before inserting it (e.g., add a
helper like shellQuoted(_:) and use it when building the command for each agent
case) so the argument is passed safely.
In `@Sources/SessionIndexView.swift`:
- Around line 103-121: The SectionReorderGap is passing a visible-array
insertIndex (from sections) into persisted-order APIs
(SessionIndexStore.currentIndex(of:) and moveSection(_:toInsertIndex:)), which
uses full directoryOrder and will misplace hidden folders; update the gap
handling to translate the visible gap to an absolute persisted index before
calling moveSection: compute the target persisted index by locating the
neighboring visible section keys in directoryOrder (e.g., find the next visible
key in sections and use SessionIndexStore.currentIndex(of:) for that key, or if
no next key exists use directoryOrder.count as append), and use that translated
index when invoking moveSection(_:toInsertIndex:); apply the same translation
fix for the other occurrences around the 309-317 region.
- Around line 331-367: The row currently uses a Button with an action (open())
plus .simultaneousGesture(TapGesture(count:2)) which causes a double-click to
trigger both open() and onResume(entry); replace that pattern with an exclusive
tap gesture so single vs double taps are dispatched exclusively. Remove the
Button action/.simultaneousGesture and instead attach an ExclusiveGesture built
from TapGesture(count:2).exclusively(before: TapGesture()) to the same view
(e.g., the VStack/contentShape), and in the gesture's onEnded handler switch the
ExclusiveGesture result to call onResume(entry) for the double-tap case and
open() for the single-tap case (keep existing helpers like open(),
onResume(entry), metadataLine, helpText and the hover handling).
---
Duplicate comments:
In `@Sources/SessionIndexStore.swift`:
- Around line 465-470: The loop that collects session files currently accepts
any ".jsonl" under the sessions directory (the enumerator loop that appends to
candidates), but the PR contract requires only rollout-*.jsonl be indexed;
change the guard so you only accept files whose filename matches the rollout
pattern (e.g., check url.lastPathComponent.hasPrefix("rollout-") in addition to
url.pathExtension == "jsonl") before adding to candidates. Ensure the check
occurs inside the same enumerator loop (the block that obtains resourceValues
and appends to candidates) so only rollout-*.jsonl files are considered.
In `@Sources/SessionIndexView.swift`:
- Around line 27-30: The onAppear handler in SessionIndexView calls
store.reload() whenever store.entries.isEmpty which can cancel an in-flight scan
kicked off by RightSidebarPanelView; change the guard so you only call
store.reload() when entries is empty AND the store is not already loading (use
store.isLoading) or use a one-time initial-load flag; update the onAppear block
that references store.entries and store.reload() (or add a private Bool like
didPerformInitialLoad checked/set there) to prevent triggering a reload while
store.isLoading is true.
- Around line 216-218: The view sets store.draggedKey inside the .onDrag closure
but never clears it if the drag is cancelled, so add a gesture-based cleanup to
reset it: attach a DragGesture().onEnded { _ in store.draggedKey = nil } (or
equivalent .gesture modifier) to the same view that has .onDrag so that
store.draggedKey is cleared when the drag ends/aborts, keeping
performDrop(info:) as the successful-drop reset path; update the view that
contains .onDrag { DispatchQueue.main.async { store.draggedKey = section.key }
... } to also include this .gesture cleanup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e9d1dbaf-0435-4ea5-943b-c7ea0c60d767
📒 Files selected for processing (5)
Resources/Localizable.xcstringsSources/ContentView.swiftSources/RightSidebarPanelView.swiftSources/SessionIndexStore.swiftSources/SessionIndexView.swift
✅ Files skipped from review due to trivial changes (2)
- Resources/Localizable.xcstrings
- Sources/ContentView.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- Sources/RightSidebarPanelView.swift
Comment on lines
+331
to
+367
| Button { | ||
| open() | ||
| } label: { | ||
| VStack(alignment: .leading, spacing: 3) { | ||
| Text(entry.displayTitle) | ||
| .font(.system(size: 12)) | ||
| .foregroundColor(.primary) | ||
| .lineLimit(2) | ||
| .truncationMode(.tail) | ||
| .multilineTextAlignment(.leading) | ||
| metadataLine | ||
| HStack(spacing: 6) { | ||
| Spacer(minLength: 0) | ||
| Text(relativeTime(entry.modified)) | ||
| .font(.system(size: 10).monospacedDigit()) | ||
| .foregroundColor(.secondary) | ||
| } | ||
| } | ||
| .padding(.horizontal, 10) | ||
| .padding(.leading, 18) | ||
| .padding(.vertical, 5) | ||
| .frame(maxWidth: .infinity, alignment: .leading) | ||
| .contentShape(Rectangle()) | ||
| .background( | ||
| RoundedRectangle(cornerRadius: 4, style: .continuous) | ||
| .fill(isHovered ? Color.primary.opacity(0.06) : Color.clear) | ||
| .padding(.horizontal, 6) | ||
| ) | ||
| } | ||
| .buttonStyle(.plain) | ||
| .onHover { isHovered = $0 } | ||
| .help(helpText) | ||
| .simultaneousGesture( | ||
| TapGesture(count: 2).onEnded { | ||
| if let onResume { onResume(entry) } | ||
| } | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
On macOS SwiftUI, when a Button has an action and also a simultaneousGesture(TapGesture(count: 2)), does a double-click invoke both the button action and the double-tap gesture?
💡 Result:
Yes, a double-click on a SwiftUI Button that has both an action closure and .simultaneousGesture(TapGesture(count: 2)) will invoke both the button's action and the double-tap gesture handler on macOS.
Citations:
- 1: https://www.hackingwithswift.com/quick-start/swiftui/how-to-make-two-gestures-recognize-at-the-same-time-using-simultaneousgesture
- 2: https://hackingwithswift.com/quick-start/swiftui/how-to-make-two-gestures-recognize-at-the-same-time-using-simultaneousgesture
- 3: https://steipete.me/posts/2021/supporting-both-tap-and-longpress-on-button-in-swiftui
- 4: https://stackoverflow.com/questions/68309352/detect-if-user-tapped-the-screen-including-buttons-in-swiftui
🏁 Script executed:
# Check the file exists and read the specific lines
wc -l Sources/SessionIndexView.swiftRepository: manaflow-ai/cmux
Length of output: 94
🏁 Script executed:
# Read lines 331-367 to verify the current state
sed -n '331,367p' Sources/SessionIndexView.swiftRepository: manaflow-ai/cmux
Length of output: 1437
🏁 Script executed:
# Look for context around this code to understand the functions and intended behavior
sed -n '300,380p' Sources/SessionIndexView.swiftRepository: manaflow-ai/cmux
Length of output: 3002
🏁 Script executed:
# Find the open() function in SessionRow context
sed -n '300,400p' Sources/SessionIndexView.swift | grep -A 5 "func open"Repository: manaflow-ai/cmux
Length of output: 42
🏁 Script executed:
# Search for the full SessionRow implementation to find the open() function
rg "func open\(\)" Sources/SessionIndexView.swift -A 3Repository: manaflow-ai/cmux
Length of output: 248
🏁 Script executed:
# Get broader view of SessionRow to find all its methods
sed -n '310,450p' Sources/SessionIndexView.swiftRepository: manaflow-ai/cmux
Length of output: 5273
Separate single-click open from double-click resume.
On macOS, when a SwiftUI Button has both an action and a .simultaneousGesture(TapGesture(count: 2)), a double-click invokes both handlers. This row will therefore open the file/directory and resume in a new tab on double-click. Use a single click-dispatch path that decides between open() and onResume(entry) based on click count.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/SessionIndexView.swift` around lines 331 - 367, The row currently
uses a Button with an action (open()) plus
.simultaneousGesture(TapGesture(count:2)) which causes a double-click to trigger
both open() and onResume(entry); replace that pattern with an exclusive tap
gesture so single vs double taps are dispatched exclusively. Remove the Button
action/.simultaneousGesture and instead attach an ExclusiveGesture built from
TapGesture(count:2).exclusively(before: TapGesture()) to the same view (e.g.,
the VStack/contentShape), and in the gesture's onEnded handler switch the
ExclusiveGesture result to call onResume(entry) for the double-tap case and
open() for the single-tap case (keep existing helpers like open(),
onResume(entry), metadataLine, helpText and the hover handling).
There was a problem hiding this comment.
3 issues found across 5 files (changes from recent commits).
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="Sources/SessionIndexView.swift">
<violation number="1" location="Sources/SessionIndexView.swift:103">
P2: Drag-reorder indices are in filtered-section space but `moveSection(_:toInsertIndex:)` operates on the full persisted `directoryOrder`/`agentOrder` array. When "This folder only" hides some directory sections, the visible gap index won't match the correct position in the backing array, causing sections to land in the wrong slot and corrupting the saved order. Translate the visible insert index to an absolute persisted-array index before calling `moveSection`.</violation>
<violation number="2" location="Sources/SessionIndexView.swift:363">
P2: Double‑clicking the row can trigger both the Button’s open() action and the resume action because the double‑tap is attached via simultaneousGesture, which does not suppress the Button tap gesture.</violation>
</file>
<file name="Sources/SessionIndexStore.swift">
<violation number="1" location="Sources/SessionIndexStore.swift:58">
P1: Shell injection risk: `sessionId` is read from external JSON/SQLite data and interpolated directly into a shell command string (`claude --resume \(sessionId)`). This command is then passed as `initialTerminalCommand` to the terminal. A session ID containing shell metacharacters (spaces, semicolons, backticks, etc.) will alter the executed command. Shell-quote the `sessionId` before interpolation, or use an argv-based launch API.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| return ScrollView { | ||
| LazyVStack(alignment: .leading, spacing: 0) { | ||
| ForEach(Array(sections.enumerated()), id: \.element.key) { index, section in | ||
| SectionReorderGap(insertIndex: index, store: store) |
There was a problem hiding this comment.
P2: Drag-reorder indices are in filtered-section space but moveSection(_:toInsertIndex:) operates on the full persisted directoryOrder/agentOrder array. When "This folder only" hides some directory sections, the visible gap index won't match the correct position in the backing array, causing sections to land in the wrong slot and corrupting the saved order. Translate the visible insert index to an absolute persisted-array index before calling moveSection.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/SessionIndexView.swift, line 103:
<comment>Drag-reorder indices are in filtered-section space but `moveSection(_:toInsertIndex:)` operates on the full persisted `directoryOrder`/`agentOrder` array. When "This folder only" hides some directory sections, the visible gap index won't match the correct position in the backing array, causing sections to land in the wrong slot and corrupting the saved order. Translate the visible insert index to an absolute persisted-array index before calling `moveSection`.</comment>
<file context>
@@ -84,64 +96,90 @@ struct SessionIndexView: View {
- agent: agent,
- entries: entries,
+ ForEach(Array(sections.enumerated()), id: \.element.key) { index, section in
+ SectionReorderGap(insertIndex: index, store: store)
+ IndexSectionView(
+ section: section,
</file context>
Contributor
Author
There was a problem hiding this comment.
Fixed (same change as the coderabbit thread on this issue): swapped to neighbor-key anchored moveSection(_:before:) so visible-vs-persisted index space is no longer mixed.
- Single-line row: title + relative time only. Drop folder/branch/PR chips and the agent chip. Remove section count badge and chevron; section header is just folder/agent icon + name. - Cap each section at 5 rows by default with a "Show more" toggle. Empty sections show "No chats". - Resume command now goes through the user's normal shell via Ghostty's initial_input (typed into the shell after it starts) instead of initialCommand (which exec'd the command directly without sourcing zshrc, so PATH was missing). Plumbs initialTerminalInput through TerminalSurface, TerminalPanel, Workspace, and TabManager.addWorkspace, and adds initialInput to Workspace.newTerminalSurface. - Smart placement on resume: if the focused workspace's terminal is already in the same cwd, open a new tab inside that workspace via newTerminalSurface; otherwise spin up a new workspace. - Fix Codex "Untitled session": Codex rollouts have a huge base_instructions block in session_meta plus an AGENTS.md user message before the first real user_message, so 64 KB head reads ran out of buffer. Switch Codex extraction to a streaming line reader that stops early once title/cwd/branch/sessionId are found, with a 4 MB cap and a response_item user-message fallback.
There was a problem hiding this comment.
3 issues found across 8 files (changes from recent commits).
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="Sources/ContentView.swift">
<violation number="1" location="Sources/ContentView.swift:2991">
P1: Resume routing can reuse a remote workspace based only on cwd text match, so session-resume commands may be sent to the wrong environment.</violation>
<violation number="2" location="Sources/ContentView.swift:3000">
P2: Only return after `newTerminalSurface` succeeds; otherwise fall back to `addWorkspace` so resume cannot silently no-op.</violation>
</file>
<file name="Sources/SessionIndexStore.swift">
<violation number="1" location="Sources/SessionIndexStore.swift:557">
P2: Codex scanning does not observe task cancellation during the long file-streaming loop, so canceled reloads can continue reading/parsing JSONL until the scan completes.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
+2991
to
+2994
| if pwdMatches, | ||
| let workspace = selected, | ||
| let paneId = workspace.bonsplitController.focusedPaneId { | ||
| workspace.newTerminalSurface( |
There was a problem hiding this comment.
P1: Resume routing can reuse a remote workspace based only on cwd text match, so session-resume commands may be sent to the wrong environment.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/ContentView.swift, line 2991:
<comment>Resume routing can reuse a remote workspace based only on cwd text match, so session-resume commands may be sent to the wrong environment.</comment>
<file context>
@@ -2975,6 +2972,40 @@ struct ContentView: View {
+ return lhs == rhs
+ }()
+
+ if pwdMatches,
+ let workspace = selected,
+ let paneId = workspace.bonsplitController.focusedPaneId {
</file context>
Suggested change
| if pwdMatches, | |
| let workspace = selected, | |
| let paneId = workspace.bonsplitController.focusedPaneId { | |
| workspace.newTerminalSurface( | |
| if pwdMatches, | |
| let workspace = selected, | |
| !workspace.isRemoteWorkspace, | |
| let paneId = workspace.bonsplitController.focusedPaneId { | |
| workspace.newTerminalSurface( | |
| inPane: paneId, | |
| focus: true, | |
| workingDirectory: targetCwd, | |
| initialInput: inputWithReturn | |
| ) | |
| return | |
| } |
Contributor
Author
There was a problem hiding this comment.
Fixed: resumeSession now skips the cwd-match path when the selected workspace is remote, so a local-indexed session is never sent into a remote shell just because the path string coincides.
There was a problem hiding this comment.
Thanks for the feedback! I've saved this as a new learning to improve future reviews.
| var leftover = Data() | ||
| var totalRead = 0 | ||
| let chunkSize = 64 * 1024 | ||
| while totalRead < maxBytes { |
There was a problem hiding this comment.
P2: Codex scanning does not observe task cancellation during the long file-streaming loop, so canceled reloads can continue reading/parsing JSONL until the scan completes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/SessionIndexStore.swift, line 557:
<comment>Codex scanning does not observe task cancellation during the long file-streaming loop, so canceled reloads can continue reading/parsing JSONL until the scan completes.</comment>
<file context>
@@ -519,13 +520,66 @@ final class SessionIndexStore: ObservableObject {
+ var leftover = Data()
+ var totalRead = 0
+ let chunkSize = 64 * 1024
+ while totalRead < maxBytes {
+ let chunk: Data
+ if #available(macOS 10.15.4, *) {
</file context>
Contributor
Author
There was a problem hiding this comment.
Fixed: loadCodexEntriesFromDisk now checks Task.isCancelled inside the per-file loop, so cancelled scans stop promptly instead of finishing the batch.
| workingDirectory: targetCwd, | ||
| initialInput: inputWithReturn | ||
| ) | ||
| return |
There was a problem hiding this comment.
P2: Only return after newTerminalSurface succeeds; otherwise fall back to addWorkspace so resume cannot silently no-op.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/ContentView.swift, line 3000:
<comment>Only return after `newTerminalSurface` succeeds; otherwise fall back to `addWorkspace` so resume cannot silently no-op.</comment>
<file context>
@@ -2975,6 +2972,40 @@ struct ContentView: View {
+ workingDirectory: targetCwd,
+ initialInput: inputWithReturn
+ )
+ return
+ }
+
</file context>
Contributor
Author
There was a problem hiding this comment.
Acknowledged. newTerminalSurface doesn't currently report success, so a real fallback would need a new return signal. The path normally hits the early-return only when paneId is missing (which we already guard with if let paneId = workspace.bonsplitController.focusedPaneId). Tracking as follow-up if we see actual silent no-ops in dogfood.
There was a problem hiding this comment.
Thanks for the feedback! I've saved this as a new learning to improve future reviews.
Resume now passes the saved per-session settings as CLI flags so the
restored session uses the same model / permission / sandbox / effort
the original was running with.
- Claude: claude --resume <id> --model <m> --permission-mode <mode>
(model from assistant.message.model with the [1m] suffix
stripped; permission mode from per-entry permissionMode)
- Codex: codex resume <id> -m <m> -a <approval> -s <sandbox>
-c model_reasoning_effort=<effort>
(all from turn_context.payload; effort uses -c since codex
has no first-class --effort flag)
- OpenCode: opencode --session <id> -m <provider/model> --agent <name>
(pulled from the most recent assistant message via a
correlated subquery on the messages table)
Effort is not stored in Claude's local jsonl, so --effort is omitted
for Claude. Values are shell-quoted when they contain unsafe characters
(the OpenCode agent name, e.g. "Sisyphus (Ultraworker)", needs this).
- Bring back collapsible sections. Click anywhere on a section header to toggle; a small chevron at the end indicates state. Default is expanded. Collapse and "Show more" are independent. - Smart resume placement was checking the focused panel's `directory` field, which isn't reliably updated. Switch to the workspace's live `currentDirectory` (kept in sync via panel directory updates), so double-click correctly opens a new tab inside the current workspace when the cwd matches.
- "Show N more" now opens a popover anchored to the leading edge of the button. Popover has a search field that filters by title, cwd, or branch, and a 420pt-tall scrollable list of all session entries for that section. Double-click in the popover resumes and dismisses; rows are draggable just like in the main list. - Section rows in the main list are no longer rendered past the 5-row limit (they live in the popover instead), so the list stays short and the scrollbar size shouldn't shift around as you scroll. - SessionRow + PopoverRow expose the resume command as a draggable text payload. Dropping a session onto an existing terminal surface inserts the command via the terminal's existing text-drop handler. Real drop-on-bonsplit-to-create-a-new-pane needs a hook in the bonsplit submodule and isn't wired yet.
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
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="Sources/SessionIndexView.swift">
<violation number="1" location="Sources/SessionIndexView.swift:627">
P2: Sessions displayed in the new popover no longer have the per-row context menu actions available in normal rows.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @@ -0,0 +1,664 @@ | |||
| import AppKit | |||
There was a problem hiding this comment.
P2: Sessions displayed in the new popover no longer have the per-row context menu actions available in normal rows.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/SessionIndexView.swift, line 627:
<comment>Sessions displayed in the new popover no longer have the per-row context menu actions available in normal rows.</comment>
<file context>
@@ -480,3 +505,160 @@ private struct SessionRow: View {
+ }
+}
+
+private struct PopoverRow: View {
+ let entry: SessionEntry
+ let onActivate: () -> Void
</file context>
Contributor
Author
There was a problem hiding this comment.
Fixed: PopoverRow now has the same right-click context menu as full SessionRow. Both call a shared sessionRowMenuItems view-builder so they can't drift.
There was a problem hiding this comment.
Thanks for addressing this—glad the popover now shares the same context menu via the common builder.
- Resume routing: skip cwd-match when the selected workspace is remote so a local-indexed session is never sent into a remote shell just because the path string coincides. - syncFileExplorerDirectory: clear sessionIndexStore.currentDirectory on the early-return paths so a previous tab's cwd doesn't stick around as a stale filter. - decodeClaudeProjectDir: validate against the filesystem before returning so directories with a real "-" segment (e.g. "my-cool-project") don't decode to a wrong path; callers fall back to the JSONL `cwd` field. - cwdLabel: compare home prefix on a path boundary so "/Users/al" doesn't match a home of "/Users/alice". - absoluteTime: cache DateFormatter as a static so tooltip hover doesn't reallocate each access. - Section reorder: anchor moves to a visible neighbor key instead of a positional index. Hidden sections (filtered by scope) keep their relative position to visible neighbors instead of getting shuffled. - SessionIndexView appear: skip reload when the store is already loading, so the mode-toggle's reload isn't immediately cancelled and restarted. - Codex disk loader: check Task.isCancelled inside the per-file loop so a cancelled scan stops promptly instead of finishing the whole batch. - Popover rows: add the same right-click context menu as full SessionRow. Extracted the menu items into a shared `sessionRowMenuItems` builder and the row actions into inline closures so both rows stay in sync.
- Switch popover back to ScrollView + LazyVStack. The List + frame(420) approach kept colliding with the surrounding popover sizing; LazyVStack is already lazy enough for our result sizes. - WorkspaceUnitTests: mock overrides of makeWorkspaceForCreation must include the new initialTerminalInput parameter or they fail to override.
This was referenced Apr 19, 2026
Closed
This was referenced Apr 20, 2026
This was referenced Apr 29, 2026
This was referenced May 6, 2026
This was referenced May 16, 2026
rodchristiansen
pushed a commit
to rodchristiansen/cmux
that referenced
this pull request
Sep 2, 2026
* Add Sessions panel to right sidebar Adds a Sessions mode to the right sidebar (cmd-option-b) alongside the existing file tree. A two-button toggle at the top of the panel switches between Files and Sessions. Sessions lists recent agent runs from: - Claude Code (~/.claude/projects/*/*.jsonl) - Codex (~/.codex/sessions/YYYY/MM/DD/rollout-*.jsonl) - OpenCode (~/.local/share/opencode/opencode.db, snapshotted before read) Each row shows the first user prompt as a title, the working directory, and a relative timestamp. Right-click for Open / Reveal in Finder / Copy File Path / Open Working Directory. A "This folder only" filter scopes the list to the current FileExplorer root. Mode selection persists in UserDefaults under "rightSidebar.mode". * Use brand icons, draggable section reorder, header tweak - Replace SF Symbols in agent section headers with the official Claude, OpenAI (Codex), and OpenCode brand marks. Icons rendered from upstream SVGs (simpleicons.org and opencode.ai/favicon.svg) at @1x/@2x/@3x and bundled under Assets.xcassets/AgentIcons. - Make agent sections drag-reorderable. Drag a section header onto another to put it first; the order persists in UserDefaults under "sessionIndex.agentOrder". - Right-sidebar mode bar: shift the toggle 2px left and bump header height from 30 to 31. * Reorder gaps + cwd/branch/PR chips on session rows - Replace per-section drop zones with insertion-gap slots between sections (and one after the last). Gaps adjacent to the dragged section are hidden, so dropping right next to itself is no longer a valid target. Gives a real "drop after the last" affordance too. - Show cwd basename, git branch, and PR number as small chips under each session title. Branch comes from per-entry gitBranch (Claude) or session_meta.payload.git.branch (Codex). PR badge taps open the URL. Adds "Open Pull Request" to the row context menu when present. - Reads file head + tail (64KB / 32KB) so late-arriving pr-link events near the end of long Claude transcripts are picked up cheaply. * Group sessions by folder or agent + double-click resume - Adds a grouping toggle at the top of the Sessions panel: "By folder" (default) or "By agent". Both modes use the same collapsible sections and gap-slot drag-to-reorder. Order persists per-mode in UserDefaults ("sessionIndex.directoryOrder" / "sessionIndex.agentOrder"). Newly discovered folders append by most-recent activity. - "By folder" mode shows the agent as a small chip on each row so you can still tell Claude/Codex/OpenCode runs apart at a glance. - Double-click a row to open a new cmux tab in the session's working directory and run the bare resume command. Per Lawrence: no override flags, no MCP handling — let each CLI restore its own settings. - Claude: claude --resume <uuid> - Codex: codex resume <uuid> - OpenCode: opencode --session <id> - Adds "Resume in New Tab" and "Copy Resume Command" to the row context menu. * Drop single-click open() on session rows; double-click resumes only * Minimal session rows + smart placement + Codex title fix - Single-line row: title + relative time only. Drop folder/branch/PR chips and the agent chip. Remove section count badge and chevron; section header is just folder/agent icon + name. - Cap each section at 5 rows by default with a "Show more" toggle. Empty sections show "No chats". - Resume command now goes through the user's normal shell via Ghostty's initial_input (typed into the shell after it starts) instead of initialCommand (which exec'd the command directly without sourcing zshrc, so PATH was missing). Plumbs initialTerminalInput through TerminalSurface, TerminalPanel, Workspace, and TabManager.addWorkspace, and adds initialInput to Workspace.newTerminalSurface. - Smart placement on resume: if the focused workspace's terminal is already in the same cwd, open a new tab inside that workspace via newTerminalSurface; otherwise spin up a new workspace. - Fix Codex "Untitled session": Codex rollouts have a huge base_instructions block in session_meta plus an AGENTS.md user message before the first real user_message, so 64 KB head reads ran out of buffer. Switch Codex extraction to a streaming line reader that stops early once title/cwd/branch/sessionId are found, with a 4 MB cap and a response_item user-message fallback. * Inject per-session flags into resume command Resume now passes the saved per-session settings as CLI flags so the restored session uses the same model / permission / sandbox / effort the original was running with. - Claude: claude --resume <id> --model <m> --permission-mode <mode> (model from assistant.message.model with the [1m] suffix stripped; permission mode from per-entry permissionMode) - Codex: codex resume <id> -m <m> -a <approval> -s <sandbox> -c model_reasoning_effort=<effort> (all from turn_context.payload; effort uses -c since codex has no first-class --effort flag) - OpenCode: opencode --session <id> -m <provider/model> --agent <name> (pulled from the most recent assistant message via a correlated subquery on the messages table) Effort is not stored in Claude's local jsonl, so --effort is omitted for Claude. Values are shell-quoted when they contain unsafe characters (the OpenCode agent name, e.g. "Sisyphus (Ultraworker)", needs this). * Per-section collapse + use live workspace cwd for resume placement - Bring back collapsible sections. Click anywhere on a section header to toggle; a small chevron at the end indicates state. Default is expanded. Collapse and "Show more" are independent. - Smart resume placement was checking the focused panel's `directory` field, which isn't reliably updated. Switch to the workspace's live `currentDirectory` (kept in sync via panel directory updates), so double-click correctly opens a new tab inside the current workspace when the cwd matches. * Show-more popover with search + draggable session rows - "Show N more" now opens a popover anchored to the leading edge of the button. Popover has a search field that filters by title, cwd, or branch, and a 420pt-tall scrollable list of all session entries for that section. Double-click in the popover resumes and dismisses; rows are draggable just like in the main list. - Section rows in the main list are no longer rendered past the 5-row limit (they live in the popover instead), so the list stays short and the scrollbar size shouldn't shift around as you scroll. - SessionRow + PopoverRow expose the resume command as a draggable text payload. Dropping a session onto an existing terminal surface inserts the command via the terminal's existing text-drop handler. Real drop-on-bonsplit-to-create-a-new-pane needs a hook in the bonsplit submodule and isn't wired yet. * Wire session drag into bonsplit drop overlays Sessions now show the same blue insert / split overlays as dragging an existing tab, and dropping creates a brand new terminal at the chosen destination (insert into pane, or split horizontally / vertically) with the resume command running in it. Approach: - The session row's NSItemProvider exposes a com.splittabbar.tabtransfer payload alongside the public.utf8-plain-text fallback. The tabtransfer payload is shaped exactly like Bonsplit.TabTransferData (mirrors the TabItem CodingKeys), with a synthetic UUID as the tab id and the current process id so isFromCurrentProcess passes. - That synthetic UUID is registered in a new SessionDragRegistry singleton paired with the originating SessionEntry. Auto-expires after 60s if the drag is cancelled. - Workspace.handleExternalTabDrop checks the registry first: if the request's tab id maps to a pending session, it spawns a new terminal at the destination instead of trying to move a non-existent tab. - Adds Workspace.splitPaneWithNewTerminal(targetPane:orientation: insertFirst:workingDirectory:initialInput:) so the .split destination case can create a brand-new terminal in a freshly split pane. The existing terminal text-drop handler still sees public.utf8-plain-text, so dropping on a terminal that's not on a pane edge inserts the resume command as text (unchanged behavior). * Use mirror Codable structs for session drag payload + log Switch the session-drag NSItemProvider from a hand-built JSONSerialization dict to a Codable struct that mirrors Bonsplit.TabTransferData / TabItem field-by-field. This guarantees the encoded JSON shape matches what Bonsplit's external-drop decoder expects, so validateDrop accepts the payload and the blue insert/split overlays appear when the cursor is over a pane. Also instrument the drag start with dlog and set suggestedName so the drag preview shows the session title. * Mirror session drag payload onto the system .drag pasteboard SwiftUI's NSItemProvider → drag pasteboard bridge doesn't always surface custom UTTypes on NSPasteboard(name: .drag), so bonsplit's decodeTransfer(from: info), which reads from that pasteboard directly, sees no com.splittabbar.tabtransfer payload and validateDrop returns false — meaning no blue insert/split overlays. Mirror our encoded MirrorTabTransferData blob onto the .drag pasteboard manually right after onDrag fires, using addTypes (preserves existing SwiftUI-provided types) + setData. This matches what bonsplit's own tab drag relies on, just done by hand since we're entering bonsplit from outside. Also adds a session.drag.pasteboard debug log so we can confirm the type and byte count actually land on the pasteboard. * Defer to bonsplit on terminal hover + use Codex thread_name as title Drag fixes: - Terminal surface now rejects drags whose pasteboard contains com.splittabbar.tabtransfer or com.cmux.sidebar-tab-reorder. Without this, dragging a session row over a terminal made the terminal claim the drop and insert the resume command as text, hiding bonsplit's blue insert/split overlays. The terminal's existing text/file drop handling for non-tab drags is unchanged. Codex titles: - Recent Codex versions emit `event_msg.thread_name_updated` with a short generated `thread_name` (e.g. "Explain PostHog metrics"). Capture it during streaming and prefer it over the first user_message fallback. Older sessions without thread_name keep using the first user message. * Drop terminal-text fallback from session drag + tighten section padding Drag fix: - Stop registering public.utf8-plain-text on the session drag's NSItemProvider. The terminal NSDraggingDestination accepts .string, so as long as our drag carried text, the terminal was hit-tested for every drag-update event and bonsplit's SwiftUI .onDrop overlay never got a chance to fire — the AppKit drag system stayed "owned" by the terminal NSView, even when draggingEntered returned []. With only com.splittabbar.tabtransfer on the provider (still mirrored onto the drag pasteboard), the terminal isn't a candidate destination and bonsplit's blue insert/split overlays render across the whole pane including its center. - Right-click → Copy Resume Command / Resume in New Tab still cover the "type into existing terminal" case if needed. Padding: - Section header vertical padding 6 → 3. - SectionReorderGap height 8 → 4. - Trailing Spacer in expanded section minLength 6 → 2. Removed temporary session.drag.* debug logs from the previous round — the log already proved the pasteboard mirror works, so they're done their job. * Host Show-more popover in a real NSPopover so search field accepts focus SwiftUI's `.popover()` doesn't reliably let an embedded TextField become first responder in cmux's focus-managed environment — the terminal's applyFirstResponderIfNeeded path keeps grabbing focus back, so clicking the search field did nothing. Switch to a custom NSPopover hosted via NSViewRepresentable, modeled on UpdatePill's existing pattern. Uses .transient behavior, anchors to the "Show N more" button via a hidden NSView, and hosts SectionPopoverView through NSHostingController. The popover gets its own key window which the search TextField can claim cleanly. * Don't steal first responder from a text editor Root cause from the dogfood log: the popover's NSPopover hosting window became key and its TextField field editor was first responder, but ~35ms later a deferred applyFirstResponderIfNeeded ran on the terminal's GhosttySurfaceScrollView. The window.isKeyWindow guard didn't reject — NSPopover propagates first responder through the parent in a way that keeps that guard true — so window.makeFirstResponder(surfaceView) ran and clobbered the field editor. Add an explicit early-return when window.firstResponder is an NSText (NSTextField/NSTextView/field editor). The terminal surface uses its own GhosttyNSView for input and never NSText, so this can't suppress a legitimate terminal-focus restore. Removed the temporary session.popover.* probes — they did their job. The new SKIP-on-NSText log is permanent (low volume, high signal). * Stop key-routing repair from stealing from popover text fields Root cause from the dogfood log: the popover's TextField uses macOS's shared field editor, which gets registered as firstResponder of the parent (cmux main) window. The first key event hits cmux_sendEvent on the main window and runs repairFocusedTerminalKeyboardRoutingIfNeeded. responderHasViableKeyRoutingOwner returns false because the field editor's owning view (the popover NSTextField) has ownerView.window !== window, so the repair logic calls ensureFocus on the focused terminal panel — clobbering the field editor's responder and routing the key to the terminal. Add an early-return when window.firstResponder is NSText. Symmetric with the earlier guard in applyFirstResponderIfNeeded. Terminals use GhosttyNSView, not NSText, so this can't suppress legitimate terminal repair. Also added Escape-to-close to the popover via an NSEvent local monitor so users can dismiss without clicking outside. Removed temporary session.popover.* and session.probe.* debug lines. * Async + cached search filter so typing isn't laggy Previously every keystroke recomputed the filter on the main thread and re-lowercased every entry's title/cwd/branch on every comparison, which became visibly slow once the popover had a few hundred sessions. Three changes: - Pre-lowercase a single combined searchable string per entry once on popover appear, so each filter pass is a cheap substring scan. - Run the filter on a detached background task at userInitiated priority and drop the result into a @State the body reads, so typing never blocks on the scan. - 30ms debounce on each new query: a flurry of fast keystrokes collapses into one filter pass instead of N. New keystrokes cancel the in-flight task so we don't render stale results. Empty query skips the task entirely and snaps back to the full list. * Add bottom padding to Show-more popover scroll content * Make session rows equatable to skip body re-eval during scroll LazyVStack's biggest scroll-perf cost in this view was SwiftUI re-evaluating each row's body on every scroll tick — rebuilding the .contextMenu, .onDrag preview, .help() text, and the RelativeDateTimeFormatter call for every visible row, every frame. Two changes: - SessionRow and PopoverRow now conform to Equatable, comparing on entry only (closures and per-instance @State aren't compared, which is correct: closures come from stable parent state and @State stays per-instance regardless). - ForEach call sites apply .equatable() so SwiftUI uses the conformance to short-circuit body re-eval when the entry hasn't changed — exactly what scroll causes. * Show agent brand icon on each session row * Bump per-agent session cap from 60 to 200 * Drop in-memory cap to 30; deep-search disk + DB on popover query Replaces the 200-cap workaround with a proper "fast cache + on-demand deep search" architecture. - perAgentLimit drops from 200 → 30. Initial scan stays cheap and the main session list always shows just the most-recent rows. - Section "Show more" popover no longer filters the cached list. Empty query shows the cached entries; non-empty query kicks off SessionIndexStore.searchSessions on a detached background task with a 200ms debounce. - searchSessions implementation: - Claude: enumerates ~/.claude/projects/*/*.jsonl, sorts by mtime desc, scans up to 1500 files reading 128 KB head + 32 KB tail each, case-insensitive substring match, parses metadata for hits, caps at 200 results. - Codex: same idea over ~/.codex/sessions/**/rollout-*.jsonl. - OpenCode: SQL with WHERE LOWER(s.title) LIKE ? OR LOWER(s.directory) LIKE ?, optionally AND s.directory = ? when the section is a folder bucket. - Scope is derived from the section key: "agent:claude" → search just Claude; "dir:/foo/bar" → fan out across all three agents and filter by cwd. - All static helpers used from the detached task are marked nonisolated so they can be called outside the @mainactor store. - Popover shows a "Searching…" spinner row while the deep search runs. No third-party dep — Foundation file enumeration + Foundation string range(of:options:.caseInsensitive) for the substring scan, plus the existing SQLite3 module already linked for OpenCode. * Paginate Show-more popover with load-more on scroll Replaces the "fetch up to 200 in one shot" popover with infinite scroll. Page size is 30; the popover loads page 0 on appear and each subsequent page when a sentinel row at the tail of the LazyVStack becomes visible. Store changes: - searchSessions gains offset+limit. Empty query is now a valid input (used for "browse the most recent" without a needle), so the inner per-agent helpers skip the substring scan when needle.isEmpty and just return the next mtime-sorted page. - Claude/Codex helpers count matches and return the [offset..offset+limit] slice. Caps at searchMaxFiles (1500) per call so deep scrolling stays bounded. - OpenCode SQL builds WHERE clauses conditionally (LIKE only when needle non-empty, AND directory= only when filtered) and uses real LIMIT/OFFSET. - Directory scope (multi-agent) fetches offset+limit from each agent, merges, sorts by mtime desc, and slices to the page. UI changes: - Popover state replaced with `loaded`/`hasMore`/`isLoading`. - `loadGeneration` counter prevents stale tasks from clobbering newer results when SwiftUI cancellation hasn't propagated yet. - "Loading…" sentinel row appears at the bottom while a page is fetching AND triggers loadMore via .onAppear when scrolled into view. - Same row also covers the empty-query open and the search-in-progress states, so users always see feedback while the popover is fetching. * Use rg to pre-filter session search when available + drop count badge - Pre-filter Claude/Codex search with `rg --files-with-matches` when rg is on PATH (cached at first lookup). Falls back to the existing Foundation file enumeration + 128 KB head substring scan when rg is missing or fails. - Skips the post-rg substring re-check when rg already confirmed the match. Net effect: when rg is present, we only do head/tail reads for files that actually contain the needle, and rg has already scanned the entire file (not just our 128 KB head), so deep-conversation matches are reachable too. - OpenCode stays on SQLite — no rg benefit there. - Drop the entry count badge from the popover header. * Fix rg pipe-buffer deadlock: drain stdout before waitUntilExit * Make rg search async + cancellable; drop Task.detached hop Per swift-guidance review: the previous shape had two related concurrency problems. 1. Subprocess survived Task cancellation. searchSessions wrapped its work in Task.detached(...).value. Detached tasks are deliberately isolated from parent cancellation, so when the popover's loadTask was cancelled (every keystroke does this), the previous rg kept running to completion. Fast typing piled up rg invocations contending for I/O. 2. process.waitUntilExit() blocked a cooperative-pool thread inside the detached task. The pool has a small fixed size, so a stuck rg stole capacity from rendering / next-debounce work. Fixes: - ripgrepMatchingPaths becomes async and is wrapped in withTaskCancellationHandler { ... } onCancel: { process.terminate() }. Cancellation now actually kills the in-flight rg. - The wait uses process.terminationHandler + withCheckedContinuation for a true async suspension — no blocked thread. - searchSessions drops the Task.detached(...).value wrapper. searchAgent / searchClaudeOnDisk / searchCodexOnDisk become async; the helpers are already nonisolated and Sendable-clean, so the async keyword is enough to push them off the @mainactor caller. - searchOpenCodeInDB stays sync (SQL is fast and synchronous). Net result: typing "ti" -> "tin" -> "tiny" terminates each previous rg on the first keystroke after, and the SwiftUI render thread stays unblocked while rg works. * DRY scan + search into one path; ensure empty-needle fast path Three pairs of functions did almost the same thing — initial scan and deep search were 90% duplicate code per agent. Unified each pair into a single load*Entries(needle:cwdFilter:offset:limit:) function. - scanClaude + searchClaudeOnDisk → loadClaudeEntries - scanCodex + searchCodexOnDisk → loadCodexEntries - scanOpenCode + searchOpenCodeInDB → loadOpenCodeEntries scanAll calls each with `needle: "", cwdFilter: nil, offset: 0, limit: perAgentLimit`. searchSessions calls them with the trimmed needle and the requested page. Empty-needle fast path is now explicit and proven by code path: - Skip the rg subprocess (already gated by `if !needle.isEmpty`). - Skip the post-rg substring re-check. - Loop breaks at `matches.count >= target` so the candidate scan stops at exactly `offset+limit` files for empty queries. - OpenCode skips its `LOWER(...) LIKE ?` clause and is just `ORDER BY time_updated DESC LIMIT ? OFFSET ?`. Also dropped the searchFileByteCap (128 KB) constant and folded both read paths to headByteCap (64 KB). With rg as the primary needle filter, the post-filter metadata read only needs the title region — the larger 128 KB cap was redundant. * Show-more popover: instant cached page on open, no spinner flash * Fix rg async deadlock: terminationHandler was registered after exit The previous shape set process.terminationHandler AFTER readDataToEndOfFile() returned. But readDataToEndOfFile returns when rg closes stdout, which is when rg exits — so the handler was being installed on an already-terminated process and never fired, leaving the awaiting continuation hung forever. Symptom: popover stuck on "Loading…" for any non-empty query that exercised the rg pre-filter. Fix: drop the terminationHandler/continuation pair and use waitUntilExit() right after readDataToEndOfFile(). The process is already exiting at that point so this is essentially instant — it just makes terminationStatus observable. The withTaskCancellationHandler is unchanged: cancel still fires process.terminate(), which closes rg's stdout, unblocks readDataToEndOfFile, and lets the body return cleanly. Trade-off vs the old shape: this briefly blocks the cooperative-pool thread during the wait. In practice the wait is microseconds (process already exited by the time we get there), so the swift-guidance "no blocking on cooperative pool" concern is satisfied for any realistic workload while restoring correctness. * Skip Codex envelope messages + reset popover state on each open Codex titles - Some Codex sessions surface as "<environment_context>..." because the first user_message in the rollout is an envelope wrapper, not the user's real prompt. The response_item path already filtered AGENTS.md / <user_instructions> / <permissions> envelopes; pulled that into realCodexUserMessage(_:) and applied it to BOTH the event_msg and response_item paths. Loop continues past envelopes until a real user prompt is found (or until thread_name_updated supersedes it entirely). Popover state - Reopening "Show more" was carrying the previous open's @State — typed query, scroll position, etc. — because NSHostingController reuses the same SwiftUI view tree across rootView replacements. Symptom: you'd see the prior search text highlighted in the field while the result list was the cached (un-filtered) entries from .onAppear, putting query and loaded out of sync. - Fix: bump a presentationCount on each present() and tag the SectionPopoverView with .id(presentationCount). SwiftUI now treats each open as a brand-new view, so all @State (query, loaded, hasMore, isLoading, searchFocused) starts clean. * Stop infinite popover refresh + ship dark Codex icon variant Infinite loop - present() was bumping presentationCount + calling refreshContent on every NSViewRepresentable update. updateNSView fires on every parent re-render (which the @ObservedObject store triggers constantly), so the SectionPopoverView .id kept incrementing, SwiftUI kept treating it as a brand-new view, @State kept resetting → loaded snapped back to section.entries forever and the popover looked stuck loading. - Fix: only bump presentationCount on a hidden→shown transition. Once the popover is up, updateNSView still flows through coordinator. update → refreshContent, but presentationCount is stable so the .id stays the same and SwiftUI preserves @State. Reopening (transition from hidden) still bumps and resets — the original goal. Codex icon - Codex (OpenAI mark) was a black PNG that became invisible in light mode and looked off in some themes. Add a white variant tagged appearance=luminosity/dark in the Codex.imageset Contents.json. The asset catalog auto-picks based on system appearance — no SwiftUI changes needed, no template-rendering coupling, Claude/OpenCode full-color icons stay untouched. * Claude/Codex cwd-filter fast paths + per-agent timing logs Directory-scope "Show more" → load more was slow because for an empty needle with cwdFilter, every per-agent helper enumerated all files and parsed their metadata just to check the cwd. Two targeted fixes, self-benchmarked against the real data on this machine: Claude - Project dir name encodes the cwd ("/Users/x/y" -> "-Users-x-y"). - When cwdFilter is set, skip enumerating any other project dir. - Bench: 141ms -> 17ms warm (~8x). Files inspected: 765 -> 313. Codex - session_meta is always the FIRST line of the rollout. - Add peekCodexSessionMetaCwd: read up to 64 KB head, parse the first line, return cwd. If it doesn't match cwdFilter, skip the file before doing the full streaming extract (which can read MBs looking for thread_name/model/etc). - Bench: ~38ms -> ~33ms warm in this dataset; the bigger win is per-skipped-file read size dropping from 256 KB+ to 64 KB. Diagnostics - searchSessions logs total ms + needle/offset/limit. - timedAgent wrapper logs per-agent ms and result count for the multi-agent directory fanout. - All gated to #if DEBUG. * Use Codex's own state_5.sqlite for session metadata Codex maintains ~/.codex/state_5.sqlite with a `threads` table that already has every field we were extracting from jsonl rollouts the hard way: id, rollout_path, cwd, title, model, git_branch, approval_mode, sandbox_policy (JSON with type), reasoning_effort, first_user_message, updated_at_ms. Indexed on updated_at(_ms). Bench against this machine's data (293 cmuxterm-hq Codex sessions of 453 total): page 0 unfiltered (30): 3 ms page 0 cwd-filtered (30): 1 ms page 5 cwd-filtered (offset 150): 7 ms search 'posthog' (30): 8 ms search 'posthog' + cwd (30): 1 ms Versus the file-scan path which was reading 64 KB head from every jsonl just to check cwd (~30+ ms warm, far worse cold). loadCodexEntries now snapshots state_5.sqlite to /tmp (same WAL-safety pattern we already use for OpenCode), runs a single query with conditional WHERE clauses for cwd / needle, and constructs SessionEntry rows directly. Falls back to the renamed loadCodexEntriesFromDisk (=former loadCodexEntries) when state_5.sqlite is missing. Codex's curated `title` column is preferred over first_user_message, which is itself filtered through realCodexUserMessage so envelope wrappers (<environment_context>...) don't leak in as titles. * Parallelize Claude per-file reads + LRU cache + timing breakdown Production timings showed reads dominating (read=2664ms parse=111ms loop=4288ms for 50 new files). The benchmark predicted 40ms because SwiftBenchmark scripts run in a tight loop on a single thread; the real app was already spending most of its time in serial FileHandle reads on big jsonls (some are 50+MB). Two changes: 1. Wrap the per-file work in withTaskGroup so reads happen across cooperative-pool threads instead of one at a time. Each task does the same work (cache lookup, head/tail read, optional needle re-check, JSON parse, build SessionEntry); we collect, sort by original mtime index, and slice. With ~8 cores this should drop the read-bound phase from 50ms*N serial to ~50ms*N/8 parallel. 2. ClaudeMetadataCache (added previous commit) is now exercised correctly by the parallel path. Repeat scrolling reuses parsed entries without re-touching disk. Investigated whether Claude has a metadata SQLite — it doesn't. The state_5.sqlite / logs_2.sqlite files exist but Claude truncates them to 0 bytes on startup. session-env/ holds env scripts, not metadata. Reading the jsonls is the only path. * SearchOutcome carries errors; popover shows them above the list User-facing change: when the Codex SQLite or OpenCode SQLite returns a schema/open error (the usual symptom of an upstream version bump changing field names), the popover now shows an orange-tinted banner above the result list with the actual sqlite3 errmsg, instead of silently rendering an empty "No matches". Refactor: - searchSessions returns SearchOutcome { entries, errors }. - ErrorBag (lock-protected, @unchecked Sendable) is threaded through searchAgent → loadCodexEntriesViaSQL / loadOpenCodeEntries. - Each SQL helper appends a human-readable message on open or prepare failure; Codex still falls back to the file-scan path so the user also gets results when possible. - SectionPopoverView holds @State errorMessages, clears on each new query, populates from the outcome, renders a small warning row. * Virtualize popover list with NSTableView-backed List Replace the LazyVStack inside ScrollView with a SwiftUI List using .listStyle(.plain) so the popover gets true cell recycling on macOS for long search results. List requires a concrete height to render rows on macOS, so the frame is set explicitly to 420. Also DRY: extract the repeated post-fetch state update into applyOutcome(_:append:) so resetAndLoad and loadMore share the loaded / hasMore / errors / loading bookkeeping. Pull the three repeated .listRow* modifiers into a PopoverListRow ViewModifier. * Address review feedback batch - Resume routing: skip cwd-match when the selected workspace is remote so a local-indexed session is never sent into a remote shell just because the path string coincides. - syncFileExplorerDirectory: clear sessionIndexStore.currentDirectory on the early-return paths so a previous tab's cwd doesn't stick around as a stale filter. - decodeClaudeProjectDir: validate against the filesystem before returning so directories with a real "-" segment (e.g. "my-cool-project") don't decode to a wrong path; callers fall back to the JSONL `cwd` field. - cwdLabel: compare home prefix on a path boundary so "/Users/al" doesn't match a home of "/Users/alice". - absoluteTime: cache DateFormatter as a static so tooltip hover doesn't reallocate each access. - Section reorder: anchor moves to a visible neighbor key instead of a positional index. Hidden sections (filtered by scope) keep their relative position to visible neighbors instead of getting shuffled. - SessionIndexView appear: skip reload when the store is already loading, so the mode-toggle's reload isn't immediately cancelled and restarted. - Codex disk loader: check Task.isCancelled inside the per-file loop so a cancelled scan stops promptly instead of finishing the whole batch. - Popover rows: add the same right-click context menu as full SessionRow. Extracted the menu items into a shared `sessionRowMenuItems` builder and the row actions into inline closures so both rows stay in sync. * Revert popover to LazyVStack; fix unit-test mock signatures - Switch popover back to ScrollView + LazyVStack. The List + frame(420) approach kept colliding with the surrounding popover sizing; LazyVStack is already lazy enough for our result sizes. - WorkspaceUnitTests: mock overrides of makeWorkspaceForCreation must include the new initialTerminalInput parameter or they fail to override. --------- Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
~/.claude/projects/*/*.jsonl), Codex (~/.codex/sessions/.../rollout-*.jsonl), and OpenCode (~/.local/share/opencode/opencode.db, snapshotted to a temp dir before reading so a running OpenCode is not disturbed).~-collapsed), and a relative timestamp. Right-click gives Open / Reveal in Finder / Copy File Path / Open Working Directory. A "This folder only" checkbox scopes the list to the current FileExplorer root.rightSidebar.mode. Section expand/collapse is in-memory.Implementation notes
Sources/SessionIndexStore.swift(scanning + model),Sources/SessionIndexView.swift(SwiftUI list),Sources/RightSidebarPanelView.swift(toolbar wrapper that hosts eitherFileExplorerPanelVieworSessionIndexView).FileExplorerStategains amode: RightSidebarModepublished property; the existingFileExplorerPanelViewis unchanged and just gets wrapped.MainActor.run. Per-agent cap of 60 most-recent files; only the first 32 KB of each.jsonlis read for title extraction.session.titlecolumn.Naming
The user asked for naming suggestions ("table of contents", "index of sessions"). Code uses
SessionIndex/RightSidebarMode.sessionsinternally; the UI label is just Sessions (paired with Files) since two short, parallel labels read well in a 2-button toggle.Testing
xcodebuild ... buildsucceeds../scripts/reload.sh --tag feat-session-indexsucceeds; tagged DEV app launches and the Sessions toggle appears at the top of the right sidebar.Test plan
Related
Summary by cubic
Adds a Sessions mode to the right sidebar (Cmd‑Opt‑B) for quick context and one‑step resume of Claude Code, Codex, and OpenCode runs, with smart tab/workspace placement,
bonsplitdrag‑to‑insert/split, and a faster “Show more” search that prefers Codex’sstate_5.sqlitewhen available. Addresses Linear task feat-session-index.New Features
rightSidebar.mode.LazyVStackfor smooth scrolling; deep search usesrg --files-with-matchesand Codex’s~/.codex/state_5.sqlitewhen present (falls back to rollout scan).Bug Fixes
com.splittabbar.tabtransfer(mirrored to the.dragpasteboard); terminals ignore tab‑drag UTIs so overlays render and drops spawn new terminals reliably.NSText; Esc closes; fixed a hidden→shown refresh loop; popover list usesLazyVStackto avoid.listsizing issues; DB/SQL errors now show in a warning banner with the sqlite error text, and Codex searches fall back to file scan when SQL fails.rgsearch is async/cancellable; drain stdout beforewaitUntilExitto prevent deadlocks.state_5.sqlitefor fast titles/cwd/model; titles prefer curated DB title orthread_name; envelope messages skipped; dark‑mode Codex icon variant improves legibility.~label on home‑prefix boundaries; section reorder preserves hidden‑section order; skip redundant reloads while already loading; Codex disk scan checks cancellation promptly; popover rows mirror the main row context menu; unit‑test mocks updated to includeinitialTerminalInput.Written for commit e055b59. Summary will update on new commits.
Summary by CodeRabbit