Skip to content

Add experimental sandbox command group#5223

Merged
pietern merged 82 commits into
mainfrom
demo-lakebox
Jun 10, 2026
Merged

Add experimental sandbox command group#5223
pietern merged 82 commits into
mainfrom
demo-lakebox

Conversation

@pietern

@pietern pietern commented May 8, 2026

Copy link
Copy Markdown
Contributor

Adds an experimental databricks sandbox command group for managing remote, SSH-accessible compute environments, with acceptance test coverage and a scoped test-sandbox CI flow so changes under cmd/sandbox/... and acceptance/cmd/sandbox/... run only their own tests.

The command surface is experimental and subject to change; the underlying service is not broadly available yet.

shuochen0311 and others added 16 commits April 10, 2026 18:20
Lakebox provides SSH-accessible development environments backed by
microVM isolation. This adds CLI commands for lifecycle management:

- `lakebox auth login` — authenticate to a Databricks workspace
- `lakebox create` — create a new lakebox (with optional SSH public key)
- `lakebox list` — list your lakeboxes (shows status, key hash, default)
- `lakebox ssh` — SSH to your default lakebox (or create one on first use)
- `lakebox status <id>` — show lakebox details
- `lakebox delete <id>` — delete a lakebox
- `lakebox set-default <id>` — change the default lakebox

Features:
- Default lakebox management stored at ~/.databricks/lakebox.json per profile
- Automatic SSH config management (~/.ssh/config)
- Public key auth only (password/keyboard-interactive disabled in SSH config)
- Creates and sets default on first `lakebox ssh` if none exists
- Remove PubkeyHashPrefix field from lakeboxEntry (no longer returned by API)
- Remove KEY column from list output
- Remove Key line from status output
- Add register-key subcommand for SSH public key registration

Co-authored-by: Isaac
…rites

- Add 'register' command: generates ~/.ssh/lakebox_rsa and registers with API
- Remove 'register-key' command (replaced by 'register')
- Remove 'login' command (use 'auth login' + 'register' separately)
- SSH command passes options directly as args instead of writing ~/.ssh/config
- Check for ssh-keygen availability with helpful install instructions

Co-authored-by: Isaac
- Hook into auth login PostRun to auto-generate ~/.ssh/lakebox_rsa and
  register it after OAuth completes
- Fix hook: match on sub.Name() not sub.Use (Use includes args)
- Export EnsureAndReadKey and RegisterKey for use by auth hook
- Update help text

Co-authored-by: Isaac
Everything after -- is passed directly to the ssh process, enabling:
  lakebox ssh -- echo hello          # run command and return
  lakebox ssh <id> -- cat /etc/os-release
  lakebox ssh -- -L 8080:localhost:8080  # port forwarding

Co-authored-by: Isaac
After 'lakebox auth login --host <url>', the post-login hook now
constructs the workspace client directly from the --host/--profile
flags instead of using MustWorkspaceClient (which started with an
empty config and fell back to the DEFAULT profile).

All lakebox commands now use a mustWorkspaceClient wrapper that reads
the last-login profile from ~/.databricks/lakebox.json, so 'lakebox ssh'
uses the correct profile without requiring --profile on every invocation.

Also adds install.sh and upload.sh scripts.
Fix workspace client init after login, persist last profile
Merge kelvich's workspace client fix. Add -- passthrough support so
extra args (remote commands, port forwarding, ssh flags) are passed
directly to the ssh process.

Co-authored-by: Isaac
Single cyan accent color throughout. Bold for IDs, dim for metadata.
Braille spinner with elapsed time during async operations.

- create: animated spinner during provisioning
- list: aligned columns with colored status, cyan bold for running
- status: clean field layout
- delete: spinner during removal
- ssh: spinner during connection
- register: spinner during key registration
- Shared ui.go with all primitives

