From b7b37e7169c21c1796dbe8331fa686f38bc74cda Mon Sep 17 00:00:00 2001 From: Louis Choquel Date: Sat, 26 Sep 2026 17:45:43 +0200 Subject: [PATCH 01/10] Add make targets that run the plugin against a local pipelex-mcp workshop `make claude-local-mcp` renders the Claude target, with the build's own renderer and the workshop's command and args laid over [vars.mcp_server], into the ignored .local-mcp/pipelex/ and starts `claude --plugin-dir` on it. `make codex-local-mcp` starts Codex with -c overrides of its pipelex entry, env_vars included, since such an entry replaces the plugin's whole. MCP names a pipelex-mcp checkout, built first with its `make build-local`; MCP_VERSION a published version, resolved by npm before anything starts. Neither touches a tracked file. The /pipelex-mcp-source skill keeps its status report and names the targets instead of editing targets/defaults.toml, and the install page's one-invocation Codex override now forwards the key. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_014AcpM2qUzbWMenP62iSBoS --- .claude/skills/pipelex-mcp-source/SKILL.md | 82 ++--- .gitignore | 2 + CHANGELOG.md | 5 + CLAUDE.md | 7 +- Makefile | 33 +- docs/build-targets.md | 6 +- docs/decisions.md | 13 + docs/development.md | 20 +- docs/install.md | 2 +- scripts/gen_skill_docs.py | 38 ++- scripts/local_mcp.py | 292 +++++++++++++++++ targets/defaults.toml | 10 +- tests/unit/test_local_mcp.py | 346 +++++++++++++++++++++ 13 files changed, 783 insertions(+), 73 deletions(-) create mode 100644 scripts/local_mcp.py create mode 100644 tests/unit/test_local_mcp.py diff --git a/.claude/skills/pipelex-mcp-source/SKILL.md b/.claude/skills/pipelex-mcp-source/SKILL.md index a147dd01..5b5acc54 100644 --- a/.claude/skills/pipelex-mcp-source/SKILL.md +++ b/.claude/skills/pipelex-mcp-source/SKILL.md @@ -1,78 +1,81 @@ --- name: pipelex-mcp-source description: > - Inspects and switches which `pipelex-mcp` build the plugin's declared MCP - server spawns — npm `@latest`, a pinned npm version, or a local - `../pipelex-mcp` checkout — and reports which version each deployment - actually serves (npm, the local build, and the hosted Alpic console). Use this - skill whenever the user says "bump pipelex-mcp", "which MCP version am I - using", "switch the MCP source", "test against my local pipelex-mcp", "pin the - MCP version", "point the plugin at the local workshop", "go back to the + Reports which `pipelex-mcp` version each deployment actually serves (npm, a + local `../pipelex-mcp` build, and the hosted Alpic console), says which make + target runs the plugin against another workshop — a local checkout or a + pinned npm version — without touching a tracked file, and changes the + workshop the plugin ships when that is really wanted. Use this skill + whenever the user says "bump pipelex-mcp", "which MCP version am I using", + "switch the MCP source", "test against my local pipelex-mcp", "pin the MCP + version", "point the plugin at the local workshop", "go back to the released MCP", "is the console up to date", or mentions the hosted console / - Alpic deployment / `@pipelex/mcp` npm package in the context of versions. Also - use it after `pipelex-mcp` ships a release, when an MCP-backed skill + Alpic deployment / `@pipelex/mcp` npm package in the context of versions. + Also use it after `pipelex-mcp` ships a release, when an MCP-backed skill (`pipelex-design`, `pipelex-organize`, `pipelex-edit`, `pipelex-inputs`) - behaves unexpectedly and the server version is suspect, or before committing, - to check that no temporary dev switch leaked into the tree. + behaves unexpectedly and the server version is suspect, or before + committing, to check that no hand-made switch leaked into the tree. --- -# pipelex-mcp source switcher +# pipelex-mcp source -Controls which `pipelex-mcp` build this plugin's declared MCP server spawns, and reports the version every deployment is actually serving. +Reports the version every deployment of `pipelex-mcp` is actually serving, and says how to run this plugin against a workshop other than the one it ships. ## The mental model -`targets/defaults.toml` `[vars.mcp_server]` is the **single source of truth**. `make build` fans it out to every generated artifact: +`targets/defaults.toml` `[vars.mcp_server]` is the **single source of truth** for the workshop the plugin ships. `make build` fans it out to every generated artifact: -- `pipelex/.claude-plugin/plugin.json` and `pipelex-codex/.codex-plugin/plugin.json` — the `mcpServers.pipelex` entry the harness spawns +- `pipelex/.claude-plugin/plugin.json` and `pipelex-codex/.codex-plugin/plugin.json` — the `mcpServers.pipelex` entry the harness spawns (on Claude, through the `hooks/launch-pipelex-mcp.sh` wrapper, whose `exec` line carries the command) - `pipelex-vibe/mcp/vibe-mcp.toml` — the `[[mcp_servers]]` fragment Vibe users copy into `~/.vibe/config.toml`, since Vibe has no manifest - every MCP-backed `SKILL.md` across all three targets — their "server isn't connected" line renders `{{ mcp_server.command }} {{ mcp_server.args | join(" ") }}`, so the quoted launcher tracks the config automatically -That fan-out is why a switch is never a one-file edit, and why it shows up as a wide diff. Prose docs (`docs/install.md`, `docs/development.md`, `docs/decisions.md`, `docs/build-targets.md`, `CLAUDE.md`) quote the launcher too, but they describe **what ships** — see "Changing the shipped default" for the only case where they move. +That fan-out is why editing the block is never a one-file change, and why it shows up as a wide diff. Prose docs (`docs/install.md`, `docs/development.md`, `docs/decisions.md`, `docs/build-targets.md`, `CLAUDE.md`) quote the launcher too, but they describe **what ships** — see "Changing the shipped default" for the only case where they move. -The critical distinction this skill exists to protect: **a dev switch is temporary local state, not a change to the plugin.** It dirties generated files across every target, and a pinned version or an absolute local path is meaningless on anyone else's machine. It must never reach a commit. +**Running against another workshop never edits that block.** `make claude-local-mcp` and `make codex-local-mcp` start the agent on the workshop of a local checkout or of a pinned npm version and change no tracked file, so nothing has to be switched back and nothing can leak into a commit (`docs/development.md`, "A local build of `pipelex-mcp`"). An edit of the block that is not a change to what ships is a hand-made switch: a pinned version or an absolute local path means nothing on anyone else's machine, and it must never reach a commit. ## The three sources All three are stdio — the renderer emits only `command`/`args`, and the hosted console is deliberately not bakeable (see "The hosted console"). -| Source | `command` | `args` | Status | +| Source | `command` | `args` | How it runs | |---|---|---|---| | `npm-latest` | `npx` | `["-y", "@pipelex/mcp@latest"]` | **What ships.** The committed default. | -| `npm-pinned` | `npx` | `["-y", "@pipelex/mcp@X.Y.Z"]` | Dev only — reproduce a specific released version. | -| `local` | `node` | `["/pipelex-mcp/packages/workshop/dist/main.js"]` | Dev only — test unreleased changes. | +| `npm-pinned` | `npx` | `["-y", "@pipelex/mcp@X.Y.Z"]` | `make claude-local-mcp MCP_VERSION=X.Y.Z`, or `codex-local-mcp` — reproduce a released version. | +| `local` | `node` | `["/pipelex-mcp/packages/workshop/dist/main.js"]` | `make claude-local-mcp MCP=`, or `codex-local-mcp` — test unreleased changes. | -`pipelex-mcp` is an npm workspace: the workshop is its `packages/workshop` member, and `make build-local`, run at the root of that checkout, bundles it into `packages/workshop/dist/main.js` with the shared capability core in `packages/core` inlined. **The old path, `dist/local/main.js`, predates the workspace split and is never rebuilt.** A checkout that built before the split still carries that file, and `make clean` there no longer removes it, so a switch still pointing at it spawns a stale workshop without any error. +`pipelex-mcp` is an npm workspace: the workshop is its `packages/workshop` member, and `make build-local`, run at the root of that checkout, bundles it into `packages/workshop/dist/main.js` with the shared capability core in `packages/core` inlined. **The old path, `dist/local/main.js`, predates the workspace split and is never rebuilt.** A checkout that built before the split still carries that file, and `make clean` there no longer removes it, so anything still pointing at it — a Vibe entry, a hand-made switch — spawns a stale workshop without any error. -`@latest` is the shipped default on purpose: `npx` re-resolves the dist-tag per spawn, so users track releases with no plugin bump, and `docs/decisions.md` records that pinning buys no offline resilience (npx contacts the registry even for cached exact specs). Pinning here is a **testing tool**, not a release posture. If the user asks to ship a pin, say that it contradicts that recorded decision and ask them to confirm before proceeding — they may have a good reason, but it should be a deliberate reversal with a `docs/decisions.md` amendment, not a side effect of this skill. +`@latest` is the shipped default on purpose: `npx` re-resolves the dist-tag per spawn, so users track releases with no plugin bump, and `docs/decisions.md` records that pinning buys no offline resilience (npx contacts the registry even for cached exact specs). Pinning is a **testing tool**, not a release posture. If the user asks to ship a pin, say that it contradicts that recorded decision and ask them to confirm before proceeding — they may have a good reason, but it should be a deliberate reversal with a `docs/decisions.md` amendment, not a side effect of this skill. ## Status — the default action When the user invokes this skill without naming a target, report current state and stop. Gather in parallel: -1. **Declared source** — read `[vars.mcp_server]` from `targets/defaults.toml`. If its `args` name `dist/local/main.js`, say that this is the pre-split path, which runs a stale build, and offer to switch it to `packages/workshop/dist/main.js`. +1. **Declared source** — read `[vars.mcp_server]` from `targets/defaults.toml`. Anything but `npm-latest` is a hand-made switch; see "Restoring the shipped default". If its `args` name `dist/local/main.js`, say too that this is the pre-split path, which runs a stale build. 2. **npm `latest`** — `npm view @pipelex/mcp dist-tags --json` (and `npm view @pipelex/mcp versions --json` if they need the list of published versions to pin to). 3. **Console live version** — the probe in "Proving what's actually running". The console has a release track of its own, so compare it as "The hosted console" says, never with npm `latest`. 4. **Local checkout version** — `node -p "require('../pipelex-mcp/packages/workshop/package.json').version"`. That manifest is the one npm publishes as `@pipelex/mcp`. Never read the root `package.json`, which carries no version since the workspace split, nor `packages/console/package.json`, which versions the console on a track npm never sees. A version behind npm `latest` means the checkout needs a pull. A version equal to it is the ordinary case on `dev`, since the version moves only at a release: what the checkout has that npm lacks is listed under `## [Unreleased]` in `../pipelex-mcp/packages/workshop/CHANGELOG.md`, and an empty section there means the local source tests nothing new. -5. **Local build** — does `../pipelex-mcp/packages/workshop/dist/main.js` exist, and is it stale? The bundle inlines the capability core, and its handshake reports the version the workshop's manifest had at build time, so it lags whenever either package's sources or that manifest are newer: `find ../pipelex-mcp/packages/core/src ../pipelex-mcp/packages/workshop/src ../pipelex-mcp/packages/workshop/package.json -type f \( -name '*.ts' -o -name package.json \) ! -name '*.test.ts' ! -name '*.e2e.ts' -newer ../pipelex-mcp/packages/workshop/dist/main.js` printing anything means the build lags; offer `make build-local` at the root of `../pipelex-mcp`. -6. **Leaked dev state** — `git diff --stat targets/defaults.toml` and `git status --porcelain pipelex/ pipelex-codex/ pipelex-vibe/`. If the declared source is not `npm-latest`, lead with that: the tree is carrying a dev switch. +5. **Local build** — does `../pipelex-mcp/packages/workshop/dist/main.js` exist, and is it stale? The bundle inlines the capability core, and its handshake reports the version the workshop's manifest had at build time, so it lags whenever either package's sources or that manifest are newer: `find ../pipelex-mcp/packages/core/src ../pipelex-mcp/packages/workshop/src ../pipelex-mcp/packages/workshop/package.json -type f \( -name '*.ts' -o -name package.json \) ! -name '*.test.ts' ! -name '*.e2e.ts' -newer ../pipelex-mcp/packages/workshop/dist/main.js` printing anything means the build lags. A lagging build matters only to a running session or a Vibe entry: the make targets rebuild before they start. +6. **Leaked dev state** — `git diff --stat targets/defaults.toml` and `git status --porcelain pipelex/ pipelex-codex/ pipelex-vibe/`. If the declared source is not `npm-latest`, lead with that: the tree is carrying a hand-made switch. +7. **The local copy** — when `.local-mcp/pipelex/hooks/launch-pipelex-mcp.sh` exists, its `exec` line says which workshop the last `make claude-local-mcp` in this checkout rendered. It is ignored by git and never shipped, so it is information, not dev state to clean up. Present it as a short table, not prose. The useful signal is usually a *mismatch* — declared source vs what npm serves vs what the local build contains — so state plainly whether they agree, and if the user is on `npm-latest` and the checkout's workshop version equals npm `latest` with nothing under `## [Unreleased]`, say there is nothing to bump. -## Switching to a dev source +## Running against another workshop -1. **Confirm the target** with the user if ambiguous. For a pin, validate the version exists (`npm view @pipelex/mcp versions --json`) — a typo'd pin fails at spawn time with a confusing npx error, long after this skill has finished. -2. For `local`, verify `packages/workshop/dist/main.js` exists and is not stale (Status, step 5); run `make build-local` at the root of `../pipelex-mcp` if it is missing or stale. Use an **absolute** path — the server spawns with the *host's* working directory, not the plugin's, so a relative path resolves somewhere unintended. -3. Edit only `command` and `args` in `targets/defaults.toml` `[vars.mcp_server]`. Leave `env_vars` and every `user_config` table alone — credential delivery is orthogonal to which build gets spawned, and the local workshop needs the same key. -4. Run `make build`, then `make check`. -5. Tell the user to `/reload-plugins` (Claude Code). On Codex add `make codex-refresh` — installed plugins run from a cache copy, so a rebuild alone does not reach the running harness. Vibe has no manifest: `make build` regenerates `pipelex-vibe/mcp/vibe-mcp.toml` and the launcher quoted in Vibe's skill prose, but a running Vibe keeps whatever entry the user copied into `~/.vibe/config.toml` until they replace that entry with the new one. Adding the new entry beside the old one stops Vibe from starting, because Vibe refuses two servers of the same name. -6. **Close with the revert reminder** and name the dirtied paths. This is the step that keeps a dev switch from shipping. +Name the target and let the user start it from a terminal: it starts an interactive agent in place of the make process, which a session's own shell cannot host. + +- A local checkout or worktree: `make claude-local-mcp MCP=` (default `../pipelex-mcp`), or `make codex-local-mcp MCP=`. The target runs `make build-local` there first. +- A released version: `make claude-local-mcp MCP_VERSION=X.Y.Z`, or the Codex twin. The target refuses a version npm does not know before anything starts. +- `WORKDIR=` starts the session in a project, since the workshop resolves `{ path }` files against it; `ARGS='…'` goes to `claude` or `codex` as it is. + +A headless prompt is the one form an agent can run itself, and it is how to dogfood a workshop change from inside a session: `make claude-local-mcp MCP= WORKDIR= ARGS='-p "…" --model sonnet'`. The workshop takes its key from the shell's `PIPELEX_API_KEY` in both targets, and the target warns when it is unset. On Claude Code, the rendered copy loads as `pipelex@inline` in place of an installed `pipelex@pipelex-plugins`, so its skills are this checkout's. On Codex, the skills are the installed copy's: pair the target with `make codex-use-local` to run this checkout's. Mistral Vibe has no target; `docs/development.md` says which entry to edit. ## Restoring the shipped default -Restore `[vars.mcp_server]` to `npx` + `["-y", "@pipelex/mcp@latest"]`, then `make build` and `make check`. +Only a hand-made switch needs this. Restore `[vars.mcp_server]` to `npx` + `["-y", "@pipelex/mcp@latest"]`, then `make build` and `make check`. -Prefer `git show origin/main:targets/defaults.toml` as the reference for what actually ships rather than assuming — if the shipped default ever moves, that reads the truth instead of a stale literal. Fall back to `HEAD` when `origin/main` is unavailable, and to the literal above if both disagree with it (which would itself mean a dev switch got committed — worth flagging). +Prefer `git show origin/main:targets/defaults.toml` as the reference for what actually ships rather than assuming — if the shipped default ever moves, that reads the truth instead of a stale literal. Fall back to `HEAD` when `origin/main` is unavailable, and to the literal above if both disagree with it (which would itself mean a switch got committed — worth flagging). Read that reference, but restore by **editing the two fields back**, not by copying the whole file over `targets/defaults.toml`. Concurrent sessions may share this checkout and a whole-file copy silently discards their unrelated edits — including a `user_config` or `env_vars` change someone is mid-way through. @@ -82,7 +85,7 @@ Verify the generated outputs came back clean: `git status --porcelain targets/de Rare, and the only case where prose docs move. This is a real change to the plugin — a new package name, a different launcher command, or a deliberate reversal of the `@latest` posture. -Do the switch steps above, then propagate to the docs that quote the launcher as *current fact*: `docs/install.md`, `docs/development.md`, `docs/decisions.md`, `docs/build-targets.md`, `CLAUDE.md`. Grep for `@pipelex/mcp@` and `npx -y @pipelex` to find them rather than trusting this list. +Edit only `command` and `args` in `targets/defaults.toml` `[vars.mcp_server]`, leaving `env_vars` and every `user_config` table alone, since credential delivery is orthogonal to which build gets spawned. Run `make build`, then `make check`. Then propagate to the docs that quote the launcher as *current fact*: `docs/install.md`, `docs/development.md`, `docs/decisions.md`, `docs/build-targets.md`, `CLAUDE.md`. Grep for `@pipelex/mcp@` and `npx -y @pipelex` to find them rather than trusting this list. **Do not rewrite** `CHANGELOG.md`, `TODOS.md`, or anything under `wip/` — those are historical records of what was true at the time, and editing them destroys the record. Add a new `CHANGELOG.md` entry describing the change instead. Amend `docs/decisions.md` where the change contradicts a recorded decision, so the reasoning stays discoverable; this repo treats decisions as durable, so supersede the entry with the new rationale rather than deleting it. @@ -120,13 +123,14 @@ curl -s -X POST "https://pipelex-mcp-a3c6a115.alpic.live/mcp" \ The local probe needs no API key, because the workshop's handshake precedes auth. **The console probe no longer answers keyless**: since console OAuth became its only auth posture, an unauthenticated `initialize` gets `401` with a `www-authenticate: Bearer … resource_metadata="…/.well-known/oauth-protected-resource"` header. That response still proves the console is up and signing callers in, but it carries no `serverInfo.version`; read the console's version from a signed-in connector session instead. A first-ever `npx` spawn can take ~10s while the cache populates; warm spawns are ~1s, so allow a generous timeout before calling it broken. -The session's own connected server is a separate question from either probe: it was spawned at session start, so it reflects the config as of *then*. After a switch, the running server is still the old one until the harness reloads — which is why the reload step is not optional. +The session's own connected server is a separate question from either probe: it was spawned at session start, so it reflects the config as of *then*. A session started by one of the make targets runs the workshop the target named for as long as it lasts; another workshop needs a new session. ## Rules -- `targets/defaults.toml` is the only file to hand-edit. Never edit a generated `plugin.json` or a generated `SKILL.md` — `make build` overwrites them. -- Always `make build` then `make check` after touching any `[vars.mcp_server]` field. A switch that skips the build leaves the config and the manifests disagreeing, which is worse than either state alone. +- Running against another workshop is a make target, never an edit of `targets/defaults.toml`. +- `targets/defaults.toml` is the only file to hand-edit, and only to change what ships or to restore it. Never edit a generated `plugin.json` or a generated `SKILL.md` — `make build` overwrites them. +- Always `make build` then `make check` after touching any `[vars.mcp_server]` field. An edit that skips the build leaves the config and the manifests disagreeing, which is worse than either state alone. - Never `git add .` or `git add -A` — other sessions may share this checkout. If the user asks to commit a **shipped-default** change, stage the specific files. -- A dev switch is never committed. If asked to commit while one is active, stop and offer to restore first. +- A hand-made switch is never committed. If asked to commit while one is in the tree, stop and offer to restore first. - Touch only `command`/`args`; leave `env_vars` and `user_config` alone. - Report versions you have probed, not versions you have inferred. If a probe fails, say it failed rather than falling back to what config claims. diff --git a/.gitignore b/.gitignore index 1219eed2..0c82df23 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,5 @@ temp/ .pytest_cache/ .mypy_cache/ .ruff_cache/ +# The plugin copy `make claude-local-mcp` renders for a local workshop. +.local-mcp/ diff --git a/CHANGELOG.md b/CHANGELOG.md index ac861b45..9caadb0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,14 @@ ## [Unreleased] +### Added + +- **`make claude-local-mcp` and `make codex-local-mcp`**: one command starts Claude Code or Codex with the Pipelex tools of a local `pipelex-mcp` checkout, `MCP=`, which it builds first, or of a published version, `MCP_VERSION=`, and changes no file git tracks. Claude Code runs this checkout's skills from a copy rendered into the ignored `.local-mcp/` directory, loaded for that session in place of the installed plugin; Codex keeps its installed skills and has its `pipelex` server entry overridden. Both take the key from `PIPELEX_API_KEY` in the shell and warn when it is unset. The repository's `/pipelex-mcp-source` skill no longer edits `targets/defaults.toml` to switch workshops: it reports which version each deployment serves and names the target to run. + ### Fixed - **`pipelex-explain` leaves no method graph page behind**: the Pipelex tools are about to write a method's flowchart, `method-graph.html`, beside the files a validation reads by path, which would have left a file in the bundle every time `pipelex-explain`, a skill that writes nothing, checked a method on disk. The skill now asks the tools not to write the page, and so does `pipelex-catalog` when it checks whether a bundle would save, since that check is a question and writes no file either. +- **The install page's Codex override keeps the key**: an entry of the same name in `~/.codex/config.toml`, or a `-c` override, replaces the plugin's entry whole, including the variable names it forwards, so the one-invocation `-c` form the page gave started the Pipelex tools without `PIPELEX_API_KEY`. The page now says an override passes the key only through its own `env` table or `env_vars`, and gives the `-c` form with `env_vars`. ## [0.9.2] - 2026-09-25 diff --git a/CLAUDE.md b/CLAUDE.md index 7f8c950f..ef1dfbb7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -83,7 +83,8 @@ scripts/ ├── gen_skill_docs.py # Template renderer (multi-target) ├── check.py # Validation / freshness / packaging checks, the hook bundle's provenance guard included ├── check_hook_fresh.py # Release gate: the vendored check.mjs against npm's latest engine and a rebuild in ../pipelex-sdk-js -└── hook_bundle.py # Reads the hook bundle's provenance banner and compares bundles below it +├── hook_bundle.py # Reads the hook bundle's provenance banner and compares bundles below it +└── local_mcp.py # Starts Claude Code or Codex on another pipelex-mcp workshop (`make claude-local-mcp` / `codex-local-mcp`) tests/unit/ # Unit tests for renderer + checks, and the hook's sweep over the corpus tests/data/mthds-corpus/ # Vendored MTHDS Test Corpus (generated by the workspace's corpus sync; never edited here) tests/recipes/ # Opt-in: executes the synthetic-inputs recipes (`make test-recipes`) @@ -111,6 +112,8 @@ make agent-test # Run unit tests quietly (output only on failure) — prefe make gen-skill-docs # Build default target (prod); use TARGET=codex for others make vendor-hook # Rebuild check.mjs in ../pipelex-sdk-js and copy it into templates/hooks/assets/ make check-hook-fresh # Release gate: fail when check.mjs is behind npm's tools-wasm or a rebuild in ../pipelex-sdk-js +make claude-local-mcp # Start Claude Code on this checkout's skills with another workshop: MCP= or MCP_VERSION=x.y.z +make codex-local-mcp # The same for Codex, overriding only its pipelex server entry; neither touches a tracked file ``` ### Editing workflow @@ -177,6 +180,6 @@ So there is nothing to enable — the bundled hook loads on its own (hooks are S ## Key dependency -The plugin imports nothing and requires no install. Validation rides on the vendored `check.mjs` bundle (wasm engine + `@pipelex/sdk` → hosted API) and, for the MCP-backed skills (`pipelex-design`, `pipelex-organize`, `pipelex-edit`, `pipelex-inputs`, `pipelex-run`, `pipelex-catalog`, `pipelex-integrate`), on the plugin-declared `pipelex-mcp` server (tools `mthds_validate` / `mthds_inputs_template`; `mthds_codegen`, whose write arm — `output_dir`, relative to the workshop's working directory — writes `pipelex-integrate`'s generated trees to disk so no artifact byte passes through the model; `mthds_prepare_inputs`, which uploads `pipelex-inputs`' file-bearing values to Pipelex storage and rewrites them to `pipelex-storage://` references so a run can reach them; plus the `mthds_run` family, which runs a method only in `pipelex-run` — `pipelex-inputs` ends by offering the run and hands it over, and `pipelex-synthetic-inputs` calls it for one thing, the image generation that makes a photograph — and `mthds_list_methods` / `mthds_save_method` / `mthds_get_method`, which are `pipelex-catalog`'s: the last two are **workshop-only**, the save reads, validates and stores one bundle in a single call and finishes by writing the `pipelex-method.json` link that makes the next save an update, and the get brings a saved method's sources to disk. Both refuse rather than overwrite, and the link file is the workshop's to write — no skill writes one. `mthds_get_method`'s **inline** arm is `pipelex-explain`'s instead, and the split is the whole of it: called with an `output_dir` it writes a pull, which is `pipelex-catalog`'s gesture, and called without one it returns the source for a skill that writes nothing — which is why a strictly read-only skill can hold this tool at all; declared in the Claude and Codex manifests, and shipped on Vibe as the `mcp/vibe-mcp.toml` config fragment the user copies into `~/.vibe/config.toml`). The baked declaration is the **local workshop launcher** — `npx -y @pipelex/mcp@latest` over stdio, from the `[vars.mcp_server]` block in `targets/defaults.toml` — never a hosted URL: the hosted console is a connector users add in their host's own UI (see `docs/install.md`, "Which app takes what", and `docs/decisions.md`). Credential delivery: on Claude the manifest's `userConfig` prompts for the API key / base URL at enable time (keychain-stored) and the MCP entry spawns the `launch-pipelex-mcp.sh` wrapper, which receives them as `PIPELEX_PLUGIN_*` via `${user_config.*}` substitution and promotes each to `PIPELEX_API_KEY`/`PIPELEX_BASE_URL` **only when non-empty** — the canonical credential channel (required for Claude Desktop, which carries no shell env), with the non-empty guard keeping an unfilled option from shadowing a shell-exported key; injecting `PIPELEX_*` directly instead makes an empty option surface as a config-class `Unauthorized` that hard-stops every MCP-backed skill; on Codex the manifest forwards `PIPELEX_API_KEY`/`PIPELEX_BASE_URL` by name via `env_vars` because Codex whitelist-filters MCP spawn env; on Vibe the user writes the key into the fragment's `env` table, because Vibe also spawns stdio servers with a minimal environment and expands no variables in its config. Dev override: point `command`/`args` at a local checkout in `targets/defaults.toml` + `make build` on Claude; a same-named `[mcp_servers.pipelex]` entry in `~/.codex/config.toml` on Codex. +The plugin imports nothing and requires no install. Validation rides on the vendored `check.mjs` bundle (wasm engine + `@pipelex/sdk` → hosted API) and, for the MCP-backed skills (`pipelex-design`, `pipelex-organize`, `pipelex-edit`, `pipelex-inputs`, `pipelex-run`, `pipelex-catalog`, `pipelex-integrate`), on the plugin-declared `pipelex-mcp` server (tools `mthds_validate` / `mthds_inputs_template`; `mthds_codegen`, whose write arm — `output_dir`, relative to the workshop's working directory — writes `pipelex-integrate`'s generated trees to disk so no artifact byte passes through the model; `mthds_prepare_inputs`, which uploads `pipelex-inputs`' file-bearing values to Pipelex storage and rewrites them to `pipelex-storage://` references so a run can reach them; plus the `mthds_run` family, which runs a method only in `pipelex-run` — `pipelex-inputs` ends by offering the run and hands it over, and `pipelex-synthetic-inputs` calls it for one thing, the image generation that makes a photograph — and `mthds_list_methods` / `mthds_save_method` / `mthds_get_method`, which are `pipelex-catalog`'s: the last two are **workshop-only**, the save reads, validates and stores one bundle in a single call and finishes by writing the `pipelex-method.json` link that makes the next save an update, and the get brings a saved method's sources to disk. Both refuse rather than overwrite, and the link file is the workshop's to write — no skill writes one. `mthds_get_method`'s **inline** arm is `pipelex-explain`'s instead, and the split is the whole of it: called with an `output_dir` it writes a pull, which is `pipelex-catalog`'s gesture, and called without one it returns the source for a skill that writes nothing — which is why a strictly read-only skill can hold this tool at all; declared in the Claude and Codex manifests, and shipped on Vibe as the `mcp/vibe-mcp.toml` config fragment the user copies into `~/.vibe/config.toml`). The baked declaration is the **local workshop launcher** — `npx -y @pipelex/mcp@latest` over stdio, from the `[vars.mcp_server]` block in `targets/defaults.toml` — never a hosted URL: the hosted console is a connector users add in their host's own UI (see `docs/install.md`, "Which app takes what", and `docs/decisions.md`). Credential delivery: on Claude the manifest's `userConfig` prompts for the API key / base URL at enable time (keychain-stored) and the MCP entry spawns the `launch-pipelex-mcp.sh` wrapper, which receives them as `PIPELEX_PLUGIN_*` via `${user_config.*}` substitution and promotes each to `PIPELEX_API_KEY`/`PIPELEX_BASE_URL` **only when non-empty** — the canonical credential channel (required for Claude Desktop, which carries no shell env), with the non-empty guard keeping an unfilled option from shadowing a shell-exported key; injecting `PIPELEX_*` directly instead makes an empty option surface as a config-class `Unauthorized` that hard-stops every MCP-backed skill; on Codex the manifest forwards `PIPELEX_API_KEY`/`PIPELEX_BASE_URL` by name via `env_vars` because Codex whitelist-filters MCP spawn env; on Vibe the user writes the key into the fragment's `env` table, because Vibe also spawns stdio servers with a minimal environment and expands no variables in its config. Dev override: `make claude-local-mcp` or `make codex-local-mcp`, never an edit of `targets/defaults.toml` (`docs/development.md`, "A local build of `pipelex-mcp`"). **`pipelex-synthetic-inputs` depends on none of that for code-rendered files.** Everything but a photograph is MCP-free — no tool, no key, no Pipelex service — and a photograph alone needs the workshop, whose absence stops that one file and nothing else. `pipelex-scaffold` is the other skill that declares no MCP tool: git, the method-app family's initializer and `make serve`, the ecosystem's initializers and its own two scripts are all it uses, and it leaves a method app running and hands every other project it creates to `pipelex-integrate`. For code-rendered files, the only dependency is a Python the skill can reach: `uv` with ephemeral `--with` packages on the normal rung, and a venv it creates itself under `${XDG_CACHE_HOME:-$HOME/.cache}/pipelex-plugins/synth-venv` when `uv` is absent. Swapping the runner line is the *only* difference between the two rungs, and `tests/recipes` proves it by running real recipes through both. When neither rung is reachable the skill stops with the exact missing piece and, called from `pipelex-inputs`, returns no path so that one input is left unfilled rather than aborting the flow. Keep it and `pipelex-scaffold` out of the `MCP_SKILLS` tuple in `tests/unit/test_gen_skill_docs.py`, and `pipelex-explain` and `pipelex-lab` with them — the lab frames and writes keys without the workshop and stops only before its loop. For explain, the workshop is **optional** — it adds a verdict line to a bundle on disk, reads a saved method's source in full through `mthds_get_method`'s inline arm, and is the only way to reach a catalog id or a published address at all, but its absence never stops the skill from explaining local source. A workshop older than the release that brought `mthds_get_method` answers the other tools without it, so there a catalog id falls back to contract level, which is a narrower explanation and not a stop. The tuple asserts a hard stop, which is a different contract. diff --git a/Makefile b/Makefile index 1c04c460..2bc61221 100644 --- a/Makefile +++ b/Makefile @@ -18,7 +18,8 @@ UV_MIN_VERSION = $(shell grep -m1 'required-version' pyproject.toml | sed -E 's/ format lint ruff-format ruff-lint pyright mypy fix-unused-imports fui \ test agent-test test-recipes tp \ cleanderived cleanenv cleanall reinstall ri \ - codex-use-local codex-use-official codex-refresh codex-status + codex-use-local codex-use-official codex-refresh codex-status \ + claude-local-mcp codex-local-mcp ########################################################################################## ### SETUP @@ -224,3 +225,33 @@ codex-status: ## Show which source is currently registered for the Codex pipelex "$$HOME/.codex/config.toml" \ | grep -E "^(source_type|source|last_revision|last_updated)" \ || echo "• No '$(CODEX_MARKETPLACE_NAME)' marketplace registered." + +########################################################################################## +### LOCAL WORKSHOP (start an agent on a pipelex-mcp the plugin does not ship) +########################################################################################## + +# MCP is a pipelex-mcp checkout or worktree, whose workshop is built with its own +# `make build-local` first; MCP_VERSION, a published @pipelex/mcp version or dist-tag, +# replaces it and builds nothing. WORKDIR is where the session starts, which is where +# the workshop resolves a { path } file. ARGS goes to claude or codex as it is. +# Neither target writes a tracked file: see "A local build of pipelex-mcp" in +# docs/development.md. +MCP ?= ../pipelex-mcp +MCP_VERSION ?= +WORKDIR ?= . +ARGS ?= + +LOCAL_MCP_SOURCE = $(if $(MCP_VERSION),--mcp-version "$(MCP_VERSION)",--mcp "$(MCP)") + +# The checkout's build runs with none of these variables: make hands a sub-make the ones +# given on its command line through MAKEFLAGS and the environment, where they would +# override or fill that Makefile's own. +LOCAL_MCP_BUILD = $(if $(MCP_VERSION),:,unset MCP MCP_VERSION WORKDIR ARGS; MAKEFLAGS= $(MAKE) --no-print-directory -C "$(MCP)" build-local) + +claude-local-mcp: install ## Start Claude Code on this checkout's skills with another workshop (MCP=path or MCP_VERSION=x.y.z) + @$(LOCAL_MCP_BUILD) + @$(VENV_PYTHON) scripts/local_mcp.py claude $(LOCAL_MCP_SOURCE) --workdir "$(WORKDIR)" -- $(ARGS) + +codex-local-mcp: install ## Start Codex with another workshop in place of the plugin's (MCP=path or MCP_VERSION=x.y.z) + @$(LOCAL_MCP_BUILD) + @$(VENV_PYTHON) scripts/local_mcp.py codex $(LOCAL_MCP_SOURCE) --workdir "$(WORKDIR)" -- $(ARGS) diff --git a/docs/build-targets.md b/docs/build-targets.md index a3933022..b6367ab9 100644 --- a/docs/build-targets.md +++ b/docs/build-targets.md @@ -73,7 +73,7 @@ title = "Pipelex API base URL" description = "..." ``` -Reintroduce a variable only when a skill or hook actually branches on it — the `[vars.mcp_server]` table arrived with MCP registration (it feeds the `mcpServers` entry of the generated Claude and Codex manifests, and the Vibe target's `mcp/vibe-mcp.toml` fragment, as the local workshop launcher; `env_vars` lists the variable names Codex forwards into the spawn, since Codex whitelist-filters MCP spawn env — see [decisions.md](decisions.md) "Dual-MCP flip". Dev override: point `command`/`args` at a local checkout + `make build` on Claude, or a same-named `[mcp_servers.pipelex]` config entry on Codex; a target's own `[vars.mcp_server]` merges into this table, as "Variable resolution" below says, so the override keeps the rest of it). The `user_config` sub-tables become the Claude manifest's `userConfig` (enable-time prompt; sensitive values keychain-stored) and drive both the MCP entry's `env` block (`${user_config.*}` → `PIPELEX_*`) and the hook wrapper's `CLAUDE_PLUGIN_OPTION_*` promotion — see [decisions.md](decisions.md) "Claude credentials move to plugin userConfig". Don't port dead switches. +Reintroduce a variable only when a skill or hook actually branches on it — the `[vars.mcp_server]` table arrived with MCP registration (it feeds the `mcpServers` entry of the generated Claude and Codex manifests, and the Vibe target's `mcp/vibe-mcp.toml` fragment, as the local workshop launcher; `env_vars` lists the variable names Codex forwards into the spawn, since Codex whitelist-filters MCP spawn env — see [decisions.md](decisions.md) "Dual-MCP flip". Running against another workshop edits none of it: `make claude-local-mcp` renders the Claude target with `command`/`args` overridden into the ignored `.local-mcp/`, and `make codex-local-mcp` overrides Codex's entry with `-c`, as [development.md](development.md#a-local-build-of-pipelex-mcp) says; a target's own `[vars.mcp_server]` merges into this table, as "Variable resolution" below says, and so does that override, which keeps the rest of it). The `user_config` sub-tables become the Claude manifest's `userConfig` (enable-time prompt; sensitive values keychain-stored) and drive both the MCP entry's `env` block (`${user_config.*}` → `PIPELEX_*`) and the hook wrapper's `CLAUDE_PLUGIN_OPTION_*` promotion — see [decisions.md](decisions.md) "Claude credentials move to plugin userConfig". Don't port dead switches. ### Per-target files (prod.toml, codex.toml, mistral-vibe.toml) @@ -110,7 +110,7 @@ Variables are resolved in this order (last wins): All variables are available in all `.j2` templates as `{{ variable_name }}`. -**A table merges; anything else replaces.** A target's table is laid over the defaults' table of the same name key by key, recursively, while a string, a boolean or an array the target sets replaces the default's whole (`merge_template_vars` in `scripts/gen_skill_docs.py`). So a target that sets `[vars.mcp_server] command` and `args` keeps the defaults' `env_vars` and `user_config`, and a target that sets `[vars.mcp_server.user_config.api_key] title` keeps that option's `type`, `description` and `sensitive`; a target that sets `env_vars` replaces the list rather than extending it. A target can give a key the defaults define another value, but has no way to remove it. So the dev override cannot drop the Claude manifest's `userConfig`, its launcher or the hook's credential promotion, nor the names the Codex manifest forwards, and `make check` fails a target whose generated files lost that wiring all the same, as "Checks over the generated targets" below says. +**A table merges; anything else replaces.** A target's table is laid over the defaults' table of the same name key by key, recursively, while a string, a boolean or an array the target sets replaces the default's whole (`merge_template_vars` in `scripts/gen_skill_docs.py`). So a target that sets `[vars.mcp_server] command` and `args` keeps the defaults' `env_vars` and `user_config`, and a target that sets `[vars.mcp_server.user_config.api_key] title` keeps that option's `type`, `description` and `sensitive`; a target that sets `env_vars` replaces the list rather than extending it. A target can give a key the defaults define another value, but has no way to remove it. So an override of `command` and `args`, a target's own or the one `make claude-local-mcp` renders its copy with, cannot drop the Claude manifest's `userConfig`, its launcher or the hook's credential promotion, nor the names the Codex manifest forwards, and `make check` fails a target whose generated files lost that wiring all the same, as "Checks over the generated targets" below says. ## Output directories @@ -230,7 +230,7 @@ All targets share the same version string in lockstep — `make check` fails on | `platform` | `defaults.toml` (overridden per target) | `frontmatter.md.j2` (Claude-only `allowed-tools`) | | `harness_name` | `defaults.toml` (overridden per target) | every template that names the harness to the user or the model: skill text and shared partials, the hook wrappers, `launch-pipelex-mcp.sh.j2` and `mcp/vibe-mcp.toml.j2` | | `skill_dir` | `defaults.toml` (`${CLAUDE_SKILL_DIR}`), overridden to `` in `codex.toml` and `mistral-vibe.toml` | a sentence that names one of the skill's own files by path — today `pipelex-integrate`'s `cp` of its gate scripts. See below | -| `mcp_server` | `defaults.toml` (`[vars.mcp_server]` table, overridable per target, key by key) | `make_plugin_json()` — the local workshop launcher baked into the plugin-declared `pipelex-mcp` entry: Claude gets `type: stdio` pointing at the `launch-pipelex-mcp.sh` wrapper (which promotes the `PIPELEX_PLUGIN_*` user-config values to their real `PIPELEX_*` names only when non-empty, then `exec`s `command`/`args`), or `command`/`args` directly when the target declares no `user_config`; Codex gets bare `command`/`args` plus `env_vars` (variable *names* forwarded from the user's env — Codex whitelist-filters MCP spawn env; see [decisions.md](decisions.md) "Dual-MCP flip"). Dev override: point `command`/`args` at a local checkout, in the defaults or in a target's own `[vars.mcp_server]`, + `make build` on Claude, or a same-named `[mcp_servers.pipelex]` entry in `~/.codex/config.toml` on Codex; the target's table merges into the defaults', so `env_vars` and `user_config` stay, and `make check` fails a target that lost the credential wiring anyway. `mcp/vibe-mcp.toml.j2`, the Vibe target's one MCP template (`MCP_TEMPLATES_BY_PLATFORM`), renders the same `command`/`args` as a Vibe `[[mcp_servers]]` stdio entry, listing every `env_vars` name as an empty `env` key the user fills in | +| `mcp_server` | `defaults.toml` (`[vars.mcp_server]` table, overridable per target, key by key) | `make_plugin_json()` — the local workshop launcher baked into the plugin-declared `pipelex-mcp` entry: Claude gets `type: stdio` pointing at the `launch-pipelex-mcp.sh` wrapper (which promotes the `PIPELEX_PLUGIN_*` user-config values to their real `PIPELEX_*` names only when non-empty, then `exec`s `command`/`args`), or `command`/`args` directly when the target declares no `user_config`; Codex gets bare `command`/`args` plus `env_vars` (variable *names* forwarded from the user's env — Codex whitelist-filters MCP spawn env; see [decisions.md](decisions.md) "Dual-MCP flip"). Dev override: `make claude-local-mcp` renders the Claude target with `command`/`args` pointed at another workshop into the ignored `.local-mcp/pipelex/` (`scripts/local_mcp.py`), and `make codex-local-mcp` starts Codex with `-c` overrides of its `pipelex` entry, `env_vars` included, since an entry at that tier replaces the plugin's whole; neither edits a tracked file. A target's own table merges into the defaults', so `env_vars` and `user_config` stay, and `make check` fails a target that lost the credential wiring anyway. `mcp/vibe-mcp.toml.j2`, the Vibe target's one MCP template (`MCP_TEMPLATES_BY_PLATFORM`), renders the same `command`/`args` as a Vibe `[[mcp_servers]]` stdio entry, listing every `env_vars` name as an empty `env` key the user fills in | | `floors` | `defaults.toml` (`[vars.floors]` table) | `pipelex-integrate` and `pipelex-scaffold`, which state the minimum versions to the user. See below | | `plugin_name` | derived from `[plugin].name` | available in all templates | diff --git a/docs/decisions.md b/docs/decisions.md index 7cbe646c..44d1aa3a 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -692,6 +692,19 @@ Until the console's own release, the hosted console and the local workshop regis - **`pipelex-organize` is left as it is.** Its baseline validation writes the page before the layout changes, and its confirmation after the swap rewrites it from the new layout, so the page it leaves matches the files. Its promises are about the bundle's `.mthds` files, which the page is not, and it deletes only `.mthds` files. Where the confirmation fails and the original layout is restored, the page shows the rejected candidate until the next validation rewrites it; that is not worth another validation call. - **The writing skills keep the page.** `pipelex-design`, `pipelex-edit`, `pipelex-inputs`, `pipelex-run` and `pipelex-integrate` validate by path and let the workshop write it. Pointing the user at it is a change of its own. +## A local workshop is one make target, and touches no tracked file (2026-09-26) + +Running the skills against an unreleased `pipelex-mcp` workshop took five steps: build the workshop, make a worktree because the switch edits tracked files, rewrite `[vars.mcp_server]` and run `make build`, which dirtied the generated files of every target, load the plugin, and switch back before committing. That was enough that people did not do it. `make claude-local-mcp` and `make codex-local-mcp` replace it with one command, from a plain terminal, that changes nothing git tracks (`docs/development.md`, "A local build of `pipelex-mcp`"). + +- **A make target, not a skill.** What changes is the plugin a session loads at startup, and a skill runs inside a session that has already loaded it, so a skill could only rewrite files and ask for a relaunch. A make target runs before the session exists and can start it, and the steps hold no judgment. `/pipelex-mcp-source` keeps its status report and loses its switching arms, which now name the targets. +- **The Claude copy is rendered by the build's own renderer.** `scripts/local_mcp.py` loads the prod target, lays the workshop's `command` and `args` over its `[vars.mcp_server]` with `merge_template_vars`, so `env_vars` and `user_config` stay, and renders it with `build_target` into `.local-mcp/pipelex/`, which git ignores. The dev copy therefore cannot drift from what ships, and `tests/unit/test_local_mcp.py` holds it to the shipped target file for file, but for the launcher. The copy is rendered beside the previous one and swapped in, so a session still running from it never meets a half-written plugin. +- **The duplicate plugin is not one.** Verified on Claude Code 2.1.283: with `pipelex@pipelex-plugins` installed and enabled, `claude --plugin-dir .local-mcp/pipelex` loads the copy as `pipelex@inline`, lists no other `pipelex` plugin, registers one `plugin:pipelex:pipelex` server and spawns it once, from the copy. So the target passes no `enabledPlugins` override. +- **The key comes from the shell.** The copy's plugin options arrive empty, since the key saved for the installed plugin belongs to `pipelex@pipelex-plugins` and not to `pipelex@inline`. A probe in place of the workshop saw `PIPELEX_PLUGIN_API_KEY` and `PIPELEX_PLUGIN_BASE_URL` empty, so `launch-pipelex-mcp.sh` promoted nothing, and `PIPELEX_API_KEY` and `PIPELEX_BASE_URL` set from the shell. The non-empty guard recorded in "Claude credentials move to plugin userConfig" is what lets the shell's key through here. The target warns when `PIPELEX_API_KEY` is unset rather than refusing, since a session may want the tools that need no key. +- **Codex's override carries `env_vars`.** Verified on Codex 0.156.1: a `-c mcp_servers.pipelex.*` override, like an entry in `~/.codex/config.toml`, replaces the plugin's entry whole, and `codex mcp list --json` then shows `env_vars: []`. A probe spawned that way saw no `PIPELEX_API_KEY`; with `-c mcp_servers.pipelex.env_vars=[…]` added, it saw both variables, and a live `mthds_list_methods` call answered. So the target sets the names the codex target forwards as well as the command, and the install page now says that an override of one's own needs them too. Codex keeps its installed skills; nothing is rendered for it. +- **A published version is pinned at start.** `MCP_VERSION` accepts a version or a dist-tag, which npm resolves to one version before the agent starts, and the copy spawns exactly that version, so the workshop checked is the one run and a typo is refused with npm's own reason instead of failing as an npx error inside the session. A range that matches several versions is refused. +- **The checkout's build sees none of the targets' variables.** make hands a sub-make the variables given on its command line, through `MAKEFLAGS` and the environment, where `ARGS` or `MCP` would override or fill the checkout's own Makefile. The build line unsets them and clears `MAKEFLAGS`, which GNU Make 3.81, the one macOS ships, needs: a target-specific `MAKEOVERRIDES =` does not stop them there. +- **Mistral Vibe is out of scope.** Its `~/.vibe/config.toml` entry is the user's own file, so there is nothing to render and nothing to override from the command line. + ## License & distribution **Apache 2.0**; repo made public when ready (required for easy marketplace install). Versions start at **0.1.0** (plugin and marketplace). GitHub home assumed `Pipelex/pipelex-plugins` — confirm at first push. diff --git a/docs/development.md b/docs/development.md index 944f9031..ba3dc1f8 100644 --- a/docs/development.md +++ b/docs/development.md @@ -67,10 +67,20 @@ The Vibe setup in [the install page](install.md#mistral-vibe) already points at `pipelex-mcp` ships two servers, each with its own tools, over one capability core: the **local workshop** (npm `@pipelex/mcp`, over stdio), whose `mthds_*` tools the skills call and which resolves `{ path }` files straight from your working directory, and the **hosted console** at `https://mcp.pipelex.com/mcp` (streamable HTTP), whose `pipelex_*` tools run saved methods for chatbots and take no files at all. No tool name is registered by both. Public text calls them the Pipelex tools and the Pipelex MCP. The plugin always declares the workshop: an agent edits local `.mthds` files, which only the workshop can read. The console is never baked into the plugin; the reasoning is in [decisions.md](decisions.md), "Dual-MCP flip". -The plugin declares the workshop as `npx -y @pipelex/mcp@latest` in the `[vars.mcp_server]` block of `targets/defaults.toml`. To run the skills against a local checkout of [`pipelex-mcp`](https://github.com/Pipelex/pipelex-mcp), first build its workshop with `make build-local` at the root of that checkout, which writes `packages/workshop/dist/main.js`. A checkout built before `pipelex-mcp` became an npm workspace still holds an older `dist/local/main.js` that nothing rebuilds any more, so never point a harness at it. Then: +The plugin declares the workshop as `npx -y @pipelex/mcp@latest` in the `[vars.mcp_server]` block of `targets/defaults.toml`. To run the skills against another workshop, a local checkout of [`pipelex-mcp`](https://github.com/Pipelex/pipelex-mcp) or a published version, start your agent with one make target. Neither target changes a tracked file, so there is nothing to switch back before you commit. -- **Claude Code.** Set `command = "node"` and `args = ["/path/to/pipelex-mcp/packages/workshop/dist/main.js"]` in that block, run `make build`, and reload as above. Switch it back before you commit, since the build writes the block into every target. -- **Codex.** Leave the plugin alone and add an entry of the same name to `~/.codex/config.toml`, which outranks the plugin's; the example is in [the install page](install.md#codex). -- **Mistral Vibe.** Set the same `command` and `args` in the `pipelex` entry you appended to `~/.vibe/config.toml`, which Vibe reads instead of the fragment in this repository. +```bash +make claude-local-mcp # Claude Code, with the workshop of ../pipelex-mcp +make claude-local-mcp MCP=../_pipelex-mcp--my-topic # any checkout or worktree of pipelex-mcp +make claude-local-mcp MCP_VERSION=0.19.0 # a published version or dist-tag, to reproduce it +make codex-local-mcp # Codex, the same way, with the same variables +make claude-local-mcp WORKDIR=~/my-methods ARGS='--model sonnet' +``` + +A target given `MCP` first builds that checkout's workshop with its own `make build-local`, which writes `packages/workshop/dist/main.js`, and the agent then spawns it by absolute path. `MCP` defaults to `../pipelex-mcp`. A target given `MCP_VERSION` builds nothing: it asks npm which version that names and spawns exactly that one through `npx`, so a version npm does not know is refused before the agent starts. `WORKDIR` is where the session starts, which matters because the workshop resolves a `{ path }` file against it; it defaults to this checkout. `ARGS` goes to `claude` or `codex` as it is, so `ARGS='-p "…"'` runs one headless prompt. Start either target from a terminal, not from inside an agent's session. + +- **Claude Code.** The target renders the Claude plugin from this checkout's templates, with the build's own renderer and the launcher pointed at the chosen workshop, into `.local-mcp/pipelex/`, which git ignores. It then starts `claude --plugin-dir` on that copy. The copy loads as `pipelex@inline` and takes the place of an installed `pipelex@pipelex-plugins` for that session only, so the skills you run are this checkout's too. Its plugin options arrive empty, since the key saved for the installed plugin is not the copy's, so the workshop takes the key exported in your shell as `PIPELEX_API_KEY`, and the target warns when there is none. +- **Codex.** The target renders nothing: it starts `codex` with `-c` overrides of the `mcp_servers.pipelex` entry. An entry given that way replaces the plugin's whole, so the overrides forward `PIPELEX_API_KEY` and `PIPELEX_BASE_URL` by name as the plugin does, and the workshop takes the key from your shell. The skills are whichever copy of the plugin Codex has installed: the published one, or this checkout after `make codex-use-local`. +- **Mistral Vibe.** There is no target. Set `command = "node"` and `args = ["/path/to/pipelex-mcp/packages/workshop/dist/main.js"]` in the `pipelex` entry you appended to `~/.vibe/config.toml`, which Vibe reads instead of the fragment in this repository, after `make build-local` in that checkout. A checkout built before `pipelex-mcp` became an npm workspace still holds an older `dist/local/main.js` that nothing rebuilds any more, so never point Vibe at it. -The repository's `/pipelex-mcp-source` skill, under `.claude/skills/`, makes that switch and the switch back, pins a version, and reports which version each deployment serves. +The repository's `/pipelex-mcp-source` skill, under `.claude/skills/`, reports which version npm, the hosted console and a local build serve, and says which target runs the one you want. Why these are make targets rather than a skill, and how their behaviour was verified, is in [decisions.md](decisions.md), "A local workshop is one make target". diff --git a/docs/install.md b/docs/install.md index d1bb9655..1c7e0160 100644 --- a/docs/install.md +++ b/docs/install.md @@ -76,7 +76,7 @@ args = ["/path/to/pipelex-mcp/packages/workshop/dist/main.js"] # e.g. a local PIPELEX_API_KEY = "plx_sk_..." ``` -The same override works for one invocation: `codex -c 'mcp_servers.pipelex.command="node"' …`. +An entry of your own replaces the plugin's whole, including the names it forwards, so it passes the key on only when it says so: with an `env` table as above, or with `env_vars = ["PIPELEX_API_KEY", "PIPELEX_BASE_URL"]` to keep taking them from your shell. The same override works for one invocation, and needs the names just as much: `codex -c 'mcp_servers.pipelex.command="node"' -c 'mcp_servers.pipelex.args=["/path/to/pipelex-mcp/packages/workshop/dist/main.js"]' -c 'mcp_servers.pipelex.env_vars=["PIPELEX_API_KEY", "PIPELEX_BASE_URL"]'`. In a clone of this repository, `make codex-local-mcp` builds a local checkout's workshop and starts Codex that way; see [the development page](development.md#a-local-build-of-pipelex-mcp). ## Mistral Vibe diff --git a/scripts/gen_skill_docs.py b/scripts/gen_skill_docs.py index f85b940c..98054f07 100644 --- a/scripts/gen_skill_docs.py +++ b/scripts/gen_skill_docs.py @@ -536,14 +536,13 @@ def make_plugin_json(base_dir: Path, config: TargetConfig) -> dict[str, object]: # - Codex: spawns with a minimal whitelist env, so its entry carries # `env_vars` — variable NAMES forwarded from each user's own env, # never values. - # Dev override: point command/args at a local checkout (e.g. - # command = "node", - # args = ["../pipelex-mcp/packages/workshop/dist/main.js"]) in - # targets/defaults.toml, or in a target's own [vars.mcp_server], which - # merges into the defaults' table and so keeps env_vars and user_config - # (merge_template_vars), + `make build` on Claude; a same-named - # [mcp_servers.pipelex] entry in ~/.codex/config.toml outranks the plugin - # tier on Codex. Vibe gets no manifest entry because it has no manifest: + # Dev override: `make claude-local-mcp` renders this target with + # command/args pointed at another workshop into the ignored .local-mcp/ + # (scripts/local_mcp.py), through merge_template_vars, so env_vars and + # user_config stay; `make codex-local-mcp` replaces the entry with `-c` + # overrides, which outrank the plugin tier on Codex and so must repeat + # env_vars. Neither edits a tracked file. + # Vibe gets no manifest entry because it has no manifest: # its target renders the same launcher as the mcp/vibe-mcp.toml config # fragment instead (see MCP_TEMPLATES_BY_PLATFORM). Skipped when the # target defines no mcp_server block. @@ -593,7 +592,7 @@ def make_plugin_json(base_dir: Path, config: TargetConfig) -> dict[str, object]: return base -def _remove(path: Path) -> None: +def remove_path(path: Path) -> None: """Delete whatever is at `path`, without following a symlink to its target.""" # is_symlink() must be checked before is_dir(): a symlink-to-dir is both, and # rmtree would chase the link and delete its target. @@ -610,7 +609,7 @@ def _refresh_copy(src: Path, dst: Path) -> None: before copytree runs. Plain files/dirs are removed too so the build is idempotent. """ - _remove(dst) + remove_path(dst) shutil.copytree(src, dst) @@ -645,7 +644,7 @@ def setup_static_assets( # A retired source directory must take its copies with it. Without this the # build leaves stale assets shipping in every target and `--check` # reports an ORPHAN no rebuild can clear. - _remove(asset_dst) + remove_path(asset_dst) def _built_skill_names(templates_dir: Path, include_skills: list[str] | None) -> list[str]: @@ -951,6 +950,15 @@ def render_codex_discovery_marketplace(base_dir: Path) -> str | None: return source_path.read_text(encoding="utf-8") +def write_files(files: Mapping[Path, str]) -> None: + """Write what a build rendered (`BuildResult.files`) to disk, making the hook scripts executable.""" + for output_path, content in files.items(): + output_path.parent.mkdir(parents=True, exist_ok=True) + output_path.write_text(content, encoding="utf-8") + if output_path.name in EXECUTABLE_OUTPUTS: + output_path.chmod(0o755) + + def generate(base_dir: Path, target_name: str = "prod") -> int: """Render templates and write output files for one or all targets.""" targets_dir = base_dir / TARGETS_DIR_NAME @@ -971,12 +979,8 @@ def generate(base_dir: Path, target_name: str = "prod") -> int: print(f" [{name}] No templates found.") continue - for output_path, content in result.files.items(): - output_path.parent.mkdir(parents=True, exist_ok=True) - output_path.write_text(content, encoding="utf-8") - # Make hook scripts executable - if output_path.name in EXECUTABLE_OUTPUTS: - output_path.chmod(0o755) + write_files(result.files) + for output_path in result.files: rel = output_path.relative_to(base_dir) print(f" [{name}] Generated {rel}") for pruned_path in result.pruned: diff --git a/scripts/local_mcp.py b/scripts/local_mcp.py new file mode 100644 index 00000000..4498e807 --- /dev/null +++ b/scripts/local_mcp.py @@ -0,0 +1,292 @@ +#!/usr/bin/env python3 +"""Start Claude Code or Codex with a Pipelex workshop the plugin does not ship, touching no tracked file. + +`make claude-local-mcp` and `make codex-local-mcp` run this. The workshop is a `pipelex-mcp` checkout's +own build (`--mcp`, after the make target has run `make build-local` there), spawned as +`node /packages/workshop/dist/main.js`, or a published `@pipelex/mcp` (`--mcp-version`), +spawned through `npx` at the exact version npm resolves it to. + +- **Claude Code.** The Claude target is rendered by the build's own renderer, with `[vars.mcp_server]` + `command` and `args` pointed at that workshop and the rest of the table kept, into + `.local-mcp/pipelex/`, which git ignores, and Claude Code starts with `--plugin-dir` on it. The copy + loads as `pipelex@inline` and takes the place of an installed `pipelex@pipelex-plugins` for that + session, so its skills are this checkout's. +- **Codex.** Nothing is rendered. Codex starts with `-c` overrides of its `mcp_servers.pipelex` entry, + which replace the plugin's entry whole, so they carry the `env_vars` the plugin forwards as well as + the command. The skills are whichever copy of the plugin Codex has installed. + +Either way the workshop takes its key from the shell's `PIPELEX_API_KEY`: the Claude copy's plugin +options arrive empty, since the key saved for the installed plugin is not the inline copy's, and Codex +forwards what `env_vars` names. docs/decisions.md records how each of these was verified. +""" + +from __future__ import annotations + +import argparse +import dataclasses +import json +import os +import shutil +import subprocess +import sys +from collections.abc import Callable +from dataclasses import dataclass +from enum import StrEnum +from pathlib import Path +from typing import NoReturn, cast + +from scripts.gen_skill_docs import ( + MCP_SERVER_NAME, + TARGETS_DIR_NAME, + build_target, + load_target_config, + merge_template_vars, + remove_path, + write_files, +) + +# Where the Claude copy is rendered, relative to the repository root. `.gitignore` lists it. +LOCAL_DIR_NAME = ".local-mcp" + +# What `make build-local` writes in a pipelex-mcp checkout, and the manifest npm publishes it from. +WORKSHOP_BUNDLE = Path("packages/workshop/dist/main.js") +WORKSHOP_MANIFEST = Path("packages/workshop/package.json") +WORKSHOP_PACKAGE = "@pipelex/mcp" + +CLAUDE_TARGET = "prod" +CODEX_TARGET = "codex" +KEY_VARIABLE = "PIPELEX_API_KEY" + + +class Harness(StrEnum): + CLAUDE = "claude" + CODEX = "codex" + + +@dataclass(frozen=True) +class Launcher: + """What spawns the workshop, and the words that say which workshop it is.""" + + command: str + args: list[str] + label: str + + +def checkout_launcher(checkout: Path) -> Launcher: + """The workshop a `pipelex-mcp` checkout or worktree has built, by absolute path. + + Absolute, because the harness spawns the workshop from the session's working directory, which is + neither this repository nor the checkout. + """ + root = checkout.expanduser().resolve() + if not root.is_dir(): + msg = f"There is no pipelex-mcp checkout at {root}: pass MCP=." + raise SystemExit(msg) + bundle = root / WORKSHOP_BUNDLE + if not bundle.is_file(): + msg = f"{bundle} does not exist: build it with `make build-local` in {root}." + raise SystemExit(msg) + return Launcher(command="node", args=[str(bundle)], label=f"the build in {root} ({WORKSHOP_PACKAGE} {_manifest_version(root)})") + + +def _manifest_version(root: Path) -> str: + """The version the checkout's workshop manifest names, which is what its handshake reports.""" + try: + manifest = json.loads((root / WORKSHOP_MANIFEST).read_text(encoding="utf-8")) + except (OSError, ValueError): + return "of unknown version" + version = cast("dict[str, object]", manifest).get("version") if isinstance(manifest, dict) else None + return str(version) if version else "of unknown version" + + +def npm_resolve(spec: str) -> str: + """The one published version of the workshop that `spec`, a version or a dist-tag, names. + + Asked before anything starts, because a spec npm cannot resolve would otherwise fail only when + the harness spawns the workshop, as an npx error inside the session. + """ + npm = shutil.which("npm") + if npm is None: + msg = "npm is not on the PATH, and a published workshop is spawned through npx." + raise SystemExit(msg) + query = f"{WORKSHOP_PACKAGE}@{spec}" + completed = subprocess.run([npm, "view", query, "version", "--json"], capture_output=True, text=True, check=False) + resolved = _json_answer(completed.stdout) + listing = f"`npm view {WORKSHOP_PACKAGE} versions` lists the published ones" + # npm answers an error as a JSON object on stdout, beside its exit status, and a version as a JSON string. + error = _npm_error_summary(resolved) + if completed.returncode != 0 or error is not None: + stderr_lines = [line.strip() for line in completed.stderr.splitlines() if line.strip()] + reason = error or (stderr_lines[-1] if stderr_lines else "no reason given") + msg = f"`npm view {query} version` failed ({reason}): {listing}." + raise SystemExit(msg) + if resolved is None: + msg = f"No published {WORKSHOP_PACKAGE} matches {spec!r}: {listing}." + raise SystemExit(msg) + if isinstance(resolved, list): + versions = [str(item) for item in cast("list[object]", resolved)] + msg = f"{spec!r} matches several published versions ({', '.join(versions)}): name one." + raise SystemExit(msg) + return str(resolved) + + +def _json_answer(stdout: str) -> object: + """What `npm view --json` printed, parsed: None for nothing, the text itself when it is not JSON.""" + text = stdout.strip() + if not text: + return None + try: + parsed: object = json.loads(text) + except ValueError: + return text + return parsed + + +def _npm_error_summary(answer: object) -> str | None: + """The summary of the error object `npm view --json` prints when it fails, or None when the answer is not one.""" + if not isinstance(answer, dict): + return None + error = cast("dict[str, object]", answer).get("error") + summary = cast("dict[str, object]", error).get("summary") if isinstance(error, dict) else None + return str(summary) if summary else "npm answered an error with no summary" + + +def published_launcher(spec: str, resolve: Callable[[str], str] = npm_resolve) -> Launcher: + """A published workshop, pinned to the exact version `spec` resolves to now, so the one checked is the one spawned.""" + version = resolve(spec) + return Launcher(command="npx", args=["-y", f"{WORKSHOP_PACKAGE}@{version}"], label=f"{WORKSHOP_PACKAGE} {version} from npm") + + +def render_claude_copy(base_dir: Path, launcher: Launcher) -> Path: + """Render the Claude target with `launcher` as its workshop into `.local-mcp//`, and return that directory. + + The target's `[vars.mcp_server]` table gets the launcher's `command` and `args` and keeps the rest, + `user_config` among it, by the merge every target's own table goes through, so the copy is what + `make build` would ship but for the workshop it spawns. It is rendered beside the previous copy + and swapped in, so a session still running from that copy never meets a half-written plugin. + """ + shipped = load_target_config(base_dir / TARGETS_DIR_NAME, CLAUDE_TARGET) + local_dir = base_dir / LOCAL_DIR_NAME + final = local_dir / shipped.plugin_name + staging = local_dir / f".{shipped.plugin_name}.staging" + retired = local_dir / f".{shipped.plugin_name}.retired" + for leftover in (staging, retired): + remove_path(leftover) + + template_vars = merge_template_vars(shipped.template_vars, {"mcp_server": {"command": launcher.command, "args": launcher.args}}) + config = dataclasses.replace(shipped, source=f"{staging.relative_to(base_dir).as_posix()}/", template_vars=template_vars) + write_files(build_target(base_dir, config).files) + + if final.exists() or final.is_symlink(): + final.rename(retired) + staging.rename(final) + remove_path(retired) + return final + + +def codex_env_vars(base_dir: Path) -> list[str]: + """The variable names the Codex plugin's entry forwards into the workshop's environment.""" + server = load_target_config(base_dir / TARGETS_DIR_NAME, CODEX_TARGET).template_vars.get("mcp_server") + names = server.get("env_vars") if isinstance(server, dict) else None + if not isinstance(names, list) or not names: + msg = f"targets/{CODEX_TARGET}.toml resolves no [vars.mcp_server] env_vars, so the workshop would start without a key." + raise SystemExit(msg) + return [str(name) for name in cast("list[object]", names)] + + +def codex_overrides(launcher: Launcher, env_vars: list[str]) -> list[str]: + """The `-c` arguments that point Codex's `mcp_servers.pipelex` entry at `launcher`. + + An entry at the configuration tier replaces the plugin's whole rather than merging into it, and + Codex spawns a server with a minimal environment, so the names the plugin forwards are set again + here or the workshop starts without the shell's key. + """ + entry = f"mcp_servers.{MCP_SERVER_NAME}" + return [ + "-c", + f"{entry}.command={toml_string(launcher.command)}", + "-c", + f"{entry}.args={toml_array(launcher.args)}", + "-c", + f"{entry}.env_vars={toml_array(env_vars)}", + ] + + +def toml_string(value: str) -> str: + """`value` as a TOML basic string, which is how Codex parses the value of a `-c` override.""" + escaped: list[str] = [] + for char in value: + if char in {'"', "\\"}: + escaped.append(f"\\{char}") + elif ord(char) < 0x20 or ord(char) == 0x7F: + escaped.append(f"\\u{ord(char):04x}") + else: + escaped.append(char) + return f'"{"".join(escaped)}"' + + +def toml_array(values: list[str]) -> str: + """`values` as a TOML array of basic strings.""" + return f"[{', '.join(toml_string(value) for value in values)}]" + + +def start(harness: Harness, launcher: Launcher, workdir: Path, passthrough: list[str], base_dir: Path) -> NoReturn: + """Prepare `harness` to spawn `launcher` as its Pipelex workshop, say what runs, and replace this process with it.""" + program = shutil.which(harness.value) + if program is None: + msg = f"`{harness.value}` is not on the PATH." + raise SystemExit(msg) + + lines = [f"• Workshop: {launcher.label}"] + match harness: + case Harness.CLAUDE: + plugin_dir = render_claude_copy(base_dir, launcher) + command = [program, "--plugin-dir", str(plugin_dir), *passthrough] + lines.append(f"• Plugin: {plugin_dir}, this checkout's skills, loaded as pipelex@inline in place of an installed pipelex") + case Harness.CODEX: + command = [program, *codex_overrides(launcher, codex_env_vars(base_dir)), *passthrough] + lines.append("• Plugin: the one Codex has installed, its pipelex server entry overridden for this session") + lines.append(f"• Starting {harness.value} in {workdir}") + if not os.environ.get(KEY_VARIABLE): + lines.append( + f"• Warning: {KEY_VARIABLE} is not set in this shell. Started this way, the workshop takes its key from there alone, " + "so the Pipelex tools will answer Unauthorized. Export the key and start again." + ) + print("\n".join(lines), flush=True) + + os.chdir(workdir) + os.execv(program, command) + + +def split_passthrough(argv: list[str]) -> tuple[list[str], list[str]]: + """This script's own arguments, and what follows the first `--`, which goes to the harness as it is.""" + if "--" in argv: + marker = argv.index("--") + return argv[:marker], argv[marker + 1 :] + return argv, [] + + +def parse_args(argv: list[str]) -> argparse.Namespace: + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0] if __doc__ else None) + parser.add_argument("harness", choices=[harness.value for harness in Harness], help="The agent to start.") + source = parser.add_mutually_exclusive_group(required=True) + source.add_argument("--mcp", type=Path, help="A pipelex-mcp checkout or worktree whose `make build-local` has run (the Makefile's MCP).") + source.add_argument("--mcp-version", help=f"A published {WORKSHOP_PACKAGE} version or dist-tag (the Makefile's MCP_VERSION).") + parser.add_argument("--workdir", type=Path, default=Path(), help="Where the session starts, and so where the workshop resolves a { path } file.") + return parser.parse_args(argv) + + +def main(argv: list[str] | None = None) -> int: + own, passthrough = split_passthrough(sys.argv[1:] if argv is None else argv) + args = parse_args(own) + base_dir = Path(__file__).resolve().parent.parent + workdir: Path = args.workdir.expanduser().resolve() + if not workdir.is_dir(): + msg = f"WORKDIR {workdir} is not a directory." + raise SystemExit(msg) + launcher = checkout_launcher(args.mcp) if args.mcp is not None else published_launcher(args.mcp_version) + start(Harness(args.harness), launcher, workdir, passthrough, base_dir) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/targets/defaults.toml b/targets/defaults.toml index 97ff89fe..79a321ea 100644 --- a/targets/defaults.toml +++ b/targets/defaults.toml @@ -86,11 +86,11 @@ python_max = "3.14" # [[mcp_servers]] stdio entry the user copies into ~/.vibe/config.toml, and # each env_vars name becomes an empty `env` key they fill in, because Vibe # forwards no shell env into the spawn and expands no variables. -# Dev override: point command/args at a local checkout's workshop, built by -# `make build-local` there (e.g. command = "node", -# args = ["../pipelex-mcp/packages/workshop/dist/main.js"]) + `make build` on -# Claude; a same-named [mcp_servers.pipelex] entry in ~/.codex/config.toml on -# Codex. +# Dev override: never an edit of this block. `make claude-local-mcp` and +# `make codex-local-mcp` start Claude Code or Codex on another workshop, a +# pipelex-mcp checkout (MCP=) or a published version (MCP_VERSION=x.y.z), +# and touch no tracked file (docs/development.md, "A local build of +# pipelex-mcp"). Edit it only to change what ships. [vars.mcp_server] command = "npx" args = ["-y", "@pipelex/mcp@latest"] diff --git a/tests/unit/test_local_mcp.py b/tests/unit/test_local_mcp.py new file mode 100644 index 00000000..7d7dc4e5 --- /dev/null +++ b/tests/unit/test_local_mcp.py @@ -0,0 +1,346 @@ +"""Tests for scripts/local_mcp.py, which `make claude-local-mcp` and `make codex-local-mcp` run.""" + +from __future__ import annotations + +import hashlib +import os +import shutil +import subprocess +import tomllib +from pathlib import Path +from typing import cast + +import pytest +from pytest_mock import MockerFixture + +from scripts import local_mcp +from scripts.gen_skill_docs import build_target, load_target_config, write_files +from scripts.local_mcp import ( + LOCAL_DIR_NAME, + WORKSHOP_BUNDLE, + Harness, + Launcher, + checkout_launcher, + codex_env_vars, + codex_overrides, + main, + npm_resolve, + parse_args, + published_launcher, + render_claude_copy, + split_passthrough, + start, + toml_string, +) + +REPO_ROOT = Path(__file__).parents[2] + +# What the build reads: rendering the Claude target needs every one of these and nothing else. +BUILD_SOURCES = ("templates", "targets", "skills", ".claude-plugin", ".codex-plugin") + + +@pytest.fixture +def repo(tmp_path: Path) -> Path: + """A copy of this repository's build sources, so a render never writes into the real checkout.""" + root = tmp_path / "pipelex-plugins" + for name in BUILD_SOURCES: + shutil.copytree(REPO_ROOT / name, root / name) + return root + + +@pytest.fixture +def checkout(tmp_path: Path) -> Path: + """A pipelex-mcp checkout whose `make build-local` has run, under a path holding a space.""" + root = tmp_path / "my checkouts" / "pipelex-mcp" + bundle = root / WORKSHOP_BUNDLE + bundle.parent.mkdir(parents=True) + bundle.write_text("// the workshop\n", encoding="utf-8") + (root / "packages" / "workshop" / "package.json").write_text('{"name": "@pipelex/mcp", "version": "1.2.3"}\n', encoding="utf-8") + return root + + +def _published(spec: str) -> Launcher: + return published_launcher(spec, resolve=lambda _spec: "0.20.0") + + +def _npm_answers(mocker: MockerFixture, *, stdout: str, stderr: str = "", returncode: int = 0) -> None: + mocker.patch.object(local_mcp.shutil, "which", return_value="/usr/bin/npm") + mocker.patch.object( + local_mcp.subprocess, + "run", + return_value=subprocess.CompletedProcess(args=[], returncode=returncode, stdout=stdout, stderr=stderr), + ) + + +def _exec_line(launcher: Launcher) -> str: + """How `launch-pipelex-mcp.sh` spawns the workshop.""" + return f"exec {launcher.command}" + "".join(f' "{arg}"' for arg in launcher.args) + + +def _spoken(launcher: Launcher) -> str: + """How a skill names the workshop's command to the user.""" + return " ".join([launcher.command, *launcher.args]) + + +def _tree(root: Path, *, skip: str | None = None) -> dict[Path, tuple[str, int]]: + """Every file under `root` with its digest and mode, leaving out the top-level directory `skip`.""" + tree: dict[Path, tuple[str, int]] = {} + for path in root.rglob("*"): + rel = path.relative_to(root) + if path.is_file() and rel.parts[0] != skip: + tree[rel] = (hashlib.sha256(path.read_bytes()).hexdigest(), path.stat().st_mode) + return tree + + +class TestLauncher: + def test_a_checkout_runs_its_build_by_absolute_path(self, checkout: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """The harness spawns the workshop from the session's directory, so a relative path would name another file.""" + monkeypatch.chdir(checkout.parent) + launcher = checkout_launcher(Path("pipelex-mcp")) + assert launcher.command == "node" + assert launcher.args == [str(checkout.resolve() / WORKSHOP_BUNDLE)] + assert "@pipelex/mcp 1.2.3" in launcher.label + + def test_a_checkout_without_its_build_is_refused_with_the_build_to_run(self, checkout: Path) -> None: + (checkout / WORKSHOP_BUNDLE).unlink() + with pytest.raises(SystemExit, match="make build-local"): + checkout_launcher(checkout) + + def test_a_missing_checkout_names_the_variable_to_set(self, tmp_path: Path) -> None: + with pytest.raises(SystemExit, match="MCP="): + checkout_launcher(tmp_path / "nowhere") + + def test_a_published_version_is_pinned_to_the_one_npm_resolves(self) -> None: + launcher = _published("latest") + assert (launcher.command, launcher.args) == ("npx", ["-y", "@pipelex/mcp@0.20.0"]) + + def test_npm_resolves_a_spec_to_one_version(self, mocker: MockerFixture) -> None: + _npm_answers(mocker, stdout='"0.20.0"\n') + assert npm_resolve("latest") == "0.20.0" + + def test_a_version_npm_does_not_know_is_refused_with_its_summary(self, mocker: MockerFixture) -> None: + _npm_answers( + mocker, + stdout='{"error": {"code": "E404", "summary": "No match found for version 9.9.9", "detail": "…"}}', + stderr="npm error code E404\nnpm error 404 …\n", + returncode=1, + ) + with pytest.raises(SystemExit, match=r"failed \(No match found for version 9\.9\.9\)"): + npm_resolve("9.9.9") + + def test_a_spec_npm_answers_nothing_for_is_refused(self, mocker: MockerFixture) -> None: + _npm_answers(mocker, stdout="") + with pytest.raises(SystemExit, match="No published @pipelex/mcp matches"): + npm_resolve("0.0.1") + + def test_a_range_matching_several_versions_is_refused(self, mocker: MockerFixture) -> None: + _npm_answers(mocker, stdout='["0.19.0", "0.20.0"]') + with pytest.raises(SystemExit, match="several published versions"): + npm_resolve("^0.19") + + +class TestClaudeCopy: + def test_the_copy_is_the_shipped_target_but_for_the_workshop(self, repo: Path, checkout: Path) -> None: + """The copy cannot drift from what ships: the same files, byte for byte and mode for mode, once the launcher is swapped back.""" + launcher = checkout_launcher(checkout) + copy = render_claude_copy(repo, launcher) + assert copy == repo / LOCAL_DIR_NAME / "pipelex" + + shipped_config = load_target_config(repo / "targets", "prod") + write_files(build_target(repo, shipped_config).files) + shipped_dir = repo / "pipelex" + server = shipped_config.template_vars["mcp_server"] + assert isinstance(server, dict) + shipped = Launcher(command=str(server["command"]), args=[str(arg) for arg in cast("list[object]", server["args"])], label="shipped") + + swaps = {_exec_line(launcher): _exec_line(shipped), _spoken(launcher): _spoken(shipped)} + copy_tree, shipped_tree = _tree(copy), _tree(shipped_dir) + assert copy_tree.keys() == shipped_tree.keys() + swapped: list[Path] = [] + for rel, (digest, mode) in copy_tree.items(): + assert mode == shipped_tree[rel][1], f"{rel}: mode differs from the shipped file" + if digest == shipped_tree[rel][0]: + continue + content = (copy / rel).read_text(encoding="utf-8") + for local_text, shipped_text in swaps.items(): + content = content.replace(local_text, shipped_text) + assert content == (shipped_dir / rel).read_text(encoding="utf-8"), f"{rel} differs from the shipped file beyond the workshop" + swapped.append(rel) + assert Path("hooks/launch-pipelex-mcp.sh") in swapped + + def test_the_launcher_keeps_the_credential_promotion(self, repo: Path, checkout: Path) -> None: + copy = render_claude_copy(repo, checkout_launcher(checkout)) + launcher = (copy / "hooks" / "launch-pipelex-mcp.sh").read_text(encoding="utf-8") + for key in ("API_KEY", "BASE_URL"): + assert f'export PIPELEX_{key}="$PIPELEX_PLUGIN_{key}"' in launcher + assert launcher.rstrip().endswith(f'exec node "{checkout.resolve() / WORKSHOP_BUNDLE}"') + assert os.access(copy / "hooks" / "launch-pipelex-mcp.sh", os.X_OK) + + def test_a_second_render_replaces_the_first_and_leaves_nothing_beside_it(self, repo: Path, checkout: Path) -> None: + copy = render_claude_copy(repo, checkout_launcher(checkout)) + (copy / "stray.txt").write_text("left by hand\n", encoding="utf-8") + + again = render_claude_copy(repo, _published("latest")) + assert again == copy + assert not (copy / "stray.txt").exists() + assert (copy / "hooks" / "launch-pipelex-mcp.sh").read_text(encoding="utf-8").rstrip().endswith('exec npx "-y" "@pipelex/mcp@0.20.0"') + assert sorted(path.name for path in (repo / LOCAL_DIR_NAME).iterdir()) == ["pipelex"] + + def test_the_render_writes_nothing_outside_the_local_directory(self, repo: Path, checkout: Path) -> None: + before = _tree(repo, skip=LOCAL_DIR_NAME) + render_claude_copy(repo, checkout_launcher(checkout)) + assert _tree(repo, skip=LOCAL_DIR_NAME) == before + + @pytest.mark.skipif(shutil.which("git") is None, reason="no git on the PATH") + def test_git_ignores_the_local_directory(self) -> None: + probe = f"{LOCAL_DIR_NAME}/pipelex/.claude-plugin/plugin.json" + completed = subprocess.run(["git", "-C", str(REPO_ROOT), "check-ignore", "-q", "--no-index", probe], check=False) + assert completed.returncode == 0, f"{LOCAL_DIR_NAME}/ must be in .gitignore, since the copy is never committed" + + +class TestCodexOverrides: + def test_the_overrides_replace_the_entry_and_forward_the_key_names(self, checkout: Path) -> None: + """A configuration-tier entry replaces the plugin's whole, so the names it forwards must be given again.""" + launcher = checkout_launcher(checkout) + overrides = codex_overrides(launcher, ["PIPELEX_API_KEY", "PIPELEX_BASE_URL"]) + assert overrides[0::2] == ["-c", "-c", "-c"] + parsed = {key: tomllib.loads(f"v = {value}")["v"] for key, value in (item.split("=", 1) for item in overrides[1::2])} + assert parsed == { + "mcp_servers.pipelex.command": "node", + "mcp_servers.pipelex.args": [str(checkout.resolve() / WORKSHOP_BUNDLE)], + "mcp_servers.pipelex.env_vars": ["PIPELEX_API_KEY", "PIPELEX_BASE_URL"], + } + + @pytest.mark.parametrize("value", ["plain", "with space", 'a "quote"', "back\\slash", "tab\tand\nnewline", "accentué ✓ 😀", "del\x7f"]) + def test_a_toml_string_reads_back_as_the_value(self, value: str) -> None: + assert tomllib.loads(f"v = {toml_string(value)}")["v"] == value + + def test_the_names_forwarded_are_the_codex_targets(self, repo: Path) -> None: + server = load_target_config(repo / "targets", "codex").template_vars["mcp_server"] + assert isinstance(server, dict) + assert codex_env_vars(repo) == server["env_vars"] + assert codex_env_vars(repo) + + def test_a_codex_target_forwarding_nothing_is_refused(self, repo: Path) -> None: + with (repo / "targets" / "codex.toml").open("a", encoding="utf-8") as codex_toml: + codex_toml.write("\n[vars.mcp_server]\nenv_vars = []\n") + with pytest.raises(SystemExit, match="without a key"): + codex_env_vars(repo) + + +class Launched(Exception): + """What the patched `os.execv` raises in place of replacing the test process.""" + + def __init__(self, program: str, argv: list[str]) -> None: + super().__init__(program) + self.program = program + self.argv = argv + + +class TestStart: + @pytest.fixture + def harness_calls(self, mocker: MockerFixture) -> list[Path]: + """Put every harness on the PATH, record where the session starts, and stop at the exec.""" + + def which(name: str) -> str: + return f"/usr/local/bin/{name}" + + mocker.patch.object(local_mcp.shutil, "which", side_effect=which) + + def execv(program: str, argv: list[str]) -> None: + raise Launched(program, argv) + + mocker.patch.object(local_mcp.os, "execv", side_effect=execv) + chdirs: list[Path] = [] + mocker.patch.object(local_mcp.os, "chdir", side_effect=chdirs.append) + return chdirs + + def test_claude_starts_on_the_copy_in_the_workdir(self, repo: Path, checkout: Path, tmp_path: Path, harness_calls: list[Path]) -> None: + with pytest.raises(Launched) as launched: + start(Harness.CLAUDE, checkout_launcher(checkout), tmp_path, ["-p", "hello"], repo) + assert launched.value.argv == ["/usr/local/bin/claude", "--plugin-dir", str(repo / LOCAL_DIR_NAME / "pipelex"), "-p", "hello"] + assert harness_calls == [tmp_path] + + def test_codex_starts_with_the_overrides_and_renders_nothing(self, repo: Path, checkout: Path, tmp_path: Path, harness_calls: list[Path]) -> None: + launcher = checkout_launcher(checkout) + with pytest.raises(Launched) as launched: + start(Harness.CODEX, launcher, tmp_path, ["exec", "hello"], repo) + assert launched.value.argv == ["/usr/local/bin/codex", *codex_overrides(launcher, codex_env_vars(repo)), "exec", "hello"] + assert not (repo / LOCAL_DIR_NAME).exists() + + @pytest.mark.parametrize(("key", "warned"), [(None, True), ("", True), ("plx_sk_test", False)]) + def test_a_missing_key_is_warned_of( + self, + repo: Path, + tmp_path: Path, + harness_calls: list[Path], + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + key: str | None, + warned: bool, + ) -> None: + if key is None: + monkeypatch.delenv("PIPELEX_API_KEY", raising=False) + else: + monkeypatch.setenv("PIPELEX_API_KEY", key) + with pytest.raises(Launched): + start(Harness.CODEX, _published("latest"), tmp_path, [], repo) + assert ("PIPELEX_API_KEY is not set" in capsys.readouterr().out) is warned + + def test_a_harness_not_on_the_path_stops_before_anything_is_rendered(self, repo: Path, tmp_path: Path, mocker: MockerFixture) -> None: + mocker.patch.object(local_mcp.shutil, "which", return_value=None) + with pytest.raises(SystemExit, match="`claude` is not on the PATH"): + start(Harness.CLAUDE, _published("latest"), tmp_path, [], repo) + assert not (repo / LOCAL_DIR_NAME).exists() + + +class TestCommandLine: + def test_what_follows_the_first_double_dash_goes_to_the_harness_whole(self) -> None: + own, passthrough = split_passthrough(["claude", "--mcp", "x", "--", "-p", "--", "y"]) + assert own == ["claude", "--mcp", "x"] + assert passthrough == ["-p", "--", "y"] + + @pytest.mark.parametrize("argv", [["claude"], ["claude", "--mcp", "x", "--mcp-version", "1.0.0"], ["vibe", "--mcp", "x"]]) + def test_one_harness_and_exactly_one_workshop_are_required(self, argv: list[str]) -> None: + with pytest.raises(SystemExit): + parse_args(argv) + + def test_a_workdir_that_is_not_a_directory_is_refused(self, checkout: Path, tmp_path: Path) -> None: + with pytest.raises(SystemExit, match="WORKDIR"): + main(["claude", "--mcp", str(checkout), "--workdir", str(tmp_path / "nowhere")]) + + +@pytest.mark.skipif(shutil.which("make") is None, reason="no make on the PATH") +class TestMakeTargets: + def _make(self, *arguments: str) -> subprocess.CompletedProcess[str]: + """This repository's make, told its venv is installed: `install` would update uv over the network.""" + return subprocess.run( + ["make", "--no-print-directory", "-C", str(REPO_ROOT), "-o", "install", *arguments], + capture_output=True, + text=True, + check=False, + ) + + @pytest.mark.skipif(not (REPO_ROOT / ".venv" / "bin" / "python").is_file(), reason="no venv: run `make install`") + def test_the_checkout_is_built_with_none_of_the_targets_variables(self, tmp_path: Path) -> None: + """make hands a sub-make its command-line variables, through MAKEFLAGS and the environment, which would override or fill the checkout's own. + + This one runs for real, since `make -n` prints the build line without running it: the checkout's + build only reports what it sees, and the script then refuses the checkout for having built no + workshop, so nothing starts. + """ + (tmp_path / "Makefile").write_text( + 'ARGS = its-own\nbuild-local:\n\t@echo "built with ARGS=[$(ARGS)] MCP=[$(MCP)] MCP_VERSION=[$(MCP_VERSION)] WORKDIR=[$(WORKDIR)]"\n', + encoding="utf-8", + ) + completed = self._make("claude-local-mcp", f"MCP={tmp_path}", "ARGS=--model sonnet", f"WORKDIR={tmp_path}") + assert "built with ARGS=[its-own] MCP=[] MCP_VERSION=[] WORKDIR=[]" in completed.stdout + assert "build it with `make build-local`" in completed.stderr + assert completed.returncode != 0 + assert not (REPO_ROOT / LOCAL_DIR_NAME / ".pipelex.staging").exists() + + def test_a_published_version_builds_nothing(self) -> None: + completed = self._make("-n", "claude-local-mcp", "MCP_VERSION=0.20.0", "MCP=/no/checkout/here", "ARGS=--model sonnet") + assert completed.returncode == 0, completed.stderr + assert "build-local" not in completed.stdout + assert 'scripts/local_mcp.py claude --mcp-version "0.20.0" --workdir "." -- --model sonnet' in completed.stdout From b4391acf037131030ea599823beb116d6488944a Mon Sep 17 00:00:00 2001 From: Louis Choquel Date: Sat, 26 Sep 2026 18:02:15 +0200 Subject: [PATCH 02/10] Render one local Claude copy per workshop, staged apart and swapped under a lock A session reads the copy's launcher again at every respawn, so the one shared .local-mcp/pipelex/ let a later start with another workshop switch the workshop of a session already running. Two starts at once also shared one staging directory, and could fail or swap in a copy missing files. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_014AcpM2qUzbWMenP62iSBoS --- .claude/skills/pipelex-mcp-source/SKILL.md | 2 +- .gitignore | 2 +- docs/build-targets.md | 2 +- docs/decisions.md | 5 +- docs/development.md | 2 +- scripts/local_mcp.py | 58 ++++++++++++++-------- tests/unit/test_local_mcp.py | 39 ++++++++++++--- 7 files changed, 75 insertions(+), 35 deletions(-) diff --git a/.claude/skills/pipelex-mcp-source/SKILL.md b/.claude/skills/pipelex-mcp-source/SKILL.md index 5b5acc54..2d1d4f98 100644 --- a/.claude/skills/pipelex-mcp-source/SKILL.md +++ b/.claude/skills/pipelex-mcp-source/SKILL.md @@ -57,7 +57,7 @@ When the user invokes this skill without naming a target, report current state a 4. **Local checkout version** — `node -p "require('../pipelex-mcp/packages/workshop/package.json').version"`. That manifest is the one npm publishes as `@pipelex/mcp`. Never read the root `package.json`, which carries no version since the workspace split, nor `packages/console/package.json`, which versions the console on a track npm never sees. A version behind npm `latest` means the checkout needs a pull. A version equal to it is the ordinary case on `dev`, since the version moves only at a release: what the checkout has that npm lacks is listed under `## [Unreleased]` in `../pipelex-mcp/packages/workshop/CHANGELOG.md`, and an empty section there means the local source tests nothing new. 5. **Local build** — does `../pipelex-mcp/packages/workshop/dist/main.js` exist, and is it stale? The bundle inlines the capability core, and its handshake reports the version the workshop's manifest had at build time, so it lags whenever either package's sources or that manifest are newer: `find ../pipelex-mcp/packages/core/src ../pipelex-mcp/packages/workshop/src ../pipelex-mcp/packages/workshop/package.json -type f \( -name '*.ts' -o -name package.json \) ! -name '*.test.ts' ! -name '*.e2e.ts' -newer ../pipelex-mcp/packages/workshop/dist/main.js` printing anything means the build lags. A lagging build matters only to a running session or a Vibe entry: the make targets rebuild before they start. 6. **Leaked dev state** — `git diff --stat targets/defaults.toml` and `git status --porcelain pipelex/ pipelex-codex/ pipelex-vibe/`. If the declared source is not `npm-latest`, lead with that: the tree is carrying a hand-made switch. -7. **The local copy** — when `.local-mcp/pipelex/hooks/launch-pipelex-mcp.sh` exists, its `exec` line says which workshop the last `make claude-local-mcp` in this checkout rendered. It is ignored by git and never shipped, so it is information, not dev state to clean up. +7. **The local copies** — `make claude-local-mcp` renders one copy of the plugin per workshop, under `.local-mcp//pipelex/`, and the `exec` line of each copy's `hooks/launch-pipelex-mcp.sh` says which workshop it spawns. They are ignored by git and never shipped, so they are information, not dev state to clean up; deleting `.local-mcp/` while no session runs from it loses nothing. Present it as a short table, not prose. The useful signal is usually a *mismatch* — declared source vs what npm serves vs what the local build contains — so state plainly whether they agree, and if the user is on `npm-latest` and the checkout's workshop version equals npm `latest` with nothing under `## [Unreleased]`, say there is nothing to bump. diff --git a/.gitignore b/.gitignore index 0c82df23..f856a656 100644 --- a/.gitignore +++ b/.gitignore @@ -7,5 +7,5 @@ temp/ .pytest_cache/ .mypy_cache/ .ruff_cache/ -# The plugin copy `make claude-local-mcp` renders for a local workshop. +# The plugin copies `make claude-local-mcp` renders, one per workshop. .local-mcp/ diff --git a/docs/build-targets.md b/docs/build-targets.md index b6367ab9..5c7ce698 100644 --- a/docs/build-targets.md +++ b/docs/build-targets.md @@ -230,7 +230,7 @@ All targets share the same version string in lockstep — `make check` fails on | `platform` | `defaults.toml` (overridden per target) | `frontmatter.md.j2` (Claude-only `allowed-tools`) | | `harness_name` | `defaults.toml` (overridden per target) | every template that names the harness to the user or the model: skill text and shared partials, the hook wrappers, `launch-pipelex-mcp.sh.j2` and `mcp/vibe-mcp.toml.j2` | | `skill_dir` | `defaults.toml` (`${CLAUDE_SKILL_DIR}`), overridden to `` in `codex.toml` and `mistral-vibe.toml` | a sentence that names one of the skill's own files by path — today `pipelex-integrate`'s `cp` of its gate scripts. See below | -| `mcp_server` | `defaults.toml` (`[vars.mcp_server]` table, overridable per target, key by key) | `make_plugin_json()` — the local workshop launcher baked into the plugin-declared `pipelex-mcp` entry: Claude gets `type: stdio` pointing at the `launch-pipelex-mcp.sh` wrapper (which promotes the `PIPELEX_PLUGIN_*` user-config values to their real `PIPELEX_*` names only when non-empty, then `exec`s `command`/`args`), or `command`/`args` directly when the target declares no `user_config`; Codex gets bare `command`/`args` plus `env_vars` (variable *names* forwarded from the user's env — Codex whitelist-filters MCP spawn env; see [decisions.md](decisions.md) "Dual-MCP flip"). Dev override: `make claude-local-mcp` renders the Claude target with `command`/`args` pointed at another workshop into the ignored `.local-mcp/pipelex/` (`scripts/local_mcp.py`), and `make codex-local-mcp` starts Codex with `-c` overrides of its `pipelex` entry, `env_vars` included, since an entry at that tier replaces the plugin's whole; neither edits a tracked file. A target's own table merges into the defaults', so `env_vars` and `user_config` stay, and `make check` fails a target that lost the credential wiring anyway. `mcp/vibe-mcp.toml.j2`, the Vibe target's one MCP template (`MCP_TEMPLATES_BY_PLATFORM`), renders the same `command`/`args` as a Vibe `[[mcp_servers]]` stdio entry, listing every `env_vars` name as an empty `env` key the user fills in | +| `mcp_server` | `defaults.toml` (`[vars.mcp_server]` table, overridable per target, key by key) | `make_plugin_json()` — the local workshop launcher baked into the plugin-declared `pipelex-mcp` entry: Claude gets `type: stdio` pointing at the `launch-pipelex-mcp.sh` wrapper (which promotes the `PIPELEX_PLUGIN_*` user-config values to their real `PIPELEX_*` names only when non-empty, then `exec`s `command`/`args`), or `command`/`args` directly when the target declares no `user_config`; Codex gets bare `command`/`args` plus `env_vars` (variable *names* forwarded from the user's env — Codex whitelist-filters MCP spawn env; see [decisions.md](decisions.md) "Dual-MCP flip"). Dev override: `make claude-local-mcp` renders the Claude target with `command`/`args` pointed at another workshop into a directory of that workshop's own under the ignored `.local-mcp/` (`scripts/local_mcp.py`), and `make codex-local-mcp` starts Codex with `-c` overrides of its `pipelex` entry, `env_vars` included, since an entry at that tier replaces the plugin's whole; neither edits a tracked file. A target's own table merges into the defaults', so `env_vars` and `user_config` stay, and `make check` fails a target that lost the credential wiring anyway. `mcp/vibe-mcp.toml.j2`, the Vibe target's one MCP template (`MCP_TEMPLATES_BY_PLATFORM`), renders the same `command`/`args` as a Vibe `[[mcp_servers]]` stdio entry, listing every `env_vars` name as an empty `env` key the user fills in | | `floors` | `defaults.toml` (`[vars.floors]` table) | `pipelex-integrate` and `pipelex-scaffold`, which state the minimum versions to the user. See below | | `plugin_name` | derived from `[plugin].name` | available in all templates | diff --git a/docs/decisions.md b/docs/decisions.md index 44d1aa3a..e0baf3ea 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -697,8 +697,9 @@ Until the console's own release, the hosted console and the local workshop regis Running the skills against an unreleased `pipelex-mcp` workshop took five steps: build the workshop, make a worktree because the switch edits tracked files, rewrite `[vars.mcp_server]` and run `make build`, which dirtied the generated files of every target, load the plugin, and switch back before committing. That was enough that people did not do it. `make claude-local-mcp` and `make codex-local-mcp` replace it with one command, from a plain terminal, that changes nothing git tracks (`docs/development.md`, "A local build of `pipelex-mcp`"). - **A make target, not a skill.** What changes is the plugin a session loads at startup, and a skill runs inside a session that has already loaded it, so a skill could only rewrite files and ask for a relaunch. A make target runs before the session exists and can start it, and the steps hold no judgment. `/pipelex-mcp-source` keeps its status report and loses its switching arms, which now name the targets. -- **The Claude copy is rendered by the build's own renderer.** `scripts/local_mcp.py` loads the prod target, lays the workshop's `command` and `args` over its `[vars.mcp_server]` with `merge_template_vars`, so `env_vars` and `user_config` stay, and renders it with `build_target` into `.local-mcp/pipelex/`, which git ignores. The dev copy therefore cannot drift from what ships, and `tests/unit/test_local_mcp.py` holds it to the shipped target file for file, but for the launcher. The copy is rendered beside the previous one and swapped in, so a session still running from it never meets a half-written plugin. -- **The duplicate plugin is not one.** Verified on Claude Code 2.1.283: with `pipelex@pipelex-plugins` installed and enabled, `claude --plugin-dir .local-mcp/pipelex` loads the copy as `pipelex@inline`, lists no other `pipelex` plugin, registers one `plugin:pipelex:pipelex` server and spawns it once, from the copy. So the target passes no `enabledPlugins` override. +- **The Claude copy is rendered by the build's own renderer.** `scripts/local_mcp.py` loads the prod target, lays the workshop's `command` and `args` over its `[vars.mcp_server]` with `merge_template_vars`, so `env_vars` and `user_config` stay, and renders it with `build_target` into `.local-mcp//pipelex/`, which git ignores. The dev copy therefore cannot drift from what ships, and `tests/unit/test_local_mcp.py` holds it to the shipped target file for file, but for the launcher. +- **One copy per workshop, swapped in whole.** The key is a digest of the workshop's command and arguments. The copy's MCP entry spawns `${CLAUDE_PLUGIN_ROOT}/hooks/launch-pipelex-mcp.sh`, which bash reads again at every spawn, and a `/mcp` reconnect respawns the server, so in one shared copy a later start with another workshop would switch the workshop of a session already running. Each start renders into a staging directory of its own and swaps it in under a lock on the workshop's directory: the first version rendered every start into one shared staging directory, and two renders at once, tried 26 times, failed one of the two every time and in several trials swapped in a copy missing files. +- **The duplicate plugin is not one.** Verified on Claude Code 2.1.283: with `pipelex@pipelex-plugins` installed and enabled, `claude --plugin-dir` on the copy loads it as `pipelex@inline`, lists no other `pipelex` plugin, registers one `plugin:pipelex:pipelex` server and spawns it once, from the copy. So the target passes no `enabledPlugins` override. - **The key comes from the shell.** The copy's plugin options arrive empty, since the key saved for the installed plugin belongs to `pipelex@pipelex-plugins` and not to `pipelex@inline`. A probe in place of the workshop saw `PIPELEX_PLUGIN_API_KEY` and `PIPELEX_PLUGIN_BASE_URL` empty, so `launch-pipelex-mcp.sh` promoted nothing, and `PIPELEX_API_KEY` and `PIPELEX_BASE_URL` set from the shell. The non-empty guard recorded in "Claude credentials move to plugin userConfig" is what lets the shell's key through here. The target warns when `PIPELEX_API_KEY` is unset rather than refusing, since a session may want the tools that need no key. - **Codex's override carries `env_vars`.** Verified on Codex 0.156.1: a `-c mcp_servers.pipelex.*` override, like an entry in `~/.codex/config.toml`, replaces the plugin's entry whole, and `codex mcp list --json` then shows `env_vars: []`. A probe spawned that way saw no `PIPELEX_API_KEY`; with `-c mcp_servers.pipelex.env_vars=[…]` added, it saw both variables, and a live `mthds_list_methods` call answered. So the target sets the names the codex target forwards as well as the command, and the install page now says that an override of one's own needs them too. Codex keeps its installed skills; nothing is rendered for it. - **A published version is pinned at start.** `MCP_VERSION` accepts a version or a dist-tag, which npm resolves to one version before the agent starts, and the copy spawns exactly that version, so the workshop checked is the one run and a typo is refused with npm's own reason instead of failing as an npx error inside the session. A range that matches several versions is refused. diff --git a/docs/development.md b/docs/development.md index ba3dc1f8..d0a80f2a 100644 --- a/docs/development.md +++ b/docs/development.md @@ -79,7 +79,7 @@ make claude-local-mcp WORKDIR=~/my-methods ARGS='--model sonnet' A target given `MCP` first builds that checkout's workshop with its own `make build-local`, which writes `packages/workshop/dist/main.js`, and the agent then spawns it by absolute path. `MCP` defaults to `../pipelex-mcp`. A target given `MCP_VERSION` builds nothing: it asks npm which version that names and spawns exactly that one through `npx`, so a version npm does not know is refused before the agent starts. `WORKDIR` is where the session starts, which matters because the workshop resolves a `{ path }` file against it; it defaults to this checkout. `ARGS` goes to `claude` or `codex` as it is, so `ARGS='-p "…"'` runs one headless prompt. Start either target from a terminal, not from inside an agent's session. -- **Claude Code.** The target renders the Claude plugin from this checkout's templates, with the build's own renderer and the launcher pointed at the chosen workshop, into `.local-mcp/pipelex/`, which git ignores. It then starts `claude --plugin-dir` on that copy. The copy loads as `pipelex@inline` and takes the place of an installed `pipelex@pipelex-plugins` for that session only, so the skills you run are this checkout's too. Its plugin options arrive empty, since the key saved for the installed plugin is not the copy's, so the workshop takes the key exported in your shell as `PIPELEX_API_KEY`, and the target warns when there is none. +- **Claude Code.** The target renders the Claude plugin from this checkout's templates, with the build's own renderer and the launcher pointed at the chosen workshop, into a directory of that workshop's own under `.local-mcp/`, which git ignores. It then starts `claude --plugin-dir` on that copy. A session reads the copy's launcher again whenever it restarts the Pipelex tools, so each workshop has its own copy, and starting another workshop never changes the one a running session uses; starting the same workshop again renders its copy afresh from this checkout's templates. Delete `.local-mcp/` whenever no session runs from it. The copy loads as `pipelex@inline` and takes the place of an installed `pipelex@pipelex-plugins` for that session only, so the skills you run are this checkout's too. Its plugin options arrive empty, since the key saved for the installed plugin is not the copy's, so the workshop takes the key exported in your shell as `PIPELEX_API_KEY`, and the target warns when there is none. - **Codex.** The target renders nothing: it starts `codex` with `-c` overrides of the `mcp_servers.pipelex` entry. An entry given that way replaces the plugin's whole, so the overrides forward `PIPELEX_API_KEY` and `PIPELEX_BASE_URL` by name as the plugin does, and the workshop takes the key from your shell. The skills are whichever copy of the plugin Codex has installed: the published one, or this checkout after `make codex-use-local`. - **Mistral Vibe.** There is no target. Set `command = "node"` and `args = ["/path/to/pipelex-mcp/packages/workshop/dist/main.js"]` in the `pipelex` entry you appended to `~/.vibe/config.toml`, which Vibe reads instead of the fragment in this repository, after `make build-local` in that checkout. A checkout built before `pipelex-mcp` became an npm workspace still holds an older `dist/local/main.js` that nothing rebuilds any more, so never point Vibe at it. diff --git a/scripts/local_mcp.py b/scripts/local_mcp.py index 4498e807..b64f1fca 100644 --- a/scripts/local_mcp.py +++ b/scripts/local_mcp.py @@ -7,8 +7,8 @@ spawned through `npx` at the exact version npm resolves it to. - **Claude Code.** The Claude target is rendered by the build's own renderer, with `[vars.mcp_server]` - `command` and `args` pointed at that workshop and the rest of the table kept, into - `.local-mcp/pipelex/`, which git ignores, and Claude Code starts with `--plugin-dir` on it. The copy + `command` and `args` pointed at that workshop and the rest of the table kept, into a directory of + that workshop's own under `.local-mcp/`, which git ignores, and Claude Code starts with `--plugin-dir` on it. The copy loads as `pipelex@inline` and takes the place of an installed `pipelex@pipelex-plugins` for that session, so its skills are this checkout's. - **Codex.** Nothing is rendered. Codex starts with `-c` overrides of its `mcp_servers.pipelex` entry, @@ -24,11 +24,14 @@ import argparse import dataclasses +import fcntl +import hashlib import json import os import shutil import subprocess import sys +import tempfile from collections.abc import Callable from dataclasses import dataclass from enum import StrEnum @@ -45,7 +48,7 @@ write_files, ) -# Where the Claude copy is rendered, relative to the repository root. `.gitignore` lists it. +# Where the Claude copies are rendered, one per workshop, relative to the repository root. `.gitignore` lists it. LOCAL_DIR_NAME = ".local-mcp" # What `make build-local` writes in a pipelex-mcp checkout, and the manifest npm publishes it from. @@ -157,30 +160,43 @@ def published_launcher(spec: str, resolve: Callable[[str], str] = npm_resolve) - return Launcher(command="npx", args=["-y", f"{WORKSHOP_PACKAGE}@{version}"], label=f"{WORKSHOP_PACKAGE} {version} from npm") +def workshop_key(launcher: Launcher) -> str: + """The name of the directory under `.local-mcp/` that holds the copy spawning `launcher`, one per workshop.""" + return hashlib.sha256("\0".join([launcher.command, *launcher.args]).encode()).hexdigest()[:12] + + def render_claude_copy(base_dir: Path, launcher: Launcher) -> Path: - """Render the Claude target with `launcher` as its workshop into `.local-mcp//`, and return that directory. + """Render the Claude target with `launcher` as its workshop into `.local-mcp///`, and return that directory. The target's `[vars.mcp_server]` table gets the launcher's `command` and `args` and keeps the rest, `user_config` among it, by the merge every target's own table goes through, so the copy is what - `make build` would ship but for the workshop it spawns. It is rendered beside the previous copy - and swapped in, so a session still running from that copy never meets a half-written plugin. + `make build` would ship but for the workshop it spawns. + + Each workshop has a copy of its own, because a session reads its launcher again whenever it + respawns the server: in one shared copy, a later start with another workshop would change the + workshop of every session already running. A copy is rendered into a staging directory of this + run's own and swapped in under a lock, so two starts at once never write into each other's files, + and a session running from the copy never meets a half-written plugin. """ shipped = load_target_config(base_dir / TARGETS_DIR_NAME, CLAUDE_TARGET) - local_dir = base_dir / LOCAL_DIR_NAME - final = local_dir / shipped.plugin_name - staging = local_dir / f".{shipped.plugin_name}.staging" - retired = local_dir / f".{shipped.plugin_name}.retired" - for leftover in (staging, retired): - remove_path(leftover) - - template_vars = merge_template_vars(shipped.template_vars, {"mcp_server": {"command": launcher.command, "args": launcher.args}}) - config = dataclasses.replace(shipped, source=f"{staging.relative_to(base_dir).as_posix()}/", template_vars=template_vars) - write_files(build_target(base_dir, config).files) - - if final.exists() or final.is_symlink(): - final.rename(retired) - staging.rename(final) - remove_path(retired) + workshop_dir = base_dir / LOCAL_DIR_NAME / workshop_key(launcher) + workshop_dir.mkdir(parents=True, exist_ok=True) + final = workshop_dir / shipped.plugin_name + retired = workshop_dir / f".{shipped.plugin_name}.retired" + staging = Path(tempfile.mkdtemp(prefix=f".{shipped.plugin_name}.staging-", dir=workshop_dir)) + try: + template_vars = merge_template_vars(shipped.template_vars, {"mcp_server": {"command": launcher.command, "args": launcher.args}}) + config = dataclasses.replace(shipped, source=f"{staging.relative_to(base_dir).as_posix()}/", template_vars=template_vars) + write_files(build_target(base_dir, config).files) + with (workshop_dir / ".lock").open("w", encoding="utf-8") as lock: + fcntl.flock(lock, fcntl.LOCK_EX) + remove_path(retired) + if final.exists() or final.is_symlink(): + final.rename(retired) + staging.rename(final) + remove_path(retired) + finally: + remove_path(staging) return final diff --git a/tests/unit/test_local_mcp.py b/tests/unit/test_local_mcp.py index 7d7dc4e5..21ddb9cc 100644 --- a/tests/unit/test_local_mcp.py +++ b/tests/unit/test_local_mcp.py @@ -7,6 +7,7 @@ import shutil import subprocess import tomllib +from concurrent.futures import ThreadPoolExecutor from pathlib import Path from typing import cast @@ -31,6 +32,7 @@ split_passthrough, start, toml_string, + workshop_key, ) REPO_ROOT = Path(__file__).parents[2] @@ -144,7 +146,7 @@ def test_the_copy_is_the_shipped_target_but_for_the_workshop(self, repo: Path, c """The copy cannot drift from what ships: the same files, byte for byte and mode for mode, once the launcher is swapped back.""" launcher = checkout_launcher(checkout) copy = render_claude_copy(repo, launcher) - assert copy == repo / LOCAL_DIR_NAME / "pipelex" + assert copy == repo / LOCAL_DIR_NAME / workshop_key(launcher) / "pipelex" shipped_config = load_target_config(repo / "targets", "prod") write_files(build_target(repo, shipped_config).files) @@ -176,15 +178,34 @@ def test_the_launcher_keeps_the_credential_promotion(self, repo: Path, checkout: assert launcher.rstrip().endswith(f'exec node "{checkout.resolve() / WORKSHOP_BUNDLE}"') assert os.access(copy / "hooks" / "launch-pipelex-mcp.sh", os.X_OK) - def test_a_second_render_replaces_the_first_and_leaves_nothing_beside_it(self, repo: Path, checkout: Path) -> None: + def test_a_second_render_of_a_workshop_replaces_its_copy_and_leaves_nothing_beside_it(self, repo: Path, checkout: Path) -> None: copy = render_claude_copy(repo, checkout_launcher(checkout)) (copy / "stray.txt").write_text("left by hand\n", encoding="utf-8") - again = render_claude_copy(repo, _published("latest")) + again = render_claude_copy(repo, checkout_launcher(checkout)) assert again == copy assert not (copy / "stray.txt").exists() - assert (copy / "hooks" / "launch-pipelex-mcp.sh").read_text(encoding="utf-8").rstrip().endswith('exec npx "-y" "@pipelex/mcp@0.20.0"') - assert sorted(path.name for path in (repo / LOCAL_DIR_NAME).iterdir()) == ["pipelex"] + assert sorted(path.name for path in copy.parent.iterdir()) == [".lock", "pipelex"] + + def test_another_workshop_gets_a_copy_of_its_own_and_leaves_the_first_as_it_was(self, repo: Path, checkout: Path) -> None: + """A session reads its launcher again at every respawn, so a shared copy would switch the workshop under it.""" + first = render_claude_copy(repo, checkout_launcher(checkout)) + before = _tree(first) + + second = render_claude_copy(repo, _published("latest")) + assert second != first + assert _tree(first) == before + assert (second / "hooks" / "launch-pipelex-mcp.sh").read_text(encoding="utf-8").rstrip().endswith('exec npx "-y" "@pipelex/mcp@0.20.0"') + + def test_renders_of_one_workshop_at_once_each_swap_in_a_whole_copy(self, repo: Path, checkout: Path) -> None: + """Two starts at the same moment render apart and swap in turn, so neither fails and neither leaves a partial copy.""" + launcher = checkout_launcher(checkout) + whole = _tree(render_claude_copy(repo, launcher)) + with ThreadPoolExecutor(max_workers=4) as pool: + copies = [future.result() for future in [pool.submit(render_claude_copy, repo, launcher) for _ in range(4)]] + assert set(copies) == {repo / LOCAL_DIR_NAME / workshop_key(launcher) / "pipelex"} + assert _tree(copies[0]) == whole + assert sorted(path.name for path in copies[0].parent.iterdir()) == [".lock", "pipelex"] def test_the_render_writes_nothing_outside_the_local_directory(self, repo: Path, checkout: Path) -> None: before = _tree(repo, skip=LOCAL_DIR_NAME) @@ -193,7 +214,7 @@ def test_the_render_writes_nothing_outside_the_local_directory(self, repo: Path, @pytest.mark.skipif(shutil.which("git") is None, reason="no git on the PATH") def test_git_ignores_the_local_directory(self) -> None: - probe = f"{LOCAL_DIR_NAME}/pipelex/.claude-plugin/plugin.json" + probe = f"{LOCAL_DIR_NAME}/0123456789ab/pipelex/.claude-plugin/plugin.json" completed = subprocess.run(["git", "-C", str(REPO_ROOT), "check-ignore", "-q", "--no-index", probe], check=False) assert completed.returncode == 0, f"{LOCAL_DIR_NAME}/ must be in .gitignore, since the copy is never committed" @@ -256,9 +277,11 @@ def execv(program: str, argv: list[str]) -> None: return chdirs def test_claude_starts_on_the_copy_in_the_workdir(self, repo: Path, checkout: Path, tmp_path: Path, harness_calls: list[Path]) -> None: + launcher = checkout_launcher(checkout) with pytest.raises(Launched) as launched: - start(Harness.CLAUDE, checkout_launcher(checkout), tmp_path, ["-p", "hello"], repo) - assert launched.value.argv == ["/usr/local/bin/claude", "--plugin-dir", str(repo / LOCAL_DIR_NAME / "pipelex"), "-p", "hello"] + start(Harness.CLAUDE, launcher, tmp_path, ["-p", "hello"], repo) + copy = repo / LOCAL_DIR_NAME / workshop_key(launcher) / "pipelex" + assert launched.value.argv == ["/usr/local/bin/claude", "--plugin-dir", str(copy), "-p", "hello"] assert harness_calls == [tmp_path] def test_codex_starts_with_the_overrides_and_renders_nothing(self, repo: Path, checkout: Path, tmp_path: Path, harness_calls: list[Path]) -> None: From c2a52d635a8652594a620f668918eb1e6dbe19e3 Mon Sep 17 00:00:00 2001 From: Louis Choquel Date: Sat, 26 Sep 2026 18:04:43 +0200 Subject: [PATCH 03/10] Build the checkout from the script, so its refusals are the ones met The Makefile's build line ran before the script, so a wrong MCP met make's own error and the script's refusal naming MCP could never be reached. The script now checks the checkout, runs its `make build-local` with the make target's variables removed from the environment, then looks for the workshop. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_014AcpM2qUzbWMenP62iSBoS --- Makefile | 19 ++----- docs/decisions.md | 2 +- scripts/local_mcp.py | 33 +++++++++-- tests/unit/test_local_mcp.py | 107 ++++++++++++++++++++++++++--------- 4 files changed, 114 insertions(+), 47 deletions(-) diff --git a/Makefile b/Makefile index 2bc61221..fa4342db 100644 --- a/Makefile +++ b/Makefile @@ -230,12 +230,12 @@ codex-status: ## Show which source is currently registered for the Codex pipelex ### LOCAL WORKSHOP (start an agent on a pipelex-mcp the plugin does not ship) ########################################################################################## -# MCP is a pipelex-mcp checkout or worktree, whose workshop is built with its own -# `make build-local` first; MCP_VERSION, a published @pipelex/mcp version or dist-tag, -# replaces it and builds nothing. WORKDIR is where the session starts, which is where -# the workshop resolves a { path } file. ARGS goes to claude or codex as it is. -# Neither target writes a tracked file: see "A local build of pipelex-mcp" in -# docs/development.md. +# MCP is a pipelex-mcp checkout or worktree, whose workshop scripts/local_mcp.py builds +# with the checkout's own `make build-local` first; MCP_VERSION, a published @pipelex/mcp +# version or dist-tag, replaces it and builds nothing. WORKDIR is where the session +# starts, which is where the workshop resolves a { path } file. ARGS goes to claude or +# codex as it is. Neither target writes a tracked file: see "A local build of +# pipelex-mcp" in docs/development.md. MCP ?= ../pipelex-mcp MCP_VERSION ?= WORKDIR ?= . @@ -243,15 +243,8 @@ ARGS ?= LOCAL_MCP_SOURCE = $(if $(MCP_VERSION),--mcp-version "$(MCP_VERSION)",--mcp "$(MCP)") -# The checkout's build runs with none of these variables: make hands a sub-make the ones -# given on its command line through MAKEFLAGS and the environment, where they would -# override or fill that Makefile's own. -LOCAL_MCP_BUILD = $(if $(MCP_VERSION),:,unset MCP MCP_VERSION WORKDIR ARGS; MAKEFLAGS= $(MAKE) --no-print-directory -C "$(MCP)" build-local) - claude-local-mcp: install ## Start Claude Code on this checkout's skills with another workshop (MCP=path or MCP_VERSION=x.y.z) - @$(LOCAL_MCP_BUILD) @$(VENV_PYTHON) scripts/local_mcp.py claude $(LOCAL_MCP_SOURCE) --workdir "$(WORKDIR)" -- $(ARGS) codex-local-mcp: install ## Start Codex with another workshop in place of the plugin's (MCP=path or MCP_VERSION=x.y.z) - @$(LOCAL_MCP_BUILD) @$(VENV_PYTHON) scripts/local_mcp.py codex $(LOCAL_MCP_SOURCE) --workdir "$(WORKDIR)" -- $(ARGS) diff --git a/docs/decisions.md b/docs/decisions.md index e0baf3ea..4ab5e91e 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -703,7 +703,7 @@ Running the skills against an unreleased `pipelex-mcp` workshop took five steps: - **The key comes from the shell.** The copy's plugin options arrive empty, since the key saved for the installed plugin belongs to `pipelex@pipelex-plugins` and not to `pipelex@inline`. A probe in place of the workshop saw `PIPELEX_PLUGIN_API_KEY` and `PIPELEX_PLUGIN_BASE_URL` empty, so `launch-pipelex-mcp.sh` promoted nothing, and `PIPELEX_API_KEY` and `PIPELEX_BASE_URL` set from the shell. The non-empty guard recorded in "Claude credentials move to plugin userConfig" is what lets the shell's key through here. The target warns when `PIPELEX_API_KEY` is unset rather than refusing, since a session may want the tools that need no key. - **Codex's override carries `env_vars`.** Verified on Codex 0.156.1: a `-c mcp_servers.pipelex.*` override, like an entry in `~/.codex/config.toml`, replaces the plugin's entry whole, and `codex mcp list --json` then shows `env_vars: []`. A probe spawned that way saw no `PIPELEX_API_KEY`; with `-c mcp_servers.pipelex.env_vars=[…]` added, it saw both variables, and a live `mthds_list_methods` call answered. So the target sets the names the codex target forwards as well as the command, and the install page now says that an override of one's own needs them too. Codex keeps its installed skills; nothing is rendered for it. - **A published version is pinned at start.** `MCP_VERSION` accepts a version or a dist-tag, which npm resolves to one version before the agent starts, and the copy spawns exactly that version, so the workshop checked is the one run and a typo is refused with npm's own reason instead of failing as an npx error inside the session. A range that matches several versions is refused. -- **The checkout's build sees none of the targets' variables.** make hands a sub-make the variables given on its command line, through `MAKEFLAGS` and the environment, where `ARGS` or `MCP` would override or fill the checkout's own Makefile. The build line unsets them and clears `MAKEFLAGS`, which GNU Make 3.81, the one macOS ships, needs: a target-specific `MAKEOVERRIDES =` does not stop them there. +- **The script builds the checkout, and the build sees none of the targets' variables.** `scripts/local_mcp.py` runs the checkout's `make build-local` itself, after it has checked that the checkout exists, so its refusal naming `MCP` is what a wrong path meets, rather than make's own error from a build line in the Makefile that ran first. make hands a target's commands the variables given on its command line, through `MAKEFLAGS` and the environment, where `ARGS` or `MCP` would override or fill the checkout's own Makefile, so the script runs the build with those and make's own variables removed from its environment. The first version ran the build from the Makefile, where GNU Make 3.81, the one macOS ships, needed `unset` and an empty `MAKEFLAGS` on the build line: a target-specific `MAKEOVERRIDES =` did not stop them there. - **Mistral Vibe is out of scope.** Its `~/.vibe/config.toml` entry is the user's own file, so there is nothing to render and nothing to override from the command line. ## License & distribution diff --git a/scripts/local_mcp.py b/scripts/local_mcp.py index b64f1fca..65151a38 100644 --- a/scripts/local_mcp.py +++ b/scripts/local_mcp.py @@ -2,7 +2,7 @@ """Start Claude Code or Codex with a Pipelex workshop the plugin does not ship, touching no tracked file. `make claude-local-mcp` and `make codex-local-mcp` run this. The workshop is a `pipelex-mcp` checkout's -own build (`--mcp`, after the make target has run `make build-local` there), spawned as +own build (`--mcp`, which runs the checkout's `make build-local` first), spawned as `node /packages/workshop/dist/main.js`, or a published `@pipelex/mcp` (`--mcp-version`), spawned through `npx` at the exact version npm resolves it to. @@ -56,6 +56,10 @@ WORKSHOP_MANIFEST = Path("packages/workshop/package.json") WORKSHOP_PACKAGE = "@pipelex/mcp" +# The variables a make target hands its commands beside its own: make's, then this repository's +# local-workshop targets'. None of them reaches the checkout's build. +MAKE_HANDOFF = frozenset({"MAKEFLAGS", "MFLAGS", "MAKEOVERRIDES", "MAKELEVEL", "MCP", "MCP_VERSION", "WORKDIR", "ARGS"}) + CLAUDE_TARGET = "prod" CODEX_TARGET = "codex" KEY_VARIABLE = "PIPELEX_API_KEY" @@ -75,8 +79,26 @@ class Launcher: label: str -def checkout_launcher(checkout: Path) -> Launcher: - """The workshop a `pipelex-mcp` checkout or worktree has built, by absolute path. +def build_checkout(root: Path) -> None: + """Build the checkout's workshop with its own `make build-local`, given none of the make target's variables. + + make hands the commands of a target the variables on its command line, through `MAKEFLAGS` and + the environment, and a make run from here would read them there: `ARGS` or `MCP` would then + override or fill the checkout's own Makefile. + """ + make = shutil.which("make") + if make is None: + msg = "make is not on the PATH, and a checkout's workshop is built with its `make build-local`." + raise SystemExit(msg) + environment = {name: value for name, value in os.environ.items() if name not in MAKE_HANDOFF} + completed = subprocess.run([make, "--no-print-directory", "-C", str(root), "build-local"], env=environment, check=False) + if completed.returncode != 0: + msg = f"`make build-local` failed in {root}, so no workshop was started." + raise SystemExit(msg) + + +def checkout_launcher(checkout: Path, build: Callable[[Path], None] = build_checkout) -> Launcher: + """The workshop of a `pipelex-mcp` checkout or worktree, built now and spawned by absolute path. Absolute, because the harness spawns the workshop from the session's working directory, which is neither this repository nor the checkout. @@ -85,9 +107,10 @@ def checkout_launcher(checkout: Path) -> Launcher: if not root.is_dir(): msg = f"There is no pipelex-mcp checkout at {root}: pass MCP=." raise SystemExit(msg) + build(root) bundle = root / WORKSHOP_BUNDLE if not bundle.is_file(): - msg = f"{bundle} does not exist: build it with `make build-local` in {root}." + msg = f"{bundle} does not exist after `make build-local` in {root}: MCP must name a pipelex-mcp checkout that builds the workshop package." raise SystemExit(msg) return Launcher(command="node", args=[str(bundle)], label=f"the build in {root} ({WORKSHOP_PACKAGE} {_manifest_version(root)})") @@ -286,7 +309,7 @@ def parse_args(argv: list[str]) -> argparse.Namespace: parser = argparse.ArgumentParser(description=__doc__.splitlines()[0] if __doc__ else None) parser.add_argument("harness", choices=[harness.value for harness in Harness], help="The agent to start.") source = parser.add_mutually_exclusive_group(required=True) - source.add_argument("--mcp", type=Path, help="A pipelex-mcp checkout or worktree whose `make build-local` has run (the Makefile's MCP).") + source.add_argument("--mcp", type=Path, help="A pipelex-mcp checkout or worktree, built with its `make build-local` first (the Makefile's MCP).") source.add_argument("--mcp-version", help=f"A published {WORKSHOP_PACKAGE} version or dist-tag (the Makefile's MCP_VERSION).") parser.add_argument("--workdir", type=Path, default=Path(), help="Where the session starts, and so where the workshop resolves a { path } file.") return parser.parse_args(argv) diff --git a/tests/unit/test_local_mcp.py b/tests/unit/test_local_mcp.py index 21ddb9cc..ec2a28fe 100644 --- a/tests/unit/test_local_mcp.py +++ b/tests/unit/test_local_mcp.py @@ -18,9 +18,11 @@ from scripts.gen_skill_docs import build_target, load_target_config, write_files from scripts.local_mcp import ( LOCAL_DIR_NAME, + MAKE_HANDOFF, WORKSHOP_BUNDLE, Harness, Launcher, + build_checkout, checkout_launcher, codex_env_vars, codex_overrides, @@ -61,6 +63,11 @@ def checkout(tmp_path: Path) -> Path: return root +def _built(checkout: Path) -> Launcher: + """The checkout's launcher, its `make build-local` taken as run: the fixture's bundle is already there.""" + return checkout_launcher(checkout, build=lambda _root: None) + + def _published(spec: str) -> Launcher: return published_launcher(spec, resolve=lambda _spec: "0.20.0") @@ -98,19 +105,53 @@ class TestLauncher: def test_a_checkout_runs_its_build_by_absolute_path(self, checkout: Path, monkeypatch: pytest.MonkeyPatch) -> None: """The harness spawns the workshop from the session's directory, so a relative path would name another file.""" monkeypatch.chdir(checkout.parent) - launcher = checkout_launcher(Path("pipelex-mcp")) + launcher = _built(Path("pipelex-mcp")) assert launcher.command == "node" assert launcher.args == [str(checkout.resolve() / WORKSHOP_BUNDLE)] assert "@pipelex/mcp 1.2.3" in launcher.label - def test_a_checkout_without_its_build_is_refused_with_the_build_to_run(self, checkout: Path) -> None: + def test_a_checkout_whose_build_writes_no_workshop_is_refused(self, checkout: Path) -> None: + (checkout / WORKSHOP_BUNDLE).unlink() + with pytest.raises(SystemExit, match="does not exist after `make build-local`"): + _built(checkout) + + def test_the_checkout_is_built_before_its_workshop_is_looked_for(self, checkout: Path) -> None: (checkout / WORKSHOP_BUNDLE).unlink() - with pytest.raises(SystemExit, match="make build-local"): - checkout_launcher(checkout) + built: list[Path] = [] + + def build(root: Path) -> None: + built.append(root) + (root / WORKSHOP_BUNDLE).write_text("// built now\n", encoding="utf-8") + + launcher = checkout_launcher(checkout, build=build) + assert built == [checkout.resolve()] + assert launcher.args == [str(checkout.resolve() / WORKSHOP_BUNDLE)] - def test_a_missing_checkout_names_the_variable_to_set(self, tmp_path: Path) -> None: + def test_a_missing_checkout_names_the_variable_to_set_and_builds_nothing(self, tmp_path: Path) -> None: + built: list[Path] = [] with pytest.raises(SystemExit, match="MCP="): - checkout_launcher(tmp_path / "nowhere") + checkout_launcher(tmp_path / "nowhere", build=built.append) + assert built == [] + + def test_the_build_is_given_none_of_the_make_targets_variables( + self, checkout: Path, mocker: MockerFixture, monkeypatch: pytest.MonkeyPatch + ) -> None: + """make hands a target's commands its command-line variables, which would override or fill the checkout's Makefile.""" + for name in MAKE_HANDOFF: + monkeypatch.setenv(name, "from the make target") + mocker.patch.object(local_mcp.shutil, "which", return_value="/usr/bin/make") + run = mocker.patch.object(local_mcp.subprocess, "run", return_value=subprocess.CompletedProcess(args=[], returncode=0)) + build_checkout(checkout) + assert run.call_args.args[0] == ["/usr/bin/make", "--no-print-directory", "-C", str(checkout), "build-local"] + environment = run.call_args.kwargs["env"] + assert not MAKE_HANDOFF & environment.keys() + assert environment["PATH"] == os.environ["PATH"] + + def test_a_failed_build_starts_nothing(self, checkout: Path, mocker: MockerFixture) -> None: + mocker.patch.object(local_mcp.shutil, "which", return_value="/usr/bin/make") + mocker.patch.object(local_mcp.subprocess, "run", return_value=subprocess.CompletedProcess(args=[], returncode=2)) + with pytest.raises(SystemExit, match="`make build-local` failed"): + build_checkout(checkout) def test_a_published_version_is_pinned_to_the_one_npm_resolves(self) -> None: launcher = _published("latest") @@ -144,7 +185,7 @@ def test_a_range_matching_several_versions_is_refused(self, mocker: MockerFixtur class TestClaudeCopy: def test_the_copy_is_the_shipped_target_but_for_the_workshop(self, repo: Path, checkout: Path) -> None: """The copy cannot drift from what ships: the same files, byte for byte and mode for mode, once the launcher is swapped back.""" - launcher = checkout_launcher(checkout) + launcher = _built(checkout) copy = render_claude_copy(repo, launcher) assert copy == repo / LOCAL_DIR_NAME / workshop_key(launcher) / "pipelex" @@ -171,7 +212,7 @@ def test_the_copy_is_the_shipped_target_but_for_the_workshop(self, repo: Path, c assert Path("hooks/launch-pipelex-mcp.sh") in swapped def test_the_launcher_keeps_the_credential_promotion(self, repo: Path, checkout: Path) -> None: - copy = render_claude_copy(repo, checkout_launcher(checkout)) + copy = render_claude_copy(repo, _built(checkout)) launcher = (copy / "hooks" / "launch-pipelex-mcp.sh").read_text(encoding="utf-8") for key in ("API_KEY", "BASE_URL"): assert f'export PIPELEX_{key}="$PIPELEX_PLUGIN_{key}"' in launcher @@ -179,17 +220,17 @@ def test_the_launcher_keeps_the_credential_promotion(self, repo: Path, checkout: assert os.access(copy / "hooks" / "launch-pipelex-mcp.sh", os.X_OK) def test_a_second_render_of_a_workshop_replaces_its_copy_and_leaves_nothing_beside_it(self, repo: Path, checkout: Path) -> None: - copy = render_claude_copy(repo, checkout_launcher(checkout)) + copy = render_claude_copy(repo, _built(checkout)) (copy / "stray.txt").write_text("left by hand\n", encoding="utf-8") - again = render_claude_copy(repo, checkout_launcher(checkout)) + again = render_claude_copy(repo, _built(checkout)) assert again == copy assert not (copy / "stray.txt").exists() assert sorted(path.name for path in copy.parent.iterdir()) == [".lock", "pipelex"] def test_another_workshop_gets_a_copy_of_its_own_and_leaves_the_first_as_it_was(self, repo: Path, checkout: Path) -> None: """A session reads its launcher again at every respawn, so a shared copy would switch the workshop under it.""" - first = render_claude_copy(repo, checkout_launcher(checkout)) + first = render_claude_copy(repo, _built(checkout)) before = _tree(first) second = render_claude_copy(repo, _published("latest")) @@ -199,7 +240,7 @@ def test_another_workshop_gets_a_copy_of_its_own_and_leaves_the_first_as_it_was( def test_renders_of_one_workshop_at_once_each_swap_in_a_whole_copy(self, repo: Path, checkout: Path) -> None: """Two starts at the same moment render apart and swap in turn, so neither fails and neither leaves a partial copy.""" - launcher = checkout_launcher(checkout) + launcher = _built(checkout) whole = _tree(render_claude_copy(repo, launcher)) with ThreadPoolExecutor(max_workers=4) as pool: copies = [future.result() for future in [pool.submit(render_claude_copy, repo, launcher) for _ in range(4)]] @@ -209,7 +250,7 @@ def test_renders_of_one_workshop_at_once_each_swap_in_a_whole_copy(self, repo: P def test_the_render_writes_nothing_outside_the_local_directory(self, repo: Path, checkout: Path) -> None: before = _tree(repo, skip=LOCAL_DIR_NAME) - render_claude_copy(repo, checkout_launcher(checkout)) + render_claude_copy(repo, _built(checkout)) assert _tree(repo, skip=LOCAL_DIR_NAME) == before @pytest.mark.skipif(shutil.which("git") is None, reason="no git on the PATH") @@ -222,7 +263,7 @@ def test_git_ignores_the_local_directory(self) -> None: class TestCodexOverrides: def test_the_overrides_replace_the_entry_and_forward_the_key_names(self, checkout: Path) -> None: """A configuration-tier entry replaces the plugin's whole, so the names it forwards must be given again.""" - launcher = checkout_launcher(checkout) + launcher = _built(checkout) overrides = codex_overrides(launcher, ["PIPELEX_API_KEY", "PIPELEX_BASE_URL"]) assert overrides[0::2] == ["-c", "-c", "-c"] parsed = {key: tomllib.loads(f"v = {value}")["v"] for key, value in (item.split("=", 1) for item in overrides[1::2])} @@ -277,7 +318,7 @@ def execv(program: str, argv: list[str]) -> None: return chdirs def test_claude_starts_on_the_copy_in_the_workdir(self, repo: Path, checkout: Path, tmp_path: Path, harness_calls: list[Path]) -> None: - launcher = checkout_launcher(checkout) + launcher = _built(checkout) with pytest.raises(Launched) as launched: start(Harness.CLAUDE, launcher, tmp_path, ["-p", "hello"], repo) copy = repo / LOCAL_DIR_NAME / workshop_key(launcher) / "pipelex" @@ -285,7 +326,7 @@ def test_claude_starts_on_the_copy_in_the_workdir(self, repo: Path, checkout: Pa assert harness_calls == [tmp_path] def test_codex_starts_with_the_overrides_and_renders_nothing(self, repo: Path, checkout: Path, tmp_path: Path, harness_calls: list[Path]) -> None: - launcher = checkout_launcher(checkout) + launcher = _built(checkout) with pytest.raises(Launched) as launched: start(Harness.CODEX, launcher, tmp_path, ["exec", "hello"], repo) assert launched.value.argv == ["/usr/local/bin/codex", *codex_overrides(launcher, codex_env_vars(repo)), "exec", "hello"] @@ -335,35 +376,45 @@ def test_a_workdir_that_is_not_a_directory_is_refused(self, checkout: Path, tmp_ @pytest.mark.skipif(shutil.which("make") is None, reason="no make on the PATH") class TestMakeTargets: - def _make(self, *arguments: str) -> subprocess.CompletedProcess[str]: + def _make(self, *arguments: str, home: Path | None = None) -> subprocess.CompletedProcess[str]: """This repository's make, told its venv is installed: `install` would update uv over the network.""" + environment = {**os.environ, "HOME": str(home)} if home is not None else None return subprocess.run( ["make", "--no-print-directory", "-C", str(REPO_ROOT), "-o", "install", *arguments], capture_output=True, text=True, check=False, + env=environment, ) @pytest.mark.skipif(not (REPO_ROOT / ".venv" / "bin" / "python").is_file(), reason="no venv: run `make install`") - def test_the_checkout_is_built_with_none_of_the_targets_variables(self, tmp_path: Path) -> None: - """make hands a sub-make its command-line variables, through MAKEFLAGS and the environment, which would override or fill the checkout's own. + @pytest.mark.parametrize("home_relative", [False, True]) + def test_the_checkout_is_built_with_none_of_the_targets_variables(self, tmp_path: Path, home_relative: bool) -> None: + """make hands a target's commands its command-line variables, which would override or fill the checkout's own. - This one runs for real, since `make -n` prints the build line without running it: the checkout's - build only reports what it sees, and the script then refuses the checkout for having built no - workshop, so nothing starts. + This one runs for real: the checkout's build only reports what it sees, and the script then + refuses the checkout for having built no workshop, so nothing starts. zsh passes `MCP=~/…` with + its tilde as it is, so that spelling is run too. """ - (tmp_path / "Makefile").write_text( + checkout = tmp_path / "pipelex-mcp" + checkout.mkdir() + (checkout / "Makefile").write_text( 'ARGS = its-own\nbuild-local:\n\t@echo "built with ARGS=[$(ARGS)] MCP=[$(MCP)] MCP_VERSION=[$(MCP_VERSION)] WORKDIR=[$(WORKDIR)]"\n', encoding="utf-8", ) - completed = self._make("claude-local-mcp", f"MCP={tmp_path}", "ARGS=--model sonnet", f"WORKDIR={tmp_path}") + mcp = "~/pipelex-mcp" if home_relative else str(checkout) + completed = self._make("claude-local-mcp", f"MCP={mcp}", "ARGS=--model sonnet", f"WORKDIR={tmp_path}", home=tmp_path) assert "built with ARGS=[its-own] MCP=[] MCP_VERSION=[] WORKDIR=[]" in completed.stdout - assert "build it with `make build-local`" in completed.stderr + assert "does not exist after `make build-local`" in completed.stderr + assert completed.returncode != 0 + + @pytest.mark.skipif(not (REPO_ROOT / ".venv" / "bin" / "python").is_file(), reason="no venv: run `make install`") + def test_a_missing_checkout_is_refused_with_the_variable_to_set(self, tmp_path: Path) -> None: + completed = self._make("claude-local-mcp", f"MCP={tmp_path / 'nowhere'}") + assert "pass MCP=" in completed.stderr assert completed.returncode != 0 - assert not (REPO_ROOT / LOCAL_DIR_NAME / ".pipelex.staging").exists() - def test_a_published_version_builds_nothing(self) -> None: + def test_a_published_version_reaches_the_script_as_a_version(self) -> None: completed = self._make("-n", "claude-local-mcp", "MCP_VERSION=0.20.0", "MCP=/no/checkout/here", "ARGS=--model sonnet") assert completed.returncode == 0, completed.stderr - assert "build-local" not in completed.stdout assert 'scripts/local_mcp.py claude --mcp-version "0.20.0" --workdir "." -- --model sonnet' in completed.stdout From ba71b039ecb82519f345ef47bbd59c580a5f813c Mon Sep 17 00:00:00 2001 From: Louis Choquel Date: Sat, 26 Sep 2026 18:06:30 +0200 Subject: [PATCH 04/10] Quote the launcher's arguments for the shell launch-pipelex-mcp.sh wrote each argument in bare double quotes, so a checkout path holding `$`, a backtick, `"` or `\` was expanded or broke the word, and under `set -u` a `$name` aborted the launcher. A renderer filter escapes the four; every shipped output renders as it did. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_014AcpM2qUzbWMenP62iSBoS --- docs/build-targets.md | 2 +- scripts/gen_skill_docs.py | 13 +++++++++++++ templates/hooks/launch-pipelex-mcp.sh.j2 | 2 +- tests/unit/test_local_mcp.py | 17 +++++++++++++++++ 4 files changed, 32 insertions(+), 2 deletions(-) diff --git a/docs/build-targets.md b/docs/build-targets.md index 5c7ce698..c45d99a2 100644 --- a/docs/build-targets.md +++ b/docs/build-targets.md @@ -230,7 +230,7 @@ All targets share the same version string in lockstep — `make check` fails on | `platform` | `defaults.toml` (overridden per target) | `frontmatter.md.j2` (Claude-only `allowed-tools`) | | `harness_name` | `defaults.toml` (overridden per target) | every template that names the harness to the user or the model: skill text and shared partials, the hook wrappers, `launch-pipelex-mcp.sh.j2` and `mcp/vibe-mcp.toml.j2` | | `skill_dir` | `defaults.toml` (`${CLAUDE_SKILL_DIR}`), overridden to `` in `codex.toml` and `mistral-vibe.toml` | a sentence that names one of the skill's own files by path — today `pipelex-integrate`'s `cp` of its gate scripts. See below | -| `mcp_server` | `defaults.toml` (`[vars.mcp_server]` table, overridable per target, key by key) | `make_plugin_json()` — the local workshop launcher baked into the plugin-declared `pipelex-mcp` entry: Claude gets `type: stdio` pointing at the `launch-pipelex-mcp.sh` wrapper (which promotes the `PIPELEX_PLUGIN_*` user-config values to their real `PIPELEX_*` names only when non-empty, then `exec`s `command`/`args`), or `command`/`args` directly when the target declares no `user_config`; Codex gets bare `command`/`args` plus `env_vars` (variable *names* forwarded from the user's env — Codex whitelist-filters MCP spawn env; see [decisions.md](decisions.md) "Dual-MCP flip"). Dev override: `make claude-local-mcp` renders the Claude target with `command`/`args` pointed at another workshop into a directory of that workshop's own under the ignored `.local-mcp/` (`scripts/local_mcp.py`), and `make codex-local-mcp` starts Codex with `-c` overrides of its `pipelex` entry, `env_vars` included, since an entry at that tier replaces the plugin's whole; neither edits a tracked file. A target's own table merges into the defaults', so `env_vars` and `user_config` stay, and `make check` fails a target that lost the credential wiring anyway. `mcp/vibe-mcp.toml.j2`, the Vibe target's one MCP template (`MCP_TEMPLATES_BY_PLATFORM`), renders the same `command`/`args` as a Vibe `[[mcp_servers]]` stdio entry, listing every `env_vars` name as an empty `env` key the user fills in | +| `mcp_server` | `defaults.toml` (`[vars.mcp_server]` table, overridable per target, key by key) | `make_plugin_json()` — the local workshop launcher baked into the plugin-declared `pipelex-mcp` entry: Claude gets `type: stdio` pointing at the `launch-pipelex-mcp.sh` wrapper (which promotes the `PIPELEX_PLUGIN_*` user-config values to their real `PIPELEX_*` names only when non-empty, then `exec`s `command`/`args`, each argument double-quoted by the renderer's `shell_double_quoted` filter so a path holding `$`, a backtick, `"` or `\` reaches the workshop as it is), or `command`/`args` directly when the target declares no `user_config`; Codex gets bare `command`/`args` plus `env_vars` (variable *names* forwarded from the user's env — Codex whitelist-filters MCP spawn env; see [decisions.md](decisions.md) "Dual-MCP flip"). Dev override: `make claude-local-mcp` renders the Claude target with `command`/`args` pointed at another workshop into a directory of that workshop's own under the ignored `.local-mcp/` (`scripts/local_mcp.py`), and `make codex-local-mcp` starts Codex with `-c` overrides of its `pipelex` entry, `env_vars` included, since an entry at that tier replaces the plugin's whole; neither edits a tracked file. A target's own table merges into the defaults', so `env_vars` and `user_config` stay, and `make check` fails a target that lost the credential wiring anyway. `mcp/vibe-mcp.toml.j2`, the Vibe target's one MCP template (`MCP_TEMPLATES_BY_PLATFORM`), renders the same `command`/`args` as a Vibe `[[mcp_servers]]` stdio entry, listing every `env_vars` name as an empty `env` key the user fills in | | `floors` | `defaults.toml` (`[vars.floors]` table) | `pipelex-integrate` and `pipelex-scaffold`, which state the minimum versions to the user. See below | | `plugin_name` | derived from `[plugin].name` | available in all templates | diff --git a/scripts/gen_skill_docs.py b/scripts/gen_skill_docs.py index 98054f07..3e62db40 100644 --- a/scripts/gen_skill_docs.py +++ b/scripts/gen_skill_docs.py @@ -347,6 +347,18 @@ def resolve_output_dir(base_dir: Path, source: str) -> Path: return base_dir / source.rstrip("/") +def shell_double_quoted(value: object) -> str: + """`value` as one double-quoted shell word, which a shell hands on exactly as it is. + + Inside double quotes a shell still acts on `$`, a backtick and `\\`, and `"` ends the word, so each + of the four is escaped. A word holding none of them renders as it always did. + """ + text = str(value) + for char in ("\\", '"', "$", "`"): + text = text.replace(char, f"\\{char}") + return f'"{text}"' + + def _render_or_die(env: Environment, template_name: str, template_vars: Mapping[str, TemplateVarValue]) -> str: """Render one template by name, turning Jinja errors into a clean SystemExit.""" try: @@ -411,6 +423,7 @@ def render_templates( keep_trailing_newline=True, undefined=StrictUndefined, ) + env.filters["shell_double_quoted"] = shell_double_quoted # Collect shared templates (must all exist — fail loudly if missing) shared_j2_paths: list[Path] = [] diff --git a/templates/hooks/launch-pipelex-mcp.sh.j2 b/templates/hooks/launch-pipelex-mcp.sh.j2 index 124b4201..687c4271 100644 --- a/templates/hooks/launch-pipelex-mcp.sh.j2 +++ b/templates/hooks/launch-pipelex-mcp.sh.j2 @@ -18,4 +18,4 @@ if [[ -n "${PIPELEX_PLUGIN_{{ key | upper }}:-}" ]]; then export PIPELEX_{{ key | upper }}="$PIPELEX_PLUGIN_{{ key | upper }}" fi {% endfor %} -exec {{ mcp_server.command }}{% for arg in mcp_server.args %} "{{ arg }}"{% endfor %} +exec {{ mcp_server.command }}{% for arg in mcp_server.args %} {{ arg | shell_double_quoted }}{% endfor %} diff --git a/tests/unit/test_local_mcp.py b/tests/unit/test_local_mcp.py index ec2a28fe..814894d1 100644 --- a/tests/unit/test_local_mcp.py +++ b/tests/unit/test_local_mcp.py @@ -219,6 +219,23 @@ def test_the_launcher_keeps_the_credential_promotion(self, repo: Path, checkout: assert launcher.rstrip().endswith(f'exec node "{checkout.resolve() / WORKSHOP_BUNDLE}"') assert os.access(copy / "hooks" / "launch-pipelex-mcp.sh", os.X_OK) + @pytest.mark.skipif(shutil.which("bash") is None, reason="no bash on the PATH") + def test_the_launcher_hands_the_workshop_a_path_the_shell_would_act_on_as_it_is(self, repo: Path, tmp_path: Path) -> None: + """A checkout is any directory, so its path reaches the launcher's `exec` line with whatever it holds.""" + odd = tmp_path / 'a $HOME `true` "quoted" back\\slash' / "pipelex-mcp" + (odd / WORKSHOP_BUNDLE).parent.mkdir(parents=True) + (odd / WORKSHOP_BUNDLE).write_text("// the workshop\n", encoding="utf-8") + copy = render_claude_copy(repo, _built(odd)) + + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + (bin_dir / "node").write_text('#!/bin/sh\nprintf "%s" "$1"\n', encoding="utf-8") + (bin_dir / "node").chmod(0o755) + environment = {"PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", "HOME": str(tmp_path)} + completed = subprocess.run([str(copy / "hooks" / "launch-pipelex-mcp.sh")], capture_output=True, text=True, check=False, env=environment) + assert completed.returncode == 0, completed.stderr + assert completed.stdout == str(odd.resolve() / WORKSHOP_BUNDLE) + def test_a_second_render_of_a_workshop_replaces_its_copy_and_leaves_nothing_beside_it(self, repo: Path, checkout: Path) -> None: copy = render_claude_copy(repo, _built(checkout)) (copy / "stray.txt").write_text("left by hand\n", encoding="utf-8") From 89e1fab455e54b4a9147ff7d186875f9f8a844eb Mon Sep 17 00:00:00 2001 From: Louis Choquel Date: Sat, 26 Sep 2026 18:06:45 +0200 Subject: [PATCH 05/10] Say which headless form each harness takes, and that an agent may run one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The development page told readers never to start a target from inside a session, while the repository skill runs a headless prompt from one; and its `ARGS='-p "…"'` selects a configuration profile on Codex, whose headless form is `exec`. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_014AcpM2qUzbWMenP62iSBoS --- .claude/skills/pipelex-mcp-source/SKILL.md | 2 +- docs/development.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.claude/skills/pipelex-mcp-source/SKILL.md b/.claude/skills/pipelex-mcp-source/SKILL.md index 2d1d4f98..cf41ec62 100644 --- a/.claude/skills/pipelex-mcp-source/SKILL.md +++ b/.claude/skills/pipelex-mcp-source/SKILL.md @@ -69,7 +69,7 @@ Name the target and let the user start it from a terminal: it starts an interact - A released version: `make claude-local-mcp MCP_VERSION=X.Y.Z`, or the Codex twin. The target refuses a version npm does not know before anything starts. - `WORKDIR=` starts the session in a project, since the workshop resolves `{ path }` files against it; `ARGS='…'` goes to `claude` or `codex` as it is. -A headless prompt is the one form an agent can run itself, and it is how to dogfood a workshop change from inside a session: `make claude-local-mcp MCP= WORKDIR= ARGS='-p "…" --model sonnet'`. The workshop takes its key from the shell's `PIPELEX_API_KEY` in both targets, and the target warns when it is unset. On Claude Code, the rendered copy loads as `pipelex@inline` in place of an installed `pipelex@pipelex-plugins`, so its skills are this checkout's. On Codex, the skills are the installed copy's: pair the target with `make codex-use-local` to run this checkout's. Mistral Vibe has no target; `docs/development.md` says which entry to edit. +A headless prompt is the one form an agent can run itself, and it is how to dogfood a workshop change from inside a session: `make claude-local-mcp MCP= WORKDIR= ARGS='-p "…" --model sonnet'`, or `make codex-local-mcp MCP= WORKDIR= ARGS='exec "…"'`, since Codex's `-p` names a configuration profile. Each workshop gets its own rendered copy, so such a run never changes the workshop of the session it was started from. The workshop takes its key from the shell's `PIPELEX_API_KEY` in both targets, and the target warns when it is unset. On Claude Code, the rendered copy loads as `pipelex@inline` in place of an installed `pipelex@pipelex-plugins`, so its skills are this checkout's. On Codex, the skills are the installed copy's: pair the target with `make codex-use-local` to run this checkout's. Mistral Vibe has no target; `docs/development.md` says which entry to edit. ## Restoring the shipped default diff --git a/docs/development.md b/docs/development.md index d0a80f2a..79c1a19e 100644 --- a/docs/development.md +++ b/docs/development.md @@ -77,7 +77,7 @@ make codex-local-mcp # Codex, the same way, wi make claude-local-mcp WORKDIR=~/my-methods ARGS='--model sonnet' ``` -A target given `MCP` first builds that checkout's workshop with its own `make build-local`, which writes `packages/workshop/dist/main.js`, and the agent then spawns it by absolute path. `MCP` defaults to `../pipelex-mcp`. A target given `MCP_VERSION` builds nothing: it asks npm which version that names and spawns exactly that one through `npx`, so a version npm does not know is refused before the agent starts. `WORKDIR` is where the session starts, which matters because the workshop resolves a `{ path }` file against it; it defaults to this checkout. `ARGS` goes to `claude` or `codex` as it is, so `ARGS='-p "…"'` runs one headless prompt. Start either target from a terminal, not from inside an agent's session. +A target given `MCP` first builds that checkout's workshop with its own `make build-local`, which writes `packages/workshop/dist/main.js`, and the agent then spawns it by absolute path. `MCP` defaults to `../pipelex-mcp`. A target given `MCP_VERSION` builds nothing: it asks npm which version that names and spawns exactly that one through `npx`, so a version npm does not know is refused before the agent starts. `WORKDIR` is where the session starts, which matters because the workshop resolves a `{ path }` file against it; it defaults to this checkout. `ARGS` goes to `claude` or `codex` as it is. An interactive session takes the place of the make process, so start one from a terminal of its own. A headless prompt runs to its end and returns, so an agent can also run one from its own shell to try a workshop change: `ARGS='-p "…"'` on Claude Code, and `ARGS='exec "…"'` on Codex, where `-p` names a configuration profile instead. - **Claude Code.** The target renders the Claude plugin from this checkout's templates, with the build's own renderer and the launcher pointed at the chosen workshop, into a directory of that workshop's own under `.local-mcp/`, which git ignores. It then starts `claude --plugin-dir` on that copy. A session reads the copy's launcher again whenever it restarts the Pipelex tools, so each workshop has its own copy, and starting another workshop never changes the one a running session uses; starting the same workshop again renders its copy afresh from this checkout's templates. Delete `.local-mcp/` whenever no session runs from it. The copy loads as `pipelex@inline` and takes the place of an installed `pipelex@pipelex-plugins` for that session only, so the skills you run are this checkout's too. Its plugin options arrive empty, since the key saved for the installed plugin is not the copy's, so the workshop takes the key exported in your shell as `PIPELEX_API_KEY`, and the target warns when there is none. - **Codex.** The target renders nothing: it starts `codex` with `-c` overrides of the `mcp_servers.pipelex` entry. An entry given that way replaces the plugin's whole, so the overrides forward `PIPELEX_API_KEY` and `PIPELEX_BASE_URL` by name as the plugin does, and the workshop takes the key from your shell. The skills are whichever copy of the plugin Codex has installed: the published one, or this checkout after `make codex-use-local`. From 3155f93f7b22c6fd3e054b639dbe9230801e984d Mon Sep 17 00:00:00 2001 From: Louis Choquel Date: Sat, 26 Sep 2026 18:19:14 +0200 Subject: [PATCH 06/10] Start the harness without the make target's variables os.execv handed the session MAKEFLAGS, MAKELEVEL and the target's MCP, MCP_VERSION, WORKDIR and ARGS, so every make the agent ran took them as its own command line: a project's ARGS was overridden, and a nested `make codex-local-mcp MCP=` started the outer run's MCP_VERSION. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_014AcpM2qUzbWMenP62iSBoS --- docs/decisions.md | 2 +- scripts/local_mcp.py | 27 ++++++++++++++++++--------- tests/unit/test_local_mcp.py | 25 ++++++++++++++++++++----- 3 files changed, 39 insertions(+), 15 deletions(-) diff --git a/docs/decisions.md b/docs/decisions.md index 4ab5e91e..c276f447 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -703,7 +703,7 @@ Running the skills against an unreleased `pipelex-mcp` workshop took five steps: - **The key comes from the shell.** The copy's plugin options arrive empty, since the key saved for the installed plugin belongs to `pipelex@pipelex-plugins` and not to `pipelex@inline`. A probe in place of the workshop saw `PIPELEX_PLUGIN_API_KEY` and `PIPELEX_PLUGIN_BASE_URL` empty, so `launch-pipelex-mcp.sh` promoted nothing, and `PIPELEX_API_KEY` and `PIPELEX_BASE_URL` set from the shell. The non-empty guard recorded in "Claude credentials move to plugin userConfig" is what lets the shell's key through here. The target warns when `PIPELEX_API_KEY` is unset rather than refusing, since a session may want the tools that need no key. - **Codex's override carries `env_vars`.** Verified on Codex 0.156.1: a `-c mcp_servers.pipelex.*` override, like an entry in `~/.codex/config.toml`, replaces the plugin's entry whole, and `codex mcp list --json` then shows `env_vars: []`. A probe spawned that way saw no `PIPELEX_API_KEY`; with `-c mcp_servers.pipelex.env_vars=[…]` added, it saw both variables, and a live `mthds_list_methods` call answered. So the target sets the names the codex target forwards as well as the command, and the install page now says that an override of one's own needs them too. Codex keeps its installed skills; nothing is rendered for it. - **A published version is pinned at start.** `MCP_VERSION` accepts a version or a dist-tag, which npm resolves to one version before the agent starts, and the copy spawns exactly that version, so the workshop checked is the one run and a typo is refused with npm's own reason instead of failing as an npx error inside the session. A range that matches several versions is refused. -- **The script builds the checkout, and the build sees none of the targets' variables.** `scripts/local_mcp.py` runs the checkout's `make build-local` itself, after it has checked that the checkout exists, so its refusal naming `MCP` is what a wrong path meets, rather than make's own error from a build line in the Makefile that ran first. make hands a target's commands the variables given on its command line, through `MAKEFLAGS` and the environment, where `ARGS` or `MCP` would override or fill the checkout's own Makefile, so the script runs the build with those and make's own variables removed from its environment. The first version ran the build from the Makefile, where GNU Make 3.81, the one macOS ships, needed `unset` and an empty `MAKEFLAGS` on the build line: a target-specific `MAKEOVERRIDES =` did not stop them there. +- **The script builds the checkout, and the build sees none of the targets' variables.** `scripts/local_mcp.py` runs the checkout's `make build-local` itself, after it has checked that the checkout exists, so its refusal naming `MCP` is what a wrong path meets, rather than make's own error from a build line in the Makefile that ran first. make hands a target's commands the variables given on its command line, through `MAKEFLAGS` and the environment, where `ARGS` or `MCP` would override or fill the checkout's own Makefile, so the script runs the build with those and make's own variables removed from its environment. It starts the harness with that environment too: exec'd with the make target's, the session handed `MAKEFLAGS` to every make the agent ran, where a project's own `ARGS` took the target's value and a nested `make codex-local-mcp MCP=` started the `MCP_VERSION` the outer run was given instead. The first version ran the build from the Makefile, where GNU Make 3.81, the one macOS ships, needed `unset` and an empty `MAKEFLAGS` on the build line: a target-specific `MAKEOVERRIDES =` did not stop them there. - **Mistral Vibe is out of scope.** Its `~/.vibe/config.toml` entry is the user's own file, so there is nothing to render and nothing to override from the command line. ## License & distribution diff --git a/scripts/local_mcp.py b/scripts/local_mcp.py index 65151a38..c1fac24a 100644 --- a/scripts/local_mcp.py +++ b/scripts/local_mcp.py @@ -57,7 +57,7 @@ WORKSHOP_PACKAGE = "@pipelex/mcp" # The variables a make target hands its commands beside its own: make's, then this repository's -# local-workshop targets'. None of them reaches the checkout's build. +# local-workshop targets'. None of them reaches the checkout's build or the harness. MAKE_HANDOFF = frozenset({"MAKEFLAGS", "MFLAGS", "MAKEOVERRIDES", "MAKELEVEL", "MCP", "MCP_VERSION", "WORKDIR", "ARGS"}) CLAUDE_TARGET = "prod" @@ -79,19 +79,24 @@ class Launcher: label: str -def build_checkout(root: Path) -> None: - """Build the checkout's workshop with its own `make build-local`, given none of the make target's variables. +def without_make_handoff() -> dict[str, str]: + """This process's environment without what the make target that ran it handed over. make hands the commands of a target the variables on its command line, through `MAKEFLAGS` and - the environment, and a make run from here would read them there: `ARGS` or `MCP` would then - override or fill the checkout's own Makefile. + the environment, and every make started below reads them there as its own command line: `ARGS` + or `MCP` would override or fill the checkout's Makefile, and in the harness's session they + would reach every make the agent runs, a project's own or a nested `make claude-local-mcp`. """ + return {name: value for name, value in os.environ.items() if name not in MAKE_HANDOFF} + + +def build_checkout(root: Path) -> None: + """Build the checkout's workshop with its own `make build-local`, given none of the make target's variables.""" make = shutil.which("make") if make is None: msg = "make is not on the PATH, and a checkout's workshop is built with its `make build-local`." raise SystemExit(msg) - environment = {name: value for name, value in os.environ.items() if name not in MAKE_HANDOFF} - completed = subprocess.run([make, "--no-print-directory", "-C", str(root), "build-local"], env=environment, check=False) + completed = subprocess.run([make, "--no-print-directory", "-C", str(root), "build-local"], env=without_make_handoff(), check=False) if completed.returncode != 0: msg = f"`make build-local` failed in {root}, so no workshop was started." raise SystemExit(msg) @@ -270,7 +275,11 @@ def toml_array(values: list[str]) -> str: def start(harness: Harness, launcher: Launcher, workdir: Path, passthrough: list[str], base_dir: Path) -> NoReturn: - """Prepare `harness` to spawn `launcher` as its Pipelex workshop, say what runs, and replace this process with it.""" + """Prepare `harness` to spawn `launcher` as its Pipelex workshop, say what runs, and replace this process with it. + + The harness gets this process's environment, the shell's key among it, without the make target's + variables, which would otherwise reach every make the agent runs in the session. + """ program = shutil.which(harness.value) if program is None: msg = f"`{harness.value}` is not on the PATH." @@ -294,7 +303,7 @@ def start(harness: Harness, launcher: Launcher, workdir: Path, passthrough: list print("\n".join(lines), flush=True) os.chdir(workdir) - os.execv(program, command) + os.execve(program, command, without_make_handoff()) def split_passthrough(argv: list[str]) -> tuple[list[str], list[str]]: diff --git a/tests/unit/test_local_mcp.py b/tests/unit/test_local_mcp.py index 814894d1..31bc1d01 100644 --- a/tests/unit/test_local_mcp.py +++ b/tests/unit/test_local_mcp.py @@ -308,12 +308,13 @@ def test_a_codex_target_forwarding_nothing_is_refused(self, repo: Path) -> None: class Launched(Exception): - """What the patched `os.execv` raises in place of replacing the test process.""" + """What the patched `os.execve` raises in place of replacing the test process.""" - def __init__(self, program: str, argv: list[str]) -> None: + def __init__(self, program: str, argv: list[str], environment: dict[str, str]) -> None: super().__init__(program) self.program = program self.argv = argv + self.environment = environment class TestStart: @@ -326,10 +327,10 @@ def which(name: str) -> str: mocker.patch.object(local_mcp.shutil, "which", side_effect=which) - def execv(program: str, argv: list[str]) -> None: - raise Launched(program, argv) + def execve(program: str, argv: list[str], environment: dict[str, str]) -> None: + raise Launched(program, argv, environment) - mocker.patch.object(local_mcp.os, "execv", side_effect=execv) + mocker.patch.object(local_mcp.os, "execve", side_effect=execve) chdirs: list[Path] = [] mocker.patch.object(local_mcp.os, "chdir", side_effect=chdirs.append) return chdirs @@ -368,6 +369,20 @@ def test_a_missing_key_is_warned_of( start(Harness.CODEX, _published("latest"), tmp_path, [], repo) assert ("PIPELEX_API_KEY is not set" in capsys.readouterr().out) is warned + @pytest.mark.parametrize("harness", list(Harness)) + def test_the_harness_gets_the_shell_environment_without_the_make_targets_variables( + self, repo: Path, tmp_path: Path, harness_calls: list[Path], monkeypatch: pytest.MonkeyPatch, harness: Harness + ) -> None: + """Left in, `MAKEFLAGS` and the target's variables would override the variables of every make the agent runs.""" + for name in MAKE_HANDOFF: + monkeypatch.setenv(name, "from the make target") + monkeypatch.setenv("PIPELEX_API_KEY", "plx_sk_test") + with pytest.raises(Launched) as launched: + start(harness, _published("latest"), tmp_path, [], repo) + assert not MAKE_HANDOFF & launched.value.environment.keys() + assert launched.value.environment["PIPELEX_API_KEY"] == "plx_sk_test" + assert launched.value.environment["PATH"] == os.environ["PATH"] + def test_a_harness_not_on_the_path_stops_before_anything_is_rendered(self, repo: Path, tmp_path: Path, mocker: MockerFixture) -> None: mocker.patch.object(local_mcp.shutil, "which", return_value=None) with pytest.raises(SystemExit, match="`claude` is not on the PATH"): From a9db5f800e5c4bb6ed857f9ba121fcb5b331f4bc Mon Sep 17 00:00:00 2001 From: Louis Choquel Date: Sat, 26 Sep 2026 18:20:58 +0200 Subject: [PATCH 07/10] Hand MCP and WORKDIR to the script as they were given The recipe spliced both into double quotes, so make expanded a `$` in the path, the shell ran a backtick, and a `"` broke the recipe, before the launcher's own escaping could matter. Each now goes as one single-quoted word of the variable's unexpanded value. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_014AcpM2qUzbWMenP62iSBoS --- Makefile | 10 +++++++--- tests/unit/test_local_mcp.py | 22 ++++++++++++---------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/Makefile b/Makefile index fa4342db..32a2796a 100644 --- a/Makefile +++ b/Makefile @@ -241,10 +241,14 @@ MCP_VERSION ?= WORKDIR ?= . ARGS ?= -LOCAL_MCP_SOURCE = $(if $(MCP_VERSION),--mcp-version "$(MCP_VERSION)",--mcp "$(MCP)") +# A path reaches the script as it was given, as one single-quoted shell word of the variable's +# unexpanded value: in double quotes, make would expand a `$` in it and the shell a backtick. +local_mcp_path = '$(subst ','\'',$(value $(1)))' + +LOCAL_MCP_SOURCE = $(if $(MCP_VERSION),--mcp-version "$(MCP_VERSION)",--mcp $(call local_mcp_path,MCP)) claude-local-mcp: install ## Start Claude Code on this checkout's skills with another workshop (MCP=path or MCP_VERSION=x.y.z) - @$(VENV_PYTHON) scripts/local_mcp.py claude $(LOCAL_MCP_SOURCE) --workdir "$(WORKDIR)" -- $(ARGS) + @$(VENV_PYTHON) scripts/local_mcp.py claude $(LOCAL_MCP_SOURCE) --workdir $(call local_mcp_path,WORKDIR) -- $(ARGS) codex-local-mcp: install ## Start Codex with another workshop in place of the plugin's (MCP=path or MCP_VERSION=x.y.z) - @$(VENV_PYTHON) scripts/local_mcp.py codex $(LOCAL_MCP_SOURCE) --workdir "$(WORKDIR)" -- $(ARGS) + @$(VENV_PYTHON) scripts/local_mcp.py codex $(LOCAL_MCP_SOURCE) --workdir $(call local_mcp_path,WORKDIR) -- $(ARGS) diff --git a/tests/unit/test_local_mcp.py b/tests/unit/test_local_mcp.py index 31bc1d01..77242aab 100644 --- a/tests/unit/test_local_mcp.py +++ b/tests/unit/test_local_mcp.py @@ -420,24 +420,26 @@ def _make(self, *arguments: str, home: Path | None = None) -> subprocess.Complet ) @pytest.mark.skipif(not (REPO_ROOT / ".venv" / "bin" / "python").is_file(), reason="no venv: run `make install`") - @pytest.mark.parametrize("home_relative", [False, True]) - def test_the_checkout_is_built_with_none_of_the_targets_variables(self, tmp_path: Path, home_relative: bool) -> None: + @pytest.mark.parametrize("spelling", ["absolute", "home-relative", "shell-special"]) + def test_the_checkout_is_built_with_none_of_the_targets_variables(self, tmp_path: Path, spelling: str) -> None: """make hands a target's commands its command-line variables, which would override or fill the checkout's own. This one runs for real: the checkout's build only reports what it sees, and the script then - refuses the checkout for having built no workshop, so nothing starts. zsh passes `MCP=~/…` with - its tilde as it is, so that spelling is run too. + refuses the checkout for having built no workshop, naming it, so nothing starts. zsh passes + `MCP=~/…` with its tilde as it is, and a path may hold what make or a shell would act on, so + both spellings are run too. """ - checkout = tmp_path / "pipelex-mcp" - checkout.mkdir() + parent = tmp_path / "a $b \"q\" 's' `true` back\\slash" if spelling == "shell-special" else tmp_path + checkout = parent / "pipelex-mcp" + checkout.mkdir(parents=True) (checkout / "Makefile").write_text( 'ARGS = its-own\nbuild-local:\n\t@echo "built with ARGS=[$(ARGS)] MCP=[$(MCP)] MCP_VERSION=[$(MCP_VERSION)] WORKDIR=[$(WORKDIR)]"\n', encoding="utf-8", ) - mcp = "~/pipelex-mcp" if home_relative else str(checkout) - completed = self._make("claude-local-mcp", f"MCP={mcp}", "ARGS=--model sonnet", f"WORKDIR={tmp_path}", home=tmp_path) + mcp = "~/pipelex-mcp" if spelling == "home-relative" else str(checkout) + completed = self._make("claude-local-mcp", f"MCP={mcp}", "ARGS=--model sonnet", f"WORKDIR={parent}", home=tmp_path) assert "built with ARGS=[its-own] MCP=[] MCP_VERSION=[] WORKDIR=[]" in completed.stdout - assert "does not exist after `make build-local`" in completed.stderr + assert f"does not exist after `make build-local` in {checkout.resolve()}:" in completed.stderr assert completed.returncode != 0 @pytest.mark.skipif(not (REPO_ROOT / ".venv" / "bin" / "python").is_file(), reason="no venv: run `make install`") @@ -449,4 +451,4 @@ def test_a_missing_checkout_is_refused_with_the_variable_to_set(self, tmp_path: def test_a_published_version_reaches_the_script_as_a_version(self) -> None: completed = self._make("-n", "claude-local-mcp", "MCP_VERSION=0.20.0", "MCP=/no/checkout/here", "ARGS=--model sonnet") assert completed.returncode == 0, completed.stderr - assert 'scripts/local_mcp.py claude --mcp-version "0.20.0" --workdir "." -- --model sonnet' in completed.stdout + assert "scripts/local_mcp.py claude --mcp-version \"0.20.0\" --workdir '.' -- --model sonnet" in completed.stdout From 9fde7178020f570c01e9be2b892083b5a73209af Mon Sep 17 00:00:00 2001 From: Louis Choquel Date: Sat, 26 Sep 2026 18:35:11 +0200 Subject: [PATCH 08/10] Read the targets' variables from make's command line alone MCP, MCP_VERSION, WORKDIR and ARGS are generic names. Read with `?=`, a WORKDIR or ARGS the shell exported for another tool became the session's directory or the harness's arguments, and the environment filter then removed it from the session. The Makefile now takes each from the command line, unexpanded, and never assigns it; the script removes one only when MAKEFLAGS names it. ARGS is no longer expanded by make, so a `$` in a prompt survives. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_014AcpM2qUzbWMenP62iSBoS --- Makefile | 22 ++++++------- docs/decisions.md | 2 +- docs/development.md | 2 +- scripts/local_mcp.py | 24 +++++++++++--- tests/unit/test_local_mcp.py | 64 ++++++++++++++++++++++++++++++------ 5 files changed, 87 insertions(+), 27 deletions(-) diff --git a/Makefile b/Makefile index 32a2796a..18e9ab44 100644 --- a/Makefile +++ b/Makefile @@ -236,19 +236,19 @@ codex-status: ## Show which source is currently registered for the Codex pipelex # starts, which is where the workshop resolves a { path } file. ARGS goes to claude or # codex as it is. Neither target writes a tracked file: see "A local build of # pipelex-mcp" in docs/development.md. -MCP ?= ../pipelex-mcp -MCP_VERSION ?= -WORKDIR ?= . -ARGS ?= +# +# Each is read from make's command line alone, and never assigned here: the names are +# generic, so a shell variable called WORKDIR or ARGS belongs to some other tool, and it +# reaches the session as it was. Each is taken unexpanded, so make acts on no `$` in it, +# and a path goes as one single-quoted shell word, which the shell does not act on either. +local_mcp_given = $(if $(filter command line,$(origin $(1))),$(value $(1)),$(2)) +local_mcp_word = '$(subst ','\'',$(call local_mcp_given,$(1),$(2)))' -# A path reaches the script as it was given, as one single-quoted shell word of the variable's -# unexpanded value: in double quotes, make would expand a `$` in it and the shell a backtick. -local_mcp_path = '$(subst ','\'',$(value $(1)))' - -LOCAL_MCP_SOURCE = $(if $(MCP_VERSION),--mcp-version "$(MCP_VERSION)",--mcp $(call local_mcp_path,MCP)) +LOCAL_MCP_SOURCE = $(if $(call local_mcp_given,MCP_VERSION,),--mcp-version $(call local_mcp_word,MCP_VERSION,),--mcp $(call local_mcp_word,MCP,../pipelex-mcp)) +LOCAL_MCP_OPTIONS = $(LOCAL_MCP_SOURCE) --workdir $(call local_mcp_word,WORKDIR,.) -- $(call local_mcp_given,ARGS,) claude-local-mcp: install ## Start Claude Code on this checkout's skills with another workshop (MCP=path or MCP_VERSION=x.y.z) - @$(VENV_PYTHON) scripts/local_mcp.py claude $(LOCAL_MCP_SOURCE) --workdir $(call local_mcp_path,WORKDIR) -- $(ARGS) + @$(VENV_PYTHON) scripts/local_mcp.py claude $(LOCAL_MCP_OPTIONS) codex-local-mcp: install ## Start Codex with another workshop in place of the plugin's (MCP=path or MCP_VERSION=x.y.z) - @$(VENV_PYTHON) scripts/local_mcp.py codex $(LOCAL_MCP_SOURCE) --workdir $(call local_mcp_path,WORKDIR) -- $(ARGS) + @$(VENV_PYTHON) scripts/local_mcp.py codex $(LOCAL_MCP_OPTIONS) diff --git a/docs/decisions.md b/docs/decisions.md index c276f447..15ec7e1f 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -703,7 +703,7 @@ Running the skills against an unreleased `pipelex-mcp` workshop took five steps: - **The key comes from the shell.** The copy's plugin options arrive empty, since the key saved for the installed plugin belongs to `pipelex@pipelex-plugins` and not to `pipelex@inline`. A probe in place of the workshop saw `PIPELEX_PLUGIN_API_KEY` and `PIPELEX_PLUGIN_BASE_URL` empty, so `launch-pipelex-mcp.sh` promoted nothing, and `PIPELEX_API_KEY` and `PIPELEX_BASE_URL` set from the shell. The non-empty guard recorded in "Claude credentials move to plugin userConfig" is what lets the shell's key through here. The target warns when `PIPELEX_API_KEY` is unset rather than refusing, since a session may want the tools that need no key. - **Codex's override carries `env_vars`.** Verified on Codex 0.156.1: a `-c mcp_servers.pipelex.*` override, like an entry in `~/.codex/config.toml`, replaces the plugin's entry whole, and `codex mcp list --json` then shows `env_vars: []`. A probe spawned that way saw no `PIPELEX_API_KEY`; with `-c mcp_servers.pipelex.env_vars=[…]` added, it saw both variables, and a live `mthds_list_methods` call answered. So the target sets the names the codex target forwards as well as the command, and the install page now says that an override of one's own needs them too. Codex keeps its installed skills; nothing is rendered for it. - **A published version is pinned at start.** `MCP_VERSION` accepts a version or a dist-tag, which npm resolves to one version before the agent starts, and the copy spawns exactly that version, so the workshop checked is the one run and a typo is refused with npm's own reason instead of failing as an npx error inside the session. A range that matches several versions is refused. -- **The script builds the checkout, and the build sees none of the targets' variables.** `scripts/local_mcp.py` runs the checkout's `make build-local` itself, after it has checked that the checkout exists, so its refusal naming `MCP` is what a wrong path meets, rather than make's own error from a build line in the Makefile that ran first. make hands a target's commands the variables given on its command line, through `MAKEFLAGS` and the environment, where `ARGS` or `MCP` would override or fill the checkout's own Makefile, so the script runs the build with those and make's own variables removed from its environment. It starts the harness with that environment too: exec'd with the make target's, the session handed `MAKEFLAGS` to every make the agent ran, where a project's own `ARGS` took the target's value and a nested `make codex-local-mcp MCP=` started the `MCP_VERSION` the outer run was given instead. The first version ran the build from the Makefile, where GNU Make 3.81, the one macOS ships, needed `unset` and an empty `MAKEFLAGS` on the build line: a target-specific `MAKEOVERRIDES =` did not stop them there. +- **The script builds the checkout, and the build sees none of the targets' variables.** `scripts/local_mcp.py` runs the checkout's `make build-local` itself, after it has checked that the checkout exists, so its refusal naming `MCP` is what a wrong path meets, rather than make's own error from a build line in the Makefile that ran first. make hands a target's commands the variables given on its command line, through `MAKEFLAGS` and the environment, where `ARGS` or `MCP` would override or fill the checkout's own Makefile, so the script runs the build with those and make's own variables removed from its environment. The Makefile reads the four from make's command line alone and never assigns them, since the names are generic, and the script removes one only when `MAKEFLAGS` names it as a command-line assignment: read with `?=` and removed by name, a `WORKDIR` or `ARGS` the user exported for another tool became the session's directory or the harness's arguments and then vanished from its environment. It starts the harness with that environment too: exec'd with the make target's, the session handed `MAKEFLAGS` to every make the agent ran, where a project's own `ARGS` took the target's value and a nested `make codex-local-mcp MCP=` started the `MCP_VERSION` the outer run was given instead. The first version ran the build from the Makefile, where GNU Make 3.81, the one macOS ships, needed `unset` and an empty `MAKEFLAGS` on the build line: a target-specific `MAKEOVERRIDES =` did not stop them there. - **Mistral Vibe is out of scope.** Its `~/.vibe/config.toml` entry is the user's own file, so there is nothing to render and nothing to override from the command line. ## License & distribution diff --git a/docs/development.md b/docs/development.md index 79c1a19e..524c1c30 100644 --- a/docs/development.md +++ b/docs/development.md @@ -77,7 +77,7 @@ make codex-local-mcp # Codex, the same way, wi make claude-local-mcp WORKDIR=~/my-methods ARGS='--model sonnet' ``` -A target given `MCP` first builds that checkout's workshop with its own `make build-local`, which writes `packages/workshop/dist/main.js`, and the agent then spawns it by absolute path. `MCP` defaults to `../pipelex-mcp`. A target given `MCP_VERSION` builds nothing: it asks npm which version that names and spawns exactly that one through `npx`, so a version npm does not know is refused before the agent starts. `WORKDIR` is where the session starts, which matters because the workshop resolves a `{ path }` file against it; it defaults to this checkout. `ARGS` goes to `claude` or `codex` as it is. An interactive session takes the place of the make process, so start one from a terminal of its own. A headless prompt runs to its end and returns, so an agent can also run one from its own shell to try a workshop change: `ARGS='-p "…"'` on Claude Code, and `ARGS='exec "…"'` on Codex, where `-p` names a configuration profile instead. +A target given `MCP` first builds that checkout's workshop with its own `make build-local`, which writes `packages/workshop/dist/main.js`, and the agent then spawns it by absolute path. `MCP` defaults to `../pipelex-mcp`. The targets read `MCP`, `MCP_VERSION`, `WORKDIR` and `ARGS` from make's command line alone, so a variable of one of those names that your shell exports for another tool is neither taken by a target nor removed from the session it starts. A target given `MCP_VERSION` builds nothing: it asks npm which version that names and spawns exactly that one through `npx`, so a version npm does not know is refused before the agent starts. `WORKDIR` is where the session starts, which matters because the workshop resolves a `{ path }` file against it; it defaults to this checkout. `ARGS` goes to `claude` or `codex` as it is. An interactive session takes the place of the make process, so start one from a terminal of its own. A headless prompt runs to its end and returns, so an agent can also run one from its own shell to try a workshop change: `ARGS='-p "…"'` on Claude Code, and `ARGS='exec "…"'` on Codex, where `-p` names a configuration profile instead. - **Claude Code.** The target renders the Claude plugin from this checkout's templates, with the build's own renderer and the launcher pointed at the chosen workshop, into a directory of that workshop's own under `.local-mcp/`, which git ignores. It then starts `claude --plugin-dir` on that copy. A session reads the copy's launcher again whenever it restarts the Pipelex tools, so each workshop has its own copy, and starting another workshop never changes the one a running session uses; starting the same workshop again renders its copy afresh from this checkout's templates. Delete `.local-mcp/` whenever no session runs from it. The copy loads as `pipelex@inline` and takes the place of an installed `pipelex@pipelex-plugins` for that session only, so the skills you run are this checkout's too. Its plugin options arrive empty, since the key saved for the installed plugin is not the copy's, so the workshop takes the key exported in your shell as `PIPELEX_API_KEY`, and the target warns when there is none. - **Codex.** The target renders nothing: it starts `codex` with `-c` overrides of the `mcp_servers.pipelex` entry. An entry given that way replaces the plugin's whole, so the overrides forward `PIPELEX_API_KEY` and `PIPELEX_BASE_URL` by name as the plugin does, and the workshop takes the key from your shell. The skills are whichever copy of the plugin Codex has installed: the published one, or this checkout after `make codex-use-local`. diff --git a/scripts/local_mcp.py b/scripts/local_mcp.py index c1fac24a..abddde64 100644 --- a/scripts/local_mcp.py +++ b/scripts/local_mcp.py @@ -28,6 +28,7 @@ import hashlib import json import os +import re import shutil import subprocess import sys @@ -56,9 +57,17 @@ WORKSHOP_MANIFEST = Path("packages/workshop/package.json") WORKSHOP_PACKAGE = "@pipelex/mcp" -# The variables a make target hands its commands beside its own: make's, then this repository's -# local-workshop targets'. None of them reaches the checkout's build or the harness. -MAKE_HANDOFF = frozenset({"MAKEFLAGS", "MFLAGS", "MAKEOVERRIDES", "MAKELEVEL", "MCP", "MCP_VERSION", "WORKDIR", "ARGS"}) +# make's own variables, which carry the command line of the make that ran this script to every +# make started below it. None of them reaches the checkout's build or the harness. +MAKE_STATE = frozenset({"MAKEFLAGS", "MFLAGS", "MAKEOVERRIDES", "MAKELEVEL"}) + +# The local-workshop targets' variables. Given on make's command line, one reaches this script's +# environment too, and is kept from the build and the harness with make's own; exported in the +# user's shell, it belongs to some other tool, which the Makefile never reads, and stays. +TARGET_VARIABLES = frozenset({"MCP", "MCP_VERSION", "WORKDIR", "ARGS"}) + +# A word of `MAKEFLAGS` that assigns a variable: `NAME=value`, a space inside a value escaped with a backslash. +MAKEFLAGS_ASSIGNMENT = re.compile(r"(?:^|(? set[str]: + """The names of the variables `makeflags` says were assigned on make's command line.""" + return {match.group(1) for match in MAKEFLAGS_ASSIGNMENT.finditer(makeflags)} + + def without_make_handoff() -> dict[str, str]: """This process's environment without what the make target that ran it handed over. @@ -86,8 +100,10 @@ def without_make_handoff() -> dict[str, str]: the environment, and every make started below reads them there as its own command line: `ARGS` or `MCP` would override or fill the checkout's Makefile, and in the harness's session they would reach every make the agent runs, a project's own or a nested `make claude-local-mcp`. + A target variable `MAKEFLAGS` does not name came from the user's shell, and is kept. """ - return {name: value for name, value in os.environ.items() if name not in MAKE_HANDOFF} + handed = MAKE_STATE | (TARGET_VARIABLES & make_command_line(os.environ.get("MAKEFLAGS", ""))) + return {name: value for name, value in os.environ.items() if name not in handed} def build_checkout(root: Path) -> None: diff --git a/tests/unit/test_local_mcp.py b/tests/unit/test_local_mcp.py index 77242aab..26bacaf6 100644 --- a/tests/unit/test_local_mcp.py +++ b/tests/unit/test_local_mcp.py @@ -18,7 +18,8 @@ from scripts.gen_skill_docs import build_target, load_target_config, write_files from scripts.local_mcp import ( LOCAL_DIR_NAME, - MAKE_HANDOFF, + MAKE_STATE, + TARGET_VARIABLES, WORKSHOP_BUNDLE, Harness, Launcher, @@ -27,6 +28,7 @@ codex_env_vars, codex_overrides, main, + make_command_line, npm_resolve, parse_args, published_launcher, @@ -63,6 +65,16 @@ def checkout(tmp_path: Path) -> Path: return root +def _handed_by_make(monkeypatch: pytest.MonkeyPatch) -> None: + """The environment a make target's command gets when every target variable was given on make's command line.""" + assignments = {name: f"from the make target {name}" for name in TARGET_VARIABLES} + for name, value in assignments.items(): + monkeypatch.setenv(name, value) + makeflags = " ".join(f"{name}={value.replace(' ', chr(92) + ' ')}" for name, value in assignments.items()) + for name in MAKE_STATE: + monkeypatch.setenv(name, makeflags if name == "MAKEFLAGS" else "1") + + def _built(checkout: Path) -> Launcher: """The checkout's launcher, its `make build-local` taken as run: the fixture's bundle is already there.""" return checkout_launcher(checkout, build=lambda _root: None) @@ -137,16 +149,37 @@ def test_the_build_is_given_none_of_the_make_targets_variables( self, checkout: Path, mocker: MockerFixture, monkeypatch: pytest.MonkeyPatch ) -> None: """make hands a target's commands its command-line variables, which would override or fill the checkout's Makefile.""" - for name in MAKE_HANDOFF: - monkeypatch.setenv(name, "from the make target") + _handed_by_make(monkeypatch) mocker.patch.object(local_mcp.shutil, "which", return_value="/usr/bin/make") run = mocker.patch.object(local_mcp.subprocess, "run", return_value=subprocess.CompletedProcess(args=[], returncode=0)) build_checkout(checkout) assert run.call_args.args[0] == ["/usr/bin/make", "--no-print-directory", "-C", str(checkout), "build-local"] environment = run.call_args.kwargs["env"] - assert not MAKE_HANDOFF & environment.keys() + assert not (MAKE_STATE | TARGET_VARIABLES) & environment.keys() assert environment["PATH"] == os.environ["PATH"] + def test_a_target_variable_from_the_users_shell_is_kept(self, monkeypatch: pytest.MonkeyPatch) -> None: + """A shell's `WORKDIR` or `ARGS` belongs to some other tool: the Makefile never reads it, and it stays.""" + monkeypatch.setenv("MAKEFLAGS", "WORKDIR=/tmp/a\\ b ARGS=-p\\ MCP=inside") + for name in ("WORKDIR", "ARGS", "MCP", "MCP_VERSION"): + monkeypatch.setenv(name, f"{name} value") + environment = local_mcp.without_make_handoff() + assert {"MCP", "MCP_VERSION"} <= environment.keys() + assert not {"WORKDIR", "ARGS", "MAKEFLAGS"} & environment.keys() + + @pytest.mark.parametrize( + ("makeflags", "names"), + [ + ("", set[str]()), + ("WORKDIR=/tmp ARGS=--from\\ cli", {"WORKDIR", "ARGS"}), + ("sk -- ARGS=a\\ b", {"ARGS"}), + ("ARGS=-p\\ WORKDIR=x", {"ARGS"}), + (" --jobserver-fds=3,4 -j MCP=/x", {"MCP"}), + ], + ) + def test_makeflags_names_the_variables_given_on_makes_command_line(self, makeflags: str, names: set[str]) -> None: + assert make_command_line(makeflags) == names + def test_a_failed_build_starts_nothing(self, checkout: Path, mocker: MockerFixture) -> None: mocker.patch.object(local_mcp.shutil, "which", return_value="/usr/bin/make") mocker.patch.object(local_mcp.subprocess, "run", return_value=subprocess.CompletedProcess(args=[], returncode=2)) @@ -374,12 +407,11 @@ def test_the_harness_gets_the_shell_environment_without_the_make_targets_variabl self, repo: Path, tmp_path: Path, harness_calls: list[Path], monkeypatch: pytest.MonkeyPatch, harness: Harness ) -> None: """Left in, `MAKEFLAGS` and the target's variables would override the variables of every make the agent runs.""" - for name in MAKE_HANDOFF: - monkeypatch.setenv(name, "from the make target") + _handed_by_make(monkeypatch) monkeypatch.setenv("PIPELEX_API_KEY", "plx_sk_test") with pytest.raises(Launched) as launched: start(harness, _published("latest"), tmp_path, [], repo) - assert not MAKE_HANDOFF & launched.value.environment.keys() + assert not (MAKE_STATE | TARGET_VARIABLES) & launched.value.environment.keys() assert launched.value.environment["PIPELEX_API_KEY"] == "plx_sk_test" assert launched.value.environment["PATH"] == os.environ["PATH"] @@ -408,9 +440,9 @@ def test_a_workdir_that_is_not_a_directory_is_refused(self, checkout: Path, tmp_ @pytest.mark.skipif(shutil.which("make") is None, reason="no make on the PATH") class TestMakeTargets: - def _make(self, *arguments: str, home: Path | None = None) -> subprocess.CompletedProcess[str]: + def _make(self, *arguments: str, home: Path | None = None, environment: dict[str, str] | None = None) -> subprocess.CompletedProcess[str]: """This repository's make, told its venv is installed: `install` would update uv over the network.""" - environment = {**os.environ, "HOME": str(home)} if home is not None else None + environment = {**os.environ, **(environment or {}), **({"HOME": str(home)} if home is not None else {})} return subprocess.run( ["make", "--no-print-directory", "-C", str(REPO_ROOT), "-o", "install", *arguments], capture_output=True, @@ -451,4 +483,16 @@ def test_a_missing_checkout_is_refused_with_the_variable_to_set(self, tmp_path: def test_a_published_version_reaches_the_script_as_a_version(self) -> None: completed = self._make("-n", "claude-local-mcp", "MCP_VERSION=0.20.0", "MCP=/no/checkout/here", "ARGS=--model sonnet") assert completed.returncode == 0, completed.stderr - assert "scripts/local_mcp.py claude --mcp-version \"0.20.0\" --workdir '.' -- --model sonnet" in completed.stdout + assert "scripts/local_mcp.py claude --mcp-version '0.20.0' --workdir '.' -- --model sonnet" in completed.stdout + + def test_the_targets_read_their_variables_from_makes_command_line_alone(self) -> None: + """The names are generic, so a shell's `WORKDIR` or `ARGS` belongs to some other tool.""" + exported = {"MCP": "/from/the/shell", "MCP_VERSION": "9.9.9", "WORKDIR": "/elsewhere", "ARGS": "--from-shell"} + completed = self._make("-n", "codex-local-mcp", environment=exported) + assert completed.returncode == 0, completed.stderr + assert "scripts/local_mcp.py codex --mcp '../pipelex-mcp' --workdir '.' -- \n" in completed.stdout + + def test_args_reaches_the_harness_unexpanded_by_make(self) -> None: + completed = self._make("-n", "claude-local-mcp", "MCP_VERSION=0.20.0", "ARGS=-p 'it costs $5' --model sonnet") + assert completed.returncode == 0, completed.stderr + assert "-- -p 'it costs $5' --model sonnet" in completed.stdout From 456ef6b78ea95d7552b99de86af8df2d51cb8a77 Mon Sep 17 00:00:00 2001 From: Louis Choquel Date: Sat, 26 Sep 2026 18:35:35 +0200 Subject: [PATCH 09/10] Record the local-workshop findings round 3 deferred Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_014AcpM2qUzbWMenP62iSBoS --- ...make-target-local-workshop-review-notes.md | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 wip/make-target-local-workshop-review-notes.md diff --git a/wip/make-target-local-workshop-review-notes.md b/wip/make-target-local-workshop-review-notes.md new file mode 100644 index 00000000..7ea5bdf6 --- /dev/null +++ b/wip/make-target-local-workshop-review-notes.md @@ -0,0 +1,28 @@ +# Local-workshop make targets: deferred review findings + +Findings the `/rev` passes on `feature/Make-target-local-workshop` (item L-260926-2b15c9) deferred rather than fixed. They were raised in round 3, whose bar (`necessity`) fixes only a defect the previous pass introduced or one too severe to ship with, and none of them is either. **None of them was verified**: each rests on its reviewer's word, read against the code but not reproduced. + +## The local copy's credential guidance points at the plugin configuration + +- **Reporter:** Codex (P2), `scripts/local_mcp.py`, `render_claude_copy`. +- **Claim:** the copy `make claude-local-mcp` renders keeps the shipped `credentials.md`, which tells the agent to answer a `config`-class authentication error by setting the key in the plugin's configuration and never by telling the user to export a shell variable. In this workflow the copy's plugin options are empty and the workshop takes its key from the shell's `PIPELEX_API_KEY`, so the recovery instruction is the wrong one. +- **Why deferred:** it predates the review passes, and the target already prints a warning naming `PIPELEX_API_KEY` when the shell has none. Whether `pipelex@inline` exposes a configuration dialog at all is unknown, which decides the right wording. +- **When picked up:** render the copy with a variable that switches the Claude branch of `templates/skills/shared/credentials.md.j2` to the shell channel, and extend `TestClaudeCopy`'s file-for-file comparison to allow that difference. + +## The Makefile's `VIRTUAL_ENV` reaches the session + +- **Reporter:** cubic (P2), `scripts/local_mcp.py`, `MAKE_STATE`. +- **Claim:** the Makefile sets `VIRTUAL_ENV := $(CURDIR)/.venv`. When the shell has a virtual environment active, make hands its commands that value, and the harness inherits it, so a tool in the user's project that reads `VIRTUAL_ENV` acts on this repository's `.venv` while `PATH` still names the user's. +- **Why deferred:** it needs an active virtual environment in the shell that starts the target, and it predates the environment filter. The filter could restore the shell's value only if the Makefile passed it along, since make has replaced it by the time the script runs. + +## For an instant during the swap there is no copy + +- **Reporter:** cubic (P3), `scripts/local_mcp.py`, `render_claude_copy`. +- **Claim:** between `final.rename(retired)` and `staging.rename(final)` the copy's path does not exist, so a session of the same workshop respawning its server or running its hook at that instant fails. +- **Why deferred:** the window is two renames long and needs a second start of the same workshop at that moment. An atomic flip, a symlink replaced with `os.replace`, would close it. + +## A relative harness path is executed after the `chdir` + +- **Reporter:** cubic (P3), `scripts/local_mcp.py`, `start`. +- **Claim:** `shutil.which` returns a relative path when the matching `PATH` entry is relative, such as `.` or `node_modules/.bin`, and `start` then changes to `WORKDIR` before `os.execve`, so the path resolves against the wrong directory. +- **Why deferred:** it needs a relative `PATH` entry ahead of the harness's real location. Making `program` absolute before the `chdir` fixes it. From 1914eecca7a4e80a5df8deda1c7210e5403bd2f2 Mon Sep 17 00:00:00 2001 From: Louis Choquel Date: Sat, 26 Sep 2026 18:45:08 +0200 Subject: [PATCH 10/10] Record the local-workshop finding round 4 deferred Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_014AcpM2qUzbWMenP62iSBoS --- wip/make-target-local-workshop-review-notes.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/wip/make-target-local-workshop-review-notes.md b/wip/make-target-local-workshop-review-notes.md index 7ea5bdf6..9230e25e 100644 --- a/wip/make-target-local-workshop-review-notes.md +++ b/wip/make-target-local-workshop-review-notes.md @@ -1,6 +1,6 @@ # Local-workshop make targets: deferred review findings -Findings the `/rev` passes on `feature/Make-target-local-workshop` (item L-260926-2b15c9) deferred rather than fixed. They were raised in round 3, whose bar (`necessity`) fixes only a defect the previous pass introduced or one too severe to ship with, and none of them is either. **None of them was verified**: each rests on its reviewer's word, read against the code but not reproduced. +Findings the `/rev` passes on `feature/Make-target-local-workshop` (item L-260926-2b15c9) deferred rather than fixed. The first four were raised in round 3, whose bar (`necessity`) fixes only a defect the previous pass introduced or one too severe to ship with, and none of them is either; Codex raised all four again in round 4. The last was raised in round 4, whose bar (`freeze`) fixes only a critical. **None of them was verified**: each rests on its reviewer's word, read against the code but not reproduced. ## The local copy's credential guidance points at the plugin configuration @@ -26,3 +26,10 @@ Findings the `/rev` passes on `feature/Make-target-local-workshop` (item L-26092 - **Reporter:** cubic (P3), `scripts/local_mcp.py`, `start`. - **Claim:** `shutil.which` returns a relative path when the matching `PATH` entry is relative, such as `.` or `node_modules/.bin`, and `start` then changes to `WORKDIR` before `os.execve`, so the path resolves against the wrong directory. - **Why deferred:** it needs a relative `PATH` entry ahead of the harness's real location. Making `program` absolute before the `chdir` fixes it. + +## The headless Codex recipe needs a git repository + +- **Reporter:** cubic (P3), `.claude/skills/pipelex-mcp-source/SKILL.md` and `docs/development.md`, the headless form. +- **Claim:** both recommend `make codex-local-mcp MCP= WORKDIR= ARGS='exec "…"'`, and `codex exec` refuses to run outside a git repository or a directory Codex already trusts unless it is given `--skip-git-repo-check`, so the recipe stops before the workshop starts in a fresh scratch directory. +- **Why deferred:** it is documentation, and a scratch project that is a git repository, or one Codex already trusts, runs as written. +- **When picked up:** add `--skip-git-repo-check` to the `exec` example in both places, or say the scratch project must be a git repository.