feat: skills for creating, reviewing, and auditing an AppKit plugin

.claude/commands/audit-core-plugin.md

@@ -0,0 +1,141 @@
+---
+description: Full audit of a core plugin against all best-practices categories with scorecard
+argument-hint: <plugin-name>
+---
+
+# Audit Core Plugin
+
+Perform a full audit of the named plugin against all AppKit plugin best-practices categories and produce a scorecard.
+
+**Plugin name:** $ARGUMENTS
+
+## Step 1: Validate Input
+
+If `$ARGUMENTS` is empty or missing, stop and output:
+
+> Usage: /audit-core-plugin <plugin-name>
+
+Otherwise, set `PLUGIN_NAME` to the value of `$ARGUMENTS` (trimmed, kebab-case).
+
+Check whether the plugin directory exists at either of these paths:
+
+- `packages/appkit/src/plugins/{PLUGIN_NAME}/`
+- `packages/appkit/src/connectors/{PLUGIN_NAME}/`
+
+If **neither** path exists, stop and output:
+
+> Error: Plugin "{PLUGIN_NAME}" not found. Checked:
+> - `packages/appkit/src/plugins/{PLUGIN_NAME}/`
+> - `packages/appkit/src/connectors/{PLUGIN_NAME}/`
+>
+> Available plugins can be listed with: `ls packages/appkit/src/plugins/`
+
+If at least one path exists, proceed.
+
+## Step 2: Load Best Practices Reference
+
+Read the full contents of:
+
+```
+.claude/references/plugin-best-practices.md
+```
+
+This defines the 9 audit categories and their NEVER/MUST/SHOULD guidelines. You will evaluate the plugin against every guideline in every category.
+
+## Step 3: File Discovery
+
+Read **all** files under:
+
+- `packages/appkit/src/plugins/{PLUGIN_NAME}/` (recursively, including `tests/` subdirectory)
+- `packages/appkit/src/connectors/{PLUGIN_NAME}/` (recursively, if this directory exists)
+
+Collect the full contents of every file. You need the complete source to evaluate all 9 categories.
+
+## 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.
+
+Otherwise, verify the following expected files exist inside `packages/appkit/src/plugins/{PLUGIN_NAME}/`:
+
+| Expected file | Required? |
+|---|---|
+| `manifest.json` | MUST |
+| Main plugin class file (any `.ts` file containing a class extending `Plugin`) | MUST |
+| `types.ts` | MUST |
+| `defaults.ts` | SHOULD |
+| `index.ts` | MUST |
+| `tests/` directory with at least one `.test.ts` file | MUST |
+
+Treat each missing `MUST` file as a **MUST**-severity finding under the "Structural Completeness" category. Treat a missing `SHOULD` file as a **SHOULD**-severity finding.
+
+`defaults.ts` is not universally required for every plugin. It should be present when the plugin exposes execution settings or defines behavior that depends on `execute()` / `executeStream()` defaults, but its absence alone should not be reported as a MUST failure for plugins that do not use those defaults.
+
+> **Note:** The structural completeness check applies only to the `plugins/{PLUGIN_NAME}/` directory. Connector directories (`connectors/{PLUGIN_NAME}/`) serve a different architectural role and are read as supporting context for the best-practices review, not audited for structural completeness.
+
+## Step 5: Full 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).
+
+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:
+
+- **Severity**: NEVER, MUST, or SHOULD (from the guideline prefix)
+- **Category**: Which of the 9 categories
+- **Description**: What the guideline requires and how the plugin violates it
+- **Location**: Specific `file:line` reference(s)
+
+A category with no findings is a pass. A category with only SHOULD findings is a warn. A category with any MUST or NEVER finding is a fail.
+
+## Step 6: Produce Output
+
+### Scorecard Table (output first)
+
+```
+## Scorecard
+
+| # | Category | Status | Findings |
+|---|----------|--------|----------|
+| 0 | Structural Completeness | {status} | {count} |
+| 1 | Manifest Design | {status} | {count} |
+| 2 | Plugin Class Structure | {status} | {count} |
+| 3 | Route Design | {status} | {count} |
+| 4 | Interceptor Usage | {status} | {count} |
+| 5 | asUser / OBO Patterns | {status} | {count} |
+| 6 | Client Config | {status} | {count} |
+| 7 | SSE Streaming | {status} | {count} |
+| 8 | Testing Expectations | {status} | {count} |
+| 9 | Type Safety | {status} | {count} |
+```
+
+Where `{status}` is one of:
+- Pass — no findings
+- Warn — SHOULD-only findings
+- Fail — any NEVER or MUST findings
+- N/A — category does not apply to this plugin (e.g., SSE Streaming for a non-streaming plugin)
+
+And `{count}` is the number of findings (0 if pass).
+
+### Detailed Findings (output second, severity-first)
+
+Group all findings across all categories and sort by severity per the Severity Ordering rule in `plugin-review-guidance.md`.
+
+For each finding, output:
+
+```
+### [{severity}] {category}: {short description}
+
+**File:** `{file_path}:{line_number}`
+
+{Explanation of what the guideline requires, what the code does wrong, and how to fix it.}
+```
+
+If there are zero findings across all categories, output:
+
+> All checks passed. No findings.
+
+### Summary (output last)
+
+End with a one-line summary:
+
+> **Audit result: {total_findings} findings ({never_count} NEVER, {must_count} MUST, {should_count} SHOULD) across {failing_categories} failing and {warning_categories} warning categories.**

