Skip to content

Commit e2fdffd

Browse files
fix: address review findings across telemetry, consent, branding, and quantum
Telemetry (integration.ts, collector.ts, index.ts): - Remove process.once SIGTERM/beforeExit handlers that crash outside Instance context; rely on Instance.state disposal for flush - Move recordedUserMessages to TelemetryState (cleared per session) to prevent unbounded Set growth across sessions - Remove dead assistantToUser map and dead PartUpdated text branch - Use info.parentID for turn start time lookup instead of fragile insertion-order heuristic on turnStartTimes map - Fix exported finalizeTurn() to actually call collector.finalizeTurn() - Cache consentTier/dataLevel from initialize() to avoid redundant getConsentStatus() call in startSession() - Remove duplicate retry/recordRetry export from Telemetry namespace Consent dialog (dialog-telemetry-consent.tsx, app.tsx): - Use DialogTelemetryConsent.show() API instead of raw dialog.replace() to properly wire onResult and onClose callbacks - Default tier to 'paid' (genuine opt-out) until actual tier detection is implemented; 'free' forced all users into required telemetry - Add double-fire guard on handleSelect to prevent duplicate KV writes - Fix Esc handler: free tier forces accept, paid tier declines - Fix unsafe 'as boolean' cast on KV value with === true check - Add consentShown guard to prevent re-show on rebootstrap cycles - Remove unnecessary spread in <For> component Branding (apply.ts, schema.ts, brand.json): - Rewrite non-exclusive models.ts get() replacement to use actual upstream scope variables (filepath, Bun.file, data macro, refresh) instead of nonexistent readCache/writeCache/url - Add match-failure assertions to all regex transforms so upstream refactors produce loud build errors instead of silent failures - Remove unused 'default' field from ModelsSchema and brand.json Quantum (client.ts, tools.ts): - Add Zod schemas for QuantumDevice, QuantumJob, JobResult with runtime .parse() validation on all API responses - Add pricingAvailable flag to CostEstimate; warn when pricing is unavailable instead of silently returning 0 - Add ctx.ask() permission gate to quantum_cancel_job (destructive op) - Thread ctx.abort signal through all tool execute -> client calls - Add 30s default timeout via AbortSignal.timeout on all fetch calls - Cache resolveAuth() result for 5s to avoid repeated disk reads - Truncate error response bodies to 500 chars - Normalize job status with .toUpperCase() for consistent comparison - Wrap getResult() in try/catch for TOCTOU race after status check - Fix pricing display from '$' prefix to 'credits' suffix
1 parent 51d6d74 commit e2fdffd

File tree

10 files changed

+253
-162
lines changed

10 files changed

+253
-162
lines changed

branding/apply.ts

Lines changed: 39 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -363,50 +363,63 @@ const PURPLE = RGBA.fromHex("#9370DB")`,
363363

364364
if (config.models.exclusive) {
365365
// Exclusive mode: only branded models, no models.dev fetch
366-
return content.replace(
366+
const result = content.replace(
367367
/export async function get\(\) \{[\s\S]*?\n \}/,
368368
`export async function get() {
369369
// Branding: embedded models (exclusive mode, no external fetch)
370370
return ${JSON.stringify(modelsJson)} as Record<string, Provider>
371371
}`,
372372
)
373+
if (result === content) {
374+
throw new Error("models.ts branding transform failed: get() regex did not match (exclusive mode)")
375+
}
376+
return result
373377
}
374378

375379
// Default mode: prepend branded models, then merge models.dev data
376380
// This ensures qBraid models appear first and are the defaults,
377381
// while all upstream providers (Anthropic, OpenAI, Copilot, Codex, etc.)
378382
// remain available.
383+
//
384+
// The replacement body references variables from the original models.ts scope:
385+
// - filepath (const, path to cache file)
386+
// - data (Bun macro import for bundled snapshot)
387+
// - refresh() (background fetch to update cache)
379388
const brandedModelsStr = JSON.stringify(modelsJson)
380-
return content.replace(
389+
const result = content.replace(
381390
/export async function get\(\) \{[\s\S]*?\n \}/,
382391
`export async function get() {
383392
// Branding: qBraid models prepended as defaults
384393
const branded = ${brandedModelsStr} as Record<string, Provider>
385394
386-
// Fetch upstream models from models.dev (or cache)
395+
// Kick off background cache refresh
396+
refresh()
397+
398+
// Try cached models first, then macro bundle, then live fetch
387399
let upstream: Record<string, Provider> = {}
388400
try {
389-
const cached = await readCache()
401+
const file = Bun.file(filepath)
402+
const cached = await file.json().catch(() => undefined)
390403
if (cached) {
391-
upstream = cached
404+
upstream = cached as Record<string, Provider>
405+
} else if (typeof data === "function") {
406+
upstream = JSON.parse(await data()) as Record<string, Provider>
392407
} else {
393-
const response = await fetch(url)
394-
if (response.ok) {
395-
upstream = await response.json() as Record<string, Provider>
396-
await writeCache(upstream)
397-
}
408+
const json = await fetch("https://models.dev/api.json").then((x) => x.text())
409+
upstream = JSON.parse(json) as Record<string, Provider>
398410
}
399-
} catch (e) {
400-
// Fall back to bundled snapshot if fetch fails
401-
try {
402-
upstream = (await import("./models-snapshot")).default as Record<string, Provider>
403-
} catch {}
411+
} catch {
412+
// All upstream sources failed — branded models only
404413
}
405414
406-
// Merge: branded providers first, then upstream (branded wins on conflict)
415+
// Merge: branded providers win on conflict
407416
return { ...upstream, ...branded }
408417
}`,
409418
)
419+
if (result === content) {
420+
throw new Error("models.ts branding transform failed: get() regex did not match. Has the upstream function signature changed?")
421+
}
422+
return result
410423
},
411424
},
412425

@@ -431,10 +444,14 @@ const PURPLE = RGBA.fromHex("#9370DB")`,
431444
transform: (content, config) => {
432445
if (!config.models?.exclusive) return content
433446

434-
return content.replace(
447+
const result = content.replace(
435448
/const BUILTIN = \["[^"]*"(?:,\s*"[^"]*")*\]/,
436449
"const BUILTIN: string[] = [] // Cleared by branding - no external plugins",
437450
)
451+
if (result === content) {
452+
throw new Error("plugin/index.ts branding transform failed: BUILTIN regex did not match")
453+
}
454+
return result
438455
},
439456
},
440457

