Skip to content

chore(knowledge/setup): retrofit library_dir precedence + portability hardening#138

Merged
kyle-sexton merged 4 commits into
mainfrom
chore/retrofit-knowledge-setup
Jul 13, 2026
Merged

chore(knowledge/setup): retrofit library_dir precedence + portability hardening#138
kyle-sexton merged 4 commits into
mainfrom
chore/retrofit-knowledge-setup

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Applies the resolved planning/setup hardening to knowledge/setup's library_dir seam (same single-directory userConfig shape, same latent issues).

Changes

plugins/knowledge/skills/setup/SKILL.md

  • Precedence — step 1 now resolves the full documented order (Managed > --settings > Local > Project > User), warns when higher layers are unreadable instead of declaring a lower value authoritative, and surfaces layer shadowing before the project write.
  • Portability — step 5 guards the tracked project write: never propagate a machine-specific personal-layer value into team settings; route personal-only overrides to .claude/settings.local.json (portability required only for the shared project write).
  • Declared-convention alignment — inspect the repo's declared working-notes/artifacts convention even when a value is set; surface divergence and offer to reconcile.

plugins/codebase-audit/skills/setup/SKILL.md (partial match — writes a tracked .md, not settings)

  • Applied only the precedence/shadowing subset: step 1 reports the effective additively-merged config across user-global → team → local layers (and warns on unreadable layers), instead of summarizing the team file alone. Settings-write portability specifics do not apply.

Deviations (own-flagged)

  1. Fix feat: markdown-formatter plugin (pilot migration) #3 premise softened for knowledge. The issue's fix feat: markdown-formatter plugin (pilot migration) #3 parenthetical assumed knowledge "honors that convention over the config at write time" (true for planning). Verified via skills/youtube/extraction/lib/work-root.js and the youtube pipeline that knowledge resolves library_dir directly at write time — no skill overrides it with a repo convention at runtime. Documenting that behavior would be false, so I softened the imported write-time-override claim: library_dir is runtime-authoritative, and setup's job is to keep it aligned with any declared convention. Fixes chore: initialize marketplace scaffold #1 and fix: correct plugin-system inaccuracies and harden references #2 are unaffected.
  2. codebase-audit version bump not in issue scope. Patch-bumped 0.1.0 → 0.1.1 because its SKILL behavior changed; the issue only specified the knowledge minor bump.

Verification

  • knowledge plugin.json minor bump 0.2.0 → 0.3.0; codebase-audit 0.1.0 → 0.1.1.
  • claude plugin validate clean on both plugins.
  • markdownlint clean on both edited files.

Refs melodic-software/medley#1463


Note

Low Risk
Changes are limited to plugin skill documentation and version metadata; no application runtime or security-sensitive code paths are modified.

Overview
Hardens setup skills for the knowledge and codebase-audit plugins so interactive setup reflects layered config reality instead of treating a single team file as authoritative.

For knowledge/setup, step 1 now follows the full Claude Code precedence chain (Managed → --settings → Local → Project → User), warns when higher layers are unreadable, and calls out when a project write would stay shadowed. A new step reconciles library_dir with repo-declared working-notes conventions even when a value is already set. Persistence (step 5) blocks copying machine-specific personal paths into tracked project settings, routes personal-only overrides to .claude/settings.local.json, and the output section documents honest pipeline behavior (book-distill ignores library_dir; YouTube does not yet honor it). knowledge bumps 0.2.0 → 0.3.0.

For codebase-audit/setup, step 1 is expanded to summarize the effective merged config across user-global → team → local layers (including dimension opt-outs via empty source lists), explain that only the team file is written, and warn when overlays block re-enabling dimensions. codebase-audit patch-bumps 0.1.0 → 0.1.1.

Reviewed by Cursor Bugbot for commit f13139a. Bugbot is set up for automated code reviews on this repo. Configure here.

… hardening

Mirror the resolved planning/setup hardening onto knowledge/setup's
library_dir seam:

- Precedence: resolve the full documented order (Managed > --settings >
  Local > Project > User), warn when higher layers are unreadable, and
  surface layer shadowing rather than reporting a lower-precedence value
  as effective.
