[codex] Structure Electron window failures (#3276)

.macroscope/check-run-agents/effect-service-conventions.md

@@ -48,14 +48,19 @@ Review changed TypeScript and directly affected call sites for the conventions b
 - Define service failures with `Schema.TaggedErrorClass` and structured attributes. Derive `message` from those attributes rather than storing an unstructured message as the only data.
 - `Schema.Defect()` is not a substitute for modeling a generic error: its tag, fields, or both must identify the failure structurally, and its `message` must not merely stringify an opaque cause. A semantically precise error tag may preserve a real `cause` without inventing a redundant singleton field when no additional variable context exists; still retain any real path, resource, request, or entity context available at the wrapping site.
 - Capture stable, serializable domain context such as the operation or stage, resource/path or entity identifier, and normalized category/status. Map failures where that context is known instead of wrapping an entire multi-step pipeline in one generic error. Do not add a `detail` field that merely copies `cause.message` and then use it to construct the wrapper message.
+- Keep direct error attributes and log annotations safe and bounded. Do not copy raw wire payloads, command arguments or output, signed URLs, credentials, query strings, fragments, selectors, or arbitrary defect text into `detail`, `reason`, `message`, or a parallel log payload. Preserve the exact underlying value only as `cause`; expose normalized categories plus lengths/counts and safe URL protocol/hostname diagnostics where useful. Logging a sanitized error must not reintroduce a removed legacy `detail` or serialized `cause` field beside it.
 - When translating or wrapping a real failure, preserve the immediate underlying error itself as `cause` alongside the structural fields so the complete error chain and stack remain available. If every construction wraps a failure, `cause` should be required; make it optional only when the same error can legitimately originate without an underlying failure.
+- At a translation boundary, pass through an already structured domain error when it is part of the declared target error channel. Wrap only unknown or genuinely lower-level failures. A static factory or mapper may perform this classification when it is reused and keeps the policy next to the target error type.
 - Derive the wrapper's `message` exclusively from its stable structural attributes, never from `cause`, `cause.message`, or a stringified defect. Do not replace the immediate error with only `error.cause`, erase a structured upstream error into a string, or manufacture an `Error` merely to populate `cause`. Pure validation/domain errors created without an underlying failure do not need a cause.
 - Do not encode the same distinction twice with both a specific error tag and a single-value `operation`, `reason`, `kind`, or `phase` literal. Choose one coherent model: use distinct error classes and omit the redundant discriminator when callers or messages treat the failures as genuinely different, or use one service-level error with a multi-value operation discriminator and a generic message derived from that operation when the failures share the same semantics.
 - Treat an error message exposed through an HTTP/RPC response, persisted state, UI, or another caller-visible boundary as behavior. Preserve those messages during a structural refactor. Existing distinct caller-visible messages are evidence that the failures should normally remain distinct error tags without redundant singleton discriminators, rather than being collapsed into a generic operation error.
 - Split semantically distinct failures into separate error classes when a `reason`, `kind`, `phase`, or similar discriminator is used to choose the user-facing message or drive caller control flow. A discriminator used only for internal diagnostics may remain a field.
 - Use `Schema.Union` of error classes when a shared schema, predicate, or helper type is useful.
 - Export direct schema predicates such as `export const isFoo = Schema.is(Foo)`. Flag a private `Schema.is` constant wrapped by a redundant function with the same signature.
 - Do not introduce a large `switch` or lookup table in an error's `message` getter to model failures that deserve separate error classes.
+- Catch statically known tagged failures with `Effect.catchTags({ ... })`, including when handling only one tag. Do not use `catchIf` with a schema predicate merely to recover one or more known `_tag` variants, and do not use `catchTag`. `Effect.catch` is appropriate when the entire error channel is intentionally handled; `catchIf` remains appropriate for genuinely structural predicates such as inspecting an underlying platform error code.
+- Do not add a helper whose only behavior is `(...args) => new SomeError({ ...args })`, including curried aliases used once with `mapError`. Construct the error at the failure boundary so its attributes and cause remain visible. Keep a mapper only when it performs real normalization, passes through existing domain errors, or adds reusable context/control flow.
+- When a reusable error-to-error translation clearly belongs to the target error type, prefer a descriptive static factory on that error class over a detached production-side switch. Do not force a static method for one-off inline mappings.
 
 ## File layout and migrations
 
@@ -73,4 +78,6 @@ Review changed TypeScript and directly affected call sites for the conventions b
 
 ## Reporting
 
-Report only concrete violations introduced or retained in the pull request's changed scope. Prefer precise inline comments on the smallest relevant line range and state the expected fix. A clear convention violation may fail the check. Do not fail for optional style preferences or unrelated legacy code. If there are no findings, report exactly `All clear`.
+Report only concrete violations introduced or retained in the pull request's changed scope. Prefer precise inline comments on the smallest relevant line range and state the expected fix. A clear convention violation may fail the check. Do not fail for optional style preferences or unrelated legacy code.
+
+This check defaults to failure. When there are no findings, stop immediately and make the entire final response exactly `All clear` on one line. Do not add a title, explanation, punctuation, Markdown, JSON, or trailing analysis, and do not continue reasoning after deciding the review is clean.

apps/desktop/src/app/DesktopApp.ts

@@ -1,8 +1,8 @@
 import * as Cause from "effect/Cause";
-import * as Data from "effect/Data";
 import * as Effect from "effect/Effect";
 import * as Option from "effect/Option";
 import * as Ref from "effect/Ref";
+import * as Schema from "effect/Schema";
 
 import * as NetService from "@t3tools/shared/Net";
 import * as Crypto from "effect/Crypto";
@@ -33,22 +33,24 @@ const makeDesktopRunId = Crypto.Crypto.pipe(
   Effect.map((value) => value.replaceAll("-", "").slice(0, 12)),
 );
 
-class DesktopBackendPortUnavailableError extends Data.TaggedError(
+export class DesktopBackendPortUnavailableError extends Schema.TaggedErrorClass<DesktopBackendPortUnavailableError>()(
   "DesktopBackendPortUnavailableError",
-)<{
-  readonly startPort: number;
-  readonly maxPort: number;
-  readonly hosts: readonly string[];
-}> {
-  override get message() {
+  {
+    startPort: Schema.Int,
+    maxPort: Schema.Int,
+    hosts: Schema.Array(Schema.String),
+  },
+) {
+  override get message(): string {
     return `No desktop backend port is available on hosts ${this.hosts.join(", ")} between ${this.startPort} and ${this.maxPort}.`;
   }
 }
 
-class DesktopDevelopmentBackendPortRequiredError extends Data.TaggedError(
+export class DesktopDevelopmentBackendPortRequiredError extends Schema.TaggedErrorClass<DesktopDevelopmentBackendPortRequiredError>()(
   "DesktopDevelopmentBackendPortRequiredError",
-)<{}> {
-  override get message() {
+  {},
+) {
+  override get message(): string {
     return "T3CODE_PORT is required in desktop development.";
   }
 }

apps/desktop/src/app/DesktopAppErrors.test.ts

@@ -0,0 +1,30 @@
+import { assert, describe, it } from "@effect/vitest";
+
+import {
+  DesktopBackendPortUnavailableError,
+  DesktopDevelopmentBackendPortRequiredError,
+} from "./DesktopApp.ts";
+
+describe("DesktopApp errors", () => {
+  it("preserves unavailable backend port context", () => {
+    const error = new DesktopBackendPortUnavailableError({
+      startPort: 3_773,
+      maxPort: 65_535,
+      hosts: ["127.0.0.1", "0.0.0.0", "::"],
+    });
+
+    assert.equal(error.startPort, 3_773);
+    assert.equal(error.maxPort, 65_535);
+    assert.deepEqual(error.hosts, ["127.0.0.1", "0.0.0.0", "::"]);
+    assert.equal(
+      error.message,
+      "No desktop backend port is available on hosts 127.0.0.1, 0.0.0.0, :: between 3773 and 65535.",
+    );
+  });
+
+  it("reports the required development port", () => {
+    const error = new DesktopDevelopmentBackendPortRequiredError();
+
+    assert.equal(error.message, "T3CODE_PORT is required in desktop development.");
+  });
+});

apps/desktop/src/app/DesktopBackendOutputLog.test.ts

@@ -0,0 +1,122 @@
+import { assert, describe, it } from "@effect/vitest";
+import * as Effect from "effect/Effect";
+import * as FileSystem from "effect/FileSystem";
+import * as Layer from "effect/Layer";
+import * as Logger from "effect/Logger";
+import * as Path from "effect/Path";
+import * as PlatformError from "effect/PlatformError";
+
+import * as DesktopBackendOutputLog from "./DesktopBackendOutputLog.ts";
+import * as DesktopConfig from "./DesktopConfig.ts";
+import * as DesktopEnvironment from "./DesktopEnvironment.ts";
+
+const LOG_FILE_PATH = "/Users/alice/.t3/userdata/logs/server-child.log";
+
+const environmentLayer = DesktopEnvironment.layer({
+  dirname: "/repo/apps/desktop/dist-electron",
+  homeDirectory: "/Users/alice",
+  platform: "darwin",
+  processArch: "arm64",
+  appVersion: "1.2.3",
+  appPath: "/Applications/T3 Code.app/Contents/Resources/app.asar",
+  isPackaged: true,
+  resourcesPath: "/Applications/T3 Code.app/Contents/Resources",
+  runningUnderArm64Translation: false,
+}).pipe(Layer.provide(Layer.merge(Path.layer, DesktopConfig.layerTest({}))));
+
+const withOutputLog = <A, E, R>(
+  effect: Effect.Effect<A, E, R | DesktopBackendOutputLog.DesktopBackendOutputLog>,
+  fileSystemLayer: Layer.Layer<FileSystem.FileSystem>,
+  messages: Array<ReadonlyArray<unknown>>,
+) => {
+  const logger = Logger.make(({ message }) => {
+    messages.push(Array.isArray(message) ? message : [message]);
+  });
+  const outputLogLayer = DesktopBackendOutputLog.layer.pipe(
+    Layer.provide(Layer.mergeAll(fileSystemLayer, Path.layer, environmentLayer)),
+    Layer.provideMerge(Logger.layer([logger], { mergeWithExisting: false })),
+  );
+  return effect.pipe(Effect.provide(outputLogLayer));
+};
+
+const loggedError = (messages: ReadonlyArray<ReadonlyArray<unknown>>): unknown =>
+  messages.flat().find((value) => typeof value === "object" && value !== null && "error" in value)
+    ?.error;
+
+describe("DesktopBackendOutputLog", () => {
+  it.effect("logs setup failures with the log path and exact cause", () => {
+    const messages: Array<ReadonlyArray<unknown>> = [];
+    const cause = PlatformError.systemError({
+      _tag: "PermissionDenied",
+      module: "FileSystem",
+      method: "makeDirectory",
+      pathOrDescriptor: "/Users/alice/.t3/userdata/logs",
+      description: "private setup diagnostic",
+    });
+    const fileSystemLayer = FileSystem.layerNoop({
+      makeDirectory: () => Effect.fail(cause),
+    });
+
+    return withOutputLog(
+      Effect.gen(function* () {
+        const outputLog = yield* DesktopBackendOutputLog.DesktopBackendOutputLog;
+        yield* outputLog.writeSessionBoundary({ phase: "START", details: "test" });
+
+        const error = loggedError(messages);
+        assert.instanceOf(error, DesktopBackendOutputLog.DesktopBackendOutputLogSetupError);
+        assert.equal(error.logFilePath, LOG_FILE_PATH);
+        assert.strictEqual(error.cause, cause);
+        assert.equal(
+          error.message,
+          `Failed to initialize the desktop backend output log at ${LOG_FILE_PATH}.`,
+        );
+        assert.notInclude(error.message, "private setup diagnostic");
+      }),
+      fileSystemLayer,
+      messages,
+    );
+  });
+
+  it.effect("logs record write failures with the operation and exact cause", () => {
+    const messages: Array<ReadonlyArray<unknown>> = [];
+    const missingCause = PlatformError.systemError({
+      _tag: "NotFound",
+      module: "FileSystem",
+      method: "stat",
+      pathOrDescriptor: LOG_FILE_PATH,
+    });
+    const writeCause = PlatformError.systemError({
+      _tag: "PermissionDenied",
+      module: "FileSystem",
+      method: "writeFile",
+      pathOrDescriptor: LOG_FILE_PATH,
+      description: "private write diagnostic",
+    });
+    const fileSystemLayer = FileSystem.layerNoop({
+      makeDirectory: () => Effect.void,
+      stat: () => Effect.fail(missingCause),
+      readDirectory: () => Effect.succeed([]),
+      writeFile: () => Effect.fail(writeCause),
+    });
+
+    return withOutputLog(
+      Effect.gen(function* () {
+        const outputLog = yield* DesktopBackendOutputLog.DesktopBackendOutputLog;
+        yield* outputLog.writeSessionBoundary({ phase: "START", details: "test" });
+
+        const error = loggedError(messages);
+        assert.instanceOf(error, DesktopBackendOutputLog.DesktopBackendOutputLogWriteError);
+        assert.equal(error.operation, "write-record");
+        assert.equal(error.logFilePath, LOG_FILE_PATH);
+        assert.strictEqual(error.cause, writeCause);
+        assert.equal(
+          error.message,
+          `Desktop backend output log operation "write-record" failed at ${LOG_FILE_PATH}.`,
+        );
+        assert.notInclude(error.message, "private write diagnostic");
+      }),
+      fileSystemLayer,
+      messages,
+    );
+  });
+});