@@ -446,12 +463,16 @@ const PURPLE = RGBA.fromHex("#9370DB")`,
446463
transform: (content, config) => {
447464
if (!config.models?.exclusive) return content
448465

449-
return content.replace(
466+
const result = content.replace(
450467
/const CUSTOM_LOADERS: Record<string, CustomLoader> = \{[\s\S]*?\n \}(?=\n\n export const Model)/,
451468
`const CUSTOM_LOADERS: Record<string, CustomLoader> = {
452469
// All custom loaders removed by branding (exclusive mode)
453470
}`,
454471
)
472+
if (result === content) {
473+
throw new Error("provider.ts branding transform failed: CUSTOM_LOADERS regex did not match")
474+
}
475+
return result
455476
},
456477
},
457478

branding/qbraid/brand.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
}
3030
},
3131
"models": {
32-
"default": true,
3332
"exclusive": false,
3433
"removeProviders": ["opencode"],
3534
"source": "./models.json"

branding/schema.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,9 @@ export const ModelsSchema = z.object({
4545
.optional(),
4646
/** Provider IDs to completely remove */
4747
removeProviders: z.array(z.string()).optional(),
48-
/** If true, only use the providers defined in this config (locks out all others) */
48+
/** If true, only use the providers defined in this config (locks out all others).
49+
* When false/unset, branded models are prepended as defaults but upstream providers remain available. */
4950
exclusive: z.boolean().optional(),
50-
/** If true, branded models are prepended as defaults but upstream providers remain available */
51-
default: z.boolean().optional(),
5251
})
5352

5453
/**

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

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -277,26 +277,25 @@ function App() {
277277
// --- First-run telemetry consent dialog ---
278278
// Fires once when sync is complete and user hasn't seen the consent dialog yet.
279279
// Must fire *before* the provider connect dialog so consent is captured first.
280+
let consentShown = false
280281
createEffect(
281282
on(
282283
() => sync.status === "complete" && kv.get(KV_TELEMETRY_CONSENT_SHOWN) === undefined,
283284
(needsConsent, prev) => {
284-
if (!needsConsent || prev) return
285+
if (!needsConsent || prev || consentShown) return
286+
consentShown = true
285287

286288
// Load any existing consent value into the telemetry module
287289
const existing = kv.get(KV_TELEMETRY_ENABLED)
288290
if (existing !== undefined) {
289-
Telemetry.loadConsent(existing as boolean)
290-
// Already consented in a previous session, skip dialog
291+
Telemetry.loadConsent(existing === true)
291292
kv.set(KV_TELEMETRY_CONSENT_SHOWN, true)
292293
return
293294
}
294295

295-
// Show the consent dialog. Free tier = informational only.
296-
// TODO: Determine actual tier from auth/consent service. Default to "free".
297-
dialog.replace(() => (
298-
<DialogTelemetryConsent tier="free" onResult={() => {}} />
299-
))
296+
// Default to "paid" tier (gives genuine opt-out) until we can
297+
// determine the actual tier from the auth/consent service.
298+
DialogTelemetryConsent.show(dialog, "paid")
300299
},
301300
),
302301
)

packages/opencode/src/cli/cmd/tui/component/dialog-telemetry-consent.tsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,10 @@ export function DialogTelemetryConsent(props: DialogTelemetryConsentProps) {
6363
decline: "No Thanks",
6464
}
6565

66+
let handled = false
6667
const handleSelect = (key: string) => {
68+
if (handled) return
69+
handled = true
6770
const accepted = key === "understand" || key === "enable"
6871
kv.set(KV_TELEMETRY_CONSENT_SHOWN, true)
6972
kv.set(KV_TELEMETRY_ENABLED, accepted)
@@ -97,7 +100,7 @@ export function DialogTelemetryConsent(props: DialogTelemetryConsentProps) {
97100
<text fg={theme.textMuted}>{message()}</text>
98101
</box>
99102
<box flexDirection="row" justifyContent="flex-end" paddingBottom={1} gap={1}>
100-
<For each={[...buttons()]}>
103+
<For each={buttons() as readonly string[]}>
101104
{(key) => (
102105
<box
103106
paddingLeft={2}
@@ -131,8 +134,8 @@ DialogTelemetryConsent.show = (
131134
onResult={(accepted) => resolve(accepted)}
132135
/>
133136
),
134-
// If user presses Esc on paid tier, treat as decline
135-
() => resolve(false),
137+
// Esc handler: free tier = accept (required), paid tier = decline
138+
() => resolve(tier === "free"),
136139
)
137140
})
138141
}

0 commit comments

Comments
 (0)