Skip to content

feat(plugin): generate the Claude plugin tree from the monorepo (WALM-669) - #964

Open
harrymove-ctrl wants to merge 1 commit into
devfrom
harryphan/walm-634-generate-the-claude-plugin-tree-from-the-monorepo
Open

harrymove-ctrl wants to merge 1 commit into
devfrom
harryphan/walm-634-generate-the-claude-plugin-tree-from-the-monorepo

Conversation

@harrymove-ctrl

Copy link
Copy Markdown
Collaborator

The published Claude plugin repo is hand-maintained, so fixes landed here never reach it. scripts/build-claude-plugin.mjs emits the whole publishable tree — 40 files — from packages/mcp/plugin/ plus a small packages/mcp/plugin-repo/ overlay, and --check re-runs the generation to prove it is reproducible and complete. The plugin assets that had no monorepo home (commands, the setup skill, docs/usage/, README) move under packages/mcp/plugin/ and become the source of truth; LICENSE is taken from the repo root, which is already byte-identical. The secret-exclusion and auto-save rules are injected into commands/remember.md, commands/analyze.md, skills/setup/SKILL.md and the README from scripts/lib/memory-policy.mjs, so those files cannot state a weaker policy than the hooks and the relayer.

Evidence: the three files review finding 4 cites exist only in the old repo, and they carry their own weaker wording:

commands/remember.md:8  Do not save secrets, private keys, access tokens, passwords, or one-time codes.
commands/analyze.md:8   Do not save secrets, private keys, access tokens, passwords, or one-time codes.
skills/setup/SKILL.md:64-69  (CLAUDE.md snippet) "call memwal_remember ... without being asked" — no secret rule at all

Three shared files are already stale in the published repo against dev: scripts/lib/hook-io.mjs (WALM-644), scripts/install_codex_hooks.mjs (WALM-641), scripts/lib/signals.mjs. Its test/signals.test.mjs still tests detectRecall/detectRemember, which no longer exist.

Impact: nobody hits this at runtime today — the plugin repo has not been transferred or listed yet. It is the mechanism that keeps the shipped artifact from drifting again, and without the policy injection the artifact would ship the always-on automatic-memory instructions with no secret-exclusion rule, which is finding 4 unfixed in the thing users install.

The emitted .github/workflows/ci.yml deliberately does not carry the old repo's server.command === 'npx' gate, which #959 would fail. It asserts the inverse: .mcp.json / .codex-mcp.json / .cursor-mcp.json must start the server as node ${<CLIENT>_PLUGIN_ROOT}/scripts/<launcher>.mjs, that launcher must exist in the tree, the pin resolved the way pinnedVersion() resolves it must be published on npm, and the four markdown surfaces must carry SECRET_EXCLUSION_RULES verbatim. The generator does not re-derive or re-verify the npm pin: it reads mcpPackageVersion ?? version from plugin/plugin.json exactly like pinnedVersion(), and existence checking stays in scripts/check-mcp-pin.mjs (#959) and in the emitted repo's own CI.

Ordering. This stacks on #961: packages/mcp/plugin/scripts/lib/memory-policy.mjs is copied byte-identical from that branch because the generator needs it and dev has no shared policy yet. Merge #961 first and drop this copy in the rebase. It should also land after #959 — run today, the emitted tree fails its own check-plugin-tree.mjs on all three MCP configs (still npx) and on the pin 0.0.14, which npm does not carry.

Test: scripts/build-claude-plugin.test.mjs, wired into the meta matrix in test.yml. One test: generating twice is byte-identical, the tree contains the expected files, .github/CODEOWNERS is never emitted, no __MEMWAL_*__ placeholder survives in any markdown, and --check --against the emitted tree exits 0.

@harrymove-ctrl

Copy link
Copy Markdown
Collaborator Author

@ducnmm ready for review. This is the generator for the new Mysten plugin repo
(MystenLabs/walrus-memory-claude-plugin, which Thanos created empty today). It exists because the
old CommandOSSLabs/walrus-memory-mcp-plugin was maintained by hand — which is exactly why three of
Thanos's six findings were still sitting in it after we fixed them upstream. scripts/build-claude-plugin.mjs
emits the full 40-file tree, --check proves determinism and completeness, and
--check --against <dir> diffs a checkout of the published repo against a fresh generation so drift
becomes a CI failure instead of a security finding. Wired into the existing meta matrix in
test.yml, one entry.

