Skip to content

Commit 6e5e9d4

Browse files
anandgupta42claude
andcommitted
fix: improve tool reliability — #469 #470 #471 #473
**#469 — `sql_execute` crash: `getStatementTypes is not a function`** - Safe-import `@altimateai/altimate-core` with `typeof` guard - Regex fallback classifier when napi binary unavailable - Null/undefined input guards on `classify()` and `classifyAndCheck()` - 42 adversarial tests (fallback parity, ReDoS, bypass attempts) **#470 — `edit` tool: "Could not find oldString" context drift** - `buildNotFoundMessage()` finds closest-matching line via Levenshtein - Error now shows line number + 5-line snippet of actual file content - Tells model to re-read the file instead of retrying blindly - 14 adversarial tests (similarity scoring, truncation, edge cases) **#471 — `webfetch`: 934 daily failures from invalid/broken URLs** - `URL` constructor validation before fetch (catches malformed URLs) - Session-level 404/410/451 failure cache with 5-min TTL - Actionable error messages per HTTP status (404: "Do NOT retry", 429: includes `Retry-After`, 500: "transient — retry once") - 24 tests (validation, cache TTL, error messages, edge cases) **#473 — Bump `yaml` 2.8.2 → 2.8.3** - Stack overflow fix during node composition (security) Closes #469, Closes #470, Closes #471, Closes #473 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent c36d6f2 commit 6e5e9d4

8 files changed

Lines changed: 856 additions & 25 deletions

File tree

bun.lock

Lines changed: 8 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/opencode/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@
137137
"web-tree-sitter": "0.25.10",
138138
"which": "6.0.1",
139139
"xdg-basedir": "5.1.0",
140-
"yaml": "2.8.2",
140+
"yaml": "2.8.3",
141141
"yargs": "18.0.0",
142142
"zod": "catalog:",
143143
"zod-to-json-schema": "3.24.5"

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

Lines changed: 53 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,27 +2,57 @@
22
//
33
// Uses altimate-core's AST-based getStatementTypes() for accurate classification.
44
// Handles CTEs, string literals, procedural blocks, all dialects correctly.
5+
// Falls back to regex-based heuristics if the napi binary fails to load.
56

6-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
7-
const core: any = require("@altimateai/altimate-core")
7+
// Safe import: napi binary may not be available on all platforms
8+
let getStatementTypes: ((sql: string, dialect?: string | null) => any) | null = null
9+
try {
10+
// eslint-disable-next-line @typescript-eslint/no-require-imports
11+
const core = require("@altimateai/altimate-core")
12+
if (typeof core?.getStatementTypes === "function") {
13+
getStatementTypes = core.getStatementTypes
14+
}
15+
} catch {
16+
// napi binary failed to load — will use regex fallback
17+
}
818

9-
// Categories from altimate-core that indicate write operations
10-
const WRITE_CATEGORIES = new Set(["dml", "ddl", "dcl", "tcl"])
1119
// Only SELECT queries are known safe. "other" (SHOW, SET, USE, etc.) is ambiguous — prompt for permission.
1220
const READ_CATEGORIES = new Set(["query"])
1321

1422
// Hard-deny patterns — blocked regardless of permissions
1523
const HARD_DENY_TYPES = new Set(["DROP DATABASE", "DROP SCHEMA", "TRUNCATE", "TRUNCATE TABLE"])
1624

