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

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

@@ -0,0 +1,151 @@
+---
+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
+
+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` | MUST |
+| `index.ts` | MUST |
+| `tests/` directory with at least one `.test.ts` file | MUST |
+
+Each missing file is a **MUST**-severity finding under the "Structural Completeness" category.
+
+> **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
+
+> **Deduplication:** If the same code issue is covered by guidelines in multiple categories, report it once under the most specific category and note that it also relates to the other category. Do not count it as separate findings in each category.
+
+Evaluate the plugin code against **all 9 categories** from the best-practices reference:
+
+1. **Manifest Design** — Check `manifest.json` against all NEVER/MUST/SHOULD rules
+2. **Plugin Class Structure** — Check the main plugin class against all NEVER/MUST/SHOULD rules
+3. **Route Design** — Check `injectRoutes()` and route handlers against all NEVER/MUST/SHOULD rules
+4. **Interceptor Usage** — Check `execute()`/`executeStream()` calls and `defaults.ts` against all NEVER/MUST/SHOULD rules. **Important:** When evaluating cache configuration in `defaults.ts`, the codebase uses a two-stage pattern where `defaults.ts` defines partial cache config (e.g., `enabled: true, ttl: 3600`) and the `cacheKey` is injected at the call site in the plugin class. Before flagging a NEVER violation for missing `cacheKey`, trace the cache config through to `execute()`/`executeStream()` call sites to verify whether a `cacheKey` is provided there. Only flag as NEVER if no `cacheKey` is provided at any point in the execution path.
+5. **asUser / OBO Patterns** — Check OBO usage, cache key scoping, context enforcement against all NEVER/MUST/SHOULD rules
+6. **Client Config** — Check `clientConfig()` return value against all NEVER/MUST/SHOULD rules
+7. **SSE Streaming** — Check streaming usage and shutdown behavior against all NEVER/MUST/SHOULD rules
+8. **Testing Expectations** — Check test files for coverage, mocking patterns, error path tests against all NEVER/MUST/SHOULD rules
+9. **Type Safety** — Check types, exports, config interface, `any` usage against all NEVER/MUST/SHOULD rules
+
+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:
+
+1. **NEVER** findings first (most critical)
+2. **MUST** findings second
+3. **SHOULD** findings last
+
+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.

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

@@ -0,0 +1,129 @@
+---
+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` to determine:
+
+- **Plugin name filter** — if `$ARGUMENTS` looks like a plugin name (kebab-case word, no slashes, not a branch name), use it to filter the diff to that specific plugin.
+- **Base branch** — if `$ARGUMENTS` looks like a branch name (contains `/` or matches known branch patterns like `main`, `develop`, `origin/*`), use it as the base branch. Otherwise default to `origin/main`.
+- If `$ARGUMENTS` is empty, review all plugins found in the diff against `origin/main`.
+
+## 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, map the changed files to the relevant best-practices categories:
+
+| Changed file pattern | Category |
+|---|---|
+| `manifest.json` | 1. Manifest Design |
+| Main plugin class file (the primary `.ts` file, not index/types/defaults) | 2. Plugin Class Structure |
+| Main plugin class file (the primary `.ts` file, not index/types/defaults) | 9. Type Safety |
+| Any file containing route registration (`this.route(`, `injectRoutes`) | 3. Route Design |
+| `defaults.ts` | 4. Interceptor Usage |
+| Any file containing `asUser(`, `.obo.sql`, `isInUserContext`, `runInUserContext` | 5. asUser / OBO Patterns |
+| Any file containing `clientConfig()` | 6. Client Config |
+| Any file containing `executeStream(`, `streamManager`, `AsyncGenerator`, SSE-related code | 7. SSE Streaming |
+| Files under `tests/` or files ending in `.test.ts` | 8. Testing Expectations |
+| `types.ts`, config interfaces, `exports()` return types | 9. Type Safety |
+
+Read the actual changed file contents with `git diff <base-branch>...HEAD -- <file>` to determine which patterns are present. A single file may trigger multiple categories.
+
+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
+
+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. **Important:** For Interceptor Usage findings (Category 4), when evaluating cache configuration, trace `cacheKey` through to `execute()`/`executeStream()` call sites before flagging. The codebase commonly defines partial cache config in `defaults.ts` and injects `cacheKey` at the call site. Only flag as NEVER if no `cacheKey` is provided anywhere in the execution path.
+- 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, not by category:**
+
+1. **NEVER violations** (most critical) — these are security or breakage blockers
+2. **MUST violations** — these are correctness requirements
+3. **SHOULD violations** — these are quality recommendations
+
+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")