From 3730dc93414d3276918546ebee21482d64e3cc54 Mon Sep 17 00:00:00 2001 From: indifferentketchup Date: Fri, 29 May 2026 04:07:43 +0000 Subject: [PATCH] =?UTF-8?q?coder(providers):=20v2.3=20provider-lifecycle?= =?UTF-8?q?=20phase=201=20=E2=80=94=20config-backed=20registry?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a config layer merged over the hardcoded built-ins (tasks 1.1-1.6): CODER_PROVIDERS_PATH env (default /data/coder-providers.json); provider-config.ts (Zod schema + never-throw loader — missing/invalid file falls back to built-ins only — + save); provider-config-registry.ts (ResolvedProviderDef + buildResolvedRegistry merge: override built-ins, add custom extends:'acp' entries, boocode always enabled + singleton); agent-probe now iterates the resolved registry, probes custom-ACP command[0] via execFile (no shell), skips disabled providers (keeps the row), reads enabled from memory only (no DB column). No snapshot/dispatch/route/UI changes (Phase 2+). 6 new unit tests; empty config provably yields exactly the built-ins. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 8 ++ apps/coder/.env.host | 1 + apps/coder/src/config.ts | 3 + .../provider-config-registry.test.ts | 77 +++++++++++ apps/coder/src/services/agent-probe.ts | 91 +++++++++---- .../src/services/provider-config-registry.ts | 125 ++++++++++++++++++ apps/coder/src/services/provider-config.ts | 65 +++++++++ 7 files changed, 346 insertions(+), 24 deletions(-) create mode 100644 apps/coder/src/services/__tests__/provider-config-registry.test.ts create mode 100644 apps/coder/src/services/provider-config-registry.ts create mode 100644 apps/coder/src/services/provider-config.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index facd591..8716b8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ All notable changes per release tag. Most recent on top, ordered by tag creation date (which matches the git history). Tag names follow `vMAJOR.MINOR.PATCH-slug` — the slug describes what shipped, so the tag name alone is enough to recall the batch. +## v2.5.4-provider-lifecycle-phase1 — 2026-05-29 + +Phase 1 of the v2.3 provider-lifecycle batch (`openspec/changes/v2-3-provider-lifecycle/design.md` §2–3): a config-backed provider layer merged over the hardcoded built-ins, with no runtime change when no config file exists. Adds `CODER_PROVIDERS_PATH` (default `/data/coder-providers.json`); `provider-config.ts` (Zod `ProviderOverride`/`CoderProvidersFile` schemas + a loader that never throws at startup — a missing file, invalid JSON, or schema mismatch all fall back to built-ins-only — plus `save` for the Phase 4 PATCH route); and `provider-config-registry.ts` (`ResolvedProviderDef` + `buildResolvedRegistry` merge: built-in overrides, custom `extends:'acp'` entries requiring label+command, `boocode` always enabled, plus a module singleton). `agent-probe.ts` now iterates the resolved registry instead of the hardcoded list — custom ACP entries resolve their binary from `command[0]` via `execFile` (no shell), disabled providers skip probing without losing their row, and `enabled` is read from memory only (no DB column this phase). Six unit tests, including a regression proving an empty config yields exactly the built-ins. No snapshot/dispatch/route/UI changes (Phase 2+). The `data/coder-providers.json` seed exists on disk but is gitignored (`data/*`). Lands on top of `v2.5.3-remove-cursor-copilot`. + +## v2.5.3-remove-cursor-copilot — 2026-05-29 + +Retire the cursor and copilot providers from BooCoder entirely. Removes their `acp-spawn` argv cases, `provider-manifest` mode blocks + manifest keys, `provider-commands` command maps, the `provider-snapshot` cursor model-CLI branch (and the now-orphaned `exec`/`promisify` imports), and the `agent-probe` copilot ACP-detect branch; deletes the dead `cursor-models.ts` module and its test. The `PROVIDERS` registry array already lacked both entries, so only the doc comment needed correcting. Built-ins unchanged: claude, opencode, goose, qwen, native boocode. Standalone cleanup; pairs with `v2.5.4-provider-lifecycle-phase1` which builds on it. + ## v2.5.2-coder-ux-fixes — 2026-05-29 Working-tree checkpoint bundling this session's fixes with in-progress coder UI work. This session: the BooCoder dispatcher now reacts to new tasks immediately via a Postgres `LISTEN/NOTIFY` (`tasks_new`) AFTER INSERT trigger, with the poll loop kept at 2s as a missed-notification fallback (`dispatcher.ts`, `apps/coder/src/schema.sql`); the mobile nav drawer no longer sticks open after returning to a backgrounded tab — `useViewport` re-syncs on `pageshow`/`visibilitychange`/`resize`/`orientationchange` (iOS reported a stale width on bfcache restore, leaving `isMobile=false`); assistant reasoning renders as a collapsible "Thinking" block in `MessageBubble`, surfacing ACP `agent_thought_chunk` from opencode/goose/qwen and native `reasoning_parts`; paste-to-chip inserts pasted text verbatim instead of wrapping it in a code fence; and a "New file from pasted text" affordance in the RightRail browser queues a `pending_changes` create through the new `POST /api/sessions/:id/pending/create` endpoint, paired with a fix repointing the DiffPanel's dead approve/reject calls to the real `/api/pending/:id/apply` and `/reject` routes. Also carried in the tree but not authored this session: the CoderPane `ChatInput` migration and `AgentComposerBar` refinements, plus backend tweaks to `auto_name`, inference `tool-phase`/`turn`, `secret_guard`, and `provider-registry`. Ships the `v2-6-persistent-agent-sessions` openspec proposal/design/tasks (free agent-switching with per-agent memory, opencode-as-server) as planning docs only — the feature is unimplemented and reserves the `v2.6.0` tag for it. Build green across server/coder/web; server suite 531 passing. (CHANGELOG note: the v2.3–v2.5.1 entries were never backfilled and remain absent above.) diff --git a/apps/coder/.env.host b/apps/coder/.env.host index 4c8bc25..48b5c3c 100644 --- a/apps/coder/.env.host +++ b/apps/coder/.env.host @@ -13,3 +13,4 @@ GITEA_USER=indifferentketchup GITEA_SSH_HOST=100.114.205.53:2222 MCP_CONFIG_PATH=/data/mcp.json SKILLS_ROOT=/opt/boocode/data/skills +CODER_PROVIDERS_PATH=/opt/boocode/data/coder-providers.json diff --git a/apps/coder/src/config.ts b/apps/coder/src/config.ts index 7ecff4e..6ce7e27 100644 --- a/apps/coder/src/config.ts +++ b/apps/coder/src/config.ts @@ -23,6 +23,9 @@ const ConfigSchema = z.object({ GITEA_TOKEN: z.string().optional(), GITEA_SSH_HOST: z.string().default('100.114.205.53:2222'), MCP_CONFIG_PATH: z.string().optional(), + // v2.3: config-backed provider overrides/custom-ACP entries merged over the + // hardcoded built-ins. Missing file = built-ins only (see provider-config.ts). + CODER_PROVIDERS_PATH: z.string().default('/data/coder-providers.json'), // v2.0.5: cheaper model for titles, summaries, labeling. FAST_MODEL: z.string().optional(), // SSH access to the host for external agent dispatch (Phase 5) diff --git a/apps/coder/src/services/__tests__/provider-config-registry.test.ts b/apps/coder/src/services/__tests__/provider-config-registry.test.ts new file mode 100644 index 0000000..3b5f761 --- /dev/null +++ b/apps/coder/src/services/__tests__/provider-config-registry.test.ts @@ -0,0 +1,77 @@ +import { describe, it, expect, vi } from 'vitest'; +import { buildResolvedRegistry } from '../provider-config-registry.js'; +import { PROVIDERS } from '../provider-registry.js'; +import type { CoderProvidersFile } from '../provider-config.js'; + +describe('buildResolvedRegistry', () => { + it('applies a built-in override (goose label)', () => { + const config: CoderProvidersFile = { providers: { goose: { label: 'Goosey' } } }; + const reg = buildResolvedRegistry(PROVIDERS, config); + const goose = reg.get('goose'); + expect(goose).toBeDefined(); + expect(goose!.label).toBe('Goosey'); + expect(goose!.configLabel).toBe('Goosey'); + expect(goose!.enabled).toBe(true); + expect(goose!.isBuiltin).toBe(true); + expect(goose!.isCustomAcp).toBe(false); + }); + + it('adds a custom ACP entry (extends:acp + label + command)', () => { + const config: CoderProvidersFile = { + providers: { + 'amp-acp': { extends: 'acp', label: 'Amp', description: 'ACP wrapper', command: ['amp-acp', '--acp'], env: { AMP: '1' } }, + }, + }; + const reg = buildResolvedRegistry(PROVIDERS, config); + const amp = reg.get('amp-acp'); + expect(amp).toBeDefined(); + expect(amp!.isCustomAcp).toBe(true); + expect(amp!.isBuiltin).toBe(false); + expect(amp!.transport).toBe('acp'); + expect(amp!.modelSource).toBe('probe'); + expect(amp!.launchCommand).toEqual(['amp-acp', '--acp']); + expect(amp!.env).toEqual({ AMP: '1' }); + expect(amp!.enabled).toBe(true); + }); + + it('keeps a disabled built-in in the registry flagged disabled (goose)', () => { + const config: CoderProvidersFile = { providers: { goose: { enabled: false } } }; + const reg = buildResolvedRegistry(PROVIDERS, config); + expect(reg.has('goose')).toBe(true); + expect(reg.get('goose')!.enabled).toBe(false); + }); + + it('skips a custom id without extends (no throw)', () => { + const config: CoderProvidersFile = { providers: { weird: { label: 'Weird', command: ['weird'] } } }; + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const reg = buildResolvedRegistry(PROVIDERS, config); + expect(reg.has('weird')).toBe(false); + // built-ins untouched + expect(reg.size).toBe(PROVIDERS.length); + expect(warn).toHaveBeenCalled(); + warn.mockRestore(); + }); + + it('ignores enabled:false on boocode and warns', () => { + const config: CoderProvidersFile = { providers: { boocode: { enabled: false } } }; + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const reg = buildResolvedRegistry(PROVIDERS, config); + expect(reg.get('boocode')!.enabled).toBe(true); + expect(warn).toHaveBeenCalled(); + warn.mockRestore(); + }); + + it('REGRESSION: empty config returns exactly the built-ins, all enabled', () => { + const reg = buildResolvedRegistry(PROVIDERS, { providers: {} }); + expect(reg.size).toBe(PROVIDERS.length); + expect([...reg.keys()]).toEqual(PROVIDERS.map((p) => p.name)); + for (const def of PROVIDERS) { + const r = reg.get(def.name)!; + expect(r.enabled).toBe(true); + expect(r.isBuiltin).toBe(true); + expect(r.isCustomAcp).toBe(false); + expect(r.launchCommand).toBeNull(); + expect(r.label).toBe(def.label); + } + }); +}); diff --git a/apps/coder/src/services/agent-probe.ts b/apps/coder/src/services/agent-probe.ts index 488b7d3..f0a77cb 100644 --- a/apps/coder/src/services/agent-probe.ts +++ b/apps/coder/src/services/agent-probe.ts @@ -1,24 +1,34 @@ import type { Sql } from '../db.js'; import type { FastifyBaseLogger } from 'fastify'; -import { exec as execCb } from 'node:child_process'; +import { exec as execCb, execFile as execFileCb } from 'node:child_process'; import { promisify } from 'node:util'; -import { PROVIDERS_BY_NAME, PROBED_AGENT_NAMES } from './provider-registry.js'; +import { PROVIDERS_BY_NAME } from './provider-registry.js'; import { resolveAcpProbeBinaries } from './acp-spawn.js'; import { clearProviderSnapshotCache } from './provider-snapshot.js'; import { readQwenSettingsModels } from './qwen-settings.js'; +import { loadConfig } from '../config.js'; +import { loadProviderConfig } from './provider-config-registry.js'; const exec = promisify(execCb); +const execFile = promisify(execFileCb); + +// `which` via execFile (no shell) — the binary name can come from the config +// file (custom ACP entries), so avoid interpolating it into a shell string. +async function whichBinary(bin: string): Promise { + try { + const { stdout } = await execFile('which', [bin], { timeout: 10_000 }); + const path = stdout.trim(); + return path || null; + } catch { + return null; + } +} async function resolveInstallPath(agentName: string): Promise { const candidates = resolveAcpProbeBinaries(agentName); for (const bin of candidates) { - try { - const { stdout } = await exec(`which ${bin}`, { timeout: 10_000 }); - const path = stdout.trim(); - if (path) return path; - } catch { - /* try next */ - } + const path = await whichBinary(bin); + if (path) return path; } return null; } @@ -46,14 +56,37 @@ async function detectAcpSupport(agentName: string, installPath: string): Promise /** * Probe for available agents on the HOST. + * + * v2.3: iterates the resolved provider registry (built-ins + config-backed + * custom ACP entries) rather than the hardcoded `PROBED_AGENT_NAMES`. Native + * boocode is not probed; disabled providers are skipped (their `available_agents` + * row is kept, not deleted). `enabled` is read from the in-memory registry only — + * no DB column in Phase 1 (design.md §3.3). */ export async function probeAgents(sql: Sql, log: FastifyBaseLogger): Promise { clearProviderSnapshotCache(); log.info('agent-probe: scanning for known agents'); - for (const agentName of PROBED_AGENT_NAMES) { + const registry = loadProviderConfig(loadConfig().CODER_PROVIDERS_PATH); + + for (const resolved of registry.values()) { + const agentName = resolved.id; + + // Native boocode is not a probed host agent. + if (resolved.transport === 'native') continue; + + // Disabled providers: skip the probe, keep any existing row. + if (!resolved.enabled) { + log.info({ agent: agentName }, 'agent-probe: skipping disabled provider'); + continue; + } + try { - const installPath = await resolveInstallPath(agentName); + // Custom ACP entries resolve their binary from command[0]; built-ins use + // the per-agent probe binaries. + const installPath = resolved.isCustomAcp && resolved.launchCommand + ? await whichBinary(resolved.launchCommand[0]) + : await resolveInstallPath(agentName); if (!installPath) continue; let version: string | null = null; @@ -64,24 +97,34 @@ export async function probeAgents(sql: Sql, log: FastifyBaseLogger): Promise = []; - if (providerDef?.modelSource === 'static' && providerDef.staticModels) { - models = providerDef.staticModels; + if (!resolved.isCustomAcp) { + const providerDef = PROVIDERS_BY_NAME.get(agentName); + if (providerDef?.modelSource === 'static' && providerDef.staticModels) { + models = providerDef.staticModels; + } + if (agentName === 'qwen') { + models = await readQwenSettingsModels(); + } } - if (agentName === 'qwen') { - models = await readQwenSettingsModels(); - } - - const label = providerDef?.label ?? agentName; - const transport = - providerDef?.transport === 'acp' && !supportsAcp ? 'pty' : (providerDef?.transport ?? 'pty'); + const label = resolved.configLabel ?? resolved.label; + const transport = resolved.isCustomAcp + ? 'acp' + : resolved.transport === 'acp' && !supportsAcp + ? 'pty' + : (resolved.transport ?? 'pty'); await sql` INSERT INTO available_agents (name, install_path, version, supports_acp, last_probed_at, models, label, transport) diff --git a/apps/coder/src/services/provider-config-registry.ts b/apps/coder/src/services/provider-config-registry.ts new file mode 100644 index 0000000..e8e3b52 --- /dev/null +++ b/apps/coder/src/services/provider-config-registry.ts @@ -0,0 +1,125 @@ +/** + * v2.3 resolved provider registry — single in-memory source of truth after + * merging the hardcoded built-ins (provider-registry.ts) with the config file + * (provider-config.ts). Mirrors Paseo's buildProviderRegistry/addDerivedProviders. + * + * Phase 1 scope: build + expose the resolved registry. `launchCommand` is null + * for built-ins (the default argv is resolved at dispatch time in Phase 3) and + * is the config `command` for custom ACP entries. No DB columns (design.md §3.3); + * `enabled` lives in memory only. + */ +import type { ProviderDef } from './provider-registry.js'; +import { PROVIDERS } from './provider-registry.js'; +import { load, type CoderProvidersFile } from './provider-config.js'; + +export interface ResolvedProviderDef extends ProviderDef { + id: string; + enabled: boolean; + isBuiltin: boolean; + isCustomAcp: boolean; + /** Full argv for spawn: [binary, ...args]. Null for built-ins (resolved at dispatch). */ + launchCommand: [string, ...string[]] | null; + env: Record | undefined; + configLabel?: string; + configDescription?: string; +} + +/** + * Merge built-ins with config overrides into the resolved registry. + * Algorithm verbatim from design.md §3.1. + */ +export function buildResolvedRegistry( + builtins: ProviderDef[], + config: CoderProvidersFile, +): Map { + const out = new Map(); + const overrides = config.providers ?? {}; + const builtinNames = new Set(builtins.map((b) => b.name)); + + // 1. Built-ins, applying a config override if one is present. + for (const def of builtins) { + const ov = overrides[def.name]; + let enabled = ov?.enabled !== false; + + // 3. boocode is always enabled; an enabled:false override is ignored + warned. + if (def.name === 'boocode' && ov?.enabled === false) { + console.warn("provider-config: ignoring enabled:false for built-in 'boocode' (always enabled)"); + enabled = true; + } + + const launchCommand = + ov?.command && ov.command.length > 0 ? (ov.command as [string, ...string[]]) : null; + + out.set(def.name, { + ...def, + label: ov?.label ?? def.label, + id: def.name, + enabled, + isBuiltin: true, + isCustomAcp: false, + launchCommand, + env: ov?.env, + configLabel: ov?.label, + configDescription: ov?.description, + }); + } + + // 2. Config ids that are not built-ins → custom ACP entries. + for (const [id, ov] of Object.entries(overrides)) { + if (builtinNames.has(id)) continue; + // §2.2 rules: "New id without extends → Reject at load with log." + if (ov.extends !== 'acp' || !ov.label || !ov.command || ov.command.length === 0) { + console.warn( + `provider-config: skipping custom provider '${id}' — requires extends:'acp', label, and command`, + ); + continue; + } + out.set(id, { + name: id, + label: ov.label, + transport: 'acp', + modelSource: 'probe', + id, + enabled: ov.enabled !== false, + isBuiltin: false, + isCustomAcp: true, + launchCommand: ov.command as [string, ...string[]], + env: ov.env, + configLabel: ov.label, + configDescription: ov.description, + }); + } + + return out; +} + +// --- Module singleton --------------------------------------------------------- + +let cachedRegistry: Map | null = null; +let cachedPath: string | null = null; + +/** Load the config file at `path`, rebuild, and cache the resolved registry. */ +export function loadProviderConfig(path: string): Map { + cachedPath = path; + cachedRegistry = buildResolvedRegistry(PROVIDERS, load(path)); + return cachedRegistry; +} + +/** Re-read the last-loaded config file and rebuild (Phase 4 calls this after PATCH). */ +export function reloadProviderConfig(): Map { + if (cachedPath == null) { + cachedRegistry = buildResolvedRegistry(PROVIDERS, { providers: {} }); + return cachedRegistry; + } + return loadProviderConfig(cachedPath); +} + +/** The cached resolved registry (built-ins only if nothing has been loaded yet). */ +export function getResolvedRegistry(): Map { + return cachedRegistry ?? buildResolvedRegistry(PROVIDERS, { providers: {} }); +} + +/** Resolved provider ids in registry order. */ +export function getResolvedProviderIds(): string[] { + return [...getResolvedRegistry().keys()]; +} diff --git a/apps/coder/src/services/provider-config.ts b/apps/coder/src/services/provider-config.ts new file mode 100644 index 0000000..12a8df5 --- /dev/null +++ b/apps/coder/src/services/provider-config.ts @@ -0,0 +1,65 @@ +/** + * v2.3 provider config file (`/data/coder-providers.json`) — schema + loader. + * + * Layers config-backed overrides/custom-ACP entries over the hardcoded built-ins + * (see provider-config-registry.ts). Loading NEVER throws at startup (design.md + * §2.1): a missing file, invalid JSON, or schema mismatch all fall back to + * `{ providers: {} }` (built-ins only, all enabled). + */ +import { readFileSync, writeFileSync } from 'node:fs'; +import { z } from 'zod'; + +// Schemas verbatim from design.md §2.2. +export const ProviderOverrideSchema = z.object({ + extends: z.enum(['acp']).optional(), // v2.3: only 'acp' for custom; built-ins omit extends + label: z.string().min(1).optional(), + description: z.string().optional(), + command: z.array(z.string().min(1)).min(1).optional(), // [binary, ...args] + env: z.record(z.string()).optional(), + enabled: z.boolean().optional(), // default true + order: z.number().int().optional(), // UI sort key + models: z.array(z.object({ id: z.string(), label: z.string() })).optional(), + additionalModels: z.array(z.object({ id: z.string(), label: z.string() })).optional(), +}); + +export const CoderProvidersFileSchema = z.object({ + providers: z.record(ProviderOverrideSchema).default({}), +}); + +export type ProviderOverride = z.infer; +export type CoderProvidersFile = z.infer; + +/** Read + parse + validate. Falls back to built-ins-only on any failure; never throws. */ +export function load(path: string): CoderProvidersFile { + let raw: string; + try { + raw = readFileSync(path, 'utf8'); + } catch { + // Missing file → built-ins only. Expected, not an error. + return { providers: {} }; + } + + let json: unknown; + try { + json = JSON.parse(raw); + } catch (err) { + console.error(`provider-config: invalid JSON in ${path} — using built-ins only`, err); + return { providers: {} }; + } + + const parsed = CoderProvidersFileSchema.safeParse(json); + if (!parsed.success) { + console.error( + `provider-config: schema validation failed for ${path} — using built-ins only`, + parsed.error.flatten(), + ); + return { providers: {} }; + } + + return parsed.data; +} + +/** Write the config back to disk (used by the Phase 4 PATCH route). */ +export function save(path: string, config: CoderProvidersFile): void { + writeFileSync(path, `${JSON.stringify(config, null, 2)}\n`, 'utf8'); +}