From fd584d44959085c8bd0bf30f0f5dcef7b14aec39 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Tue, 10 Nov 2026 16:16:28 -0500 Subject: [PATCH] refactor: remove test-only planning and native UI runtime seams --- .../ProcessScenarioPreparation.test.ts | 45 ++++++++++--------- .../process/ProcessScenarioPreparation.ts | 11 +---- src/native/NativeUiHelperRuntime.ts | 3 +- src/native/NativeUiObservation.test.ts | 6 +++ src/native/NativeUiObservation.ts | 2 +- tests/boundary/mcp/nativeUiFactsMcp.test.ts | 3 +- .../mcp/nativeUiFinalizationMcp.test.ts | 1 + .../native/nativeUiFinalization.test.ts | 1 + 8 files changed, 38 insertions(+), 34 deletions(-) diff --git a/src/application/process/ProcessScenarioPreparation.test.ts b/src/application/process/ProcessScenarioPreparation.test.ts index 697ca28f5f..05d1ec3740 100644 --- a/src/application/process/ProcessScenarioPreparation.test.ts +++ b/src/application/process/ProcessScenarioPreparation.test.ts @@ -1,6 +1,7 @@ import { writeFile } from "node:fs/promises"; import { join } from "node:path"; -import { expect, it } from "vitest"; +import { afterEach, expect, it, vi } from "vitest"; +import * as filesystemSnapshot from "../../process/capture/FilesystemSnapshot.js"; import { awaitAcknowledgement } from "../../../tests/fixtures/lifecycleAcknowledgement.js"; import { createTestTempDirectory } from "../../../tests/fixtures/temporaryDirectory.js"; @@ -10,6 +11,8 @@ import { err, ok, type Result } from "../../domain/result.js"; import { ProcessCaptureError } from "../../process/capture/ProcessCaptureError.js"; import { prepareProcessScenarioPlan } from "./ProcessScenarioPreparation.js"; +afterEach(() => vi.restoreAllMocks()); + it("rejects observation roots that capture admission rejects", async () => { const root = await createTestTempDirectory("rea-process-plan-"); const file = join(root, "selected"); @@ -40,23 +43,24 @@ it("reports cancellation normalized inside admission as planning cancellation", working_directory: ".", }); const controller = new AbortController(); + vi.spyOn( + filesystemSnapshot, + "admitFilesystemObservationRoots", + ).mockImplementationOnce(() => { + controller.abort("caller stopped during admission"); + return Promise.resolve( + err( + new ProcessCaptureError("filesystem observation admission failed", { + cause: controller.signal.reason, + }), + ), + ); + }); const prepared = await prepareProcessScenarioPlan( scenario, {}, process.platform, controller.signal, - { - admitRoots: () => { - controller.abort("caller stopped during admission"); - return Promise.resolve( - err( - new ProcessCaptureError("filesystem observation admission failed", { - cause: controller.signal.reason, - }), - ), - ); - }, - }, ); expect(prepared.ok).toBe(false); @@ -85,18 +89,19 @@ it("reports cancellation after the last awaited admission step", async () => { ); let admissionCalls = 0; + vi.spyOn( + filesystemSnapshot, + "admitFilesystemObservationRoots", + ).mockImplementationOnce(() => { + admissionCalls += 1; + admissionEntered(); + return pendingAdmission; + }); const planning = prepareProcessScenarioPlan( scenario, {}, process.platform, controller.signal, - { - admitRoots: () => { - admissionCalls += 1; - admissionEntered(); - return pendingAdmission; - }, - }, ); // Abort only once admission is actually running. Aborting earlier would be diff --git a/src/application/process/ProcessScenarioPreparation.ts b/src/application/process/ProcessScenarioPreparation.ts index 5103f2ece2..d2bdb6f9ca 100644 --- a/src/application/process/ProcessScenarioPreparation.ts +++ b/src/application/process/ProcessScenarioPreparation.ts @@ -14,11 +14,6 @@ import { err, ok, type Result } from "../../domain/result.js"; /** A scenario whose executable and observation paths are resolved for execution. */ export type ResolvedProcessScenario = Omit; -/** Test seam for holding the final admission step while cancellation arrives. */ -export interface ProcessScenarioPreparationHost { - readonly admitRoots?: typeof admitFilesystemObservationRoots; -} - /** * Resolve a scenario for a dry run. This applies the same executable, working * directory, path and platform admission checks as a capture, then stops @@ -29,7 +24,6 @@ export const prepareProcessScenarioPlan = async ( environment: Readonly> = process.env, platform: NodeJS.Platform = process.platform, signal?: AbortSignal, - host: ProcessScenarioPreparationHost = {}, ): Promise> => { if (signal?.aborted === true) return err( @@ -60,10 +54,7 @@ export const prepareProcessScenarioPlan = async ( if (aborted()) return cancelled(); const privacy = createProcessSensitiveOutput(scenario); if (!privacy.ok) return err(privacy.error); - const admitted = await (host.admitRoots ?? admitFilesystemObservationRoots)( - resolved, - signal, - ); + const admitted = await admitFilesystemObservationRoots(resolved, signal); if (!admitted.ok) return aborted() ? cancelled() : admitted; if (aborted()) return cancelled(); return ok(resolved); diff --git a/src/native/NativeUiHelperRuntime.ts b/src/native/NativeUiHelperRuntime.ts index 07a71c418c..2141c534b4 100644 --- a/src/native/NativeUiHelperRuntime.ts +++ b/src/native/NativeUiHelperRuntime.ts @@ -16,8 +16,7 @@ export interface NativeUiHelperRuntime { readonly invoke: NativeUiHelper; readonly cleanupLease: AnalysisCleanupLease; readonly signal: AbortSignal; - /** Omitted by test doubles; production runtimes always set the selected deadline. */ - readonly deadlineMs?: number; + readonly deadlineMs: number; close(): Promise; resources(): readonly string[]; } diff --git a/src/native/NativeUiObservation.test.ts b/src/native/NativeUiObservation.test.ts index db38eac62e..1390ce9b97 100644 --- a/src/native/NativeUiObservation.test.ts +++ b/src/native/NativeUiObservation.test.ts @@ -374,6 +374,7 @@ describe("native UI deadline execution", () => { environment: {}, runtime: { signal: deadline.signal, + deadlineMs: 180_000, invoke: async () => ({ ok: true, result: snapshot }), cleanupLease: new AnalysisCleanupLease(async () => undefined), close: async () => undefined, @@ -396,6 +397,7 @@ describe("native UI deadline execution", () => { environment: {}, runtime: { signal: deadline.signal, + deadlineMs: 180_000, invoke: async () => ({ ok: true, result: snapshot }), cleanupLease: new AnalysisCleanupLease(async () => undefined), close: async () => undefined, @@ -423,6 +425,7 @@ describe("native UI deadline execution", () => { environment: {}, runtime: { signal: deadline.signal, + deadlineMs: 180_000, invoke: async () => ({ ok: true, result: snapshot }), cleanupLease: new AnalysisCleanupLease(async () => undefined), close: async () => undefined, @@ -493,6 +496,7 @@ describe("native UI interruption provenance", () => { environment: {}, runtime: { signal: deadline.signal, + deadlineMs: 180_000, invoke: async () => ({ ok: true, result: snapshot }), cleanupLease: new AnalysisCleanupLease(async () => undefined), close: async () => undefined, @@ -531,6 +535,7 @@ describe("native UI interruption provenance", () => { environment: {}, runtime: { signal: deadline.signal, + deadlineMs: 180_000, invoke: async () => ({ ok: true, result: snapshot }), cleanupLease: new AnalysisCleanupLease(async () => undefined), close: async () => undefined, @@ -552,6 +557,7 @@ describe("native UI interruption provenance", () => { environment: {}, runtime: { signal: deadline.signal, + deadlineMs: 180_000, invoke: async () => ({ ok: true, result: snapshot }), cleanupLease: new AnalysisCleanupLease(async () => undefined), close: async () => undefined, diff --git a/src/native/NativeUiObservation.ts b/src/native/NativeUiObservation.ts index 7067201413..fec0df544c 100644 --- a/src/native/NativeUiObservation.ts +++ b/src/native/NativeUiObservation.ts @@ -82,7 +82,7 @@ export const observeNativeUi = async ( new NativeUiInterruptionControl( options.signal, runtime.signal, - runtime.deadlineMs ?? DEFAULT_NATIVE_UI_DEADLINE_MS, + runtime.deadlineMs, ); const ownsInterruption = options.interruption === undefined; let result: Result, AnalysisError>; diff --git a/tests/boundary/mcp/nativeUiFactsMcp.test.ts b/tests/boundary/mcp/nativeUiFactsMcp.test.ts index 2e32f12100..aee838457b 100644 --- a/tests/boundary/mcp/nativeUiFactsMcp.test.ts +++ b/tests/boundary/mcp/nativeUiFactsMcp.test.ts @@ -32,10 +32,11 @@ const connectNativeUi = async () => { const replies: unknown[] = []; const factory = vi .spyOn(runtimes, "createNativeUiHelperRuntime") - .mockImplementation(() => { + .mockImplementation((_environment, deadlineMs = 180_000) => { const cleanupLease = new AnalysisCleanupLease(async () => {}); return { signal: new AbortController().signal, + deadlineMs, cleanupLease, invoke: async () => { if (replies.length === 0) diff --git a/tests/boundary/mcp/nativeUiFinalizationMcp.test.ts b/tests/boundary/mcp/nativeUiFinalizationMcp.test.ts index 55583a411f..d1bfee117f 100644 --- a/tests/boundary/mcp/nativeUiFinalizationMcp.test.ts +++ b/tests/boundary/mcp/nativeUiFinalizationMcp.test.ts @@ -42,6 +42,7 @@ it.each([false, true])( }); const runtime = { signal: deadline.signal, + deadlineMs: 180_000, cleanupLease, invoke: async () => ({ ok: true, result: snapshot }), close: () => cleanupLease.release(), diff --git a/tests/boundary/providers/native/nativeUiFinalization.test.ts b/tests/boundary/providers/native/nativeUiFinalization.test.ts index 9ead2d933c..f642a18c8f 100644 --- a/tests/boundary/providers/native/nativeUiFinalization.test.ts +++ b/tests/boundary/providers/native/nativeUiFinalization.test.ts @@ -39,6 +39,7 @@ const controlledRuntime = ( return { cleanupLease, signal, + deadlineMs: 180_000, invoke: vi.fn(async () => ({ ok: true, result: snapshot })), close: vi.fn(() => cleanupLease.release()), resources: () => (cleanupLease.released ? [] : ["controlled-ui-runtime"]),