From b6602939e04b37f670e44b2ce510cd45b168ad84 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Tue, 11 Aug 2026 19:45:45 +0200 Subject: [PATCH 1/3] refactor(cli): route doctor's SDK access through the shared workspace-client facade MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that the SDK facade lives in packages/shared/src/workspace-client, the doctor command reaches the SDK through it instead of a dynamic import + noRestrictedImports suppression: - getServiceClient uses createWorkspaceClient({profile}).toLegacyWorkspaceClient() - getProfileHost uses the facade's re-exported loadConfigFile - drops both `biome-ignore` suppressions and the dynamic import(...) dance - adds loadConfigFile to the facade's re-exports SdkNotInstalledError is kept exported (no longer thrown — shared now hard-depends on the SDK) so the SDK_NOT_INSTALLED diagnostic branch and tests keep compiling. The Lakebase probe keeps its dynamic @databricks/appkit import (optional peer). Also removes @databricks/sdk-experimental from packages/appkit/package.json: with the facade moved to shared, appkit no longer imports the SDK directly. It still ships to consumers via shared's deps (dist-appkit.ts merges them into the tarball). Signed-off-by: MarioCadenas --- .../cli/commands/doctor/databricks-client.ts | 56 ++++++++----------- packages/shared/src/workspace-client/index.ts | 1 + .../shared/src/workspace-client/legacy.ts | 2 +- 3 files changed, 25 insertions(+), 34 deletions(-) diff --git a/packages/shared/src/cli/commands/doctor/databricks-client.ts b/packages/shared/src/cli/commands/doctor/databricks-client.ts index 24304640e..5c1b563c0 100644 --- a/packages/shared/src/cli/commands/doctor/databricks-client.ts +++ b/packages/shared/src/cli/commands/doctor/databricks-client.ts @@ -1,28 +1,33 @@ /** - * The single seam where `appkit doctor` crosses into the Databricks SDK. The - * SDK-free `shared` package reaches it via a runtime `import(...)`, degrading - * gracefully when it's absent. - * - * `noRestrictedImports` normally routes SDK access through - * `packages/appkit/src/workspace-client`, but that isn't available here: `appkit` - * depends on `shared`, so importing it back would be a dependency cycle. `shared` - * also can't depend on the SDK directly — hence the dynamic `import(...)` and the - * per-call suppressions below. This file *is* the wrapper for this package, and - * every SDK reference in the doctor command is confined to it. + * The seam where `appkit doctor` crosses into the Databricks SDK. It reaches + * the SDK through the shared `workspace-client` facade — the one sanctioned SDK + * import site — so no dynamic import or `noRestrictedImports` suppression is + * needed. (The Lakebase probe still dynamically imports `@databricks/appkit` + * below, since that's an optional peer `shared` deliberately doesn't depend on.) */ -/** Raised when `@databricks/sdk-experimental` is not resolvable at runtime. */ +import { + createWorkspaceClient, + loadConfigFile, +} from "../../../workspace-client"; + +/** + * Retained for backward compatibility. Since `shared` now depends on the SDK + * (reached via the workspace-client facade), the SDK is always resolvable at + * runtime and this is no longer thrown; kept exported so existing callers and + * the `SDK_NOT_INSTALLED` diagnostic branch keep compiling. + */ export class SdkNotInstalledError extends Error { constructor() { super( - "The 'doctor' command requires the Databricks SDK (a dependency of @databricks/appkit). Please install @databricks/appkit to run connection checks.", + "The 'doctor' command requires the Databricks SDK (a dependency of @databricks/appkit).", ); this.name = "SdkNotInstalledError"; } } interface ServiceClientHandle { - /** WorkspaceClient, typed as unknown to keep `shared` SDK-free. */ + /** WorkspaceClient, typed as unknown to keep doctor's call sites SDK-agnostic. */ client: unknown; } @@ -35,24 +40,15 @@ function isModuleNotFound(err: unknown): boolean { ); } -/** Constructs a `WorkspaceClient` via the SDK's unified-auth chain. An explicit +/** 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. */ export async function getServiceClient( profile?: string, ): Promise { - let sdk: { WorkspaceClient: new (opts: Record) => unknown }; - try { - // biome-ignore lint/style/noRestrictedImports: shared can't reach appkit's workspace-client wrapper (appkit depends on shared); this file is the SDK seam - sdk = (await import("@databricks/sdk-experimental")) as typeof sdk; - } catch (err) { - if (isModuleNotFound(err)) { - throw new SdkNotInstalledError(); - } - throw err; - } - - const client = new sdk.WorkspaceClient(profile ? { profile } : {}); + const client = createWorkspaceClient( + profile ? { profile } : {}, + ).toLegacyWorkspaceClient(); return { client }; } @@ -67,13 +63,7 @@ export async function getProfileHost( profile: string, ): Promise { try { - // biome-ignore lint/style/noRestrictedImports: see the note at the top of this file — shared can't import appkit's wrapper - const sdk = (await import("@databricks/sdk-experimental")) as { - loadConfigFile: ( - file?: string, - ) => Promise<{ iniFile: Record }>; - }; - const { iniFile } = await sdk.loadConfigFile( + const { iniFile } = await loadConfigFile( process.env.DATABRICKS_CONFIG_FILE, ); const host = iniFile?.[profile]?.host; diff --git a/packages/shared/src/workspace-client/index.ts b/packages/shared/src/workspace-client/index.ts index 7645328e2..992be412b 100644 --- a/packages/shared/src/workspace-client/index.ts +++ b/packages/shared/src/workspace-client/index.ts @@ -18,6 +18,7 @@ export type { export { ConfigError, Context, + loadConfigFile, Time, TimeUnits, } from "./legacy"; diff --git a/packages/shared/src/workspace-client/legacy.ts b/packages/shared/src/workspace-client/legacy.ts index b8f467f4e..307d87a93 100644 --- a/packages/shared/src/workspace-client/legacy.ts +++ b/packages/shared/src/workspace-client/legacy.ts @@ -87,7 +87,7 @@ export type { // named 'Time'" at ESM link time. `Time` is only reachable via the module // object, so we fall back to `SDK.default.Time` (matching the original genie // connector's `SDK.Time ?? SDK.default.Time` guard). -export const { ConfigError, Context, TimeUnits } = SDK; +export const { ConfigError, Context, TimeUnits, loadConfigFile } = SDK; export const Time = SDK.Time ?? (SDK as unknown as { default: typeof SDK }).default.Time; From 78f054cf622b095db0cd961e737867d9aacf4116 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Fri, 14 Aug 2026 09:55:12 +0200 Subject: [PATCH 2/3] fix(cli): ship the shared workspace-client in the appkit tarball The doctor command now reaches the SDK through the shared workspace-client facade, so shared's compiled CLI imports ../../../workspace-client/{factory,legacy}.js. dist-appkit copied shared's dist/cli into the tarball but not dist/workspace-client, so any appkit CLI invocation (e.g. the generate-types postinstall, which loads all commands) crashed with ERR_MODULE_NOT_FOUND. Copy dist/workspace-client alongside the other CLI leaf modules so those relative imports resolve. Signed-off-by: MarioCadenas --- tools/dist-appkit.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tools/dist-appkit.ts b/tools/dist-appkit.ts index 9005e8905..fee75f96a 100644 --- a/tools/dist-appkit.ts +++ b/tools/dist-appkit.ts @@ -101,6 +101,20 @@ if (fs.existsSync(sharedBin)) { fs.copyFileSync(sharedNaming, "tmp/dist/naming.js"); } + // The doctor command reaches the SDK through the shared workspace-client + // facade, whose modules live outside dist/cli. Copy them so the CLI's + // relative imports (../../../workspace-client/*) resolve in the published + // tarball — appkit's own dist/workspace-client is only a re-export shim. + const sharedWorkspaceClient = path.join( + __dirname, + "../packages/shared/dist/workspace-client", + ); + if (fs.existsSync(sharedWorkspaceClient)) { + fs.cpSync(sharedWorkspaceClient, "tmp/dist/workspace-client", { + recursive: true, + }); + } + // Copy JSON schemas so CLI (e.g. plugin validate/sync) can load them at runtime. // Place in both dist/schemas and dist/cli/schemas so resolution works whether // the running module's __dirname is under dist/ or dist/cli/ (e.g. after bundling). From 94b61c155b5d954eadf2525f9844d4291609f20b Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Fri, 14 Aug 2026 10:08:04 +0200 Subject: [PATCH 3/3] test(cli): stub loadConfigFile in the workspace-client SDK mock The doctor routing re-exports loadConfigFile from legacy.ts's SDK destructure; add it to legacy.test.ts's vi.mock so module load doesn't throw "No loadConfigFile export is defined on the mock". Signed-off-by: MarioCadenas --- packages/shared/src/workspace-client/tests/legacy.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/shared/src/workspace-client/tests/legacy.test.ts b/packages/shared/src/workspace-client/tests/legacy.test.ts index a7e705212..95d67e544 100644 --- a/packages/shared/src/workspace-client/tests/legacy.test.ts +++ b/packages/shared/src/workspace-client/tests/legacy.test.ts @@ -18,6 +18,7 @@ vi.mock("@databricks/sdk-experimental", () => ({ Context: class Context {}, Time: class Time {}, TimeUnits: { milliseconds: 0 }, + loadConfigFile: vi.fn(), })); import { buildLegacyWorkspaceClient } from "../legacy";