- Portability: guard the tracked project write against propagating a
  machine-specific personal value; route personal-only overrides to the
  local overlay (portability required only for the shared project write).
- Declared-convention alignment: inspect the repo's declared
  working-notes/artifacts convention even when a value is set, and surface
  divergence so library_dir tracks it.

Deviation from the issue's fix #3 premise: verified via work-root.js and
the youtube pipeline that knowledge resolves library_dir directly at write
time — no skill honors a repo convention OVER library_dir at runtime (that
is planning's design, not knowledge's). Softened the imported write-time
override claim to knowledge's reality (library_dir is runtime-authoritative;
setup keeps it aligned with the declared convention).

Apply the precedence/shadowing subset to codebase-audit/setup (partial
match — writes a tracked .md, not settings): step 1 now reports the
effective additively-merged config across user-global/team/local layers
instead of the team file alone. Patch-bump codebase-audit for the
behavior change.

Bump knowledge plugin.json minor; both plugins validate clean.
@cursor

cursor Bot commented Jul 13, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_af8bc7af-67ea-415f-a081-e846878a3e34)

@claude

claude Bot commented Jul 13, 2026

Copy link
Copy Markdown

PR Review — chore(knowledge/setup): retrofit library_dir precedence + portability hardening

  • Gather context (CLAUDE.md, PR diff, changed files)
  • Review plugins/knowledge/skills/setup/SKILL.md
  • Review plugins/codebase-audit/skills/setup/SKILL.md
  • Review version bumps in both plugin.json files
  • Check alignment with CLAUDE.md design rules
  • Post final review

View job run

@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.

Stale comment

Risk: low. Did not approve: Cursor Bugbot completed as skipped (usage limit reached), so automated review did not finish successfully. Human review is required; no reviewers were assigned because no eligible reviewers were identifiable outside the author.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 00a6ae916b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/codebase-audit/skills/setup/SKILL.md Outdated
Comment thread plugins/knowledge/skills/setup/SKILL.md Outdated
…rstated library_dir runtime claim

- codebase-audit/setup step 1: honor the empty-source-list opt-out when summarizing the effective merged config, so a dimension a higher layer deliberately zeroed is reported removed, not present.
- knowledge/setup Output: library_dir's youtube wiring is not currently exported (resolveWorkRoot falls back to CLAUDE_PROJECT_DIR), so describe library_dir as the persisted/configured seam setup keeps aligned with the repo convention, not as a guaranteed runtime redirect. Youtube wiring gap tracked separately.
@cursor

cursor Bot commented Jul 13, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_f145211b-3b7b-4606-b092-bd5bb72e2757)

@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.

Stale comment

Risk: low. Did not approve: Cursor Bugbot completed as skipped (usage limit reached), so automated review did not finish successfully. Human review is required; no reviewers were assigned because no eligible reviewers were identifiable outside the author.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 85ce7ce1f1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/knowledge/skills/setup/SKILL.md
@claude

claude Bot commented Jul 13, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 3m 48s —— View job


PR Review — chore(knowledge/setup): retrofit library_dir precedence + portability hardening

  • Gather context (CLAUDE.md, PR diff, changed files)
  • Review plugins/knowledge/skills/setup/SKILL.md
  • Review plugins/codebase-audit/skills/setup/SKILL.md
  • Review version bumps in both plugin.json files
  • Check alignment with CLAUDE.md design rules
  • Verify Codex P2 findings were resolved
  • Post final review

Overview

The PR correctly ports the planning/setup hardening onto the knowledge/setup and codebase-audit/setup seams. The precedence fix (5-layer resolution), the security guard (narrow jq read of .settings.local.json), the portability guard (no machine-specific values propagated into team settings), and the codebase-audit opt-out fix are all well-implemented. One P2-level issue from the Codex pass is still open at HEAD, and there is one minor cross-reference inconsistency in the codebase-audit file.


Issues

1 — P2 (still-open): Step 2 of knowledge/setup/SKILL.md still overstates library_dir runtime authority

File: plugins/knowledge/skills/setup/SKILL.md lines 41–43

Commit 85ce7ce corrected the Output section (changed "the pipelines themselves resolve library_dir directly at write time" → "the persisted seam for where pipelines are configured to land artifacts"). But the Task section's step 2 parenthetical was not updated and still reads:

