[realtonyyoung]

Problem

The Capacitor.Cli.Tests.Unit suite shows 1–3 flaky failures per full parallel run, always in the MCP-marker / env plugin suites (PluginCommand{Cursor,Copilot,Gemini}Tests, McpMarkerTests) — every failing test passes in isolation. Intermittent red CI for the whole team.

Root cause

McpMarker's central store is the real, process-global ~/.kcap/mcp-markers, resolved via GetFolderPath(UserProfile) — which ignores a redirected $HOME/KCAP_CONFIG_DIR. So any test that constructs a real McpMarker (the plugin suites, UninstallCommandTests, and McpMarkerTests.Two_configs_in_same_dir_have_independent_ownership, which had no [NotInParallel] key) reads/writes/deletes there, and FakeUserHome.Dispose enumerated that dir to sweep leaked markers. A concurrent create/delete during that enumeration — e.g. the unkeyed Two_configs test racing a plugin suite's FakeUserHome sweep — is the race. It also pollutes the developer's real ~/.kcap.

Fix

Mirrors the existing DaemonPathsGlobalSetup pattern:

• Add McpMarker.OverrideCentralRootForTesting(string?) — a test seam for the central .kcap root (analogous to DaemonLockPaths.OverrideDirectoryForTesting).
• McpMarkerGlobalSetup ([Before(Assembly)]) pins that root to a throwaway temp dir for the whole test assembly, cleaned in [After(Assembly)]. Because it's set once for the process (never a per-test null window), no test touches the real ~/.kcap/mcp-markers — the cross-suite race is structurally impossible and the real home is no longer polluted. Marker files are keyed by config-path hash, so parallel tests write distinct files and nothing enumerates the dir.
• Remove FakeUserHome's now-unnecessary central-marker sweep (the enumerate-shared-dir race) and per-test override; the one real-resolution marker test no longer needs cleanup or its own key.

Verification

