Test suite reaches into real user state: unsealed tilde expansion, real ClaudeIntegration in two beds

[bguidolim]

Summary

An isolation audit of Tests/MCSTests/ found that the suite still reads real user state, and is one added test away from writing to the developer's real ~/.claude.json.

This is the same class of defect as the previously-fixed GitignoreManager leak, where an injected ShellRunner still resolved the process home and ran git config --global against the user's real global gitignore. That specific instance is fixed and regression-guarded; the pattern survives elsewhere.

Findings, ranked by severity

1. Real ClaudeIntegration wired into two test beds (latent write)

Tests/MCSTests/ComponentExecutorTests.swift and Tests/MCSTests/ProjectSyncTests.swift both build their executor as:

let env = Environment()                      // real home
ComponentExecutor(
    environment: env,
    output: ...,
    shell: ShellRunner(environment: env),    // real shell
    claudeCLI: ClaudeIntegration(shell: ...) // real CLI, not MockClaudeCLI
)

ClaudeIntegration.mcpAdd/mcpRemove run claude mcp add|remove and never pass HOME, so they write the real ~/.claude.json. No current test in either bed installs an .mcpServer component, so it does not fire today — adding one MCP-component test to either file silently mutates the developer's real Claude config.

LifecycleTestBed already injects MockClaudeCLI correctly; these two beds do not.

2. ScopedPathCheck has no environment seam at all (live read, structurally unsealable)

protocol ScopedPathCheck: DoctorCheck {
    var path: String { get }
    var scope: ExternalDoctorCheckScope { get }
    var projectRoot: URL? { get }
}

There is no environment member. resolvePath() calls expandTilde(path), which is NSString(string: path).expandingTildeInPath — the process home. No amount of correct injection upstream reaches it.

Every global-scoped external-pack check (fileExists, directoryExists, fileContains, fileNotContains) therefore reads the real home under a fully sandboxed DoctorRunner. It only reads today, but a fixCommand/fixScript on such a check would write.

This is a different failure class from the others: the seam is absent, not bypassed.

3. A test that reads real user state and cannot fail

Tests/MCSTests/CoreDoctorCheckTests.swift constructs PackGitignoreCheck with the default real Environment() and calls check(), hitting real git config --global and the real ~/.config/git/ignore. Its own comment concedes the problem, and it asserts nothing about the outcome:

// The actual result depends on the system's global gitignore — just verify the struct works
// Result will be either .pass or .fail depending on system state — no assertion on value

4. Process-global mutation under a parallel suite

• SyncCommandTests and PackSourceResolverTests mutate the process-wide cwd (restored in a defer, but Swift Testing runs tests in parallel).
• LifecycleIntegrationTests exercises interactive: true, reaching ShellRunner.runInteractive → forkpty() + tcsetattr(STDIN_FILENO, …). This puts the developer's terminal into raw mode mid-suite and competes for keystrokes. The test's own comment concedes it is not verifying interactive behavior.

5. ExternalPackAdapter defaults to a real-home shell

ExternalPackAdapter defaults shell: to ShellRunner(environment: Environment()), and ~10 test sites take that default. Related: ComponentExecutor calls component.supplementaryChecks(nil, Environment()), building a fresh process-home Environment instead of threading the injected one.

Context worth recording (not defects)

• No test hits the network. But the UpdateChecker guard is a test argument (entries: [], checkCLI: false), not a seam — one checkCLI: true goes live.
• ~15% of tests spawn real processes. The PackUpdaterTests run real git against local bare repos (no network) but with the real ~/.gitconfig, so a user's global core.hooksPath or init.templateDir executes inside the suite.
• Environment.resolvedGitPath/resolvedBrewPath are static let resolved once per process via a real which. Environment(home:) isolates paths, not tool discovery — there is no way to fake a missing git/brew. Avoid adding more static let command resolution.
• One PackUpdaterTests case asserts on inherited stdin TTY state and can block on a developer's keyboard when run from an interactive terminal.

Proposed fix

Items 1-3 are small and independent — do them first.

• 1. Inject MockClaudeCLI in both test beds.
• 2. Add environment to ScopedPathCheck and change expandTilde to take the injected home.
• 3. Sandbox the PackGitignoreCheck test, and give it a real assertion.
• 4. Replace the interactive-execution test with one asserting the command is dispatched, rather than allocating a real PTY. Remove or serialize the cwd mutations.
• 5. Drop ExternalPackAdapter's real-home default and thread the environment through supplementaryDoctorChecks.
• Pass HOME explicitly to subprocesses that resolve it themselves — GitignoreManager is the pattern to copy.

Acceptance criteria

Containment assertions, not happy-path ones: assert the resolved path is inside the injected home and outside FileManager.default.homeDirectoryForCurrentUser. GitignoreManagerTests already has an example to follow.

Expect sealing to turn some currently-green tests red — that is the signal that they were never testing what they claimed. Fix the code, not the test.

Add NSString(...).expandingTildeInPath and ProcessInfo.processInfo.environment to the isolation-audit grep patterns; neither was in the existing list, and the first is the most severe unsealed escape.