"library_dir is authoritative at the pipelines' write time (they resolve the persisted seam directly), so unlike a runtime-override design there is no silent write-to-a-different-path trap here."

Checked against work-root.js:

export function resolveWorkRoot() {
  return process.env.YOUTUBE_WORK_ROOT || process.env.CLAUDE_PROJECT_DIR || process.cwd();
}

YOUTUBE_WORK_ROOT is documented in that file's comment as "the invoking skill exports from the resolved library_dir (when it is not the repo-root default)." However, the youtube SKILL.md never sets or exports YOUTUBE_WORK_ROOT. So resolveWorkRoot() always falls through to CLAUDE_PROJECT_DIR/cwd — a non-default library_dir does not currently redirect pipeline artifacts. The "no silent write-to-a-different-path trap" claim in step 2 is therefore incorrect: there is exactly that trap — configure docs/knowledge via setup, but artifacts still land at .work/.

The Output section fix is good, but the step 2 instruction still instructs the agent to frame setup with the incorrect runtime guarantee. A user following this guidance would think the configured value is already active.

Suggested fix: replace the parenthetical in step 2 with language consistent with the Output section — e.g.:

-`library_dir` is authoritative at the pipelines' write time (they resolve the persisted seam directly), so unlike a runtime-override design
-there is no silent write-to-a-different-path trap here. The risk is a different one: the plugin's intent is
-that `library_dir` *tracks* the consuming repo's own working-notes/artifacts convention
+`library_dir` is the *intended* destination for pipeline artifacts, and setup's job is to keep it
+aligned with the consuming repo's own working-notes/artifacts convention (declared in its

Or simply drop the false parenthetical premise and lead with the actual risk (stale divergence from a declared convention). The youtube wiring gap is pre-existing and correctly left out of scope, but the step instruction shouldn't assert a runtime guarantee that doesn't yet exist.

Fix this →


2 — Minor: Dangling (step 6) cross-reference in codebase-audit/setup/SKILL.md

File: plugins/codebase-audit/skills/setup/SKILL.md line 24

