feat: configurable ports, optional SSH, exec, and canonical schema - #39
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The current implementation introduces avoidable fragility (comma-operator side-effect in port selection) and non-deterministic port ordering in status output that should be cleaned up before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR extends devbox’s workspace lifecycle to support publishing multiple ports with automatic allocation, makes the bundled SSH runner optional (using the first published port when enabled), and adds a new devbox exec -- <command> [args...] path that runs non-interactive commands inside the managed container while preserving stdio and exit codes.
Changes:
- Add multi-port selection/allocation and persist port lists in workspace state (STATE_VERSION bump) while keeping the first port as the canonical “primary” port.
- Make bundled SSH enablement configurable (
--no-ssh/--ssh) and ensure status reporting hides SSH-only fields when SSH is disabled. - Introduce
devbox exec -- ...support and expand simulated + live tests to cover the new behaviors.
File summaries
| File | Description |
|---|---|
| tests/status.test.ts | Adds coverage for multi-port status reporting and SSH-disabled behavior. |
| tests/runtime.test.ts | Adds tests for multi-port allocation helper and devcontainer exec command builder. |
| tests/examples.test.ts | Extends simulated CLI tests for exec, multi-port publishing, and SSH toggling. |
| tests/examples.live.test.ts | Adds real-container coverage for multi-port publishing, SSH optionality, and exec stdio/exit forwarding; adjusts fixture permissions. |
| tests/core.test.ts | Updates parser/help text tests and adds coverage for port count + SSH option validation and multi-port config generation. |
| src/status.ts | Adds ports and sshEnabled to status, resolves effective ports list, and suppresses SSH-only fields/warnings when SSH is disabled. |
| src/runtime.ts | Adds multi-port availability helpers, devcontainer exec command construction, and stdio inheritance support in process execution. |
| src/core.ts | Adds argument parsing for --ports, --ssh/--no-ssh, exec -- ...; persists ports/sshEnabled in state; updates managed config port publishing and ready message formatting. |
| src/constants.ts | Bumps workspace state version to 3. |
| src/cli.ts | Wires CLI handling for multi-port up/rebuild, optional SSH flow, and new exec command. |
| README.md | Documents multi-port publishing, optional SSH mode, and the new exec subcommand. |
| package.json | Updates package description to reflect optional bundled SSH. |
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Updated in dab85da: state.json and devbox status now use only the canonical schema v4 |
There was a problem hiding this comment.
🟡 Changes recommended
Exec parsing can silently discard arguments, and reducing a running workspace’s port count leaves unreported ports published.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/runtime.ts:629
- This message is inaccurate when only part of the requested range is available: the loop may have added ports before exhaustion, yet it reports that no ports were found. Report the partial count so users can distinguish total exhaustion from insufficient capacity.
- Files reviewed: 13/13 changed files
- Comments generated: 3
- Review effort level: Balanced
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Also addressed the suppressed partial-allocation review note in e0a66c1: exhausted-range errors now distinguish zero available ports from a partial allocation and report how many ports were found. Added runtime coverage for the partial case. |
Summary
devbox exec -- <command> [args...]with managed-container resolution, inherited stdio, and exit-code forwarding.portsthe only persisted and reported port representation, using workspace state schema version 4.portalias instead of silently migrating them.--portCLI input only as the selector forports[0]; it is not persisted as a singular JSON field.Breaking change
Existing
.devbox/state.jsonfiles from older schema versions are intentionally rejected. Remove.devbox/state.jsonand rundevbox upagain with the desired options, for example:The resulting state has this shape:
{ "version": 4, "ports": [5001, 5002], "sshEnabled": false }Validation
bun run test:fastbun run typecheckbun run buildPATH="$HOME/.devcontainers/bin:$PATH" bun test tests/examples.live.test.tsThe live suite passed all 6 real-container tests against Docker and Dev Container CLI 0.89.0.