Conversation
|
@ejc3 is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedWe couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesThe PR adds a beta setting for remote tmux host colors, assigns stable palette colors per remote destination, resolves those colors for mirrored workspaces, and propagates them through sidebar snapshots and cache keys while preserving manual workspace color precedence. Remote tmux origin colors
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant BetaFeatureSetting
participant ContentView
participant RemoteTmuxController
participant RemoteHostColorRegistry
participant SidebarWorkspaceSnapshotFactory
BetaFeatureSetting->>ContentView: change remoteTmuxOriginColorsEnabled
ContentView->>ContentView: refresh workspace snapshots
ContentView->>RemoteTmuxController: resolve workspace host destination
RemoteTmuxController->>RemoteHostColorRegistry: resolve destination color
ContentView->>SidebarWorkspaceSnapshotFactory: provide originColorHex
SidebarWorkspaceSnapshotFactory->>ContentView: return effective snapshot and presentation key
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 2 warnings)
✅ Passed checks (21 passed)
Full details: Description checkExplanation The description explains the origin-color feature, implementation, tests added, and changelog entry. However, it omits the required Demo Video and Checklist sections, does not identify tests executed or commands run, and describes an additional host-title feature not supported by the provided changeset summary. Resolution Use the required Summary, Testing, Changelog, Demo Video, and Checklist sections. State the exact test commands and results, or identify what remains unverified. Add the required localization and review checklist status. Remove or separately document the originHostTitles.beta feature unless it is included in this pull request. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
e07902d to
290aad9
Compare
395702a to
fca798a
Compare
35b9b3d to
fca798a
Compare
fca798a to
ab9a33b
Compare
Greptile SummaryThis PR introduces per-host origin colors for remote tmux workspaces: each sidebar row/tab is tinted with a palette color derived from its SSH host, gated behind
Confidence Score: 5/5Safe to merge. The feature is fully gated behind a beta flag and the cache-miss rebuild path that was missing All three snapshot-build paths (batch refresh, full repopulate, and the per-row cache-miss inline path) pass No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant SB as VerticalTabsSidebar (body)
participant RC as RenderContext
participant RTC as RemoteTmuxController
participant RHCR as RemoteHostColorRegistry
participant SWSF as SidebarWorkspaceSnapshotFactory
Note over SB: originColorsEnabled = true
SB->>RC: mirrorDestinationsForOriginColors()
RC->>RTC: hostDestinationsByWorkspaceId()
RTC-->>RC: [UUID: String] (one pass over sessionMirrors)
RC-->>SB: mirrorOriginDestinations
loop per workspace row
SB->>SB: originColorHex(for: workspace, mirrorDestinations:)
SB->>RTC: hostColorRegistry.colorHex(for: destination)
RTC->>RHCR: colorHex(for: destination)
RHCR->>RHCR: slot(for:) — hash+probe, cache
RHCR-->>SB: "#hexColor (or nil)"
SB->>SWSF: makeWorkspaceSnapshot(originColorHex:)
SWSF-->>SB: Snapshot(customColorHex: manual ?? origin, hasManualCustomColor:)
end
Note over SB: Cache keyed by PresentationKey(customColorHex:)
Reviews (6): Last reviewed commit: "remote-tmux: drop the origin-color looku..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmuxTests/RemoteHostColorRegistryTests.swift`:
- Around line 18-23: Update stableHashIsDeterministic to evaluate
RemoteHostColorRegistry.stableHash("cmux-srvA") twice into separate local
constants, then compare those variables for equality; keep the existing
assertion verifying different inputs produce different hashes.
In `@Sources/RemoteHostColorRegistry.swift`:
- Around line 85-89: Remove the test-only reset() method from
RemoteHostColorRegistry in production source, including its documentation.
Update tests to instantiate a new RemoteHostColorRegistry when fresh state is
needed instead of resetting an existing instance.
In `@Sources/RemoteTmuxController.swift`:
- Around line 336-343: Replace the linear sessionMirrors.values scan in
hostDestination(forWorkspaceId:) with a maintained secondary index keyed by
mirroredWorkspaceId, storing the mirror or host destination for O(1) lookup.
Update this index whenever sessionMirrors entries are added, removed, or
changed, while preserving nil for unmapped workspace IDs.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0a2531f2-9b36-4222-9f6e-26d124737d29
⛔ Files ignored due to path filters (1)
cmux.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolvedis excluded by!**/Package.resolved
📒 Files selected for processing (16)
Packages/macOS/CmuxSettings/Sources/CmuxSettings/Keys/BetaFeaturesCatalogSection.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/CuratedSettingEntry+Default.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/BetaFeaturesSection.swiftPackages/macOS/CmuxSettingsUI/Tests/CmuxSettingsUITests/SettingsRowAnchorResolutionTests.swiftResources/Localizable.xcstringsSources/ContentView.swiftSources/RemoteHostColorRegistry.swiftSources/RemoteTmuxController.swiftSources/Sidebar/SidebarWorkspaceSnapshotRefreshPolicy.swiftSources/SidebarWorkspaceSnapshotBuilder.swiftSources/SidebarWorkspaceSnapshotFactory.swiftSources/TabItemView+WorkspaceContextMenu.swiftcmux.xcodeproj/project.pbxprojcmuxTests/RemoteHostColorRegistryTests.swiftcmuxTests/SidebarAppKitRowCellTests.swiftcmuxTests/SidebarWorkspaceSnapshotRefreshPolicyTests.swift
9583bc9 to
e0f042f
Compare
manaflow-ai#8428 and manaflow-ai#7193 each add an identical hostDestination(forWorkspaceId:) standalone, which only collides when both land. Keep one. At real merge time the second PR to land drops its copy; this commit lives only on the dogfood roll-up.
e0f042f to
90409b9
Compare
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
54f74ec to
af690c8
Compare
af690c8 to
dc51a4e
Compare
508c635 to
2ca1cd0
Compare
2ca1cd0 to
296317b
Compare
2e5743c to
1b16dc7
Compare
|
All contributors have signed the CLA ✍️ ✅ |
CI failure attributionCI passes on Written by |
|
|
# Conflicts: # Packages/macOS/CmuxSettings/Sources/CmuxSettings/Keys/BetaFeaturesCatalogSection.swift # Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/BetaFeaturesSection.swift # Packages/macOS/CmuxSettingsUI/Tests/CmuxSettingsUITests/SettingsRowAnchorResolutionTests.swift
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
There was a problem hiding this comment.
Review completed against the latest diff
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
The string catalog is main's text with this branch's entries inserted, so the branch differs from main only where it adds or edits strings.
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
Deployment failed for project cmux with the following error: |
|
Taking this: reviewing the remote-host sidebar colors and the current bot findings. OrchardSpoon g1 🌀 |
Merge-main commit by scripts/merge-main.sh. Merged by scripts/merge-main.sh: origin/main at 51b60f3. Resolved conflicts: - Resources/Localizable.xcstrings: xcstrings key-level union - cmux.xcodeproj/project.pbxproj: union of added entries, then normalize-pbxproj.py Merge-main-previous-head: 8997f5f Merge-main-base: 51b60f3 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ressions Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Merge-main commit by scripts/merge-main.sh. Merged by scripts/merge-main.sh: origin/main at 00f182f. Resolved conflicts: - Resources/Localizable.xcstrings: xcstrings key-level union - cmux.xcodeproj/project.pbxproj: union of added entries, then normalize-pbxproj.py Merge-main-previous-head: 1305970 Merge-main-base: 00f182f
# Conflicts: # Sources/RemoteTmuxController.swift
What this adds
Per-host origin colors for remote workspaces: tint each remote/mirror workspace's sidebar row with a stable color derived from its host, so sessions from different hosts are easy to tell apart at a glance and a given host shows the same color in every window. A user's manually chosen workspace color always wins. Off by default behind
remoteTmux.originColors.beta.A second beta setting,
remoteTmux.originHostTitles.beta, covers the case color alone does not: sidebar workspaces that share a title but come from different places. Each remote one shows its host after the title, without the domain the hosts have in common. Also off by default.How it works
RemoteHostColorRegistryassigns each destination a palette slot: a stable FNV-1a hash of the host name picks a start index, then linear-probes to the next free slot on collision, and caches the assignment. The stable hash (not Swift's per-processHasher) keeps a lone host's color steady across launches. The registry is owned byRemoteTmuxController(itself owned byAppDelegateat the app seam) and reached through it — no ambient singleton.customColorHex(effective color = manual color ?? origin color) andhasManualCustomColor(so "Clear Color" only shows for a real manual color). Resolved above the row boundary; mirror workspaces carry their host through the session mirror (viahostDestination), so it works even without aremoteConfiguration.Tests
RemoteHostColorRegistryTests(stable-hash determinism, collision probing, full-palette fallback, empty-palette nil),SidebarWorkspaceSnapshotRefreshPolicyTests(color feeds the presentation key / cache invalidation), the AppKit row-cell snapshot test updated for the new fields, andRemoteHostTitleSuffixesTestsfor the host suffix (only the part that tells the hosts apart is shown, a remote workspace beside a local one shows its whole host, and a host is never trimmed to nothing).Changelog
Added: Beta settings that tint each remote tmux workspace row in the sidebar with a stable color per host, and show the host after workspace titles that would otherwise be identical
Summary by CodeRabbit
New Features
Bug Fixes
Tests