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

[atilafassina]

• Adds 3 Claude Code skills (slash commands) for the AppKit plugin workflow:
  a. audit-core-plugin.md (new) — Full audit of a core plugin against all best-practices categories
  with a scorecard
  b. review-core-plugin.md (new) — Reviews plugin changes against AppKit best practices (composes with
  review-pr)
  c. create-core-plugin.md (modified) — Minor update to the existing plugin creation skill
• Adds a shared plugin-best-practices.md reference guide (181 lines) that the review and audit skills
  reference

to test, checkout this PR and try the slash commands

[Copilot]

Pull request overview

Adds internal reference material and Claude commands to standardize how AppKit core plugins are created, reviewed, and audited against a shared set of best practices.

Changes:

• Added a new plugin best-practices reference document with tiered (NEVER/MUST/SHOULD) guidelines across 9 categories.
• Added new Claude commands to (a) review plugin diffs against the best-practices doc and (b) fully audit a plugin with a scorecard.
• Updated the existing core-plugin creation command to load and apply the best-practices reference up front.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
.claude/references/plugin-best-practices.md	New reference guide defining best practices for manifests, routes, interceptors, OBO/asUser, client config, streaming, testing, and typing.
.claude/commands/review-core-plugin.md	New command to compose review-pr with a scoped best-practices review of plugin-related diffs.
.claude/commands/create-core-plugin.md	Updated to explicitly load the best-practices reference before scaffolding decisions.
.claude/commands/audit-core-plugin.md	New command to audit an entire plugin across all categories and output a scorecard + findings.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[pkosiec]

Overall looks good - probably we'll need to test it in action. Before merge, it would be great to address some improvement suggestions 👍

[pkosiec]

Generally I see a bit of duplication across those commands: create-core-plugin.md, review-pr.md, review-file.md / review-commit.md commands

IMO those guidance could be too much? E.g. see sql-connector vs analytics plugin, or lakebase V1 connector 🤔

Here's an output of my agentic review:

Code Review: PR #257 — Plugin Skills & Best Practices

PR: #257
Title: feat: skills for creating, reviewing, and auditing an AppKit plugin
Branch: build-pluing-skill → main
Date: 2026-04-10

---

Scope

4 files, ~474 lines added:

• .claude/commands/audit-core-plugin.md (new, 153 lines)
• .claude/commands/create-core-plugin.md (modified, +10 lines)
• .claude/commands/review-core-plugin.md (new, 140 lines)
• .claude/references/plugin-best-practices.md (new, 181 lines)

Intent: Add Claude Code skills (audit, review) and a shared best-practices reference to standardize how AppKit core plugins are created, reviewed, and audited. Minor update to existing create-core-plugin skill.

Reviewers: correctness, maintainability, project-standards, adversarial, previous-comments, agent-native, testing

---

Prior Review Comments (Copilot)

All 3 prior Copilot suggestions have been addressed:

1. Route prefix trailing slash (/api/{name}/ → /api/{name}) — fixed in current code
2. Deterministic input parsing (check directory existence instead of kebab-case heuristic) — implemented
3. defaults.ts downgraded from MUST to SHOULD — done, with explanation

---

Findings

P1 — High

1. as vs satisfies mismatch with official docs

• File: .claude/references/plugin-best-practices.md:35
• Reviewers: correctness, project-standards
• Confidence: 0.95

Best-practices says MUST use:

static manifest = manifest as PluginManifest<"my-plugin">;

But the official docs (docs/docs/plugins/custom-plugins.md:44) show:

static manifest = { ... } satisfies PluginManifest<"myPlugin">;

satisfies is stricter — it preserves literal types without widening and catches structural errors at compile time. as is a type assertion that suppresses errors. The best-practices is recommending a weaker pattern than the official docs.

