Skip to content

fix(sidebar): isolate output by cmux instance#327

Merged
EtanHey merged 3 commits into
mainfrom
wt/cmuxlayer-worker-gz5e9w
Jul 15, 2026
Merged

fix(sidebar): isolate output by cmux instance#327
EtanHey merged 3 commits into
mainfrom
wt/cmuxlayer-worker-gz5e9w

Conversation

@EtanHey

@EtanHey EtanHey commented Jul 15, 2026

Copy link
Copy Markdown
Owner

Summary

  • preserve Stable's exact ~/.config/cmux/sidebars/fleet.swift output contract
  • derive isolated Nightly/dev sidebar and collapse-state paths from CMUX_SOCKET_PATH or CMUX_BUNDLE_ID
  • honor CMUXLAYER_FLEET_SIDEBAR_OUTPUT_PATH, and hash non-canonical full identities so distinct sockets cannot collide after sanitization
  • add hermetic TDD coverage for Stable, Nightly, dev, overrides, collapse state, publisher isolation, and collision resistance

Test plan

  • env -u CMUX_SOCKET_PATH -u CMUX_DAEMON_SOCKET bun run test — 104 files / 2,159 tests passed
  • env -u CMUX_SOCKET_PATH -u CMUX_DAEMON_SOCKET bun run typecheck — passed
  • env -u CMUX_SOCKET_PATH -u CMUX_DAEMON_SOCKET bun run build — passed
  • pre-push scripts/run_tests.sh — exit 0

Review disposition

Status: CodeRabbit local review SKIPPED — timed out after the bounded three-minute window while still reviewing; stopped with exit 130.

  • HIGH: Stable's non-UID ~/.local/state/cmux/cmux.sock was treated as a named instance — FIXED in f94fa8d with a RED/GREEN regression test
  • HIGH: Stable bundle aliases (stable, prod, production) were hashed instead of preserving the legacy path — FIXED in 0fae9f4 with a RED/GREEN regression test
  • MEDIUM: distinct full socket identities could collide after basename/sanitization — FIXED with a short SHA-256 suffix and RED/GREEN regression tests
  • Red-team re-review: prior finding resolved; zero remaining issues
  • Independent code review and blue-team review: zero findings

Scope

MCP-side path-keying enabler only. No app-side Nightly activation, render changes, or single-writer-authority changes.

Note

Isolate Fleet sidebar output and collapse state paths by cmux instance

  • Adds fleetSidebarInstanceKey to derive a per-instance key from CMUX_SOCKET_PATH or CMUX_BUNDLE_ID, returning null for stable/production and canonical or hashed keys for Nightly, Dev, and other instances.
  • Updates defaultFleetSidebarPath and defaultFleetSidebarCollapseStatePath to write under ~/.config/cmux-<instance>/ and ~/.local/state/cmuxlayer/<instance>/ for non-stable instances; stable remains at the legacy paths.
  • CMUXLAYER_FLEET_SIDEBAR_OUTPUT_PATH overrides both paths; the collapse state is colocated as <override>.collapse.json.
  • FleetSidebarPublisher now derives its output and collapse paths via the updated helpers instead of reading env vars directly.
  • Behavioral Change: existing non-stable instances will write to new per-instance paths rather than the shared stable path, so any previously written sidebar files will not be read from the new location.

Macroscope summarized 0fae9f4.

@cursor

cursor Bot commented Jul 15, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_b7565f8d-cef1-431d-b051-bdd4f523e32d)

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Fleet sidebar output and collapse-state paths now derive from CMUX socket or bundle identity, isolating Stable, Nightly, development, and similarly named instances. Explicit output overrides remain supported, publisher wiring uses canonical paths, and tests cover path derivation and cross-instance publishing.

Changes

Fleet sidebar instance isolation

Layer / File(s) Summary
Instance identity and path derivation
src/fleet-sidebar.ts, tests/fleet-sidebar.test.ts, docs/releases-and-brew.md
Socket and bundle identities are normalized and hashed into per-instance configuration and state paths; tests cover Stable, Nightly, development, collision avoidance, bundle selection, and explicit output overrides. Documentation describes the resulting paths and naming rules.
Publisher output and collapse-state wiring
src/fleet-sidebar.ts, tests/fleet-sidebar.test.ts
FleetSidebarPublisher selects explicit or canonical output paths and passes the canonical collapse-state path explicitly; tests verify isolated publishing and collapse preferences across instances.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • EtanHey/cmuxlayer#319: Overlaps the fleet sidebar output-path selection and FleetSidebarPublisher wiring.

