[addyCooks]

Description

ACP reserves method names starting with _ for custom methods and says extensions should advertise themselves under _meta in capability objects. Nanocoder's four custom requests used bare names (renameSession, timeline/list, timeline/revert, retryTurn) that could clash with future protocol methods, and nothing was advertised, so a client had to guess from the version.

• The methods now answer to _nanocoder/renameSession, _nanocoder/timeline/list, _nanocoder/timeline/revert and _nanocoder/retryTurn, with the same results as before.
• The bare names still work as deprecated aliases and log a one-time notice per name. They will be removed in a later release announced in the changelog.
• initialize returns agentCapabilities._meta.nanocoder.extMethods and extNotifications with the exact names, so clients can detect support and use the advertised names as they are.
• An unrecognised custom request fails with the standard "Method not found" (-32601) instead of a plain error.
• The VS Code extension uses the advertised names and falls back to the bare ones for an older agent.
• _nanocoder/sessionTitleChanged and the existing nanocoder/... _meta keys are unchanged.

Closes #1893

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update

Changeset

• Added a changeset (pnpm changeset) describing this change for the changelog

Testing

Automated Tests

• New features include passing tests in .spec.ts/tsx files
• All existing tests pass (pnpm test:all completes successfully)
• Tests cover both success and error scenarios

Manual Testing

• Tested with Ollama
• Tested with OpenRouter
• Tested with OpenAI-compatible API
• Tested MCP integration (if applicable)

Checklist

• If this was for an open issue, I was assigned to it
• Code follows project style guidelines
• Self-review completed
• Documentation updated (if needed)
• No breaking changes (or clearly documented)
• Appropriate logging added using structured logging (see CONTRIBUTING.md)

[github-actions]

nc-review: comments — 2 important, 4 nits

@addyCooks — a few things worth a look, none blocking.

The PR prefixes Nanocoder's four ACP extension request methods with _nanocoder/, keeps the bare names as deprecated aliases that log a one-time notice, advertises the exact method and notification names under agentCapabilities._meta.nanocoder, returns the standard -32601 for unrecognised custom requests, and teaches the VS Code extension to use the advertised names with a fallback to the bare ones. The design matches issue #1893 and every acceptance criterion is covered by tests; the two things worth fixing before merge are a test that patches the process-global logger without test.serial (it will flake against a sibling test in the same file) and a duplicated notification-name constant that can drift from the name the agent actually sends. I could not verify from this checkout that RequestError.methodNotFound is exported by @agentclientprotocol/sdk 1.4.0 (node_modules is not installed here); the CI tsc check would catch it if it is not.

🟠 important · tests · source/acp/acp-agent.spec.ts

The new deprecation-notice test replaces warn on the process-global logger singleton:

const logger = getLogger();
const original = logger.warn;
logger.warn = ((message: string) => { warnings.push(message); }) as typeof logger.warn;

It is declared as a plain test(...), but AVA runs tests within a file concurrently by default (the serial: true in package.json only serialises test files). The sibling test added in this same PR — "answers on the prefixed name and on the bare alias" — calls the bare names timeline/list, retryTurn and renameSession, and each of those now emits a logger.warn deprecation notice through that same patched global. The two tests interleave at their await points, so the bare-name warnings can land inside the patch window and break t.deepEqual(warnings, []) and t.is(warnings.length, 2) (and warnings[0] may no longer be the expected message). The existing repo precedent for exactly this pattern is source/utils/message-queue.spec.tsx, which marks its logger-patching test test.serial.

Fix: mark this test test.serial (and either serialise the sibling or make it use only prefixed names plus a dedicated bare-name call), or capture the notices per-agent rather than by patching the global logger.

🟠 important · design · source/acp/acp-capabilities.ts

ACP_EXT_NOTIFICATIONS is a second, independent copy of the notification name:

const ACP_EXT_NOTIFICATIONS = ['_nanocoder/sessionTitleChanged'];

while the agent actually sends it from source/acp/acp-agent.ts:490 as a bare literal (this.conn.notify('_nanocoder/sessionTitleChanged', …)). If either side is edited without the other, agentCapabilities._meta.nanocoder.extNotifications will advertise a notification the agent never emits — a client that trusts the advertisement would wait for an event that never arrives. Issue #1893 explicitly asked that the advertised list be built from the same source as the dispatch "so the two never drift", and this is the one place that requirement is not met.

Fix: export the notification name as a constant (e.g. ACP_EXT_NOTIFICATION_SESSION_TITLE) and use it at the notify() call site as well as in the advertised list.

⚪ nit · tests · plugins/vscode/src/acp-client.spec.ts

The PR adds a makeExtClient helper that is a near-duplicate of the makeHandshakenClient helper already present in the same file (same constructor, same initialize stub, same _sessionId restore); the only differences are the extMethod stub and the returned calls array. Two near-identical fixtures in one file is drift waiting to happen.

Fix: extend makeHandshakenClient with an optional extMethod stub and reuse it, rather than adding a second helper.

⚪ nit · tests · plugins/vscode/src/acp-client.spec.ts

extMethodName(advertised, legacy) resolves each method independently, so an agent that advertises _nanocoder/renameSession but not _nanocoder/retryTurn should get the prefixed name for one call and the bare name for the other. The fallback test only covers agents that advertise nothing ({}, {_meta: {nanocoder: {}}}, {_meta: {nanocoder: {extMethods: 'x'}}}), so the per-method resolution is untested — a regression that made extMethodName all-or-nothing would still pass.

Fix: add a case where extMethods lists only _nanocoder/renameSession and assert the two calls use ['_nanocoder/renameSession', 'retryTurn'].

⚪ nit · design · source/acp/acp-capabilities.ts

getAgentCapabilities() copies the method list (ACP_EXT_METHODS.map(...)) but returns the notification list by reference (extNotifications: ACP_EXT_NOTIFICATIONS). A caller that mutates the returned array would corrupt the module-level constant for every later getAgentCapabilities() call.

Fix: return a copy, e.g. extNotifications: [...ACP_EXT_NOTIFICATIONS], or make the constant readonly and copy both.

⚪ nit · design · plugins/vscode/src/acp-client.ts

The extension hard-codes '_nanocoder/renameSession' and '_nanocoder/retryTurn' as literals while the agent side derives them from ACP_EXT_PREFIX + ACP_EXT_METHODS in source/acp/acp-capabilities.ts. The plugin cannot import that module today, but plugins/vscode/tsconfig.json already maps @/* to ../../source/* and the plugin already pulls files in from ../../source/, so a shared constant is available.

This is not a hard break today (a rename would silently degrade the extension to the bare names, which still work as aliases), but it is exactly the "two places that must be kept in sync" pattern this PR set out to remove on the agent side.

Fix: import ACP_EXT_PREFIX and the method names from @/acp/acp-capabilities and build the advertised names from them.

---

_{🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional}

_{Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with /re-review.}