From ed6ba7439d55b8c2cf7324d5107c2c1cb3ee0920 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 20 Jun 2026 11:19:57 -0700 Subject: [PATCH] [codex] sanitize provider runtime failure diagnostics (#3414) Co-authored-by: codex --- .../src/provider/Layers/ClaudeProvider.ts | 16 +++++---- .../src/provider/Layers/CursorProvider.ts | 11 +++--- .../src/provider/Layers/GrokProvider.test.ts | 6 ++-- .../src/provider/Layers/GrokProvider.ts | 27 +++++++++------ .../provider/Layers/ProviderRegistry.test.ts | 13 ++++--- .../src/provider/Layers/ProviderService.ts | 6 ++-- .../src/provider/providerStatusCache.test.ts | 34 +++++++++++++++++++ .../src/provider/providerStatusCache.ts | 4 +-- 8 files changed, 85 insertions(+), 32 deletions(-) diff --git a/apps/server/src/provider/Layers/ClaudeProvider.ts b/apps/server/src/provider/Layers/ClaudeProvider.ts index d677de7a3138a1d86a981c97928937a61701d9b9..bd5f7ebffc4cb69f29db7af0d9bf4f8f115d3fa9 100644 --- a/apps/server/src/provider/Layers/ClaudeProvider.ts +++ b/apps/server/src/provider/Layers/ClaudeProvider.ts @@ -31,7 +31,6 @@ import { buildSelectOptionDescriptor, buildServerProvider, DEFAULT_TIMEOUT_MS, - detailFromResult, isCommandMissingCause, parseGenericCliVersion, providerModelsFromSettings, @@ -661,6 +660,9 @@ export const checkClaudeProviderStatus = Effect.fn("checkClaudeProviderStatus")( if (Result.isFailure(versionProbe)) { const error = versionProbe.failure; + yield* Effect.logWarning("Claude Agent CLI health check failed.", { + errorTag: error._tag, + }); return buildServerProvider({ presentation: CLAUDE_PRESENTATION, enabled: claudeSettings.enabled, @@ -673,7 +675,7 @@ export const checkClaudeProviderStatus = Effect.fn("checkClaudeProviderStatus")( auth: { status: "unknown" }, message: isCommandMissingCause(error) ? "Claude Agent CLI (`claude`) is not installed or not on PATH." - : `Failed to execute Claude Agent CLI health check: ${error instanceof Error ? error.message : String(error)}.`, + : "Failed to execute Claude Agent CLI health check.", }, }); } @@ -698,7 +700,11 @@ export const checkClaudeProviderStatus = Effect.fn("checkClaudeProviderStatus")( const version = versionProbe.success.value; const parsedVersion = parseGenericCliVersion(`${version.stdout}\n${version.stderr}`); if (version.code !== 0) { - const detail = detailFromResult(version); + yield* Effect.logWarning("Claude Agent CLI version probe exited with a non-zero status.", { + exitCode: version.code, + stdoutLength: version.stdout.length, + stderrLength: version.stderr.length, + }); return buildServerProvider({ presentation: CLAUDE_PRESENTATION, enabled: claudeSettings.enabled, @@ -709,9 +715,7 @@ export const checkClaudeProviderStatus = Effect.fn("checkClaudeProviderStatus")( version: parsedVersion, status: "error", auth: { status: "unknown" }, - message: detail - ? `Claude Agent CLI is installed but failed to run. ${detail}` - : "Claude Agent CLI is installed but failed to run.", + message: "Claude Agent CLI is installed but failed to run.", }, }); } diff --git a/apps/server/src/provider/Layers/CursorProvider.ts b/apps/server/src/provider/Layers/CursorProvider.ts index 94faac60647b1884cb6af0e6eb340321ac24c5f2..ff96ece93492a7ea9d4728658acfbd58a1ffdfee 100644 --- a/apps/server/src/provider/Layers/CursorProvider.ts +++ b/apps/server/src/provider/Layers/CursorProvider.ts @@ -10,7 +10,7 @@ import type { } from "@t3tools/contracts"; import { ProviderDriverKind } from "@t3tools/contracts"; import type * as EffectAcpSchema from "effect-acp/schema"; -import * as Cause from "effect/Cause"; +import { causeErrorTag } from "@t3tools/shared/observability"; import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; import * as Exit from "effect/Exit"; @@ -1006,6 +1006,9 @@ export const checkCursorProviderStatus = Effect.fn("checkCursorProviderStatus")( if (Result.isFailure(aboutProbe)) { const error = aboutProbe.failure; + yield* Effect.logWarning("Cursor Agent CLI health check failed.", { + errorTag: error._tag, + }); return buildServerProvider({ presentation: CURSOR_PRESENTATION, enabled: cursorSettings.enabled, @@ -1018,7 +1021,7 @@ export const checkCursorProviderStatus = Effect.fn("checkCursorProviderStatus")( auth: { status: "unknown" }, message: isCommandMissingCause(error) ? "Cursor Agent CLI (`agent`) is not installed or not on PATH." - : `Failed to execute Cursor Agent CLI health check: ${error instanceof Error ? error.message : String(error)}.`, + : "Failed to execute Cursor Agent CLI health check.", }, }); } @@ -1074,7 +1077,7 @@ export const checkCursorProviderStatus = Effect.fn("checkCursorProviderStatus")( ); if (Exit.isFailure(discoveryExit)) { yield* Effect.logWarning("Cursor ACP model discovery failed", { - cause: Cause.pretty(discoveryExit.cause), + errorTag: causeErrorTag(discoveryExit.cause), }); discoveryWarning = "Cursor ACP model discovery failed. Check server logs for details."; } else if (Option.isNone(discoveryExit.value)) { @@ -1130,7 +1133,7 @@ export const enrichCursorSnapshot = (input: { ), Effect.catchCause((cause) => Effect.logWarning("Cursor version advisory enrichment failed", { - cause: Cause.pretty(cause), + errorTag: causeErrorTag(cause), }).pipe(Effect.asVoid), ), ); diff --git a/apps/server/src/provider/Layers/GrokProvider.test.ts b/apps/server/src/provider/Layers/GrokProvider.test.ts index 75d0982565ed8c9ef4c696bf6dca544e3e8f7a1e..000243869c9e6f218ca1383d22b830d9b7e08896 100644 --- a/apps/server/src/provider/Layers/GrokProvider.test.ts +++ b/apps/server/src/provider/Layers/GrokProvider.test.ts @@ -54,6 +54,7 @@ it.layer(NodeServices.layer)("checkGrokProviderStatus", (it) => { it.effect("reports an installed CLI as unhealthy when --version exits non-zero", () => Effect.gen(function* () { + const secretStderr = "broken grok install: secret-token-value"; const snapshot = yield* Effect.scoped( Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; @@ -62,7 +63,7 @@ it.layer(NodeServices.layer)("checkGrokProviderStatus", (it) => { const grokPath = path.join(dir, "grok"); yield* fs.writeFileString( grokPath, - ["#!/bin/sh", 'printf "%s\\n" "broken grok install" >&2', "exit 2", ""].join("\n"), + ["#!/bin/sh", `printf "%s\\n" "${secretStderr}" >&2`, "exit 2", ""].join("\n"), ); yield* fs.chmod(grokPath, 0o755); @@ -75,7 +76,8 @@ it.layer(NodeServices.layer)("checkGrokProviderStatus", (it) => { expect(snapshot.enabled).toBe(true); expect(snapshot.installed).toBe(true); expect(snapshot.status).toBe("error"); - expect(snapshot.message).toContain("broken grok install"); + expect(snapshot.message).toBe("Grok CLI is installed but failed to run."); + expect(snapshot.message).not.toContain(secretStderr); }), ); diff --git a/apps/server/src/provider/Layers/GrokProvider.ts b/apps/server/src/provider/Layers/GrokProvider.ts index b1c84fb3a036facd772d70114884893745f6277e..cf5d5ad9c8d87544931affa3c8e1609e00df15ee 100644 --- a/apps/server/src/provider/Layers/GrokProvider.ts +++ b/apps/server/src/provider/Layers/GrokProvider.ts @@ -6,7 +6,7 @@ import { type ServerProviderModel, } from "@t3tools/contracts"; import type * as EffectAcpSchema from "effect-acp/schema"; -import * as Cause from "effect/Cause"; +import { causeErrorTag } from "@t3tools/shared/observability"; import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; import * as Exit from "effect/Exit"; @@ -19,7 +19,6 @@ import { resolveSpawnCommand } from "@t3tools/shared/shell"; import { buildServerProvider, - detailFromResult, isCommandMissingCause, parseGenericCliVersion, providerModelsFromSettings, @@ -195,6 +194,9 @@ export const checkGrokProviderStatus = Effect.fn("checkGrokProviderStatus")(func if (Result.isFailure(versionResult)) { const error = versionResult.failure; + yield* Effect.logWarning("Grok CLI health check failed.", { + errorTag: error._tag, + }); return buildServerProvider({ presentation: GROK_PRESENTATION, enabled: grokSettings.enabled, @@ -207,7 +209,7 @@ export const checkGrokProviderStatus = Effect.fn("checkGrokProviderStatus")(func auth: { status: "unknown" }, message: isCommandMissingCause(error) ? "Grok CLI (`grok`) is not installed or not on PATH." - : `Failed to execute Grok CLI health check: ${error instanceof Error ? error.message : String(error)}.`, + : "Failed to execute Grok CLI health check.", }, }); } @@ -231,7 +233,11 @@ export const checkGrokProviderStatus = Effect.fn("checkGrokProviderStatus")(func const versionOutput = versionResult.success.value; const version = parseGenericCliVersion(`${versionOutput.stdout}\n${versionOutput.stderr}`); if (versionOutput.code !== 0) { - const detail = detailFromResult(versionOutput); + yield* Effect.logWarning("Grok CLI version probe exited with a non-zero status.", { + exitCode: versionOutput.code, + stdoutLength: versionOutput.stdout.length, + stderrLength: versionOutput.stderr.length, + }); return buildServerProvider({ presentation: GROK_PRESENTATION, enabled: grokSettings.enabled, @@ -242,9 +248,7 @@ export const checkGrokProviderStatus = Effect.fn("checkGrokProviderStatus")(func version, status: "error", auth: { status: "unknown" }, - message: detail - ? `Grok CLI is installed but failed to run. ${detail}` - : "Grok CLI is installed but failed to run.", + message: "Grok CLI is installed but failed to run.", }, }); } @@ -254,8 +258,9 @@ export const checkGrokProviderStatus = Effect.fn("checkGrokProviderStatus")(func Effect.exit, ); if (Exit.isFailure(discoveryExit)) { - const detail = Cause.pretty(discoveryExit.cause); - yield* Effect.logWarning("Grok ACP model discovery failed", { cause: detail }); + yield* Effect.logWarning("Grok ACP model discovery failed", { + errorTag: causeErrorTag(discoveryExit.cause), + }); return buildServerProvider({ presentation: GROK_PRESENTATION, enabled: grokSettings.enabled, @@ -266,7 +271,7 @@ export const checkGrokProviderStatus = Effect.fn("checkGrokProviderStatus")(func version, status: "error", auth: { status: "unknown" }, - message: `Grok CLI is installed but ACP startup failed. ${detail}`, + message: "Grok CLI is installed but ACP startup failed. Check server logs for details.", }, }); } @@ -324,7 +329,7 @@ export const enrichGrokSnapshot = (input: { Effect.flatMap((enrichedSnapshot) => publishSnapshot(enrichedSnapshot)), Effect.catchCause((cause) => Effect.logWarning("Grok version advisory enrichment failed", { - cause: Cause.pretty(cause), + errorTag: causeErrorTag(cause), }), ), Effect.asVoid, diff --git a/apps/server/src/provider/Layers/ProviderRegistry.test.ts b/apps/server/src/provider/Layers/ProviderRegistry.test.ts index 1805b6ed27722d7bae7034cafebdb0b6c1df5076..b3ab11454956fbea89e8a574f17fc745cc5857f7 100644 --- a/apps/server/src/provider/Layers/ProviderRegistry.test.ts +++ b/apps/server/src/provider/Layers/ProviderRegistry.test.ts @@ -1875,14 +1875,17 @@ it.layer(Layer.mergeAll(NodeServices.layer, ServerSettingsModule.layerTest(), Te }).pipe(Effect.provide(failingSpawnerLayer("spawn claude ENOENT"))), ); - it.effect("returns error when version check fails with non-zero exit code", () => - Effect.gen(function* () { + it.effect("returns error when version check fails with non-zero exit code", () => { + const secretStderr = "Something went wrong: secret-token-value"; + return Effect.gen(function* () { const status = yield* checkClaudeProviderStatus( defaultClaudeSettings, claudeCapabilities(), ); assert.strictEqual(status.status, "error"); assert.strictEqual(status.installed, true); + assert.strictEqual(status.message, "Claude Agent CLI is installed but failed to run."); + assert.ok(!(status.message ?? "").includes(secretStderr)); }).pipe( Effect.provide( mockSpawnerLayer((args) => { @@ -1890,14 +1893,14 @@ it.layer(Layer.mergeAll(NodeServices.layer, ServerSettingsModule.layerTest(), Te if (joined === "--version") return { stdout: "", - stderr: "Something went wrong", + stderr: secretStderr, code: 1, }; throw new Error(`Unexpected args: ${joined}`); }), ), - ), - ); + ); + }); it.effect("returns warning when the Claude initialization result is unavailable", () => Effect.gen(function* () { diff --git a/apps/server/src/provider/Layers/ProviderService.ts b/apps/server/src/provider/Layers/ProviderService.ts index c15d50eed628dcbc8d981b15a6403670872d760b..2eaaeb8ce3c03071c543c64ac8ae65dc9c00b420 100644 --- a/apps/server/src/provider/Layers/ProviderService.ts +++ b/apps/server/src/provider/Layers/ProviderService.ts @@ -24,7 +24,7 @@ import { type ProviderRuntimeEvent, type ProviderSession, } from "@t3tools/contracts"; -import * as Cause from "effect/Cause"; +import { causeErrorTag } from "@t3tools/shared/observability"; import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; @@ -1061,7 +1061,9 @@ const makeProviderService = Effect.fn("makeProviderService")(function* ( yield* Effect.addFinalizer(() => runStopAll().pipe( Effect.catchCause((cause) => - Effect.logWarning("failed to stop provider service", { cause: Cause.pretty(cause) }), + Effect.logWarning("failed to stop provider service", { + errorTag: causeErrorTag(cause), + }), ), ), ); diff --git a/apps/server/src/provider/providerStatusCache.test.ts b/apps/server/src/provider/providerStatusCache.test.ts index 64cb9ccd4177c9bdd8aa7a7595dd0975956c0159..07f67cd7de8f95d3dbde02dc493bee4b7ed9365f 100644 --- a/apps/server/src/provider/providerStatusCache.test.ts +++ b/apps/server/src/provider/providerStatusCache.test.ts @@ -9,6 +9,7 @@ import { createModelCapabilities } from "@t3tools/shared/model"; import { assert, it } from "@effect/vitest"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; +import * as Logger from "effect/Logger"; import { hydrateCachedProvider, @@ -42,6 +43,39 @@ const makeProvider = ( }); it.layer(NodeServices.layer)("providerStatusCache", (it) => { + it.effect("logs structural diagnostics without retaining invalid cache contents", () => { + const messages: Array = []; + const logger = Logger.make((options) => { + if (Array.isArray(options.message)) { + messages.push(...options.message); + } else { + messages.push(options.message); + } + }); + + return Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const tempDir = yield* fs.makeTempDirectoryScoped({ prefix: "t3-provider-cache-invalid-" }); + const cachePath = `${tempDir}/provider.json`; + const secretCacheValue = "secret-cache-value"; + yield* fs.writeFileString(cachePath, `{ "token": "${secretCacheValue}" }`); + + const result = yield* readProviderStatusCache(cachePath); + + assert.strictEqual(result, undefined); + const failure = messages.find( + (message): message is Record => + typeof message === "object" && message !== null && "path" in message, + ); + assert.exists(failure); + assert.strictEqual(failure.path, cachePath); + assert.strictEqual(typeof failure.errorTag, "string"); + assert.ok(!("cause" in failure)); + assert.ok(!("issues" in failure)); + assert.ok(!Object.values(failure).map(String).join("\n").includes(secretCacheValue)); + }).pipe(Effect.provide(Logger.layer([logger], { mergeWithExisting: false }))); + }); + it.effect("writes and reads provider status snapshots", () => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; diff --git a/apps/server/src/provider/providerStatusCache.ts b/apps/server/src/provider/providerStatusCache.ts index 0b9b365f360a70c4e5897b04b27461e6be91d903..2fe0424b4f57f7d139eedde5b3cc291a809f7125 100644 --- a/apps/server/src/provider/providerStatusCache.ts +++ b/apps/server/src/provider/providerStatusCache.ts @@ -4,7 +4,7 @@ import { type ServerProvider, ServerProvider as ServerProviderSchema, } from "@t3tools/contracts"; -import * as Cause from "effect/Cause"; +import { causeErrorTag } from "@t3tools/shared/observability"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Path from "effect/Path"; @@ -134,7 +134,7 @@ export const readProviderStatusCache = (filePath: string) => onFailure: (cause) => Effect.logWarning("failed to parse provider status cache, ignoring", { path: filePath, - issues: Cause.pretty(cause), + errorTag: causeErrorTag(cause), }).pipe(Effect.as(undefined)), onSuccess: Effect.succeed, }), -- 2.54.0