Skip to content

fix(knowledge): resolve the plugin data dir explicitly instead of trusting the Bash env - #6004

Merged
cursor[bot] merged 5 commits into
mainfrom
agent-a4b2f59daea66d5a7
Oct 3, 2026
Merged

cursor[bot] merged 5 commits into
mainfrom
agent-a4b2f59daea66d5a7

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

No related issue: partially addresses #5982 (resolves E1, Q2; E2, E3, I1, I2, I3, Q1, S2 remain open)

Summary

/knowledge:video-digest installed its extraction dependencies into another plugin's data directory. The Bash tool's environment never carries a plugin's CLAUDE_PLUGIN_DATA (plugins reference, fetched 2026-10-02). The codex plugin's SessionStart hook (scripts/session-lifecycle-hook.mjs:80, codex 1.0.6) appends export CLAUDE_PLUGIN_DATA=<codex data dir> to CLAUDE_ENV_FILE, so every Bash call saw codex's directory. setup-deps.mjs wrote node_modules/ and a stamp there, and run.mjs resolved @melodic/* from it. course-digest shares the same data directory and had the same defect.

Fix

  • run.mjs and setup-deps.mjs in video-digest and course-digest take a leading --data-dir. Every documented command passes it as "${CLAUDE_PLUGIN_DATA}", which is substituted when the skill loads, or as "<plugin-data>" in the spoke files and the watch checklist template.
  • Without the flag, an inherited value is accepted only when its last path segment names this plugin. This is the order docs/conventions/on-demand-dependencies sets.
  • An unsubstituted placeholder fails loudly.
  • setup-deps.mjs exits before writing anything when no directory resolves. run.mjs strips the inherited value and hands its child only the resolved one.
  • The pre-computed dependency checks read the substituted path from argv instead of the environment.
  • The bootstrap recovery command names the launcher by absolute path and carries the resolved --data-dir. It used to emit a literal ${CLAUDE_PLUGIN_ROOT}, which expands to nothing in Bash.
  • knowledge 0.18.1, with a CHANGELOG entry.

Verification

All commands ran on Windows (Git Bash) after merging origin/main:

  • bash plugins/knowledge/skills/video-digest/scripts/run-tests.sh build: exit 0.
  • bash plugins/knowledge/skills/video-digest/scripts/run-tests.sh test: exit 0, 559 passed.
  • bash plugins/knowledge/skills/course-digest/scripts/run-tests.sh build: exit 0.
  • bash plugins/knowledge/skills/course-digest/scripts/run-tests.sh test: exit 1, 129 passed and 27 failed. All 27 failures are in cli-entry.test.js and cli-browser-artifacts.test.js, which this PR does not touch, and they predate it. Those tests build paths with new URL(import.meta.url).pathname, which yields C:\C:\... on Windows and fails module resolution. All 13 new course-digest tests pass.
  • Regression tests:
    • setup-deps.mjs with a foreign CLAUDE_PLUGIN_DATA (codex-openai-codex) and no flag exits 1 and writes nothing there.
    • run.mjs --data-dir overrides a foreign inherited value.
    • A flag-less course run.mjs call leaves the foreign directory empty. With the strip removed, this test fails.
  • The rewritten pre-computed check reports installed for a populated sandbox directory and MISSING for an empty one.
  • Gates, all exit 0:
    • scripts/check-changed-skills.sh origin/main
    • scripts/check-changelog-parity.sh --check, --check-order, --check-bump origin/main
    • scripts/check-spoke-plugin-root.sh --check
    • scripts/check-test-tmp-cleanup.sh --check
    • scripts/check-purged-em-dashes.sh
    • markdownlint-cli2, typos, editorconfig-checker on the changed files

Related

How this PR maps to the findings in #5982:

Finding State after this PR Basis
E1, data dir unreachable from the Bash tool Resolved E1 option 1: --data-dir on setup-deps.mjs and run.mjs. run.mjs passes it to the child as CLAUDE_PLUGIN_DATA, so resolve-hook.mjs resolves from it. Every SKILL.md invocation passes "${CLAUDE_PLUGIN_DATA}". Option 2 too: the preflight probe reads the substituted path. course-digest is fixed the same way
E2, ${user_config.*} unsubstituted in spokes Open Not touched
E3, compound ! preflight refused, no fallback Partly The node probe no longer always reports MISSING. The block is still compound, with no allowed-tools entry and no fallback line
I1, no Bootstrap STOP for node deps Open setup-deps.mjs now fails loudly, but the Bootstrap gate's STOP list is unchanged
I2, no install-location override Partly --data-dir accepts any directory, which relocates the install. There is no Prerequisites line on when to use it, and no .work/ rung
I3, read-only check unreachable Open Not touched
Q1, install target not announced Open Not touched
Q2, misleading setup-deps message Resolved The message now names the missing flag, and says an inherited value that names another plugin is ignored
S2, environment-only resolution Partly Rungs 1 and 2 of the convention's order: the flag, then the inherited value only when it names this plugin. The .work/ and config-dir rungs are not implemented
  • Handoff item 20261003-034047 (local inbox): this defect.
  • Handoff item 20261003-045221 (local inbox): the same env-read pattern in agent-invoked scripts of other plugins (repo-hygiene, harness-ops, session-flow, source-control, code-metrics, ai-briefing). It is out of scope here because each plugin needs its own version bump.

🤖 Generated with Claude Code

kyle-sexton and others added 3 commits October 3, 2026 00:14
…itly

The Bash tool's environment does not carry CLAUDE_PLUGIN_DATA, and the
codex plugin's SessionStart hook appends its own data dir under that
name to CLAUDE_ENV_FILE, so every Bash call saw codex's directory.
video-digest's setup-deps.mjs then installed the extraction deps there,
and run.mjs resolved @melodic/* from it.

run.mjs and setup-deps.mjs (video-digest and course-digest) now take a
leading --data-dir. Every documented command passes it as
"${CLAUDE_PLUGIN_DATA}", substituted at skill load, or "<plugin-data>"
in the spoke files. Without the flag, an inherited value is accepted
only when its last segment names this plugin, per the
on-demand-dependencies convention. setup-deps.mjs stops before writing
when nothing resolves, and run.mjs hands its child only the resolved
value. The pre-computed checks read the substituted path from argv, and
the recovery command names the launcher by absolute path with the
resolved --data-dir.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
A flag-less run.mjs call with another plugin's CLAUDE_PLUGIN_DATA must
leave that directory untouched; the child falls back to the auth-store
home default instead. The test fails when the launcher's strip of the
inherited value is removed.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts:
#	plugins/knowledge/.claude-plugin/plugin.json
#	plugins/knowledge/CHANGELOG.md
# Conflicts:
#	plugins/knowledge/.claude-plugin/plugin.json
#	plugins/knowledge/CHANGELOG.md

Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
@cursor
cursor Bot marked this pull request as ready for review October 3, 2026 07:15
The machine-path lint rejects /home and C:/Users prefixes. The assertions
only depend on the last path segment, so the fixtures use /var/fixture and
C:/fixture.

Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
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.

2 participants