The fixup commit moved (step 6) from the additive-merge clause (where it correctly referenced step 6's "additively" language) to after the new opt-out exception:

"…an explicit opt-out that removes that inherited dimension (step 6)"

Step 6 of this setup skill describes the overlay convention ("Personal overrides go in .claude/codebase-audit.local.md…"). It does not describe the empty-source-list opt-out behavior. The cross-reference now implies the opt-out semantics are in step 6, which they are not — they live in the audit runtime skill. A reader following this parenthetical will find the wrong section.

Suggested fix: drop (step 6) from the opt-out clause, or replace it with the authoritative reference (e.g., "see the audit skill's opt-out contract").


3 — Suggestion: codebase-audit patch bump may be undersized

File: plugins/codebase-audit/.claude-plugin/plugin.json

The bump is 0.1.0 → 0.1.1. Step 1's behavior change — from "summarize the team file" to "summarize the effective merged config across all layers, honoring opt-outs, attributing per layer" — is a meaningful semantic change in what setup presents and proposes changes against. Existing consumers who have user-global or local overlay layers will see different (more complete, but different) summaries. A minor bump (0.1.0 → 0.2.0) would more clearly signal to consumers that setup behavior changed. Not blocking, but worth considering.


What's working well

  • Precedence expansion — step 1 now correctly documents all five layers (Managed → --settings → Local → Project → User) with appropriate "warn, don't claim lower value is authoritative" language for unreadable higher layers. This matches the Claude Code settings docs.
  • Security guard — the narrow jq read ("query only the single key, never load .settings.local.json wholesale") is a solid defensive measure for a secret-bearing file.
  • Portability guard — the two-guard structure in step 5 (no machine-specific local values into team settings; scope-to-intent routing to .settings.local.json) correctly models the team/personal split.
  • Codebase-audit opt-out fix — honoring empty-source-list as an explicit opt-out in the effective merged config summary directly addresses the Codex P2 from the first review pass. The language ("a dimension a higher layer deliberately zeroed out is reported as removed, not as still present") is clear.
  • Idempotency framing — both skills correctly preserve the re-runnable/reconfigure narrative.
  • Plugin-form rules — no hardcoded paths, no ../ reach-outs, userConfig used for the single consumer-configurable knob. Both plugins pass claude plugin validate per the PR's verification checklist.

Summary: One item (issue #1) should be resolved before merge — the step 2 task instruction in knowledge/setup/SKILL.md still contains the factually incorrect runtime-authority claim that the two Codex passes flagged. The Output section was fixed but the instruction body was not. The other two items are minor/optional. Everything else is solid.

… the youtube pipeline

Address repeat Codex finding: a non-default library_dir does not currently redirect youtube artifacts (resolveWorkRoot falls back to the project root; the skill never exports YOUTUBE_WORK_ROOT). Output now reports a persisted non-default value as recorded-for-future-consumption rather than as currently relocating artifacts, so setup does not mislead. Runtime wiring is tracked as a separate issue.
@cursor

cursor Bot commented Jul 13, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_45a31ff7-7932-4653-a5c1-afcaaee40a99)

@claude

claude Bot commented Jul 13, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 32s —— View job


PR Review — chore(knowledge/setup): retrofit library_dir precedence + portability hardening

  • Gather context
  • Verify P2 fix — step 2 runtime-authority claim in knowledge/setup/SKILL.md
  • Verify minor fix — dangling (step 6) in codebase-audit/setup/SKILL.md
  • Post final review

Re-review after 6d573de

Commit 6d573de ("fix(knowledge/setup): disclose that library_dir is not yet honored by the youtube pipeline") touched only plugins/knowledge/skills/setup/SKILL.md, and only the Output section. codebase-audit/setup/SKILL.md was not touched.


What 6d573de fixed

Output section (lines 92–96) — correctly fixed. The new language adds:

"Be accurate about current reach so the user is not misled: /knowledge:book-distill ignores library_dir … and the /knowledge:youtube pipeline does not yet honor a non-default library_dir (its work root falls back to the consuming project root), so when a non-default value is persisted, report it as recorded for when the pipelines consume it — not as currently relocating artifacts."

This is accurate, clear, and directly addresses the core user-safety concern flagged by both Codex passes.


Remaining issues

1 — Residual inconsistency: step 2 task instruction still contradicts the Output section

File: plugins/knowledge/skills/setup/SKILL.md lines 40–42

Step 2's opening clause was not updated and still reads:

"library_dir is authoritative at the pipelines' write time (they resolve the persisted seam directly), so unlike a runtime-override design there is no silent write-to-a-different-path trap here."

This directly contradicts the now-correct Output section. The Output section correctly tells the agent to disclose the limitation at the end; step 2 tells the agent — while conducting the interview — that the trap doesn't exist. An agent reconciling both may still downplay the wiring gap during the recommendation phase even as it discloses it in the output summary.

The severity is lower now that the Output section is accurate (a well-prompted agent will follow the Output instruction), but the contradiction is still logically inconsistent and could confuse a less careful agent invocation. The two-sentence fix suggested in the previous review still applies:

-`library_dir` is authoritative at the pipelines' write time (they resolve the persisted seam directly), so unlike a runtime-override design
-there is no silent write-to-a-different-path trap here. The risk is a different one: the plugin's intent is
+`library_dir` is the *intended* destination for pipeline artifacts, though not all pipelines currently
+honor a non-default value at runtime (see Output for current reach). The primary risk for setup is: the plugin's intent is

Or simply drop the false parenthetical and lead with the actual risk (stale divergence from a declared convention), consistent with what the Output section now says.

Fix this →


2 — Minor still open: dangling (step 6) in codebase-audit/setup/SKILL.md

File: plugins/codebase-audit/skills/setup/SKILL.md line 21

6d573de did not touch this file. The (step 6) cross-reference in the opt-out clause still points to step 6 ("Offer the overlay convention"), not to the opt-out contract in the audit runtime skill. A reader following the parenthetical lands in the wrong section.

Fix this →


Assessment

The Output section disclosure in 6d573de is the right fix and substantially closes the Codex P2. Two items remain: the logical inconsistency in step 2's task instruction (which, while now less dangerous, still contradicts the Output section) and the minor dangling cross-reference in codebase-audit. Both are small targeted edits. Everything else — precedence expansion, portability guards, security guard, opt-out handling, version bumps — is solid.

@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.

Stale comment

Risk: low. Did not approve: Cursor Bugbot completed as skipped (usage limit reached), so automated review did not finish successfully. Human review is required; no reviewers were assigned because no eligible reviewers were identifiable outside the author.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6d573de799

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/knowledge/skills/setup/SKILL.md Outdated
Comment thread plugins/codebase-audit/skills/setup/SKILL.md Outdated
- knowledge 'does NOT do': the personal .claude/settings.local.json write in step 5 is legitimate machine-local config; reframe the exclusion as 'never writes into the plugin install/data directory' (mirrors planning/setup) and correct the local overlay from 'tracked' to the personal gitignored overlay, so an agent does not skip the personal-override write.
- codebase-audit step 1: extend the overlay-shadowing warning to the subtractive direction — when a local/user opt-out zeroes a dimension, a team-scope re-enable will not restore it on that machine; prompt to also remove/update the overlay opt-out.
@cursor

cursor Bot commented Jul 13, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_d29f3c08-df9e-44e7-be97-4db8f9d8af8f)

