From 554086cb4573700fa5a1be361b1b7557f14e8d25 Mon Sep 17 00:00:00 2001 From: Andro Marces Date: Sat, 12 Sep 2026 17:42:01 +0800 Subject: [PATCH 1/7] fix(discovery): async bounded model probes for opencode and antigravity Replace spawn.sync in OpenCodeAdapter.listModels and AntigravityAdapter.listModels with an async bounded probe: async spawn, 20s budget covering the observed worst case, 1MB output cap, timeout rejection, and confirmed child termination on every path. Record per-adapter discovery failures as models_error in registeredAdapters so the dialog distinguishes failed-retry from no-models, and correct the discoverModels comment that claimed non-blocking behavior. Refs rjx18/codor#29 --- packages/adapters/antigravity/NOTES.md | 12 +- .../adapters/antigravity/src/adapter.spec.ts | 49 +++++++ packages/adapters/antigravity/src/adapter.ts | 121 +++++++++++++++-- packages/adapters/opencode/NOTES.md | 15 ++- .../adapters/opencode/src/adapter.spec.ts | 47 ++++++- packages/adapters/opencode/src/adapter.ts | 122 ++++++++++++++++-- packages/switchboard/src/daemon.spec.ts | 30 +++++ packages/switchboard/src/daemon.ts | 25 +++- packages/web-next/src/room/AgentControls.tsx | 13 +- packages/web-next/src/room/agent-spec.ts | 2 + packages/web-next/src/runtime/api.ts | 2 + 11 files changed, 395 insertions(+), 43 deletions(-) diff --git a/packages/adapters/antigravity/NOTES.md b/packages/adapters/antigravity/NOTES.md index d7640e1d..d11a38f7 100644 --- a/packages/adapters/antigravity/NOTES.md +++ b/packages/adapters/antigravity/NOTES.md @@ -23,11 +23,13 @@ switchboard's supervised child lifecycle. ## Model catalog The probe reported human-facing names such as `Gemini 3.5 Flash (High)`. -`listModels()` runs `agy models` locally with a fixed argv and timeout, reports -slug-safe ids to Codor, and owns the reverse mapping back to display names at -spawn time. The catalog is never hard-coded here. If two names collapse to the -same slug, discovery fails rather than routing an operator selection to the -wrong model. +`listModels()` runs `agy models` as an async child with a fixed argv, a 20 s budget and +capped output, reports slug-safe ids to Codor, and owns the reverse mapping back to +display names at spawn time. The probe never occupies the event loop, and the child is +confirmed terminated on every path, including the timeout. The catalog is never hard-coded +here. If two names collapse to the same slug, discovery fails rather than routing an +operator selection to the wrong model; any failure is recorded per harness (`models_error`) +so the dialog offers a retry. ## Resume boundary diff --git a/packages/adapters/antigravity/src/adapter.spec.ts b/packages/adapters/antigravity/src/adapter.spec.ts index 4fae14a5..66930fd7 100644 --- a/packages/adapters/antigravity/src/adapter.spec.ts +++ b/packages/adapters/antigravity/src/adapter.spec.ts @@ -197,6 +197,55 @@ describe('AntigravityAdapter', () => { await expect(collision.listModels()).rejects.toThrow("collide at slug 'a-b'"); }); + it('lets the event loop run while the CLI answers', async () => { + // Requirement: async bounded probe — discovery must not freeze the daemon. + const dir = mkdtempSync(join(tmpdir(), 'codor-antigravity-')); + temporary.push(dir); + const path = join(dir, 'agy'); + writeFileSync(path, '#!/usr/bin/env node\nsetTimeout(() => { process.stdout.write("Gemini 3.5 Flash (High)\\n"); }, 1200);\n'); + chmodSync(path, 0o755); + const start = Date.now(); + let firedAt = -1; + setTimeout(() => { firedAt = Date.now(); }, 20); + const catalog = await new AntigravityAdapter(path).listModels(); + expect(catalog.models).toEqual(['gemini-3.5-flash-high']); + expect(firedAt).toBeGreaterThanOrEqual(start); + expect(firedAt - start).toBeLessThan(1000); + }); + + it('fails rather than buffering unbounded CLI output', async () => { + // Requirement: bounded probe — a runaway listing must fail, not balloon memory. + const dir = mkdtempSync(join(tmpdir(), 'codor-antigravity-')); + temporary.push(dir); + const path = join(dir, 'agy'); + writeFileSync(path, "#!/usr/bin/env node\nprocess.stdout.write('x'.repeat(1_200_000));\n"); + chmodSync(path, 0o755); + await expect(new AntigravityAdapter(path).listModels()).rejects.toThrow(/exceeded/); + }); + + it('terminates the child when the budget runs out', async () => { + // Requirement: confirmed cleanup — no orphan survives a timed-out probe. + const dir = mkdtempSync(join(tmpdir(), 'codor-antigravity-')); + temporary.push(dir); + const path = join(dir, 'agy'); + const pidFile = join(dir, 'pid'); + writeFileSync(path, `#!/usr/bin/env node +require('node:fs').writeFileSync(${JSON.stringify(pidFile)}, String(process.pid)); +setInterval(() => {}, 1000); +`); + chmodSync(path, 0o755); + await expect(new AntigravityAdapter(path, 2000).listModels()).rejects.toThrow(/timed out/); + const pid = Number(readFileSync(pidFile, 'utf8')); + expect(Number.isInteger(pid)).toBe(true); + let alive = true; + try { + process.kill(pid, 0); + } catch { + alive = false; + } + expect(alive).toBe(false); + }); + it('classifies missing commands and nonzero exits as failed', async () => { const missing = new AntigravityAdapter(join(tmpdir(), 'codor-agy-missing')); expect((await collect(missing, missing.spawn({ cwd: process.cwd() }), 'hello')).at(-1)) diff --git a/packages/adapters/antigravity/src/adapter.ts b/packages/adapters/antigravity/src/adapter.ts index 2dc507ca..092341a5 100644 --- a/packages/adapters/antigravity/src/adapter.ts +++ b/packages/adapters/antigravity/src/adapter.ts @@ -48,6 +48,104 @@ function finalOutput(output: string, truncated: boolean): string { return `${utf8Prefix(output, MAX_OUTPUT_BYTES - markerBytes)}${TRUNCATED}`; } +/** Budget for the async `agy models` probe: covers the observed worst case + * without ever stalling the daemon, since the child no longer occupies the loop. */ +const MODEL_DISCOVERY_TIMEOUT_MS = 20_000; +/** Hard cap on captured `agy models` stdout, in bytes. */ +const MODEL_DISCOVERY_MAX_BYTES = 1_000_000; + +/** Confirm a discovery child is gone: SIGTERM, then SIGKILL on a finite bound. */ +async function stopDiscoveryProbe(child: ChildProcess, label: string): Promise { + if (child.exitCode !== null || child.signalCode !== null) return; + await new Promise((resolve, reject) => { + const finish = (error?: Error): void => { + clearTimeout(escalate); clearTimeout(deadline); child.off('exit', exited); + if (error) reject(error); else resolve(); + }; + const exited = (): void => finish(); + const escalate = setTimeout(() => { + try { child.kill('SIGKILL'); } catch { /* exit confirmation below remains required */ } + }, 1_000); + const deadline = setTimeout(() => finish(new Error(`${label} model probe did not exit`)), 2_000); + child.once('exit', exited); + try { + child.kill('SIGTERM'); + } catch (error) { finish(error instanceof Error ? error : new Error(`${label} model probe cleanup failed`)); } + }); +} + +/** + * Run `command models` without occupying the event loop: async spawn, capped + * output, timeout rejection, and confirmed child termination on every path. + */ +async function runBoundedModelsCommand( + command: string, + timeoutMs: number, + maxBytes: number, +): Promise { + let child: ChildProcess | undefined; + let retired = false; + let deadline: ReturnType | undefined; + const budget = Number.isFinite(timeoutMs) && timeoutMs > 0 ? timeoutMs : MODEL_DISCOVERY_TIMEOUT_MS; + const work = async (): Promise => { + const spawned = spawn(command, ['models'], { stdio: ['ignore', 'pipe', 'pipe'] }); + if (retired) { + await stopDiscoveryProbe(spawned, command); + throw new Error(`${command} models probe expired during spawn`); + } + child = spawned; + return await new Promise((resolve, reject) => { + let stdout = ''; + let stdoutBytes = 0; + let stderr = ''; + let settled = false; + const fail = (error: Error): void => { if (!settled) { settled = true; reject(error); } }; + const done = (value: string): void => { if (!settled) { settled = true; resolve(value); } }; + spawned.stdout?.setEncoding('utf8'); + spawned.stderr?.setEncoding('utf8'); + spawned.stdout?.on('data', (chunk: string) => { + stdoutBytes += Buffer.byteLength(chunk, 'utf8'); + if (stdoutBytes > maxBytes) { + fail(new Error(`${command} models output exceeded ${String(maxBytes)} bytes`)); + return; + } + stdout += chunk; + }); + spawned.stderr?.on('data', (chunk: string) => { + stderr = `${stderr}${chunk}`.slice(-8192); + }); + spawned.once('error', (error) => { + fail(error instanceof Error ? error : new Error(String(error))); + }); + spawned.once('close', (code, signal) => { + if (code !== 0) { + const detail = stderr.trim(); + fail(new Error( + `Command failed: ${command} models${signal ? ` (${signal})` : ''}${detail === '' ? '' : `\n${detail}`}`, + )); + return; + } + done(stdout); + }); + }); + }; + try { + return await Promise.race([ + work(), + new Promise((_resolve, reject) => { + deadline = setTimeout(() => { + retired = true; + reject(new Error(`${command} models timed out after ${String(budget)}ms`)); + }, budget); + }), + ]); + } finally { + retired = true; + clearTimeout(deadline); + if (child) await stopDiscoveryProbe(child, command); + } +} + export interface AntigravityInvocation { mode: 'plan' | 'accept-edits'; skipPermissions: boolean; @@ -116,7 +214,10 @@ export class AntigravityAdapter implements HarnessAdapter { private readonly children = new WeakMap(); private readonly displayNames = new Map(); - constructor(private readonly command = 'agy') {} + constructor( + private readonly command = 'agy', + private readonly modelDiscoveryTimeoutMs = MODEL_DISCOVERY_TIMEOUT_MS, + ) {} spawn(opts: SpawnOpts): Session { antigravityMode(opts.policy); @@ -132,17 +233,13 @@ export class AntigravityAdapter implements HarnessAdapter { } // harn:assume adapters-own-their-model-catalog ref=antigravity-model-catalog - listModels(): Promise { - const result = spawn.sync(this.command, ['models'], { - timeout: 5_000, - maxBuffer: 1_000_000, - encoding: 'utf8', - }); - if (result.error) return Promise.reject(result.error); - if (result.status !== 0) { - return Promise.reject(new Error(`Command failed: ${this.command} models`)); - } - const displays = String(result.stdout ?? '') + async listModels(): Promise { + const stdout = await runBoundedModelsCommand( + this.command, + this.modelDiscoveryTimeoutMs, + MODEL_DISCOVERY_MAX_BYTES, + ); + const displays = stdout .split(/\r?\n/) .map((line) => line.trim()) .filter(Boolean); diff --git a/packages/adapters/opencode/NOTES.md b/packages/adapters/opencode/NOTES.md index cb0b1d88..ece48c10 100644 --- a/packages/adapters/opencode/NOTES.md +++ b/packages/adapters/opencode/NOTES.md @@ -133,17 +133,22 @@ It is captured once and never regenerated by the normal test suite. ## Model catalog (U3): discovered, not curated opencode is the one harness that can enumerate its own models, so `listModels()` DISCOVERS -them: it runs `opencode models` with a fixed argument vector (no shell), a 5s timeout and a -capped buffer, and reports `source: 'discovered'`. Nothing is hardcoded. +them: it runs `opencode models` as an async child with a fixed argument vector (no shell), +a 20 s budget and capped output, and reports `source: 'discovered'`. Nothing is hardcoded. +The probe never occupies the event loop, and the child is confirmed terminated on every +path, including the timeout. This matters because opencode's catalog is per-installation: run on the development host 2026-07-12 (no model spend) it printed 79 models drawn entirely from THAT machine's configured providers — `openai`, `opencode`, and `synthetic`, with zero Anthropic entries. Any hardcoded list would have been wrong for somebody. -Discovery is best-effort by design. A missing binary, a non-zero exit, a hang, or output the -daemon cannot validate all degrade silently to no list, and the dialog falls back to its -`Custom…` escape (placeholder `provider/model`, the form opencode's `--model` takes). +Discovery is best-effort by design. A missing binary, a non-zero exit, a hang past the +budget, or output the daemon cannot validate all throw, and the daemon records the failure +per harness (`models_error`) so the dialog offers a retry instead of reporting no models; +a prior catalog is kept when one exists. The dialog falls back to its `Custom…` escape +(placeholder `provider/model`, the form opencode's `--model` takes) only while no catalog +was ever reported. Thinking is supported and maps to `run --variant `. diff --git a/packages/adapters/opencode/src/adapter.spec.ts b/packages/adapters/opencode/src/adapter.spec.ts index fca0c076..d395f02e 100644 --- a/packages/adapters/opencode/src/adapter.spec.ts +++ b/packages/adapters/opencode/src/adapter.spec.ts @@ -1,4 +1,4 @@ -import { chmodSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { chmodSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -178,6 +178,51 @@ describe('opencode model discovery', () => { await expect(new OpenCodeAdapter(stub('process.stderr.write("boom\\n"); process.exit(1);')).listModels()) .rejects.toThrow(); }); + + it('lets the event loop run while the CLI answers', async () => { + // Requirement: async bounded probe — discovery must not freeze the daemon. + const dir = mkdtempSync(join(tmpdir(), 'codor-opencode-models-')); + dirs.push(dir); + const command = join(dir, 'opencode'); + writeFileSync(command, '#!/usr/bin/env node\nsetTimeout(() => { console.log("anthropic/claude-sonnet-5\\nopenai/gpt-4o"); }, 1200);\n'); + chmodSync(command, 0o755); + const start = Date.now(); + let firedAt = -1; + setTimeout(() => { firedAt = Date.now(); }, 20); + const catalog = await new OpenCodeAdapter(command).listModels(); + expect(catalog.models).toEqual(['anthropic/claude-sonnet-5', 'openai/gpt-4o']); + expect(firedAt).toBeGreaterThanOrEqual(start); + expect(firedAt - start).toBeLessThan(1000); + }); + + it('fails rather than buffering unbounded CLI output', async () => { + // Requirement: bounded probe — a runaway listing must fail, not balloon memory. + const command = stub("process.stdout.write('x'.repeat(1_200_000));"); + await expect(new OpenCodeAdapter(command).listModels()).rejects.toThrow(/exceeded/); + }); + + it('terminates the child when the budget runs out', async () => { + // Requirement: confirmed cleanup — no orphan survives a timed-out probe. + const dir = mkdtempSync(join(tmpdir(), 'codor-opencode-models-')); + dirs.push(dir); + const command = join(dir, 'opencode'); + const pidFile = join(dir, 'pid'); + writeFileSync(command, `#!/usr/bin/env node +require('node:fs').writeFileSync(${JSON.stringify(pidFile)}, String(process.pid)); +setInterval(() => {}, 1000); +`); + chmodSync(command, 0o755); + await expect(new OpenCodeAdapter(command, 2000).listModels()).rejects.toThrow(/timed out/); + const pid = Number(readFileSync(pidFile, 'utf8')); + expect(Number.isInteger(pid)).toBe(true); + let alive = true; + try { + process.kill(pid, 0); + } catch { + alive = false; + } + expect(alive).toBe(false); + }); // harn:end windows-cli-adapters-resolve-command-shims }); diff --git a/packages/adapters/opencode/src/adapter.ts b/packages/adapters/opencode/src/adapter.ts index 63debd83..d5dbd2df 100644 --- a/packages/adapters/opencode/src/adapter.ts +++ b/packages/adapters/opencode/src/adapter.ts @@ -19,6 +19,104 @@ import { createTurnTranslator } from './translate.js'; const DISCOVER_QUERY = 'SELECT id FROM session WHERE parent_id IS NULL ORDER BY time_updated DESC'; +/** Budget for the async `opencode models` probe: covers the observed worst case + * without ever stalling the daemon, since the child no longer occupies the loop. */ +const MODEL_DISCOVERY_TIMEOUT_MS = 20_000; +/** Hard cap on captured `opencode models` stdout, in bytes. */ +const MODEL_DISCOVERY_MAX_BYTES = 1_000_000; + +/** Confirm a discovery child is gone: SIGTERM, then SIGKILL on a finite bound. */ +async function stopDiscoveryProbe(child: ChildProcess, label: string): Promise { + if (child.exitCode !== null || child.signalCode !== null) return; + await new Promise((resolve, reject) => { + const finish = (error?: Error): void => { + clearTimeout(escalate); clearTimeout(deadline); child.off('exit', exited); + if (error) reject(error); else resolve(); + }; + const exited = (): void => finish(); + const escalate = setTimeout(() => { + try { child.kill('SIGKILL'); } catch { /* exit confirmation below remains required */ } + }, 1_000); + const deadline = setTimeout(() => finish(new Error(`${label} model probe did not exit`)), 2_000); + child.once('exit', exited); + try { + child.kill('SIGTERM'); + } catch (error) { finish(error instanceof Error ? error : new Error(`${label} model probe cleanup failed`)); } + }); +} + +/** + * Run `command models` without occupying the event loop: async spawn, capped + * output, timeout rejection, and confirmed child termination on every path. + */ +async function runBoundedModelsCommand( + command: string, + timeoutMs: number, + maxBytes: number, +): Promise { + let child: ChildProcess | undefined; + let retired = false; + let deadline: ReturnType | undefined; + const budget = Number.isFinite(timeoutMs) && timeoutMs > 0 ? timeoutMs : MODEL_DISCOVERY_TIMEOUT_MS; + const work = async (): Promise => { + const spawned = spawn(command, ['models'], { stdio: ['ignore', 'pipe', 'pipe'] }); + if (retired) { + await stopDiscoveryProbe(spawned, command); + throw new Error(`${command} models probe expired during spawn`); + } + child = spawned; + return await new Promise((resolve, reject) => { + let stdout = ''; + let stdoutBytes = 0; + let stderr = ''; + let settled = false; + const fail = (error: Error): void => { if (!settled) { settled = true; reject(error); } }; + const done = (value: string): void => { if (!settled) { settled = true; resolve(value); } }; + spawned.stdout?.setEncoding('utf8'); + spawned.stderr?.setEncoding('utf8'); + spawned.stdout?.on('data', (chunk: string) => { + stdoutBytes += Buffer.byteLength(chunk, 'utf8'); + if (stdoutBytes > maxBytes) { + fail(new Error(`${command} models output exceeded ${String(maxBytes)} bytes`)); + return; + } + stdout += chunk; + }); + spawned.stderr?.on('data', (chunk: string) => { + stderr = `${stderr}${chunk}`.slice(-8192); + }); + spawned.once('error', (error) => { + fail(error instanceof Error ? error : new Error(String(error))); + }); + spawned.once('close', (code, signal) => { + if (code !== 0) { + const detail = stderr.trim(); + fail(new Error( + `Command failed: ${command} models${signal ? ` (${signal})` : ''}${detail === '' ? '' : `\n${detail}`}`, + )); + return; + } + done(stdout); + }); + }); + }; + try { + return await Promise.race([ + work(), + new Promise((_resolve, reject) => { + deadline = setTimeout(() => { + retired = true; + reject(new Error(`${command} models timed out after ${String(budget)}ms`)); + }, budget); + }), + ]); + } finally { + retired = true; + clearTimeout(deadline); + if (child) await stopDiscoveryProbe(child, command); + } +} + // harn:assume harness-declares-supported-thinking-levels ref=opencode-thinking-level-declaration export const OPENCODE_THINKING_LEVELS = [ 'low', @@ -91,7 +189,10 @@ export class OpenCodeAdapter implements HarnessAdapter { private readonly children = new WeakMap(); - constructor(private readonly command = 'opencode') {} + constructor( + private readonly command = 'opencode', + private readonly modelDiscoveryTimeoutMs = MODEL_DISCOVERY_TIMEOUT_MS, + ) {} spawn(opts: SpawnOpts): Session { if (opts.policy !== undefined && !PolicySchema.safeParse(opts.policy).success) { @@ -115,19 +216,16 @@ export class OpenCodeAdapter implements HarnessAdapter { /** * opencode's models come from the operator's OWN configured providers, so no * fixed list can be right for every install — ask the CLI. Fixed argv (no - * shell), hard timeout, capped output; a failure throws and the daemon - * silently degrades this harness to the custom escape. + * shell), async probe with a 20 s budget and capped output; a failure throws + * and the daemon records it per harness for a retryable dialog. */ async listModels(): Promise { - const result = spawn.sync(this.command, ['models'], { - timeout: 5_000, - maxBuffer: 1_000_000, - encoding: 'utf8', - }); - if (result.error) throw result.error; - if (result.status !== 0) throw new Error(`Command failed: ${this.command} models`); - const listed = result.stdout; - const models = listed.split('\n').map((line) => line.trim()).filter((line) => line !== ''); + const stdout = await runBoundedModelsCommand( + this.command, + this.modelDiscoveryTimeoutMs, + MODEL_DISCOVERY_MAX_BYTES, + ); + const models = stdout.split('\n').map((line) => line.trim()).filter((line) => line !== ''); if (models.length === 0) throw new Error('opencode listed no models'); return { models, source: 'discovered' }; } diff --git a/packages/switchboard/src/daemon.spec.ts b/packages/switchboard/src/daemon.spec.ts index 4fa597fe..1c93c2c6 100644 --- a/packages/switchboard/src/daemon.spec.ts +++ b/packages/switchboard/src/daemon.spec.ts @@ -4118,6 +4118,36 @@ describe('adapter model discovery', () => { expect(adapter!.id).toBe('broken'); }); + it('records a discovery failure on the adapter entry until a retry succeeds', async () => { + // Requirement: a failed discovery must surface as retryable, not as "no models". + const listModels = vi.fn(async (): Promise => { throw new Error('harness blew up'); }); + const daemon = daemonWith([adapterWith('flaky', listModels)]); + try { + await settle(); await settle(); + expect(daemon.registeredAdapters()[0]).toMatchObject({ id: 'flaky', models_error: 'harness blew up' }); + expect(daemon.registeredAdapters()[0]?.models).toBeUndefined(); + listModels.mockResolvedValue({ models: ['a/b'], source: 'discovered' }); + daemon.refreshAdapterAvailability(); await settle(); await settle(); + expect(daemon.registeredAdapters()[0]).toMatchObject({ models: ['a/b'] }); + expect(daemon.registeredAdapters()[0]?.models_error).toBeUndefined(); + } finally { await daemon.close(); } + }); + + it('keeps a prior catalog while reporting the latest failure', async () => { + // Requirement: a later failure must not erase a working list, but must say so. + const listModels = vi.fn(async (): Promise => ({ models: ['native/model'], source: 'discovered' })); + const daemon = daemonWith([adapterWith('codex', listModels)]); + try { + await settle(); await settle(); + listModels.mockRejectedValueOnce(new Error('probe timed out')); + daemon.refreshAdapterAvailability(); await settle(); await settle(); + expect(daemon.registeredAdapters()[0]).toMatchObject({ + models: ['native/model'], + models_error: 'probe timed out', + }); + } finally { await daemon.close(); } + }); + it('drops output it cannot validate rather than trusting harness stdout', async () => { const daemon = daemonWith([ adapterWith('noisy', () => Promise.resolve({ diff --git a/packages/switchboard/src/daemon.ts b/packages/switchboard/src/daemon.ts index 59ede080..35544859 100644 --- a/packages/switchboard/src/daemon.ts +++ b/packages/switchboard/src/daemon.ts @@ -532,6 +532,9 @@ export class Daemon { private acpProviderCatalog: AcpProviderMetadata[] = []; // harn:end adapter-catalog-distinguishes-installed-and-configurable private readonly modelCatalogs = new Map(); + /** Latest discovery failure per adapter, so the dialog can tell "failed, retry" + * apart from "this harness reports no models". Cleared on the next success. */ + private readonly modelDiscoveryErrors = new Map(); private pendingDiscoveries = 0; private readonly modelDiscoveryWork = new Map>(); private readonly sessions = new Map(); @@ -1364,10 +1367,11 @@ export class Daemon { // harn:assume adapters-own-their-model-catalog ref=adapter-model-discovery /** * Ask every adapter that can answer what models its harness takes. Runs in - * the background at registration or explicit refresh — never blocking a request, because a hung - * CLI must not be able to wedge /api/adapters, which gates both dialogs. - * Any failure leaves the harness without a list: the dialog then offers the - * custom escape, which is a worse UI, not a broken one. + * the background at registration or explicit refresh — never blocking a request, + * because each probe is asynchronous and a hung CLI must not be able to wedge + * /api/adapters, which gates both dialogs. A failure is recorded per harness + * (see `models_error` in `registeredAdapters`) so the dialog can offer a retry; + * a prior catalog is kept when one exists. */ private discoverModels(): void { for (const adapter of this.adapters.values()) { @@ -1382,9 +1386,12 @@ export class Daemon { if (!Array.isArray(catalog.models)) throw new Error(`${adapter.id} returned an invalid model catalog`); const models = catalog.models.filter((model) => MODEL_ID.test(model)).slice(0, MAX_MODELS); this.modelCatalogs.set(adapter.id, { ...catalog, models }); - }).catch((error: unknown) => this.onBackgroundError( - error instanceof Error ? error : new Error(`${adapter.id} model discovery failed`), - )).finally(() => { + this.modelDiscoveryErrors.delete(adapter.id); + }).catch((error: unknown) => { + const failure = error instanceof Error ? error : new Error(`${adapter.id} model discovery failed`); + this.modelDiscoveryErrors.set(adapter.id, failure.message); + this.onBackgroundError(failure); + }).finally(() => { this.pendingDiscoveries -= 1; if (this.modelDiscoveryWork.get(adapter.id) === work) this.modelDiscoveryWork.delete(adapter.id); }); @@ -1441,6 +1448,8 @@ export class Daemon { capabilities: HarnessAdapter['capabilities']; models?: string[]; models_source?: ModelCatalog['source']; + /** Latest discovery failure message, when the last attempt did not produce a catalog. */ + models_error?: string; }[] { // Every entry carries its runtime harness id. The generic configurable ACP transport // is the sole `advanced` custom-command tile; named providers (acp:) follow as a @@ -1449,6 +1458,7 @@ export class Daemon { ...[...this.adapters.values()] .map((adapter) => { const catalog = this.modelCatalogs.get(adapter.id); + const modelsError = this.modelDiscoveryErrors.get(adapter.id); const configurable = (adapter as RegisteredHarnessAdapter).configurable === true; return { id: adapter.id, @@ -1457,6 +1467,7 @@ export class Daemon { ...(configurable && { configurable: true, transport: 'acp' as const, advanced: true }), capabilities: adapter.capabilities, ...(catalog && { models: catalog.models, models_source: catalog.source }), + ...(modelsError !== undefined && { models_error: modelsError }), }; }) .sort((a, b) => a.id.localeCompare(b.id)), diff --git a/packages/web-next/src/room/AgentControls.tsx b/packages/web-next/src/room/AgentControls.tsx index 91eb5b4d..b2bdbdf8 100644 --- a/packages/web-next/src/room/AgentControls.tsx +++ b/packages/web-next/src/room/AgentControls.tsx @@ -297,7 +297,13 @@ export function AgentControls(props: { aria-label="Model" data-testid={`${id}-model-input`} /> -

