Uh oh!
There was an error while loading. Please reload this page.
Akshay/lakebox bugbash followups - #5372
Merged
akshaysingla-db merged 5 commits intoMay 29, 2026
Merged
Conversation
Symptom: $ databricks lakebox ssh <id> -- 'echo hi' # works hi $ databricks lakebox ssh <id> -- bash -c 'echo hi' # broken, empty ssh joins remote-command words with spaces and the remote shell re-parses them. So the user's local args `["bash", "-c", "echo hi"]` land on the wire as the literal string `bash -c echo hi`, which bash splits into `-c=echo` and `$0=hi` — running `echo` with no args and printing an empty line. Fix: shell-quote each extra arg before append, using the repo's existing `libs/shellquote.BashArg`. Single-arg-with-whitespace is left untouched so the existing `lakebox ssh -- 'cmd | head'` shape (user expects the remote shell to do the split) keeps working; multi-arg invocations get exec-style preservation. Extracted `buildSSHArgs` so the argv construction is unit-testable; table-driven test covers the regression cases plus single-quote escaping, globs, and empty args. Co-authored-by: Isaac
…ommand The framework-global `-o, --output` flag validates its argument but lakebox subcommands never read it: passing `-o json` silently produced text. The lakebox commands had grown their own `--json` flag instead. Wire both forms through a tiny `jsonOutput(cmd, jsonFlag) bool` helper checked at the rendering branch. Touches the four subcommands that have a JSON branch (list, status, create, ssh-key list). `--json` stays as a short-form alias; `-o json` now works the same way users expect from every other databricks subcommand. The other lakebox verbs (stop, start, delete, config, ssh, default, register, ssh-key delete) emit only human-readable status, so they're untouched. Co-authored-by: Isaac
Before this change ssh.go printed `✓ Connected to <id>` immediately before handing off to the ssh binary, with no actual handshake having happened. On any ssh failure (gateway timeout, missing key, network hiccup) the user saw a confident "Connected" line *followed* by the real error from ssh. Drop the s.ok call. The Connecting spinner stays — it shows for the brief window before exec replaces this process. ssh's own stdout/stderr is the success signal; nothing the CLI adds at this point can be more truthful than that. Co-authored-by: Isaac
The server rejected oversize names with `name exceeds 256 bytes` but didn't say how many bytes the client sent, which makes the error useless when emoji (4 bytes each) push the input past 256 in fewer visible characters than the user expects. Mirror the limit client-side and validate `--name` before issuing `create` / `config update`. The new error names the observed byte count and reminds the user that non-ASCII characters count for more: Error: --name is 260 bytes; limit is 256 (emoji and most non-ASCII characters count as 2-4 bytes each) Side effect: zero-RTT failure — bad names never reach the server. Tracking deprecation: if the server-side cap ever changes, this constant goes out of date; the worst case is the CLI is briefly too strict, and the server still enforces. Co-authored-by: Isaac
The gateway hostname was already on the JSON side of `status` but missing from text. It's the most useful field for debugging an `ssh` failure (wrong region, stale cache, gateway down), so omitting it from the human-readable view forced users to round-trip through `--json` to see it. Render it in Faint between `status` and any existing `fqdn`, only when populated. Co-authored-by: Isaac
Uh oh!
There was an error while loading. Please reload this page.
Contributor
Waiting for approvalBased on git history, these people are best suited to review:
Eligible reviewers: Suggestions based on git history. See OWNERS for ownership rules. |
akshaysingla-db added a commit
that referenced
this pull request
May 29, 2026
## Summary Two CLI fixes from Anwell's bug-bash form submissions (the third was the `-o json` issue, already fixed in #5372 and verified live; the fourth — `stop` printing `✓ Stopped` on already-stopped — skipped intentionally). ### `273d68a3e` — measure NAME column widths in terminal cells `lakebox list` and `ssh-key list` padded NAME columns using `len()` (bytes). Emoji and CJK glyphs are 1 rune / multi-byte / **2 cells**, so rows with `🚀 rocket box` or `测试盒子` in NAME shifted STATUS / AUTOSTOP / DEFAULT out of alignment. Switched padding math to `runewidth.StringWidth` (East-Asian-Width-correct). Promoted `mattn/go-runewidth` from indirect to direct require + NOTICE entry. ### `d6e585163` — `--idle-timeout` errors echo in Go-duration units `--idle-timeout 25h` was rejected with `between 60s and 86400s, got 90000s` — every number in a different unit than what the user typed, even though `--help` already documented bounds as `60s to 24h`. Routed through the existing `formatDurationSecs` helper so error reads `between 1m and 24h, got 25h`. Also tweaked `--help` lower bound `60s` → `1m` so both strings agree. ## Test plan - [x] `lakebox list` with emoji `--name` aligns cleanly - [x] `lakebox list` with CJK `--name` aligns cleanly - [x] `--idle-timeout 25h` error: `between 1m and 24h, got 25h` - [x] `--idle-timeout 30s` error: `between 1m and 24h, got 30s` - [x] `--idle-timeout 90m` (valid) — no error - [x] license_test (NOTICE allowlist) passes - [x] all existing unit tests pass
akshaysingla-db added a commit
that referenced
this pull request
May 29, 2026
## Summary Four CLI fixes driven by the bug-bash form submissions. (The `-o json` issue Anwell reported was already fixed in #5372; verified live.) | Commit | Source | What | |---|---|---| | `273d68a3e` | Anwell | `lakebox list` columns mis-align with emoji / CJK names. Measure widths in terminal cells via `runewidth.StringWidth`. | | `d6e585163` | Anwell | `--idle-timeout` errors echo raw seconds (`86400s` / `90000s`) while `--help` uses durations (`24h`). Route bounds and value through `formatDurationSecs` so both match. | | `9e315d71c` | Mitch | Deleting the last sandbox on a profile left an orphan `gatewayHosts.<profile>` entry in `~/.databricks/lakebox.json`. `removeSandbox` now drops the gateway when the sandbox list goes to zero. | | `6d32203ce` | Mitch + tsanyu | `lakebox start` returned ✓ instantly while the box was still Creating; the next `ssh` would block on the same cold-start. `start` now polls `api.get` until Running (or 10 min) — symmetric with `create`. | ## Skipped (per discussion) - **#6** (`stop` on already-stopped lies) — left as-is. ## Filed elsewhere Server / network / sandbox-image: Codex bubblewrap missing, pip JSON truncation, universe IP-allowlist, in-sandbox Provisioning banner, Cursor deep-link port mis-parse, gateway publickey advertising, ESM token-minting logs/failure. Tracking under their respective epics, no CLI work. ## Test plan - [x] `lakebox list` with emoji `--name` aligns - [x] `lakebox list` with CJK `--name` aligns - [x] `lakebox start <stopped-id>` blocks until Running (verified live) - [x] `lakebox start <stuck-id>` times out cleanly at 10m - [x] All existing tests pass
2 tasks
akshaysingla-db added a commit
that referenced
this pull request
Jun 8, 2026
## Summary Bootstraps `acceptance/cmd/lakebox/` — lakebox had zero acceptance tests today. Six leaf scenarios cover the rendered output of every subcommand surface that's exercised by user-visible flows. | Test | What it locks down | |---|---| | `list/empty` | Empty result → `No lakeboxes found.` hint | | `list/with-entries` | Multi-row table: NAME column always present, mixed `running`/`stopped`/`creating…` statuses, autostop label including the `noAutostop=true` "never" case | | `status/not-found` | 404 → friendly `no lakebox named "X" — `databricks lakebox list` shows available IDs` (the message added in #5460) | | `ssh-key/list/empty` | Empty result → "Run `databricks lakebox register` to add one" hint | | `config/no-flags` | No flags → `nothing to update` error | | `config/idle-timeout-bounds` | Out-of-range values (`30s`, `48h`) → client-side bounds error before any API call, with the duration-formatted bounds from #5372 | Each test stubs the relevant API endpoint via the testserver framework's `[[Server]]` entries. The two `config` tests need no stub because validation errors fire before any API call. Parent `acceptance/cmd/lakebox/test.toml` adds `.databricks` to `Ignore` so the CLI's local state file (`lakebox.json`) doesn't bleed into the test directory's diff. All six pass under both `DATABRICKS_BUNDLE_ENGINE=terraform` and `=direct` with identical output, which is correct — lakebox is engine-independent. These would have caught the FQDN-displayed-in-status inconsistency and the `start.go` 5-vs-10-minute help-text mismatch that the bugfix-bundle PR (#5460) caught manually. ## Test plan - [x] `go test ./acceptance -run TestAccept/cmd/lakebox` passes (12 subtests, both engine variants) - [x] `-update` produces stable golden files (re-run without `-update` is clean) ## Out of scope (potential follow-ups) - Acceptance coverage for write-path commands (`create`, `delete`, `start`, `stop`, `default`, `register`) — these are interactive or have stateful side effects worth a separate, more involved test design. - Coverage for the `ssh` command — that path execs `ssh` directly so it isn't testable through this framework without an SSH server stub. This pull request and its description were written by Isaac.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Why
Tests