Five things worth your attention:

1. The security-relevant part — please check this hardest. Thanos's finding 4 cites
commands/remember.md:8, commands/analyze.md and skills/setup/SKILL.md:65. Those files exist
only in the old plugin repo, nowhere in this monorepo, so #961's byte-identical policy pin across
its three memory-policy copies never reached them. Copying them across as-is would have shipped
finding 4 unfixed. The worst case is skills/setup/SKILL.md:60-73 — the block users paste into
their own ~/.claude/CLAUDE.md, which tells the agent to save "without being asked" with no secret
rule at all, and which outlives the plugin in the user's config. This PR moves those files into
packages/mcp/plugin/ as the source of truth and injects SECRET_EXCLUSION_RULES and
AUTO_SAVE_OPT_IN_RULE into all four save-driving surfaces from the single source.

2. It stacks on #961. dev has no shared policy module yet, so
packages/mcp/plugin/scripts/lib/memory-policy.mjs here is a byte-identical copy from #961's
cdade4a4 (verified with cmp). It must be dropped in the rebase once #961 lands. I chose a
green PR with a duplicated file over a red PR blocked on #961 — tell me if you'd rather the reverse.

3. 14 markdown files move into the monorepo (commands, skills/setup, docs/usage, README). That
is the structural call I would most like a second opinion on: it makes this repo the source of
truth for plugin prose, which is the point, but it does put user-facing docs in a package directory.

4. The generated CI asserts the inverse of the old gate. The old ci.yml hard-asserts
server.command === 'npx', which would reject #959's trusted launcher outright. The emitted one
instead requires node ${<CLIENT>_PLUGIN_ROOT}/scripts/<launcher>.mjs, that the launcher file
exists, that the pin is actually published on npm, that no __MEMWAL_*__ placeholder survived, and
that the four markdown surfaces carry the policy.

5. No comments in the new code, per Harry's standing rule — which departs from the header-comment
style of the neighbouring check-*.mjs scripts. Say the word if you want headers here.

Honest note on CI: the generated check-plugin-tree.mjs fails against today's dev by design —
the manifests still use npx and the 0.0.14 pin is not published (npm has latest=0.0.13,
dev=0.0.14-dev.2, no stable 0.0.14). It passes against a tree carrying #959. Nothing has been
pushed to the new repo; Harry does the transfer once the three open security PRs land and the npm
release is out.

@harrymove-ctrl harrymove-ctrl changed the title feat(plugin): generate the Claude plugin tree from the monorepo (WALM-634) feat(plugin): generate the Claude plugin tree from the monorepo (WALM-669) Sep 21, 2026
@harrymove-ctrl

Copy link
Copy Markdown
Collaborator Author

Ticket correction: this PR is WALM-669, not WALM-634 — title updated.

WALM-634 is "Transfer walrus-memory-mcp-plugin repo to MystenLabs", an admin/permissions task assigned to @ducnmm whose acceptance criteria are all about someone at Mysten running a GitHub org transfer. It contains no code work and this PR does none of what it asks. I picked it originally because it was the nearest-sounding ticket, which was wrong.

What this PR actually maps to:

