Skip to content

chore(discovery): add re-runnable setup skill for notes_dir seam#111

Merged
kyle-sexton merged 1 commit into
mainfrom
chore/retrofit-discovery
Jul 12, 2026
Merged

chore(discovery): add re-runnable setup skill for notes_dir seam#111
kyle-sexton merged 1 commit into
mainfrom
chore/retrofit-discovery

Conversation

@melodic-ai

@melodic-ai melodic-ai Bot commented Jul 12, 2026

Copy link
Copy Markdown

What

Adds an idempotent setup skill to the discovery plugin, closing the extensibility-contract gap flagged by retrofit-audit: discovery exposed a notes_dir userConfig seam but shipped no setup/configure action.

Changes

  • plugins/discovery/skills/setup/SKILL.md — new re-runnable setup skill, modeled on knowledge/setup. Convention-resolution ladder (config present → use; absent → infer from repo layout & persist; else safe default .claude/notes), narrow secret-aware .claude/settings.local.json reads (single-key jq), Local > Project > User effective precedence, project-scope persistence at pluginConfigs["discovery@melodic-software"].options.notes_dir.
  • plugins/discovery/.claude-plugin/plugin.json — version 0.1.00.2.0 (minor).
  • plugins/discovery/README.md — Configuration section references /discovery:setup (axis-titled skills table left intact — setup is a config action, not a discovery skill).

Verification

  • claude plugin validate plugins/discovery → passed.
  • Frontmatter (user-invocable: true, disable-model-invocation: false) consistent with shipped setup exemplars.

Refs melodic-software/medley#1429

cursor[bot]
cursor Bot approved these changes Jul 12, 2026
@kyle-sexton
kyle-sexton force-pushed the chore/retrofit-discovery branch from fc7700a to 64a6241 Compare July 12, 2026 18:35
@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 28s —— View job


PR Review

  • Load tools
  • Read changed files
  • Review for correctness, security, and CLAUDE.md alignment
  • Post findings

Summary

No blockers. This is a well-structured, security-aware, idempotent setup skill that correctly closes the extensibility-contract gap. Three files changed: new SKILL.md, version bump, and a one-line README addition. All three are correct.


Correctness

Plugin key alignmentSKILL.md L15–16: pluginConfigs["discovery@melodic-software"].options.notes_dir matches the plugin.json name: "discovery" + author field. ${user_config.notes_dir} substitution is consistent with the userConfig declaration. ✓

Default valueSKILL.md L44: The fallback .claude/notes matches plugin.json's userConfig.notes_dir.default. ✓

Type annotation in step 4SKILL.md L52: Correctly calls this a string option (not directory, as in the knowledge exemplar, where library_dir is typed directory). ✓

Version bump0.1.0 → 0.2.0 is the right semver level; adding a new skill is a backward-compatible feature addition, not a patch. ✓

Precedence orderSKILL.md L29: Local > Project > User is stated correctly. ✓


Security

Narrow jq readSKILL.md L33–36: The instruction to query only the single notes_dir key and never load settings.local.json wholesale is the correct mitigation for secret leakage (that file can carry API tokens, env secrets). This pattern matches the knowledge/setup exemplar. ✓

Local-override shadow warningSKILL.md L31–33: Explicitly calling out that a project-scope write stays shadowed by a local override on the developer's machine is important — avoids a support footgun. ✓

No egress, no remote MCP, no code execution risks.


CLAUDE.md alignment

Rule Status
Repo-agnostic — no hardcoded paths ✓ reads consumer's settings.json, no hardcoded repo values
Configurable without editing the plugin ✓ uses userConfig
Plugin-form-safe — no ../ reach-outs ✓ persists to consumer's .claude/settings.json, not plugin dir
No PII/secrets ✓ guards against reading secrets from local overlay
Versioned ✓ 0.2.0

Minor observation (no action required)

SKILL.md L6 adds disable-model-invocation: false explicitly; the knowledge/setup exemplar omits it (the default is false). Both behave identically — this is just an inconsistency between the two setup skills, not a correctness issue. Worth back-filling knowledge/setup for consistency, or removing the explicit key here — either is fine, but pick one convention.


Overall: Clean implementation. The idempotency design (read first → offer update, not blind overwrite), the inference ladder, and the "What this skill does NOT do" scope guard are all well-executed. Ready to merge.

cursor[bot]
cursor Bot approved these changes Jul 12, 2026
@kyle-sexton
kyle-sexton force-pushed the chore/retrofit-discovery branch from 64a6241 to a0b5edf Compare July 12, 2026 20:08
@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 22s —— View job


PR Review

  • Load tools
  • Read changed files
  • Review for correctness, security, and CLAUDE.md alignment
  • Post findings

