Akshay/lakebox staging gateway ue1 by akshaysingla-db · Pull Request #4 · shuochen0311/cli

akshaysingla-db · 2026-05-20T18:57:18Z

Changes

Why

Tests

## Summary - Add license comments for 7 direct dependencies that were missing them (`charmbracelet/huh`, `golang.org/x/{exp,mod,oauth2,sync,sys,text}`) This pull request was AI-assisted by Isaac.

## Summary - Move `gopkg.in/yaml.v3` from a standalone indirect `require` line into the grouped indirect block via `go mod tidy` - databricks#4353 migrated direct imports to `go.yaml.in/yaml/v3` and moved `gopkg.in/yaml.v3` into the grouped indirect block - databricks#4289 added a direct dependency on `gopkg.in/yaml.v3` (via `palantir/pkg/yamlpatch`), pulling it back out as a standalone `require` line - databricks#4400 marked it as `// indirect` but left it as a standalone line ## Test plan - [x] `go mod tidy` produces no diff This pull request was AI-assisted by Isaac.

…atabricks#4940) ## Summary - Normalize all license comments in `go.mod` to use standard SPDX identifiers (e.g. `Apache 2.0` → `Apache-2.0`, `BSD 3-Clause` → `BSD-3-Clause`) - Use `MIT AND Apache-2.0` for `go.yaml.in/yaml/v3` (different files under different licenses) - Add `internal/build/license_test.go` that parses `go.mod` with `x/mod/modfile` and validates every direct dependency has a valid SPDX license comment ## Test plan - [x] `go test ./internal/build/ -run TestRequireSPDXLicenseComment` passes - [x] Cross-checked all 38 license comments against upstream LICENSE files This pull request was AI-assisted by Isaac.

) ## Summary - Fix NOTICE file: add 13 missing BSD-3-Clause entries, remove 2 stale entries, move 3 wrongly attributed entries from MIT to BSD-3-Clause - Fix typos (`hashicopr` → `hashicorp`, `LIcense` → `License`, `—--` → `---`) - Fix broken license URLs (`palantir/pkg` branch `develop` → `master`, `tailscale/hujson` branch `main` → `master`) - Add `internal/build/notice_test.go` that cross-references go.mod against NOTICE, checking section order and exact entry sets per license ## Test plan - [x] `go test ./internal/build/` — all 5 tests pass - [x] All 38 license URLs verified with curl (HTTP 200, valid license text) This pull request was AI-assisted by Isaac.

## Changes Pass correct ID to permissions API for resources.models. ## Why This resource has two IDs, one coming from Name field - used for all CRUD operations on the resource itself and another numerical ID that needs to be used for permissions request. Since we handle "id" specially in direct, we cannot reference it via $resources.models.foo.id anymore - it always points to CRUD id. So we make a wrapper struct that stores numerical ID under "model_id" and reference that. ## Tests New acceptance test and invariant test. Also fixes deployment of mlops-stack template with direct and enables that test on local (previously it was cloud-only).

## Summary - Replace all `golang.org/x/exp/maps` usage with stdlib equivalents (`maps` and `slices` packages) - `maps.Clone`/`maps.Copy` are drop-in replacements (Go 1.21+) - `maps.Keys`/`maps.Values` return iterators in stdlib (Go 1.23+), so slice-consuming call sites are wrapped with `slices.Collect` - Remove `golang.org/x/exp` from NOTICE file; it remains as an indirect dependency through databricks-sdk-go This pull request was AI-assisted by Isaac.

