AOI-2: MCP env-merge at instance scope #731

Closed
opened 2026-05-02 11:01:46 +00:00 by claude-desktop · 1 comment
Collaborator

As an operator running multiple instances of an agent type that need different MCP credentials (e.g. per-instance Penpot tokens or per-instance secrets paths), I want instance-scope mcp_server rows to merge their env map into the inherited row, so that I can tweak per-instance secrets without redefining command / args / transport / url — while forge MCP auth stays bound to type identity by construction.

Acceptance criteria

Resolver

  • resolveMcpServers (apps/server/src/domain/agent-config/resolver.ts) detects when an instance-scope row's name matches an inherited row from agent_type / global / builtin and produces a merged record:
    • env: instance keys override parent keys per-key; absent keys inherit
    • command / args / transport / url: inherited from parent; instance row's values for these MUST be null OR identical to parent (validation rejects divergence)
  • Net-new instance-scope MCP (no inherited row by name) keeps current full-row behavior.
  • AOI-1's enabled=false invariant still holds at instance scope.

Forge MCP identity guardrail (option C)

  • Forge MCP (name = "forge", formerly "forgejo") is special-cased: the resolver synthesizes its auth env from the agent's type-level identity (ResolvedAgent.token_file / forgejo_user) at render time, ignoring any env override at any scope for the auth keys (FORGEJO_ACCESS_TOKEN, FORGE_TOKEN_FILE, or whichever keys the binary reads).
  • List of locked env keys lives next to mcp-builtin-defs.json (e.g. mcp-builtin-locks.json mapping MCP name → array of locked env keys). Builtin layer is the source of truth; operator cannot edit at runtime.
  • Validation rejects writes to mcp_server.env (any scope ≥ global) that touch a locked key, with: "<env_key> on <mcp_name> is bound to type identity; remove from override".
  • Resolver merge skips locked keys from instance/agent_type/global env maps even if they slipped past validation; locked keys come exclusively from the type identity at render time.
  • Pattern is generic: when forge-mcp ships multi-forge (FM-3 — github, gitlab MCP names), they get the same lock treatment automatically by appearing in mcp-builtin-locks.json.

Validation

  • Instance-scope MCP row whose name matches an inherited row AND whose command, args, transport, or url differs from inherited → reject with: "instance scope can only override env on inherited MCP; differing <field> rejected".
  • Instance-scope MCP row with no inherited match → treated as additive net-new MCP, full record required (transport + command at minimum, validated by existing schema).
  • Any-scope MCP row that includes a locked env key (per mcp-builtin-locks.json) → rejected per forge-identity guardrail above.

Render path

  • agent-env-sync.renderForInstance (apps/server/src/infrastructure/agent-env-sync/render-for-instance.ts) writes the merged env into the materialized .mcp.json (or the MCP config file the agent reads).
  • For locked-key MCPs (forge, future github/gitlab), the render path injects auth env from ResolvedAgent.token_file / forgejo_user. Locked keys never come from the resolver's merged env.
  • No double-write: instance row's env is not written separately; the resolver's merged output (minus locked keys) is the single source for non-locked env.

Tests

  • Resolver: instance row with partial env (e.g. {LOG_LEVEL: "debug"}) merges with parent env (e.g. {LOG_LEVEL: "info", X: "y"}) → result {LOG_LEVEL: "debug", X: "y"}.
  • Resolver: instance row attempting to override command rejected at validation.
  • Render: materialized config on disk reflects merged env after renderForInstance(agent).
  • Forge identity guardrail: instance-scope row for forge with env.FORGEJO_ACCESS_TOKEN set → validation rejects.
  • Forge identity guardrail: even if a locked env key sneaks into the DB (e.g. via direct SQL), renderForInstance writes the type-derived value, not the row's value.
  • Forge identity guardrail: setting non-locked env keys on forge (e.g. LOG_LEVEL) still works as expected.