Part of this PR Ticket
The generator + drift check, and moving plugin prose into the monorepo WALM-669 (new)
Injecting the secret policy into commands/*.md and skills/setup/SKILL.md WALM-642 — this is the plugin half of finding 4, which #961 does not reach
Emitting plugin.json with the MystenLabs repository URL WALM-635

WALM-634's premise is also now stale and I have corrected it separately: Thanos created a new repo (MystenLabs/walrus-memory-claude-plugin) rather than transferring the existing one, so there is no GitHub redirect from the CommandOSS URL and the canonical name differs from what that ticket assumed.

No code change here — bookkeeping only.

@harrymove-ctrl

Copy link
Copy Markdown
Collaborator Author

@ducnmm CI is 21/21 green on this and it is ready whenever you have a slot.

Quick re-orientation since the ticket changed under it: this is now WALM-669, not WALM-634. WALM-634 is a repo-configuration task assigned to you and this PR does none of it — I retitled and left a comment above explaining the mapping.

The one part I would most like a second opinion on is the structural call: 14 markdown files move into packages/mcp/plugin/ (commands, skills/setup, docs/usage, README) so the monorepo becomes the source of truth for plugin prose. That is what makes the generator possible, but it does put user-facing docs in a package directory.

The security-relevant half is the injection: Thanos's finding 4 names commands/remember.md:8, commands/analyze.md and skills/setup/SKILL.md:65, and those files live only in the plugin repo — so #961's byte-identical policy pin never reaches them. Copying them across as-is would ship finding 4 unfixed. The worst case is skills/setup/SKILL.md:60-73, the block users paste into their own ~/.claude/CLAUDE.md: it says to save "without being asked" with no secret rule at all, and it outlives the plugin in the user's config.

Still true and worth remembering at merge time: this stacks on #961, so the byte-identical copy of memory-policy.mjs here must be dropped on rebase once #961 lands.

@ducnmm ducnmm left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Summary

The generator is the right shape for WALM-669: walk packages/mcp/plugin/ plus the overlay, substitute policy/pin from one module, refuse CODEOWNERS/collisions/forbidden paths, and --check --against names missing/extra/differing files. The SKILL.md snippet now carries both rules. The emitted check-plugin-tree.mjs asserts the WALM-640 node ${PLUGIN_ROOT}/scripts/<launcher>.mjs shape rather than npx. No current correctness bug in the generated tree; the holes below are CI completeness and rebase leftovers (#959/#961).

Issue counts by severity

  • bugs: 0
  • suggestions: 5
  • nits: 1

assert.ok(!paths.includes(".github/CODEOWNERS"), "CODEOWNERS belongs to the published repository");

for (const target of paths.filter((name) => name.endsWith(".md"))) {
assert.doesNotMatch(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[suggestion] This job only asserts placeholders are gone. Deleting __MEMWAL_SECRET_POLICY__ from commands/remember.md (or the CLAUDE.md span in skills/setup/SKILL.md) still produces a tree with no leftover __MEMWAL_*__ tokens, so this loop and --check --against of a just-generated tree both pass. The emitted check-plugin-tree.mjs would catch a missing secret block, but MemWal CI does not run it, and MystenLabs/walrus-memory-claude-plugin is still empty.

Suggestion: After generating, import SECRET_EXCLUSION_RULES / AUTO_SAVE_OPT_IN_RULE from the generated scripts/lib/memory-policy.mjs and assert they appear in commands/remember.md, commands/analyze.md, skills/setup/SKILL.md (including the <!-- memwal:start --> span) and README.md. Also exec the generated .github/scripts/check-plugin-tree.mjs with cwd set to the output tree (after the #959 rebase).

/\.tgz$/,
];

const REQUIRED = [

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[suggestion] REQUIRED is the completeness list --check claims to enforce, but it omits files the walk still copies today: scripts/install_codex_hooks.mjs and scripts/lib/hook-template.mjs. Deleting either still yields a "complete" tree; docs/usage/codex.md Option B would then tell users to run a script that is not there. After rebase onto #959/#961 the same list will also omit scripts/launch_mcp.mjs and scripts/lib/mcp-launch.mjs, so --check will not notice if the WALM-640 launcher is removed from the source tree.

Suggestion: Make REQUIRED the full emitted set (or derive it from the walk and a small denylist). On rebase, include the trusted launcher and its helper explicitly.

".codex-mcp.json": "PLUGIN_ROOT",
".cursor-mcp.json": "CURSOR_PLUGIN_ROOT",
})) {
if (!existsSync(file)) continue;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[suggestion] Missing .codex-mcp.json / .cursor-mcp.json is continue, not a failure, so those two WALM-640 configs are optional. commands/analyze.md is in the policy loop but not the required-files list, and that loop also continues on absence. Placeholder and SECRET_EXCLUSION_RULES checks cover only four markdown files (SETUP.md and docs/usage/* can carry a leftover __MEMWAL_*__ after a hand-edit). AUTO_SAVE_OPT_IN_RULE is never imported or asserted.

Suggestion: Require the three MCP configs and commands/analyze.md. Fail closed on missing files. Scan the whole tree for __MEMWAL_*__. Assert AUTO_SAVE_OPT_IN_RULE on skills/setup/SKILL.md and README.md, specifically inside the <!-- memwal:start --> … <!-- memwal:end --> span.


If `$ARGUMENTS` is empty, ask the user what they want remembered.

__MEMWAL_SECRET_POLICY__

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[suggestion] WALM-669 says both SECRET_EXCLUSION_RULES and AUTO_SAVE_OPT_IN_RULE are injected into commands/remember.md, commands/analyze.md, and skills/setup/SKILL.md. Only __MEMWAL_SECRET_POLICY__ is in the two slash commands. That is probably the right product call — a /remember invocation is an explicit request — but then auto-save still needs a real assertion on SKILL.md / README, which are the surfaces that outlive uninstall.

Suggestion: Keep SECRET-only on the explicit-save commands if that is intentional, and say so. Assert AUTO_SAVE_OPT_IN_RULE on skills/setup/SKILL.md (especially the paste-into-CLAUDE.md block) and README.md. Do not put the opt-in rule under README's "What is never saved" heading; it is a standing user choice, not an exclusion list.

]
],
"mcpServers": "./.mcp.json",
"skills": "./skills/",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[suggestion] These pointers are redundant with Claude Code's defaults. Claude's plugin spec adds skills paths to the default skills/ scan rather than replacing it, so "skills": "./skills/" can register the setup skill twice. "commands": "./commands/" replaces the default scan with the same directory (harmless but unnecessary). check-plugin-tree.mjs then requires those keys to exist, which locks in the redundant pointers.

Suggestion: Drop the four pointers and have check-plugin-tree.mjs exist-check the default layout. If a pointer is kept, do not set skills to the default directory. Run claude plugin validate . --strict against a generated tree before merge.


Walrus Memory gives Claude Code durable, user-owned memory. This plugin packages the published [`@mysten-incubation/memwal-mcp`](https://www.npmjs.com/package/@mysten-incubation/memwal-mcp) server with setup guidance, slash commands, and lifecycle hooks.

This tree is generated from [`MystenLabs/MemWal/packages/mcp/plugin`](https://github.com/MystenLabs/MemWal/tree/dev/packages/mcp/plugin) by `scripts/build-claude-plugin.mjs`. Do not hand-edit a published copy: change it in MemWal and regenerate, or the fix will be lost at the next build.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[nit] The "source of truth" link is hardcoded to MemWal/tree/dev/packages/mcp/plugin. A generation from main or a release tag will still advertise dev.

Suggestion: Point at /tree/HEAD/packages/mcp/plugin or at the repo root without a branch, or substitute a placeholder the generator fills from the build ref.

…-634)

The published plugin repo was hand-maintained, so three of the six findings
from the 17 Sep review were still sitting in it after being fixed here.
scripts/build-claude-plugin.mjs emits the whole publishable tree from
packages/mcp/plugin plus a small plugin-repo overlay, so nothing in the
published repo is ever edited by hand again.

The plugin assets that had no monorepo home — commands, the setup skill,
docs/usage and the README — move under packages/mcp/plugin and become the
source of truth. The secret-exclusion and auto-save rules are injected into
commands/remember.md, commands/analyze.md, skills/setup/SKILL.md and the
README from scripts/lib/memory-policy.mjs, so the markdown cannot state a
weaker policy than the hooks and the relayer.
@harrymove-ctrl
harrymove-ctrl force-pushed the harryphan/walm-634-generate-the-claude-plugin-tree-from-the-monorepo branch from 44b1ea9 to 21aab02 Compare September 25, 2026 06:46
@harrymove-ctrl

Copy link
Copy Markdown
Collaborator Author

Rebased onto current dev and re-verified — ready for review.

  • node scripts/build-claude-plugin.mjs --check → claude plugin tree OK; 44 files, pinned to the plugin manifest. The count moved 40 → 44 against the commands/docs that landed on dev since this branch was cut, which is exactly the drift this generator exists to catch.
  • node scripts/build-claude-plugin.test.mjs → 1/1 pass (deterministic output, complete tree, no placeholders).

This branch has not been deployed

No deployments
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.

3 participants