diff --git a/packages/drivers/src/postgres.ts b/packages/drivers/src/postgres.ts index cf0a475756..e1b69465eb 100644 --- a/packages/drivers/src/postgres.ts +++ b/packages/drivers/src/postgres.ts @@ -22,6 +22,13 @@ export async function connect(config: ConnectionConfig): Promise { if (config.connection_string) { poolConfig.connectionString = config.connection_string } else { + // Validate required credentials before connecting to avoid cryptic + // SASL/SCRAM errors from the pg driver when password is missing + if (config.password != null && typeof config.password !== "string") { + throw new Error( + "PostgreSQL password must be a string. Check your warehouse configuration.", + ) + } poolConfig.host = config.host ?? "127.0.0.1" poolConfig.port = config.port ?? 5432 poolConfig.database = config.database ?? "postgres" @@ -43,9 +50,10 @@ export async function connect(config: ConnectionConfig): Promise { const client = await pool.connect() try { if (config.statement_timeout) { - await client.query( - `SET statement_timeout = '${Number(config.statement_timeout)}ms'`, - ) + const timeoutMs = Number(config.statement_timeout) + if (Number.isFinite(timeoutMs) && timeoutMs > 0) { + await client.query(`SET statement_timeout TO ${Math.round(timeoutMs)}`) + } } let query = sql diff --git a/packages/drivers/src/redshift.ts b/packages/drivers/src/redshift.ts index 752791174e..5893777102 100644 --- a/packages/drivers/src/redshift.ts +++ b/packages/drivers/src/redshift.ts @@ -25,6 +25,12 @@ export async function connect(config: ConnectionConfig): Promise { if (config.connection_string) { poolConfig.connectionString = config.connection_string } else { + // Validate password type to prevent cryptic SASL/SCRAM errors + if (config.password != null && typeof config.password !== "string") { + throw new Error( + "Redshift password must be a string. Check your warehouse configuration.", + ) + } poolConfig.host = config.host ?? "127.0.0.1" poolConfig.port = config.port ?? 5439 // Redshift default poolConfig.database = config.database ?? "dev" diff --git a/packages/opencode/src/altimate/native/connections/registry.ts b/packages/opencode/src/altimate/native/connections/registry.ts index 5aaafdd640..7dfb99fcf6 100644 --- a/packages/opencode/src/altimate/native/connections/registry.ts +++ b/packages/opencode/src/altimate/native/connections/registry.ts @@ -133,6 +133,18 @@ const DRIVER_MAP: Record = { async function createConnector(name: string, config: ConnectionConfig): Promise { const driverPath = DRIVER_MAP[config.type.toLowerCase()] if (!driverPath) { + // altimate_change start — friendlier error for known-but-unsupported databases + const KNOWN_UNSUPPORTED: Record = { + clickhouse: "ClickHouse is not yet supported. Use the bash tool with `clickhouse-client` or `curl` to query ClickHouse directly.", + cassandra: "Cassandra is not yet supported. Use the bash tool with `cqlsh` to query Cassandra directly.", + cockroachdb: "CockroachDB is not yet supported. It is PostgreSQL-compatible — try type: postgres instead.", + timescaledb: "TimescaleDB is a PostgreSQL extension — use type: postgres instead.", + } + const hint = KNOWN_UNSUPPORTED[config.type.toLowerCase()] + if (hint) { + throw new Error(hint) + } + // altimate_change end throw new Error(`Unsupported database type: ${config.type}. Supported: ${Object.keys(DRIVER_MAP).join(", ")}`) } @@ -143,6 +155,22 @@ async function createConnector(name: string, config: ConnectionConfig): Promise< // Resolve credentials from keychain resolvedConfig = await resolveConfig(name, resolvedConfig) + // altimate_change start — validate password is a string for drivers that require it + // Prevents cryptic SASL/SCRAM errors from database drivers + const PASSWORD_DRIVERS = new Set(["postgres", "postgresql", "redshift", "mysql", "mariadb", "sqlserver", "mssql", "oracle", "snowflake"]) + if ( + PASSWORD_DRIVERS.has(resolvedConfig.type.toLowerCase()) && + !resolvedConfig.connection_string && + resolvedConfig.password != null && + typeof resolvedConfig.password !== "string" + ) { + throw new Error( + `Database password must be a string for ${resolvedConfig.type}. ` + + "Check your warehouse configuration or re-add the connection with warehouse.add.", + ) + } + // altimate_change end + // Handle SSH tunnel const sshConfig = extractSshConfig(resolvedConfig) if (sshConfig) { diff --git a/packages/opencode/src/altimate/telemetry/index.ts b/packages/opencode/src/altimate/telemetry/index.ts index 9e1564eae6..6ddd77f421 100644 --- a/packages/opencode/src/altimate/telemetry/index.ts +++ b/packages/opencode/src/altimate/telemetry/index.ts @@ -403,7 +403,7 @@ export namespace Telemetry { session_id: string tool_name: string tool_category: string - error_class: "parse_error" | "connection" | "timeout" | "validation" | "internal" | "permission" | "unknown" + error_class: "parse_error" | "connection" | "timeout" | "validation" | "internal" | "permission" | "http_error" | "unknown" error_message: string input_signature: string masked_args?: string @@ -425,6 +425,9 @@ export namespace Telemetry { } // altimate_change end + // altimate_change start — expanded error classification patterns for better triage + // Order matters: earlier patterns take priority. Use specific phrases, not + // single words, to avoid false positives (e.g., "connection refused" not "connection"). const ERROR_PATTERNS: Array<{ class: Telemetry.Event & { type: "core_failure" } extends { error_class: infer C } ? C : never keywords: string[] @@ -432,13 +435,47 @@ export namespace Telemetry { { class: "parse_error", keywords: ["parse", "syntax", "binder", "unexpected token", "sqlglot"] }, { class: "connection", - keywords: ["econnrefused", "connection", "socket", "enotfound", "econnreset"], + keywords: [ + "econnrefused", + "enotfound", + "econnreset", + "connection refused", + "connection reset", + "connection closed", + "connect failed", + "connect etimedout", + "socket hang up", + "sasl", + "scram", + "password must be", + "driver not installed", + "not found. available:", + "no warehouse configured", + "unsupported database type", + ], }, { class: "timeout", keywords: ["timeout", "etimedout", "bridge timeout", "timed out"] }, - { class: "permission", keywords: ["permission", "denied", "unauthorized", "forbidden"] }, - { class: "validation", keywords: ["invalid params", "invalid", "missing", "required"] }, + { class: "permission", keywords: ["permission", "access denied", "permission denied", "unauthorized", "forbidden", "authentication"] }, + { + class: "validation", + keywords: [ + "invalid params", + "invalid", + "missing", + "required", + "must read file", + "has been modified since", + "does not exist", + "before overwriting", + ], + }, { class: "internal", keywords: ["internal", "assertion"] }, + { + class: "http_error", + keywords: ["status code: 4", "status code: 5", "request failed with status"], + }, ] + // altimate_change end export function classifyError( message: string, @@ -453,6 +490,12 @@ export namespace Telemetry { export function computeInputSignature(args: Record): string { const sig: Record = {} for (const [k, v] of Object.entries(args)) { + // altimate_change start — redact sensitive keys in input signatures + if (isSensitiveKey(k)) { + sig[k] = "****" + continue + } + // altimate_change end if (v === null || v === undefined) { sig[k] = "null" } else if (typeof v === "string") { diff --git a/packages/opencode/src/tool/tool.ts b/packages/opencode/src/tool/tool.ts index d0a8ec0e59..2e054ebc92 100644 --- a/packages/opencode/src/tool/tool.ts +++ b/packages/opencode/src/tool/tool.ts @@ -145,10 +145,25 @@ export namespace Tool { } if (isSoftFailure) { // prettier-ignore - const errorMsg = - typeof result.metadata?.error === "string" - ? result.metadata.error - : "unknown error" + // altimate_change start — propagate non-string error metadata to avoid blind "unknown error" + const rawError = result.metadata?.error + let errorMsg: string + if (typeof rawError === "string") { + errorMsg = rawError + } else if (rawError != null) { + if (typeof rawError === "object") { + try { + errorMsg = Telemetry.maskArgs(rawError as Record) + } catch { + errorMsg = `soft_failure:${id}:unserializable_error` + } + } else { + errorMsg = String(rawError) + } + } else { + errorMsg = `soft_failure:${id}:no_error_metadata` + } + // altimate_change end const maskedErrorMsg = Telemetry.maskString(errorMsg).slice(0, 500) Telemetry.track({ type: "core_failure", diff --git a/packages/opencode/src/tool/webfetch.ts b/packages/opencode/src/tool/webfetch.ts index 47c6f2b8c7..6c2d0a3ff6 100644 --- a/packages/opencode/src/tool/webfetch.ts +++ b/packages/opencode/src/tool/webfetch.ts @@ -84,7 +84,17 @@ export const WebFetchTool = Tool.define("webfetch", { } if (!response.ok) { - throw new Error(`Request failed with status code: ${response.status}`) + // altimate_change start — include URL domain in error for easier triage + let domain: string + try { + domain = new URL(params.url).hostname + } catch { + domain = params.url.slice(0, 60) + } + throw new Error( + `Request failed with status code: ${response.status} (${domain})`, + ) + // altimate_change end } // Check content length diff --git a/packages/opencode/test/telemetry/telemetry.test.ts b/packages/opencode/test/telemetry/telemetry.test.ts index f1b7990eb9..4049296f96 100644 --- a/packages/opencode/test/telemetry/telemetry.test.ts +++ b/packages/opencode/test/telemetry/telemetry.test.ts @@ -1538,6 +1538,16 @@ describe("telemetry.classifyError", () => { expect(Telemetry.classifyError("Socket hang up")).toBe("connection") expect(Telemetry.classifyError("ENOTFOUND db.example.com")).toBe("connection") expect(Telemetry.classifyError("ECONNRESET")).toBe("connection") + // altimate_change start — expanded connection patterns + expect(Telemetry.classifyError("SASL: SCRAM-SERVER-FIRST-MESSAGE: client password must be a string")).toBe("connection") + expect(Telemetry.classifyError("password must be a string")).toBe("connection") + expect(Telemetry.classifyError("PostgreSQL driver not installed. Run: npm install pg")).toBe("connection") + expect(Telemetry.classifyError("Error: Connection mydb not found. Available: (none)")).toBe("connection") + expect(Telemetry.classifyError("No warehouse configured. Use warehouse.add")).toBe("connection") + expect(Telemetry.classifyError("Unsupported database type: clickhouse")).toBe("connection") + expect(Telemetry.classifyError("Connection reset by peer")).toBe("connection") + expect(Telemetry.classifyError("Connection closed unexpectedly")).toBe("connection") + // altimate_change end }) test("classifies timeout errors", () => { @@ -1552,6 +1562,12 @@ describe("telemetry.classifyError", () => { expect(Telemetry.classifyError("Invalid dialect specified")).toBe("validation") expect(Telemetry.classifyError("Missing required field")).toBe("validation") expect(Telemetry.classifyError("Required parameter 'query' not provided")).toBe("validation") + // altimate_change start — expanded validation patterns + expect(Telemetry.classifyError("You must read file /path/to/file before overwriting it")).toBe("validation") + expect(Telemetry.classifyError("File has been modified since it was last read")).toBe("validation") + expect(Telemetry.classifyError("error: column foo does not exist")).toBe("validation") + expect(Telemetry.classifyError("You must read file before overwriting it. Use the Read tool first")).toBe("validation") + // altimate_change end }) test("classifies permission errors", () => { @@ -1559,6 +1575,9 @@ describe("telemetry.classifyError", () => { expect(Telemetry.classifyError("Access denied for user")).toBe("permission") expect(Telemetry.classifyError("Unauthorized access to resource")).toBe("permission") expect(Telemetry.classifyError("403 Forbidden")).toBe("permission") + // altimate_change start — authentication classified as permission + expect(Telemetry.classifyError("Authentication failed for user admin")).toBe("permission") + // altimate_change end }) test("classifies internal errors", () => { @@ -1566,6 +1585,24 @@ describe("telemetry.classifyError", () => { expect(Telemetry.classifyError("Assertion failed: x > 0")).toBe("internal") }) + // altimate_change start — http_error class and priority ordering tests + test("classifies http errors", () => { + expect(Telemetry.classifyError("Request failed with status code: 404 (example.com)")).toBe("http_error") + expect(Telemetry.classifyError("Request failed with status code: 500")).toBe("http_error") + expect(Telemetry.classifyError("status code: 403")).toBe("http_error") + expect(Telemetry.classifyError("Request failed with status")).toBe("http_error") + }) + + test("priority ordering: earlier patterns win over later ones", () => { + // SASL is connection, even though "authentication" is in permission + expect(Telemetry.classifyError("SASL authentication failed")).toBe("connection") + // parse_error wins over validation for "invalid syntax" + expect(Telemetry.classifyError("parse error: invalid syntax")).toBe("parse_error") + // permission ("forbidden") wins over http_error ("status code: 4") + expect(Telemetry.classifyError("403 Forbidden, status code: 403")).toBe("permission") + }) + // altimate_change end + test("returns unknown for unrecognized errors", () => { expect(Telemetry.classifyError("Something went wrong")).toBe("unknown") expect(Telemetry.classifyError("")).toBe("unknown") @@ -1634,11 +1671,11 @@ describe("telemetry.computeInputSignature", () => { expect(sig).not.toContain("sk-abc123") expect(sig).not.toContain("SELECT") expect(sig).not.toContain("admin@example.com") - // Only type descriptors appear as values + // Only type descriptors appear as values; sensitive keys are fully redacted const parsed = JSON.parse(sig) expect(parsed.sql).toBe("string:60") - expect(parsed.secret).toBe("string:16") - expect(parsed.api_key).toBe("string:9") + expect(parsed.secret).toBe("****") + expect(parsed.api_key).toBe("****") }) test("truncates output at 1000 chars with valid JSON", () => {