• Deterministic: after a full parallel run of the unit suite, the real ~/.kcap/mcp-markers is byte-for-byte untouched (incl. directory mtime) — proving no test touches the shared dir, so the race surface is gone.
• No regression: full-suite failure count is unchanged vs main (the remaining failures are pre-existing local dev-home environment collisions — unrelated home-writing tests — which are green in CI's clean home).
• The de-flake is a parallel-load property, confirmed by the mechanism (nothing touches the shared dir) + CI.

Closes AI-1294.

🤖 Generated with Claude Code

[linear-code]

AI-1294

[qodo-code-review]

PR Summary by Qodo

Isolate McpMarker central store from real ~/.kcap during unit tests

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Add a test-only override for McpMarker’s central marker root directory.
• Pin central marker storage to an assembly-scoped temp directory to eliminate parallel test races.
• Remove FakeUserHome sweeping of real ~/.kcap/mcp-markers and simplify affected unit tests.

Diagram

graph TD
  A["Unit tests"] --> B["McpMarkerGlobalSetup"] --> C["McpMarker"] --> D[("Temp marker root")]
  F["FakeUserHome"] --> A
  C -. "prod default" .-> E[("Real ~/.kcap")]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Serialize affected suites (NotInParallel everywhere)

• ➕ Minimal production-code changes
• ➕ Reduces concurrency surface quickly
• ➖ Slower CI/local runs
• ➖ Easy to miss new unkeyed tests and regress flakiness
• ➖ Still risks polluting real ~/.kcap when tests run outside the serialized group

2. Inject markerPathFor in all call sites (avoid global override)

• ➕ Keeps McpMarker purely instance-configured
• ➕ No process-global mutable state
• ➖ Hard to guarantee coverage (plugin/command paths may construct real McpMarker)
• ➖ Requires broader refactors/DI pluming through production code just for tests

3. Make McpMarker respect KCAP_CONFIG_DIR/$HOME for central root

• ➕ More configurable behavior in real usage
• ➕ Could reduce other environment-collision issues
• ➖ Behavioral change for production users; higher compatibility risk
• ➖ Doesn’t by itself eliminate per-test ‘null window’ races unless combined with assembly pinning

Recommendation: Keep the PR’s assembly-scoped pinning approach. It mirrors the proven DaemonPathsGlobalSetup pattern, structurally prevents any test from touching the real ~/.kcap regardless of parallelism/order, and removes the brittle “enumerate and sweep shared dir” cleanup logic that caused the race. Alternatives either slow the suite, require wider refactors, or introduce production behavior changes with higher risk.

Files changed (4) +70 / -51

Bug fix (1) +10 / -1

McpMarker.csAdd central-root override seam for tests and route central markers through it +10/-1

Add central-root override seam for tests and route central markers through it

• Introduces a process-global (internal) test seam to override the central '.kcap' root directory. Central marker path resolution now uses the override when set, otherwise falls back to '~/.kcap' under the real user profile.

src/Capacitor.Cli.Core/Mcp/McpMarker.cs

Other (3) +60 / -50

FakeUserHome.csRemove central-marker sweeping from FakeUserHome disposal +14/-42

Remove central-marker sweeping from FakeUserHome disposal

• Deletes the dispose-time enumeration and deletion of files under the real '~/.kcap/mcp-markers'. Updates documentation to rely on the new assembly-level McpMarker central-root pinning.

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

McpMarkerTests.csStop cleaning up real central marker state in central-path resolution test +8/-8

Stop cleaning up real central marker state in central-path resolution test

• Updates the central-path resolution test to rely on the assembly-pinned temp root rather than explicit cleanup of a marker under '~/.kcap'. Adds clarifying comments about the isolation guarantee and parallel safety.

test/Capacitor.Cli.Tests.Unit/Mcp/McpMarkerTests.cs

McpMarkerGlobalSetup.csPin McpMarker central root to a temp directory for the whole test assembly +38/-0

Pin McpMarker central root to a temp directory for the whole test assembly

• Adds '[Before(Assembly)]'/'[After(Assembly)]' hooks that create a unique temp directory, set it as McpMarker’s central root override for the process, and delete it on teardown. This prevents any unit test from reading/writing the developer’s real '~/.kcap/mcp-markers'.

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

[qodo-code-review]

Code Review by Qodo

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

1. Overly verbose McpMarkerGlobalSetup docs ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The newly added XML documentation is unusually long and restates detailed rationale that could live
in the PR description or external docs instead. This reduces readability and violates the guideline
to keep comments concise and prefer self-explanatory code.

Code

test/Capacitor.Cli.Tests.Unit/McpMarkerGlobalSetup.cs[R5-19]

+/// <summary>
+/// Assembly-level safety net for MCP ownership markers (mirrors <see cref="DaemonPathsGlobalSetup"/>).
+///
+/// <para><see cref="McpMarker"/>'s central store is pinned under the real user profile
+/// (<c>~/.kcap/mcp-markers</c>) via <c>GetFolderPath(UserProfile)</c>, which ignores a redirected
+/// <c>$HOME</c>/<c>KCAP_CONFIG_DIR</c>. So any test that constructs a real <c>McpMarker</c> (the plugin
+/// suites, <c>UninstallCommandTests</c>, <c>McpMarkerTests</c>) reads/writes/deletes the developer's
+/// real central dir. Under parallel execution those cross-suite touches raced (AI-1294) — and they
+/// pollute the real home regardless.</para>
+///
+/// <para>Pinning the central root to a throwaway temp dir for the whole process makes the real
+/// directory unreachable regardless of test order or parallelism, with no per-test override to
+/// maintain (and no <c>null</c> window that would fall back to the real dir). Marker files are keyed
+/// by config-path hash, so parallel tests write distinct files here and never collide; nothing
+/// enumerates the dir, so concurrent writes are safe.</para>

Evidence

PR Compliance ID 5 requires comments to be concise. The new McpMarkerGlobalSetup XML doc spans
multiple paragraphs with detailed background and justification beyond minimal inline context.

CLAUDE.md: Keep comments concise and prefer self-explanatory code over verbose commentary
test/Capacitor.Cli.Tests.Unit/McpMarkerGlobalSetup.cs[5-19]

Agent prompt

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

## Issue description
New/modified comments are excessively verbose relative to what is needed to understand the code.

## Issue Context
Compliance prefers concise comments that add only necessary context and avoid duplicating detailed rationale that can be inferred from code or kept in PR/ticket documentation.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/McpMarkerGlobalSetup.cs[5-19]

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

---

2. Cleanup re-exposes real home ✓ Resolved 🐞 Bug ☼ Reliability

Description

McpMarkerGlobalSetup clears the central-root override before deleting the temp directory, creating a
window where any late/background McpMarker call will fall back to the real user-profile path
(~/.kcap/mcp-markers). This can reintroduce home-directory pollution/flakes if any asynchronous work
outlives the assembly teardown.

Code

test/Capacitor.Cli.Tests.Unit/McpMarkerGlobalSetup.cs[R33-36]

+    [After(Assembly)]
+    public static void CleanupCentralMarkerRoot() {
+        McpMarker.OverrideCentralRootForTesting(null);
+        try { Directory.Delete(SharedMarkerRoot, recursive: true); } catch { /* best effort */ }

Evidence

The teardown explicitly nulls the override, and McpMarker falls back to the real home path whenever
the override is null; therefore any call in that interval targets the real ~/.kcap tree.

test/Capacitor.Cli.Tests.Unit/McpMarkerGlobalSetup.cs[33-37]
src/Capacitor.Cli.Core/Mcp/McpMarker.cs[77-91]

Agent prompt

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

### Issue description
`CleanupCentralMarkerRoot()` sets `McpMarker.OverrideCentralRootForTesting(null)` before removing the temp root directory. If any late/background code still calls `McpMarker` after teardown begins, `MarkerPath()` will immediately fall back to the real home (`Path.Combine(home, ".kcap")`), potentially touching `~/.kcap/mcp-markers`.

### Issue Context
This PR’s primary goal is eliminating all writes to the real `~/.kcap/mcp-markers` during tests. Even if rare, the teardown ordering creates an avoidable gap where the real path becomes reachable again.

### Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/McpMarkerGlobalSetup.cs[33-37]
- src/Capacitor.Cli.Core/Mcp/McpMarker.cs[77-91]

### Suggested change
- Delete `SharedMarkerRoot` **while the override is still set**.
- Only after deletion attempt, clear the override in a `finally`.
- Optionally: keep the override pinned for process lifetime (omit clearing) if no other assemblies in the same process depend on restoring default behavior.

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

---

3. AI-1294 in GlobalSetup XML ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

Multiple comments and XML/assembly-level documentation include the Linear issue identifier
AI-1294, which PR Compliance ID 4 forbids in code comments and long-lived documentation. This
leaves external tracker references embedded in the repository (including in tests and setup docs).

Code

test/Capacitor.Cli.Tests.Unit/McpMarkerGlobalSetup.cs[12]

+/// real central dir. Under parallel execution those cross-suite touches raced (AI-1294) — and they

Evidence

PR Compliance ID 4 prohibits Linear issue identifiers in comments, and the cited files show
AI-1294 present in documentation/comments: test/Capacitor.Cli.Tests.Unit/McpMarkerGlobalSetup.cs
includes it in XML documentation, test/Capacitor.Cli.Tests.Unit/FakeUserHome.cs contains it in an
XML doc comment, src/Capacitor.Cli.Core/Mcp/McpMarker.cs includes it in an updated source comment,
and test/Capacitor.Cli.Tests.Unit/Mcp/McpMarkerTests.cs has it in the comment above
Two_configs_in_same_dir_have_independent_ownership. Together these locations demonstrate the
disallowed Linear identifier is embedded across both source and test documentation/comments.

CLAUDE.md: Do not include Linear issue numbers in source code comments
test/Capacitor.Cli.Tests.Unit/McpMarkerGlobalSetup.cs[8-13]
test/Capacitor.Cli.Tests.Unit/FakeUserHome.cs[17-17]
src/Capacitor.Cli.Core/Mcp/McpMarker.cs[29-29]
test/Capacitor.Cli.Tests.Unit/Mcp/McpMarkerTests.cs[97-100]

Agent prompt

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

## Issue description
Linear issue identifiers (specifically `AI-1294`) appear in XML documentation and source/test comments, which is disallowed by compliance.

## Issue Context
Compliance requires that source code comments and long-lived documentation do not include Linear issue numbers; if an issue reference is necessary, a GitHub issue number should be used instead.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/McpMarkerGlobalSetup.cs[8-13]
- test/Capacitor.Cli.Tests.Unit/FakeUserHome.cs[14-18]
- src/Capacitor.Cli.Core/Mcp/McpMarker.cs[27-33]
- test/Capacitor.Cli.Tests.Unit/Mcp/McpMarkerTests.cs[97-100]

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

---

4. Override lacks visibility guarantees ✓ Resolved 🐞 Bug ☼ Reliability

Description

_centralRootOverride is written and read across potentially parallel test threads without
volatile/Volatile.Read/Write semantics, relying on test-runner lifecycle ordering for publication.
Making the override explicitly visible across threads is a low-cost hardening to ensure no thread
observes the default real-home path after the assembly hook sets the override.

Code

src/Capacitor.Cli.Core/Mcp/McpMarker.cs[R27-34]

+    // Test seam: redirect the CENTRAL marker root (normally the user profile's `.kcap`) so unit tests
+    // never read/write the real shared `~/.kcap/mcp-markers` — a single process-global dir that races
+    // across parallel suites and pollutes the developer's home (AI-1294). Pinned once for the whole
+    // test assembly by `McpMarkerGlobalSetup` (mirrors `DaemonLockPaths.OverrideDirectoryForTesting` /
+    // `DaemonPathsGlobalSetup`), so there is no per-test null window that would fall back to the real dir.
+    static string? _centralRootOverride;
+    internal static void OverrideCentralRootForTesting(string? kcapRoot) => _centralRootOverride = kcapRoot;
+

Evidence

The override is a plain static field with unsynchronized writes/reads, and the code path explicitly
falls back to the real home when the field is null.

src/Capacitor.Cli.Core/Mcp/McpMarker.cs[24-34]
src/Capacitor.Cli.Core/Mcp/McpMarker.cs[77-91]
test/Capacitor.Cli.Tests.Unit/McpMarkerGlobalSetup.cs[27-31]

Agent prompt

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

### Issue description
`_centralRootOverride` is a mutable static field used to redirect central marker storage during tests. It is assigned in an assembly-level hook and read from `MarkerPath()` potentially on different threads, but there are no explicit memory-visibility guarantees.

### Issue Context
The override is intended to be set once and then treated as effectively immutable during the test run. Using `volatile` or `Volatile.Read/Write` makes that intent explicit and avoids relying on any implicit synchronization in the test framework.

### Fix Focus Areas
- src/Capacitor.Cli.Core/Mcp/McpMarker.cs[27-34]
- src/Capacitor.Cli.Core/Mcp/McpMarker.cs[77-91]

### Suggested change
- Change the field to `static volatile string? _centralRootOverride;` **or** use `System.Threading.Volatile`:
 - `OverrideCentralRootForTesting(...) => Volatile.Write(ref _centralRootOverride, kcapRoot);`
 - `var centralRoot = Volatile.Read(ref _centralRootOverride) ?? Path.Combine(home, ".kcap");`
- Keep behavior identical otherwise.

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

---

[realtonyyoung]

Static API review: one merge-blocking finding posted inline. The MCP-marker isolation and parallel-safety changes themselves look sound.

[realtonyyoung]

NO FINDINGS