Skip to content

Commit 04a807e

Browse files
anandgupta42claude
andauthored
fix: address multi-model code review findings for recent merges (#307)
- Fix TUI trace dialog ignoring custom `tracing.dir` config — plumb config dir through `DialogTraceList`, `getTraceViewerUrl`, and `openTraceInBrowser` so custom trace directories work in the TUI - Fix WebFetch `clearTimeout` leak — move both `fetch` calls inside the `try` block so the `finally` clause always clears the timer, even on DNS failures or `AbortError` - Fix `cleanTitle` empty string fallback — add `"(Untitled)"` as final fallback when all parsing yields empty - Add error logging to `openTraceInBrowser` — log actual error before showing toast so failures are debuggable - Add `altimate_change` markers around `HONEST_UA` branding constant in `webfetch.ts` for upstream merge compatibility - Update tracing docs for new `/trace` dialog behavior - Update TUI docs to include `/trace` in slash command examples Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent b1e6520 commit 04a807e

6 files changed

Lines changed: 56 additions & 43 deletions

File tree

.github/meta/commit.txt

Lines changed: 15 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,18 @@
1-
fix: harden auth field handling for all warehouse drivers (#267)
1+
fix: address multi-model code review findings for recent merges
22

3-
Add config field name normalization so LLM-generated, dbt, and SDK
4-
field names all resolve to canonical snake_case before reaching drivers.
5-
6-
- Add `normalizeConfig()` in `@altimateai/drivers` with per-driver alias
7-
maps (Snowflake, BigQuery, Databricks, Redshift, PostgreSQL, MySQL,
8-
SQL Server, Oracle) and common aliases (`username` → `user`,
9-
`dbname` → `database`)
10-
- Call `normalizeConfig()` before both `resolveConfig()` and
11-
`saveConnection()` in registry so credential store uses canonical names
12-
- Add Snowflake inline PEM support via `config.private_key`
13-
- Add BigQuery `credentials_json` (inline JSON) support
14-
- Add MySQL SSL construction from top-level `ssl_ca`/`ssl_cert`/`ssl_key`
15-
fields at connection time (not during normalization, to preserve
16-
credential store detection)
17-
- Expand `SENSITIVE_FIELDS` with `private_key`, `token`,
18-
`credentials_json`, `keyfile_json`, `ssl_key`, `ssl_cert`, `ssl_ca`
19-
- Update `sensitiveKeys` in `project-scan.ts` to match
20-
- Fix `detectAuthMethod()` to recognize inline `private_key`
21-
- Update `warehouse_add` tool description with per-warehouse field docs
22-
- Add 72 unit tests for normalization logic
23-
24-
Closes #267
3+
- Fix TUI trace dialog ignoring custom `tracing.dir` config — plumb
4+
config dir through `DialogTraceList`, `getTraceViewerUrl`, and
5+
`openTraceInBrowser` so custom trace directories work in the TUI
6+
- Fix WebFetch `clearTimeout` leak — move both `fetch` calls inside the
7+
`try` block so the `finally` clause always clears the timer, even on
8+
DNS failures or `AbortError`
9+
- Fix `cleanTitle` empty string fallback — add `"(Untitled)"` as final
10+
fallback when all parsing yields empty
11+
- Add error logging to `openTraceInBrowser` — log actual error before
12+
showing toast so failures are debuggable
13+
- Add `altimate_change` markers around `HONEST_UA` branding constant in
14+
`webfetch.ts` for upstream merge compatibility
15+
- Update tracing docs for new `/trace` dialog behavior
16+
- Update TUI docs to include `/trace` in slash command examples
2517

2618
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

docs/docs/configure/tracing.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,9 @@ The `--live` flag adds a green "LIVE" indicator and polls for updates every 2 se
144144

145145
### From the TUI
146146

147-
Type `/trace` in the TUI to open the trace viewer for the current session in your browser. The viewer launches in live mode automatically, so you can watch spans appear as the agent works.
147+
Type `/trace` in the TUI to open a trace history dialog listing all recent sessions. Select any trace to open it in your browser with the interactive viewer. The current session appears at the top, and traces are grouped by date with duration and timestamp info.
148+
149+
The viewer launches in live mode automatically for in-progress sessions, so you can watch spans appear as the agent works.
148150

149151
## Remote Exporters
150152

docs/docs/usage/tui.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ The TUI has three main areas:
2020
|--------|--------|---------|
2121
| `@` | Reference a file | `@src/models/user.sql explain this model` |
2222
| `!` | Run a shell command | `!dbt run --select my_model` |
23-
| `/` | Slash command | `/discover`, `/connect`, `/review`, `/models`, `/theme` |
23+
| `/` | Slash command | `/discover`, `/connect`, `/review`, `/models`, `/theme`, `/trace` |
2424

2525
## Leader Key
2626

packages/opencode/src/cli/cmd/tui/app.tsx

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,10 @@ import fsAsync from "fs/promises"
3535

3636
// altimate_change start - shared trace viewer server
3737
let traceViewerServer: ReturnType<typeof Bun.serve> | undefined
38-
function getTraceViewerUrl(sessionID: string): string {
38+
let traceViewerTracesDir: string | undefined
39+
function getTraceViewerUrl(sessionID: string, tracesDir?: string): string {
3940
if (!traceViewerServer) {
40-
const tracesDir = Tracer.getTracesDir()
41+
traceViewerTracesDir = Tracer.getTracesDir(tracesDir)
4142
traceViewerServer = Bun.serve({
4243
port: 0, // random available port
4344
hostname: "127.0.0.1",
@@ -56,7 +57,7 @@ function getTraceViewerUrl(sessionID: string): string {
5657
}
5758

5859
const safeId = sid.replace(/[/\\.:]/g, "_")
59-
const traceFile = `${tracesDir}/${safeId}.json`
60+
const traceFile = `${traceViewerTracesDir}/${safeId}.json`
6061

6162
if (action === "api") {
6263
try {
@@ -273,21 +274,34 @@ function App() {
273274
const promptRef = usePromptRef()
274275

275276
// altimate_change start - shared trace viewer helper
277+
// Load custom tracing dir from config (same as worker.ts and trace.ts)
278+
const [tracesDir, setTracesDir] = createSignal<string | undefined>(undefined)
279+
onMount(async () => {
280+
try {
281+
const { Config } = await import("@/config/config")
282+
const cfg = await Config.get()
283+
setTracesDir(cfg.tracing?.dir)
284+
} catch {
285+
// Config failure should not prevent TUI from working
286+
}
287+
})
288+
276289
async function openTraceInBrowser(sessionID: string) {
277290
try {
278291
// Check if trace file exists on disk before opening browser
279292
const safeId = sessionID.replace(/[/\\.:]/g, "_")
280-
const traceFile = `${Tracer.getTracesDir()}/${safeId}.json`
293+
const traceFile = `${Tracer.getTracesDir(tracesDir())}/${safeId}.json`
281294
const exists = await fsAsync.access(traceFile).then(() => true).catch(() => false)
282295
if (!exists) {
283296
toast.show({ variant: "warning", message: "Trace not available yet — send a prompt first", duration: 4000 })
284297
return
285298
}
286-
const url = getTraceViewerUrl(sessionID)
299+
const url = getTraceViewerUrl(sessionID, tracesDir())
287300
await open(url)
288301
toast.show({ variant: "info", message: `Trace viewer: ${url}`, duration: 6000 })
289-
} catch {
290-
toast.show({ variant: "warning", message: `Failed to open browser. Trace files: ${Tracer.getTracesDir()}`, duration: 8000 })
302+
} catch (err) {
303+
Log.Default.error(`Failed to open trace viewer: ${err}`)
304+
toast.show({ variant: "warning", message: `Failed to open browser. Trace files: ${Tracer.getTracesDir(tracesDir())}`, duration: 8000 })
291305
}
292306
}
293307
// altimate_change end
@@ -683,6 +697,7 @@ function App() {
683697
dialog.replace(() => (
684698
<DialogTraceList
685699
currentSessionID={currentSessionID}
700+
tracesDir={tracesDir()}
686701
onSelect={openTraceInBrowser}
687702
/>
688703
))

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ function cleanTitle(raw: unknown): string {
1010
// Strip quotes, markdown headings, and take first non-empty line
1111
const stripped = raw.replace(/^["'`]+|["'`]+$/g, "").trim()
1212
const lines = stripped.split("\n").map((l) => l.replace(/^#+\s*/, "").trim()).filter(Boolean)
13-
return lines.find((l) => l.length > 5) || lines[0] || stripped
13+
return lines.find((l) => l.length > 5) || lines[0] || stripped || "(Untitled)"
1414
}
1515

1616
function formatDuration(ms: number): string {
@@ -23,12 +23,13 @@ function formatDuration(ms: number): string {
2323

2424
export function DialogTraceList(props: {
2525
currentSessionID?: string
26+
tracesDir?: string
2627
onSelect: (sessionID: string) => void
2728
}) {
2829
const dialog = useDialog()
2930

3031
const [traces] = createResource(async () => {
31-
return Tracer.listTraces()
32+
return Tracer.listTraces(props.tracesDir)
3233
})
3334

3435
const options = createMemo(() => {
@@ -37,7 +38,7 @@ export function DialogTraceList(props: {
3738
{
3839
title: "Failed to load traces",
3940
value: "__error__",
40-
description: `Check ${Tracer.getTracesDir()}`,
41+
description: `Check ${Tracer.getTracesDir(props.tracesDir)}`,
4142
},
4243
]
4344
}

packages/opencode/src/tool/webfetch.ts

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@ import { abortAfterAny } from "../util/abort"
77
const MAX_RESPONSE_SIZE = 5 * 1024 * 1024 // 5MB
88
const DEFAULT_TIMEOUT = 30 * 1000 // 30 seconds
99
const MAX_TIMEOUT = 120 * 1000 // 2 minutes
10+
// altimate_change start — branding: honest bot UA
1011
const HONEST_UA = "altimate-code/1.0 (+https://github.com/AltimateAI/altimate-code)"
12+
// altimate_change end
1113
const BROWSER_UA =
1214
"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/143.0.0.0 Safari/537.36"
1315
// Status codes that warrant a retry with a different User-Agent
@@ -70,16 +72,17 @@ export const WebFetchTool = Tool.define("webfetch", {
7072
const honestHeaders = { ...baseHeaders, "User-Agent": HONEST_UA }
7173
const browserHeaders = { ...baseHeaders, "User-Agent": BROWSER_UA }
7274

73-
let response = await fetch(params.url, { signal, headers: honestHeaders })
74-
75-
// Retry with browser UA if the honest UA was rejected
76-
if (!response.ok && RETRYABLE_STATUSES.has(response.status)) {
77-
await response.body?.cancel().catch(() => {})
78-
response = await fetch(params.url, { signal, headers: browserHeaders })
79-
}
80-
8175
let arrayBuffer: ArrayBuffer
76+
let response: Response
8277
try {
78+
response = await fetch(params.url, { signal, headers: honestHeaders })
79+
80+
// Retry with browser UA if the honest UA was rejected
81+
if (!response.ok && RETRYABLE_STATUSES.has(response.status)) {
82+
await response.body?.cancel().catch(() => {})
83+
response = await fetch(params.url, { signal, headers: browserHeaders })
84+
}
85+
8386
if (!response.ok) {
8487
throw new Error(`Request failed with status code: ${response.status}`)
8588
}

0 commit comments

Comments
 (0)