Co-authored-by: Isaac
The lakebox manager moved its REST surface to a proto-defined service with
JSON transcoding (databricks-eng/universe#1839855 + follow-ups). That
changed three things this CLI was depending on:

1. JSON field name: each Lakebox message now serializes as `lakeboxId`
   (proto3 lowerCamelCase default), not `name`. List/status/create were
   parsing into `Name string \`json:"name"\`` and silently getting the
   empty string for every entry — the visible symptom was `lakebox list`
   showing rows with blank ID columns.

2. Status codes: proto-transcoded handlers return 200 OK uniformly. The
   CLI was checking 201 Created on POST /api/2.0/lakebox and 204
   NoContent on DELETE, both of which now look like errors.

3. Key registration moved to its own top-level collection at
   /api/2.0/lakebox-keys (was /api/2.0/lakebox/register-key), to avoid a
   path collision with /api/2.0/lakebox/{lakebox_id}.

Drop the now-unused `extractLakeboxID` helper — the wire field is the
customer-facing ID directly.

Verified against dev-aws-us-west-2: list, status, create, delete all
work end-to-end. register hits a separate manager-side issue (stale
UserKey records in TiDB that the new schema can't deserialize) — not
fixed here.

Co-authored-by: Isaac
Reynold's restructure (databricks-eng/universe#1874214) nested the two
lakebox resources under the service namespace — moving sandboxes from
/api/2.0/lakebox to /api/2.0/lakebox/sandboxes and SSH keys from
/api/2.0/lakebox-keys to /api/2.0/lakebox/ssh-keys — and renamed the
resource type from Lakebox to Sandbox, which surfaces on the wire as
sandboxId / sandboxes (was lakeboxId / lakeboxes).

CLI still pointed at the old paths and decoded the old field names, so
list / status / create returned empty IDs and 404s. Fix both endpoint
constants, rename the request/response types and fields to match the
proto, and update the four call sites in create / list / ssh / status.
User-facing copy ("Lakebox …") is unchanged — the product is still
Lakebox; only the resource type renamed.

Verified end-to-end against dev-aws-us-west-2: create / list / status
/ delete all work; ssh passthrough works.

Co-authored-by: Isaac
Surfaces the new per-sandbox auto-stop knobs the manager added
(databricks-eng/universe#1875183) so users can see at a glance how long
their sandbox will live before the watchdog reaps it.

- `sandboxEntry` gains pointer fields `IdleTimeoutSecs` and `Persist` so
  we keep the proto3 explicit-presence semantics ("not in response" vs
  "explicitly set to 0 / false").
- `autoStopLabel()` collapses the policy to one short token:
  - `persist == true` → `never`
  - `idle_timeout_secs > 0` → compact duration (`90s`, `15m`, `2h`,
    `1h30m`)
  - otherwise → the manager's global default (10m), rendered
    explicitly so the column never says `default`
- `lakebox list` adds an AUTOSTOP column between STATUS and DEFAULT.
- `lakebox status` adds an `autostop` field after `fqdn`.

Verified end-to-end against dev-aws-us-west-2 — list and status both
render `10m` for sandboxes with no per-record override.

Co-authored-by: Isaac
Surfaces the per-sandbox auto-stop knobs the manager added in
databricks-eng/universe#1875183 so users can flip them from the CLI
instead of curl + JSON.

  lakebox config <id> --idle-timeout 15m       # 15-minute timeout
  lakebox config <id> --idle-timeout 1h30m     # any Go duration
  lakebox config <id> --idle-timeout 0         # clear → manager default
  lakebox config <id> --persist                # never auto-stop
  lakebox config <id> --persist=false          # back to timeout path
  lakebox config <id> --idle-timeout 30m --persist=false   # combined

Implementation notes:

- `updateBody` is the inner Sandbox sent in the PATCH body. The proto's
  `(google.api.http)` declares `body: "sandbox"`, so the HTTP body is
  the inner `Sandbox` message, NOT a `{"sandbox": {...}}` envelope.
  First wired-up version got this wrong and the manager rejected with
  "unknown field `sandbox`" — kept the type comment to flag the gotcha
  for the next reader.
- `IdleTimeoutSecs` carries `,string` JSON tag because proto3 JSON
  canonical form serializes int64 as a quoted string. The manager
  accepts both bare-number and quoted-string on input but always
  emits quoted on output, so without the tag we hit "cannot unmarshal
  string into Go struct field … int64" on the response read-back.
- Pointer fields (`*int64`, `*bool`) carry proto3 explicit-presence
  through to the wire — only the flags the user actually passed get
  emitted, so a `--persist`-only invocation does not clobber an
  existing idle_timeout (and vice-versa).
- Client-side range pre-flight (`[60s, 86400s]` plus the 0 clear
  sentinel) mirrors the manager's `MIN_IDLE_TIMEOUT_SECS` /
  `MAX_IDLE_TIMEOUT_SECS` constants so users get a clearer error
  than the server's `INVALID_ARGUMENT`.

Verified end-to-end against dev-aws-us-west-2:
  config --idle-timeout 15m  → status shows `15m`
  config --persist           → status shows `never`
  config --idle-timeout 0 --persist=false → status shows `10m`

Co-authored-by: Isaac
Tracks the matching rename in the lakebox manager
(databricks-eng/universe#1875183 follow-up). The manager-side flag
moved from `persist` to `no_autostop` because the original name
conflicted with the storage-persistence concept already in this
codebase.

CLI changes:
  --persist          → --no-autostop
  --persist=false    → --no-autostop=false

Plus a help-text note on the manager's new auto-clear behavior:
setting `--idle-timeout` to a non-zero value in a follow-up call
clears `--no-autostop` automatically, on the assumption that the
caller wants timeout-based stopping back. The CLI itself does not
need any extra logic for this — the manager handles it server-side
based on field presence in the PATCH body, and the CLI's existing
"omit unset flags from the wire payload" semantics (proto3
explicit-presence via *bool / *int64) feed straight into that.

Verified the marshal output matches what the new manager expects:
  --no-autostop      → {"sandbox_id":"x","no_autostop":true}
  --idle-timeout 15m → {"sandbox_id":"x","idle_timeout_secs":"900"}
  no flags           → {"sandbox_id":"x"}                      (rejected)

End-to-end against staging blocked until the manager PR rolls out.

Co-authored-by: Isaac
Tracks the matching change in the lakebox manager
(databricks-eng/universe#1875183) which moved the per-sandbox idle
timeout off `optional int64 idle_timeout_secs = 7` and onto
`optional google.protobuf.Duration idle_timeout = 7`. Drops the
sentinel-overloaded int64 in favor of a duration-typed field.

Wire shape:
- Response field is now `idleTimeout` carrying a proto3-canonical
  Duration string (e.g. `"900s"`); parsed into seconds via
  `time.ParseDuration` for the autostop column.
- Request body sends `idle_timeout` as the same string format.

The CLI flag stays `--idle-timeout` (Go duration string in / Go
duration string out); only the wire encoding changes. `list` and
`status` show the manager's global default for any sandbox whose
per-record value isn't yet visible under the new field name — that's
deliberate forward-compat behavior so an older manager + newer CLI
combination just degrades to showing the default rather than crashing.

Co-authored-by: Isaac
- ssh: auto-pick uw2.s.dbrx.dev when the workspace host has `.staging.` in it,
  otherwise keep using prod uw2.dbrx.dev. `--gateway` still overrides.
- api: when the workspace host carries a `?o=<id>` selector or the SDK config
  has a workspace_id, send `X-Databricks-Org-Id` so multi-workspace gateways
  (dogfood.staging.databricks.com) route the request to the right workspace.
  Without it the gateway rejects PATs with "Credential was not sent or was
  of an unsupported type for this API".

Co-authored-by: Isaac
@pietern
pietern temporarily deployed to test-trigger-is May 8, 2026 14:39 — with GitHub Actions Inactive
@pietern
pietern temporarily deployed to test-trigger-is May 8, 2026 14:39 — with GitHub Actions Inactive
@pietern
pietern temporarily deployed to test-trigger-is May 8, 2026 14:40 — with GitHub Actions Inactive
@pietern
pietern temporarily deployed to test-trigger-is May 8, 2026 14:40 — with GitHub Actions Inactive
@pietern pietern changed the title demo: lakebox subcommand demo: lakebox May 8, 2026
@pietern
pietern temporarily deployed to test-trigger-is May 8, 2026 14:51 — with GitHub Actions Inactive
@pietern
pietern temporarily deployed to test-trigger-is May 8, 2026 14:51 — with GitHub Actions Inactive
@pietern
pietern temporarily deployed to test-trigger-is May 8, 2026 14:53 — with GitHub Actions Inactive
@pietern
pietern temporarily deployed to test-trigger-is May 8, 2026 14:53 — with GitHub Actions Inactive
pietern added 4 commits May 8, 2026 17:01
…onments

Brings in the original cmd/lakebox/* sources from
#4930 with full commit-history
attribution. Subsequent commits adapt the standalone CLI into a
'databricks lakebox' subcommand, replace hand-rolled HTTP/spinner/color
plumbing with libs primitives, and add unit tests.
Wire the cmd/lakebox tree from #4930 into the main CLI:

 - cmd/cmd.go registers lakebox.New() under the 'development' command
   group alongside bundle and sync.
 - cmd/fuzz_panic_test.go adds 'lakebox' to manualRoots so TestCountFuzz
   doesn't fuzz hand-written commands as if they were auto-generated.
 - cmd/lakebox tree: the original PR's standalone-CLI scaffolding is
   adapted for subcommand use — drop the auth-login hijacking and its
   helper exports, drop the 'last_profile' state field that only mattered
   when lakebox owned the whole CLI, switch PreRunE to root.MustWorkspaceClient
   directly, and update help text from 'lakebox foo' to
   'databricks lakebox foo' throughout.

Also conforms cmd/lakebox to project lint rules: env.UserHomeDir(ctx)
in place of os.UserHomeDir, errors.Is(err, fs.ErrNotExist) instead of
os.IsNotExist, atomic.Bool over sync.Once in the spinner gate, errors.New
for static error strings.

Co-authored-by: Isaac
Replace the hand-rolled braille spinner, TTY detection, and stderr
plumbing with the existing cmdio facilities:

- spin(ctx, msg) wraps cmdio.NewSpinner — capability-aware, runs through
  the same Bubble Tea program slot as other CLI spinners. ok/fail markers
  are logged via cmdio.LogString after Close.
- ok(ctx, ...) and warn(ctx, ...) are now ctx-based and route to stderr
  through cmdio rather than taking a writer. Call sites drop their
  cmd.ErrOrStderr() locals where they were only used for these helpers.
- field/blank still take an io.Writer because callers need to target
  stdout for structured output (list, status, config).

Drops the local isTTY, atomic.Bool spinner gate, and ticker goroutine.

Co-authored-by: Isaac
Drops the cyan/bold/dim/reset constants and the local accent/bold/dim
wrappers in favor of cmdio.Cyan and cmdio.HiBlack, which respect the
SupportsStdoutColor capability check. Bold-for-emphasis is folded into
Cyan since cmdio does not expose a Go-level Bold helper today; visually
this means lakebox IDs and emphasized command names render in cyan
rather than uncolored bold, consistent with the rest of the CLI.

field/status now take a context so they can call cmdio.HiBlack /
cmdio.Cyan; their writer parameter stays for callers that target stdout.

Co-authored-by: Isaac
@pietern
pietern temporarily deployed to test-trigger-is June 3, 2026 12:22 — with GitHub Actions Inactive
@pietern
pietern temporarily deployed to test-trigger-is June 3, 2026 12:22 — with GitHub Actions Inactive
## Summary

Replaces the bare `warn(ctx, "Starting <id>…")` notice in `lakebox ssh`
with an explicit `api.start` + `waitForRunning` sequence before exec-ing
ssh.

### Why

**The user-principal that triggers the start should be the user, not the
gateway.** Today the SSH gateway implicitly starts a stopped sandbox on
connect, which means audit trails, billing attribution, and any
server-side hooks see the *gateway service principal* as the actor — not
the human who actually wanted the sandbox up. Routing the start through
`api.start` from the CLI carries the workspace credential of the running
user, so the StartSandbox RPC is attributed to them.

Nice-to-haves that fall out for free:
- Visible spinner with elapsed seconds instead of an opaque "ssh hangs
for 5 minutes" wait.
- Deterministic timeout (the same one `databricks lakebox start` uses)
rather than racing the SSH gateway's cold-start window.
- Already-transitioning states (`Creating`, `Starting`) get polled
cleanly via the same `ensureRunning` helper — no double-start RPC.
- The fresh `sandboxEntry` we get back lets us refresh the cached
`GatewayHost` if it changed during the start.

## User-visible change

Before:
```
! Starting happy-panda-1234… (may take a few minutes)
⠋ Connecting to happy-panda-1234…   ← hangs silently, can time out at the ssh layer
```

After:
```
⠋ Starting happy-panda-1234… (8s)
⠋ Starting happy-panda-1234… (10s)
…
✓ Started happy-panda-1234
⠋ Connecting to happy-panda-1234…   ← fast, sandbox already running
```

## Test plan

- [x] `go test ./cmd/lakebox/...` passes
- [x] `go build ./...` clean
- [ ] Live test against a stopped sandbox (deferred — 5–10 min wall
clock per attempt)
- [ ] Confirm StartSandbox audit-log entry is attributed to the user,
not the gateway

This pull request and its description were written by Isaac.
…y key registered before ssh (#5458)

## Summary

Three changes on the `lakebox register` → `lakebox ssh` path that
together make *"register once, then everything just works"* actually
true.

### 1. `register` writes a `Host lakebox-gw` alias to `~/.ssh/config`

Gated by a one-time Y/n prompt:

```
? Add a `Host lakebox-gw` alias to /home/me/.ssh/config? This lets editor
  Remote-SSH (VS Code / Cursor) and `ssh <id>@lakebox-gw` connect without
  further setup. [Y/n]
```

Re-runs are silent — the prior `Include` line signals opt-in, so we just
refresh the managed file's contents. The alias name and shape match [the
workspace UI's "First time setup?"
disclosure](https://github.com/databricks-eng/universe/pull/2042253), so
editor Remote-SSH deep links resolve identically whether the user set up
via the CLI or pasted the UI snippet.

Layout: managed block lives in `~/.ssh/databricks-lakebox`; only one
marker-bracketed `Include` line lands in the user's main config,
trivially greppable and removable. The `-o` flags in the alias mirror
`buildSSHArgs` in ssh.go so connections through the alias behave
identically to `databricks lakebox ssh`.

In non-interactive contexts (no TTY), we skip the write rather than fail
— `lakebox ssh` still works via argv-explicit flags; only IDE Remote-SSH
from the workspace UI needs the alias.

### 2. Generated keys switched to ed25519

`~/.ssh/lakebox_ed25519` instead of `lakebox_rsa`. The lakebox gateway
is a modern Go SSH server — no compatibility reason to prefer RSA.
ed25519 also matches what the UI work recommends and what the "First
time setup?" snippet defaults to. No backward-compat path because the
CLI isn't shipped yet.

### 3. `lakebox ssh` pre-flights an `api.listKeys` check

The gateway already does the check (see `verify_ssh_key` in
`lakebox/src/ssh/handler.rs`), but the SSH protocol's reply surface is
too narrow to carry the reason back to the user:
`SSH_MSG_USERAUTH_FAILURE` has no free-form reason field, and
`SSH_MSG_USERAUTH_BANNER` is widely swallowed by client wrappers. So
*every* publickey-stage rejection — "unknown key", "key registered but
you don't own this sandbox", "ESM is down" — looks like the same flat
`Permission denied (publickey)` to the user.

The HTTP API is the only out-of-band wire that can surface a structured
pointer. The CLI lists registered hashes (auth'd via OAuth/PAT, separate
from the SSH key), computes the local key's hash, and if not present
prints "run `databricks lakebox register`" before ever opening the SSH
socket. `listKeys` errors fall through with a warning so a transient API
hiccup doesn't block a connection the gateway could still route.

## Test plan

- [x] `go test ./cmd/lakebox/...` passes (new tests for block shape,
idempotency, 0600 perms, marker detection, user-config preservation,
managed-block-precedes-user-content ordering)
- [x] `go build ./...` clean
- [x] `gofmt -l cmd/lakebox/` clean
- [x] `databricks lakebox register --help` shows the new behaviour
- [ ] Live: register on a fresh machine; verify ~/.ssh/config has the
Include line and ~/.ssh/databricks-lakebox has the `lakebox-gw` block
- [ ] Live: `ssh <sandbox-id>@lakebox-gw` connects without any extra
flags
- [ ] Live: "Open in VS Code" from the workspace UI connects (via
Remote-SSH resolving the alias)
- [ ] Live: delete the registered key from the UI; verify `lakebox ssh`
errors with the actionable message immediately (not after the SSH
handshake timeout)

This pull request and its description were written by Isaac.
…ion (#5459)

## Summary

Pre-flight cleanup for shipping `cmd/lakebox/` to the public CLI
release. No behavior change — pure comments + dead-file removal. After
this, the lakebox subfolder is appropriate for the public
`databricks/cli` repo.

### What got changed

| Category | Files | Change |
|---|---|---|
| Drop employee-name attributions | `config.go`, `list.go`, `start.go`,
`state.go` | Removed comments crediting specific Databricks employees.
Kept the *technical* context they hung off. |
| Drop bug-bash form IDs | `ssh.go`, `ssh_test.go`, `start.go` | `F10`,
`F22` references were internal coordination tags meaningless to external
readers. |
| Drop private-repo paths | `api.go`, `config.go`, `ssh.go` |
`lakebox/src/...`, `universe#1966484`, `lakebox.proto`,
`LakeboxService.*` proto refs — external readers can't navigate to any
of them. |
| Drop internal subsystem names | `api.go` | `ESM`, `LakeboxChecker`,
`lakeboxCheckerEnabled` SAFE flag, "dogfood staging" |
| Drop roadmap mention | `ssh.go` | "Both are dev-tier listeners; there
is no prod listener yet" — advertising rollout state |
| Fix help-text inconsistency | `status.go` | Examples now use
`databricks lakebox status …` to match sibling commands |
| Delete lakebox-specific install script | `internal/bugbash/install.sh`
| Added in #5359 specifically for the demo-lakebox snapshot lifecycle;
no longer wanted in the public repo. |

### What was deliberately kept

- `internal/bugbash/exec.sh` and `internal/bugbash/README.md` — these
predate lakebox (authored by Pieter in #1844, May 2024; extended by
Lennart and Andrew). They expose a curl-pipe-bash URL that may be
referenced elsewhere; removing them is out of scope for a
lakebox-comment cleanup. Whoever revisits the public surface of
`internal/bugbash/` can decide separately.
- The SSH-protocol explanation around `verifyKeyRegistered` —
`USERAUTH_FAILURE` reason-poverty / `USERAUTH_BANNER` swallowing is
genuinely educational and applies to any SSH gateway, not just lakebox.
- The `validate_sandbox_id` / `foo;rm -rf /` security context —
load-bearing.
- The proto3 optional / Duration string-encoding notes — generic
protobuf wire knowledge, useful for any future maintainer.
- The `databricks-sdk-go` `makeRequestBody` quirk reference —
`databricks-sdk-go` is a public repo.
- Pieter's `cmdio.Width` / `cmdio.PadRight` width-aware padding
rationale.

### Notes for follow-up

- `release-build.yml` triggers on `bugbash-*` branches. This is a CI
convention pattern (matches any branch starting with `bugbash-`), not a
link to the deleted install.sh. Benign enough to keep — one-line
follow-up if we want it scrubbed.

## Test plan

- [x] `go test ./cmd/lakebox/...` passes
- [x] `go build ./...` clean
- [x] `gofmt -l cmd/lakebox/` clean
- [x] No internal-attribution strings left in `cmd/lakebox/` (verified
by grep sweep for names, F-IDs, internal paths)

This pull request and its description were written by Isaac.
…Hash` trailing-newline, plus UX (#5460)

## Summary

Three correctness fixes and several small UX cleanups, surfaced by a
final ship-readiness audit. Bundled because each is small and they all
sit on the same `register → ssh` path that gets exercised together.

## BLOCKERs (real bugs)

### 1. `start.go` documented timeout was half the actual timeout
`Long:` said *"blocks until the sandbox reaches Running (or up to 5
minutes)"* while `startWaitTimeout = 10 * time.Minute`. Fix the text.

### 2. `create.go` clobbered the saved default on any transient API
error
Previous code:
```go
if _, err := api.get(currentDefault); err != nil {
    shouldSetDefault = true   // any error
}
```
A 500, network blip, or rate-limit silently overwrote the user's chosen
default. Every sibling (`delete`, `default`, `ssh`) already uses
`errors.Is(err, apierr.ErrNotFound)` — `create` now matches.

### 3. `keyHash` included a trailing newline in its sha256 input
`.pub` files end with `\n`. Locally-generated keys are fine because
`ssh-keygen -C "lakebox"` always adds a comment that gets stripped
before the `\n` is reached. But a user-supplied key without `-C` would
produce a hash that doesn't match the server's, making
`verifyKeyRegistered` falsely report "key not registered". Fix:
`strings.TrimSpace` the input first. Two new test cases pin the behavior
for `\n` and surrounding whitespace.

## UX fixes

- **`status.go` exposed `fqdn`** — `api.go` documents FQDN as the
manager's internal routing host, not user-actionable. Dropped from
non-JSON output (still in `-o json` for completeness).
- **`status.go` 404 message** — wrapped as "failed to get lakebox X:
..." while `delete`/`default`/`ssh` returned the friendlier "no lakebox
named X — `databricks lakebox list` shows available IDs". Now
consistent.
- **`ui.go warn()` switched to yellow** — was cyan, indistinguishable
from the success ✓ marker.
- **`delete.go` clean cancel** — returning `errors.New("aborted")`
surfaced as `Error: aborted` + non-zero exit. Now prints `Cancelled.`
and returns nil.
- **Consistent sentence-case** across all `warn()` messages.

## Test plan

- [x] `go test ./cmd/lakebox/...` passes (including 2 new keyHash cases
for trailing-newline and leading/trailing whitespace)
- [x] `go build ./...` clean
- [x] `gofmt -l cmd/lakebox/` clean

## Out of scope

Acceptance test scaffold for `cmd/lakebox/` (the bigger gap surfaced by
the same audit) is its own PR — these are zero today and adding them is
a separate larger change.

This pull request and its description were written by Isaac.
#5475)

## Summary

Two `os.IsNotExist(err)` callsites in `cmd/lakebox/sshconfig.go` slipped
through before `./task lint` was re-run. The repo's golangci-lint config
forbids `os.IsNotExist` (`forbidigo` rule) in favor of `errors.Is(err,
fs.ErrNotExist)`.

The replacement is functionally identical — `fs.ErrNotExist` is the
sentinel the standard library wraps for "file does not exist", and
`errors.Is` unwraps through Go's error chain so wrapped variants are
also handled correctly.

## Test plan

- [x] `./task lint` is now clean (was `2 issues: forbidigo`)
- [x] `go test ./cmd/lakebox/...` passes

This pull request and its description were written by Isaac.
## 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.
)

## Summary

Second batch of lakebox acceptance tests, following #5474. Covers 14
leaf scenarios on the write/connection-path commands and JSON output
shapes that weren't in the first batch.

| Test | What it locks down |
|---|---|
| `list/json` | `-o json` shape (no FQDN field, gatewayHost present,
idleTimeout serialized as Duration string) |
| `status/running` | Happy-path text output with all rendered fields |
| `status/json` | JSON shape — verifies the FQDN-removal in #5460 didn't
get reverted |
| `ssh-key/list/with-entries` | Multi-row table including unnamed key
falling back to `(unset)` |
| `ssh-key/delete/success` | DELETE wire route + confirmation message |
| `delete/success` | Happy path with `--auto-approve` to bypass the
prompt |
| `delete/not-found` | 404 → friendly `no lakebox named X` (the
consistency fix from #5460) |
| `delete/no-tty-no-auto-approve` | Non-interactive context fast-fails
pointing at the `--auto-approve` flag |
| `default/set` | Set default → confirmation message |
| `default/not-found` | 404 → friendly error (same message as delete) |
| `start/already-running` | API returns Running → CLI short-circuits
with "Already running X" (skips waitForRunning) |
| `stop/success` | Happy path with refreshed status |
| `config/update-name` | PATCH wire route + post-update output rendering
|
| `create/with-name` | POST wire route + stdout ID + default-set side
effect |

## Two test-infrastructure additions

1. **`acceptance/cmd/lakebox/script.prepare`** sets `HOME=$TEST_TMP_DIR`
so each test's `~/.databricks/lakebox.json` is isolated. Without this,
parallel lakebox tests race each other writing to the shared HOME-rooted
state file and one read sees the other's half-written content. Auth is
unaffected — the framework passes `DATABRICKS_HOST` / `DATABRICKS_TOKEN`
explicitly via env, so the CLI doesn't fall back to `~/.databrickscfg`.

2. **`EnvMatrix.DATABRICKS_BUNDLE_ENGINE = []`** in the parent
`test.toml` overrides the root matrix to skip the per-engine variants.
Lakebox is engine-independent and running both variants in parallel was
the original symptom of the HOME race.

Two of the ssh-key tests also pin the test key hash via a high-priority
`[[Repls]]` entry so the framework's default 3+-digit-run → `[NUMID]`
regex doesn't shred it mid-hash.

## Test plan

- [x] `go test ./acceptance -run TestAccept/cmd/lakebox` — all 20 tests
pass (6 from #5474 + 14 new)
- [x] `./task lint` clean
- [x] Re-running with `-update` and then without is idempotent (no
drift)

## Still out of scope

- `register` — generates and reads real SSH keys via `ssh-keygen`; needs
a heavier fixture / process stub
- `ssh` — `execv`s into ssh; not testable through this framework

This pull request and its description were written by Isaac.
## Summary

`TestWriteManagedConfigCreatesWithRightPerms` (added in #5458) fails on
Windows CI: `os.Stat` reports `0o666` regardless of what was passed to
`os.WriteFile` because Windows doesn't honor Unix permission bits.

`state_test.go` already had this carve-out for the equivalent state-file
test (lines 154-159). Mirroring the same `runtime.GOOS != "windows"`
check here.

Unix tests still pin `0o600`; only the Windows path skips the assertion.

## Failure

From the demo-lakebox `task test (windows, direct)` run:

```
FAIL cmd/lakebox.TestWriteManagedConfigCreatesWithRightPerms (0.01s)
    sshconfig_test.go:48:
        Error:      Not equal:
                    expected: 0x180   (0o600)
                    actual  : 0x1b6   (0o666)
```

## Test plan

- [x] `go test ./cmd/lakebox/... -run
TestWriteManagedConfigCreatesWithRightPerms` passes on Linux
- [x] `./task lint` clean
- [ ] Windows CI on this PR (the original symptom)

This pull request and its description were written by Isaac.
akshaysingla-db and others added 6 commits June 9, 2026 12:22
## Summary

User-visible rename. The command is now `databricks sandbox …` instead
of `databricks lakebox …`. Internals follow the same naming: Go package,
directory, identifiers, state file, SSH key file, SSH config alias, and
managed include file all switch from `lakebox` to `sandbox`.

The **wire-format API path is the one exception** — `sandboxAPIPath` and
`sandboxKeysAPIPath` still point at `/api/2.0/lakebox/...` because the
server-side rename is happening separately. A comment on the const block
in `api.go` flags this as the sole intentional `lakebox` string in the
codebase.

## What changed

| Layer | Before | After |
|---|---|---|
| CLI command | `databricks lakebox …` | `databricks sandbox …` |
| Go package + directory | `cmd/lakebox/` (25 files) | `cmd/sandbox/` |
| Identifiers | `lakeboxAPI`, `newLakeboxAPI`, `lakeboxKeyPath`, etc. |
`sandboxAPI`, `newSandboxAPI`, `sandboxKeyPath`, etc. |
| State file | `~/.databricks/lakebox.json` |
`~/.databricks/sandbox.json` |
| SSH key | `~/.ssh/lakebox_ed25519` | `~/.ssh/sandbox_ed25519` |
| Managed include | `~/.ssh/databricks-lakebox` |
`~/.ssh/databricks-sandbox` |
| SSH config alias | `Host lakebox-gw` | `Host sandbox-gw` |
| Include-block markers | `# >>> databricks lakebox >>>` | `# >>>
databricks sandbox >>>` |
| Acceptance dir | `acceptance/cmd/lakebox/` (20 scenarios) |
`acceptance/cmd/sandbox/` |
| API path values | `/api/2.0/lakebox/sandboxes` and
`/api/2.0/lakebox/ssh-keys` | **unchanged** — server rename pending |

`cmd/cmd.go` import + `AddCommand` updated; `cmd/fuzz_panic_test.go`
manual-roots skip-list updated. Workspace UI needs a matching update to
the SSH alias (`lakebox-gw` → `sandbox-gw`) so the "First time setup?"
disclosure and the editor deep-links stay in sync — flagging for the UI
side.

## Migration impact

Existing CLI users (snapshot installs from
`internal/bugbash/install.sh`) will need to re-register on first use
after the upgrade — old key, state, and SSH-config entries are orphaned
under the lakebox names. Low risk because we haven't shipped publicly
yet.

## Test plan

- [x] `go test ./cmd/sandbox/...` passes (22 unit tests)
- [x] `go test ./acceptance -run TestAccept/cmd/sandbox` passes (20
acceptance tests, all goldens regenerated)
- [x] `go test ./cmd/...` passes overall (no other suite indexed the
lakebox name)
- [x] `go build ./...` clean
- [x] `./task lint` clean
- [x] `grep -rE '\blakebox\b'` outside `internal/bugbash/` returns only
the intentional `/api/2.0/lakebox` API-path values

This pull request and its description were written by Isaac.
## Summary

Follow-up to #5487. The bulk sed in that PR incorrectly rewrote 14
documentation comments that name the actual HTTP endpoints — the wire
path is `/api/2.0/lakebox/...` (server-side rename pending; see the
`sandboxAPIRoot` const), but the doc comments on `list` / `get` /
`create` / `update` / `delete` / `start` / `stop` / `registerKey` /
`listKeys` / `deleteKey` / `listResponse` / `registerKeyRequest` were
left saying `/api/2.0/sandbox/...`.

That mismatch is misleading — any reader treating the comment as
canonical would look for a non-existent endpoint. Restoring all 14 to
the actual wire path. Also restores the `keyhash_test.go` header comment
that documents where the algorithm was verified.

Behavior unchanged; comments now match the `[[Server]] Pattern` strings
in the acceptance tests and the URLs the CLI actually sends.

## Test plan

- [x] `go test ./cmd/sandbox/...` passes
- [x] `go build ./...` clean

## Note

This is stacked on #5487 (`lakebox` → `sandbox` rename). Until that
merges, the diff here shows the rename + the doc-comment fix together.
After #5487 lands on `demo-lakebox`, this PR's diff collapses to just
the 14 doc-comment line edits.

This pull request and its description were written by Isaac.
## Summary

Pairs with a server-side change that adds `gateway_host` to the `SshKey`
proto and stamps it on every `CreateSshKey` / `ListSshKeys` /
`UpdateSshKey` response. With that landed, the CLI learns the
workspace's gateway host on the first key call and stops guessing.

The previous flow had two real bugs:

1. **Hardcoded heuristic** — `resolveGatewayHost()` returned
`uw2.dbrx.dev` or `ue1.s.dbrx.dev` based on the workspace URL pattern.
Real workspaces use other gateways (e.g.
`us-west-2.service-direct.dev.databricks.com`), so the heuristic was
wrong for at least one customer-facing case.
2. **Alias / deep-link mismatch** — the CLI wrote a `Host sandbox-gw`
block; UI deep links emit `ssh-remote+<id>@<actual-gateway>`. Different
host keys → SSH falls through to defaults (port 22) → connection fails.

Both go away when the CLI keys the block on the literal `gateway_host`
the server hands back.

## What changed

| File | Change |
|---|---|
| `api.go` | `registerKey` now returns `*sshKeyEntry` (carrying
`GatewayHost`) instead of just `error`. Adds `GatewayHost` to
`sshKeyEntry`. |
| `register.go` | Captures the response, caches `GatewayHost` via
`setGatewayHost`, and passes it to `maybeWriteSSHConfig` directly. No
probe `list`/`create` call, no heuristic. |
| `sshconfig.go` | `maybeWriteSSHConfig` takes the gateway as a
parameter (was: derived inside). Block shape now matches the workspace
UI's "First time setup?" snippet exactly. |
| `sshconfig_test.go` | `TestBuildSSHConfigBlockShape` asserts the
minimal form (positive + negative). |
| `acceptance/.../ssh-key/list/with-entries/test.toml` | Mock returns
`gatewayHost` on each key, matching the new server contract. |

## New SSH config block

Before (9 lines, synthetic alias, several overrides):
```
# Managed by `databricks sandbox register`.
# Manual edits will be overwritten on the next run.
Host sandbox-gw
    HostName uw2.dbrx.dev
    Port 2222
    IdentityFile ~/.ssh/sandbox_ed25519
    IdentitiesOnly yes
    StrictHostKeyChecking no
    UserKnownHostsFile /dev/null
    LogLevel ERROR
```

After (5 lines, literal hostname, no overrides beyond Port + Identity):
```
# Managed by `databricks sandbox register`. Manual edits will be overwritten.
Host us-west-2.service-direct.dev.databricks.com
    Port 2222
    IdentityFile ~/.ssh/sandbox_ed25519
    IdentitiesOnly yes
```

Deliberately dropped (now SSH defaults):
- `Host sandbox-gw` alias — Host is the literal hostname; IDE deep links
resolve directly.
- `HostName` — would be a redundant self-reference.
- `StrictHostKeyChecking no` + `UserKnownHostsFile /dev/null` — IDE /
raw-ssh use should follow SSH's TOFU default; the CLI's own `sandbox
ssh` still passes `-o` flags via argv.
- `LogLevel ERROR` — default fine for these flows.

## Empty-state behavior

Even with zero sandboxes on the workspace, `register` works —
`gateway_host` is workspace-scoped (per the server change). If the
server is somehow older and omits the field, we log a clear "skipped SSH
config update — server did not return a gateway host" and proceed; the
key is still registered.

## Test plan

- [x] `go test ./cmd/sandbox/...` passes
- [x] `go test ./acceptance -run TestAccept/cmd/sandbox` passes
- [x] `go build ./...` clean
- [x] `./task lint` clean
- [ ] Live test against staging after the server change deploys:
`databricks sandbox register` writes a block keyed on the real gateway
host; "Open in VS Code" from the workspace UI connects without further
setup.

This pull request and its description were written by Isaac.
Changes that exclusively touch `cmd/sandbox/...` or `acceptance/cmd/sandbox/...` now trigger a dedicated `test-sandbox` CI job instead of the full test matrix, following the same testmask pattern as `test-pipelines` and the experimental flows.

Co-authored-by: Isaac
A plain HOME export does not isolate the state file on Windows, where
os.UserHomeDir() reads USERPROFILE. The dedicated test-sandbox CI job
runs all sandbox tests concurrently, making the shared-state race
likely enough to fail the Windows job on every run.

Co-authored-by: Isaac
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants