diff --git a/cli/__tests__/adapters.opencode.test.mjs b/cli/__tests__/adapters.opencode.test.mjs index 86b149b2e6..2906b278da 100644 --- a/cli/__tests__/adapters.opencode.test.mjs +++ b/cli/__tests__/adapters.opencode.test.mjs @@ -28,6 +28,8 @@ const { buildMcpConfig, buildProviderConfig, makeEventParser, + PUBLIC_TOOL_FAILED_CODE, + publicToolFailure, publicPermissions, qualifiedProviderModel, resolveProviderKeyFile, @@ -296,6 +298,139 @@ describe('opencode MCP config', () => { }); }); +// 2026-10-21: a canary of Commonly HQ's support seat (this adapter, its prompt, +// an unreachable API) told the asker "every Commonly tool call (`list_pods`, +// `list_artifacts`) is returning `fetch failed`". A public seat withholds such a +// turn: nothing is posted and the event stays unacked. Events are the shape +// OpenCode 1.19.0 emits, captured from that canary. +// Vera's HOLD on #2374 (79595): matching connection failures let an API 5xx +// through, because the shipped MCP reports it as `{ status, body, message }`. +// The rule is now inverted: only named answerable errors leave the turn to post. +describe('opencode public seat withholds a turn whose Commonly tools failed', () => { + const toolError = (tool, error) => event('tool_use', 'ses_test', { + type: 'tool', tool, state: { status: 'error', input: {}, error, time: {} }, + }); + const narration = 'Every Commonly call is returning fetch failed, so the backend is unreachable.'; + const turn = (...extra) => [ + event('step_start', 'ses_test', { type: 'step-start' }), + ...extra, + event('text', 'ses_test', { type: 'text', text: narration }), + event('step_finish', 'ses_test', { type: 'step-finish', tokens: { input: 1, output: 1 } }), + ]; + const COMMONLY_MCP = [{ name: 'commonly', transport: 'stdio', command: ['npx', '-y', '@commonlyai/mcp@latest'] }]; + + const runSeat = async (chunks, { trust = 'public', mcp = COMMONLY_MCP } = {}) => { + let outcome; + await withTemp(async (root) => { + const home = join(root, 'mcp-home'); + const versionDir = join(home, '0.3.13'); + const pkgDir = join(versionDir, 'node_modules', '@commonlyai', 'mcp'); + await mkdir(join(pkgDir, 'src'), { recursive: true }); + await writeFile(join(pkgDir, 'package.json'), JSON.stringify({ + name: '@commonlyai/mcp', version: '0.3.13', bin: { 'commonly-mcp': 'src/index.js' }, + })); + await writeFile(join(pkgDir, 'src', 'index.js'), ''); + await writeFile(join(home, 'current'), '0.3.13\n'); + const workspace = join(root, 'workspace'); + await mkdir(workspace); + const keyFile = join(root, 'provider-key'); + await writeFile(keyFile, 'provider-secret', { mode: 0o600 }); + return withProcessEnv('COMMONLY_MCP_HOME', home, () => opencode.spawn('a stranger asks which pods they are in', { + cwd: workspace, + env: { PATH: process.env.PATH }, + environment: { + model: 'gpt-5.4', + ...(trust === 'public' ? { sandbox: { trust: 'public', mode: 'read-only' } } : {}), + provider: { id: 'local', baseURL: 'https://llm.example.test/v1', keyFile }, + mcp, + }, + agentName: 'hq-canary', + _opencodeHomeRoot: join(root, 'opencode-state'), + _binaryPath: '/usr/local/bin/opencode', + _platform: 'darwin', + _spawnImpl: makeSpawnImpl({ chunks }).impl, + _wrapArgvWithSeatbelt: (argv) => argv, + }).then((result) => { outcome = result; })); + }); + return outcome; + }; + + // The shipped MCP's error payloads (commonly-mcp/src/tools.js `err`). + const http = (status, message, body = { message }) => JSON.stringify({ status, body, message }); + + test('a Commonly tool that could not connect withholds the reply, and the narration is not in the error', async () => { + const outcome = runSeat(turn(toolError('commonly_commonly_list_pods', '{"message":"fetch failed"}'))); + await expect(outcome).rejects.toMatchObject({ code: PUBLIC_TOOL_FAILED_CODE }); + await expect(runSeat(turn(toolError('commonly_commonly_list_pods', '{"message":"fetch failed"}')))) + .rejects.toThrow(/^(?!.*backend is unreachable).*commonly_commonly_list_pods failed in a way the model must not narrate/); + }); + + test.each([ + ['an ingress 503 page while the socket connects', http(503, '503 Service Temporarily Unavailable', '503 Service Temporarily Unavailable')], + ['a 502 with only its status', http(502, 'HTTP 502', null)], + ['a backend 500 with its own message', http(500, 'Internal server error')], + ['a 504 whose text names no timeout', http(504, 'Bad gateway upstream')], + ['a rejected seat token', http(401, 'Invalid agent token')], + ['a rate limit', http(429, 'Too many requests')], + ['an MCP transport failure', 'MCP error -32000: Connection closed'], + ['an error-flagged result with no text', 'MCP tool returned an error'], + ['an empty error', ''], + ['text this guard does not recognise', 'something unexpected happened'], + ])('%s withholds the reply', async (_label, error) => { + await expect(runSeat(turn(toolError('commonly_commonly_list_pods', error)))) + .rejects.toMatchObject({ code: PUBLIC_TOOL_FAILED_CODE }); + }); + + test.each([ + ['a 403 the person can be told about', http(403, 'You are not a member of this pod')], + ['a 404', http(404, 'Pod not found')], + ['a 422 validation refusal', http(422, 'content is required')], + ['a 400', http(400, 'podId must be an ObjectId')], + ['a 409', http(409, 'Already claimed')], + ['OpenCode\'s own schema check', 'The commonly_commonly_post_message tool was called with invalid arguments: podId: Required. Please rewrite the input so it satisfies the expected schema.'], + ])('%s is still answered', async (_label, error) => { + const result = await runSeat(turn(toolError('commonly_commonly_post_message', error))); + expect(result.text).toBe(narration); + }); + + test('one answerable error does not excuse another that is not', async () => { + await expect(runSeat(turn( + toolError('commonly_commonly_get_context', http(404, 'Pod not found')), + toolError('commonly_commonly_list_pods', http(503, 'HTTP 503', null)), + ))).rejects.toMatchObject({ code: PUBLIC_TOOL_FAILED_CODE }); + }); + + test('a connection failure in a tool that is not Commonly\'s is still answered', async () => { + const result = await runSeat(turn(toolError('webfetch', 'fetch failed'))); + expect(result.text).toBe(narration); + }); + + test('a private seat is not affected', async () => { + const result = await runSeat(turn(toolError('commonly_commonly_list_pods', '{"message":"fetch failed"}')), { trust: 'private' }); + expect(result.text).toBe(narration); + }); + + test('the parser records failed tool calls, whether the error is text or an object', () => { + const parser = makeEventParser(); + parser.consume(toolError('commonly_commonly_list_pods', '{"message":"fetch failed"}')); + parser.consume(toolError('commonly_commonly_get_context', { message: 'connect ECONNREFUSED 127.0.0.1:9' })); + parser.consume(event('tool_use', 'ses_test', { type: 'tool', tool: 'read', state: { status: 'completed', output: 'ok' } })); + parser.flush(); + expect(parser.toolErrors).toEqual([ + { tool: 'commonly_commonly_list_pods', error: '{"message":"fetch failed"}' }, + { tool: 'commonly_commonly_get_context', error: '{"message":"connect ECONNREFUSED 127.0.0.1:9"}' }, + ]); + }); + + test('only the shipped Commonly server counts, under whatever name it was declared', () => { + const failed = [{ tool: 'hq_commonly_list_pods', error: 'fetch failed' }]; + expect(publicToolFailure(failed, [{ name: 'hq', command: ['npx', '-y', '@commonlyai/mcp@0.3.13'] }])) + .toEqual(failed[0]); + expect(publicToolFailure(failed, [{ name: 'hq', command: ['node', '/elsewhere/server.js'] }])).toBeNull(); + expect(publicToolFailure(failed, undefined)).toBeNull(); + }); +}); + describe('opencode external provider config', () => { test('proxies provider requests with a per-spawn bearer and never forwards that bearer upstream', async () => { await withTemp(async (root) => { diff --git a/cli/package-lock.json b/cli/package-lock.json index fee9b4f5c7..6b56df2011 100644 --- a/cli/package-lock.json +++ b/cli/package-lock.json @@ -1,12 +1,12 @@ { "name": "@commonlyai/cli", - "version": "0.1.118", + "version": "0.1.119", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@commonlyai/cli", - "version": "0.1.118", + "version": "0.1.119", "license": "Apache-2.0", "dependencies": { "commander": "^12.0.0" diff --git a/cli/package.json b/cli/package.json index ee5c1ff0d7..857fd23ca3 100644 --- a/cli/package.json +++ b/cli/package.json @@ -1,6 +1,6 @@ { "name": "@commonlyai/cli", - "version": "0.1.118", + "version": "0.1.119", "license": "Apache-2.0", "description": "The Commonly CLI — connect agents, manage pods, iterate fast", "type": "module", diff --git a/cli/src/lib/adapters/opencode.js b/cli/src/lib/adapters/opencode.js index c9929b8c31..55db8b2b7c 100644 --- a/cli/src/lib/adapters/opencode.js +++ b/cli/src/lib/adapters/opencode.js @@ -51,6 +51,8 @@ import { detectBwrap, wrapArgvWithBwrap } from '../sandbox/bwrap.js'; import { wrapArgvWithSeatbelt } from '../sandbox/seatbelt.js'; import { resolvePublicSandboxMode } from '../sandbox/mode.js'; import { resolveProviderKeyFile, startProviderProxy } from '../provider-proxy.js'; +import { isShippedCommonlyMcpCommand } from '../declared-mcp-guard.js'; +import { isAnswerableToolError } from '../public-tool-error.js'; const DEFAULT_TIMEOUT_MS = 15 * 60 * 1000; const TERMINATION_GRACE_MS = 5000; @@ -297,6 +299,9 @@ const makeEventParser = () => { let text = ''; let usage = null; let errorMessage = null; + // Every tool call that came back as an error, for the public-seat check in + // spawn (publicToolFailure). + const toolErrors = []; const consumeLine = (line) => { if (!line.trim()) return; try { @@ -306,6 +311,12 @@ const makeEventParser = () => { text += event.part.text; } else if (event.type === 'step_finish' && event.part?.tokens) { usage = event.part.tokens; + } else if (event.type === 'tool_use' && event.part?.state?.status === 'error') { + const { error } = event.part.state; + toolErrors.push({ + tool: String(event.part.tool || ''), + error: typeof error === 'string' ? error : JSON.stringify(error ?? ''), + }); } else if (event.type === 'error') { errorMessage = event.error?.data?.message || event.error?.message || 'OpenCode turn failed'; } @@ -332,9 +343,50 @@ const makeEventParser = () => { get text() { return text; }, get usage() { return usage; }, get errorMessage() { return errorMessage; }, + get toolErrors() { return toolErrors; }, }; }; +// A public seat must never show a stranger its own plumbing. When a Commonly +// tool can't reach the API, the model is left narrating the failure, and the +// run loop posted that narration: on 2026-10-21 a canary of Commonly HQ's +// support seat (this adapter, its prompt, an unreachable API) told the asker +// "every Commonly tool call (`list_pods`, `list_artifacts`) is returning +// `fetch failed`", and an instruction to answer NO_REPLY in that case held in +// only 1 of 3 turns. So the turn is withheld here instead: nothing is posted, +// and the run loop keeps the event unacked for its retry. +// +// OpenCode names an MCP tool `_`, and a failed call reports +// `state.status: 'error'` with the message in `state.error` (measured on +// OpenCode 1.19.0). Only tools of the declared Commonly server count. Of +// those, only the errors `isAnswerableToolError` names (an API refusal the +// person can be told about, or a schema mistake the model fixes) leave the +// turn to be posted; any other failure withholds it. This adapter-level check +// stays beside the adapter-agnostic readiness witness (TASK-256): it sees the +// runtime's own view of each call, and the witness sees a server that never +// started, so each catches what the other cannot. +export const PUBLIC_TOOL_FAILED_CODE = 'public_tool_failed'; + +export const publicToolFailure = (toolErrors, mcpServers) => { + const prefixes = (Array.isArray(mcpServers) ? mcpServers : []) + .filter((server) => server?.name && isShippedCommonlyMcpCommand(server.command)) + .map((server) => `${server.name}_`); + if (!prefixes.length) return null; + return (toolErrors || []).find(({ tool, error }) => ( + prefixes.some((prefix) => tool.startsWith(prefix)) && !isAnswerableToolError(error) + )) || null; +}; + +const publicToolFailedError = ({ tool, error }) => { + const reason = String(error).replace(/\s+/g, ' ').slice(0, 160) || '(empty error)'; + const failure = new Error( + `public seat withheld this turn's reply: Commonly tool ${tool} failed in a way the model must not narrate ` + + `(${reason}), so nothing was posted and the event stays unacked for retry`, + ); + failure.code = PUBLIC_TOOL_FAILED_CODE; + return failure; +}; + const runOpenCode = ({ cmd, args, cwd, env, timeoutMs, credentials, spawnImpl = childSpawn }) => new Promise((resolve, reject) => { let proc; try { @@ -376,7 +428,9 @@ const runOpenCode = ({ cmd, args, cwd, env, timeoutMs, credentials, spawnImpl = credentials, exitCode: code, limit: 500, })); } - resolve({ sessionId: events.sessionId, text: events.text, usage: events.usage }); + resolve({ + sessionId: events.sessionId, text: events.text, usage: events.usage, toolErrors: events.toolErrors, + }); }); }); @@ -783,6 +837,10 @@ export default { credentials, spawnImpl: ctx._spawnImpl, }); + if (isPublic) { + const failure = publicToolFailure(result.toolErrors, ctx.environment?.mcp); + if (failure) throw publicToolFailedError(failure); + } return { text: result.text.trim(), newSessionId: result.sessionId || ctx.sessionId || null, diff --git a/cli/src/lib/public-tool-error.js b/cli/src/lib/public-tool-error.js new file mode 100644 index 0000000000..4191c2feb1 --- /dev/null +++ b/cli/src/lib/public-tool-error.js @@ -0,0 +1,54 @@ +/** + * Which failed Commonly tool calls a public seat's model may answer in plain + * words. Everything else withholds the turn (#2374, Vera 79595, Wren 79596). + * + * The rule is inverted on purpose. Matching known connection failures let an + * API 5xx through: the shipped MCP turns a non-2xx into a tool error carrying + * the status, and during a rollout the ingress answers 503 while the socket + * connects, so the model narrated the outage to a stranger. A list of failures + * fails open on the next one nobody listed; a list of answers fails closed. + * + * The answerable ones: + * - an API refusal the person can be told about: 400, 403, 404, 409, 410, 413 + * or 422. 401 is NOT among them: a seat whose token the API rejects is + * broken plumbing, and every later call fails the same way. + * - the runtime's own schema check, which the model fixes on its next call. + * OpenCode 1.19.0 words it "The tool was called with invalid + * arguments: …" (ToolInvalidArgumentsError, read from the shipped binary). + * + * Everything else withholds: a 5xx, 401, 429 or any other status; a tool error + * with no status (the shipped MCP's network, timeout and token failures); an + * MCP transport error ("MCP error -32000: Connection closed"); an empty error; + * and any text this module does not recognise. + * + * How the status arrives: the shipped MCP answers a failed call with + * `isError: true` and one text part, `JSON.stringify({ status, body, message })` + * for an HTTP error or `{ message }` otherwise (commonly-mcp/src/tools.js, + * `err`). OpenCode throws that text as the tool's error, so the event's + * `state.error` is the JSON (measured on the 2026-10-21 canary). + * + * The TASK-256 readiness witness uses this same predicate, so the two + * witnesses cannot disagree about what counts as a failure. + */ + +export const ANSWERABLE_API_STATUSES = new Set([400, 403, 404, 409, 410, 413, 422]); + +const INVALID_ARGUMENTS_RE = /^The \S+ tool was called with invalid arguments: /; + +const parsePayload = (text) => { + try { + const parsed = JSON.parse(text); + return parsed && typeof parsed === 'object' && !Array.isArray(parsed) ? parsed : null; + } catch { + return null; + } +}; + +/** Whether a failed tool call's error text is one the model may answer. */ +export const isAnswerableToolError = (error) => { + const text = typeof error === 'string' ? error.trim() : ''; + if (!text) return false; + if (INVALID_ARGUMENTS_RE.test(text)) return true; + const payload = parsePayload(text); + return Boolean(payload && Number.isInteger(payload.status) && ANSWERABLE_API_STATUSES.has(payload.status)); +};