From 5aced13d89e7750ed5a7b55b219db126359e8fb8 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Fri, 10 Apr 2026 12:58:06 -0700 Subject: [PATCH 1/2] Harden secret store and remove redundant instanceof checks - harden server secret store file handling and publish-time catalog resolution - tighten desktop artifact typing - remove redundant instanceof checks from shared provider and runtime paths Co-authored-by: codex --- apps/server/scripts/cli.ts | 26 ++++++++- .../src/auth/Layers/ServerSecretStore.ts | 15 ++--- apps/server/src/git/Layers/GitCore.ts | 2 +- apps/server/src/git/Layers/GitHubCli.ts | 4 +- .../src/provider/Layers/CodexAdapter.test.ts | 7 --- .../src/provider/Layers/CodexAdapter.ts | 15 ++--- .../src/provider/Layers/CodexProvider.ts | 7 +-- apps/server/src/provider/providerSnapshot.ts | 3 +- apps/server/src/terminal/Layers/Manager.ts | 57 ++++++++++--------- .../src/workspace/Layers/WorkspaceEntries.ts | 5 +- apps/server/src/ws.ts | 5 +- apps/web/src/environments/primary/auth.ts | 36 ++++++------ apps/web/src/environments/primary/context.ts | 8 +-- package.json | 6 ++ scripts/build-desktop-artifact.ts | 6 +- scripts/lib/resolve-catalog.ts | 6 +- 16 files changed, 106 insertions(+), 102 deletions(-) diff --git a/apps/server/scripts/cli.ts b/apps/server/scripts/cli.ts index 21bc515aa7..be2f51c786 100644 --- a/apps/server/scripts/cli.ts +++ b/apps/server/scripts/cli.ts @@ -14,6 +14,22 @@ import { resolveCatalogDependencies } from "../../../scripts/lib/resolve-catalog import rootPackageJson from "../../../package.json" with { type: "json" }; import serverPackageJson from "../package.json" with { type: "json" }; +interface PackageJson { + name: string; + repository: { + type: string; + url: string; + directory: string; + }; + bin: Record; + type: string; + version: string; + engines: Record; + files: string[]; + dependencies: Record; + overrides: Record; +} + class CliError extends Data.TaggedError("CliError")<{ readonly message: string; readonly cause?: unknown; @@ -192,7 +208,7 @@ const publishCmd = Command.make( // Resolve catalog dependencies before any file mutations. If this throws, // acquire fails and no release hook runs, so filesystem must still be untouched. const version = Option.getOrElse(config.appVersion, () => serverPackageJson.version); - const pkg = { + const pkg: PackageJson = { name: serverPackageJson.name, repository: serverPackageJson.repository, bin: serverPackageJson.bin, @@ -200,7 +216,8 @@ const publishCmd = Command.make( version, engines: serverPackageJson.engines, files: serverPackageJson.files, - dependencies: serverPackageJson.dependencies as Record, + dependencies: serverPackageJson.dependencies, + overrides: rootPackageJson.overrides, }; pkg.dependencies = resolveCatalogDependencies( @@ -208,6 +225,11 @@ const publishCmd = Command.make( rootPackageJson.workspaces.catalog, "apps/server dependencies", ); + pkg.overrides = resolveCatalogDependencies( + pkg.overrides, + rootPackageJson.workspaces.catalog, + "root overrides", + ); const original = yield* fs.readFileString(packageJsonPath); yield* fs.writeFileString(backupPath, original); diff --git a/apps/server/src/auth/Layers/ServerSecretStore.ts b/apps/server/src/auth/Layers/ServerSecretStore.ts index a106d15fd5..cf5196bfb8 100644 --- a/apps/server/src/auth/Layers/ServerSecretStore.ts +++ b/apps/server/src/auth/Layers/ServerSecretStore.ts @@ -1,6 +1,6 @@ import * as Crypto from "node:crypto"; -import { Effect, FileSystem, Layer, Path } from "effect"; +import { Effect, FileSystem, Layer, Path, Predicate } from "effect"; import * as PlatformError from "effect/PlatformError"; import { ServerConfig } from "../../config.ts"; @@ -28,17 +28,14 @@ export const makeServerSecretStore = Effect.gen(function* () { const resolveSecretPath = (name: string) => path.join(serverConfig.secretsDir, `${name}.bin`); - const isMissingSecretFileError = (cause: unknown): cause is PlatformError.PlatformError => - cause instanceof PlatformError.PlatformError && cause.reason._tag === "NotFound"; - - const isAlreadyExistsSecretFileError = (cause: unknown): cause is PlatformError.PlatformError => - cause instanceof PlatformError.PlatformError && cause.reason._tag === "AlreadyExists"; + const isPlatformError = (u: unknown): u is PlatformError.PlatformError => + Predicate.isTagged(u, "PlatformError"); const get: ServerSecretStoreShape["get"] = (name) => fileSystem.readFile(resolveSecretPath(name)).pipe( Effect.map((bytes) => Uint8Array.from(bytes)), Effect.catch((cause) => - isMissingSecretFileError(cause) + cause.reason._tag === "NotFound" ? Effect.succeed(null) : Effect.fail( new SecretStoreError({ @@ -108,7 +105,7 @@ export const makeServerSecretStore = Effect.gen(function* () { return create(name, generated).pipe( Effect.as(Uint8Array.from(generated)), Effect.catchTag("SecretStoreError", (error) => - isAlreadyExistsSecretFileError(error.cause) + isPlatformError(error.cause) && error.cause.reason._tag === "AlreadyExists" ? get(name).pipe( Effect.flatMap((created) => created !== null @@ -129,7 +126,7 @@ export const makeServerSecretStore = Effect.gen(function* () { const remove: ServerSecretStoreShape["remove"] = (name) => fileSystem.remove(resolveSecretPath(name)).pipe( Effect.catch((cause) => - isMissingSecretFileError(cause) + cause instanceof PlatformError.PlatformError && cause.reason._tag === "NotFound" ? Effect.void : Effect.fail( new SecretStoreError({ diff --git a/apps/server/src/git/Layers/GitCore.ts b/apps/server/src/git/Layers/GitCore.ts index 3942935932..ae0670a91c 100644 --- a/apps/server/src/git/Layers/GitCore.ts +++ b/apps/server/src/git/Layers/GitCore.ts @@ -2002,7 +2002,7 @@ export const makeGitCore = Effect.fn("makeGitCore")(function* (options?: { "GitCore.removeWorktree", input.cwd, args, - `${commandLabel(args)} failed (cwd: ${input.cwd}): ${error instanceof Error ? error.message : String(error)}`, + `${commandLabel(args)} failed (cwd: ${input.cwd}): ${error.message}`, error, ), ), diff --git a/apps/server/src/git/Layers/GitHubCli.ts b/apps/server/src/git/Layers/GitHubCli.ts index 280679e337..ce56e91d53 100644 --- a/apps/server/src/git/Layers/GitHubCli.ts +++ b/apps/server/src/git/Layers/GitHubCli.ts @@ -1,4 +1,4 @@ -import { Effect, Layer, Schema } from "effect"; +import { Effect, Layer, Schema, SchemaIssue } from "effect"; import { PositiveInt, TrimmedNonEmptyString } from "@t3tools/contracts"; import { runProcess } from "../../processRunner"; @@ -154,7 +154,7 @@ function decodeGitHubJson( (error) => new GitHubCliError({ operation, - detail: error instanceof Error ? `${invalidDetail}: ${error.message}` : invalidDetail, + detail: `${invalidDetail}: ${SchemaIssue.makeFormatterDefault()(error.issue)}`, cause: error, }), ), diff --git a/apps/server/src/provider/Layers/CodexAdapter.test.ts b/apps/server/src/provider/Layers/CodexAdapter.test.ts index db91e8da0d..e2d22d49f2 100644 --- a/apps/server/src/provider/Layers/CodexAdapter.test.ts +++ b/apps/server/src/provider/Layers/CodexAdapter.test.ts @@ -238,14 +238,7 @@ sessionErrorLayer("CodexAdapterLive session errors", (it) => { .pipe(Effect.result); assert.equal(result._tag, "Failure"); - if (result._tag !== "Failure") { - return; - } - assert.equal(result.failure._tag, "ProviderAdapterSessionNotFoundError"); - if (result.failure._tag !== "ProviderAdapterSessionNotFoundError") { - return; - } assert.equal(result.failure.provider, "codex"); assert.equal(result.failure.threadId, "sess-missing"); assert.equal(result.failure.cause instanceof Error, true); diff --git a/apps/server/src/provider/Layers/CodexAdapter.ts b/apps/server/src/provider/Layers/CodexAdapter.ts index 957ae1b2bc..b830955e51 100644 --- a/apps/server/src/provider/Layers/CodexAdapter.ts +++ b/apps/server/src/provider/Layers/CodexAdapter.ts @@ -51,18 +51,11 @@ export interface CodexAdapterLiveOptions { readonly nativeEventLogger?: EventNdjsonLogger; } -function toMessage(cause: unknown, fallback: string): string { - if (cause instanceof Error && cause.message.length > 0) { - return cause.message; - } - return fallback; -} - function toSessionError( threadId: ThreadId, cause: unknown, ): ProviderAdapterSessionNotFoundError | ProviderAdapterSessionClosedError | undefined { - const normalized = toMessage(cause, "").toLowerCase(); + const normalized = cause instanceof Error ? cause.message.toLowerCase() : ""; if (normalized.includes("unknown session") || normalized.includes("unknown provider session")) { return new ProviderAdapterSessionNotFoundError({ provider: PROVIDER, @@ -88,7 +81,7 @@ function toRequestError(threadId: ThreadId, method: string, cause: unknown): Pro return new ProviderAdapterRequestError({ provider: PROVIDER, method, - detail: toMessage(cause, `${method} failed`), + detail: cause instanceof Error ? `${method} failed: ${cause.message}` : `${method} failed`, cause, }); } @@ -1427,7 +1420,7 @@ const makeCodexAdapter = Effect.fn("makeCodexAdapter")(function* ( new ProviderAdapterProcessError({ provider: PROVIDER, threadId: input.threadId, - detail: toMessage(cause, "Failed to start Codex adapter session."), + detail: `Failed to start Codex adapter session: ${cause instanceof Error ? cause.message : String(cause)}.`, cause, }), }); @@ -1455,7 +1448,7 @@ const makeCodexAdapter = Effect.fn("makeCodexAdapter")(function* ( new ProviderAdapterRequestError({ provider: PROVIDER, method: "turn/start", - detail: toMessage(cause, "Failed to read attachment file."), + detail: `Failed to read attachment file: ${cause.message}.`, cause, }), ), diff --git a/apps/server/src/provider/Layers/CodexProvider.ts b/apps/server/src/provider/Layers/CodexProvider.ts index 3509fa9257..085abe6fe2 100644 --- a/apps/server/src/provider/Layers/CodexProvider.ts +++ b/apps/server/src/provider/Layers/CodexProvider.ts @@ -389,7 +389,7 @@ export const checkCodexProviderStatus = Effect.fn("checkCodexProviderStatus")(fu auth: { status: "unknown" }, message: isCommandMissingCause(error) ? "Codex CLI (`codex`) is not installed or not on PATH." - : `Failed to execute Codex CLI health check: ${error instanceof Error ? error.message : String(error)}.`, + : `Failed to execute Codex CLI health check: ${error.message}.`, }, }); } @@ -489,10 +489,7 @@ export const checkCodexProviderStatus = Effect.fn("checkCodexProviderStatus")(fu version: parsedVersion, status: "warning", auth: { status: "unknown" }, - message: - error instanceof Error - ? `Could not verify Codex authentication status: ${error.message}.` - : "Could not verify Codex authentication status.", + message: `Could not verify Codex authentication status: ${error.message}.`, }, }); } diff --git a/apps/server/src/provider/providerSnapshot.ts b/apps/server/src/provider/providerSnapshot.ts index 4c80d78e20..232d2d3582 100644 --- a/apps/server/src/provider/providerSnapshot.ts +++ b/apps/server/src/provider/providerSnapshot.ts @@ -32,8 +32,7 @@ export function nonEmptyTrimmed(value: string | undefined): string | undefined { return trimmed.length > 0 ? trimmed : undefined; } -export function isCommandMissingCause(error: unknown): boolean { - if (!(error instanceof Error)) return false; +export function isCommandMissingCause(error: Error): boolean { const lower = error.message.toLowerCase(); return lower.includes("enoent") || lower.includes("notfound"); } diff --git a/apps/server/src/terminal/Layers/Manager.ts b/apps/server/src/terminal/Layers/Manager.ts index 5dc216e37e..9cb58bc161 100644 --- a/apps/server/src/terminal/Layers/Manager.ts +++ b/apps/server/src/terminal/Layers/Manager.ts @@ -8,7 +8,6 @@ import { } from "@t3tools/contracts"; import { makeKeyedCoalescingWorker } from "@t3tools/shared/KeyedCoalescingWorker"; import { - Data, Effect, Encoding, Equal, @@ -17,6 +16,7 @@ import { FileSystem, Layer, Option, + Schema, Scope, Semaphore, SynchronizedRef, @@ -54,22 +54,28 @@ const DEFAULT_OPEN_COLS = 120; const DEFAULT_OPEN_ROWS = 30; const TERMINAL_ENV_BLOCKLIST = new Set(["PORT", "ELECTRON_RENDERER_PORT", "ELECTRON_RUN_AS_NODE"]); -type TerminalSubprocessChecker = ( - terminalPid: number, -) => Effect.Effect; - -class TerminalSubprocessCheckError extends Data.TaggedError("TerminalSubprocessCheckError")<{ - readonly message: string; - readonly cause?: unknown; - readonly terminalPid: number; - readonly command: "powershell" | "pgrep" | "ps"; -}> {} +class TerminalSubprocessCheckError extends Schema.TaggedErrorClass()( + "TerminalSubprocessCheckError", + { + message: Schema.String, + cause: Schema.optional(Schema.Defect), + terminalPid: Schema.Number, + command: Schema.Literals(["powershell", "pgrep", "ps"]), + }, +) {} + +class TerminalProcessSignalError extends Schema.TaggedErrorClass()( + "TerminalProcessSignalError", + { + message: Schema.String, + cause: Schema.optional(Schema.Defect), + signal: Schema.Literals(["SIGTERM", "SIGKILL"]), + }, +) {} -class TerminalProcessSignalError extends Data.TaggedError("TerminalProcessSignalError")<{ - readonly message: string; - readonly cause?: unknown; - readonly signal: "SIGTERM" | "SIGKILL"; -}> {} +interface TerminalSubprocessChecker { + (terminalPid: number): Effect.Effect; +} interface ShellCandidate { shell: string; @@ -271,9 +277,8 @@ function isRetryableShellSpawnError(error: PtySpawnError): boolean { if (current instanceof Error) { messages.push(current.message); - const cause = (current as { cause?: unknown }).cause; - if (cause) { - queue.push(cause); + if (current.cause) { + queue.push(current.cause); } continue; } @@ -876,7 +881,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith Effect.logWarning("failed to persist terminal history", { threadId, terminalId, - error: error instanceof Error ? error.message : String(error), + error, }), ), ); @@ -959,7 +964,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith Effect.catch((cleanupError) => Effect.logWarning("failed to remove legacy terminal history", { threadId, - error: cleanupError instanceof Error ? cleanupError.message : String(cleanupError), + error: cleanupError, }), ), ); @@ -975,7 +980,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith Effect.logWarning("failed to delete terminal history", { threadId, terminalId, - error: error instanceof Error ? error.message : String(error), + error, }), ), ); @@ -985,7 +990,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith Effect.logWarning("failed to delete terminal history", { threadId, terminalId, - error: error instanceof Error ? error.message : String(error), + error, }), ), ); @@ -1011,7 +1016,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith Effect.catch((error) => Effect.logWarning("failed to delete terminal histories for thread", { threadId, - error: error instanceof Error ? error.message : String(error), + error, }), ), ), @@ -1463,12 +1468,12 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith const terminalPid = session.pid; const hasRunningSubprocess = yield* subprocessChecker(terminalPid).pipe( Effect.map(Option.some), - Effect.catch((error) => + Effect.catch((reason) => Effect.logWarning("failed to check terminal subprocess activity", { threadId: session.threadId, terminalId: session.terminalId, terminalPid, - error: error instanceof Error ? error.message : String(error), + reason, }).pipe(Effect.as(Option.none())), ), ); diff --git a/apps/server/src/workspace/Layers/WorkspaceEntries.ts b/apps/server/src/workspace/Layers/WorkspaceEntries.ts index 12af8601ca..56f8e5959e 100644 --- a/apps/server/src/workspace/Layers/WorkspaceEntries.ts +++ b/apps/server/src/workspace/Layers/WorkspaceEntries.ts @@ -214,9 +214,6 @@ function directoryAncestorsOf(relativePath: string): string[] { return directories; } -const processErrorDetail = (cause: unknown): string => - cause instanceof Error ? cause.message : String(cause); - export const makeWorkspaceEntries = Effect.gen(function* () { const path = yield* Path.Path; const gitOption = yield* Effect.serviceOption(GitCore); @@ -319,7 +316,7 @@ export const makeWorkspaceEntries = Effect.gen(function* () { new WorkspaceEntriesError({ cwd, operation: "workspaceEntries.readDirectoryEntries", - detail: processErrorDetail(cause), + detail: cause instanceof Error ? cause.message : String(cause), cause, }), }).pipe( diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index 8d4f946691..c90e9ce303 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -313,10 +313,7 @@ const makeWsRpcLayer = (currentSessionId: AuthSessionId) => worktreePath: input.worktreePath, scriptId: input.scriptId, terminalId: input.terminalId, - detail: - error instanceof Error - ? error.message - : "Unknown setup activity dispatch failure.", + detail: error.message, }, ), ), diff --git a/apps/web/src/environments/primary/auth.ts b/apps/web/src/environments/primary/auth.ts index b2b330ce75..ae2d6bd64d 100644 --- a/apps/web/src/environments/primary/auth.ts +++ b/apps/web/src/environments/primary/auth.ts @@ -16,6 +16,14 @@ import { } from "../../pairingUrl"; import { resolvePrimaryEnvironmentHttpUrl } from "./target"; +import { Data, Predicate } from "effect"; + +export class BootstrapHttpError extends Data.TaggedError("BootstrapHttpError")<{ + readonly message: string; + readonly status: number; +}> {} +const isBootstrapHttpError = (u: unknown): u is BootstrapHttpError => + Predicate.isTagged(u, "BootstrapHttpError"); export interface ServerPairingLinkRecord { readonly id: string; @@ -87,10 +95,10 @@ export async function fetchSessionState(): Promise { credentials: "include", }); if (!response.ok) { - throw new BootstrapHttpError( - `Failed to load server auth session state (${response.status}).`, - response.status, - ); + throw new BootstrapHttpError({ + message: `Failed to load server auth session state (${response.status}).`, + status: response.status, + }); } return (await response.json()) as AuthSessionState; }); @@ -115,10 +123,10 @@ async function exchangeBootstrapCredential(credential: string): Promise(operation: () => Promise): Promise { const startedAt = Date.now(); while (true) { @@ -182,7 +180,7 @@ function waitForBootstrapRetry(delayMs: number): Promise { } function isTransientBootstrapError(error: unknown): boolean { - if (error instanceof BootstrapHttpError) { + if (isBootstrapHttpError(error)) { return TRANSIENT_BOOTSTRAP_STATUS_CODES.has(error.status); } diff --git a/apps/web/src/environments/primary/context.ts b/apps/web/src/environments/primary/context.ts index df8c5d1dbb..dea1683c4d 100644 --- a/apps/web/src/environments/primary/context.ts +++ b/apps/web/src/environments/primary/context.ts @@ -52,10 +52,10 @@ async function fetchPrimaryEnvironmentDescriptor(): Promise | undefined, - catalog: Record, -): Record { + dependencies: Record | undefined, + catalog: Record, +): Record { if (!dependencies || Object.keys(dependencies).length === 0) { return {}; } diff --git a/scripts/lib/resolve-catalog.ts b/scripts/lib/resolve-catalog.ts index 2946c4a5d9..597bd06c24 100644 --- a/scripts/lib/resolve-catalog.ts +++ b/scripts/lib/resolve-catalog.ts @@ -5,10 +5,10 @@ * the concrete version string found in `catalog`. Throws on missing entries. */ export function resolveCatalogDependencies( - dependencies: Record, - catalog: Record, + dependencies: Record, + catalog: Record, label: string, -): Record { +): Record { return Object.fromEntries( Object.entries(dependencies).map(([name, spec]) => { if (typeof spec !== "string" || !spec.startsWith("catalog:")) { From 83abc7623b7023c995a57cf09a05a230d793c626 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Fri, 10 Apr 2026 13:06:52 -0700 Subject: [PATCH 2/2] missed one --- apps/server/src/auth/Layers/ServerSecretStore.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/server/src/auth/Layers/ServerSecretStore.ts b/apps/server/src/auth/Layers/ServerSecretStore.ts index cf5196bfb8..c8acf11bab 100644 --- a/apps/server/src/auth/Layers/ServerSecretStore.ts +++ b/apps/server/src/auth/Layers/ServerSecretStore.ts @@ -126,7 +126,7 @@ export const makeServerSecretStore = Effect.gen(function* () { const remove: ServerSecretStoreShape["remove"] = (name) => fileSystem.remove(resolveSecretPath(name)).pipe( Effect.catch((cause) => - cause instanceof PlatformError.PlatformError && cause.reason._tag === "NotFound" + cause.reason._tag === "NotFound" ? Effect.void : Effect.fail( new SecretStoreError({