You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Retrofits the review-toolkit plugin's ecosystem-specialist agent to consume the marketplace-wide ecosystem-commands contract (docs/conventions/ecosystem-commands/README.md) instead of carrying its own inline command defaults.
Changes
agents/ecosystem-specialist.md — new Command-truth resolution ladder: (1) consumer .claude/ecosystems/<ecosystem>.yaml (authoritative for build-cmd/test-cmd/check-cmd, globs, install-hint, with user-global + .local. overlay layering); (2) the project's documented conventions; (3) the agent's generic defaults, demoted to explicit last-resort ("when neither exists"). Ecosystem detection may use the contract's globs when config exists. MISSING-tool handling ties to the ecosystem file's install-hint when present. Report format, MISSING handling, and detection behavior otherwise unchanged — only the command-truth sourcing moved.
README.md — new "Command truth from .claude/ecosystems/" bullet citing the contract by path.
CHANGELOG.md — added (new file); [0.2.0] entry.
plugin.json — version 0.1.0 → 0.2.0 (minor).
Pre-baked decisions honored (from #1390)
Config surface is concern-named .claude/ecosystems/ — no reference to the implementation plugin's paths.
No userConfig path knob added; location is conventional per the contract.
Verification
scripts/validate-plugins.sh → all manifests + catalog pass.
markdownlint-cli2 on the three changed markdown files → 0 errors.
Refs melodic-software/medley#1421
Note
Low Risk
Changes are limited to plugin prompts, README, changelog, and version metadata—no executable code paths or auth/data handling.
Overview
Bumps review-toolkit to 0.2.0 and documents the change in a new CHANGELOG and README.
ecosystem-specialist now follows the marketplace ecosystem-commands contract (docs/conventions/ecosystem-commands/README.md) for build/test/lint truth instead of treating inline defaults as peers. A Command-truth resolution ladder applies: consumer .claude/ecosystems/<ecosystem>.yaml (with user-global and .local. overlays, per-key null/omit rules, and placeholder binding) → project docs/CI/scripts → generic defaults only as last resort. Ecosystem detection can override path heuristics with resolved globs and honor enabled: false. MISSING-tool reports prefer the file’s install-hint. The agent explicitly skips infer-and-persist / ask-user rungs because it stays read-only; report shape and overall behavior are otherwise unchanged.
Reviewed by Cursor Bugbot for commit 53bc956. Bugbot is set up for automated code reviews on this repo. Configure here.
…ds contract
Resolve each ecosystem's build/test/lint command truth from the consumer's
.claude/ecosystems/<ecosystem>.yaml files (authoritative when present) per the
marketplace-wide ecosystem-commands contract, then the project's documented
conventions, then the agent's bundled generic defaults as an explicit last
resort. Detection may use the contract's globs when config exists. Report
format, MISSING-tool handling, and detection behavior unchanged.
Cite the contract by path in the agent body and README; add CHANGELOG; bump
plugin.json to 0.2.0 (minor).
Co-Authored-By: Claude <noreply@anthropic.com>
Check alignment with CLAUDE.md and ecosystem-commands contract
Final summary
Overall: The direction is correct and the core contract wiring is sound. The agent's field names, three-rung priority ladder, and overlay-layering description all match the schema and contract README. There are a few correctness gaps worth addressing before merge.
Findings
1. null vs. absent command — semantic gap (correctness)
The schema explicitly allows build-cmd: null / test-cmd: null / check-cmd: null to mean "this ecosystem has no such phase". An absent key is different: it means the consumer didn't declare a command (fall-through to rung 2/3 is appropriate). The current instruction doesn't distinguish these two cases:
null → skip the phase entirely; do not fall through to project conventions or generic defaults
key absent → fall through per the resolution ladder
Without this rule the agent may execute a tool the consumer repo deliberately disabled, or surface a false positive about a missing tool. Fix this →
2. gates field silently ignored (correctness/coverage)
The ecosystem YAML schema has a gates array for additional CI-parity checks — lockfile drift, generated-artifact freshness, schema regeneration. The schema description says "Repo-specific CI-parity gates beyond plain build/test/lint." Because the agent only runs build/test/lint, it silently skips any declared gates. A consumer who adds a lockfile-drift gate to their .claude/ecosystems/dotnet.yaml will never see it run. The Verification workflow section should mention that after the three standard steps, any declared gates entries should also run. Fix this →
3. Step ordering creates a prerequisite circularity (clarity/correctness)
Resolve command truth (requires knowing which ecosystem YAML to open)
Identify the change set
Detect affected ecosystems (provides the input step 1 needs)
Steps 2 and 3 are prerequisites for step 1. Combined with the note in step 3 that YAML globs are authoritative for classifying changed files, this creates a two-pass requirement (detect by built-in heuristics → read YAMLs → re-classify by contract globs) that isn't described. Reordering to 2→3→1 and documenting the two-pass glob-reclassification would make this unambiguous. Fix this →
4. Resolution ladder omits contract rungs 2 and 3 without explanation
The contract's canonical resolution ladder has 4 rungs (YAML → infer-and-persist → ask-user → bundled defaults). The agent implements 3, collapsing rungs 2 and 3 into "project's documented conventions." This is almost certainly correct (the agent is read-only and can't write the inferred config back), but without a note it looks like an oversight and diverges from the contract's documented behavior. A brief (this agent is read-only — omits the contract's infer-and-persist and ask-user rungs) comment would record the intentional deviation. Fix this →
5. notes field not surfaced in reports (minor/coverage)
The schema defines a notes field: "Free-form caveats a resolver should surface (trap flags, ordering constraints)." The agent instruction doesn't mention reading or surfacing notes. A consumer who adds notes: "run dotnet build before tests or test discovery fails" will have that trap silently ignored.
The file declares it follows Keep a Changelog 1.1.0, which specifies ## [x.y.z] - YYYY-MM-DD. Both entries ([0.2.0] and [0.1.0]) are missing dates. Not a blocker, but the format claim is false until fixed.
…files when present — the marketplace-wide ecosystem-commands contract
(`docs/conventions/ecosystem-commands/README.md`) — then your documented conventions…
The contract reference is parenthetical clarification, but the double-em-dash structure makes it scan as a third fallback item. Using (…) for the clarifying aside and a single em-dash for the actual fallback transition would be clearer.
Non-issues / confirmed correct
Field names (build-cmd, test-cmd, check-cmd, install-hint, globs) all match the schema exactly.
Overlay order (user-global → team → .local.) matches the contract's documented resolution.
Version bump0.1.0 → 0.2.0 is correct semver for a behavioral change with no breaking interface.
markdownlint pass claimed in the PR description; no formatting issues visible in the diff.
Plugin-form safety — no hardcoded paths, no ../ reach-outs, no userConfig added (per the pre-baked decision in #1390).
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: low. Cursor Bugbot skipped (usage limit reached) and did not complete automated review, so I am not approving. Human review is needed before merge.
Sent by Cursor Approval Agent: Pull Request Router and Approver
…esolution
- Distinguish an explicit `null` command (skip the phase, no fall-through) from
an absent key (fall through the ladder).
- Reorder "Before running": identify change set → detect ecosystems (re-classify
by contract globs) → resolve command truth, removing the prerequisite
circularity.
- Record that this read-only agent intentionally omits the contract's
infer-and-persist and ask-user rungs.
- Reword the README bullet so the contract citation is a parenthetical aside,
not a second fallback rung.
Co-Authored-By: Claude <noreply@anthropic.com>
The schema defines enabled: boolean, default: true with the description: "False disables detection without deleting the file." Step 2 re-classifies changed files using contract globs for any ecosystem that has a YAML file — but never checks enabled. A consumer who sets enabled: false to temporarily suppress an ecosystem (e.g., a shell-heavy repo mid-migration that isn't ready for shellcheck) will still trigger detection and command resolution from that file, which is exactly the behavior enabled: false is meant to prevent.
Add a check: when the YAML file is present but enabled: false, skip that ecosystem entirely (no re-classification, no command resolution). Fix this →
2. Contract placeholders not bound before execution (correctness)
The contract README documents four placeholders that consuming surfaces are responsible for binding: <files> (ecosystem-scoped changed-file list), <solution-or-project-file> (resolved via anchor), <project-dir> (from project-discovery), and $REPO_ROOT (repo root). The instruction says to use commands "verbatim" but never tells the agent to substitute those placeholders before running. A valid consumer command like shellcheck <files> or dotnet build <solution-or-project-file> would be handed to the shell literally — <files> parses as input redirection, causing a crash or unexpected behavior even though the ecosystem config is correct.
The contract README is explicit that context binding is the consuming surface's concern (not the schema's), so the binding instructions belong here in the agent. Add a note in the command-truth resolution section explaining how to bind each placeholder before execution. Fix this →
The schema defines a gates array for CI-parity checks beyond build/test/lint (lockfile drift, generated-artifact freshness, schema regeneration). Each gate has name, cmd, optional trigger-globs, and remediation. The agent's Verification workflow only runs three steps; any declared gates are silently skipped. A consumer who adds gates to .claude/ecosystems/dotnet.yaml expecting CI-parity will never see them run. Add a step 4 in the Verification workflow for gates. Fix this →
The schema's notes field is "Free-form caveats a resolver should surface (trap flags, ordering constraints)." The agent never reads or reports it, so a consumer notes: "run dotnet build before tests or test discovery fails" is silently ignored. Mention that when a notes value is present, it should appear at the top of that ecosystem's section in the report. Small fix; low risk.
Both [0.2.0] and [0.1.0] entries are missing the YYYY-MM-DD date required by Keep a Changelog 1.1.0 (## [x.y.z] - YYYY-MM-DD). The file's own header claims to follow that format.
Read-only rung note — accurately records the intentional deviation from the contract's 4-rung ladder.
Field names (build-cmd, test-cmd, check-cmd, install-hint, globs) match the schema exactly.
Overlay order (user-global → team → .local.) matches the contract.
Version bump0.1.0 → 0.2.0 is correct semver.
Plugin-form safety — no hardcoded paths, no ../ reach-outs.
README bullet is clear and correctly scoped after the em-dash rewrite.
Overall: The 86f04e6 fixes are all correct. Issues 1 and 2 (above) are correctness gaps worth addressing before merge; 3–5 are lower priority but straightforward to fix.
chore: initialize marketplace scaffold #1null vs absent command — fixed. The skip rule now distinguishes an explicit null (skip the phase, no fall-through) from an absent key (fall through the ladder).
feat: markdown-formatter plugin (pilot migration) #3 step-ordering circularity — fixed. "Before running" reordered to identify change set → detect ecosystems (re-classify by contract globs) → resolve command truth.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: low. Cursor Bugbot skipped (usage limit reached) and did not complete automated review, so I am not approving. Human review is needed before merge; no non-author reviewers were assigned.
Sent by Cursor Approval Agent: Pull Request Router and Approver
…alse
- Instruct the agent to expand the contract placeholders (<files>,
<solution-or-project-file>, <project-dir>, $REPO_ROOT) before running a
configured command, instead of passing it to the shell literally.
- Drop any ecosystem whose config sets enabled:false during glob-based
detection, honoring the schema's disable-without-delete semantics.
Co-Authored-By: Claude <noreply@anthropic.com>
Read-only rung note — intentional omission of infer-and-persist and ask-user rungs is documented. (L26)
Field names (build-cmd, test-cmd, check-cmd, install-hint, globs) match the schema exactly.
Overlay order (user-global → team → .local.) matches the contract's documented resolution.
enabled: false — checked during detection (step 2), before resolution (step 3). No late-check gap.
Placeholder list — all four contract placeholders (<files>, <solution-or-project-file>, <project-dir>, $REPO_ROOT) are named with the shellcheck -x <files> expansion example illustrating why literal hand-off fails.
Version bump0.1.0 → 0.2.0 is correct semver for a behavioral change.
Plugin-form safety — no hardcoded paths, no ../ reach-outs, no userConfig added.
README bullet is clear and correctly scoped after the em-dash rewrite.
One minor note
Placeholder resolution methods not specified (L22)
The placeholder-binding note says "bind…to this run's values" but doesn't say how <solution-or-project-file> resolves (via the ecosystem file's anchor) or how <project-dir> resolves (via project-discovery). The contract README is cited in the same sentence, so a motivated agent will follow it — not a blocker, but explicitly naming the resolution methods would make the instruction self-contained. Easily addressed in a follow-up.
Deferred items — rationale accepted
gates and notes — the author's scope fence is coherent: those fields add new verification behavior (running CI-parity gates; surfacing trap caveats in the report) rather than changing command-truth sourcing. Expanding here would exceed the pre-baked scope and create a larger-than-intended diff. Both are worth a follow-up issue.
CHANGELOG dates — dateless format matches the existing sibling plugins/songwriting/CHANGELOG.md; repo convention takes precedence over the format declaration.
Overall: All correctness gaps identified across three review passes have been resolved. The PR is ready to merge. The one minor note above (placeholder resolution method specificity) and the two deferred items (gates, notes) are reasonable follow-up candidates.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: low. Cursor Bugbot skipped (usage limit reached) and did not complete automated review, so I am not approving. Human review is needed before merge; no non-author reviewers were assigned.
Sent by Cursor Approval Agent: Pull Request Router and Approver
An omitted build-cmd/test-cmd/check-cmd is undeclared (fall through the ladder
for that one command), distinct from an explicit null (skip the phase). A file
present only for classification no longer suppresses valid build/test/lint.
Consolidate the null semantics into the resolution ladder to avoid duplication.
Co-Authored-By: Claude <noreply@anthropic.com>
Verify alignment with ecosystem-commands contract + schema
Final summary
What 54b7271 fixed
The Codex finding was: a YAML file that only declares globs (for classification) but omits one or more command keys would be treated as the authoritative rung-1 source, suppressing valid fallthrough to project conventions or generic defaults. The fix adds an explicit three-way distinction:
Condition
Behavior
Key present + non-null
Authoritative — use verbatim (with placeholder binding)
Key set to null
Phase does not apply — skip, no fall-through
Key omitted
Undeclared at rung 1 — fall through to rung 2, then rung 3, for that one command
This is the correct semantics per the contract. A .claude/ecosystems/shell.yaml with only globs now correctly lets build-cmd, test-cmd, and check-cmd each fall through independently — it no longer suppresses valid verification just because the file exists for classification purposes.
Field names (build-cmd, test-cmd, check-cmd, install-hint, globs) match the schema exactly. Overlay order (user-global → team → .local.) matches the contract's documented resolution. Version bump 0.1.0 → 0.2.0 is correct semver. Plugin-form safety: no hardcoded paths, no ../ reach-outs, no userConfig added.
Deferred items — rationale accepted
gates and notes — these add new verification behavior (running CI-parity gates; surfacing trap caveats in the report) rather than changing command-truth sourcing, which was the defined scope. Reasonable follow-up candidates.
CHANGELOG dates — dateless format matches plugins/songwriting/CHANGELOG.md; repo convention takes precedence over the format declaration.
Overall: All correctness gaps identified across four review passes have been resolved. The PR is ready to merge.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: low. Cursor Bugbot skipped (usage limit reached) and did not complete automated review, so I am not approving. Human review is needed before merge; no non-author reviewers were assigned.
Sent by Cursor Approval Agent: Pull Request Router and Approver
…chain at detection
Detection now merges globs/enabled across user-global -> team -> .local. before
classifying, so a .local. overlay that disables an ecosystem or overrides its
globs takes effect at detection, not only at command resolution.
Co-Authored-By: Claude <noreply@anthropic.com>
Review commit 53bc956 — overlay resolution at detection
Verify alignment with ecosystem-commands contract + schema
Final summary
What 53bc956 fixed
The Codex finding was: detection read only the tracked team file to evaluate globs/enabled, so a user-global ~/.claude/ecosystems/<ecosystem>.yaml or a .local. overlay that sets enabled: false or overrides globs would be silently ignored at classification time — the agent could still detect and run an ecosystem the overlay deliberately disabled.
The fix integrates overlay resolution directly into step 2:
"for each ecosystem that has a consumer .claude/ecosystems/<ecosystem>.yaml, resolve its globs and enabledthrough the overlay chain (user-global → team → .local., key-by-key) and use the resolved globs to re-classify the changed files — authoritative over these built-in heuristics — dropping any ecosystem whose resolved enabled is false … even when its globs match."
The overlay order (user-global → team → .local.) matches the contract exactly. The enabled check runs on the resolved value, not the raw team-file value. ✅
Full five-commit review — confirmed correct
Concern
Status
Commit
Step ordering (identify → detect → resolve)
✅
86f04e6
null vs absent command distinction
✅
86f04e6
Read-only rung deviation documented
✅
86f04e6
README em-dash structure
✅
86f04e6
enabled: false honored before re-classification
✅
e0eff0d
Contract placeholders bound before execution
✅
e0eff0d
Per-command-key fallthrough for omitted keys
✅
54b7271
globs + enabled resolved through full overlay chain at detection
✅
53bc956
Field names (build-cmd, test-cmd, check-cmd, install-hint, globs) match the schema exactly. Overlay order (user-global → team → .local.) is consistent across step 2 (detection) and rung 1 (command resolution). Version bump0.1.0 → 0.2.0 is correct semver. Plugin-form safety: no hardcoded paths, no ../ reach-outs, no userConfig added.
Deferred items — rationale accepted
gates and notes — add new verification/report behavior beyond command-truth sourcing; clearly outside the pre-baked scope fence.
CHANGELOG dates — dateless format matches plugins/songwriting/CHANGELOG.md; repo convention takes precedence over the format declaration.
Placeholder resolution method specificity (minor) — the contract README is cited inline; the resolution methods for <solution-or-project-file> (via anchor) and <project-dir> (via project-discovery) are specified there. Not a correctness risk given the citation, and a reasonable follow-up candidate.
Overall: All correctness gaps identified across five review passes have been resolved. The PR is ready to merge.
The reason will be displayed to describe this comment to others. Learn more.
Risk: low. Cursor Bugbot skipped (usage limit reached) and did not complete automated review, so I am not approving. Human review is needed before merge; no non-author reviewers were assigned.
Sent by Cursor Approval Agent: Pull Request Router and Approver
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
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.
Retrofits the
review-toolkitplugin'secosystem-specialistagent to consume the marketplace-wide ecosystem-commands contract (docs/conventions/ecosystem-commands/README.md) instead of carrying its own inline command defaults.Changes
agents/ecosystem-specialist.md— new Command-truth resolution ladder: (1) consumer.claude/ecosystems/<ecosystem>.yaml(authoritative forbuild-cmd/test-cmd/check-cmd,globs,install-hint, with user-global +.local.overlay layering); (2) the project's documented conventions; (3) the agent's generic defaults, demoted to explicit last-resort ("when neither exists"). Ecosystem detection may use the contract'sglobswhen config exists. MISSING-tool handling ties to the ecosystem file'sinstall-hintwhen present. Report format, MISSING handling, and detection behavior otherwise unchanged — only the command-truth sourcing moved.README.md— new "Command truth from.claude/ecosystems/" bullet citing the contract by path.CHANGELOG.md— added (new file);[0.2.0]entry.plugin.json— version0.1.0→0.2.0(minor).Pre-baked decisions honored (from #1390)
.claude/ecosystems/— no reference to theimplementationplugin's paths.userConfigpath knob added; location is conventional per the contract.Verification
scripts/validate-plugins.sh→ all manifests + catalog pass.markdownlint-cli2on the three changed markdown files → 0 errors.Refs melodic-software/medley#1421
Note
Low Risk
Changes are limited to plugin prompts, README, changelog, and version metadata—no executable code paths or auth/data handling.
Overview
Bumps review-toolkit to 0.2.0 and documents the change in a new CHANGELOG and README.
ecosystem-specialistnow follows the marketplace ecosystem-commands contract (docs/conventions/ecosystem-commands/README.md) for build/test/lint truth instead of treating inline defaults as peers. A Command-truth resolution ladder applies: consumer.claude/ecosystems/<ecosystem>.yaml(with user-global and.local.overlays, per-keynull/omit rules, and placeholder binding) → project docs/CI/scripts → generic defaults only as last resort. Ecosystem detection can override path heuristics with resolvedglobsand honorenabled: false. MISSING-tool reports prefer the file’sinstall-hint. The agent explicitly skips infer-and-persist / ask-user rungs because it stays read-only; report shape and overall behavior are otherwise unchanged.Reviewed by Cursor Bugbot for commit 53bc956. Bugbot is set up for automated code reviews on this repo. Configure here.