…om bundle config (databricks#4942) ## Changes Added a test to reproduce run_as not being reset when removed from bundle config ## Why Reproducing databricks#4873

databricks#4946) ## Changes Fixed apps incorrectly using local filesystem path from artifacts path ## Why Fixes databricks#4924 ## Tests Added an acceptance test

- 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

## Summary - Replace `math/rand` with `math/rand/v2` and `Intn` with `IntN` in the two files that used it - Add depguard lint rule to prevent reintroduction of `math/rand` v1 Per the [Go 1.22 release notes](https://go.dev/doc/go1.22#math/rand/v2), `math/rand/v2` is the recommended replacement for `math/rand`. This pull request was AI-assisted by Isaac.

…cks#4953) ## Summary - Replace manual `for i := len(s) - 1; i >= 0; i--` loops with `slices.Backward` (4 sites) - Replace manual swap-based reversals with `slices.Reverse` (2 sites) - Simplify `commandString` by reversing in place instead of copying to a new slice ## Test plan - [x] `go build ./...` - [x] `go test` passes for all 6 affected packages This pull request was AI-assisted 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

## Summary - Remove the validation requiring `--accelerator` when using `--name` (serverless mode) in `ssh connect` - Add a proactive yellow warning at connect time when `--accelerator` is omitted, informing users that serverless CPU is in private preview - Add a reactive hint appended to the error message when the server fails to start without an accelerator ## Test plan - [x] Unit tests pass (`go test ./experimental/ssh/internal/client/`) - [x] Build succeeds (`make build`) - [x] Manual test: `./cli ssh connect --name test-conn --profile p` shows warning and submits job without `--accelerator` - [x] Existing `--accelerator` usage is unaffected (validation for valid values still in place) This pull request was AI-assisted by Isaac.

## Summary Migrate from the `sort` package to the `slices` and `cmp` packages across the codebase. The `slices` package (stable since Go 1.21, extended in Go 1.23) provides type-safe, generic alternatives to `sort`. Go 1.23 added `slices.Sorted` and `slices.SortedFunc`, making the full migration more compelling since the `sort` import can often be dropped entirely. Key replacements: - `sort.Strings` → `slices.Sort` - `sort.Slice` → `slices.SortFunc` - `sort.SliceStable` → `slices.SortStableFunc` The original prompt identified ~25 `sort.Strings` call sites and ~3 `sort.SliceStable` call sites across ~46 files importing `sort`. This is a low-risk, mechanical migration. The `.golangci.yaml` configuration is updated to flag any new usage of the `sort` package. ## Test plan - [x] Existing unit tests pass (`make test`) - [x] Linter passes (`make lint`) This pull request was AI-assisted by Isaac.

… pattern (databricks#4954) ## Summary - Replace the 3-4 line "make slice, range map, append, sort" idiom with the Go 1.23 one-liner `slices.Sorted(maps.Keys(...))` - Remove `utils.SortedKeys` and two package-local duplicates (`sortedKeys`, `sortKeys`), inlining all call sites - Also replace `slices.Collect(maps.Values(...))` + `slices.Sort(...)` with `slices.Sorted(maps.Values(...))` in `dynloc/locations.go` ## Test plan - [x] `go build ./...` passes - [x] Unit tests pass for all affected packages This pull request was AI-assisted by Isaac.

…#4956) ## Summary - Replace remaining `wg.Add`/`wg.Done`/`go` patterns with `wg.Go(...)` in `libs/dagrun`, `libs/filer`, and `libs/sync` - These weren't caught by the `use-waitgroup-go` revive rule because they call `Done()` inside separate functions rather than inline ## Test plan - [x] `go test ./libs/dagrun/ ./libs/filer/ ./libs/sync/` passes This pull request was AI-assisted by Isaac.

…card (databricks#4963) ## Changes Pass changed fields into update mask for apps instead of wildcard ## Why Apps Update API does not support "*" for update mask yet. ## Tests Existing (Cloud) tests pass ``` DATABRICKS_BUNDLE_ENGINE=direct go test ./acceptance -v -run TestAccept/bundle/run/app-with-job === RUN TestAccept ... --- PASS: TestAccept (32.48s) --- SKIP: TestAccept/bundle/run_as (0.00s) --- PASS: TestAccept/bundle/run/app-with-job (0.00s) --- PASS: TestAccept/bundle/run/app-with-job/DATABRICKS_BUNDLE_ENGINE=direct (294.22s) --- PASS: TestAccept/bundle/run/app-with-job/DATABRICKS_BUNDLE_ENGINE=terraform (299.11s) ```

## Why The approval workflow comments currently @-mention everyone it suggests as a reviewer. This generates a lot of notification noise, especially on PRs that touch multiple ownership areas. The comments are useful for showing who should review, but the pings are disruptive. ## Changes Before: approval comments used `@username` for all suggested reviewers, eligible owners, and maintainers, triggering GitHub notifications for each. Now: all mentions are wrapped in backticks (`` `@username` ``), so they render as inline code on GitHub. This preserves the familiar `@` prefix for readability but prevents GitHub from treating them as mentions that trigger notifications. All hardcoded `@` mentions in the comment templates are now routed through a single `fmtLogin` helper controlled by the existing `MENTION_REVIEWERS` flag. ### Example comment (cross-domain PR) > ## Approval status: pending > > ### `/cmd/pipelines/` - approved by `@jefferycheng1` > Files: `cmd/pipelines/foo.go` > > ### `/bundle/` - needs approval > Files: `bundle/config.go` > Eligible: `@bundleowner1`, `@bundleowner2`, `@bundleowner3` > > <sub>Any maintainer (`@maintainer1`, `@maintainer2`, `@maintainer3`) can approve all areas. > See OWNERS for ownership rules.</sub> ## Test plan - All 20 existing unit tests pass - Ran a verification script against 5 scenarios (single domain, cross-domain partial approval, wildcard-only, team-owned paths, three domains mixed) confirming zero bare @-mentions in generated comments

) Also don't mark it as Slow so it runs on PRs and enable it on Cloud.

…bricks#4961) ## Changes Allow run_as for dashboards with embed_credentials set to false. ## Why When dashboard embed_credentials set to false it means dashboard is executed with credentials of the runner of the dashboard meaning we don't really need to prohibit use of a global run_as. Even if this changes later, it will be likely in line with run_as behaviour. Fixes databricks#4394 ## Tests Added an acceptance test

## Summary - Replace all uses of `os.IsNotExist`, `os.IsExist`, and `os.IsPermission` with their `errors.Is` equivalents (`errors.Is(err, fs.ErrNotExist)`, etc.) - Add `forbidigo` linter rules to reject future use of these functions - The [Go documentation](https://pkg.go.dev/os#IsNotExist) recommends `errors.Is` because it unwraps the error chain, while the `os.Is*` functions only recognize errors returned by the `os` package directly ## Test plan - [x] `go build ./...` passes - [x] `make lint` passes with 0 issues This pull request was AI-assisted by Isaac.

## Changes For apps, mark compute_size as backend_default. ## Why Backend sets it to "MEDIUM", causing drift. ## Tests Update testserver to set compute_size default to "MEDIUM", same as cloud, this makes invariant test apps.yml fail locally same as on cloud. This PR then fixes both local and cloud.

…databricks#4964) ## Changes Fix resource references not correctly resolved in apps config section ## Why Fixes databricks#4962 ## Tests Added an acceptance test

…#4958) ## Summary - Migrate all `sync.Once` usage to `sync.OnceFunc`, `sync.OnceValue`, or `sync.OnceValues` - Add a `forbidigo` lint rule to prevent `sync.Once` from being reintroduced - Deduplicate config loading in `bundle/direct/dresources` This pull request was AI-assisted by Isaac. --------- Co-authored-by: simon <4305831+simonfaltum@users.noreply.github.com>

…atabricks#4929) ## Summary - The SDK's `ConfigType()` classifies hosts by URL prefix (`accounts.*` → account, everything else → workspace). SPOG hosts don't match the `accounts.*` prefix, so they were misclassified as `WorkspaceConfig` and validated with `CurrentUser.Me`, which fails on account-scoped SPOG hosts. - Use the resolved `DiscoveryURL` from `/.well-known/databricks-config` to detect SPOG hosts with account-scoped OIDC (contains `/oidc/accounts/`), matching the routing logic in `auth.AuthArguments.ToOAuthArgument()` and the approach from databricks#4853. - Add a fallback for legacy profiles with `experimental_is_unified_host` where `.well-known` is unreachable. ### Why not just check `account_id`? Since databricks#4809, `runHostDiscovery` populates `account_id` on every workspace profile from the `.well-known` endpoint. A regular workspace profile now routinely carries `account_id`. The only reliable discriminator is the `oidc_endpoint` shape from `.well-known`, resolved at runtime (as established in databricks#4853). ## Test plan - [x] Unit tests: `TestProfileLoadSPOGConfigType` — table-driven with mock HTTP servers covering SPOG account, SPOG workspace, SPOG with `workspace_id=none`, and classic workspace with discovery-populated `account_id`. - [x] Unit test: `TestProfileLoadUnifiedHostFallback` — `experimental_is_unified_host` profile with unreachable `.well-known` falls back to account validation. - [x] Unit test: `TestProfileLoadClassicAccountHost` — classic account-scoped OIDC host. - [x] Acceptance test: `cmd/auth/profiles/spog-account` — end-to-end: SPOG profile with `workspace_id=none` shows `valid:true`. - [x] `go test ./cmd/auth` and `go test ./acceptance -run TestAccept/cmd/auth/profiles` pass. --------- Co-authored-by: simon <4305831+simonfaltum@users.noreply.github.com>

## Summary - Extract the SPOG detection heuristic into a shared `IsSPOG(cfg, accountID)` predicate in `libs/auth/config_type.go`, replacing inline checks in both `profiles.go` and `ToOAuthArgument`. - Extract `ResolveConfigType(cfg)` that wraps `ConfigType()` with SPOG-aware overrides, used by `profiles.go`. - `ToOAuthArgument` now calls `IsSPOG` instead of duplicating the `DiscoveryURL` + `IsUnifiedHost` checks inline. Follow-up to databricks#4929 as suggested in review comment shuochen0311#2 (logic duplication). ### Note on IsUnifiedHost fallback scope The `IsSPOG` predicate checks `Experimental_IsUnifiedHost` unconditionally (not gated on `configType == InvalidConfig`). This is broader than the previous inline check in `profiles.go`, which only fired for `InvalidConfig`. This is safe because the SDK currently returns `InvalidConfig` for all unified hosts (the `UnifiedHost` case was removed from `ConfigType()` in v0.126.0). It is also more robust against future SDK changes that might reclassify unified hosts differently. ## Test plan - [x] `TestResolveConfigType` — 9-case table-driven unit test in `libs/auth/config_type_test.go`. - [x] All existing `TestProfileLoad*` and `TestToOAuthArgument*` tests pass. - [x] `go test ./libs/auth/ ./cmd/auth/` passes.

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

The Unix path used syscall.Exec to replace the Go process with ssh directly, saving one fork. The Windows path already used exec.Command(...).Run(), and that works on all platforms — terminal signals are delivered to ssh via the foreground process group either way. Collapse to one cross-platform path; drop the build-tagged file and the runtime.GOOS check. Co-authored-by: Isaac

libs/execv already wraps the syscall.Exec / Windows-emulation pattern the previous version reimplemented inline. Switch to it so ssh truly replaces the CLI process on Unix instead of running as a child — fewer moving parts when the user hits Ctrl-C, and one fewer Go process in the ps tree for the lifetime of the session. Co-authored-by: Isaac

Replace the hand-rolled HTTP plumbing with client.DatabricksClient.Do, following the pattern in cmd/api/api.go and bundle/deploy/filer.go. Each method becomes a single Do() call; the SDK handles auth, JSON marshal, JSON unmarshal, error parsing, and retries. Removed: - doRequest (manual http.NewRequestWithContext + Config.Authenticate) - parseAPIError + the local apiError type (SDK returns apierr.APIError) - manual json.Marshal / json.NewDecoder.Decode in every method - net/http response status-code branching Preserved: - X-Databricks-Org-Id is still injected on every call. The SDK's Config.WorkspaceID is the source of truth; we fall back to parsing `?o=<id>` off the host because some staging gateways are configured that way and the SDK doesn't lift the query into Config.WorkspaceID. newLakeboxAPI now returns (*lakeboxAPI, error) since client.New can fail on bad config; callers updated. Co-authored-by: Isaac

Today if a code path between spin(...) and s.ok/s.fail returns early, the cmdio Bubble Tea program keeps running and we leak a goroutine plus garble the terminal. The wrapper kept its own `finished` gate but exposed no way to close without printing a marker. Add Close() that stops the spinner with no marker (wired through the same `finished` gate, so calling Close() after ok/fail is a no-op), and `defer s.Close()` at every spin site. ok/fail still print the ✓/✗ line on the success/failure paths; Close is just the cleanup safety net. Co-authored-by: Isaac

…gate Define a local cmdioSpinner interface (just Close()) that the unexported cmdio.spinner type satisfies structurally. Embed it on our wrapper so spinner.Close comes for free, and drop the func() workaround. The 'finished' bool was only preventing double-printing the ✓/✗ marker if a caller called ok/fail twice — caller pilot error rather than a real hazard, and cmdio's own Close is already idempotent (sync.OnceFunc on sendQuit), so the gate isn't needed for resource safety. Net effect: shorter, the embedded Close() is still safe to defer, and double-calls to ok/fail print twice (which they always should have). Co-authored-by: Isaac

Add Update(msg string) to the cmdioSpinner interface so callers can re-suffix the spinner mid-spin without reaching past our wrapper. No current call site uses it, but it's a free pass-through via embedding and matches the underlying cmdio API. Co-authored-by: Isaac

If the default lakebox stored at ~/.databricks/lakebox.json gets removed out-of-band (auto-stop expiry, admin reap, deletion from another machine), 'lakebox ssh' would happily try to ssh to it via the gateway and the user would get a confusing 'Permission denied (publickey)' from ssh. There was no signal that the default was stale. api.get the saved default first; if it 404s (or any other error), warn, clearDefault, and fall through to the existing 'no default → provision a fresh one' branch. Mirrors the validation already in 'lakebox create'. Co-authored-by: Isaac

Cover load/save/clear round-trips, missing-file and corrupt-JSON paths, multi-profile independence, and the legacy 'last_profile' field that older CLI versions wrote — loadState must accept it (silently dropping the unknown key) and saveState must rewrite the file without it so it naturally falls off on the next mutation. All tests use env.WithUserHomeDir(t.Context(), t.TempDir()) so they operate on an isolated state file. Co-authored-by: Isaac

The /api/2.0/lakebox/ssh-keys endpoint identifies registered keys by hash. Live exploration confirmed the algorithm: sha256 of '<type> <base64-blob>' (comment stripped) truncated to 16 bytes, hex encoded — looks like MD5 (32 hex chars) but isn't. Encode this client-side so we can answer 'is this local key registered?' without a list call. Tests use the exact hashes captured from the live API as ground truth, plus an edge case for empty input. Co-authored-by: Isaac

Two small fixes to the keyHash helper: - Replace the nested IndexByte calls with a range over strings.SplitSeq that breaks after the second token. Tracks a running byte offset so we still slice the original string instead of allocating a joined copy. - Drop the misleading 'without a list call' phrasing. You still need to call GET /ssh-keys; the helper just means you can match a local key against the listing by hash, without re-uploading the key contents. Co-authored-by: Isaac

The SplitSeq approach needed a running offset and a 'first iteration?' guard inside the loop. Walking bytes directly until we see the second space is shorter and reads more directly: count spaces, slice when the counter hits 2. Single pass, no allocation. Co-authored-by: Isaac

The previous wording implied the expected hashes were pulled from real registered keys returned by the API. They aren't — they're sha256[:16] of synthetic strings I posted during exploration. The algorithm was verified live; the test pins the algorithm rather than any specific captured registration. Co-authored-by: Isaac

Every case in TestKeyHash already pins an exact 32-char hex string, so a separate length-only test buys nothing. Co-authored-by: Isaac

Drop the bespoke resolveWorkspaceID helper and the cached wsID field on lakeboxAPI. Match the minimal pattern that libs/telemetry, libs/filer, and SDK-generated workspace services already use: read cfg.WorkspaceID directly, send the X-Databricks-Org-Id header if set. Removes the '?o=<id>' fallback that parsed the host's query string. That behavior was unique to lakebox and inconsistent with how every other CLI surface handles SPOG hosts; the SDK's host-metadata discovery populates cfg.WorkspaceID for hosts that need it, and users who run into edge cases set workspace_id explicitly the same way they would for `bundle deploy` or `databricks api`. Adds the auth.WorkspaceIDNone ("none") sentinel strip so a profile created via `databricks auth login` for SPOG account-level access doesn't send the literal string "none" as the routing identifier. This fix matches what cmd/api/api.go (databricks#5137) and libs/auth do; the four other orgIDHeaders helpers in the codebase still have the latent bug, which is a separate cleanup. Co-authored-by: Isaac

setDefault and clearDefault unconditionally rewrote ~/.databricks/ lakebox.json even when the in-memory state was identical to what was already on disk: clearing a profile that wasn't in the map, or re-setting the same value. That created or touched the file for no-op operations. Add change-detection guards to both: setDefault is a no-op when the profile already maps to the requested ID; clearDefault is a no-op when the profile isn't in the map. Result: a CLI invocation that doesn't change state can no longer cause a file to spring into existence on a fresh machine. Tests: - clearDefault on a missing profile must leave the file absent - setDefault with an unchanged value must not bump the mtime - getDefault on a fresh state must not create the file (regression test for the read-only path) Co-authored-by: Isaac

Mark the lakebox parent command as Hidden so it doesn't appear in 'databricks --help' under Developer Tools. The subcommands themselves are still reachable — 'databricks lakebox --help' lists them — but the feature stays out of the discoverable surface while it remains internal. This also reverts the acceptance/help/output.txt regen from the previous push, since hiding the command means the golden file already matches the actual help output. Co-authored-by: Isaac

## Changes - Stop applying `presets.name_prefix` (and the dev-mode `[dev <user>]` rename) to `vector_search_endpoints` in `bundle/config/mutator/resourcemutator/apply_presets.go`. - Add `.agent/rules/name-prefix.md` capturing the principle (only prefix display-name fields; never primary-key / object-id Names), scoped via globs to `apply_presets.go`, `apply_target_mode*.go`, and `bundle/direct/dresources/*.go`. Mirror as `.cursor/rules/name-prefix.mdc`. - Rename `TestAllNonUcResourcesAreRenamed` → `TestAppropriateResourcesAreRenamed` (the carve-out list now includes a non-UC resource), and refactor the long `resourceType ==` OR chain into a `slices.Contains` over a named slice hoisted to the outer loop. - `NEXT_CHANGELOG.md` entry under Bundles. ## Why The vector search endpoint name is the API primary key — it's how GET, UPDATE, and DELETE address the resource (`bundle/direct/dresources/vector_search_endpoint.go`: `id := config.Name`; `recreate_on_changes` for the resource doesn't list `name` only because there's no rename API at all, so a name change would silently drift). Prefixing it changed which remote endpoint the bundle pointed at, not just the label the user saw. The rule we want to encode is broader (display-name fields can be prefixed; identity-bearing Names cannot), but this PR only fixes the vector_search_endpoints case to keep the change focused; mlflow Models, ModelServingEndpoints, etc. have the same issue and are tracked for follow-up. ## Tests - `go test ./bundle/config/mutator/resourcemutator/` passes; `TestProcessTargetModeDevelopment` now asserts `vs_endpoint1` (not `dev_lennart_vs_endpoint1`), and `TestAppropriateResourcesAreRenamed` includes `*resources.VectorSearchEndpoint` in the carve-out list and verifies the Name doesn't pick up a `dev` prefix. - Confirmed locally that re-introducing the prefix loop in `apply_presets.go` causes both the explicit assertion and the reflective sweep to fail with clear diffs. - `./task fmt`, `./task checks`, `./task lint`, `./task test` clean. _This PR was written by Claude Code._

…ricks#5215) ## Changes Two independent refactors in `bundle/`: 1. **`bundle/config/mutator/validate_direct_only_resources.go`** — drop the local `directOnlyResource` struct (whose `pluralName`/`singularName` fields actually held `SingularTitle`/`SingularName` values) and the per-type `getResources` closures. Iterate `b.Config.Resources.AllResources()` and key off `PluralName` via a small `map[string]bool` instead, reading `SingularTitle`/`SingularName` from the canonical `ResourceDescription`. Adds a unit test covering direct/terraform engines × the three direct-only resource types. 2. **`bundle/phases/{deploy,destroy}.go`** — collapse the eight (deploy) and seven (destroy) near-identical `if len(xActions) != 0 { LogString(message); for _ { Log(action) } }` blocks into a table-driven helper `logApprovalGroups` in a new `bundle/phases/approval.go`. The deploy version also replaces the eight-clause `len(...) == 0 &&` early-return with a single `total == 0` check returned from the helper. The schema child-resource skip (deploy only) and trailing blank lines (destroy only) are preserved via per-group `skipChildren`/`trailingGap` flags. The outer "all deletes" preamble in destroy stays as-is — it's structurally different. Net diff: −215 source LOC, +110 test LOC. ## Why Both files had grown into mechanical repetition. `validate_direct_only_resources.go` re-declared resource metadata that already lives on each resource's `ResourceDescription()` and named the fields incorrectly. The approval functions repeated the same eight-/seven-times pattern inline, with an opaque eight-clause boolean expression for the early-return. There is one minor user-visible wording change: for `external_locations` and `vector_search_endpoints`, the Detail message now reads `... use external_location resources.` / `... use vector_search_endpoint resources.` (snake_case, from `SingularName`) instead of `... use external location resources.` / `... use vector search endpoint resources.` (the old struct's hand-written field with spaces). The catalog message is byte-identical. There are no acceptance tests covering the other two messages. ## Tests - New unit test `validate_direct_only_resources_test.go` (table-driven over the three direct-only types). - Existing acceptance test `bundle/validate/catalog_requires_direct_mode` passes byte-identically. - `bundle/destroy/...`, `bundle/deploy/...` (excluding the pre-existing `spark-jar-task` Java env failure that also fails on `main`), `bundle/resources/grants/schemas/...`, `bundle/config-remote-sync/...`, and `bundle/user_agent/simple` all pass byte-identically. - `./task fmt`, `./task checks`, `./task lint` clean. _This PR was written by Claude Code._

…cks#5228) ## Summary - PR databricks#4596 (just merged on main) committed `out.test.toml` for `acceptance/bundle/generate/job_nested_notebooks` in the pre-databricks#5146 multi-line `[EnvMatrix]` form. - Regenerated via `go test ./acceptance -run '^TestAccept$' -only-out-test-toml`. ## Test plan - [x] `go test ./acceptance -run '^TestAccept$' -only-out-test-toml` produces only this one-file diff - [x] `git diff` after regeneration matches the diff CI was reporting This pull request and its description were written by Isaac.

databricks#5227) Since databricks#5170 the CLI's JSON output uses the standard `: ` separator (via `json.MarshalIndent`) rather than the compact `":"` form produced by the previous `nwidger/jsoncolor` dependency. `TestClustersGet`'s substring assertion still expected the compact form and has been failing in every nightly environment; updating it to match the new output. This pull request and its description were written by Isaac.

…ks#5206) ## Summary - Follow-up to databricks#5203. Adds a unit test for `collectTargets` that exercises the nil-entry path so the regression is locked in. - Test stays at the function level because no YAML pattern I tried produces a nil `*config.Target` entry through the loader pipeline (`staging:`, `staging: null`, `staging: ~`, `staging: {}` all yield non-nil entries). Direct unit coverage is the practical guard. ## Test plan - [ ] `go test ./cmd/bundle/debug -run TestCollectTargetsHandlesNilEntries` passes. - [ ] Reverting the nil guard in `collectTargets` makes the test fail with a nil pointer dereference.

## Changes Move the creation of `proxyCommand` to _after_ interactive cluster selection ## Why Before `proxyCommand` was created before interactive cluster selection, meaning we would output a broken proxy command in the generate SSH config. Moving creation of `proxyCommand` to after interactive cluster selection means the selected cluster is properly populated in the generated SSH config ## Tests Added test

## Summary - Remove the `Default` field from `cmdio.PromptOptions`. No production caller sets it; `databricks auth login` renders the default in the label and substitutes it manually on empty input (see PR databricks#3252). Follow-up to databricks#5177, which removed the now-orphaned `AllowEdit` companion field. - Drop the corresponding `--default` flag from `databricks selftest tui prompt`, which only existed to exercise the field. This pull request and its description were written by Isaac.

…5190) Moves `TestGenerateFromExistingJobAndDeploy` from `integration/` to `acceptance/`. Acceptance tests are the only path that wires up terraform installs now, so the integration variant can't run end-to-end. The new test runs the same flow (upload notebook → create job → `bundle generate job` → `bundle deploy` → `bundle destroy`) on both modes: - `localupdate`: ~5s - `cloudupdate` (azure-prod-ucws, terraform + direct): ~3-4m Also fixes a Windows path-separator bug in `bundle/generate/downloader.go` that the new test caught: `filepath.Rel` returned `..\src\test_notebook.py` on Windows, so the generated YAML had OS-native separators. Mirrors the existing `filepath.ToSlash` pattern in `pipeline.go`. This pull request was AI-assisted by Isaac. We did not have coverage for generate + deploy in our existing tests, that's why I translated it instead. --------- Co-authored-by: Andrew Nester <andrew.nester@databricks.com>

Go on Windows synthesizes file mode from the read-only attribute (0o666/0o777), so the 0o600/0o700 assertions can never hold. Matches the existing pattern used in libs/cache, libs/completion, and the ssh vscode settings tests. Co-authored-by: Isaac

Wrap the create payload as `{"sandbox": {...}}` (the proto's `body: "*"` form) and drop the dead `public_key` field — the unwrapped payload was rejected by the server, breaking the `lakebox ssh` auto-create path. While here: - Surface `name`, `createTime`, `lastStartTime` on `sandboxEntry` so `status --json` / `list --json` stop dropping these fields. - Add `--name` to `lakebox create` and `lakebox config`, and print it in human `status` / `config` output when set. - Paginate `list` using `page_size`/`page_token` so callers stop silently capping at the server's per-page limit. Co-authored-by: Isaac

…ull body (databricks#5290) Three related ssh-key UX changes plus a wire-format bug in `list`: 1. `ssh-key list` (new subcommand): GET /api/2.0/lakebox/ssh-keys. Renders the user's registered keys with a `*` gutter marker on the key matching this machine (using the existing keyHash helper from bd72f85). `--json` for scripting. 2. `ssh-key delete <key-hash>` (new subcommand): DELETE /api/2.0/lakebox/ssh-keys/{key_hash}. 3. `register --name` (new flag): defaults to this machine's hostname when not provided so `ssh-key list` is meaningful across multiple machines. Previously every locally-registered key landed with an empty name and surfaced as `(unset)`. 4. Fix `lakebox list` against any workspace that actually has lakebox deployed. Pieter's SDK ApiClient rewrite (08a56be) wrote the pagination call as `Do(..., headers, query, nil, &resp)` — but the SDK's makeRequestBody treats slot 6 (`request`) as the GET query string and slot 5 (`queryParams`) as ignored on GET. Routing the query through slot 5 with a nil body in slot 6 makes the SDK serialize nil as the literal JSON `null`, sent as the request body on a GET, which the lakebox manager rejects with `INVALID_PARAMETER_VALUE: Request body must be a JSON object`. Swap the args. Co-authored-by: Isaac ## Changes  ## Why  ## Tests  Tested against `dbsql-dev-testing-default` (workspace with lakebox deployed but no sandboxes): - [x] `lakebox list` returns `200 OK` with no body, prints "No lakeboxes found." (previously 400'd) - [x] `lakebox ssh-key list` renders both keys with correct column alignment; `*` gutter marks the local key - [x] `lakebox ssh-key list --json` round-trips through `jq` - [x] `lakebox ssh-key delete <hash>` — not yet exercised end-to-end - [x] `lakebox register --name laptop` against a clean workspace — confirm name lands

us-east-1 is the staging region where the SSH gateway is reachable in practice; uw2.s.dbrx.dev does not have a routable lakebox listener. Users targeting a different staging region can still override with --gateway. Co-authored-by: Isaac

dependabot Bot and others added 30 commits April 13, 2026 11:15

Add missing license comments in go.mod (databricks#4938)

cf6e193

## Summary - Add license comments for 7 direct dependencies that were missing them (`charmbracelet/huh`, `golang.org/x/{exp,mod,oauth2,sync,sys,text}`) This pull request was AI-assisted by Isaac.

Exclude continue_293 test on models with perms and apps (databricks#4966

fa37f5c

) Also don't mark it as Slow so it runs on PRs and enable it on Cloud.

pietern and others added 29 commits May 8, 2026 17:02

lakebox: drop superfluous TestKeyHashIsStableLength

4df1daf

Every case in TestKeyHash already pins an exact 32-char hex string, so a separate length-only test buys nothing. Co-authored-by: Isaac

Merge branch 'main' into demo-lakebox

29b16f9

akshaysingla-db force-pushed the akshay/lakebox-staging-gateway-ue1 branch from 94a42b0 to 6c7204a Compare May 20, 2026 19:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Akshay/lakebox staging gateway ue1#4

Akshay/lakebox staging gateway ue1#4
akshaysingla-db wants to merge 256 commits into
shuochen0311:mainfrom
akshaysingla-db:akshay/lakebox-staging-gateway-ue1

akshaysingla-db commented May 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

20 participants

Conversation

akshaysingla-db commented May 20, 2026

Changes

Why

Tests

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

20 participants