From 210487c3bdb12a57e07e08a4c9ab5eada644f767 Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Tue, 7 Jul 2026 13:16:51 +0200 Subject: [PATCH 1/3] feat(cli): agent automation contract with JSON output, documented exit 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) Signed-off-by: Jannes Stubbemann --- docs/cli.md | 111 +++++++++++++- internal/agentcli/automation_test.go | 214 +++++++++++++++++++++++++++ internal/agentcli/backend.go | 27 +++- internal/agentcli/cli.go | 44 ++++-- internal/agentcli/clusterbackend.go | 12 ++ internal/agentcli/commands.go | 98 ++++++++---- internal/agentcli/exitcodes.go | 85 +++++++++++ internal/agentcli/output.go | 156 +++++++++++++++++++ internal/agentcli/workspace_cmd.go | 44 +++++- 9 files changed, 728 insertions(+), 63 deletions(-) create mode 100644 internal/agentcli/automation_test.go create mode 100644 internal/agentcli/exitcodes.go create mode 100644 internal/agentcli/output.go diff --git a/docs/cli.md b/docs/cli.md index fa97265c8..a3f621d4b 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -19,10 +19,12 @@ mitos run [--pool P] [--timeout N] create a sandbox, run the command, terminate, exit with the command's exit code mitos sandbox create [--pool P] create a sandbox, print its id -mitos sandbox ls [-n namespace] [-A] list sandboxes + [--wait|--no-wait] [--timeout N] +mitos sandbox ls [-n namespace] [-A] [-o json] list sandboxes (table or JSON) mitos sandbox exec run a command in a sandbox -mitos sandbox fork [--replicas N] fork a sandbox, print new ids -mitos sandbox terminate destroy a sandbox +mitos sandbox fork [--count N] fork a sandbox, print new ids + [--wait|--no-wait] [--timeout N] +mitos sandbox terminate [--timeout N] destroy a sandbox mitos ws create create an empty workspace mitos ws ls [-n namespace] list workspaces mitos ws log list revisions, newest first @@ -38,6 +40,103 @@ mitos doctor [-n namespace] run the node + install preflight and print remediation ``` +## Agent automation contract + +`mitos` is built to be driven by an agent or a shell pipeline, not only a human +at a prompt. Three surfaces make it scriptable: a documented exit-code contract, +machine-readable `-o json` output on the read verbs, and uniform +`--wait`/`--timeout` control on the lifecycle verbs. + +### Exit codes + +Every invocation returns one of a small, stable set of exit codes. An automated +caller can branch on the code without parsing stderr; the human-facing +diagnostic on stderr carries the cause and remediation. + +| Code | Name | Meaning | +|---|---|---| +| `0` | success | The command succeeded. For `run` this is the executed command's own exit code (0 on success). | +| `1` | error | A general, remediable runtime error (backend unreachable, a failed operation). The stderr diagnostic names the cause. | +| `2` | usage | A usage error: an unknown subcommand, a missing argument, a bad flag, or an unknown `-o` output format. | +| `3` | not found | The targeted sandbox or workspace does not exist. | +| `124` | timeout | A `--wait`/`--timeout` deadline elapsed before the operation completed. The value matches the coreutils `timeout` tool so it is familiar in pipelines. | + +`run` is the one verb that passes through: its exit code is the executed +command's exit code, so `mitos run false` exits non-zero exactly as `false` +would. The other verbs use the table above. + +### Structured output: `-o json` + +The read and inspect verbs accept `-o json` (equivalently `--output json` or the +`--json` shorthand) and emit a stable JSON envelope on stdout. The default +remains the human-aligned table. An unrecognized format is a usage error (exit +`2`), never a silent fall-back to the human render, so an agent that asked for +JSON always gets JSON or a clear failure. + +`mitos sandbox ls -o json`: + +```json +{ + "sandboxes": [ + { + "name": "sbx-abc123", + "pool": "python", + "phase": "Ready", + "node": "node-a", + "endpoint": "10.0.0.1:9091", + "ageSeconds": 90 + } + ] +} +``` + +`mitos ws ls -o json`: + +```json +{ + "workspaces": [ + { "name": "w1", "head": "rev-2", "revisions": 2, "resumable": true } + ] +} +``` + +`mitos ws log -o json`: + +```json +{ + "revisions": [ + { "name": "rev-2", "phase": "Committed", "resumable": true, "lineage": "root" } + ] +} +``` + +An empty listing renders an empty array (`{"sandboxes": []}`), never `null`, so a +consumer can iterate unconditionally. `ageSeconds` is a whole-second integer so a +caller never has to parse the human `90s`/`2m`/`3h` rendering. + +### Waiting and timeouts: `--wait` / `--timeout` + +The lifecycle verbs that start an asynchronous operation accept a uniform +`--wait`/`--no-wait` and `--timeout` pair: + +- `mitos sandbox create` and `mitos sandbox fork` (and the `mitos fork` alias) + wait for the new sandboxes to become `Ready` by default. Pass `--no-wait` (or + `--wait=false`) to return as soon as the object is created without polling for + readiness. On the cluster backend the created sandbox name is assigned + client-side, so `--no-wait` on `create` returns the id immediately; the hosted + gateway returns as soon as the sandbox is provisioned, so `--no-wait` is a + no-op there. +- `--timeout N` bounds the wait to `N` seconds. When the deadline elapses the + command exits `124` (timeout) rather than blocking indefinitely. `--timeout 0` + (the default) uses the backend's own bound. +- `mitos sandbox terminate` accepts `--timeout N` to bound the delete call. + Terminate is asynchronous (the controller reaps the object); waiting until the + sandbox is fully reaped is a named follow-up, so `--wait` is not offered on + terminate yet. + +`mitos run`'s existing `--timeout` is the in-sandbox exec deadline (the executed +command's timeout), distinct from the lifecycle wait above. + ### First-run setup: `mitos init` `mitos init` takes you from key-in-hand to a verified working setup in one @@ -390,4 +489,8 @@ at the object level. `cp` and `port-forward` for operators are not yet available - a `curl | sh` installer and `get.mitos.run` distribution; - `mitos init` browser/device-flow login (no pasted key) and optional MCP server / editor configuration; -- shell completions and a code-interpreter-compatible API shim. +- shell completions and a code-interpreter-compatible API shim; +- the agent-automation verbs deferred from the first `-o json` increment: + `mitos cp` (host <-> sandbox file copy), `mitos logs` (stream a sandbox or + claim console), a raw `mitos api` passthrough, and reading secrets from stdin + so a key never lands in shell history or `argv`. diff --git a/internal/agentcli/automation_test.go b/internal/agentcli/automation_test.go new file mode 100644 index 000000000..3f143f56c --- /dev/null +++ b/internal/agentcli/automation_test.go @@ -0,0 +1,214 @@ +package agentcli + +import ( + "context" + "encoding/json" + "strings" + "testing" + "time" +) + +// TestSandboxLsJSONShape asserts `sandbox ls -o json` emits the documented +// stable shape: a top-level object with a "sandboxes" array of typed rows. +func TestSandboxLsJSONShape(t *testing.T) { + fb := NewFakeBackend() + fb.ListInfos = []SandboxInfo{ + {Name: "sbx-1", Pool: "python", Phase: "Ready", Node: "node-a", Endpoint: "10.0.0.1:9091", Age: 90 * time.Second}, + } + for _, form := range [][]string{ + {"sandbox", "ls", "-o", "json"}, + {"sandbox", "ls", "--json"}, + {"sandbox", "ls", "-o=json"}, + {"sandbox", "ls", "--output", "json"}, + } { + code, out, _ := runCLI(t, fb, form...) + if code != ExitOK { + t.Fatalf("%v: exit code = %d, want %d", form, code, ExitOK) + } + var got struct { + Sandboxes []struct { + Name string `json:"name"` + Pool string `json:"pool"` + Phase string `json:"phase"` + Node string `json:"node"` + Endpoint string `json:"endpoint"` + AgeSeconds int `json:"ageSeconds"` + } `json:"sandboxes"` + } + if err := json.Unmarshal(out.Bytes(), &got); err != nil { + t.Fatalf("%v: output is not valid JSON: %v\noutput=%q", form, err, out.String()) + } + if len(got.Sandboxes) != 1 { + t.Fatalf("%v: got %d sandboxes, want 1", form, len(got.Sandboxes)) + } + s := got.Sandboxes[0] + if s.Name != "sbx-1" || s.Pool != "python" || s.Phase != "Ready" || s.Node != "node-a" || s.Endpoint != "10.0.0.1:9091" || s.AgeSeconds != 90 { + t.Fatalf("%v: row = %+v, want the injected values with ageSeconds=90", form, s) + } + } +} + +// TestSandboxLsJSONEmptyIsArray asserts an empty listing emits an empty JSON +// array, never null, so a consumer can iterate unconditionally. +func TestSandboxLsJSONEmptyIsArray(t *testing.T) { + fb := NewFakeBackend() + fb.ListInfos = nil + code, out, _ := runCLI(t, fb, "sandbox", "ls", "--json") + if code != ExitOK { + t.Fatalf("exit code = %d, want %d", code, ExitOK) + } + if !strings.Contains(out.String(), `"sandboxes": []`) { + t.Fatalf("empty ls json = %q, want an empty sandboxes array", out.String()) + } +} + +// TestOutputFormatUnknownIsUsageError asserts an unrecognized output format is a +// usage error (exit 2), not a silent human render. +func TestOutputFormatUnknownIsUsageError(t *testing.T) { + fb := NewFakeBackend() + code, _, errw := runCLI(t, fb, "sandbox", "ls", "-o", "yaml") + if code != ExitUsage { + t.Fatalf("exit code = %d, want %d for unknown format", code, ExitUsage) + } + if errw.Len() == 0 { + t.Fatalf("want a diagnostic on stderr for an unknown format") + } +} + +// TestWorkspaceLsAndLogJSON asserts the workspace list and log read verbs also +// honor -o json with documented shapes. +func TestWorkspaceLsAndLogJSON(t *testing.T) { + fb := NewFakeBackend() + ws := fb.Workspace().(*FakeWorkspaceBackend) + ws.Workspaces = []WorkspaceInfo{{Name: "w1", Head: "rev-2", Revisions: 2, Resumable: true}} + ws.Revisions = []RevisionInfo{{Name: "rev-2", Phase: "Committed", Resumable: true, Lineage: "root"}} + + code, out, _ := runCLI(t, fb, "ws", "ls", "--json") + if code != ExitOK { + t.Fatalf("ws ls json: exit code = %d, want %d", code, ExitOK) + } + var wl struct { + Workspaces []struct { + Name string `json:"name"` + Head string `json:"head"` + Revisions int `json:"revisions"` + Resumable bool `json:"resumable"` + } `json:"workspaces"` + } + if err := json.Unmarshal(out.Bytes(), &wl); err != nil { + t.Fatalf("ws ls json invalid: %v\n%q", err, out.String()) + } + if len(wl.Workspaces) != 1 || wl.Workspaces[0].Name != "w1" || wl.Workspaces[0].Revisions != 2 { + t.Fatalf("ws ls json = %+v, want one w1 with 2 revisions", wl.Workspaces) + } + + code, out, _ = runCLI(t, fb, "ws", "log", "w1", "--json") + if code != ExitOK { + t.Fatalf("ws log json: exit code = %d, want %d", code, ExitOK) + } + var rl struct { + Revisions []struct { + Name string `json:"name"` + Phase string `json:"phase"` + Lineage string `json:"lineage"` + } `json:"revisions"` + } + if err := json.Unmarshal(out.Bytes(), &rl); err != nil { + t.Fatalf("ws log json invalid: %v\n%q", err, out.String()) + } + if len(rl.Revisions) != 1 || rl.Revisions[0].Name != "rev-2" || rl.Revisions[0].Lineage != "root" { + t.Fatalf("ws log json = %+v, want one rev-2 root revision", rl.Revisions) + } +} + +// TestExitCodeNotFound asserts a not-found backend error maps to the documented +// not-found exit code, distinct from a generic runtime error. +func TestExitCodeNotFound(t *testing.T) { + fb := NewFakeBackend() + fb.Errors["terminate"] = ErrNotFound + code, _, errw := runCLI(t, fb, "sandbox", "terminate", "sbx-missing") + if code != ExitNotFound { + t.Fatalf("exit code = %d, want %d (not found)", code, ExitNotFound) + } + if errw.Len() == 0 { + t.Fatalf("want a diagnostic on stderr") + } +} + +// TestExitCodeTimeout asserts a deadline-exceeded error maps to the documented +// timeout exit code so an agent can distinguish a slow op from a hard failure. +func TestExitCodeTimeout(t *testing.T) { + fb := NewFakeBackend() + fb.Errors["create"] = context.DeadlineExceeded + code, _, _ := runCLI(t, fb, "sandbox", "create", "--pool", "p") + if code != ExitTimeout { + t.Fatalf("exit code = %d, want %d (timeout)", code, ExitTimeout) + } +} + +// TestCreateTimeoutFlagSetsDeadline asserts --timeout wires a context deadline +// into the backend call. +func TestCreateTimeoutFlagSetsDeadline(t *testing.T) { + fb := NewFakeBackend() + code, _, _ := runCLI(t, fb, "sandbox", "create", "--pool", "p", "--timeout", "5") + if code != ExitOK { + t.Fatalf("exit code = %d, want %d", code, ExitOK) + } + calls := fb.RecordedCalls() + if len(calls) != 1 || calls[0].Method != "create" { + t.Fatalf("calls = %v, want a single create", calls) + } + if !calls[0].HasDeadline { + t.Fatalf("create call had no context deadline; --timeout was not wired") + } +} + +// TestCreateNoWaitFlag asserts --no-wait (and --wait=false) thread the no-wait +// signal to the backend. +func TestCreateNoWaitFlag(t *testing.T) { + for _, args := range [][]string{ + {"sandbox", "create", "--pool", "p", "--no-wait"}, + {"sandbox", "create", "--pool", "p", "--wait=false"}, + } { + fb := NewFakeBackend() + code, _, _ := runCLI(t, fb, args...) + if code != ExitOK { + t.Fatalf("%v: exit code = %d, want %d", args, code, ExitOK) + } + calls := fb.RecordedCalls() + if len(calls) != 1 || !calls[0].NoWait { + t.Fatalf("%v: create call NoWait = %v, want true", args, calls[0].NoWait) + } + } +} + +// TestCreateWaitsByDefault asserts the default is to wait: no no-wait signal is +// threaded when neither flag is given. +func TestCreateWaitsByDefault(t *testing.T) { + fb := NewFakeBackend() + code, _, _ := runCLI(t, fb, "sandbox", "create", "--pool", "p") + if code != ExitOK { + t.Fatalf("exit code = %d, want %d", code, ExitOK) + } + if fb.RecordedCalls()[0].NoWait { + t.Fatalf("default create threaded a no-wait signal; want wait by default") + } +} + +// TestForkTimeoutAndWaitFlags asserts fork accepts the same --wait/--timeout +// contract as create. +func TestForkTimeoutAndWaitFlags(t *testing.T) { + fb := NewFakeBackend() + fb.ForkIDs = []string{"f1", "f2"} + code, _, _ := runCLI(t, fb, "fork", "sbx-1", "--count", "2", "--timeout", "10", "--no-wait") + if code != ExitOK { + t.Fatalf("exit code = %d, want %d", code, ExitOK) + } + calls := fb.RecordedCalls() + if calls[0].Method != "fork" || calls[0].Replicas != 2 { + t.Fatalf("calls = %v, want fork x2", calls) + } + if !calls[0].HasDeadline || !calls[0].NoWait { + t.Fatalf("fork call HasDeadline=%v NoWait=%v, want both true", calls[0].HasDeadline, calls[0].NoWait) + } +} diff --git a/internal/agentcli/backend.go b/internal/agentcli/backend.go index 5634659aa..94a2530ea 100644 --- a/internal/agentcli/backend.go +++ b/internal/agentcli/backend.go @@ -80,6 +80,18 @@ type FakeCall struct { Content string Replicas int Namespace string + // NoWait records whether the call carried the no-wait lifecycle signal + // (set by --no-wait / --wait=false). HasDeadline records whether the call + // context carried a deadline (set by --timeout). + NoWait bool + HasDeadline bool +} + +// ctxLifecycle extracts the no-wait signal and deadline presence from ctx so +// the fake can record what the CLI threaded through the lifecycle flags. +func ctxLifecycle(ctx context.Context) (noWait, hasDeadline bool) { + _, hasDeadline = ctx.Deadline() + return noWaitRequested(ctx), hasDeadline } // FakeBackend is a test double that records every call and returns canned @@ -166,8 +178,9 @@ func (f *FakeBackend) RecordedCalls() []FakeCall { } // Create implements Backend. -func (f *FakeBackend) Create(_ context.Context, pool string) (string, error) { - f.record(FakeCall{Method: "create", Pool: pool}) +func (f *FakeBackend) Create(ctx context.Context, pool string) (string, error) { + noWait, hasDeadline := ctxLifecycle(ctx) + f.record(FakeCall{Method: "create", Pool: pool, NoWait: noWait, HasDeadline: hasDeadline}) if e := f.err("create"); e != nil { return "", e } @@ -199,8 +212,9 @@ func (f *FakeBackend) WriteFile(_ context.Context, sandboxID, path, content stri } // Fork implements Backend. -func (f *FakeBackend) Fork(_ context.Context, sandboxID string, n int) ([]string, error) { - f.record(FakeCall{Method: "fork", SandboxID: sandboxID, Replicas: n}) +func (f *FakeBackend) Fork(ctx context.Context, sandboxID string, n int) ([]string, error) { + noWait, hasDeadline := ctxLifecycle(ctx) + f.record(FakeCall{Method: "fork", SandboxID: sandboxID, Replicas: n, NoWait: noWait, HasDeadline: hasDeadline}) if e := f.err("fork"); e != nil { return nil, e } @@ -208,8 +222,9 @@ func (f *FakeBackend) Fork(_ context.Context, sandboxID string, n int) ([]string } // Terminate implements Backend. -func (f *FakeBackend) Terminate(_ context.Context, sandboxID string) error { - f.record(FakeCall{Method: "terminate", SandboxID: sandboxID}) +func (f *FakeBackend) Terminate(ctx context.Context, sandboxID string) error { + _, hasDeadline := ctxLifecycle(ctx) + f.record(FakeCall{Method: "terminate", SandboxID: sandboxID, HasDeadline: hasDeadline}) return f.err("terminate") } diff --git a/internal/agentcli/cli.go b/internal/agentcli/cli.go index d2807286e..6c22a40d0 100644 --- a/internal/agentcli/cli.go +++ b/internal/agentcli/cli.go @@ -16,12 +16,13 @@ Usage: command, terminate, and exit with the command's exit code mitos sandbox create [--pool P] create a sandbox, print its id - mitos sandbox ls [-n namespace] [-A] list sandboxes + [--wait|--no-wait] [--timeout N] + mitos sandbox ls [-n namespace] [-A] [-o json] list sandboxes (table or JSON) mitos sandbox exec run a command in a sandbox mitos fork [--count N] fork a running sandbox into N - live children, print new ids + [--wait|--no-wait] [--timeout N] live children, print new ids mitos sandbox fork [--count N] alias of the above - mitos sandbox terminate destroy a sandbox + mitos sandbox terminate [--timeout N] destroy a sandbox mitos ws create|ls|log|diff|fork|revert|rm workspace lifecycle (git verbs) mitos ws bind bind a sandbox to a workspace mitos template build --name N build a template from a @@ -36,33 +37,46 @@ Usage: Flags: --pool string pool to create sandboxes from - --timeout int exec timeout in seconds (0 = backend default) + --timeout int seconds to wait for a lifecycle op (run: exec timeout) + --wait/--no-wait wait for Ready on create/fork (default wait) + -o, --output fmt output format for read verbs: table (default) or json + --json shorthand for -o json -n string namespace (ls) -A all namespaces (ls) --count int number of children to fork (fork; alias --replicas) -h, --help print this help + +Exit codes: + 0 success (run: the executed command's exit code) + 1 a general, remediable runtime error + 2 usage error (unknown subcommand, missing argument, bad flag or format) + 3 the targeted sandbox or workspace was not found + 124 a --wait/--timeout deadline elapsed ` // Run is the testable CLI entry point. It dispatches args (without the program // name) against backend, writing normal output to out and diagnostics to errw, -// and returns a process exit code: +// and returns a process exit code from the documented contract (see exitcodes.go +// and docs/cli.md): // -// 0 success (for run: the command's exit code) -// 2 usage error (unknown subcommand, missing argument, bad flag) -// 1 a backend or runtime error +// ExitOK (0) success (for run: the executed command's exit code) +// ExitError (1) a general, remediable runtime error +// ExitUsage (2) usage error (unknown subcommand, missing arg, bad flag) +// ExitNotFound (3) the targeted sandbox or workspace was not found +// ExitTimeout (124) a --wait/--timeout deadline elapsed // // For run, the exit code is the executed command's exit code so callers can // chain mitos in shell pipelines. func Run(ctx context.Context, args []string, backend Backend, out, errw io.Writer) int { if len(args) == 0 { fmt.Fprint(errw, usage) - return 2 + return ExitUsage } switch args[0] { case "-h", "--help", "help": fmt.Fprint(out, usage) - return 0 + return ExitOK case "run": return cmdRun(ctx, args[1:], backend, out, errw) case "sandbox": @@ -74,13 +88,13 @@ func Run(ctx context.Context, args []string, backend Backend, out, errw io.Write case "ws": if backend == nil || backend.Workspace() == nil { fmt.Fprint(errw, "ws: this backend does not support workspaces\n") - return 2 + return ExitUsage } return cmdWorkspace(ctx, args[1:], backend.Workspace(), out, errw) case "template": if backend == nil { fmt.Fprint(errw, "template: this backend does not support templates\n") - return 2 + return ExitUsage } return cmdTemplate(ctx, args[1:], backend.Template(), out, errw) case "auth": @@ -97,7 +111,7 @@ func Run(ctx context.Context, args []string, backend Backend, out, errw io.Write // before agentcli.Run and calls CmdInit with the real seams. Reaching // here means init was invoked through a path that did not wire them. fmt.Fprint(errw, "init: run via the mitos binary, which wires the key validator and terminal\n") - return 1 + return ExitError case "doctor": // doctor builds a real node + k8s probe (reads /dev, /proc, and the // cluster), which the pure CLI dispatcher does not do; cmd/mitos @@ -105,9 +119,9 @@ func Run(ctx context.Context, args []string, backend Backend, out, errw io.Write // Reaching here means doctor was invoked through a path that did not wire // the probe, so it reports that and returns nonzero. fmt.Fprint(errw, "doctor: run via the mitos binary, which wires the node + cluster probe\n") - return 1 + return ExitError default: fmt.Fprintf(errw, "unknown subcommand %q\n\n%s", args[0], usage) - return 2 + return ExitUsage } } diff --git a/internal/agentcli/clusterbackend.go b/internal/agentcli/clusterbackend.go index 1e64df313..03de73d57 100644 --- a/internal/agentcli/clusterbackend.go +++ b/internal/agentcli/clusterbackend.go @@ -107,6 +107,12 @@ func (b *ClusterBackend) Create(ctx context.Context, pool string) (string, error if b.readyHook != nil { b.readyHook(ctx, name) } + // --no-wait returns the created sandbox name immediately without polling for + // Ready. The name is assigned client-side so it is known even before the + // controller reconciles the object. + if noWaitRequested(ctx) { + return name, nil + } if err := b.waitSandboxReady(ctx, name); err != nil { return "", err } @@ -148,6 +154,9 @@ func (b *ClusterBackend) waitSandboxReady(ctx context.Context, name string) erro func (b *ClusterBackend) sandboxHTTP(ctx context.Context, name string) (*mcp.HTTPBackend, error) { var sandbox v1.Sandbox if err := b.client.Get(ctx, client.ObjectKey{Namespace: b.namespace, Name: name}, &sandbox); err != nil { + if apierrors.IsNotFound(err) { + return nil, fmt.Errorf("sandbox %s not found: %w", name, ErrNotFound) + } return nil, fmt.Errorf("get sandbox %s: %w", name, err) } endpoint := sandbox.Status.Endpoint @@ -262,6 +271,9 @@ func (b *ClusterBackend) Terminate(ctx context.Context, sandboxID string) error ObjectMeta: metav1.ObjectMeta{Name: sandboxID, Namespace: b.namespace}, } if err := b.client.Delete(ctx, sandbox); err != nil { + if apierrors.IsNotFound(err) { + return fmt.Errorf("sandbox %s not found: %w", sandboxID, ErrNotFound) + } return fmt.Errorf("delete sandbox %s: %w", sandboxID, err) } return nil diff --git a/internal/agentcli/commands.go b/internal/agentcli/commands.go index e21aa5643..7dc824694 100644 --- a/internal/agentcli/commands.go +++ b/internal/agentcli/commands.go @@ -27,18 +27,18 @@ func cmdRun(ctx context.Context, args []string, backend Backend, out, errw io.Wr timeout := fs.Int("timeout", 0, "exec timeout in seconds (0 = backend default)") if err := fs.Parse(args); err != nil { fmt.Fprint(errw, usage) - return 2 + return ExitUsage } command := strings.Join(fs.Args(), " ") if command == "" { fmt.Fprintf(errw, "run: a command is required\n\n%s", usage) - return 2 + return ExitUsage } id, err := backend.Create(ctx, *pool) if err != nil { fmt.Fprintf(errw, "create sandbox: %v\n", err) - return 1 + return exitCodeFor(err) } result, execErr := backend.Exec(ctx, id, command, *timeout) @@ -52,7 +52,7 @@ func cmdRun(ctx context.Context, args []string, backend Backend, out, errw io.Wr if execErr != nil { fmt.Fprintf(errw, "exec: %v\n", execErr) - return 1 + return exitCodeFor(execErr) } if result.Stdout != "" { fmt.Fprint(out, result.Stdout) @@ -67,7 +67,7 @@ func cmdRun(ctx context.Context, args []string, backend Backend, out, errw io.Wr func cmdSandbox(ctx context.Context, args []string, backend Backend, out, errw io.Writer) int { if len(args) == 0 { fmt.Fprintf(errw, "sandbox: a subcommand is required\n\n%s", usage) - return 2 + return ExitUsage } switch args[0] { case "create": @@ -82,33 +82,43 @@ func cmdSandbox(ctx context.Context, args []string, backend Backend, out, errw i return cmdSandboxTerminate(ctx, args[1:], backend, out, errw) default: fmt.Fprintf(errw, "unknown sandbox subcommand %q\n\n%s", args[0], usage) - return 2 + return ExitUsage } } func cmdSandboxCreate(ctx context.Context, args []string, backend Backend, out, errw io.Writer) int { fs := newFlagSet("sandbox create", errw) pool := fs.String("pool", "", "pool to create the sandbox from") + wait := fs.Bool("wait", true, "wait for the sandbox to become Ready before returning") + noWait := fs.Bool("no-wait", false, "return as soon as the sandbox is created; do not wait for Ready") + timeout := fs.Int("timeout", 0, "max seconds to wait for Ready (0 = backend default)") if err := fs.Parse(args); err != nil { fmt.Fprint(errw, usage) - return 2 + return ExitUsage } - id, err := backend.Create(ctx, *pool) + octx, cancel := lifecycleContext(ctx, *wait, *noWait, *timeout) + defer cancel() + id, err := backend.Create(octx, *pool) if err != nil { fmt.Fprintf(errw, "create sandbox: %v\n", err) - return 1 + return exitCodeFor(err) } fmt.Fprintln(out, id) - return 0 + return ExitOK } func cmdSandboxLs(ctx context.Context, args []string, backend Backend, out, errw io.Writer) int { + jsonOut, rest, ferr := extractOutputFlag(args) + if ferr != nil { + fmt.Fprintf(errw, "sandbox ls: %v\n", ferr) + return ExitUsage + } fs := newFlagSet("sandbox ls", errw) namespace := fs.String("n", "", "namespace") allNamespaces := fs.Bool("A", false, "all namespaces") - if err := fs.Parse(args); err != nil { + if err := fs.Parse(rest); err != nil { fmt.Fprint(errw, usage) - return 2 + return ExitUsage } ns := *namespace if *allNamespaces { @@ -117,23 +127,32 @@ func cmdSandboxLs(ctx context.Context, args []string, backend Backend, out, errw infos, err := backend.List(ctx, ns) if err != nil { fmt.Fprintf(errw, "list sandboxes: %v\n", err) - return 1 + return exitCodeFor(err) + } + if jsonOut { + s, err := jsonSandboxInfos(infos) + if err != nil { + fmt.Fprintf(errw, "sandbox ls: %v\n", err) + return ExitError + } + fmt.Fprint(out, s) + return ExitOK } fmt.Fprint(out, formatSandboxInfos(infos)) - return 0 + return ExitOK } func cmdSandboxExec(ctx context.Context, args []string, backend Backend, out, errw io.Writer) int { if len(args) < 2 { fmt.Fprintf(errw, "sandbox exec: and a command are required\n\n%s", usage) - return 2 + return ExitUsage } id := args[0] command := strings.Join(args[1:], " ") result, err := backend.Exec(ctx, id, command, 0) if err != nil { fmt.Fprintf(errw, "exec: %v\n", err) - return 1 + return exitCodeFor(err) } if result.Stdout != "" { fmt.Fprint(out, result.Stdout) @@ -154,44 +173,63 @@ func cmdSandboxFork(ctx context.Context, args []string, backend Backend, out, er // alias. --count wins when set (default 0 means "not set, use --replicas"). count := fs.Int("count", 0, "number of forks (alias of --replicas)") replicas := fs.Int("replicas", 1, "number of forks") + wait := fs.Bool("wait", true, "wait for the forks to become Ready before returning") + noWait := fs.Bool("no-wait", false, "return as soon as the forks are created; do not wait for Ready") + timeout := fs.Int("timeout", 0, "max seconds to wait for Ready (0 = backend default)") if err := fs.Parse(rest); err != nil { fmt.Fprint(errw, usage) - return 2 + return ExitUsage } if id == "" && fs.NArg() > 0 { id = fs.Arg(0) } if id == "" { fmt.Fprintf(errw, "sandbox fork: a sandbox id is required\n\n%s", usage) - return 2 + return ExitUsage } n := *replicas if *count > 0 { n = *count } - ids, err := backend.Fork(ctx, id, n) + octx, cancel := lifecycleContext(ctx, *wait, *noWait, *timeout) + defer cancel() + ids, err := backend.Fork(octx, id, n) if err != nil { fmt.Fprintf(errw, "fork: %v\n", err) - return 1 + return exitCodeFor(err) } for _, fid := range ids { fmt.Fprintln(out, fid) } - return 0 + return ExitOK } func cmdSandboxTerminate(ctx context.Context, args []string, backend Backend, out, errw io.Writer) int { - if len(args) < 1 { + id, rest := splitFirstPositional(args) + fs := newFlagSet("sandbox terminate", errw) + // terminate is asynchronous: the object is deleted and the controller reaps + // it. --timeout bounds the delete call itself; waiting until the sandbox is + // fully reaped is a named follow-up, so --wait is not offered here yet. + timeout := fs.Int("timeout", 0, "max seconds to bound the delete call (0 = no bound)") + if err := fs.Parse(rest); err != nil { + fmt.Fprint(errw, usage) + return ExitUsage + } + if id == "" && fs.NArg() > 0 { + id = fs.Arg(0) + } + if id == "" { fmt.Fprintf(errw, "sandbox terminate: a sandbox id is required\n\n%s", usage) - return 2 + return ExitUsage } - id := args[0] - if err := backend.Terminate(ctx, id); err != nil { + octx, cancel := lifecycleContext(ctx, true, false, *timeout) + defer cancel() + if err := backend.Terminate(octx, id); err != nil { fmt.Fprintf(errw, "terminate: %v\n", err) - return 1 + return exitCodeFor(err) } fmt.Fprintf(out, "terminated %s\n", id) - return 0 + return ExitOK } // cmdDev validates the `dev up|down` arguments. The dev orchestration shells out @@ -202,15 +240,15 @@ func cmdSandboxTerminate(ctx context.Context, args []string, backend Backend, ou func cmdDev(_ context.Context, args []string, _, errw io.Writer) int { if len(args) == 0 { fmt.Fprintf(errw, "dev: 'up' or 'down' is required\n\n%s", usage) - return 2 + return ExitUsage } switch args[0] { case "up", "down": fmt.Fprintf(errw, "dev %s: run via the mitos binary, which wires the kind/kubectl runner\n", args[0]) - return 1 + return ExitError default: fmt.Fprintf(errw, "unknown dev subcommand %q\n\n%s", args[0], usage) - return 2 + return ExitUsage } } diff --git a/internal/agentcli/exitcodes.go b/internal/agentcli/exitcodes.go new file mode 100644 index 000000000..491240f0c --- /dev/null +++ b/internal/agentcli/exitcodes.go @@ -0,0 +1,85 @@ +package agentcli + +import ( + "context" + "errors" + "time" +) + +// The mitos CLI exit-code contract. These constants are the single source of +// truth for the process exit codes and are documented in docs/cli.md. They are +// stable: an agent or shell pipeline can branch on them without parsing stderr. +// +// ExitOK the command succeeded. For `run` this is the executed +// command's own exit code (which is 0 on success). +// ExitError a general, remediable runtime error (backend unreachable, a +// failed operation). The stderr diagnostic carries the cause. +// ExitUsage a usage error: an unknown subcommand, a missing argument, a +// bad flag, or an unknown output format. +// ExitNotFound the targeted sandbox or workspace does not exist. +// ExitTimeout a --wait/--timeout deadline elapsed before the operation +// completed. The value matches the coreutils `timeout` tool so it +// is familiar in shell pipelines. +const ( + ExitOK = 0 + ExitError = 1 + ExitUsage = 2 + ExitNotFound = 3 + ExitTimeout = 124 +) + +// ErrNotFound is the sentinel a Backend wraps (with %w) when the targeted +// sandbox or workspace does not exist, so the CLI can map it to ExitNotFound +// without string matching. It never carries a secret value. +var ErrNotFound = errors.New("not found") + +// exitCodeFor classifies a backend or runtime error into the documented exit +// code. A nil error is ExitOK. A deadline (from a --timeout bound or a canceled +// context deadline) is ExitTimeout. A wrapped ErrNotFound is ExitNotFound. +// Everything else is the general ExitError. +func exitCodeFor(err error) int { + switch { + case err == nil: + return ExitOK + case errors.Is(err, context.DeadlineExceeded): + return ExitTimeout + case errors.Is(err, ErrNotFound): + return ExitNotFound + default: + return ExitError + } +} + +// lifecycleCtxKey is the private context key type for the CLI lifecycle seam. +type lifecycleCtxKey int + +const noWaitKey lifecycleCtxKey = iota + +// withNoWait marks ctx so a Backend that would otherwise poll for readiness +// returns as soon as the object is created. It is set by the --no-wait / +// --wait=false lifecycle flags. +func withNoWait(ctx context.Context) context.Context { + return context.WithValue(ctx, noWaitKey, true) +} + +// noWaitRequested reports whether the caller asked not to wait for readiness. +func noWaitRequested(ctx context.Context) bool { + v, _ := ctx.Value(noWaitKey).(bool) + return v +} + +// lifecycleContext derives the operation context for a lifecycle verb from its +// --wait/--no-wait/--timeout flags. When wait is false or noWait is true the +// returned context carries the no-wait signal. When timeoutSec is positive the +// context carries a deadline; on expiry the backend call returns a +// deadline-exceeded error that classifies to ExitTimeout. The returned cancel +// func must always be called. +func lifecycleContext(ctx context.Context, wait, noWait bool, timeoutSec int) (context.Context, context.CancelFunc) { + if noWait || !wait { + ctx = withNoWait(ctx) + } + if timeoutSec > 0 { + return context.WithTimeout(ctx, time.Duration(timeoutSec)*time.Second) + } + return ctx, func() {} +} diff --git a/internal/agentcli/output.go b/internal/agentcli/output.go new file mode 100644 index 000000000..88495c8d6 --- /dev/null +++ b/internal/agentcli/output.go @@ -0,0 +1,156 @@ +package agentcli + +import ( + "encoding/json" + "fmt" + "strings" +) + +// extractOutputFlag pulls the output-format flags out of args and reports +// whether structured JSON was requested. It recognizes, in any position: +// +// --json +// -o json / -o=json +// --output json / --output=json +// +// The value must be "json" or a human format ("table", "text", "human"); an +// unrecognized value is a usage error so an agent never silently gets the human +// render when it asked for a machine one. The returned rest has the consumed +// tokens removed so the caller can pass it to its own flag set. +func extractOutputFlag(args []string) (jsonOut bool, rest []string, err error) { + rest = make([]string, 0, len(args)) + setFormat := func(v string) error { + switch v { + case "json": + jsonOut = true + case "table", "text", "human", "": + // The human render is the default; nothing to set. + default: + return fmt.Errorf("unknown output format %q (want json or table)", v) + } + return nil + } + for i := 0; i < len(args); i++ { + a := args[i] + switch { + case a == "--json": + if e := setFormat("json"); e != nil { + return false, nil, e + } + case a == "-o" || a == "--output": + if i+1 >= len(args) { + return false, nil, fmt.Errorf("%s requires a value (json or table)", a) + } + i++ + if e := setFormat(args[i]); e != nil { + return false, nil, e + } + case strings.HasPrefix(a, "-o="): + if e := setFormat(strings.TrimPrefix(a, "-o=")); e != nil { + return false, nil, e + } + case strings.HasPrefix(a, "--output="): + if e := setFormat(strings.TrimPrefix(a, "--output=")); e != nil { + return false, nil, e + } + default: + rest = append(rest, a) + } + } + return jsonOut, rest, nil +} + +// encodeJSON renders v as indented JSON with a trailing newline. It is the one +// place the CLI's structured output is formatted so every -o json shape is +// consistent. +func encodeJSON(v any) (string, error) { + b, err := json.MarshalIndent(v, "", " ") + if err != nil { + return "", fmt.Errorf("encode json: %w", err) + } + return string(b) + "\n", nil +} + +// sandboxJSONRow is the stable per-sandbox JSON shape emitted by `sandbox ls +// -o json`. Age is reported in whole seconds so a consumer does not have to +// parse the human "90s"/"2m" rendering. +type sandboxJSONRow struct { + Name string `json:"name"` + Pool string `json:"pool"` + Phase string `json:"phase"` + Node string `json:"node"` + Endpoint string `json:"endpoint"` + AgeSeconds int `json:"ageSeconds"` +} + +// jsonSandboxInfos renders a sandbox listing as the documented JSON envelope +// {"sandboxes": [...]}. An empty listing renders an empty array, never null. +func jsonSandboxInfos(infos []SandboxInfo) (string, error) { + rows := make([]sandboxJSONRow, 0, len(infos)) + for i := range infos { + in := &infos[i] + age := in.Age + if age < 0 { + age = 0 + } + rows = append(rows, sandboxJSONRow{ + Name: in.Name, + Pool: in.Pool, + Phase: in.Phase, + Node: in.Node, + Endpoint: in.Endpoint, + AgeSeconds: int(age.Seconds()), + }) + } + return encodeJSON(struct { + Sandboxes []sandboxJSONRow `json:"sandboxes"` + }{Sandboxes: rows}) +} + +// workspaceJSONRow is the stable per-workspace JSON shape for `ws ls -o json`. +type workspaceJSONRow struct { + Name string `json:"name"` + Head string `json:"head"` + Revisions int `json:"revisions"` + Resumable bool `json:"resumable"` +} + +// jsonWorkspaceInfos renders a workspace listing as {"workspaces": [...]}. +func jsonWorkspaceInfos(infos []WorkspaceInfo) (string, error) { + rows := make([]workspaceJSONRow, 0, len(infos)) + for _, w := range infos { + rows = append(rows, workspaceJSONRow{ + Name: w.Name, + Head: w.Head, + Revisions: w.Revisions, + Resumable: w.Resumable, + }) + } + return encodeJSON(struct { + Workspaces []workspaceJSONRow `json:"workspaces"` + }{Workspaces: rows}) +} + +// revisionJSONRow is the stable per-revision JSON shape for `ws log -o json`. +type revisionJSONRow struct { + Name string `json:"name"` + Phase string `json:"phase"` + Resumable bool `json:"resumable"` + Lineage string `json:"lineage"` +} + +// jsonRevisionLog renders a revision log as {"revisions": [...]}. +func jsonRevisionLog(revs []RevisionInfo) (string, error) { + rows := make([]revisionJSONRow, 0, len(revs)) + for _, r := range revs { + rows = append(rows, revisionJSONRow{ + Name: r.Name, + Phase: r.Phase, + Resumable: r.Resumable, + Lineage: r.Lineage, + }) + } + return encodeJSON(struct { + Revisions []revisionJSONRow `json:"revisions"` + }{Revisions: rows}) +} diff --git a/internal/agentcli/workspace_cmd.go b/internal/agentcli/workspace_cmd.go index adcae673d..2ae2f58a5 100644 --- a/internal/agentcli/workspace_cmd.go +++ b/internal/agentcli/workspace_cmd.go @@ -51,26 +51,54 @@ func runWs(ctx context.Context, args []string, b WorkspaceBackend, out, errw io. fmt.Fprintln(out, args[1]) return 0 case "ls": - ns := parseNamespace(args[1:]) + jsonOut, rest, ferr := extractOutputFlag(args[1:]) + if ferr != nil { + fmt.Fprintf(errw, "ws ls: %v\n", ferr) + return ExitUsage + } + ns := parseNamespace(rest) infos, err := b.ListWorkspaces(ctx, ns) if err != nil { fmt.Fprintf(errw, "ws ls: %v\n", err) - return 1 + return exitCodeFor(err) + } + if jsonOut { + s, e := jsonWorkspaceInfos(infos) + if e != nil { + fmt.Fprintf(errw, "ws ls: %v\n", e) + return ExitError + } + fmt.Fprint(out, s) + return ExitOK } fmt.Fprint(out, formatWorkspaceList(infos)) - return 0 + return ExitOK case "log": - if len(args) < 2 { + jsonOut, rest, ferr := extractOutputFlag(args[1:]) + if ferr != nil { + fmt.Fprintf(errw, "ws log: %v\n", ferr) + return ExitUsage + } + if len(rest) < 1 { fmt.Fprint(errw, "ws log: a workspace is required\n\n"+wsUsage) - return 2 + return ExitUsage } - revs, err := b.Log(ctx, args[1]) + revs, err := b.Log(ctx, rest[0]) if err != nil { fmt.Fprintf(errw, "ws log: %v\n", err) - return 1 + return exitCodeFor(err) + } + if jsonOut { + s, e := jsonRevisionLog(revs) + if e != nil { + fmt.Fprintf(errw, "ws log: %v\n", e) + return ExitError + } + fmt.Fprint(out, s) + return ExitOK } fmt.Fprint(out, formatRevisionLog(revs)) - return 0 + return ExitOK case "diff": if len(args) < 3 { fmt.Fprint(errw, "ws diff: a workspace and a revision are required\n\n"+wsUsage) From e5651d826a0cbbd3051e804dd4c806ad21a678d6 Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Tue, 7 Jul 2026 13:59:47 +0200 Subject: [PATCH 2/3] fix(cli): resolve CodeRabbit review on the automation contract - 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 . - 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) Signed-off-by: Jannes Stubbemann --- internal/agentcli/automation_test.go | 64 ++++++++++++++++++++++ internal/agentcli/clusterbackend.go | 32 ++++++++--- internal/agentcli/clusterbackend_test.go | 34 ++++++++++++ internal/agentcli/commands.go | 68 ++++++++++-------------- internal/agentcli/output.go | 24 ++++++++- internal/agentcli/workspace_cmd.go | 28 +++------- 6 files changed, 181 insertions(+), 69 deletions(-) diff --git a/internal/agentcli/automation_test.go b/internal/agentcli/automation_test.go index 3f143f56c..d3c19a188 100644 --- a/internal/agentcli/automation_test.go +++ b/internal/agentcli/automation_test.go @@ -212,3 +212,67 @@ func TestForkTimeoutAndWaitFlags(t *testing.T) { t.Fatalf("fork call HasDeadline=%v NoWait=%v, want both true", calls[0].HasDeadline, calls[0].NoWait) } } + +// TestForkIDAfterFlags asserts the sandbox id parses correctly when it follows +// value-taking flags (e.g. --count 2 sbx-1). A naive single flag.Parse would +// mis-read the flag value "2" as the id. +func TestForkIDAfterFlags(t *testing.T) { + for _, args := range [][]string{ + {"fork", "--count", "2", "--timeout", "10", "sbx-1"}, + {"fork", "sbx-1", "--count", "2"}, + {"fork", "--count", "2", "sbx-1"}, + } { + fb := NewFakeBackend() + fb.ForkIDs = []string{"f1", "f2"} + code, _, errw := runCLI(t, fb, args...) + if code != ExitOK { + t.Fatalf("%v: exit code = %d (%s), want %d", args, code, errw.String(), ExitOK) + } + calls := fb.RecordedCalls() + if len(calls) != 1 || calls[0].Method != "fork" || calls[0].SandboxID != "sbx-1" || calls[0].Replicas != 2 { + t.Fatalf("%v: call = %+v, want fork sbx-1 x2", args, calls) + } + } +} + +// TestTerminateIDAfterFlags asserts terminate parses the id after --timeout. +func TestTerminateIDAfterFlags(t *testing.T) { + fb := NewFakeBackend() + code, _, errw := runCLI(t, fb, "sandbox", "terminate", "--timeout", "5", "sbx-9") + if code != ExitOK { + t.Fatalf("exit code = %d (%s), want %d", code, errw.String(), ExitOK) + } + calls := fb.RecordedCalls() + if len(calls) != 1 || calls[0].SandboxID != "sbx-9" || !calls[0].HasDeadline { + t.Fatalf("call = %+v, want terminate sbx-9 with deadline", calls) + } +} + +// TestOutputFormatLastFlagWins asserts a later human format overrides an earlier +// --json (last flag wins), so an explicit -o table after --json renders human. +func TestOutputFormatLastFlagWins(t *testing.T) { + fb := NewFakeBackend() + fb.ListInfos = []SandboxInfo{{Name: "sbx-1", Pool: "python", Phase: "Ready"}} + code, out, _ := runCLI(t, fb, "sandbox", "ls", "--json", "-o", "table") + if code != ExitOK { + t.Fatalf("exit code = %d, want %d", code, ExitOK) + } + if strings.Contains(out.String(), `"sandboxes"`) { + t.Fatalf("got JSON despite a trailing -o table (last flag should win): %q", out.String()) + } +} + +// TestWsLogNotFoundExitCode asserts a not-found workspace maps to ExitNotFound +// on the ws log path. +func TestWsLogNotFoundExitCode(t *testing.T) { + fb := NewFakeBackend() + ws := fb.Workspace().(*FakeWorkspaceBackend) + ws.Errors["ws_log"] = ErrNotFound + code, _, errw := runCLI(t, fb, "ws", "log", "ghost") + if code != ExitNotFound { + t.Fatalf("exit code = %d, want %d (not found)", code, ExitNotFound) + } + if errw.Len() == 0 { + t.Fatalf("want a diagnostic on stderr") + } +} diff --git a/internal/agentcli/clusterbackend.go b/internal/agentcli/clusterbackend.go index 03de73d57..38c505689 100644 --- a/internal/agentcli/clusterbackend.go +++ b/internal/agentcli/clusterbackend.go @@ -232,12 +232,17 @@ func (b *ClusterBackend) Fork(ctx context.Context, sandboxID string, n int) ([]s if b.forkReadyHook != nil { b.forkReadyHook(ctx, name, n) } - return b.waitChildrenReady(ctx, name, n) + // --no-wait returns as soon as the n children are named in Status.Children, + // regardless of phase, so the caller still gets the child ids without waiting + // for the microVMs to reach Ready. The default waits for Ready. + return b.waitChildren(ctx, name, n, !noWaitRequested(ctx)) } -// waitChildrenReady polls the Sandbox until at least n children are Ready, -// then returns their names. Children are reported in Status.Children. -func (b *ClusterBackend) waitChildrenReady(ctx context.Context, name string, n int) ([]string, error) { +// waitChildren polls the Sandbox until at least n children are present in +// Status.Children, then returns their names. When requireReady is true a child +// counts only once its phase is Ready; when false it counts as soon as it is +// named, so a --no-wait fork returns the child ids without waiting for boot. +func (b *ClusterBackend) waitChildren(ctx context.Context, name string, n int, requireReady bool) ([]string, error) { deadline := b.now().Add(b.pollTimeout) for { var sandbox v1.Sandbox @@ -247,7 +252,7 @@ func (b *ClusterBackend) waitChildrenReady(ctx context.Context, name string, n i ready := make([]string, 0, n) for i := range sandbox.Status.Children { child := &sandbox.Status.Children[i] - if child.Phase == v1.SandboxReady { + if !requireReady || child.Phase == v1.SandboxReady { ready = append(ready, child.Name) } } @@ -255,7 +260,11 @@ func (b *ClusterBackend) waitChildrenReady(ctx context.Context, name string, n i return ready[:n], nil } if b.now().After(deadline) { - return nil, fmt.Errorf("fork %s not ready after %s", name, b.pollTimeout) + state := "ready" + if !requireReady { + state = "created" + } + return nil, fmt.Errorf("fork %s: %d of %d children %s after %s", name, len(ready), n, state, b.pollTimeout) } select { case <-ctx.Done(): @@ -425,8 +434,17 @@ func (w *ClusterWorkspaceBackend) ListWorkspaces(ctx context.Context, namespace return out, nil } -// Log lists the revisions belonging to workspace, newest first. +// Log lists the revisions belonging to workspace, newest first. A missing +// workspace is reported as ErrNotFound (exit 3) rather than an empty log, so an +// automated caller can tell "no revisions yet" from "no such workspace". func (w *ClusterWorkspaceBackend) Log(ctx context.Context, workspace string) ([]RevisionInfo, error) { + var ws v1.Workspace + if err := w.client.Get(ctx, client.ObjectKey{Namespace: w.namespace, Name: workspace}, &ws); err != nil { + if apierrors.IsNotFound(err) { + return nil, fmt.Errorf("workspace %s not found: %w", workspace, ErrNotFound) + } + return nil, fmt.Errorf("get workspace %s: %w", workspace, err) + } var list v1.WorkspaceRevisionList if err := w.client.List(ctx, &list, client.InNamespace(w.namespace)); err != nil { return nil, fmt.Errorf("list revisions: %w", err) diff --git a/internal/agentcli/clusterbackend_test.go b/internal/agentcli/clusterbackend_test.go index db28b6602..f3bad8e5a 100644 --- a/internal/agentcli/clusterbackend_test.go +++ b/internal/agentcli/clusterbackend_test.go @@ -235,6 +235,40 @@ func TestClusterBackendForkCreatesFork(t *testing.T) { } } +func TestClusterBackendForkNoWaitReturnsUnreadyChildren(t *testing.T) { + scheme := testScheme(t) + c := fake.NewClientBuilder(). + WithScheme(scheme). + WithStatusSubresource(&v1.Sandbox{}). + Build() + be := &ClusterBackend{ + client: c, namespace: "default", now: time.Now, + pollInterval: time.Millisecond, pollTimeout: 2 * time.Second, + } + // Children are named but left in the zero (not Ready) phase. + be.forkReadyHook = func(ctx context.Context, name string, n int) { + var sandbox v1.Sandbox + if err := c.Get(ctx, client.ObjectKey{Namespace: "default", Name: name}, &sandbox); err != nil { + return + } + children := make([]v1.SandboxChild, 0, n) + for i := 0; i < n; i++ { + children = append(children, v1.SandboxChild{Name: name + "-" + string(rune('a'+i))}) + } + sandbox.Status.Children = children + _ = c.Status().Update(ctx, &sandbox) + } + + // --no-wait must return the child ids without waiting for Ready. + ids, err := be.Fork(withNoWait(context.Background()), "sbx-1", 2) + if err != nil { + t.Fatalf("Fork no-wait: %v", err) + } + if len(ids) != 2 { + t.Fatalf("Fork no-wait ids = %v, want 2 even though children are not Ready", ids) + } +} + func TestClusterBackendExecSendsBearerAndRedactsToken(t *testing.T) { const token = "super-secret-token-value" // The Connect runtime server echoes the presented bearer token into its error diff --git a/internal/agentcli/commands.go b/internal/agentcli/commands.go index 7dc824694..f36cd5449 100644 --- a/internal/agentcli/commands.go +++ b/internal/agentcli/commands.go @@ -129,17 +129,9 @@ func cmdSandboxLs(ctx context.Context, args []string, backend Backend, out, errw fmt.Fprintf(errw, "list sandboxes: %v\n", err) return exitCodeFor(err) } - if jsonOut { - s, err := jsonSandboxInfos(infos) - if err != nil { - fmt.Fprintf(errw, "sandbox ls: %v\n", err) - return ExitError - } - fmt.Fprint(out, s) - return ExitOK - } - fmt.Fprint(out, formatSandboxInfos(infos)) - return ExitOK + return renderList(out, errw, "sandbox ls", jsonOut, + func() (string, error) { return jsonSandboxInfos(infos) }, + func() string { return formatSandboxInfos(infos) }) } func cmdSandboxExec(ctx context.Context, args []string, backend Backend, out, errw io.Writer) int { @@ -164,10 +156,6 @@ func cmdSandboxExec(ctx context.Context, args []string, backend Backend, out, er } func cmdSandboxFork(ctx context.Context, args []string, backend Backend, out, errw io.Writer) int { - // Accept the sandbox id either before or after the flags, so both - // `fork sbx-1 --replicas 3` and `fork --replicas 3 sbx-1` work. The stdlib - // flag parser stops at the first non-flag token, so split the id out first. - id, rest := splitFirstPositional(args) fs := newFlagSet("sandbox fork", errw) // --count is the documented flag (#311); --replicas is kept as a back-compat // alias. --count wins when set (default 0 means "not set, use --replicas"). @@ -176,13 +164,13 @@ func cmdSandboxFork(ctx context.Context, args []string, backend Backend, out, er wait := fs.Bool("wait", true, "wait for the forks to become Ready before returning") noWait := fs.Bool("no-wait", false, "return as soon as the forks are created; do not wait for Ready") timeout := fs.Int("timeout", 0, "max seconds to wait for Ready (0 = backend default)") - if err := fs.Parse(rest); err != nil { + // Accept the sandbox id either before or after the flags, so both + // `fork sbx-1 --count 3` and `fork --count 3 sbx-1` parse correctly. + id, err := parseLeadingID(fs, args) + if err != nil { fmt.Fprint(errw, usage) return ExitUsage } - if id == "" && fs.NArg() > 0 { - id = fs.Arg(0) - } if id == "" { fmt.Fprintf(errw, "sandbox fork: a sandbox id is required\n\n%s", usage) return ExitUsage @@ -205,19 +193,16 @@ func cmdSandboxFork(ctx context.Context, args []string, backend Backend, out, er } func cmdSandboxTerminate(ctx context.Context, args []string, backend Backend, out, errw io.Writer) int { - id, rest := splitFirstPositional(args) fs := newFlagSet("sandbox terminate", errw) // terminate is asynchronous: the object is deleted and the controller reaps // it. --timeout bounds the delete call itself; waiting until the sandbox is // fully reaped is a named follow-up, so --wait is not offered here yet. timeout := fs.Int("timeout", 0, "max seconds to bound the delete call (0 = no bound)") - if err := fs.Parse(rest); err != nil { + id, err := parseLeadingID(fs, args) + if err != nil { fmt.Fprint(errw, usage) return ExitUsage } - if id == "" && fs.NArg() > 0 { - id = fs.Arg(0) - } if id == "" { fmt.Fprintf(errw, "sandbox terminate: a sandbox id is required\n\n%s", usage) return ExitUsage @@ -252,21 +237,26 @@ func cmdDev(_ context.Context, args []string, _, errw io.Writer) int { } } -// splitFirstPositional returns the first argument that is not a flag (does not -// start with "-") and the remaining args with that token removed, so a leading -// positional id can appear before flags that the stdlib flag parser would -// otherwise stop at. If there is no positional token, id is empty and rest is -// args unchanged. -func splitFirstPositional(args []string) (id string, rest []string) { - for i, a := range args { - if !strings.HasPrefix(a, "-") { - rest = make([]string, 0, len(args)-1) - rest = append(rest, args[:i]...) - rest = append(rest, args[i+1:]...) - return a, rest - } - } - return "", args +// parseLeadingID parses fs from args when the command takes a single positional +// id that may appear either before or after its flags. The stdlib flag parser +// stops at the first non-flag token, so a single parse mis-reads a flag value as +// the id (`fork --count 2 sbx-1` would take "2"). This parses once to consume any +// leading flags, lifts the first remaining positional as the id, then parses the +// tokens that followed the id so trailing flags (`fork sbx-1 --count 2`) are +// still honored. It returns an empty id when no positional is present. +func parseLeadingID(fs *flag.FlagSet, args []string) (id string, err error) { + if err = fs.Parse(args); err != nil { + return "", err + } + pos := fs.Args() + if len(pos) == 0 { + return "", nil + } + id = pos[0] + if err = fs.Parse(pos[1:]); err != nil { + return "", err + } + return id, nil } // formatSandboxInfos renders SandboxInfo rows as an aligned table with columns diff --git a/internal/agentcli/output.go b/internal/agentcli/output.go index 88495c8d6..4b5f9247e 100644 --- a/internal/agentcli/output.go +++ b/internal/agentcli/output.go @@ -3,9 +3,29 @@ package agentcli import ( "encoding/json" "fmt" + "io" "strings" ) +// renderList writes a read-verb listing either as JSON (when jsonOut, via +// jsonFn) or as the human table (via textFn), and returns the process exit code. +// It is the single place the read verbs branch on output format so the JSON/text +// shape does not drift as more listing verbs are added. verb names the command +// for the diagnostic on an encode failure. +func renderList(out, errw io.Writer, verb string, jsonOut bool, jsonFn func() (string, error), textFn func() string) int { + if jsonOut { + s, err := jsonFn() + if err != nil { + fmt.Fprintf(errw, "%s: %v\n", verb, err) + return ExitError + } + fmt.Fprint(out, s) + return ExitOK + } + fmt.Fprint(out, textFn()) + return ExitOK +} + // extractOutputFlag pulls the output-format flags out of args and reports // whether structured JSON was requested. It recognizes, in any position: // @@ -24,7 +44,9 @@ func extractOutputFlag(args []string) (jsonOut bool, rest []string, err error) { case "json": jsonOut = true case "table", "text", "human", "": - // The human render is the default; nothing to set. + // A later human format wins over an earlier --json (last flag wins), + // so an explicit -o table after --json resets to the human render. + jsonOut = false default: return fmt.Errorf("unknown output format %q (want json or table)", v) } diff --git a/internal/agentcli/workspace_cmd.go b/internal/agentcli/workspace_cmd.go index 2ae2f58a5..d37dae3bd 100644 --- a/internal/agentcli/workspace_cmd.go +++ b/internal/agentcli/workspace_cmd.go @@ -62,17 +62,9 @@ func runWs(ctx context.Context, args []string, b WorkspaceBackend, out, errw io. fmt.Fprintf(errw, "ws ls: %v\n", err) return exitCodeFor(err) } - if jsonOut { - s, e := jsonWorkspaceInfos(infos) - if e != nil { - fmt.Fprintf(errw, "ws ls: %v\n", e) - return ExitError - } - fmt.Fprint(out, s) - return ExitOK - } - fmt.Fprint(out, formatWorkspaceList(infos)) - return ExitOK + return renderList(out, errw, "ws ls", jsonOut, + func() (string, error) { return jsonWorkspaceInfos(infos) }, + func() string { return formatWorkspaceList(infos) }) case "log": jsonOut, rest, ferr := extractOutputFlag(args[1:]) if ferr != nil { @@ -88,17 +80,9 @@ func runWs(ctx context.Context, args []string, b WorkspaceBackend, out, errw io. fmt.Fprintf(errw, "ws log: %v\n", err) return exitCodeFor(err) } - if jsonOut { - s, e := jsonRevisionLog(revs) - if e != nil { - fmt.Fprintf(errw, "ws log: %v\n", e) - return ExitError - } - fmt.Fprint(out, s) - return ExitOK - } - fmt.Fprint(out, formatRevisionLog(revs)) - return ExitOK + return renderList(out, errw, "ws log", jsonOut, + func() (string, error) { return jsonRevisionLog(revs) }, + func() string { return formatRevisionLog(revs) }) case "diff": if len(args) < 3 { fmt.Fprint(errw, "ws diff: a workspace and a revision are required\n\n"+wsUsage) From cb4e3dd3a7fb8c332358f120dec621cd6f4cf328 Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Thu, 9 Jul 2026 23:07:42 +0200 Subject: [PATCH 3/3] docs(cli): scope the exit-3 contract to the verbs that implement it 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 Signed-off-by: Jannes Stubbemann --- docs/cli.md | 11 +++++++---- internal/agentcli/cli.go | 4 ++-- internal/agentcli/hostedbackend_test.go | 6 +++--- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/docs/cli.md b/docs/cli.md index a3f621d4b..ef8319953 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -58,8 +58,8 @@ diagnostic on stderr carries the cause and remediation. | `0` | success | The command succeeded. For `run` this is the executed command's own exit code (0 on success). | | `1` | error | A general, remediable runtime error (backend unreachable, a failed operation). The stderr diagnostic names the cause. | | `2` | usage | A usage error: an unknown subcommand, a missing argument, a bad flag, or an unknown `-o` output format. | -| `3` | not found | The targeted sandbox or workspace does not exist. | -| `124` | timeout | A `--wait`/`--timeout` deadline elapsed before the operation completed. The value matches the coreutils `timeout` tool so it is familiar in pipelines. | +| `3` | not found | The targeted object does not exist. Wired for the sandbox verbs (`create`, `fork`, `terminate`, `run`) and for `ws ls`/`ws log`; the remaining `ws` verbs (`diff`, `fork`, `revert`, `rm`, `bind`) still exit `1` for a missing target, and converting them is a named follow-up. | +| `124` | timeout | An explicit `--wait`/`--timeout` deadline elapsed before the operation completed. The value matches the coreutils `timeout` tool so it is familiar in pipelines. | `run` is the one verb that passes through: its exit code is the executed command's exit code, so `mitos run false` exits non-zero exactly as `false` @@ -69,7 +69,8 @@ would. The other verbs use the table above. The read and inspect verbs accept `-o json` (equivalently `--output json` or the `--json` shorthand) and emit a stable JSON envelope on stdout. The default -remains the human-aligned table. An unrecognized format is a usage error (exit +remains the human-aligned table (`-o table`; `text` and `human` are accepted +aliases for it). An unrecognized format is a usage error (exit `2`), never a silent fall-back to the human render, so an agent that asked for JSON always gets JSON or a clear failure. @@ -128,7 +129,9 @@ The lifecycle verbs that start an asynchronous operation accept a uniform no-op there. - `--timeout N` bounds the wait to `N` seconds. When the deadline elapses the command exits `124` (timeout) rather than blocking indefinitely. `--timeout 0` - (the default) uses the backend's own bound. + (the default) uses the backend's own bound; that internal bound elapsing is + reported as a not-ready runtime error (exit `1`), so `124` always means the + deadline the caller asked for. - `mitos sandbox terminate` accepts `--timeout N` to bound the delete call. Terminate is asynchronous (the controller reaps the object); waiting until the sandbox is fully reaped is a named follow-up, so `--wait` is not offered on diff --git a/internal/agentcli/cli.go b/internal/agentcli/cli.go index 6c22a40d0..0126f5a2d 100644 --- a/internal/agentcli/cli.go +++ b/internal/agentcli/cli.go @@ -50,7 +50,7 @@ Exit codes: 0 success (run: the executed command's exit code) 1 a general, remediable runtime error 2 usage error (unknown subcommand, missing argument, bad flag or format) - 3 the targeted sandbox or workspace was not found + 3 the target was not found (sandbox verbs and ws ls/log; other ws verbs exit 1) 124 a --wait/--timeout deadline elapsed ` @@ -62,7 +62,7 @@ Exit codes: // ExitOK (0) success (for run: the executed command's exit code) // ExitError (1) a general, remediable runtime error // ExitUsage (2) usage error (unknown subcommand, missing arg, bad flag) -// ExitNotFound (3) the targeted sandbox or workspace was not found +// ExitNotFound (3) the target was not found (sandbox verbs and ws ls/log) // ExitTimeout (124) a --wait/--timeout deadline elapsed // // For run, the exit code is the executed command's exit code so callers can diff --git a/internal/agentcli/hostedbackend_test.go b/internal/agentcli/hostedbackend_test.go index 6d5af80bb..fd5fd32fa 100644 --- a/internal/agentcli/hostedbackend_test.go +++ b/internal/agentcli/hostedbackend_test.go @@ -144,9 +144,9 @@ func (g *hostedGateway) ServeHTTP(w http.ResponseWriter, r *http.Request) { id = raw } writeJSON(http.StatusOK, map[string]any{ - "id": id, - "template_id": c.Body["template"], - "endpoint": "sandbox-endpoint:9091", + "id": id, + "template_id": c.Body["template"], + "endpoint": "sandbox-endpoint:9091", "fork_time_ms": 12.5, })