diff --git a/packages/appkit/src/connectors/lakebase/index.ts b/packages/appkit/src/connectors/lakebase/index.ts index 17d7491f6..8de651471 100644 --- a/packages/appkit/src/connectors/lakebase/index.ts +++ b/packages/appkit/src/connectors/lakebase/index.ts @@ -13,11 +13,9 @@ import { createLogger } from "../../logging/logger"; * @returns PostgreSQL pool with appkit integration */ export function createLakebasePool(config?: Partial): Pool { - const logger = createLogger("connectors:lakebase"); - return createLakebasePoolBase({ + logger: createLogger("connectors:lakebase"), ...config, - logger, }); } diff --git a/packages/shared/package.json b/packages/shared/package.json index 7045336fc..e155769ae 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -26,6 +26,7 @@ }, "devDependencies": { "@types/express": "4.17.23", + "@types/js-yaml": "4.0.9", "@types/json-schema": "7.0.15", "@types/node": "25.2.3", "@types/ws": "8.18.1" @@ -43,6 +44,9 @@ "@standard-schema/spec": "1.1.0", "@clack/prompts": "1.0.1", "commander": "12.1.0", + "dotenv": "16.6.1", + "js-yaml": "4.2.0", + "picocolors": "1.1.1", "zod": "4.3.6" } } diff --git a/packages/shared/src/cli/commands/doctor/README.md b/packages/shared/src/cli/commands/doctor/README.md new file mode 100644 index 000000000..a2ddae53b --- /dev/null +++ b/packages/shared/src/cli/commands/doctor/README.md @@ -0,0 +1,172 @@ +# `appkit doctor` + +Diagnoses whether an AppKit app's declared Databricks resources are actually +usable — going beyond the startup env-var presence check to probe existence and +reachability against the live workspace. + +## The check model — three layers + +For each declared resource, doctor runs the offline `config` layer then the live +`existence` layer; `auth` runs once up front. It stops at the first hard failure +so the reported problem is the *root* cause, not a symptom. + +| Layer | Question | How | +| ------------ | ----------------------------------------------------- | ---------------------------------------------------------- | +| `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` | + +`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` +validates it structurally, since a bad host means no client can be built.) + +Every live call (`auth`'s `currentUser.me()` and each `existence` probe) is +bounded by a 10s wall-clock deadline (`withTimeout`). A reachable-but-unresponsive +endpoint must never hang doctor — it's a CI gate that has to return — so a +timeout surfaces as an error (`PROBE_TIMEOUT`, or an auth failure) rather than a +hung process. The race can't cancel the underlying request, but it stops doctor +*waiting* on it. (Lakebase connections are separately bounded at 10s by the +pool's `connectionTimeoutMillis`.) + +## Which plugins are checked + +`plugin sync` writes `appkit.plugins.json` cataloguing *every* plugin the +installed packages ship (for `apps init`), and marks the ones actually wired +into `createApp` with `requiredByTemplate: true`. Doctor checks exactly those, so +an unused built-in doesn't produce phantom "missing env var" errors. + +> **Known limitation — non-GA plugins aren't checked yet.** `plugin sync` strips +> `requiredByTemplate` for beta/experimental plugins (its step 6b), so a *used* +> non-GA plugin (e.g. the agents plugin requiring a serving endpoint) is +> currently skipped. A proper fix needs a usage signal that survives sync +> regardless of stability tier (e.g. a separate `used` marker written before the +> strip); that's a `plugin sync` change tracked as a fast-follow. Until then, +> doctor covers GA plugins wired into your app. + +## Resource provenance: external vs bundle-managed + +Each resource is one of two kinds, which changes what doctor does with it: + +- **external** — referenced by id/name (`${var.*}` or a literal in + `databricks.yml`). It should exist now, so the live `existence` probe applies. +- **bundle-managed** — created by this bundle on deploy + (`${resources...*}`). It doesn't exist until `bundle deploy`, so + probing it would be a false `NOT_FOUND`; doctor reports it as *will be created + on deploy* instead. + +Provenance comes from `bundle.ts`, which reads `databricks.yml` + `app.yaml` and +classifies each binding, then overlays `origin` onto the manifest-sourced +targets. Apps with no `databricks.yml` behave exactly as before (everything +treated as external). + +### Wiring check (`checks-wiring.ts`) + +An offline, auth-independent check of the three-file join that +`databricks bundle validate` can't see (it doesn't know `app.yaml`↔plugin +wiring): + +- `VALUEFROM_UNBOUND` — an `app.yaml` `valueFrom: ` matches no + `databricks.yml` binding (the env var would never be injected). +- `BUNDLE_REF_MISSING` — a `${resources...*}` binding references a + resource not declared in the bundle (deploy would fail). +- `ENV_UNWIRED` (warn) — a plugin needs an env var no `app.yaml` entry provides. + +A wiring `error` gates the exit code, so `appkit doctor` catches deploy-breaking +misconfiguration pre-deploy. + +## Report rendering (`report.ts`) + +One flat, severity-sorted checklist — no titled sub-sections (which read as +inconsistent next to the header-less `Auth` row). `Auth` is always the first +row; every resource and wiring finding follows in the same list, sorted +most-severe first. Bundle-managed resources appear as *will be created on +deploy* rather than a probe result. + +Optimised for a quiet happy path: a healthy resource is just a green tick and +its name — no plugin/type attribution, no per-layer output. Detail (and a +`Hint:`, offset by a blank line so it sits apart from the error) appears only +beneath rows that aren't `ok`. There's no header block; the profile in play is +shown only when auth *fails* (attached under the auth row, where it's the first +thing to sanity-check). + +Colour (via `picocolors`) uses one tight palette: + +- **glyphs** carry status — green `✓` / yellow `⚠` / red `✗` / dim `•`; +- **cyan** marks a literal token you'd type or reference — a `"quoted"` id or + binding name, or a `` `backticked` `` code/command span; +- **bold** marks a `SCREAMING_SNAKE` env-var name (bold + cyan when it sits + inside a code span); +- resource *names* are left unstyled — only the id/code you'd act on is coloured. + +`picocolors` auto-disables for non-TTY output and honours `NO_COLOR`, so +piped/CI output is plain text. + +### Auth outcomes + +Host and credentials aren't resolved by doctor — an empty `WorkspaceClient({})` +defers to the SDK's unified-auth chain (explicit env, then the selected +`~/.databrickscfg` profile, then OAuth). `--profile` is forwarded via +`DATABRICKS_CONFIG_PROFILE`; with neither host nor profile set, the SDK falls +back to the default profile. Doctor emits: + +| Code | When | Detected | +| ---- | ---- | -------- | +| `HOST_INVALID` | `DATABRICKS_HOST` set but malformed / placeholder | offline, before any network | +| `SDK_NOT_INSTALLED` | Databricks SDK not resolvable | client build | +| `AUTH_OK` | `currentUser.me()` succeeded | live | +| `AUTH_FAILED` | `me()` threw | live | + +`AUTH_FAILED` carries an action-first `hint` inferred from the SDK message +(workspace unreachable, profile not found, expired login / no credentials). Each +hint says what to *run* and to confirm the profile/host actually in use — a +failure can mean the wrong target, not just a stale token. `detail` is the short +headline `authentication failed`; the full SDK message is kept in `raw` and shown +only with `--detail` — in both the human report and `--json` (i.e. `--json` +alone omits `raw`, since it can carry sensitive detail and CI often captures it; +`--json --detail` opts back in). `DATABRICKS_HOST` is stored with any embedded +`user:pass@` userinfo stripped, so credentials never reach the report or JSON. +The report labels hints `Hint:`. + +## Files + +- `types.ts` — the contract: layers, statuses, `ResourceTarget`, `DoctorReport`. +- `resolve-targets.ts` — reads `appkit.plugins.json` → `ResourceTarget[]`. +- `bundle.ts` — reads `databricks.yml` + `app.yaml` for binding provenance + (external vs bundle-managed) and the env↔binding wiring map. +- `databricks-client.ts` — the sole SDK seam: dynamic `import()` of the SDK / + `@databricks/appkit`, builds a `WorkspaceClient` and Lakebase pool, graceful + fallback when uninstalled. +- `checks.ts` — `checkAuth`, `checkConfig`. +- `checks-existence.ts` — per-type existence probe dispatch + error classifier. +- `checks-wiring.ts` — offline three-file join / bundle-ref consistency check. +- `run.ts` — orchestration: auth once → overlay origin → per resource + (config → existence, skipping bundle-managed) → wiring check. +- `report.ts` — runtime / deploy sections, `--json`, exit code. +- `index.ts` — the Commander command + flags. + +## Flags + +- `--profile ` — Databricks CLI profile to authenticate with. +- `--env-file ` — load an env file (e.g. `.env.local`) before checking. +- `--detail` — show full underlying error messages. +- `--json` — machine-readable report. + +Exit code is non-zero if auth, any resource, or any wiring finding is in an +`error` state, so `appkit doctor` can gate CI / pre-deploy. + +The report's `summary` counts *everything* with a status — resources, the auth +check, and wiring findings — so a `--json` consumer can trust +`summary.error === 0` to mean "nothing failed". The report also carries a +top-level `exitCode` (0/1) as the single unambiguous pass/fail signal. Both are +computed once in `runDoctor`, so `--json` and the human report never disagree. + +Checks run as the identity that runs doctor (the developer locally, the app in +deployment). + +## Existence coverage + +Control-plane `.get()` for `sql_warehouse`, `serving_endpoint`, `genie_space`, +`job`, `volume`, `vector_search_index`, and `uc_function`; a real `SELECT 1` +connection for `postgres`/Lakebase. Other types report `skipped`. + +Requires live credentials (a configured Databricks profile/token). diff --git a/packages/shared/src/cli/commands/doctor/bundle.test.ts b/packages/shared/src/cli/commands/doctor/bundle.test.ts new file mode 100644 index 000000000..6fa9d1535 --- /dev/null +++ b/packages/shared/src/cli/commands/doctor/bundle.test.ts @@ -0,0 +1,116 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { originForEnvVars, readBundleInfo } from "./bundle"; + +function tmp(files: Record): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "doctor-bundle-")); + for (const [name, content] of Object.entries(files)) { + fs.writeFileSync(path.join(dir, name), content); + } + return dir; +} + +const BUNDLE = ` +resources: + apps: + app: + name: my-app + resources: + - name: sql-warehouse # external: \${var.*} + sql_warehouse: + id: \${var.sql_warehouse_id} + permission: CAN_USE + - name: report-job # bundle-managed: \${resources.*} + job: + id: \${resources.jobs.report.id} + permission: CAN_MANAGE_RUN + jobs: + report: + name: report +`; + +const APP_YAML = ` +env: + - name: DATABRICKS_WAREHOUSE_ID + valueFrom: sql-warehouse + - name: DATABRICKS_JOB_REPORT + valueFrom: report-job +`; + +describe("readBundleInfo", () => { + const dirs: string[] = []; + afterEach(() => { + for (const d of dirs) fs.rmSync(d, { recursive: true, force: true }); + dirs.length = 0; + }); + + it("returns present:false when no databricks.yml exists", () => { + const dir = tmp({}); + dirs.push(dir); + const info = readBundleInfo(dir); + expect(info.present).toBe(false); + expect(info.bindings.size).toBe(0); + }); + + it("throws on malformed databricks.yml instead of treating it as absent", () => { + // A present-but-unparseable bundle is a deploy-breaking error; swallowing it + // would let doctor skip all wiring checks and report a false all-clear. + const dir = tmp({ "databricks.yml": "resources: [ this: is: not: valid" }); + dirs.push(dir); + expect(() => readBundleInfo(dir)).toThrow(/databricks\.yml/); + }); + + it("throws on malformed app.yaml", () => { + const dir = tmp({ + "databricks.yml": BUNDLE, + "app.yaml": "env:\n - name: X\n valueFrom: bad-indent", + }); + dirs.push(dir); + expect(() => readBundleInfo(dir)).toThrow(/app\.yaml/); + }); + + it("classifies external (var) vs bundle-managed (resources ref) bindings", () => { + const dir = tmp({ "databricks.yml": BUNDLE, "app.yaml": APP_YAML }); + dirs.push(dir); + const info = readBundleInfo(dir); + expect(info.present).toBe(true); + expect(info.bindings.get("sql-warehouse")?.origin).toBe("external"); + const job = info.bindings.get("report-job"); + expect(job?.origin).toBe("bundle-managed"); + expect(job?.ref).toEqual({ type: "jobs", key: "report" }); + }); + + it("records declared bundle resources and the app.yaml env→binding map", () => { + const dir = tmp({ "databricks.yml": BUNDLE, "app.yaml": APP_YAML }); + dirs.push(dir); + const info = readBundleInfo(dir); + expect(info.declaredResources.has("jobs.report")).toBe(true); + expect(info.envToBinding.get("DATABRICKS_WAREHOUSE_ID")).toBe( + "sql-warehouse", + ); + }); +}); + +describe("originForEnvVars", () => { + const dirs: string[] = []; + afterEach(() => { + for (const d of dirs) fs.rmSync(d, { recursive: true, force: true }); + dirs.length = 0; + }); + + it("resolves origin by walking env var → binding → origin", () => { + const dir = tmp({ "databricks.yml": BUNDLE, "app.yaml": APP_YAML }); + dirs.push(dir); + const info = readBundleInfo(dir); + expect(originForEnvVars(["DATABRICKS_WAREHOUSE_ID"], info)).toBe( + "external", + ); + expect(originForEnvVars(["DATABRICKS_JOB_REPORT"], info)).toBe( + "bundle-managed", + ); + // Unknown env / no bundle → undefined (caller treats as external). + expect(originForEnvVars(["NOPE"], info)).toBeUndefined(); + }); +}); diff --git a/packages/shared/src/cli/commands/doctor/bundle.ts b/packages/shared/src/cli/commands/doctor/bundle.ts new file mode 100644 index 000000000..1d450cdf1 --- /dev/null +++ b/packages/shared/src/cli/commands/doctor/bundle.ts @@ -0,0 +1,181 @@ +/** + * Reads `databricks.yml` + `app.yaml` for two things doctor can't get from + * `appkit.plugins.json`: + * + * 1. **Provenance** — whether each binding references an existing resource + * (`${var.*}`/literal → `external`) or one this bundle creates + * (`${resources...*}` → `bundle-managed`). + * 2. **Wiring** — the binding `name` joins the three files: `app.yaml`'s + * `valueFrom: ` must match a `databricks.yml` binding `name`. + * + * SDK-free; pure YAML parsing. Absent files degrade to empty results. + */ + +import fs from "node:fs"; +import path from "node:path"; +import yaml from "js-yaml"; +import type { ResourceOrigin } from "./types"; +import { errorMessage } from "./utils"; + +export const DEFAULT_BUNDLE_FILE = "databricks.yml"; +export const DEFAULT_APP_YAML_FILE = "app.yaml"; + +/** A `${resources...}` reference — a bundle-created resource. */ +const RESOURCES_REF = /\$\{resources\.([^.]+)\.([^.}]+)\.[^}]+\}/; + +/** One app resource binding from `resources.apps..resources[]`. */ +export interface BundleBinding { + /** The binding name — the join key with `app.yaml` `valueFrom`. */ + name: string; + /** The typed sub-key that names the kind (`sql_warehouse`, `genie_space`, …). */ + type?: string; + origin: ResourceOrigin; + /** For bundle-managed bindings: the `.` it references, if parseable. */ + ref?: { type: string; key: string }; +} + +export interface BundleInfo { + /** Bindings under every app, keyed by binding name. */ + bindings: Map; + /** `app.yaml` env var name → binding name (from `valueFrom`). */ + envToBinding: Map; + /** Declared bundle resource keys, as `"."`, for ref-integrity. */ + declaredResources: Set; + /** True when a `databricks.yml` was found and parsed. */ + present: boolean; +} + +interface AppResourceBlock { + name?: string; + [k: string]: unknown; +} +interface BundleAppBlock { + resources?: AppResourceBlock[]; +} +interface BundleDoc { + resources?: { + apps?: Record; + [otherType: string]: Record | undefined; + }; +} +interface AppYamlDoc { + env?: Array<{ name?: string; valueFrom?: string }>; +} + +/** The typed sub-key of a binding is its single non-`name` object property. */ +function bindingType(block: AppResourceBlock): string | undefined { + for (const [k, v] of Object.entries(block)) { + if (k === "name") continue; + if (v && typeof v === "object") return k; + } + return undefined; +} + +/** Classifies a binding by its typed sub-key and origin: scanning its field + * values for a `${resources.*}` reference (bundle-managed) vs anything else + * (external). */ +function classifyBinding(block: AppResourceBlock): { + type?: string; + origin: ResourceOrigin; + ref?: { type: string; key: string }; +} { + const type = bindingType(block); + const typed = type ? block[type] : undefined; + if (typed && typeof typed === "object") { + for (const value of Object.values(typed as Record)) { + if (typeof value !== "string") continue; + const m = value.match(RESOURCES_REF); + if (m) { + return { + type, + origin: "bundle-managed", + ref: { type: m[1], key: m[2] }, + }; + } + } + } + return { type, origin: "external" }; +} + +/** + * Reads and parses a YAML file. Returns `null` when the file is absent (a + * legitimate "no bundle" state); throws on invalid YAML, since silently + * ignoring it would let doctor report a false all-clear. + */ +function readYaml(filePath: string): T | null { + if (!fs.existsSync(filePath)) return null; + try { + return (yaml.load(fs.readFileSync(filePath, "utf-8")) ?? {}) as T; + } catch (err) { + throw new Error( + `Failed to parse ${path.basename(filePath)}: ${errorMessage(err)}`, + ); + } +} + +/** + * Parses `databricks.yml` (+ `app.yaml`) into {@link BundleInfo}. Returns an + * empty-but-`present:false` result when no bundle is found, so callers can treat + * "no bundle" as "everything external" without special-casing. + */ +export function readBundleInfo( + cwd: string = process.cwd(), + bundleFile: string = DEFAULT_BUNDLE_FILE, + appYamlFile: string = DEFAULT_APP_YAML_FILE, +): BundleInfo { + const bindings = new Map(); + const envToBinding = new Map(); + const declaredResources = new Set(); + + const doc = readYaml(path.resolve(cwd, bundleFile)); + if (!doc) { + return { bindings, envToBinding, declaredResources, present: false }; + } + + for (const [type, group] of Object.entries(doc.resources ?? {})) { + if (!group || typeof group !== "object") continue; + for (const key of Object.keys(group)) + declaredResources.add(`${type}.${key}`); + } + + for (const app of Object.values(doc.resources?.apps ?? {})) { + for (const block of app.resources ?? []) { + if (!block?.name) continue; + const { type, origin, ref } = classifyBinding(block); + bindings.set(block.name, { + name: block.name, + type, + origin, + ref, + }); + } + } + + const appYaml = readYaml(path.resolve(cwd, appYamlFile)); + for (const entry of appYaml?.env ?? []) { + if (entry?.name && entry.valueFrom) { + envToBinding.set(entry.name, entry.valueFrom); + } + } + + return { bindings, envToBinding, declaredResources, present: true }; +} + +/** + * Resolves the origin for a target given its env vars, by walking + * env var → binding name → binding origin. Returns undefined when the bundle + * has nothing to say (so the target stays external by default). + */ +export function originForEnvVars( + envVars: string[], + info: BundleInfo, +): ResourceOrigin | undefined { + if (!info.present) return undefined; + for (const env of envVars) { + const bindingName = info.envToBinding.get(env); + if (!bindingName) continue; + const binding = info.bindings.get(bindingName); + if (binding) return binding.origin; + } + return undefined; +} diff --git a/packages/shared/src/cli/commands/doctor/checks-existence.test.ts b/packages/shared/src/cli/commands/doctor/checks-existence.test.ts new file mode 100644 index 000000000..7fe559d2b --- /dev/null +++ b/packages/shared/src/cli/commands/doctor/checks-existence.test.ts @@ -0,0 +1,330 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { runExistenceProbe, toThreeLevelVolumeName } from "./checks-existence"; +import type { ResourceTarget } from "./types"; + +// Mock the SDK bridge so the postgres probe runs without a real connection. +const { mockGetLakebasePool, endSpy } = vi.hoisted(() => ({ + mockGetLakebasePool: vi.fn(), + endSpy: vi.fn(async () => {}), +})); +vi.mock("./databricks-client", () => ({ + AppkitNotInstalledError: class AppkitNotInstalledError extends Error {}, + getLakebasePool: mockGetLakebasePool, +})); + +function target(overrides: Partial = {}): ResourceTarget { + return { + type: "sql_warehouse", + resourceKey: "sql-warehouse", + alias: "SQL Warehouse", + plugin: "analytics", + requiredPermission: "CAN_USE", + required: true, + envVars: ["DATABRICKS_WAREHOUSE_ID"], + fieldValues: { id: "wh-123" }, + ...overrides, + }; +} + +/** Builds an SDK-like error carrying a statusCode (as ApiError does). */ +function apiError(statusCode: number, message = "boom"): Error { + return Object.assign(new Error(message), { statusCode }); +} + +describe("runExistenceProbe — sql_warehouse", () => { + it("ok when the warehouse exists and is RUNNING", async () => { + const client = { + warehouses: { get: async () => ({ state: "RUNNING" }) }, + }; + const r = await runExistenceProbe(client, target()); + expect(r.status).toBe("ok"); + }); + + it("warns when the warehouse exists but is STOPPED", async () => { + const client = { + warehouses: { get: async () => ({ state: "STOPPED" }) }, + }; + const r = await runExistenceProbe(client, target()); + expect(r.status).toBe("warn"); + expect(r.code).toBe("WAREHOUSE_NOT_RUNNING"); + }); + + it("errors NOT_FOUND on a 404", async () => { + const client = { + warehouses: { + get: async () => { + throw apiError(404); + }, + }, + }; + const r = await runExistenceProbe(client, target()); + expect(r.status).toBe("error"); + expect(r.code).toBe("NOT_FOUND"); + }); + + it("errors INVALID_VALUE on a 400, quoting the value (no type/blob leak)", async () => { + const client = { + warehouses: { + get: async () => { + throw Object.assign( + new Error( + 'Response from server (Bad Request) {"error_code":"INVALID_PARAMETER_VALUE","message":"bogus is not a valid endpoint id."}', + ), + { statusCode: 400, errorCode: "INVALID_PARAMETER_VALUE" }, + ); + }, + }, + }; + const r = await runExistenceProbe( + client, + target({ fieldValues: { id: "wh-123" } }), + ); + expect(r.status).toBe("error"); + expect(r.code).toBe("INVALID_VALUE"); + expect(r.detail).toContain('"wh-123"'); + expect(r.detail).not.toContain("error_code"); + expect(r.detail).not.toContain("sql_warehouse"); + }); + + it("errors ACCESS_DENIED on a 403", async () => { + const client = { + warehouses: { + get: async () => { + throw apiError(403); + }, + }, + }; + const r = await runExistenceProbe(client, target()); + expect(r.status).toBe("error"); + expect(r.code).toBe("ACCESS_DENIED"); + }); + + it("skips when the id field is missing", async () => { + const client = { warehouses: { get: async () => ({}) } }; + const r = await runExistenceProbe(client, target({ fieldValues: {} })); + expect(r.status).toBe("skipped"); + expect(r.code).toBe("MISSING_FIELD"); + }); +}); + +describe("runExistenceProbe — job", () => { + it("errors on a non-integer job id", async () => { + const client = { jobs: { get: async () => ({}) } }; + const r = await runExistenceProbe( + client, + target({ type: "job", fieldValues: { id: "not-a-number" } }), + ); + expect(r.status).toBe("error"); + expect(r.code).toBe("INVALID_ID"); + }); + + it("ok for a valid integer job id", async () => { + const client = { jobs: { get: async () => ({}) } }; + const r = await runExistenceProbe( + client, + target({ type: "job", fieldValues: { id: "42" } }), + ); + expect(r.status).toBe("ok"); + }); +}); + +describe("runExistenceProbe — unsupported type", () => { + it("skips NOT_IMPLEMENTED for a type with no probe", async () => { + const r = await runExistenceProbe( + {}, + target({ type: "secret", fieldValues: {} }), + ); + expect(r.status).toBe("skipped"); + expect(r.code).toBe("NOT_IMPLEMENTED"); + }); +}); + +describe("runExistenceProbe — postgres (Lakebase)", () => { + afterEach(() => { + mockGetLakebasePool.mockReset(); + endSpy.mockClear(); + }); + + function pgTarget(overrides: Partial = {}): ResourceTarget { + return target({ + type: "postgres", + requiredPermission: "CAN_CONNECT_AND_CREATE", + fieldValues: { host: "ep.example.com", endpointPath: "projects/x" }, + ...overrides, + }); + } + + it("ok when SELECT 1 succeeds, and closes the pool", async () => { + const query = vi.fn(async () => ({ rows: [{ "?column?": 1 }] })); + mockGetLakebasePool.mockResolvedValue({ query, end: endSpy }); + + const r = await runExistenceProbe({}, pgTarget()); + expect(r.status).toBe("ok"); + expect(query).toHaveBeenCalledWith("SELECT 1"); + expect(endSpy).toHaveBeenCalled(); // pool always closed + }); + + it("errors CONNECTION_FAILED when the query throws, and still closes", async () => { + const query = vi.fn(async () => { + throw new Error("ECONNREFUSED 10.0.0.1:5432"); + }); + mockGetLakebasePool.mockResolvedValue({ query, end: endSpy }); + + const r = await runExistenceProbe({}, pgTarget()); + expect(r.status).toBe("error"); + expect(r.code).toBe("CONNECTION_FAILED"); + expect(r.detail).toContain("ECONNREFUSED"); + expect(endSpy).toHaveBeenCalled(); + }); + + it("adds an OAuth/PGUSER hint on 'password authentication failed'", async () => { + const query = vi.fn(async () => { + throw new Error( + 'password authentication failed for user "galymzhan.zhangazy%40databricks.com"', + ); + }); + mockGetLakebasePool.mockResolvedValue({ query, end: endSpy }); + + const r = await runExistenceProbe({}, pgTarget()); + expect(r.status).toBe("error"); + expect(r.code).toBe("CONNECTION_FAILED"); + expect(r.detail).toMatch(/password authentication failed/i); + expect(r.hint).toMatch(/OAuth token as the password/i); + expect(r.hint).toMatch(/PGUSER/); + }); + + it("does NOT add the auth hint for unrelated connection errors", async () => { + const query = vi.fn(async () => { + throw new Error("ECONNREFUSED 10.0.0.1:5432"); + }); + mockGetLakebasePool.mockResolvedValue({ query, end: endSpy }); + + const r = await runExistenceProbe({}, pgTarget()); + expect(r.hint).toBeUndefined(); + }); + + it("skips with MISSING_FIELD when neither host nor endpoint resolved", async () => { + const r = await runExistenceProbe({}, pgTarget({ fieldValues: {} })); + expect(r.status).toBe("skipped"); + expect(r.code).toBe("MISSING_FIELD"); + expect(mockGetLakebasePool).not.toHaveBeenCalled(); + }); +}); + +describe("runExistenceProbe — per-type coverage with real manifest keys", () => { + it("serving_endpoint: ok via `name`", async () => { + const client = { servingEndpoints: { get: async () => ({}) } }; + const r = await runExistenceProbe( + client, + target({ + type: "serving_endpoint", + fieldValues: { name: "my-endpoint" }, + }), + ); + expect(r.status).toBe("ok"); + }); + + 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 }); + }, + }, + }; + const r = await runExistenceProbe( + client, + target({ type: "serving_endpoint", fieldValues: { id: "abc-123" } }), + ); + expect(r.status).toBe("error"); + expect(r.hint).toMatch(/looked up by name/i); + }); + + 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 }); + }, + }, + }; + const r = await runExistenceProbe( + client, + target({ type: "serving_endpoint", fieldValues: { name: "my-ep" } }), + ); + expect(r.status).toBe("error"); + expect(r.hint).toBeUndefined(); + }); + + it("genie_space: ok via `id`", async () => { + const client = { genie: { getSpace: async () => ({}) } }; + const r = await runExistenceProbe( + client, + target({ type: "genie_space", fieldValues: { id: "01ef" } }), + ); + expect(r.status).toBe("ok"); + }); + + it("volume: ok via `path` (real manifest key)", async () => { + const client = { volumes: { read: async () => ({}) } }; + const r = await runExistenceProbe( + client, + target({ + type: "volume", + fieldValues: { path: "/Volumes/main/default/files" }, + }), + ); + expect(r.status).toBe("ok"); + }); + + it("uc_function: ok via `name`", async () => { + const client = { functions: { get: async () => ({}) } }; + const r = await runExistenceProbe( + client, + target({ type: "uc_function", fieldValues: { name: "cat.sch.fn" } }), + ); + expect(r.status).toBe("ok"); + }); + + it("vector_search_index: probes via camelCase `indexName`", async () => { + const getIndex = vi.fn(async () => ({})); + const client = { vectorSearchIndexes: { getIndex } }; + const r = await runExistenceProbe( + client, + target({ + type: "vector_search_index", + fieldValues: { indexName: "main.default.idx", endpointName: "ep" }, + }), + ); + expect(r.status).toBe("ok"); + expect(getIndex).toHaveBeenCalledWith({ index_name: "main.default.idx" }); + }); + + it("vector_search_index: skips MISSING_FIELD when index name absent", async () => { + const client = { vectorSearchIndexes: { getIndex: async () => ({}) } }; + const r = await runExistenceProbe( + client, + target({ type: "vector_search_index", fieldValues: {} }), + ); + expect(r.status).toBe("skipped"); + expect(r.code).toBe("MISSING_FIELD"); + }); +}); + +describe("toThreeLevelVolumeName", () => { + it("passes through a dotted 3-level name", () => { + expect(toThreeLevelVolumeName("main.default.files")).toBe( + "main.default.files", + ); + }); + + it("extracts from a /Volumes path", () => { + expect(toThreeLevelVolumeName("/Volumes/main/default/files/sub/x")).toBe( + "main.default.files", + ); + }); + + it("returns null for an unparseable value", () => { + expect(toThreeLevelVolumeName("just-a-name")).toBeNull(); + }); +}); diff --git a/packages/shared/src/cli/commands/doctor/checks-existence.ts b/packages/shared/src/cli/commands/doctor/checks-existence.ts new file mode 100644 index 000000000..49e6473fe --- /dev/null +++ b/packages/shared/src/cli/commands/doctor/checks-existence.ts @@ -0,0 +1,355 @@ +/** + * 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. + */ + +import { + AppkitNotInstalledError, + getLakebasePool, + type LakebasePoolHandle, +} from "./databricks-client"; +import type { LayerResult, ResourceTarget } from "./types"; +import { errorMessage } from "./utils"; + +/** A passing existence check. Shared, never mutated by callers. */ +const EXISTENCE_OK: LayerResult = { layer: "existence", status: "ok" }; + +interface DoctorWorkspaceClient { + warehouses: { + get: (r: { id: string }) => Promise<{ state?: string }>; + }; + servingEndpoints: { + get: (r: { name: string }) => Promise; + }; + genie: { + getSpace: (r: { space_id: 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; + }; +} + +type ExistenceProbe = ( + client: DoctorWorkspaceClient, + target: ResourceTarget, +) => Promise; + +// Read statusCode/errorCode off the SDK's ApiError structurally. +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; +} + +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; +} + +// The SDK message often embeds a JSON blob; pull the inner `message` out so +// doctor prints one clean line, not a dump. +function cleanMessage(err: unknown): string { + const raw = errorMessage(err); + const m = raw.match(/"message"\s*:\s*"([^"]+)"/); + return m ? m[1] : raw; +} + +/** The resource's configured identifier (id / name / path / host), for short + * messages. */ +function displayId(target: ResourceTarget): string | null { + return field(target, "id", "name", "path", "index_name", "indexName", "host"); +} + +function classifyError(err: unknown, target: ResourceTarget): LayerResult { + const status = statusCodeOf(err); + const errorCode = errorCodeOf(err); + const message = cleanMessage(err); + const id = displayId(target); + const quoted = id ? `"${id}"` : null; + + if (status === 404 || errorCode === "RESOURCE_DOES_NOT_EXIST") { + return { + layer: "existence", + status: "error", + code: "NOT_FOUND", + detail: quoted ? `${quoted} not found` : "not found", + }; + } + // A malformed id/name often comes back as 400 rather than 404. + if (status === 400 || errorCode === "INVALID_PARAMETER_VALUE") { + return { + layer: "existence", + status: "error", + code: "INVALID_VALUE", + detail: quoted + ? `${quoted} is not a valid id/name` + : `invalid id/name: ${message}`, + }; + } + if (status === 403 || errorCode === "PERMISSION_DENIED") { + return { + layer: "existence", + status: "error", + code: "ACCESS_DENIED", + detail: quoted + ? `no permission to read ${quoted}` + : "no permission to read it", + }; + } + return { + layer: "existence", + status: "error", + code: "PROBE_FAILED", + detail: quoted + ? `read failed for ${quoted}: ${message}` + : `read failed: ${message}`, + }; +} + +/** Normalizes a field key so camelCase (`indexName`) and snake_case + * (`index_name`) spellings match. */ +function normalizeKey(key: string): string { + return key.toLowerCase().replace(/[_-]/g, ""); +} + +/** Looks up a resolved field value by any of the accepted key spellings. */ +function field(target: ResourceTarget, ...names: string[]): string | null { + const wanted = new Set(names.map(normalizeKey)); + for (const [key, value] of Object.entries(target.fieldValues)) { + if (wanted.has(normalizeKey(key)) && value.length > 0) return value; + } + return null; +} + +function missingField(fieldName: string): LayerResult { + return { + layer: "existence", + status: "skipped", + code: "MISSING_FIELD", + detail: `no ${fieldName} configured — can't probe`, + }; +} + +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 state = wh.state; + if (state && state !== "RUNNING") { + return { + layer: "existence", + status: "warn", + code: "WAREHOUSE_NOT_RUNNING", + detail: `warehouse exists but is ${state} (will cold-start on first query)`, + }; + } + return EXISTENCE_OK; + } catch (err) { + return classifyError(err, target); + } +}; + +const probeServing: ExistenceProbe = async (client, target) => { + const name = field(target, "name"); + // The API only accepts a name; if configured by id, probe with it anyway and + // flag the likely cause on failure. + const idOnly = name === null ? field(target, "id") : null; + const value = name ?? idOnly; + if (!value) return missingField("name"); + try { + await client.servingEndpoints.get({ name: value }); + return EXISTENCE_OK; + } catch (err) { + const result = classifyError(err, target); + if (idOnly && result.status === "error") { + result.hint = + "Serving endpoints are looked up by name, but this resource is configured by id. Set DATABRICKS_SERVING_ENDPOINT_NAME to the endpoint's name."; + } + return result; + } +}; + +const probeGenie: ExistenceProbe = async (client, target) => { + const spaceId = field(target, "id"); + if (!spaceId) return missingField("id"); + try { + await client.genie.getSpace({ space_id: spaceId }); + return EXISTENCE_OK; + } catch (err) { + return classifyError(err, target); + } +}; + +const probeJob: ExistenceProbe = async (client, target) => { + const raw = field(target, "id"); + if (!raw) return missingField("id"); + const jobId = Number(raw); + if (!Number.isInteger(jobId)) { + return { + layer: "existence", + status: "error", + code: "INVALID_ID", + detail: `"${raw}" is not a valid job id (expected an integer)`, + }; + } + try { + await client.jobs.get({ job_id: jobId }); + return EXISTENCE_OK; + } catch (err) { + return classifyError(err, target); + } +}; + +const probeVolume: ExistenceProbe = async (client, target) => { + // The read API wants the 3-level name (catalog.schema.volume), but the env + // value is usually a /Volumes/... path. + const raw = field(target, "path", "name"); + if (!raw) return missingField("path"); + const name = toThreeLevelVolumeName(raw); + if (!name) { + return { + layer: "existence", + status: "error", + code: "INVALID_NAME", + detail: `"${raw}" is not a valid volume path (expected /Volumes/catalog/schema/volume or catalog.schema.volume)`, + }; + } + try { + await client.volumes.read({ name }); + return EXISTENCE_OK; + } catch (err) { + return classifyError(err, target); + } +}; + +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 }); + return EXISTENCE_OK; + } catch (err) { + return classifyError(err, target); + } +}; + +const probeFunction: ExistenceProbe = async (client, target) => { + const name = field(target, "name"); + if (!name) return missingField("name"); + try { + await client.functions.get({ name }); + return EXISTENCE_OK; + } catch (err) { + return classifyError(err, target); + } +}; + +function lakebaseAuthHint(message: string): string | undefined { + if (/password authentication failed/i.test(message)) { + return ( + "Lakebase uses an OAuth token as the password, so this usually means the" + + " PGUSER/role doesn't match your identity. Check PGUSER is your exact" + + " login." + ); + } + return undefined; +} + +// Lakebase has no cheap control-plane `.get()`, so existence is proven by a +// real connection + `SELECT 1`. +const probePostgres: ExistenceProbe = async (client, target) => { + if (!field(target, "endpointPath") && !field(target, "host")) { + return missingField("host/endpoint"); + } + + let pool: LakebasePoolHandle | null = null; + try { + pool = await getLakebasePool(client); + await pool.query("SELECT 1"); + return EXISTENCE_OK; + } catch (err) { + if (err instanceof AppkitNotInstalledError) { + return { + layer: "existence", + status: "skipped", + code: "APPKIT_NOT_INSTALLED", + detail: err.message, + }; + } + const message = cleanMessage(err); + return { + layer: "existence", + status: "error", + code: "CONNECTION_FAILED", + detail: `could not connect to Lakebase Postgres: ${message}`, + hint: lakebaseAuthHint(message), + }; + } finally { + if (pool) { + try { + await pool.end(); + } catch { + // best-effort close + } + } + } +}; + +/** + * Normalizes a volume reference to the SDK's 3-level `catalog.schema.volume` + * name, accepting either an already-dotted name or a `/Volumes/c/s/v/...` path. + */ +export function toThreeLevelVolumeName(raw: string): string | null { + const trimmed = raw.trim(); + if (/^[^./\s]+\.[^./\s]+\.[^./\s]+$/.test(trimmed)) return trimmed; + + const m = trimmed.match(/^\/Volumes\/([^/]+)\/([^/]+)\/([^/]+)/); + if (m) return `${m[1]}.${m[2]}.${m[3]}`; + + return null; +} + +// Unlisted types fall through to NOT_IMPLEMENTED in runExistenceProbe. +const PROBES: Record = { + sql_warehouse: probeWarehouse, + serving_endpoint: probeServing, + genie_space: probeGenie, + job: probeJob, + volume: probeVolume, + vector_search_index: probeVectorIndex, + uc_function: probeFunction, + postgres: probePostgres, +}; + +export async function runExistenceProbe( + client: unknown, + target: ResourceTarget, +): Promise { + const probe = PROBES[target.type]; + if (!probe) { + return { + layer: "existence", + status: "skipped", + code: "NOT_IMPLEMENTED", + detail: `existence check not implemented for ${target.type}`, + }; + } + return probe(client as DoctorWorkspaceClient, target); +} diff --git a/packages/shared/src/cli/commands/doctor/checks-wiring.test.ts b/packages/shared/src/cli/commands/doctor/checks-wiring.test.ts new file mode 100644 index 000000000..49973158e --- /dev/null +++ b/packages/shared/src/cli/commands/doctor/checks-wiring.test.ts @@ -0,0 +1,134 @@ +import { describe, expect, it } from "vitest"; +import type { BundleInfo } from "./bundle"; +import { checkWiring } from "./checks-wiring"; +import type { ResourceTarget } from "./types"; + +function info(overrides: Partial = {}): BundleInfo { + return { + bindings: new Map(), + envToBinding: new Map(), + declaredResources: new Set(), + present: true, + ...overrides, + }; +} + +function target(overrides: Partial = {}): ResourceTarget { + return { + type: "sql_warehouse", + resourceKey: "sql-warehouse", + alias: "SQL Warehouse", + plugin: "analytics", + requiredPermission: "CAN_USE", + required: true, + envVars: [], + fieldValues: {}, + ...overrides, + }; +} + +describe("checkWiring", () => { + it("returns nothing when no bundle is present", () => { + expect(checkWiring(info({ present: false }), [])).toEqual([]); + }); + + it("flags an app.yaml valueFrom that matches no binding", () => { + const findings = checkWiring( + info({ envToBinding: new Map([["DATABRICKS_WAREHOUSE_ID", "sql-wh"]]) }), + [], + ); + expect(findings).toHaveLength(1); + expect(findings[0].code).toBe("VALUEFROM_UNBOUND"); + expect(findings[0].status).toBe("error"); + }); + + it("flags a bundle-managed ref to an undeclared resource", () => { + const findings = checkWiring( + info({ + bindings: new Map([ + [ + "job", + { + name: "job", + type: "job", + origin: "bundle-managed", + ref: { type: "jobs", key: "ghost" }, + }, + ], + ]), + declaredResources: new Set(), // jobs.ghost NOT declared + }), + [], + ); + expect(findings.some((f) => f.code === "BUNDLE_REF_MISSING")).toBe(true); + }); + + it("passes a bundle-managed ref that resolves to a declared resource", () => { + const findings = checkWiring( + info({ + bindings: new Map([ + [ + "job", + { + name: "job", + type: "job", + origin: "bundle-managed", + ref: { type: "jobs", key: "report" }, + }, + ], + ]), + declaredResources: new Set(["jobs.report"]), + }), + [], + ); + expect(findings.some((f) => f.code === "BUNDLE_REF_MISSING")).toBe(false); + }); + + it("warns for an OPTIONAL plugin's env var that app.yaml doesn't provide", () => { + const findings = checkWiring( + info({ + // app.yaml provides one env, bindings exist for it. + envToBinding: new Map([["DATABRICKS_WAREHOUSE_ID", "sql-warehouse"]]), + bindings: new Map([ + [ + "sql-warehouse", + { + name: "sql-warehouse", + type: "sql_warehouse", + origin: "external", + }, + ], + ]), + }), + [ + target({ + required: false, + envVars: ["DATABRICKS_WAREHOUSE_ID", "DATABRICKS_MISSING"], + }), + ], + ); + const unwired = findings.find((f) => f.code === "ENV_UNWIRED"); + expect(unwired?.status).toBe("warn"); + expect(unwired?.label).toBe("DATABRICKS_MISSING"); + }); + + it("errors for a REQUIRED plugin's unwired env var, gating the exit code", () => { + // The "works locally, breaks on deploy" case: a required env is set via + // local .env but has no app.yaml entry, so it's unset in the deployed + // container — a guaranteed break, so it must be an error, not a warning. + const findings = checkWiring( + info({ present: true }), // empty bindings + empty envToBinding + [ + target({ + required: true, + alias: "SQL Warehouse", + envVars: ["DATABRICKS_WAREHOUSE_ID"], + }), + ], + ); + const unwired = findings.find((f) => f.code === "ENV_UNWIRED"); + expect(unwired?.status).toBe("error"); + expect(unwired?.label).toBe("DATABRICKS_WAREHOUSE_ID"); + expect(unwired?.detail).toMatch(/unset in the deployed app/i); + }); +}); diff --git a/packages/shared/src/cli/commands/doctor/checks-wiring.ts b/packages/shared/src/cli/commands/doctor/checks-wiring.ts new file mode 100644 index 000000000..7e6fd87d7 --- /dev/null +++ b/packages/shared/src/cli/commands/doctor/checks-wiring.ts @@ -0,0 +1,76 @@ +/** + * Layer: wiring (offline, deploy-declaration). Validates the three-file join + * that `databricks bundle validate` can't see, because it spans `app.yaml` and + * the AppKit plugin layer: + * + * - every `app.yaml` `valueFrom: ` must match a `databricks.yml` binding + * `name` — otherwise the platform has nothing to inject and the env var the + * plugin reads stays empty at runtime; + * - every bundle-managed `${resources...*}` binding must reference a + * resource actually declared in the bundle — otherwise deploy fails. + * + * Runs regardless of auth; these are declaration problems, not connectivity. + */ + +import type { BundleInfo } from "./bundle"; +import type { ResourceTarget, WiringFinding } from "./types"; + +export function checkWiring( + info: BundleInfo, + targets: ResourceTarget[], +): WiringFinding[] { + if (!info.present) return []; + + const findings: WiringFinding[] = []; + + // 1. Each app.yaml valueFrom must resolve to a real binding. + for (const [envVar, bindingName] of info.envToBinding) { + if (!info.bindings.has(bindingName)) { + findings.push({ + status: "error", + code: "VALUEFROM_UNBOUND", + label: envVar, + detail: `app.yaml binds it to "${bindingName}", which databricks.yml doesn't declare`, + hint: `Add a resource named "${bindingName}" under resources.apps..resources, or fix the valueFrom to match an existing binding.`, + }); + } + } + + // 2. Each bundle-managed binding must reference a declared bundle resource. + for (const binding of info.bindings.values()) { + if (binding.origin !== "bundle-managed" || !binding.ref) continue; + const ref = `${binding.ref.type}.${binding.ref.key}`; + if (!info.declaredResources.has(ref)) { + findings.push({ + status: "error", + code: "BUNDLE_REF_MISSING", + label: binding.name, + detail: `references \${resources.${ref}.*}, but no resources.${binding.ref.type}.${binding.ref.key} is declared in the bundle`, + hint: `Declare resources.${binding.ref.type}.${binding.ref.key} in databricks.yml, or point the binding at an existing resource.`, + }); + } + } + + // 3. A used plugin's env var with no app.yaml entry: set locally via .env, but + // .env isn't uploaded, so it's unset in the deployed container — the headline + // "works locally, breaks on deploy" case. For a *required* resource this + // guarantees a broken deploy, so it's an error that gates the exit code; for + // an optional one it's a warning. + for (const target of targets) { + for (const envVar of target.envVars) { + if (!info.envToBinding.has(envVar)) { + findings.push({ + status: target.required ? "error" : "warn", + code: "ENV_UNWIRED", + label: envVar, + detail: target.required + ? "has no app.yaml entry — it will be unset in the deployed app" + : "has no app.yaml entry — the optional resource will be unset in the deployed app", + hint: `Add to app.yaml: \`{ name: ${envVar}, valueFrom: }\`, and declare that binding in databricks.yml.`, + }); + } + } + } + + return findings; +} diff --git a/packages/shared/src/cli/commands/doctor/checks.test.ts b/packages/shared/src/cli/commands/doctor/checks.test.ts new file mode 100644 index 000000000..040579a17 --- /dev/null +++ b/packages/shared/src/cli/commands/doctor/checks.test.ts @@ -0,0 +1,289 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { checkAuth, checkConfig, sanitizeHost, validateHost } from "./checks"; +import type { ResourceTarget } from "./types"; + +const { mockGetServiceClient } = vi.hoisted(() => ({ + mockGetServiceClient: vi.fn(), +})); +vi.mock("./databricks-client", () => ({ + SdkNotInstalledError: class SdkNotInstalledError extends Error {}, + getServiceClient: mockGetServiceClient, +})); + +function target(overrides: Partial = {}): ResourceTarget { + return { + type: "sql_warehouse", + resourceKey: "sql-warehouse", + alias: "SQL Warehouse", + plugin: "analytics", + requiredPermission: "CAN_USE", + required: true, + envVars: ["DOCTOR_TEST_ENV"], + fieldValues: {}, + ...overrides, + }; +} + +describe("checkConfig", () => { + const saved = { ...process.env }; + + beforeEach(() => { + delete process.env.DOCTOR_TEST_ENV; + delete process.env.DOCTOR_TEST_ENV_2; + }); + + afterEach(() => { + process.env = { ...saved }; + }); + + it("errors when a required resource's env var is unset", async () => { + const result = await checkConfig(target()); + expect(result.status).toBe("error"); + expect(result.code).toBe("ENV_MISSING"); + }); + + it("warns (not errors) when an optional resource's env var is unset", async () => { + const result = await checkConfig(target({ required: false })); + expect(result.status).toBe("warn"); + expect(result.code).toBe("ENV_MISSING_OPTIONAL"); + }); + + it("passes an unfilled-looking value (existence check is the authority)", async () => { + process.env.DOCTOR_TEST_ENV = "your_sql_warehouse_id"; + const result = await checkConfig(target()); + expect(result.status).toBe("ok"); + }); + + it("treats an empty string as missing", async () => { + process.env.DOCTOR_TEST_ENV = " "; + const result = await checkConfig(target()); + expect(result.status).toBe("error"); + expect(result.code).toBe("ENV_MISSING"); + }); + + it("reports all missing env vars when several are unset", async () => { + const result = await checkConfig( + target({ envVars: ["DOCTOR_TEST_ENV", "DOCTOR_TEST_ENV_2"] }), + ); + expect(result.status).toBe("error"); + expect(result.detail).toContain("DOCTOR_TEST_ENV"); + expect(result.detail).toContain("DOCTOR_TEST_ENV_2"); + }); +}); + +describe("validateHost", () => { + it("accepts an unset host (SDK auth chain takes over)", () => { + expect(validateHost(undefined)).toBeNull(); + expect(validateHost("")).toBeNull(); + }); + + it("accepts a real workspace URL", () => { + expect(validateHost("https://foo.cloud.databricks.com")).toBeNull(); + expect(validateHost("https://adb-123.4.azuredatabricks.net")).toBeNull(); + }); + + it("rejects the unfilled template placeholder https://...", () => { + expect(validateHost("https://...")).toMatch(/placeholder/i); + }); + + it("rejects a non-URL value", () => { + expect(validateHost("not-a-url")).toMatch(/not a valid URL/i); + }); + + it("rejects a non-http(s) scheme", () => { + expect(validateHost("ftp://foo.databricks.com")).toMatch(/http/i); + }); +}); + +describe("sanitizeHost", () => { + it("passes through an unset or credential-free host unchanged", () => { + expect(sanitizeHost(undefined)).toBeUndefined(); + expect(sanitizeHost("https://foo.cloud.databricks.com")).toBe( + "https://foo.cloud.databricks.com", + ); + }); + + it("strips embedded userinfo (user:pass@) so it can't leak", () => { + const cleaned = sanitizeHost( + "https://user:secret@workspace.databricks.com", + ); + expect(cleaned).not.toContain("secret"); + expect(cleaned).not.toContain("user"); + expect(cleaned).toContain("workspace.databricks.com"); + }); + + it("leaves a non-URL value as-is (nothing to strip)", () => { + expect(sanitizeHost("not-a-url")).toBe("not-a-url"); + }); +}); + +describe("checkAuth", () => { + const savedHost = process.env.DATABRICKS_HOST; + + afterEach(() => { + mockGetServiceClient.mockReset(); + if (savedHost === undefined) delete process.env.DATABRICKS_HOST; + else process.env.DATABRICKS_HOST = savedHost; + }); + + it("ok + returns the client when currentUser.me() succeeds", async () => { + process.env.DATABRICKS_HOST = "https://foo.cloud.databricks.com"; + const client = { currentUser: { me: async () => ({ userName: "sp-1" }) } }; + mockGetServiceClient.mockResolvedValue({ client }); + + const { result, client: returned } = await checkAuth({}); + expect(result.status).toBe("ok"); + expect(result.code).toBe("AUTH_OK"); + expect(result.detail).toContain("sp-1"); + expect(returned).toBe(client); // handed to the live layers + }); + + it("stores the sanitized host (no embedded credentials) in the result", async () => { + process.env.DATABRICKS_HOST = + "https://user:secret@foo.cloud.databricks.com"; + const client = { currentUser: { me: async () => ({ userName: "sp-1" }) } }; + mockGetServiceClient.mockResolvedValue({ client }); + + const { result } = await checkAuth({}); + expect(result.host).not.toContain("secret"); + expect(result.host).toContain("foo.cloud.databricks.com"); + }); + + it("errors HOST_INVALID before touching the SDK", async () => { + process.env.DATABRICKS_HOST = "https://..."; + const { result, client } = await checkAuth({}); + expect(result.status).toBe("error"); + expect(result.code).toBe("HOST_INVALID"); + expect(client).toBeUndefined(); + expect(mockGetServiceClient).not.toHaveBeenCalled(); + }); + + it("errors SDK_NOT_INSTALLED when the bridge reports the SDK is absent", async () => { + process.env.DATABRICKS_HOST = "https://foo.cloud.databricks.com"; + const { SdkNotInstalledError } = await import("./databricks-client"); + mockGetServiceClient.mockRejectedValue(new SdkNotInstalledError()); + + const { result } = await checkAuth({}); + expect(result.status).toBe("error"); + expect(result.code).toBe("SDK_NOT_INSTALLED"); + }); + + it("errors AUTH_FAILED on any other auth error", async () => { + process.env.DATABRICKS_HOST = "https://foo.cloud.databricks.com"; + mockGetServiceClient.mockRejectedValue(new Error("something odd")); + + const { result } = await checkAuth({}); + expect(result.status).toBe("error"); + expect(result.code).toBe("AUTH_FAILED"); + // detail stays a short headline; the raw message is carried separately. + expect(result.detail).toBe("authentication failed"); + expect(result.raw).toContain("something odd"); + expect(result.hint).toBeUndefined(); // unrecognized failure → no guess + }); + + it("fails (not hangs) when currentUser.me() never responds", async () => { + vi.useFakeTimers(); + process.env.DATABRICKS_HOST = "https://foo.cloud.databricks.com"; + // Client resolves, but the live me() call never settles. + mockGetServiceClient.mockResolvedValue({ + client: { currentUser: { me: () => new Promise(() => {}) } }, + }); + + const authPromise = checkAuth({}); + await vi.advanceTimersByTimeAsync(10_000); + const { result } = await authPromise; + expect(result.status).toBe("error"); + expect(result.code).toBe("AUTH_FAILED"); + expect(result.raw).toMatch(/timed out/i); + vi.useRealTimers(); + }); + + it("hints to reauthenticate on an expired/failed CLI token (real SDK message)", async () => { + process.env.DATABRICKS_HOST = "https://foo.cloud.databricks.com"; + mockGetServiceClient.mockRejectedValue( + new Error( + "default auth: databricks-cli: cannot get access token: Command failed: databricks auth token", + ), + ); + + const { result } = await checkAuth({ profile: "prod" }); + expect(result.hint).toContain("databricks auth login --profile prod"); + expect(result.hint).toMatch(/confirm the profile\/host/i); + }); + + it("hints about a missing profile (real SDK message)", async () => { + process.env.DATABRICKS_HOST = "https://foo.cloud.databricks.com"; + mockGetServiceClient.mockRejectedValue( + new Error("resolve: ~/.databrickscfg has no nope profile configured"), + ); + + const { result } = await checkAuth({}); + expect(result.hint).toContain("databricks auth login"); + expect(result.hint).toMatch(/--profile/i); + }); + + it("reports the profile from DATABRICKS_CONFIG_PROFILE when --profile is unset", async () => { + process.env.DATABRICKS_HOST = "https://foo.cloud.databricks.com"; + process.env.DATABRICKS_CONFIG_PROFILE = "from-env"; + const client = { currentUser: { me: async () => ({ userName: "u" }) } }; + mockGetServiceClient.mockResolvedValue({ client }); + try { + const { result } = await checkAuth({}); + expect(result.profile).toBe("from-env"); + } finally { + delete process.env.DATABRICKS_CONFIG_PROFILE; + } + }); + + it("mines the profile the SDK used when none was passed, so the login hint targets it", async () => { + process.env.DATABRICKS_HOST = "https://foo.cloud.databricks.com"; + delete process.env.DATABRICKS_CONFIG_PROFILE; + mockGetServiceClient.mockRejectedValue( + new Error( + "default auth: databricks-cli: cannot get access token: the refresh token is invalid. To reauthenticate, run\n $ databricks auth login --profile DEFAULT", + ), + ); + + const { result } = await checkAuth({}); + expect(result.hint).toContain("databricks auth login --profile DEFAULT"); + // Not the bare command, which would reauth the wrong profile. + expect(result.hint).not.toContain("`databricks auth login`"); + }); + + it("hints that the workspace is unreachable on a network error", async () => { + process.env.DATABRICKS_HOST = "https://wrong.cloud.databricks.com"; + mockGetServiceClient.mockRejectedValue( + new Error("getaddrinfo ENOTFOUND wrong.cloud.databricks.com"), + ); + + const { result } = await checkAuth({}); + expect(result.hint).toMatch(/host is correct and reachable/i); + expect(result.hint).toMatch(/DATABRICKS_HOST/); + }); + + it("hints to log in and confirm the target when creds fail with nothing set", async () => { + delete process.env.DATABRICKS_HOST; + delete process.env.DATABRICKS_CONFIG_PROFILE; + mockGetServiceClient.mockRejectedValue( + new Error("default auth: cannot configure default credentials"), + ); + + const { result } = await checkAuth({}); + expect(result.hint).toContain("databricks auth login"); + expect(result.hint).toMatch( + /confirm the profile\/host is the one you intend/i, + ); + }); + + it("keeps detail short and carries the full message in raw for --detail/--json", async () => { + process.env.DATABRICKS_HOST = "https://foo.cloud.databricks.com"; + const sprawling = `first line of the failure\n${"x".repeat(300)}`; + mockGetServiceClient.mockRejectedValue(new Error(sprawling)); + + const { result } = await checkAuth({}); + expect(result.detail).toBe("authentication failed"); + expect(result.raw).toBe(sprawling); + expect(result.raw).toContain("first line of the failure"); + expect(result.raw).toContain("x".repeat(300)); + }); +}); diff --git a/packages/shared/src/cli/commands/doctor/checks.ts b/packages/shared/src/cli/commands/doctor/checks.ts new file mode 100644 index 000000000..18c6e1a53 --- /dev/null +++ b/packages/shared/src/cli/commands/doctor/checks.ts @@ -0,0 +1,223 @@ +/** + * The `auth` and `config` layer checks. (The `existence` layer's per-type + * probes live in `checks-existence.ts`.) + */ + +import { getServiceClient, SdkNotInstalledError } from "./databricks-client"; +import type { + AuthCheckResult, + DoctorOptions, + LayerResult, + ResourceTarget, +} from "./types"; +import { errorMessage, withTimeout } from "./utils"; + +/** The auth result plus the resolved client (present only on success). */ +export interface AuthOutcome { + result: AuthCheckResult; + client?: unknown; +} + +interface CurrentUserClient { + currentUser: { + me: () => Promise<{ id?: string; userName?: string }>; + }; +} + +/** + * Strips URL userinfo (`user:pass@`) from a host, so credentials someone + * embedded in `DATABRICKS_HOST` never reach the report or `--json`. Returns the + * input unchanged when it has no userinfo or isn't a parseable URL. + */ +export function sanitizeHost(host: string | undefined): string | undefined { + if (host === undefined) return undefined; + try { + const url = new URL(host); + if (!url.username && !url.password) return host; + url.username = ""; + url.password = ""; + return url.toString(); + } catch { + return host; + } +} + +/** + * Validates `DATABRICKS_HOST` before the SDK sees it, so an unfilled placeholder + * gets a clear message instead of the SDK's opaque credentials error. Returns an + * error message, or null when the host is acceptable or unset. + */ +export function validateHost(host: string | undefined): string | null { + if (host === undefined || host.trim().length === 0) return null; + + let url: URL; + try { + url = new URL(host); + } catch { + return `DATABRICKS_HOST is not a valid URL: "${host}"`; + } + + if (url.protocol !== "https:" && url.protocol !== "http:") { + return `DATABRICKS_HOST must be an http(s) URL: "${host}"`; + } + + // Placeholders like "https://..." parse but have no real dotted label. + const hostname = url.hostname; + const hasRealLabel = /[a-z0-9]/i.test(hostname) && hostname.includes("."); + if (!hasRealLabel || /^[.\-_]+$/.test(hostname)) { + return `DATABRICKS_HOST looks like an unfilled placeholder: "${host}"`; + } + + return null; +} + +/** + * Layer: auth. Runs once, app-wide; a failure short-circuits every resource's + * existence check (they all need the client). + */ +export async function checkAuth(options: DoctorOptions): Promise { + // Validate the raw value, but only ever store/report the sanitized one so + // credentials embedded in the URL (user:pass@) never reach the report/--json. + const rawHost = process.env.DATABRICKS_HOST; + const host = sanitizeHost(rawHost); + // The identity source shown in the report: explicit --profile, else the env + // var the SDK would pick up. + const profile = options.profile ?? process.env.DATABRICKS_CONFIG_PROFILE; + const hostError = validateHost(rawHost); + if (hostError) { + return { + result: { + status: "error", + code: "HOST_INVALID", + detail: hostError, + host, + profile, + }, + }; + } + + try { + const { client } = await getServiceClient(options.profile); + // 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(), + ); + const who = me.userName ?? me.id ?? "unknown"; + + return { + client, + result: { + status: "ok", + code: "AUTH_OK", + detail: `authenticated as ${who}`, + host, + profile, + }, + }; + } catch (err) { + if (err instanceof SdkNotInstalledError) { + return { + result: { + status: "error", + code: "SDK_NOT_INSTALLED", + detail: err.message, + profile, + }, + }; + } + const raw = errorMessage(err); + // When no profile was given up front, recover the one the SDK resolved (it's + // embedded in the error) and mark it "(resolved)". + const sdkProfile = raw.match(/--profile\s+(\S+)/)?.[1]; + const usedProfile = profile ?? sdkProfile; + const shownProfile = + profile ?? (sdkProfile ? `${sdkProfile} (resolved)` : undefined); + return { + result: { + status: "error", + code: "AUTH_FAILED", + detail: "authentication failed", + hint: authFailureHint(raw, usedProfile), + host, + profile: shownProfile, + raw, + }, + }; + } +} + +/** + * Suggests a next action for common auth failures, matched most specific first. + * Returns undefined for an unrecognized failure. + * + * `profile` is the explicit --profile, else the one the SDK resolved (DEFAULT / + * DATABRICKS_CONFIG_PROFILE) and embedded in its error, so the login hint + * targets what actually failed. + */ +function authFailureHint( + message: string, + profile?: string, +): string | undefined { + const loginCmd = profile + ? `databricks auth login --profile ${profile}` + : "databricks auth login"; + // Network-level failure (unreachable host, DNS, TLS). Matched first — more + // specific than a credential problem and needs a different fix. + if ( + /ENOTFOUND|EAI_AGAIN|ECONNREFUSED|ECONNRESET|ETIMEDOUT|getaddrinfo|certificate|self.signed|unable to verify|TLS/i.test( + message, + ) + ) { + 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( + message, + ) + ) { + return `Run \`${loginCmd}\`, or pass an existing profile via --profile.`; + } + // 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( + message, + ) + ) { + return `Run \`${loginCmd}\` and confirm the profile/host is the one you intend.`; + } + return undefined; +} + +/** + * Layer: config. Offline presence check of each declared env var; whether a set + * value points at a real resource is the existence layer's job. + */ +export async function checkConfig( + target: ResourceTarget, +): Promise { + const missing: string[] = []; + + for (const envVar of target.envVars) { + const value = process.env[envVar]; + if (value === undefined || value.trim().length === 0) { + missing.push(envVar); + } + } + + if (missing.length > 0) { + const plural = missing.length > 1; + const names = missing.join(", "); + return { + layer: "config", + status: target.required ? "error" : "warn", + code: target.required ? "ENV_MISSING" : "ENV_MISSING_OPTIONAL", + detail: target.required + ? `${names} ${plural ? "are" : "is"} not set` + : `${names} ${plural ? "are" : "is"} not set (optional)`, + hint: `Set ${plural ? "them" : "it"} in your .env (local) and wire ${plural ? "them" : "it"} through app.yaml + databricks.yml for deploy.`, + }; + } + + return { layer: "config", status: "ok" }; +} diff --git a/packages/shared/src/cli/commands/doctor/databricks-client.ts b/packages/shared/src/cli/commands/doctor/databricks-client.ts new file mode 100644 index 000000000..fdf5457b0 --- /dev/null +++ b/packages/shared/src/cli/commands/doctor/databricks-client.ts @@ -0,0 +1,97 @@ +/** + * 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. + */ + +/** Raised when `@databricks/sdk-experimental` is not resolvable at runtime. */ +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.", + ); + this.name = "SdkNotInstalledError"; + } +} + +export interface ServiceClientHandle { + /** WorkspaceClient, typed as unknown to keep `shared` SDK-free. */ + client: unknown; +} + +function isModuleNotFound(err: unknown): boolean { + return ( + err instanceof Error && + (err.message.includes("Cannot find module") || + err.message.includes("Cannot find package") || + (err as NodeJS.ErrnoException).code === "ERR_MODULE_NOT_FOUND") + ); +} + +/** Constructs a `WorkspaceClient` 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 { + 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 } : {}); + return { client }; +} + +/** Raised when `@databricks/appkit` (needed for the Lakebase probe) is absent. */ +export class AppkitNotInstalledError extends Error { + constructor() { + super( + "The Lakebase connection check requires @databricks/appkit. Please install it to run this check.", + ); + this.name = "AppkitNotInstalledError"; + } +} + +/** Minimal `pg.Pool`-shaped handle. The caller owns it and must call `end()`. */ +export interface LakebasePoolHandle { + query: (sql: string) => Promise; + end: () => Promise; +} + +/** Builds a Lakebase pool via `createLakebasePool`. Connection settings + * (`PGHOST`, `LAKEBASE_ENDPOINT`, …) come from the environment; we inject only + * the resolved workspace client for OAuth token minting. */ +export async function getLakebasePool( + client: unknown, +): Promise { + // A variable specifier stops TS from statically resolving `@databricks/appkit`, + // an optional peer that `shared` has no dependency on. + const appkitPkg = "@databricks/appkit"; + let appkit: { + createLakebasePool: (cfg: Record) => unknown; + }; + try { + appkit = await import(appkitPkg); + } catch (err) { + if (isModuleNotFound(err)) { + throw new AppkitNotInstalledError(); + } + throw err; + } + + // Silence Lakebase's own logger. It defaults to an error-only console logger + // 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. + const pool = appkit.createLakebasePool({ + workspaceClient: client, + logger: { error: false }, + }); + return pool as unknown as LakebasePoolHandle; +} diff --git a/packages/shared/src/cli/commands/doctor/index.test.ts b/packages/shared/src/cli/commands/doctor/index.test.ts new file mode 100644 index 000000000..d23027a7e --- /dev/null +++ b/packages/shared/src/cli/commands/doctor/index.test.ts @@ -0,0 +1,40 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { loadEnvFile } from "./index"; + +function makeTempDir(): string { + return fs.mkdtempSync(path.join(os.tmpdir(), "doctor-envfile-")); +} + +describe("loadEnvFile", () => { + const dirs: string[] = []; + const savedKeys = ["DOCTOR_ENVFILE_B"]; + + afterEach(() => { + for (const d of dirs) fs.rmSync(d, { recursive: true, force: true }); + dirs.length = 0; + for (const k of savedKeys) delete process.env[k]; + }); + + function writeEnv(contents: string): string { + const dir = makeTempDir(); + dirs.push(dir); + const p = path.join(dir, ".env.local"); + fs.writeFileSync(p, contents); + return p; + } + + it("overrides an already-set value (so it beats the auto-loaded .env)", () => { + process.env.DOCTOR_ENVFILE_B = "from-default-env"; + loadEnvFile(writeEnv("DOCTOR_ENVFILE_B=from-explicit-file\n")); + expect(process.env.DOCTOR_ENVFILE_B).toBe("from-explicit-file"); + }); + + it("throws a clear error when the file does not exist", () => { + expect(() => loadEnvFile("/no/such/.env.local")).toThrow( + /env file not found/, + ); + }); +}); diff --git a/packages/shared/src/cli/commands/doctor/index.ts b/packages/shared/src/cli/commands/doctor/index.ts new file mode 100644 index 000000000..4d95f49b4 --- /dev/null +++ b/packages/shared/src/cli/commands/doctor/index.ts @@ -0,0 +1,63 @@ +import fs from "node:fs"; +import process from "node:process"; +import { Command } from "commander"; +import dotenv from "dotenv"; +import { exitCodeFor, printReport, printReportJson } from "./report"; +import { runDoctor } from "./run"; +import type { DoctorOptions } from "./types"; + +/** + * Loads an explicit env file into `process.env`, overriding the auto-loaded + * `.env`. Throws if the file is missing. + */ +export function loadEnvFile(envFile: string): void { + if (!fs.existsSync(envFile)) { + throw new Error(`env file not found: ${envFile}`); + } + dotenv.config({ path: envFile, override: true }); +} + +async function runDoctorCommand(options: DoctorOptions): Promise { + if (options.envFile) loadEnvFile(options.envFile); + + const report = await runDoctor(options); + + if (options.json) { + printReportJson(report, options.detail); + } else { + printReport(report, options.detail); + } + + process.exit(exitCodeFor(report)); +} + +export const doctorCommand = new Command("doctor") + .description( + "Diagnose an AppKit app's Databricks resources: authentication, config, and resource existence/reachability", + ) + .option("-p, --profile ", "Databricks CLI profile to authenticate with") + .option( + "-e, --env-file ", + "Load this env file before checking (overrides the auto-loaded .env), e.g. .env.local", + ) + .option( + "-d, --detail", + "Show full underlying error messages (raw SDK output) for failures", + ) + .option("--json", "Output the diagnostic report as JSON") + .addHelpText( + "after", + ` +Examples: + $ appkit doctor + $ appkit doctor --profile my-profile + $ appkit doctor --env-file .env.local + $ appkit doctor --detail + $ appkit doctor --json`, + ) + .action((opts: DoctorOptions) => + runDoctorCommand(opts).catch((err) => { + console.error(err); + process.exit(1); + }), + ); diff --git a/packages/shared/src/cli/commands/doctor/report.test.ts b/packages/shared/src/cli/commands/doctor/report.test.ts new file mode 100644 index 000000000..b8909a2db --- /dev/null +++ b/packages/shared/src/cli/commands/doctor/report.test.ts @@ -0,0 +1,220 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { exitCodeFor, printReport, printReportJson } from "./report"; +import type { DoctorReport, ResourceCheckResult } from "./types"; + +function report(overrides: Partial = {}): DoctorReport { + return { + auth: { status: "ok" }, + resources: [], + wiring: [], + summary: { ok: 0, warn: 0, error: 0, skipped: 0 }, + exitCode: 0, + ...overrides, + }; +} + +// biome-ignore lint/suspicious/noControlCharactersInRegex: matching ANSI escapes +const ANSI = /\x1b\[[0-9;]*m/g; + +/** Runs `fn` with console.log captured; returns the printed lines with any ANSI + * colour codes stripped, so assertions are colour-agnostic (picocolors emits + * codes when CI forces colour / a TTY is present, and none otherwise). */ +function capture(fn: () => void): string[] { + const lines: string[] = []; + const spy = vi.spyOn(console, "log").mockImplementation((msg?: unknown) => { + lines.push(String(msg).replace(ANSI, "")); + }); + try { + fn(); + return lines; + } finally { + spy.mockRestore(); + } +} + +function res( + status: ResourceCheckResult["status"], + type: string, +): ResourceCheckResult { + return { + target: { + type, + resourceKey: type, + alias: type, + plugin: "p", + requiredPermission: "X", + required: true, + envVars: [], + fieldValues: {}, + }, + status, + layers: [], + }; +} + +describe("printReport ordering", () => { + afterEach(() => vi.restoreAllMocks()); + + it("prints resources most-severe first (error, warn, skipped, ok)", () => { + const lines: string[] = []; + vi.spyOn(console, "log").mockImplementation((msg?: unknown) => { + lines.push(String(msg)); + }); + + // Deliberately supplied in non-severity order. + const report: DoctorReport = { + auth: { status: "ok" }, + resources: [ + res("warn", "warned"), + res("ok", "okay"), + res("error", "errored"), + res("skipped", "skip"), + ], + wiring: [], + summary: { ok: 1, warn: 1, error: 1, skipped: 1 }, + exitCode: 1, + }; + + printReport(report); + + const order = ["errored", "warned", "skip", "okay"].map((t) => + lines.findIndex((l) => l.includes(t)), + ); + // Every row was printed and appears in strictly increasing (severity) order. + expect(order.every((i) => i >= 0)).toBe(true); + expect(order).toEqual([...order].sort((a, b) => a - b)); + }); + + it("hides raw error by default, shows it with detail=true", () => { + const withRaw = report({ + auth: { + status: "error", + detail: "authentication failed", + hint: "Run `databricks auth login`.", + raw: "default auth: databricks-cli: cannot get access token: ", + }, + summary: { ok: 0, warn: 0, error: 0, skipped: 0 }, + }); + + // Default: headline + hint, no raw blob, plus a nudge toward --detail. + const plain = capture(() => printReport(withRaw)); + expect(plain.some((l) => l.includes("authentication failed"))).toBe(true); + expect(plain.some((l) => l.includes("Hint:"))).toBe(true); + expect(plain.some((l) => l.includes(""))).toBe(false); + expect(plain.some((l) => /Run with --detail/.test(l))).toBe(true); + + // --detail: raw blob is surfaced under a "Details:" block; no nudge. + const detailed = capture(() => printReport(withRaw, true)); + expect(detailed.some((l) => l.includes("Details:"))).toBe(true); + expect(detailed.some((l) => l.includes(""))).toBe(true); + expect(detailed.some((l) => /Run with --detail/.test(l))).toBe(false); + }); + + it("summary shows only non-zero categories", () => { + const lines = capture(() => + printReport( + report({ summary: { ok: 3, warn: 0, error: 0, skipped: 0 } }), + ), + ); + expect(lines.some((l) => l === "3 ok")).toBe(true); + expect(lines.some((l) => /warning|error|skipped/.test(l))).toBe(false); + }); + + it("renders bundle-managed resources and wiring findings in the flat list", () => { + const external = res("ok", "sql_warehouse"); // origin undefined ⇒ runtime + const managed = res("skipped", "job"); + managed.target.origin = "bundle-managed"; + managed.target.alias = "Report Job"; + managed.layers = [ + { layer: "existence", status: "skipped", code: "BUNDLE_MANAGED" }, + ]; + + const lines = capture(() => + printReport( + report({ + resources: [external, managed], + wiring: [ + { + status: "error", + code: "VALUEFROM_UNBOUND", + label: "SOME_ENV", + detail: "app.yaml binds X to Y, no such binding", + }, + ], + summary: { ok: 1, warn: 0, error: 0, skipped: 1 }, + }), + ), + ); + const out = lines.join("\n"); + // No titled sub-sections: a single flat, severity-sorted checklist. + expect(out).not.toContain("Runtime connectivity"); + expect(out).not.toContain("Deploy declaration"); + // A bundle-managed resource is shown as deploy-created, not probed. + expect(out).toContain("will be created on deploy"); + expect(out).toContain("Report Job"); + // The wiring error sorts to the top (most severe), above the ok resource. + const wiringIdx = lines.findIndex((l) => l.includes("no such binding")); + const okIdx = lines.findIndex((l) => l.includes("sql_warehouse")); + expect(wiringIdx).toBeGreaterThanOrEqual(0); + expect(wiringIdx).toBeLessThan(okIdx); + }); + + it("renders the authoritative summary verbatim (no re-folding)", () => { + // summary is built in runDoctor and already includes auth + wiring; the + // renderer must print it as-is, not recompute. + const lines = capture(() => + printReport( + report({ + auth: { status: "error", detail: "bad creds" }, + summary: { ok: 0, warn: 0, error: 1, skipped: 0 }, + exitCode: 1, + }), + ), + ); + expect(lines.some((l) => l.startsWith("1 error"))).toBe(true); + }); +}); + +describe("exitCodeFor", () => { + // The exit code is computed once in runDoctor and stored on the report; + // exitCodeFor just surfaces it. (Aggregation is covered in run.test.ts.) + it("returns the report's exitCode field", () => { + expect(exitCodeFor(report({ exitCode: 0 }))).toBe(0); + expect(exitCodeFor(report({ exitCode: 1 }))).toBe(1); + }); +}); + +describe("printReportJson", () => { + it("emits the full report as parseable JSON", () => { + const r = report({ summary: { ok: 1, warn: 0, error: 0, skipped: 0 } }); + const lines = capture(() => printReportJson(r)); + expect(JSON.parse(lines.join("\n"))).toEqual(r); + }); + + it("strips the raw SDK error by default (matches the --detail gate)", () => { + const r = report({ + auth: { + status: "error", + detail: "authentication failed", + raw: "default auth: cannot get access token: ", + }, + }); + const parsed = capture(() => printReportJson(r)); + const out = JSON.parse(parsed.join("\n")) as DoctorReport; + expect(out.auth.raw).toBeUndefined(); + expect(parsed.join("\n")).not.toContain(""); + }); + + it("includes raw when detail=true (opt-in)", () => { + const r = report({ + auth: { + status: "error", + detail: "authentication failed", + raw: "default auth: cannot get access token: ", + }, + }); + const parsed = capture(() => printReportJson(r, true)); + const out = JSON.parse(parsed.join("\n")) as DoctorReport; + expect(out.auth.raw).toContain(""); + }); +}); diff --git a/packages/shared/src/cli/commands/doctor/report.ts b/packages/shared/src/cli/commands/doctor/report.ts new file mode 100644 index 000000000..ef06a43cf --- /dev/null +++ b/packages/shared/src/cli/commands/doctor/report.ts @@ -0,0 +1,256 @@ +/** + * Rendering for `appkit doctor` — turns a {@link DoctorReport} into a + * human-readable console report or JSON. + */ + +import pc from "picocolors"; +import { + AUTH_UNAVAILABLE_CODE, + type CheckStatus, + type DoctorReport, + type ResourceCheckResult, + STATUS_SEVERITY, + type WiringFinding, +} from "./types"; + +/** Column at which a row's label starts, to align detail/hint lines beneath. */ +const SUB_INDENT = " "; + +/** + * True when a resource's only finding is an auth-skipped existence probe, so it + * can be collapsed into one summary line. + */ +function isOnlyAuthSkipped(r: ResourceCheckResult): boolean { + if (r.status !== "skipped") return false; + const hasAuthSkip = r.layers.some( + (l) => l.layer === "existence" && l.code === AUTH_UNAVAILABLE_CODE, + ); + if (!hasAuthSkip) return false; + return r.layers.every( + (l) => + l.status === "ok" || + (l.layer === "existence" && l.code === AUTH_UNAVAILABLE_CODE), + ); +} + +/** Left-pads every line of `text` by `spaces`, for indented detail blocks. */ +function indent(text: string, spaces: number): string { + const pad = " ".repeat(spaces); + return text + .split("\n") + .map((l) => `${pad}${l}`) + .join("\n"); +} + +/** A status glyph, coloured by severity. */ +function glyph(status: CheckStatus): string { + switch (status) { + case "ok": + return pc.green("✓"); + case "warn": + return pc.yellow("⚠"); + case "error": + return pc.red("✗"); + default: + return pc.dim("•"); + } +} + +/** + * Highlights actionable tokens in a detail/hint line: cyan for a "quoted" id or + * `backticked` code span (delimiters stripped), bold for a SCREAMING_SNAKE env + * var. Nesting is safe — bold's reset (22) doesn't clear the cyan (39). + */ +function highlight(text: string): string { + return text + .replace(/`([^`]+)`/g, (_, code) => pc.cyan(code)) + .replace(/"([^"]+)"/g, (_, id) => pc.cyan(id)) + .replace(/\b[A-Z][A-Z0-9]*(?:_[A-Z0-9]+)+\b/g, (id) => pc.bold(id)); +} + +/** A sub-line beneath a row (detail or hint), consistently indented. */ +function subLine(text: string): void { + console.log(`${SUB_INDENT}${text}`); +} + +/** + * Prints the expanded body of a non-ok row: each indented `detail` line, then + * any `Hint:` set off by a blank line. Callers own the surrounding blank lines. + */ +function printRowBody( + details: Array, + hints: Array, +): void { + for (const d of details) if (d) subLine(highlight(d)); + const realHints = hints.filter((h): h is string => Boolean(h)); + if (realHints.length > 0) { + console.log(""); + for (const h of realHints) subLine(`${pc.dim("Hint:")} ${highlight(h)}`); + } +} + +/** Shows only the non-zero categories, coloured by severity. */ +function summaryLine(counts: { + ok: number; + warn: number; + error: number; + skipped: number; +}): string { + const parts: string[] = []; + if (counts.error) + parts.push(pc.red(`${counts.error} error${counts.error > 1 ? "s" : ""}`)); + if (counts.warn) + parts.push( + pc.yellow(`${counts.warn} warning${counts.warn > 1 ? "s" : ""}`), + ); + if (counts.ok) parts.push(pc.green(`${counts.ok} ok`)); + if (counts.skipped) parts.push(pc.dim(`${counts.skipped} skipped`)); + return parts.length > 0 ? parts.join(pc.dim(", ")) : "nothing to check"; +} + +/** Prints the leading `Auth` check row (always first). On failure, appends the + * target profile, a hint, and — with `--detail` — the raw SDK error. */ +function printAuthRow(report: DoctorReport, detail: boolean): void { + const { auth } = report; + console.log(` ${glyph(auth.status)} Auth — ${auth.detail ?? auth.status}`); + if (auth.status !== "error") return; + + if (auth.profile) { + subLine(`${pc.dim("profile:")} ${pc.cyan(auth.profile)}`); + } + if (auth.hint) { + console.log(""); + subLine(`${pc.dim("Hint:")} ${highlight(auth.hint)}`); + } + if (detail && auth.raw) { + console.log(""); + subLine(pc.dim("Details:")); + console.log(pc.dim(indent(auth.raw, SUB_INDENT.length + 2))); + } +} + +/** + * Prints one resource row: a tick and the name on the happy path, actionable + * layer details beneath otherwise. Bundle-managed resources are noted as + * deploy-created, not probed. + */ +function printResourceRow(r: ResourceCheckResult): void { + const { target } = r; + if (target.origin === "bundle-managed") { + console.log( + ` ${pc.dim("⧗")} ${target.alias} ${pc.dim("will be created on deploy")}`, + ); + return; + } + console.log(` ${glyph(r.status)} ${target.alias}`); + const problems = r.layers.filter((l) => l.status !== "ok"); + if (problems.length > 0) { + printRowBody( + problems.map((l) => l.detail), + problems.map((l) => l.hint), + ); + } +} + +/** Prints a wiring finding with the same shape as a resource row: + * `glyph label` on the first line, then the indented detail and hint. */ +function printWiringRow(f: WiringFinding): void { + console.log(` ${glyph(f.status)} ${highlight(f.label)}`); + printRowBody([f.detail], [f.hint]); +} + +/** A unified printable row. `expanded` rows get a blank line on either side; + * plain `ok` rows pack together. */ +interface Row { + status: CheckStatus; + expanded: boolean; + print: () => void; +} + +export function printReport(report: DoctorReport, detail = false): void { + const { resources, wiring, summary, auth } = report; + + // One flat, severity-sorted list. Auth leads and never sorts. + const rows: Row[] = []; + rows.push({ + status: auth.status, + expanded: auth.status === "error", + print: () => printAuthRow(report, detail), + }); + + // When auth failed, resources whose only finding is the auth-skip carry no + // new information, so collapse them into a single line. + const authSkipped: ResourceCheckResult[] = []; + const checked: Row[] = []; + for (const r of resources) { + if (isOnlyAuthSkipped(r)) { + authSkipped.push(r); + continue; + } + checked.push({ + status: r.status, + expanded: r.status !== "ok" && r.target.origin !== "bundle-managed", + print: () => printResourceRow(r), + }); + } + if (authSkipped.length > 0) { + checked.push({ + status: "skipped", + expanded: false, + print: () => + console.log( + ` ${pc.dim("•")} ${pc.dim( + `${authSkipped.length} resource${authSkipped.length > 1 ? "s" : ""} not checked (auth failed)`, + )}`, + ), + }); + } + for (const f of wiring) { + checked.push({ + status: f.status, + expanded: true, + print: () => printWiringRow(f), + }); + } + + // Most severe first (descending severity); stable within a status. + checked.sort((a, b) => STATUS_SEVERITY[b.status] - STATUS_SEVERITY[a.status]); + rows.push(...checked); + + // A blank line sets any expanded row apart from its neighbours. + console.log(""); + rows.forEach((row, i) => { + const prev = rows[i - 1]; + if (i > 0 && (row.expanded || prev.expanded)) console.log(""); + row.print(); + }); + + // `summary` already counts auth + wiring (built authoritatively in run.ts), + // so render it directly — no re-folding, and it matches what --json emits. + console.log(""); + console.log(summaryLine(summary)); + // Point at --detail when there's more to show and it wasn't requested. + if (!detail && auth.raw) { + console.log(pc.dim("\nRun with --detail for full error output.")); + } + console.log(""); +} + +/** + * Emits the report as JSON. The raw SDK error (`auth.raw`) can carry sensitive + * detail, so — matching the human report's `--detail` gate — it's stripped + * unless `detail` is set. `--json --detail` opts back into the full error. + */ +export function printReportJson(report: DoctorReport, detail = false): void { + const emitted = + detail || report.auth.raw === undefined + ? report + : { ...report, auth: { ...report.auth, raw: undefined } }; + console.log(JSON.stringify(emitted, null, 2)); +} + +/** The report's exit code (non-zero if anything errored), so `appkit doctor` + * can gate CI / pre-deploy. Computed in `runDoctor`; this reads the field. */ +export function exitCodeFor(report: DoctorReport): number { + return report.exitCode; +} diff --git a/packages/shared/src/cli/commands/doctor/resolve-targets.test.ts b/packages/shared/src/cli/commands/doctor/resolve-targets.test.ts new file mode 100644 index 000000000..3988c82c9 --- /dev/null +++ b/packages/shared/src/cli/commands/doctor/resolve-targets.test.ts @@ -0,0 +1,301 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { + resolveTargetsFromCwd, + targetsFromManifestFile, +} from "./resolve-targets"; + +function makeTempDir(): string { + return fs.mkdtempSync(path.join(os.tmpdir(), "doctor-targets-")); +} + +function cleanDir(dir: string): void { + try { + fs.rmSync(dir, { recursive: true, force: true }); + } catch { + // best effort + } +} + +const MANIFEST = { + version: "2.0", + plugins: { + analytics: { + requiredByTemplate: true, + resources: { + required: [ + { + type: "sql_warehouse", + resourceKey: "sql-warehouse", + alias: "SQL Warehouse", + permission: "CAN_USE", + fields: { id: { env: "DATABRICKS_WAREHOUSE_ID" } }, + }, + ], + optional: [], + }, + }, + agents: { + requiredByTemplate: true, + resources: { + required: [], + optional: [ + { + type: "serving_endpoint", + resourceKey: "serving", + alias: "Chat model", + permission: "CAN_QUERY", + fields: { name: { env: "DATABRICKS_SERVING_ENDPOINT_NAME" } }, + }, + ], + }, + }, + server: { + requiredByTemplate: true, + resources: { required: [], optional: [] }, + }, + }, +}; + +describe("targetsFromManifestFile", () => { + const dirs: string[] = []; + afterEach(() => { + for (const d of dirs) cleanDir(d); + dirs.length = 0; + }); + + function writeManifest(obj: unknown): string { + const dir = makeTempDir(); + dirs.push(dir); + const p = path.join(dir, "appkit.plugins.json"); + fs.writeFileSync(p, JSON.stringify(obj)); + return p; + } + + it("flattens required and optional resources across plugins", () => { + const targets = targetsFromManifestFile(writeManifest(MANIFEST)); + expect(targets).toHaveLength(2); + + const warehouse = targets.find((t) => t.type === "sql_warehouse"); + expect(warehouse).toMatchObject({ + type: "sql_warehouse", + resourceKey: "sql-warehouse", + alias: "SQL Warehouse", + plugin: "analytics", + requiredPermission: "CAN_USE", + required: true, + envVars: ["DATABRICKS_WAREHOUSE_ID"], + }); + + const serving = targets.find((t) => t.type === "serving_endpoint"); + expect(serving).toMatchObject({ + plugin: "agents", + required: false, + envVars: ["DATABRICKS_SERVING_ENDPOINT_NAME"], + }); + }); + + it("checks only requiredByTemplate plugins when any are marked", () => { + // analytics is marked (used in the app); agents is discovered-but-unused. + const targets = targetsFromManifestFile( + writeManifest({ + version: "2.0", + plugins: { + analytics: { + requiredByTemplate: true, + resources: { + required: [ + { + type: "sql_warehouse", + resourceKey: "sql-warehouse", + permission: "CAN_USE", + fields: { id: { env: "DATABRICKS_WAREHOUSE_ID" } }, + }, + ], + }, + }, + agents: { + // no requiredByTemplate → discovered but not used → skipped + resources: { + optional: [ + { + type: "serving_endpoint", + resourceKey: "serving", + permission: "CAN_QUERY", + fields: { name: { env: "DATABRICKS_SERVING_ENDPOINT_NAME" } }, + }, + ], + }, + }, + }, + }), + ); + expect(targets).toHaveLength(1); + expect(targets[0].plugin).toBe("analytics"); + }); + + it("returns nothing when no plugin is marked requiredByTemplate", () => { + // A manifest whose plugins are all discovered-but-unused → nothing to check. + const targets = targetsFromManifestFile( + writeManifest({ + version: "2.0", + plugins: { + analytics: { + resources: { + required: [ + { + type: "sql_warehouse", + resourceKey: "sql-warehouse", + permission: "CAN_USE", + fields: { id: { env: "DATABRICKS_WAREHOUSE_ID" } }, + }, + ], + }, + }, + }, + }), + ); + expect(targets).toEqual([]); + }); + + it("excludes value-default and platform-injected fields from envVars", () => { + const targets = targetsFromManifestFile( + writeManifest({ + version: "2.0", + plugins: { + lakebase: { + requiredByTemplate: true, + resources: { + required: [ + { + type: "postgres", + resourceKey: "pg", + alias: "Postgres", + permission: "CAN_CONNECT_AND_CREATE", + fields: { + endpointPath: { env: "LAKEBASE_ENDPOINT", origin: "cli" }, + host: { + env: "PGHOST", + localOnly: true, + origin: "platform", + }, + port: { + env: "PGPORT", + value: "5432", + localOnly: true, + origin: "platform", + }, + sslmode: { + env: "PGSSLMODE", + value: "require", + localOnly: true, + origin: "platform", + }, + }, + }, + ], + optional: [], + }, + }, + }, + }), + ); + const pg = targets.find((t) => t.type === "postgres"); + expect(pg?.envVars).toEqual(["LAKEBASE_ENDPOINT"]); + }); + + it("resolves fieldValues from a static `value` when the env var is unset", () => { + delete process.env.SOME_UNSET_ENV; + const targets = targetsFromManifestFile( + writeManifest({ + version: "2.0", + plugins: { + demo: { + requiredByTemplate: true, + resources: { + required: [ + { + type: "sql_warehouse", + resourceKey: "wh", + alias: "WH", + permission: "CAN_USE", + fields: { + id: { env: "SOME_UNSET_ENV", value: "baked-in-id" }, + }, + }, + ], + optional: [], + }, + }, + }, + }), + ); + const wh = targets.find((t) => t.type === "sql_warehouse"); + expect(wh?.fieldValues.id).toBe("baked-in-id"); + }); + + it("prefers the env value over a static `value` default", () => { + process.env.DOCTOR_ENV_WINS = "from-env"; + try { + const targets = targetsFromManifestFile( + writeManifest({ + version: "2.0", + plugins: { + demo: { + requiredByTemplate: true, + resources: { + required: [ + { + type: "sql_warehouse", + resourceKey: "wh", + alias: "WH", + permission: "CAN_USE", + fields: { + id: { env: "DOCTOR_ENV_WINS", value: "baked-in-id" }, + }, + }, + ], + optional: [], + }, + }, + }, + }), + ); + const wh = targets.find((t) => t.type === "sql_warehouse"); + expect(wh?.fieldValues.id).toBe("from-env"); + } finally { + delete process.env.DOCTOR_ENV_WINS; + } + }); + + it("returns an empty list for a manifest with no resources", () => { + const targets = targetsFromManifestFile( + writeManifest({ version: "2.0", plugins: { server: {} } }), + ); + expect(targets).toEqual([]); + }); + + it("throws on invalid JSON", () => { + const dir = makeTempDir(); + dirs.push(dir); + const p = path.join(dir, "appkit.plugins.json"); + fs.writeFileSync(p, "{ not json"); + expect(() => targetsFromManifestFile(p)).toThrow(/Failed to parse/); + }); +}); + +describe("resolveTargetsFromCwd", () => { + const dirs: string[] = []; + afterEach(() => { + for (const d of dirs) cleanDir(d); + dirs.length = 0; + }); + + it("returns an empty list when no manifest is present", () => { + const dir = makeTempDir(); + dirs.push(dir); + expect(resolveTargetsFromCwd(dir)).toEqual([]); + }); +}); diff --git a/packages/shared/src/cli/commands/doctor/resolve-targets.ts b/packages/shared/src/cli/commands/doctor/resolve-targets.ts new file mode 100644 index 000000000..35e56cdb8 --- /dev/null +++ b/packages/shared/src/cli/commands/doctor/resolve-targets.ts @@ -0,0 +1,161 @@ +/** + * Reads the app's resolved template manifest (`appkit.plugins.json`) and + * flattens each plugin's declared resources into the flat {@link ResourceTarget} + * shape the checks consume. Offline and SDK-free — parses the same file + * `appkit plugin list` reads. + */ + +import fs from "node:fs"; +import path from "node:path"; +import type { ResourceTarget } from "./types"; +import { errorMessage } from "./utils"; + +export const DEFAULT_MANIFEST_FILE = "appkit.plugins.json"; + +interface ManifestField { + env?: string; + /** Static default value baked into the manifest. */ + value?: string; + origin?: "user" | "platform" | "static" | "cli"; + /** Only generated into the local .env; the platform injects it at deploy. */ + localOnly?: boolean; +} + +interface ManifestResource { + type: string; + resourceKey: string; + alias?: string; + permission: string; + fields?: Record; +} + +interface ManifestPlugin { + resources?: { + required?: ManifestResource[]; + optional?: ManifestResource[]; + }; + /** + * True when the plugin is actually used by the app (imported and passed to + * `createApp`). `plugin sync` discovers *every* plugin shipped by installed + * packages but only marks the used ones; doctor checks only those, so an + * unused built-in doesn't produce phantom "missing env var" errors. + */ + requiredByTemplate?: boolean; +} + +interface TemplateManifest { + plugins?: Record; +} + +/** A field's env var is the developer's to supply only when it has no static + * default and isn't platform-injected at deploy time. */ +function isUserSuppliedEnv(field: ManifestField): boolean { + if (field.value !== undefined) return false; + if (field.origin === "platform" || field.origin === "static") return false; + if (field.localOnly) return false; + return true; +} + +/** Env vars the config layer should presence-check (i.e. user-supplied ones). */ +function envVarsOf(resource: ManifestResource): string[] { + const fields = resource.fields ?? {}; + const envs: string[] = []; + for (const field of Object.values(fields)) { + if (field?.env && isUserSuppliedEnv(field)) envs.push(field.env); + } + return envs; +} + +/** Resolves each field's value keyed by manifest field name, preferring the + * environment over a static `value` default. Unset/empty fields are omitted. */ +function fieldValuesOf(resource: ManifestResource): Record { + const fields = resource.fields ?? {}; + const values: Record = {}; + for (const [fieldName, field] of Object.entries(fields)) { + if (!field) continue; + const envValue = field.env ? process.env[field.env] : undefined; + const resolved = + envValue !== undefined && envValue.trim().length > 0 + ? envValue + : field.value; + if (resolved !== undefined && resolved.trim().length > 0) { + values[fieldName] = resolved; + } + } + return values; +} + +function toTarget( + plugin: string, + resource: ManifestResource, + required: boolean, +): ResourceTarget { + return { + type: resource.type, + resourceKey: resource.resourceKey, + alias: resource.alias ?? resource.resourceKey, + plugin, + requiredPermission: resource.permission, + required, + envVars: envVarsOf(resource), + fieldValues: fieldValuesOf(resource), + }; +} + +/** @throws if the file cannot be read or parsed. */ +export function targetsFromManifestFile( + manifestPath: string, +): ResourceTarget[] { + let raw: string; + try { + raw = fs.readFileSync(manifestPath, "utf-8"); + } catch (err) { + throw new Error( + `Failed to read manifest file ${manifestPath}: ${errorMessage(err)}`, + ); + } + + let data: TemplateManifest; + try { + data = JSON.parse(raw) as TemplateManifest; + } catch (err) { + throw new Error( + `Failed to parse manifest file ${manifestPath}: ${errorMessage(err)}`, + ); + } + + // `plugin sync` catalogues every plugin the installed packages ship, marking + // only those wired into `createApp` with `requiredByTemplate`. Check exactly + // those, else doctor reports phantom "missing env var" errors for unimported + // plugins. + // KNOWN LIMITATION: sync strips `requiredByTemplate` for non-GA plugins, so a + // used beta/experimental plugin is not checked here yet. See the README. + const selected = Object.entries(data.plugins ?? {}).filter( + ([, plugin]) => plugin.requiredByTemplate === true, + ); + + const targets: ResourceTarget[] = []; + for (const [pluginName, plugin] of selected) { + const resources = plugin.resources ?? {}; + for (const resource of resources.required ?? []) { + targets.push(toTarget(pluginName, resource, true)); + } + for (const resource of resources.optional ?? []) { + targets.push(toTarget(pluginName, resource, false)); + } + } + return targets; +} + +/** Returns an empty list if the manifest is absent — an app may legitimately + * declare no resources. */ +export function resolveTargetsFromCwd( + cwd: string = process.cwd(), + manifestFile: string = DEFAULT_MANIFEST_FILE, +): ResourceTarget[] { + const manifestPath = path.resolve(cwd, manifestFile); + if (!fs.existsSync(manifestPath)) { + return []; + } + return targetsFromManifestFile(manifestPath); +} diff --git a/packages/shared/src/cli/commands/doctor/run.test.ts b/packages/shared/src/cli/commands/doctor/run.test.ts new file mode 100644 index 000000000..d93bb5eb6 --- /dev/null +++ b/packages/shared/src/cli/commands/doctor/run.test.ts @@ -0,0 +1,208 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { runDoctor } from "./run"; + +/** + * Tests for the doctor orchestrator. The auth layer reaches the Databricks SDK, + * so these mock the bridge to keep the orchestration assertions deterministic + * and offline. + */ +vi.mock("./databricks-client", () => ({ + SdkNotInstalledError: class SdkNotInstalledError extends Error {}, + // Default: a client whose currentUser.me() succeeds. + getServiceClient: vi.fn(async () => ({ + client: { currentUser: { me: async () => ({ userName: "app-sp" }) } }, + })), +})); + +// Real probes by default; individual tests override to simulate a hung probe. +vi.mock("./checks-existence", async (importActual) => ({ + ...(await importActual()), +})); + +describe("runDoctor", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("reports ok auth by default", async () => { + const report = await runDoctor({}); + expect(report.auth.status).toBe("ok"); + expect(report.auth.code).toBe("AUTH_OK"); + }); + + it("counts the auth check in the summary (no resources ⇒ ok auth = 1 ok)", async () => { + const spy = vi.spyOn(process, "cwd").mockReturnValue("/nonexistent-doctor"); + const report = await runDoctor({}); + expect(report.resources).toEqual([]); + // Auth ok, no resources/wiring → summary reflects the auth check alone. + expect(report.summary).toEqual({ ok: 1, warn: 0, error: 0, skipped: 0 }); + expect(report.exitCode).toBe(0); + spy.mockRestore(); + }); + + it("folds an auth failure into summary.error and exitCode (the --json gap)", async () => { + const { getServiceClient } = await import("./databricks-client"); + vi.mocked(getServiceClient).mockRejectedValueOnce(new Error("no creds")); + // No manifest ⇒ no resources; only the failed auth contributes. + const spy = vi.spyOn(process, "cwd").mockReturnValue("/nonexistent-doctor"); + + const report = await runDoctor({}); + + expect(report.auth.status).toBe("error"); + // The bug this guards: a --json consumer reading summary.error must see 1, + // not 0, and exitCode must be non-zero. + expect(report.summary.error).toBe(1); + expect(report.exitCode).toBe(1); + spy.mockRestore(); + }); + + it("still resolves and offline-checks resources when auth fails", async () => { + const { getServiceClient } = await import("./databricks-client"); + vi.mocked(getServiceClient).mockRejectedValueOnce(new Error("no creds")); + + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "doctor-run-")); + fs.writeFileSync( + path.join(dir, "appkit.plugins.json"), + JSON.stringify({ + version: "2.0", + plugins: { + analytics: { + requiredByTemplate: true, + resources: { + required: [ + { + type: "sql_warehouse", + resourceKey: "sql-warehouse", + alias: "SQL Warehouse", + permission: "CAN_USE", + fields: { id: { env: "DOCTOR_RUN_MISSING_ENV" } }, + }, + ], + optional: [], + }, + }, + }, + }), + ); + const spy = vi.spyOn(process, "cwd").mockReturnValue(dir); + delete process.env.DOCTOR_RUN_MISSING_ENV; + + const report = await runDoctor({}); + + expect(report.auth.status).toBe("error"); + // Resource was still resolved and offline-checked despite auth failure. + expect(report.resources).toHaveLength(1); + expect(report.resources[0].status).toBe("error"); + const configLayer = report.resources[0].layers.find( + (l) => l.layer === "config", + ); + expect(configLayer?.status).toBe("error"); + + spy.mockRestore(); + fs.rmSync(dir, { recursive: true, force: true }); + }); + + it("bounds a hung probe with a timeout instead of hanging forever", async () => { + vi.useFakeTimers(); + const existence = await import("./checks-existence"); + // A probe that never settles — the reachable-but-unresponsive endpoint case. + vi.spyOn(existence, "runExistenceProbe").mockReturnValue( + new Promise(() => {}), + ); + + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "doctor-hang-")); + fs.writeFileSync( + path.join(dir, "appkit.plugins.json"), + JSON.stringify({ + version: "2.0", + plugins: { + analytics: { + requiredByTemplate: true, + resources: { + required: [ + { + type: "sql_warehouse", + resourceKey: "sql-warehouse", + alias: "SQL Warehouse", + permission: "CAN_USE", + fields: { id: { env: "DOCTOR_HANG_ENV" } }, + }, + ], + optional: [], + }, + }, + }, + }), + ); + const spy = vi.spyOn(process, "cwd").mockReturnValue(dir); + process.env.DOCTOR_HANG_ENV = "wh-123"; // config passes → reach existence + + const runPromise = runDoctor({}); + // Fire the deadline; without the timeout this promise would never resolve. + await vi.advanceTimersByTimeAsync(10_000); + const report = await runPromise; + + const existenceLayer = report.resources[0].layers.find( + (l) => l.layer === "existence", + ); + expect(existenceLayer?.status).toBe("error"); + expect(existenceLayer?.code).toBe("PROBE_TIMEOUT"); + + delete process.env.DOCTOR_HANG_ENV; + spy.mockRestore(); + fs.rmSync(dir, { recursive: true, force: true }); + vi.useRealTimers(); + }); + + it("maps an unexpected throw to a PROBE_EXCEPTION row (never crashes the report)", async () => { + const existence = await import("./checks-existence"); + // A synchronous throw — before withTimeout wraps it — so it bypasses the + // probe's own .catch and would otherwise reject Promise.all. + vi.spyOn(existence, "runExistenceProbe").mockImplementation(() => { + throw new Error("kaboom"); + }); + + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "doctor-throw-")); + fs.writeFileSync( + path.join(dir, "appkit.plugins.json"), + JSON.stringify({ + version: "2.0", + plugins: { + analytics: { + requiredByTemplate: true, + resources: { + required: [ + { + type: "sql_warehouse", + resourceKey: "sql-warehouse", + alias: "SQL Warehouse", + permission: "CAN_USE", + fields: { id: { env: "DOCTOR_THROW_ENV" } }, + }, + ], + optional: [], + }, + }, + }, + }), + ); + const spy = vi.spyOn(process, "cwd").mockReturnValue(dir); + process.env.DOCTOR_THROW_ENV = "wh-123"; // config passes → reach existence + + // Must resolve (not reject) — the whole report survives one bad resource. + const report = await runDoctor({}); + expect(report.resources).toHaveLength(1); + expect(report.resources[0].status).toBe("error"); + const layer = report.resources[0].layers.find( + (l) => l.code === "PROBE_EXCEPTION", + ); + expect(layer?.detail).toContain("kaboom"); + + delete process.env.DOCTOR_THROW_ENV; + spy.mockRestore(); + fs.rmSync(dir, { recursive: true, force: true }); + }); +}); diff --git a/packages/shared/src/cli/commands/doctor/run.ts b/packages/shared/src/cli/commands/doctor/run.ts new file mode 100644 index 000000000..ef2a8e787 --- /dev/null +++ b/packages/shared/src/cli/commands/doctor/run.ts @@ -0,0 +1,138 @@ +/** Orchestration for `appkit doctor`. */ + +import { originForEnvVars, readBundleInfo } from "./bundle"; +import { checkAuth, checkConfig } from "./checks"; +import { runExistenceProbe } from "./checks-existence"; +import { checkWiring } from "./checks-wiring"; +import { resolveTargetsFromCwd } from "./resolve-targets"; +import { + AUTH_UNAVAILABLE_CODE, + type CheckStatus, + type DoctorOptions, + type DoctorReport, + type LayerResult, + type ResourceCheckResult, + type ResourceTarget, + STATUS_SEVERITY, +} from "./types"; +import { errorMessage, TimeoutError, withTimeout } from "./utils"; + +function worst(a: CheckStatus, b: CheckStatus): CheckStatus { + return STATUS_SEVERITY[b] > STATUS_SEVERITY[a] ? b : a; +} + +async function checkResource( + target: ResourceTarget, + // undefined = auth failed, so the live existence layer is skipped. + client: unknown, +): Promise { + const layers: LayerResult[] = []; + let rolled: CheckStatus = "ok"; + + const configResult = await checkConfig(target); + layers.push(configResult); + rolled = worst(rolled, configResult.status); + // A hard config failure makes the existence probe meaningless. + if (configResult.status === "error") { + return { target, status: rolled, layers }; + } + + // A bundle-managed resource doesn't exist until deploy, so probing would be a + // false NOT_FOUND. + if (target.origin === "bundle-managed") { + layers.push({ + layer: "existence", + status: "skipped", + code: "BUNDLE_MANAGED", + detail: "created by this bundle on deploy — not probed", + }); + return { target, status: worst(rolled, "skipped"), layers }; + } + + if (client === undefined) { + layers.push({ + layer: "existence", + status: "skipped", + code: AUTH_UNAVAILABLE_CODE, + detail: "skipped because workspace authentication failed", + }); + rolled = worst(rolled, "skipped"); + } else { + // A reachable-but-unresponsive endpoint must not hang doctor, so bound the + // probe; a timeout becomes an error row rather than a hung process. + const result = await withTimeout(runExistenceProbe(client, target)).catch( + (err): LayerResult => ({ + layer: "existence", + status: "error", + code: err instanceof TimeoutError ? "PROBE_TIMEOUT" : "PROBE_FAILED", + detail: `probe did not complete: ${errorMessage(err)}`, + }), + ); + layers.push(result); + rolled = worst(rolled, result.status); + } + + return { target, status: rolled, layers }; +} + +/** + * Runs {@link checkResource} behind a guaranteed boundary: any unexpected throw + * (a probe's own `.catch` handles the common case, but this covers config + * errors and anything else) becomes a single error row instead of rejecting the + * `Promise.all` and losing the entire report to a stack trace. + */ +async function checkResourceSafe( + target: ResourceTarget, + client: unknown, +): Promise { + try { + return await checkResource(target, client); + } catch (err) { + return { + target, + status: "error", + layers: [ + { + layer: "existence", + status: "error", + code: "PROBE_EXCEPTION", + detail: `unexpected error while checking: ${errorMessage(err)}`, + }, + ], + }; + } +} + +export async function runDoctor(options: DoctorOptions): Promise { + const { result: auth, client } = await checkAuth(options); + + // Resolve and config-check resources even when auth failed, so a bad + // connection still surfaces config problems instead of hiding them. + const cwd = process.cwd(); + const targets = resolveTargetsFromCwd(cwd); + + const bundle = readBundleInfo(cwd); + for (const target of targets) { + const origin = originForEnvVars(target.envVars, bundle); + if (origin) target.origin = origin; + } + + // Probes are independent reads; Promise.all preserves order for a + // deterministic report. + const resources = await Promise.all( + targets.map((target) => checkResourceSafe(target, client)), + ); + const wiring = checkWiring(bundle, targets); + + // The summary counts *everything* with a status — resources, the auth check, + // and wiring findings — so a --json consumer reading summary.error can trust + // it, and the human/JSON outputs share one source of truth. + const summary = { ok: 0, warn: 0, error: 0, skipped: 0 }; + for (const result of resources) summary[result.status] += 1; + summary[auth.status] += 1; + for (const finding of wiring) summary[finding.status] += 1; + + const exitCode = summary.error > 0 ? 1 : 0; + + return { auth, resources, wiring, summary, exitCode }; +} diff --git a/packages/shared/src/cli/commands/doctor/types.ts b/packages/shared/src/cli/commands/doctor/types.ts new file mode 100644 index 000000000..acca790d2 --- /dev/null +++ b/packages/shared/src/cli/commands/doctor/types.ts @@ -0,0 +1,111 @@ +/** Type definitions for the `appkit doctor` command. */ + +/** Layers run per resource, in order; a hard failure short-circuits the rest. */ +export type CheckLayer = "auth" | "config" | "existence"; + +export type CheckStatus = "ok" | "warn" | "error" | "skipped"; + +/** + * The existence-layer `code` marking a probe skipped because auth failed. Set in + * `run.ts` and matched in `report.ts` to collapse those resources into one line; + * shared so the two sides can't drift. + */ +export const AUTH_UNAVAILABLE_CODE = "AUTH_UNAVAILABLE"; + +/** + * Severity ranking for a status. Used both to roll up the worst status across + * layers and to sort report rows most-severe-first (descending severity). + */ +export const STATUS_SEVERITY: Record = { + ok: 0, + skipped: 1, + warn: 2, + error: 3, +}; + +/** + * Where a resource's value comes from: `external` (exists now, so probe it) or + * `bundle-managed` (created by this bundle on deploy, so not probed). + * Undefined ⇒ external. + */ +export type ResourceOrigin = "external" | "bundle-managed"; + +export interface LayerResult { + layer: CheckLayer; + status: CheckStatus; + detail?: string; + /** Inferred guidance for fixing the finding. */ + hint?: string; + /** Machine-readable code for `--json` consumers (e.g. `NOT_FOUND`). */ + code?: string; +} + +export interface ResourceTarget { + type: string; + resourceKey: string; + /** Human-readable label. */ + alias: string; + plugin: string; + /** Declared permission level; shown for context, not checked. */ + requiredPermission: string; + /** Mandatory (vs optional) for the app. */ + required: boolean; + envVars: string[]; + /** Resolved field values keyed by manifest field name; unset fields omitted. */ + fieldValues: Record; + origin?: ResourceOrigin; +} + +export interface ResourceCheckResult { + target: ResourceTarget; + /** Worst status across all layers run for this resource. */ + status: CheckStatus; + layers: LayerResult[]; +} + +export interface AuthCheckResult { + status: CheckStatus; + detail?: string; + hint?: string; + code?: string; + host?: string; + profile?: string; + /** Full underlying error, shown only with `--detail` or in `--json`. */ + raw?: string; +} + +/** A finding from the offline three-file wiring check. */ +export interface WiringFinding { + status: CheckStatus; + /** Machine-readable code (e.g. `VALUEFROM_UNBOUND`, `BUNDLE_REF_MISSING`). */ + code: string; + /** The env var or binding at fault. */ + label: string; + detail: string; + hint?: string; +} + +export interface DoctorReport { + auth: AuthCheckResult; + resources: ResourceCheckResult[]; + wiring: WiringFinding[]; + /** + * Authoritative counts across *everything* that has a status — resources, + * the auth check, and wiring findings — not just resources. A `--json` + * consumer can trust `summary.error === 0` to mean "nothing failed". + */ + summary: { ok: number; warn: number; error: number; skipped: number }; + /** Process exit code (0 ok, 1 if anything errored). The single unambiguous + * pass/fail signal for programmatic consumers. */ + exitCode: number; +} + +export interface DoctorOptions { + profile?: string; + json?: boolean; + /** Show full underlying error messages in the human report. */ + detail?: boolean; + /** Env file to load before checking (e.g. `.env.local`); overrides the .env + * the CLI auto-loads at startup. */ + envFile?: string; +} diff --git a/packages/shared/src/cli/commands/doctor/utils.test.ts b/packages/shared/src/cli/commands/doctor/utils.test.ts new file mode 100644 index 000000000..b17ccdc60 --- /dev/null +++ b/packages/shared/src/cli/commands/doctor/utils.test.ts @@ -0,0 +1,57 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { + DEFAULT_CHECK_TIMEOUT_MS, + errorMessage, + TimeoutError, + withTimeout, +} from "./utils"; + +describe("errorMessage", () => { + it("returns an Error's message", () => { + expect(errorMessage(new Error("boom"))).toBe("boom"); + }); + + it("stringifies a non-Error value", () => { + expect(errorMessage("plain")).toBe("plain"); + expect(errorMessage(42)).toBe("42"); + }); +}); + +describe("withTimeout", () => { + afterEach(() => { + vi.useRealTimers(); + }); + + it("resolves with the value when the promise settles in time", async () => { + await expect(withTimeout(Promise.resolve("done"), 1000)).resolves.toBe( + "done", + ); + }); + + it("propagates the promise's own rejection unchanged", async () => { + await expect( + withTimeout(Promise.reject(new Error("real failure")), 1000), + ).rejects.toThrow("real failure"); + }); + + it("rejects with TimeoutError when the promise hangs past the deadline", async () => { + vi.useFakeTimers(); + // A promise that never settles — the classic reachable-but-unresponsive case. + const hung = new Promise(() => {}); + const raced = withTimeout(hung, 10_000); + const assertion = expect(raced).rejects.toBeInstanceOf(TimeoutError); + await vi.advanceTimersByTimeAsync(10_000); + await assertion; + }); + + it("defaults to the shared deadline", async () => { + vi.useFakeTimers(); + const hung = new Promise(() => {}); + const raced = withTimeout(hung); + const assertion = expect(raced).rejects.toThrow( + new RegExp(`${DEFAULT_CHECK_TIMEOUT_MS}ms`), + ); + await vi.advanceTimersByTimeAsync(DEFAULT_CHECK_TIMEOUT_MS); + await assertion; + }); +}); diff --git a/packages/shared/src/cli/commands/doctor/utils.ts b/packages/shared/src/cli/commands/doctor/utils.ts new file mode 100644 index 000000000..65d10e164 --- /dev/null +++ b/packages/shared/src/cli/commands/doctor/utils.ts @@ -0,0 +1,38 @@ +/** Small shared helpers for the `appkit doctor` command. */ + +/** Extracts a human-readable message from an unknown thrown value. */ +export function errorMessage(err: unknown): string { + return err instanceof Error ? err.message : String(err); +} + +/** Default per-check wall-clock deadline (ms). A reachable-but-unresponsive + * endpoint must never hang doctor — it's a CI gate that has to return. */ +export const DEFAULT_CHECK_TIMEOUT_MS = 10_000; + +/** Thrown when a check exceeds its deadline. */ +export class TimeoutError extends Error { + constructor(ms: number) { + super(`timed out after ${ms}ms`); + this.name = "TimeoutError"; + } +} + +/** + * Races `promise` against a wall-clock deadline, rejecting with + * {@link TimeoutError} if it isn't settled in time. This bounds doctor even + * when the underlying SDK/driver call has no cancellation of its own — we can't + * abort the request, but we stop *waiting* on it so the report still returns. + * The timer is always cleared so a fast result leaves nothing pending. + */ +export function withTimeout( + promise: Promise, + ms = DEFAULT_CHECK_TIMEOUT_MS, +): Promise { + let timer: ReturnType; + const deadline = new Promise((_, reject) => { + timer = setTimeout(() => reject(new TimeoutError(ms)), ms); + }); + return Promise.race([promise, deadline]).finally(() => + clearTimeout(timer), + ) as Promise; +} diff --git a/packages/shared/src/cli/index.ts b/packages/shared/src/cli/index.ts index aa60157c8..53398b0e1 100644 --- a/packages/shared/src/cli/index.ts +++ b/packages/shared/src/cli/index.ts @@ -6,6 +6,7 @@ import { fileURLToPath } from "node:url"; import { Command } from "commander"; import { codemodCommand } from "./commands/codemod/index.js"; import { docsCommand } from "./commands/docs.js"; +import { doctorCommand } from "./commands/doctor/index.js"; import { generateTypesCommand } from "./commands/generate-types.js"; import { lintCommand } from "./commands/lint.js"; import { pluginCommand } from "./commands/plugin/index.js"; @@ -28,5 +29,6 @@ cmd.addCommand(lintCommand); cmd.addCommand(docsCommand); cmd.addCommand(pluginCommand); cmd.addCommand(codemodCommand); +cmd.addCommand(doctorCommand); await cmd.parseAsync(); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c06f7db67..2e45b03b5 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -563,6 +563,15 @@ importers: commander: specifier: 12.1.0 version: 12.1.0 + dotenv: + specifier: 16.6.1 + version: 16.6.1 + js-yaml: + specifier: 4.2.0 + version: 4.2.0 + picocolors: + specifier: 1.1.1 + version: 1.1.1 zod: specifier: 4.3.6 version: 4.3.6 @@ -570,6 +579,9 @@ importers: '@types/express': specifier: 4.17.23 version: 4.17.23 + '@types/js-yaml': + specifier: 4.0.9 + version: 4.0.9 '@types/json-schema': specifier: 7.0.15 version: 7.0.15