This harness did not report a model list.

+ {adapter?.models_error !== undefined ? ( +

+ Model discovery failed: {adapter.models_error} Press Refresh to retry. +

+ ) : ( +

This harness did not report a model list.

+ )} ) : ( <> @@ -348,6 +354,11 @@ export function AgentControls(props: { aria-label="Custom model" data-testid={`${id}-model-custom`} /> + {adapter?.models_error !== undefined && ( +

+ Last refresh failed; showing the previous list. Press Refresh to retry. +

+ )} )} diff --git a/packages/web-next/src/room/agent-spec.ts b/packages/web-next/src/room/agent-spec.ts index dd6c2d0c..43891d49 100644 --- a/packages/web-next/src/room/agent-spec.ts +++ b/packages/web-next/src/room/agent-spec.ts @@ -50,6 +50,8 @@ export interface AdapterLike { approvals?: string; }; models?: string[]; + /** Latest discovery failure message: the list is missing or stale, press Refresh to retry. */ + models_error?: string; } /** Handle length cap enforced by the protocol's member schema. */ diff --git a/packages/web-next/src/runtime/api.ts b/packages/web-next/src/runtime/api.ts index 9ff1fd58..8d038ff4 100644 --- a/packages/web-next/src/runtime/api.ts +++ b/packages/web-next/src/runtime/api.ts @@ -65,6 +65,8 @@ export interface AdapterRegistration { /** Models the harness itself reported. The web never hardcodes a model id. */ models?: string[]; models_source?: 'discovered' | 'curated'; + /** Latest discovery failure message: the list is missing or stale, press Refresh to retry. */ + models_error?: string; } // harn:end agent-selection-shows-detected-acp-and-advanced-custom From 1a68ecf38db75e0763549618963f7af828a37be6 Mon Sep 17 00:00:00 2001 From: Andro Marces Date: Sat, 12 Sep 2026 17:58:49 +0800 Subject: [PATCH 2/7] fix(discovery): bound client error, widen poll window, drop dead probe flag P1: useAdapterCatalog polls 45x500ms (~22.5s) so a 20s probe result is still collected. P2: models_error carries one capped line; full CLI diagnostics stay in the server log. UI: AgentControls failure/stale branches covered by AgentControls.spec.tsx. Cleanup: remove the unreachable retired check from both probes. Docs: state the 20s cut-off against upstream retry behavior. Refs rjx18/codor#29 --- packages/adapters/antigravity/NOTES.md | 4 +- packages/adapters/antigravity/src/adapter.ts | 7 --- packages/adapters/opencode/NOTES.md | 7 ++- packages/adapters/opencode/src/adapter.ts | 7 --- packages/switchboard/src/daemon.spec.ts | 16 ++++++ packages/switchboard/src/daemon.ts | 16 +++++- packages/web-next/src/app/session.ts | 15 ++++-- .../web-next/src/room/AgentControls.spec.tsx | 52 +++++++++++++++++++ packages/web-next/src/room/AgentControls.tsx | 2 +- packages/web-next/src/room/agent-spec.ts | 2 +- packages/web-next/src/runtime/api.ts | 2 +- 11 files changed, 104 insertions(+), 26 deletions(-) create mode 100644 packages/web-next/src/room/AgentControls.spec.tsx diff --git a/packages/adapters/antigravity/NOTES.md b/packages/adapters/antigravity/NOTES.md index d11a38f7..36db13a3 100644 --- a/packages/adapters/antigravity/NOTES.md +++ b/packages/adapters/antigravity/NOTES.md @@ -26,7 +26,9 @@ The probe reported human-facing names such as `Gemini 3.5 Flash (High)`. `listModels()` runs `agy models` as an async child with a fixed argv, a 20 s budget and capped output, reports slug-safe ids to Codor, and owns the reverse mapping back to display names at spawn time. The probe never occupies the event loop, and the child is -confirmed terminated on every path, including the timeout. The catalog is never hard-coded +confirmed terminated on every path, including the timeout. The 20 s budget covers the +observed spread of `agy models`; a slower run times out and surfaces as the same retryable +failure. The catalog is never hard-coded here. If two names collapse to the same slug, discovery fails rather than routing an operator selection to the wrong model; any failure is recorded per harness (`models_error`) so the dialog offers a retry. diff --git a/packages/adapters/antigravity/src/adapter.ts b/packages/adapters/antigravity/src/adapter.ts index 092341a5..da9cee61 100644 --- a/packages/adapters/antigravity/src/adapter.ts +++ b/packages/adapters/antigravity/src/adapter.ts @@ -84,15 +84,10 @@ async function runBoundedModelsCommand( maxBytes: number, ): Promise { let child: ChildProcess | undefined; - let retired = false; let deadline: ReturnType | undefined; const budget = Number.isFinite(timeoutMs) && timeoutMs > 0 ? timeoutMs : MODEL_DISCOVERY_TIMEOUT_MS; const work = async (): Promise => { const spawned = spawn(command, ['models'], { stdio: ['ignore', 'pipe', 'pipe'] }); - if (retired) { - await stopDiscoveryProbe(spawned, command); - throw new Error(`${command} models probe expired during spawn`); - } child = spawned; return await new Promise((resolve, reject) => { let stdout = ''; @@ -134,13 +129,11 @@ async function runBoundedModelsCommand( work(), new Promise((_resolve, reject) => { deadline = setTimeout(() => { - retired = true; reject(new Error(`${command} models timed out after ${String(budget)}ms`)); }, budget); }), ]); } finally { - retired = true; clearTimeout(deadline); if (child) await stopDiscoveryProbe(child, command); } diff --git a/packages/adapters/opencode/NOTES.md b/packages/adapters/opencode/NOTES.md index ece48c10..641669c7 100644 --- a/packages/adapters/opencode/NOTES.md +++ b/packages/adapters/opencode/NOTES.md @@ -145,8 +145,11 @@ Any hardcoded list would have been wrong for somebody. Discovery is best-effort by design. A missing binary, a non-zero exit, a hang past the budget, or output the daemon cannot validate all throw, and the daemon records the failure -per harness (`models_error`) so the dialog offers a retry instead of reporting no models; -a prior catalog is kept when one exists. The dialog falls back to its `Custom…` escape +per harness (`models_error`, bounded to one short line) so the dialog offers a retry instead +of reporting no models; a prior catalog is kept when one exists. The 20 s budget covers the +observed 0.9–11.97 s spread of `opencode models`, but upstream allows 10 s per fetch attempt +with retries, so a slower run can still time out — and then surfaces as the same retryable +failure. The dialog falls back to its `Custom…` escape (placeholder `provider/model`, the form opencode's `--model` takes) only while no catalog was ever reported. Thinking is supported and maps to `run --variant `. diff --git a/packages/adapters/opencode/src/adapter.ts b/packages/adapters/opencode/src/adapter.ts index d5dbd2df..81d09c22 100644 --- a/packages/adapters/opencode/src/adapter.ts +++ b/packages/adapters/opencode/src/adapter.ts @@ -55,15 +55,10 @@ async function runBoundedModelsCommand( maxBytes: number, ): Promise { let child: ChildProcess | undefined; - let retired = false; let deadline: ReturnType | undefined; const budget = Number.isFinite(timeoutMs) && timeoutMs > 0 ? timeoutMs : MODEL_DISCOVERY_TIMEOUT_MS; const work = async (): Promise => { const spawned = spawn(command, ['models'], { stdio: ['ignore', 'pipe', 'pipe'] }); - if (retired) { - await stopDiscoveryProbe(spawned, command); - throw new Error(`${command} models probe expired during spawn`); - } child = spawned; return await new Promise((resolve, reject) => { let stdout = ''; @@ -105,13 +100,11 @@ async function runBoundedModelsCommand( work(), new Promise((_resolve, reject) => { deadline = setTimeout(() => { - retired = true; reject(new Error(`${command} models timed out after ${String(budget)}ms`)); }, budget); }), ]); } finally { - retired = true; clearTimeout(deadline); if (child) await stopDiscoveryProbe(child, command); } diff --git a/packages/switchboard/src/daemon.spec.ts b/packages/switchboard/src/daemon.spec.ts index 1c93c2c6..b5664ef2 100644 --- a/packages/switchboard/src/daemon.spec.ts +++ b/packages/switchboard/src/daemon.spec.ts @@ -4148,6 +4148,22 @@ describe('adapter model discovery', () => { } finally { await daemon.close(); } }); + it('publishes only a bounded first line of a discovery failure', async () => { + // Requirement: raw CLI diagnostics stay in the server log; the API carries a short line. + const daemon = daemonWith([ + adapterWith('leaky', () => Promise.reject(new Error(`boom\n${'x'.repeat(8000)}`))), + adapterWith('chatty', () => Promise.reject(new Error(`w${'o'.repeat(500)}`))), + ]); + try { + await settle(); await settle(); + const entries = new Map(daemon.registeredAdapters().map((entry) => [entry.id, entry])); + expect(entries.get('leaky')?.models_error).toBe('boom'); + const chatty = entries.get('chatty')?.models_error ?? ''; + expect(chatty).not.toContain('\n'); + expect(chatty.length).toBeLessThanOrEqual(200); + } finally { await daemon.close(); } + }); + it('drops output it cannot validate rather than trusting harness stdout', async () => { const daemon = daemonWith([ adapterWith('noisy', () => Promise.resolve({ diff --git a/packages/switchboard/src/daemon.ts b/packages/switchboard/src/daemon.ts index 35544859..8007f593 100644 --- a/packages/switchboard/src/daemon.ts +++ b/packages/switchboard/src/daemon.ts @@ -343,6 +343,18 @@ function countDiffLines(diff: string): { additions: number; deletions: number } */ const MODEL_ID = /^\w[\w.:-]*(?:\/[\w.:-]+)*$/; const MAX_MODELS = 200; +/** Client-facing discovery errors carry one short line. The full diagnostic + * stays in the server log via onBackgroundError — raw CLI stderr must not + * cross the API boundary into the browser. */ +const MAX_DISCOVERY_ERROR_CHARS = 200; + +function sanitizeDiscoveryError(message: string): string { + const firstLine = message.split('\n', 1)[0] ?? ''; + const trimmed = firstLine.trim(); + return trimmed.length > MAX_DISCOVERY_ERROR_CHARS + ? `${trimmed.slice(0, MAX_DISCOVERY_ERROR_CHARS - 1)}…` + : trimmed; +} export interface DaemonOptions { dbPath: string; @@ -1389,7 +1401,7 @@ export class Daemon { this.modelDiscoveryErrors.delete(adapter.id); }).catch((error: unknown) => { const failure = error instanceof Error ? error : new Error(`${adapter.id} model discovery failed`); - this.modelDiscoveryErrors.set(adapter.id, failure.message); + this.modelDiscoveryErrors.set(adapter.id, sanitizeDiscoveryError(failure.message)); this.onBackgroundError(failure); }).finally(() => { this.pendingDiscoveries -= 1; @@ -1448,7 +1460,7 @@ export class Daemon { capabilities: HarnessAdapter['capabilities']; models?: string[]; models_source?: ModelCatalog['source']; - /** Latest discovery failure message, when the last attempt did not produce a catalog. */ + /** Latest discovery failure, bounded to one short line; full detail stays server-side. */ models_error?: string; }[] { // Every entry carries its runtime harness id. The generic configurable ACP transport diff --git a/packages/web-next/src/app/session.ts b/packages/web-next/src/app/session.ts index ea8642ac..fb452998 100644 --- a/packages/web-next/src/app/session.ts +++ b/packages/web-next/src/app/session.ts @@ -114,13 +114,20 @@ export interface AdapterCatalog { } // harn:assume agent-selection-shows-detected-acp-and-advanced-custom ref=detected-acp-catalog-client +/** + * Discovery poll budget: the slowest model probe runs 20 s, so keep asking + * well past that before treating the catalog as final. + */ +const ADAPTER_POLL_ATTEMPTS = 45; +const ADAPTER_POLL_MS = 500; + export function useAdapterCatalog(token: () => string): AdapterCatalog { const [adapters, setAdapters] = useState([]); const [refreshing, setRefreshing] = useState(false); const [refreshError, setRefreshError] = useState(); const active = useRef(true); const timer = useRef>(); - const attemptsLeft = useRef(10); + const attemptsLeft = useRef(ADAPTER_POLL_ATTEMPTS); const poll = useCallback((listing: { adapters: AdapterRegistration[]; discovering: boolean }): void => { if (!active.current) return; @@ -130,12 +137,12 @@ export function useAdapterCatalog(token: () => string): AdapterCatalog { if (timer.current !== undefined) clearTimeout(timer.current); timer.current = setTimeout(() => { void fetchAdapters({ token: token() }).then(poll).catch(() => undefined); - }, 500); + }, ADAPTER_POLL_MS); }, [token]); useEffect(() => { active.current = true; - attemptsLeft.current = 10; + attemptsLeft.current = ADAPTER_POLL_ATTEMPTS; void fetchAdapters({ token: token() }).then(poll).catch(() => undefined); return () => { active.current = false; @@ -147,7 +154,7 @@ export function useAdapterCatalog(token: () => string): AdapterCatalog { if (refreshing) return; setRefreshing(true); setRefreshError(undefined); - attemptsLeft.current = 10; + attemptsLeft.current = ADAPTER_POLL_ATTEMPTS; void refreshAdapters({ token: token() }).then( (listing) => poll(listing), (error: unknown) => { diff --git a/packages/web-next/src/room/AgentControls.spec.tsx b/packages/web-next/src/room/AgentControls.spec.tsx new file mode 100644 index 00000000..a5159628 --- /dev/null +++ b/packages/web-next/src/room/AgentControls.spec.tsx @@ -0,0 +1,52 @@ +import { renderToStaticMarkup } from 'react-dom/server'; +import { describe, expect, it } from 'vitest'; + +import { AgentControls } from './AgentControls.js'; +import type { AdapterLike, AgentConfig } from './agent-spec.js'; + +const adapter = (over: Partial = {}): AdapterLike => ({ + id: 'opencode', + capabilities: { thinking: false }, + ...over, +}); + +const config = (over: Partial = {}): AgentConfig => ({ + harness: 'opencode', model: '', thinking: '', policy: '', ...over, +}); + +function render(adapters: AdapterLike[], current: AgentConfig): string { + return renderToStaticMarkup( + undefined} + behaviourSection={2} + permissionsSection={3} + idPrefix="agent" + />, + ); +} + +// harn:assume adapters-own-their-model-catalog ref=adapter-model-discovery-dialog +describe('model discovery failure', () => { + it('offers a retry when discovery failed instead of reporting no models', () => { + // Requirement: a failed discovery reads as retryable, not as "no models". + const markup = render([adapter({ models_error: 'timed out' })], config()); + expect(markup).toContain('data-testid="agent-model-error"'); + expect(markup).toContain('Press Refresh to retry'); + expect(markup).not.toContain('did not report a model list'); + }); + + it('keeps the no-models note when discovery neither reported nor failed', () => { + const markup = render([adapter()], config()); + expect(markup).toContain('did not report a model list'); + expect(markup).not.toContain('agent-model-error'); + }); + + it('warns on a stale list after a later refresh failure', () => { + // Requirement: a later failure keeps the working list but says so. + const markup = render([adapter({ models: ['a/b'], models_error: 'timed out' })], config()); + expect(markup).toContain('data-testid="agent-model-stale"'); + expect(markup).toContain('a/b'); + }); +}); diff --git a/packages/web-next/src/room/AgentControls.tsx b/packages/web-next/src/room/AgentControls.tsx index b2bdbdf8..4f4e348f 100644 --- a/packages/web-next/src/room/AgentControls.tsx +++ b/packages/web-next/src/room/AgentControls.tsx @@ -299,7 +299,7 @@ export function AgentControls(props: { /> {adapter?.models_error !== undefined ? (

