Skip to content

Commit 7d9b09d

Browse files
authored
Merge branch 'main' into fix/doom-loop-escalation
2 parents 7671cf9 + b66d9f5 commit 7d9b09d

8 files changed

Lines changed: 566 additions & 12 deletions

File tree

packages/drivers/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
export type { Connector, ConnectorResult, SchemaColumn, ConnectionConfig } from "./types"
33

44
// Re-export config normalization
5-
export { normalizeConfig } from "./normalize"
5+
export { normalizeConfig, sanitizeConnectionString } from "./normalize"
66

77
// Re-export driver connect functions
88
export { connect as connectPostgres } from "./postgres"

packages/drivers/src/normalize.ts

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,95 @@ const DRIVER_ALIASES: Record<string, AliasMap> = {
111111
// duckdb and sqlite have simple configs — no aliases needed
112112
}
113113

114+
// ---------------------------------------------------------------------------
115+
// Connection string password encoding
116+
// ---------------------------------------------------------------------------
117+
118+
/**
119+
* URI-style connection strings (postgres://, mongodb://, etc.) embed
120+
* credentials in the userinfo section: `scheme://user:password@host/db`.
121+
* If the password contains special characters (@, #, :, /, ?, etc.) and
122+
* they are NOT percent-encoded, drivers will mis-parse the URI and fail
123+
* with cryptic auth errors.
124+
*
125+
* This function detects an unencoded password in the userinfo portion and
126+
* re-encodes it so the connection string is valid. Already-encoded
127+
* passwords (containing %XX sequences) are left untouched.
128+
*/
129+
export function sanitizeConnectionString(connectionString: string): string {
130+
// Only touch scheme://... URIs.
131+
const schemeMatch = connectionString.match(/^[a-zA-Z][a-zA-Z0-9+.-]*:\/\//)
132+
if (!schemeMatch) return connectionString
133+
134+
const scheme = schemeMatch[0]
135+
const afterScheme = connectionString.slice(scheme.length)
136+
137+
// Find the userinfo/host separator. A password can legitimately contain
138+
// '@', '#', '/', '?', or ':' characters, so the URI spec is ambiguous
139+
// when those are unencoded. We use the LAST '@' as the separator because
140+
// that correctly handles the common case of passwords with special
141+
// characters (the stated purpose of this function — see issue #589).
142+
//
143+
// The known trade-off: if the query string or path also contains an
144+
// unencoded '@' (e.g. `?email=alice@example.com`), the rightmost '@'
145+
// is NOT the userinfo separator. We detect this ambiguous case below
146+
// with a guard and bail to avoid corrupting the URI.
147+
const lastAt = afterScheme.lastIndexOf("@")
148+
if (lastAt < 0) return connectionString // No userinfo — nothing to fix
149+
150+
const beforeAt = afterScheme.slice(0, lastAt)
151+
const afterAt = afterScheme.slice(lastAt + 1)
152+
153+
// Ambiguity guard: if the content AFTER the '@' has no path/query/
154+
// fragment delimiter ('/', '?', or '#') but the content BEFORE the
155+
// '@' does, then the '@' is almost certainly inside a path, query,
156+
// or fragment — not the userinfo separator. Bail and leave the URI
157+
// untouched so the caller can pre-encode explicitly.
158+
//
159+
// Examples that trigger the guard (correctly left alone):
160+
// postgresql://u:p@host/db?email=alice@example.com ('@' in query)
161+
// postgresql://u:p@host:5432/db@archive ('@' in path)
162+
// postgresql://u:p@host/db#at@frag ('@' in fragment)
163+
//
164+
// Examples that pass the guard (correctly encoded):
165+
// postgresql://u:p@ss@host/db (last '@' has '/' after)
166+
// postgresql://u:f@ke#PLACEHOLDER@host/db (last '@' has '/' after)
167+
const afterAtHasDelim = /[/?#]/.test(afterAt)
168+
const beforeAtHasDelim = /[/?#]/.test(beforeAt)
169+
if (!afterAtHasDelim && beforeAtHasDelim) {
170+
return connectionString
171+
}
172+
173+
// Idempotent re-encoding: decode any existing percent-escapes and
174+
// re-encode. Already-encoded values round-trip unchanged; raw values
175+
// with special characters get encoded. Malformed percent sequences
176+
// fall back to encoding the raw input.
177+
const encodeIdempotent = (v: string): string => {
178+
if (v.length === 0) return v
179+
try {
180+
return encodeURIComponent(decodeURIComponent(v))
181+
} catch {
182+
return encodeURIComponent(v)
183+
}
184+
}
185+
186+
// Split userinfo on the FIRST ':' only (password may contain ':').
187+
const colonIdx = beforeAt.indexOf(":")
188+
let encodedUserinfo: string
189+
if (colonIdx < 0) {
190+
// Username-only userinfo: still encode if it contains special chars
191+
// (e.g. email addresses used as usernames).
192+
encodedUserinfo = encodeIdempotent(beforeAt)
193+
} else {
194+
const user = beforeAt.slice(0, colonIdx)
195+
const password = beforeAt.slice(colonIdx + 1)
196+
encodedUserinfo = `${encodeIdempotent(user)}:${encodeIdempotent(password)}`
197+
}
198+
199+
const rebuilt = `${scheme}${encodedUserinfo}@${afterAt}`
200+
return rebuilt === connectionString ? connectionString : rebuilt
201+
}
202+
114203
// ---------------------------------------------------------------------------
115204
// Core logic
116205
// ---------------------------------------------------------------------------
@@ -178,6 +267,15 @@ export function normalizeConfig(config: ConnectionConfig): ConnectionConfig {
178267
const aliases = DRIVER_ALIASES[type]
179268
let result = aliases ? applyAliases(config, aliases) : { ...config }
180269

270+
// Sanitize connection_string: if the password contains special characters
271+
// (@, #, :, /, etc.) that are not percent-encoded, URI-based drivers will
272+
// mis-parse the string and fail with cryptic auth errors. This is the
273+
// single integration point — every caller of normalizeConfig() gets the
274+
// fix automatically.
275+
if (typeof result.connection_string === "string") {
276+
result.connection_string = sanitizeConnectionString(result.connection_string)
277+
}
278+
181279
// Type-specific post-processing
182280
// Note: MySQL SSL fields (ssl_ca, ssl_cert, ssl_key) are NOT constructed
183281
// into an ssl object here. They stay as top-level fields so the credential

packages/drivers/test/clickhouse-unit.test.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,38 @@ describe("ClickHouse driver unit tests", () => {
203203
await connector.execute("SELECT 'it''s -- LIMIT 5' FROM t", 10)
204204
expect(mockQueryCalls[0].query).toContain("LIMIT 11")
205205
})
206+
207+
test("leading block comment does NOT bypass LIMIT injection", async () => {
208+
mockQueryResult = [{ id: 1 }]
209+
await connector.execute("/* analytics query */ SELECT * FROM t", 10)
210+
expect(mockQueryCalls[0].query).toContain("LIMIT 11")
211+
})
212+
213+
test("multi-line block comment before SELECT does NOT bypass LIMIT", async () => {
214+
mockQueryResult = [{ id: 1 }]
215+
await connector.execute("/*\n * Report query\n * Author: test\n */\nSELECT * FROM t", 10)
216+
expect(mockQueryCalls[0].query).toContain("LIMIT 11")
217+
})
218+
219+
test("backslash-escaped string does NOT break LIMIT check", async () => {
220+
mockQueryResult = [{ id: 1 }]
221+
await connector.execute("SELECT 'path\\\\to\\'s -- LIMIT 5' FROM t", 10)
222+
expect(mockQueryCalls[0].query).toContain("LIMIT 11")
223+
})
224+
225+
test("leading comment does NOT misroute SELECT as DDL", async () => {
226+
mockQueryResult = [{ id: 1 }]
227+
await connector.execute("-- INSERT note\nSELECT * FROM t", 10)
228+
expect(mockQueryCalls.length).toBe(1)
229+
expect(mockCommandCalls.length).toBe(0)
230+
})
231+
232+
test("block comment containing DDL keyword does NOT misroute SELECT", async () => {
233+
mockQueryResult = [{ id: 1 }]
234+
await connector.execute("/* DROP TABLE note */ SELECT * FROM t", 10)
235+
expect(mockQueryCalls.length).toBe(1)
236+
expect(mockCommandCalls.length).toBe(0)
237+
})
206238
})
207239

208240
// --- Truncation detection ---
@@ -285,6 +317,31 @@ describe("ClickHouse driver unit tests", () => {
285317
const cols = await connector.describeTable("default", "t")
286318
expect(cols[0].nullable).toBe(false)
287319
})
320+
321+
test("LowCardinality(Nullable(FixedString(32))) IS nullable — nested inner type", async () => {
322+
mockQueryResult = [{ name: "col1", type: "LowCardinality(Nullable(FixedString(32)))" }]
323+
const cols = await connector.describeTable("default", "t")
324+
expect(cols[0].nullable).toBe(true)
325+
})
326+
327+
test("Map(String, Nullable(UInt32)) is NOT nullable at column level", async () => {
328+
// The map values are nullable, but the column itself is not
329+
mockQueryResult = [{ name: "col1", type: "Map(String, Nullable(UInt32))" }]
330+
const cols = await connector.describeTable("default", "t")
331+
expect(cols[0].nullable).toBe(false)
332+
})
333+
334+
test("Tuple(Nullable(String), UInt32) is NOT nullable at column level", async () => {
335+
mockQueryResult = [{ name: "col1", type: "Tuple(Nullable(String), UInt32)" }]
336+
const cols = await connector.describeTable("default", "t")
337+
expect(cols[0].nullable).toBe(false)
338+
})
339+
340+
test("handles empty/missing type gracefully", async () => {
341+
mockQueryResult = [{ name: "col1", type: undefined }]
342+
const cols = await connector.describeTable("default", "t")
343+
expect(cols[0].nullable).toBe(false)
344+
})
288345
})
289346