.claude/commands/create-core-plugin.md

@@ -2,6 +2,16 @@
 
 User input: $ARGUMENTS
 
+## 0. Load Best Practices Reference
+
+Before making any scaffolding decisions, read the plugin best-practices reference:
+
+```
+.claude/references/plugin-best-practices.md
+```
+
+This document defines NEVER/MUST/SHOULD guidelines for manifest design, plugin class structure, route design, interceptor usage, asUser/OBO patterns, client config, SSE streaming, testing, and type safety. Apply these guidelines throughout the scaffolding process — especially when deciding interceptor defaults, cache key scoping, OBO enforcement, and route registration patterns.
+
 ## 1. Gather Requirements
 
 The user may have provided a plugin name and/or description above in `$ARGUMENTS`. Parse what was given.
@@ -65,6 +75,8 @@ connectors/{name}/
 
 ## 4. Plugin Patterns to Follow
 
+> The scaffolding templates below implement the guidelines from `.claude/references/plugin-best-practices.md`. Refer to that document for the full NEVER/MUST/SHOULD rules and rationale behind each pattern.
+
 ### 4a. Config Interface (`types.ts`)
 
 Extend `BasePluginConfig` from `shared`:
@@ -166,7 +178,7 @@ const logger = createLogger("{name}");
 
 export class {PascalName}Plugin extends Plugin {
   name = "{name}";
-  static manifest = manifest as PluginManifest;
+  static manifest = manifest as PluginManifest<"{name}">;
   protected declare config: I{PascalName}Config;
 
   private connector: {PascalName}Connector;
@@ -320,15 +332,7 @@ export * from "./{name}";
 
 ## 6. Key Conventions
 
-### Route Registration
-Routes mount at `/api/{pluginName}/...`. Use `this.route(router, { name, method, path, handler })`.
-
-### User-Scoped Execution (OBO)
-`this.asUser(req)` returns a proxy where all method calls use the user's Databricks credentials. Use it in route handlers for user-scoped operations. Inside the proxied method, `getWorkspaceClient()` automatically returns the user-scoped client. **Important:** any plugin using `asUser()` must declare the appropriate `user_api_scopes` in the bundle config — see section 4f.
-
-### Execution Methods
-- `this.execute(fn, options)` — single request-response with interceptors (telemetry, timeout, retry, cache)
-- `this.executeStream(res, fn, options)` — SSE streaming with interceptors
+For full NEVER/MUST/SHOULD rules on routes, interceptors, OBO, streaming, and type safety, see `.claude/references/plugin-best-practices.md`.
 
 ### Context Utilities
 Import from `../../context`:
@@ -339,12 +343,8 @@ Import from `../../context`:
 ### Logging
 Use `createLogger("plugin-name")` from `../../logging/logger`.
 
-### Setup/Teardown
-- Override `setup()` for async init (e.g., connection pools). Called by AppKit during startup.
-- Override `abortActiveOperations()` for cleanup. Call `super.abortActiveOperations()` first.
-
-### Plugin Phases
-Set `static phase: PluginPhase` if needed: `"core"` (first), `"normal"` (default), `"deferred"` (last).
+### OBO Scopes Reminder
+Any plugin using `this.asUser(req)` must declare `user_api_scopes` in the bundle config — see section 4f.
 
 ## 7. After Scaffolding
 

.claude/commands/review-core-plugin.md

@@ -0,0 +1,125 @@
+---
+description: Review plugin changes against AppKit best practices (composes with review-pr)
+argument-hint: [plugin-name or base-branch]
+---
+
+# Review Core Plugin Changes
+
+User input: $ARGUMENTS
+
+## Step 0: Parse Input
+
+Parse `$ARGUMENTS` deterministically:
+
+- If `$ARGUMENTS` is empty:
+  - Use no plugin name filter.
+  - Use `origin/main` as the base branch.
+- Otherwise, check whether either of these paths exists:
+  - `packages/appkit/src/plugins/$ARGUMENTS`
+  - `packages/appkit/src/connectors/$ARGUMENTS`
+- If either path exists:
+  - Treat `$ARGUMENTS` as the **plugin name filter**.
+  - Use `origin/main` as the base branch.
+- Otherwise:
+  - Treat `$ARGUMENTS` as the **base branch**.
+  - Use no plugin name filter.
+
+Do not use a name-pattern heuristic such as "kebab-case with no slashes" to decide whether `$ARGUMENTS` is a plugin name, because common branch names like `feature-x` and `bugfix-foo` would be ambiguous.
+
+## Step 1: Core Principles Review
+
+First, invoke the `review-pr` skill to run the standard Core Principles review. Pass the base branch as the argument (not the plugin name).
+
+Use the Skill tool:
+- skill: `review-pr`
+- args: the base branch determined in Step 0 (or empty to use default)
+
+Wait for this review to complete before continuing.
+
+## Step 2: Diff Analysis
+
+Run `git diff <base-branch>...HEAD --name-only` to get all changed files.
+
+Filter the file list to plugin-relevant paths:
+- `packages/appkit/src/plugins/**`
+- `packages/appkit/src/connectors/**`
+
+If a specific plugin name was provided in Step 0, further filter to only files matching that plugin name in the path.
+
+**If no plugin files are found in the diff**, output:
+
+> No plugin files were changed in this branch. Plugin best-practices review is not applicable. Only the Core Principles review above applies.
+
+Then stop. Do not continue to subsequent steps.
+
+## Step 3: Multi-Plugin Detection
+
+If no specific plugin name was provided, detect all distinct plugins touched in the diff by extracting the plugin directory name from each changed path:
+- From `packages/appkit/src/plugins/{name}/...` extract `{name}`
+- From `packages/appkit/src/connectors/{name}/...` extract `{name}`
+
+Deduplicate the list. You will run Steps 4-6 for **each** detected plugin.
+
+## Step 4: Category Scoping
+
+For each plugin being reviewed, use the Category Index from `.claude/references/plugin-review-guidance.md` as the canonical list of categories. Map changed files to relevant categories using the "What to check" column as a guide — match file names and code patterns (e.g., `this.route(` → Route Design, `asUser(` → asUser / OBO Patterns). A single file may trigger multiple categories.
+
+Read the actual changed file contents with `git diff <base-branch>...HEAD -- <file>` to determine which patterns are present.
+
+Record which of the 9 categories are **relevant** (at least one changed file maps to them) and which are **skipped** (no changed files map to them).
+
+## Step 5: Load Best Practices Reference
+
+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 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 finding:
+- Identify the **severity** (NEVER, MUST, or SHOULD)
+- Identify the **category** (e.g., "Manifest Design", "Route Design")
+- Cite the specific guideline being violated or satisfied
+- Reference the exact file and line(s) involved
+- Provide a concrete fix if it is a violation
+
+## Step 7: Output
+
+### Format
+
+For each plugin reviewed, output a section with the plugin name as the heading.
+
+Order findings by severity per the Severity Ordering rule in `plugin-review-guidance.md`.
+
+Each finding should follow this format:
+
+```
+### [SEVERITY] Category Name: Brief description
+- **File:** `path/to/file.ts:L42`
+- **Guideline:** <quote the specific guideline>
+- **Finding:** <what the code does wrong or right>
+- **Fix:** <concrete fix, if a violation>
+```
+
+If a plugin has **no findings** (all scoped guidelines are satisfied), state that explicitly.
+
+### Skipped Categories
+
+At the end of the output (after all plugin reviews), list the categories that were **not relevant** to this diff:
+
+```
+### Skipped Categories (not relevant to this diff)
+- Category N: <Name> — no changed files matched this category
+- ...
+```
+
+### Summary
+
+End with an overall summary:
+- Total findings by severity (e.g., "0 NEVER, 2 MUST, 3 SHOULD")
+- Whether the changes are ready to merge from a plugin best-practices perspective
+- Any categories that deserve attention even though they were skipped (e.g., "No tests were changed — consider adding tests for the new route")