No blockers. Three files changed — new SKILL.md, version bump, one-line README update. All correct.


Correctness

Plugin key alignmentSKILL.md L15–16: pluginConfigs["discovery@melodic-software"].options.notes_dir correctly derives from plugin.json name: "discovery" + author.name: "Melodic Software". ${user_config.notes_dir} substitution matches the userConfig declaration. ✓

Default valueSKILL.md L44: Fallback .claude/notes matches plugin.json's userConfig.notes_dir.default. ✓

Type annotationSKILL.md L52: Correctly calls this a string option (vs directory in knowledge/setup, where library_dir is typed directory). ✓

Version bump0.1.0 → 0.2.0 is the right semver level for a backward-compatible feature addition. ✓

Precedence orderSKILL.md L29: Local > Project > User is correct per Claude Code settings resolution. ✓

Idempotency design — Read-first / offer-update pattern on re-run is sound; the description's "Re-runnable — safe to invoke again to reconfigure" is backed by the instruction text. ✓


Security

Narrow jq readSKILL.md L33–36: Querying only the single notes_dir key and never loading .claude/settings.local.json wholesale is the correct mitigation for secret leakage. Matches the knowledge/setup exemplar. ✓

Local-override shadow warningSKILL.md L31–33: Explicitly surfacing that a project-scope write stays shadowed by a local override avoids a confusing support scenario. ✓

Gitignore recommendationSKILL.md L55–56: Recommending the consumer gitignore .claude/settings.local.json is good defensive guidance, especially given the secret-bearing note above. ✓

No egress, no remote MCP, no code execution risks.


CLAUDE.md alignment

Rule Status
Repo-agnostic — no hardcoded paths ✓ reads consumer's settings via pluginConfigs path
Configurable without editing the plugin ✓ uses userConfig
Plugin-form-safe — no ../ reach-outs ✓ Step 4 explicitly forbids writing to plugin dir or ${CLAUDE_PLUGIN_DATA}
No PII/secrets ✓ narrow-read guard on settings.local.json
Versioned ✓ 0.2.0

Minor observation (no action required)

SKILL.md L6 sets disable-model-invocation: false explicitly; knowledge/setup omits it (the default is false). Both behave identically — cosmetic inconsistency between the two setup skills. Worth either removing the explicit key here or back-filling knowledge/setup to match, but no urgency either way. Fix this →


Overall: Clean, well-scoped implementation. The idempotency design, inference ladder, secret-aware narrow-read, and scope guard are all well-executed. Ready to merge.

cursor[bot]
cursor Bot approved these changes Jul 12, 2026
@kyle-sexton
kyle-sexton force-pushed the chore/retrofit-discovery branch from a0b5edf to 420e9d7 Compare July 12, 2026 21:11
@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 20s —— View job


PR Review

  • Load tools
  • Read changed files and diff
  • Review for correctness, security, and CLAUDE.md alignment
  • Post findings

No blockers. This sync carries the same single commit as the prior reviews; findings from those passes all hold. Three files changed: new SKILL.md (71 lines), version bump, one-line README update.


Correctness

Plugin keySKILL.md L15–16: pluginConfigs["discovery@melodic-software"].options.notes_dir correctly derives from plugin.json name: "discovery" + author.name: "Melodic Software". ✓

Default valueSKILL.md L44: Fallback .claude/notes matches plugin.json's userConfig.notes_dir.default. ✓

Version bump0.1.0 → 0.2.0 is the correct semver level for a backward-compatible feature addition. ✓

Precedence orderSKILL.md L28–29: Local > Project > User is correct per Claude Code settings resolution. ✓

Idempotency — Read-first / offer-update-on-re-run pattern is sound and backed by the instruction text. ✓


Security

Narrow jq readSKILL.md L33–36: Querying only the single notes_dir key and never loading .claude/settings.local.json wholesale is the correct mitigation for secret leakage. ✓

Local-override shadow warningSKILL.md L31–33: Explicitly surfacing that a project-scope write stays shadowed by a local override avoids a confusing support scenario. ✓

Gitignore recommendationSKILL.md L55–56: Recommending consumers gitignore .claude/settings.local.json is good defensive guidance. ✓

No egress, no remote MCP, no code execution risks.


CLAUDE.md alignment

Rule Status
Repo-agnostic — no hardcoded paths ✓ reads consumer's settings via pluginConfigs path
Configurable without editing the plugin ✓ uses userConfig
Plugin-form-safe — no ../ reach-outs ✓ Step 4 explicitly forbids writing to plugin dir or ${CLAUDE_PLUGIN_DATA}
No PII/secrets ✓ narrow-read guard on settings.local.json
Versioned ✓ 0.2.0

Minor observation (no action required)

