From 8570f1bf4f62a3bf3f8e0f0fa250eb9186a67b18 Mon Sep 17 00:00:00 2001 From: FlopBut Date: Mon, 21 Sep 2026 00:14:28 +0200 Subject: [PATCH 1/6] Check types, and make the tree pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With no build step, nothing checks types unless something asks: `tsx` transpiles and throws the types away, which is why a strict `tsc --noEmit` over the sources found 17 errors that had been sitting in running code. `yarn typecheck` is that ask, and it carries the compiler flags in place of the `tsconfig.json` this repository deliberately does not have — until now those flags existed only inside the audit document. The errors were real, if mostly harmless. `src/client.ts` read `status` and `artifact` off stream payloads as though a protobuf message field were ever required; a status update without a message would have thrown in the printer rather than printing nothing. Its `default:` branch reached for `$case` on an exhausted union, so the one line meant to describe an unrecognised payload could not have described it. `src/acp/client.ts` declared its adapters as `ChildProcessWithoutNullStreams` while spawning them with stderr inherited — two pipes, not three — and read fields off the `initialize` response the SDK hands back as `unknown`. Verified: `yarn typecheck` reports nothing across `src/`, and `yarn client` against the Revisor stub still walks the documented cycle — `SUBMITTED → INPUT_REQUIRED → WORKING → artifact → COMPLETED` — with every event rendered as before. Co-Authored-By: Claude Opus 5 (1M context) --- package.json | 1 + src/acp/client.ts | 23 +++++++++++++++++------ src/client.ts | 30 ++++++++++++++++++++---------- 3 files changed, 38 insertions(+), 16 deletions(-) diff --git a/package.json b/package.json index 7e336a8..a9c25b7 100644 --- a/package.json +++ b/package.json @@ -7,6 +7,7 @@ "test": "vitest run", "test:watch": "vitest", "test:coverage": "vitest run --coverage", + "typecheck": "tsc --noEmit --module nodenext --moduleResolution nodenext --target es2023 --lib es2023,dom --allowImportingTsExtensions --skipLibCheck --strict --types node,express src/*.ts src/acp/*.ts src/mcp/*.ts", "agent": "tsx src/agent.ts", "agent:claude": "ACP_AGENT=claude tsx src/acp/agent.ts", "agent:codex": "ACP_AGENT=codex tsx src/acp/agent.ts", diff --git a/src/acp/client.ts b/src/acp/client.ts index 7148c32..7ba3ea1 100644 --- a/src/acp/client.ts +++ b/src/acp/client.ts @@ -6,7 +6,7 @@ * editor plays. This file owns that side: spawning adapters, opening sessions, * keeping one turn in flight at a time, and reaping processes that went quiet. */ -import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process'; +import { spawn, type ChildProcessByStdio } from 'node:child_process'; import { join } from 'node:path'; import { Readable, Writable } from 'node:stream'; import { fileURLToPath } from 'node:url'; @@ -78,8 +78,17 @@ const CLIENT_CAPABILITIES = { terminal: false, }; +/** + * An adapter as it is actually spawned: two pipes and an inherited stderr. + * + * Not `ChildProcessWithoutNullStreams`, which promises three — stderr is inherited on + * purpose, so the adapter's own diagnostics land in the terminal beside ours rather than + * in a buffer nobody reads. + */ +type Adapter = ChildProcessByStdio; + interface Connected { - child: ChildProcessWithoutNullStreams; + child: Adapter; conn: ClientConnection; } @@ -132,7 +141,7 @@ const connect = (backend: Backend, cwd: string, supervise: () => Supervision): C * `error` counts as an ending too: a child that never spawned — a missing binary — emits * no `exit`, and waiting for one would hang the caller cleaning up after it. */ -const stop = async (child: ChildProcessWithoutNullStreams): Promise => { +const stop = async (child: Adapter): Promise => { if (child.exitCode !== null || child.signalCode !== null) return; const exited = new Promise((done) => { child.once('exit', () => done()); @@ -192,7 +201,7 @@ export class Runtime { constructor( readonly contextId: string, readonly boundary: Boundary, - private readonly child: ChildProcessWithoutNullStreams, + private readonly child: Adapter, private readonly conn: ClientConnection, readonly session: ActiveSession, /** The very object the permission handlers read from — shared, not copied. */ @@ -303,11 +312,13 @@ export class AcpRegistry { const { child, conn } = connect(this.opts.backend, process.cwd(), () => ({ boundary })); const started = Date.now(); try { - const result = await conn.agent.request(acp.methods.agent.initialize, { + // The SDK types `request` by method only for its own method constants; this one + // comes back as `unknown`, and the response shape is what the probe exists to read. + const result = (await conn.agent.request(acp.methods.agent.initialize, { protocolVersion: acp.PROTOCOL_VERSION, clientCapabilities: CLIENT_CAPABILITIES, clientInfo: { name: CLIENT_NAME, version: '0.1.0' }, - }); + })) as InitializeResponse; this.opts.logs.call('handshake', { backend: this.opts.backend.id, bin: this.opts.backend.bin, diff --git a/src/client.ts b/src/client.ts index ef4907e..ba340b4 100644 --- a/src/client.ts +++ b/src/client.ts @@ -6,7 +6,7 @@ * Turn 2 — we send the text into THE SAME task → the agent drives it to COMPLETED. * That is the difference between A2A and a plain "POST /do": the call has state. */ -import { Role, TaskState, type Message, type Part, type StreamResponse } from '@a2a-js/sdk'; +import { Role, TaskState, type Message, type Part, type StreamResponse, type TaskStatus } from '@a2a-js/sdk'; import { ClientFactory } from '@a2a-js/sdk/client'; const BASE_URL = process.env.AGENT_URL || 'http://localhost:41241'; @@ -33,26 +33,36 @@ const renderParts = (parts: Part[]): string => }) .join('\n'); +// `status` and `artifact` are optional in the generated types, because every message +// field of a protobuf is. One helper keeps that fact out of the printing below. +const stateOf = (status: TaskStatus | undefined): TaskState => status?.state ?? TaskState.TASK_STATE_UNSPECIFIED; + /** Prints a stream event and returns whatever task identifiers it carried. */ function render(event: StreamResponse): { taskId?: string; contextId?: string; state?: TaskState } { const p = event.payload; switch (p?.$case) { case 'task': - console.log(` [task] id=${p.value.id.slice(0, 8)} state=${TaskState[p.value.status.state]}`); - return { taskId: p.value.id, contextId: p.value.contextId, state: p.value.status.state }; + console.log(` [task] id=${p.value.id.slice(0, 8)} state=${TaskState[stateOf(p.value.status)]}`); + return { taskId: p.value.id, contextId: p.value.contextId, state: stateOf(p.value.status) }; case 'statusUpdate': { - const note = p.value.status.message ? ` — ${renderParts(p.value.status.message.parts)}` : ''; - console.log(` [statusUpdate] ${TaskState[p.value.status.state]}${note}`); - return { taskId: p.value.taskId, contextId: p.value.contextId, state: p.value.status.state }; + const message = p.value.status?.message; + const note = message ? ` — ${renderParts(message.parts)}` : ''; + console.log(` [statusUpdate] ${TaskState[stateOf(p.value.status)]}${note}`); + return { taskId: p.value.taskId, contextId: p.value.contextId, state: stateOf(p.value.status) }; } - case 'artifactUpdate': - console.log(` [artifactUpdate] «${p.value.artifact.name}»\n${renderParts(p.value.artifact.parts)}`); + case 'artifactUpdate': { + const artifact = p.value.artifact; + const body = artifact ? `«${artifact.name}»\n${renderParts(artifact.parts)}` : '(an update with no artifact)'; + console.log(` [artifactUpdate] ${body}`); return { taskId: p.value.taskId, contextId: p.value.contextId }; + } case 'message': console.log(` [message] ${renderParts(p.value.parts)}`); return {}; default: - console.log(` [${p?.$case ?? 'unknown'}] ${JSON.stringify(p)}`); + // The union is exhausted above, so what lands here is either an absent payload or a + // case from a newer protocol than these branches know: nothing to name it by. + console.log(` [unrecognised] ${JSON.stringify(p)}`); return {}; } } @@ -104,7 +114,7 @@ async function main() { // The task outlived both calls and sits on the server in full — history and artifacts included. const task = await client.getTask({ tenant: '', id: taskId, historyLength: 0 }); console.log( - `\n── Result: GetTask(${taskId.slice(0, 8)}) → ${TaskState[task.status.state]}, ` + + `\n── Result: GetTask(${taskId.slice(0, 8)}) → ${TaskState[stateOf(task.status)]}, ` + `artifacts: ${task.artifacts?.length ?? 0}, history messages: ${task.history?.length ?? 0} ──`, ); } From 617d266258adda2aab841360b93ed41276932321 Mon Sep 17 00:00:00 2001 From: FlopBut Date: Mon, 21 Sep 2026 00:14:29 +0200 Subject: [PATCH 2/6] Run types and tests on every push, publish docs to Pages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two workflows, and neither introduces a thing to maintain. `ci.yml` installs with `--immutable` and runs `yarn typecheck` and `yarn test`. Corepack is enabled before the install, because that is what resolves the Yarn version pinned in `packageManager`, and setup-node's cache is deliberately not used: it reaches for the Yarn 1 shim on the runner and asks it about a Berry cache folder that is global here anyway. `docs.yml` publishes `docs/` to GitHub Pages through `actions/jekyll-build-pages` — the same Jekyll that Pages runs by itself. So the site is the Markdown already in the repository rather than a second copy of it, and `docs/_config.yml` is four settings: a theme, and `jekyll-relative-links` so that `[permissions](permissions.md)` keeps working both in the repository and on the rendered site. It runs on pushes to `main` that touch `docs/`, with a `pages` concurrency group so a queued deployment waits rather than replacing what is live. One thing the workflow cannot do for itself: Pages has to be set to "GitHub Actions" as its source in the repository settings, or the deploy step fails with nothing to deploy to. Verified by parsing both workflows and the Jekyll config, and by running `yarn typecheck` through the script CI calls. `yarn test` is still the `npm init` stub on this branch and fails as it always has; it becomes real with the unit-test branch, which is why CI calls it now rather than later. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 33 ++++++++++++++++++++++++++ .github/workflows/docs.yml | 47 ++++++++++++++++++++++++++++++++++++++ AGENTS.md | 18 +++++++++++++++ docs/_config.yml | 13 +++++++++++ 4 files changed, 111 insertions(+) create mode 100644 .github/workflows/ci.yml create mode 100644 .github/workflows/docs.yml create mode 100644 docs/_config.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..c0a3624 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,33 @@ +# Types and tests, on every push and every pull request. +# +# There is no build step here — tsx runs TypeScript directly — so nothing checks types +# unless something asks. `yarn typecheck` is that ask, and it carries the compiler flags +# in place of the tsconfig.json this repository deliberately does not have. +name: CI + +on: + push: + branches: [main] + pull_request: + +jobs: + check: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-node@v4 + with: + node-version: '24' + + # Yarn 4, pinned by the packageManager field; Corepack resolves that version itself. + # Deliberately before the install and without setup-node's cache, which reaches for + # the Yarn 1 shim on the runner and asks it about a Berry cache folder. + - run: corepack enable + + # --immutable: a lockfile that would have to change is a failure, not a fixup. + - run: yarn install --immutable + + - run: yarn typecheck + + - run: yarn test diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml new file mode 100644 index 0000000..521b7cb --- /dev/null +++ b/.github/workflows/docs.yml @@ -0,0 +1,47 @@ +# Publishes docs/ to GitHub Pages. +# +# The site is the Markdown already in the repository, rendered by the same Jekyll that +# Pages runs by itself — no generator, no second copy of the documentation, and nothing +# to keep in step with it. Requires Pages to be set to "GitHub Actions" in the repository +# settings; the workflow cannot turn that on for you. +name: Docs + +on: + push: + branches: [main] + paths: + - 'docs/**' + - '.github/workflows/docs.yml' + workflow_dispatch: + +permissions: + contents: read + pages: write + id-token: write + +# One deployment at a time, and a queued one waits rather than replacing what is live. +concurrency: + group: pages + cancel-in-progress: false + +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/configure-pages@v5 + - uses: actions/jekyll-build-pages@v1 + with: + source: ./docs + destination: ./_site + - uses: actions/upload-pages-artifact@v3 + + deploy: + needs: build + runs-on: ubuntu-latest + environment: + name: github-pages + url: ${{ steps.deployment.outputs.page_url }} + steps: + - id: deployment + uses: actions/deploy-pages@v4 diff --git a/AGENTS.md b/AGENTS.md index 0b90ee8..1874ce4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -22,6 +22,7 @@ yarn raw # the same protocol over bare curl, no SDK yarn tap # wire-tap proxy :41242 → :41241 yarn mcp # MCP endpoint on :41243 fronting the A2A agent, for a calling agent yarn service # install and drive the whole thing as a background service +yarn typecheck # tsc --noEmit with the flags that stand in for tsconfig.json ``` All three servers listen on the same port and serve the same protocol, so `client`, `raw` and @@ -116,6 +117,23 @@ End-to-end verification is unchanged and still worth running: `yarn client` agai checking the cycle `SUBMITTED → INPUT_REQUIRED → WORKING → artifact → COMPLETED`. Full write-up: [docs/testing.md](docs/testing.md). +## Continuous integration + +`.github/workflows/ci.yml` runs `yarn typecheck` and `yarn test` on every push and every pull +request. The typecheck is there because there is no build step: `tsx` transpiles and throws the +types away, so nothing checks them unless something asks, and the flags that would live in a +`tsconfig.json` live in that script instead. It is expected to pass — the tree is at zero +diagnostics — so a new one is a regression rather than a number to compare against. + +`.github/workflows/docs.yml` publishes `docs/` to GitHub Pages through the same Jekyll that Pages +runs anyway, so the site is the Markdown in this repository rather than a second copy of it. It +needs Pages set to "GitHub Actions" in the repository settings; the workflow cannot turn that on +by itself. + +Corepack is enabled before the install, because that is what resolves the Yarn version pinned in +`packageManager`. `setup-node`'s cache is deliberately not used: it reaches for the Yarn 1 shim on +the runner and asks it about a Berry cache folder, which is global here anyway. + ## Architecture Each server is an entry point that reads the environment, builds an Agent Card and binds a port; diff --git a/docs/_config.yml b/docs/_config.yml new file mode 100644 index 0000000..e199537 --- /dev/null +++ b/docs/_config.yml @@ -0,0 +1,13 @@ +# Jekyll's share of publishing docs/ — deliberately four settings and no site to maintain. +title: Ambassy +description: A learning sandbox for the A2A protocol, and a bridge from it to a real coding agent. +theme: jekyll-theme-primer + +# The documents link to each other as files — [permissions](permissions.md) — because they +# are read in the repository as often as on the web. This rewrites those links to the +# rendered pages, so one set of links works in both places. +plugins: + - jekyll-relative-links +relative_links: + enabled: true + collections: false From 1403d935553c37e153e01d03f6eeb32fbe2fdc94 Mon Sep 17 00:00:00 2001 From: FlopBut Date: Mon, 21 Sep 2026 00:14:29 +0200 Subject: [PATCH 3/6] Say what the audit's findings have become MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The document was a snapshot of one review and has been read since as a plan, which left no way to tell an open finding from one that was answered three commits ago. Each finding now carries a **Status** line, and a summary near the top says which are closed, which are partly closed and which are untouched. The findings themselves are left exactly as they were written. The value of an audit is that it records what was true at a moment; editing a finding away once it is fixed loses the reason anyone should keep checking that it stays fixed. So `F01` still describes a context id becoming a trusted root, and the line under it says that directories now come from `mkdtemp`. Ten are closed, two are partly closed — F18, whose two MCP deadlines stopped throwing, and F21, which has the typecheck and CI but not the smoke exit codes — and eleven are open. The open ones include the group the implementation order deliberately keeps together, F04–F07: they share the adapter registry, and fixing one alone moves the bug rather than removing it. Co-Authored-By: Claude Opus 5 (1M context) --- docs/repository-audit.md | 64 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/docs/repository-audit.md b/docs/repository-audit.md index 403b652..0064619 100644 --- a/docs/repository-audit.md +++ b/docs/repository-audit.md @@ -35,6 +35,22 @@ cancellation routing; it does not establish behavior of the actual model adapter The MCP HTTP transport, actual adapter authentication, model execution, and a dependency vulnerability scan were not exercised. +## What has been done since + +The findings are being answered in the code, not edited out of this document: the record of +what was found stays as it was written, and each entry carries a **Status** line saying where +it was answered. As of `109fb67`: + +| | Findings | +|---|---| +| Closed | F01, F02, F08, F09, F10, F11, F12, F13, F14, F22 | +| Partly | F18 (the two MCP deadlines), F21 (typecheck and CI, not the smoke exit codes) | +| Open | F03–F07, F15, F16, F17, F19, F20, F23 | + +The open ones are not a leftover pile. F04–F07 are process and task ownership, which the +implementation order below deliberately groups: they share the adapter registry, and fixing +one of them alone moves the bug rather than removing it. + Priority meanings: **P0** = isolation boundary bypass; **P1** = fix before routine bridge use; **P2** = correctness or reliability work; **P3** = smaller edge case. Evidence is marked **reproduced** or **source review**. Source locations refer to the reviewed commit. @@ -43,6 +59,8 @@ Priority meanings: **P0** = isolation boundary bypass; **P1** = fix before routi ### F01 — P0: Client-controlled context IDs become trusted filesystem roots +**Status:** Closed by `3ddd7d4`. Directories come from `mkdtemp` under the sandbox parent, with an explicit context-to-directory map and a containment check on reuse; nothing is derived from the context id but the label on the name. + **Location:** [src/acp/client.ts](../src/acp/client.ts), `boundaryFor`, lines 284–301. **Evidence:** reproduced, plus live verification that the A2A SDK accepts `contextId: ".."`. @@ -64,6 +82,8 @@ command or write a file outside its disposable fixture. ### F02 — P1: The unauthenticated A2A server binds beyond localhost +**Status:** Closed by `3ddd7d4`. The two A2A servers had already gained `HOST` by the time this was acted on; the wire-tap binds it now as well, and `TARGET_HOST` names what it forwards to. + **Location:** [src/acp/agent.ts](../src/acp/agent.ts), lines 609–623; [src/agent.ts](../src/agent.ts), lines 248–260; [src/proxy.ts](../src/proxy.ts), line 58. **Evidence:** source review. @@ -83,6 +103,8 @@ Verify the actual bound address, not just the startup message. ### F03 — P1: Copying the supplied environment template breaks startup +**Status:** Open. + **Location:** [.env.example](../.env.example), lines 20 and 32; [src/acp/agent.ts](../src/acp/agent.ts), lines 51–62; [src/mcp/server.ts](../src/mcp/server.ts), lines 40–68. **Evidence:** reproduced. @@ -100,6 +122,8 @@ and a valid public URL. Test this in isolation without overwriting the operator' ### F04 — P1: Concurrent acquisition creates two adapters for one context +**Status:** Open. + **Location:** [src/acp/client.ts](../src/acp/client.ts), `acquire`, lines 305–364. **Evidence:** reproduced with two simultaneous acquisitions and a fake ACP adapter. @@ -117,6 +141,8 @@ A failed acquisition can be retried, and shutdown leaves no pending or hidden ch ### F05 — P1: Failed startup leaks adapter processes; startup has no deadline +**Status:** Open. The one piece taken early: `acquire` disposes a half-started adapter, because F08 cannot be honoured without it. + **Location:** [src/acp/client.ts](../src/acp/client.ts), lines 127–135, 250–281, 314–364. **Evidence:** failed acquisition reproduced; missing deadlines found by source review. @@ -136,6 +162,8 @@ also completes when spawn fails. ### F06 — P1: Canceling a queued task cancels the task currently running +**Status:** Open. + **Location:** [src/acp/agent.ts](../src/acp/agent.ts), lines 143–161, 199–241; [src/acp/client.ts](../src/acp/client.ts), lines 193–214. **Evidence:** reproduced with controlled turns. @@ -153,6 +181,8 @@ Also cover cancellation during acquisition and a cancel racing normal completion ### F07 — P1: Canceling an INPUT_REQUIRED task hangs +**Status:** Open. + **Location:** [src/agent.ts](../src/agent.ts), lines 114–118, 137–153; [src/acp/agent.ts](../src/acp/agent.ts), lines 143–161, 202–218, 257–263. **Evidence:** live Revisor reproduction and ACP executor bookkeeping fixture. @@ -175,6 +205,8 @@ executors and cancellation during startup with empty input. ### F08 — P1: Missing supervised modes silently retain an unsafe default +**Status:** Closed by `3ddd7d4`. A supervised mode that cannot be established fails the session and takes the adapter with it, and `switch_mode` is decided on the option the adapter offered — which is where both adapters put the destination, `rawInput` holding only the plan. + **Location:** [src/acp/client.ts](../src/acp/client.ts), `superviseMode`, lines 367–385. **Evidence:** reproduced with an adapter offering only `auto`. @@ -192,6 +224,8 @@ Known supported modes continue to work. ### F09 — P1: MCP cannot resume a task that needs more input +**Status:** Closed by `109fb67`. `taskId` runs through the tool schema, the pool and the outgoing message; the result names the task to reply into, and omitting it still opens new work in the same context. + **Location:** [src/mcp/a2a.ts](../src/mcp/a2a.ts), lines 71–79, 119–143; [src/mcp/tools.ts](../src/mcp/tools.ts), lines 54–60, 99–105. **Evidence:** live reproduction. @@ -210,6 +244,8 @@ task, and eventually releases its open-task reference. See the specification's ### F10 — P1: The advertised recovery path loses the task handle on errors +**Status:** Closed by `109fb67`. Identity travels with the first progress notification, with a typed `TurnError` and with the silence timeout; a caller that hung up is recorded as `mcp.ask.abandoned`. + **Location:** [src/mcp/a2a.ts](../src/mcp/a2a.ts), `AskUpdate` and `ask`; [src/mcp/tools.ts](../src/mcp/tools.ts), lines 138–145, 155–158. **Evidence:** source review. @@ -229,6 +265,8 @@ recovery path. A silence timeout includes the known agent, task ID, and context ### F11 — P1: Discovery bypasses the MCP abort gate +**Status:** Closed by `109fb67`. `MCP_DISCOVERY_TIMEOUT_MS` bounds the shared handshake and `MCP_REQUEST_TIMEOUT_MS` the three request/response tools; a caller's abort ends only its own wait. + **Location:** [src/mcp/a2a.ts](../src/mcp/a2a.ts), lines 103–109, 124–125, 184–205; [src/mcp/tools.ts](../src/mcp/tools.ts), lines 115–145. **Evidence:** reproduced with stalled HTTP discovery. @@ -246,6 +284,8 @@ tool call indefinitely. Verify the same behavior for each of the four tools and ### F12 — P2: MCP corrupts replacement artifacts and ignores snapshot content +**Status:** Closed by `109fb67`. Artifacts aggregate by `artifactId` honouring `append`, seeded from the task snapshot, with every data part kept. + **Location:** [src/mcp/a2a.ts](../src/mcp/a2a.ts), lines 150–170. **Evidence:** reproduced with an injected SDK stream. @@ -264,6 +304,8 @@ multiple data parts, and a direct Message response. ### F13 — P2: An incomplete stream is rendered as a successful tool result +**Status:** Closed by `109fb67`. A turn reports an outcome as well as a state, and `truncated` — a stream that stopped before any ending — is an error carrying the recovery identity. + **Location:** [src/mcp/a2a.ts](../src/mcp/a2a.ts), lines 146–181; [src/mcp/tools.ts](../src/mcp/tools.ts), lines 66–69. **Evidence:** reproduced with an injected stream. @@ -280,6 +322,8 @@ remain supported, and interrupted states tell the caller what action is needed. ### F14 — P2: Final failure reasons and recovery data disappear in MCP results +**Status:** Closed by `109fb67`. The last status message is retained and reported by `a2a_ask` and `a2a_task`, and `a2a_cancel` renders like a read rather than dropping what the task produced. + **Location:** [src/mcp/a2a.ts](../src/mcp/a2a.ts), lines 155–162; [src/mcp/tools.ts](../src/mcp/tools.ts), lines 46–83. **Evidence:** source review. @@ -297,6 +341,8 @@ its text, structured result, and final status explanation. ### F15 — P2: Agent Cards advertise part labels instead of media types +**Status:** Open. + **Location:** [src/agent.ts](../src/agent.ts), lines 230–240; [src/acp/agent.ts](../src/acp/agent.ts), lines 578–588; [docs/a2a.md](a2a.md), discovery example. **Evidence:** live card and specification check. @@ -312,6 +358,8 @@ the example. See the specification's ### F16 — P2: MCP and ACP rotate the same logs independently +**Status:** Open. + **Location:** [src/acp/agent.ts](../src/acp/agent.ts), lines 58–62; [src/mcp/server.ts](../src/mcp/server.ts), lines 47–68; [src/acp/log.ts](../src/acp/log.ts), lines 41–85. **Evidence:** source review. @@ -329,6 +377,8 @@ accounted for within each service's documented retention window. ### F17 — P2: Logging can still throw into bridge control flow +**Status:** Open. + **Location:** [src/acp/log.ts](../src/acp/log.ts), lines 50–60 and 89–92. **Evidence:** reproduced with an invalid log destination and a non-JSON field. @@ -346,6 +396,8 @@ without repeated exceptions or accidental task failure. ### F18 — P2: Environment values are cast instead of validated +**Status:** Partly narrowed by `109fb67`: the two MCP deadlines fall back to their defaults rather than throwing out of `AbortSignal.timeout`, and `ACP_CWD` must now be a directory rather than merely exist. Everything else in this finding stands. + **Location:** [src/acp/agent.ts](../src/acp/agent.ts), lines 44–60; [src/acp/client.ts](../src/acp/client.ts), lines 286–291; [src/mcp/server.ts](../src/mcp/server.ts), lines 40–59, 73–78. **Evidence:** source review. @@ -366,6 +418,8 @@ Explicitly test `null`, `[]`, inherited property names, blank values, and non-lo ### F19 — P2: Every ACP -32000 error is mislabeled as authentication failure +**Status:** Open. + **Location:** [src/acp/agent.ts](../src/acp/agent.ts), `describeError`, lines 118–124; [docs/troubleshooting.md](troubleshooting.md), authentication section. **Evidence:** source review. @@ -381,6 +435,8 @@ failure reports its actual cause. ### F20 — P2: The proxy does not manage stream backpressure or disconnects +**Status:** Open. + **Location:** [src/proxy.ts](../src/proxy.ts), lines 22–55. **Evidence:** source review. The request body is buffered without a limit. Response chunks are written regardless of @@ -398,6 +454,8 @@ does not cause unbounded buffering, and an upstream reset settles the downstream ### F21 — P2: Verification does not detect the existing TypeScript and failure-path defects +**Status:** Partly closed. `yarn typecheck` carries the flags this document used, CI runs it on every push and pull request, and the source tree is at zero diagnostics. Still open: the smoke client returns 0 when its first turn does not reach `INPUT_REQUIRED`, and offline regression coverage of the lifecycle defects. + **Location:** [package.json](../package.json), scripts; [src/acp/client.ts](../src/acp/client.ts), lines 106–124, 255–269; [src/client.ts](../src/client.ts), lines 84–108. **Evidence:** compiler run and source review. @@ -421,6 +479,8 @@ fail before their corresponding fixes. Do not make paid model calls part of rout ### F22 — P3: Valid filenames starting with two dots are refused +**Status:** Closed by `3ddd7d4`. `insideRoot` refuses `..` and anything beginning `../`, and nothing else. + **Location:** [src/acp/permissions.ts](../src/acp/permissions.ts), lines 86–88. **Evidence:** reproduced: `insideRoot(process.cwd(), "..notes")` returned false. @@ -434,6 +494,8 @@ and symlink escapes fail. ### F23 — P3: LOG_MAX_FILES=1 retains an extra file +**Status:** Open. + **Location:** [src/acp/log.ts](../src/acp/log.ts), `rotate`, lines 78–85. **Evidence:** reproduced with `maxFiles: 1` and two records forcing rotation. @@ -489,6 +551,8 @@ additional concrete boundary failures despite those limitations. ## Suggested implementation order +Steps 1 and 3 have landed, minus F03; step 5's typecheck is in CI and the tree is clean. + 1. **Restore safe defaults:** F01–F03 and F08. Add boundary and template regression fixtures. 2. **Fix process/task ownership together:** F04–F07, then turn bookkeeping and shutdown. 3. **Make MCP continuation and recovery real:** F09–F14, with bounded upstream operations. From e9e11b74b1c756e3f327b3cb51566fd45e9e96d7 Mon Sep 17 00:00:00 2001 From: FlopBut Date: Mon, 21 Sep 2026 00:16:36 +0200 Subject: [PATCH 4/6] Link the two protocols that had none MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A2A was the only one with a specification link anywhere: the README, the docs index, the A2A page and the audit all pointed at it, while ACP and MCP were named repeatedly and never sourced. That is backwards for the two that are harder to look up — A2A is searchable by name, and "ACP" is not. Each of the three now sits next to the version this repository actually speaks, read off the handshakes rather than the package names, because the two disagree in a way that misleads: `@agentclientprotocol/sdk` is 1.4.0 while the ACP protocol version it negotiates is `1`, and `@modelcontextprotocol/server` is 2.0.0 while the MCP version our `initialize` answers with is `2025-06-18`. The bridge pages now open the way `docs/a2a.md` already did, with the specification and the SDK on the first line. All five URLs checked with curl: 200. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 16 +++++++++++++--- README.md | 6 +++++- docs/README.md | 16 +++++++++------- docs/acp-bridge.md | 4 ++++ docs/mcp-bridge.md | 4 ++++ 5 files changed, 35 insertions(+), 11 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 1874ce4..addcb29 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -448,7 +448,17 @@ working guide, not the explanation. ## Source of truth -The specification outranks both the SDK and this file: . -The SDK implements v1.0.0; when behavior disagrees, check the spec rather than guessing from the +Three protocols meet here, and each has a specification that outranks both its SDK and this file. +The versions are the ones this repository actually speaks — read off the handshakes, not off the +package names: + +| Protocol | Specification | What we speak | +|---|---|---| +| **A2A** | | v1.0.0, via `@a2a-js/sdk` 1.2.0 | +| **ACP** | | `PROTOCOL_VERSION` 1, via `@agentclientprotocol/sdk` 1.4.0 | +| **MCP** | | `2025-06-18`, the version our `initialize` answers with, via `@modelcontextprotocol/server` 2.0.0 | + +For A2A in particular: when behavior disagrees, check the spec rather than guessing from the bundle — `dist` also ships the v0.3 compatibility layer, and its constants are easy to mistake for -the current ones. +the current ones. The same caution applies to ACP, where the SDK's own version (1.4.0) and the +protocol's (1) are different numbers that both appear in the handshake. diff --git a/README.md b/README.md index e3c6b43..22b49b1 100644 --- a/README.md +++ b/README.md @@ -187,7 +187,11 @@ The official samples worth reading first — the SDK's own README points at them `authentication` (Bearer/JWT via Passport), `verify-signing` (signed cards, JWS + JWKS), and `extensions` (protocol extensions). -Specification: https://a2a-protocol.org/v1.0.0/specification/ +Specifications, one per protocol this repository speaks: + +- A2A v1.0.0 — +- ACP — +- MCP `2025-06-18` — ## License diff --git a/docs/README.md b/docs/README.md index 305c42d..ec9d918 100644 --- a/docs/README.md +++ b/docs/README.md @@ -5,13 +5,15 @@ A2A interface, but answered by a real coding agent over **ACP** instead of by a Three protocols meet here, and the first two are easy to confuse because both are JSON-RPC: -- **A2A** (Agent2Agent) is horizontal — one agent delegates a *task* to another over HTTP. The - task is addressable, has a lifecycle, and survives the call that created it. -- **ACP** (Agent Client Protocol) is what an editor speaks to a coding agent over stdin/stdout. - There the roles are reversed: the agent is a child process, and we are the client. - -- **MCP** (Model Context Protocol) points the other way: it is how a calling agent reaches tools, - and it is what publishes this A2A agent to Claude Code as something callable. +- **[A2A](https://a2a-protocol.org/v1.0.0/specification/)** (Agent2Agent) is horizontal — one + agent delegates a *task* to another over HTTP. The task is addressable, has a lifecycle, and + survives the call that created it. +- **[ACP](https://agentclientprotocol.com/protocol/overview)** (Agent Client Protocol) is what an + editor speaks to a coding agent over stdin/stdout. There the roles are reversed: the agent is a + child process, and we are the client. +- **[MCP](https://modelcontextprotocol.io/specification/2025-06-18)** (Model Context Protocol) + points the other way: it is how a calling agent reaches tools, and it is what publishes this A2A + agent to Claude Code as something callable. The bridge sits between them. Everything in these pages was checked against running code, not inferred from specifications — where behaviour and documentation disagreed, the observed diff --git a/docs/acp-bridge.md b/docs/acp-bridge.md index 5809503..bc52dbf 100644 --- a/docs/acp-bridge.md +++ b/docs/acp-bridge.md @@ -1,5 +1,9 @@ # The ACP bridge +Specification: . The SDK is +`@agentclientprotocol/sdk` 1.4.0, and the protocol version it negotiates is `1` — two different +numbers that both travel in the same handshake, so it is worth knowing which is which. + `src/acp/agent.ts` is the A2A server from `src/agent.ts` with the placeholder executor replaced by one that forwards the task to a real coding agent — `AcpExecutor`, in `src/acp/executor.ts`. The backend is chosen by the launch command: diff --git a/docs/mcp-bridge.md b/docs/mcp-bridge.md index f807d34..b3d99cc 100644 --- a/docs/mcp-bridge.md +++ b/docs/mcp-bridge.md @@ -1,5 +1,9 @@ # The MCP bridge +Specification: — the version this +bridge answers `initialize` with. The SDK is `@modelcontextprotocol/server` 2.0.0, over +Streamable HTTP. + A third protocol joins the two others, and it points the other way. A2A and ACP are both about reaching *down* to an agent that does work. MCP is how a calling agent — Claude Code, here — reaches *out* to tools. This bridge publishes the A2A agent as four such tools, so calling it From 336f43eafe0f0f843552f4722e2d2724b3b82043 Mon Sep 17 00:00:00 2001 From: FlopBut Date: Mon, 21 Sep 2026 00:25:47 +0200 Subject: [PATCH 5/6] Keep a changelog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sixteen commits in and the only way to learn what changed between two points was to read the log, which records what each commit did and not what any of it added up to. Two squash merges in particular hid a lot: `3ddd7d4` and `109fb67` are one line each in `git log` and ten audit findings between them. `CHANGELOG.md` at the root, Keep a Changelog format, with the audit's `F01`/`F09` numbering carried through so an entry can be traced to the finding that provoked it and the evidence recorded there. One section, `0.1.0`, because nothing has been released yet and everything here is what that version will contain. The isolation work sits under `Security` rather than `Fixed`, because a sandbox that could be steered onto the repository root is not a bug in the ordinary sense. `AGENTS.md` says who updates it and when — a change to behaviour, configuration or the protocol surface, in the commit that makes it, not afterwards from memory; refactors and documentation edits do not qualify, or the file becomes the log again. And once 0.1.0 has gone out, new entries open an `## [Unreleased]` above it rather than editing what shipped. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 8 +++++ CHANGELOG.md | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++ README.md | 4 +++ 3 files changed, 109 insertions(+) create mode 100644 CHANGELOG.md diff --git a/AGENTS.md b/AGENTS.md index addcb29..cf9b948 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -440,6 +440,14 @@ On the ACP side, verified the same way: - `ClientSideConnection` is deprecated in SDK 1.4.0; the current API is `acp.client({name}).onRequest(...).connect(stream)`. +## Changelog + +`CHANGELOG.md` at the root, Keep a Changelog format. A change to behaviour, configuration or the +protocol surface gets an entry in the same commit that makes it — one sentence on what changed, +and a second on why when the reason is not obvious from the first. Refactors, comments and +documentation edits do not. While a version is unreleased the entry goes under its heading; +once it has gone out, open an `## [Unreleased]` above it rather than editing what shipped. + ## Longer prose `docs/` carries the human-facing documentation: architecture, both protocols, the permission diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..3f90ff5 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,97 @@ +# Changelog + +Notable changes, newest first. The format follows +[Keep a Changelog](https://keepachangelog.com/en/1.1.0/) and the numbers follow +[Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +Entries say what changed and, where the reason is not obvious from the change, why — this +repository exists to be read, and a list of verbs is no use to whoever comes next. Findings +referenced as `F01`, `F09` and so on are from [docs/repository-audit.md](docs/repository-audit.md), +which records them in full along with the evidence. + +## [0.1.0] — 2026-09-21 + +The first version worth a number, and an honest one: tasks live in an `InMemoryTaskStore`, the +A2A side runs `UserBuilder.noAuthentication`, and the permission classifier is a placeholder for +an external channel — a human, a policy, or a supervising agent. + +### Added + +- **An A2A v1.0 server and client** (`src/agent.ts`, `src/client.ts`) — Agent Card discovery, + streaming, `INPUT_REQUIRED` and the artifact cycle, with a placeholder executor (the "Revisor") + that computes text statistics and nothing else. +- **The same protocol over bare curl** (`src/raw.sh`) and **a wire-tap** (`src/proxy.ts`) that + prints raw JSON-RPC and SSE frames, because the SDK surfaces parsed objects and what you need to + see is the wire. +- **The ACP bridge** (`src/acp/`) — the same A2A server with its executor replaced by a real + coding agent addressed over stdin/stdout: one adapter process per `contextId`, a permission + classifier that answers instead of the calling agent, and two rotating JSON Lines logs. +- **The MCP bridge** (`src/mcp/`) — the A2A agent published to a calling agent as four tools over + Streamable HTTP, bearer-guarded, with progress ticks derived from upstream liveness. + `a2a_ask` takes a `taskId` as well as a `contextId`, which is the only way to answer an + `INPUT_REQUIRED` question rather than open a second task beside it, and every call reports the + task and context it reached so an interrupted one can still be recovered (`F09`, `F10`). +- **Deadlines on everything that does not stream.** The silence gate covers `a2a_ask`; + `MCP_DISCOVERY_TIMEOUT_MS` (20 s) bounds the shared agent handshake and + `MCP_REQUEST_TIMEOUT_MS` (30 s) bounds `a2a_task`, `a2a_cancel` and `a2a_card`. Discovery is + bounded separately because it is shared: a caller's abort ends only its own wait, while the + deadline ends the handshake for everyone (`F11`). +- **`bin/ambassyctl`** — both processes installed and driven as background services, LaunchAgents + on macOS and `systemd --user` units on Linux. +- **`docs/`** — architecture, both protocols, the permission model, configuration, logging, + testing, troubleshooting, and the repository audit, published to GitHub Pages by + `.github/workflows/docs.yml` through the same Jekyll that Pages runs anyway. +- **A test suite** — `yarn test` runs 288 vitest cases over the paths that are hard to provoke by + hand: a cancel that arrives before the session exists, a log that rotates mid-record, a stream + that stops before its task reaches a state. Nothing is mocked at the module level; every + substitution goes through an argument, so the seams stay visible in the production code. + [docs/testing.md](docs/testing.md) says what they reach and what they leave alone. +- **`yarn typecheck` and CI.** There is no build step, so nothing checked types unless asked; the + script carries the compiler flags in place of the `tsconfig.json` this repository deliberately + lacks, and `.github/workflows/ci.yml` runs it with the tests on every push and pull request. + +### Fixed + +Found by the repository audit and closed before this version went out. + +- **Artifacts aggregate instead of concatenating (`F12`).** Two replacements of one artifact, + `old` then `new`, produced `oldnew`, and artifacts that were never the same artifact merged. + They are now keyed by `artifactId`, honour `append`, are seeded from the task snapshot, and keep + every data part. +- **A stream that loses its ending is an error (`F13`).** One that died mid-turn came back as a + tidy result saying the agent "finished in `TASK_STATE_WORKING`". A turn now reports an outcome — + `task`, `message`, `interrupted` or `truncated` — as well as a state. +- **The agent's last word survives (`F14`).** A refusal from the ACP bridge travels in a status + message and nowhere else, and it was dropped for everything but `INPUT_REQUIRED`. It is kept now + and reported by `a2a_ask` and `a2a_task`; `a2a_cancel` renders like a read rather than discarding + what the task had already produced. +- **Leaving plan mode is not a refusal.** The `switch_mode` rule denied Claude's `ExitPlanMode`, + which the adapter turns into an interrupt and reports as a cancellation — so a task that planned + before acting would have died as it was ready to start. The destination is in the option ids, + never in `rawInput`, and is read from an explicit table of both adapters' effects. +- **A deadline that is not a number falls back.** `Number(process.env.…)` of a typo is `NaN`, and + `AbortSignal.timeout(NaN)` throws rather than ignoring it, which took out every call the + deadline was meant to protect. +- **`..notes` is a filename (`F22`).** The containment check refused any relative path beginning + with two dots, which is a climb out only when the next character is a separator. +- **17 type errors in running code.** Found by the new type check: stream payload fields read as + though a protobuf message field were ever required, an exhausted union asked for its `$case`, + adapters declared with three pipes and spawned with two. +- **`ACP_CWD` must be a directory**, not merely a path that exists. + +### Security + +- **A conversation's sandbox is not named after the caller (`F01`).** The A2A `contextId` is text + the caller chooses, and joining it onto `.acp-sandboxes/` made `contextId: ".."` resolve to the + repository root — returned as `owned: true`, the flag that lets the classifier pass `execute` + and `fetch`. Directories come from `mkdtemp`, which fails unless the directory is new, with an + explicit context-to-directory map and a containment check on reuse. +- **A session that cannot be supervised is not used (`F08`).** When the supervised mode was + unavailable, the bridge logged the miss and carried on in whatever mode the adapter chose — for + both backends, one where it approves its own tool calls. It now fails the session and disposes + the half-started adapter. `switch_mode` is its own classifier case, decided on the option the + adapter offered rather than on who owns the directory. +- **The wire-tap binds loopback (`F02`).** It called `listen(PORT)` with no host, which opened a + wider door than the unauthenticated agent it fronts. It reads `HOST` now, like the two servers. + +[0.1.0]: https://github.com/Flopsstuff/ambassy/commits/main diff --git a/README.md b/README.md index 22b49b1..5cbbc8d 100644 --- a/README.md +++ b/README.md @@ -193,6 +193,10 @@ Specifications, one per protocol this repository speaks: - ACP — - MCP `2025-06-18` — +## Changes + +What changed and why, release by release: [CHANGELOG.md](CHANGELOG.md). + ## License Apache License 2.0 — see [LICENSE](LICENSE). From ebea1facb3342f0bad48e825aa1ca499c3fa1dfa Mon Sep 17 00:00:00 2001 From: FlopBut Date: Mon, 21 Sep 2026 00:36:00 +0200 Subject: [PATCH 6/6] Move the workflows onto current actions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Written a day ago against versions that were already one to three majors behind, because the ones in every example online are the ones from whenever that example was written. Checked against each action's releases rather than from memory: | Action | Was | Now | Latest release | |---|---|---|---| | `actions/checkout` | v4 | v7 | v7.0.1, 2026-07-20 | | `actions/setup-node` | v4 | v7 | v7.0.0, 2026-07-14 | | `actions/configure-pages` | v5 | v6 | v6.0.0, 2026-03-25 | | `actions/upload-pages-artifact` | v3 | v5 | v5.0.0, 2026-04-10 | | `actions/deploy-pages` | v4 | v5 | v5.0.1, 2026-09-01 | | `actions/jekyll-build-pages` | v1 | v1 | v1.0.13 — no major since 2024 | Nothing we use changed shape across those majors: the two big ones are ESM migrations and a move to the Node 24 runtime, and `configure-pages`' breaking change was to `static_site_generator: next`, an input this does not pass. The three Pages actions are bumped together and now say so in a comment — they agree on an artifact format, so raising one alone is the usual way a Pages job starts failing. `node-version` stays at `'24'`, which is the active LTS until 20 October 2026; the comment now says that, and why it is a literal rather than `lts/*` — a version that changes itself on a date nobody remembers is not what CI is for. Both workflows still parse. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 6 ++++-- .github/workflows/docs.yml | 10 ++++++---- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c0a3624..2c5a14b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,10 +14,12 @@ jobs: check: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 - - uses: actions/setup-node@v4 + - uses: actions/setup-node@v7 with: + # The active LTS. Node 26 takes that title at the end of October 2026; this is + # pinned rather than `lts/*` so the jump is a commit somebody made on purpose. node-version: '24' # Yarn 4, pinned by the packageManager field; Corepack resolves that version itself. diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 521b7cb..12bff86 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -28,13 +28,15 @@ jobs: build: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - - uses: actions/configure-pages@v5 + - uses: actions/checkout@v7 + # These three move together: the artifact format they agree on is what ties a build + # to the deployment that consumes it, so bumping one alone is how Pages jobs break. + - uses: actions/configure-pages@v6 - uses: actions/jekyll-build-pages@v1 with: source: ./docs destination: ./_site - - uses: actions/upload-pages-artifact@v3 + - uses: actions/upload-pages-artifact@v5 deploy: needs: build @@ -44,4 +46,4 @@ jobs: url: ${{ steps.deployment.outputs.page_url }} steps: - id: deployment - uses: actions/deploy-pages@v4 + uses: actions/deploy-pages@v5