Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions packages/drivers/src/postgres.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down
6 changes: 6 additions & 0 deletions packages/drivers/src/redshift.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
28 changes: 28 additions & 0 deletions packages/opencode/src/altimate/native/connections/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(", ")}`)
}

Expand All @@ -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) {
Expand Down
51 changes: 47 additions & 4 deletions packages/opencode/src/altimate/telemetry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -425,20 +425,57 @@ 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[]
}> = [
{ 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,
Expand All @@ -453,6 +490,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") {
Expand Down
23 changes: 19 additions & 4 deletions packages/opencode/src/tool/tool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
12 changes: 11 additions & 1 deletion packages/opencode/src/tool/webfetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 40 additions & 3 deletions packages/opencode/test/telemetry/telemetry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand All @@ -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")
Expand Down Expand Up @@ -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", () => {
Expand Down
Loading