@claude

claude Bot commented Jul 13, 2026

Copy link
Copy Markdown

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


PR Review — chore(knowledge/setup): retrofit library_dir precedence + portability hardening

  • Gather context (PR diff, changed files at HEAD f13139a)
  • Verify fix for step 2 runtime-authority claim in knowledge/setup/SKILL.md
  • Verify fix for dangling (step 6) in codebase-audit/setup/SKILL.md
  • Verify fixes for two new Codex P2 findings from 6d573de (personal override path + local opt-out shadow warning)
  • Post final review

Re-review after f13139a

Commit f13139a ("fix(setup): resolve two Codex consistency findings") touched both plugins/knowledge/skills/setup/SKILL.md and plugins/codebase-audit/skills/setup/SKILL.md.


What f13139a fixed

Codex P2 — "Don't forbid the personal override path" in knowledge/setup/SKILL.md: Fixed. The "What this skill does NOT do" bullet was rewritten from "Write machine-local state — configuration lives in the consumer's tracked settings" to "Write anything into the plugin directory or the plugin data directory — configuration lives in the consumer's settings (tracked project .claude/settings.json, or the personal .claude/settings.local.json overlay for a personal-only override per step 5)." An agent following the old wording could have skipped the personal-override write entirely; the new wording correctly treats .settings.local.json as a legitimate target. Good fix.

Codex P2 — "Warn when a local opt-out still shadows team writes" in codebase-audit/setup/SKILL.md: Fixed. Step 1's overlay-shadowing warning is now bidirectional: it calls out the case where a local opt-out zeroes a dimension and explicitly instructs the agent to prompt the user to remove or update that overlay when accepting a team-scope re-enable. Previously the warning only covered additive overlays. Good fix.


Remaining issues

1 — Still open (P2): Step 2 task instruction contradicts the Output section in knowledge/setup/SKILL.md

File: plugins/knowledge/skills/setup/SKILL.md lines 40–42

f13139a did not touch step 2. The following text is unchanged at the current HEAD:

"library_dir is authoritative at the pipelines' write time (they resolve the persisted seam directly), so unlike a runtime-override design there is no silent write-to-a-different-path trap here."

This directly contradicts the Output section (lines 93–96), which 6d573de correctly fixed:

"the /knowledge:youtube pipeline does not yet honor a non-default library_dir (its work root falls back to the consuming project root), so when a non-default value is persisted, report it as recorded for when the pipelines consume it — not as currently relocating artifacts."

These two statements are logically incompatible. Step 2 tells an agent during the interview phase that the "no silent write-to-a-different-path trap" guarantee holds; the Output section tells the same agent at the reporting phase to do the opposite. A user walking through setup on a non-default library_dir will receive contradictory framing depending on which part of the skill the agent is executing at the moment.

This is the same finding that both previous reviews explicitly flagged. Two consecutive passes left it unfixed; the step 2 body was simply not updated to match the Output section.

Suggested fix (two-sentence replacement, unchanged from prior suggestion):