290347
// --- Connection guard ---

packages/opencode/src/altimate/observability/tracing.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1000,6 +1000,32 @@ export class Trace {
10001000
}
10011001
}
10021002

1003+
/**
1004+
* List traces with pagination support.
1005+
* Returns a page of traces plus total count for building pagination UI.
1006+
*/
1007+
static async listTracesPaginated(
1008+
dir?: string,
1009+
options?: { offset?: number; limit?: number },
1010+
): Promise<{
1011+
traces: Array<{ sessionId: string; file: string; trace: TraceFile }>
1012+
total: number
1013+
offset: number
1014+
limit: number
1015+
}> {
1016+
const all = await Trace.listTraces(dir)
1017+
const rawOffset = options?.offset ?? 0
1018+
const rawLimit = options?.limit ?? 20
1019+
const offset = Number.isFinite(rawOffset) ? Math.max(0, Math.trunc(rawOffset)) : 0
1020+
const limit = Number.isFinite(rawLimit) ? Math.max(1, Math.trunc(rawLimit)) : 20
1021+
return {
1022+
traces: all.slice(offset, offset + limit),
1023+
total: all.length,
1024+
offset,
1025+
limit,
1026+
}
1027+
}
1028+
10031029
static async loadTrace(sessionId: string, dir?: string): Promise<TraceFile | null> {
10041030
const tracesDir = dir ?? DEFAULT_TRACES_DIR
10051031
try {

packages/opencode/src/cli/cmd/trace.ts

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,23 @@ function truncate(str: string, len: number): string {
4949
}
5050

5151
// altimate_change start — trace: list session traces (recordings/recaps of agent sessions)
52-
function listTraces(traces: Array<{ sessionId: string; trace: TraceFile }>, tracesDir?: string) {
53-
if (traces.length === 0) {
52+
function listTraces(
53+
traces: Array<{ sessionId: string; trace: TraceFile }>,
54+
pagination: { total: number; offset: number; limit: number },
55+
tracesDir?: string,
56+
) {
57+
if (traces.length === 0 && pagination.total === 0) {
5458
UI.println("No traces found. Run a command with tracing enabled:")
5559
UI.println(" altimate-code run \"your prompt here\"")
5660
return
5761
}
5862

63+
if (traces.length === 0 && pagination.total > 0) {
64+
UI.println(`No traces on this page (offset ${pagination.offset} past end of ${pagination.total} traces).`)
65+
UI.println(UI.Style.TEXT_DIM + `Try: altimate-code trace list --offset 0 --limit ${pagination.limit}` + UI.Style.TEXT_NORMAL)
66+
return
67+
}
68+
5969
// Header
6070
const header = [
6171
"DATE".padEnd(13),
@@ -97,8 +107,13 @@ function listTraces(traces: Array<{ sessionId: string; trace: TraceFile }>, trac
97107
}
98108

99109
UI.empty()
100-
// altimate_change start — trace: session trace messages
101-
UI.println(UI.Style.TEXT_DIM + `${traces.length} trace(s) in ${Trace.getTracesDir(tracesDir)}` + UI.Style.TEXT_NORMAL)
110+
// altimate_change start — trace: session trace messages with pagination footer
111+
const rangeStart = pagination.offset + 1
112+
const rangeEnd = pagination.offset + traces.length
113+
UI.println(UI.Style.TEXT_DIM + `Showing ${rangeStart}-${rangeEnd} of ${pagination.total} trace(s) in ${Trace.getTracesDir(tracesDir)}` + UI.Style.TEXT_NORMAL)
114+
if (rangeEnd < pagination.total) {
115+
UI.println(UI.Style.TEXT_DIM + `Next page: altimate-code trace list --offset ${rangeEnd} --limit ${pagination.limit}` + UI.Style.TEXT_NORMAL)
116+
}
102117
UI.println(UI.Style.TEXT_DIM + "View a trace: altimate-code trace view <session-id>" + UI.Style.TEXT_NORMAL)
103118
// altimate_change end
104119
}
@@ -134,6 +149,11 @@ export const TraceCommand = cmd({
134149
describe: "number of traces to show",
135150
default: 20,
136151
})
152+
.option("offset", {
153+
type: "number",
154+
describe: "number of traces to skip (for pagination)",
155+
default: 0,
156+
})
137157
.option("live", {
138158
type: "boolean",
139159
describe: "auto-refresh the viewer as the trace updates (for in-progress sessions)",
@@ -148,8 +168,16 @@ export const TraceCommand = cmd({
148168
const tracesDir = (cfg as any).tracing?.dir as string | undefined
149169

150170
if (action === "list") {
151-
const traces = await Trace.listTraces(tracesDir)
152-
listTraces(traces.slice(0, args.limit || 20), tracesDir)
171+
// Use nullish coalescing so an explicit 0 is preserved and reaches
172+
// listTracesPaginated() for clamping. `args.offset || 0` would
173+
// treat `--offset 0` as unset (no semantic change, harmless), but
174+
// `args.limit || 20` would promote `--limit 0` to 20 instead of
175+
// letting the API clamp it to 1.
176+
const page = await Trace.listTracesPaginated(tracesDir, {
177+
offset: args.offset ?? 0,
178+
limit: args.limit ?? 20,
179+
})
180+
listTraces(page.traces, page, tracesDir)
153181
return
154182
}
155183

@@ -168,7 +196,7 @@ export const TraceCommand = cmd({
168196
if (!match) {
169197
UI.error(`Trace not found: ${args.id}`)
170198
UI.println("Available traces:")
171-
listTraces(traces.slice(0, 10), tracesDir)
199+
listTraces(traces.slice(0, 10), { total: traces.length, offset: 0, limit: 10 }, tracesDir)
172200
process.exit(1)
173201
}
174202

packages/opencode/src/cli/cmd/tui/component/dialog-trace-list.tsx

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,16 @@ export function DialogTraceList(props: {
4747
}
4848
// altimate_change end
4949

50-
const items = traces() ?? []
50+
// Cap rendered items for TUI perf — DialogSelect creates reactive
51+
// nodes per item via <For>, so very large trace directories
52+
// (thousands of entries) can cause noticeable lag. Users with more
53+
// than MAX_TUI_ITEMS traces should use `altimate-code trace list
54+
// --offset N` from the CLI to navigate the full set.
55+
const MAX_TUI_ITEMS = 500
56+
const allItems = traces() ?? []
57+
const items =
58+
allItems.length > MAX_TUI_ITEMS ? allItems.slice(0, MAX_TUI_ITEMS) : allItems
59+
const truncated = allItems.length > MAX_TUI_ITEMS
5160
const today = new Date().toDateString()
5261
const result: Array<{ title: string; value: string; category: string; footer: string }> = []
5362

@@ -61,7 +70,7 @@ export function DialogTraceList(props: {
6170
})
6271
}
6372

64-
result.push(...items.slice(0, 50).map((item) => {
73+
result.push(...items.map((item) => {
6574
const rawStartedAt = item.trace.startedAt
6675
const parsedDate = typeof rawStartedAt === "string" || typeof rawStartedAt === "number"
6776
? new Date(rawStartedAt)
@@ -96,6 +105,16 @@ export function DialogTraceList(props: {
96105
}
97106
}))
98107

108+
// Append truncation hint if we capped the list
109+
if (truncated) {
110+
result.push({
111+
title: `... ${allItems.length - MAX_TUI_ITEMS} more not shown`,
112+
value: "__truncated__",
113+
category: "Older",
114+
footer: `Showing ${MAX_TUI_ITEMS} of ${allItems.length} — use CLI --offset to navigate`,
115+
})
116+
}
117+
99118
return result
100119
})
101120

@@ -113,7 +132,7 @@ export function DialogTraceList(props: {
113132
options={options()}
114133
current={props.currentSessionID}
115134
onSelect={(option) => {
116-
if (option.value === "__error__") {
135+
if (option.value === "__error__" || option.value === "__truncated__") {
117136
dialog.clear()
118137
return
119138
}

0 commit comments

Comments
 (0)