Out of scope

  • Env merge at agent_type or global scope (those still replace today). Revisit if a use case appears; out of this story.
  • Plugin-level config overrides (plugins don't carry env today).
  • Per-instance Forgejo identity / token rotation across the rest of the stack — git commit author, ForgePort host-side calls, PR creator. Those stay type-level until session key moves off type.
  • Multi-forge MCP rename / FM-3 work — separate milestone; this story only ensures the lock mechanism is generic enough for FM-3 to drop in github / gitlab names without code changes.

References

  • apps/server/src/infrastructure/agent-env-sync/render-for-instance.ts
  • apps/server/src/domain/agent-config/resolver.ts resolveMcpServers
  • config/mcp-builtin.json / config/mcp-builtin-defs.json (sibling: new mcp-builtin-locks.json)
  • Discussion 2026-05-02 — option (c) chosen: forge auth bound to type identity by construction, no operator footgun.
  • Depends on AOI-1 invariant (no enabled=false at instance scope).
As an operator running multiple instances of an agent type that need different MCP credentials (e.g. per-instance Penpot tokens or per-instance secrets paths), I want instance-scope `mcp_server` rows to merge their `env` map into the inherited row, so that I can tweak per-instance secrets without redefining `command` / `args` / `transport` / `url` — while forge MCP auth stays bound to type identity by construction. ## Acceptance criteria ### Resolver - [ ] `resolveMcpServers` (`apps/server/src/domain/agent-config/resolver.ts`) detects when an instance-scope row's `name` matches an inherited row from `agent_type` / `global` / `builtin` and produces a merged record: - `env`: instance keys override parent keys per-key; absent keys inherit - `command` / `args` / `transport` / `url`: inherited from parent; instance row's values for these MUST be null OR identical to parent (validation rejects divergence) - [ ] Net-new instance-scope MCP (no inherited row by `name`) keeps current full-row behavior. - [ ] AOI-1's `enabled=false` invariant still holds at instance scope. ### Forge MCP identity guardrail (option C) - [ ] Forge MCP (`name = "forge"`, formerly `"forgejo"`) is special-cased: the resolver synthesizes its auth env from the agent's type-level identity (`ResolvedAgent.token_file` / `forgejo_user`) at render time, **ignoring any `env` override at any scope** for the auth keys (`FORGEJO_ACCESS_TOKEN`, `FORGE_TOKEN_FILE`, or whichever keys the binary reads). - [ ] List of locked env keys lives next to `mcp-builtin-defs.json` (e.g. `mcp-builtin-locks.json` mapping MCP name → array of locked env keys). Builtin layer is the source of truth; operator cannot edit at runtime. - [ ] Validation rejects writes to `mcp_server.env` (any scope ≥ global) that touch a locked key, with: `"<env_key> on <mcp_name> is bound to type identity; remove from override"`. - [ ] Resolver merge skips locked keys from instance/agent_type/global env maps even if they slipped past validation; locked keys come exclusively from the type identity at render time. - [ ] Pattern is generic: when forge-mcp ships multi-forge (FM-3 — `github`, `gitlab` MCP names), they get the same lock treatment automatically by appearing in `mcp-builtin-locks.json`. ### Validation - [ ] Instance-scope MCP row whose `name` matches an inherited row AND whose `command`, `args`, `transport`, or `url` differs from inherited → reject with: `"instance scope can only override env on inherited MCP; differing <field> rejected"`. - [ ] Instance-scope MCP row with no inherited match → treated as additive net-new MCP, full record required (transport + command at minimum, validated by existing schema). - [ ] Any-scope MCP row that includes a locked env key (per `mcp-builtin-locks.json`) → rejected per forge-identity guardrail above. ### Render path - [ ] `agent-env-sync.renderForInstance` (`apps/server/src/infrastructure/agent-env-sync/render-for-instance.ts`) writes the merged env into the materialized `.mcp.json` (or the MCP config file the agent reads). - [ ] For locked-key MCPs (forge, future github/gitlab), the render path injects auth env from `ResolvedAgent.token_file` / `forgejo_user`. Locked keys never come from the resolver's merged env. - [ ] No double-write: instance row's `env` is not written separately; the resolver's merged output (minus locked keys) is the single source for non-locked env. ### Tests - [ ] Resolver: instance row with partial env (e.g. `{LOG_LEVEL: "debug"}`) merges with parent env (e.g. `{LOG_LEVEL: "info", X: "y"}`) → result `{LOG_LEVEL: "debug", X: "y"}`. - [ ] Resolver: instance row attempting to override `command` rejected at validation. - [ ] Render: materialized config on disk reflects merged env after `renderForInstance(agent)`. - [ ] Forge identity guardrail: instance-scope row for `forge` with `env.FORGEJO_ACCESS_TOKEN` set → validation rejects. - [ ] Forge identity guardrail: even if a locked env key sneaks into the DB (e.g. via direct SQL), `renderForInstance` writes the type-derived value, not the row's value. - [ ] Forge identity guardrail: setting non-locked env keys on `forge` (e.g. `LOG_LEVEL`) still works as expected. ## Out of scope - Env merge at `agent_type` or `global` scope (those still replace today). Revisit if a use case appears; out of this story. - Plugin-level config overrides (plugins don't carry env today). - Per-instance Forgejo identity / token rotation across the rest of the stack — git commit author, ForgePort host-side calls, PR creator. Those stay type-level until session key moves off type. - Multi-forge MCP rename / FM-3 work — separate milestone; this story only ensures the lock mechanism is generic enough for FM-3 to drop in `github` / `gitlab` names without code changes. ## References - `apps/server/src/infrastructure/agent-env-sync/render-for-instance.ts` - `apps/server/src/domain/agent-config/resolver.ts` `resolveMcpServers` - `config/mcp-builtin.json` / `config/mcp-builtin-defs.json` (sibling: new `mcp-builtin-locks.json`) - Discussion 2026-05-02 — option (c) chosen: forge auth bound to type identity by construction, no operator footgun. - Depends on AOI-1 invariant (no `enabled=false` at instance scope).
Collaborator

🦵 @charles kicked the queue — re-running implement on @code-lead.

🦵 @charles kicked the queue — re-running implement on @code-lead.
Sign in to join this conversation.
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
charles/claude-hooks#731
No description provided.