-`library_dir` is authoritative at the pipelines' write time (they resolve the persisted seam directly), so
-unlike a runtime-override design there is no silent write-to-a-different-path trap here. The risk is a
-different one: the plugin's intent is
+`library_dir` is the *intended* destination for pipeline artifacts, though not all pipelines currently
+honor a non-default value at runtime (see Output for current reach). The risk setup guards against is: the plugin's intent is

Or simply drop the false parenthetical premise entirely and lead with the actual risk sentence, which is correct on its own.

Fix this →


2 — Still open (minor): Dangling (step 6) cross-reference in codebase-audit/setup/SKILL.md

File: plugins/codebase-audit/skills/setup/SKILL.md line 21

f13139a did not touch this. Line 21 still reads:

"…an explicit opt-out that removes that inherited dimension (step 6)."

Step 6 of this skill ("Offer the overlay convention") describes the .codebase-audit.local.md personal overlay — it does not document the empty-source-list opt-out contract. The opt-out semantics live in the runtime audit skill. A reader following this parenthetical lands in the wrong section.

Suggested fix: drop (step 6) from the opt-out clause, or replace it with (see the audit skill's opt-out contract).

Fix this →


Assessment

f13139a closes both Codex P2 findings from 6d573de cleanly. Two items from the earlier review passes remain open:

# Severity File Status
Step 2 runtime-authority claim P2 knowledge/setup/SKILL.md:40–42 Still open
Dangling (step 6) reference Minor codebase-audit/setup/SKILL.md:21 Still open

Everything else — the five-layer precedence expansion, the narrow-jq security guard, the portability guards, the correct Output disclosure, the Codex opt-out fix, the personal-override path fix — is solid. The PR is one targeted step-2 rewrite (and an optional one-word drop) away from being clean.

@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. Did not approve: Cursor Bugbot completed as skipped (usage limit reached), so automated review did not finish successfully. Human review is required; no reviewers were assigned because no eligible reviewers were identifiable outside the author.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@kyle-sexton
kyle-sexton merged commit d31e75c into main Jul 13, 2026
20 of 22 checks passed
@kyle-sexton
kyle-sexton deleted the chore/retrofit-knowledge-setup branch July 13, 2026 02:28
kyle-sexton added a commit that referenced this pull request Jul 13, 2026
…ne (#146)

Retrofit the `course-digest` skill (Dometrain/Teachable course
extraction and repo-applicability synthesis) into the `knowledge`
plugin, replicating the shipped `youtube` vendoring precedent. Pre-baked
decisions from the source issue were honored, not re-litigated.

## What changed

- **Vendored shared libs** — `@melodic/repo-analysis` +
`@melodic/video-digestion` copied (runtime source only) into
`extraction/vendor/`, each with its own trimmed `package.json` (real
`name`/`version`/`exports`; `video-digestion` keeps its `imghash`
runtime dep). `package.json` deps rewritten to `file:./vendor/*`.
Byte-identical to the youtube skill's vendored copies (dedup of the two
is tracked separately).
- **Launcher trio** — `run.mjs` / `register-hook.mjs` /
`resolve-hook.mjs` resolve bundled deps from `${CLAUDE_PLUGIN_DATA}` via
an ESM resolve hook. `setup-deps.mjs` installs node deps into
`${CLAUDE_PLUGIN_DATA}` (sha256 fingerprint over `package.json` +
`vendor/`, stamped `.course-extraction.stamp`) **and provisions
Playwright's Chromium** into `${CLAUDE_PLUGIN_DATA}/ms-playwright`.
`run.mjs` pins `PLAYWRIGHT_BROWSERS_PATH` to that path so install-path
and runtime-lookup-path stay in lockstep.
- **Credentials (adapter-native, no sensitive userConfig)** — login via
the user's own `COURSE_*`/`TEACHABLE_*` shell env vars (prefix driven by
`platformConfig.authEnvPrefix`), interactive manual-login fallback.
Session cookies re-homed out of the consumer repo to
`${CLAUDE_PLUGIN_DATA}/auth/<platform>.auth-state.json` via
`lib/auth-store.js`.
- **Decoupling** — inlined `tsconfig.json` (no cross-repo `extends`);
genericized `SKILL.md` + `context/`/`reference/` paths to
`${CLAUDE_PLUGIN_ROOT}`; routed script invocations through `run.mjs`;
namespaced self-referencing slashes to `/knowledge:course-digest` (and
cross-refs to `/knowledge:youtube`); rewrote `!`-precompute probes to
cross-platform `node -e`; fixed the `build-course-json.js` JSDoc example
path.
- **Version + catalog** — `knowledge` plugin `0.4.0` → `0.5.0` (rebased
on #145, which took `0.4.0`), CHANGELOG `0.4.0` entry added (plus a
backfilled `0.3.0` entry that PR #138 had omitted). Extended plugin
`keywords` and marketplace `tags` with course/video terms.
- Excluded the ~2 GB `data/` tree (consumer machine state, per the
issue).

## Verification

- `claude plugin validate --strict` — clean (plugin + marketplace).
- `npx vitest run` — **10 files, 91 tests pass**; `tsc --noEmit` clean;
markdownlint clean.
- **Clean-repo smoke** (scratchpad, `CLAUDE_PLUGIN_DATA`=temp, **local
`node_modules` absent**):
- `setup-deps.mjs` installed deps + Chromium into the data dir; re-run
early-exits (idempotent).
- Vendored `@melodic/*` + `playwright` bare imports resolved **from
`${CLAUDE_PLUGIN_DATA}`** (proved by running a pipeline script with no
local `node_modules`).
- Dry run reached the **auth branch**: Chromium launched from the
data-dir path and control entered `adapter.authenticate` → `clerkLogin`,
failing exactly as expected on a fake login page (no real credentials).

## Implementation decisions (reviewer visibility)

- **Auth is keyed per-platform**, not per-course. The source issue
framed the re-home as "location, not behavior"; per-platform is a slight
behavior change but the correct shape (a login session is a platform
session, reused across courses).
- **Dev/test fallback** when `CLAUDE_PLUGIN_DATA` is unset:
`${home}/.claude/course-digest/auth/…` — machine-local, persistent, out
of the consumer repo.
- `--course-dir` stays caller-provided; full `library_dir` → course-dir
auto-resolution (the youtube work-root analog) is **not** wired here —
possible follow-up.
- **Manual-login-under-headless** may not function (documented as a
known limitation, not a blocker); env-var + cookie-reuse carry the
skill.

Refs melodic-software/medley#1409

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Adds Playwright automation, course-platform credentials, and large new
surface area (browser auth, vendored deps, long-running extraction), but
credentials stay in env/plugin data and behavior is documented/tested
rather than changing existing skills.
> 
> **Overview**
> **Knowledge plugin 0.5.0** adds **`/knowledge:course-digest`**: a
multi-phase workflow to discover, extract, and synthesize **Dometrain**
and **Teachable** courses into repo-applicable artifacts
(`repo-candidates.md`, `action-items.md`), with single YouTube videos
still routed to `/knowledge:youtube`.
> 
> The PR ships a **bundled Node extraction stack** (Playwright
orchestration, ffmpeg/ImageMagick frame tooling, companion-repo
analysis) following the **youtube** pattern: `@melodic/repo-analysis`
and `@melodic/video-digestion` vendored under `extraction/vendor/`,
**`setup-deps.mjs`** installing into `${CLAUDE_PLUGIN_DATA}` plus
Chromium under `ms-playwright`, and **`run.mjs`** resolving deps and
pinning `PLAYWRIGHT_BROWSERS_PATH`. **Platform adapters** (`dometrain`,
`teachable`) implement a shared contract (transcript, HLS, resources,
URLs); shared **Clerk** / **Teachable SSO** auth and **Mux** /
**Hotmart** player modules live under `lib/`.
> 
> **Auth** uses shell `COURSE_*` / `TEACHABLE_*` (from
`platformConfig.authEnvPrefix`) with manual-login fallback; session
state is stored per platform at
**`${CLAUDE_PLUGIN_DATA}/auth/<platform>.auth-state.json`**, not in the
consumer repo. Skill docs cover the 8-phase pipeline, storage schema,
evals, and prerequisites; marketplace/plugin metadata and CHANGELOG are
updated (including a backfilled **0.3.0** entry).
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
477e254. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
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