Skip to content

Commit 71d2ab0

Browse files
anandgupta42claude
anandgupta42
and
claude
committed
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 x 11 dialects x 14 use-case categories Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent d66354c commit 71d2ab0

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
@@ -34,15 +34,15 @@ export const SchemaInspectTool = Tool.define("schema_inspect", {
3434
}
3535
// altimate_change end
3636
return {
37-
title: `Schema: ${result.table}`,
38-
metadata: { columnCount: result.columns.length, rowCount: result.row_count },
37+
title: `Schema: ${result.table ?? args.table}`,
38+
metadata: { columnCount: (result.columns ?? []).length, rowCount: result.row_count },
3939
output,
4040
}
4141
} catch (e) {
4242
const msg = e instanceof Error ? e.message : String(e)
4343
return {
4444
title: "Schema: ERROR",
45-
metadata: { columnCount: 0, rowCount: undefined },
45+
metadata: { columnCount: 0, rowCount: undefined, error: msg },
4646
output: `Failed to inspect schema: ${msg}\n\nEnsure the dispatcher is running and a warehouse connection is configured.`,
4747
}
4848
}
@@ -51,17 +51,18 @@ export const SchemaInspectTool = Tool.define("schema_inspect", {
5151

5252
function formatSchema(result: SchemaInspectResult): string {
5353
const lines: string[] = []
54-
const qualified = result.schema_name ? `${result.schema_name}.${result.table}` : result.table
54+
const table = result.table ?? "unknown"
55+
const qualified = result.schema_name ? `${result.schema_name}.${table}` : table
5556
lines.push(`Table: ${qualified}`)
5657
if (result.row_count !== null && result.row_count !== undefined) {
5758
lines.push(`Rows: ${result.row_count.toLocaleString()}`)
5859
}
5960
lines.push("")
6061
lines.push("Column | Type | Nullable | PK")
6162
lines.push("-------|------|----------|---")
62-
for (const col of result.columns) {
63+
for (const col of result.columns ?? []) {
6364
lines.push(
64-
`${col.name} | ${col.data_type} | ${col.nullable ? "YES" : "NO"} | ${col.primary_key ? "YES" : ""}`,
65+
`${col.name} | ${col.data_type ?? "unknown"} | ${col.nullable ? "YES" : "NO"} | ${col.primary_key ? "YES" : ""}`,
6566
)
6667
}
6768
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
@@ -38,7 +38,7 @@ export const SqlAnalyzeTool = Tool.define("sql_analyze", {
3838
// there's an actual error (e.g. parse failure).
3939
const isRealFailure = !!result.error
4040
// altimate_change start — sql quality findings for telemetry
41-
const findings: Telemetry.Finding[] = result.issues.map((issue) => ({
41+
const findings: Telemetry.Finding[] = (result.issues ?? []).map((issue) => ({
4242
category: issue.rule ?? issue.type,
4343
}))
4444
// altimate_change end
@@ -56,7 +56,7 @@ export const SqlAnalyzeTool = Tool.define("sql_analyze", {
5656
}
5757
// altimate_change end
5858
return {
59-
title: `Analyze: ${result.error ? "ERROR" : `${result.issue_count} issue${result.issue_count !== 1 ? "s" : ""}`} [${result.confidence}]`,
59+
title: `Analyze: ${result.error ? "ERROR" : `${result.issue_count ?? 0} issue${(result.issue_count ?? 0) !== 1 ? "s" : ""}`} [${result.confidence ?? "unknown"}]`,
6060
metadata: {
6161
success: !isRealFailure,
6262
issueCount: result.issue_count,
@@ -91,24 +91,27 @@ function formatAnalysis(result: SqlAnalyzeResult): string {
9191
return `Analysis failed: ${result.error}`
9292
}
9393

94-
if (result.issues.length === 0) {
94+
const issues = result.issues ?? []
95+
if (issues.length === 0) {
9596
return "No anti-patterns or issues detected."
9697
}
9798

99+
const issueCount = result.issue_count ?? issues.length
98100
const lines: string[] = [
99-
`Found ${result.issue_count} issue${result.issue_count !== 1 ? "s" : ""} (confidence: ${result.confidence}):`,
101+
`Found ${issueCount} issue${issueCount !== 1 ? "s" : ""} (confidence: ${result.confidence ?? "unknown"}):`,
100102
]
101-
if (result.confidence_factors.length > 0) {
102-
lines.push(` Note: ${result.confidence_factors.join("; ")}`)
103+
const factors = result.confidence_factors ?? []
104+
if (factors.length > 0) {
105+
lines.push(` Note: ${factors.join("; ")}`)
103106
}
104107
lines.push("")
105108

106-
for (const issue of result.issues) {
109+
for (const issue of issues) {
107110
const loc = issue.location ? ` — ${issue.location}` : ""
108-
const conf = issue.confidence !== "high" ? ` [${issue.confidence} confidence]` : ""
109-
lines.push(` [${issue.severity.toUpperCase()}] ${issue.type}${conf}`)
110-
lines.push(` ${issue.message}${loc}`)
111-
lines.push(` → ${issue.recommendation}`)
111+
const conf = issue.confidence !== "high" ? ` [${issue.confidence ?? "unknown"} confidence]` : ""
112+
lines.push(` [${(issue.severity ?? "unknown").toUpperCase()}] ${issue.type ?? "unknown"}${conf}`)
113+
lines.push(` ${issue.message ?? ""}${loc}`)
114+
lines.push(` → ${issue.recommendation ?? ""}`)
112115
lines.push("")
113116
}
114117

‎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)