Harden SRV resolver tests for restricted CI runners

.github/copilot-instructions.md

@@ -50,14 +50,22 @@ is missing. Report all blockers; do not mark the PR ready until the checks pass
 and the other requirements are satisfied. If there is no PR, stay on Case 1.
 
 ```bash
-npm run l10n            # only if a vscode.l10n.t() string was added, changed, or removed
+npm run l10n             # only if a vscode.l10n.t() string was added, changed, or removed
 npm run prettier-fix
+npm run l10n:check       # always; CI validates localization even when generation is skipped
 npm run lint
-npx vitest run          # full suite
 npm run build
-npm run package         # catches bundling and missing-asset failures
+npm run typecheck:tests  # strict package-test checks across all workspaces, as in CI
+npx vitest run           # full runtime suite; does not replace typecheck:tests
+npm run package          # catches bundling and missing-asset failures
 ```
 
+`npm run build` checks the root project and package source, but does not check package
+tests under their stricter `tsconfig.test.json` settings. Vitest runtime success (or
+an ad hoc `vitest --typecheck` invocation) is not a substitute for the exact
+`npm run typecheck:tests` script. Keep this list aligned with the
+[CI static-check job](workflows/main.yml), and report that script's result separately.
+
 Also confirm the AI pre-review (CONTRIBUTING.md §6) has run and its review file is
 committed under `docs/ai-and-plans/features/<name>/iterations/`.
 

CONTRIBUTING.md

@@ -248,22 +248,32 @@ Nothing else. `l10n`, `prettier-fix`, `lint`, and `package` each cost minutes an
 All of these must pass locally. The same checks run in CI, so catching failures here saves time.
 
 ```bash
-npm run l10n            # only if a vscode.l10n.t() string was added, changed, or removed
+npm run l10n             # only if a vscode.l10n.t() string was added, changed, or removed
 npm run prettier-fix
+npm run l10n:check       # always; CI validates localization even when generation is skipped
 npm run lint
-npx vitest run          # full suite
 npm run build
-npm run package         # catches bundling and missing-asset failures
+npm run typecheck:tests  # strict package-test checks across all workspaces, as in CI
+npx vitest run           # full runtime suite; does not replace typecheck:tests
+npm run package          # catches bundling and missing-asset failures
 ```
 
+`npm run build` checks the root project and package source, but does not check package
+tests under their stricter `tsconfig.test.json` settings. Vitest runtime success (or
+an ad hoc `vitest --typecheck` invocation) is not a substitute for the exact
+`npm run typecheck:tests` script. Keep this list aligned with the
+[CI static-check job](.github/workflows/main.yml), and report that script's result separately.
+
 Commit whatever these change, including regenerated files under `l10n/` and anything Prettier reformats.
 
 Case 2 is also where the AI pre-review in [§6](#6-ai-assisted-review-workflow) is due. Do not mark a PR ready for human review until that pass has run and its review file is committed under the feature's `iterations/` folder.
 
 ### 4.3. Notes on individual steps
 
 - **`npm run l10n`** regenerates the localization bundle from the strings passed to `vscode.l10n.t()`. **Never resolve a conflict in `l10n/bundle.l10n.json` by hand** — the file is generated. Take either side, or delete it, then re-run `npm run l10n` and commit the result.
+- **`npm run l10n:check`** validates localization against the source without regenerating files. Run it even when no strings changed, matching CI.
 - **`npm run prettier-fix`** covers Markdown as well as source, so documentation formatting does not drift whenever a file is touched. Fenced code blocks are left exactly as authored: they are often partial or illustrative, and reformatting them to the Markdown tab width would leave every example disagreeing with the code it documents.
+- **`npm run typecheck:tests`** checks every participating workspace's tests with its own TypeScript configuration, including strict checks that the root build and Vitest runtime do not cover.
 - **`npm run package`** catches Vite host and webview bundling issues and missing assets that unit tests alone will not surface.
 
 ## 5. Documenting Work with AI

docs/ai-and-plans/features/srv-dns-fallback/README.md

@@ -85,6 +85,20 @@ Increment 1 ([plan](./iterations/01-srv-fallback/plan.md), progress recorded per
 | `docs/user-manual/troubleshooting-srv-dns.md`                                                        | Docs                          | Symptoms, confirmation, workarounds, timing, trace, switches                                                                                                                                                                                              |
 | `src/services/localQuickStart/QuickStartService.ts`, `connectToClient.ts`                            | —                             | Out of scope (D0007); `connectToClient` is deprecated                                                                                                                                                                                                     |
 
+## Test isolation
+
+The resolver package tests do not query external DNS servers. Transport tests use UDP/TCP
+stubs bound to `127.0.0.1`; system DNS server discovery and driver resolver probes are mocked.
+Transport tests freeze both the clock and resolver timers, advancing time only for timeout
+scenarios, so slow loopback I/O cannot exhaust the resolver's time budget. TCP frame assembly
+is tested by splitting client data events into individual bytes, not by delaying server writes.
+Retry tests do not assume randomly generated query IDs differ. Stub startup cleans up partial
+binds and retries address-in-use conflicts up to three attempts; other bind failures remain errors.
+The UDP refusal tests inject `ECONNREFUSED` and `ECONNRESET` without sending a datagram, and
+check immediate rejection and timer cleanup without advancing fake timers. They do not rely on
+closed-port ICMP errors, which may be suppressed on Windows or restricted runners.
+Loopback sockets must still be permitted.
+
 ## Timeline
 
 | Date       | PR   | What changed                                                                                                   | Docs                                                                                                                           |

packages/documentdb-js-srv-resolver/src/fallbackDnsClient.test.ts

@@ -3,11 +3,12 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { afterEach, describe, expect, it, vi } from 'vitest';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 
 import * as dnsPacket from 'dns-packet';
 import dgram from 'node:dgram';
 import dns from 'node:dns';
+import net from 'node:net';
 
 import { FallbackDnsError, resolveSrvFallback, resolveTxtFallback } from './fallbackDnsClient.js';
 import { DnsStubServer } from './test/dnsStubServer.js';
@@ -72,11 +73,22 @@ function compressedReply(query: dnsPacket.Packet, targets = ['fc-abc-000']): Buf
 describe('fallback DNS client', (): void => {
     const stubs: DnsStubServer[] = [];
 
+    beforeEach((): void => {
+        vi.useFakeTimers({ toFake: ['Date', 'setTimeout', 'clearTimeout'] });
+        vi.spyOn(dns, 'getServers').mockImplementation((): string[] => {
+            throw new Error('Tests must explicitly supply loopback DNS servers');
+        });
+    });
+
     afterEach(async (): Promise<void> => {
-        vi.useRealTimers();
-        await Promise.all(stubs.map((stub): Promise<void> => stub.close()));
-        stubs.length = 0;
-        vi.restoreAllMocks();
+        try {
+            await Promise.all(stubs.map((stub): Promise<void> => stub.close()));
+            expect(vi.getTimerCount()).toBe(0);
+        } finally {
+            stubs.length = 0;
+            vi.restoreAllMocks();
+            vi.useRealTimers();
+        }
     });
 
     async function stub(handler: (request: StubRequest) => void): Promise<DnsStubServer> {
@@ -119,7 +131,7 @@ describe('fallback DNS client', (): void => {
         );
     });
 
-    it('ignores replies that do not match the query or come from another source', async (): Promise<void> => {
+    it('ignores malformed replies and replies that do not match the query', async (): Promise<void> => {
         const server = await stub((request): void => {
             const wrongId = reply(request);
             wrongId.writeUInt16BE(((request.query.id ?? 0) + 1) % 65536, 0);
@@ -133,7 +145,7 @@ describe('fallback DNS client', (): void => {
                 }),
             );
             request.send(Buffer.from([0xff, 0xff, 0xff]));
-            server.later((): void => request.send(reply(request)), 10);
+            request.send(reply(request));
         });
         const result = await resolveSrvFallback(srvName, options([server]));
         expect(result.records.map((record): string => record.name)).toEqual([targetName]);
@@ -144,13 +156,33 @@ describe('fallback DNS client', (): void => {
             if (request.transport === 'udp') {
                 request.send(reply(request, [], dnsPacket.TRUNCATED_RESPONSE));
             } else {
-                request.send(compressedReply(request.query), true);
+                request.send(compressedReply(request.query));
             }
         });
+        const emit: (this: net.Socket, event: string | symbol, ...args: unknown[]) => boolean =
+            net.Socket.prototype.emit;
+        let fragments = 0;
+        // TCP can coalesce writes, so split the client's data events rather than timing server writes.
+        vi.spyOn(net.Socket.prototype, 'emit').mockImplementation(function (
+            this: net.Socket,
+            event: string | symbol,
+            ...args: unknown[]
+        ): boolean {
+            if (event === 'data' && `127.0.0.1:${this.remotePort}` === server.address && Buffer.isBuffer(args[0])) {
+                const bytes = args[0];
+                for (let offset = 0; offset < bytes.length; offset++) {
+                    fragments++;
+                    emit.call(this, event, bytes.subarray(offset, offset + 1));
+                }
+                return true;
+            }
+            return emit.call(this, event, ...args) === true;
+        });
         const result = await resolveSrvFallback(srvName, options([server]));
         expect(result).toMatchObject({ transport: 'tcp', serversTried: 1 });
         expect(result.records[0].name).toBe(targetName);
         expect(server.queries.map((request): string => request.transport)).toEqual(['udp', 'tcp']);
+        expect(fragments).toBeGreaterThan(2);
     });
 
     it.each([
@@ -180,7 +212,6 @@ describe('fallback DNS client', (): void => {
     });
 
     it('tries the next server after a silent server times out', async (): Promise<void> => {
-        vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] });
         const first = await stub((): void => {
             vi.advanceTimersByTime(25);
         });
@@ -190,7 +221,6 @@ describe('fallback DNS client', (): void => {
     });
 
     it('retries a server once after a UDP timeout', async (): Promise<void> => {
-        vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] });
         const server = await stub((request): void => {
             if (server.queries.length === 1) {
                 vi.advanceTimersByTime(25);
@@ -202,7 +232,6 @@ describe('fallback DNS client', (): void => {
 
         expect(result.serversTried).toBe(1);
         expect(server.queries).toHaveLength(2);
-        expect(server.queries[0].query.id).not.toBe(server.queries[1].query.id);
     });
 
     it('does not retry a server after SERVFAIL', async (): Promise<void> => {
@@ -226,7 +255,6 @@ describe('fallback DNS client', (): void => {
     });
 
     it('bounds all servers by one shared deadline', async (): Promise<void> => {
-        vi.useFakeTimers({ toFake: ['Date', 'setTimeout', 'clearTimeout'] });
         const first = await stub((): void => {
             vi.advanceTimersByTime(200);
         });
@@ -272,23 +300,25 @@ describe('fallback DNS client', (): void => {
         await expect(outcome).rejects.toMatchObject({ code: dns.CONNREFUSED, syscall: 'queryTxt', serversTried: 0 });
     });
 
-    it('fails fast when a UDP server refuses the query', async (): Promise<void> => {
-        const port = await new Promise<number>((resolve, reject): void => {
-            const socket = dgram.createSocket('udp4');
-            socket.once('error', reject);
-            socket.bind(0, '127.0.0.1', (): void => {
-                const address = socket.address();
-                socket.close((): void => resolve(address.port));
+    it.each(['ECONNREFUSED', 'ECONNRESET'])('fails fast when a UDP socket reports %s', async (code): Promise<void> => {
+        const connect = vi.spyOn(dgram.Socket.prototype, 'connect');
+        const close = vi.spyOn(dgram.Socket.prototype, 'close');
+        // Closed-port ICMP errors are not reliably delivered on every OS or restricted runner.
+        const send = vi.spyOn(dgram.Socket.prototype, 'send').mockImplementation(function (this: dgram.Socket): void {
+            queueMicrotask((): void => {
+                this.emit('error', Object.assign(new Error(code), { code }));
             });
         });
-        const started = Date.now();
         const outcome = resolveSrvFallback(srvName, {
-            servers: [`127.0.0.1:${port}`],
-            deadlineAt: started + 3000,
+            servers: ['127.0.0.1:53'],
+            deadlineAt: Date.now() + 3000,
             udpTimeoutMs: 2000,
         });
 
         await expect(outcome).rejects.toMatchObject({ code: dns.CONNREFUSED, serversTried: 1 });
-        expect(Date.now() - started).toBeLessThan(1000);
+        expect(connect).toHaveBeenCalledExactlyOnceWith(53, '127.0.0.1', expect.any(Function));
+        expect(send).toHaveBeenCalledTimes(1);
+        expect(close).toHaveBeenCalledTimes(1);
+        expect(vi.getTimerCount()).toBe(0);
     });
 });

packages/documentdb-js-srv-resolver/src/resolveSrvForDriver.test.ts

@@ -164,17 +164,20 @@ describe('resolveSrvForDriver', (): void => {
     it('cancels a pending probe and rejects with the abort reason', async (): Promise<void> => {
         const controller = new AbortController();
         let rejectProbe!: (error: unknown) => void;
-        probe.mockImplementation(
-            (): Promise<never> =>
-                new Promise((_resolve, reject): void => {
-                    rejectProbe = reject;
-                }),
-        );
+        const probeStarted = new Promise<void>((resolve): void => {
+            probe.mockImplementation(
+                (): Promise<never> =>
+                    new Promise((_resolve, reject): void => {
+                        rejectProbe = reject;
+                        resolve();
+                    }),
+            );
+        });
         const cancel = vi
             .spyOn(dns.promises.Resolver.prototype, 'cancel')
             .mockImplementation((): void => rejectProbe(probeError('ECANCELLED')));
         const result = run({ signal: controller.signal });
-        await Promise.resolve();
+        await probeStarted;
         const reason = new Error('stop');
         controller.abort(reason);
         await expect(result).rejects.toBe(reason);

packages/documentdb-js-srv-resolver/src/srvUriRewrite.differential.test.ts

@@ -3,7 +3,7 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { afterEach, describe, expect, it, vi } from 'vitest';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 
 import { MongoClient } from 'mongodb';
 import * as dns from 'node:dns';
@@ -167,6 +167,12 @@ function rewriteOutcome(fixture: Fixture): Outcome {
 }
 
 describe('SRV rewrite differential against the installed MongoDB API driver', (): void => {
+    beforeEach((): void => {
+        const error = new Error('Driver DNS queries must use the mocked resolve function');
+        vi.spyOn(dns.promises, 'resolveSrv').mockRejectedValue(error);
+        vi.spyOn(dns.promises, 'resolveTxt').mockRejectedValue(error);
+    });
+
     afterEach((): void => {
         vi.restoreAllMocks();
     });
@@ -178,10 +184,13 @@ describe('SRV rewrite differential against the installed MongoDB API driver', ()
                 if (rrtype === 'SRV') {
                     return srvRecords.map((record): SrvRecord => ({ ...record }));
                 }
-                if (fixture.txtRecords === undefined) {
-                    throw Object.assign(new Error('queryTxt ENODATA'), { code: 'ENODATA' });
+                if (rrtype === 'TXT') {
+                    if (fixture.txtRecords === undefined) {
+                        throw Object.assign(new Error('queryTxt ENODATA'), { code: 'ENODATA' });
+                    }
+                    return fixture.txtRecords.map((record): string[] => [...record]);
                 }
-                return fixture.txtRecords.map((record): string[] => [...record]);
+                throw new Error(`Unexpected DNS record type: ${rrtype}`);
             },
         );
         const expected = await driverOutcome(fixture);