Uh oh!
There was an error while loading. Please reload this page.
feat: skills for creating, reviewing, and auditing an AppKit plugin - #257
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>
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.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- 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
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 👍
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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>
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Split the rule into config-gated flip (optional→required) and dynamic discovery (required placeholder + dynamic emission) variants. Reference the custom-plugins.md caching example for the flip pattern and FilesPlugin/ServingPlugin for the discovery pattern. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
Add a note that category 0 (Structural Completeness) is a pre-check with no matching section in plugin-best-practices.md, while 1–9 mirror the best-practices sections. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
Uh oh!
There was an error while loading. Please reload this page.
…257) * docs: add plugin best-practices reference guide 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> * docs: integrate best-practices reference into create-core-plugin skill 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> * fix: correct inaccuracies in plugin best-practices reference 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> * docs: add review-core-plugin and audit-core-plugin skills Signed-off-by: Atila Fassina <atila@fassina.eu> * fix: address dry-run findings in audit and review skills - 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> * fix: address review feedback on plugin skills - 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> * chore: address code review * refactor: extract shared plugin review guidance to reduce duplication 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> * docs: clarify getResourceRequirements patterns in plugin best practices Split the rule into config-gated flip (optional→required) and dynamic discovery (required placeholder + dynamic emission) variants. Reference the custom-plugins.md caching example for the flip pattern and FilesPlugin/ServingPlugin for the discovery pattern. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * docs: clarify scorecard numbering in audit-core-plugin Add a note that category 0 (Structural Completeness) is a pre-check with no matching section in plugin-best-practices.md, while 1–9 mirror the best-practices sections. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> --------- Signed-off-by: Atila Fassina <atila@fassina.eu>
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