Skip to content

Commit 7293dd1

Browse files
anandgupta42claude
andcommitted
fix: defensive null guards in tool formatters and DuckDB concurrent access retry (#570)
- Add null/undefined guards across 8 tool formatters to prevent literal `undefined` in user-facing output (sql-analyze, schema-inspect, sql-translate, dbt-manifest, finops-analyze-credits, warehouse-list, altimate-core-check, altimate-core-rewrite) - Add `error: msg` to catch block metadata in schema-inspect, dbt-manifest, warehouse-list so telemetry can classify exceptions - DuckDB driver: auto-retry in `READ_ONLY` mode on `database is locked` errors, with clear actionable error message - Add simulation suite (839 mock + 346 real E2E scenarios) covering 10 personas × 11 dialects × 14 use-case categories Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent ffe03b4 commit 7293dd1

11 files changed

Lines changed: 3229 additions & 68 deletions

File tree

packages/drivers/src/duckdb.ts

Lines changed: 64 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,24 @@ export async function connect(config: ConnectionConfig): Promise<Connector> {
1717
let db: any
1818
let connection: any
1919

20+
// altimate_change start — improve DuckDB error messages
21+
function wrapDuckDBError(err: Error): Error {
22+
const msg = err.message || String(err)
23+
if (msg.includes("locked") || msg.includes("SQLITE_BUSY")) {
24+
return new Error(
25+
`Database "${dbPath}" is locked by another process. ` +
26+
`DuckDB does not support concurrent write access. ` +
27+
`Close other connections to this file and try again.`,
28+
)
29+
}
30+
return err
31+
}
32+
// altimate_change end
33+
2034
function query(sql: string): Promise<any[]> {
2135
return new Promise((resolve, reject) => {
2236
connection.all(sql, (err: Error | null, rows: any[]) => {
23-
if (err) reject(err)
37+
if (err) reject(wrapDuckDBError(err))
2438
else resolve(rows ?? [])
2539
})
2640
})
@@ -37,25 +51,56 @@ export async function connect(config: ConnectionConfig): Promise<Connector> {
3751

3852
return {
3953
async connect() {
40-
db = await new Promise<any>((resolve, reject) => {
41-
let resolved = false
42-
const instance = new duckdb.Database(
43-
dbPath,
44-
(err: Error | null) => {
45-
if (resolved) return // Already resolved via timeout
46-
resolved = true
47-
if (err) reject(err)
48-
else resolve(instance)
49-
},
50-
)
51-
// Bun: native callback may not fire; fall back after 2s
52-
setTimeout(() => {
53-
if (!resolved) {
54-
resolved = true
55-
resolve(instance)
54+
// altimate_change start — retry with read-only on lock errors
55+
const tryConnect = (accessMode?: string): Promise<any> =>
56+
new Promise<any>((resolve, reject) => {
57+
let resolved = false
58+
const opts = accessMode ? { access_mode: accessMode } : undefined
59+
const instance = new duckdb.Database(
60+
dbPath,
61+
opts,
62+
(err: Error | null) => {
63+
if (resolved) return
64+
resolved = true
65+
if (err) {
66+
const msg = err.message || String(err)
67+
if (msg.includes("locked") || msg.includes("SQLITE_BUSY")) {
68+
reject(new Error("DUCKDB_LOCKED"))
69+
} else {
70+
reject(err)
71+
}
72+
} else {
73+
resolve(instance)
74+
}
75+
},
76+
)
77+
setTimeout(() => {
78+
if (!resolved) {
79+
resolved = true
80+
resolve(instance)
81+
}
82+
}, 2000)
83+
})
84+
85+
try {
86+
db = await tryConnect()
87+
} catch (err: any) {
88+
if (err.message === "DUCKDB_LOCKED" && dbPath !== ":memory:") {
89+
// Retry in read-only mode — allows concurrent reads
90+
try {
91+
db = await tryConnect("READ_ONLY")
92+
} catch {
93+
throw new Error(
94+
`Database "${dbPath}" is locked by another process. ` +
95+
`DuckDB does not support concurrent write access. ` +
96+
`Close other connections to this file and try again.`,
97+
)
5698
}
57-
}, 2000)
58-
})
99+
} else {
100+
throw err
101+
}
102+
}
103+
// altimate_change end
59104
connection = db.connect()
60105
},
61106

packages/opencode/src/altimate/tools/altimate-core-check.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ export function formatCheck(data: Record<string, any>): string {
8484
lines.push("No lint findings.")
8585
} else {
8686
for (const f of data.lint?.findings ?? []) {
87-
lines.push(` [${f.severity}] ${f.rule}: ${f.message}`)
87+
lines.push(` [${f.severity ?? "warning"}] ${f.rule ?? "lint"}: ${f.message ?? ""}`)
8888
}
8989
}
9090

@@ -93,7 +93,7 @@ export function formatCheck(data: Record<string, any>): string {
9393
lines.push("Safe — no threats.")
9494
} else {
9595
for (const t of data.safety?.threats ?? []) {
96-
lines.push(` [${t.severity}] ${t.type}: ${t.description}`)
96+
lines.push(` [${t.severity ?? "warning"}] ${t.type ?? "safety"}: ${t.description ?? ""}`)
9797
}
9898
}
9999

@@ -102,7 +102,7 @@ export function formatCheck(data: Record<string, any>): string {
102102
lines.push("No PII detected.")
103103
} else {
104104
for (const p of data.pii?.findings ?? []) {
105-
lines.push(` ${p.column}: ${p.category} (${p.confidence} confidence)`)
105+
lines.push(` ${p.column ?? "unknown"}: ${p.category ?? "PII"} (${p.confidence ?? "unknown"} confidence)`)
106106
}
107107
}
108108

packages/opencode/src/altimate/tools/altimate-core-rewrite.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ function formatRewrite(data: Record<string, any>): string {
5454
}
5555
lines.push("Rewrites applied:")
5656
for (const r of suggestions) {
57-
lines.push(` - ${r.rule ?? r.type}: ${r.explanation ?? r.description ?? r.improvement}`)
57+
lines.push(` - ${r.rule ?? r.type ?? "rewrite"}: ${r.explanation ?? r.description ?? r.improvement ?? ""}`)
5858
}
5959
return lines.join("\n")
6060
}

packages/opencode/src/altimate/tools/dbt-manifest.ts

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ export const DbtManifestTool = Tool.define("dbt_manifest", {
1414
const result = await Dispatcher.call("dbt.manifest", { path: args.path })
1515

1616
return {
17-
title: `Manifest: ${result.model_count} models, ${result.source_count} sources`,
17+
title: `Manifest: ${result.model_count ?? 0} models, ${result.source_count ?? 0} sources`,
1818
metadata: {
1919
model_count: result.model_count,
2020
source_count: result.source_count,
@@ -28,7 +28,7 @@ export const DbtManifestTool = Tool.define("dbt_manifest", {
2828
const msg = e instanceof Error ? e.message : String(e)
2929
return {
3030
title: "Manifest: ERROR",
31-
metadata: { model_count: 0, source_count: 0, test_count: 0, snapshot_count: 0, seed_count: 0 },
31+
metadata: { model_count: 0, source_count: 0, test_count: 0, snapshot_count: 0, seed_count: 0, error: msg },
3232
output: `Failed to parse manifest: ${msg}\n\nEnsure the manifest.json exists and the dispatcher is running.`,
3333
}
3434
}
@@ -39,31 +39,36 @@ function formatManifest(result: DbtManifestResult): string {
3939
const lines: string[] = []
4040

4141
lines.push("=== Project Summary ===")
42-
lines.push(`Models: ${result.model_count}`)
43-
lines.push(`Sources: ${result.source_count}`)
44-
lines.push(`Tests: ${result.test_count}`)
45-
lines.push(`Snapshots: ${result.snapshot_count}`)
46-
lines.push(`Seeds: ${result.seed_count}`)
42+
lines.push(`Models: ${result.model_count ?? 0}`)
43+
lines.push(`Sources: ${result.source_count ?? 0}`)
44+
lines.push(`Tests: ${result.test_count ?? 0}`)
45+
lines.push(`Snapshots: ${result.snapshot_count ?? 0}`)
46+
lines.push(`Seeds: ${result.seed_count ?? 0}`)
4747

48-
if (result.models.length > 0) {
48+
const models = result.models ?? []
49+
const sources = result.sources ?? []
50+
51+
if (models.length > 0) {
4952
lines.push("")
5053
lines.push("=== Models ===")
5154
lines.push("Name | Schema | Materialized | Dependencies | Columns")
5255
lines.push("-----|--------|-------------|-------------|--------")
53-
for (const model of result.models) {
54-
const deps = model.depends_on.length > 0 ? model.depends_on.map((d) => d.split(".").pop()).join(", ") : "-"
55-
const cols = model.columns.length > 0 ? model.columns.map((c) => c.name).join(", ") : "-"
56+
for (const model of models) {
57+
const depsArr = model.depends_on ?? []
58+
const colsArr = model.columns ?? []
59+
const deps = depsArr.length > 0 ? depsArr.map((d) => d.split(".").pop()).join(", ") : "-"
60+
const cols = colsArr.length > 0 ? colsArr.map((c) => c.name).join(", ") : "-"
5661
lines.push(`${model.name} | ${model.schema_name ?? "-"} | ${model.materialized ?? "-"} | ${deps} | ${cols}`)
5762
}
5863
}
5964

60-
if (result.sources.length > 0) {
65+
if (sources.length > 0) {
6166
lines.push("")
6267
lines.push("=== Sources ===")
6368
lines.push("Source | Table | Schema | Columns")
6469
lines.push("-------|-------|--------|--------")
65-
for (const source of result.sources) {
66-
const cols = source.columns.length > 0 ? source.columns.map((c) => c.name).join(", ") : "-"
70+
for (const source of sources) {
71+
const cols = (source.columns ?? []).length > 0 ? (source.columns ?? []).map((c) => c.name).join(", ") : "-"
6772
lines.push(`${source.source_name} | ${source.name} | ${source.schema_name ?? "-"} | ${cols}`)
6873
}
6974
}

packages/opencode/src/altimate/tools/finops-analyze-credits.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -88,14 +88,16 @@ export const FinopsAnalyzeCreditsTool = Tool.define("finops_analyze_credits", {
8888
}
8989
}
9090

91+
const totalCredits = Number(result.total_credits ?? 0)
92+
const daysAnalyzed = result.days_analyzed ?? args.days
9193
return {
92-
title: `Credits: ${result.total_credits.toFixed(2)} over ${result.days_analyzed}d`,
93-
metadata: { success: true, total_credits: result.total_credits },
94+
title: `Credits: ${totalCredits.toFixed(2)} over ${daysAnalyzed}d`,
95+
metadata: { success: true, total_credits: totalCredits },
9496
output: formatCreditsAnalysis(
95-
result.total_credits as number,
96-
result.warehouse_summary as unknown[],
97-
result.recommendations as unknown[],
98-
result.daily_usage as unknown[],
97+
totalCredits,
98+
(result.warehouse_summary ?? []) as unknown[],
99+
(result.recommendations ?? []) as unknown[],
100+
(result.daily_usage ?? []) as unknown[],
99101
),
100102
}
101103
} catch (e) {

packages/opencode/src/altimate/tools/schema-inspect.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,15 @@ export const SchemaInspectTool = Tool.define("schema_inspect", {
1919
})
2020

2121
return {
22-
title: `Schema: ${result.table}`,
23-
metadata: { columnCount: result.columns.length, rowCount: result.row_count },
22+
title: `Schema: ${result.table ?? args.table}`,
23+
metadata: { columnCount: (result.columns ?? []).length, rowCount: result.row_count },
2424
output: formatSchema(result),
2525
}
2626
} catch (e) {
2727
const msg = e instanceof Error ? e.message : String(e)
2828
return {
2929
title: "Schema: ERROR",
30-
metadata: { columnCount: 0, rowCount: undefined },
30+
metadata: { columnCount: 0, rowCount: undefined, error: msg },
3131
output: `Failed to inspect schema: ${msg}\n\nEnsure the dispatcher is running and a warehouse connection is configured.`,
3232
}
3333
}
@@ -36,17 +36,18 @@ export const SchemaInspectTool = Tool.define("schema_inspect", {
3636

3737
function formatSchema(result: SchemaInspectResult): string {
3838
const lines: string[] = []
39-
const qualified = result.schema_name ? `${result.schema_name}.${result.table}` : result.table
39+
const table = result.table ?? "unknown"
40+
const qualified = result.schema_name ? `${result.schema_name}.${table}` : table
4041
lines.push(`Table: ${qualified}`)
4142
if (result.row_count !== null && result.row_count !== undefined) {
4243
lines.push(`Rows: ${result.row_count.toLocaleString()}`)
4344
}
4445
lines.push("")
4546
lines.push("Column | Type | Nullable | PK")
4647
lines.push("-------|------|----------|---")
47-
for (const col of result.columns) {
48+
for (const col of result.columns ?? []) {
4849
lines.push(
49-
`${col.name} | ${col.data_type} | ${col.nullable ? "YES" : "NO"} | ${col.primary_key ? "YES" : ""}`,
50+
`${col.name} | ${col.data_type ?? "unknown"} | ${col.nullable ? "YES" : "NO"} | ${col.primary_key ? "YES" : ""}`,
5051
)
5152
}
5253
return lines.join("\n")

packages/opencode/src/altimate/tools/sql-analyze.ts

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,12 @@ export const SqlAnalyzeTool = Tool.define("sql_analyze", {
3535
// there's an actual error (e.g. parse failure).
3636
const isRealFailure = !!result.error
3737
// altimate_change start — sql quality findings for telemetry
38-
const findings: Telemetry.Finding[] = result.issues.map((issue) => ({
38+
const findings: Telemetry.Finding[] = (result.issues ?? []).map((issue) => ({
3939
category: issue.rule ?? issue.type,
4040
}))
4141
// altimate_change end
4242
return {
43-
title: `Analyze: ${result.error ? "ERROR" : `${result.issue_count} issue${result.issue_count !== 1 ? "s" : ""}`} [${result.confidence}]`,
43+
title: `Analyze: ${result.error ? "ERROR" : `${result.issue_count ?? 0} issue${(result.issue_count ?? 0) !== 1 ? "s" : ""}`} [${result.confidence ?? "unknown"}]`,
4444
metadata: {
4545
success: !isRealFailure,
4646
issueCount: result.issue_count,
@@ -75,24 +75,27 @@ function formatAnalysis(result: SqlAnalyzeResult): string {
7575
return `Analysis failed: ${result.error}`
7676
}
7777

78-
if (result.issues.length === 0) {
78+
const issues = result.issues ?? []
79+
if (issues.length === 0) {
7980
return "No anti-patterns or issues detected."
8081
}
8182

83+
const issueCount = result.issue_count ?? issues.length
8284
const lines: string[] = [
83-
`Found ${result.issue_count} issue${result.issue_count !== 1 ? "s" : ""} (confidence: ${result.confidence}):`,
85+
`Found ${issueCount} issue${issueCount !== 1 ? "s" : ""} (confidence: ${result.confidence ?? "unknown"}):`,
8486
]
85-
if (result.confidence_factors.length > 0) {
86-
lines.push(` Note: ${result.confidence_factors.join("; ")}`)
87+
const factors = result.confidence_factors ?? []
88+
if (factors.length > 0) {
89+
lines.push(` Note: ${factors.join("; ")}`)
8790
}
8891
lines.push("")
8992

90-
for (const issue of result.issues) {
93+
for (const issue of issues) {
9194
const loc = issue.location ? ` — ${issue.location}` : ""
92-
const conf = issue.confidence !== "high" ? ` [${issue.confidence} confidence]` : ""
93-
lines.push(` [${issue.severity.toUpperCase()}] ${issue.type}${conf}`)
94-
lines.push(` ${issue.message}${loc}`)
95-
lines.push(` → ${issue.recommendation}`)
95+
const conf = issue.confidence !== "high" ? ` [${issue.confidence ?? "unknown"} confidence]` : ""
96+
lines.push(` [${(issue.severity ?? "unknown").toUpperCase()}] ${issue.type ?? "unknown"}${conf}`)
97+
lines.push(` ${issue.message ?? ""}${loc}`)
98+
lines.push(` → ${issue.recommendation ?? ""}`)
9699
lines.push("")
97100
}
98101

packages/opencode/src/altimate/tools/sql-translate.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ export const SqlTranslateTool = Tool.define("sql_translate", {
2929
success: result.success,
3030
source_dialect: result.source_dialect,
3131
target_dialect: result.target_dialect,
32-
warningCount: result.warnings.length,
32+
warningCount: (result.warnings ?? []).length,
3333
...(result.error && { error: result.error }),
3434
},
3535
output: formatTranslation(result, args.sql),
@@ -70,9 +70,10 @@ function formatTranslation(result: SqlTranslateResult, originalSql: string): str
7070
lines.push(result.translated_sql ?? "")
7171
lines.push("")
7272

73-
if (result.warnings.length > 0) {
73+
const warnings = result.warnings ?? []
74+
if (warnings.length > 0) {
7475
lines.push("--- Warnings ---")
75-
for (const warning of result.warnings) {
76+
for (const warning of warnings) {
7677
lines.push(` ! ${warning}`)
7778
}
7879
lines.push("")

packages/opencode/src/altimate/tools/warehouse-list.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ export const WarehouseListTool = Tool.define("warehouse_list", {
99
try {
1010
const result = await Dispatcher.call("warehouse.list", {})
1111

12-
if (result.warehouses.length === 0) {
12+
const warehouses = result.warehouses ?? []
13+
if (warehouses.length === 0) {
1314
return {
1415
title: "Warehouses: none configured",
1516
metadata: { count: 0 },
@@ -18,20 +19,20 @@ export const WarehouseListTool = Tool.define("warehouse_list", {
1819
}
1920

2021
const lines: string[] = ["Name | Type | Database", "-----|------|--------"]
21-
for (const wh of result.warehouses) {
22+
for (const wh of warehouses) {
2223
lines.push(`${wh.name} | ${wh.type} | ${wh.database ?? "-"}`)
2324
}
2425

2526
return {
26-
title: `Warehouses: ${result.warehouses.length} configured`,
27-
metadata: { count: result.warehouses.length },
27+
title: `Warehouses: ${warehouses.length} configured`,
28+
metadata: { count: warehouses.length },
2829
output: lines.join("\n"),
2930
}
3031
} catch (e) {
3132
const msg = e instanceof Error ? e.message : String(e)
3233
return {
3334
title: "Warehouses: ERROR",
34-
metadata: { count: 0 },
35+
metadata: { count: 0, error: msg },
3536
output: `Failed to list warehouses: ${msg}\n\nCheck your connection configuration and try again.`,
3637
}
3738
}

0 commit comments

Comments
 (0)