feat(knowledge/youtube): honor library_dir — wire resolved work root into the pipeline#145
Conversation
…into the pipeline
The youtube pipeline defined a `library_dir` userConfig seam but never
consumed it: a non-default value (e.g. docs/knowledge) did not redirect
watch/transcript/queue artifacts, which still landed at the repo root via
resolveWorkRoot()'s CLAUDE_PROJECT_DIR fallback.
Wire it end to end:
- run.mjs learns an optional leading `--work-root <dir>` flag and translates
it into the YOUTUBE_WORK_ROOT env var the extraction scripts already read.
A double-quoted CLI arg is cross-platform; an inline `YOUTUBE_WORK_ROOT=… node`
prefix is bash-only (the same reason run.mjs already avoids `NODE_PATH=… node`).
Arg parsing + child-env layering extracted to lib/run-args.js (unit-tested).
- youtube/SKILL.md documents passing `--work-root "${CLAUDE_PROJECT_DIR}/${user_config.library_dir}"`
on every run.mjs invocation when library_dir is non-default; unchanged (no flag)
for the repo-root default, keeping the resolveWorkRoot() fallback intact.
- setup/SKILL.md Output restores the stronger "library_dir governs where youtube
artifacts land" wording softened while the seam was unwired (book-distill remains
the documented exception).
- knowledge plugin.json 0.3.0 -> 0.4.0 + CHANGELOG entry.
Refs melodic-software/medley#1466
Bugbot couldn't run - usage limit reachedBugbot 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_c0a1417f-84b2-4490-ab8a-d285d1deb061) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ba7806238
ℹ️ 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".
There was a problem hiding this comment.
Stale comment
Risk: medium. Cursor Bugbot did not complete (check skipped due to usage limit), so the required automated review signal is incomplete. Human review is needed before merge; no reviewers were assigned because the only prior human contributor is the PR author.
Sent by Cursor Approval Agent: Pull Request Router and Approver
…ved work root Addresses a Codex review finding: the `--work-root` flag redirected only the script-written artifacts under a non-default `library_dir`, while the skill's hand-materialized `.work/<watch-epic>/…` paths (QUEUE.md, claim stubs, and the agent-authored Output-contract deliverables) still resolved to the repo root — splitting the queue table from its concurrency claims across two directories. Document that every `.work/<watch-epic>/…` path in the skill and its context files is relative to the same resolved work root, so a non-default library_dir co-locates the whole slice instead of splitting it.
Bugbot couldn't run - usage limit reachedBugbot 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_36c5aeb4-9451-489e-991a-62251e2038bf) |
|
Good catch — fixed in eb2138d. The |
There was a problem hiding this comment.
Risk: medium. Cursor Bugbot did not complete (check skipped due to usage limit), so the required automated review signal is incomplete. Human review is needed before merge; no reviewers were assigned because the only prior contributor to these paths is the PR author.
Sent by Cursor Approval Agent: Pull Request Router and Approver
…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 -->


What
Wire the knowledge plugin's
library_diruserConfigseam into the youtube pipeline at runtime. Before this,library_dirwas configurable and persisted bysetup, but the youtube pipeline never consumed it — a non-default value (e.g.docs/knowledge) did not redirect watch/transcript/queue artifacts; they still landed at the consuming repo root viaresolveWorkRoot()'sCLAUDE_PROJECT_DIRfallback.How
run.mjslearns an optional leading--work-root <dir>flag and translates it into theYOUTUBE_WORK_ROOTenv var the extraction scripts'resolveWorkRoot()already reads (forwarded to the spawned child). Arg parsing + child-env layering extracted tolib/run-args.js(pure, unit-tested inlib/run-args.test.js).youtube/SKILL.md— new "Artifact landing (work root)" section: when${user_config.library_dir}is non-default, pass--work-root "${CLAUDE_PROJECT_DIR}/${user_config.library_dir}"on everyrun.mjsinvocation (call sites enumerated). Default/empty/unexpanded → no flag;resolveWorkRoot()fallback intact.setup/SKILL.mdOutput — restores the stronger "library_dirgoverns where youtube artifacts land" wording that was softened while the seam was unwired;book-distillremains the documented exception.plugin.json0.3.0→0.4.0+ CHANGELOG entry.Decision (unbriefed, called out)
run.mjs --work-root) instead of the issue's literal "exportYOUTUBE_WORK_ROOTbefore invoking" as a shell env prefixrun.mjs(issue framed the change as SKILL.md-only)YOUTUBE_WORK_ROOT=… nodeprefix is bash-only and fails under PowerShell —run.mjs's own docstring already avoidsNODE_PATH=… nodefor exactly this reason (Cross-Platform-by-Default). The in-repo idiom for wiring a${user_config.X}seam into a command is a double-quoted CLI arg (claude-troubleshootingpasses--data-dir "…").resolveWorkRoot()is left untouched per the issue.Verification
vitest run— 221/221 pass (incl. newlib/run-args.test.js)tsc --noEmit— cleanclaude plugin validate+scripts/validate-plugins.sh— passscripts/run-plugin-tests.sh— 18 checks + all plugin tests passrun.mjs --work-root X <probe>→ child seesYOUTUBE_WORK_ROOT=X; no flag → falls back toCLAUDE_PROJECT_DIR; bad flag → clean usage exit 2Refs melodic-software/medley#1466
Note
Low Risk
Additive launcher flag and skill documentation; default artifact paths unchanged when library_dir is
..Overview
/knowledge:youtubenow respectslibrary_dir. Non-default values redirect watch, transcript, and queue.work/…output under the configured directory instead of always using the repo root.The skill passes a leading
run.mjs --work-root <dir>on every extraction invocation;lib/run-args.jsparses that flag and setsYOUTUBE_WORK_ROOTfor the child so existingresolveWorkRoot()behavior applies. Default.omits the flag (unchanged fallback). Skill docs require agent-written queue/claim/output paths to use the same root so scripts and hand-edited artifacts stay aligned.setupoutput again states thatlibrary_dirgoverns YouTube artifact landing (book-distillstill documented as the exception). Plugin version 0.4.0 + CHANGELOG.Reviewed by Cursor Bugbot for commit eb2138d. Bugbot is set up for automated code reviews on this repo. Configure here.