feat(cli): agent automation contract with JSON output, documented exit codes, and wait/timeout flags#825
Merged
Merged
Conversation
…t codes, and wait/timeout flags Make the mitos CLI scriptable by an agent or a shell pipeline: - Documented exit-code contract: named constants (ExitOK, ExitError, ExitUsage, ExitNotFound, ExitTimeout) in one place (exitcodes.go), used consistently across the dispatcher, sandbox verbs, and workspace verbs. A not-found sentinel (ErrNotFound) is wrapped by the cluster backend so a missing sandbox maps to exit 3, and a --timeout deadline maps to exit 124. - Structured output: -o json / --output json / --json on the read verbs (sandbox ls, ws ls, ws log) with stable documented shapes. Human table stays the default; an unknown format is a usage error, never a silent fall-back. - Uniform --wait / --no-wait and --timeout on the lifecycle verbs (sandbox create, sandbox fork, the fork alias; --timeout on terminate). --timeout bounds the wait via a context deadline; --no-wait skips the readiness poll on the cluster create path. Docs: docs/cli.md gains an agent-automation section with the exit-code table, the JSON envelopes, and the wait/timeout semantics. cp, logs, a raw api passthrough, and stdin secrets are named as explicit follow-ups. Refs #775 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Jannes Stubbemann <jannes@openclaw.rocks>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR defines a stable agent-automation CLI contract for ChangesAgent automation contract implementation
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as agentcli.Run
participant Commands as cmdSandbox
participant Backend as ClusterBackend
participant Output as renderList
CLI->>Commands: invoke sandbox or workspace verb
Commands->>Backend: pass lifecycle context and call backend operation
Backend-->>Commands: result or classified error
Commands->>Output: render JSON or human output
Output-->>CLI: output and stable exit code
Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/agentcli/commands.go (1)
166-233: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
splitFirstPositionalstill breaks value-taking flags before the sandbox id
fork --replicas 3 sbx-1andterminate --timeout 5 sbx-1both treat the flag value as the id, so parsing later fails onsbx-1. Make positional extraction aware of flags that consume values, or parse flags before splitting out the id.🤖 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 `@internal/agentcli/commands.go` around lines 166 - 233, `cmdSandboxFork` and `cmdSandboxTerminate` are still using `splitFirstPositional` in a way that grabs values for flags like `--replicas` and `--timeout` as the sandbox id, causing later parse failures. Fix the argument handling so value-taking flags are recognized before extracting the id, either by parsing flags first or by making `splitFirstPositional` skip flag values. Keep the behavior aligned in both command handlers and preserve support for id-before/after-flags forms.
🤖 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 `@internal/agentcli/automation_test.go`:
- Around line 198-214: Add a fork CLI test that exercises the “id after flags”
ordering so the splitFirstPositional parsing bug is covered. In
TestForkTimeoutAndWaitFlags, alongside the existing fork invocation, add a case
using runCLI with the sandbox id placed after value-taking flags like --count
and --timeout, and assert it still records a fork call with the expected
replicas and deadline/no-wait behavior. This should validate the parsing fix in
the command handling path that uses splitFirstPositional and protect against
regressions when the positional id comes after flags.
In `@internal/agentcli/commands.go`:
- Around line 194-196: Honor the no-wait path in ClusterBackend.Fork: the
current fork flow always proceeds into waitChildrenReady even when --no-wait is
requested, so sandbox fork blocks until children are Ready. Update Fork (and the
caller in commands.go that builds octx with lifecycleContext) to check the same
noWaitRequested(ctx) condition used by Create and return immediately after the
fork succeeds when no-wait is set, skipping waitChildrenReady and any readiness
polling.
In `@internal/agentcli/output.go`:
- Around line 20-61: In extractOutputFlag, the format selection is not honoring
“last flag wins” because setFormat only turns jsonOut on for "json" and never
clears it for human/table/text values. Update setFormat so the later occurrence
of a human format resets jsonOut to false, and keep the parsing behavior
consistent for --json, -o/--output, and the --output=/-o= forms.
In `@internal/agentcli/workspace_cmd.go`:
- Around line 53-101: The ws ls and ws log handlers repeat the same output-flag
parsing and JSON/text rendering flow, which should be centralized. Extract a
small shared helper around the workspace command handling in workspace_cmd.go
(for example, the logic used by the ls and log branches) that takes the backend
result plus JSON/text render functions and returns the appropriate exit code.
Update the existing ls and log cases to call that helper so the behavior stays
consistent and easier to extend alongside the similar pattern in commands.go.
- Around line 76-90: The ws log path in workspace_cmd.go is treating a
nonexistent workspace as success because b.Log returns an empty revision list
instead of an error. Update the Log method so it returns ErrNotFound when the
workspace itself is missing, and keep the existing ws log error handling in the
command path so exitCodeFor(err) maps that case to ExitNotFound. Use the Log and
exitCodeFor symbols to locate the behavior and ensure the not-found case no
longer prints a success response.
---
Outside diff comments:
In `@internal/agentcli/commands.go`:
- Around line 166-233: `cmdSandboxFork` and `cmdSandboxTerminate` are still
using `splitFirstPositional` in a way that grabs values for flags like
`--replicas` and `--timeout` as the sandbox id, causing later parse failures.
Fix the argument handling so value-taking flags are recognized before extracting
the id, either by parsing flags first or by making `splitFirstPositional` skip
flag values. Keep the behavior aligned in both command handlers and preserve
support for id-before/after-flags forms.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a71d5f87-7e3e-4a4e-bbcf-77ef3a6b1b0f
📒 Files selected for processing (9)
docs/cli.mdinternal/agentcli/automation_test.gointernal/agentcli/backend.gointernal/agentcli/cli.gointernal/agentcli/clusterbackend.gointernal/agentcli/commands.gointernal/agentcli/exitcodes.gointernal/agentcli/output.gointernal/agentcli/workspace_cmd.go
- parse a sandbox id that follows value-taking flags: splitFirstPositional grabbed a flag value (fork --count 2 sbx-1 took "2"). Replace it with parseLeadingID, a double-parse that honors the id before or after flags, and cover both orderings plus terminate --timeout <id>. - honor --no-wait in ClusterBackend.Fork: it now returns the child ids as soon as they are named in Status.Children instead of always blocking on Ready. - ws log on a missing workspace now returns ErrNotFound (exit 3) instead of an empty success, via a Workspace existence check. - output format is last-flag-wins: a human -o table after --json resets to the human render instead of staying stuck on JSON. - extract a shared renderList helper so sandbox ls, ws ls, and ws log no longer duplicate the JSON/text branch. Refs #775 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Jannes Stubbemann <jannes@openclaw.rocks>
The contract table claimed exit 3 globally, but ws diff/fork/revert/rm/bind still exit 1 for a missing target; scope the claim honestly and name the follow-up. Also state that the backend's own poll bound elapsing is exit 1 so 124 always means the caller's explicit deadline, document the text/human table aliases, and gofmt a pre-existing test-file alignment nit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Jannes Stubbemann <jannes@openclaw.rocks>
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)
internal/agentcli/cli.go (1)
19-25: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDocument all supported lifecycle flags for
sandbox terminate.The PR contract says terminate supports
--wait,--no-wait, and--timeout, but the synopsis only advertises--timeout. Add the wait flags so automation users can discover the complete interface.🤖 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 `@internal/agentcli/cli.go` around lines 19 - 25, Update the CLI synopsis in the command documentation so `sandbox terminate` advertises all supported lifecycle flags, not just `--timeout`. Use the existing CLI help text around the `mitos sandbox terminate` entry to add `--wait` and `--no-wait` alongside `--timeout`, matching the interface already exposed by the lifecycle commands and keeping the wording consistent with the nearby `mitos fork` and `sandbox exec` entries.
🤖 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 `@internal/agentcli/cli.go`:
- Around line 19-25: Update the CLI synopsis in the command documentation so
`sandbox terminate` advertises all supported lifecycle flags, not just
`--timeout`. Use the existing CLI help text around the `mitos sandbox terminate`
entry to add `--wait` and `--no-wait` alongside `--timeout`, matching the
interface already exposed by the lifecycle commands and keeping the wording
consistent with the nearby `mitos fork` and `sandbox exec` entries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a732ab29-e245-4907-a88b-202c013462e8
📒 Files selected for processing (3)
docs/cli.mdinternal/agentcli/cli.gointernal/agentcli/hostedbackend_test.go
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
1 participant
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.
Thinking Path
Linked Issues or Issue Description
Related: #775
This lands the FIRST increment of #775 (JSON output, exit-code contract, wait/timeout). It intentionally defers the
cp,logs, and rawapiverbs and stdin-secret input to a named follow-up (see docs/cli.md Follow-ups), so the issue stays open for that second increment.What Changed
internal/agentcli/exitcodes.go: named exit-code constants (ExitOK,ExitError,ExitUsage,ExitNotFound,ExitTimeout), anErrNotFoundsentinel, anexitCodeForclassifier, and the--wait/--timeoutcontext seam (lifecycleContext,withNoWait).internal/agentcli/output.go:-o json/--output json/--jsonflag parsing plus stable JSON envelopes forsandbox ls({"sandboxes": [...]}),ws ls({"workspaces": [...]}), andws log({"revisions": [...]}). Empty listings render[], nevernull.commands.goandcli.go: the read verbsandbox lsgains-o json;sandbox create,sandbox fork(and theforkalias) gain--wait/--no-wait/--timeout;terminategains--timeout. Every dispatcher return now uses the named exit-code constants, and backend errors flow throughexitCodeForso not-found and timeout map to their codes.workspace_cmd.go:ws lsandws loggain-o jsonand the exit-code contract.clusterbackend.go: a deleted or missing sandbox now wrapsErrNotFound(viaapierrors.IsNotFound), andcreate --no-waitreturns the client-assigned name without polling for Ready.backend.go: theFakeBackendrecords the no-wait signal and deadline presence so the flags are testable offline.docs/cli.md: new "Agent automation contract" section with the exit-code table, the JSON envelopes, and the wait/timeout semantics; Follow-ups names the deferred cp/logs/api/stdin-secret work.Verification
go test ./internal/agentcli/... ./cmd/mitos/...passes (newautomation_test.gocovers JSON shapes, empty-array, unknown-format usage error, not-found and timeout exit codes, and the--wait/--no-wait/--timeoutflag wiring; existing suite green).gofmt -lclean on the changed files.golangci-lint run ./internal/agentcli/... ./cmd/mitos/... --timeout=5mclean, both default andGOOS=linux.go build ./...andgo vet ./internal/agentcli/... ./cmd/mitos/...clean.Risks
Low risk. The change is CLI-surface only: no controller, forkd, fork engine, or guest changes, so no fork-correctness or node-loss surface moves. Defaults are unchanged (
--waitdefaults to true, human table stays the default output), so existing scripts and tests keep their behavior.--no-waitonsandbox forkis accepted uniformly but the cluster backend still waits there because child ids are assigned during readiness; this is documented.Model Used
Claude Opus 4.8 (1M context), model id claude-opus-4-8, extended thinking enabled.
Checklist
#NNN/ mitos-run/mitos URLs)Signed-off-bytrailer (git commit -s)Summary by CodeRabbit
New Features
sandbox ls,ws ls, andws log(revision logs), including consistent empty-list behavior.sandbox create,sandbox fork, andsandbox terminatewith standardized--wait/--no-waitand--timeoutsemantics.Bug Fixes
Documentation
Tests