Skip to content

Commit 63b1dbd

Browse files
authored
Merge branch 'main' into feat/plan-agent-and-feature-discovery
2 parents 734d173 + ffe03b4 commit 63b1dbd

7 files changed

Lines changed: 162 additions & 15 deletions

File tree

packages/drivers/src/postgres.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,13 @@ export async function connect(config: ConnectionConfig): Promise<Connector> {
2222
if (config.connection_string) {
2323
poolConfig.connectionString = config.connection_string
2424
} else {
25+
// Validate required credentials before connecting to avoid cryptic
26+
// SASL/SCRAM errors from the pg driver when password is missing
27+
if (config.password != null && typeof config.password !== "string") {
28+
throw new Error(
29+
"PostgreSQL password must be a string. Check your warehouse configuration.",
30+
)
31+
}
2532
poolConfig.host = config.host ?? "127.0.0.1"
2633
poolConfig.port = config.port ?? 5432
2734
poolConfig.database = config.database ?? "postgres"
@@ -43,9 +50,10 @@ export async function connect(config: ConnectionConfig): Promise<Connector> {
4350
const client = await pool.connect()
4451
try {
4552
if (config.statement_timeout) {
46-
await client.query(
47-
`SET statement_timeout = '${Number(config.statement_timeout)}ms'`,
48-
)
53+
const timeoutMs = Number(config.statement_timeout)
54+
if (Number.isFinite(timeoutMs) && timeoutMs > 0) {
55+
await client.query(`SET statement_timeout TO ${Math.round(timeoutMs)}`)
56+
}
4957
}
5058

5159
let query = sql

packages/drivers/src/redshift.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@ export async function connect(config: ConnectionConfig): Promise<Connector> {
2525
if (config.connection_string) {
2626
poolConfig.connectionString = config.connection_string
2727
} else {
28+
// Validate password type to prevent cryptic SASL/SCRAM errors
29+
if (config.password != null && typeof config.password !== "string") {
30+
throw new Error(
31+
"Redshift password must be a string. Check your warehouse configuration.",
32+
)
33+
}
2834
poolConfig.host = config.host ?? "127.0.0.1"
2935
poolConfig.port = config.port ?? 5439 // Redshift default
3036
poolConfig.database = config.database ?? "dev"

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,18 @@ const DRIVER_MAP: Record<string, string> = {
133133
async function createConnector(name: string, config: ConnectionConfig): Promise<Connector> {
134134
const driverPath = DRIVER_MAP[config.type.toLowerCase()]
135135
if (!driverPath) {
136+
// altimate_change start — friendlier error for known-but-unsupported databases
137+
const KNOWN_UNSUPPORTED: Record<string, string> = {
138+
clickhouse: "ClickHouse is not yet supported. Use the bash tool with `clickhouse-client` or `curl` to query ClickHouse directly.",
139+
cassandra: "Cassandra is not yet supported. Use the bash tool with `cqlsh` to query Cassandra directly.",
140+
cockroachdb: "CockroachDB is not yet supported. It is PostgreSQL-compatible — try type: postgres instead.",
141+
timescaledb: "TimescaleDB is a PostgreSQL extension — use type: postgres instead.",
142+
}
143+
const hint = KNOWN_UNSUPPORTED[config.type.toLowerCase()]
144+
if (hint) {
145+
throw new Error(hint)
146+
}
147+
// altimate_change end
136148
throw new Error(`Unsupported database type: ${config.type}. Supported: ${Object.keys(DRIVER_MAP).join(", ")}`)
137149
}
138150

@@ -143,6 +155,22 @@ async function createConnector(name: string, config: ConnectionConfig): Promise<
143155
// Resolve credentials from keychain
144156
resolvedConfig = await resolveConfig(name, resolvedConfig)
145157

158+
// altimate_change start — validate password is a string for drivers that require it
159+
// Prevents cryptic SASL/SCRAM errors from database drivers
160+
const PASSWORD_DRIVERS = new Set(["postgres", "postgresql", "redshift", "mysql", "mariadb", "sqlserver", "mssql", "oracle", "snowflake"])
161+
if (
162+
PASSWORD_DRIVERS.has(resolvedConfig.type.toLowerCase()) &&
163+
!resolvedConfig.connection_string &&
164+
resolvedConfig.password != null &&
165+
typeof resolvedConfig.password !== "string"
166+
) {
167+
throw new Error(
168+
`Database password must be a string for ${resolvedConfig.type}. ` +
169+
"Check your warehouse configuration or re-add the connection with warehouse.add.",
170+
)
171+
}
172+
// altimate_change end
173+
146174
// Handle SSH tunnel
147175
const sshConfig = extractSshConfig(resolvedConfig)
148176
if (sshConfig) {

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

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,7 @@ export namespace Telemetry {
425425
session_id: string
426426
tool_name: string
427427
tool_category: string
428-
error_class: "parse_error" | "connection" | "timeout" | "validation" | "internal" | "permission" | "unknown"
428+
error_class: "parse_error" | "connection" | "timeout" | "validation" | "internal" | "permission" | "http_error" | "unknown"
429429
error_message: string
430430
input_signature: string
431431
masked_args?: string
@@ -447,20 +447,57 @@ export namespace Telemetry {
447447
}
448448
// altimate_change end
449449

450+
// altimate_change start — expanded error classification patterns for better triage
451+
// Order matters: earlier patterns take priority. Use specific phrases, not
452+
// single words, to avoid false positives (e.g., "connection refused" not "connection").
450453
const ERROR_PATTERNS: Array<{
451454
class: Telemetry.Event & { type: "core_failure" } extends { error_class: infer C } ? C : never
452455
keywords: string[]
453456
}> = [
454457
{ class: "parse_error", keywords: ["parse", "syntax", "binder", "unexpected token", "sqlglot"] },
455458
{
456459
class: "connection",
457-
keywords: ["econnrefused", "connection", "socket", "enotfound", "econnreset"],
460+
keywords: [
461+
"econnrefused",
462+
"enotfound",
463+
"econnreset",
464+
"connection refused",
465+
"connection reset",
466+
"connection closed",
467+
"connect failed",
468+
"connect etimedout",
469+
"socket hang up",
470+
"sasl",
471+
"scram",
472+
"password must be",
473+
"driver not installed",
474+
"not found. available:",
475+
"no warehouse configured",
476+
"unsupported database type",
477+
],
458478
},
459479
{ class: "timeout", keywords: ["timeout", "etimedout", "bridge timeout", "timed out"] },
460-
{ class: "permission", keywords: ["permission", "denied", "unauthorized", "forbidden"] },
461-
{ class: "validation", keywords: ["invalid params", "invalid", "missing", "required"] },
480+
{ class: "permission", keywords: ["permission", "access denied", "permission denied", "unauthorized", "forbidden", "authentication"] },
481+
{
482+
class: "validation",
483+
keywords: [
484+
"invalid params",
485+
"invalid",
486+
"missing",
487+
"required",
488+
"must read file",
489+
"has been modified since",
490+
"does not exist",
491+
"before overwriting",
492+
],
493+
},
462494
{ class: "internal", keywords: ["internal", "assertion"] },
495+
{
496+
class: "http_error",
497+
keywords: ["status code: 4", "status code: 5", "request failed with status"],
498+
},
463499
]
500+
// altimate_change end
464501

465502
export function classifyError(
466503
message: string,
@@ -475,6 +512,12 @@ export namespace Telemetry {
475512
export function computeInputSignature(args: Record<string, unknown>): string {
476513
const sig: Record<string, string> = {}
477514
for (const [k, v] of Object.entries(args)) {
515+
// altimate_change start — redact sensitive keys in input signatures
516+
if (isSensitiveKey(k)) {
517+
sig[k] = "****"
518+
continue
519+
}
520+
// altimate_change end
478521
if (v === null || v === undefined) {
479522
sig[k] = "null"
480523
} else if (typeof v === "string") {

packages/opencode/src/tool/tool.ts

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,10 +145,25 @@ export namespace Tool {
145145
}
146146
if (isSoftFailure) {
147147
// prettier-ignore
148-
const errorMsg =
149-
typeof result.metadata?.error === "string"
150-
? result.metadata.error
151-
: "unknown error"
148+
// altimate_change start — propagate non-string error metadata to avoid blind "unknown error"
149+
const rawError = result.metadata?.error
150+
let errorMsg: string
151+
if (typeof rawError === "string") {
152+
errorMsg = rawError
153+
} else if (rawError != null) {
154+
if (typeof rawError === "object") {
155+
try {
156+
errorMsg = Telemetry.maskArgs(rawError as Record<string, unknown>)
157+
} catch {
158+
errorMsg = `soft_failure:${id}:unserializable_error`
159+
}
160+
} else {
161+
errorMsg = String(rawError)
162+
}
163+
} else {
164+
errorMsg = `soft_failure:${id}:no_error_metadata`
165+
}
166+
// altimate_change end
152167
const maskedErrorMsg = Telemetry.maskString(errorMsg).slice(0, 500)
153168
Telemetry.track({
154169
type: "core_failure",

packages/opencode/src/tool/webfetch.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,17 @@ export const WebFetchTool = Tool.define("webfetch", {
8484
}
8585

8686
if (!response.ok) {
87-
throw new Error(`Request failed with status code: ${response.status}`)
87+
// altimate_change start — include URL domain in error for easier triage
88+
let domain: string
89+
try {
90+
domain = new URL(params.url).hostname
91+
} catch {
92+
domain = params.url.slice(0, 60)
93+
}
94+
throw new Error(
95+
`Request failed with status code: ${response.status} (${domain})`,
96+
)
97+
// altimate_change end
8898
}
8999

90100
// Check content length

packages/opencode/test/telemetry/telemetry.test.ts

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1506,6 +1506,16 @@ describe("telemetry.classifyError", () => {
15061506
expect(Telemetry.classifyError("Socket hang up")).toBe("connection")
15071507
expect(Telemetry.classifyError("ENOTFOUND db.example.com")).toBe("connection")
15081508
expect(Telemetry.classifyError("ECONNRESET")).toBe("connection")
1509+
// altimate_change start — expanded connection patterns
1510+
expect(Telemetry.classifyError("SASL: SCRAM-SERVER-FIRST-MESSAGE: client password must be a string")).toBe("connection")
1511+
expect(Telemetry.classifyError("password must be a string")).toBe("connection")
1512+
expect(Telemetry.classifyError("PostgreSQL driver not installed. Run: npm install pg")).toBe("connection")
1513+
expect(Telemetry.classifyError("Error: Connection mydb not found. Available: (none)")).toBe("connection")
1514+
expect(Telemetry.classifyError("No warehouse configured. Use warehouse.add")).toBe("connection")
1515+
expect(Telemetry.classifyError("Unsupported database type: clickhouse")).toBe("connection")
1516+
expect(Telemetry.classifyError("Connection reset by peer")).toBe("connection")
1517+
expect(Telemetry.classifyError("Connection closed unexpectedly")).toBe("connection")
1518+
// altimate_change end
15091519
})
15101520

15111521
test("classifies timeout errors", () => {
@@ -1520,20 +1530,47 @@ describe("telemetry.classifyError", () => {
15201530
expect(Telemetry.classifyError("Invalid dialect specified")).toBe("validation")
15211531
expect(Telemetry.classifyError("Missing required field")).toBe("validation")
15221532
expect(Telemetry.classifyError("Required parameter 'query' not provided")).toBe("validation")
1533+
// altimate_change start — expanded validation patterns
1534+
expect(Telemetry.classifyError("You must read file /path/to/file before overwriting it")).toBe("validation")
1535+
expect(Telemetry.classifyError("File has been modified since it was last read")).toBe("validation")
1536+
expect(Telemetry.classifyError("error: column foo does not exist")).toBe("validation")
1537+
expect(Telemetry.classifyError("You must read file before overwriting it. Use the Read tool first")).toBe("validation")
1538+
// altimate_change end
15231539
})
15241540

15251541
test("classifies permission errors", () => {
15261542
expect(Telemetry.classifyError("Permission denied on table")).toBe("permission")
15271543
expect(Telemetry.classifyError("Access denied for user")).toBe("permission")
15281544
expect(Telemetry.classifyError("Unauthorized access to resource")).toBe("permission")
15291545
expect(Telemetry.classifyError("403 Forbidden")).toBe("permission")
1546+
// altimate_change start — authentication classified as permission
1547+
expect(Telemetry.classifyError("Authentication failed for user admin")).toBe("permission")
1548+
// altimate_change end
15301549
})
15311550

15321551
test("classifies internal errors", () => {
15331552
expect(Telemetry.classifyError("Internal server error")).toBe("internal")
15341553
expect(Telemetry.classifyError("Assertion failed: x > 0")).toBe("internal")
15351554
})
15361555

1556+
// altimate_change start — http_error class and priority ordering tests
1557+
test("classifies http errors", () => {
1558+
expect(Telemetry.classifyError("Request failed with status code: 404 (example.com)")).toBe("http_error")
1559+
expect(Telemetry.classifyError("Request failed with status code: 500")).toBe("http_error")
1560+
expect(Telemetry.classifyError("status code: 403")).toBe("http_error")
1561+
expect(Telemetry.classifyError("Request failed with status")).toBe("http_error")
1562+
})
1563+
1564+
test("priority ordering: earlier patterns win over later ones", () => {
1565+
// SASL is connection, even though "authentication" is in permission
1566+
expect(Telemetry.classifyError("SASL authentication failed")).toBe("connection")
1567+
// parse_error wins over validation for "invalid syntax"
1568+
expect(Telemetry.classifyError("parse error: invalid syntax")).toBe("parse_error")
1569+
// permission ("forbidden") wins over http_error ("status code: 4")
1570+
expect(Telemetry.classifyError("403 Forbidden, status code: 403")).toBe("permission")
1571+
})
1572+
// altimate_change end
1573+
15371574
test("returns unknown for unrecognized errors", () => {
15381575
expect(Telemetry.classifyError("Something went wrong")).toBe("unknown")
15391576
expect(Telemetry.classifyError("")).toBe("unknown")
@@ -1602,11 +1639,11 @@ describe("telemetry.computeInputSignature", () => {
16021639
expect(sig).not.toContain("sk-abc123")
16031640
expect(sig).not.toContain("SELECT")
16041641
expect(sig).not.toContain("admin@example.com")
1605-
// Only type descriptors appear as values
1642+
// Only type descriptors appear as values; sensitive keys are fully redacted
16061643
const parsed = JSON.parse(sig)
16071644
expect(parsed.sql).toBe("string:60")
1608-
expect(parsed.secret).toBe("string:16")
1609-
expect(parsed.api_key).toBe("string:9")
1645+
expect(parsed.secret).toBe("****")
1646+
expect(parsed.api_key).toBe("****")
16101647
})
16111648

16121649
test("truncates output at 1000 chars with valid JSON", () => {

0 commit comments

Comments
 (0)