feat: support Amp code review checks (.agents/checks/*.md)#2295
Merged
Conversation
Add a new first-class 'checks' feature axis. Checks are per-check code review instructions that Amp runs as one subagent per check during code_review. - Canonical source: .rulesync/checks/<name>.md with frontmatter targets / description / severity (low|medium|high|critical) / tools; unknown keys are preserved for round-trip. - Amp output: .agents/checks/<name>.md (project) and ~/.config/amp/checks/<name>.md (global). The Amp-required 'name' field is derived from the source file basename, and 'severity' maps to Amp's 'severity-default'. - Import lifts Amp checks back into .rulesync/checks/, mapping severity-default to severity and dropping the re-derivable name. - Wire the new feature through the registry, generate/import/convert, MCP server tools, gitignore derivation, fetch paths, and the generated README / supported-tools matrix. - Subtree-scoped checks (api/.agents/checks/) are documented as out of scope for v1. Closes #2206 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Merge the tool-scoped 'amp:' frontmatter section with precedence over canonical values (per feature-change-guidelines), exclude all tool-target sections from the passthrough so they no longer leak into generated check files, and keep 'name' pinned to the file basename. - Reuse AMP_GLOBAL_DIR for AMP_CHECKS_GLOBAL_DIR. - Add checksCount to the result-shape assertions in the MCP tests and add checks feature suites to lib/import and lib/convert tests. - Update the DIR_FEATURES comment to mention checks. - Document the tool-scoped section behavior for checks. The reviewer's note about 'does not support the feature checks' warning noise for wildcard features is intentionally unchanged: it is the established warnUnsupportedTargets pattern shared by every feature core, and changing warning policy is a cross-feature UX decision out of scope here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Imported Amp check keys that collide with a rulesync tool-target name are treated as tool-scoped sections and are not re-emitted, so the round-trip claim needed the exception spelled out. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner
Author
|
@dyoshikawa Thank you! |
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
2 participants
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.
Summary
Adds a new first-class
checksfeature axis that generates and imports Amp's code review checks — Markdown files with YAML frontmatter that Amp runs as one subagent per check duringcode_review(announcement, manual "Checks" section).This implements the design proposal recorded on the issue: a new feature target (not a sub-surface of rules/subagents/skills), driven by a new
.rulesync/checks/*.mdsource, with both project and global scope (the issue's original "project scope only" premise was corrected in the comments — Amp also reads~/.config/amp/checks/).Design
.rulesync/checks/<name>.md: frontmattertargets(standard targeting),description(optional),severity(optional strict enumlow | medium | high | critical, kept generically named so future tools can map their own severity naming),tools(optional string array). Schema isz.looseObject, so unknown keys round-trip..agents/checks/<name>.md, global~/.config/amp/checks/<name>.md. The Amp-requirednamefield is derived from the source file basename;severitymaps to Amp'sseverity-default..agents/checks/*.md(and the global dir) back into.rulesync/checks/, mappingseverity-default→severityand droppingname(re-derived on generate, so a divergent value cannot survive a round-trip anyway).api/.agents/checks/) — rulesync sources carry no directory-placement semantics (same reasoning as Hermes nested context files in Follow up Hermes Agent upstream updates: nested subdirectory context files (progressive discovery) #2214). Toolboxes (deprecated upstream) and the.amp/plugins/*.tsplugin API (Follow up Amp upstream updates: Plugin API (hooks via .amp/plugins/*.ts) #1938) are untouched.Changes
src/features/checks/:RulesyncCheck,ToolCheck,AmpCheck,ChecksProcessor(+ co-located tests), modeled on the subagents/skills feature classes.ALL_FEATURES,checksProcessorToolTargetTuple = ["amp"],PROCESSOR_REGISTRY, generate/import/convert cores,NON_SHARED_WRITE_FEATURES, fetchFEATURE_PATHS, MCP server tools (src/mcp/checks.ts), gitignore derivation (**/.agents/checks/added bypnpm dev gitignore).checksCountadded to the sharedCountableResultand every generate/import/convert result surface so check files are included in totals..rulesync/checks/*.mdsection indocs/reference/file-formats.md(frontmatter mapping table + v1 subtree limitation); hand-enumerated feature lists indocs/reference/cli-commands.md; README /docs/reference/supported-tools.mdmatrices regenerated viapnpm run generate:tables;skills/rulesync/synced.src/e2e/e2e-checks.spec.ts— project generate, import round-trip, global generate, andassertGenerateMatrixCoversTargetscoverage for both scopes.Verification
pnpm cicheck— pass (typecheck, lint, 7161 unit tests, content checks).npx vitest run --config vitest.e2e.config.ts src/e2e/e2e-checks.spec.ts— 5/5 pass.Closes #2206
🤖 Generated with Claude Code