Note: Core plugins import manifest.json and cast it (since JSON imports can't use satisfies), so as is correct for that specific pattern. But the best-practices doesn't explain this nuance — someone reading it in isolation would think as is universally preferred.

Fix: Add a note explaining that as is used for JSON imports specifically, and that inline manifests should use satisfies per the official docs.

---

2. Conflicting severity for PluginManifest generic parameter

• File: .claude/references/plugin-best-practices.md:35,177
• Reviewer: project-standards
• Confidence: 0.95

Three sources give three different rules:

Source	Rule
Section 2 (Plugin Class Structure)	MUST use PluginManifest<"name">
Section 9 (Type Safety)	SHOULD use PluginManifest<"name">
create-core-plugin.md:179 template	Uses PluginManifest (no generic)

Fix: Align all three. If the generic is mandatory for core plugins, make it MUST everywhere and update the create template. If optional, make it SHOULD everywhere.

---

3. Kebab-case MUST contradicts official docs examples

• File: .claude/references/plugin-best-practices.md:14
• Reviewer: correctness
• Confidence: 0.85

Best-practices says:

MUST use lowercase kebab-case for name (pattern: ^[a-z][a-z0-9-]*$)

Official docs (custom-plugins.md:25,81) use camelCase: "myPlugin".

Core plugins do use kebab-case (analytics, genie, files). But the official docs example creates confusion for plugin authors who read both sources.

Fix: Clarify scope — this is a core-plugin monorepo convention. Custom plugins may use camelCase per the official docs. Or update the official docs to align (preferred if kebab-case is truly required by the route system).

---

4. Structural completeness check doesn't handle connector-only plugins

• File: .claude/commands/audit-core-plugin.md:54
• Reviewers: correctness, adversarial
• Confidence: 0.92

Step 1 accepts connector-only plugins (where plugins/{PLUGIN_NAME}/ doesn't exist), but Step 4 only checks files in plugins/{PLUGIN_NAME}/. No guidance on what to do when that directory doesn't exist (e.g., sql-warehouse connector).

Fix: Add explicit handling: "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."

---

P2 — Moderate

5. Significant content duplication between best-practices and create-core-plugin

• Files: .claude/references/plugin-best-practices.md ↔ .claude/commands/create-core-plugin.md
• Reviewer: maintainability
• Confidence: 0.88

The best-practices reference (181 lines) largely restates patterns already documented in create-core-plugin.md sections 4a–4g:

Topic	create-core-plugin section	best-practices section
Manifest design	4e	Section 1
Plugin class structure	4d	Section 2
Route design	4d (injectRoutes)	Section 3
Interceptor usage	4b	Section 4
OBO / asUser	4f	Section 5
Client config	—	Section 6
SSE streaming	4b (stream defaults)	Section 7
Testing	—	Section 8
Type safety	4a, 4d	Section 9

When patterns change, both files must be updated independently.

Fix: Make plugin-best-practices.md the single source of truth for rules. Refactor create-core-plugin.md to reference it for guidelines and keep only the scaffolding templates and step-by-step workflow.

---

6. Cache-key tracing logic duplicated across skills

• Files: .claude/commands/audit-core-plugin.md:82 ↔ .claude/commands/review-core-plugin.md:97
• Reviewer: maintainability
• Confidence: 0.85

Both skills contain near-identical instructions for tracing cacheKey through the two-stage pattern (where defaults.ts defines partial config and cacheKey is injected at the call site).

Fix: Move the cache-key tracing guidance into plugin-best-practices.md Section 4 as a note, and reference it from both skills.

---

7. Rules too strict for external plugin authors

• File: .claude/references/plugin-best-practices.md (various)
• Reviewer: adversarial
• Confidence: 0.80

Several MUST rules apply specifically to core plugins in the monorepo but would be unreasonable for custom plugins:

Rule	Core plugin	Custom plugin (per official docs)
Separate manifest.json file	Yes	No — inline satisfies shown
Separate defaults.ts file	Yes	Not documented
tests/ co-located in plugin dir	Yes	No convention documented
toPlugin() factory export	Yes	Yes — docs agree
this.route() for registration	Yes	Not documented for custom

The official docs (custom-plugins.md) show inline manifests, simpler patterns, and no NEVER/MUST/SHOULD tiers.

Fix: Scope the document explicitly: rename or add subtitle "Core Plugin Best Practices (monorepo conventions)" and add a note that custom plugins have more flexibility per the official documentation.

---

8. Category scoping maps executeStream only to SSE Streaming

• File: .claude/commands/review-core-plugin.md:72
• Reviewer: adversarial
• Confidence: 0.75

Step 4's mapping table routes executeStream patterns to Category 7 (SSE Streaming) but not to Category 4 (Interceptor Usage). A file calling executeStream without cacheKey is an Interceptor Usage violation but would only be scoped to SSE Streaming.

Fix: Add execute( and executeStream( to the Category 4 (Interceptor Usage) row in the mapping table.

---

9. Hardcoded 9-category list with no sync mechanism

• Files: .claude/commands/audit-core-plugin.md ↔ .claude/commands/review-core-plugin.md ↔ .claude/references/plugin-best-practices.md
• Reviewer: maintainability
• Confidence: 0.72

Both audit and review commands enumerate the same 9 categories separately. If a category is added to best-practices, both must be manually updated. No mechanism prevents drift.

Fix: Add a category index at the top of plugin-best-practices.md that audit/review can reference as the source of truth.

---

10. Missing deduplication guidance in review skill

• File: .claude/commands/review-core-plugin.md
• Reviewer: maintainability
• Confidence: 0.78

audit-core-plugin.md has explicit deduplication guidance (Step 5: "If the same code issue is covered by guidelines in multiple categories, report it once under the most specific category"). review-core-plugin.md has none — the same issue could be reported multiple times.

Fix: Add deduplication guidance to review-core-plugin.md Step 6.

---

P3 — Low

11. Plugin name / branch name collision edge case

• File: .claude/commands/review-core-plugin.md:17
• Reviewer: adversarial
• Confidence: 0.62

If a branch is named after an existing plugin (e.g., analytics), the deterministic check will interpret it as a plugin name filter instead of a branch. The skill should output a disambiguation notice when this occurs.

---

12. JSON.stringify(params) in cacheKey example is fragile

• File: .claude/references/plugin-best-practices.md:88
• Reviewer: adversarial
• Confidence: 0.68

Key ordering in JSON.stringify is not guaranteed for all cases. Semantically equivalent objects can produce different cache keys. Advisory: note that deterministic serialization is preferred.

---

13. N/A status definition is vague

• File: .claude/commands/audit-core-plugin.md:89
• Reviewer: adversarial
• Confidence: 0.72

"Does not apply to this plugin" doesn't specify the criterion. Should be operationalized: "N/A if zero files in the plugin contain patterns matching that category's detection rules."

---

Duplication Analysis

Within this PR

Duplicated content	Location A	Location B	Recommendation
Plugin patterns (manifest, class, routes, etc.)	plugin-best-practices.md (Sections 1–9)	create-core-plugin.md (Sections 4a–4g)	Make best-practices the single source; create-core-plugin references it
Cache-key tracing instructions	audit-core-plugin.md:82	review-core-plugin.md:97	Move to plugin-best-practices.md Section 4
9-category enumeration	audit-core-plugin.md:77–87	review-core-plugin.md:65–78	Reference category index in best-practices

With existing commands

Existing command	Overlap with new skills	Verdict
review-pr.md	review-core-plugin.md composes with it (good — no duplication)	Clean
review-commit.md	7 Core Principles duplicated verbatim with review-pr.md	Pre-existing issue, not introduced by this PR
review-file.md	Different concern (JSDoc + code review) — no overlap	Clean
create-core-plugin.md	Sections 4a–4g overlap heavily with plugin-best-practices.md	Needs refactoring (see P2 #5)

---

Strictness vs Official Docs

The best-practices reference was compared against the official AppKit documentation at docs/docs/plugins/custom-plugins.md and docs/docs/development/llm-guide.md.

Best-practices rule	Official docs	Alignment
MUST use kebab-case for name	Examples use camelCase ("myPlugin")	Misaligned
MUST use as PluginManifest<"name">	Shows satisfies PluginManifest<"name">	Misaligned (satisfies is stricter)
MUST use separate manifest.json	Shows inline manifest object	Misaligned (core vs custom plugin difference)
MUST use toPlugin() factory	Shows same pattern	Aligned
MUST register routes via this.route()	Shows injectRoutes() as extension point	Aligned
MUST use execute()/executeStream()	Shows as "Execution interceptors" extension point	Aligned
SHOULD use PluginManifest<"name"> generic	Shows satisfies PluginManifest<"myPlugin">	Aligned (both recommend it)

Recommendation: The document should explicitly state it covers core plugin conventions for the monorepo, not universal AppKit plugin rules. External developers following the official docs should not be told they're violating MUSTs.

---

Verdict: Ready with fixes

The content is well-structured and the Copilot feedback was addressed. The main issues are:

1. Correctness (P1): Fix as vs satisfies discrepancy, resolve MUST/SHOULD conflict, handle connector-only plugins
2. Scope (P2): Explicitly scope to core plugins; align with or acknowledge differences from official docs
3. Duplication (P2): Reduce overlap between best-practices reference and create-core-plugin sections 4a–4g
4. Completeness (P2): Add executeStream to Interceptor Usage category scoping, add deduplication guidance to review skill

Suggested fix order: P1 #1–2 (correctness) → P1 #4 (completeness) → P2 #5,7 (scope/duplication) → P2 #6,8,10 (consistency)

[pkosiec]

An idea for future: maybe it would be worth to convert those skills to a "Best practices" page in our docs? That would be awesome 👍