[AI-1703] Scope vendor MCP exclusion to the execution cwd's ancestor chain

[realtonyyoung]

The defect

WorktreeManager.WorkspaceMcpConfigPaths is root-relative, and every consumer matched it that way. But a borrowed snapshot can execute below the repository root — CreateBorrowedSnapshotAsync takes the git root and the user's cwd independently, and returns the execution path as WorktreeInfo.Path. A review flow started from <repo>/src therefore left src/.codex/config.toml live in the tree the reviewer runs in.

Separately, .github/mcp.json was not on the list at all — it carried .github/copilot/mcp.json, a different path — so that one was unprotected at the root of every borrowed snapshot regardless of scope.

This is a gap in already-merged containment (AI-1632, #427), not a follow-up nicety.

Vendor discovery

Re-checked against vendor documentation rather than inferred:

Path	Vendor	Discovery
.mcp.json	Claude Code	upward through parents, merging; continues above the git root
.mcp.json, .github/mcp.json	Copilot CLI	cwd → repository root
.codex/config.toml	Codex	repository root → cwd, closest wins
.cursor/mcp.json, .gemini/settings.json, .kiro/settings/mcp.json, .vscode/mcp.json	various	workspace root (a subset of the chain)

Opposite directions, same reachable set: the ancestor chain of the execution cwd. No supported vendor is documented to search downward or into a sibling — which is why a sibling is deliberately left alone, and why this repo's own committed kcap/.mcp.json survives.

The two structural changes

One prefix, from git, for both the classifier and the launch. It comes from rev-parse --show-prefix, not the filesystem. A .NET-derived prefix is not in the same pathname namespace as the paths ls-files reports — NFD/NFC on macOS, rooted results across Windows volumes — and deriving the launch cwd separately from the classified prefix lets the two disagree: an unexcluded path then materialises the alternate-spelling directory itself, and the vendor launches somewhere nothing was excluded.

One classifier, not two. Vendor paths no longer appear in the snapshot exclusion list at all; they are matched only through ClassifyReservedPath, which the review-context extractor also uses. The two matchers had different case semantics (OrdinalIgnoreCase vs ASCII-only) — unobservable while the list was ASCII constants, and exactly how a path becomes contained but unreviewable.

Also in here

• skip-worktree intersects the destination index (the source index holds staged-but-uncommitted paths the destination clone does not — batching those would have failed update-index on a legitimate snapshot), batched on stdin so a deep cwd cannot hit ARG_MAX or be read as pathspec syntax.
• EnsureSeparateRoots compares resolved paths as well as lexical ones — a WorktreeRoot symlinked inside the source passed the string test, and Claude Code's upward walk would then reach the source's own root config.
• The refresh path carries the prefix rather than re-deriving it; SyncFromSourceAsync's target-only overloads are replaced by one requiring a source cwd (no production callers).
• The execution directory is created rather than required to exist — widening the exclusion made a cwd whose only content is vendor config yield no directory at all, and throwing there would refuse launches for exactly the repositories this protects.

Testing

27 new tests, each with a positive control — including the case-sensitive a/A sibling pair (an earlier design folded case unconditionally, which handed a hostile branch a launch-refusal primitive), the cross-volume alternate-prefix bypass, the staged-only addition, --show-prefix framing against real git, and containment-plus-reviewability moving together.

Known residuals, stated not hidden

• Bind-mount and SUBST aliasing of WorktreeRoot — trusted-configuration residual; it is daemon operator config, not branch content.
• A non-ASCII cwd prefix is refused on case-insensitive volumes only, as a compatibility limitation rather than a security property.
• The pre-existing review-context capacity DoS (one tracked ancestor config >256 KiB refuses every launch) is unchanged here and tracked separately.

Spec (5 rounds of codex spec review): docs/superpowers/specs/2026-08-04-ai1703-vendor-config-discovery-scope-design.md

🤖 Generated with Claude Code

[linear-code]

AI-1703

[qodo-code-review]

PR Summary by Qodo

Scope vendor MCP exclusion to execution cwd ancestor chain

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Exclude vendor MCP configs across the execution cwd’s ancestor chain, not just repo root.
• Derive one git-native cwd prefix (rev-parse --show-prefix) for both exclusion and launch.
• Unify reserved-path matching and add coverage/tests for missing Copilot discovery paths.

Diagram

graph TD
AO["AgentOrchestrator"] --> WM["WorktreeManager (borrowed snapshot)"] --> GIT[("git: rev-parse/ls-files")]
GIT --> PLAN["SnapshotExclusionPlan"] --> BUILD["Build snapshot + review context"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Whole-tree vendor config exclusion

• ➕ Covers nested launches into descendants not on the ancestor chain
• ➕ Simpler reasoning: remove any vendor config anywhere in snapshot
• ➖ Functional regression: would delete sibling configs no vendor can discover (e.g., committed kcap/.mcp.json)
• ➖ Expands scope beyond stated threat model; increases disruption without clear benefit

2. Keep root-relative list, but also exclude cwd-relative variants

• ➕ Smaller surface-area change than introducing a plan
• ➕ Avoids some plan wiring through snapshot + review context
• ➖ Reintroduces risk of drift between multiple matchers/lists (the PR’s core defect)
• ➖ Still vulnerable to namespace mismatches (FS-derived vs git-derived spellings) and case semantics divergence

3. Unicode-aware case folding on case-insensitive destinations

• ➕ Could allow non-ASCII prefixes on case-insensitive volumes without refusal
• ➖ Adds a second matcher/normalization surface, undermining the ‘one classifier’ goal
• ➖ Hard to guarantee equivalence across platforms/filesystems; increases complexity and risk

Recommendation: Proceed with the PR’s approach: a single git-derived cwd prefix and a single byte-level reserved-path classifier shared by snapshot filtering, skip-worktree targeting, and review-context extraction/validation. This directly closes the reported under-exclusion (.codex under sub-cwd) and the missing path gap (.github/mcp.json) while avoiding the multi-matcher drift and namespace mismatch issues that alternatives tend to reintroduce.

Files changed (10) +1452 / -68 · 1 not counted

Enhancement (1) +14 / -2

WorktreeManager.WorkspaceMcp.csClarify scope semantics and add missing Copilot discovery paths +14/-2

Clarify scope semantics and add missing Copilot discovery paths

• Documents that entries are directory-relative and that scope is determined by PlanSnapshotExclusions. Adds '.github/mcp.json' and '.copilot/mcp-config.json' to the canonical vendor config path list and clarifies '.vscode/mcp.json' rationale.

src/Capacitor.Cli.Daemon/Services/WorktreeManager.WorkspaceMcp.cs

Bug fix (3) +176 / -57

AgentOrchestrator.csPersist and pass GitRelativeCwd when refreshing borrowed snapshots +7/-1

Persist and pass GitRelativeCwd when refreshing borrowed snapshots

• Updates borrowed snapshot refresh to require and pass the creation-time GitRelativeCwd into SyncBorrowedSnapshotFromSourceAsync. Prevents re-deriving cwd scope from the target filesystem, which could diverge from the exclusion classifier’s spelling.

src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs

WorktreeManager.ReviewContext.csAlign review-context extraction/validation with exclusion plan; cap manifest size _{not counted}

Align review-context extraction/validation with exclusion plan; cap manifest size

• Threads SnapshotExclusionPlan into review-context extraction so reserved-path matching uses the same expanded set and shared classifier semantics. Adds a serialized-manifest size ceiling with pre-allocation check, and validates entries against the exact matched reserved paths to avoid introducing a second matcher.

src/Capacitor.Cli.Daemon/Services/WorktreeManager.ReviewContext.cs

WorktreeManager.csUse git-relative cwd for launch + exclusion; remove static excluded list +169/-56

Use git-relative cwd for launch + exclusion; remove static excluded list

• Captures GitRelativeCwd at borrowed snapshot creation, uses it for execution path, and persists it on WorktreeInfo for refresh. Removes SnapshotExcludedPaths, builds a per-attempt SnapshotExclusionPlan, routes vendor exclusions through ClassifyReservedPath on raw bytes, batches skip-worktree updates based on destination index membership, strengthens EnsureSeparateRoots with resolved-path checks, and changes SyncFromSourceAsync to require a source-side cwd.

src/Capacitor.Cli.Daemon/Services/WorktreeManager.cs

Refactor (1) +252 / -0

WorktreeManager.ExclusionPlan.csIntroduce SnapshotExclusionPlan and git-derived cwd prefix parsing +252/-0

Introduce SnapshotExclusionPlan and git-derived cwd prefix parsing

• Adds plan construction that expands WorkspaceMcpConfigPaths over the execution cwd’s ancestor chain with depth/byte caps. Implements bounded byte capture for 'git rev-parse --show-prefix', strict framing/UTF-8 parsing, and a stdin-batched git runner for skip-worktree updates.

src/Capacitor.Cli.Daemon/Services/WorktreeManager.ExclusionPlan.cs

Tests (4) +501 / -9

BorrowedSnapshotExclusionScopeTests.csAdd end-to-end tests for ancestor-chain scoping, parsing, caps, and refresh +475/-0

Add end-to-end tests for ancestor-chain scoping, parsing, caps, and refresh

• Creates real git fixtures to verify exclusion across root→cwd chain, sibling non-exclusion, and '.github/mcp.json' coverage. Adds tests for '--show-prefix' framing, non-ASCII rules, depth/aggregate-byte caps, destination-index skip-worktree behavior, review-context reachability, symlinked worktree-root rejection, and refresh reuse of persisted GitRelativeCwd.

test/Capacitor.Cli.Tests.Unit/BorrowedSnapshotExclusionScopeTests.cs

AcpHostedAgentRuntimeFactoryLiveTests.csUpdate SyncFromSourceAsync calls to supply source cwd +1/-1

Update SyncFromSourceAsync calls to supply source cwd

• Adjusts the live runtime factory test to use the new SyncFromSourceAsync signature that requires sourceRepoRoot and sourceCwd.

test/Capacitor.Cli.Tests.Unit/Services/AcpHostedAgentRuntimeFactoryLiveTests.cs

WorkspaceMcpNeutralizationTests.csUpdate path coverage assertions and plan-based exclusion expectations +23/-6

Update path coverage assertions and plan-based exclusion expectations

• Extends the expected vendor path list to include '.github/mcp.json' and '.copilot/mcp-config.json'. Replaces SnapshotExcludedPaths usage with PlanSnapshotExclusions at root and adds an explicit test that the new Copilot paths are present in WorkspaceMcpConfigPaths.

test/Capacitor.Cli.Tests.Unit/WorkspaceMcpNeutralizationTests.cs

WorktreeManagerTests.csUpdate unit tests for new SyncFromSourceAsync signature +2/-2

Update unit tests for new SyncFromSourceAsync signature

• Updates borrowed snapshot refresh-related tests to pass a sourceCwd argument instead of (now removed) target-side executionPath overloads.

test/Capacitor.Cli.Tests.Unit/WorktreeManagerTests.cs

Documentation (1) +509 / -0

2026-08-04-ai1703-vendor-config-discovery-scope-design.mdAdd design spec for git-derived cwd scoping and single-classifier exclusion +509/-0

Add design spec for git-derived cwd scoping and single-classifier exclusion

• Introduces a detailed threat model and vendor discovery matrix, explains the root-relative defect, and specifies a git byte-protocol for deriving cwd prefix. Defines the single-plan/single-classifier architecture, refresh semantics, bounds, and a comprehensive test plan (including new Copilot paths and symlink-root residuals).

docs/superpowers/specs/2026-08-04-ai1703-vendor-config-discovery-scope-design.md

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Invalid array length type 🐞 Bug ≡ Correctness

Description

ReadManifestBytesAsync allocates with new byte[stream.Length] where FileStream.Length is a
long, which does not compile as an array length expression. This breaks the daemon build and
prevents review-context manifest validation from working.

Code

src/Capacitor.Cli.Daemon/Services/WorktreeManager.ReviewContext.cs[R62-66]

+        if (stream.Length > MaxReviewContextManifestBytes)
+            throw new InvalidOperationException(
+                "borrowed_snapshot_review_context_manifest_too_large");
+        var buffer = new byte[stream.Length];
+        await stream.ReadExactlyAsync(buffer, ct);

Evidence

The method checks stream.Length against a max, then uses it directly as an array length, which is
a compile-time type error because FileStream.Length is long.

src/Capacitor.Cli.Daemon/Services/WorktreeManager.ReviewContext.cs[57-67]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`ReadManifestBytesAsync` uses `new byte[stream.Length]` but `stream.Length` is `long`, which is not a valid array length type in C#.

### Issue Context
The method already enforces `stream.Length <= MaxReviewContextManifestBytes` (4 MiB), so converting to `int` is safe once made explicit.

### Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/WorktreeManager.ReviewContext.cs[57-67]

### Suggested change
After the size check, convert length explicitly:
- `var length = checked((int)stream.Length);`
- `var buffer = new byte[length];`
Then read exactly `length` bytes.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Verbose comments in SnapshotExclusionPlan ✗ Dismissed 📘 Rule violation ⚙ Maintainability

Description

New/updated code introduces large, narrative-style comment blocks that restate behavior and
historical context instead of keeping comments minimal and focusing on non-obvious rationale. This
increases maintenance cost and makes core logic harder to scan and review.

Code

src/Capacitor.Cli.Daemon/Services/WorktreeManager.ExclusionPlan.cs[R27-30]

+    /// <para><b>Why a plan rather than a static list.</b> Vendors resolve workspace MCP config along the
+    /// ancestor chain of their cwd — Codex layers <c>.codex/config.toml</c> from the repository root down
+    /// to the cwd, Copilot and Claude Code walk from the cwd upward — so a root-relative list is only
+    /// complete when the cwd IS the root. A borrowed snapshot may execute below the root, and a root-only

Evidence

PR Compliance ID 2 requires avoiding large verbose comments in favor of self-explanatory code. The
added multi-paragraph XML documentation and narrative inline comments (e.g., the long "Why a
plan"/"One list"/"Not a security boundary" sections) meet the checklist's failure criteria for
overly verbose commentary.

CLAUDE.md: Avoid verbose comments; prefer self-explanatory code
src/Capacitor.Cli.Daemon/Services/WorktreeManager.ExclusionPlan.cs[23-44]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Several new/updated files add very long explanatory comments (multi-paragraph XML docs and inline blocks). This violates the guideline to avoid verbose comments and prefer self-explanatory code with concise, additive commentary.

## Issue Context
This PR adds substantial narrative/history/threat-model explanation directly in code comments. That material is valuable, but should mostly live in design/spec docs, leaving code comments short and focused on the most non-obvious invariants.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/WorktreeManager.ExclusionPlan.cs[23-44]
- src/Capacitor.Cli.Daemon/Services/WorktreeManager.ReviewContext.cs[41-50]
- src/Capacitor.Cli.Daemon/Services/WorktreeManager.cs[888-901]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Unvalidated sourceCwd in sync ✓ Resolved 🐞 Bug ⛨ Security

Description

The new public SyncFromSourceAsync runs git rev-parse --show-prefix in the caller-provided
sourceCwd without verifying it is inside sourceRepoRoot (or even that it belongs to the same git
worktree). If sourceCwd is a valid git directory outside the intended repo (e.g., another repo),
the derived prefix/exclusion plan can be wrong and may leave vendor config unexcluded in the
generated target tree.

Code

src/Capacitor.Cli.Daemon/Services/WorktreeManager.cs[R563-566]

    public async Task SyncFromSourceAsync(
-            string sourceRepoRoot, string targetWorktreePath,
-            string[] excludePaths, CancellationToken ct) {
-        await SyncFromSourceAsync(
-            sourceRepoRoot, targetWorktreePath, targetWorktreePath, excludePaths, ct);
-    }
-
-    public async Task SyncFromSourceAsync(
-            string sourceRepoRoot, string targetWorktreePath, string executionPath,
+            string sourceRepoRoot, string sourceCwd, string targetWorktreePath,
            string[] excludePaths, CancellationToken ct) {
+        var gitRelativeCwd = await ReadGitRelativeCwdAsync(Path.GetFullPath(sourceCwd), ct);

Evidence

SyncFromSourceAsync derives gitRelativeCwd from sourceCwd with no containment check, while
ReadGitRelativeCwdAsync runs git in whatever directory it is given. CreateBorrowedSnapshotAsync
shows the intended containment validation pattern that is missing from the sync API.

src/Capacitor.Cli.Daemon/Services/WorktreeManager.cs[501-513]
src/Capacitor.Cli.Daemon/Services/WorktreeManager.cs[563-570]
src/Capacitor.Cli.Daemon/Services/WorktreeManager.ExclusionPlan.cs[71-78]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`WorktreeManager.SyncFromSourceAsync(sourceRepoRoot, sourceCwd, ...)` derives `gitRelativeCwd` by running git in `sourceCwd`, but never checks that `sourceCwd` is contained by `sourceRepoRoot` or that git’s top-level directory matches `sourceRepoRoot`.

### Issue Context
`CreateBorrowedSnapshotAsync` already performs explicit containment/existence checks on `requestedCwd` before calling `ReadGitRelativeCwdAsync`; the sync path should enforce the same invariant.

### Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/WorktreeManager.cs[501-513]
- src/Capacitor.Cli.Daemon/Services/WorktreeManager.cs[563-570]
- src/Capacitor.Cli.Daemon/Services/WorktreeManager.ExclusionPlan.cs[71-78]

### Suggested change
In `SyncFromSourceAsync`:
1) `source = Path.GetFullPath(sourceRepoRoot)` and `cwd = Path.GetFullPath(sourceCwd)`.
2) Reject if `!Directory.Exists(cwd)`.
3) Reject if `Path.GetRelativePath(source, cwd)` is rooted or escapes via `..` (same logic as `CreateBorrowedSnapshotAsync`).
4) (Stronger) Run `git rev-parse --show-toplevel` in `cwd` and ensure it equals `sourceRepoRoot` under your existing path comparison rules; fail if it differs.
Then call `ReadGitRelativeCwdAsync(cwd, ct)`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. NUL in XML doc comment ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

