Repository navigation
Add selected-workspace provider actions to cmux TUI - #8943
Conversation
|
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:
📝 WalkthroughWalkthroughThe PR extends provider action protocol models with capability negotiation and target context, binds machine/workspace identifiers through invocation, stages destructive actions for confirmation, localizes provider presentation, and adds durable-notice and external-connect coordination. ChangesProvider action and runtime integration
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant App
participant ProviderMachineRuntime
participant ProviderClient
participant Provider
App->>ProviderMachineRuntime: submit scoped provider action
ProviderMachineRuntime->>ProviderClient: invoke with machine/workspace identifiers
ProviderClient->>Provider: send InvokeAction request
Provider->>ProviderClient: deliver durable notice or UI update
ProviderClient->>ProviderMachineRuntime: forward notice or snapshot refresh
ProviderMachineRuntime->>App: publish MachineUpdate
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (22 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 SummaryAdds typed provider-action targets and binds selected machine/workspace context before invocation.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant TUI
participant Client as Provider Client
participant Provider
TUI->>Provider: hello
Provider-->>TUI: capabilities
TUI->>Provider: negotiate_client_capabilities
Provider-->>TUI: accepted targeted-action capability
TUI->>Provider: snapshot
Provider-->>TUI: actions with typed targets
User->>TUI: select provider action
TUI->>TUI: bind active machine/workspace context
alt destructive action
TUI->>User: request confirmation
User->>TUI: CONFIRM
end
TUI->>Client: invoke action with bound IDs
Client->>Provider: invoke_action
Reviews (12): Last reviewed commit: "Mark workspace action test session activ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
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. |
…kspace-ports-contract
|
@coderabbitai review |
|
@greptileai review |
✅ Action performedReview finished.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2d08d2ae1
ℹ️ 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".
…kspace-ports-contract
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 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 `@cmux-tui/crates/cmux-tui-machine-protocol/src/lib.rs`:
- Around line 162-170: Add a localized sidebar catalog entry for invalid
external-machine specifiers, then update connect_external_after_refresh at the
runtime boundary to detect InvalidExternalMachineSpecifier and map it to that
catalog message before rendering. Preserve existing handling for other
external-connect failures and ensure rejected host or pairing inputs use the
selected UI language.
- Around line 622-641: Update ProviderActionTarget to include an Unsupported
catch-all deserialization variant for unknown target values, preserving existing
defaults and serialization behavior. In provider presentation construction, skip
actions whose target is Unsupported so an unknown individual action cannot
invalidate the surrounding SnapshotResult.
In `@cmux-tui/crates/cmux-tui/src/app.rs`:
- Around line 4737-4752: Remove the direct assignment to
machine_provider_reconnect_retry_at in the durable-notice acknowledgement error
branch. Route the retry through schedule_machine_provider_reconnect(),
preserving the existing failure counter and reconnect request behavior so that
provider reconnect backoff remains owned by that method.
- Around line 7710-7725: Update provider_action_context to derive workspace_id
from the active machine snapshot identified while resolving machine_id,
selecting only that machine’s active workspace and preserving the non-empty
filter. Do not use tree.active_workspace() independently, so
ProviderActionContext cannot combine a machine ID with a workspace owned by
another machine.
In `@cmux-tui/crates/cmux-tui/src/localization.rs`:
- Around line 533-556: Centralize the provider action IDs in the provider
protocol crate, then import and use those shared constants in localization.rs.
Update provider_action_label and provider_action_field_label to replace every
hard-coded workspace port action string while preserving the existing match
behavior and fallback handling.
In `@cmux-tui/crates/cmux-tui/src/machine_agent/mod.rs`:
- Around line 55-64: Update run so it preserves run_agent’s actionable error
categories instead of mapping every failure to messages.runtime_failed. Classify
errors into localized outcomes for already-running agents, non-private state
directories, invalid connection options, and transport failures, with messages
stating the product impact and one or two next actions; retain raw error details
only for CMUX_MACHINE_AGENT_DEBUG diagnostics.
In `@cmux-tui/crates/cmux-tui/src/machine_agent/runtime.rs`:
- Around line 669-676: The worker runtime currently discards the error returned
by `state.run` and only sends a `Closed` event. Capture the run result in this
generation flow, and when it fails, convert it with `generation_failure_kind`
and report it through `reporter.diagnostic`, matching the existing
`start_generation` failure handling while preserving the existing shutdown and
`Closed` notification behavior.
- Around line 453-476: The synchronous wait in commit_migration_replacement
blocks the coordinator for the full HANDSHAKE_TIMEOUT and prevents event
processing. Replace the direct result.recv_timeout wait with a short,
coordinator-safe commit deadline or an event-draining wait loop, while
preserving successful acknowledgment behavior and the existing failure
cleanup/reporting paths.
In `@cmux-tui/crates/cmux-tui/src/machine_agent/transport.rs`:
- Around line 220-245: Extract the accumulate-until-newline logic from
read_bounded_json_line into a shared read_bounded_line(reader, limit) helper,
preserving its size-limit and EOF error semantics. Update read_bounded_json_line
and read_frame to call the shared helper, with each retaining only its
format-specific decoding or framing behavior.
In `@cmux-tui/crates/cmux-tui/src/machine.rs`:
- Around line 1049-1073: Extend provider_action_binds_selected_workspace_context
to also cover the SelectedMachine target and a context with machine_id set to
None. Assert that the request returns
Err(ProviderActionInputError::MissingSelectedMachine), preserving the
fail-closed behavior before any unbound provider action is submitted.
In `@cmux-tui/crates/cmux-tui/src/main.rs`:
- Around line 329-343: Update usage_for_platform so disabling machine-agent
support removes the entire corresponding usage line, not merely replacing its
placeholder with an empty string. Preserve the existing machine-agent entry when
supports_machine_agent is true and keep the resulting non-Unix help output free
of whitespace-only lines.
- Around line 1670-1679: Update
startup_help_localizes_the_machine_agent_entrypoint and its usage_for invocation
so the test explicitly renders the machine-agent usage as supported, independent
of the host compile target; preserve the English and Japanese localization
assertions while avoiding the cfg!(unix) unsupported-platform path.
In `@cmux-tui/docs/machines.md`:
- Around line 89-109: Update cmux-tui/docs/machines.md at lines 89-109 to show
the interactive SSH prerequisite using the resolved cloud host, user, port, and
identity, and include --cloud-host in the documented overrides. Update
cmux-tui/spec/machine-agent.md at lines 21-23 to require that this interactive
trust/authentication step uses the same resolved destination and identity as the
agent.
In `@cmux-tui/README.md`:
- Line 34: Add an explicit Unix-only caveat to both machine-agent examples in
the README, covering the direct cargo command and the npx command. Keep the
command examples unchanged while clearly stating that non-Unix platforms do not
expose machine-agent.
🪄 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: 4eb0d0df-e2e3-41c8-8348-1c485182cd5f
⛔ Files ignored due to path filters (1)
cmux-tui/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (31)
cmux-tui/Cargo.tomlcmux-tui/README.mdcmux-tui/crates/cmux-tui-machine-agent-protocol/Cargo.tomlcmux-tui/crates/cmux-tui-machine-agent-protocol/src/lib.rscmux-tui/crates/cmux-tui-machine-protocol/Cargo.tomlcmux-tui/crates/cmux-tui-machine-protocol/src/lib.rscmux-tui/crates/cmux-tui/Cargo.tomlcmux-tui/crates/cmux-tui/src/app.rscmux-tui/crates/cmux-tui/src/localization.rscmux-tui/crates/cmux-tui/src/machine.rscmux-tui/crates/cmux-tui/src/machine_agent/identity.rscmux-tui/crates/cmux-tui/src/machine_agent/mod.rscmux-tui/crates/cmux-tui/src/machine_agent/protocol_io.rscmux-tui/crates/cmux-tui/src/machine_agent/runtime.rscmux-tui/crates/cmux-tui/src/machine_agent/transport.rscmux-tui/crates/cmux-tui/src/machine_provider_client.rscmux-tui/crates/cmux-tui/src/machine_provider_runtime.rscmux-tui/crates/cmux-tui/src/machine_provider_transport.rscmux-tui/crates/cmux-tui/src/main.rscmux-tui/crates/cmux-tui/src/ui/mod.rscmux-tui/crates/cmux-tui/src/ui/rail.rscmux-tui/crates/cmux-tui/tests/cli.rscmux-tui/docs/README.mdcmux-tui/docs/configuration.mdcmux-tui/docs/getting-started.mdcmux-tui/docs/machines.mdcmux-tui/docs/mouse.mdcmux-tui/spec/README.mdcmux-tui/spec/cli.mdcmux-tui/spec/machine-agent.mdcmux-tui/spec/machine-provider.md
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. |
|
@coderabbitai review |
|
@greptileai review |
✅ Action performedReview finished.
|
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 `@cmux-tui/crates/cmux-tui/src/machine_agent/mod.rs`:
- Around line 68-69: Update the StderrReporter::new error mapping in the
machine-agent initialization path to use a new pairing_terminal_unavailable
user-facing message instead of runtime_failed; add this message to
MachineAgentMessages for both English and Japanese, instructing users to start
the agent from an interactive terminal while preserving the existing diagnostic
emission.
🪄 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: 331de10d-667d-45f0-b560-52b32211ed02
📒 Files selected for processing (5)
cmux-tui/crates/cmux-tui/src/localization.rscmux-tui/crates/cmux-tui/src/machine_agent/identity.rscmux-tui/crates/cmux-tui/src/machine_agent/mod.rscmux-tui/crates/cmux-tui/src/machine_agent/runtime.rscmux-tui/crates/cmux-tui/src/machine_provider_runtime.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7179859e52
ℹ️ 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".
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. |
|
@coderabbitai review |
|
@codex review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7752a5090f
ℹ️ 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmux-tui/crates/cmux-tui-machine-protocol/src/lib.rs (1)
1248-1261: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd a wire-encoding test for non-default action targets.
The tests cover legacy/default omission and unknown-target handling, but do not verify that
SelectedMachineandSelectedWorkspaceserialize to their protocol values. Add at least one non-default target to the snapshot fixture and assert its encoded"target"value, since a regression here could silently drop invocation scoping.Also applies to: 1397-1490
🤖 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 `@cmux-tui/crates/cmux-tui-machine-protocol/src/lib.rs` around lines 1248 - 1261, Extend the ProviderAction serialization tests around the existing action fixtures to include a non-default target, preferably SelectedMachine or SelectedWorkspace, and assert that serde_json::to_value encodes its target as the corresponding protocol string. Preserve the existing legacy/default omission and unknown-target assertions while covering the wire encoding for at least one non-default scope.
🤖 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 `@cmux-tui/crates/cmux-tui-machine-protocol/src/lib.rs`:
- Around line 1248-1261: Extend the ProviderAction serialization tests around
the existing action fixtures to include a non-default target, preferably
SelectedMachine or SelectedWorkspace, and assert that serde_json::to_value
encodes its target as the corresponding protocol string. Preserve the existing
legacy/default omission and unknown-target assertions while covering the wire
encoding for at least one non-default scope.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 24b02bbd-11e7-49a6-ae5e-43c0665780e5
📒 Files selected for processing (3)
cmux-tui/crates/cmux-tui-machine-protocol/src/lib.rscmux-tui/crates/cmux-tui/src/localization.rscmux-tui/crates/cmux-tui/src/machine.rs
|
@codex review |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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: 2
🤖 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 `@cmux-tui/crates/cmux-tui-machine-protocol/src/lib.rs`:
- Around line 580-591: Update
SnapshotResult::retain_actions_for_client_capabilities to fail closed by
retaining only actions whose target is ProviderActionTarget::Scope when the
client lacks PROVIDER_ACTION_TARGETS_CLIENT_CAPABILITY; explicitly exclude
ProviderActionTarget::Unsupported so it cannot reach the provider-presentation
path.
In `@cmux-tui/crates/cmux-tui/src/machine_provider_runtime.rs`:
- Around line 3244-3304: Update the capability-negotiation response in
targeted_actions_are_negotiated_before_the_initial_snapshot to use
protocol::ResponseEnvelope::success(...) instead of hand-built json!, preserving
the negotiated capabilities payload. Read the subsequent frame through the typed
protocol envelope path, using the existing read_frame helpers and keeping the
test’s negotiation assertions 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: e72232b5-2a17-4dba-9a64-67aa096726ba
📒 Files selected for processing (4)
cmux-tui/crates/cmux-tui-machine-protocol/src/lib.rscmux-tui/crates/cmux-tui/src/machine_provider_client.rscmux-tui/crates/cmux-tui/src/machine_provider_runtime.rscmux-tui/spec/machine-provider.md
|
@codex review |
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 `@cmux-tui/crates/cmux-tui/src/machine_provider_runtime.rs`:
- Around line 3245-3304: Extend
targeted_actions_are_negotiated_before_the_initial_snapshot to advertise
client-capability-negotiation-v1 together with durable-notices-v1 and
machine-lifecycle-v1, and assert the negotiation request remains the first
post-Hello message before any subscription or snapshot request. Return the
combined negotiated capabilities and preserve the existing targeted-action
snapshot assertions.
🪄 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: 04c2722d-01d8-4b73-a504-2e7193c7d272
📒 Files selected for processing (4)
cmux-tui/crates/cmux-tui-machine-protocol/src/lib.rscmux-tui/crates/cmux-tui/src/machine_provider_client.rscmux-tui/crates/cmux-tui/src/machine_provider_runtime.rscmux-tui/spec/machine-provider.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1f4ee8e5e
ℹ️ 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".
|
@codex review |
|
@codex review |
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Adds typed scope, machine, and workspace targets to provider actions.
Binds selected machine/workspace IDs before destructive confirmation, forwards them through the shared provider client, localizes known port actions, and covers keyboard/mouse prompt submission plus legacy protocol decoding.
Tests:
cargo +1.97.1 test -p cmux-tui-machine-protocolcargo +1.97.1 test -p cmux-tui --no-fail-fastcargo +1.97.1 clippy -p cmux-tui -p cmux-tui-machine-protocol --all-targets -- -D warningsSummary by CodeRabbit