Add nushell support: wrapper shim routing, resume envelope, fish-parity shell integration - #8104
Conversation
|
@RemiKalbe is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
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:
📝 WalkthroughWalkthroughNushell support was added across shell startup, command rendering, session resume paths, cmux shell integration, PATH shim ordering, CI setup, and regression tests. ChangesNushell shell integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant TerminalSurface
participant Nushell
participant POSIXShell
participant cmux_nushell_integration
TerminalSurface->>Nushell: launch with -l -e startup payload
Nushell->>cmux_nushell_integration: source integration and register hooks
Nushell->>POSIXShell: execute wrapped resume command through /bin/sh -c
cmux_nushell_integration->>TerminalSurface: report shell activity and session state
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (23 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR adds Nushell support for startup, resume, and shell integration. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (6): Last reviewed commit: "Mark NushellTypedShellCommand nonisolate..." | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89f555214e
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@Resources/shell-integration/nushell/cmux-nushell-integration.nu`:
- Around line 31-36: Memoize the result of the `_cmux_socket_is_unix` probe for
the session instead of running `^/bin/test -S` on every hook invocation. Add a
session-scoped cached value and have `_cmux_report_shell_activity_state` and
`_cmux_ports_kick` reuse it, while preserving the existing empty/non-absolute
socket checks and activity-state deduplication behavior.
🪄 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: 43069287-b41f-46d6-a6a3-2f84a3b7dc4c
📒 Files selected for processing (19)
.github/workflows/ci.ymlPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/NushellTypedShellCommand.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/NushellTypedShellCommandTests.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Spawn/TerminalSurface+StartupEnvironment.swiftResources/shell-integration/nushell/cmux-nushell-bootstrap.nuResources/shell-integration/nushell/cmux-nushell-integration.nuSources/RestorableAgentSession.swiftSources/SessionIndexView.swiftSources/SessionPersistence.swiftSources/SessionRestoredTerminalCommandStore.swiftSources/SurfaceResumeCommandCanonicalizer+PortableAgentExecutable.swiftSources/Workspace.swiftcmuxTests/SessionPersistenceResumeBindingTests.swiftcmuxTests/SessionPersistenceTests.swiftcmuxTests/ShellStartupMatrixTests.swiftcmuxTests/ShellStartupMissingBundleTests.swifttests/test_nushell_integration_hooks.pytests/test_nushell_resume_command_dialect.pytests/test_nushell_shim_path_refront.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/NushellTypedShellCommand.swift`:
- Around line 17-18: Mark the pure value-model struct NushellTypedShellCommand
as nonisolated while preserving its public initializer and existing behavior.
🪄 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: 574372de-86be-450a-8960-5e03b617f71f
📒 Files selected for processing (9)
Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/NushellTypedShellCommand.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/NushellTypedShellCommandTests.swiftResources/shell-integration/nushell/cmux-nushell-integration.nuSources/RestorableAgentSession.swiftSources/SessionIndexView.swiftSources/SessionPersistence.swiftSources/Workspace.swiftcmuxTests/SessionPersistenceTests.swiftcmuxTests/ShellStartupMatrixTests.swift
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/NushellTypedShellCommand.swift (1)
17-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMark the pure value struct as
nonisolated.As per coding guidelines, pure value-model structs should be marked as
nonisolatedto prevent implicit@MainActorisolation in Swift 6.♻️ Proposed refactor
-public struct NushellTypedShellCommand { +nonisolated public struct NushellTypedShellCommand { /// The renderer is stateless; construct at the call site. public init() {}🤖 Prompt for 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. In `@Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/NushellTypedShellCommand.swift` around lines 17 - 19, Mark the pure value-model struct NushellTypedShellCommand as nonisolated while preserving its public initializer and stateless behavior.Source: Coding guidelines
🤖 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.
Duplicate comments:
In
`@Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/NushellTypedShellCommand.swift`:
- Around line 17-19: Mark the pure value-model struct NushellTypedShellCommand
as nonisolated while preserving its public initializer and stateless behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: aae8502c-0ffa-4071-adbd-0cc3c1c0a925
📒 Files selected for processing (7)
Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/NushellTypedShellCommand.swiftResources/shell-integration/nushell/cmux-nushell-integration.nuSources/RestorableAgentSession.swiftSources/SessionPersistence.swifttests/test_nushell_integration_hooks.pytests/test_nushell_resume_command_dialect.pytests/test_nushell_shim_path_refront.py
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. |
44a60cf to
05f7796
Compare
|
@lawrencecchen with the amount of PRs I fear this is never gonna get reviewed. Without this cmux is pretty much useless when you run Nushell. |
cmux has no shell integration for nushell: the cmux-cli-shims claude wrapper gets shadowed by user PATH prepends in env.nu (so sessions are never captured and resume never works), resume command strings are POSIX-only (parse errors when typed into or dispatched to nu), and the fish-parity socket reporting (tty/activity/pwd/ports) never happens. Red on purpose (two-commit regression policy): - tests/test_nushell_shim_path_refront.py drives the bundled nushell bootstrap (not yet present) through real nu and asserts the shim wins over user PATH prepends. - tests/test_nushell_integration_hooks.py drives the bundled nushell integration (not yet present) and asserts fish-format socket payloads. - tests/test_nushell_resume_command_dialect.py pins the nushell resume dialect semantics on real nu (green; the Swift builders adopt these golden shapes in the fix commit) and documents that the legacy POSIX resume string is a nushell parse error. CI installs a pinned, checksum-verified nushell 0.113.1 in the app-host-unit-tests focused-regression shard so the new tests actually run there; locally they skip loudly when nu is absent but fail if CI is set, so they can never silently skip on CI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ty integration Nushell login shells previously got no cmux shell integration at all (applyManagedShellSpecificStartupEnvironment fell through default:), so: user env.nu PATH prepends shadowed the per-surface cmux-cli-shims claude wrapper (sessions were never captured into ~/.cmuxterm/claude-hook-sessions.json and resume/notifications never worked), every resume string was a nushell parse error, and the tty/activity/pwd/ports socket reporting never ran. Capture: `case "nu"` returns a fish-style replacement launch command `'<shell>' -l -e '<payload>'` — nu's --execute runs the payload after the user's env.nu/config.nu and enters the REPL. The payload is the new bundled Resources/shell-integration/nushell/cmux-nushell-bootstrap.nu squashed to one line (re-fronts cmux-cli-shims PATH entries after user prepends, normalizing string PATHs) plus a `source` of the new integration file with the bundle path baked in (nushell `source` needs a parse-time constant). Missing bundle files degrade to a vanilla shell, matching the zsh/fish guards. Resume: cmux-generated resume/relaunch commands stay POSIX everywhere; NushellTypedShellCommand (CMUXAgentLaunch) wraps them at the final typed boundary as `^/bin/sh -c "<escaped>"` — the same portable-envelope approach as manaflow-ai#5639's /bin/sh -c wrapper token and the /bin/zsh launcher-script inputs. TerminalStartupTypedShellCommand applies it (from $SHELL dialect) at the typed-keystroke chokepoints only: sessions-panel resume, session drag-drop, clipboard copy, and agent startup/fork/hibernation inline inputs. Inline inputs embedded into the zsh launcher scripts stay raw POSIX — wrapping them made the launcher's `nu) /bin/sh -c '<cmd>'` dispatch print "/bin/sh: ^/bin/sh: No such file or directory" (caught in dogfood). The restore launcher script and the restored terminal command script gain a `nu)` dispatch branch that runs the POSIX command through /bin/sh, and the launcher re-enters nushell with the cmux bootstrap payload rebuilt at runtime from CMUX_SHELL_INTEGRATION_DIR so the resumed surface keeps the shim re-front and integration. Integration: Resources/shell-integration/nushell/cmux-nushell-integration.nu brings nushell to fish parity — report_tty/report_shell_state/report_pwd/ ports_kick over the cmux socket (ncat/socat/nc chain, `job spawn` background sends) from pre_execution/pre_prompt string hooks (def --env state persists in _CMUX_* env vars), claude/grok wrapper defs, scrollback restore, keyboard-protocol reset, and the remote-relay fallback. zsh-only extras (git-branch probes, PR polling, Ghostty job-table patching) are intentionally not replicated — fish does not have them either. Tests: the commit turns the red nushell regression suite green (tests/test_nushell_shim_path_refront.py, test_nushell_integration_hooks.py; test_nushell_resume_command_dialect.py pins the envelope semantics on real nu). Swift coverage: nu rows in ShellStartupMatrixTests (payload squash, quoting, dialect detection, typed-input wrap), the nu missing-bundle fallback, a real-nu typed-resume dispatch regression mirroring the fish/tcsh manaflow-ai#5639 tests, launcher-script regressions for the nu) dispatch and the raw-inline/typed-boundary split (including an end-to-end run of the generated launcher script under zsh with a fake nu login shell), and NushellTypedShellCommandTests in CMUXAgentLaunch. /usr/local/bin/nu is removed from the unsupported-shells matrix row. Verified on nushell 0.113.1: -e runs post-config with persistent env and hooks, `cd` persists from `if` blocks, quoted command heads are parse errors (hence the /bin/sh envelope), and job spawn exists for background sends. Dogfooded on a nushell login shell: capture, auto-resume, panel resume, and session drag-drop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…round-send coverage
Codex: resumeStartupInput/forkStartupInput are also used for remote
workspace restores and remote forks, where the returned input is typed
into the remote host's shell after attach — deriving the dialect from
the local $SHELL leaked the nushell `^/bin/sh -c` envelope to remote
POSIX shells. Thread an explicit dialect through startupInput and pass
.posix at the four remote call sites in Workspace.
Greptile P2: the resume launcher script interpolated
CMUX_SHELL_INTEGRATION_DIR into the nushell source literal unescaped;
escape backslashes and double quotes first (verified against a path
containing a double quote).
Greptile P1 claimed `job spawn { _cmux_send … }` jobs cannot resolve the
sourced def — disproven against real nu (closures capture command decls
at parse time; all payloads deliver), but the sync-only test coverage it
pointed at was a real gap: tests/test_nushell_integration_hooks.py now
exercises the background job-spawn path without CMUX_TEST_SYNC_SEND.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ket probe cache The no-ambient-global-state pre-merge rule forbids new caseless-enum static namespaces: NushellTypedShellCommand and TerminalStartupTypedShellCommand are now constructable structs (NushellTypedShellCommand() matching the AgentLaunchEnvironmentPolicy idiom in CMUXAgentLaunch; TerminalStartupTypedShellCommand owns its dialect). The four remote call sites now go through a named TerminalStartupShellDialect.remoteHost seam that documents the remote-shells-are-POSIX assumption in one place until the SSH bootstrap reports the actual remote shell back. CodeRabbit's hot-path finding: _cmux_socket_is_unix forked /bin/test on every prompt hook, before the activity-state dedupe. The probe now caches its positive result for the session (negative results re-probe so a socket that comes up late still gets found), and the dedupe check runs before the probe. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Swift gets DocC comments on the members that lacked them (TerminalStartupShellDialect.forShellPath, the TerminalStartupTypedShellCommand members, the startupInputWithLauncherScript overloads, and the dialect-carrying resume/fork startup inputs). Every def in the nushell integration and bootstrap now has a doc comment directly above it — nushell renders those in `help <command>` — and the never-called _cmux_relay_params helper found during this audit is deleted. The Python test helpers get PEP 257 one-liners. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The stateless renderer is nonisolated by default today (the package sets no default isolation), but the explicit marker keeps it callable from nonisolated contexts if CMUXAgentLaunch ever adopts MainActor default isolation, and Sendable matches the module's convention for value types (AgentLaunchEnvironmentPolicy, ClaudeConfigDirectoryPath). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
forkProjectedTmuxAgentConversationToNewWorkspace types its startup input into the remote host's shell after SSH attach; the default .loginShell dialect would wrap the POSIX payload as ^/bin/sh for local nushell logins, which the remote POSIX shell cannot parse — the same remote leak the PR review already fixed at the other remote fork sites. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
05f7796 to
b75d5bc
Compare
Fixes #10050
Nushell login shells get no shell integration:
applyManagedShellSpecificStartupEnvironmenthas cases for zsh, bash, and fish, andnufalls throughdefault:. For a nushell user this breaks in three places:env.nurebuilds PATH with its own prepends, which shadows the per-surfacecmux-cli-shimsshim. The zsh and fish integrations recover from this; nushell has nothing, socmux-claude-wrappernever runs and~/.cmuxterm/claude-hook-sessions.jsonnever gets written. Session resume and Claude notifications never work at all.cd -- '…' 2>/dev/null || [ ! -d '…' ] && 'claude' '--resume' '<id>'), and nushell rejects&&,||,[ … ], and quoted command heads like'claude'.The fix
Capture: a
case "nu"that launches nushell as'<shell>' -l -e '<payload>', mirroring the fish replacement command.--executeruns the payload after the user's config and then opens the REPL. The payload is the new bundledcmux-nushell-bootstrap.nusquashed to one line: it movescmux-cli-shimsentries back to the front of PATH, then sources the integration file (path baked in as a literal, since nushellsourceneeds a parse-time constant). If the bundle files are missing, the shell starts vanilla, same as the zsh/fish guards.Resume: commands stay POSIX everywhere. At the points where cmux types a command into the user's interactive shell (panel resume, session drag-drop, clipboard copy, startup/fork/hibernation inline inputs) they get wrapped as
^/bin/sh -c "<escaped>", the same delegate-to-sh idea as the #5639 wrapper token. Inline inputs embedded into the zsh launcher scripts are not wrapped. The scripts instead grow anu)dispatch branch that runs the command through /bin/sh and then execs nushell with the bootstrap payload rebuilt fromCMUX_SHELL_INTEGRATION_DIR, so a resumed surface keeps its shim. (The first dogfood round caught a wrapped string reaching /bin/sh:/bin/sh: ^/bin/sh: No such file or directory. There are regression tests for exactly that now.)Integration:
cmux-nushell-integration.nuprovides what the fish integration provides, with the same wire formats: report_tty, report_shell_state, report_pwd, ports_kick, claude/grok wrapper defs, scrollback restore, keyboard-protocol reset, remote relay fallback. State lives in_CMUX_*env vars and the hooks are registered as strings sodef --envmutations persist across prompts; background sends usejob spawn. The zsh-only extras (git-branch probes, PR polling, Ghostty job-table patching) are not replicated; fish doesn't have them either.Commits
Two commits, red then green.
nuagainst the bundled files: shim re-fronting under a hostileenv.nu, the sh envelope (quoting edges, non-ASCII printf substitutions, env prefixes, and a pin that the legacy POSIX string stays a parse error), and the hook wire formats against a unix-socket fixture. CI installs a pinned, checksum-verified nushell 0.113.1 in the shard that runs the wrapper tests. Locally the tests skip with a message whennuis absent; on CI they fail instead, so they can never silently skip.ShellStartupMatrixTests, the missing-bundle fallback, real-nu resume dispatch regressions next to the existing fish/tcsh Claude resume still drops cmux hooks after #5430: bareclaudedoesn't resolve to the wrapper inside the $SHELL -lic restore launcher #5639 tests, an end-to-end run of the generated launcher script under zsh with a fake nu login shell, andNushellTypedShellCommandTestsin CMUXAgentLaunch./usr/local/bin/nucomes out of the unsupported-shells matrix row, which is the clearest red-to-green flip in the diff.Verified
Dogfooded over two rounds on a machine whose login shell is nushell 0.113.1: capture, app-relaunch auto-resume (including the resumed surface keeping its shim), panel resume, and session drag-drop. The design choices (why
-e, why delegate to /bin/sh instead of teaching every builder a second dialect) came from probing real nu; the Python tests encode those probes.No user-facing strings changed, so nothing to localize.
Not covered
Full zsh parity (git-branch probes, PR polling, job-table patching), nushell on remote
cmux sshhosts, and users with a custom Ghosttycommand, who skip the replacement command; fish has the same limitation.🤖 Generated with Claude Code
Summary by CodeRabbit
nu) support for shell startup and typed, shell-safe session resume/command execution.cmux-cli-shimsare re-fronted in Nushell PATH during bootstrap./bin/sh -cboundary and preserving integration on re-entry.