[REMOTE-2178] Add oz runner CRUD CLI commands#13760
Conversation
Add `oz runner list|create|update|delete` commands backed by the server GraphQL `getRunners` query and `upsertRunner`/`deleteRunner` mutations, gated behind a new `CloudAgentRunnerCLICommands` feature flag (enabled on dogfood). - Mirror the server runner schema (types, inputs, enums, unions) into the client `schema.graphql` and add cynic operations. - Add the `runner` subcommand to `warp_cli` with clap arg definitions, a pure `validate_os_config` helper, and unit tests. - Implement the SDK runner runner in the app crate (list/create/update/ delete via `send_graphql_request`), with a table/JSON output model. - Wire the feature flag, dispatch, auth, and telemetry. Co-Authored-By: Warp <agent@warp.dev>
|
@warp-dev-github-integration[bot] I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR adds a feature-flagged oz runner CRUD command group, GraphQL operations/schema bindings, telemetry events, and CLI argument tests for managing cloud agent runners. No approved or repository spec context was available, so there are no spec-drift findings.
Concerns
runner updateaccepts both a positional UID and--name, but the implementation resolves by UID first and silently ignores--name, so ambiguous or mismatched targets can update the wrong runner instead of failing fast.runner deleterefuses non-interactive deletes without--force, but that refusal currently follows the same success termination path as an interactive cancellation, which can make automation treat a failed delete as successful.- For this user-facing CLI change, please include a terminal transcript, screenshot, or short recording demonstrating the runner command flows end to end.
Verdict
Found: 0 critical, 3 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| } | ||
|
|
||
| #[derive(Debug, Clone, Args)] | ||
| #[command(group(ArgGroup::new("runner_identifier").required(true).multiple(true).args(["id", "name"])))] |
There was a problem hiding this comment.
multiple(true) allows oz runner update <uid> --name <other>; resolve_runner then ignores --name and updates the UID, so ambiguous targets are accepted instead of rejected.
| #[command(group(ArgGroup::new("runner_identifier").required(true).multiple(true).args(["id", "name"])))] | |
| #[command(group(ArgGroup::new("runner_identifier").required(true).multiple(false).args(["id", "name"])))] |
There was a problem hiding this comment.
Fixed — the identifier ArgGroup is now exclusive (multiple(false)), so passing both a UID and --name is rejected (91363df).
|
|
||
| fn delete(&self, args: DeleteRunnerArgs, ctx: &mut ModelContext<Self>) { | ||
| if !args.force && !confirm_delete(&args.id) { | ||
| ctx.terminate_app(TerminationMode::ForceTerminate, None); |
There was a problem hiding this comment.
confirm_delete returns false, including the non-interactive refusal path, so scripts can treat a failed delete as success; report a fatal error for non-interactive refusal and reserve zero exit for an interactive “no”.
There was a problem hiding this comment.
Fixed — a non-interactive refusal now returns a fatal (non-zero) error via report_fatal_error; only an interactive decline exits 0 (91363df).
| request_context: get_request_context(), | ||
| sort_by, | ||
| }); | ||
| let response = server_api.send_graphql_request(op, None).await?; |
There was a problem hiding this comment.
Instead of calling send_graphql_request directly, we should use a ServerApi client trait. This doesn't fit neatly into any of the existing ones, so we can add a new FactoryClient trait - see ManagedMcpClient as a recent example
There was a problem hiding this comment.
Done in 91363df — added a FactoryClient ServerApi trait (app/src/server/server_api/factory.rs) with a get_factory_client() accessor following ManagedMcpClient; all runner GraphQL now routes through it.
|
|
||
| ctx.spawn( | ||
| async move { | ||
| let op = UpsertRunner::build(UpsertRunnerVariables { |
There was a problem hiding this comment.
This should also go in the FactoryClient
There was a problem hiding this comment.
Moved into FactoryClient::upsert_runner (91363df).
|
|
||
| let input = build_update_input(&args, &existing.config)?; | ||
|
|
||
| let op = UpsertRunner::build(UpsertRunnerVariables { |
There was a problem hiding this comment.
Ideally, one FactoryClient method for upserting a runner handles both create and update commands
There was a problem hiding this comment.
Done — a single FactoryClient::upsert_runner method now serves both create and update (91363df).
|
|
||
| ctx.spawn( | ||
| async move { | ||
| let op = DeleteRunner::build(DeleteRunnerVariables { |
There was a problem hiding this comment.
This should also go in the FactoryClient
There was a problem hiding this comment.
Moved into FactoryClient::delete_runner (91363df).
| } | ||
|
|
||
| /// Convert a resolved [`Owner`] into the GraphQL owner input. | ||
| fn owner_to_input(owner: Owner) -> GqlOwner { |
There was a problem hiding this comment.
I think this conversion already exists
There was a problem hiding this comment.
Removed the duplicate helper and reused the existing owner→GraphQL owner-input conversion (91363df).
| Cell::new("Shape"), | ||
| Cell::new("OS"), | ||
| Cell::new("Arch"), | ||
| Cell::new("Docker image / macOS version"), |
There was a problem hiding this comment.
Instead of this kind of confusing column, let's call it OS Settings and have the value specify whether it's the Docker image or macOS version
There was a problem hiding this comment.
Renamed the column to OS Settings; the value now states whether it's the Docker image or macOS version (91363df).
| command: RunnerCommand, | ||
| } | ||
|
|
||
| fn parse_command(argv: &[&str]) -> RunnerCommand { |
There was a problem hiding this comment.
We don't need tests for clap validation, only our own functions
There was a problem hiding this comment.
Removed the clap argument-parsing tests; kept only tests for our own functions (validate_os_config, confirm_delete) (91363df).
Address the review feedback on #13760: - Introduce a `FactoryClient` ServerApi trait (app/src/server/server_api/factory.rs) and route all runner GraphQL (get/upsert/delete) through it instead of calling send_graphql_request directly from the handler, following ManagedMcpClient. - Use a single `upsert_runner` method for both `create` and `update`. - Reuse the existing owner->GraphQL conversion instead of a duplicate helper. - Rename the list column to "OS Settings" with the value stating Docker image vs macOS version. - Reject ambiguous `update <uid> --name` via an exclusive ArgGroup, and make a non-interactive `delete` refusal return a fatal (non-zero) error. - Drop the clap-argument-parsing tests, keeping tests for our own functions (validate_os_config, confirm_delete). Co-Authored-By: Warp <agent@warp.dev>
|
All review points addressed in 91363df (runner ops now go through a new Re: the request for an end-to-end command transcript — the commands are gated behind the |
|
Smoke test update (ran the CLI against a live backend):
Two minor notes: the top-level |
|
|
||
| /// Target CPU architecture for a runner sandbox. | ||
| #[derive(Debug, Clone, Copy, PartialEq, Eq, ValueEnum)] | ||
| pub enum RunnerArchArg { |
There was a problem hiding this comment.
This should also have a default auto value, which resolves to the default architecture for the runner OS (x64 on Linux, arm64 on macOS)
There was a problem hiding this comment.
Done in cb3b784 — --arch now has an auto value (the new default) that resolves to the OS default: x86-64 on Linux, aarch64 on macOS (resolve_arch).
| /// macOS version to use for a runner sandbox. | ||
| #[derive(Debug, Clone, Copy, PartialEq, Eq, ValueEnum)] | ||
| pub enum RunnerMacosVersionArg { | ||
| #[value(name = "macos-14")] |
There was a problem hiding this comment.
Remove the macos- prefix from these; it's redundant
There was a problem hiding this comment.
Done in cb3b784 — dropped the macos- prefix; values are now --macos-version 14|15|26|27.
| } | ||
| }; | ||
|
|
||
| let instance_shape = match (args.vcpus, args.memory_gb) { |
There was a problem hiding this comment.
On update, it should be possible to change either vCPUs or memory independently, preserving the existing value of the other setting
There was a problem hiding this comment.
Done in cb3b784 — update now changes vCPUs and memory independently: an unspecified dimension is preserved from the existing shape (merge_instance_shape), and --vcpus/--memory-gb no longer require each other.
…dent vCPU/memory updates Addresses bnavetta's review feedback on the oz runner CLI: - warp_cli runner args: add an `Auto` variant to `RunnerArchArg` and default `--arch` to "auto"; drop the `macos-` prefix from `RunnerMacosVersionArg` value names (now "14"/"15"/"26"/"27"); make `--vcpus`/`--memory-gb` on `UpdateRunnerArgs` no longer require each other so they can be updated independently. - agent SDK runner: add `resolve_arch(arch, os)` (auto -> x86-64 on Linux, aarch64 on macOS) replacing `arch_to_gql`, used in both create and update; add `merge_instance_shape(new_vcpus, new_memory_gb, existing)` used in `build_update_input` so vCPUs/memory update independently while preserving the other dimension. - Add tests for `resolve_arch` and `merge_instance_shape`. Co-Authored-By: Warp <agent@warp.dev>
oz runner CRUD CLI commandsoz runner CRUD CLI commands
oz runner CRUD CLI commandsoz runner CRUD CLI commands
`oz runner update` can now rename a runner. `--name` is no longer mutually exclusive with a UID: when a UID is given, `--name` sets the runner's new name (rename); when no UID is given, `--name` is still required and identifies the runner to update. The identifier ArgGroup is now `required(true).multiple(true)`, and a new `resolve_updated_name` helper applies the rename only when a UID was used to locate the runner. Co-Authored-By: Warp <agent@warp.dev>
… resolve 3 conflicts Merge origin/master into i18n (downstream sync; keeps i18n SHA stable for stacked PR warpdotdev#13374). 3 content conflicts resolved per SOP merge-pattern (upstream structural refactor wins; restore menu_label wraps at call sites): - app/src/ai/blocklist/inline_action/orchestration_controls.rs (16 hunks): 15 wraps preserved - crates/warp_tui/src/terminal_session_view.rs (1 hunk): 29 wraps preserved - crates/warp_tui/src/zero_state.rs (1 hunk): 12 wraps preserved Auto-merged i18n files retained wraps: terminal/view.rs(99), agent_management/view.rs(50), lib.rs(22), tui_export(1), warp_tui/slash_commands(3), agent_block_sections(4). Known follow-up (tracked): upstream extracted orchestration UI into new module app/src/ai/orchestration/ (snapshots/validation/providers/config_state) with raw English literals — ~16 wrap sites regressed. Restore as next wave. New unwrapped literals for future wave: zero_state MCP section ("MCP", status labels), terminal_session_view "ctrl-c again to exit", orchestration "Loading…"/"No harnesses available". Gate: cargo check -p warp OK (5 upstream warnings in mcp/, not i18n scope); cargo test -p i18n 12/12 passed.
Description
Adds a
oz runnerCRUD command group to the Oz CLI for managing cloud agent runners:oz runner list— list runners (name, description, instance shape, OS, arch, and OS-specific config: Linux Docker image or macOS version). Supports--sort-by name|last-updatedand--jq <FILTER>filtering over the JSON representation.oz runner create— create a runner.--name(required),--description,--setup-command(repeatable),--os linux|macos(defaultlinux),--arch x86-64|aarch64(defaultx86-64),--docker-image(Linux only),--macos-version(macOS only),--vcpus+--memory-gb(instance shape, both-or-neither). Owner scope defaults to Team (--team/--personalto override).oz runner update <uid>— update by UID, or by unambiguous--name.oz runner delete <uid>— with--force.Backed by the server GraphQL
getRunnersquery andupsertRunner/deleteRunnermutations (schema mirrored into the clientschema.graphql, cynic operations added). Client-side validation mirrors the server rule that OS-specific config may only be set with the matching--os.Everything is gated behind a new
CloudAgentRunnerCLICommandsfeature flag (enabled on dogfood), so the subcommand is hidden/rejected until runners launch.Linked Issue
Requested via the factory-client Slack thread (linked below).
ready-to-specorready-to-implement.Testing
CLI-only change (no GUI surface), so no computer-use UI verification is required. Validated with
CARGO_BUILD_JOBS=1:cargo build(warp_graphql_schema,warp_graphql,warp_cli,warp_features,warpapp) — passes; cynic validates the new operations against the mirrored schema at compile time.cargo nextest run -p warp_cli -p warp_graphql— passes (includes new arg-parsing /validate_os_config/ scope-default unit tests inrunner_tests.rs)../script/format— clean.cargo clippy --workspace ... -- -D warnings(presubmit form) — clean.Validation criteria confirmed:
list(+--sort-by, +--jq),create(--namerequired,--description>240 rejected,--vcpus/--memory-gbboth-or-neither),update <uid>/update --name,delete <uid> --force.--macos-versionrejected with--os linux;--docker-imagerejected with--os macos; matching combos accepted.runner createwith no scope flag resolves to Team.runner list --jq '<expr>'filters over the serialized runner JSON via the existing--jqplumbing (JsonOutput+print_raw_json).Screenshots / Videos
N/A — CLI-only change.
Agent Mode