SKILL.md L6 sets disable-model-invocation: false explicitly; the knowledge/setup exemplar omits it (the default is false). Both behave identically — cosmetic inconsistency between the two setup skills. Worth either removing the explicit key here or back-filling knowledge/setup to match, but no urgency. Fix this →


Overall: Clean, well-scoped implementation. The idempotency design, inference ladder, secret-aware narrow-read, scope guard, and "What this skill does NOT do" section are all well-executed. Ready to merge.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Risk: low. Approved — adds a re-runnable discovery setup skill with README and patch version bump; Cursor Bugbot check was not present on this sync. No reviewers assigned.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@kyle-sexton
kyle-sexton enabled auto-merge (squash) July 12, 2026 21:37
The discovery plugin exposed a notes_dir userConfig seam but shipped no
setup/configure action, violating the extensibility playbook's "every
configurable plugin ships a setup action". Add an idempotent setup skill
modeled on knowledge/setup: convention-resolution ladder, narrow
secret-aware settings reads, project-scope persistence. Bump minor
version and reference the skill from the README Configuration section.

Refs melodic-software/medley#1429
@kyle-sexton
kyle-sexton force-pushed the chore/retrofit-discovery branch from 420e9d7 to 70cccff Compare July 12, 2026 21:44
@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 9s —— View job


PR Review

  • Load tools
  • Read changed files and diff
  • Review for correctness, security, and CLAUDE.md alignment
  • Post findings

No blockers. Three files changed: new SKILL.md (71 lines), version bump 0.1.0 → 0.2.0, one-line README update. All correct. Consistent with the three prior reviews on this branch.


Correctness

Plugin key alignmentSKILL.md L14–16: pluginConfigs["discovery@melodic-software"].options.notes_dir correctly derives from plugin.json name: "discovery" + author.name: "Melodic Software". ${user_config.notes_dir} substitution matches the userConfig declaration in plugin.json L13. ✓

Default valueSKILL.md L44: Fallback .claude/notes matches plugin.json L18 userConfig.notes_dir.default. ✓

Type annotationSKILL.md L52: Correctly calls this a string option (vs directory in knowledge/setup, where library_dir is typed directory). ✓

Version bump0.1.0 → 0.2.0 is the correct semver level for a backward-compatible feature addition. ✓

Precedence orderSKILL.md L28–29: Local > Project > User is correct per Claude Code settings resolution. ✓

Idempotency — Read-first / offer-update pattern on re-run is sound and backed by the instruction text. ✓


Security

Narrow jq readSKILL.md L33–36: Querying only the single notes_dir key and never loading .claude/settings.local.json wholesale is the correct mitigation for secret leakage. Matches the knowledge/setup exemplar. ✓

Local-override shadow warningSKILL.md L31–33: Explicitly surfacing that a project-scope write stays shadowed by a local override avoids a confusing support scenario. ✓

Gitignore recommendationSKILL.md L55–56: Recommending consumers gitignore .claude/settings.local.json is good defensive guidance given the secret-bearing caveat. ✓

No egress, no remote MCP, no code execution risks.


CLAUDE.md alignment

Rule Status
Repo-agnostic — no hardcoded paths ✓ reads consumer's settings via pluginConfigs path
Configurable without editing the plugin ✓ uses userConfig
Plugin-form-safe — no ../ reach-outs ✓ Step 4 explicitly forbids writing to plugin dir or ${CLAUDE_PLUGIN_DATA}
No PII/secrets ✓ narrow-read guard on settings.local.json
Versioned ✓ 0.2.0

Minor observation (no action required)

SKILL.md L6 sets disable-model-invocation: false explicitly; the knowledge/setup exemplar omits it (the default is false). Both behave identically — cosmetic inconsistency between the two setup skills. Worth removing the explicit key here or back-filling knowledge/setup to match, but no functional impact either way. Fix this →


Overall: Clean, well-scoped implementation. The idempotency design, inference ladder, secret-aware narrow-read, "What this skill does NOT do" scope guard, and plugin-form-safe persistence are all well-executed. Ready to merge.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Risk: low. Approved — adds a re-runnable discovery setup skill with README and version bump; Cursor Bugbot check was not present on this sync. No reviewers assigned.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@kyle-sexton
kyle-sexton merged commit 9c74948 into main Jul 12, 2026
36 checks passed
@kyle-sexton
kyle-sexton deleted the chore/retrofit-discovery branch July 12, 2026 21:56
kyle-sexton added a commit that referenced this pull request Jul 12, 2026
Base main advanced to discovery@0.2.0 (PR #111) while this PR was open, so
the eval files added here no longer shipped with a version bump relative to
base. Bump to 0.3.0 so consumers receive the new eval assets on marketplace
update. Addresses Codex review finding.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant