Repository navigation
Add Fork Conversation action to tab context menu - #135
Conversation
Exposes a host-provided availability provider and a new `.forkConversation` case so embedders (cmux) can surface a "Fork Conversation" entry in the tab right-click menu when the tab hosts an agent session worth forking. Item is hidden when the provider returns false, mirroring the existing browser-only items pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR adds a "Fork Conversation" action to tab context menus. It introduces a host-provided availability provider on ChangesFork Conversation Tab Context Menu Action
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Greptile SummaryAdds a "Fork Conversation" entry to the tab context menu, surfaced only when a new host-provided
Confidence Score: 4/5Safe to merge; all changes follow established patterns and the build is clean. The implementation is consistent with the existing provider/action patterns and introduces no new delegate surface. The only gap is that the canForkConversation: true branch — menu item appearance, enabled state, and .forkConversation action dispatch — has no automated test coverage, so a regression in that path could go undetected. Tests/BonsplitTests/BonsplitTests.swift — the new canForkConversation: true menu path is not exercised by any test. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant TabBarView
participant Provider as tabContextForkConversationAvailabilityProvider
participant TabContextMenuBuilder
participant Delegate as BonsplitDelegate
User->>TabBarView: right-click tab
TabBarView->>Provider: (TabID, PaneID) → Bool
Provider-->>TabBarView: true / false
TabBarView->>TabBarView: "build TabContextMenuState (canForkConversation = result)"
TabBarView->>TabContextMenuBuilder: makeMenu(snapshot, target)
alt "canForkConversation == true"
TabContextMenuBuilder->>TabContextMenuBuilder: addSeparator + add Fork Conversation item
end
TabContextMenuBuilder-->>User: NSMenu displayed
User->>TabContextMenuBuilder: click Fork Conversation
TabContextMenuBuilder->>Delegate: didRequestTabContextAction(.forkConversation, tab, pane)
Reviews (1): Last reviewed commit: "Add Fork Conversation action to tab cont..." | Re-trigger Greptile |
| canMoveToNewWorkspace: true, | ||
| canMoveToLeftPane: false, | ||
| canMoveToRightPane: true, | ||
| canForkConversation: false, |
There was a problem hiding this comment.
Missing test coverage for the
true path
The existing test only exercises canForkConversation: false. No test verifies that when the field is true the "Fork Conversation" item actually appears in the built menu, is enabled, and — when activated via target.performContextAction — dispatches exactly .forkConversation to the onContextAction handler. Without this, a future accidental guard inversion (e.g. if !state.canForkConversation) or a typo in the representedObject raw value would go undetected by the suite.
* Add Fork Conversation to tab right-click menu Adds a `Fork Conversation` item to the bonsplit tab context menu. It appears only when the tab is running an agent session that supports forking without a probe (Claude, Codex). On click, opens a new sibling tab to the right of the source with `claude --resume <id> --fork-session` (or the analogous Codex form) as the startup input, so divergence is owned by the agent's native fork support instead of any cmux-side history copy. Companion to manaflow-ai/bonsplit#135 which adds the menu item plumbing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Lock down Fork Conversation isolation + handler guard Addresses PR #4888 review feedback: - Annotate `liveAgentIndexCache` / `liveAgentIndexCacheLoadedAt` / their accessor with `@MainActor` so the implicit MainActor isolation from the enclosing `Workspace` class is explicit at the static-storage level. The provider closure already runs during SwiftUI body evaluation on the main actor, but the explicit annotation prevents a Swift 6 strict-concurrency violation when that mode is enabled (CodeRabbit critical / Greptile P1). - Tighten the bonsplit `.forkConversation` handler to mirror the menu visibility gate exactly (`== .supportedWithoutProbe`) instead of the weaker `!= .unsupported`. Closes the gap where the action could let a `.requiresProbe` snapshot through if ever wired up outside the controlled menu (Greptile summary note). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Add unit tests for Fork Conversation tab action Covers the new Workspace API and the bonsplit context-menu wiring without needing to drive the NSMenu UI (which XCUI can't reach reliably): - forkAgentConversationToNewTab creates a sibling tab in the same pane, not a split pane, with the snapshot's `--fork-session` command as its initial input. - The forked tab lands immediately to the right of its source even when there are other tabs further right. - canForkAgentConversationFromPanel flips on once a restored Claude snapshot is associated with the panel. - The bonsplit `.forkConversation` action dispatches end-to-end through splitTabBar(_:didRequestTabContextAction:for:inPane:) to the same forkAgentConversationToNewTab path, locking in the menu wiring. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix Fork Conversation tests: use public TabID API CI test target failed to compile because the tests reached into `TabID.id` (which is `internal` in bonsplit), accessible only inside the bonsplit module via `@testable`. Local `./scripts/reload.sh --tag` builds the main app but not the test target, which is why the issue slipped through locally. Compare TabIDs directly (TabID is Hashable) instead of unwrapping the underlying UUID. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Background-refresh live agent index; add Codex parity test Addresses two PR #4888 review findings: - Greptile P1: `RestorableAgentSessionIndex.load()` performs blocking `sysctl(KERN_PROCARGS2)` per hook record (for live-PID filtering), which is N×2 syscalls on `@MainActor` per right-click within the TTL window. Replace the synchronous main-actor load with an off-main `Task.detached(priority: .utility)` refresh, keyed on a 1s freshness window. The provider reads the cached value synchronously and tolerates a slightly-stale snapshot — claude's `--resume`/`--fork-session` still works against the persisted transcript even if the cmux-recorded PID has died, so showing the menu item briefly for a recently-terminated session is benign. First right-click after a fresh Claude launch may miss the item until the background refresh completes (~hundreds of ms); subsequent SwiftUI body re-renders pick up the new snapshot. - CodeRabbit: add a Codex-parity test that drives the same context-menu dispatcher path with a Codex snapshot and asserts the sibling tab is created with the Codex `--fork-session` command. (The Claude-path test already asserted focus on the new tab.) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Trigger immediate UI refresh after live-index load Addresses PR #4888 CodeRabbit follow-up: the previous static cache returned nil on the first call after a stale-out and relied on a later SwiftUI body refresh (hover/focus) to pick up the new index, which is why the first right-click could miss the menu item. Move the cache off the static slot and onto the Workspace instance as a `@Published` property. When the background `Task.detached` completes and writes the new index, `objectWillChange` fires on the workspace, ContentView re-renders, and bonsplit's TabBarView re-evaluates its context-menu state on the same frame — so "Fork Conversation" becomes visible the moment the index lands, without a second right-click. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Beep when Fork Conversation tab creation fails silently Greptile P1 (PR #4888): the bonsplit `.forkConversation` handler discarded `forkAgentConversationToNewTab`'s return value with `_ = …`, so if `newTerminalSurface` ever returned nil (e.g. layout reject, workspace resource constraint) the click produced no visible result and no audio cue, even though every other early-exit in this action branch already calls `NSSound.beep()`. Match the existing convention. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Use shared singleton for live agent index across workspaces PR #4888 CodeRabbit follow-up: the previous per-Workspace `@Published` cache meant each open workspace performed its own background load of `RestorableAgentSessionIndex.load()`, redundantly running the same JSON + per-PID work N times for N open workspaces. Replace with a single `SharedLiveAgentIndex.shared` actor-isolated singleton that loads once and serves every workspace; each Workspace subscribes to its `objectWillChange` in its initializer and forwards it as its own so the SwiftUI re-render path stays intact (ContentView re-renders → bonsplit's TabBarView re-evaluates Fork Conversation visibility on the same frame). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add Fork Conversation to tab right-click menu Adds a `Fork Conversation` item to the bonsplit tab context menu. It appears only when the tab is running an agent session that supports forking without a probe (Claude, Codex). On click, opens a new sibling tab to the right of the source with `claude --resume <id> --fork-session` (or the analogous Codex form) as the startup input, so divergence is owned by the agent's native fork support instead of any cmux-side history copy. Companion to manaflow-ai/bonsplit#135 which adds the menu item plumbing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Lock down Fork Conversation isolation + handler guard Addresses PR #4888 review feedback: - Annotate `liveAgentIndexCache` / `liveAgentIndexCacheLoadedAt` / their accessor with `@MainActor` so the implicit MainActor isolation from the enclosing `Workspace` class is explicit at the static-storage level. The provider closure already runs during SwiftUI body evaluation on the main actor, but the explicit annotation prevents a Swift 6 strict-concurrency violation when that mode is enabled (CodeRabbit critical / Greptile P1). - Tighten the bonsplit `.forkConversation` handler to mirror the menu visibility gate exactly (`== .supportedWithoutProbe`) instead of the weaker `!= .unsupported`. Closes the gap where the action could let a `.requiresProbe` snapshot through if ever wired up outside the controlled menu (Greptile summary note). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Add unit tests for Fork Conversation tab action Covers the new Workspace API and the bonsplit context-menu wiring without needing to drive the NSMenu UI (which XCUI can't reach reliably): - forkAgentConversationToNewTab creates a sibling tab in the same pane, not a split pane, with the snapshot's `--fork-session` command as its initial input. - The forked tab lands immediately to the right of its source even when there are other tabs further right. - canForkAgentConversationFromPanel flips on once a restored Claude snapshot is associated with the panel. - The bonsplit `.forkConversation` action dispatches end-to-end through splitTabBar(_:didRequestTabContextAction:for:inPane:) to the same forkAgentConversationToNewTab path, locking in the menu wiring. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix Fork Conversation tests: use public TabID API CI test target failed to compile because the tests reached into `TabID.id` (which is `internal` in bonsplit), accessible only inside the bonsplit module via `@testable`. Local `./scripts/reload.sh --tag` builds the main app but not the test target, which is why the issue slipped through locally. Compare TabIDs directly (TabID is Hashable) instead of unwrapping the underlying UUID. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Background-refresh live agent index; add Codex parity test Addresses two PR #4888 review findings: - Greptile P1: `RestorableAgentSessionIndex.load()` performs blocking `sysctl(KERN_PROCARGS2)` per hook record (for live-PID filtering), which is N×2 syscalls on `@MainActor` per right-click within the TTL window. Replace the synchronous main-actor load with an off-main `Task.detached(priority: .utility)` refresh, keyed on a 1s freshness window. The provider reads the cached value synchronously and tolerates a slightly-stale snapshot — claude's `--resume`/`--fork-session` still works against the persisted transcript even if the cmux-recorded PID has died, so showing the menu item briefly for a recently-terminated session is benign. First right-click after a fresh Claude launch may miss the item until the background refresh completes (~hundreds of ms); subsequent SwiftUI body re-renders pick up the new snapshot. - CodeRabbit: add a Codex-parity test that drives the same context-menu dispatcher path with a Codex snapshot and asserts the sibling tab is created with the Codex `--fork-session` command. (The Claude-path test already asserted focus on the new tab.) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Trigger immediate UI refresh after live-index load Addresses PR #4888 CodeRabbit follow-up: the previous static cache returned nil on the first call after a stale-out and relied on a later SwiftUI body refresh (hover/focus) to pick up the new index, which is why the first right-click could miss the menu item. Move the cache off the static slot and onto the Workspace instance as a `@Published` property. When the background `Task.detached` completes and writes the new index, `objectWillChange` fires on the workspace, ContentView re-renders, and bonsplit's TabBarView re-evaluates its context-menu state on the same frame — so "Fork Conversation" becomes visible the moment the index lands, without a second right-click. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Beep when Fork Conversation tab creation fails silently Greptile P1 (PR #4888): the bonsplit `.forkConversation` handler discarded `forkAgentConversationToNewTab`'s return value with `_ = …`, so if `newTerminalSurface` ever returned nil (e.g. layout reject, workspace resource constraint) the click produced no visible result and no audio cue, even though every other early-exit in this action branch already calls `NSSound.beep()`. Match the existing convention. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Use shared singleton for live agent index across workspaces PR #4888 CodeRabbit follow-up: the previous per-Workspace `@Published` cache meant each open workspace performed its own background load of `RestorableAgentSessionIndex.load()`, redundantly running the same JSON + per-PID work N times for N open workspaces. Replace with a single `SharedLiveAgentIndex.shared` actor-isolated singleton that loads once and serves every workspace; each Workspace subscribes to its `objectWillChange` in its initializer and forwards it as its own so the SwiftUI re-render path stays intact (ContentView re-renders → bonsplit's TabBarView re-evaluates Fork Conversation visibility on the same frame). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
.forkConversationcase inTabContextActiontabContextForkConversationAvailabilityProvideronBonsplitControllerTabContextMenuBuilder(shown only when the provider returns true)canForkConversationfieldUsed by cmux (see manaflow-ai/cmux companion PR) to surface a Fork Conversation entry when a tab is running an active forkable Claude or Codex session.
Test plan
swift build --build-testscleantruefor the provider, confirm "Fork Conversation" appears🤖 Generated with Claude Code
Summary by cubic
Adds a “Fork Conversation” action to the tab context menu, shown only when the host says it’s available. This lets apps surface forking for tabs running an active, forkable agent session.
New Features
TabContextAction.forkConversation.BonsplitController.tabContextForkConversationAvailabilityProviderto control visibility.Migration
TabContextAction, add a.forkConversationcase (or a default).Written for commit 082bf66. Summary will update on new commits. Review in cubic
Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Summary by CodeRabbit