diff --git a/.claude/commands/audit-core-plugin.md b/.claude/commands/audit-core-plugin.md index 4fd1027be..3cb90dcac 100644 --- a/.claude/commands/audit-core-plugin.md +++ b/.claude/commands/audit-core-plugin.md @@ -28,7 +28,8 @@ If **neither** path exists, stop and output: > - `packages/appkit/src/plugins/{PLUGIN_NAME}/` > - `packages/appkit/src/connectors/{PLUGIN_NAME}/` > -> Available plugins can be listed with: `ls packages/appkit/src/plugins/` +> Available plugins can be listed with: `npx @databricks/appkit plugin list --dir packages/appkit/src/plugins --json` +> (Falls back to `ls packages/appkit/src/plugins/` if the CLI is unavailable.) If at least one path exists, proceed. @@ -51,6 +52,33 @@ Read **all** files under: Collect the full contents of every file. You need the complete source to evaluate all 9 categories. +## Step 3.5: CLI Cross-Checks + +Before evaluating files by hand, run the AppKit CLI checks below and capture their output. Each result feeds into a specific category in Step 5; treat any non-zero exit as a finding under that category. + +```bash +# Schema-validates manifest.json against the plugin-manifest schema. +# Failures → Category 1 (Manifest Design), severity MUST. +npx @databricks/appkit plugin validate packages/appkit/src/plugins/{PLUGIN_NAME} + +# Confirms the plugin is discoverable from the synced manifest with the expected +# displayName, package, stability, and resource counts. Mismatches → Category 1 +# (Manifest Design) or Category 0 (Structural Completeness). +npx @databricks/appkit plugin list --dir packages/appkit/src/plugins --json \ + | jq '.[] | select(.name == "{PLUGIN_NAME}")' + +# Previews the template manifest the loader would emit (no --write). +# Use the output to verify the plugin appears, that resources match manifest.json, +# and that no warnings about orphaned resources / removed plugins are printed. +# Sync-time warnings → Category 1 (Manifest Design), severity SHOULD unless the +# plugin is missing entirely (then MUST). +npx @databricks/appkit plugin sync --json +``` + +If `plugin validate` exits non-zero, record a MUST finding under Category 1 with the validator's error output as the description, and continue to Step 4 — the rest of the audit still applies. + +If `packages/appkit/src/plugins/{PLUGIN_NAME}/` does not exist (connector-only package), skip the three CLI checks and proceed. + ## Step 4: Structural Completeness Check If `packages/appkit/src/plugins/{PLUGIN_NAME}/` does not exist (connector-only package), mark Structural Completeness as **N/A** in the scorecard and proceed to Step 5. @@ -76,6 +104,12 @@ Treat each missing `MUST` file as a **MUST**-severity finding under the "Structu Before evaluating, read the shared review rules in `.claude/references/plugin-review-guidance.md` and apply them throughout this step (deduplication, cache-key tracing). +Fold the Step 3.5 CLI results into the matching categories: +- `plugin validate` failures → Category 1 (Manifest Design), MUST. +- `plugin list --json` mismatches between manifest fields and synced output → Category 1 (Manifest Design), SHOULD unless the plugin is absent (MUST). +- `plugin sync --json` warnings about orphaned resources / removed plugins → Category 0 (Structural Completeness) or Category 1, severity per the warning text. +- If the manifest declares `"stability": "beta"`, also run `npx @databricks/appkit plugin promote {PLUGIN_NAME} --to ga --dry-run`. Any rewrites it would perform that conflict with the current `/beta` re-export wiring → Category 0 (Structural Completeness), SHOULD. + Evaluate the plugin code against **all 9 categories** from the Category Index in `plugin-review-guidance.md`. Check each category's NEVER/MUST/SHOULD rules from the best-practices reference. For each guideline in each category, determine whether the plugin **passes**, **violates**, or is **not applicable** (e.g., SSE rules for a non-streaming plugin). Record findings with: diff --git a/.claude/commands/create-core-plugin.md b/.claude/commands/create-core-plugin.md index a72c6e194..bf5c19281 100644 --- a/.claude/commands/create-core-plugin.md +++ b/.claude/commands/create-core-plugin.md @@ -38,13 +38,47 @@ Use the answers to determine: ## 2. Scaffold with the CLI -Run the scaffolding command to generate boilerplate. Pipe answers non-interactively if possible, or run it interactively: +Run the scaffolding command to generate boilerplate. **Always prefer the non-interactive form** — the agent already gathered every required answer in Step 1, and the interactive prompts will hang in headless sessions. + +### 2a. Non-interactive (default) + +Required flags: `--placement`, `--path`, `--name`, `--description`. Optional but recommended: `--display-name`, plus one of `--resources` (CSV of types) or `--resources-json` (full specs). + +Simple case (just resource types, defaults filled in): + +```bash +npx @databricks/appkit plugin create \ + --placement in-repo \ + --path packages/appkit/src/plugins/{name} \ + --name {name} \ + --display-name "{Display Name}" \ + --description "{description}" \ + --resources sql_warehouse,volume +``` + +Full-spec case (custom `resourceKey`, `permission`, `fields.env`): + +```bash +npx @databricks/appkit plugin create \ + --placement in-repo \ + --path packages/appkit/src/plugins/{name} \ + --name {name} \ + --display-name "{Display Name}" \ + --description "{description}" \ + --resources-json '[{"type":"sql_warehouse","resourceKey":"sql-warehouse","permission":"CAN_USE","fields":{"id":{"env":"DATABRICKS_WAREHOUSE_ID","description":"SQL Warehouse ID"}}}]' +``` + +Add `--force` only when intentionally regenerating into a non-empty directory. + +### 2b. Interactive fallback + +Use this **only** when the user explicitly asks for non-GA stability — `--placement`/`--path`/`--name`/`--description` work non-interactively, but the `stability` prompt (`ga` vs `beta`) is interactive-only. Run: ```bash npx @databricks/appkit plugin create ``` -Select **In-repo** placement and target path `packages/appkit/src/plugins/{name}`. Fill in the name, display name, description, and resources based on the gathered requirements. +Pick **In-repo** placement and target path `packages/appkit/src/plugins/{name}`. If you need `beta` and cannot run interactively, scaffold with the non-interactive command above (which defaults to `ga`), then hand-edit `manifest.json` to add `"stability": "beta"` — promotion to GA later goes through `appkit plugin promote` (see section 8). This generates `manifest.json`, the plugin class file, and `index.ts`. Then enhance the generated files following the patterns below. @@ -274,6 +308,19 @@ Example resource entry: } ``` +> **Tip:** When the resource was already known at scaffold time, prefer wiring it through `--resources-json` in Step 2a instead of hand-editing `manifest.json`. When a resource is discovered *after* scaffolding, use `appkit plugin add-resource` instead of editing the JSON by hand: +> +> ```bash +> npx @databricks/appkit plugin add-resource \ +> --path packages/appkit/src/plugins/{name} \ +> --type sql_warehouse \ +> --resource-key sql-warehouse \ +> --permission CAN_USE \ +> --fields-json '{"id":{"env":"DATABRICKS_WAREHOUSE_ID","description":"SQL Warehouse ID"}}' +> ``` +> +> Use `--no-required` for optional resources, `--dry-run` to preview the updated manifest without writing. + ### 4f. User API Scopes (OBO) If the plugin performs operations on behalf of the logged-in user via `this.asUser(req)`, it requires one or more `user_api_scopes` in the Databricks Apps bundle config (`databricks.yml`). Without the correct scopes, OBO calls will fail at runtime. @@ -348,11 +395,46 @@ Any plugin using `this.asUser(req)` must declare `user_api_scopes` in the bundle ## 7. After Scaffolding -Run these to verify: +Run these to verify, in order: ```bash +# 1. Schema-check the generated/edited manifest before anything else. +npx @databricks/appkit plugin validate packages/appkit/src/plugins/{name} + +# 2. Type/build/test the plugin. pnpm build pnpm typecheck pnpm test -npx @databricks/appkit plugin sync --write + +# 3. Re-sync the template manifest so the new plugin is picked up. +# In the AppKit monorepo prefer the workspace script — it points sync at +# `template/appkit.plugins.json` (the file shipped to consumers) instead of +# the project-root default that the raw CLI would write. +pnpm run sync:template + +# Outside the monorepo (or when sync:template is unavailable) fall back to: +# npx @databricks/appkit plugin sync --write +``` + +If the plugin must always ship with the template (i.e. be marked mandatory) even when not auto-detected via the server file's `plugins: [...]` array, pass it explicitly: + +```bash +pnpm run sync:template -- --require-plugins {name} +# or: npx @databricks/appkit plugin sync --write --require-plugins {name} +``` + +Use `npx @databricks/appkit plugin list --json` to confirm the plugin shows up in the synced manifest with the expected `displayName`, `package`, `stability`, and resource counts. + +## 8. Stability and Promotion (only if the plugin is non-GA) + +If the plugin was scaffolded as `beta` (see Section 2b), it must be re-exported from the `/beta` import path, not the GA root. When ready to graduate, **do not edit `manifest.json` and import barrels by hand** — use `promote`, which updates the manifest, rewrites `@databricks/appkit` ↔ `@databricks/appkit/beta` (and `appkit-ui/js` / `appkit-ui/react`) imports across the repo, and re-runs `sync:template`: + +```bash +# Preview every change first. +npx @databricks/appkit plugin promote {name} --to ga --dry-run + +# Apply (will run the generators + sync:template automatically). +npx @databricks/appkit plugin promote {name} --to ga ``` + +Promotion is one-way (`beta → ga` only). Use `--skip-imports` or `--skip-sync` only if you intend to run those steps separately. diff --git a/.claude/commands/review-core-plugin.md b/.claude/commands/review-core-plugin.md index 6db26038c..628342b56 100644 --- a/.claude/commands/review-core-plugin.md +++ b/.claude/commands/review-core-plugin.md @@ -74,11 +74,51 @@ Read the file `.claude/references/plugin-best-practices.md`. For each **relevant** category identified in Step 4, extract all NEVER, MUST, and SHOULD guidelines from that category section. +## Step 5.5: CLI Cross-Checks (per touched plugin) + +For **each** plugin detected in Step 3, run the AppKit CLI checks below before doing the textual review. Treat each non-zero exit or warning as a finding under the indicated category in Step 6. + +```bash +# Schema-validate the touched manifest. Failures → Category 1 (Manifest Design), MUST. +npx @databricks/appkit plugin validate packages/appkit/src/plugins/{plugin-name} + +# Preview the synced template manifest. Watch for: +# - The plugin missing from the output when the diff was supposed to add it +# → Category 0 / Category 1, MUST. +# - "Plugin '...' was removed. The following resource env vars may be orphaned: ..." +# when the diff deletes a plugin → flag as a release-note / migration concern. +# - displayName / package / resource counts that differ from manifest.json +# → Category 1, SHOULD. +npx @databricks/appkit plugin sync --json +``` + +Conditional checks (only when the diff matches): + +```bash +# Diff changes manifest.json's "stability" field, OR adds/removes the /beta +# import path for this plugin. The dry-run shows exactly which manifest fields +# and which import sites promote would touch — the diff should match. Any +# divergence → Category 0 (Structural Completeness), MUST. +npx @databricks/appkit plugin promote {plugin-name} --to ga --dry-run + +# Diff adds entries to manifest.json's resources.required / resources.optional +# arrays. Re-run the canonical scaffolder against an unmodified copy and compare +# — the diff should match what add-resource would have produced (alias, +# resourceKey, permission, fields.env defaults). Hand-rolled entries that drift +# from the defaults → Category 1, SHOULD. +npx @databricks/appkit plugin add-resource \ + --path packages/appkit/src/plugins/{plugin-name} \ + --type {resource-type} \ + --dry-run +``` + +Capture the relevant output and reference it from the corresponding findings in Step 6. + ## Step 6: Best-Practices Review Before evaluating, read the shared review rules in `.claude/references/plugin-review-guidance.md` and apply them throughout this step (deduplication, cache-key tracing). -For each plugin detected in Step 3, review the changed code against the scoped guidelines from Step 5. +For each plugin detected in Step 3, review the changed code against the scoped guidelines from Step 5, and **fold in the CLI results from Step 5.5** under the categories noted there. For each finding: - Identify the **severity** (NEVER, MUST, or SHOULD)