-
-
Notifications
You must be signed in to change notification settings - Fork 144
feat(cli-acp): enable vendor resume for configured ACP backends advertising session/load #352
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
|
|
||
| import { AcpBackend } from '../AcpBackend'; | ||
| import { writeAcpTestAgentScript } from '../testkit/subprocessHarness'; | ||
| import { withTempDir } from '@/testkit/fs/tempDir'; | ||
|
|
||
| function writeCapabilityAgentScript(params: { dir: string; declareLoadSession: boolean }): string { | ||
| return writeAcpTestAgentScript({ | ||
| dir: params.dir, | ||
| fileName: params.declareLoadSession ? 'fake-acp-load-capable.mjs' : 'fake-acp-load-incapable.mjs', | ||
| source: ` | ||
| import readline from 'node:readline'; | ||
| const rl = readline.createInterface({ input: process.stdin }); | ||
| const send = (value) => process.stdout.write(JSON.stringify(value) + '\\n'); | ||
| rl.on('line', (line) => { | ||
| const request = JSON.parse(line); | ||
| if (request.method === 'initialize') { | ||
| send({ | ||
| jsonrpc: '2.0', | ||
| id: request.id, | ||
| result: { | ||
| protocolVersion: 1, | ||
| authMethods: [], | ||
| agentCapabilities: { loadSession: ${params.declareLoadSession} }, | ||
| }, | ||
| }); | ||
| return; | ||
| } | ||
| if (request.method === 'session/new') { | ||
| send({ jsonrpc: '2.0', id: request.id, result: { sessionId: 'fresh-session' } }); | ||
| return; | ||
| } | ||
| send({ jsonrpc: '2.0', id: request.id, result: {} }); | ||
| }); | ||
| `, | ||
| }); | ||
| } | ||
|
|
||
| describe('AcpBackend session load capability', () => { | ||
| it('reports session load support when the agent advertises loadSession', async () => { | ||
| await withTempDir('happier-acp-load-capable-', async (dir) => { | ||
| const scriptPath = writeCapabilityAgentScript({ dir, declareLoadSession: true }); | ||
| const backend = new AcpBackend({ | ||
| agentName: 'test', | ||
| cwd: dir, | ||
| command: process.execPath, | ||
| args: [scriptPath], | ||
| }); | ||
| try { | ||
| await backend.startSession(); | ||
| expect(backend.supportsSessionLoad()).toBe(true); | ||
| } finally { | ||
| await backend.dispose(); | ||
| } | ||
| }); | ||
| }, 20_000); | ||
|
|
||
| it('reports no session load support when the agent omits the capability', async () => { | ||
| await withTempDir('happier-acp-load-incapable-', async (dir) => { | ||
| const scriptPath = writeCapabilityAgentScript({ dir, declareLoadSession: false }); | ||
| const backend = new AcpBackend({ | ||
| agentName: 'test', | ||
| cwd: dir, | ||
| command: process.execPath, | ||
| args: [scriptPath], | ||
| }); | ||
| try { | ||
| await backend.startSession(); | ||
| expect(backend.supportsSessionLoad()).toBe(false); | ||
| } finally { | ||
| await backend.dispose(); | ||
| } | ||
| }); | ||
| }, 20_000); | ||
|
|
||
| it('reports no session load support before initialize completes', () => { | ||
| const backend = new AcpBackend({ | ||
| agentName: 'test', | ||
| cwd: '/tmp', | ||
| command: process.execPath, | ||
| args: ['-e', ''], | ||
| }); | ||
| expect(backend.supportsSessionLoad()).toBe(false); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
|
|
||
| import { createConfiguredAcpSessionIdentityPublication } from './createConfiguredAcpSessionIdentityPublication'; | ||
|
|
||
| function createFakeSession(initial: Record<string, unknown> = {}) { | ||
| let metadata: Record<string, unknown> = { ...initial }; | ||
| const updateCalls: Array<Record<string, unknown>> = []; | ||
| return { | ||
| session: { | ||
| sessionId: 'happier-session-1', | ||
| getMetadataSnapshot: () => metadata as never, | ||
| updateMetadata: (updater: (value: Record<string, unknown>) => Record<string, unknown>) => { | ||
| metadata = updater(metadata); | ||
| updateCalls.push(metadata); | ||
| }, | ||
| }, | ||
| getMetadata: () => metadata, | ||
| updateCalls, | ||
| }; | ||
| } | ||
|
|
||
| describe('createConfiguredAcpSessionIdentityPublication', () => { | ||
| it('publishes the bound ACP session id to customAcpSessionId metadata when the adapter supports session load', async () => { | ||
| const fake = createFakeSession(); | ||
| const publication = createConfiguredAcpSessionIdentityPublication({ | ||
| session: fake.session as never, | ||
| isSessionLoadSupported: () => true, | ||
| }); | ||
|
|
||
| expect(publication.kind).toBe('persist-bound'); | ||
| if (publication.kind !== 'persist-bound') throw new Error('expected persist-bound'); | ||
| await publication.persistBound({ generation: 0, operation: 'create', vendorSessionId: 'acp-session-1' }); | ||
|
|
||
| expect(fake.getMetadata().customAcpSessionId).toBe('acp-session-1'); | ||
| }); | ||
|
|
||
| it('publishes nothing when the adapter does not support session load', async () => { | ||
| const fake = createFakeSession(); | ||
| const publication = createConfiguredAcpSessionIdentityPublication({ | ||
| session: fake.session as never, | ||
| isSessionLoadSupported: () => false, | ||
| }); | ||
|
|
||
| if (publication.kind !== 'persist-bound') throw new Error('expected persist-bound'); | ||
| await publication.persistBound({ generation: 0, operation: 'create', vendorSessionId: 'acp-session-1' }); | ||
|
|
||
| expect(fake.updateCalls).toHaveLength(0); | ||
| expect(fake.getMetadata().customAcpSessionId).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('re-evaluates adapter support on every publication so capability is read after initialize', async () => { | ||
| const fake = createFakeSession(); | ||
| let supported = false; | ||
| const publication = createConfiguredAcpSessionIdentityPublication({ | ||
| session: fake.session as never, | ||
| isSessionLoadSupported: () => supported, | ||
| }); | ||
|
|
||
| if (publication.kind !== 'persist-bound') throw new Error('expected persist-bound'); | ||
| await publication.persistBound({ generation: 0, operation: 'create', vendorSessionId: 'acp-session-1' }); | ||
| expect(fake.getMetadata().customAcpSessionId).toBeUndefined(); | ||
|
|
||
| supported = true; | ||
| await publication.persistBound({ generation: 1, operation: 'create', vendorSessionId: 'acp-session-2' }); | ||
| expect(fake.getMetadata().customAcpSessionId).toBe('acp-session-2'); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| import type { AcpSessionIdentityPublication } from '@/agent/acp/runtime/sessionIdentityBinding'; | ||
| import type { ApiSessionClient } from '@/api/session/sessionClient'; | ||
| import { createVendorResumeIdMetadataPublisher } from '@/session/metadata/createVendorResumeIdMetadataPublisher'; | ||
|
|
||
| /** | ||
| * Session identity publication for configured (user-defined) ACP backends. | ||
| * | ||
| * Unlike built-in catalog agents, a configured backend is an arbitrary ACP | ||
| * adapter: some implement `session/load` (vendor resume), many do not. The ACP | ||
| * `initialize` handshake advertises this via `agentCapabilities.loadSession`, | ||
| * so publication is gated on the adapter's declared capability at bind time: | ||
| * adapters without load support publish nothing, their sessions carry no | ||
| * `customAcpSessionId`, and the runtime_checked resume policy keeps resume | ||
| * unavailable for them instead of failing against an adapter that cannot load. | ||
| */ | ||
| export function createConfiguredAcpSessionIdentityPublication(params: Readonly<{ | ||
| session: ApiSessionClient; | ||
| isSessionLoadSupported: () => boolean; | ||
| }>): AcpSessionIdentityPublication { | ||
| const publisher = createVendorResumeIdMetadataPublisher({ | ||
| agentId: 'customAcp', | ||
| getMetadataSnapshot: () => params.session.getMetadataSnapshot(), | ||
| updateMetadata: (updater) => params.session.updateMetadata(updater), | ||
| }); | ||
| return { | ||
| kind: 'persist-bound', | ||
| persistBound: async (event) => { | ||
| if (!params.isSessionLoadSupported()) return; | ||
| await publisher.persistBound(event); | ||
|
Comment on lines
+24
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win Invalidate 🤖 Prompt for AI Agents |
||
| }, | ||
| }; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes normal session metadata publication depend on the live ACP
initializecapability reported by the adapter. That violates the repository directive that capabilities are diagnostic and must not gate normal UI or CLI behavior. The decision belongs in the canonical configured-backend capability or policy owner rather than in the running adapter's handshake, and this repository requirement must be satisfied before merging. The same runtime-gating pattern is introduced whereAcpBackendstores the capability and where the publication helper checks it.Context Used: AGENTS.md (source)
Knowledge Base Used: Agent integration layer
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!