Summary
cli/test/unit/cli.test.ts mocks the MCP client but not probeEndpoint, so npm test makes a real network request to https://learn.microsoft.com/api/mcp. The suite passes when that request succeeds and fails when it does not, which makes the test result depend on network conditions rather than on the code under test.
Root cause
runDoctorChecks in cli/src/commands/doctor.ts calls two things:
const reachability = await probeEndpoint(endpoint); // real fetch, not injectable via CliContext
const client = context.createClient({ endpoint }); // injectable, mocked in tests
probeEndpoint (cli/src/mcp/client.ts:48) accepts a fetchImpl parameter, but doctor.ts calls it with one argument, so it always uses globalThis.fetch against DEFAULT_ENDPOINT (https://learn.microsoft.com/api/mcp, cli/src/utils/contracts.ts:1). CliContext exposes createClient but no seam for probeEndpoint, so tests cannot substitute it.
report.ok requires reachability.ok, so a failed probe turns the doctor exit code into 1.
Reproduction
With network access, all tests pass:
Test Files 5 passed (5)
Tests 32 passed (32)
With the network removed (unshare -rn), the same commit fails:
❯ test/unit/cli.test.ts:144:22
142| const exitCode = await runCli(['node', 'mslearn', 'doctor'], conte…
144| expect(exitCode).toBe(0);
- 0
+ 1
Test Files 1 failed | 4 passed (5)
Tests 1 failed | 31 passed (32)
The failing case is "forces a fresh connection check in doctor instead of using cached tool mappings" (cli/test/unit/cli.test.ts:132). It asserts on getToolMapping call behaviour, which is fully mocked; the live endpoint probe is incidental to what the test is verifying.
Why this matters
Observed on the Copilot coding agent run for #174 (https://github.com/MicrosoftDocs/mcp/actions/runs/33598478336/job/100146752501), where the agent firewall blocks DNS for learn.microsoft.com:
⚠️ Warning: I tried to connect to the following addresses, but was blocked by firewall rules:
- `learn.microsoft.com.`
Triggering Command: ... vitest/dist/workers/forks.js (dns block)
Two consequences:
- Unit tests are not hermetic. A Learn endpoint outage, a proxy, or an offline developer produces a red suite for reasons unrelated to the change under review.
- Any agent or sandboxed environment with egress restrictions gets a spurious failure on lockfile-only dependency bumps.
Suggested fix
Add a probeEndpoint seam to CliContext alongside createClient, defaulting to the real implementation and overridden in createTestContext. probeEndpoint already takes an injectable fetchImpl, so the change is limited to threading it through the context rather than reworking the probe itself.
If a test that exercises real endpoint reachability is wanted, it belongs in a separate integration suite that is not part of npm test.
Summary
cli/test/unit/cli.test.tsmocks the MCP client but notprobeEndpoint, sonpm testmakes a real network request tohttps://learn.microsoft.com/api/mcp. The suite passes when that request succeeds and fails when it does not, which makes the test result depend on network conditions rather than on the code under test.Root cause
runDoctorChecksincli/src/commands/doctor.tscalls two things:probeEndpoint(cli/src/mcp/client.ts:48) accepts afetchImplparameter, butdoctor.tscalls it with one argument, so it always usesglobalThis.fetchagainstDEFAULT_ENDPOINT(https://learn.microsoft.com/api/mcp, cli/src/utils/contracts.ts:1).CliContextexposescreateClientbut no seam forprobeEndpoint, so tests cannot substitute it.report.okrequiresreachability.ok, so a failed probe turns the doctor exit code into 1.Reproduction
With network access, all tests pass:
With the network removed (
unshare -rn), the same commit fails:The failing case is "forces a fresh connection check in doctor instead of using cached tool mappings" (cli/test/unit/cli.test.ts:132). It asserts on
getToolMappingcall behaviour, which is fully mocked; the live endpoint probe is incidental to what the test is verifying.Why this matters
Observed on the Copilot coding agent run for #174 (https://github.com/MicrosoftDocs/mcp/actions/runs/33598478336/job/100146752501), where the agent firewall blocks DNS for
learn.microsoft.com:Two consequences:
Suggested fix
Add a
probeEndpointseam toCliContextalongsidecreateClient, defaulting to the real implementation and overridden increateTestContext.probeEndpointalready takes an injectablefetchImpl, so the change is limited to threading it through the context rather than reworking the probe itself.If a test that exercises real endpoint reachability is wanted, it belongs in a separate integration suite that is not part of
npm test.