🤖 fix: inherit workspace MCP overrides in sub-agent and forked workspaces - #4130
Conversation
|
@codex review |
This comment has been minimized.
This comment has been minimized.
|
@codex security review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/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: f08b02a521
ℹ️ 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".
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review |
|
@codex security review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 644f0a1d36
ℹ️ 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".
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review |
|
@codex security review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41220dc65e
ℹ️ 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".
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review |
|
@codex security review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b908b39a42
ℹ️ 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".
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review |
|
@codex security review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4021b7a42
ℹ️ 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".
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review |
|
@codex security review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 211137c5db
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: 211137c5db
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
|
@codex review |
|
@codex security review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf5cdef83d
ℹ️ 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".
|
@codex review |
|
@codex security review |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
dd55cbc to
0aa31d1
Compare
|
@codex review |
|
@codex security review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0aa31d1a32
ℹ️ 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".
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
0aa31d1 to
b99e03b
Compare
|
@codex review |
|
@codex security review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b99e03bb8f
ℹ️ 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".
There was a problem hiding this comment.
🛡️ Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: b99e03bb8f
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
b99e03b to
697404f
Compare
|
@codex review |
|
@codex security review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 697404f4a8
ℹ️ 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".
There was a problem hiding this comment.
🛡️ Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: 697404f4a8
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
|
@codex review |
|
@codex security review |
697404f to
ac29963
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac29963391
ℹ️ 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".
…aces
Per-workspace MCP enables live in the gitignored `<checkout>/.xum/mcp.local.jsonc`;
sub-agent children and `workspace.fork` get fresh worktrees, so a server that is
disabled globally and enabled only for the parent silently disappeared for every
child — and with it `tool_catalog_search`.
- WorkspaceMcpOverridesService resolves through the `parentWorkspaceId` chain
(read-through) when a workspace's own probes are positively absent and its own
document/legacy value is recognized-empty; saves/prunes publish inheriting
descendants; a cross-process epoch invalidates other processes' caches.
- Fork creation copies the effective source document raw into the new checkout
(epoch/chain/precedence-checked commit point under the override lock).
- MCPServerManager re-validates caller override snapshots against disk/epoch and
fails closed when it cannot vouch for enablement; TurnRequestBuilder derives
the prompt inventory from the validated serve.
- Plugin-key pruning edits the parse tree (jsonc.modify corrupts compact arrays)
and rejects documents that retain keys in fields this build does not own.
- Rebased onto main's additive server publication: retained entries carry the
repaired inventory/generation; the cached and published serves go through
serveResult (call-time gated tools, validated prompt descriptors).
- Round 77: a repair save is refused when authority was lost in an ANCESTOR
(own document intact; `{}` is a fallback for the parent's settings); prompt
discovery returns nothing for a fail-closed serve instead of listing the
published entry; override path guards probe host files (local/worktree and
devcontainer host worktrees) with lstat instead of exec'ing into a container.
- Round 78: a non-empty save is refused while the canonical document cannot be
probed (the write would truncate fields it never read); the stdio launch fence
aborts a launch that has not produced its exec stream by the deadline (startup
timeout, retried) instead of releasing the writer's lock under it.
- Round 79: the migration's mkdir is a tracked side effect (joined before the
locks release); recursive serves forward the caller's read abort signal.
- Round 80: devcontainer checkouts share the host filesystem identity (aliases
contend for one lock); a child detaching from an inheriting chain carries the
inherited document's forward-compatible fields (or refuses when unmergeable).
- Round 81: devcontainer sources are host-backed for path guards; prompt
discovery re-serves when the entry or serve provenance changed during
prompts/list; saves refuse to retire a legacy value mixing known settings
(never shown) with unknown fields. Known limitation: a child cannot override
an inherited value back to the project default (needs a detach marker).
- Round 82: host override documents whose bytes leave the checkout (fork copy,
carried unknown fields) are read through an O_NOFOLLOW handle so a symlink
swapped in after the path guards cannot exfiltrate a host file; the call-time
gate compares recorded options by authorization equivalence, not identity.
- Round 83: checkout lock keys fall back to a lenient registry walk (accepted
only when every requested id is still registered) so an unrelated legacy
entry's malformed metadata.json cannot block rename/removal.
- Round 84: plugin-key pruning validates every override document before
rewriting any (and publishes the epoch if a write fails after a rewrite); a
directory at an override path is indeterminate, never absence (no
inheritance through it); Windows separators normalized in the rollback match.
- Round 85: the call gate's disk-verdict invalidation uses authorization
equivalence like dispatch (an equivalent pre-edit serve cannot mask a disk
revocation); zero-timeout cross-process lock attempts reject without sleeping.
- Round 86: host document reads verify the opened handle after the O_NOFOLLOW
open (segments re-guarded under the checkout's realpath; the canonical file
must be the handle's inode), so a parent segment swapped to a symlink cannot
redirect the fork copy to a sibling checkout's document.
- Round 87: the override epoch and writer locks live under the MCP
configuration root (`coordinationRootDir`), so `xum run` (disposable registry
root) coordinates with a desktop/server backend on the persistent home.
Squashed from 82 review-round commits when rebasing onto main (pre-rebase history
kept on local backup branches).
_Generated with `xum` • Model: `anthropic:claude-fable-5-1` • Thinking: `high` • Cost: `$712`_
<!-- mux-attribution: model=anthropic:claude-fable-5-1 thinking=high costs=712 -->
ac29963 to
cf3a984
Compare
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
1 participant
Summary
A server that is disabled in global MCP config and enabled only for one workspace silently disappeared for every sub-agent (
task) child and for forks of that workspace — and with it thetool_catalog_searchtool, which is only created when MCP tools exist. Children now inherit the parent's per-workspace overrides, and forks snapshot them.Background
Per-workspace MCP enables are stored in the gitignored
<checkout>/.xum/mcp.local.jsonc. Sub-agent workspaces (TaskService→runBackgroundInit) andworkspace.forkcreate a fresh worktree from committed state, so that file never reaches them. The child'sgetOverridesForWorkspacereturned{}, the globaldisabled: truewon inapplyServerOverrides,mcpToolswas empty,TurnRequestBuildernever createdtoolSearchRuntime, and the model saw neither the MCP tools nortool_catalog_search. Onlyisolation: "none"tasks (shared checkout) were unaffected.The same-workspace path was verified to work in every variant tried (exec/plan/ask agents, untrusted project, disable→enable sequencing); the failure is specific to derived workspaces.
Implementation
WorkspaceMcpOverridesService.loadOverrides: when a workspace has no local file and no legacy config overrides, resolve throughmetadata.parentWorkspaceId(read-through, no copy, depth-bounded). Parent edits and plugin-uninstall prunes therefore apply to children on their next request. A child that saves its own overrides opts out; clearing them resumes inheriting. Lenient reads tolerate a removed/unreachable parent (send never fails); strict reads surface it.WorkspaceService.fork:copyOverridesToForkedCheckoutsnapshots the source overrides into the new checkout (forks are independent workspaces, so no read-through link), placed before the existing plugin-override sanitization so staleplugin:enables are pruned exactly as for a tracked file. Best-effort; a failed copy never fails the fork. Shared-checkout (project-dir) forks are skipped.publishcallback now carries the target workspaceId. Every workspace on the same runtime identity whose canonical override path equals the written file (isolation: "none"tasks share the parent's checkout — in either direction) is treated as affected and re-published too, followed by its inheriting descendants. Sharers are matched on filesystem identity (host / SSH host / devcontainer config; never Docker) withrealpath-canonicalized paths on the host, not on the whole runtime config or path spelling. Publication is authoritative-or-evict: a workspace whose effective state cannot be established (unreachable checkout) — and its entire inheriting subtree — getspublish(null), whichMCPServerManager.forgetWorkspaceOverridesturns into an eviction of both the overlay cache and the recorded request options (the next serve of any kind, including prompt paths, re-reads disk) rather than a cached guess; a non-authoritative disk re-read (indeterminate probe, read or parse failure) keeps that invalidation alive — invalidations are generation-tracked so a forget landing mid-read is never retired by the older result — and the workspace fails closed (no MCP servers) until disk answers authoritatively. Resolution runs on already-loaded metadata (one config scan per save) and probes an ancestor's candidate paths in parallel. Clearing a child's own overrides publishes its inherited state instead of{}, soMCPServerManager'slatestWorkspaceOverridesnever pins a child to stale enablement.file/absent/indeterminate). Inheritance requires every candidate path to be positively absent (errnoENOENT/ENOTDIR, or stat(1)'s own no-such-file diagnostic line on exec-backed remote runtimes — transport noise such as OpenSSH's identity-file warning stays indeterminate); an indeterminate probe keeps the pre-existing "no overrides" behaviour so a hidden child disable can never be overridden by the parent's enable.config.json→ file migration), the same-path skip applies only tolocal(project-dir) runtimes that genuinely share the checkout (Docker reports/srcfor every container), the copy runs beforerunBackgroundInitso the init hook sees the fork's initial configuration deterministically, and the raw document that actually supplied the overrides (the source's own, or the ancestor's when the source is an inheriting sub-agent) is copied whenever one exists — even if this build normalizes it to nothing — with only canonicalplugin:keys stripped (the prune text transform was extracted fromprunePluginOverrideKeysfor this), so comments and forward-compatible fields survive per the upgrade↔downgrade rule while a fresh checkout never inherits a plugin enable it has no consent context for. Whatever survives pruning is scanned once more: any canonical plugin key in any decoded string of the parsed tree (including fields this build does not own; JSON escapes cannot slip past) refuses the copy. The write is refused when the target.xum/.xum/mcp.local.jsoncis a repo-controlled symlink or an already-tracked file (writes follow links on every runtime, so a tracked link could redirect the copy outside the checkout).WorkspaceServiceoverrides port is now aPick<WorkspaceMcpOverridesService, ...>instead of an ad-hoc interface.prunePluginOverrideKeysForWorkspacespublishes after every prune landed (an inheriting descendant swept before its parent would otherwise publish the parent's pre-prune state) through the same fan-out publisher, and aConfigSnapshotmemoizes the authoritative workspace enumeration, the legacyconfig.jsonsnapshot and hostrealpaths across one resolution/publication — or one whole sweep — so inheritance and sharer detection never re-parse config per workspace (the sweep stays one metadata load up front plus one post-sweep re-resolution). When the post-sweep enumeration fails, every cache is evicted and every swept workspace keeps its tombstone.runtimeFactory(resolveCoderSSHHost), so distinct Coder machines sharing the rawcoder://placeholder never coincide; legacyconfig.jsonmigration never writes over an existing document (even one this build normalizes to nothing).Validation
Unit tests in
workspaceMcpOverridesService.test.ts: parent-chain inheritance (grandchild), read-through (no file materialized in the child), child overrides win + CAS against the inherited revision, clearing resumes inheritance, removed-parent behaviour (strict re-reads must not throw, or the plugin uninstaller would retry an orphan's tombstone forever), descendant re-publication on save/prune/clear, raw fork copy, legacy migration before same-path skip.New integration test
tests/ipc/config/mcpWorkspaceOverridesInheritance.test.tsreproduces the user-visible symptom end-to-end with the realMCPServerManager+ fixture stdio server by capturing the assembled request atStreamManager.startStream: global disable → no tools; workspace enable →shots_take_screenshot+tool_catalog_search(deferred); the same surface in aparentWorkspaceIdchild and in a fork. Confirmed red onmain, green with this change.make static-checkgreen;workspaceService,workspaceMcpOverridesService,mcpServerManager,installServicesuites (755 tests) green.Exec-backed
statnow runs withLC_ALL=Cso the absence classifier is locale-stable on remote hosts; the fork copy targetstargetRuntime.getWorkspacePath(...)(multi-project forks persist the container root, not the primary checkout).Risks
Low–moderate. Inheritance only kicks in when a child has no overrides of its own, and only via
parentWorkspaceId(task children). The fork copy runs in the existing best-effort copy phase and is followed by the existing sanitization, so the plugin-consent security posture is unchanged. Every send in a child without its own file now performs one extrastatagainst the parent's checkout (remote for SSH parents), matching the cost of the child's own read.Generated with
xum• Model:anthropic:claude-fable-5-1• Thinking:high• Cost:$12.80