- Model discovery failed: {adapter.models_error} Press Refresh to retry. + Model discovery failed ({adapter.models_error}). Press Refresh to retry.

) : (

This harness did not report a model list.

diff --git a/packages/web-next/src/room/agent-spec.ts b/packages/web-next/src/room/agent-spec.ts index 43891d49..9681d763 100644 --- a/packages/web-next/src/room/agent-spec.ts +++ b/packages/web-next/src/room/agent-spec.ts @@ -50,7 +50,7 @@ export interface AdapterLike { approvals?: string; }; models?: string[]; - /** Latest discovery failure message: the list is missing or stale, press Refresh to retry. */ + /** Latest discovery failure, bounded to one short line: the list is missing or stale, press Refresh to retry. */ models_error?: string; } diff --git a/packages/web-next/src/runtime/api.ts b/packages/web-next/src/runtime/api.ts index 8d038ff4..ac5b9996 100644 --- a/packages/web-next/src/runtime/api.ts +++ b/packages/web-next/src/runtime/api.ts @@ -65,7 +65,7 @@ export interface AdapterRegistration { /** Models the harness itself reported. The web never hardcodes a model id. */ models?: string[]; models_source?: 'discovered' | 'curated'; - /** Latest discovery failure message: the list is missing or stale, press Refresh to retry. */ + /** Latest discovery failure, bounded to one short line: the list is missing or stale, press Refresh to retry. */ models_error?: string; } // harn:end agent-selection-shows-detected-acp-and-advanced-custom From b2de57a83dc471002c900cd9b65e707413e4bba7 Mon Sep 17 00:00:00 2001 From: Andro Marces Date: Sat, 12 Sep 2026 18:10:38 +0800 Subject: [PATCH 3/7] fix(discovery): cover slowest probe, publish error categories P1: poll window is 70x500ms (~35s) to outlast the copilot 30s probe; hook test drives useAdapterCatalog past 31s and to the plateau. P2: models_error is a controlled category (timed out, harness not installed, harness reported no models, output limit exceeded, unexpected error); raw CLI text stays server-side. Docs: correct the agy spread claim to what issue #29 measured. Refs rjx18/codor#29 --- packages/adapters/antigravity/NOTES.md | 10 +-- packages/adapters/opencode/NOTES.md | 2 +- packages/switchboard/src/daemon.spec.ts | 30 +++++--- packages/switchboard/src/daemon.ts | 24 +++---- packages/web-next/src/app/session.spec.ts | 83 ++++++++++++++++++++++- packages/web-next/src/app/session.ts | 7 +- packages/web-next/src/room/agent-spec.ts | 2 +- packages/web-next/src/runtime/api.ts | 2 +- 8 files changed, 124 insertions(+), 36 deletions(-) diff --git a/packages/adapters/antigravity/NOTES.md b/packages/adapters/antigravity/NOTES.md index 36db13a3..c5a3cae6 100644 --- a/packages/adapters/antigravity/NOTES.md +++ b/packages/adapters/antigravity/NOTES.md @@ -26,12 +26,12 @@ The probe reported human-facing names such as `Gemini 3.5 Flash (High)`. `listModels()` runs `agy models` as an async child with a fixed argv, a 20 s budget and capped output, reports slug-safe ids to Codor, and owns the reverse mapping back to display names at spawn time. The probe never occupies the event loop, and the child is -confirmed terminated on every path, including the timeout. The 20 s budget covers the -observed spread of `agy models`; a slower run times out and surfaces as the same retryable -failure. The catalog is never hard-coded +confirmed terminated on every path, including the timeout. The 20 s budget matches the +opencode probe; no slow `agy models` run was measured here, so a slower run surfaces as +the same retryable failure. The catalog is never hard-coded here. If two names collapse to the same slug, discovery fails rather than routing an -operator selection to the wrong model; any failure is recorded per harness (`models_error`) -so the dialog offers a retry. +operator selection to the wrong model; any failure is recorded per harness (`models_error`, +a controlled category, never raw CLI text) so the dialog offers a retry. ## Resume boundary diff --git a/packages/adapters/opencode/NOTES.md b/packages/adapters/opencode/NOTES.md index 641669c7..3f3596cd 100644 --- a/packages/adapters/opencode/NOTES.md +++ b/packages/adapters/opencode/NOTES.md @@ -145,7 +145,7 @@ Any hardcoded list would have been wrong for somebody. Discovery is best-effort by design. A missing binary, a non-zero exit, a hang past the budget, or output the daemon cannot validate all throw, and the daemon records the failure -per harness (`models_error`, bounded to one short line) so the dialog offers a retry instead +per harness (`models_error`, a controlled category) so the dialog offers a retry instead of reporting no models; a prior catalog is kept when one exists. The 20 s budget covers the observed 0.9–11.97 s spread of `opencode models`, but upstream allows 10 s per fetch attempt with retries, so a slower run can still time out — and then surfaces as the same retryable diff --git a/packages/switchboard/src/daemon.spec.ts b/packages/switchboard/src/daemon.spec.ts index b5664ef2..abb8cfbd 100644 --- a/packages/switchboard/src/daemon.spec.ts +++ b/packages/switchboard/src/daemon.spec.ts @@ -4124,7 +4124,7 @@ describe('adapter model discovery', () => { const daemon = daemonWith([adapterWith('flaky', listModels)]); try { await settle(); await settle(); - expect(daemon.registeredAdapters()[0]).toMatchObject({ id: 'flaky', models_error: 'harness blew up' }); + expect(daemon.registeredAdapters()[0]).toMatchObject({ id: 'flaky', models_error: 'unexpected error' }); expect(daemon.registeredAdapters()[0]?.models).toBeUndefined(); listModels.mockResolvedValue({ models: ['a/b'], source: 'discovered' }); daemon.refreshAdapterAvailability(); await settle(); await settle(); @@ -4143,24 +4143,34 @@ describe('adapter model discovery', () => { daemon.refreshAdapterAvailability(); await settle(); await settle(); expect(daemon.registeredAdapters()[0]).toMatchObject({ models: ['native/model'], - models_error: 'probe timed out', + models_error: 'timed out', }); } finally { await daemon.close(); } }); - it('publishes only a bounded first line of a discovery failure', async () => { - // Requirement: raw CLI diagnostics stay in the server log; the API carries a short line. + it('publishes a controlled error category instead of raw CLI diagnostics', async () => { + // Requirement: credentials and paths in CLI output never cross the API boundary. const daemon = daemonWith([ - adapterWith('leaky', () => Promise.reject(new Error(`boom\n${'x'.repeat(8000)}`))), - adapterWith('chatty', () => Promise.reject(new Error(`w${'o'.repeat(500)}`))), + adapterWith('leaky', () => Promise.reject(new Error( + `Command failed: /opt/agy models\nAUTH_TOKEN=hunter2\n${'x'.repeat(8000)}`, + ))), + adapterWith('slow', () => Promise.reject(new Error('agy models timed out after 20000ms'))), + adapterWith('missing', () => Promise.reject(new Error('spawn agy ENOENT'))), + adapterWith('empty', () => Promise.reject(new Error('agy listed no models'))), + adapterWith('huge', () => Promise.reject(new Error('agy models output exceeded 1000000 bytes'))), ]); try { await settle(); await settle(); const entries = new Map(daemon.registeredAdapters().map((entry) => [entry.id, entry])); - expect(entries.get('leaky')?.models_error).toBe('boom'); - const chatty = entries.get('chatty')?.models_error ?? ''; - expect(chatty).not.toContain('\n'); - expect(chatty.length).toBeLessThanOrEqual(200); + expect(entries.get('leaky')?.models_error).toBe('unexpected error'); + expect(entries.get('slow')?.models_error).toBe('timed out'); + expect(entries.get('missing')?.models_error).toBe('harness not installed'); + expect(entries.get('empty')?.models_error).toBe('harness reported no models'); + expect(entries.get('huge')?.models_error).toBe('output limit exceeded'); + for (const entry of entries.values()) { + expect(entry.models_error ?? '').not.toContain('hunter2'); + expect(entry.models_error ?? '').not.toContain('/opt'); + } } finally { await daemon.close(); } }); diff --git a/packages/switchboard/src/daemon.ts b/packages/switchboard/src/daemon.ts index 8007f593..689f6e0d 100644 --- a/packages/switchboard/src/daemon.ts +++ b/packages/switchboard/src/daemon.ts @@ -343,17 +343,15 @@ function countDiffLines(diff: string): { additions: number; deletions: number } */ const MODEL_ID = /^\w[\w.:-]*(?:\/[\w.:-]+)*$/; const MAX_MODELS = 200; -/** Client-facing discovery errors carry one short line. The full diagnostic - * stays in the server log via onBackgroundError — raw CLI stderr must not - * cross the API boundary into the browser. */ -const MAX_DISCOVERY_ERROR_CHARS = 200; - -function sanitizeDiscoveryError(message: string): string { - const firstLine = message.split('\n', 1)[0] ?? ''; - const trimmed = firstLine.trim(); - return trimmed.length > MAX_DISCOVERY_ERROR_CHARS - ? `${trimmed.slice(0, MAX_DISCOVERY_ERROR_CHARS - 1)}…` - : trimmed; +/** Client-facing discovery outcome: a controlled category, never raw CLI text. + * A credential, path, or other diagnostic inside the raw message stays in the + * server log via onBackgroundError and never crosses the API boundary. */ +function classifyDiscoveryError(message: string): string { + if (/timed out/i.test(message)) return 'timed out'; + if (/ENOENT/i.test(message)) return 'harness not installed'; + if (/listed no models/i.test(message)) return 'harness reported no models'; + if (/exceeded/i.test(message)) return 'output limit exceeded'; + return 'unexpected error'; } export interface DaemonOptions { @@ -1401,7 +1399,7 @@ export class Daemon { this.modelDiscoveryErrors.delete(adapter.id); }).catch((error: unknown) => { const failure = error instanceof Error ? error : new Error(`${adapter.id} model discovery failed`); - this.modelDiscoveryErrors.set(adapter.id, sanitizeDiscoveryError(failure.message)); + this.modelDiscoveryErrors.set(adapter.id, classifyDiscoveryError(failure.message)); this.onBackgroundError(failure); }).finally(() => { this.pendingDiscoveries -= 1; @@ -1460,7 +1458,7 @@ export class Daemon { capabilities: HarnessAdapter['capabilities']; models?: string[]; models_source?: ModelCatalog['source']; - /** Latest discovery failure, bounded to one short line; full detail stays server-side. */ + /** Latest discovery failure as a controlled category; full detail stays server-side. */ models_error?: string; }[] { // Every entry carries its runtime harness id. The generic configurable ACP transport diff --git a/packages/web-next/src/app/session.spec.ts b/packages/web-next/src/app/session.spec.ts index 4b566fe0..b85ba483 100644 --- a/packages/web-next/src/app/session.spec.ts +++ b/packages/web-next/src/app/session.spec.ts @@ -1,7 +1,10 @@ // @vitest-environment happy-dom -import { afterEach, describe, expect, it } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { act } from 'react-dom/test-utils'; +import { createRoot, type Root } from 'react-dom/client'; +import { createElement } from 'react'; -import { pageParams, roomUrl } from './session.js'; +import { pageParams, roomUrl, useAdapterCatalog } from './session.js'; // harn:assume registered-worktree-navigation-is-promotion-gated ref=worktree-url-selector describe('worktree url selector', () => { @@ -30,3 +33,79 @@ describe('worktree url selector', () => { }); }); // harn:end registered-worktree-navigation-is-promotion-gated + +declare global { + var IS_REACT_ACT_ENVIRONMENT: boolean | undefined; +} + +globalThis.IS_REACT_ACT_ENVIRONMENT = true; + +function CatalogProbe(): null { + // Stable identity like the production accessors: a fresh closure per render + // would retrigger the catalog effect on every state update. + useAdapterCatalog(stableToken); + return null; +} + +function stableToken(): string { + return 't'; +} + +// harn:assume model-catalogs-reach-a-browser-that-arrives-early ref=adapter-discovery-poll-budget +describe('adapter discovery poll budget', () => { + let container: HTMLDivElement; + let root: Root | undefined; + let discovering = true; + + const fetchMock = vi.fn(async () => + new Response(JSON.stringify({ adapters: [], discovering }), { status: 200 }), + ); + + beforeEach(() => { + discovering = true; + fetchMock.mockClear(); + vi.stubGlobal('fetch', fetchMock); + vi.useFakeTimers(); + container = document.createElement('div'); + document.body.appendChild(container); + }); + + afterEach(() => { + if (root !== undefined) { + const unmounting = root; + root = undefined; + void act(async () => { unmounting.unmount(); }); + } + container.remove(); + vi.unstubAllGlobals(); + vi.useRealTimers(); + }); + + async function mount(): Promise { + await act(async () => { + root = createRoot(container); + root.render(createElement(CatalogProbe)); + }); + } + + it('keeps polling while discovering past the slowest probe budget', async () => { + // Requirement: the slowest probe runs 30 s, so the client must still ask at 31 s. + await mount(); + await act(async () => { await vi.advanceTimersByTimeAsync(31_000); }); + expect(fetchMock.mock.calls.length).toBeGreaterThan(62); + await act(async () => { await vi.advanceTimersByTimeAsync(60_000); }); + expect(fetchMock.mock.calls.length).toBe(71); + await act(async () => { await vi.advanceTimersByTimeAsync(30_000); }); + expect(fetchMock.mock.calls.length).toBe(71); + }); + + it('stops polling once the catalog reports discovery finished', async () => { + await mount(); + await act(async () => { await vi.advanceTimersByTimeAsync(2_000); }); + discovering = false; + await act(async () => { await vi.advanceTimersByTimeAsync(10_000); }); + const settled = fetchMock.mock.calls.length; + await act(async () => { await vi.advanceTimersByTimeAsync(30_000); }); + expect(fetchMock.mock.calls.length).toBe(settled); + }); +}); diff --git a/packages/web-next/src/app/session.ts b/packages/web-next/src/app/session.ts index fb452998..4d8a3251 100644 --- a/packages/web-next/src/app/session.ts +++ b/packages/web-next/src/app/session.ts @@ -115,10 +115,11 @@ export interface AdapterCatalog { // harn:assume agent-selection-shows-detected-acp-and-advanced-custom ref=detected-acp-catalog-client /** - * Discovery poll budget: the slowest model probe runs 20 s, so keep asking - * well past that before treating the catalog as final. + * Discovery poll budget: the slowest model probe is the copilot 30 s HTTP + * timeout, so keep asking well past that before treating the catalog as final. + * Probe budgets live in the adapters; this window must outlast the slowest one. */ -const ADAPTER_POLL_ATTEMPTS = 45; +const ADAPTER_POLL_ATTEMPTS = 70; const ADAPTER_POLL_MS = 500; export function useAdapterCatalog(token: () => string): AdapterCatalog { diff --git a/packages/web-next/src/room/agent-spec.ts b/packages/web-next/src/room/agent-spec.ts index 9681d763..3219b3a5 100644 --- a/packages/web-next/src/room/agent-spec.ts +++ b/packages/web-next/src/room/agent-spec.ts @@ -50,7 +50,7 @@ export interface AdapterLike { approvals?: string; }; models?: string[]; - /** Latest discovery failure, bounded to one short line: the list is missing or stale, press Refresh to retry. */ + /** Latest discovery failure as a controlled category: the list is missing or stale, press Refresh to retry. */ models_error?: string; } diff --git a/packages/web-next/src/runtime/api.ts b/packages/web-next/src/runtime/api.ts index ac5b9996..bfefcd5c 100644 --- a/packages/web-next/src/runtime/api.ts +++ b/packages/web-next/src/runtime/api.ts @@ -65,7 +65,7 @@ export interface AdapterRegistration { /** Models the harness itself reported. The web never hardcodes a model id. */ models?: string[]; models_source?: 'discovered' | 'curated'; - /** Latest discovery failure, bounded to one short line: the list is missing or stale, press Refresh to retry. */ + /** Latest discovery failure as a controlled category; full detail stays server-side. */ models_error?: string; } // harn:end agent-selection-shows-detected-acp-and-advanced-custom From 578cf8f98a6e3f19e4b8b16970af074ea98da4b7 Mon Sep 17 00:00:00 2001 From: Andro Marces Date: Sat, 12 Sep 2026 18:25:26 +0800 Subject: [PATCH 4/7] fix(discovery): keep CLI stderr out of probe errors, await test cleanup Probe failures carry exit evidence only; harness stderr can hold secrets and this text reaches the server log. Deliberate trade-off: the category plus exit code replaces the stderr tail; full turn diagnostics are unaffected. Classifier narrows to /output exceeded/ so unrelated exceed messages stay unexpected-error. Tests import act from react and await hook unmount. Gate: pnpm -r --no-bail test exits 1 on pre-existing environment failures only (copilot/gemini/grok/opencode cwd argv tests, switchboard artifact/worktree/path tests, web-next clipboard/generator/timeout tests, cli linux-service test); each reproduced on the clean tree or in an untouched package. Touched paths: daemon discovery 18/18, web subset 118/118, antigravity 14/14, opencode 16/17. Refs rjx18/codor#29 --- packages/adapters/antigravity/src/adapter.spec.ts | 14 ++++++++++++++ packages/adapters/antigravity/src/adapter.ts | 12 ++++-------- packages/adapters/opencode/src/adapter.spec.ts | 10 ++++++++++ packages/adapters/opencode/src/adapter.ts | 12 ++++-------- packages/switchboard/src/daemon.ts | 2 +- packages/web-next/src/app/session.spec.ts | 6 +++--- 6 files changed, 36 insertions(+), 20 deletions(-) diff --git a/packages/adapters/antigravity/src/adapter.spec.ts b/packages/adapters/antigravity/src/adapter.spec.ts index 66930fd7..026d28f2 100644 --- a/packages/adapters/antigravity/src/adapter.spec.ts +++ b/packages/adapters/antigravity/src/adapter.spec.ts @@ -246,6 +246,20 @@ setInterval(() => {}, 1000); expect(alive).toBe(false); }); + it('keeps CLI diagnostics out of probe errors', async () => { + // Requirement: harness stderr can carry secrets; it must not enter error text. + const dir = mkdtempSync(join(tmpdir(), 'codor-antigravity-')); + temporary.push(dir); + const path = join(dir, 'agy'); + writeFileSync(path, "#!/usr/bin/env node\nprocess.stderr.write('AUTH_TOKEN=hunter2\\n');\nprocess.exit(1);\n"); + chmodSync(path, 0o755); + const outcome = await new AntigravityAdapter(path).listModels().then( + () => 'resolved', + (reason: unknown) => (reason instanceof Error ? reason.message : String(reason)), + ); + expect(outcome).toBe(`Command failed: ${path} models (exit 1)`); + }); + it('classifies missing commands and nonzero exits as failed', async () => { const missing = new AntigravityAdapter(join(tmpdir(), 'codor-agy-missing')); expect((await collect(missing, missing.spawn({ cwd: process.cwd() }), 'hello')).at(-1)) diff --git a/packages/adapters/antigravity/src/adapter.ts b/packages/adapters/antigravity/src/adapter.ts index da9cee61..cbd3133c 100644 --- a/packages/adapters/antigravity/src/adapter.ts +++ b/packages/adapters/antigravity/src/adapter.ts @@ -87,17 +87,15 @@ async function runBoundedModelsCommand( let deadline: ReturnType | undefined; const budget = Number.isFinite(timeoutMs) && timeoutMs > 0 ? timeoutMs : MODEL_DISCOVERY_TIMEOUT_MS; const work = async (): Promise => { - const spawned = spawn(command, ['models'], { stdio: ['ignore', 'pipe', 'pipe'] }); + const spawned = spawn(command, ['models'], { stdio: ['ignore', 'pipe', 'ignore'] }); child = spawned; return await new Promise((resolve, reject) => { let stdout = ''; let stdoutBytes = 0; - let stderr = ''; let settled = false; const fail = (error: Error): void => { if (!settled) { settled = true; reject(error); } }; const done = (value: string): void => { if (!settled) { settled = true; resolve(value); } }; spawned.stdout?.setEncoding('utf8'); - spawned.stderr?.setEncoding('utf8'); spawned.stdout?.on('data', (chunk: string) => { stdoutBytes += Buffer.byteLength(chunk, 'utf8'); if (stdoutBytes > maxBytes) { @@ -106,17 +104,15 @@ async function runBoundedModelsCommand( } stdout += chunk; }); - spawned.stderr?.on('data', (chunk: string) => { - stderr = `${stderr}${chunk}`.slice(-8192); - }); spawned.once('error', (error) => { fail(error instanceof Error ? error : new Error(String(error))); }); spawned.once('close', (code, signal) => { + // No stderr body here: harness diagnostics can carry secrets, and this + // text reaches both the server log and the client category. Exit evidence only. if (code !== 0) { - const detail = stderr.trim(); fail(new Error( - `Command failed: ${command} models${signal ? ` (${signal})` : ''}${detail === '' ? '' : `\n${detail}`}`, + `Command failed: ${command} models${signal ? ` (signal ${signal})` : ` (exit ${String(code)})`}`, )); return; } diff --git a/packages/adapters/opencode/src/adapter.spec.ts b/packages/adapters/opencode/src/adapter.spec.ts index d395f02e..8fa11f22 100644 --- a/packages/adapters/opencode/src/adapter.spec.ts +++ b/packages/adapters/opencode/src/adapter.spec.ts @@ -179,6 +179,16 @@ describe('opencode model discovery', () => { .rejects.toThrow(); }); + it('keeps CLI diagnostics out of probe errors', async () => { + // Requirement: harness stderr can carry secrets; it must not enter error text. + const command = stub("process.stderr.write('AUTH_TOKEN=hunter2\\n'); process.exit(1);"); + const outcome = await new OpenCodeAdapter(command).listModels().then( + () => 'resolved', + (reason: unknown) => (reason instanceof Error ? reason.message : String(reason)), + ); + expect(outcome).toBe(`Command failed: ${command} models (exit 1)`); + }); + it('lets the event loop run while the CLI answers', async () => { // Requirement: async bounded probe — discovery must not freeze the daemon. const dir = mkdtempSync(join(tmpdir(), 'codor-opencode-models-')); diff --git a/packages/adapters/opencode/src/adapter.ts b/packages/adapters/opencode/src/adapter.ts index 81d09c22..45c01604 100644 --- a/packages/adapters/opencode/src/adapter.ts +++ b/packages/adapters/opencode/src/adapter.ts @@ -58,17 +58,15 @@ async function runBoundedModelsCommand( let deadline: ReturnType | undefined; const budget = Number.isFinite(timeoutMs) && timeoutMs > 0 ? timeoutMs : MODEL_DISCOVERY_TIMEOUT_MS; const work = async (): Promise => { - const spawned = spawn(command, ['models'], { stdio: ['ignore', 'pipe', 'pipe'] }); + const spawned = spawn(command, ['models'], { stdio: ['ignore', 'pipe', 'ignore'] }); child = spawned; return await new Promise((resolve, reject) => { let stdout = ''; let stdoutBytes = 0; - let stderr = ''; let settled = false; const fail = (error: Error): void => { if (!settled) { settled = true; reject(error); } }; const done = (value: string): void => { if (!settled) { settled = true; resolve(value); } }; spawned.stdout?.setEncoding('utf8'); - spawned.stderr?.setEncoding('utf8'); spawned.stdout?.on('data', (chunk: string) => { stdoutBytes += Buffer.byteLength(chunk, 'utf8'); if (stdoutBytes > maxBytes) { @@ -77,17 +75,15 @@ async function runBoundedModelsCommand( } stdout += chunk; }); - spawned.stderr?.on('data', (chunk: string) => { - stderr = `${stderr}${chunk}`.slice(-8192); - }); spawned.once('error', (error) => { fail(error instanceof Error ? error : new Error(String(error))); }); spawned.once('close', (code, signal) => { + // No stderr body here: harness diagnostics can carry secrets, and this + // text reaches both the server log and the client category. Exit evidence only. if (code !== 0) { - const detail = stderr.trim(); fail(new Error( - `Command failed: ${command} models${signal ? ` (${signal})` : ''}${detail === '' ? '' : `\n${detail}`}`, + `Command failed: ${command} models${signal ? ` (signal ${signal})` : ` (exit ${String(code)})`}`, )); return; } diff --git a/packages/switchboard/src/daemon.ts b/packages/switchboard/src/daemon.ts index 689f6e0d..b25e7e4c 100644 --- a/packages/switchboard/src/daemon.ts +++ b/packages/switchboard/src/daemon.ts @@ -350,7 +350,7 @@ function classifyDiscoveryError(message: string): string { if (/timed out/i.test(message)) return 'timed out'; if (/ENOENT/i.test(message)) return 'harness not installed'; if (/listed no models/i.test(message)) return 'harness reported no models'; - if (/exceeded/i.test(message)) return 'output limit exceeded'; + if (/output exceeded/i.test(message)) return 'output limit exceeded'; return 'unexpected error'; } diff --git a/packages/web-next/src/app/session.spec.ts b/packages/web-next/src/app/session.spec.ts index b85ba483..dac3cbcf 100644 --- a/packages/web-next/src/app/session.spec.ts +++ b/packages/web-next/src/app/session.spec.ts @@ -1,6 +1,6 @@ // @vitest-environment happy-dom import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { act } from 'react-dom/test-utils'; +import { act } from 'react'; import { createRoot, type Root } from 'react-dom/client'; import { createElement } from 'react'; @@ -70,11 +70,11 @@ describe('adapter discovery poll budget', () => { document.body.appendChild(container); }); - afterEach(() => { + afterEach(async () => { if (root !== undefined) { const unmounting = root; root = undefined; - void act(async () => { unmounting.unmount(); }); + await act(async () => { unmounting.unmount(); }); } container.remove(); vi.unstubAllGlobals(); From 705ce39a5fb18525680e354952aee4aec6e15354 Mon Sep 17 00:00:00 2001 From: Andro Marces Date: Sat, 12 Sep 2026 18:35:48 +0800 Subject: [PATCH 5/7] docs(discovery): point operators at the direct CLI for failed probes The probe keeps no CLI output on failure (exit evidence only), so both NOTES.md files now tell the operator to run opencode models / agy models on the daemon host. Gate record (pnpm -r --no-bail test, exit 1, all pre-existing/environmental): - copilot/gemini/grok/opencode: one argv-cwd test each, /var-vs-/private/var symlink class (opencode pkg: 26 passed, 1 failed, 1 skipped of 28). - switchboard 22 in the full parallel run: 11 symlink-class (worktrees 7, local-dirs 1, server 2, daemon worktree-roster 1; all pass with TMPDIR resolved) + 8 produced-artifact failures identical on the clean tree (8 failed/8 passed both; my daemon.spec diff is discovery-block only) + a few run-to-run flakes in server/post-correlation/default-recipient under full parallel load. - web-next: clipboard/generator/timeout class plus one computer-sessions failure that fails identically clean; my four files pass 118/118 standalone. - cli: linux-service rendering test on macOS, untouched package. - Playwright: room10-dialogs 29/29 and room31-acp-providers 14/14 (model list, adapter tiles, axe checks). Refs rjx18/codor#29 --- packages/adapters/antigravity/NOTES.md | 4 +++- packages/adapters/opencode/NOTES.md | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/adapters/antigravity/NOTES.md b/packages/adapters/antigravity/NOTES.md index c5a3cae6..1a7f2d7f 100644 --- a/packages/adapters/antigravity/NOTES.md +++ b/packages/adapters/antigravity/NOTES.md @@ -31,7 +31,9 @@ opencode probe; no slow `agy models` run was measured here, so a slower run surf the same retryable failure. The catalog is never hard-coded here. If two names collapse to the same slug, discovery fails rather than routing an operator selection to the wrong model; any failure is recorded per harness (`models_error`, -a controlled category, never raw CLI text) so the dialog offers a retry. +a controlled category, never raw CLI text) so the dialog offers a retry. The probe keeps +no CLI output on failure, so to diagnose a failing harness run `agy models` directly on +the daemon host. ## Resume boundary diff --git a/packages/adapters/opencode/NOTES.md b/packages/adapters/opencode/NOTES.md index 3f3596cd..56178a96 100644 --- a/packages/adapters/opencode/NOTES.md +++ b/packages/adapters/opencode/NOTES.md @@ -151,7 +151,8 @@ observed 0.9–11.97 s spread of `opencode models`, but upstream allows 10 s per with retries, so a slower run can still time out — and then surfaces as the same retryable failure. The dialog falls back to its `Custom…` escape (placeholder `provider/model`, the form opencode's `--model` takes) only while no catalog -was ever reported. +was ever reported. The probe keeps no CLI output on failure, so to diagnose a failing +harness run `opencode models` directly on the daemon host. Thinking is supported and maps to `run --variant `. From a482779ffc0ac8f093dc01f7f3e27d21f1ded444 Mon Sep 17 00:00:00 2001 From: Andro Marces Date: Sat, 12 Sep 2026 18:52:43 +0800 Subject: [PATCH 6/7] fix(discovery): correct log-retention claims in comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Probe stderr is discarded at the OS boundary, so no full diagnostic reaches the server log — only command and exit evidence. Previous comments claimed otherwise. --- packages/switchboard/src/daemon.ts | 6 +++--- packages/web-next/src/runtime/api.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/switchboard/src/daemon.ts b/packages/switchboard/src/daemon.ts index b25e7e4c..7ba954ed 100644 --- a/packages/switchboard/src/daemon.ts +++ b/packages/switchboard/src/daemon.ts @@ -344,8 +344,8 @@ function countDiffLines(diff: string): { additions: number; deletions: number } const MODEL_ID = /^\w[\w.:-]*(?:\/[\w.:-]+)*$/; const MAX_MODELS = 200; /** Client-facing discovery outcome: a controlled category, never raw CLI text. - * A credential, path, or other diagnostic inside the raw message stays in the - * server log via onBackgroundError and never crosses the API boundary. */ + * The probes discard harness stderr at the OS boundary, so the raw output is + * unrecoverable by design; only command and exit evidence reach even the log. */ function classifyDiscoveryError(message: string): string { if (/timed out/i.test(message)) return 'timed out'; if (/ENOENT/i.test(message)) return 'harness not installed'; @@ -1458,7 +1458,7 @@ export class Daemon { capabilities: HarnessAdapter['capabilities']; models?: string[]; models_source?: ModelCatalog['source']; - /** Latest discovery failure as a controlled category; full detail stays server-side. */ + /** Latest discovery failure as a controlled category; only command and exit evidence is logged. */ models_error?: string; }[] { // Every entry carries its runtime harness id. The generic configurable ACP transport diff --git a/packages/web-next/src/runtime/api.ts b/packages/web-next/src/runtime/api.ts index bfefcd5c..60ac16c8 100644 --- a/packages/web-next/src/runtime/api.ts +++ b/packages/web-next/src/runtime/api.ts @@ -65,7 +65,7 @@ export interface AdapterRegistration { /** Models the harness itself reported. The web never hardcodes a model id. */ models?: string[]; models_source?: 'discovered' | 'curated'; - /** Latest discovery failure as a controlled category; full detail stays server-side. */ + /** Latest discovery failure as a controlled category; only command and exit evidence is logged. */ models_error?: string; } // harn:end agent-selection-shows-detected-acp-and-advanced-custom From 3e0f0f9bbdfaf2af77fd361da5fc47aee77e9065 Mon Sep 17 00:00:00 2001 From: Andro Marces Date: Sat, 12 Sep 2026 19:03:10 +0800 Subject: [PATCH 7/7] fix(discovery): narrow the log contract to the API boundary Only the controlled category crosses the API; other adapters retain their own diagnostics, so no broader log claim holds here. --- packages/switchboard/src/daemon.ts | 8 ++++---- packages/web-next/src/runtime/api.ts | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/switchboard/src/daemon.ts b/packages/switchboard/src/daemon.ts index 7ba954ed..768602bb 100644 --- a/packages/switchboard/src/daemon.ts +++ b/packages/switchboard/src/daemon.ts @@ -343,9 +343,9 @@ function countDiffLines(diff: string): { additions: number; deletions: number } */ const MODEL_ID = /^\w[\w.:-]*(?:\/[\w.:-]+)*$/; const MAX_MODELS = 200; -/** Client-facing discovery outcome: a controlled category, never raw CLI text. - * The probes discard harness stderr at the OS boundary, so the raw output is - * unrecoverable by design; only command and exit evidence reach even the log. */ +/** Client-facing discovery outcome: a controlled category. Adapter failure + * details never cross the API boundary. (These two probes also discard + * harness stderr at the OS boundary, so their raw output is unrecoverable.) */ function classifyDiscoveryError(message: string): string { if (/timed out/i.test(message)) return 'timed out'; if (/ENOENT/i.test(message)) return 'harness not installed'; @@ -1458,7 +1458,7 @@ export class Daemon { capabilities: HarnessAdapter['capabilities']; models?: string[]; models_source?: ModelCatalog['source']; - /** Latest discovery failure as a controlled category; only command and exit evidence is logged. */ + /** Latest discovery failure as a controlled category; adapter failure details never cross the API. */ models_error?: string; }[] { // Every entry carries its runtime harness id. The generic configurable ACP transport diff --git a/packages/web-next/src/runtime/api.ts b/packages/web-next/src/runtime/api.ts index 60ac16c8..357e9354 100644 --- a/packages/web-next/src/runtime/api.ts +++ b/packages/web-next/src/runtime/api.ts @@ -65,7 +65,7 @@ export interface AdapterRegistration { /** Models the harness itself reported. The web never hardcodes a model id. */ models?: string[]; models_source?: 'discovered' | 'curated'; - /** Latest discovery failure as a controlled category; only command and exit evidence is logged. */ + /** Latest discovery failure as a controlled category; adapter failure details never cross the API. */ models_error?: string; } // harn:end agent-selection-shows-detected-acp-and-advanced-custom