feat: skills for creating, reviewing, and auditing an AppKit plugin#257
Open
atilafassina wants to merge 8 commits intomainfrom
Open
feat: skills for creating, reviewing, and auditing an AppKit plugin#257atilafassina wants to merge 8 commits intomainfrom
atilafassina wants to merge 8 commits intomainfrom
Conversation
Extracts patterns from the 5 core plugins (analytics, genie, files, lakebase, server) into a structured reference with 9 sections and three severity tiers (NEVER/MUST/SHOULD). Signed-off-by: Atila Fassina <atila@fassina.eu>
Adds a "Load Best Practices Reference" step before scaffolding decisions so guidelines are applied during plugin creation. Signed-off-by: Atila Fassina <atila@fassina.eu>
Fixes 3 issues found during dry-run validation against analytics and files plugins: - Plugin extends Plugin (not Plugin<TConfig>) - @internal goes on toPlugin export, not the class - isInUserContext() patterns clarified per actual usage Signed-off-by: Atila Fassina <atila@fassina.eu>
Signed-off-by: Atila Fassina <atila@fassina.eu>
- Add cacheKey two-stage pattern guidance to prevent false positives - Add N/A status option for non-applicable scorecard categories - Clarify connector scope in structural completeness check - Add deduplication guidance for cross-category findings - Add recovery hint to plugin-not-found error message Signed-off-by: Atila Fassina <atila@fassina.eu>
Contributor
There was a problem hiding this comment.
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.
- Remove trailing slash from route prefix example in best-practices - Use deterministic directory-existence check instead of name-pattern heuristic for plugin vs branch disambiguation in review-core-plugin - Downgrade defaults.ts from MUST to SHOULD in audit-core-plugin since not all plugins (e.g. server, lakebase) require it Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
pkosiec
reviewed
Apr 10, 2026
Member
pkosiec
left a comment
pkosiec
left a comment
There was a problem hiding this comment.
Overall looks good - probably we'll need to test it in action. Before merge, it would be great to address some improvement suggestions 👍
Member
There was a problem hiding this comment.
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 👍
Move category index, severity ordering, deduplication rules, and cache-key tracing instructions into a shared reference file so audit and review skills stay in sync from a single source of truth. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
pkosiec
approved these changes
Apr 13, 2026
Comment on lines
+99
to
+108
| | 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} | |
Member
There was a problem hiding this comment.
Is it correct numbering? Looking at the plugin-review-guidance I can see only 1-9 points 🤔
|
|
||
| **SHOULD** set `hidden: true` on infrastructure plugins (like `server`) that should not appear in the template manifest. | ||
|
|
||
| **MUST** use `getResourceRequirements(config)` for resources that depend on runtime config. Declare them as `optional` in the static manifest, then return them as `required: true` from the static method. See `FilesPlugin.getResourceRequirements` for the canonical pattern. |
Member
There was a problem hiding this comment.
(Ran the skill against the serving pluign)
Not sure if this is true - as in case of Model Serving plugin, you can define multiple endpoints in the config, but at least one is required. Why? to support the apps init flow.
It will change in future (I'll work on updating the plugin manifest to support multiple resources (at least 1)) but for now I think the comment is not applicable 🤔
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
reference
to test, checkout this PR and try the slash commands