25+
// Regex fallback: patterns that indicate write operations (case-insensitive, anchored to statement start)
26+
const WRITE_PATTERN =
27+
/^\s*(INSERT|UPDATE|DELETE|MERGE|CREATE|ALTER|DROP|TRUNCATE|GRANT|REVOKE|CALL|EXEC)\b/i
28+
const HARD_DENY_PATTERN =
29+
/^\s*(DROP\s+(DATABASE|SCHEMA)\b|TRUNCATE(\s+TABLE)?\s)/i
30+
31+
/**
32+
* Regex-based fallback classifier for when altimate-core is unavailable.
33+
* Conservative: treats anything not clearly a SELECT/WITH/SHOW/EXPLAIN as "write".
34+
*/
35+
function classifyFallback(sql: string): { queryType: "read" | "write"; blocked: boolean } {
36+
const trimmed = sql.replace(/\/\*[\s\S]*?\*\//g, "").trim()
37+
const blocked = HARD_DENY_PATTERN.test(trimmed)
38+
const queryType = WRITE_PATTERN.test(trimmed) ? "write" : "read"
39+
return { queryType, blocked }
40+
}
41+
1742
/**
1843
* Classify a SQL string as "read" or "write" using AST parsing.
1944
* If ANY statement is a write, returns "write".
2045
*/
2146
export function classify(sql: string): "read" | "write" {
22-
const result = core.getStatementTypes(sql)
23-
if (!result?.categories?.length) return "read"
24-
// Treat unknown categories (not in WRITE or READ sets) as write to fail safe
25-
return result.categories.some((c: string) => !READ_CATEGORIES.has(c)) ? "write" : "read"
47+
if (!sql || typeof sql !== "string") return "read"
48+
if (!getStatementTypes) return classifyFallback(sql).queryType
49+
try {
50+
const result = getStatementTypes(sql)
51+
if (!result?.categories?.length) return "read"
52+
return result.categories.some((c: string) => !READ_CATEGORIES.has(c)) ? "write" : "read"
53+
} catch {
54+
return classifyFallback(sql).queryType
55+
}
2656
}
2757

2858
/**
@@ -38,15 +68,21 @@ export function classifyMulti(sql: string): "read" | "write" {
3868
* Returns both the overall query type and whether a hard-deny pattern was found.
3969
*/
4070
export function classifyAndCheck(sql: string): { queryType: "read" | "write"; blocked: boolean } {
41-
const result = core.getStatementTypes(sql)
42-
if (!result?.statements?.length) return { queryType: "read", blocked: false }
71+
if (!sql || typeof sql !== "string") return { queryType: "read", blocked: false }
72+
if (!getStatementTypes) return classifyFallback(sql)
73+
try {
74+
const result = getStatementTypes(sql)
75+
if (!result?.statements?.length) return { queryType: "read", blocked: false }
4376

44-
const blocked = result.statements.some((s: { statement_type: string }) =>
45-
s.statement_type && HARD_DENY_TYPES.has(s.statement_type.toUpperCase()),
46-
)
77+
const blocked = result.statements.some(
78+
(s: { statement_type: string }) =>
79+
s.statement_type && HARD_DENY_TYPES.has(s.statement_type.toUpperCase()),
80+
)
4781

48-
const categories = result.categories ?? []
49-
// Unknown categories (not in WRITE or READ sets) are treated as write to fail safe
50-
const queryType = categories.some((c: string) => !READ_CATEGORIES.has(c)) ? "write" : "read"
51-
return { queryType: queryType as "read" | "write", blocked }
82+
const categories = result.categories ?? []
83+
const queryType = categories.some((c: string) => !READ_CATEGORIES.has(c)) ? "write" : "read"
84+
return { queryType: queryType as "read" | "write", blocked }
85+
} catch {
86+
return classifyFallback(sql)
87+
}
5288
}

packages/opencode/src/tool/edit.ts

Lines changed: 62 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -629,6 +629,67 @@ export function trimDiff(diff: string): string {
629629
return trimmedLines.join("\n")
630630
}
631631

632+
/**
633+
* Build a helpful error message when oldString isn't found.
634+
* Includes a snippet of the closest-matching region so the model can self-correct.
635+
*/
636+
export function buildNotFoundMessage(content: string, oldString: string): string {
637+
const base = "Could not find oldString in the file."
638+
639+
// Find the first line of oldString and search for it in the file
640+
const firstLine = oldString.split("\n")[0].trim()
641+
if (!firstLine) return base + " The oldString appears to be empty or whitespace-only."
642+
643+
const contentLines = content.split("\n")
644+
let bestLine = -1
645+
let bestScore = 0
646+
647+
// Search for the line with highest similarity to the first line of oldString
648+
for (let i = 0; i < contentLines.length; i++) {
649+
const trimmed = contentLines[i].trim()
650+
if (!trimmed) continue
651+
652+
// Skip very short lines — they produce false similarity matches
653+
const minLen = Math.min(trimmed.length, firstLine.length)
654+
if (minLen < 4) continue
655+
656+
// Exact substring match is best
657+
if (trimmed.includes(firstLine) || firstLine.includes(trimmed)) {
658+
bestLine = i
659+
bestScore = 1
660+
break
661+
}
662+
663+
// Skip if lengths are too different (>3x ratio) — not a meaningful comparison
664+
const maxLen = Math.max(trimmed.length, firstLine.length)
665+
if (minLen * 3 < maxLen) continue
666+
667+
// Levenshtein similarity for close matches
668+
const score = 1 - levenshtein(trimmed, firstLine) / maxLen
669+
if (score > bestScore && score > 0.6) {
670+
bestScore = score
671+
bestLine = i
672+
}
673+
}
674+
675+
if (bestLine === -1) {
676+
return base + ` The first line of your oldString ("${firstLine.slice(0, 80)}") was not found anywhere in the file. Re-read the file before editing.`
677+
}
678+
679+
// Show a small window around the best match
680+
const start = Math.max(0, bestLine - 1)
681+
const end = Math.min(contentLines.length, bestLine + 4)
682+
const snippet = contentLines
683+
.slice(start, end)
684+
.map((l, i) => ` ${start + i + 1} | ${l}`)
685+
.join("\n")
686+
687+
return (
688+
base +
689+
` A similar line was found at line ${bestLine + 1}. The file may have changed since you last read it.\n\nNearest match:\n${snippet}\n\nRe-read the file and use the exact current content for oldString.`
690+
)
691+
}
692+
632693
export function replace(content: string, oldString: string, newString: string, replaceAll = false): string {
633694
if (oldString === newString) {
634695
throw new Error("No changes to apply: oldString and newString are identical.")
@@ -661,9 +722,7 @@ export function replace(content: string, oldString: string, newString: string, r
661722
}
662723

663724
if (notFound) {
664-
throw new Error(
665-
"Could not find oldString in the file. It must match exactly, including whitespace, indentation, and line endings.",
666-
)
725+
throw new Error(buildNotFoundMessage(content, oldString))
667726
}
668727
throw new Error("Found multiple matches for oldString. Provide more surrounding context to make the match unique.")
669728
}

packages/opencode/src/tool/webfetch.ts

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,51 @@ const BROWSER_UA =
1515
// Status codes that warrant a retry with a different User-Agent
1616
const RETRYABLE_STATUSES = new Set([403, 406])
1717

18+
// altimate_change start — session-level URL failure cache (#471)
19+
// Prevents repeated fetches to URLs that already returned 404/410 in this session.
20+
// Keyed by URL string. Cleared when the process restarts (new session).
21+
const failedUrls = new Map<string, { status: number; timestamp: number }>()
22+
const FAILURE_CACHE_TTL = 5 * 60 * 1000 // 5 minutes
23+
24+
function isUrlCachedFailure(url: string): { status: number } | null {
25+
const entry = failedUrls.get(url)
26+
if (!entry) return null
27+
if (Date.now() - entry.timestamp > FAILURE_CACHE_TTL) {
28+
failedUrls.delete(url)
29+
return null
30+
}
31+
return { status: entry.status }
32+
}
33+
34+
function cacheUrlFailure(url: string, status: number): void {
35+
// Only cache permanent-ish failures, not transient ones
36+
if (status === 404 || status === 410 || status === 451) {
37+
failedUrls.set(url, { status, timestamp: Date.now() })
38+
}
39+
}
40+
41+
/** Build an actionable error message so the model knows whether to retry. */
42+
function buildFetchError(url: string, status: number, headers?: Headers): string {
43+
switch (status) {
44+
case 404:
45+
return `HTTP 404: ${url} does not exist. Do NOT retry this URL — it will fail again. Try a different URL or search for the correct page.`
46+
case 410:
47+
return `HTTP 410: ${url} has been permanently removed. Do NOT retry. Find an alternative resource.`
48+
case 403:
49+
return `HTTP 403: Access to ${url} is forbidden. The server rejected both bot and browser User-Agents. Try a different source.`
50+
case 429: {
51+
const retryAfter = headers?.get("retry-after")
52+
const wait = retryAfter ? ` (retry after ${retryAfter}s)` : ""
53+
return `HTTP 429: Rate limited by ${new URL(url).hostname}${wait}. Wait before fetching from this domain again, or use a different source.`
54+
}
55+
case 451:
56+
return `HTTP 451: ${url} is unavailable for legal reasons. Do NOT retry.`
57+
default:
58+
return `HTTP ${status}: Request to ${url} failed. This may be transient — retry once if needed.`
59+
}
60+
}
61+
// altimate_change end
62+
1863
export const WebFetchTool = Tool.define("webfetch", {
1964
description: DESCRIPTION,
2065
parameters: z.object({
@@ -26,10 +71,23 @@ export const WebFetchTool = Tool.define("webfetch", {
2671
timeout: z.number().describe("Optional timeout in seconds (max 120)").optional(),
2772
}),
2873
async execute(params, ctx) {
29-
// Validate URL
74+
// altimate_change start — URL validation and failure cache (#471)
75+
// Validate URL format
3076
if (!params.url.startsWith("http://") && !params.url.startsWith("https://")) {
3177
throw new Error("URL must start with http:// or https://")
3278
}
79+
try {
80+
new URL(params.url)
81+
} catch {
82+
throw new Error(`Invalid URL: "${params.url.slice(0, 200)}" is not a valid URL. Check the format and try again.`)
83+
}
84+
85+
// Check failure cache — avoid re-fetching URLs that already returned 404/410
86+
const cached = isUrlCachedFailure(params.url)
87+
if (cached) {
88+
throw new Error(buildFetchError(params.url, cached.status))
89+
}
90+
// altimate_change end
3391

3492
await ctx.ask({
3593
permission: "webfetch",
@@ -83,9 +141,12 @@ export const WebFetchTool = Tool.define("webfetch", {
83141
response = await fetch(params.url, { signal, headers: browserHeaders })
84142
}
85143

144+
// altimate_change start — actionable error messages and failure caching (#471)
86145
if (!response.ok) {
87-
throw new Error(`Request failed with status code: ${response.status}`)
146+
cacheUrlFailure(params.url, response.status)
147+
throw new Error(buildFetchError(params.url, response.status, response.headers))
88148
}
149+
// altimate_change end
89150

90151
// Check content length
91152
const contentLength = response.headers.get("content-length")

0 commit comments

Comments
 (0)