Conversation
Three separate problems, each of which made a git operation either fail
outright or succeed in a way the caller could not act on. Found while
building a non-interactive CLI on top of this namespace.
updateConfig sent its request to /v2/box/{id}/git-config, which the
coordinator does not serve. The identity endpoint is /v2/box/{id}/config/git,
so every call returned 404 and no git identity was ever set through either
SDK. A unit test had pinned the wrong URL, so the suite agreed with the bug.
exec results now carry exit_code. The API has always returned it (see
shared/types.go GitExecResponse), but the type declared only output, so a
caller could not tell a failed git command from a successful one. Exit 128,
the code for "not a repository", was indistinguishable from success. This is
additive: the field was already on the wire.
clone accepts folder, naming the directory the repository is cloned into.
Every other git operation derives its folder from the tracked cwd via cd(),
but for clone the folder is the destination and does not exist yet, so cd()
fails on it. There was no way to express "clone into my-app" at all.
Both SDKs are covered. The Python SDK gets the route fix and the clone
folder, and is bumped to 0.3.1 with a changelog entry, since it does not go
through changesets. Its git.exec still returns a bare string and drops the
exit code; changing that is a breaking API change and belongs on its own.
The CLI was REPL-first: every path into a box went through an interactive session. That makes it unusable from a script, a CI job, or a coding agent, none of which have a terminal to drive. This adds a plain command for every operation, so a box can be driven by arguments alone. Commands: exec, files (9 verbs), git (9 verbs), expose, run, status, use, delete, pause, plus create --no-repl. The REPL is untouched as a way of working; it simply is no longer the only one. Three conventions hold across all of them: Box resolution is --box, then BOX_ID, then the nearest .box file, searching upward. `box create --no-repl` writes that file, so subsequent commands need no arguments at all. Which box was chosen is printed to stderr, never stdout, and BOX_ID silently winning over a checked-out .box is called out, since that is where someone is most likely to be talking to a box they did not mean. Data goes to stdout and diagnostics to stderr, so output survives a pipe. `--json` prints the result with no envelope and works program-wide. Exit codes distinguish the two kinds of failure. A remote command's own exit code passes through, so `box exec -- npm test && ...` chains the way it would locally. A failure of the CLI itself is 125, which no ordinary command produces and so cannot be mistaken for a status the remote command returned. Every command now reports its own failures through one error boundary rather than calling process.exit from wherever the error was noticed. Fixes found along the way, each with a regression test: - `box create` on a terminal with no flags ran the setup wizard and then rejected its own result with "agent harness is required", because the harness was resolved before the wizard's answer was merged. That is the bare first-run path. - Headless create still opened the wizard, because the gate only considered the older flags. `--no-repl --clone-repo ...` from a developer's shell is the same scripted path as one from CI. - `box get` fetched the box and printed only its id. - `/git status` in the REPL reported a missing repository as a clean tree. Both halves now share one check, so they cannot drift. - The zsh completion script did not parse: an apostrophe in a description ended its quoting. Both scripts are now generated from one list and are syntax-checked by the test suite. - `box files write` reported a character count as a byte count. Deleting is the one irreversible operation, so it asks first and refuses outright when there is no terminal to ask unless --yes is given. If this directory's own .box names the box being deleted, that file goes too, whether the box was named by id or came from the pin; a pin to a deleted box makes every later command fail as though the CLI were broken. A parent's pin belongs to the project rather than to this command, so it is reported as stale instead of removed. scripts/smoke.sh runs the whole surface against a real box and cleans up after itself, including on failure.
There was a problem hiding this comment.
Pull request overview
Adds a scriptable CLI surface while retaining the REPL. It depends on PR #230 for SDK git exit codes and clone destinations.
Changes:
- Adds non-interactive box, file, git, execution, exposure, agent, and lifecycle commands.
- Introduces shared box resolution, output, and exit-code handling.
- Expands REPL support, documentation, completions, and automated tests.
Reviewed changes
Copilot reviewed 59 out of 60 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
packages/cli/src/repl/types.ts |
Registers new REPL commands. |
packages/cli/src/repl/commands/status.ts |
Adds REPL status, runs, and logs. |
packages/cli/src/repl/commands/snapshot.ts |
Adds snapshot listing and deletion. |
packages/cli/src/repl/commands/git.ts |
Adds config and repository checks. |
packages/cli/src/repl/commands/files.ts |
Expands REPL file operations. |
packages/cli/src/repl/commands/expose.ts |
Adds REPL public-URL management. |
packages/cli/src/repl/commands/args.ts |
Adds REPL argument parsing helpers. |
packages/cli/src/repl/client.ts |
Wires expanded REPL commands. |
packages/cli/src/README.md |
Updates source architecture documentation. |
packages/cli/src/core/io.ts |
Centralizes output and error handling. |
packages/cli/src/core/git-repo.ts |
Detects missing git repositories. |
packages/cli/src/core/exec.ts |
Implements collected and streaming execution. |
packages/cli/src/core/errors.ts |
Defines CLI errors and exit codes. |
packages/cli/src/core/box-ref.ts |
Resolves and persists box selections. |
packages/cli/src/commands/use.ts |
Implements box pinning. |
packages/cli/src/commands/status.ts |
Reports selected-box status. |
packages/cli/src/commands/snapshot.ts |
Uses centralized errors. |
packages/cli/src/commands/run.ts |
Adds non-interactive agent runs. |
packages/cli/src/commands/lifecycle.ts |
Adds pause and guarded deletion. |
packages/cli/src/commands/init-demo.ts |
Migrates failures to CliError. |
packages/cli/src/commands/git.ts |
Adds non-interactive git operations. |
packages/cli/src/commands/get.ts |
Returns detailed box information. |
packages/cli/src/commands/from-snapshot.ts |
Migrates validation errors. |
packages/cli/src/commands/files.ts |
Adds non-interactive file operations. |
packages/cli/src/commands/expose.ts |
Adds public-URL commands. |
packages/cli/src/commands/exec.ts |
Adds remote command execution. |
packages/cli/src/commands/env.ts |
Migrates validation errors. |
packages/cli/src/commands/create.ts |
Adds headless creation and workspace options. |
packages/cli/src/commands/connect.ts |
Uses centralized errors. |
packages/cli/src/commands/completion.ts |
Generates expanded shell completions. |
packages/cli/src/cli.ts |
Registers the new command surface. |
packages/cli/src/auth.ts |
Delegates token validation centrally. |
packages/cli/src/__tests__/repl/commands/status.test.ts |
Tests REPL status behavior. |
packages/cli/src/__tests__/repl/commands/snapshot.test.ts |
Tests snapshot subcommands. |
packages/cli/src/__tests__/repl/commands/git.test.ts |
Tests expanded REPL git behavior. |
packages/cli/src/__tests__/repl/commands/files.test.ts |
Tests expanded REPL file operations. |
packages/cli/src/__tests__/repl/commands/expose.test.ts |
Tests REPL exposure commands. |
packages/cli/src/__tests__/core/io.test.ts |
Tests output and error boundaries. |
packages/cli/src/__tests__/core/exec.test.ts |
Tests execution helpers. |
packages/cli/src/__tests__/core/box-ref.test.ts |
Tests box resolution and pinning. |
packages/cli/src/__tests__/commands/use.test.ts |
Tests box pin management. |
packages/cli/src/__tests__/commands/snapshot.test.ts |
Updates snapshot error tests. |
packages/cli/src/__tests__/commands/run.test.ts |
Tests agent command output. |
packages/cli/src/__tests__/commands/lifecycle.test.ts |
Tests pause, deletion, and confirmation. |
packages/cli/src/__tests__/commands/init-demo.test.ts |
Updates init-demo error tests. |
packages/cli/src/__tests__/commands/git.test.ts |
Tests non-interactive git commands. |
packages/cli/src/__tests__/commands/get.test.ts |
Tests detailed box retrieval. |
packages/cli/src/__tests__/commands/from-snapshot.test.ts |
Updates snapshot-creation error tests. |
packages/cli/src/__tests__/commands/files.test.ts |
Tests non-interactive file commands. |
packages/cli/src/__tests__/commands/expose.test.ts |
Tests public-URL commands. |
packages/cli/src/__tests__/commands/exec.test.ts |
Tests command execution and exit codes. |
packages/cli/src/__tests__/commands/env.test.ts |
Updates environment validation tests. |
packages/cli/src/__tests__/commands/create.test.ts |
Tests headless creation behavior. |
packages/cli/src/__tests__/commands/connect.test.ts |
Updates connection error tests. |
packages/cli/src/__tests__/commands/completion.test.ts |
Tests generated completion scripts. |
packages/cli/src/__tests__/auth.test.ts |
Tests centralized token errors. |
packages/cli/scripts/smoke.sh |
Adds real-box smoke verification. |
packages/cli/README.md |
Documents the expanded CLI. |
.gitignore |
Ignores the local pnpm store. |
.changeset/cli-non-interactive.md |
Adds the CLI release note. |
Suppressed comments (2)
packages/cli/src/cli.ts:66
- Commander's own validation errors still bypass
runCommand: missing required arguments/options and unknown options are handled during parsing, before any action runs, and therefore retain Commander's exit code 1 rather than the promised CLI-failure code 125. Configure Commander to throw parser errors and runparseAsync()through the same error boundary (while preserving normal help/version exits).
packages/cli/src/commands/files.ts:78 - The byte count is wrong when
--encoding base64is used:textis the encoded representation, while the server writes its decoded bytes. For example,aGVsbG8=reports 8 bytes although the resulting file is 5 bytes.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Nine findings, all real. Two were serious enough to have hurt in normal use.
Headless detection only looked at stdin, so `ID=$(box create --runtime node)`
kept a terminal on stdin, failed the check, and opened a REPL whose output
was captured by the command substitution. The caller saw a hang and the box
went on billing. Both streams are checked now. An earlier attempt to verify
this used script(1), which gives the child a pty for stdout as well, so the
case was never actually reproduced; redirecting inside the pty session does
reproduce it, and the command now exits with the id on stdout.
box exec joined argv with spaces, which throws away the boundaries the local
shell had already resolved: `node -e 'console.log("hello world")'` arrived as
three words and was re-split by the remote shell. Quoting every argument
would have broken the documented `box exec -- '( npm run dev & )'` form,
where the single argument is deliberately a shell expression. The rule is now
explicit: one argument is a shell expression and is sent as written, several
are argv and are quoted individually. Both forms are covered by tests and
were checked against a real box.
The rest:
- The program-level --box, --json and --token were advertised but never
merged into create, connect, from-snapshot, list, snapshot, init-demo, env
or labels, so `box --token X list` failed while `box list --token X`
worked.
- A clone that failed after the box was created exited without ever printing
the id, leaving a running, billable box the caller could not name. It now
pins and reports the box before rethrowing.
- The delete confirmation reads stdin and writes stderr, but was gated on
stdout, so `box delete > delete.log` refused to ask despite a terminal
being attached.
- box files read validated only --length. --offset accepted anything, so a
typo sent NaN to the API. Both are now bounded whole numbers, with --length
capped at the 8 MiB the server will serve.
- The REPL advertised `snapshot create <name>` but had no such branch, so the
name became "create <name>".
- The --unset help promised to remove the nearest .box; it deliberately
removes only this directory's own.
- The REPL's git probe returned "(clean)" when the probe itself failed,
contradicting its own comment and giving the wrong answer this check exists
to prevent. It now reports that it could not check.
Each fix has a regression test, and every one was mutation-checked.
README and the CLI skill document the exec rule, since it is a contract.
One argument is a shell expression sent as written, so pipes, redirection and backgrounding work. Several arguments are argv and are quoted individually, so an argument containing spaces stays one argument. The CLI changed to behave this way (upstash/box#231); before, argv was joined with spaces and the remote shell re-split it.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 59 out of 60 changed files in this pull request and generated 4 comments.
Suppressed comments (9)
Previously missed (7) — in code that hasn't changed since the last review.
packages/cli/src/commands/files.ts:100
- When
--encoding base64is used,textis the encoded transport string, while the server writes the decoded bytes. This reports 8 bytes foraGVsbG8=although the resulting file is 5 bytes. Compute the decoded buffer length for base64 writes.
packages/cli/src/commands/run.ts:62 Number("Infinity")passes this validation and is sent as an infinite timeout (which JSON serialization commonly turns intonull). Reject all non-finite values, not onlyNaN, so--timeoutalways represents an actual positive duration.
packages/cli/src/commands/git.ts:161- These catches turn transport/authentication/API failures into a successful
(unset)identity response. A missing config value is already represented bygit execreturning a nonzeroexit_codewith empty output, so unexpected request failures should propagate to the CLI error boundary instead of being hidden.
packages/cli/src/repl/commands/git.ts:97 - These catches also hide network or authentication failures in the REPL and report the identity as unset. Since
git execreturnsexit_codefor an actually unset key, let request failures propagate rather than converting every rejection into normal output.
packages/cli/src/repl/commands/expose.ts:32 - The delete form validates only the lower bound, so
/expose delete 70000calls the API with an invalid TCP port even though the create form correctly rejects it. Apply the same upper bound here.
packages/cli/src/repl/commands/status.ts:31 - This parser silently treats malformed limits such as
logs nopeorlogs 0as the default 20 and forwards negative, fractional, or infinite values to the API. Validate an explicitly supplied limit as a positive integer and show usage instead of changing or forwarding invalid input.
packages/cli/src/commands/git.ts:98 - A clean status is the empty string, but
emitalways appends a newline in text mode, so this is not actually silent as the command/tests describe. This extra byte can also make shell checks treat clean status as output. Skip text emission for the empty result while still emitting""under--json.
This issue also appears on line 106 of the same file.
packages/cli/src/cli.ts:63
--jsonis advertised as program-wide, but several commands that now receive it throughmerged()still ignore it and print human text (list,snapshot, allenv/labelsverbs, and interactive commands). For example,box --json listemits the table fromcommands/list.ts, not JSON. Either limit this option to commands that implement it or update every command accepting the global flag to emit JSON.
packages/cli/src/commands/git.ts:106- An empty diff is converted into a newline by
emit, so the command produces output even when there are no changes. Preserve the empty JSON value for--json, but do not write a text line for an empty diff.
Four findings, all real. The 125 convention had a hole in it. Commander rejects unknown options, unknown commands and missing arguments before any action runs, and exits 1 by default, so a whole class of CLI failures was exiting with a status indistinguishable from a remote command that exited 1 — the ambiguity 125 exists to remove. exitOverride now routes them through the same boundary, applied recursively: a first attempt covered only top-level commands, so `box files read` still exited 1. Help and version raise through the same channel and are mapped back to 0. That fix lives in the entrypoint and cannot be reached by calling a command function, so it is covered by a test that spawns the built binary across five failure cases and five help cases. buildCommand filtered empty strings out of argv, so `printf '<%s>' ''` lost its final argument. Preserving argv is the entire purpose of that function. Empty arguments are kept; a lone empty argument still means no command. execStream started its exit code at 0, so a stream that ended without an exit chunk was reported as success. A truncated response could therefore let `box exec ... && next` run next without ever receiving a remote status. An absent status is now an error, which the boundary maps to 125. The clone-failure recovery pin wrote .box for interactive creates too, where a successful create does not pin, so a failed `box create --clone-repo ...` could overwrite a project's existing pin. It is now limited to headless creates, which are the ones that pin on success. Each fix has a regression test and was mutation-checked.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 60 out of 61 changed files in this pull request and generated 3 comments.
Suppressed comments (10)
Previously missed (7) — in code that hasn't changed since the last review.
packages/cli/src/commands/files.ts:100
- The reported byte count is incorrect when
--encoding base64is used:textis the encoded representation, while the server writes its decoded bytes. For example,aGVsbG8=writes 5 bytes but this reports 8. Count the decoded buffer in base64 mode.
packages/cli/src/commands/git.ts:189 - This trims arbitrary git output and then
emitappends a newline. That corrupts whitespace-sensitive output from commands such asgit cat-file blob, and a command with no output emits a spurious blank line. Preserveresult.outputexactly in text mode while retaining the structured object for JSON mode.
packages/cli/src/repl/commands/git.ts:97 - These catches also classify network/auth/server failures as an unset identity in the REPL.
git config --getreturns a regular result for an absent key, so only its empty output needs the(unset)fallback; request rejections should surface to the REPL error handler.
packages/cli/src/commands/run.ts:64 - Values such as
Infinityand timeouts above Node's approximately 2^31-1 ms timer limit pass this validation. The SDK feeds the milliseconds tosetTimeout; Node overflows such delays to roughly 1 ms, so a supposedly long run times out immediately. Reject non-finite and out-of-range values before starting the run.
packages/cli/src/repl/commands/status.ts:31 - Invalid limits such as
-1,1.5, orInfinityare passed directly to the API, while0silently becomes 20. Validate the optional argument as a positive integer and show usage instead of making a malformed logs request.
packages/cli/src/repl/commands/expose.ts:32 - The delete form accepts ports above 65535 even though the create form correctly rejects them. This sends an invalid port to the API instead of returning the usage message locally.
.changeset/cli-non-interactive.md:2 - This additive feature is marked as a minor bump, which advances the current 0.x package to a breaking-change release. The repository release guideline says to prefer patch releases while packages are on 0.x; use a patch changeset unless this PR intentionally introduces a breaking change.
"@upstash/box-cli": minor
packages/cli/src/cli.ts:64
--jsonis exposed as a program-wide option, but several commands that receive the merged global flags still ignore it. For example,listCommand(packages/cli/src/commands/list.ts:10-47) always prints a table, and the env/labels commands always useconsole.log; thereforebox --json listsucceeds with non-JSON output. Either implement JSON output for every command that can receive this flag or scope the option to commands that honor it.
packages/cli/src/commands/git.ts:161- Catching every
git.execrejection turns transport/auth/server failures into a successful(unset)response. A missing config key is already represented by a normal result with a nonzeroexit_code, so unexpected request failures should propagate to the CLI error boundary instead of being masked.
packages/cli/src/cli.ts:557 - Commander writes usage errors before
exitOverride()throws, so writing the exception message again here duplicates diagnostics for unknown options and missing arguments. Keep the exit-code remapping, but do not print the same Commander error a second time.
Third review round. git checkout reported success without checking what happened. The endpoint runs `git checkout X || git checkout -b X` and reports success on either, and that is also what happens when X is a path: `checkout README` restores the file and leaves HEAD where it was, so a script carried on believing it had switched branches. The branch is now read back before anything is reported, and a detached head is still treated as a real checkout. The review supposed a dirty tree as the trigger. Tested against a real box, that case does propagate: the fallback fails too, the endpoint returns 500 and the CLI exits 125 with HEAD unmoved. The comment claiming otherwise was wrong and is gone. A path is the case that actually slips through. The 125 convention was overstated. `box exec` and `box git exec` pass the remote status through unchanged, so a command that genuinely exits 125 cannot be told apart from a CLI failure. Remapping it would make the reported remote status a lie, so the wording now says convention rather than guarantee and points at --json, where the command's exit_code is a separate field from whether the CLI succeeded. AGENTS.md asks for integration tests as well as unit tests, and there were none. Adds 18 that spawn the built binary against a real box, covering argument parsing, global flag placement, stream separation and exit-code propagation, plus the config and scripts to run them. The unit config did not exclude *.integration.test.ts, so it does now; the root test:integration script runs every package that defines one. Writing those found a fidelity problem no one had reported: streamed output is not byte-exact. `box exec -- printf abc` prints "abc\n" while `box exec --json -- printf abc` reports "abc", because the streaming endpoint appends a trailing newline. Raw SDK chunks confirm it arrives that way, so it cannot be fixed here without buffering the whole output and giving up streaming. Both behaviours are now pinned by tests and the README says which one to use when the exact bytes matter.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 66 out of 67 changed files in this pull request and generated 2 comments.
Suppressed comments (5)
Previously missed (3) — in code that hasn't changed since the last review.
packages/cli/src/repl/commands/args.ts:25
- Treating every dash-prefixed token as a flag makes valid file paths disappear from the positional list. Commands such as
/files read -notesand/files rename -old newnow report usage or operate on the wrong arguments, and even--cannot escape the path because it and the following dashed token are both classified as flags. Support an end-of-options marker or classify only flags recognized by the selected verb.
packages/cli/src/commands/files.ts:100 - When
--encoding base64is used, the server decodestextbefore writing it, but this reports the UTF-8 size of the encoded string. For example,YQ==writes one byte and reports four. Calculate the decoded byte length for base64 input so the command'sbytesresult describes the file that was written.
packages/cli/src/repl/commands/git.ts:97 - These catches convert transport/API failures into
(unset). A missing git config key is already represented by a resolvedgit.exec()result with a nonzeroexit_code, so rejected requests should propagate to the REPL error handling instead of displaying a false identity state.
packages/cli/src/cli.ts:64
--jsonis now advertised and accepted globally, but several actions receiving the merged flag still emit plain text:listCommand,snapshotCommand, everyenvaction, and everylabelsaction ignorejson. For example,box --json listsucceeds with a formatted table, so callers cannot rely on this global contract. Either make those handlers useemitfor JSON results or do not expose--jsonglobally until all applicable commands support it.
packages/cli/src/commands/git.ts:183git.exec()returns normal git failures throughexit_code; it rejects for transport/API failures. Swallowing rejections here therefore reports a network or service outage as a successfully read(unset)identity. Let request failures propagate and use the resolved result's empty output/nonzero exit code to represent an unset value.
Fourth review round. Two findings, both real, and the first was mine to begin with. The checkout read-back added last round caught a false success and then recreated it. A probe that threw, or returned non-zero, left head undefined, and the command printed "On branch <requested>" and exited 0 — the exact condition the read-back exists to prevent, moved one step later. It is now a CLI error, so the caller learns the branch is unconfirmed instead of being told the wrong one. The test written alongside that fix asserted the broken behaviour, passed, and made the bug look covered. It is inverted, and the first mutation used to check the new version came back "caught" only because a second guard threw anyway; mutating to the original fallback fails two tests, which is the real signal. git config had the milder form of the same problem. `git config --get` exits non-zero when a key is genuinely unset, which is an answer, but a request that failed landed in the same catch and was reported as "(unset)". Those are now separate: non-zero means unset, a failed request propagates. The changeset still carried the 125 wording that the README and errors.ts had already been corrected for. Since that text is what ships in the changelog, it now says convention rather than guarantee and points at --json.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 66 out of 67 changed files in this pull request and generated 2 comments.
Suppressed comments (5)
Previously missed (5) — in code that hasn't changed since the last review.
packages/cli/src/core/exec.ts:33
- A completed
box.exec.command()without an exit status is currently reported as exit 0. That turns a malformed or truncated response into success, unlike the streaming path below, and can incorrectly allowbox exec ... && nextto continue. Treat a null/undefined status as a CLI failure instead of defaulting it to zero.
packages/cli/src/commands/files.ts:100 - When
--encoding base64is used, the server decodestextbefore writing it, but this counts the UTF-8 bytes of the encoded representation. For example, a 4-byte payload represented by 8 base64 characters is reported as 8 bytes. Compute the decoded buffer length in base64 mode so the reported file size remains accurate.
packages/cli/src/commands/run.ts:64 Number("Infinity")is positive and not NaN, so--timeout Infinityreaches the SDK as an infinite timeout (and may serialize asnullor schedule incorrectly). Require a finite value as well as a positive one.
packages/cli/src/commands/run.ts:89- The loop treats a stream that closes without a
finishchunk as success; under--jsonit emits partial output with null session/usage and exits 0. A dropped connection can therefore masquerade as a completed agent run. Track whether a finish chunk was received and throw aCliErrorafter the loop when it was not.
packages/cli/src/repl/commands/expose.ts:32 - The delete form only validates the lower bound, so ports such as 70000 are sent to the API even though the create form rejects them. Apply the same 1–65535 validation to both operations.
Fifth review round. Both findings are the same defect as before in new places. A detached head was accepted as proof the requested target was checked out. `rev-parse --abbrev-ref HEAD` reads "HEAD" whether the repository detached to the requested commit or was already detached and the request merely restored a file, so `box git checkout README` on a detached repository reported success with HEAD unmoved. Reproduced against a real box before changing anything. The requested ref and the current head are now both resolved to commits and compared, so a tag or commit still succeeds and a path does not. The REPL's identity lookup still turned a failed request into "(unset)", which the non-interactive path stopped doing last round. A network failure looked like a successful lookup finding nothing configured. Non-zero from `git config --get` means unset, which is an answer; a failed request propagates. The test alongside it asserted the old behaviour, mocking a rejected request and expecting "(unset)". That is the second test this round found protecting a bug rather than catching one. Also makes the smoke script test that a refused delete left the box alive by asking whether it still exists, rather than by running a command in it: an exec there has to wait out a resume and can fail for reasons unrelated to what is being checked.
exec.stream() could finish without ever yielding an exit chunk, leaving a caller unable to tell whether the command succeeded. The `event: exit` marker and its `data:` payload do not always arrive in the same network read. When the marker completed one read, the parser matched the payload line with a pattern that does not require the line to be complete, failed to parse it, and then returned — ending the stream with no exit chunk and discarding the rest, including the flush that would have recovered it. It now requires a terminated data line before parsing, and keeps reading when the payload has not fully arrived. The lenient match on the stream-end flush path is unchanged, so a final event without a trailing newline still parses. Found from the CLI, where a command that reports no exit status is treated as a failure: `box exec -- true` failed roughly twice in thirty runs. Against a real box, 60 consecutive streams now carry an exit chunk; before, about one in thirty did not.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 66 out of 67 changed files in this pull request and generated 12 comments.
Suppressed comments (2)
packages/cli/src/commands/files.ts:96
- The write path also silently ignores an unsupported
--encoding. A misspelledbase64value therefore writes the encoded text itself and reports success, corrupting the intended binary file. Validate the option before writing.
packages/cli/src/commands/git.ts:134 - An empty diff similarly emits a newline instead of being silent. This makes the text-mode output differ from an empty git diff and adds data to pipelines. Only call
emitfor empty output in JSON mode.
Seven items the reviewer had suppressed as previously missed, in code that had not changed since it first saw them. All were real. A bare repository exits 0 from `rev-parse --is-inside-work-tree` and prints "false", so trusting the exit code alone classified it as a working tree and its empty status read as clean, which is the ambiguity that helper exists to remove. The answer is now what git printed. messageFor called JSON.stringify unguarded, which throws for a BigInt or a circular object and returns undefined for a function. It runs inside the catch that is supposed to turn any failure into a diagnostic and exit 125, so such a value escaped the boundary rather than being reported by it. `/git config --name` with no value was indistinguishable from no flag at all, so it silently became a read, and `--name --email x` updated only the email. A flag token present without a value now reports usage. `/expose delete 70000` skipped the upper bound the create form applies. `/status logs nope` silently became the default of 20, and a negative count reached the API unchanged. `--timeout Infinity` passed validation, and Node clamps a delay beyond its timer range to about a millisecond, so the run aborted almost immediately instead of waiting longer. Timeouts must now be finite and within range. The changeset was corrected to a patch bump last commit but the pull request description still promised a minor one; the description has been updated to match, and to say why the SDK change has to publish first. Two mutation checks passed when they should not have. One was a real overlap between guards and is fine. The other was not: the messageFor tests had never been added, because the edit was guarded on a name the file already imported, so the fix had no test at all. Both are covered now.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 66 out of 67 changed files in this pull request and generated no new comments.
Suppressed comments (8)
Previously missed (6) — in code that hasn't changed since the last review.
packages/cli/src/cli.ts:557
- Commander has already written usage errors before
exitOverride()throws, so printing the thrown message here reports unknown options, commands, and missing arguments twice. Suppress the second write forcommander.*errors while retaining it for unexpected synchronous failures.
packages/cli/src/tests/integration/setup.ts:32 - The runner inherits
BOX_ID, whose documented precedence is higher than the newly created.boxpin. If a developer or CI environment already hasBOX_IDset, these integration tests mutate that unrelated box instead of the disposable fixture. Force an emptyBOX_IDin the child environment so resolution falls through to the temporary directory's pin.
This issue also appears on line 44 of the same file.
env: { ...process.env, UPSTASH_BOX_API_KEY, NO_COLOR: "1" },
packages/cli/README.md:39
- The quick-start writes an untracked file and immediately invokes
git commit, but this command only commits staged changes. As shown, the advertised workflow fails with “nothing added to commit”; stage the file throughgit execfirst.
box files write my-app/src/patch.ts - < patch.ts
box git status -C my-app
box git commit -C my-app -m "apply patch"
packages/cli/README.md:341
- The delete form requires a snapshot id, but the REPL command table omits it, so copying the documented command only returns usage. Include the required argument.
| `/snapshot list`, `/snapshot delete` | List or delete snapshots |
packages/cli/scripts/smoke.sh:35
- This smoke test relies on the new
.boxpin, but it leaves an ambientBOX_IDintact even thoughBOX_IDtakes precedence. Running the script withBOX_IDset would execute writes, git operations, pause, and the no-argument delete against that pre-existing box while the newly created fixture remains separate. Unset it before any CLI invocation.
unset UPSTASH_BOX_BASE_URL
packages/cli/src/commands/lifecycle.ts:96
- The remote deletion has already succeeded before this unguarded local-file read. If
.boxis unreadable, malformed as a directory, or disappears in a race,readOwnBoxFile()throws and the command reports exit 125 without ever reporting the successful irreversible delete. Make the entire post-delete pin inspection best-effort, not onlyclearBoxFile().
packages/cli/src/cli.ts:64
--jsonis exposed as a global option, but several reachable handlers ignore it. For example,box --json listpasses the flag intolistCommand, which always prints a table (commands/list.ts:10-47), and the snapshot/env/labels handlers likewise emit text. This silently violates the PR's program-wide machine-readable-output contract. Either implement JSON output for every command that accepts this root option or reject/scope it for unsupported commands.
packages/cli/src/tests/integration/setup.ts:44- The stdin runner has the same ambient-
BOX_IDhazard as the regular runner: file-write integration tests can target and modify an unrelated box instead of the fixture created for this suite. ClearBOX_IDin the spawned environment.
env: { ...process.env, UPSTASH_BOX_API_KEY, NO_COLOR: "1" },
CI runs prettier --check per package and the new split-chunk tests were not formatted, which failed the JS test jobs on both Node versions.
Seventh review round, from a set of comments the reviewer had suppressed as previously missed. Two were hazards rather than defects. BOX_ID outranks the .box pin, and neither the integration runners nor the smoke script cleared it, so with one exported the whole suite — including its no-argument delete — ran against whatever box the developer had set rather than its own fixture. Both now clear it, and the integration suite passes with a hostile BOX_ID in the environment. box delete inspected the local pin after the remote delete had already succeeded, unguarded. An unreadable, missing or racing .box turned an irreversible success into exit 125. The whole post-delete step is best-effort now. --json was advertised program-wide but ignored by list, env, labels and snapshot, which printed tables regardless: `box --json list` returned a formatted table. All four emit real data now, and each declares the flag so both spellings work. Converting them moved their output from console.log to stdout, which is what the seven unit-test failures were, and it also dropped the "No labels." line for an empty list; that is restored on stderr, matching box expose, so stdout still holds only data. Commander writes its own diagnostic before exitOverride throws, so every usage error was printed twice. One review claim did not hold. The README quick start was said to fail with "nothing added to commit" because git commit only commits staged changes. Tested against a real box it exits 0: CommitChanges runs `git add -A` first. The example stays, but that staging behaviour was undocumented, so the README now states it and points at `git exec -- add` for choosing what goes in. The missing snapshot id in the REPL table was real. Also fixes the integration fixture, which used a fixed box name. One leaked box made every later run fail in beforeAll with "name already in use", and vitest reports a throwing beforeAll as skipped, so a broken suite looked deliberately gated. The name is unique per run, verified by planting a box under the old name and watching the suite still pass.
Answers the review question on the changeset. The CLI was the only surface calling this "expose": public docs page title "Public URLs" JS SDK getPublicURL / listPublicURLs / deletePublicURL Python SDK get_public_url / list_public_urls / delete_public_url docs prose "a public URL", box.getPublicURL(3000) CLI and REPL expose Neither name has shipped: main has no occurrence of expose, and the REPL command file is new in this branch, so this is a rename rather than a deprecation. `box public-url` keeps the same shape — a bare port creates one, plus list and delete <port> — and the REPL command becomes /public-url. Kebab case rather than the publicUrl spelling suggested, to match create-pr, from-snapshot and init-demo, and because camel case is awkward to type as a shell argument. The messages follow: "No exposed ports." becomes "No public URLs.", and the usage lines name the new command. Verified against a real box: created a URL, fetched it, listed it and deleted it. The smoke script covers the same three steps. This leaves one inconsistency untouched, in another repository: the MCP server calls it box_preview, and the API path and hostname are both preview. So the feature is now "public URL" in the SDK, CLI and documentation, and "preview" on the wire.
The CLI command was renamed to match the SDK and the public documentation, which both call this a public URL (upstash/box#231).
The generated completion scripts are syntax-checked by running the shell, but zsh is the default on macOS and is not installed on the Linux CI runner, so the check failed there with spawnSync ENOENT rather than telling anyone anything about the script. It now runs where zsh is present and is skipped where it is not. bash is everywhere, so that half always runs, and reverting the zsh quoting fix still fails the test on a machine that has zsh. The detection itself was silently broken first time round: spawnSync was never imported, so the probe threw, the catch reported "no zsh", and the test skipped even on macOS. That is the same swallowed-failure shape this branch keeps finding, in my own test this time.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 69 out of 70 changed files in this pull request and generated 7 comments.
Suppressed comments (2)
packages/cli/src/cli.ts:68
- Registering
--jsonon the root makesbox --json connect,from-snapshot,init-demo, andcompletionaccept the flag, but those handlers remain interactive or emit plain text. This silently violates the documented program-wide JSON contract and can leave an automation caller in a REPL. Either implement JSON/headless behavior for these actions or reject the root flag when a command does not support it.
packages/cli/src/cli.ts:508 - This advertises
--json, butenvSetAllCommandignores it and always callsconsole.log, so the command returns plain text while succeeding. Emit a defined JSON result (for example, the updated key names or count) or remove the option rather than silently violating the flag contract.
…t is absent Eighth review round: seven inline findings and two suppressed. Eight were real. box run reported success when the stream ended without a finish chunk, which is the same defect already fixed one level down in exec.stream: the iterator can end at EOF without the run ever completing, and a partial answer consumed as a whole one is worse than a failure. It also assigned the finish output only when truthy, so an authoritative empty answer was discarded in favour of the partial deltas. The chunk is required now and its output is taken as given. snapshot --json wrote selection and progress text to stdout before the result, so the output was not parseable. That text goes to stderr. Two contract violations came from advertising --json program-wide in the last round. env set-all accepted the flag and printed a sentence; it returns the keys and a count now. connect, from-snapshot, init-demo and completion cannot produce machine-readable output at all, and `box --json connect` would have answered an automation caller with a REPL prompt; they refuse the flag and say what to use instead. The REPL argument splitter treated every dash-prefixed token as a flag, so `files read -notes` lost its path with no way to escape it. `--` now ends the options. Shell completion offered top-level command names as the argument to a subcommand, so `box files read <TAB>` suggested `git` and `status` where a path belongs. Both shells hand back to their own file completion past that position. One finding did not hold: the architecture document was said to have dropped repl/commands/exec.ts, with the handler still present. That file does not exist on this branch or on main; the listing named a file that never existed and removing it was a correction. Checking it did show five real handlers missing from the listing, which are added.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 69 out of 70 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
packages/cli/src/commands/git.ts:30
--github-tokenis currently ignored forbox git clone. The SDK reads clone authentication fromBox.get(..., { gitToken })(packages/sdk/src/client.ts:1122-1155and2974-2985);GitCloneOptionshas no per-callgithubToken, so the property added at line 115 never reaches the request. Pass the flag when opening the box (and remove the ignored clone property) so private clones authenticate.
packages/cli/src/repl/commands/git.ts:90- This whitespace-only flag parser truncates ordinary multi-word identities. For example,
/git config --name Jane Doe --email jane@example.comstores onlyJane, while quoting the name stores the literal token"Jane. Parse quoted arguments (or otherwise consume the complete value up to the next recognized flag) before callingupdateConfig.
The exit-code table still used `expose delete` for its missing-subcommand- argument row. That case kept passing after the rename, but for the wrong reason: `expose` is now an unknown command, which the row above it already covers, so the table had two tests of the same path and none of the one it claimed. It now uses `public-url delete`, which really does fail on a missing required argument. Also clears the last references to the old name: a comment in labels.ts pointing at a command that no longer exists, and two test names still saying "exposed ports".
The integration suite has been failing on "Monthly managed Agent API key cost limit reached ($100)" — on main as well as on open branches — so the signal is dark until the limit resets. The tests pass no agent key of their own, so every run bills the Upstash-managed one. The spend was concentrated: of 21 files naming a model, 9 actually call agent.run or agent.stream, for about 26 runs. Five of those were on Opus while asserting only that a run completed and produced output. The rest were on Sonnet for the same kind of assertion: status is "completed", the output is non-empty, a file landed, a webhook arrived. Every model used to create a box is now Haiku 4.5. The twelve files that name a model but never run the agent are unaffected either way; they are included so the suite reads consistently. Two are deliberately left alone: configure-model cycles Sonnet, Haiku and Opus 5 to prove reconfiguration takes effect and survives a reconnect. Collapsing it to one model would leave it asserting that setting Haiku yields Haiku, which is not the behaviour it exists to check. It never runs the agent, so it costs nothing. The Codex case in agent.integration.test.ts uses a different harness and asserts the output contains OPENAI_OK. It is there to prove a non-Anthropic harness works, so changing its model would remove the point of the test. 431 unit tests still pass. The integration suite itself runs on workflow_dispatch against a deployed API, so this cannot be verified here; the saving is in what it bills when it next runs.
* Add an upstash-box-cli skill and correct the box git docs Adds a skill for driving a box from the terminal with the `box` CLI, and fixes what the two box SDK skills said about git. The CLI skill covers the non-interactive surface: selecting a box, running commands, files, git, expose, the agent, and cleaning up. It leads with the thing an agent has to understand first, that `box` operates on a remote container while its own file and shell tools act locally. It also names the two traps that cost real time in testing: a background server dies with the command that started it unless detached, and `box create` without --no-repl opens a REPL that will hang a headless run. Both SDK skills gain the same correction. Every git call except clone runs in the box's current directory, so `cd` into the clone first; at the workspace root there is no repository and `status` comes back empty, which reads as a clean tree. That ambiguity is exactly what a skill exists to prevent, and it is not visible from the method signatures. The JS skill documents `exit_code` on `git.exec()` and `folder` on `git.clone()`. The Python skill documents `folder` only, since its `git.exec()` returns a bare string and drops the exit code. Generated bundle rebuilt, 11 sub-skills. * Document the box exec argument rule One argument is a shell expression sent as written, so pipes, redirection and backgrounding work. Several arguments are argv and are quoted individually, so an argument containing spaces stays one argument. The CLI changed to behave this way (upstash/box#231); before, argv was joined with spaces and the remote shell re-split it. * Rename expose to public-url in the box CLI skill The CLI command was renamed to match the SDK and the public documentation, which both call this a public URL (upstash/box#231). * Match the CLI skill description to the renamed command The description is what an agent matches on to decide whether to load the skill, and it still said "expose a preview URL" — a verb the CLI no longer uses and a third name for the feature, alongside the SDK's public URL and the API's preview. * Show the CLI skill finishing a task, and correct the --json claim The skill described every command but never a whole job, so "make me a snake game with Upstash Box" could stop at a written file. Adds the end-to-end shape: create, write, run detached, check the port answers, publish, and reply with the URL. It serves with node's own http module rather than a package, so there is no install step or network fetch on a bare node runtime. It checks the port before publishing because a public URL for a port nothing is listening on returns 502, which reads as a broken game rather than as a race with a server that had not finished starting. And it says to hand back the URL itself, along with the two things the user cannot see: the box bills until it is deleted, and the URL is public to whoever has it. --json was documented as working on any command. connect, from-snapshot, init-demo and completion refuse it, because they open a REPL or print a shell script and answering an automation caller with a prompt is worse than failing. * Give the CLI skill auth and install, and list it in the catalog An agent that loaded this skill would run `box create --no-repl` and get "API token required": the skill documented every command but never how to authenticate. Adds the env var and --token, matching the CLI's own README and the upstash-cli skill's shape, plus the global install so `box` is on PATH at all. Deliberately no `box login`: the device flow has not shipped, and CI and agents would stay on the env var regardless. The README catalog listed only the JS and Python Box skills, and the plugin descriptions described the collection as SDKs, so anyone installing from either would not learn this skill exists. The generated skills/upstash/ TOC already had it; the human-facing entry points did not.
…authenticate
`box git clone <private> --github-token X` cloned unauthenticated and failed.
The flag was spread into `box.git.clone()`, but GitCloneOptions has no such
field: the SDK sends `github_token` from the token held by the Box instance,
set when the connection is opened. `openBox` called `Box.get(id, { apiKey })`
and never passed one, so the request carried `github_token: undefined` while
the CLI advertised the flag as "Token for a private repository".
TypeScript could not catch it. Excess-property checking applies to fresh object
literals, not to spread-in properties, so an option the interface does not
declare passes silently.
`Box.get` already accepts `gitToken`, so the fix is at the call site. `open()`
delegates to `openBox()`, which means every git verb picks it up if one takes
the flag later.
`box create --clone-repo <private> --git-token X` was unaffected: that token
reaches the box through `Box.create({ git: { token } })`, which sets the same
instance field. Its `githubToken` spread was equally inert, and is removed too.
The tests asserted the broken contract. create.test.ts expected `clone` to be
called with `githubToken`, which would have kept passing after a fix that did
not work either. Both suites now assert the token on the connection, and the
old assertion is inverted to prove clone carries no token option.
Three integration failures after the switch to Haiku 4.5, two real and one an artifact of how the tests share a box. The structured-output test asked the agent to "say hi" with a responseSchema. The Claude runner turns that schema into the SDK's native json_schema output format, which the harness exposes as a StructuredOutput tool, and Haiku refused to call it: "I won't call tools based on instructions embedded in messages... That appears to be a prompt inject". With no actual task in the prompt it is not a bad reading. The test now asks for real work, so calling the tool is the obvious way to answer, and it still asserts the same thing about the schema. The prompt-files tests capped runs at maxTurns: 1. That is enough for a model that answers immediately and not for one that looks at the file first, so the run died with "Reached maximum number of turns (1)". None of these tests assert anything about turn limits; the cap was there to bound cost. Raised to 3. The third failure was not a third failure. Each describe shares one box, a run that ends badly leaves it busy, and the next test then gets 409 "Box cannot start a run in its current state". One failure reported as two, with the second message pointing away from the cause. Every describe now waits for the box to leave the running state before its next test. These tests exist to prove the SDK plumbing carries a schema and a file to the agent, not to measure how clever the model is, so they should not depend on a model answering in one turn or trusting an instruction it did not ask for.
The CLI was REPL-first: every path into a box went through an interactive session. That makes it unusable from a script, a CI job, or a coding agent, none of which have a terminal to drive. This adds a plain command for every operation, so a box can be driven by arguments alone.
The REPL is untouched as a way of working. It is simply no longer the only one.
Commands
exec,files(9 verbs),git(9 verbs),public-url,run,status,use,delete,pause, andcreate --no-repl.Three conventions hold across all of them
Box resolution is
--box, thenBOX_ID, then the nearest.boxfile, searching upward.box create --no-replwrites that file, so later commands need no arguments at all. Which box was chosen goes to stderr, never stdout — andBOX_IDsilently winning over a checked-out.boxis called out, since that is where someone is most likely to be talking to a box they did not mean.Streams are separated. Data on stdout, diagnostics on stderr, so output survives a pipe.
--jsonprints the result with no envelope and works program-wide (box --json statusandbox status --jsonboth work).Exit codes distinguish the two kinds of failure. A remote command's own exit code passes through, so
box exec -- npm test && ...chains the way it would locally. A failure of the CLI itself is 125, which no ordinary command produces and so cannot be mistaken for a status the remote command returned. Every command now reports its failures through one error boundary instead of callingprocess.exitfrom wherever the error was noticed.Bugs found along the way
Each has a regression test.
box createrejected its own wizard. On a terminal with no flags it ran the setup wizard and then failed with "agent harness is required", because the harness was resolved before the wizard's answer was merged. That is the bare first-run path.--no-repl --clone-repo ...from a developer's shell prompted. That is the same scripted path as one from CI.box getfetched the box and printed only its id./git statusin the REPL called a missing repository a clean tree. The status endpoint discards git's exit code, so empty output is ambiguous. Both halves now share one check and cannot drift.box files writereported a character count as a byte count.Deleting
The one irreversible operation, so it asks first, and refuses outright when there is no terminal to ask unless
--yesis given. The confirmation settles on end-of-input rather than hanging, which otherwise left the command exiting having silently done nothing.If this directory's own
.boxnames the box being deleted, that file goes too — whether the box was named by id or came from the pin. A pin to a deleted box makes every later command fail as though the CLI were broken. A parent's pin belongs to the project rather than to this command, so it is reported as stale instead of removed.Verification
scripts/smoke.shruns the whole surface against a real box — 39 checks, cleaning up after itself including on failure.Release note
@upstash/box-cliis a patch bump, per AGENTS.md, which prefers patch on these pre-1.0 packages since minor is the breaking-change line. This change is additive.It must publish after #230, and not only for the type additions: without the exec-stream fix in that PR,
box execintermittently reports no exit status, and against the currently published SDK-Con clone is silently dropped.