From 034555674fe0037c0e396b922619caa8975e289e Mon Sep 17 00:00:00 2001 From: pratikbin <68642400+pratikbin@users.noreply.github.com> Date: Tue, 11 Aug 2026 18:57:59 +0530 Subject: [PATCH 1/3] feat(sandbox): add JSON output, stderr errors, exec stdin Make the sandbox CLI usable from CI pipelines and AI agents without scraping human-readable text. Every sandbox subcommand that creates, changes, or deletes something now reports its result as JSON via a shared renderResult helper. Each object carries an "action" naming what happened plus the ids needed to chain the next call, using field names shared with the read commands. Errors now go to stderr instead of stdout, so pipes and 2>/dev/null behave. In JSON mode they become a machine-readable envelope on stdout, wiring up output.RenderError, which was written but never called. A new APIError.Code() maps HTTP status to a stable slug. Global flags (--output, --debug, --api-url, --api-key, --sandbox-api-url, --sandbox-gateway) now work after the subcommand. urfave/cli v2 stops parsing app-level flags at the first subcommand token, so argv is rewritten before app.Run rather than mirroring hidden flags onto each command, which would parse them but never read their values. Tokens after a bare -- are left alone. sandbox exec forwards piped stdin to the command and gains --stdin FILE. The API already carried a Stdin field that the CLI never populated. exec gates JSON on an explicit --output json rather than the non-TTY auto-detect, because its stdout is the command's own output. ANSI styling is disabled when stdout is not a terminal or NO_COLOR is set, and in JSON mode all pterm output is redirected to stderr so stdout holds exactly one document. Interactive streams (shell, sync, editor, exec --stream, template logs) stay human-readable. tunnel and vpn up emit their result before blocking. Options weighed and rejected are recorded in docs/decisions.md. --- CLAUDE.md | 129 +++++++++++++++++++++++---------- README.md | 86 ++++++++++++++++++++++ cmd/root/root.go | 20 +++++ cmd/sandbox/create.go | 10 ++- cmd/sandbox/devices.go | 39 +++++++--- cmd/sandbox/disk.go | 33 ++++++++- cmd/sandbox/edit.go | 47 ++++++++---- cmd/sandbox/exec.go | 73 +++++++++++++++++-- cmd/sandbox/firewall.go | 20 +++-- cmd/sandbox/fork.go | 28 ++++--- cmd/sandbox/jsonout.go | 35 +++++++++ cmd/sandbox/network.go | 55 +++++++++++--- cmd/sandbox/pause.go | 5 ++ cmd/sandbox/pull.go | 6 ++ cmd/sandbox/push.go | 6 ++ cmd/sandbox/resume.go | 8 ++ cmd/sandbox/rm.go | 13 ++++ cmd/sandbox/template.go | 16 +++- cmd/sandbox/tunnel.go | 12 ++- cmd/sandbox/vpn.go | 17 ++++- docs/decisions.md | 121 +++++++++++++++++++++++++++++++ internal/api/types.go | 24 ++++++ internal/cliargs/hoist.go | 80 ++++++++++++++++++++ internal/cliargs/hoist_test.go | 74 +++++++++++++++++++ internal/output/render.go | 36 ++++++++- internal/terminal/tty.go | 8 ++ main.go | 17 ++++- 27 files changed, 907 insertions(+), 111 deletions(-) create mode 100644 cmd/sandbox/jsonout.go create mode 100644 docs/decisions.md create mode 100644 internal/cliargs/hoist.go create mode 100644 internal/cliargs/hoist_test.go diff --git a/CLAUDE.md b/CLAUDE.md index 36dc7cb..addcca6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,44 +8,43 @@ mirrors these commands. On any command / flag change, run the protocol below β€” the concrete website-regen steps live in "Keeping the Website Docs in Sync" near the end of this file. This repo is one of five in the product mesh. -### Repo map - -| repo | path | role | public? | changes that ripple across the mesh | -|---|---|---|---|---| -| **fc** | `../fc` | control-plane β€” **source of truth** | πŸ”’ private | HTTP API, wire/JSON fields, error shapes, lifecycle/state, limits/quotas, behavior | -| **fc-sdk** | `../fc-sdk` | TypeScript SDK **+ `examples/`** | 🌐 public | public SDK methods, wire types, example apps | -| **createos-cli** | `../createos-cli` | Go CLI | 🌐 public | commands, flags, help/UX text | -| **website-04** | `../website-04` (`content/docs/Sandbox`) | public docs | 🌐 public | REST / SDK / CLI reference + concept pages | -| **createos-plugin** | `../createos-plugin` | Claude Code plugin over the `createos` CLI | 🌐 public | skills, slash commands, hooks | - -### What counts as a shared surface - -HTTP endpoint or method Β· wire or JSON field Β· error shape Β· sandbox -lifecycle/state Β· limit or quota Β· CLI command or flag Β· public SDK method Β· -documented behavior. A change confined to internals β€” refactor, comment, -private helper, test-only β€” is **not** a shared surface, so skip the mesh for it. - -### Protocol β€” run before finalizing a shared-surface change - -1. **Classify origin.** `fc` is the source of truth; SDK / CLI / docs / plugin - are downstream consumers. A downstream change that implies a backend change - (new field, new endpoint) β†’ surface it to the user; never invent server - behavior inside a client. -2. **Search every sibling** for the touched symbol / endpoint / flag β€” - `semble search` first, then `rg`. -3. **Build a status matrix** per sibling: `already-present` Β· - `missing-needs-update` Β· `n/a`. **Flag the already-present ones to the user** - ("already exists in fc-sdk + docs"). Never silently duplicate a change that is - already there β€” that is the whole point of this check. -4. **`fc` β†’ any public repo is a leak-guard boundary (security).** Strip private - implementation, security internals, infra, threat-model notes, and - internal-only tooling (`fcctl`, host filesystem paths, mTLS/CA internals) - before anything lands in a public repo. Respect each public repo's own wording - rules (e.g. `fc-sdk/AGENTS.md` forbids the word "VM"). Report the proposed diff - and **ask for approval before landing any public edit** β€” never auto-write - across the boundary. -5. **Use the `sync-docs` skill** to execute SDK / CLI / website reconciliation - against upstream `fc` where it applies. +| repo | role | public? | +|---|---|---| +| **fc** | control-plane β€” source of truth | πŸ”’ private | +| **fc-sdk** | TypeScript SDK + `examples/` | 🌐 public | +| **createos-cli** | Go CLI | 🌐 public | +| **website-04** (`content/docs/Sandbox`) | public docs | 🌐 public | +| **createos-plugin** | Claude Code plugin over the `createos` CLI | 🌐 public | + +**Shared surface** = HTTP endpoint or method Β· wire or JSON field Β· error shape Β· +sandbox lifecycle/state Β· limit or quota Β· CLI command or flag Β· public SDK +method Β· documented behavior. Internals β€” refactor, private helper, test-only β€” +are not, so skip the mesh for them. + +On a shared-surface change: search every sibling for the touched symbol +(`semble search` first, then `rg`), report a per-sibling matrix +(`already-present` / `missing-needs-update` / `n/a`) before finalizing, and never +silently duplicate what is already there. Origin matters β€” `fc` is upstream; a +downstream change implying new server behavior goes to the user, never invented +inside a client. **`fc` β†’ any public repo is a leak-guard boundary:** strip +private implementation, security internals, infra, threat-model notes, and +internal-only tooling (`fcctl`, host filesystem paths, mTLS/CA internals), +respect each public repo's own wording rules (e.g. `fc-sdk/AGENTS.md` forbids +the word "VM"), and get approval before landing any public edit. The `sync-docs` +skill executes SDK / CLI / website reconciliation against upstream `fc`. + +### Downstream reference implementations (not mesh-protocol members) + +Two public repos shell out to this CLI and are worth checking before a +command/flag/help-text change ships, even though neither owns shared +surface and both sit outside the formal 5-repo protocol above: + +- **createos-plugin** (`../createos-plugin/createos-sandbox`) β€” the primary + CLI wrapper; most exposed, since it parses `createos` stdout. +- **createos-sandbox-ghar** (`../createos-sandbox-ghar`) β€” its + `.github/workflows/bump-runner.yml` daily job shells out to this CLI to + rebuild the `ghar-runner` rootfs template; a flag/output change here can + break that job silently. ## Project Structure @@ -91,6 +90,60 @@ The API has two response shapes β€” use the right one: 2. Match field names exactly to the JSON response β€” use nullable pointers (`*string`) for fields that can be `null` 3. For errors, return `ParseAPIError(resp.StatusCode(), resp.Body())` β€” never `fmt.Errorf("API error %d: %s", ...)` +## Machine-readable Output + +Decisions and rejected alternatives: `docs/decisions.md`. + +### Every mutation must emit JSON + +A `sandbox` command that creates, changes, or deletes something calls +`renderResult` (`cmd/sandbox/jsonout.go`), never a bare `pterm.Success`: + +```go +renderResult(c, "created", map[string]any{ + "id": resp.ID, + "name": str(resp.Name), +}, func() { + pterm.Success.Printfln("Created %s", resp.ID) +}) +``` + +The human renderer goes in the closure; it runs only in table mode. Rules: + +- `action` names what happened (past tense: `created`, `paused`, `disk_attached`). +- Field names match the read commands β€” a caller diffs `create` against `get`. +- Nullable API pointers go through `str()` so a key is always present. +- Batch commands return a `results` array with one entry per ref, plus + `deleted` / `failed` counts. An exit code cannot express a partial batch. +- Interactive streams (`shell`, `sync`, `editor`, `exec --stream`, + `template logs`) stay text-only. Blocking commands that have a result + (`tunnel`, `vpn up`) emit it *before* they block. + +### Two JSON checks β€” pick the right one + +| Helper | Use for | +|---|---| +| `output.IsJSON` / `output.Render` | Normal commands. True when `--output json` **or** stdout is not a TTY. | +| `output.IsJSONExplicit` | Commands whose stdout **is** the payload (`exec`). Only true when the user typed `--output json`, so `exec … > file` still writes raw bytes. | + +### Streams + +- **stdout** β€” data only. In JSON mode it holds exactly one document; the + root `Before` hook redirects all pterm output to stderr to guarantee it. +- **stderr** β€” errors, progress, hints, spinners. +- Errors are formatted in `main.go`: a JSON envelope on stdout in JSON mode, + otherwise plain text on stderr. Add new status codes to + `api.APIError.Code()`, which produces the envelope's `code` slug. +- Colour is disabled automatically for non-TTY stdout and for `NO_COLOR`. + +### Global flags + +`internal/cliargs.Hoist` rewrites argv before `app.Run` so global flags work +after the subcommand. **When adding or renaming a global flag in +`root.go`, add it to `globalStringFlags` / `globalBoolFlags` too** β€” a +missing entry silently reintroduces the "flag provided but not defined" +error. Tokens after a bare `--` are never hoisted. + ## Error Handling ### API errors diff --git a/README.md b/README.md index 57afc48..369c0d2 100644 --- a/README.md +++ b/README.md @@ -491,6 +491,8 @@ createos sandbox list --status paused --quiet | xargs createos sandbox rm --forc createos sandbox get createos sandbox exec my-box -- uname -a createos sandbox exec my-box --stream -- pip install requests +echo "hello" | createos sandbox exec my-box -- cat +createos sandbox exec my-box --stdin ./setup.sh -- bash createos sandbox shell my-box createos sandbox shell my-box --ssh createos sandbox push my-box ./script.py /root/script.py @@ -595,6 +597,90 @@ createos projects get --project --output json createos environments list --project -o json ``` +### Global flags work anywhere on the line + +`--output`, `--debug`, `--api-url`, `--api-key`, `--sandbox-api-url`, and +`--sandbox-gateway` can go before or after the subcommand β€” both of these +are the same command: + +```bash +createos --output json sandbox create +createos sandbox create --output json +``` + +Anything after a bare `--` is passed through untouched, so +`createos sandbox exec my-box -- ./ci.sh --debug` still sends `--debug` to +your script rather than to the CLI. + +### Sandbox commands that change something + +Every `sandbox` subcommand that creates, changes, or deletes something also +reports its result as JSON. Each object carries an `action` naming what +happened, plus the ids needed to chain the next command: + +```bash +$ createos sandbox create --output json +{ + "action": "created", + "id": "sb-01k...", + "name": "quiet-lake-4821", + "shape": "s-2vcpu-4gb", + "rootfs": "devbox", + "ip": "10.0.4.19", + "ingress_url": "https://-sb-01k....sb.createos.sh", + "shell_command": "createos sandbox shell sb-01k..." +} + +# chain it +ID=$(createos sandbox create -o json | jq -r .id) +createos sandbox exec "$ID" -- uname -a +createos sandbox rm "$ID" --force +``` + +Batch commands (`rm`, `disk rm`, `network rm`, `template rm`) return one +entry per reference you passed, so you can tell which ones actually went: + +```json +{ + "action": "deleted", + "results": [ + { "ref": "my-box", "id": "sb-01k...", "deleted": true }, + { "ref": "typo-box", "deleted": false, "error": "no sandbox named typo-box" } + ], + "deleted": 1, + "failed": 1 +} +``` + +`sandbox shell`, `sandbox sync`, `sandbox editor`, and `sandbox exec --stream` +stay human-readable β€” they are interactive streams with no single result to +report. `sandbox tunnel` and `sandbox vpn up` print their JSON result when the +connection comes up, before they block. + +### Errors + +Errors go to **stderr**, so `2>/dev/null` and pipes behave. In JSON mode the +error is a machine-readable envelope on stdout: + +```bash +$ createos sandbox get nope --output json +{ + "error": { + "code": "not_found", + "message": "no sandbox named nope" + } +} +``` + +Codes: `bad_request`, `unauthorized`, `forbidden`, `not_found`, `conflict`, +`rate_limited`, `server_error`, `api_error`, `error`. The exit code is 1 for +any failure. + +### Colour + +ANSI colour is switched off automatically when stdout is not a terminal, and +whenever `NO_COLOR` is set. CI logs stay greppable without any flag. + ## Options | Flag | Description | diff --git a/cmd/root/root.go b/cmd/root/root.go index c640814..1f9831c 100644 --- a/cmd/root/root.go +++ b/cmd/root/root.go @@ -4,8 +4,10 @@ package root import ( "fmt" "net/url" + "os" "time" + "github.com/pterm/pterm" "github.com/urfave/cli/v2" "github.com/NodeOps-app/createos-cli/cmd/ask" @@ -36,6 +38,7 @@ import ( internaloauth "github.com/NodeOps-app/createos-cli/internal/oauth" "github.com/NodeOps-app/createos-cli/internal/output" "github.com/NodeOps-app/createos-cli/internal/pkg/version" + "github.com/NodeOps-app/createos-cli/internal/terminal" ) // NewApp creates and configures the root CLI application. @@ -89,6 +92,23 @@ func NewApp() *cli.App { Before: func(c *cli.Context) error { // Store the output format in metadata c.App.Metadata[output.FormatKey] = output.DetectFormat(c) + c.App.Metadata[output.FormatExplicitKey] = c.String("output") != "" + + // One choke point for colour: a pipe, a CI log, or NO_COLOR + // must never receive ANSI escapes. pterm styles every helper + // through this global, so disabling it here covers every + // command at once. + if os.Getenv("NO_COLOR") != "" || !terminal.IsInteractive() { + pterm.DisableStyling() + } + + // In JSON mode stdout carries one machine-readable document and + // nothing else. Sending every pterm print to stderr keeps that + // true even for commands that still narrate progress, so a + // consumer never has to strip prose out of the stream. + if output.IsJSON(c) { + pterm.SetDefaultOutput(os.Stderr) + } // Skip auth for --help / -h on any command for _, a := range c.Args().Slice() { diff --git a/cmd/sandbox/create.go b/cmd/sandbox/create.go index d4775fd..dd68206 100644 --- a/cmd/sandbox/create.go +++ b/cmd/sandbox/create.go @@ -203,7 +203,15 @@ func runCreate(c *cli.Context) error { } spinner.Success("Sandbox is ready") - printCreateResult(resp) + renderResult(c, "created", map[string]any{ + "id": resp.ID, + "name": str(resp.Name), + "shape": resp.Shape, + "rootfs": str(resp.Rootfs), + "ip": resp.IP, + "ingress_url": resp.IngressURLTemplate, + "shell_command": fmt.Sprintf("createos sandbox shell %s", resp.ID), + }, func() { printCreateResult(resp) }) return nil } diff --git a/cmd/sandbox/devices.go b/cmd/sandbox/devices.go index 8ed053e..25a02d5 100644 --- a/cmd/sandbox/devices.go +++ b/cmd/sandbox/devices.go @@ -16,7 +16,6 @@ import ( "github.com/urfave/cli/v2" "github.com/NodeOps-app/createos-cli/internal/api" - "github.com/NodeOps-app/createos-cli/internal/output" ) // deviceState is what we persist locally after `devices register`. The @@ -204,9 +203,15 @@ func runDeviceRegister(c *cli.Context) error { if err := saveDeviceState(st); err != nil { return fmt.Errorf("could not save device state: %w", err) } - pterm.Success.Printfln("Registered %q (%s)", view.Name, view.ClientIP) - pterm.Println(pterm.Gray(" Attach this device to a network in the UI, then:")) - pterm.Println(pterm.Gray(" createos sb vpn up")) + renderResult(c, "device_registered", map[string]any{ + "id": view.ID, + "name": view.Name, + "client_ip": view.ClientIP, + }, func() { + pterm.Success.Printfln("Registered %q (%s)", view.Name, view.ClientIP) + pterm.Println(pterm.Gray(" Attach this device to a network in the UI, then:")) + pterm.Println(pterm.Gray(" createos sb vpn up")) + }) return nil } @@ -237,7 +242,12 @@ func runDeviceUnregister(c *cli.Context) error { if err := clearDeviceState(); err != nil { return fmt.Errorf("clear local state: %w", err) } - pterm.Success.Printfln("Unregistered %q.", st.Name) + renderResult(c, "device_unregistered", map[string]any{ + "id": st.DeviceID, + "name": st.Name, + }, func() { + pterm.Success.Printfln("Unregistered %q.", st.Name) + }) return nil } @@ -364,15 +374,24 @@ func runDeviceRemove(c *cli.Context) error { if err := client.DeleteDevice(ctx, target.ID); err != nil { return err } - pterm.Success.Printfln("Removed %q (%s).", target.Name, target.ClientIP) - + clearedLocal := false // If we just deleted the row backing this machine's device.json, // scrub local state too so `vpn up` doesn't loop against a ghost id. if local, _ := loadDeviceState(); local != nil && local.DeviceID == target.ID { //nolint:errcheck _ = clearDeviceState() //nolint:errcheck - pterm.Println(pterm.Gray(" (also cleared local device.json for this machine)")) + clearedLocal = true } + + renderResult(c, "device_removed", map[string]any{ + "id": target.ID, + "name": target.Name, + "client_ip": target.ClientIP, + "cleared_local_state": clearedLocal, + }, func() { + pterm.Success.Printfln("Removed %q (%s).", target.Name, target.ClientIP) + if clearedLocal { + pterm.Println(pterm.Gray(" (also cleared local device.json for this machine)")) + } + }) return nil } - -var _ = output.Render diff --git a/cmd/sandbox/disk.go b/cmd/sandbox/disk.go index 758ef1c..7320049 100644 --- a/cmd/sandbox/disk.go +++ b/cmd/sandbox/disk.go @@ -143,8 +143,13 @@ func runDiskCreate(c *cli.Context) error { return err } spinner.Success(fmt.Sprintf("Registered disk %s (%s)", d.Name, d.ID)) - pterm.Println(pterm.Gray(" Attach it at create time: createos sandbox create --disk " + d.Name + ":/mnt/data")) - pterm.Println(pterm.Gray(" Or live-attach it later: createos sandbox disk attach " + d.Name + " /mnt/data")) + renderResult(c, "disk_created", map[string]any{ + "id": d.ID, + "name": d.Name, + }, func() { + pterm.Println(pterm.Gray(" Attach it at create time: createos sandbox create --disk " + d.Name + ":/mnt/data")) + pterm.Println(pterm.Gray(" Or live-attach it later: createos sandbox disk attach " + d.Name + " /mnt/data")) + }) return nil } @@ -302,14 +307,22 @@ func runDiskRm(c *cli.Context) error { } } failed := 0 + results := make([]map[string]any, 0, len(refs)) for _, ref := range refs { if err := deleteDiskCascade(c, client, ref); err != nil { pterm.Error.Printfln("%s: %v", ref, err) + results = append(results, map[string]any{"ref": ref, "deleted": false, "error": err.Error()}) failed++ continue } + results = append(results, map[string]any{"ref": ref, "deleted": true}) pterm.Success.Printfln("Deleted disk %s", ref) } + renderResult(c, "disk_deleted", map[string]any{ + "results": results, + "deleted": len(results) - failed, + "failed": failed, + }, func() {}) if failed > 0 { return fmt.Errorf("%d of %d deletes failed", failed, len(refs)) } @@ -484,7 +497,13 @@ func runDiskAttach(c *cli.Context) error { return err } spinner.Success(fmt.Sprintf("Attached %s β†’ %s:%s", diskRef, refLabel(sandboxRef, sandboxID), mountPath)) - pterm.Println(pterm.Gray(" The mount appears inside the sandbox within a few seconds.")) + renderResult(c, "disk_attached", map[string]any{ + "disk": diskRef, + "sandbox_id": sandboxID, + "mount_path": mountPath, + }, func() { + pterm.Println(pterm.Gray(" The mount appears inside the sandbox within a few seconds.")) + }) return nil } @@ -596,7 +615,13 @@ func runDiskDetach(c *cli.Context) error { if err := client.DetachDisk(c.Context, sandboxID, diskRef, mountPath); err != nil { return err } - pterm.Success.Printfln("Detached %s from %s at %s", diskRef, refLabel(sandboxRef, sandboxID), mountPath) + renderResult(c, "disk_detached", map[string]any{ + "disk": diskRef, + "sandbox_id": sandboxID, + "mount_path": mountPath, + }, func() { + pterm.Success.Printfln("Detached %s from %s at %s", diskRef, refLabel(sandboxRef, sandboxID), mountPath) + }) return nil } diff --git a/cmd/sandbox/edit.go b/cmd/sandbox/edit.go index 4090d09..4b47981 100644 --- a/cmd/sandbox/edit.go +++ b/cmd/sandbox/edit.go @@ -322,15 +322,21 @@ func applyIngressFlag(c *cli.Context, client *api.SandboxClient, label, id, valu if err != nil { return err } - if target { - pterm.Success.Printfln("Public URL is on for %s", refLabel(label, id)) - if updated.IngressURLTemplate != "" { - fmt.Printf(" %s\n", updated.IngressURLTemplate) - pterm.Println(pterm.Gray(" Replace with the port your service is listening on.")) + renderResult(c, "ingress_updated", map[string]any{ + "id": id, + "ingress": target, + "ingress_url": updated.IngressURLTemplate, + }, func() { + if target { + pterm.Success.Printfln("Public URL is on for %s", refLabel(label, id)) + if updated.IngressURLTemplate != "" { + fmt.Printf(" %s\n", updated.IngressURLTemplate) + pterm.Println(pterm.Gray(" Replace with the port your service is listening on.")) + } + } else { + pterm.Success.Printfln("Public URL is off for %s", refLabel(label, id)) } - } else { - pterm.Success.Printfln("Public URL is off for %s", refLabel(label, id)) - } + }) return nil } @@ -347,7 +353,13 @@ func applyAddSSHKeys(c *cli.Context, client *api.SandboxClient, label, id string if err != nil { return err } - pterm.Success.Printfln("Added %d SSH key(s) to %s β€” total now %d", len(keys), refLabel(label, id), count) + renderResult(c, "ssh_keys_added", map[string]any{ + "id": id, + "keys_added": len(keys), + "total_keys": count, + }, func() { + pterm.Success.Printfln("Added %d SSH key(s) to %s β€” total now %d", len(keys), refLabel(label, id), count) + }) return nil } @@ -368,12 +380,17 @@ func applyAutoPauseFlag(c *cli.Context, client *api.SandboxClient, label, id, va if err != nil { return err } - if updated.AutoPauseAfterSeconds != nil { - d := time.Duration(*updated.AutoPauseAfterSeconds) * time.Second - pterm.Success.Printfln("Auto-pause set to %s for %s", formatDuration(d), refLabel(label, id)) - } else { - pterm.Success.Printfln("Auto-pause turned off for %s", refLabel(label, id)) - } + renderResult(c, "auto_pause_updated", map[string]any{ + "id": id, + "auto_pause_after_seconds": updated.AutoPauseAfterSeconds, + }, func() { + if updated.AutoPauseAfterSeconds != nil { + d := time.Duration(*updated.AutoPauseAfterSeconds) * time.Second + pterm.Success.Printfln("Auto-pause set to %s for %s", formatDuration(d), refLabel(label, id)) + } else { + pterm.Success.Printfln("Auto-pause turned off for %s", refLabel(label, id)) + } + }) return nil } diff --git a/cmd/sandbox/exec.go b/cmd/sandbox/exec.go index 6adfbb9..bc8fc7e 100644 --- a/cmd/sandbox/exec.go +++ b/cmd/sandbox/exec.go @@ -10,6 +10,8 @@ import ( "github.com/urfave/cli/v2" "github.com/NodeOps-app/createos-cli/internal/api" + "github.com/NodeOps-app/createos-cli/internal/cliargs" + "github.com/NodeOps-app/createos-cli/internal/output" "github.com/NodeOps-app/createos-cli/internal/terminal" ) @@ -23,11 +25,17 @@ func newExecCommand() *cli.Command { arrives all at once when the command finishes. Pass --stream to see stdout/stderr live as it happens. +Anything piped in is forwarded to the command's standard input, so +'cat script.sh | createos sandbox exec my-box -- bash' works. Use +--stdin to read from a file instead. + Examples: createos sandbox exec my-box -- uname -a createos sandbox exec my-box -- python3 -c 'print("hi")' createos sandbox exec my-box --stream -- pip install requests createos sandbox exec my-box -- bash -c "echo $USER && date" + echo "hello" | createos sandbox exec my-box -- cat + createos sandbox exec my-box --stdin ./setup.sh -- bash The command's exit code is preserved β€” if the program inside the sandbox exits with 1, this CLI also exits with 1.`, @@ -37,6 +45,10 @@ sandbox exits with 1, this CLI also exits with 1.`, Aliases: []string{"s"}, Usage: "Show output live as the command runs", }, + &cli.StringFlag{ + Name: "stdin", + Usage: "Read the command's standard input from `FILE` ('-' for piped input)", + }, &cli.StringSliceFlag{ Name: "env", Usage: "Override an environment variable for this exec (repeatable): KEY=VALUE. " + @@ -108,10 +120,15 @@ func runExec(c *cli.Context) error { if err != nil { return err } + stdin, err := readExecStdin(c) + if err != nil { + return err + } req := api.SandboxExecReq{ - Cmd: cmd, - Args: args, - Env: envs, + Cmd: cmd, + Args: args, + Env: envs, + Stdin: stdin, } if c.Bool("stream") { @@ -120,11 +137,52 @@ func runExec(c *cli.Context) error { return runExecBuffered(c, client, id, req) } +// readExecStdin collects the payload for the command's standard input: +// an explicit --stdin FILE, or whatever was piped in. On a TTY with no +// --stdin there is nothing to read, so the command gets empty stdin +// rather than blocking on the keyboard. +func readExecStdin(c *cli.Context) (string, error) { + path := c.String("stdin") + + switch { + case path != "" && path != "-": + data, err := os.ReadFile(path) // #nosec G304 -- the user names the file to send + if err != nil { + return "", fmt.Errorf("could not read %s\n\n Check the path is correct and the file exists", path) + } + return string(data), nil + case path == "-" || terminal.HasPipedStdin(): + data, err := io.ReadAll(os.Stdin) + if err != nil { + return "", fmt.Errorf("could not read the piped input: %w", err) + } + return string(data), nil + } + return "", nil +} + func runExecBuffered(c *cli.Context, client *api.SandboxClient, id string, req api.SandboxExecReq) error { resp, err := client.ExecSandbox(c.Context, id, req) if err != nil { return err } + + // JSON mode keeps stdout a single parseable document: the command's + // own output becomes fields rather than raw bytes on the stream. + if output.IsJSONExplicit(c) { + output.Render(c, map[string]any{ + "sandbox_id": id, + "exit_code": resp.Result.ExitCode, + "stdout": resp.Result.Stdout, + "stderr": resp.Result.Stderr, + "error": resp.Result.Error, + }, func() {}) + if resp.Result.ExitCode != 0 { + os.Exit(resp.Result.ExitCode) + } + return nil + } + if resp.Result.Stdout != "" { fmt.Print(resp.Result.Stdout) if !strings.HasSuffix(resp.Result.Stdout, "\n") { @@ -190,10 +248,13 @@ func parseExecArgs(c *cli.Context) (ref, cmd string, args []string) { return "", "", nil } - // First: did the user write `... exec -- …`? Scan os.Args. + // First: did the user write `... exec -- …`? Scan os.Args, hoisted the + // same way main.go hoists it, so a global flag typed after the + // subcommand ("exec --output json -- ls") doesn't hide the separator. + argv := cliargs.Hoist(os.Args) leadingDoubleDash := false - for i, a := range os.Args { - if a == "exec" && i+1 < len(os.Args) && os.Args[i+1] == "--" { + for i, a := range argv { + if a == "exec" && i+1 < len(argv) && argv[i+1] == "--" { leadingDoubleDash = true break } diff --git a/cmd/sandbox/firewall.go b/cmd/sandbox/firewall.go index d542842..880212a 100644 --- a/cmd/sandbox/firewall.go +++ b/cmd/sandbox/firewall.go @@ -173,7 +173,12 @@ func runFirewallClear(c *cli.Context) error { if _, err := client.SetEgress(c.Context, id, []string{}); err != nil { return err } - pterm.Success.Printfln("Firewall cleared on %s β€” all outbound traffic allowed.", refLabel(ref, id)) + renderResult(c, "firewall_cleared", map[string]any{ + "id": id, + "rules": []string{}, + }, func() { + pterm.Success.Printfln("Firewall cleared on %s β€” all outbound traffic allowed.", refLabel(ref, id)) + }) return nil } @@ -183,10 +188,15 @@ func applyFirewall(c *cli.Context, client *api.SandboxClient, id, ref string, ru if err != nil { return err } - pterm.Success.Printfln("Firewall updated on %s β€” %d rule(s) active.", refLabel(ref, id), len(stored)) - for _, r := range stored { - pterm.Println(pterm.Gray(" β€’ " + r)) - } + renderResult(c, "firewall_updated", map[string]any{ + "id": id, + "rules": stored, + }, func() { + pterm.Success.Printfln("Firewall updated on %s β€” %d rule(s) active.", refLabel(ref, id), len(stored)) + for _, r := range stored { + pterm.Println(pterm.Gray(" β€’ " + r)) + } + }) return nil } diff --git a/cmd/sandbox/fork.go b/cmd/sandbox/fork.go index 458f079..aa6026c 100644 --- a/cmd/sandbox/fork.go +++ b/cmd/sandbox/fork.go @@ -101,16 +101,24 @@ func runForkByID(c *cli.Context, client *api.SandboxClient, ref, srcID string) e return fmt.Errorf("sandbox %s is %s β€” see `createos sandbox get %s` for details", sb.ID, sb.Status, sb.ID) } - name := "" - if sb.Name != nil { - name = *sb.Name - } + name := str(sb.Name) spinner.Success(fmt.Sprintf("Forked into %s", refLabel(name, sb.ID))) - if sb.IP != nil && *sb.IP != "" { - fmt.Printf(" IP: %s\n", *sb.IP) - } - if sb.IngressURLTemplate != "" { - fmt.Printf(" URL: %s\n", sb.IngressURLTemplate) - } + + renderResult(c, "forked", map[string]any{ + "id": sb.ID, + "name": name, + "status": sb.Status, + "ip": str(sb.IP), + "ingress_url": sb.IngressURLTemplate, + "source_id": srcID, + "shell_command": fmt.Sprintf("createos sandbox shell %s", sb.ID), + }, func() { + if sb.IP != nil && *sb.IP != "" { + fmt.Printf(" IP: %s\n", *sb.IP) + } + if sb.IngressURLTemplate != "" { + fmt.Printf(" URL: %s\n", sb.IngressURLTemplate) + } + }) return nil } diff --git a/cmd/sandbox/jsonout.go b/cmd/sandbox/jsonout.go new file mode 100644 index 0000000..1f1bb55 --- /dev/null +++ b/cmd/sandbox/jsonout.go @@ -0,0 +1,35 @@ +package sandbox + +import ( + "maps" + + "github.com/urfave/cli/v2" + + "github.com/NodeOps-app/createos-cli/internal/output" +) + +// renderResult is how every sandbox mutation reports what it did. +// +// In JSON mode it writes one object to stdout; otherwise it runs the +// human renderer untouched. The "action" field names the operation that +// completed, so a caller reading a stream of results can tell a pause from +// a resume without tracking which command it invoked. +// +// Keys are shared with the read commands on purpose β€” a caller can diff the +// object `create` returned against the one `get` returns later. +func renderResult(c *cli.Context, action string, fields map[string]any, human func()) { + obj := make(map[string]any, len(fields)+1) + obj["action"] = action + maps.Copy(obj, fields) + output.Render(c, obj, human) +} + +// str dereferences an optional API string field. The sandbox API returns +// null for anything not yet assigned (a name, an IP on a still-creating +// box), and JSON consumers are better served by "" than by a missing key. +func str(p *string) string { + if p == nil { + return "" + } + return *p +} diff --git a/cmd/sandbox/network.go b/cmd/sandbox/network.go index bf7adc0..ed93e6b 100644 --- a/cmd/sandbox/network.go +++ b/cmd/sandbox/network.go @@ -68,9 +68,14 @@ func runNetworkCreate(c *cli.Context) error { if err != nil { return err } - pterm.Success.Printfln("Created network %s (%s)", n.Name, n.ID) - pterm.Println(pterm.Gray(" Attach at create time: createos sandbox create --network " + n.Name)) - pterm.Println(pterm.Gray(" Or live-attach later: createos sandbox network attach " + n.Name)) + renderResult(c, "network_created", map[string]any{ + "id": n.ID, + "name": n.Name, + }, func() { + pterm.Success.Printfln("Created network %s (%s)", n.Name, n.ID) + pterm.Println(pterm.Gray(" Attach at create time: createos sandbox create --network " + n.Name)) + pterm.Println(pterm.Gray(" Or live-attach later: createos sandbox network attach " + n.Name)) + }) return nil } @@ -240,14 +245,22 @@ func runNetworkRm(c *cli.Context) error { } } failed := 0 + results := make([]map[string]any, 0, len(refs)) for _, ref := range refs { if err := deleteNetworkCascade(c, client, ref); err != nil { pterm.Error.Printfln("%s: %v", ref, err) + results = append(results, map[string]any{"ref": ref, "deleted": false, "error": err.Error()}) failed++ continue } + results = append(results, map[string]any{"ref": ref, "deleted": true}) pterm.Success.Printfln("Deleted network %s", ref) } + renderResult(c, "network_deleted", map[string]any{ + "results": results, + "deleted": len(results) - failed, + "failed": failed, + }, func() {}) if failed > 0 { return fmt.Errorf("%d of %d deletes failed", failed, len(refs)) } @@ -372,8 +385,14 @@ func runNetworkAttach(c *cli.Context) error { if err := client.AttachDeviceToNetwork(c.Context, ref, netRef); err != nil { return err } - pterm.Success.Printfln("Attached device %s β†’ network %s", ref, netRef) - pterm.Println(pterm.Gray(" The device can now reach VMs on this network once it brings up the tunnel.")) + renderResult(c, "network_attached", map[string]any{ + "network": netRef, + "endpoint": ref, + "type": "device", + }, func() { + pterm.Success.Printfln("Attached device %s β†’ network %s", ref, netRef) + pterm.Println(pterm.Gray(" The device can now reach VMs on this network once it brings up the tunnel.")) + }) return nil } sandboxID, err := resolveSandboxRef(c.Context, client, ref) @@ -383,8 +402,14 @@ func runNetworkAttach(c *cli.Context) error { if err := client.AttachNetwork(c.Context, sandboxID, netRef); err != nil { return err } - pterm.Success.Printfln("Attached %s β†’ network %s", refLabel(ref, sandboxID), netRef) - pterm.Println(pterm.Gray(" Other sandboxes on this network can now reach this one by name.")) + renderResult(c, "network_attached", map[string]any{ + "network": netRef, + "endpoint": sandboxID, + "type": "sandbox", + }, func() { + pterm.Success.Printfln("Attached %s β†’ network %s", refLabel(ref, sandboxID), netRef) + pterm.Println(pterm.Gray(" Other sandboxes on this network can now reach this one by name.")) + }) return nil } @@ -474,7 +499,13 @@ func runNetworkDetach(c *cli.Context) error { if err := client.DetachDeviceFromNetwork(c.Context, ref, netRef); err != nil { return err } - pterm.Success.Printfln("Detached device %s from network %s", ref, netRef) + renderResult(c, "network_detached", map[string]any{ + "network": netRef, + "endpoint": ref, + "type": "device", + }, func() { + pterm.Success.Printfln("Detached device %s from network %s", ref, netRef) + }) return nil } sandboxID, err := resolveSandboxRef(c.Context, client, ref) @@ -484,7 +515,13 @@ func runNetworkDetach(c *cli.Context) error { if err := client.DetachNetwork(c.Context, sandboxID, netRef); err != nil { return err } - pterm.Success.Printfln("Detached %s from network %s", refLabel(ref, sandboxID), netRef) + renderResult(c, "network_detached", map[string]any{ + "network": netRef, + "endpoint": sandboxID, + "type": "sandbox", + }, func() { + pterm.Success.Printfln("Detached %s from network %s", refLabel(ref, sandboxID), netRef) + }) return nil } diff --git a/cmd/sandbox/pause.go b/cmd/sandbox/pause.go index 3c8f120..0a0e532 100644 --- a/cmd/sandbox/pause.go +++ b/cmd/sandbox/pause.go @@ -67,5 +67,10 @@ func runPauseByID(c *cli.Context, client *api.SandboxClient, ref, id string) err return fmt.Errorf("sandbox %s is %s β€” see `createos sandbox get %s` for details", refLabel(ref, id), sb.Status, id) } spinner.Success(fmt.Sprintf("Paused %s", refLabel(ref, id))) + renderResult(c, "paused", map[string]any{ + "id": sb.ID, + "name": str(sb.Name), + "status": sb.Status, + }, func() {}) return nil } diff --git a/cmd/sandbox/pull.go b/cmd/sandbox/pull.go index 28ca621..910b923 100644 --- a/cmd/sandbox/pull.go +++ b/cmd/sandbox/pull.go @@ -69,5 +69,11 @@ func runPull(c *cli.Context) error { return err } spinner.Success(fmt.Sprintf("Downloaded %s:%s β†’ %s (%s)", refLabel(ref, id), remote, local, humanBytes(n))) + renderResult(c, "pulled", map[string]any{ + "id": id, + "remote_path": remote, + "local_path": local, + "bytes": n, + }, func() {}) return nil } diff --git a/cmd/sandbox/push.go b/cmd/sandbox/push.go index a8a6d1b..8be1a57 100644 --- a/cmd/sandbox/push.go +++ b/cmd/sandbox/push.go @@ -117,6 +117,12 @@ func runPush(c *cli.Context) error { pterm.Println(pterm.Gray(fmt.Sprintf(" %s in %s (%s)", humanBytes(sent), formatElapsed(elapsed), throughput(sent, elapsed)))) } + renderResult(c, "pushed", map[string]any{ + "id": id, + "remote_path": remote, + "bytes": sent, + "elapsed_ms": elapsed.Milliseconds(), + }, func() {}) return nil } diff --git a/cmd/sandbox/resume.go b/cmd/sandbox/resume.go index b9cc866..d384827 100644 --- a/cmd/sandbox/resume.go +++ b/cmd/sandbox/resume.go @@ -66,5 +66,13 @@ func runResumeByID(c *cli.Context, client *api.SandboxClient, ref, id string) er return fmt.Errorf("sandbox %s is %s β€” see `createos sandbox get %s` for details", refLabel(ref, id), sb.Status, id) } spinner.Success(fmt.Sprintf("Resumed %s", refLabel(ref, id))) + renderResult(c, "resumed", map[string]any{ + "id": sb.ID, + "name": str(sb.Name), + "status": sb.Status, + "ip": str(sb.IP), + "ingress_url": sb.IngressURLTemplate, + "shell_command": fmt.Sprintf("createos sandbox shell %s", sb.ID), + }, func() {}) return nil } diff --git a/cmd/sandbox/rm.go b/cmd/sandbox/rm.go index 50facbf..1f733ad 100644 --- a/cmd/sandbox/rm.go +++ b/cmd/sandbox/rm.go @@ -105,18 +105,24 @@ func runRm(c *cli.Context) error { // are reported per-ref so a typo in one name doesn't kill the rest // of the batch. failed := 0 + // One entry per ref, deleted or not, so a batch caller can tell which + // of the refs it passed actually went away. + results := make([]map[string]any, 0, len(ids)) for _, ref := range ids { id, err := resolveSandboxRef(c.Context, client, ref) if err != nil { pterm.Error.Printfln("%s: %v", ref, err) + results = append(results, map[string]any{"ref": ref, "deleted": false, "error": err.Error()}) failed++ continue } if err := client.DestroySandbox(c.Context, id); err != nil { pterm.Error.Printfln("%s: %v", ref, err) + results = append(results, map[string]any{"ref": ref, "id": id, "deleted": false, "error": err.Error()}) failed++ continue } + results = append(results, map[string]any{"ref": ref, "id": id, "deleted": true}) // Echo the friendly ref the user typed; if it was already an // id this reads the same, if it was a name they see what was // actually removed. @@ -126,6 +132,13 @@ func runRm(c *cli.Context) error { pterm.Success.Printfln("Deleted %s", id) } } + + renderResult(c, "deleted", map[string]any{ + "results": results, + "deleted": len(results) - failed, + "failed": failed, + }, func() {}) + if failed > 0 { // Non-zero exit so scripts can tell something went wrong. os.Exit(1) diff --git a/cmd/sandbox/template.go b/cmd/sandbox/template.go index 8ba246c..18d1a0d 100644 --- a/cmd/sandbox/template.go +++ b/cmd/sandbox/template.go @@ -79,7 +79,13 @@ func runTemplateSubmit(c *cli.Context) error { if err != nil { return err } - pterm.Success.Printfln("Submitted template %s (status: %s)", view.Name, view.Status) + renderResult(c, "template_submitted", map[string]any{ + "id": view.ID, + "name": view.Name, + "status": view.Status, + }, func() { + pterm.Success.Printfln("Submitted template %s (status: %s)", view.Name, view.Status) + }) if !follow { pterm.Println(pterm.Gray(fmt.Sprintf(" Watch progress with: createos sandbox template logs %s --follow", view.Name))) return nil @@ -378,14 +384,22 @@ func runTemplateRm(c *cli.Context) error { } } failed := 0 + results := make([]map[string]any, 0, len(refs)) for _, ref := range refs { if err := client.DeleteTemplate(c.Context, ref); err != nil { pterm.Error.Printfln("%s: %v", ref, err) + results = append(results, map[string]any{"ref": ref, "deleted": false, "error": err.Error()}) failed++ continue } + results = append(results, map[string]any{"ref": ref, "deleted": true}) pterm.Success.Printfln("Deleted template %s", ref) } + renderResult(c, "template_deleted", map[string]any{ + "results": results, + "deleted": len(results) - failed, + "failed": failed, + }, func() {}) if failed > 0 { return fmt.Errorf("%d of %d deletes failed", failed, len(refs)) } diff --git a/cmd/sandbox/tunnel.go b/cmd/sandbox/tunnel.go index 230042a..d7cd8fb 100644 --- a/cmd/sandbox/tunnel.go +++ b/cmd/sandbox/tunnel.go @@ -169,8 +169,16 @@ func runTunnel(c *cli.Context) error { return err } - pterm.Success.Printfln("Forwarding %s β†’ %s:%d", listenAddr, refLabel(ref, id), remote) - pterm.Println(pterm.Gray(" Press Ctrl+C to stop.")) + // Emitted before the blocking accept loop: a caller in JSON mode needs + // the bound address now, not when the tunnel is torn down. + renderResult(c, "tunnel_started", map[string]any{ + "id": id, + "listen_addr": listenAddr, + "remote_port": remote, + }, func() { + pterm.Success.Printfln("Forwarding %s β†’ %s:%d", listenAddr, refLabel(ref, id), remote) + pterm.Println(pterm.Gray(" Press Ctrl+C to stop.")) + }) // Trap Ctrl+C so we can close cleanly and not leave half-open conns. sigCh := make(chan os.Signal, 1) diff --git a/cmd/sandbox/vpn.go b/cmd/sandbox/vpn.go index 690652d..937497d 100644 --- a/cmd/sandbox/vpn.go +++ b/cmd/sandbox/vpn.go @@ -183,10 +183,19 @@ func runVPNUp(c *cli.Context) error { } ifaceName := strings.TrimSuffix(filepath.Base(confPath), ".conf") - pterm.Success.Printfln("VPN connected as %s (%s).", st.Name, st.ClientIP) - pterm.Println(pterm.Gray(fmt.Sprintf(" device: %s", st.Name))) - pterm.Println(pterm.Gray(fmt.Sprintf(" iface: %s", ifaceName))) - pterm.Println(pterm.Gray("Press Ctrl-C to disconnect.")) + // Emitted before the block on Ctrl-C, for the same reason as the + // tunnel: the caller needs the interface and address up front. + renderResult(c, "vpn_connected", map[string]any{ + "device_id": st.DeviceID, + "device": st.Name, + "client_ip": st.ClientIP, + "interface": ifaceName, + }, func() { + pterm.Success.Printfln("VPN connected as %s (%s).", st.Name, st.ClientIP) + pterm.Println(pterm.Gray(fmt.Sprintf(" device: %s", st.Name))) + pterm.Println(pterm.Gray(fmt.Sprintf(" iface: %s", ifaceName))) + pterm.Println(pterm.Gray("Press Ctrl-C to disconnect.")) + }) // Block until Ctrl-C / SIGTERM (user disconnect) or until the // renewal goroutine signals that the server-side session is gone. diff --git a/docs/decisions.md b/docs/decisions.md new file mode 100644 index 0000000..e1b2f03 --- /dev/null +++ b/docs/decisions.md @@ -0,0 +1,121 @@ +# Decisions + +Running log of design decisions in this repo, with the options that were on +the table and why one won. Written for whoever β€” human or agent β€” picks this +up next. + +## 2026-08-11 β€” Machine-readable output for CI and agents + +An external review produced ~40 UX/DX/CI/agent findings. We shipped the five +that were blocking automation, plus the scope the user widened. + +### D1. Errors go to stderr; JSON mode gets an error envelope + +**Problem.** `main.go` printed errors with `pterm.Error`, whose default writer +is stdout. `createos … > out.json` captured the error text as data, and +`2>/dev/null` hid nothing. + +Options considered: + +- **(a) Print errors to stderr always.** Simple, conventional. In JSON mode a + consumer reading only stdout gets an empty stream on failure and has to + infer the error from the exit code. +- **(b) Error envelope on stdout in JSON mode, stderr otherwise.** ← chosen. + A JSON consumer reading one stream always parses a valid document, success + or failure. Human users get the Unix convention. +- (c) Envelope on stderr in JSON mode. Rejected: forces every consumer to + merge two streams to find out what happened. + +The envelope is `{"error":{"code","message"}}`. `code` is a stable slug +derived from the HTTP status (`api.APIError.Code()`), so callers branch on +the failure class instead of matching on message text. + +### D2. `--output json` implies "stdout is only JSON" + +`pterm.SetDefaultOutput(os.Stderr)` is set in the root `Before` hook whenever +JSON mode is active. Every pterm print in the codebase β€” including commands +not yet converted β€” lands on stderr, so no narration can corrupt the +document on stdout. Spinners already wrote to stderr (pterm's default). + +### D3. `sandbox exec` uses `IsJSONExplicit`, not `IsJSON` + +`output.DetectFormat` auto-selects JSON when stdout is not a TTY. For most +commands that is right. For `exec` it is actively wrong: stdout **is** the +payload, so `createos sandbox exec box -- cat data.csv > out` would have +written a JSON envelope instead of the file the caller expected. + +So `exec` only wraps its result when the user typed `--output json`. Every +other command keeps the auto-detect behaviour. `output.IsJSONExplicit` marks +this distinction; use it for any future command whose stdout is a payload +rather than a report. + +### D4. Global flags are hoisted in argv, not mirrored onto commands + +**Problem.** urfave/cli v2 only parses app-level flags before the first +subcommand token, so `createos sandbox create --output json` died with +"flag provided but not defined: -output". + +Options considered: + +- **(a) Declare hidden copies of each global flag on every top-level command.** + Rejected: the flags would *parse* but their values would never be read β€” + the `App.Before` hook that consumes them runs before subcommand parsing, so + `sandbox create --api-key X` would silently ignore the key. Silently-wrong + is worse than the error it replaces. +- **(b) Re-detect the format in a per-command `Before` hook.** Fixes + `--output` only; leaves `--api-key`, `--api-url`, `--debug` broken. +- **(c) Rewrite argv before `app.Run`.** ← chosen. `internal/cliargs.Hoist` + moves known global flags (and their values) in front of the subcommand. + One change, every flag and every command fixed, no per-command upkeep. + +Tokens after a bare `--` are never touched, so `exec box -- ./ci.sh --debug` +keeps passing `--debug` to the user's script. `cmd/sandbox/exec.go`'s own +argv scan runs `Hoist` first, so it sees the same normalized line. + +Verified safe: no subcommand declares any of the hoisted names or uses `-o` +/ `-d` as an alias. + +### D5. Which sandbox commands emit JSON + +The user's call: **all** sandbox subcommands, not just `create`/`fork`/`edit`. + +Every command that creates, changes, or deletes now calls `renderResult` +(`cmd/sandbox/jsonout.go`) with an `action` field plus the ids needed to +chain the next call. Batch deletes (`rm`, `disk rm`, `network rm`, +`template rm`) return a per-ref `results` array so a caller can tell which +references succeeded β€” a single exit code cannot express a partial batch. + +Left as human-readable, deliberately: + +| Command | Why | +|---|---| +| `shell`, `editor`, `sync` | Interactive sessions. No single result to report. | +| `exec --stream` | The stream *is* the output; framing it would break live piping. | +| `template logs` | Same β€” a log stream. | +| `edit` interactive menu | TTY-only path; JSON mode never reaches it. | + +`tunnel` and `vpn up` block until Ctrl-C, so they emit their JSON result +when the connection comes up rather than on exit β€” a caller needs the bound +address while the tunnel is alive, not after it dies. + +### D6. `exec` stdin + +`api.SandboxExecReq.Stdin` existed and was never populated. Now: piped stdin +is forwarded automatically, `--stdin FILE` reads from a file, `--stdin -` +forces reading the pipe. On a TTY with no `--stdin` nothing is read, so the +command does not block waiting on a keyboard. + +Known limitation: consuming piped stdin means it is not available for +interactive prompts. In practice a caller that pipes data also passes the +sandbox ref and command explicitly, so no prompt is reached. + +### Not done (and why) + +- **`--wait` on create/get** β€” real CI value, but a caller can poll `get`. + Next tier, not blocking. +- **Cancel exit code 130** β€” correct convention; needs the ~52 scattered + cancel strings unified in the same pass. Deferred as one coherent change. +- **`--quiet` on mutations** β€” superseded for machine callers by JSON output. + Still worth it for shell users. +- **Shape price hints in the picker** β€” `api.Shape` carries no pricing. That + is an `fc` change, not a CLI change. diff --git a/internal/api/types.go b/internal/api/types.go index 9c43d9b..6f6ab7c 100644 --- a/internal/api/types.go +++ b/internal/api/types.go @@ -44,6 +44,30 @@ func (e *APIError) Hint() string { } } +// Code returns a stable, machine-readable slug for the status code. It is +// the "code" field of the JSON error envelope, so callers (CI, agents) can +// branch on the failure class without string-matching Message. +func (e *APIError) Code() string { + switch e.StatusCode { + case http.StatusBadRequest: + return "bad_request" + case http.StatusUnauthorized: + return "unauthorized" + case http.StatusForbidden: + return "forbidden" + case http.StatusNotFound: + return "not_found" + case http.StatusConflict: + return "conflict" + case http.StatusTooManyRequests: + return "rate_limited" + } + if e.StatusCode >= 500 { + return "server_error" + } + return "api_error" +} + // ParseAPIError extracts a human-readable message from an API error response body. // // JSend "fail" bodies can shape `data` three different ways: diff --git a/internal/cliargs/hoist.go b/internal/cliargs/hoist.go new file mode 100644 index 0000000..8d0da6e --- /dev/null +++ b/internal/cliargs/hoist.go @@ -0,0 +1,80 @@ +// Package cliargs normalizes the raw process argv before urfave/cli sees it. +package cliargs + +import "strings" + +// globalStringFlags are the app-level flags that take a value. Their value +// may arrive as either "--api-url X" (two tokens) or "--api-url=X" (one). +var globalStringFlags = map[string]bool{ + "--output": true, + "-o": true, + "--api-url": true, + "--api-key": true, + "--sandbox-api-url": true, + "--sandbox-gateway": true, +} + +// globalBoolFlags are the app-level flags that take no value. +var globalBoolFlags = map[string]bool{ + "--debug": true, + "-d": true, +} + +// Hoist moves app-level flags in front of the subcommand so that +// "createos sandbox create --output json" behaves like +// "createos --output json sandbox create". +// +// urfave/cli v2 only parses app-level flags before the first subcommand +// token; anything after it is handed to the subcommand's own flag set and +// fails with "flag provided but not defined". Rewriting argv is the one +// change that fixes every command at once β€” mirroring hidden flags onto each +// command would let them parse but leave their values unread, because the +// App.Before hook that consumes them runs before subcommand parsing. +// +// Everything after a bare "--" is left untouched: those tokens belong to the +// user's own command line (e.g. "sandbox exec box -- ./ci.sh --debug"). +func Hoist(args []string) []string { + if len(args) < 2 { + return args + } + + hoisted := []string{} + rest := []string{} + + for i := 1; i < len(args); i++ { + arg := args[i] + + if arg == "--" { + rest = append(rest, args[i:]...) + break + } + + name, _, hasInlineValue := strings.Cut(arg, "=") + + switch { + case globalBoolFlags[arg]: + hoisted = append(hoisted, arg) + case globalStringFlags[name] && hasInlineValue: + hoisted = append(hoisted, arg) + case globalStringFlags[arg]: + // Value is the next token; take it along, if present. + hoisted = append(hoisted, arg) + if i+1 < len(args) { + i++ + hoisted = append(hoisted, args[i]) + } + default: + rest = append(rest, arg) + } + } + + if len(hoisted) == 0 { + return args + } + + out := make([]string, 0, len(args)) + out = append(out, args[0]) + out = append(out, hoisted...) + out = append(out, rest...) + return out +} diff --git a/internal/cliargs/hoist_test.go b/internal/cliargs/hoist_test.go new file mode 100644 index 0000000..e8cb7a3 --- /dev/null +++ b/internal/cliargs/hoist_test.go @@ -0,0 +1,74 @@ +package cliargs + +import ( + "reflect" + "testing" +) + +func TestHoistGlobalFlags(t *testing.T) { + tests := []struct { + name string + in []string + want []string + }{ + { + name: "flag after subcommand is hoisted", + in: []string{"createos", "sandbox", "create", "--output", "json"}, + want: []string{"createos", "--output", "json", "sandbox", "create"}, + }, + { + name: "inline value is hoisted as one token", + in: []string{"createos", "sandbox", "list", "--output=json"}, + want: []string{"createos", "--output=json", "sandbox", "list"}, + }, + { + name: "bool flag and alias", + in: []string{"createos", "sandbox", "get", "box", "-d", "-o", "json"}, + want: []string{"createos", "-d", "-o", "json", "sandbox", "get", "box"}, + }, + { + name: "already-global order is preserved", + in: []string{"createos", "--output", "json", "sandbox", "list"}, + want: []string{"createos", "--output", "json", "sandbox", "list"}, + }, + { + name: "nothing to hoist returns input untouched", + in: []string{"createos", "sandbox", "list"}, + want: []string{"createos", "sandbox", "list"}, + }, + { + name: "tokens after -- are never hoisted", + in: []string{"createos", "sandbox", "exec", "box", "--", "./ci.sh", "--debug", "--output", "json"}, + want: []string{"createos", "sandbox", "exec", "box", "--", "./ci.sh", "--debug", "--output", "json"}, + }, + { + name: "global before -- is hoisted, passthrough after is not", + in: []string{"createos", "sandbox", "exec", "box", "--output", "json", "--", "env", "-d"}, + want: []string{"createos", "--output", "json", "sandbox", "exec", "box", "--", "env", "-d"}, + }, + { + name: "subcommand flags keep their own values", + in: []string{"createos", "sandbox", "sync", "box", "--exclude", "*.log", "--output", "json"}, + want: []string{"createos", "--output", "json", "sandbox", "sync", "box", "--exclude", "*.log"}, + }, + { + name: "trailing value-less string flag does not panic", + in: []string{"createos", "sandbox", "list", "--output"}, + want: []string{"createos", "--output", "sandbox", "list"}, + }, + { + name: "bare program name", + in: []string{"createos"}, + want: []string{"createos"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := Hoist(tt.in) + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("Hoist(%q)\n got: %q\nwant: %q", tt.in, got, tt.want) + } + }) + } +} diff --git a/internal/output/render.go b/internal/output/render.go index 4c7316f..8cf0b5c 100644 --- a/internal/output/render.go +++ b/internal/output/render.go @@ -14,9 +14,33 @@ import ( // FormatKey is the metadata key for the output format. const FormatKey = "output_format" +// FormatExplicitKey records whether the format came from --output rather +// than from TTY detection. +const FormatExplicitKey = "output_format_explicit" + +// IsJSONExplicit reports whether the user actually asked for JSON with +// --output json. Commands whose stdout IS the payload β€” `sandbox exec` +// forwarding a program's own output β€” must use this instead of IsJSON, or +// a plain `… exec box -- cat data.csv > out` would silently write a JSON +// envelope instead of the file the caller expected. +func IsJSONExplicit(c *cli.Context) bool { + explicit, ok := c.App.Metadata[FormatExplicitKey].(bool) + return ok && explicit && IsJSON(c) +} + // IsJSON returns true if the output format is JSON. func IsJSON(c *cli.Context) bool { - if f, ok := c.App.Metadata[FormatKey].(string); ok { + return AppIsJSON(c.App) +} + +// AppIsJSON reports whether the app is in JSON mode. It reads the same +// metadata IsJSON does, but works from the *cli.App alone β€” main.go handles +// errors after Run returns, where no *cli.Context is available. +func AppIsJSON(app *cli.App) bool { + if app == nil { + return false + } + if f, ok := app.Metadata[FormatKey].(string); ok { return f == "json" } return false @@ -37,7 +61,15 @@ func Render(c *cli.Context, data any, tableRenderer func()) { // RenderError outputs an error as JSON if --output json is set, otherwise returns false. func RenderError(c *cli.Context, code string, message string) bool { - if !IsJSON(c) { + return AppRenderError(c.App, code, message) +} + +// AppRenderError is RenderError for callers that only hold the *cli.App. +// The envelope goes to stdout so a JSON consumer reading one stream still +// gets valid JSON on failure; the human-readable path in main.go writes to +// stderr instead. +func AppRenderError(app *cli.App, code string, message string) bool { + if !AppIsJSON(app) { return false } enc := json.NewEncoder(os.Stdout) diff --git a/internal/terminal/tty.go b/internal/terminal/tty.go index c6abafc..a400d72 100644 --- a/internal/terminal/tty.go +++ b/internal/terminal/tty.go @@ -12,3 +12,11 @@ import ( func IsInteractive() bool { return term.IsTerminal(int(os.Stdout.Fd())) // #nosec G115 -- uintptr->int safe on all supported platforms for fd values } + +// HasPipedStdin reports whether stdin is a pipe or a file rather than a +// keyboard, i.e. `echo hi | createos …` or `createos … < file`. Checked +// separately from IsInteractive because the two streams are redirected +// independently β€” a human on a TTY can still pipe data in. +func HasPipedStdin() bool { + return !term.IsTerminal(int(os.Stdin.Fd())) // #nosec G115 -- uintptr->int safe on all supported platforms for fd values +} diff --git a/main.go b/main.go index bf8bc16..8903133 100644 --- a/main.go +++ b/main.go @@ -10,6 +10,8 @@ import ( "github.com/NodeOps-app/createos-cli/cmd/root" "github.com/NodeOps-app/createos-cli/internal/api" + "github.com/NodeOps-app/createos-cli/internal/cliargs" + "github.com/NodeOps-app/createos-cli/internal/output" ) func main() { @@ -20,12 +22,19 @@ func main() { app := root.NewApp() - if err := app.Run(os.Args); err != nil { + if err := app.Run(cliargs.Hoist(os.Args)); err != nil { + code, message := "error", err.Error() var apiErr *api.APIError if errors.As(err, &apiErr) { - pterm.Error.Println(apiErr.Message) - } else { - pterm.Error.Println(err.Error()) + code, message = apiErr.Code(), apiErr.Message + } + + // JSON mode emits a machine-readable envelope on stdout so a + // consumer reading a single stream still parses valid JSON. + // Otherwise the human-readable error goes to stderr, keeping + // stdout clean for data in pipes and CI logs. + if !output.AppRenderError(app, code, message) { + pterm.Error.WithWriter(os.Stderr).Println(message) } os.Exit(1) } From c012afb72fc96d0a91cfa8b19e34f783b5784a2d Mon Sep 17 00:00:00 2001 From: pratikbin <68642400+pratikbin@users.noreply.github.com> Date: Tue, 11 Aug 2026 23:55:16 +0530 Subject: [PATCH 2/3] fix(sandbox): recover exec's own flags after the ref, restore not-found error taxonomy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by fanning out AI-agent probes against a compiled binary on a real account, driving the CLI exactly as a scripting agent would. exec's own flags (--stdin, --env, --stream) silently no-op whenever they're placed after the sandbox ref β€” `exec --stdin file -- cmd` sends empty stdin with zero error, because urfave/cli v2 stops flag parsing at the first positional and exec had no re-scanner for its own flags (only the ref/cmd split around `--` was recovered). This is the exact ordering shown in the command's own --help examples, so the documented usage didn't work as documented. parseExecFlags recovers --stdin/--env/--stream from the raw tokens between the ref and `--`, mirroring parseSyncArgs's established pattern for the same class of bug. Separately, resolveSandboxRef's "no sandbox named X" error was a bare fmt.Errorf, not an *api.APIError, so it carried no stable error code and got flattened by api.UserMessageVerbose's raw-error fallback into a useless "something went wrong β€” please try again or contact support" in batch results (rm/disk/network/template), and defaulted to a generic "error" code instead of "not_found" everywhere else. Shaping it as an *api.APIError with a 404 status fixes both: batch results keep the real message, and every command that resolves a sandbox by name now reports "not_found" with the matching hint. --- cmd/sandbox/exec.go | 73 ++++++++++++++++++++++++++++++++++++++---- cmd/sandbox/resolve.go | 13 +++++++- 2 files changed, 79 insertions(+), 7 deletions(-) diff --git a/cmd/sandbox/exec.go b/cmd/sandbox/exec.go index bc8fc7e..78ea5d7 100644 --- a/cmd/sandbox/exec.go +++ b/cmd/sandbox/exec.go @@ -116,11 +116,13 @@ func runExec(c *cli.Context) error { } } - envs, err := parseEnvFlags(c.StringSlice("env")) + stdinPath, envFlags, stream := parseExecFlags(c) + + envs, err := parseEnvFlags(envFlags) if err != nil { return err } - stdin, err := readExecStdin(c) + stdin, err := readExecStdin(stdinPath) if err != nil { return err } @@ -131,19 +133,78 @@ func runExec(c *cli.Context) error { Stdin: stdin, } - if c.Bool("stream") { + if stream { return runExecStream(c, client, id, req) } return runExecBuffered(c, client, id, req) } +// parseExecFlags recovers exec's own flags (--stdin, --env, --stream) from +// the raw tokens between the sandbox ref and the `--` command delimiter. +// urfave/cli v2 stops flag parsing at the first positional (the ref), so +// `exec --stdin file -- cmd` never reaches c.String("stdin") β€” the +// CLI's own --help examples show exactly that ordering. Same class of bug, +// same fix shape, as parseSyncArgs. Seeds from whatever urfave DID parse +// (covers flags placed before the ref) and overrides with anything found in +// the ref..`--` window, so either ordering works. +func parseExecFlags(c *cli.Context) (stdin string, envs []string, stream bool) { + stdin = c.String("stdin") + envs = append([]string{}, c.StringSlice("env")...) + stream = c.Bool("stream") + + all := c.Args().Slice() + sep := -1 + for i, a := range all { + if a == "--" { + sep = i + break + } + } + if sep <= 1 { + // No ref..`--` window to scan (no ref, or `--` is the first token). + return stdin, envs, stream + } + + own := all[1:sep] + for i := 0; i < len(own); i++ { + a := strings.TrimSpace(own[i]) + if !strings.HasPrefix(a, "-") { + continue + } + raw := strings.TrimLeft(a, "-") + key, inline, hasInline := raw, "", false + if eq := strings.IndexByte(raw, '='); eq >= 0 { + key, inline, hasInline = raw[:eq], raw[eq+1:], true + } + switch key { + case "stream", "s": + stream = true + case "stdin": + val := inline + if !hasInline && i+1 < len(own) { + val = own[i+1] + i++ + } + stdin = strings.TrimSpace(val) + case "env": + val := inline + if !hasInline && i+1 < len(own) { + val = own[i+1] + i++ + } + if val != "" { + envs = append(envs, val) + } + } + } + return stdin, envs, stream +} + // readExecStdin collects the payload for the command's standard input: // an explicit --stdin FILE, or whatever was piped in. On a TTY with no // --stdin there is nothing to read, so the command gets empty stdin // rather than blocking on the keyboard. -func readExecStdin(c *cli.Context) (string, error) { - path := c.String("stdin") - +func readExecStdin(path string) (string, error) { switch { case path != "" && path != "-": data, err := os.ReadFile(path) // #nosec G304 -- the user names the file to send diff --git a/cmd/sandbox/resolve.go b/cmd/sandbox/resolve.go index 355424c..cb8a20e 100644 --- a/cmd/sandbox/resolve.go +++ b/cmd/sandbox/resolve.go @@ -3,6 +3,7 @@ package sandbox import ( "context" "fmt" + "net/http" "sort" "strings" @@ -89,7 +90,17 @@ func resolveSandboxRef(ctx context.Context, client *api.SandboxClient, ref strin } } if len(matches) == 0 { - return "", fmt.Errorf("no sandbox named %q\n\n To see your sandboxes, run:\n createos sandbox list", ref) + // Shaped as an *api.APIError, not a bare fmt.Errorf, even though + // this never touched the network: it's the only client-side "not + // found" in the whole CLI, and without this every consumer of the + // error β€” main.go's JSON envelope code/hint, batch-loop results in + // rm/disk/network/template β€” silently lost both the "not_found" + // code and, through api.UserMessageVerbose's raw-error fallback, + // the message itself down to a generic "something went wrong". + return "", &api.APIError{ + StatusCode: http.StatusNotFound, + Message: fmt.Sprintf("no sandbox named %q\n\n To see your sandboxes, run:\n createos sandbox list", ref), + } } // Most-recent wins. Stable sort so deterministic when timestamps tie. sort.SliceStable(matches, func(i, j int) bool { From 18cc82fa82358cc3f43c00058f80d85af732c766 Mon Sep 17 00:00:00 2001 From: pratikbin <68642400+pratikbin@users.noreply.github.com> Date: Tue, 11 Aug 2026 23:56:20 +0530 Subject: [PATCH 3/3] docs: record the agent-fleet test round and the two bugs it found --- docs/decisions.md | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/docs/decisions.md b/docs/decisions.md index f2029e0..026411a 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -150,3 +150,49 @@ Resolved as follows: Still worth it for shell users. - **Shape price hints in the picker** β€” `api.Shape` carries no pricing. That is an `fc` change, not a CLI change. + +## 2026-08-11 β€” Six-agent fleet test against a compiled binary, real account + +Before treating the JSON-output work as done, six agents drove a locally +compiled binary against a real authenticated account in parallel β€” each one +briefed only on the feature surface, not told what to expect β€” covering the +full createβ†’execβ†’rm lifecycle, exec's stdin contract, flag placement and the +error envelope, `NO_COLOR`/non-TTY hygiene, the network mutation lifecycle, +and a cold-start agent with zero prior knowledge of the CLI working from +`--help` alone. Full transcripts aren't kept here; the two real bugs they +surfaced and their fixes are. + +**Bug 1 β€” `exec`'s own flags silently no-op after the ref.** `exec +--stdin file -- cmd` and `exec --env K=V -- cmd` both sent nothing, +with no error β€” exactly the ordering shown in the command's own `--help` +examples, so the documented usage didn't work as documented. Root cause: +`parseExecArgs` (added when `--stdin` shipped) only recovered `ref`/`cmd` +from the raw tokens around `--`, never `--stdin`/`--env`/`--stream` +themselves β€” those still went through `c.String("stdin")` etc., which is +empty because urfave/cli v2 stops flag parsing at the first positional (the +ref). This is the identical bug class `parseSyncArgs` already exists to work +around in `sync.go`; `--env` had silently had this bug since before this +work started, `--stdin` inherited it on arrival. Fixed with a +`parseExecFlags` following `parseSyncArgs`'s exact shape: seed from whatever +urfave did parse, override with anything found in the ref..`--` window. + +**Bug 2 β€” batch-delete error messages collapsed to a generic placeholder.** +Traced to the `57d2ebf` merge conflict resolution: `resolveSandboxRef`'s +"no sandbox named X" error was a bare `fmt.Errorf`, not an `*api.APIError`. +Passing it through `api.UserMessageVerbose` β€” correct for actually-raw Go +errors, which is what that function exists to sanitize β€” flattened it to +"something went wrong β€” please try again or contact support" in every batch +result (`rm`, and by the same path `disk rm`/`network rm`/`template rm` +share). It also meant every direct not-found (`get`, `pause`, …) reported +`"code": "not_found"`'s intended slug as the placeholder `"code": "error"` +instead. Fixed by shaping `resolveSandboxRef`'s not-found error as a real +`*api.APIError` (404) β€” the one client-side "not found" in the CLI, so this +was a one-function fix, not a policy change to `UserMessageVerbose` itself. + +**Found, not fixed β€” flagged for follow-up:** `network attach|detach --yes` +silently fails to parse when `--yes` trails the two positionals (same class +of bug as above, but on a command with no re-scanner of its own yet β€” would +need a `parseNetworkArgs` mirroring `parseSyncArgs`/`parseExecFlags`). +Everything else the fleet touched β€” JSON shape, batch results, `NO_COLOR`, +stdout/stderr separation, flag hoisting, the redirect-safety boundary on +`exec`'s stdout β€” held up clean on live account against real and bogus refs.