From 74aa464dc0d79efbb1a325d87aa371db8c9a731c Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Thu, 8 Oct 2026 16:18:46 +0200 Subject: [PATCH] feat(shared): migrate the appkit CLI off the legacy workspace client doctor and the registry picker now use the facade's modular clients (warehouses, jobs, genie, currentUser) and its raw request() seam for services with no modular client yet (serving, volumes, vector search, UC functions, connections, database, experiments, apps). - Profile host is read via sdk-core profiles resolve (disableEnv), exposed from the facade as resolveProfile. - Resolved host comes from getHost(), which needs no network, so it is also shown when credentials are rejected. The ConfigError host= parsing is gone. - Existence probes classify both ApiError shapes (statusCode/errorCode and httpStatusCode/code), so NOT_FOUND/INVALID_VALUE/ACCESS_DENIED are kept. - Auth hints also match the sdk-core "profile not found" and "Host is required" messages. - The Lakebase probe hands createLakebasePool a legacy-shaped adapter, since that package calls currentUser.me() with no argument. Co-authored-by: Isaac Signed-off-by: MarioCadenas # Conflicts: # packages/shared/src/workspace-client/modular.ts --- .../shared/src/cli/commands/doctor/README.md | 17 ++-- .../commands/doctor/checks-existence.test.ts | 78 +++++++++------ .../cli/commands/doctor/checks-existence.ts | 75 +++++++------- .../src/cli/commands/doctor/checks.test.ts | 65 ++++++------ .../shared/src/cli/commands/doctor/checks.ts | 50 ++++------ .../cli/commands/doctor/databricks-client.ts | 26 ++--- .../registry/workspace-picker.test.ts | 75 +++++++++----- .../cli/commands/registry/workspace-picker.ts | 99 ++++++++++++------- packages/shared/src/workspace-client/index.ts | 1 + .../shared/src/workspace-client/modular.ts | 6 ++ 10 files changed, 281 insertions(+), 211 deletions(-) diff --git a/packages/shared/src/cli/commands/doctor/README.md b/packages/shared/src/cli/commands/doctor/README.md index 85a519906..c360a3de6 100644 --- a/packages/shared/src/cli/commands/doctor/README.md +++ b/packages/shared/src/cli/commands/doctor/README.md @@ -14,7 +14,7 @@ so the reported problem is the *root* cause, not a symptom. | ------------ | ----------------------------------------------------- | ---------------------------------------------------------- | | `auth` | Can we authenticate to the workspace at all? | validate `DATABRICKS_HOST` is a real URL, then `currentUser.me()` — once, app-wide; a failure skips the live layer | | `config` | Are the resource's field env vars **present**? | offline presence check of `process.env` (presence only — see note) | -| `existence` | Does the resource exist and is it reachable? | cheapest per-type live probe (`warehouses.get`, `servingEndpoints.get`, …); Lakebase runs a real `SELECT 1` | +| `existence` | Does the resource exist and is it reachable? | cheapest per-type live probe (`warehouses.getWarehouse`, `GET /api/2.0/serving-endpoints/{name}`, …); Lakebase runs a real `SELECT 1` | `config` checks env-var presence only; whether a value points at a real resource is the `existence` layer's job. (`DATABRICKS_HOST` is the exception — `auth` @@ -217,15 +217,16 @@ skipped outright, not merged.) Because of that, a failed or conflicted auth row reports **both** `host:` and `profile:`. The host shown is the one the SDK actually resolved, read from -`client.config.host` — which the SDK populates lazily on the first API call, so -it's read *after* `me()`, not at construction. When the client never got built, -the host is recovered from the `host=…` fragment the SDK appends to its -`ConfigError`; failing that, it falls back to `DATABRICKS_HOST`. Every path is -passed through `sanitizeHost`, so embedded `user:pass@` credentials can't leak. +`client.getHost()`. That resolves from env + profile only (no network), so it's +available even when the credentials are rejected. When resolution itself fails +(missing profile, no host anywhere), it falls back to `DATABRICKS_HOST`. Every +path is passed through `sanitizeHost`, so embedded `user:pass@` credentials +can't leak. `HOST_PROFILE_CONFLICT` compares `DATABRICKS_HOST` against the profile's own -declared host, read offline from `~/.databrickscfg` via the SDK's exported -`loadConfigFile` (the resolved config is useless here — env has already won). +declared host, read offline from `~/.databrickscfg` via sdk-core's profile +`resolve` with the env overlay disabled (the resolved config is useless here — +env has already won). Comparison ignores scheme, case, and trailing slash. It's a **warning**, not an error: the credentials do work, so it must not gate CI, but a green tick would hide a real misconfiguration. When auth *also* fails, the conflict replaces the diff --git a/packages/shared/src/cli/commands/doctor/checks-existence.test.ts b/packages/shared/src/cli/commands/doctor/checks-existence.test.ts index f4adb3fe9..9ed0ca2bb 100644 --- a/packages/shared/src/cli/commands/doctor/checks-existence.test.ts +++ b/packages/shared/src/cli/commands/doctor/checks-existence.test.ts @@ -35,7 +35,7 @@ function apiError(statusCode: number, message = "boom"): Error { describe("runExistenceProbe — sql_warehouse", () => { it("ok when the warehouse exists and is RUNNING", async () => { const client = { - warehouses: { get: async () => ({ state: "RUNNING" }) }, + warehouses: { getWarehouse: async () => ({ state: "RUNNING" }) }, }; const r = await runExistenceProbe(client, target()); expect(r.status).toBe("ok"); @@ -43,7 +43,7 @@ describe("runExistenceProbe — sql_warehouse", () => { it("warns when the warehouse exists but is STOPPED", async () => { const client = { - warehouses: { get: async () => ({ state: "STOPPED" }) }, + warehouses: { getWarehouse: async () => ({ state: "STOPPED" }) }, }; const r = await runExistenceProbe(client, target()); expect(r.status).toBe("warn"); @@ -53,7 +53,7 @@ describe("runExistenceProbe — sql_warehouse", () => { it("errors NOT_FOUND on a 404", async () => { const client = { warehouses: { - get: async () => { + getWarehouse: async () => { throw apiError(404); }, }, @@ -66,7 +66,7 @@ describe("runExistenceProbe — sql_warehouse", () => { it("errors INVALID_VALUE on a 400, quoting the value (no type/blob leak)", async () => { const client = { warehouses: { - get: async () => { + getWarehouse: async () => { throw Object.assign( new Error( 'Response from server (Bad Request) {"error_code":"INVALID_PARAMETER_VALUE","message":"bogus is not a valid endpoint id."}', @@ -90,7 +90,7 @@ describe("runExistenceProbe — sql_warehouse", () => { it("errors ACCESS_DENIED on a 403", async () => { const client = { warehouses: { - get: async () => { + getWarehouse: async () => { throw apiError(403); }, }, @@ -103,7 +103,7 @@ describe("runExistenceProbe — sql_warehouse", () => { it("hedges on a 403, which several APIs also return for a missing resource", async () => { const client = { warehouses: { - get: async () => { + getWarehouse: async () => { throw apiError(403); }, }, @@ -114,8 +114,31 @@ describe("runExistenceProbe — sql_warehouse", () => { expect(r.detail).toContain("wh-1"); }); + // The modular SDK's ApiError carries `httpStatusCode` / `code`, not the + // legacy `statusCode` / `errorCode`; both must classify the same. + it("classifies the modular ApiError shape (httpStatusCode / code)", async () => { + const modular = (httpStatusCode: number, code: string) => ({ + getWarehouse: async () => { + throw Object.assign(new Error("boom"), { httpStatusCode, code }); + }, + }); + const cases: Array<[number, string, string]> = [ + [404, "RESOURCE_DOES_NOT_EXIST", "NOT_FOUND"], + [400, "INVALID_PARAMETER_VALUE", "INVALID_VALUE"], + [403, "PERMISSION_DENIED", "ACCESS_DENIED"], + [-1, "RESOURCE_DOES_NOT_EXIST", "NOT_FOUND"], + ]; + for (const [status, code, expected] of cases) { + const r = await runExistenceProbe( + { warehouses: modular(status, code) }, + target(), + ); + expect(r.code, `${status}/${code}`).toBe(expected); + } + }); + it("skips when the id field is missing", async () => { - const client = { warehouses: { get: async () => ({}) } }; + const client = { warehouses: { getWarehouse: async () => ({}) } }; const r = await runExistenceProbe(client, target({ fieldValues: {} })); expect(r.status).toBe("skipped"); expect(r.code).toBe("MISSING_FIELD"); @@ -124,7 +147,7 @@ describe("runExistenceProbe — sql_warehouse", () => { describe("runExistenceProbe — job", () => { it("errors on a non-integer job id", async () => { - const client = { jobs: { get: async () => ({}) } }; + const client = { jobs: { getJob: async () => ({}) } }; const r = await runExistenceProbe( client, target({ type: "job", fieldValues: { id: "not-a-number" } }), @@ -134,7 +157,7 @@ describe("runExistenceProbe — job", () => { }); it("ok for a valid integer job id", async () => { - const client = { jobs: { get: async () => ({}) } }; + const client = { jobs: { getJob: async () => ({}) } }; const r = await runExistenceProbe( client, target({ type: "job", fieldValues: { id: "42" } }), @@ -149,8 +172,8 @@ describe("runExistenceProbe — job", () => { let seen: unknown; const client = { jobs: { - get: async (req: { job_id: unknown }) => { - seen = req.job_id; + getJob: async (req: { jobId: unknown }) => { + seen = req.jobId; return {}; }, }, @@ -164,7 +187,7 @@ describe("runExistenceProbe — job", () => { }); it("rejects numeric forms the API can't take (1e3, 0x10)", async () => { - const client = { jobs: { get: async () => ({}) } }; + const client = { jobs: { getJob: async () => ({}) } }; for (const id of ["1e3", "0x10", "4.5", "-1"]) { const r = await runExistenceProbe( client, @@ -260,7 +283,7 @@ describe("runExistenceProbe — postgres (Lakebase)", () => { describe("runExistenceProbe — per-type coverage with real manifest keys", () => { it("serving_endpoint: ok via `name`", async () => { - const client = { servingEndpoints: { get: async () => ({}) } }; + const client = { request: async () => ({}) }; const r = await runExistenceProbe( client, target({ @@ -273,10 +296,8 @@ describe("runExistenceProbe — per-type coverage with real manifest keys", () = it("serving_endpoint: hints id-vs-name when configured by id and the probe fails", async () => { const client = { - servingEndpoints: { - get: async () => { - throw Object.assign(new Error("not found"), { statusCode: 404 }); - }, + request: async () => { + throw Object.assign(new Error("not found"), { statusCode: 404 }); }, }; const r = await runExistenceProbe( @@ -289,10 +310,8 @@ describe("runExistenceProbe — per-type coverage with real manifest keys", () = it("serving_endpoint: no id-vs-name hint when configured by name", async () => { const client = { - servingEndpoints: { - get: async () => { - throw Object.assign(new Error("not found"), { statusCode: 404 }); - }, + request: async () => { + throw Object.assign(new Error("not found"), { statusCode: 404 }); }, }; const r = await runExistenceProbe( @@ -304,7 +323,7 @@ describe("runExistenceProbe — per-type coverage with real manifest keys", () = }); it("genie_space: ok via `id`", async () => { - const client = { genie: { getSpace: async () => ({}) } }; + const client = { genie: { genieGetSpace: async () => ({}) } }; const r = await runExistenceProbe( client, target({ type: "genie_space", fieldValues: { id: "01ef" } }), @@ -313,7 +332,7 @@ describe("runExistenceProbe — per-type coverage with real manifest keys", () = }); it("volume: ok via `path` (real manifest key)", async () => { - const client = { volumes: { read: async () => ({}) } }; + const client = { request: async () => ({}) }; const r = await runExistenceProbe( client, target({ @@ -325,7 +344,7 @@ describe("runExistenceProbe — per-type coverage with real manifest keys", () = }); it("uc_function: ok via `name`", async () => { - const client = { functions: { get: async () => ({}) } }; + const client = { request: async () => ({}) }; const r = await runExistenceProbe( client, target({ type: "uc_function", fieldValues: { name: "cat.sch.fn" } }), @@ -334,8 +353,8 @@ describe("runExistenceProbe — per-type coverage with real manifest keys", () = }); it("vector_search_index: probes via camelCase `indexName`", async () => { - const getIndex = vi.fn(async () => ({})); - const client = { vectorSearchIndexes: { getIndex } }; + const request = vi.fn(async () => ({})); + const client = { request }; const r = await runExistenceProbe( client, target({ @@ -344,11 +363,14 @@ describe("runExistenceProbe — per-type coverage with real manifest keys", () = }), ); expect(r.status).toBe("ok"); - expect(getIndex).toHaveBeenCalledWith({ index_name: "main.default.idx" }); + expect(request).toHaveBeenCalledWith({ + method: "GET", + path: "/api/2.0/vector-search/indexes/main.default.idx", + }); }); it("vector_search_index: skips MISSING_FIELD when index name absent", async () => { - const client = { vectorSearchIndexes: { getIndex: async () => ({}) } }; + const client = { request: async () => ({}) }; const r = await runExistenceProbe( client, target({ type: "vector_search_index", fieldValues: {} }), diff --git a/packages/shared/src/cli/commands/doctor/checks-existence.ts b/packages/shared/src/cli/commands/doctor/checks-existence.ts index 10a63e2f1..cf07f13bd 100644 --- a/packages/shared/src/cli/commands/doctor/checks-existence.ts +++ b/packages/shared/src/cli/commands/doctor/checks-existence.ts @@ -2,7 +2,7 @@ * Layer: existence — per-resource-type probes that prove a declared resource * exists and is reachable via the cheapest read the SDK offers. * - * The client is typed structurally (not via the SDK) so `shared` stays SDK-free. + * The client is typed structurally (the facade's modular clients + `request()`). */ import { @@ -18,26 +18,29 @@ const EXISTENCE_OK: LayerResult = { layer: "existence", status: "ok" }; interface DoctorWorkspaceClient { warehouses: { - get: (r: { id: string }) => Promise<{ state?: string }>; - }; - servingEndpoints: { - get: (r: { name: string }) => Promise; + getWarehouse: (r: { id: string }) => Promise<{ state?: string }>; }; genie: { - getSpace: (r: { space_id: string }) => Promise; + genieGetSpace: (r: { spaceId: string }) => Promise; }; jobs: { - get: (r: { job_id: number }) => Promise; - }; - volumes: { - read: (r: { name: string }) => Promise; - }; - vectorSearchIndexes: { - getIndex: (r: { index_name: string }) => Promise; - }; - functions: { - get: (r: { name: string }) => Promise; + getJob: (r: { jobId: bigint }) => Promise; }; + /** Raw REST GET for services the facade has no modular client for yet + * (serving, volumes, vector search, UC functions). Throws on non-2xx. */ + request: (r: { method: string; path: string }) => Promise; +} + +/** A REST `GET` on a resource path, with the name URL-encoded. */ +function getResource( + client: DoctorWorkspaceClient, + basePath: string, + name: string, +): Promise { + return client.request({ + method: "GET", + path: `${basePath}/${encodeURIComponent(name)}`, + }); } type ExistenceProbe = ( @@ -45,21 +48,21 @@ type ExistenceProbe = ( target: ResourceTarget, ) => Promise; -// Read statusCode/errorCode off the SDK's ApiError structurally. +// Read the HTTP status / Databricks error code off an ApiError structurally. +// Two shapes reach here: the wrapper's `ApiError` (from `request()`: +// `statusCode` / `errorCode`) and the modular SDK's (`httpStatusCode` / `code`). function statusCodeOf(err: unknown): number | undefined { - if (err && typeof err === "object" && "statusCode" in err) { - const code = (err as { statusCode?: unknown }).statusCode; - if (typeof code === "number") return code; - } - return undefined; + if (!err || typeof err !== "object") return undefined; + const e = err as { statusCode?: unknown; httpStatusCode?: unknown }; + const code = e.statusCode ?? e.httpStatusCode; + return typeof code === "number" && code >= 0 ? code : undefined; } function errorCodeOf(err: unknown): string | undefined { - if (err && typeof err === "object" && "errorCode" in err) { - const code = (err as { errorCode?: unknown }).errorCode; - if (typeof code === "string" && code.length > 0) return code; - } - return undefined; + if (!err || typeof err !== "object") return undefined; + const e = err as { errorCode?: unknown; code?: unknown }; + const code = e.errorCode ?? e.code; + return typeof code === "string" && code.length > 0 ? code : undefined; } // The SDK message often embeds a JSON blob; pull the inner `message` out so @@ -153,7 +156,7 @@ const probeWarehouse: ExistenceProbe = async (client, target) => { const id = field(target, "id"); if (!id) return missingField("id"); try { - const wh = await client.warehouses.get({ id }); + const wh = await client.warehouses.getWarehouse({ id }); const state = wh.state; if (state && state !== "RUNNING") { return { @@ -177,7 +180,7 @@ const probeServing: ExistenceProbe = async (client, target) => { const value = name ?? idOnly; if (!value) return missingField("name"); try { - await client.servingEndpoints.get({ name: value }); + await getResource(client, "/api/2.0/serving-endpoints", value); return EXISTENCE_OK; } catch (err) { const result = classifyError(err, target); @@ -193,7 +196,7 @@ const probeGenie: ExistenceProbe = async (client, target) => { const spaceId = field(target, "id"); if (!spaceId) return missingField("id"); try { - await client.genie.getSpace({ space_id: spaceId }); + await client.genie.genieGetSpace({ spaceId }); return EXISTENCE_OK; } catch (err) { return classifyError(err, target); @@ -217,10 +220,8 @@ const probeJob: ExistenceProbe = async (client, target) => { }; } try { - // Pass the digits through unconverted to preserve int64 precision; the SDK - // serialises them straight into the request, so a string works even though - // the type says number. - await client.jobs.get({ job_id: id as unknown as number }); + // BigInt keeps int64 precision (the digits were validated above). + await client.jobs.getJob({ jobId: BigInt(id) }); return EXISTENCE_OK; } catch (err) { return classifyError(err, target); @@ -242,7 +243,7 @@ const probeVolume: ExistenceProbe = async (client, target) => { }; } try { - await client.volumes.read({ name }); + await getResource(client, "/api/2.1/unity-catalog/volumes", name); return EXISTENCE_OK; } catch (err) { return classifyError(err, target); @@ -253,7 +254,7 @@ const probeVectorIndex: ExistenceProbe = async (client, target) => { const name = field(target, "indexName", "index_name", "name"); if (!name) return missingField("indexName"); try { - await client.vectorSearchIndexes.getIndex({ index_name: name }); + await getResource(client, "/api/2.0/vector-search/indexes", name); return EXISTENCE_OK; } catch (err) { return classifyError(err, target); @@ -264,7 +265,7 @@ const probeFunction: ExistenceProbe = async (client, target) => { const name = field(target, "name"); if (!name) return missingField("name"); try { - await client.functions.get({ name }); + await getResource(client, "/api/2.1/unity-catalog/functions", name); return EXISTENCE_OK; } catch (err) { return classifyError(err, target); diff --git a/packages/shared/src/cli/commands/doctor/checks.test.ts b/packages/shared/src/cli/commands/doctor/checks.test.ts index e64b9d41a..d8de04bd7 100644 --- a/packages/shared/src/cli/commands/doctor/checks.test.ts +++ b/packages/shared/src/cli/commands/doctor/checks.test.ts @@ -375,7 +375,7 @@ describe("checkAuth", () => { delete process.env.DATABRICKS_HOST; mockGetServiceClient.mockResolvedValue({ client: { - config: { host: "https://from-profile.cloud.databricks.com" }, + getHost: async () => "https://from-profile.cloud.databricks.com", currentUser: { me: async () => ({ userName: "u" }) }, }, }); @@ -384,33 +384,35 @@ describe("checkAuth", () => { expect(result.host).toBe("https://from-profile.cloud.databricks.com"); }); - it("reads the resolved host only after me(), which is when the SDK resolves", async () => { - // The SDK populates config.host lazily on the first API call, so reading it - // at construction time would always yield undefined. - delete process.env.DATABRICKS_HOST; - const client: { - config: { host?: string }; - currentUser: { me: () => Promise<{ userName: string }> }; - } = { - config: {}, - currentUser: { - me: async () => { - client.config.host = "https://resolved-late.cloud.databricks.com"; - return { userName: "u" }; + it("falls back to DATABRICKS_HOST when host resolution itself fails", async () => { + process.env.DATABRICKS_HOST = "https://env.cloud.databricks.com"; + mockGetServiceClient.mockResolvedValue({ + client: { + getHost: async () => { + throw new Error( + 'profile not found: "prod" in /home/u/.databrickscfg', + ); + }, + currentUser: { + me: async () => { + throw new Error( + 'profile not found: "prod" in /home/u/.databrickscfg', + ); + }, }, }, - }; - mockGetServiceClient.mockResolvedValue({ client }); + }); const { result } = await checkAuth({ profile: "prod" }); - expect(result.host).toBe("https://resolved-late.cloud.databricks.com"); + expect(result.status).toBe("error"); + expect(result.host).toBe("https://env.cloud.databricks.com"); }); it("keeps the resolved host on a failure after the client was built", async () => { delete process.env.DATABRICKS_HOST; mockGetServiceClient.mockResolvedValue({ client: { - config: { host: "https://from-profile.cloud.databricks.com" }, + getHost: async () => "https://from-profile.cloud.databricks.com", currentUser: { me: async () => { throw new Error("boom"); @@ -424,35 +426,24 @@ describe("checkAuth", () => { expect(result.host).toBe("https://from-profile.cloud.databricks.com"); }); - it("recovers the host from the SDK error when no client was built", async () => { - delete process.env.DATABRICKS_HOST; + // sdk-core's profile resolver words this differently from the legacy SDK + // ("profile not found: …" vs "has no … profile configured"). + it("hints an existing --profile when the modular SDK can't find the profile", async () => { mockGetServiceClient.mockRejectedValue( - new Error( - "default auth: cannot configure default credentials. Config: host=https://dbc-abc123.cloud.databricks.com, profile=DEFAULT", - ), + new Error('profile not found: "nope" in /home/u/.databrickscfg'), ); - const { result } = await checkAuth({}); - expect(result.host).toBe("https://dbc-abc123.cloud.databricks.com"); - }); - - it("strips prose punctuation off a host recovered from an error", async () => { - delete process.env.DATABRICKS_HOST; - mockGetServiceClient.mockRejectedValue( - new Error( - "cannot configure default credentials. host=https://foo.cloud.databricks.com.", - ), + const { result } = await checkAuth({ profile: "nope" }); + expect(result.hint).toBe( + "Run `databricks auth login --profile nope`, or pass an existing profile via --profile.", ); - - const { result } = await checkAuth({}); - expect(result.host).toBe("https://foo.cloud.databricks.com"); }); it("never leaks credentials embedded in a resolved host", async () => { delete process.env.DATABRICKS_HOST; mockGetServiceClient.mockResolvedValue({ client: { - config: { host: "https://user:secret@foo.cloud.databricks.com" }, + getHost: async () => "https://user:secret@foo.cloud.databricks.com", currentUser: { me: async () => ({ userName: "u" }) }, }, }); diff --git a/packages/shared/src/cli/commands/doctor/checks.ts b/packages/shared/src/cli/commands/doctor/checks.ts index b61d8f21d..e5ca39b33 100644 --- a/packages/shared/src/cli/commands/doctor/checks.ts +++ b/packages/shared/src/cli/commands/doctor/checks.ts @@ -24,14 +24,13 @@ export interface AuthOutcome { interface CurrentUserClient { currentUser: { - me: () => Promise<{ id?: string; userName?: string }>; + me: (req: object) => Promise<{ id?: string; userName?: string }>; }; } -/** Just the resolved config off a WorkspaceClient, read structurally to keep - * `shared` SDK-free. */ -interface ConfiguredClient { - config?: { host?: unknown }; +/** The host resolver off a WorkspaceClient, read structurally. */ +interface HostClient { + getHost?: () => Promise; } /** @@ -39,12 +38,17 @@ interface ConfiguredClient { * contacted: `DATABRICKS_HOST` wins the host when set, but a profile supplies it * otherwise, so the env var alone can't tell you the workspace in play. * - * Only populated once the SDK has resolved its config, which it does lazily on - * the first API call — reading it right after construction yields undefined. + * Resolved from env + profile only (no network), so it's available even when + * the credentials fail. Undefined when resolution itself fails (e.g. a missing + * profile, or no host anywhere). */ -function resolvedHostOf(client: unknown): string | undefined { - const host = (client as ConfiguredClient | undefined)?.config?.host; - return typeof host === "string" && host.length > 0 ? host : undefined; +async function resolvedHostOf(client: unknown): Promise { + try { + const host = await (client as HostClient | undefined)?.getHost?.(); + return typeof host === "string" && host.length > 0 ? host : undefined; + } catch { + return undefined; + } } /** Compares hosts ignoring scheme, trailing slash, and case, so @@ -80,17 +84,6 @@ async function hostProfileConflict( ); } -/** - * Last-resort host recovery for when the client never got built: the SDK's - * ConfigError appends the resolved config as `host=, profile=…`. - */ -function hostFromError(message: string): string | undefined { - const match = message.match(/host=([^\s,]+)/); - // The config list is prose-terminated ("…databricks.com."), so drop trailing - // punctuation the capture picked up. - return match ? match[1].replace(/[.,]+$/, "") : undefined; -} - /** * Strips URL userinfo (`user:pass@`) from a host, so credentials someone * embedded in `DATABRICKS_HOST` never reach the report or `--json`. Returns the @@ -186,12 +179,10 @@ export async function checkAuth(options: DoctorOptions): Promise { // Bound the live call so an unresponsive workspace can't hang the CLI; a // timeout throws and is reported as an auth failure below. const me = await withTimeout( - (client as CurrentUserClient).currentUser.me(), + (client as CurrentUserClient).currentUser.me({}), ); const who = me.userName ?? me.id ?? "unknown"; - // Read only now: the SDK resolves its config lazily on the first call, so - // before me() the host is still undefined. - const resolvedHost = sanitizeHost(resolvedHostOf(client)) ?? host; + const resolvedHost = sanitizeHost(await resolvedHostOf(client)) ?? host; // Credentials work, but the env/profile split still needs fixing — surface // it as a warning rather than letting a green tick imply all is well. @@ -238,9 +229,8 @@ export async function checkAuth(options: DoctorOptions): Promise { const shownProfile = profile ?? (sdkProfile ? `${sdkProfile} (resolved)` : undefined); // Prefer the host the SDK resolved (covers a profile-supplied host), then - // the one embedded in its error, then the raw env var. - const shownHost = - sanitizeHost(resolvedHostOf(built) ?? hostFromError(raw)) ?? host; + // the raw env var. + const shownHost = sanitizeHost(await resolvedHostOf(built)) ?? host; return { result: { status: "error", @@ -281,7 +271,7 @@ function authFailureHint( return "Check that the workspace host is correct and reachable (verify DATABRICKS_HOST or the profile's host, and that you're online)."; } if ( - /has no .* profile configured|profile .* (does not exist|not found)/i.test( + /has no .* profile configured|profile .*(does not exist|not found)/i.test( message, ) ) { @@ -289,7 +279,7 @@ function authFailureHint( } // Expired/failed token, or no credentials resolved: same next action. if ( - /cannot get access token|refresh token|reauthenticate|databricks auth token|token .*expired|cannot configure default credentials|default auth|no .*credentials/i.test( + /cannot get access token|refresh token|reauthenticate|databricks auth token|token .*expired|cannot configure default credentials|default auth|no .*credentials|host is required/i.test( message, ) ) { diff --git a/packages/shared/src/cli/commands/doctor/databricks-client.ts b/packages/shared/src/cli/commands/doctor/databricks-client.ts index 5c1b563c0..bb915b4f1 100644 --- a/packages/shared/src/cli/commands/doctor/databricks-client.ts +++ b/packages/shared/src/cli/commands/doctor/databricks-client.ts @@ -8,7 +8,8 @@ import { createWorkspaceClient, - loadConfigFile, + resolveProfile, + type WorkspaceClient, } from "../../../workspace-client"; /** @@ -41,14 +42,12 @@ function isModuleNotFound(err: unknown): boolean { } /** Constructs a workspace client via the SDK's unified-auth chain. An explicit - * `profile` is passed through `Config.profile` rather than mutating - * `process.env`, so it doesn't leak beyond this call. */ + * `profile` is passed as a client option rather than mutating `process.env`, so + * it doesn't leak beyond this call. */ export async function getServiceClient( profile?: string, ): Promise { - const client = createWorkspaceClient( - profile ? { profile } : {}, - ).toLegacyWorkspaceClient(); + const client = createWorkspaceClient(profile ? { profile } : {}); return { client }; } @@ -63,10 +62,8 @@ export async function getProfileHost( profile: string, ): Promise { try { - const { iniFile } = await loadConfigFile( - process.env.DATABRICKS_CONFIG_FILE, - ); - const host = iniFile?.[profile]?.host; + // `disableEnv`: only the file's value counts; `DATABRICKS_HOST` already won. + const { host } = await resolveProfile({ profile, disableEnv: true }); return typeof host === "string" && host.length > 0 ? host : undefined; } catch { return undefined; @@ -114,8 +111,15 @@ export async function getLakebasePool( // that dumps the raw SDK ApiError (stack + full response blob) to stderr on a // failed token fetch. doctor classifies and prints that failure itself, so the // library's dump is just noise on top of our clean one-line report. + // `@databricks/lakebase` is still typed against the legacy client: it calls + // `apiClient.request` and an argument-less `currentUser.me()`, which the + // modular SCIM client rejects, so hand it that exact shape. + const ws = client as WorkspaceClient; const pool = appkit.createLakebasePool({ - workspaceClient: client, + workspaceClient: { + apiClient: ws.apiClient, + currentUser: { me: () => ws.currentUser.me({}) }, + }, logger: { error: false }, }); return pool as unknown as LakebasePoolHandle; diff --git a/packages/shared/src/cli/commands/registry/workspace-picker.test.ts b/packages/shared/src/cli/commands/registry/workspace-picker.test.ts index 7153e0d96..819150577 100644 --- a/packages/shared/src/cli/commands/registry/workspace-picker.test.ts +++ b/packages/shared/src/cli/commands/registry/workspace-picker.test.ts @@ -70,7 +70,9 @@ describe("listWorkspaceResources", () => { it("returns choices from a successful warehouse list (SDK)", async () => { const factory = () => fakeClient({ - warehouses: { list: asyncList([{ id: "w1", name: "One" }]) }, + warehouses: { + listWarehousesIter: asyncList([{ id: "w1", name: "One" }]), + }, }); const res = await listWorkspaceResources( "sql_warehouse", @@ -83,10 +85,12 @@ describe("listWorkspaceResources", () => { }); }); - it("maps job_id + settings.name for jobs", async () => { + it("maps jobId (bigint) + settings.name for jobs", async () => { const factory = () => fakeClient({ - jobs: { list: asyncList([{ job_id: 42, settings: { name: "ETL" } }]) }, + jobs: { + listJobsIter: asyncList([{ jobId: 42n, settings: { name: "ETL" } }]), + }, }); const res = await listWorkspaceResources("job", undefined, factory); expect(res.choices).toEqual([{ value: "42", label: "ETL (42)" }]); @@ -96,8 +100,8 @@ describe("listWorkspaceResources", () => { const factory = () => fakeClient({ genie: { - listSpaces: async () => ({ - spaces: [{ space_id: "s1", title: "Sales" }], + genieListSpaces: async () => ({ + spaces: [{ spaceId: "s1", title: "Sales" }], }), }, }); @@ -105,23 +109,21 @@ describe("listWorkspaceResources", () => { expect(res.choices).toEqual([{ value: "s1", label: "Sales (s1)" }]); }); - // genie listSpaces is single-page; the adapter must follow next_page_token + // genie listSpaces is single-page; the adapter must follow nextPageToken // so large workspaces aren't capped at one page. - it("follows genie next_page_token across pages", async () => { - const pages: Record< - string, - { spaces: unknown[]; next_page_token?: string } - > = { - "": { spaces: [{ space_id: "s1" }], next_page_token: "p2" }, - p2: { spaces: [{ space_id: "s2" }] }, - }; + it("follows genie nextPageToken across pages", async () => { + const pages: Record = + { + "": { spaces: [{ spaceId: "s1" }], nextPageToken: "p2" }, + p2: { spaces: [{ spaceId: "s2" }] }, + }; const seen: (string | undefined)[] = []; const factory = () => fakeClient({ genie: { - listSpaces: async (req: { page_token?: string }) => { - seen.push(req.page_token); - return pages[req.page_token ?? ""]; + genieListSpaces: async (req: { pageToken?: string }) => { + seen.push(req.pageToken); + return pages[req.pageToken ?? ""]; }, }, }); @@ -134,9 +136,9 @@ describe("listWorkspaceResources", () => { const factory = () => fakeClient({ genie: { - listSpaces: async () => ({ - spaces: [{ space_id: "s1" }], - next_page_token: "same", + genieListSpaces: async () => ({ + spaces: [{ spaceId: "s1" }], + nextPageToken: "same", }), }, }); @@ -145,9 +147,34 @@ describe("listWorkspaceResources", () => { expect(res.choices.length).toBeGreaterThan(0); }); + // Services without a modular client go through the facade's raw request(). + it("lists REST-backed types via request(), following next_page_token", async () => { + const calls: unknown[] = []; + const pages: Record = { + "": { apps: [{ name: "a1" }], next_page_token: "p2" }, + p2: { apps: [{ name: "a2" }] }, + }; + const factory = () => + fakeClient({ + request: async (req: { + path: string; + query?: { page_token?: string }; + }) => { + calls.push(req); + return Response.json(pages[req.query?.page_token ?? ""]); + }, + }); + const res = await listWorkspaceResources("app", undefined, factory); + expect(res.choices.map((c) => c.value)).toEqual(["a1", "a2"]); + expect(calls).toEqual([ + { method: "GET", path: "/api/2.0/apps", query: undefined }, + { method: "GET", path: "/api/2.0/apps", query: { page_token: "p2" } }, + ]); + }); + it("passes the profile to the client factory", async () => { const factory = vi.fn(() => - fakeClient({ warehouses: { list: asyncList([]) } }), + fakeClient({ warehouses: { listWarehousesIter: asyncList([]) } }), ); await listWorkspaceResources("sql_warehouse", "my-profile", factory); expect(factory).toHaveBeenCalledWith("my-profile"); @@ -159,7 +186,7 @@ describe("listWorkspaceResources", () => { const factory = () => fakeClient({ warehouses: { - list: () => + listWarehousesIter: () => (async function* () { for (let i = 0; i < 10_000; i++) { yielded++; @@ -190,7 +217,7 @@ describe("listWorkspaceResources", () => { const factory = () => fakeClient({ warehouses: { - list: () => { + listWarehousesIter: () => { throw new Error("auth failed"); }, }, @@ -205,7 +232,7 @@ describe("listWorkspaceResources", () => { const factory = () => fakeClient({ warehouses: { - list: () => { + listWarehousesIter: () => { throw new Error( "default auth: cannot configure default credentials", ); diff --git a/packages/shared/src/cli/commands/registry/workspace-picker.ts b/packages/shared/src/cli/commands/registry/workspace-picker.ts index c2e5e9c49..ea98c50a4 100644 --- a/packages/shared/src/cli/commands/registry/workspace-picker.ts +++ b/packages/shared/src/cli/commands/registry/workspace-picker.ts @@ -2,15 +2,15 @@ import { spawnSync } from "node:child_process"; import { createWorkspaceClient, - type LegacyWorkspaceClient, + type WorkspaceClient, } from "../../../workspace-client"; /** * Lists a user's real Databricks workspace resources so `appkit add` can offer * a picker instead of blind free-text entry. * - * Flat resource types are listed via the Databricks SDK client (typed, - * auto-paginating) obtained through the sanctioned `workspace-client` facade. + * Flat resource types are listed through the sanctioned `workspace-client` + * facade: its modular SDK clients where one exists, else a raw REST `GET`. * Parent-context types (volume, uc_function, secret, vector_search_index) still * shell out to the `databricks` CLI for their drill-down. Every path fails * soft: any error returns an empty list and the caller drops to free-text entry. @@ -31,7 +31,7 @@ export interface WorkspaceChoice { * we simply iterate to completion. */ interface SdkLister { - list: (client: LegacyWorkspaceClient) => AsyncIterable; + list: (client: WorkspaceClient) => AsyncIterable; toChoice: (item: Record) => WorkspaceChoice | null; } @@ -53,61 +53,94 @@ function choiceFrom( } /** - * Genie listSpaces returns a single page, not an auto-paginating iterable like - * the other services. Adapt it to one that follows `next_page_token`; the - * repeated-token guard avoids an infinite loop if the API echoes a token back. + * Follows page tokens until exhausted. The repeated-token guard avoids an + * infinite loop if the API echoes a token back. */ -async function* iterateGenieSpaces( - client: LegacyWorkspaceClient, +async function* paginate( + fetchPage: ( + pageToken?: string, + ) => Promise<{ items?: unknown[]; next?: string }>, ): AsyncIterable { let pageToken: string | undefined; do { - const res = await client.genie.listSpaces( - pageToken ? { page_token: pageToken } : {}, - ); - for (const space of res.spaces ?? []) yield space; - const next = res.next_page_token; + const { items, next } = await fetchPage(pageToken); + for (const item of items ?? []) yield item; if (next && next === pageToken) break; pageToken = next; } while (pageToken); } +/** + * Lists a REST collection (snake_case body, `page_token`/`next_page_token`), + * for services the facade has no modular client for yet. + */ +function restList( + client: WorkspaceClient, + path: string, + key: string, +): AsyncIterable { + return paginate(async (pageToken) => { + const res = await client.request({ + method: "GET", + path, + query: pageToken ? { page_token: pageToken } : undefined, + }); + const body = (await res.json()) as Record; + return { + items: body[key] as unknown[] | undefined, + next: body.next_page_token as string | undefined, + }; + }); +} + +/** Genie listSpaces returns a single page; follow `nextPageToken`. */ +function iterateGenieSpaces(client: WorkspaceClient): AsyncIterable { + return paginate(async (pageToken) => { + const res = await client.genie.genieListSpaces( + pageToken ? { pageToken } : {}, + ); + return { items: res.spaces, next: res.nextPageToken }; + }); +} + /** Flat, top-level listable resource types, backed by SDK services. */ export const SDK_LISTERS: Record = { sql_warehouse: { - list: (c) => c.warehouses.list({}), + list: (c) => c.warehouses.listWarehousesIter({}), toChoice: (i) => choiceFrom(i, "id", "name"), }, job: { - list: (c) => c.jobs.list({}), - // job name lives under settings.name; id is top-level job_id + list: (c) => c.jobs.listJobsIter({}), + // job name lives under settings.name; id is top-level jobId (a bigint) toChoice: (i) => { const settings = i.settings as { name?: string } | undefined; - return choiceFrom({ ...i, name: settings?.name }, "job_id", "name"); + return choiceFrom({ ...i, name: settings?.name }, "jobId", "name"); }, }, serving_endpoint: { - list: (c) => c.servingEndpoints.list(), + list: (c) => restList(c, "/api/2.0/serving-endpoints", "endpoints"), toChoice: (i) => choiceFrom(i, "name", "name"), }, uc_connection: { - list: (c) => c.connections.list({}), + list: (c) => + restList(c, "/api/2.1/unity-catalog/connections", "connections"), toChoice: (i) => choiceFrom(i, "name", "full_name"), }, database: { - list: (c) => c.database.listDatabaseInstances({}), + list: (c) => + restList(c, "/api/2.0/database/instances", "database_instances"), toChoice: (i) => choiceFrom(i, "name", "name"), }, genie_space: { list: iterateGenieSpaces, - toChoice: (i) => choiceFrom(i, "space_id", "title"), + toChoice: (i) => choiceFrom(i, "spaceId", "title"), }, experiment: { - list: (c) => c.experiments.listExperiments({}), + list: (c) => restList(c, "/api/2.0/mlflow/experiments/list", "experiments"), toChoice: (i) => choiceFrom(i, "experiment_id", "name"), }, app: { - list: (c) => c.apps.list({}), + list: (c) => restList(c, "/api/2.0/apps", "apps"), toChoice: (i) => choiceFrom(i, "name", "name"), }, }; @@ -118,19 +151,15 @@ export function isFlatListable(resourceType: string): boolean { } /** - * Constructs a raw SDK workspace client for the given profile (or default - * resolution), via the sanctioned `workspace-client` facade. Uses the legacy - * escape hatch because the picker needs services (connections, database, - * experiments, apps) the facade doesn't yet proxy directly. + * Constructs a workspace client for the given profile (or default resolution), + * via the sanctioned `workspace-client` facade. */ -export function makeWorkspaceClient(profile?: string): LegacyWorkspaceClient { - return createWorkspaceClient( - profile ? { profile } : {}, - ).toLegacyWorkspaceClient(); +export function makeWorkspaceClient(profile?: string): WorkspaceClient { + return createWorkspaceClient(profile ? { profile } : {}); } /** - * Max resources fetched for the picker. `list()` auto-paginates, so on a large + * Max resources fetched for the picker. `list()` paginates lazily, so on a large * workspace (5000+ warehouses) breaking out at the cap stops pagination early; * the picker's "Enter manually" option covers anything beyond it. */ @@ -161,9 +190,7 @@ function shortErrorMessage(err: unknown): string { export async function listWorkspaceResources( resourceType: string, profile?: string, - clientFactory: ( - profile?: string, - ) => LegacyWorkspaceClient = makeWorkspaceClient, + clientFactory: (profile?: string) => WorkspaceClient = makeWorkspaceClient, ): Promise { const lister = SDK_LISTERS[resourceType]; if (!lister) return { choices: [], truncated: false }; diff --git a/packages/shared/src/workspace-client/index.ts b/packages/shared/src/workspace-client/index.ts index 74f08cb15..1813c20da 100644 --- a/packages/shared/src/workspace-client/index.ts +++ b/packages/shared/src/workspace-client/index.ts @@ -6,6 +6,7 @@ */ export { ApiError } from "./errors"; export { createWorkspaceClient } from "./factory"; +export { resolveProfile } from "./modular"; export type { CancellationToken, ClientOptions, diff --git a/packages/shared/src/workspace-client/modular.ts b/packages/shared/src/workspace-client/modular.ts index a4b68a77e..e47438c87 100644 --- a/packages/shared/src/workspace-client/modular.ts +++ b/packages/shared/src/workspace-client/modular.ts @@ -393,6 +393,12 @@ export function buildTablesClient(opts: WorkspaceClientOptions): TablesClient { return new TablesClient(mapToClientOptions(opts)); } +/** + * Resolve a `~/.databrickscfg` profile (+ env overlay, unless disabled). Exposed + * so the CLI can read a profile offline without importing the SDK directly. + */ +export { resolve as resolveProfile } from "@databricks/sdk-core/profiles"; + // ── Client type re-exports (for the facade accessor types) ─────────────── export type { FilesClient } from "@databricks/sdk-files/v2"; export type { GenieClient } from "@databricks/sdk-genie/v1";