WorktreeManager.ReviewContext.cs contains an embedded NUL control character in an XML doc comment
((<c>\0</c>) rendered as a literal NUL). This can cause source tooling to treat the file as binary
and may fail compilation/formatting in some environments.

Code

src/Capacitor.Cli.Daemon/Services/WorktreeManager.ReviewContext.cs[R45-47]

+    /// <para><b>Derived from the content cap, not chosen.</b> Each admitted byte can appear twice — once
+    /// base64-encoded (4/3) and once in <c>Text</c>, where JSON escaping of a control character costs six
+    /// bytes (<c>\u0000</c>). Worst case is therefore about <c>256 KiB × (4/3 + 6) ≈ 1.9 MiB</c> before

Evidence

The comment line visibly includes a literal control character inside <c>...</c>, indicating the
source file contains a NUL byte/character rather than an escaped sequence.

src/Capacitor.Cli.Daemon/Services/WorktreeManager.ReviewContext.cs[41-48]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
An XML doc comment includes a literal NUL character, which can break text-processing tools and may be rejected by some compilers/linters.

### Issue Context
The intent is to *describe* a NUL/control character expansion cost, not to embed the byte in source.

### Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/WorktreeManager.ReviewContext.cs[45-48]

### Suggested change
Replace the literal NUL with a safe textual representation, e.g.:
- `(<c>\\0</c>)` or `(<c>\\u0000</c>)` or `(<c>NUL</c>)`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}