feat(cli): add appkit doctor command - #489
Conversation
|
📦 Bundle size reportCompared against
|
| dist | raw | gzip |
|---|---|---|
| JS (runtime) | 855 KB (-16 B) | 298 KB (-8 B) |
| Type declarations | 314 KB | 109 KB (+9 B) |
| Source maps | 1.7 MB (-30 B) | 557 KB (-17 B) |
| Other | 11 KB | 3.7 KB |
| Total | 2.8 MB (-46 B) | 967 KB (-16 B) |
Per-entry composition (own code — deps external (as shipped))
| Entry | Initial (gz) | Lazy (gz) | Total (gz) | node_modules (min) | Own code (min) |
|---|---|---|---|---|---|
. |
87 KB (+3 B) | 2.5 KB | 90 KB (+3 B) | external | 285 KB (-8 B) |
./beta |
48 KB (+1 B) | 459 B | 48 KB (+1 B) | external | 140 KB (-8 B) |
./type-generator |
20 KB | 0 B | 20 KB | external | 57 KB |
Chunks:
| Entry | Chunk | Load | Size (gz) |
|---|---|---|---|
. |
index.js |
initial | 83 KB |
. |
utils.js |
initial | 4.0 KB |
. |
remote-tunnel-manager.js |
lazy | 2.5 KB |
./beta |
beta.js |
initial | 32 KB |
./beta |
stream-manager.js |
initial | 5.8 KB |
./beta |
wide-event-emitter.js |
initial | 3.2 KB |
./beta |
databricks.js |
initial | 3.0 KB |
./beta |
configuration.js |
initial | 2.1 KB |
./beta |
service-context.js |
initial | 1.3 KB |
./beta |
client.js |
initial | 431 B |
./beta |
client-options.js |
initial | 219 B |
./beta |
supervisor-api.js |
lazy | 194 B |
./beta |
databricks.js |
lazy | 142 B |
./beta |
index.js |
lazy | 123 B |
./type-generator |
index.js |
initial | 20 KB |
@databricks/appkit-ui
npm tarball (packed): 309 KB — gzipped download (dist + bin; excludes release-only docs/NOTICE).
| dist | raw | gzip |
|---|---|---|
| JS (runtime) | 362 KB | 121 KB |
| Type declarations | 209 KB | 76 KB |
| Source maps | 693 KB | 227 KB |
| CSS | 16 KB | 3.3 KB |
| Total | 1.3 MB | 427 KB |
Per-entry composition (consumer bundle — deps bundled, peerDeps external)
| Entry | Initial (gz) | Lazy (gz) | Total (gz) | node_modules (min) | Own code (min) |
|---|---|---|---|---|---|
./js |
4.3 KB | 49 KB | 54 KB | 208 KB | 12 KB |
./js/beta |
20 B | 0 B | 20 B | 0 B | 0 B |
./react |
429 KB | 49 KB | 478 KB | 1.3 MB | 168 KB |
./react/beta |
1.0 KB | 0 B | 1.0 KB | 0 B | 1.9 KB |
Chunks:
| Entry | Chunk | Load | Size (gz) |
|---|---|---|---|
./js |
index.js |
initial | 4.2 KB |
./js |
chunk |
initial | 120 B |
./js |
apache-arrow |
lazy | 49 KB |
./js/beta |
beta.js |
initial | 20 B |
./react |
index.js |
initial | 427 KB |
./react |
tslib |
initial | 2.1 KB |
./react |
apache-arrow |
lazy | 49 KB |
./react/beta |
beta.js |
initial | 1.0 KB |
Diagnose whether an AppKit app's declared Databricks resources are actually usable, beyond the startup env-var check. Registered as `appkit doctor`. Three layers per resource: - auth: validate DATABRICKS_HOST, then currentUser.me() (once, app-wide) - config: offline env-var presence check - existence: live per-type probe — control-plane .get() for warehouse, serving, genie, job, volume, vector index, uc_function; a real SELECT 1 connection for Lakebase/postgres. Errors are classified (NOT_FOUND, INVALID_VALUE, ACCESS_DENIED) with clean one-line messages. Actionable hints translate opaque failures into the fix: expired/missing credentials to the right `databricks auth login`, a serving endpoint keyed by id to its name, and a Lakebase auth failure to the PGUSER/identity mismatch. Reaches the Databricks SDK / @databricks/appkit only through a runtime import in databricks-client.ts, keeping the SDK-free shared package free of the dependency and degrading gracefully when it is absent. Output is a friendly list (errors first; plugin/type + reason shown only on rows needing attention) or --json; exit code is non-zero on any error so it can gate CI. Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
8907117 to
0744f9a
Compare
🤖 AppKit PR bot🔬 Run evalsStart an eval for this PR from the evals-monitor app: Go to Evals Monitor → 📦 Try this PR's app templateScaffolds a new app from this PR's SDK build. Run it in any folder (requires the GitHub CLI — gh run download 31104647008 -R databricks/appkit -n appkit-template-0.55.0-pr.49a2d04-feat-doctor-command-489 -D appkit-pr-489 \
&& unzip -o "appkit-pr-489/appkit-template-0.55.0-pr.49a2d04-feat-doctor-command-489.zip" -d "appkit-pr-489" \
&& databricks apps init --template "appkit-pr-489"The template pins |
fc846c6 to
1dde6f9
Compare
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
1dde6f9 to
7569c08
Compare
Run existence probes concurrently via Promise.all (independent network reads; input order preserved), cache DATABRICKS_HOST in a local, and drop a redundant `unknown | undefined` parameter type. Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
…bility - Add --env-file flag to load an explicit env file (e.g. .env.local), overriding the auto-loaded .env so doctor checks the app's real env. - Add --detail flag to show the full raw SDK error; by default the report shows a short "authentication failed" headline plus an actionable hint. - Make auth hints action-first and reference the profile/host in use: classify unreachable-host (DNS/TLS) failures, recover the SDK-resolved profile so the login hint targets the right one, and surface it in the header. - Collapse auth-skipped resources into a single line, keep rows with real findings, and nudge toward --detail at the end. Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
Match the capitalized "Details:" label used in the rendered report. Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
…port Extend `appkit doctor` to understand Databricks Asset Bundles and to render a cleaner, single-list report. - Classify each resource's provenance from databricks.yml + app.yaml (bundle.ts): external (probed live) vs bundle-managed (created on deploy, reported as such rather than a false NOT_FOUND). - Add an offline three-file wiring check (checks-wiring.ts) that spans app.yaml <-> databricks.yml <-> plugin env vars, catching gaps the official validators miss: VALUEFROM_UNBOUND, BUNDLE_REF_MISSING, and ENV_UNWIRED. A wiring error gates the exit code for pre-deploy CI. - Filter to plugins marked requiredByTemplate: true. - Redesign the report: one flat, severity-sorted checklist (no titled sub-sections), every row sharing one shape (glyph + label, indented detail, hint set off by blank lines). Reserve colour for actionable tokens (cyan ids/code spans, bold env vars) via picocolors, which auto-disables for non-TTY / NO_COLOR. - Shorten and standardise error messages: drop redundant snake_case types and SDK noise, quote the value you'd act on, and give missing env vars an actionable hint. - Add --env-file and --detail flags. Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
CI forces colour on, so picocolors wraps the summary line in escape codes; the exact-match and startsWith assertions failed against `\x1b[32m3 ok\x1b[39m`. Strip ANSI in the capture helper so assertions are colour-agnostic in any environment. Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
…L, drop --manifest Act on code-review findings for the doctor command: - Declare js-yaml + @types/js-yaml in shared's package.json. bundle.ts imported js-yaml but only resolved via pnpm hoisting / being bundled into appkit — a fragile packaging gap. Declare it like dotenv/picocolors. - Surface malformed bundle YAML instead of swallowing it. readYaml now throws on a present-but-unparseable databricks.yml/app.yaml rather than returning null (which set present:false, skipping all wiring checks and reporting a false all-clear on deploy-breaking config). Absent files still degrade gracefully. - Remove the --manifest flag. Multiple plugin manifests is a non-scenario (appkit.plugins.json is generated at a fixed path), and the flag opened two footguns: a mistyped path silently checked nothing, and it could desync from the fixed bundle-file paths. Dropping it closes both. - Document the known limitation that used non-GA plugins aren't checked (plugin sync strips requiredByTemplate for non-GA), with a fast-follow note, in both the filter comment and the README. - Comment the deliberate, unrestored process.env mutation in getServiceClient (safe in a one-shot CLI). Adds tests for the malformed-YAML throw. Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
…tion
The prior code set process.env.DATABRICKS_CONFIG_PROFILE to apply an
explicit --profile — a hidden, unrestored global side effect. The SDK's
own Config exposes a first-class `profile` field, so pass it straight
into new WorkspaceClient({ profile }) instead. No env mutation, no leak
past the call, and it correctly overrides an ambient
DATABRICKS_CONFIG_PROFILE. Verified against the real SDK.
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
Address a security finding: sensitive data could reach captured output. - Strip URL userinfo (user:pass@) from DATABRICKS_HOST at capture, so credentials embedded in the host never reach the report or --json. Validation still runs on the raw value. - Gate auth.raw in --json behind --detail, matching the human report. A bare --json omits the raw SDK error (CI commonly captures --json); --json --detail opts back in. Also folds in pending doctor cleanups the tree already carried and that these changes build on: an errorMessage() helper in a new utils.ts, and STATUS_SEVERITY / AUTH_UNAVAILABLE_CODE constants shared via types.ts to stop run.ts and report.ts drifting. Adds tests for host sanitization and the --json raw gate (92 total). Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
ENV_UNWIRED was always a warning, but exitCodeFor gates only on errors — so a required plugin's env var with no app.yaml entry (set locally via .env, unset in the deployed container) exited 0, the exact works-locally-breaks-on-deploy case the wiring check exists to catch. Gate on target.required: error for required (fails the exit code, blocks pre-deploy CI), warn for optional. Tests cover both. Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
No probe, currentUser.me(), or Lakebase SELECT 1 had a deadline, so a reachable-but-unresponsive endpoint would hang doctor forever — and as a CI gate, the job would never return. Add withTimeout (Promise.race against a 10s wall-clock deadline) around each existence probe and the auth me() call. It can't cancel the underlying request, but it stops doctor *waiting* so the report returns: a timed-out probe becomes a PROBE_TIMEOUT error row, a timed-out auth becomes an auth failure. The timer is always cleared, so a fast result leaves nothing pending and the CLI exits promptly. Lakebase connection setup was already bounded (connectionTimeoutMillis: 10s); this covers a hung query on an established connection too. Tests: withTimeout unit tests (fake timers), a hung-probe run test, and a hung-auth test. Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
summary counted only resources; printReport folded auth + wiring in at render time but never wrote it back, so printReportJson emitted the unfolded counts. A --json consumer reading summary.error === 0 missed auth failures and wiring errors (e.g. a required ENV_UNWIRED) — the exact CI-gate case. Build summary over everything with a status (resources + auth + wiring) once in runDoctor, and add a top-level exitCode as the unambiguous pass/fail signal. printReport now renders the summary verbatim (no re-folding, so human and --json agree) and exitCodeFor just reads the field. Moves the aggregation coverage to run.test.ts. Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
The probe's own .catch covered a probe rejection, but checkResource had no top-level guard: an unexpected throw (a config-layer error, a synchronous throw in probe dispatch, anything else) rejected Promise.all and lost the entire report to a stack trace. Wrap each resource in checkResourceSafe, mapping any throw to a PROBE_EXCEPTION error row so the run always resolves and the rest of the report survives. Test verifies a synchronous probe throw becomes a row rather than crashing (confirmed it fails without the guard). Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
The Lakebase pool's default error-only logger dumps the raw SDK ApiError
(stack + full response blob) to stderr on a failed token fetch, burying
doctor's clean one-line classification. Pass logger: { error: false } so
doctor owns the failure output. Reorder createLakebasePool so a
caller-supplied logger overrides the appkit default.
Also trims the Lakebase auth hint and prunes redundant tests.
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
- checkConfig is synchronous (no async work); update its call site. - Drop the trivial exitCodeFor wrapper; read report.exitCode directly. - validateHost drops a redundant placeholder branch (!hasRealLabel covers it). - Prune tests made obsolete by the above. Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
calvarjorge
left a comment
There was a problem hiding this comment.
Great work, looks very nice! Some questions about how it works:
- For auth, what error do we print if no profile is set? Do we prioritize profile or host? (I think both are ways to point to a workspace)
- For env vars, we should somehow differentiate where the var is missing and the impact. In your example screenshot, it says:
DATABRICKS_GENIE_SPACE_ID is not set. We should probably clarify it refers to the env file, as oposed toapp.yamlordatabricks.yml. - How is the env determined? Do we just expect the user to have
.env, or we have a flexible way to determine which to use? - For the job, where permissions are missing, can we easily add more info about it? For example, adding the name can help identify it quicker than just the numeric id. Also, can we provide some instructions about how to fix it (ie, how to add the permissions)? is in the UI (then maybe we could even add a link) or in the databricks.yml?
- Re. copy in "Files are not added to the app.yaml", I think saying "it won't be set in the environment of the deployed app" would be more clear.
Addresses PR review feedback on the doctor command, plus two bugs found
while verifying it against a live workspace.
Auth now reports the workspace, not just the profile. The SDK resolves
auth per-field — env is read first and a profile fills only what env
left unset — so DATABRICKS_HOST always wins the host while a profile can
still supply the credentials. A failed row showing only the profile
therefore hid which workspace was actually contacted. The host is read
from the resolved SDK config (falling back to the host embedded in the
SDK's ConfigError, then the env var) and always sanitized, so embedded
credentials can't leak.
That per-field split also gets its own warning, HOST_PROFILE_CONFLICT:
when DATABRICKS_HOST and the named profile target different workspaces,
you authenticate with one workspace's token against another's URL. The
profile's declared host is read offline from ~/.databrickscfg, so the
check costs nothing and runs whether or not the credentials work. It's a
warning, not an error — the credentials do work, so it must not gate CI.
Config-layer messages now name the env file ("is not set in `.env`", or
the --env-file path). An env var can be declared in three places — the
local env file, app.yaml, databricks.yml — so a bare "is not set" was
ambiguous about which one was meant. The hint is dropped entirely when
app.yaml already wires the var: deploy is fine and "set it locally" only
restates the detail line.
ENV_UNWIRED now says the var "won't be set in the environment of the
deployed app", naming the concrete consequence. Its optional variant no
longer conflates an optional resource with an unset var.
A 403 no longer asserts a cause. Several APIs (jobs, warehouses) return
403 for a resource that doesn't exist as well as one you can't read —
verified live — so claiming "no permission" misdiagnoses a typo'd id.
New setup notice: doctor run outside the app root found no manifest and
no .env, then printed a bare green tick and exited 0 having checked
nothing — a CI gate passing an app it never looked at. It now says so.
The manifest is the signal for "this is an app root", so the two notices
are mutually exclusive: without it only the wrong-directory cause is
reported, and the .env notice is withheld as noise on top.
Two bugs fixed along the way: config.host is undefined until the SDK
resolves lazily on the first API call, so it must be read after me(),
not at construction; and the summary now counts setup notices, keeping
--json and the human report in agreement.
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
main added a noRestrictedImports rule (f6e81eb) routing all Databricks SDK access through packages/appkit/src/workspace-client. CI lints the PR merge result, so the rule applies to this branch even though it predates the merge — and it fails on doctor's two dynamic SDK imports. The rule's suggested remedy doesn't apply here: appkit depends on shared, so importing appkit's wrapper back into shared would be a dependency cycle. shared also can't take a direct SDK dependency, which is why these imports are dynamic in the first place. databricks-client.ts already serves as this package's equivalent wrapper — the sole file in shared that touches the SDK — so the two call sites are suppressed with that reasoning recorded at the top of the file. Verified by linting against main's biome.json: no diagnostics. Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
|
I have done few fixes
Appkit doctor makes requests to the host if it is set with token of profile, on a mismatch of profile and .env host gives a warn.
Done
There is --env-file option for doctor to override default .env
There is no way no distinguish between no permission and job doesn't exists errors, so now it just gives a generic error doesn't exist or you don't have permission.
Done |
There was a problem hiding this comment.
ran this e2e against a scaffolded app on a real workspace — behaviour is solid, auth / config / existence (404, stopped) / wiring both directions / --json / exit codes all check out. two things id want fixed before merge: the bundle-managed false-error (hides the error but still exits 1) and the host cred leak in auth.detail. rest are small.
| layers.push(configResult); | ||
| rolled = worst(rolled, configResult.status); | ||
| // A hard config failure makes the existence probe meaningless. | ||
| if (configResult.status === "error") { |
There was a problem hiding this comment.
think theres a bug here for bundle-managed resources. the config check + early return at 48-49 runs before the bundle-managed branch at 54, so if the env var isnt set locally (normal — the resource is created on deploy) we return error and never hit the bundle-managed skip. then report.ts printResourceRow (146) collapses it to "⧗ will be created on deploy" and drops the error layer, so you get a green-looking row but summary says "1 error" and exit 1.
repro on a bundle-managed warehouse with DATABRICKS_WAREHOUSE_ID unset:
✓ Auth — authenticated as ...
⧗ SQL Warehouse will be created on deploy
1 error, 1 ok <- exit 1, no visible error row
this fails CI for a correctly configured app. can we move the bundle-managed short-circuit above checkConfig? and printResourceRow should still print non-ok layers instead of hiding them for bundle-managed
| try { | ||
| url = new URL(host); | ||
| } catch { | ||
| return `DATABRICKS_HOST is not a valid URL: "${host}"`; |
There was a problem hiding this comment.
the comment on checkAuth (146) says we only ever report the sanitized host so embedded creds dont leak, but validateHost builds all three messages (124/128/135) from the raw host, and that goes into auth.detail (159) which prints unconditionally + only auth.raw gets stripped from --json. so this leaks:
DATABRICKS_HOST=ftp://user:s3cr3t@secret.example.com appkit doctor --json
...
"host": "ftp://secret.example.com/", <- redacted, good
"detail": "DATABRICKS_HOST must be an http(s) URL: \"ftp://user:s3cr3t@secret.example.com\"" <- leaks
s3cr3t shows up in both human + json without --detail. can we run validateHost against sanitizeHost(rawHost), or build the messages from the sanitized value
| url.username = ""; | ||
| url.password = ""; | ||
| return url.toString(); | ||
| } catch { |
There was a problem hiding this comment.
small follow-up to the host leak — on a URL that doesnt parse this catch returns the raw host as-is, so ht!tp://user:pass@x keeps the creds in the host field too (and also trips the "not a valid URL" message above). maybe strip a leading ...@ or return a fixed sentinel on parse failure
|
|
||
| // Placeholders like "https://..." parse but have no real dotted label. | ||
| const hostname = url.hostname; | ||
| const hasRealLabel = /[a-z0-9]/i.test(hostname) && hostname.includes("."); |
There was a problem hiding this comment.
this rejects https://localhost:8080 as an "unfilled placeholder" since theres no dot in the hostname. breaks local/tunnel dev hosts — can we allow localhost explicitly
| const probeJob: ExistenceProbe = async (client, target) => { | ||
| const raw = field(target, "id"); | ||
| if (!raw) return missingField("id"); | ||
| const jobId = Number(raw); |
There was a problem hiding this comment.
job ids are int64, Number() loses precision past 2^53 and Number.isInteger still passes, so we'd probe the wrong id and report a false not-found. safer to validate with /^\d+$/ on the raw string
| */ | ||
| function highlight(text: string): string { | ||
| return text | ||
| .replace(/`([^`]+)`/g, (_, code) => pc.cyan(code)) |
There was a problem hiding this comment.
tiny one — the jsdoc + readme say env vars go bold+cyan inside a code span, but they dont: the cyan replaces at 66/67 wrap the token in ansi first, so the \b in the bold regex at 68 wont match. either drop that claim from the docs or run the bold pass before the cyan ones
appkit doctor - cli command that checks connectivity to the Databricks resources
The command does four level of checks:
DATABRICKS_HOSTand authenticates to the workspaceappkit.plugings.json↔apps.yaml↔databricks.ymlOutput is a list of resources and problems if any with hints. Also there is options --json for machine readable output. and --detail for more detailed overview with error itself.
Examples:
Failed auth
Genie Space is not provided, Job is not accessible or doesn't exists
Files are not added to the app.yaml
Everything is setup correctly
How it works:
appkit.plugins.json