Merge remote-tracking branch 'origin/main' into feat/telemetry-quality-signal

# Conflicts:
#	packages/opencode/src/altimate/telemetry/index.ts

Author: anandgupta42

.github/workflows/ci.yml

@@ -360,9 +360,11 @@ jobs:
         run: bun install
 
       - name: Build CLI binary
+        # target-index=1 = linux-x64 (see release.yml matrix)
         run: bun run packages/opencode/script/build.ts --target-index=1
         env:
           OPENCODE_VERSION: 0.0.0-sanity-${{ github.sha }}
+          OPENCODE_RELEASE: "1"
           MODELS_DEV_API_JSON: test/tool/fixtures/models-api.json
 
       - name: Build dbt-tools

packages/drivers/src/postgres.ts

@@ -22,6 +22,13 @@ export async function connect(config: ConnectionConfig): Promise<Connector> {
       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<Connector> {
       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

packages/drivers/src/redshift.ts

@@ -25,6 +25,12 @@ export async function connect(config: ConnectionConfig): Promise<Connector> {
       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"

packages/opencode/src/altimate/native/connections/registry.ts

@@ -133,6 +133,18 @@ const DRIVER_MAP: Record<string, string> = {
 async function createConnector(name: string, config: ConnectionConfig): Promise<Connector> {
   const driverPath = DRIVER_MAP[config.type.toLowerCase()]
   if (!driverPath) {
+    // altimate_change start — friendlier error for known-but-unsupported databases
+    const KNOWN_UNSUPPORTED: Record<string, string> = {
+      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) {

packages/opencode/src/altimate/telemetry/index.ts

@@ -410,7 +410,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
@@ -633,20 +633,57 @@ export namespace Telemetry {
     }
   }
 
+  // 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[]
   }> = [
     { 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,
@@ -661,6 +698,12 @@ export namespace Telemetry {
   export function computeInputSignature(args: Record<string, unknown>): string {
     const sig: Record<string, string> = {}
     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") {

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<string, unknown>)
+                  } 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",

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

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,20 +1562,47 @@ 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", () => {
     expect(Telemetry.classifyError("Permission denied on table")).toBe("permission")
     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", () => {
     expect(Telemetry.classifyError("Internal server error")).toBe("internal")
     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("[email protected]")
-    // 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", () => {

test/sanity/fixtures/injection-test.sql

@@ -0,0 +1,9 @@
+-- SQL injection test patterns for security phase
+-- These should be caught/handled gracefully, never executed literally
+
+SELECT * FROM users WHERE id = 1; DROP TABLE users; --
+SELECT * FROM users WHERE name = '' OR '1'='1';
+SELECT * FROM users WHERE id = 1 UNION SELECT username, password FROM admin_users;
+SELECT * FROM users WHERE name = 'Robert''); DROP TABLE students;--';
+SELECT LOAD_FILE('/etc/passwd');
+SELECT * INTO OUTFILE '/tmp/pwned.txt' FROM users;