Add Herdr CLI compatibility shim (Step toward native Herdr ) - #8736
Closed
RaviTharuma wants to merge 28 commits into
Closed
RaviTharuma wants to merge 28 commits into
RaviTharuma wants to merge 28 commits into
Conversation
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. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Author
|
Broader design / parity tracking issue: #8737. The external fallback bridge and full design package live at https://github.com/RaviTharuma/cmux-herdr. |
13 tasks
|
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:
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 hidden ChangesHerdr compatibility bridge
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant cmux
participant CMUXCLI
participant ExecutableResolver
participant PATH
participant herdr
cmux->>CMUXCLI: Run __herdr-compat command
CMUXCLI->>ExecutableResolver: Resolve herdr from supplied PATH
ExecutableResolver->>PATH: Search configured directories
PATH-->>ExecutableResolver: Return executable path
CMUXCLI->>herdr: execv translated arguments
herdr-->>cmux: Return output and exit status
Suggested reviewers: Merge Risk: 🟡 Moderate · up to The hidden compatibility command currently has two behavior risks: some aliases may handle explicit JSON flags incorrectly, and localized errors or help may fall back to untranslated text when launched from the packaged application; it also adds translations outside the supported locale set. These bounded issues should be addressed before merging. Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning, 1 inconclusive)
✅ Passed checks (22 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Greptile SummaryThis PR adds a hidden
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant cmux as cmux CLI
participant Resolver as resolveExecutableInSuppliedSearchPath
participant PATH as Process PATH
participant herdr as herdr binary
User->>cmux: "cmux [--json] __herdr-compat <alias> [args]"
cmux->>cmux: Strip leading --json flags → effectiveJSONOutput
cmux->>cmux: Check --help / missing command → print usage or exit 2
cmux->>cmux: Reject alias-local --json on JSON-only aliases (exit 2)
cmux->>cmux: herdrCompatArguments(alias) → herdr subcommand args
note right of cmux: status→[status], snapshot→[api,snapshot],<br/>list-workspaces→[workspace,list], etc.<br/>Inject --json only for status
cmux->>Resolver: resolveExecutableInSuppliedSearchPath("herdr", PATH)
Resolver->>PATH: Split on ":", empty component → cwd
Resolver->>Resolver: normalizedDirectories (validate pre-normalization)
Resolver->>Resolver: Skip directories shadowing executables
Resolver->>Resolver: Exclude cmux bundled bin
Resolver-->>cmux: /path/to/herdr (or nil → exit 127)
cmux->>cmux: "Filter CMUX_*/CMUXD_* from child environment"
cmux->>cmux: Build C argv/envp with strdup (defer frees on failure)
cmux->>herdr: execve(herdr, [herdr, subcmd…], filtered_env)
note over herdr: Process replaced — stdout/stderr/<br/>signals/exit owned by herdr
Reviews (13): Last reviewed commit: "fix: isolate Herdr execution environment..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@CLI/CMUXCLI`+HerdrCompat.swift:
- Around line 24-27: Centralize the supported Herdr compatibility commands in
one shared symbol, then update herdrCompatArguments, the unknown-command
CLIError message, and herdrCompatUsage to derive their command lists from it.
Preserve the existing command ordering and behavior while ensuring future
aliases automatically appear consistently in dispatch, errors, and help text.
- Around line 71-84: Update resolveHerdrExecutable to reject directory-valued
herdr candidates before accepting them as executables: verify the candidate is
not a directory, then apply isExecutableFile(atPath:). Preserve the existing
PATH traversal and continue searching when a matching directory is encountered.
In `@cmuxTests/CMUXCLIErrorOutputRegressionTests.swift`:
- Around line 57-93: Extend testHerdrCompatAliasesAndUnknownCommand with
hermetic process cases using an empty PATH directory: verify a Herdr-dependent
compatibility command reports executable-resolution failure with the required
nonzero status and output, and invoke __herdr-compat --help to assert its
documented exit status and output contract. Keep the existing alias and
unknown-command coverage unchanged.
🪄 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 Plus
Run ID: d4ef324e-f407-4bef-8488-16352148274f
📒 Files selected for processing (5)
CLI/CMUXCLI+CommandSuggestions.swiftCLI/CMUXCLI+HerdrCompat.swiftCLI/cmux.swiftcmux.xcodeproj/project.pbxprojcmuxTests/CMUXCLIErrorOutputRegressionTests.swift
Author
|
Addressed the Greptile review in
Verification: Built CLI smoke tests pass for all aliases plus unknown-command and missing-provider exit paths. |
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 `@CLI/CMUXCLI`+HerdrCompat.swift:
- Around line 46-52: Update the launch-failure handling in the Herdr
compatibility flow to use a provider-neutral recovery message without
interpolating executable or strerror details. Revise the
cli.herdrCompat.error.launchFailed localization entry and all catalog values to
remove format placeholders, and adjust the message construction to match the new
non-parameterized string while preserving the existing exit code.
🪄 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 Plus
Run ID: e9dd767b-9717-4269-9033-59260aefdd30
📒 Files selected for processing (2)
CLI/CMUXCLI+HerdrCompat.swiftResources/Localizable.xcstrings
Author
|
Pushed a comprehensive review follow-up in
I did not forward cmux Verification:
|
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 `@CLI/CMUXCLI`+HerdrCompat.swift:
- Around line 49-52: Update the Herdr compatibility argument construction used
by freeHerdrCompatArguments so any strdup allocation failure is detected
immediately before execv runs. Abort the launch with the existing
failure-handling path and ensure partially built argv data is released, rather
than appending nil or continuing with incomplete arguments.
In `@cmuxTests/CMUXCLIErrorOutputRegressionTests.swift`:
- Around line 93-94: Pin the locale to English for the subprocesses exercised by
the output assertions in CMUXCLIErrorOutputRegressionTests, including the cases
covering the assertions around delete-everything and status output. Configure
the subprocess environment or invocation so these English string checks are
stable regardless of the host locale.
- Around line 96-105: Update the __herdr-compat --help test around runProcess so
it cannot resolve the fake herdr executable: remove that fixture or invoke the
help process with an empty PATH. Preserve the existing help output and
successful-status assertions while ensuring the test passes independently of
Herdr provider resolution.
🪄 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 Plus
Run ID: ba1be4f5-abe1-4135-8117-81fa885c3ba1
📒 Files selected for processing (4)
CLI/CMUXCLI+ExecutableResolution.swiftCLI/CMUXCLI+HerdrCompat.swiftResources/Localizable.xcstringscmuxTests/CMUXCLIErrorOutputRegressionTests.swift
Author
|
Addressed all three latest CodeRabbit findings in |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CLI/CMUXCLI+HerdrCompat.swift (1)
71-73: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winForward explicit
--jsonfor every translated command.The CLI parser supplies
jsonOutputout-of-band, but this branch drops it forsnapshotand the list aliases. Thuscmux --json __herdr-compat snapshotlaunchesherdr api snapshotinstead of forwarding--json, silently turning Herdr’s required rejection into success. Forward it uniformly and add a regression case for a non-statusalias.Proposed fix
- if command == "status", jsonOutput, !arguments.contains("--json") { + if jsonOutput, !arguments.contains("--json") { prefix.append("--json") }🤖 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 `@CLI/CMUXCLI`+HerdrCompat.swift around lines 71 - 73, Update the translated-command argument handling around the command == "status" check so jsonOutput appends "--json" for every Herdr-compatible command, not only status or when the flag is already present. Preserve the duplicate-prevention behavior for explicit arguments, and add a regression case covering a non-status alias such as snapshot.
🤖 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.
Outside diff comments:
In `@CLI/CMUXCLI`+HerdrCompat.swift:
- Around line 71-73: Update the translated-command argument handling around the
command == "status" check so jsonOutput appends "--json" for every
Herdr-compatible command, not only status or when the flag is already present.
Preserve the duplicate-prevention behavior for explicit arguments, and add a
regression case covering a non-status alias such as snapshot.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a97b0fa5-edda-40e6-bd59-ff0a7b430ad7
📒 Files selected for processing (2)
CLI/CMUXCLI+HerdrCompat.swiftcmuxTests/CMUXCLIErrorOutputRegressionTests.swift
Author
Follow-up context from the stopgap + design packageNative remains the primary path; this PR stays intentionally a hidden compat shim, not nested topology. Living docs (plugin repo):
Tracking issue for the real tree model: #8737 Happy to address the open review notes on this shim ( |
This was referenced Jul 23, 2026
This was referenced Jul 24, 2026
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. |
5 tasks
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 7 files
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
Re-trigger cubic
austinywang
force-pushed
the
feat/herdr-native-mvp
branch
from
August 1, 2026 06:14
10bff07 to
650dfec
Compare
6 tasks
This was referenced Aug 3, 2026
Document that only `status` maps cmux `--json` to a provider flag; list and snapshot aliases always emit JSON and ignore `--json` so we do not inject an unsupported flag. Soften usage copy away from a hard-coded vendor label.
Co-authored-by: Ravi Tharuma <RaviTharuma@users.noreply.github.com>
Co-authored-by: Ravi Tharuma <RaviTharuma@users.noreply.github.com>
Remove committed conflict markers from Localizable.xcstrings, rebuild the three herdr-compat keys on top of current main (all 20 locales), and align en/ja launchFailed/unknownCommand copy with the provider-neutral runtime strings. Drop superseded duplicate herdr process tests from CMUXCLIErrorOutputRegressionTests so coverage lives only in CMUXCLIHerdrCompatTests. Co-authored-by: Ravi Tharuma <RaviTharuma@users.noreply.github.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: cmux reload-cloud <cmux-reload-cloud@users.noreply.github.com>
Co-authored-by: cmux reload-cloud <cmux-reload-cloud@users.noreply.github.com>
Co-authored-by: cmux reload-cloud <cmux-reload-cloud@users.noreply.github.com>
Co-authored-by: cmux reload-cloud <cmux-reload-cloud@users.noreply.github.com>
The French localization assertions cubic flagged (P0) were removed with the test that used them; the helper kept mapping locale == "fr" to fr_FR, which no caller reaches and which no longer has catalog coverage. Both live callers pass "en" or "ja", so the POSIX locale stays en_US either way.
A rebase of this branch reverted the en and ja values of the three cli.herdrCompat.* keys to their pre-review text while the other 18 locales kept the corrected wording, so en/ja disagreed with both the code and the tests: - error.launchFailed: en/ja had resurfaced the provider name and the resolved filesystem path via a 2-argument format, but the call site passes no format arguments, so users would have seen literal %1$@/%2$@. - error.unknownCommand: en/ja used a 1-argument '%@' with the alias list frozen into the translation, while the call site passes command plus Self.herdrCompatCommandList as %1$@/%2$@; the list would drift silently. - usage: en/ja dropped the note that an alias-local --json is rejected, which the dispatcher now enforces before translation. en now matches each defaultValue exactly and ja matches the assertions in testHerdrCompatDiagnosticsUseSuppliedPATHAndRemainProviderNeutral.
RaviTharuma
force-pushed
the
feat/herdr-native-mvp
branch
from
September 10, 2026 06:52
07122d7 to
d543bda
Compare
Author
|
@austinywang friendly ping — Herdr CLI compatibility shim is still open and ready for a maintainer look. Happy to rebase onto current main or address any follow-up notes. |
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
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
Add a small, hidden
cmux __herdr-compatdispatcher that translates a stable set of discovery aliases to the installed Herdr CLI:status→herdr statussnapshot→herdr api snapshotlist-workspaces→herdr workspace listlist-tabs→herdr tab listlist-panes→herdr pane listThe dispatcher uses
execv, so Herdr owns stdout, stderr, signals, and exit status. It does not require a running cmux socket and does not claim native topology/UI parity.Why
Herdr is an agent-focused terminal multiplexer with a structured workspace/tab/pane/agent API. This narrow compatibility seam gives launchers and future integrations one cmux-owned entry point while keeping the first change independently reviewable. A separate user-space bridge remains available as a stopgap: https://github.com/RaviTharuma/cmux-herdr
The broader native nested-topology design is documented in that repository under
docs/upstream/and is intentionally not bundled into this PR.Tests
CODE_SIGNING_ALLOWED=NO.__herdr-compat --help__herdr-compat snapshot(valid JSON)__herdr-compat list-panes--json, child exit propagation, unknown commands, missing Herdr, and help.cmux-unitcurrently cannot complete in this checkout because pre-existingAgentResumeLivenessTests.swiftfails to compile (missing argument for parameter 'processLiveness'). The Herdr test file itself compiles after fixing its initial direct-internal-type assumption.Scope
This is additive and hidden. It does not:
tmuxbinary,Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds a hidden
cmux __herdr-compatcommand that forwards discovery aliases to an installedherdrCLI, giving launchers a cmux-owned entry point while cmux works toward native Herdr parity. The command replaces cmux withherdrviaexec, so Herdr owns output, signals, and exit status without needing a cmux socket.status,snapshot,list-workspaces,list-tabs, andlist-panes.--jsontostatus --json; JSON-only aliases ignore it and reject alias-local--json.PATH, treats empty components as the current directory, skips directories and cmux-bundled binaries, and rejects malformed components.CMUX_*andCMUXD_*variables from the child environment.Written for commit 951f519. Summary will update on new commits.
Summary by CodeRabbit
New Features
herdrCLI commands.Bug Fixes
Localization
Note
Medium Risk
Introduces a new exec-based CLI bridge and tightens PATH resolution semantics; impact is limited because the command is hidden and scoped, but incorrect PATH handling could still cause surprising launch failures.
Overview
Adds a hidden top-level command
cmux __herdr-compatthat maps a small set of discovery aliases to the installed Herdr CLI and replaces the process withexecv, so stdout, stderr, signals, and exit codes come from Herdr—not from a cmux socket.Aliases:
status,snapshot,list-workspaces,list-tabs, andlist-panesforward to the correspondingherdrsubcommands. Top-level cmux--jsonmaps tostatus --jsononly; for the list/snapshot aliases it is accepted but not passed through because those commands always emit JSON.Executable lookup for this path uses
resolveExecutableInSuppliedSearchPath: only the processPATH, with shell-like empty components and an emptyPATHtreated as the current directory—no HOME/provider fallbacks. Shared resolution logic now rejects directory names that shadow binaries and centralizes directory normalization.Localized help and provider-neutral errors, command-suggestion registration for
__herdr-compat, and process-level regression tests cover translation, PATH edge cases, and missing-binary behavior.Reviewed by Cursor Bugbot for commit 11a6127. Bugbot is set up for automated code reviews on this repo. Configure here.