refactor(desktop): put browser cookie import on Effect services

The import module reached for `node:fs/promises`, `node:os`, `node:path` and
`node:sqlite` directly and threaded results through hand-rolled promises, which
meant a blanket `nodeBuiltinImport:off` on two files and failures that were
plain thrown `Error`s.

It now uses FileSystem, Path and the shared Effect SQL client, and the cookie
snapshot is a scoped resource rather than a try/finally with a cleanup
callback. `node:crypto` stays — it implements the OSCrypt primitives Chromium
uses and has no Effect equivalent — with the suppression narrowed to it and a
reason attached.

Failure reasons are a typed union in contracts rather than free strings, so the
renderer maps a reason to copy instead of substring-matching an error message.

Porting `isSourceRunning` to `FileSystem.stat` reintroduced the dangling
SingletonLock bug, because `stat` and `exists` both follow symlinks; `readLink`
is the probe that answers for the entry itself. The regression test caught it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: juliusmarminge

apps/desktop/src/preview/BrowserImport/BrowserImport.test.ts

@@ -1,21 +1,23 @@
-// @effect-diagnostics nodeBuiltinImport:off - Builds the on-disk browser layout the import reads.
+import * as NodeServices from "@effect/platform-node/NodeServices";
 import { assert, describe, it } from "@effect/vitest";
+import {
+  HostProcessEnvironment,
+  HostProcessExecutablePath,
+  HostProcessPlatform,
+} from "@t3tools/shared/hostProcess";
 import * as Effect from "effect/Effect";
+import * as FileSystem from "effect/FileSystem";
 import * as Layer from "effect/Layer";
-import * as NodeFSP from "node:fs/promises";
-import * as NodeOS from "node:os";
-import * as NodePath from "node:path";
 
-import { HostProcessExecutablePath, HostProcessPlatform } from "@t3tools/shared/hostProcess";
 import * as BrowserSession from "../BrowserSession.ts";
 import * as BrowserImport from "./BrowserImport.ts";
-import { BROWSER_IMPORT_SOURCES } from "./Sources.ts";
+import { BROWSER_IMPORT_SOURCES, sourcePaths } from "./Sources.ts";
 
 const helium = BROWSER_IMPORT_SOURCES.find((source) => source.id === "helium")!;
 
 /**
- * Fails loudly if the import ever reaches session work: every test here covers
- * a request that must be rejected before a cookie is read or written.
+ * Dies if the import reaches session work: every case here covers a request
+ * that must be rejected before a cookie is read or written.
  */
 const rejectedBeforeSession = Layer.succeed(BrowserSession.BrowserSession, {
   derivePartition: () => Effect.die("derivePartition must not be reached"),
@@ -24,41 +26,46 @@ const rejectedBeforeSession = Layer.succeed(BrowserSession.BrowserSession, {
   clearCache: () => Effect.die("clearCache must not be reached"),
 } as unknown as BrowserSession.BrowserSession["Service"]);
 
-const layer = BrowserImport.layer.pipe(
-  Layer.provide(rejectedBeforeSession),
-  Layer.provide(Layer.succeed(HostProcessPlatform, "darwin")),
-  Layer.provide(Layer.succeed(HostProcessExecutablePath, "/Applications/T3 Code.app")),
-);
-
-const withScratchHome = Effect.fnUntraced(function* () {
-  const realHome = process.env.HOME;
-  const home = yield* Effect.acquireRelease(
-    Effect.promise(() => NodeFSP.mkdtemp(NodePath.join(NodeOS.tmpdir(), "t3code-import-"))),
-    (dir) =>
-      Effect.promise(async () => {
-        if (realHome === undefined) delete process.env.HOME;
-        else process.env.HOME = realHome;
-        await NodeFSP.rm(dir, { recursive: true, force: true });
-      }),
+/**
+ * Builds the service against a scratch home containing an installed, closed
+ * copy of the source browser.
+ */
+const withImporter = Effect.fnUntraced(function* () {
+  const fileSystem = yield* FileSystem.FileSystem;
+  const home = yield* fileSystem.makeTempDirectoryScoped({ prefix: "t3code-import-" });
+  const environment = Layer.succeed(HostProcessEnvironment, { HOME: home });
+  const paths = yield* sourcePaths.pipe(
+    Effect.provideService(HostProcessEnvironment, { HOME: home }),
   );
-  process.env.HOME = home;
-  yield* Effect.promise(() =>
-    NodeFSP.mkdir(NodePath.join(helium.userDataDirectory(), "Default"), { recursive: true }),
+  yield* fileSystem.makeDirectory(`${helium.userDataDirectory(paths)}/Default`, {
+    recursive: true,
+  });
+
+  const importer = yield* BrowserImport.BrowserImport.pipe(
+    Effect.provide(
+      BrowserImport.layer.pipe(
+        Layer.provide(rejectedBeforeSession),
+        Layer.provide(environment),
+        Layer.provide(Layer.succeed(HostProcessPlatform, "darwin")),
+        Layer.provide(Layer.succeed(HostProcessExecutablePath, "/Applications/T3 Code.app")),
+        Layer.provide(NodeServices.layer),
+      ),
+    ),
   );
-  return home;
+  return { importer, home, paths };
 });
 
 describe("BrowserImport.importCookies", () => {
-  it.effect("rejects a profile directory the source never reported", () =>
+  it.effect("rejects a source profile the browser never reported", () =>
     Effect.gen(function* () {
-      const home = yield* withScratchHome();
-      // A cookie database that is reachable on disk but outside the browser's
+      const fileSystem = yield* FileSystem.FileSystem;
+      const { importer, home } = yield* withImporter();
+
+      // A cookie database reachable on disk but outside the browser's
       // user-data directory — the payoff a traversal would be after.
-      const secrets = NodePath.join(home, "secrets");
-      yield* Effect.promise(() => NodeFSP.mkdir(secrets, { recursive: true }));
-      yield* Effect.promise(() => NodeFSP.writeFile(NodePath.join(secrets, "Cookies"), "not-a-db"));
+      yield* fileSystem.makeDirectory(`${home}/secrets`, { recursive: true });
+      yield* fileSystem.writeFileString(`${home}/secrets/Cookies`, "not-a-db");
 
-      const importer = yield* BrowserImport.BrowserImport;
       const error = yield* importer
         .importCookies({
           input: {
@@ -73,6 +80,33 @@ describe("BrowserImport.importCookies", () => {
 
       assert.instanceOf(error, BrowserImport.BrowserImportFailedError);
       assert.equal(error.reason, "unknownSourceProfile");
-    }).pipe(Effect.provide(layer), Effect.scoped),
+    }).pipe(Effect.provide(NodeServices.layer), Effect.scoped),
+  );
+
+  it.effect("refuses to import while the source browser holds its profile", () =>
+    Effect.gen(function* () {
+      const fileSystem = yield* FileSystem.FileSystem;
+      const { importer, paths } = yield* withImporter();
+      // The lock Chromium leaves while it is running, dangling target and
+      // all. This must stop the import before it ever asks the keychain.
+      yield* fileSystem.symlink(
+        "host-that-does-not-exist-1234",
+        `${helium.userDataDirectory(paths)}/SingletonLock`,
+      );
+
+      const error = yield* importer
+        .importCookies({
+          input: {
+            sourceId: "helium",
+            sourceProfileDirectory: "Default",
+            targetProfileId: "default",
+          },
+          scope: "persist:t3code-preview-test",
+          persistent: true,
+        })
+        .pipe(Effect.flip);
+
+      assert.equal(error.reason, "browserRunning");
+    }).pipe(Effect.provide(NodeServices.layer), Effect.scoped),
   );
 });

apps/desktop/src/preview/BrowserImport/BrowserImport.ts

@@ -10,35 +10,40 @@ import type {
   BrowserImportSource,
   BrowserImportUnavailableReason,
 } from "@t3tools/contracts";
+import { BrowserImportFailureReason } from "@t3tools/contracts";
 import * as Context from "effect/Context";
 import * as Effect from "effect/Effect";
+import * as FileSystem from "effect/FileSystem";
 import * as Layer from "effect/Layer";
+import * as Path from "effect/Path";
 import * as Schema from "effect/Schema";
 
 import { HostProcessExecutablePath, HostProcessPlatform } from "@t3tools/shared/hostProcess";
 
 import * as BrowserSession from "../BrowserSession.ts";
-import {
-  ChromiumCookieReadError,
-  readChromiumCookies,
-  type ChromiumCookie,
-} from "./ChromiumCookies.ts";
+import { readChromiumCookies } from "./ChromiumCookies.ts";
 import {
   BROWSER_IMPORT_SOURCES,
   cookieDatabasePath,
   isSourceInstalled,
   isSourceRunning,
   listSourceProfiles,
+  sourcePaths,
   type BrowserImportSourceDefinition,
+  type SourcePaths,
 } from "./Sources.ts";
 
 export class BrowserImportFailedError extends Schema.TaggedErrorClass<BrowserImportFailedError>()(
   "BrowserImportFailedError",
   {
     sourceId: Schema.String,
-    reason: Schema.String,
+    reason: BrowserImportFailureReason,
+    /** Kept for the log; the user only ever sees the reason's copy. */
+    cause: Schema.optional(Schema.Defect()),
   },
 ) {
+  // The reason token is part of the message on purpose: IPC flattens the error
+  // to its message, and the renderer maps that token back to user-facing copy.
   override get message(): string {
     return `Importing cookies from ${this.sourceId} failed: ${this.reason}.`;
   }
@@ -57,36 +62,40 @@ export class BrowserImport extends Context.Service<
   }
 >()("@t3tools/desktop/preview/BrowserImport/BrowserImport") {}
 
-const unavailableReason = async (
+const unavailableReason = Effect.fn("BrowserImport.unavailableReason")(function* (
   definition: BrowserImportSourceDefinition,
   platform: NodeJS.Platform,
-): Promise<BrowserImportUnavailableReason | undefined> => {
+  paths: SourcePaths,
+): Effect.fn.Return<BrowserImportUnavailableReason | undefined, never, FileSystem.FileSystem> {
   if (!definition.platforms.includes(platform)) return "unsupportedPlatform";
-  if (!(await isSourceInstalled(definition))) return "notInstalled";
-  if (await isSourceRunning(definition)) return "browserRunning";
+  if (!(yield* isSourceInstalled(definition, paths))) return "notInstalled";
+  if (yield* isSourceRunning(definition, paths)) return "browserRunning";
   return undefined;
-};
+});
 
 export const make = Effect.gen(function* BrowserImportMake() {
   const browserSession = yield* BrowserSession.BrowserSession;
   const platform = yield* HostProcessPlatform;
   const executablePath = yield* HostProcessExecutablePath;
-
-  const listSources = Effect.promise(async (): Promise<ReadonlyArray<BrowserImportSource>> => {
-    const sources: BrowserImportSource[] = [];
-    for (const definition of BROWSER_IMPORT_SOURCES) {
-      const unavailable = await unavailableReason(definition, platform);
-      sources.push({
+  // Captured here so the service's methods stay free of a requirements
+  // channel: the layer is built where NodeServices is already in scope.
+  const platformServices = yield* Effect.context<FileSystem.FileSystem | Path.Path>();
+  const paths = yield* sourcePaths;
+
+  const listSources: Effect.Effect<ReadonlyArray<BrowserImportSource>> = Effect.forEach(
+    BROWSER_IMPORT_SOURCES,
+    Effect.fnUntraced(function* (definition) {
+      const unavailable = yield* unavailableReason(definition, platform, paths);
+      return {
         id: definition.id,
         name: definition.name,
         // Listing profiles touches the source's own files, so skip it when the
         // source is unusable anyway.
-        profiles: unavailable === undefined ? await listSourceProfiles(definition) : [],
+        profiles: unavailable === undefined ? yield* listSourceProfiles(definition, paths) : [],
         ...(unavailable === undefined ? {} : { unavailable }),
-      });
-    }
-    return sources;
-  });
+      } satisfies BrowserImportSource;
+    }),
+  ).pipe(Effect.provide(platformServices));
 
   const importCookies = Effect.fn("BrowserImport.importCookies")(function* (input: {
     readonly input: BrowserImportInput;
@@ -99,11 +108,13 @@ export const make = Effect.gen(function* BrowserImportMake() {
     if (!definition) {
       return yield* new BrowserImportFailedError({
         sourceId: input.input.sourceId,
-        reason: "unknown source",
+        reason: "unknownSource",
       });
     }
 
-    const blocked = yield* Effect.promise(() => unavailableReason(definition, platform));
+    const blocked = yield* unavailableReason(definition, platform, paths).pipe(
+      Effect.provide(platformServices),
+    );
     if (blocked !== undefined) {
       return yield* new BrowserImportFailedError({ sourceId: definition.id, reason: blocked });
     }
@@ -120,7 +131,9 @@ export const make = Effect.gen(function* BrowserImportMake() {
     // source itself reported it. Forwarding it unchecked would let `..`
     // segments walk out of the browser's user-data directory and read any
     // cookie database reachable on disk.
-    const sourceProfiles = yield* Effect.promise(() => listSourceProfiles(definition));
+    const sourceProfiles = yield* listSourceProfiles(definition, paths).pipe(
+      Effect.provide(platformServices),
+    );
     const requestedProfile = sourceProfiles.find(
       (profile) => profile.directory === input.input.sourceProfileDirectory,
     );
@@ -131,28 +144,30 @@ export const make = Effect.gen(function* BrowserImportMake() {
       });
     }
 
-    const cookies: ReadonlyArray<ChromiumCookie> = yield* Effect.tryPromise({
-      try: () =>
-        readChromiumCookies({
-          cookieDatabasePath: cookieDatabasePath(definition, requestedProfile.directory),
-          keychainService: definition.keychainService,
-          keychainAccount: definition.keychainAccount,
-          platform,
-        }),
-      catch: (cause) =>
-        new BrowserImportFailedError({
-          sourceId: definition.id,
-          reason: cause instanceof ChromiumCookieReadError ? cause.failure.reason : "readFailed",
-        }),
-    });
+    const cookies = yield* readChromiumCookies({
+      cookieDatabasePath: cookieDatabasePath(definition, paths, requestedProfile.directory),
+      keychainService: definition.keychainService,
+      keychainAccount: definition.keychainAccount,
+      platform,
+    }).pipe(
+      Effect.scoped,
+      Effect.provide(platformServices),
+      Effect.mapError(
+        (cause) =>
+          new BrowserImportFailedError({ sourceId: definition.id, reason: cause.reason, cause }),
+      ),
+    );
 
-    const session = yield* browserSession
-      .getSession(input.scope, input.persistent)
-      .pipe(
-        Effect.mapError(
-          () => new BrowserImportFailedError({ sourceId: definition.id, reason: "readFailed" }),
-        ),
-      );
+    const session = yield* browserSession.getSession(input.scope, input.persistent).pipe(
+      Effect.mapError(
+        (cause) =>
+          new BrowserImportFailedError({
+            sourceId: definition.id,
+            reason: "sessionUnavailable",
+            cause,
+          }),
+      ),
+    );
 
     // Written one at a time rather than in parallel: Chromium's cookie store
     // serialises writes anyway, and a rejected cookie should only cost itself.