fix: mock _ensure_executable in mount tests for cross-platform compatibility

[HDMowri]

Fixes #4

Summary

Adds missing _ensure_executable mock to mount tests so they pass on all platforms.

Problem

Commit 8738bbd added _ensure_executable() inside _find_copilot_cli() to fix a real uv package manager issue (execute bit stripped from bundled CLI binary). However, the existing mount tests were not updated to account for this new function, which calls os.access() and os.stat() on test fixture paths that don't exist on disk.

This causes:

• 14 test failures on Windows (WinError 3 on Unix-style fake paths)
• 7 test failures on Linux/WSL (FileNotFoundError on non-existent paths)

Solution

Add patch("amplifier_module_provider_github_copilot._ensure_executable") to every mount test that expects _find_copilot_cli() to return a valid path. This mock replaces the function with a no-op, preventing filesystem access on fake test paths.

The mock patches at the Python function level, so it works identically on Windows, Linux, and macOS.

Testing

Platform	Mount Tests	Full Suite
Windows (Python 3.13.12 ARM64)	29 passed, 0 failed	534 passed, 0 failed, 33 skipped
WSL Ubuntu 24.04 (Python 3.12.3)	29 passed, 0 failed	534 passed, 0 failed, 33 skipped

Files Changed

• tests/test_mount.py — 10 tests updated
• tests/test_mount_coverage.py — 7 tests updated

[mowree]

@microsoft-github-policy-service agree company="Microsoft"

[Copilot]

Pull request overview

This PR fixes cross-platform test failures in mount tests by adding missing _ensure_executable mocks. The _ensure_executable function was added in a previous commit to handle a real issue with the uv package manager stripping execute bits from bundled binaries, but the existing tests weren't updated to account for this new filesystem access, causing 14 test failures on Windows and 7 on Linux/WSL.

Changes:

• Added patch("amplifier_module_provider_github_copilot._ensure_executable") to 10 tests in test_mount.py that expect _find_copilot_cli() to return a valid path
• Added the same mock to 7 tests in test_mount_coverage.py for the same reason
• All mocks prevent filesystem access (os.access(), os.stat()) on non-existent test fixture paths

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
tests/test_mount.py	Added _ensure_executable mocks to 10 tests that verify successful CLI discovery and mount operations
tests/test_mount_coverage.py	Added _ensure_executable mocks to 7 tests covering edge cases in CLI discovery and exception handling

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[HDMowri]

@microsoft-github-policy-service agree company="Microsoft"

[robotdad]

PR Review: #5 — fix: mock _ensure_executable in mount tests for cross-platform compatibility

Verdict: ✅ APPROVE

Author: HDMowri | Commit: d6f5afbc | Reviewed: 2026-02-18

This PR correctly adds patch("amplifier_module_provider_github_copilot._ensure_executable") to mount tests that exercise successful CLI discovery paths. Without this mock, _ensure_executable calls os.access() / os.stat() / os.chmod() on paths that don't exist on disk, breaking tests on Windows and in CI. The fix is correct, narrowly scoped, and test-only — no production code is changed.

---

Test Results

Check	Result	Details
Cross-Repo Contracts	✅ PASS	mount(coordinator, config) signature matches core. Zero copilot-specific references in amplifier-core.
Security Audit	✅ PASS	0 critical, 0 high, 0 medium, 0 low findings. No secrets, no injection vectors, mocks properly scoped.
Unit Tests (Shadow)	✅ PASS	534 passed, 33 skipped (live integration). test_mount.py: 18/18, test_mount_coverage.py: 10/10.
Smoke Tests (Shadow)	✅ PASS	8/9 phases passed. Phase 9 failure is pre-existing infrastructure bug in bundle-composition recipe path parsing — unrelated to this PR.
Host Isolation	✅ PASS	Host environment unchanged after shadow testing.
Python Quality Checks	⚠️ WARN	Linting/formatting issues detected. See suggestion below.
Source Verification	✅ PASS	Commit d6f5afb verified at inject, post-install, and post-test.

---

Critical Issues

None. This PR is safe to merge as-is.

---

Suggestions for Follow-Up

These are structural improvements that would prevent the next version of this same PR. None are merge-blocking.

1. Extract a mock_cli_found fixture (High Value)

Nearly every happy-path test has 5–6 levels of nested with patch context managers. The actual test logic is buried at indentation level 6–7. A single fixture in conftest.py would eliminate ~80% of the nesting:

# conftest.py
@pytest.fixture
def mock_cli_found():
    """Patch all OS-level checks to simulate a found, executable CLI."""
    with (
        patch("shutil.which", return_value="/usr/bin/copilot"),
        patch("os.path.isfile", return_value=True),
        patch("os.path.isabs", return_value=True),
        patch("amplifier_module_provider_github_copilot._ensure_executable"),
    ):
        yield

Then tests become readable:

async def test_mount_success(self, mock_coordinator, mock_cli_found):
    cleanup = await mount(mock_coordinator, {"model": "claude-opus-4.5"})
    assert cleanup is not None

This is the highest-ROI change — it fixes readability immediately and makes future internal changes a 1-line fixture edit instead of a mass-edit PR.

2. Mock at the right boundary for mount() tests (Medium Value)

mount() tests currently mock 4–5 stdlib calls to simulate "CLI found." This means any refactor to _find_copilot_cli internals breaks every mount test. Better approach:

• mount() tests → mock _find_copilot_cli itself (one patch, tests mount logic)
• _find_copilot_cli tests → mock OS-level calls (tests discovery logic in isolation)

This separation would have prevented the need for this entire PR — the _ensure_executable addition would only have affected _find_copilot_cli tests.

3. Consolidate test files (Low-Medium Value)

test_mount.py and test_mount_coverage.py have substantial overlap (mount exception paths, absolute path not found, relative path not in PATH, CLI not found, discovery exceptions). The two-file split no longer serves a clear purpose. Consolidating would reduce maintenance burden — as this PR demonstrates, the same mechanical fix had to be applied to both files independently.

4. Resolve Python quality check warnings

The automated Python quality checks (ruff format/lint, pyright) reported issues. Consider running ruff check --fix and ruff format on the test files.

---

Positive Notes

• Correct diagnosis and targeted fix. The PR identifies the exact problem (_ensure_executable calling OS functions on mocked paths) and addresses it precisely.
• No production code touched. The change is entirely in test infrastructure, eliminating any deployment risk.
• Cross-platform awareness. The PR title and approach show good attention to CI/cross-platform compatibility — the kind of fix that prevents flaky builds.
• Error paths well-tested. Both files include solid negative test cases (mount failures, missing CLIs, permission errors, exceptions during discovery).
• Contract safety verified. The mount(coordinator, config) interface exactly matches amplifier-core's calling convention. Zero risk of cross-repo breakage.

---

Reviewed with: cross-repo contract analysis, code architecture review, OWASP security audit, shadow environment testing (534 unit tests + 9-phase smoke test), and Python quality checks.