Poem

A rabbit hops where CMUX paths divide,
Stable and Nightly keep their files beside.
Hashed little names prevent a clash,
Collapse states rest in their proper stash.
Swift sidebar trails now know each lane—
Hop, hop! No instance writes in vain.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: isolating sidebar output by cmux instance.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch wt/cmuxlayer-worker-gz5e9w

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@EtanHey

EtanHey commented Jul 15, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@EtanHey

EtanHey commented Jul 15, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@EtanHey

EtanHey commented Jul 15, 2026

Copy link
Copy Markdown
Owner Author

@cursor @BugBot review

@EtanHey

EtanHey commented Jul 15, 2026

Copy link
Copy Markdown
Owner Author

@greptileai review

@cursor

cursor Bot commented Jul 15, 2026

Copy link
Copy Markdown

You need to increase your spend limit or enable usage-based billing to run background agents. Go to Cursor

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Comment thread src/fleet-sidebar.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3961432637

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/fleet-sidebar.ts Outdated
Comment on lines +733 to +735
const productionStateSocket =
dirname(upstreamSocket) === join(home, ".local", "state", "cmux") &&
/^cmux-\d+\.sock$/i.test(socketName);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Treat legacy state cmux.sock as production

When stable cmux is pinned to the legacy state socket $HOME/.local/state/cmux/cmux.sock, this predicate does not classify it as production because it only accepts cmux-<uid>.sock; however cmuxSocketPathCandidates() still includes join(stateDir, "cmux.sock") as a production candidate. In that environment defaultFleetSidebarPath() hashes the stable socket into ~/.config/cmux-instance-.../sidebars/fleet.swift, so the stable publisher stops updating the canonical ~/.config/cmux/sidebars/fleet.swift that the change is intended to preserve.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in f94fa8d. The Stable state-directory classifier now accepts both cmux.sock and cmux-.sock, matching cmuxSocketPathCandidates(). A RED/GREEN regression covers the non-UID path; current head 0fae9f4 passes 104 files / 2,159 tests, typecheck, build, and the pre-push gate.

@cursor

cursor Bot commented Jul 15, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_701a090a-7290-4eee-b15d-f27bf552d58a)

Comment thread src/fleet-sidebar.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f94fa8dea7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/fleet-sidebar.ts
Comment on lines +749 to +754
if (upstreamSocket === "/tmp/cmux-nightly.sock") return "nightly";
if (upstreamSocket === "/tmp/cmux-dev.sock") return "dev";
const socketStem = socketName
.replace(/\.sock$/i, "")
.replace(/^cmux(?:[-_.]+)?/i, "");
return hashedFleetSidebarInstanceKey(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep daemon axis aligned for non-nightly instances

This adds isolated sidebar paths for /tmp/cmux-dev.sock and arbitrary named sockets, but in the normal daemon-first path I checked runDaemonFirstEntry still connects to defaultDaemonSocketPath(env), and src/daemon-socket-path.ts only creates a separate daemon socket for Nightly. If a stable daemon is already running and a dev/custom-socket cmuxlayer starts, it proxies to the existing stable daemon, so the FleetSidebarPublisher was constructed with the stable daemon's environment and continues writing the canonical Stable sidebar instead of this derived instance path. The sidebar and daemon instance axes need to match for these new non-nightly instance keys.

Useful? React with 👍 / 👎.

@cursor

cursor Bot commented Jul 15, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_489082d7-f5e4-4643-bc85-f6554a7d97d3)

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0fae9f47bc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/fleet-sidebar.ts
Comment on lines +742 to +746
if (
productionStateSocket ||
productionTmpSocket ||
productionApplicationSupportSocket
) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Treat last-socket-path stable pins as production

When CMUX_SOCKET_PATH is the stable socket recorded in ~/.local/state/cmux/last-socket-path (which cmuxSocketPathCandidates() treats as a production candidate before the uid/default paths), this guard only recognizes hardcoded state, /tmp, and legacy Application Support names. If the stable app exports an alias from last-socket-path, Stable is hashed into ~/.config/cmux-<hash>/sidebars/fleet.swift instead of continuing to update the canonical ~/.config/cmux/sidebars/fleet.swift; the last-socket-path target should be read/canonicalized here as production as well.

Useful? React with 👍 / 👎.

@EtanHey
EtanHey merged commit 7f8137b into main Jul 15, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant