Code Review

Clean, well-structured PR. The mock primitives are well-designed with proper cleanup semantics, good error messages, and thorough test coverage. The conformance suite fixes are solid improvements.

Blocking Issues

None.

Suggestions

1. mock_fetch.ts:100 — Request constructor consumes init when input is already a Request: When input is a Request and init is also provided (e.g. fetch(existingRequest, { headers: ... })), the current code ignores init. This is a narrow edge case since the Request(input, init) constructor already merges them correctly, but worth noting that input instanceof Request ? input : new Request(input, init) skips the init merge for existing Request objects. Consider new Request(input, init) unconditionally, which the spec handles correctly for both cases.

2. mock_command.ts:113 — options?.args as string[] cast: The as string[] cast on options args bypasses type checking. Since the CommandOptions type already declares args?: string[], this cast is safe in practice, but the [key: string]: unknown index signature on the options type is what forces it. Minor — no action needed.

3. DDD perspective: The mock primitives sit cleanly in the testing infrastructure layer and don't leak into domain code. The with* pattern (resource acquisition + guaranteed cleanup) is the right choice for global state replacement. The conformance suite changes correctly tighten the behavioral contracts for locks and vaults, which strengthens the domain boundary enforcement for extension implementations.

Adversarial Review

Critical / High

None found.

Medium

1. packages/testing/mock_fetch.ts:100 — init silently dropped when input is a Request

   The mock does:

   const request = input instanceof Request ? input : new Request(input, init);

   The real fetch(request, init) API merges init overrides (headers, method, signal, etc.) into the request. The mock ignores init entirely when input is a Request, so tests using fetch(existingRequest, { headers: { "X-Extra": "val" } }) will silently lose the override. The captured CapturedFetchCall.headers will also be wrong.

   Breaking example: An extension does fetch(req, { signal }) and the test asserts on captured headers from the init — assertion passes in mock but the header was never on the request.

   Suggested fix:

   const request = input instanceof Request
     ? new Request(input, init)
     : new Request(input, init);

   (Or simply: const request = new Request(input, init); — the Request constructor already handles both overloads.)

2. Both mocks are unsafe under concurrent test execution

   withMockedFetch and withMockedCommand replace globals (globalThis.fetch, Deno.Command). Deno runs tests concurrently by default. If two tests use withMockedFetch simultaneously, Test B captures Test A's mock as originalFetch, then restores it after Test A already cleaned up — leaving a stale mock permanently installed.

   This is a well-known limitation of global-replacement mocking and may be acceptable for a testing library, but it should be documented (e.g., "use Deno.test(..., { sanitizeOps: false }) or run tests with --jobs=1 when multiple test files use these mocks").

Low

1. packages/testing/mock_command.ts:158 — outputSync() throws but callers may expect sync output

   The mock throws on outputSync(). If extension code under test uses outputSync() (reasonable for simple CLI wrappers), the mock will fail with a confusing error rather than returning mock data. This limits the mock's coverage but is clearly documented in the error message, so it's not a real bug — just a limitation to be aware of.

Verdict

PASS — The code is well-structured with proper cleanup in finally blocks, good error messages, and correct sequential/handler dispatch. The conformance test fixes (vault keys.includes(), datastore per-lock map, report Uint8Array clone, lock nonce assertions) are all genuine improvements. The fetch(Request, init) issue (Medium #1) is worth fixing but is not a blocker since it only affects an uncommon calling pattern.