From 762ec3e8de9c9bba5927978cf8bd6bdd66047249 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 30 Sep 2026 06:23:01 +0000 Subject: [PATCH 01/13] task: add ACP runtime permission bridge --- ...026-09-30-acp-runtime-permission-bridge.md | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 tasks/active/2026-09-30-acp-runtime-permission-bridge.md diff --git a/tasks/active/2026-09-30-acp-runtime-permission-bridge.md b/tasks/active/2026-09-30-acp-runtime-permission-bridge.md new file mode 100644 index 0000000000..938432e506 --- /dev/null +++ b/tasks/active/2026-09-30-acp-runtime-permission-bridge.md @@ -0,0 +1,70 @@ +# ACP Runtime Permission Bridge + +## Problem + +The VM agent currently broadcasts raw ACP `permission/request` payloads to viewers and automatically selects the first option. This bypasses the durable Cloudflare interaction authority delivered by foundation PRs #2182 and #2187, leaks untrusted request content onto the viewer channel, and cannot safely handle reconnects, duplicate answers, cancellation, deadlines, or runtime loss. + +Slice B connects ACP `RequestPermission` to the shipped Cloudflare create/answer/settle contracts. Cloudflare remains the sole request and answer authority. This task owns `packages/vm-agent` plus the narrow Worker start-contract and runtime fixture changes needed for that bridge; UI, forms, URL elicitation, auth flows, and token custody are out of scope. + +## Authority and Constraints + +- Canonical Idea `01M3P2E0JJNQRXX020P65ZRKEJ`, approved execution plan v2, supersedes historical review. +- SAM task `01M3RF53QVR8ZWK446ZSZAFWB6`; coordinator task `01M3REXNKF8VSKT3QY4P0JPAVM`. +- Required branch: `sam/execute-task-using-skill-zafwb6`. +- Keep `ACP_INTERACTIONS_ENABLED=false`; do not mark the PR ready, merge, deploy, activate production flags, or mutate shared staging. +- Do not edit `apps/web` or `packages/acp-client` and do not advertise form or URL capability. +- Final integrated staging is explicitly deferred to the coordinator; local and CI evidence remain required. + +## Research Findings + +- PRs #2182/#2187 are present on current `main` and provide the shared `acp-interactions.ts` envelope, encrypted `InteractionStore`, callback-JWT create/settle routes, browser answer route, no-wake Worker delivery service, capability probe, and dormant VM answer endpoint. +- `sessionHostClient.RequestPermission` in `packages/vm-agent/internal/acp/session_host_client.go` still broadcasts the raw request and selects `params.Options[0]`; both behaviors must be removed. +- `SessionHost` owns the ACP connection and survives browser disconnects. `attachACPConnection` runs for each new ACP connection, so it is the correct point to mint a fresh opaque UUID generation. A recreated `SessionHost` also reaches this path and therefore receives a distinct generation. +- The VM answer endpoint already uses node-management JWT auth bound to the route workspace and checks the server execution runtime identity. Its dormant `no_waiter` response must be replaced with a session-host registry lookup that also validates connection generation. +- Worker runtime create/settle routes are mounted before browser-authenticated project routes and verify workspace callback JWTs plus the current running agent-session row. The Go client must use the workspace callback token and those existing endpoints. +- `apps/api/src/services/node-agent.ts` is the single start-session choke point for VM and Instant paths. An additive per-session interaction config here covers both runtimes without adding a second launch path. +- The approved defaults are centralized in `packages/shared/src/acp-interactions.ts`. The Worker must serialize the relevant limits/deadline into the start contract so Go does not invent divergent production constants. +- Existing no-wake delivery already calls low-level `nodeAgentRequest` with `recoverContainerOnTimeout=false`; runtime answer handling must never invoke prompt delivery or recovery. +- Relevant retained incident: callback routes placed under browser session middleware silently return 401. Existing extracted ACP callback routes follow rule 34 and must remain there. + +## Implementation Checklist + +- [ ] Add an additive versioned ACP interaction start contract derived from the centralized Worker config and task mode; missing/off/unsupported remains explicit fail-closed. +- [ ] Store per-session permission bridge settings in the VM agent and mint a new UUID generation for every ACP connection attachment. +- [ ] Add a bounded, concurrency-safe in-memory waiter and receipt registry keyed by interaction ID and bound to agent session, execution runtime identity, and connection generation. +- [ ] Replace raw viewer broadcast and first-option fallback with validated permission detail creation, durable Worker create, wait for exact option ID, and explicit cancellation on every unsupported/error path. +- [ ] Implement deadline and inbound context cancellation, connection replacement/process loss, and explicit `Stop` settlement without waking or recreating a runtime. +- [ ] Deliver settle callbacks with bounded retry on an independent bounded context and structural logging only. +- [ ] Complete the trusted answer endpoint with consumed/duplicate/conflict/stale-generation/no-waiter receipts and bounded tombstones. +- [ ] Add deterministic real ACP fixture behavior with reversed safety options for the coordinator's final staged roundtrip. +- [ ] Add contract and race tests for callback JWT workspace/session identity, recreated `SessionHost` generation, reversed options, duplicate/conflicting answer, process loss, explicit Stop, deadlines/cancellation, feature-off/unsupported behavior, and VM/Instant no-wake transport. +- [ ] Update narrow API/runtime contract documentation and fixtures without claiming unproven form/URL capability. +- [ ] Run package and repository validation, task-completion validation, and relevant Go, Cloudflare, security, constitution, test, and documentation reviews. +- [ ] Open an implementation-ready draft PR and record exact branch/contracts/test/review evidence for the coordinator. + +## Acceptance Criteria + +- A permission request creates a durable Cloudflare interaction before waiting and returns only the exact answered option ID; option order never grants authority. +- Cloudflare is the only request/answer authority. No raw permission request is broadcast to browsers and no browser-to-VM response channel exists. +- Every ACP connection attachment has a fresh UUID generation, including after `SessionHost` recreation. Answers are bound to workspace, agent session, execution runtime, connection generation, and interaction ID. +- Duplicate delivery is idempotent, conflicting delivery is rejected, and bounded receipt eviction returns `no_waiter` without re-execution. +- Request cancellation, deadline, process loss, connection replacement, and explicit Stop resolve waiters once and settle the durable record honestly. +- Missing/off/version-skew/invalid/oversized/create-failed paths cancel explicitly and never choose an option. +- Answer delivery uses the existing no-wake Worker transport for both VM and Instant. A lost runtime is rejected and is never started, restored, or woken. +- Payloads contain only bounded permission detail needed by the creator UI; arbitrary tool arguments/content never enter logs, viewer frames, or plaintext durable storage. +- Global rollout remains disabled and forms/URLs are not advertised. +- Meaningful local tests and specialist reviews pass; the draft PR remains unmerged and undeployed for coordinator review and integrated staging. + +## References + +- `packages/shared/src/acp-interactions.ts` +- `apps/api/src/routes/projects/acp-interaction-callback.ts` +- `apps/api/src/services/acp-interaction-delivery.ts` +- `apps/api/src/services/node-agent.ts` +- `packages/vm-agent/internal/acp/session_host_client.go` +- `packages/vm-agent/internal/acp/session_host_startup.go` +- `packages/vm-agent/internal/server/workspaces.go` +- `specs/001-mvp/contracts/api.md` +- `.claude/rules/34-vm-agent-callback-auth.md` +- `packages/vm-agent/.claude/rules/54-vm-agent-rollout-compatibility.md` +- `packages/vm-agent/.claude/rules/71-request-context-must-not-outlive-its-request.md` From c517b1611f93332864f58cd8e739fcceb45f5c24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 30 Sep 2026 07:18:24 +0000 Subject: [PATCH 02/13] feat(acp): bridge runtime permission requests --- .../src/services/acp-interaction-delivery.ts | 17 + .../acp-interaction-runtime-config.ts | 36 ++ apps/api/src/services/node-agent.ts | 7 +- .../vm-prompt-delivery-adapter-schemas.ts | 8 + .../tests/acp-interaction-delivery.test.ts | 93 +++- .../acp-interaction-runtime-config.test.ts | 30 ++ .../vm-agent-cross-boundary-contract.test.ts | 11 + packages/shared/src/acp-interactions.ts | 21 + packages/vm-agent/internal/acp/gateway.go | 2 + .../vm-agent/internal/acp/session_host.go | 25 +- .../internal/acp/session_host_client.go | 36 +- .../acp/session_host_interaction_transport.go | 110 +++++ .../internal/acp/session_host_interactions.go | 463 ++++++++++++++++++ .../acp/session_host_interactions_test.go | 350 +++++++++++++ .../internal/acp/session_host_startup.go | 10 +- .../server/execution_protocol_test.go | 29 +- packages/vm-agent/internal/server/server.go | 4 +- .../vm-agent/internal/server/workspaces.go | 61 ++- scripts/e2e/workspace-mock/mock-acp-agent.sh | 16 +- scripts/quality/mock-acp-agent.test.ts | 49 ++ specs/001-mvp/contracts/api.md | 12 +- ...026-09-30-acp-runtime-permission-bridge.md | 20 +- .../durable-execution-protocol-v1.json | 1 + 23 files changed, 1310 insertions(+), 101 deletions(-) create mode 100644 apps/api/src/services/acp-interaction-runtime-config.ts create mode 100644 apps/api/tests/acp-interaction-runtime-config.test.ts create mode 100644 packages/vm-agent/internal/acp/session_host_interaction_transport.go create mode 100644 packages/vm-agent/internal/acp/session_host_interactions.go create mode 100644 packages/vm-agent/internal/acp/session_host_interactions_test.go create mode 100644 scripts/quality/mock-acp-agent.test.ts diff --git a/apps/api/src/services/acp-interaction-delivery.ts b/apps/api/src/services/acp-interaction-delivery.ts index 8b75af59c2..e294455a19 100644 --- a/apps/api/src/services/acp-interaction-delivery.ts +++ b/apps/api/src/services/acp-interaction-delivery.ts @@ -1,4 +1,5 @@ import { + ACP_INTERACTION_CAPABILITY_VERSION, type AcpInteractionAnswerDecision, AcpRuntimeAnswerResponseSchema, buildAcpInteractionAnswerPath, @@ -134,6 +135,14 @@ export async function deliverAcpInteractionAnswer( if (capabilities.runtimeIdentity !== input.runtimeIdentity) { return { outcome: 'interrupted', reason: 'runtime identity changed before delivery' }; } + if ( + !capabilities.interactions?.supported || + capabilities.interactions.version !== ACP_INTERACTION_CAPABILITY_VERSION || + !capabilities.interactions.answerEndpoint || + !capabilities.interactions.permissionBridge + ) { + return { outcome: 'interrupted', reason: 'runtime permission bridge unsupported' }; + } const raw = await nodeAgentRequest( target.nodeId, env, @@ -159,6 +168,14 @@ export async function deliverAcpInteractionAnswer( } return { outcome: 'interrupted', reason: response.status }; } catch (error) { + if (error instanceof NodeAgentHttpError && error.statusCode === 409) { + try { + const response = v.parse(AcpRuntimeAnswerResponseSchema, JSON.parse(error.responseBody)); + return { outcome: 'interrupted', reason: response.status }; + } catch { + return { outcome: 'unconfirmed', reason: 'runtime conflict response was invalid' }; + } + } if (error instanceof NodeAgentHttpError && error.statusCode === 404) { return { outcome: 'interrupted', reason: 'runtime waiter missing' }; } diff --git a/apps/api/src/services/acp-interaction-runtime-config.ts b/apps/api/src/services/acp-interaction-runtime-config.ts new file mode 100644 index 0000000000..218b728af6 --- /dev/null +++ b/apps/api/src/services/acp-interaction-runtime-config.ts @@ -0,0 +1,36 @@ +import { + ACP_INTERACTION_PROTOCOL_VERSION, + type AcpInteractionRuntimeConfig, + DEFAULT_ACP_INTERACTION_DEADLINE_MARGIN_MS, + DEFAULT_ACP_INTERACTION_PERMISSION_CONVERSATION_DEADLINE_MS, + DEFAULT_ACP_INTERACTION_PERMISSION_TASK_DEADLINE_MS, + DEFAULT_ACP_INTERACTION_OPTION_ID_MAX_CHARS, + DEFAULT_ACP_INTERACTION_RUNTIME_RECEIPT_LIMIT, +} from '@simple-agent-manager/shared'; + +import type { Env } from '../env'; +import { getAcpInteractionConfig } from './acp-interaction-config'; + +export function buildAcpInteractionRuntimeConfig( + env: Env, + taskMode: string | null | undefined +): AcpInteractionRuntimeConfig { + const config = getAcpInteractionConfig(env); + return { + enabled: config.enabled, + protocolVersion: ACP_INTERACTION_PROTOCOL_VERSION, + permissionDeadlineMs: + taskMode === 'conversation' + ? DEFAULT_ACP_INTERACTION_PERMISSION_CONVERSATION_DEADLINE_MS + : DEFAULT_ACP_INTERACTION_PERMISSION_TASK_DEADLINE_MS, + maxDeadlineMs: config.maxDeadlineMs, + deadlineMarginMs: DEFAULT_ACP_INTERACTION_DEADLINE_MARGIN_MS, + requestMaxBytes: config.requestMaxBytes, + optionsMaxCount: config.optionsMaxCount, + optionIdMaxChars: DEFAULT_ACP_INTERACTION_OPTION_ID_MAX_CHARS, + optionNameMaxChars: config.optionNameMaxChars, + receiptLimit: DEFAULT_ACP_INTERACTION_RUNTIME_RECEIPT_LIMIT, + settleRetryDelaysMs: config.retryDelaysMs, + settleRetrySteadyMs: config.retrySteadyMs, + }; +} diff --git a/apps/api/src/services/node-agent.ts b/apps/api/src/services/node-agent.ts index 764b3d629b..2fdd9eab56 100644 --- a/apps/api/src/services/node-agent.ts +++ b/apps/api/src/services/node-agent.ts @@ -12,6 +12,7 @@ import { import type { Env } from '../env'; import { expectJsonRecord, maybeJsonRecord } from '../lib/runtime-validation'; import { AppError } from '../middleware/error'; +import { buildAcpInteractionRuntimeConfig } from './acp-interaction-runtime-config'; import { fetchWithTimeout, getTimeoutMs } from './fetch-timeout'; import { signNodeManagementToken, signTerminalToken } from './jwt'; import { @@ -618,7 +619,11 @@ export async function startAgentSessionOnNode( injectedInstructions?: string, options?: GuardedNodeAgentMutationOptions ): Promise { - const body: Record = { agentType, initialPrompt }; + const body: Record = { + agentType, + initialPrompt, + acpInteractions: buildAcpInteractionRuntimeConfig(env, taskContext?.taskMode), + }; if (injectedInstructions != null && injectedInstructions !== '') { // SAM-injected system instructions delivered as a separate origin="system" // prompt block (see buildInjectedInstructions). The agent reads it as model diff --git a/apps/api/src/services/vm-prompt-delivery-adapter-schemas.ts b/apps/api/src/services/vm-prompt-delivery-adapter-schemas.ts index 06ad9838b8..fb3cb156ca 100644 --- a/apps/api/src/services/vm-prompt-delivery-adapter-schemas.ts +++ b/apps/api/src/services/vm-prompt-delivery-adapter-schemas.ts @@ -20,6 +20,14 @@ export const CapabilitiesSchema = v.object({ lookup: v.boolean(), states: v.array(v.picklist(['accepted', 'in_flight', 'completed', 'not_found', 'ambiguous'])), }), + interactions: v.optional( + v.object({ + supported: v.boolean(), + version: v.number(), + answerEndpoint: v.boolean(), + permissionBridge: v.optional(v.boolean(), false), + }) + ), checkpointRollover: v.object({ supported: v.boolean(), automatic: v.boolean(), diff --git a/apps/api/tests/acp-interaction-delivery.test.ts b/apps/api/tests/acp-interaction-delivery.test.ts index 5a1a7c042b..88679604fe 100644 --- a/apps/api/tests/acp-interaction-delivery.test.ts +++ b/apps/api/tests/acp-interaction-delivery.test.ts @@ -46,6 +46,12 @@ describe('ACP interaction answer delivery', () => { lookup: true, states: ['accepted', 'in_flight', 'completed', 'not_found', 'ambiguous'], }, + interactions: { + supported: true, + version: 1, + answerEndpoint: true, + permissionBridge: true, + }, checkpointRollover: { supported: true, automatic: false, @@ -56,9 +62,14 @@ describe('ACP interaction answer delivery', () => { }, }); - it.each(['consumed', 'duplicate'] as const)( - 'confirms %s runtime receipts without waking', - async (status) => { + it.each([ + ['vm', 'consumed'], + ['vm', 'duplicate'], + ['cf-container', 'consumed'], + ['cf-container', 'duplicate'], + ] as const)( + 'confirms %s %s runtime receipts without recovery', + async (runtime, status) => { nodeAgentRequest.mockResolvedValueOnce(capabilities()).mockResolvedValueOnce({ status, interactionId: input.interactionId, @@ -66,10 +77,9 @@ describe('ACP interaction answer delivery', () => { runtimeIdentity: input.runtimeIdentity, }); - await expect(deliverAcpInteractionAnswer({} as never, target, input)).resolves.toMatchObject({ - outcome: 'confirmed', - runtimeStatus: status, - }); + await expect( + deliverAcpInteractionAnswer({} as never, { ...target, runtime }, input) + ).resolves.toMatchObject({ outcome: 'confirmed', runtimeStatus: status }); expect(nodeAgentRequest).toHaveBeenCalledWith( 'node-1', expect.anything(), @@ -100,25 +110,43 @@ describe('ACP interaction answer delivery', () => { } ); - it('interrupts dead runtime generations before sending', async () => { - nodeAgentRequest.mockResolvedValueOnce(capabilities('runtime-2')); - await expect( - deliverAcpInteractionAnswer({} as never, target, { ...input, runtimeIdentity: 'old-runtime' }) - ).resolves.toMatchObject({ + it.each(['vm', 'cf-container'] as const)( + 'interrupts a dead %s runtime generation without recovery', + async (runtime) => { + nodeAgentRequest.mockResolvedValueOnce(capabilities('runtime-2')); + await expect( + deliverAcpInteractionAnswer( + {} as never, + { ...target, runtime }, + { ...input, runtimeIdentity: 'old-runtime' } + ) + ).resolves.toMatchObject({ + outcome: 'interrupted', + reason: 'runtime identity changed before delivery', + }); + expect(nodeAgentRequest).toHaveBeenCalledTimes(1); + expect(nodeAgentRequest).toHaveBeenCalledWith( + 'node-1', + expect.anything(), + '/workspaces/workspace-1/agent-capabilities', + expect.objectContaining({ + recoverContainerOnTimeout: false, + method: 'GET', + requestTimeoutMs: 5_000, + }) + ); + } + ); + + it('fails closed when the runtime does not advertise the permission bridge', async () => { + const { interactions: _interactions, ...unsupported } = capabilities(); + nodeAgentRequest.mockResolvedValueOnce(unsupported); + + await expect(deliverAcpInteractionAnswer({} as never, target, input)).resolves.toEqual({ outcome: 'interrupted', - reason: 'runtime identity changed before delivery', + reason: 'runtime permission bridge unsupported', }); expect(nodeAgentRequest).toHaveBeenCalledTimes(1); - expect(nodeAgentRequest).toHaveBeenCalledWith( - 'node-1', - expect.anything(), - '/workspaces/workspace-1/agent-capabilities', - expect.objectContaining({ - recoverContainerOnTimeout: false, - method: 'GET', - requestTimeoutMs: 5_000, - }) - ); }); it('marks 404 as interrupted and ambiguous transport loss as unconfirmed', async () => { @@ -138,4 +166,23 @@ describe('ACP interaction answer delivery', () => { reason: 'transport outcome unknown', }); }); + + it('classifies a generation conflict returned after the capability probe', async () => { + nodeAgentRequest.mockResolvedValueOnce(capabilities()).mockRejectedValueOnce( + new NodeAgentHttpError( + 409, + JSON.stringify({ + status: 'stale_generation', + interactionId: input.interactionId, + generation: input.generation, + runtimeIdentity: 'runtime-2', + }) + ) + ); + + await expect(deliverAcpInteractionAnswer({} as never, target, input)).resolves.toEqual({ + outcome: 'interrupted', + reason: 'stale_generation', + }); + }); }); diff --git a/apps/api/tests/acp-interaction-runtime-config.test.ts b/apps/api/tests/acp-interaction-runtime-config.test.ts new file mode 100644 index 0000000000..1bfd721d52 --- /dev/null +++ b/apps/api/tests/acp-interaction-runtime-config.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from 'vitest'; + +import type { Env } from '../src/env'; +import { buildAcpInteractionRuntimeConfig } from '../src/services/acp-interaction-runtime-config'; + +describe('ACP interaction runtime start config', () => { + it('keeps rollout disabled by default and selects the task deadline', () => { + expect(buildAcpInteractionRuntimeConfig({} as Env, 'task')).toMatchObject({ + enabled: false, + protocolVersion: 1, + permissionDeadlineMs: 30 * 60 * 1000, + maxDeadlineMs: 4 * 60 * 60 * 1000, + deadlineMarginMs: 60 * 1000, + optionsMaxCount: 16, + receiptLimit: 256, + }); + }); + + it('uses the conversation deadline only for conversation sessions', () => { + expect( + buildAcpInteractionRuntimeConfig( + { ACP_INTERACTIONS_ENABLED: 'true' } as Env, + 'conversation' + ) + ).toMatchObject({ + enabled: true, + permissionDeadlineMs: 2 * 60 * 60 * 1000, + }); + }); +}); diff --git a/apps/api/tests/unit/vm-agent-cross-boundary-contract.test.ts b/apps/api/tests/unit/vm-agent-cross-boundary-contract.test.ts index fdc9d41448..92b669cc0f 100644 --- a/apps/api/tests/unit/vm-agent-cross-boundary-contract.test.ts +++ b/apps/api/tests/unit/vm-agent-cross-boundary-contract.test.ts @@ -650,6 +650,14 @@ describe('Contract 4: Send Prompt to Agent (API Worker → VM Agent)', () => { expect(parsedBody.projectId).toBe('proj-abc'); expect(parsedBody.taskId).toBe('task-xyz'); expect(parsedBody.taskMode).toBe('task'); + expect(parsedBody.acpInteractions).toEqual( + expect.objectContaining({ + enabled: false, + protocolVersion: 1, + permissionDeadlineMs: 30 * 60 * 1000, + receiptLimit: 256, + }) + ); }); it('omits optional fields when not provided', async () => { @@ -678,6 +686,9 @@ describe('Contract 4: Send Prompt to Agent (API Worker → VM Agent)', () => { expect(parsedBody.opencodeBaseUrl).toBeUndefined(); expect(parsedBody.projectId).toBeUndefined(); expect(parsedBody.taskId).toBeUndefined(); + expect(parsedBody.acpInteractions).toEqual( + expect.objectContaining({ enabled: false, permissionDeadlineMs: 30 * 60 * 1000 }) + ); }); it('preserves OpenCode Go provider and GLM 5.2 model overrides', async () => { diff --git a/packages/shared/src/acp-interactions.ts b/packages/shared/src/acp-interactions.ts index 3997d5fc05..0a9cc0dd38 100644 --- a/packages/shared/src/acp-interactions.ts +++ b/packages/shared/src/acp-interactions.ts @@ -44,6 +44,7 @@ export const DEFAULT_ACP_INTERACTION_DEADLINE_MARGIN_MS = 60 * 1000; export const DEFAULT_ACP_INTERACTION_MAX_PENDING_PER_SESSION = 8; export const DEFAULT_ACP_INTERACTION_REQUEST_MAX_BYTES = 32 * 1024; export const DEFAULT_ACP_INTERACTION_OPTIONS_MAX_COUNT = 16; +export const DEFAULT_ACP_INTERACTION_OPTION_ID_MAX_CHARS = 128; export const DEFAULT_ACP_INTERACTION_OPTION_NAME_MAX_CHARS = 200; export const DEFAULT_ACP_INTERACTION_FORM_SCHEMA_MAX_BYTES = 16 * 1024; export const DEFAULT_ACP_INTERACTION_FORM_SCHEMA_MAX_PROPERTIES = 20; @@ -62,6 +63,7 @@ export const DEFAULT_ACP_INTERACTION_OUTBOX_BATCH_SIZE = 25; export const DEFAULT_ACP_INTERACTION_DELIVERY_BATCH_SIZE = 1; export const DEFAULT_ACP_INTERACTION_ALARM_WALL_TIME_MS = 15_000; export const DEFAULT_ACP_INTERACTION_ALARM_REARM_DELAY_MS = 1_000; +export const DEFAULT_ACP_INTERACTION_RUNTIME_RECEIPT_LIMIT = 256; export const AcpInteractionIdSchema = v.pipe(v.string(), v.uuid()); export const AcpInteractionGenerationSchema = v.pipe(v.string(), v.uuid()); @@ -161,6 +163,21 @@ export const AcpRuntimeAnswerResponseSchema = v.object({ runtimeIdentity: v.string(), }); +export const AcpInteractionRuntimeConfigSchema = v.object({ + enabled: v.boolean(), + protocolVersion: v.literal(ACP_INTERACTION_PROTOCOL_VERSION), + permissionDeadlineMs: v.pipe(v.number(), v.integer(), v.minValue(1)), + maxDeadlineMs: v.pipe(v.number(), v.integer(), v.minValue(1)), + deadlineMarginMs: v.pipe(v.number(), v.integer(), v.minValue(0)), + requestMaxBytes: v.pipe(v.number(), v.integer(), v.minValue(1)), + optionsMaxCount: v.pipe(v.number(), v.integer(), v.minValue(1)), + optionIdMaxChars: v.pipe(v.number(), v.integer(), v.minValue(1)), + optionNameMaxChars: v.pipe(v.number(), v.integer(), v.minValue(1)), + receiptLimit: v.pipe(v.number(), v.integer(), v.minValue(1)), + settleRetryDelaysMs: v.array(v.pipe(v.number(), v.integer(), v.minValue(0))), + settleRetrySteadyMs: v.pipe(v.number(), v.integer(), v.minValue(1)), +}); + export type AcpInteractionKind = v.InferOutput; export type AcpInteractionState = v.InferOutput; export type AcpInteractionSafeSummary = v.InferOutput; @@ -170,10 +187,14 @@ export type AcpInteractionAnswerDecision = v.InferOutput; export type AcpRuntimeAnswerRequest = v.InferOutput; export type AcpRuntimeAnswerResponse = v.InferOutput; +export type AcpInteractionRuntimeConfig = v.InferOutput< + typeof AcpInteractionRuntimeConfigSchema +>; export interface AcpInteractionCapabilities { version: typeof ACP_INTERACTION_CAPABILITY_VERSION; answerEndpoint: boolean; + permissionBridge: boolean; } export function buildAcpInteractionAnswerPath( diff --git a/packages/vm-agent/internal/acp/gateway.go b/packages/vm-agent/internal/acp/gateway.go index 54d28f06ff..cfc1373cc7 100644 --- a/packages/vm-agent/internal/acp/gateway.go +++ b/packages/vm-agent/internal/acp/gateway.go @@ -145,6 +145,8 @@ type GatewayConfig struct { WorkspaceID string // SessionID is the agent session identifier (used for persistence). SessionID string + // RuntimeIdentity identifies this vm-agent process for delivery fencing. + RuntimeIdentity string // CallbackToken is the JWT for authenticating with the control plane. CallbackToken string // ContainerResolver returns the devcontainer's Docker container ID. diff --git a/packages/vm-agent/internal/acp/session_host.go b/packages/vm-agent/internal/acp/session_host.go index e4c26486ca..f1ed5e7e99 100644 --- a/packages/vm-agent/internal/acp/session_host.go +++ b/packages/vm-agent/internal/acp/session_host.go @@ -311,6 +311,15 @@ type SessionHost struct { // Lifecycle ctx context.Context cancel context.CancelFunc + + // Durable ACP permission waiters are in-memory by design: Cloudflare owns + // durable state, while only the live connection generation may consume an answer. + interactionMu sync.Mutex + interactionConfig AcpInteractionRuntimeConfig + interactionGeneration string + interactionWaiters map[string]*acpInteractionWaiter + interactionReceipts map[string]acpInteractionReceipt + interactionReceiptOrder []string } func (h *SessionHost) now() time.Time { @@ -336,12 +345,14 @@ func NewSessionHost(config SessionHostConfig) *SessionHost { ctx, cancel := context.WithCancel(context.Background()) return &SessionHost{ - config: config, - status: HostIdle, - viewers: make(map[string]*Viewer), - messageBuf: make([]BufferedMessage, 0, 256), - ctx: ctx, - cancel: cancel, + config: config, + status: HostIdle, + viewers: make(map[string]*Viewer), + messageBuf: make([]BufferedMessage, 0, 256), + interactionWaiters: make(map[string]*acpInteractionWaiter), + interactionReceipts: make(map[string]acpInteractionReceipt), + ctx: ctx, + cancel: cancel, } } @@ -528,6 +539,7 @@ func (h *SessionHost) autoSuspend() { // as stopped. This is the only way to terminate the agent — browser disconnects // do NOT call this. func (h *SessionHost) Stop() { + h.cancelInteractionWaiters("session_stopped") h.promptMu.Lock() attempt := h.promptAttempt activePrompt := h.promptInFlight @@ -625,6 +637,7 @@ func (h *SessionHost) ensureAgentInstalled(ctx context.Context, info agentComman // stopCurrentAgentLocked stops the current agent process. Must hold h.mu. func (h *SessionHost) stopCurrentAgentLocked() { + h.cancelInteractionWaiters("connection_closed") // Stop the process-scoped harness heartbeat before clearing the ACP // connection. A replacement connection will establish fresh state. h.clearHarnessWork() diff --git a/packages/vm-agent/internal/acp/session_host_client.go b/packages/vm-agent/internal/acp/session_host_client.go index 58d0642cb8..00ff44ed14 100644 --- a/packages/vm-agent/internal/acp/session_host_client.go +++ b/packages/vm-agent/internal/acp/session_host_client.go @@ -19,10 +19,11 @@ import ( // sessionHostClient implements the acp-go-sdk Client interface. // Instead of writing to a single WebSocket, it broadcasts to all viewers. type sessionHostClient struct { - host *SessionHost - processedCh chan struct{} // Signaled only after session/update completes (used by orderedPipe). - usageAttribution credentialAttribution - hasUsageAttribution bool + host *SessionHost + processedCh chan struct{} // Signaled only after session/update completes (used by orderedPipe). + usageAttribution credentialAttribution + hasUsageAttribution bool + interactionGeneration string } // signalProcessed signals that a session/update handler completed, allowing @@ -89,31 +90,8 @@ func (c *sessionHostClient) SessionUpdate(_ context.Context, params acpsdk.Sessi return nil } -func (c *sessionHostClient) RequestPermission(_ context.Context, params acpsdk.RequestPermissionRequest) (acpsdk.RequestPermissionResponse, error) { - data, err := json.Marshal(map[string]interface{}{ - "jsonrpc": "2.0", - "method": "permission/request", - "params": params, - }) - if err != nil { - return acpsdk.RequestPermissionResponse{}, fmt.Errorf("failed to marshal permission request: %w", err) - } - c.host.broadcastMessage(data) - - mode := c.host.permissionMode - if mode == "" { - mode = "default" - } - slog.Info("Permission request", "mode", mode, "optionsCount", len(params.Options)) - - if len(params.Options) > 0 { - return acpsdk.RequestPermissionResponse{ - Outcome: acpsdk.NewRequestPermissionOutcomeSelected(params.Options[0].OptionId), - }, nil - } - return acpsdk.RequestPermissionResponse{ - Outcome: acpsdk.NewRequestPermissionOutcomeCancelled(), - }, nil +func (c *sessionHostClient) RequestPermission(ctx context.Context, params acpsdk.RequestPermissionRequest) (acpsdk.RequestPermissionResponse, error) { + return c.host.requestPermission(ctx, c.interactionGeneration, params) } func (c *sessionHostClient) ReadTextFile(ctx context.Context, params acpsdk.ReadTextFileRequest) (acpsdk.ReadTextFileResponse, error) { diff --git a/packages/vm-agent/internal/acp/session_host_interaction_transport.go b/packages/vm-agent/internal/acp/session_host_interaction_transport.go new file mode 100644 index 0000000000..818ea2ca38 --- /dev/null +++ b/packages/vm-agent/internal/acp/session_host_interaction_transport.go @@ -0,0 +1,110 @@ +package acp + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "log/slog" + "net/http" + "net/url" + "strings" + "time" +) + +const maxAcpInteractionResponseBytes = 64 * 1024 + +func (h *SessionHost) createAcpInteraction(ctx context.Context, request acpInteractionCreateRequest) error { + body, err := json.Marshal(request) + if err != nil { + return fmt.Errorf("marshal ACP interaction create: %w", err) + } + endpoint := strings.TrimRight(h.config.ControlPlaneURL, "/") + "/api/projects/" + + url.PathEscape(h.config.ProjectID) + "/workspaces/" + url.PathEscape(h.config.WorkspaceID) + + "/acp-interactions" + httpRequest, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(body)) + if err != nil { + return fmt.Errorf("build ACP interaction create: %w", err) + } + httpRequest.Header.Set("Authorization", "Bearer "+h.config.CallbackToken) + httpRequest.Header.Set("Content-Type", "application/json") + response, err := h.httpClient().Do(httpRequest) + if err != nil { + return fmt.Errorf("send ACP interaction create: %w", err) + } + defer response.Body.Close() + var result struct { + Status string `json:"status"` + } + if err := json.NewDecoder(io.LimitReader(response.Body, maxAcpInteractionResponseBytes)).Decode(&result); err != nil { + return fmt.Errorf("decode ACP interaction create response: %w", err) + } + if response.StatusCode < 200 || response.StatusCode >= 300 || + (result.Status != "created" && result.Status != "existing") { + return fmt.Errorf("ACP interaction create rejected with status %d (%s)", response.StatusCode, result.Status) + } + return nil +} + +func (h *SessionHost) settleAcpInteraction(request acpInteractionSettleRequest, deadline time.Time) { + config := h.acpInteractionConfigSnapshot() + if !config.Enabled || h.config.CallbackToken == "" || h.config.ControlPlaneURL == "" { + return + } + maxDuration := time.Duration(config.MaxDeadlineMs) * time.Millisecond + remaining := deadline.Sub(h.now()) + minimum := time.Duration(config.DeadlineMarginMs) * time.Millisecond + if remaining < minimum { + remaining = minimum + } + if remaining > maxDuration { + remaining = maxDuration + } + ctx, cancel := context.WithTimeout(context.Background(), remaining) + defer cancel() + body, err := json.Marshal(request) + if err != nil { + return + } + endpoint := strings.TrimRight(h.config.ControlPlaneURL, "/") + "/api/projects/" + + url.PathEscape(h.config.ProjectID) + "/workspaces/" + url.PathEscape(h.config.WorkspaceID) + + "/acp-interactions/" + url.PathEscape(request.InteractionID) + "/settle" + delays := append([]int{0}, config.SettleRetryDelaysMs...) + for attempt := 0; ; attempt++ { + if attempt > 0 { + delay := config.SettleRetrySteadyMs + if attempt-1 < len(delays)-1 { + delay = delays[attempt] + } + timer := time.NewTimer(time.Duration(delay) * time.Millisecond) + select { + case <-ctx.Done(): + timer.Stop() + return + case <-timer.C: + } + } + httpRequest, buildErr := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(body)) + if buildErr != nil { + return + } + httpRequest.Header.Set("Authorization", "Bearer "+h.config.CallbackToken) + httpRequest.Header.Set("Content-Type", "application/json") + response, sendErr := h.httpClient().Do(httpRequest) + if sendErr == nil { + _, _ = io.Copy(io.Discard, io.LimitReader(response.Body, maxAcpInteractionResponseBytes)) + response.Body.Close() + if response.StatusCode >= 200 && response.StatusCode < 300 { + return + } + if response.StatusCode == http.StatusBadRequest || response.StatusCode == http.StatusUnauthorized || + response.StatusCode == http.StatusForbidden || response.StatusCode == http.StatusNotFound || + response.StatusCode == http.StatusConflict || response.StatusCode == http.StatusGone { + return + } + } + slog.Warn("acp_interaction.settle_retry", "interactionId", request.InteractionID, + "attempt", attempt+1, "reason", request.Reason) + } +} diff --git a/packages/vm-agent/internal/acp/session_host_interactions.go b/packages/vm-agent/internal/acp/session_host_interactions.go new file mode 100644 index 0000000000..5d7f9f15c2 --- /dev/null +++ b/packages/vm-agent/internal/acp/session_host_interactions.go @@ -0,0 +1,463 @@ +package acp + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "log/slog" + "strings" + "time" + + acpsdk "github.com/coder/acp-go-sdk" + "github.com/google/uuid" +) + +const ( + acpInteractionProtocolVersion = 1 +) + +// AcpInteractionRuntimeConfig is the versioned Worker -> vm-agent start contract. +// A missing or disabled contract always fails closed. +type AcpInteractionRuntimeConfig struct { + Enabled bool `json:"enabled"` + ProtocolVersion int `json:"protocolVersion"` + PermissionDeadlineMs int64 `json:"permissionDeadlineMs"` + MaxDeadlineMs int64 `json:"maxDeadlineMs"` + DeadlineMarginMs int64 `json:"deadlineMarginMs"` + RequestMaxBytes int `json:"requestMaxBytes"` + OptionsMaxCount int `json:"optionsMaxCount"` + OptionIDMaxChars int `json:"optionIdMaxChars"` + OptionNameMaxChars int `json:"optionNameMaxChars"` + ReceiptLimit int `json:"receiptLimit"` + SettleRetryDelaysMs []int `json:"settleRetryDelaysMs"` + SettleRetrySteadyMs int `json:"settleRetrySteadyMs"` +} + +func (c AcpInteractionRuntimeConfig) validate() error { + if !c.Enabled { + return nil + } + if c.ProtocolVersion != acpInteractionProtocolVersion { + return fmt.Errorf("unsupported ACP interaction protocol version %d", c.ProtocolVersion) + } + if c.PermissionDeadlineMs <= 0 || c.MaxDeadlineMs <= 0 || c.DeadlineMarginMs < 0 || + c.PermissionDeadlineMs > c.MaxDeadlineMs { + return errors.New("invalid ACP interaction deadline configuration") + } + if c.RequestMaxBytes <= 0 || c.OptionsMaxCount <= 0 || c.OptionIDMaxChars <= 0 || + c.OptionNameMaxChars <= 0 || c.ReceiptLimit <= 0 || c.SettleRetrySteadyMs <= 0 { + return errors.New("invalid ACP interaction bounds") + } + for _, delay := range c.SettleRetryDelaysMs { + if delay <= 0 { + return errors.New("invalid ACP interaction settle retry delay") + } + } + return nil +} + +// ValidateAcpInteractionRuntimeConfig validates an inbound session-start contract. +func ValidateAcpInteractionRuntimeConfig(config AcpInteractionRuntimeConfig) error { + return config.validate() +} + +type acpPermissionOption struct { + ID string `json:"id"` + Kind string `json:"kind"` + Name string `json:"name"` +} + +type acpPermissionDetail struct { + ToolCallID string `json:"toolCallId"` + Title string `json:"title,omitempty"` + ToolKind string `json:"toolKind,omitempty"` + Options []acpPermissionOption `json:"options"` +} + +type acpInteractionCreateRequest struct { + ProtocolVersion int `json:"protocolVersion"` + InteractionID string `json:"interactionId"` + Generation string `json:"generation"` + RuntimeIdentity string `json:"runtimeIdentity"` + AgentSessionID string `json:"agentSessionId"` + Kind string `json:"kind"` + PayloadHash string `json:"payloadHash"` + Detail acpPermissionDetail `json:"detail"` + SafeSummary struct { + ToolCallID string `json:"toolCallId,omitempty"` + OptionCount int `json:"optionCount"` + } `json:"safeSummary"` + DeadlineAt int64 `json:"deadlineAt"` +} + +type acpInteractionSettleRequest struct { + ProtocolVersion int `json:"protocolVersion"` + InteractionID string `json:"interactionId"` + Generation string `json:"generation"` + RuntimeIdentity string `json:"runtimeIdentity"` + AgentSessionID string `json:"agentSessionId"` + Reason string `json:"reason"` +} + +// AcpInteractionAnswerDecision is the bounded decision subset used by permissions. +type AcpInteractionAnswerDecision struct { + Kind string `json:"kind"` + OptionID string `json:"optionId,omitempty"` + AnswerHash string `json:"answerHash"` +} + +// ValidatePermissionAcpInteractionDecision mirrors the shared permission answer +// boundary without accepting form or URL decision shapes. +func ValidatePermissionAcpInteractionDecision(decision AcpInteractionAnswerDecision) error { + decodedHash, err := hex.DecodeString(decision.AnswerHash) + if err != nil || len(decodedHash) != sha256.Size { + return errors.New("answerHash must be a SHA-256 hex digest") + } + switch decision.Kind { + case "selected_option": + if strings.TrimSpace(decision.OptionID) == "" { + return errors.New("selected_option requires optionId") + } + case "declined", "cancelled": + if decision.OptionID != "" { + return errors.New("cancel decision must not include optionId") + } + default: + return errors.New("unsupported permission decision kind") + } + return nil +} + +type acpInteractionWaitResult struct { + optionID string + cancel bool + reason string +} + +type acpInteractionWaiter struct { + generation string + options map[string]struct{} + result chan acpInteractionWaitResult +} + +type acpInteractionReceipt struct { + generation string + decisionHash string +} + +func (h *SessionHost) configureAcpInteractions(config AcpInteractionRuntimeConfig) { + h.interactionMu.Lock() + defer h.interactionMu.Unlock() + h.interactionConfig = config +} + +// ConfigureAcpInteractions applies the trusted session-start contract before the +// ACP process attaches. Invalid enabled contracts are rejected by the caller. +func (h *SessionHost) ConfigureAcpInteractions(config AcpInteractionRuntimeConfig) { + h.configureAcpInteractions(config) +} + +func (h *SessionHost) acpInteractionConfigSnapshot() AcpInteractionRuntimeConfig { + h.interactionMu.Lock() + defer h.interactionMu.Unlock() + config := h.interactionConfig + config.SettleRetryDelaysMs = append([]int(nil), config.SettleRetryDelaysMs...) + return config +} + +func (h *SessionHost) attachAcpInteractionGeneration() string { + generation := uuid.NewString() + h.interactionMu.Lock() + h.cancelInteractionWaitersLocked("connection_replaced") + h.interactionGeneration = generation + h.interactionMu.Unlock() + return generation +} + +func (h *SessionHost) cancelInteractionWaiters(reason string) { + h.interactionMu.Lock() + defer h.interactionMu.Unlock() + h.cancelInteractionWaitersLocked(reason) +} + +func (h *SessionHost) cancelInteractionWaitersLocked(reason string) { + for interactionID, waiter := range h.interactionWaiters { + delete(h.interactionWaiters, interactionID) + waiter.result <- acpInteractionWaitResult{cancel: true, reason: reason} + } +} + +func (h *SessionHost) registerInteractionWaiter( + interactionID string, + generation string, + options map[string]struct{}, +) (*acpInteractionWaiter, error) { + h.interactionMu.Lock() + defer h.interactionMu.Unlock() + if !h.interactionConfig.Enabled || generation == "" || generation != h.interactionGeneration { + return nil, errors.New("ACP interaction generation is not active") + } + if _, exists := h.interactionWaiters[interactionID]; exists { + return nil, errors.New("ACP interaction waiter already exists") + } + waiter := &acpInteractionWaiter{ + generation: generation, + options: options, + result: make(chan acpInteractionWaitResult, 1), + } + h.interactionWaiters[interactionID] = waiter + return waiter, nil +} + +// CancelAcpInteractionWaiter removes one waiter if cancellation wins the race. +// A false result means an answer or lifecycle cancellation already claimed it. +func (h *SessionHost) cancelAcpInteractionWaiter(interactionID, generation, reason string) bool { + h.interactionMu.Lock() + defer h.interactionMu.Unlock() + waiter, ok := h.interactionWaiters[interactionID] + if !ok || waiter.generation != generation { + return false + } + delete(h.interactionWaiters, interactionID) + waiter.result <- acpInteractionWaitResult{cancel: true, reason: reason} + return true +} + +func canonicalDecisionHash(decision AcpInteractionAnswerDecision) string { + encoded, _ := json.Marshal(decision) + hash := sha256.Sum256(encoded) + return hex.EncodeToString(hash[:]) +} + +func (h *SessionHost) rememberInteractionReceiptLocked( + interactionID string, + receipt acpInteractionReceipt, +) { + if _, exists := h.interactionReceipts[interactionID]; !exists { + h.interactionReceiptOrder = append(h.interactionReceiptOrder, interactionID) + } + h.interactionReceipts[interactionID] = receipt + limit := h.interactionConfig.ReceiptLimit + for len(h.interactionReceiptOrder) > limit { + evicted := h.interactionReceiptOrder[0] + h.interactionReceiptOrder = h.interactionReceiptOrder[1:] + delete(h.interactionReceipts, evicted) + } +} + +// ResolveAcpInteractionAnswer consumes a trusted Worker answer without creating +// work. It is intentionally only an in-memory registry lookup. +func (h *SessionHost) ResolveAcpInteractionAnswer( + interactionID string, + generation string, + decision AcpInteractionAnswerDecision, +) string { + h.interactionMu.Lock() + defer h.interactionMu.Unlock() + if h.interactionGeneration == "" { + return "no_waiter" + } + if generation == "" || generation != h.interactionGeneration { + return "stale_generation" + } + decisionHash := canonicalDecisionHash(decision) + if receipt, exists := h.interactionReceipts[interactionID]; exists { + if receipt.generation != generation { + return "stale_generation" + } + if receipt.decisionHash == decisionHash { + return "duplicate" + } + return "conflict" + } + waiter, exists := h.interactionWaiters[interactionID] + if !exists { + return "no_waiter" + } + if waiter.generation != generation { + return "stale_generation" + } + result := acpInteractionWaitResult{} + switch decision.Kind { + case "selected_option": + if _, allowed := waiter.options[decision.OptionID]; !allowed { + return "conflict" + } + result.optionID = decision.OptionID + case "declined", "cancelled": + result.cancel = true + result.reason = "completed" + default: + return "conflict" + } + delete(h.interactionWaiters, interactionID) + h.rememberInteractionReceiptLocked(interactionID, acpInteractionReceipt{ + generation: generation, decisionHash: decisionHash, + }) + waiter.result <- result + return "consumed" +} + +func (h *SessionHost) permissionDeadline(ctx context.Context, config AcpInteractionRuntimeConfig) (time.Time, bool) { + now := h.now() + duration := time.Duration(config.PermissionDeadlineMs) * time.Millisecond + maxDuration := time.Duration(config.MaxDeadlineMs) * time.Millisecond + if duration > maxDuration { + duration = maxDuration + } + deadline := now.Add(duration) + if promptDeadline, ok := ctx.Deadline(); ok { + promptDeadline = promptDeadline.Add(-time.Duration(config.DeadlineMarginMs) * time.Millisecond) + if !promptDeadline.After(now) { + return time.Time{}, false + } + if promptDeadline.Before(deadline) { + deadline = promptDeadline + } + } + return deadline, true +} + +func permissionDetail( + params acpsdk.RequestPermissionRequest, + config AcpInteractionRuntimeConfig, +) (acpPermissionDetail, map[string]struct{}, error) { + if len(params.Options) == 0 || len(params.Options) > config.OptionsMaxCount { + return acpPermissionDetail{}, nil, errors.New("permission options are empty or exceed the configured limit") + } + detail := acpPermissionDetail{ToolCallID: string(params.ToolCall.ToolCallId)} + if params.ToolCall.Title != nil { + detail.Title = *params.ToolCall.Title + } + if params.ToolCall.Kind != nil { + detail.ToolKind = string(*params.ToolCall.Kind) + } + detail.Options = make([]acpPermissionOption, 0, len(params.Options)) + optionIDs := make(map[string]struct{}, len(params.Options)) + for _, option := range params.Options { + id := string(option.OptionId) + kind := string(option.Kind) + if id == "" || len([]rune(id)) > config.OptionIDMaxChars { + return acpPermissionDetail{}, nil, errors.New("permission option id is invalid") + } + if option.Name == "" || len([]rune(option.Name)) > config.OptionNameMaxChars { + return acpPermissionDetail{}, nil, errors.New("permission option name is invalid") + } + switch option.Kind { + case acpsdk.PermissionOptionKindAllowOnce, acpsdk.PermissionOptionKindAllowAlways, + acpsdk.PermissionOptionKindRejectOnce, acpsdk.PermissionOptionKindRejectAlways: + default: + return acpPermissionDetail{}, nil, errors.New("permission option kind is unsupported") + } + if _, duplicate := optionIDs[id]; duplicate { + return acpPermissionDetail{}, nil, errors.New("permission option ids must be unique") + } + optionIDs[id] = struct{}{} + detail.Options = append(detail.Options, acpPermissionOption{ID: id, Kind: kind, Name: option.Name}) + } + encoded, err := json.Marshal(detail) + if err != nil || len(encoded) > config.RequestMaxBytes { + return acpPermissionDetail{}, nil, errors.New("permission detail exceeds the configured limit") + } + return detail, optionIDs, nil +} + +func cancelledPermissionResponse() acpsdk.RequestPermissionResponse { + return acpsdk.RequestPermissionResponse{ + Outcome: acpsdk.NewRequestPermissionOutcomeCancelled(), + } +} + +func (h *SessionHost) requestPermission( + ctx context.Context, + generation string, + params acpsdk.RequestPermissionRequest, +) (acpsdk.RequestPermissionResponse, error) { + config := h.acpInteractionConfigSnapshot() + if !config.Enabled || config.validate() != nil || generation == "" || + h.config.ProjectID == "" || h.config.WorkspaceID == "" || h.config.SessionID == "" || + h.config.RuntimeIdentity == "" || h.config.CallbackToken == "" || h.config.ControlPlaneURL == "" { + slog.Info("acp_interaction.permission_cancelled", "reason", "unsupported") + return cancelledPermissionResponse(), nil + } + deadline, ok := h.permissionDeadline(ctx, config) + if !ok { + slog.Info("acp_interaction.permission_cancelled", "reason", "deadline_elapsed") + return cancelledPermissionResponse(), nil + } + detail, optionIDs, err := permissionDetail(params, config) + if err != nil { + slog.Info("acp_interaction.permission_cancelled", "reason", "invalid_request") + return cancelledPermissionResponse(), nil + } + interactionID := uuid.NewString() + request := acpInteractionCreateRequest{ + ProtocolVersion: acpInteractionProtocolVersion, + InteractionID: interactionID, + Generation: generation, + RuntimeIdentity: h.config.RuntimeIdentity, + AgentSessionID: h.config.SessionID, + Kind: "permission", + Detail: detail, + DeadlineAt: deadline.UnixMilli(), + } + request.SafeSummary.ToolCallID = detail.ToolCallID + request.SafeSummary.OptionCount = len(detail.Options) + canonical, marshalErr := json.Marshal(request) + if marshalErr != nil { + slog.Info("acp_interaction.permission_cancelled", "reason", "request_invalid") + return cancelledPermissionResponse(), nil + } + payloadHash := sha256.Sum256(canonical) + request.PayloadHash = hex.EncodeToString(payloadHash[:]) + waiter, err := h.registerInteractionWaiter(interactionID, generation, optionIDs) + if err != nil { + slog.Info("acp_interaction.permission_cancelled", "reason", "generation_unavailable") + return cancelledPermissionResponse(), nil + } + settle := func(reason string) { + go h.settleAcpInteraction(acpInteractionSettleRequest{ + ProtocolVersion: acpInteractionProtocolVersion, + InteractionID: interactionID, + Generation: generation, + RuntimeIdentity: h.config.RuntimeIdentity, + AgentSessionID: h.config.SessionID, + Reason: reason, + }, deadline) + } + if err := h.createAcpInteraction(ctx, request); err != nil { + h.cancelAcpInteractionWaiter(interactionID, generation, "wrapper_cancelled") + settle("wrapper_cancelled") + slog.Warn("acp_interaction.create_failed", "interactionId", interactionID, + "reason", "control_plane_rejected") + return cancelledPermissionResponse(), nil + } + + timer := time.NewTimer(deadline.Sub(h.now())) + defer timer.Stop() + var result acpInteractionWaitResult + select { + case result = <-waiter.result: + case <-ctx.Done(): + h.cancelAcpInteractionWaiter(interactionID, generation, "wrapper_cancelled") + result = <-waiter.result + case <-timer.C: + h.cancelAcpInteractionWaiter(interactionID, generation, "expired") + result = <-waiter.result + } + if result.cancel { + reason := result.reason + if reason == "" { + reason = "wrapper_cancelled" + } + settle(reason) + return cancelledPermissionResponse(), nil + } + settle("completed") + return acpsdk.RequestPermissionResponse{ + Outcome: acpsdk.NewRequestPermissionOutcomeSelected(acpsdk.PermissionOptionId(result.optionID)), + }, nil +} diff --git a/packages/vm-agent/internal/acp/session_host_interactions_test.go b/packages/vm-agent/internal/acp/session_host_interactions_test.go new file mode 100644 index 0000000000..bfed19ff6d --- /dev/null +++ b/packages/vm-agent/internal/acp/session_host_interactions_test.go @@ -0,0 +1,350 @@ +package acp + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + "time" + + acpsdk "github.com/coder/acp-go-sdk" +) + +func testInteractionConfig() AcpInteractionRuntimeConfig { + return AcpInteractionRuntimeConfig{ + Enabled: true, ProtocolVersion: acpInteractionProtocolVersion, + PermissionDeadlineMs: 2_000, MaxDeadlineMs: 4_000, DeadlineMarginMs: 10, + RequestMaxBytes: 32 * 1024, OptionsMaxCount: 16, OptionIDMaxChars: 128, + OptionNameMaxChars: 200, ReceiptLimit: 2, + SettleRetryDelaysMs: []int{1, 5}, SettleRetrySteadyMs: 10, + } +} + +func permissionRequest() acpsdk.RequestPermissionRequest { + title := "Run deterministic fixture" + kind := acpsdk.ToolKindExecute + return acpsdk.RequestPermissionRequest{ + SessionId: "acp-session", + ToolCall: acpsdk.ToolCallUpdate{ + ToolCallId: "tool-call-1", + Title: &title, + Kind: &kind, + RawInput: map[string]any{"secret": "must-not-cross-the-boundary"}, + }, + Options: []acpsdk.PermissionOption{ + {OptionId: "reject", Name: "Reject", Kind: acpsdk.PermissionOptionKindRejectOnce}, + {OptionId: "allow", Name: "Allow once", Kind: acpsdk.PermissionOptionKindAllowOnce}, + }, + } +} + +type interactionRecorder struct { + server *httptest.Server + creates chan acpInteractionCreateRequest + settles chan acpInteractionSettleRequest +} + +func newInteractionRecorder(t *testing.T, createStatus int) *interactionRecorder { + t.Helper() + recorder := &interactionRecorder{ + creates: make(chan acpInteractionCreateRequest, 8), + settles: make(chan acpInteractionSettleRequest, 8), + } + recorder.server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") != "Bearer callback-token" { + t.Errorf("authorization = %q", r.Header.Get("Authorization")) + w.WriteHeader(http.StatusUnauthorized) + return + } + if r.URL.Path == "/api/projects/project-1/workspaces/workspace-1/acp-interactions" { + var request acpInteractionCreateRequest + if err := json.NewDecoder(r.Body).Decode(&request); err != nil { + t.Errorf("decode create: %v", err) + } + recorder.creates <- request + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(createStatus) + status := "created" + if createStatus != http.StatusCreated { + status = "disabled" + } + _ = json.NewEncoder(w).Encode(map[string]any{"status": status}) + return + } + if !strings.HasSuffix(r.URL.Path, "/settle") { + w.WriteHeader(http.StatusNoContent) + return + } + var request acpInteractionSettleRequest + if err := json.NewDecoder(r.Body).Decode(&request); err != nil { + t.Errorf("decode settle: %v", err) + } + recorder.settles <- request + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{"status": "settled"}) + })) + t.Cleanup(recorder.server.Close) + return recorder +} + +func newInteractionHost(recorder *interactionRecorder) (*SessionHost, *sessionHostClient) { + host := NewSessionHost(SessionHostConfig{GatewayConfig: GatewayConfig{ + ControlPlaneURL: recorder.server.URL, + ProjectID: "project-1", + WorkspaceID: "workspace-1", + SessionID: "agent-session-1", + RuntimeIdentity: "runtime-1", + CallbackToken: "callback-token", + HTTPClient: recorder.server.Client(), + }}) + host.ConfigureAcpInteractions(testInteractionConfig()) + generation := host.attachAcpInteractionGeneration() + return host, &sessionHostClient{host: host, interactionGeneration: generation} +} + +func waitCreate(t *testing.T, recorder *interactionRecorder) acpInteractionCreateRequest { + t.Helper() + select { + case request := <-recorder.creates: + return request + case <-time.After(time.Second): + t.Fatal("timed out waiting for durable interaction create") + return acpInteractionCreateRequest{} + } +} + +func waitSettle(t *testing.T, recorder *interactionRecorder) acpInteractionSettleRequest { + t.Helper() + select { + case request := <-recorder.settles: + return request + case <-time.After(time.Second): + t.Fatal("timed out waiting for durable interaction settle") + return acpInteractionSettleRequest{} + } +} + +func TestRequestPermissionUsesDurableAuthorityAndExactOptionID(t *testing.T) { + recorder := newInteractionRecorder(t, http.StatusCreated) + host, client := newInteractionHost(recorder) + defer host.Stop() + + type result struct { + response acpsdk.RequestPermissionResponse + err error + } + done := make(chan result, 1) + go func() { + response, err := client.RequestPermission(context.Background(), permissionRequest()) + done <- result{response: response, err: err} + }() + created := waitCreate(t, recorder) + if created.Generation != client.interactionGeneration || created.RuntimeIdentity != "runtime-1" || + created.AgentSessionID != "agent-session-1" { + t.Fatalf("create identity = %+v", created) + } + if created.Detail.Options[0].ID != "reject" || created.Detail.Options[1].ID != "allow" { + t.Fatalf("option order changed: %+v", created.Detail.Options) + } + encoded, _ := json.Marshal(created.Detail) + if len(encoded) == 0 || bytes.Contains(encoded, []byte("must-not-cross-the-boundary")) { + t.Fatalf("raw tool input crossed the interaction boundary: %s", encoded) + } + decision := AcpInteractionAnswerDecision{ + Kind: "selected_option", OptionID: "allow", AnswerHash: "answer-hash", + } + if status := host.ResolveAcpInteractionAnswer(created.InteractionID, created.Generation, decision); status != "consumed" { + t.Fatalf("answer status = %q", status) + } + permissionResult := <-done + if permissionResult.err != nil || permissionResult.response.Outcome.Selected == nil || + permissionResult.response.Outcome.Selected.OptionId != "allow" { + t.Fatalf("permission result = %+v err=%v", permissionResult.response, permissionResult.err) + } + host.bufMu.RLock() + bufferedMessages := len(host.messageBuf) + host.bufMu.RUnlock() + if bufferedMessages != 0 { + t.Fatalf("permission request was broadcast to viewer buffer, messages=%d", bufferedMessages) + } + if settle := waitSettle(t, recorder); settle.Reason != "completed" { + t.Fatalf("settle reason = %q", settle.Reason) + } + if status := host.ResolveAcpInteractionAnswer(created.InteractionID, created.Generation, decision); status != "duplicate" { + t.Fatalf("duplicate status = %q", status) + } + decision.OptionID = "reject" + if status := host.ResolveAcpInteractionAnswer(created.InteractionID, created.Generation, decision); status != "conflict" { + t.Fatalf("conflicting status = %q", status) + } +} + +func TestRequestPermissionCancellationLifecycleSettlesExactlyOnce(t *testing.T) { + tests := []struct { + name string + cancel func(*SessionHost) + reason string + }{ + {name: "process loss", cancel: func(host *SessionHost) { host.cancelInteractionWaiters("connection_closed") }, reason: "connection_closed"}, + {name: "connection replacement", cancel: func(host *SessionHost) { host.attachAcpInteractionGeneration() }, reason: "connection_replaced"}, + {name: "explicit stop", cancel: func(host *SessionHost) { host.Stop() }, reason: "session_stopped"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + recorder := newInteractionRecorder(t, http.StatusCreated) + host, client := newInteractionHost(recorder) + done := make(chan acpsdk.RequestPermissionResponse, 1) + go func() { + response, _ := client.RequestPermission(context.Background(), permissionRequest()) + done <- response + }() + waitCreate(t, recorder) + test.cancel(host) + response := <-done + if response.Outcome.Cancelled == nil { + t.Fatalf("outcome = %+v", response.Outcome) + } + if settle := waitSettle(t, recorder); settle.Reason != test.reason { + t.Fatalf("settle reason = %q, want %q", settle.Reason, test.reason) + } + host.Stop() + }) + } +} + +func TestRequestPermissionContextCancelAndDeadlineFailClosed(t *testing.T) { + t.Run("context cancellation", func(t *testing.T) { + recorder := newInteractionRecorder(t, http.StatusCreated) + host, client := newInteractionHost(recorder) + defer host.Stop() + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan acpsdk.RequestPermissionResponse, 1) + go func() { + response, _ := client.RequestPermission(ctx, permissionRequest()) + done <- response + }() + waitCreate(t, recorder) + cancel() + if response := <-done; response.Outcome.Cancelled == nil { + t.Fatalf("outcome = %+v", response.Outcome) + } + if settle := waitSettle(t, recorder); settle.Reason != "wrapper_cancelled" { + t.Fatalf("settle reason = %q", settle.Reason) + } + }) + + t.Run("deadline", func(t *testing.T) { + recorder := newInteractionRecorder(t, http.StatusCreated) + host, client := newInteractionHost(recorder) + defer host.Stop() + config := testInteractionConfig() + config.PermissionDeadlineMs = 20 + host.ConfigureAcpInteractions(config) + response, err := client.RequestPermission(context.Background(), permissionRequest()) + if err != nil || response.Outcome.Cancelled == nil { + t.Fatalf("response = %+v err=%v", response, err) + } + waitCreate(t, recorder) + if settle := waitSettle(t, recorder); settle.Reason != "expired" { + t.Fatalf("settle reason = %q", settle.Reason) + } + }) +} + +func TestRequestPermissionFeatureOffAndCreateFailureNeverSelect(t *testing.T) { + recorder := newInteractionRecorder(t, http.StatusCreated) + host, client := newInteractionHost(recorder) + config := testInteractionConfig() + config.Enabled = false + host.ConfigureAcpInteractions(config) + response, err := client.RequestPermission(context.Background(), permissionRequest()) + if err != nil || response.Outcome.Cancelled == nil { + t.Fatalf("feature-off response = %+v err=%v", response, err) + } + select { + case request := <-recorder.creates: + t.Fatalf("feature-off path created interaction: %+v", request) + default: + } + host.Stop() + + rejected := newInteractionRecorder(t, http.StatusConflict) + host, client = newInteractionHost(rejected) + defer host.Stop() + response, err = client.RequestPermission(context.Background(), permissionRequest()) + if err != nil || response.Outcome.Cancelled == nil { + t.Fatalf("create-failed response = %+v err=%v", response, err) + } + waitCreate(t, rejected) +} + +func TestAcpConnectionGenerationsAreOpaqueAndUniqueAcrossHosts(t *testing.T) { + first := NewSessionHost(SessionHostConfig{}) + second := NewSessionHost(SessionHostConfig{}) + defer first.Stop() + defer second.Stop() + firstGeneration := first.attachAcpInteractionGeneration() + replacementGeneration := first.attachAcpInteractionGeneration() + recreatedHostGeneration := second.attachAcpInteractionGeneration() + if firstGeneration == replacementGeneration || firstGeneration == recreatedHostGeneration || + replacementGeneration == recreatedHostGeneration { + t.Fatalf("connection generations were reused: %q %q %q", firstGeneration, replacementGeneration, recreatedHostGeneration) + } +} + +func TestInteractionAnswerRaceAndReceiptEvictionAreBounded(t *testing.T) { + host := NewSessionHost(SessionHostConfig{}) + config := testInteractionConfig() + config.ReceiptLimit = 1 + host.ConfigureAcpInteractions(config) + generation := host.attachAcpInteractionGeneration() + options := map[string]struct{}{"allow": {}} + first, err := host.registerInteractionWaiter("first", generation, options) + if err != nil { + t.Fatal(err) + } + decision := AcpInteractionAnswerDecision{Kind: "selected_option", OptionID: "allow", AnswerHash: "a"} + var wait sync.WaitGroup + wait.Add(2) + statuses := make(chan string, 2) + go func() { + defer wait.Done() + statuses <- host.ResolveAcpInteractionAnswer("first", generation, decision) + }() + go func() { + defer wait.Done() + if host.cancelAcpInteractionWaiter("first", generation, "wrapper_cancelled") { + statuses <- "cancelled" + } else { + statuses <- "lost_race" + } + }() + wait.Wait() + close(statuses) + seen := make(map[string]int) + for status := range statuses { + seen[status]++ + } + if seen["consumed"]+seen["cancelled"] != 1 || seen["lost_race"] != 1 { + t.Fatalf("race statuses = %+v", seen) + } + if len(first.result) != 1 { + t.Fatalf("waiter resolved %d times", len(first.result)) + } + + second, err := host.registerInteractionWaiter("second", generation, options) + if err != nil { + t.Fatal(err) + } + if status := host.ResolveAcpInteractionAnswer("second", generation, decision); status != "consumed" { + t.Fatalf("second status = %q", status) + } + <-second.result + if status := host.ResolveAcpInteractionAnswer("first", generation, decision); status != "no_waiter" { + t.Fatalf("evicted receipt status = %q", status) + } +} diff --git a/packages/vm-agent/internal/acp/session_host_startup.go b/packages/vm-agent/internal/acp/session_host_startup.go index 5e48e4ed14..aa3530f87e 100644 --- a/packages/vm-agent/internal/acp/session_host_startup.go +++ b/packages/vm-agent/internal/acp/session_host_startup.go @@ -579,13 +579,15 @@ func (h *SessionHost) attachACPConnection(process agentProcess, agentType string // startAgentWithSessionMode is called with h.mu held, so use the already // resolved agent type instead of re-entering AgentType's RWMutex. h.resetHarnessWorkForAgent(agentType) + interactionGeneration := h.attachAcpInteractionGeneration() processedCh := make(chan struct{}, 1) attr, hasAttr := h.credentialAttributionSnapshot() client := &sessionHostClient{ - host: h, - processedCh: processedCh, - usageAttribution: attr, - hasUsageAttribution: hasAttr, + host: h, + processedCh: processedCh, + usageAttribution: attr, + hasUsageAttribution: hasAttr, + interactionGeneration: interactionGeneration, } serializeTimeout := h.config.NotifSerializeTimeout diff --git a/packages/vm-agent/internal/server/execution_protocol_test.go b/packages/vm-agent/internal/server/execution_protocol_test.go index d79c0f5418..8e692cd017 100644 --- a/packages/vm-agent/internal/server/execution_protocol_test.go +++ b/packages/vm-agent/internal/server/execution_protocol_test.go @@ -70,7 +70,7 @@ func TestAcpInteractionAnswerEndpointReturnsNoWaiterForDormantRuntime(t *testing s.sessionHosts["ws-existing:session"] = acp.NewSessionHost(acp.SessionHostConfig{}) token := signWorkspaceCreateNodeToken(t, privateKey, "node-1", "ws-existing") - body := `{"protocolVersion":1,"interactionId":"11111111-1111-4111-8111-111111111111","generation":"22222222-2222-4222-8222-222222222222","runtimeIdentity":"runtime-vm-01","decision":{"kind":"permission","outcome":"approved"}}` + body := `{"protocolVersion":1,"interactionId":"11111111-1111-4111-8111-111111111111","generation":"22222222-2222-4222-8222-222222222222","runtimeIdentity":"runtime-vm-01","decision":{"kind":"selected_option","optionId":"allow","answerHash":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}}` req := httptest.NewRequest(http.MethodPost, "/workspaces/ws-existing/agent-sessions/session/interactions/11111111-1111-4111-8111-111111111111/answer", strings.NewReader(body)) req.SetPathValue("workspaceId", "ws-existing") req.SetPathValue("sessionId", "session") @@ -93,6 +93,29 @@ func TestAcpInteractionAnswerEndpointReturnsNoWaiterForDormantRuntime(t *testing } } +func TestAcpInteractionAnswerEndpointRejectsWrongWorkspaceJWT(t *testing.T) { + validator, privateKey := newWorkspaceCreateJWTValidator(t, "node-1") + s := newContractTestServer() + s.config.NodeID = "node-1" + s.jwtValidator = validator + s.executionRuntimeID = "runtime-vm-01" + token := signWorkspaceCreateNodeToken(t, privateKey, "node-1", "ws-other") + body := `{"protocolVersion":1,"interactionId":"11111111-1111-4111-8111-111111111111","generation":"22222222-2222-4222-8222-222222222222","runtimeIdentity":"runtime-vm-01","decision":{"kind":"selected_option","optionId":"allow","answerHash":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}}` + req := httptest.NewRequest(http.MethodPost, "/workspaces/ws-existing/agent-sessions/session/interactions/11111111-1111-4111-8111-111111111111/answer", strings.NewReader(body)) + req.SetPathValue("workspaceId", "ws-existing") + req.SetPathValue("sessionId", "session") + req.SetPathValue("interactionId", "11111111-1111-4111-8111-111111111111") + req.Header.Set("Authorization", "Bearer "+token) + req.Header.Set("X-SAM-Node-Id", "node-1") + req.Header.Set("X-SAM-Workspace-Id", "ws-existing") + + rec := httptest.NewRecorder() + s.handleAcpInteractionAnswer(rec, req) + if rec.Code != http.StatusUnauthorized { + t.Fatalf("status = %d body=%s", rec.Code, rec.Body.String()) + } +} + func TestAcpInteractionAnswerEndpointRequiresActiveSessionHost(t *testing.T) { validator, privateKey := newWorkspaceCreateJWTValidator(t, "node-1") s := newContractTestServer() @@ -100,7 +123,7 @@ func TestAcpInteractionAnswerEndpointRequiresActiveSessionHost(t *testing.T) { s.jwtValidator = validator s.executionRuntimeID = "runtime-vm-01" token := signWorkspaceCreateNodeToken(t, privateKey, "node-1", "ws-existing") - body := `{"protocolVersion":1,"interactionId":"11111111-1111-4111-8111-111111111111","generation":"22222222-2222-4222-8222-222222222222","runtimeIdentity":"runtime-vm-01","decision":{"kind":"permission","outcome":"approved"}}` + body := `{"protocolVersion":1,"interactionId":"11111111-1111-4111-8111-111111111111","generation":"22222222-2222-4222-8222-222222222222","runtimeIdentity":"runtime-vm-01","decision":{"kind":"selected_option","optionId":"allow","answerHash":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}}` req := httptest.NewRequest(http.MethodPost, "/workspaces/ws-existing/agent-sessions/missing/interactions/11111111-1111-4111-8111-111111111111/answer", strings.NewReader(body)) req.SetPathValue("workspaceId", "ws-existing") req.SetPathValue("sessionId", "missing") @@ -125,7 +148,7 @@ func TestAcpInteractionAnswerEndpointRejectsStaleRuntimeIdentity(t *testing.T) { s.sessionHosts["ws-existing:session"] = acp.NewSessionHost(acp.SessionHostConfig{}) token := signWorkspaceCreateNodeToken(t, privateKey, "node-1", "ws-existing") - body := `{"protocolVersion":1,"interactionId":"11111111-1111-4111-8111-111111111111","generation":"22222222-2222-4222-8222-222222222222","runtimeIdentity":"runtime-old","decision":{"kind":"permission","outcome":"approved"}}` + body := `{"protocolVersion":1,"interactionId":"11111111-1111-4111-8111-111111111111","generation":"22222222-2222-4222-8222-222222222222","runtimeIdentity":"runtime-old","decision":{"kind":"selected_option","optionId":"allow","answerHash":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}}` req := httptest.NewRequest(http.MethodPost, "/workspaces/ws-existing/agent-sessions/session/interactions/11111111-1111-4111-8111-111111111111/answer", strings.NewReader(body)) req.SetPathValue("workspaceId", "ws-existing") req.SetPathValue("sessionId", "session") diff --git a/packages/vm-agent/internal/server/server.go b/packages/vm-agent/internal/server/server.go index ca0e835f47..98d1867776 100644 --- a/packages/vm-agent/internal/server/server.go +++ b/packages/vm-agent/internal/server/server.go @@ -429,6 +429,7 @@ func New(cfg *config.Config) (*Server, error) { if cfg.IsStandaloneMode() { processLauncher = acp.LocalLauncher{} } + executionRuntimeID := uuid.NewString() // Build ACP gateway configuration acpGatewayConfig := acp.GatewayConfig{ @@ -440,6 +441,7 @@ func New(cfg *config.Config) (*Server, error) { ControlPlaneURL: cfg.ControlPlaneURL, ProjectID: cfg.ProjectID, NodeID: cfg.NodeID, + RuntimeIdentity: executionRuntimeID, WorkspaceID: defaultWorkspaceScope(cfg.WorkspaceID, cfg.NodeID), CallbackToken: cfg.CallbackToken, ContainerResolver: containerResolver, @@ -594,7 +596,7 @@ func New(cfg *config.Config) (*Server, error) { sessionProfileOvr: make(map[string]profileOverrides), sessionTaskCtx: make(map[string]taskCallbackContext), store: store, - executionRuntimeID: uuid.NewString(), + executionRuntimeID: executionRuntimeID, errorReporter: errorReporter, messageReporters: messageReporters, worktreeCache: make(map[string]cachedWorktreeList), diff --git a/packages/vm-agent/internal/server/workspaces.go b/packages/vm-agent/internal/server/workspaces.go index 69c28886f7..98646bee6c 100644 --- a/packages/vm-agent/internal/server/workspaces.go +++ b/packages/vm-agent/internal/server/workspaces.go @@ -14,6 +14,7 @@ import ( "strings" "time" + "github.com/google/uuid" "github.com/workspace/vm-agent/internal/acp" "github.com/workspace/vm-agent/internal/agentsessions" "github.com/workspace/vm-agent/internal/bootstrap" @@ -1239,20 +1240,21 @@ func (s *Server) handleStartAgentSession(w http.ResponseWriter, r *http.Request) } var body struct { - ProtocolVersion int `json:"protocolVersion,omitempty"` - DeliveryID string `json:"deliveryId,omitempty"` - MessageID string `json:"messageId,omitempty"` - AgentType string `json:"agentType"` - InitialPrompt string `json:"initialPrompt"` - McpServers []acp.McpServerEntry `json:"mcpServers,omitempty"` - Model string `json:"model,omitempty"` - PermissionMode string `json:"permissionMode,omitempty"` - Effort string `json:"effort,omitempty"` - OpencodeProvider string `json:"opencodeProvider,omitempty"` - OpencodeBaseURL string `json:"opencodeBaseUrl,omitempty"` - ProjectID string `json:"projectId,omitempty"` - TaskID string `json:"taskId,omitempty"` - TaskMode string `json:"taskMode,omitempty"` + ProtocolVersion int `json:"protocolVersion,omitempty"` + DeliveryID string `json:"deliveryId,omitempty"` + MessageID string `json:"messageId,omitempty"` + AgentType string `json:"agentType"` + InitialPrompt string `json:"initialPrompt"` + McpServers []acp.McpServerEntry `json:"mcpServers,omitempty"` + Model string `json:"model,omitempty"` + PermissionMode string `json:"permissionMode,omitempty"` + Effort string `json:"effort,omitempty"` + OpencodeProvider string `json:"opencodeProvider,omitempty"` + OpencodeBaseURL string `json:"opencodeBaseUrl,omitempty"` + ProjectID string `json:"projectId,omitempty"` + TaskID string `json:"taskId,omitempty"` + TaskMode string `json:"taskMode,omitempty"` + AcpInteractions acp.AcpInteractionRuntimeConfig `json:"acpInteractions,omitempty"` // InjectedInstructions is SAM-managed system-injected prompt text (e.g. the // get_instructions reminder). Delivered to the agent as model input but // mirrored as a separate origin="system" user message the UI collapses. @@ -1270,6 +1272,10 @@ func (s *Server) handleStartAgentSession(w http.ResponseWriter, r *http.Request) writeError(w, http.StatusBadRequest, "initialPrompt is required") return } + if err := acp.ValidateAcpInteractionRuntimeConfig(body.AcpInteractions); err != nil { + writeError(w, http.StatusBadRequest, err.Error()) + return + } body.DeliveryID = strings.TrimSpace(body.DeliveryID) body.MessageID = strings.TrimSpace(body.MessageID) if body.DeliveryID != "" { @@ -1364,6 +1370,7 @@ func (s *Server) handleStartAgentSession(w http.ResponseWriter, r *http.Request) writeError(w, http.StatusConflict, "workspace snapshot restore is in progress") return } + host.ConfigureAcpInteractions(body.AcpInteractions) s.appendNodeEvent(workspaceID, "info", "agent_session.starting", "Starting agent with initial prompt", map[string]interface{}{ "sessionId": sessionID, @@ -1761,11 +1768,11 @@ func (s *Server) writePromptReceiptNotFound(w http.ResponseWriter, deliveryID st } type acpInteractionAnswerRequest struct { - ProtocolVersion int `json:"protocolVersion"` - InteractionID string `json:"interactionId"` - Generation string `json:"generation"` - RuntimeIdentity string `json:"runtimeIdentity"` - Decision json.RawMessage `json:"decision"` + ProtocolVersion int `json:"protocolVersion"` + InteractionID string `json:"interactionId"` + Generation string `json:"generation"` + RuntimeIdentity string `json:"runtimeIdentity"` + Decision acp.AcpInteractionAnswerDecision `json:"decision"` } type acpInteractionAnswerResponse struct { @@ -1814,12 +1821,20 @@ func (s *Server) handleAcpInteractionAnswer(w http.ResponseWriter, r *http.Reque writeError(w, http.StatusBadRequest, "interactionId has an invalid format") return } + if _, err := uuid.Parse(body.InteractionID); err != nil { + writeError(w, http.StatusBadRequest, "interactionId must be a UUID") + return + } if !validExecutionProtocolID(body.Generation, maxAcpInteractionGenerationLength) { writeError(w, http.StatusBadRequest, "generation has an invalid format") return } - if body.Decision == nil || !json.Valid(body.Decision) { - writeError(w, http.StatusBadRequest, "decision is required") + if _, err := uuid.Parse(body.Generation); err != nil { + writeError(w, http.StatusBadRequest, "generation must be a UUID") + return + } + if err := acp.ValidatePermissionAcpInteractionDecision(body.Decision); err != nil { + writeError(w, http.StatusBadRequest, err.Error()) return } if body.RuntimeIdentity != s.executionRuntimeID { @@ -1832,8 +1847,9 @@ func (s *Server) handleAcpInteractionAnswer(w http.ResponseWriter, r *http.Reque return } + status := host.ResolveAcpInteractionAnswer(body.InteractionID, body.Generation, body.Decision) writeJSON(w, http.StatusOK, acpInteractionAnswerResponse{ - Status: "no_waiter", + Status: status, InteractionID: body.InteractionID, Generation: body.Generation, RuntimeIdentity: s.executionRuntimeID, @@ -1866,6 +1882,7 @@ func (s *Server) agentCapabilities() map[string]interface{} { "supported": true, "version": acpInteractionCapabilityVersion, "answerEndpoint": true, + "permissionBridge": true, "deliverySemantics": "best_effort_no_wake", "noWaiterStatus": "no_waiter", "staleStatus": "stale_generation", diff --git a/scripts/e2e/workspace-mock/mock-acp-agent.sh b/scripts/e2e/workspace-mock/mock-acp-agent.sh index 397f28efa7..ab5c36ee9a 100644 --- a/scripts/e2e/workspace-mock/mock-acp-agent.sh +++ b/scripts/e2e/workspace-mock/mock-acp-agent.sh @@ -8,6 +8,8 @@ fi log_file="${ACP_LOG_FILE:-/tmp/mock-acp-input.log}" session_id="${ACP_SESSION_ID:-session-e2e}" +permission_request_id=9001 +pending_permission_prompt_id="" touch "$log_file" @@ -29,6 +31,18 @@ while IFS= read -r line; do prompt_text="${prompt_text:-empty}" escaped_text="$(printf '%s' "$prompt_text" | sed 's/\\/\\\\/g; s/"/\\"/g')" - printf '{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"%s","update":{"sessionUpdate":"agent_message_chunk","content":{"type":"text","text":"E2E:%s"}}}}\n' "$session_id" "$escaped_text" + if [[ "$prompt_text" == *"permission-reversed"* ]]; then + pending_permission_prompt_id="$rpc_id" + printf '{"jsonrpc":"2.0","id":%s,"method":"session/request_permission","params":{"sessionId":"%s","toolCall":{"toolCallId":"fixture-tool-call","title":"Deterministic permission fixture","kind":"execute","rawInput":{"canary":"fixture-raw-input-must-not-leak"}},"options":[{"optionId":"reject","name":"Reject","kind":"reject_once"},{"optionId":"allow","name":"Allow once","kind":"allow_once"}]}}\n' "$permission_request_id" "$session_id" + else + printf '{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"%s","update":{"sessionUpdate":"agent_message_chunk","content":{"type":"text","text":"E2E:%s"}}}}\n' "$session_id" "$escaped_text" + fi + + elif [[ -n "$pending_permission_prompt_id" && "$line" == *'"id":9001'* ]]; then + selected_option="$(printf '%s' "$line" | sed -n 's/.*"optionId":"\([^"]*\)".*/\1/p')" + selected_option="${selected_option:-cancelled}" + printf '{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"%s","update":{"sessionUpdate":"agent_message_chunk","content":{"type":"text","text":"PERMISSION:%s"}}}}\n' "$session_id" "$selected_option" + printf '{"jsonrpc":"2.0","id":%s,"result":{"stopReason":"end_turn"}}\n' "$pending_permission_prompt_id" + pending_permission_prompt_id="" fi done diff --git a/scripts/quality/mock-acp-agent.test.ts b/scripts/quality/mock-acp-agent.test.ts new file mode 100644 index 0000000000..4a78ca123e --- /dev/null +++ b/scripts/quality/mock-acp-agent.test.ts @@ -0,0 +1,49 @@ +import { execFileSync } from 'node:child_process'; +import { resolve } from 'node:path'; + +import { describe, expect, it } from 'vitest'; + +describe('deterministic ACP permission fixture', () => { + it('emits reversed safety options and completes with the exact selected option', () => { + const fixture = resolve(process.cwd(), 'scripts/e2e/workspace-mock/mock-acp-agent.sh'); + const input = [ + { jsonrpc: '2.0', id: 1, method: 'initialize', params: {} }, + { jsonrpc: '2.0', id: 2, method: 'session/new', params: {} }, + { + jsonrpc: '2.0', + id: 3, + method: 'session/prompt', + params: { prompt: [{ type: 'text', text: 'permission-reversed' }] }, + }, + { + jsonrpc: '2.0', + id: 9001, + result: { outcome: { outcome: 'selected', optionId: 'allow' } }, + }, + ] + .map((entry) => JSON.stringify(entry)) + .join('\n'); + + const output = execFileSync('bash', [fixture], { + encoding: 'utf8', + input: `${input}\n`, + env: { ...process.env, ACP_LOG_FILE: '/tmp/sam-mock-acp-agent-test.log' }, + }) + .trim() + .split('\n') + .map((line) => JSON.parse(line) as Record); + + const request = output.find((entry) => entry.method === 'session/request_permission'); + expect(request?.params.options).toEqual([ + { optionId: 'reject', name: 'Reject', kind: 'reject_once' }, + { optionId: 'allow', name: 'Allow once', kind: 'allow_once' }, + ]); + expect(JSON.stringify(request)).toContain('fixture-raw-input-must-not-leak'); + + const permissionResult = output.find( + (entry) => entry.method === 'session/update' && entry.params?.update?.content?.text + ); + expect(permissionResult?.params.update.content.text).toBe('PERMISSION:allow'); + expect(output).toContainEqual({ jsonrpc: '2.0', id: 3, result: { stopReason: 'end_turn' } }); + }); +}); diff --git a/specs/001-mvp/contracts/api.md b/specs/001-mvp/contracts/api.md index 35cce24d69..3cb75e4c14 100644 --- a/specs/001-mvp/contracts/api.md +++ b/specs/001-mvp/contracts/api.md @@ -35,10 +35,13 @@ Authorization: Bearer {API_TOKEN} ## Endpoints -### Dormant ACP interaction foundation +### ACP interaction foundation and runtime permission bridge These routes exist for the durable ACP interaction foundation. New interaction creation remains disabled while `ACP_INTERACTIONS_ENABLED=false`; existing records remain readable and serviceable. +The Worker includes a versioned `acpInteractions` object in every agent-session start request. The +VM agent only creates permission interactions when that per-session contract is enabled; missing, +disabled, invalid, or unsupported contracts cancel the ACP permission request explicitly. - `POST /api/projects/:projectId/workspaces/:workspaceId/acp-interactions` creates an interaction. It requires a workspace-scoped callback JWT; project, workspace, @@ -58,6 +61,13 @@ binding, `409` for stale or conflicting state, and `410` for terminal workspaces Browser mutation routes reject callback/MCP bearer tokens because they require the normal authenticated browser session in addition to the Origin check. +Permission requests use a fresh UUID generation for every ACP connection attachment. The VM agent +persists bounded option labels and structural tool metadata through the callback route, waits for +the durable answer, and accepts only an exact option ID. Raw tool input/content is never sent on the +viewer WebSocket or copied into the interaction payload. Runtime answers use the dedicated +node-management-JWT endpoint and an in-memory waiter/receipt registry; missing or stale runtimes +return `no_waiter`/`stale_generation` and are never woken or recreated to consume an answer. + ### GET /projects/:projectId/library/:fileId/preview Return an inline preview for supported project library files. Supported MIME diff --git a/tasks/active/2026-09-30-acp-runtime-permission-bridge.md b/tasks/active/2026-09-30-acp-runtime-permission-bridge.md index 938432e506..61220b6a42 100644 --- a/tasks/active/2026-09-30-acp-runtime-permission-bridge.md +++ b/tasks/active/2026-09-30-acp-runtime-permission-bridge.md @@ -29,16 +29,16 @@ Slice B connects ACP `RequestPermission` to the shipped Cloudflare create/answer ## Implementation Checklist -- [ ] Add an additive versioned ACP interaction start contract derived from the centralized Worker config and task mode; missing/off/unsupported remains explicit fail-closed. -- [ ] Store per-session permission bridge settings in the VM agent and mint a new UUID generation for every ACP connection attachment. -- [ ] Add a bounded, concurrency-safe in-memory waiter and receipt registry keyed by interaction ID and bound to agent session, execution runtime identity, and connection generation. -- [ ] Replace raw viewer broadcast and first-option fallback with validated permission detail creation, durable Worker create, wait for exact option ID, and explicit cancellation on every unsupported/error path. -- [ ] Implement deadline and inbound context cancellation, connection replacement/process loss, and explicit `Stop` settlement without waking or recreating a runtime. -- [ ] Deliver settle callbacks with bounded retry on an independent bounded context and structural logging only. -- [ ] Complete the trusted answer endpoint with consumed/duplicate/conflict/stale-generation/no-waiter receipts and bounded tombstones. -- [ ] Add deterministic real ACP fixture behavior with reversed safety options for the coordinator's final staged roundtrip. -- [ ] Add contract and race tests for callback JWT workspace/session identity, recreated `SessionHost` generation, reversed options, duplicate/conflicting answer, process loss, explicit Stop, deadlines/cancellation, feature-off/unsupported behavior, and VM/Instant no-wake transport. -- [ ] Update narrow API/runtime contract documentation and fixtures without claiming unproven form/URL capability. +- [x] Add an additive versioned ACP interaction start contract derived from the centralized Worker config and task mode; missing/off/unsupported remains explicit fail-closed. +- [x] Store per-session permission bridge settings in the VM agent and mint a new UUID generation for every ACP connection attachment. +- [x] Add a bounded, concurrency-safe in-memory waiter and receipt registry keyed by interaction ID and bound to agent session, execution runtime identity, and connection generation. +- [x] Replace raw viewer broadcast and first-option fallback with validated permission detail creation, durable Worker create, wait for exact option ID, and explicit cancellation on every unsupported/error path. +- [x] Implement deadline and inbound context cancellation, connection replacement/process loss, and explicit `Stop` settlement without waking or recreating a runtime. +- [x] Deliver settle callbacks with bounded retry on an independent bounded context and structural logging only. +- [x] Complete the trusted answer endpoint with consumed/duplicate/conflict/stale-generation/no-waiter receipts and bounded tombstones. +- [x] Add deterministic real ACP fixture behavior with reversed safety options for the coordinator's final staged roundtrip. +- [x] Add contract and race tests for callback JWT workspace/session identity, recreated `SessionHost` generation, reversed options, duplicate/conflicting answer, process loss, explicit Stop, deadlines/cancellation, feature-off/unsupported behavior, and VM/Instant no-wake transport. +- [x] Update narrow API/runtime contract documentation and fixtures without claiming unproven form/URL capability. - [ ] Run package and repository validation, task-completion validation, and relevant Go, Cloudflare, security, constitution, test, and documentation reviews. - [ ] Open an implementation-ready draft PR and record exact branch/contracts/test/review evidence for the coordinator. diff --git a/tests/fixtures/durable-execution-protocol-v1.json b/tests/fixtures/durable-execution-protocol-v1.json index d4d68d3784..f33d4a37eb 100644 --- a/tests/fixtures/durable-execution-protocol-v1.json +++ b/tests/fixtures/durable-execution-protocol-v1.json @@ -11,6 +11,7 @@ "supported": true, "version": 1, "answerEndpoint": true, + "permissionBridge": true, "deliverySemantics": "best_effort_no_wake", "noWaiterStatus": "no_waiter", "staleStatus": "stale_generation" From c40fe42bf61d254d28d145a118e0dd884dee34d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 30 Sep 2026 07:22:23 +0000 Subject: [PATCH 03/13] test(acp): cover no-recovery runtime variants --- apps/api/src/services/acp-interaction-runtime-config.ts | 2 +- apps/api/tests/acp-interaction-delivery.test.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/api/src/services/acp-interaction-runtime-config.ts b/apps/api/src/services/acp-interaction-runtime-config.ts index 218b728af6..6a2adb3d5e 100644 --- a/apps/api/src/services/acp-interaction-runtime-config.ts +++ b/apps/api/src/services/acp-interaction-runtime-config.ts @@ -2,9 +2,9 @@ import { ACP_INTERACTION_PROTOCOL_VERSION, type AcpInteractionRuntimeConfig, DEFAULT_ACP_INTERACTION_DEADLINE_MARGIN_MS, + DEFAULT_ACP_INTERACTION_OPTION_ID_MAX_CHARS, DEFAULT_ACP_INTERACTION_PERMISSION_CONVERSATION_DEADLINE_MS, DEFAULT_ACP_INTERACTION_PERMISSION_TASK_DEADLINE_MS, - DEFAULT_ACP_INTERACTION_OPTION_ID_MAX_CHARS, DEFAULT_ACP_INTERACTION_RUNTIME_RECEIPT_LIMIT, } from '@simple-agent-manager/shared'; diff --git a/apps/api/tests/acp-interaction-delivery.test.ts b/apps/api/tests/acp-interaction-delivery.test.ts index 88679604fe..53bc228788 100644 --- a/apps/api/tests/acp-interaction-delivery.test.ts +++ b/apps/api/tests/acp-interaction-delivery.test.ts @@ -139,7 +139,8 @@ describe('ACP interaction answer delivery', () => { ); it('fails closed when the runtime does not advertise the permission bridge', async () => { - const { interactions: _interactions, ...unsupported } = capabilities(); + const unsupported: Partial> = capabilities(); + delete unsupported.interactions; nodeAgentRequest.mockResolvedValueOnce(unsupported); await expect(deliverAcpInteractionAnswer({} as never, target, input)).resolves.toEqual({ From f247d254f516a9f0cb5354d85fe9789a2866f0ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 30 Sep 2026 07:31:51 +0000 Subject: [PATCH 04/13] test(acp): preserve permission capability at checkpoints --- .../unit/services/vm-prompt-delivery-adapter.test.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/apps/api/tests/unit/services/vm-prompt-delivery-adapter.test.ts b/apps/api/tests/unit/services/vm-prompt-delivery-adapter.test.ts index 1d3f68542c..986ebd27bf 100644 --- a/apps/api/tests/unit/services/vm-prompt-delivery-adapter.test.ts +++ b/apps/api/tests/unit/services/vm-prompt-delivery-adapter.test.ts @@ -58,9 +58,15 @@ const protocolFixture = JSON.parse( notReadyPrompt: Record; notFoundReceipt: Record; }; -const promptProtocolCapabilities = { ...protocolFixture.capabilities }; -delete promptProtocolCapabilities.interactions; - +const promptProtocolCapabilities = { + ...protocolFixture.capabilities, + interactions: { + supported: true, + version: 1, + answerEndpoint: true, + permissionBridge: true, + }, +}; const targetRow = { workspace_id: 'workspace-1', user_id: 'user-1', From 0bba2b306846e306b6f740414f5c348850b7a695 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 30 Sep 2026 07:38:02 +0000 Subject: [PATCH 05/13] test(acp): accept either permission race winner --- .../vm-agent/internal/acp/session_host_interactions_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/vm-agent/internal/acp/session_host_interactions_test.go b/packages/vm-agent/internal/acp/session_host_interactions_test.go index bfed19ff6d..ac3b60ee76 100644 --- a/packages/vm-agent/internal/acp/session_host_interactions_test.go +++ b/packages/vm-agent/internal/acp/session_host_interactions_test.go @@ -329,7 +329,9 @@ func TestInteractionAnswerRaceAndReceiptEvictionAreBounded(t *testing.T) { for status := range statuses { seen[status]++ } - if seen["consumed"]+seen["cancelled"] != 1 || seen["lost_race"] != 1 { + answerWon := seen["consumed"] == 1 && seen["lost_race"] == 1 + cancelWon := seen["cancelled"] == 1 && seen["no_waiter"] == 1 + if len(seen) != 2 || (!answerWon && !cancelWon) { t.Fatalf("race statuses = %+v", seen) } if len(first.result) != 1 { From 783a897bdb64984e49fa3605356c2aa77af460c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 30 Sep 2026 08:11:25 +0000 Subject: [PATCH 06/13] fix(acp): enforce permission bridge boundaries --- .claude/skills/env-reference/SKILL.md | 4 +- apps/api/.env.example | 6 +++ .../src/durable-objects/vm-agent-container.ts | 13 +++++ apps/api/src/env.ts | 6 +++ .../projects/acp-interaction-callback.ts | 19 ++++++-- .../src/services/acp-interaction-config.ts | 42 +++++++++++++++++ .../src/services/acp-interaction-delivery.ts | 2 + .../acp-interaction-runtime-config.ts | 16 +++---- apps/api/src/services/node-agent.ts | 35 ++++++++++---- apps/api/src/services/vm-agent-container.ts | 9 ++++ .../tests/acp-interaction-delivery.test.ts | 47 +++++++++---------- .../acp-interaction-runtime-config.test.ts | 26 ++++++++-- .../vm-agent-container-wake-state.test.ts | 46 ++++++++++++++++++ .../routes/acp-interaction-callback.test.ts | 2 +- .../services/vm-agent-container-guard.test.ts | 20 ++++++-- packages/shared/src/acp-interactions.ts | 8 ++-- .../acp/session_host_interaction_transport.go | 7 ++- .../internal/acp/session_host_interactions.go | 4 +- .../acp/session_host_interactions_test.go | 19 ++++++++ .../server/execution_protocol_test.go | 10 ++++ .../vm-agent/internal/server/workspaces.go | 20 ++++++-- specs/001-mvp/contracts/api.md | 36 +++++++++++++- 22 files changed, 328 insertions(+), 69 deletions(-) diff --git a/.claude/skills/env-reference/SKILL.md b/.claude/skills/env-reference/SKILL.md index c3e82157c7..077eda7f5c 100644 --- a/.claude/skills/env-reference/SKILL.md +++ b/.claude/skills/env-reference/SKILL.md @@ -117,9 +117,11 @@ See `apps/api/.env.example` for the full list. Key variables: - `ACP_ACTIVITY_BINDING_CACHE_MAX_ENTRIES` — Maximum cached ACP activity bindings retained by one Worker isolate (default: `2048`) - `ACP_INTERACTIONS_ENABLED` — Dormant durable ACP interaction foundation kill switch. Slice A defaults this to `false`; later slices must intentionally enable producers/consumers (default: `false`) +- `ACP_INTERACTION_PERMISSION_TASK_DEADLINE_MS` / `ACP_INTERACTION_PERMISSION_CONVERSATION_DEADLINE_MS` / `ACP_INTERACTION_DEADLINE_MARGIN_MS` — Runtime permission deadlines for task and conversation sessions and the margin before an enclosing prompt deadline (defaults: `1800000` / `7200000` / `60000`) - `ACP_INTERACTION_MAX_DEADLINE_MS` — Absolute deadline ceiling for runtime-created requests (default: `14400000`) - `ACP_INTERACTION_MAX_PENDING_PER_SESSION` — Maximum pending durable ACP interactions per chat (default: `8`) -- `ACP_INTERACTION_REQUEST_MAX_BYTES`, `ACP_INTERACTION_OPTIONS_MAX_COUNT`, `ACP_INTERACTION_OPTION_NAME_MAX_CHARS`, `ACP_INTERACTION_FORM_SCHEMA_MAX_BYTES`, `ACP_INTERACTION_FORM_SCHEMA_MAX_PROPERTIES`, `ACP_INTERACTION_FORM_SCHEMA_MAX_ENUM` — Request/detail and schema bounds for encrypted ACP interaction payloads (defaults: `32768`, `16`, `200`, `16384`, `20`, `50`) +- `ACP_INTERACTION_REQUEST_MAX_BYTES`, `ACP_INTERACTION_OPTIONS_MAX_COUNT`, `ACP_INTERACTION_OPTION_ID_MAX_CHARS`, `ACP_INTERACTION_OPTION_NAME_MAX_CHARS`, `ACP_INTERACTION_FORM_SCHEMA_MAX_BYTES`, `ACP_INTERACTION_FORM_SCHEMA_MAX_PROPERTIES`, `ACP_INTERACTION_FORM_SCHEMA_MAX_ENUM` — Request/detail and schema bounds for encrypted ACP interaction payloads (defaults: `32768`, `16`, `128`, `200`, `16384`, `20`, `50`) +- `ACP_INTERACTION_RUNTIME_RECEIPT_LIMIT` / `ACP_INTERACTION_RUNTIME_RESPONSE_MAX_BYTES` — Per-SessionHost idempotency receipt bound and maximum create/settle response body read by the runtime (defaults: `256` / `65536`) - `ACP_INTERACTION_ANSWER_MAX_BYTES` / `ACP_INTERACTION_ANSWER_STRING_MAX_BYTES` — Answer decision and individual answer string bounds before encrypted storage (defaults: `16384` / `4096`) - `ACP_INTERACTION_RETRY_DELAYS_MS` / `ACP_INTERACTION_RETRY_STEADY_MS` / `ACP_INTERACTION_DELIVERY_WINDOW_MS` — Durable answer outbox retry sequence, steady retry delay, and max post-answer retry window (defaults: `1000,5000,30000,120000,300000`, `300000`, `900000`) - `ACP_INTERACTION_SENSITIVE_PURGE_MS`, `ACP_INTERACTION_SUMMARY_RETENTION_MS`, `ACP_INTERACTION_SUMMARY_LAST_SETTLED`, `ACP_INTERACTION_SNAPSHOT_LAST_SETTLED` — Sensitive encrypted payload purge, settled summary retention, and bounded snapshot controls (defaults: `3600000`, `2592000000`, `100`, `20`) diff --git a/apps/api/.env.example b/apps/api/.env.example index c52125b4c9..b4bebfeda4 100644 --- a/apps/api/.env.example +++ b/apps/api/.env.example @@ -799,11 +799,17 @@ INFOMANIAK_IP_POLL_INTERVAL_MS=3000 # ACP_ACTIVITY_BINDING_CACHE_MAX_ENTRIES=2048 # Max cached ACP activity bindings per Worker isolate # Dormant durable ACP interaction foundation. Slice A keeps this disabled; later slices wire runtime/UI producers and consumers. # ACP_INTERACTIONS_ENABLED=false # Fail-closed feature flag for durable ACP interaction creation +# ACP_INTERACTION_PERMISSION_TASK_DEADLINE_MS=1800000 # Runtime permission deadline for task sessions (30m) +# ACP_INTERACTION_PERMISSION_CONVERSATION_DEADLINE_MS=7200000 # Runtime permission deadline for conversation sessions (2h) # ACP_INTERACTION_MAX_DEADLINE_MS=14400000 # Absolute max request deadline (4h) +# ACP_INTERACTION_DEADLINE_MARGIN_MS=60000 # Margin before an enclosing prompt deadline # ACP_INTERACTION_MAX_PENDING_PER_SESSION=8 # Pending durable interactions allowed per chat # ACP_INTERACTION_REQUEST_MAX_BYTES=32768 # Encrypted request detail cap # ACP_INTERACTION_OPTIONS_MAX_COUNT=16 # Permission option count cap +# ACP_INTERACTION_OPTION_ID_MAX_CHARS=128 # Permission option identifier cap # ACP_INTERACTION_OPTION_NAME_MAX_CHARS=200 # Permission option display cap +# ACP_INTERACTION_RUNTIME_RECEIPT_LIMIT=256 # In-memory idempotency receipts retained per SessionHost +# ACP_INTERACTION_RUNTIME_RESPONSE_MAX_BYTES=65536 # Max runtime create/settle response body read # ACP_INTERACTION_FORM_SCHEMA_MAX_BYTES=16384 # Encrypted form schema cap # ACP_INTERACTION_FORM_SCHEMA_MAX_PROPERTIES=20 # Form schema property cap # ACP_INTERACTION_FORM_SCHEMA_MAX_ENUM=50 # Form enum option cap diff --git a/apps/api/src/durable-objects/vm-agent-container.ts b/apps/api/src/durable-objects/vm-agent-container.ts index d407c8afad..145c5cfbc7 100644 --- a/apps/api/src/durable-objects/vm-agent-container.ts +++ b/apps/api/src/durable-objects/vm-agent-container.ts @@ -169,6 +169,19 @@ export class VmAgentContainer extends Container { return this.proxyHttpAuthorized(request, port); } + /** Forward only to an already-running runtime; never wake or start recovery. */ + async proxyHttpNoWake(request: Request, port?: number): Promise { + const status = await this.ctx.storage.get('lifecycleStatus'); + if (status !== 'running') { + return recoveryResponse('RUNTIME_STOPPED', RUNTIME_STOPPED_MESSAGE, 410); + } + try { + return await this.containerFetch(request, port ?? this.defaultPort); + } catch { + return interruptedRequestResponse(request); + } + } + private async proxyHttpAuthorized( request: Request, port?: number, diff --git a/apps/api/src/env.ts b/apps/api/src/env.ts index 1176d69a94..5859c5a3d9 100644 --- a/apps/api/src/env.ts +++ b/apps/api/src/env.ts @@ -846,11 +846,17 @@ export interface Env extends WebhookTriggerEnv, TaskRecoveryEnv { PROJECT_EVENT_WAKE_MAX_PER_SUBSCRIPTION?: string; PROJECT_EVENT_SOURCE_OUTBOX_SWEEP_WALL_MS?: string; ACP_INTERACTIONS_ENABLED?: string; + ACP_INTERACTION_PERMISSION_TASK_DEADLINE_MS?: string; + ACP_INTERACTION_PERMISSION_CONVERSATION_DEADLINE_MS?: string; ACP_INTERACTION_MAX_DEADLINE_MS?: string; + ACP_INTERACTION_DEADLINE_MARGIN_MS?: string; ACP_INTERACTION_MAX_PENDING_PER_SESSION?: string; ACP_INTERACTION_REQUEST_MAX_BYTES?: string; ACP_INTERACTION_OPTIONS_MAX_COUNT?: string; + ACP_INTERACTION_OPTION_ID_MAX_CHARS?: string; ACP_INTERACTION_OPTION_NAME_MAX_CHARS?: string; + ACP_INTERACTION_RUNTIME_RECEIPT_LIMIT?: string; + ACP_INTERACTION_RUNTIME_RESPONSE_MAX_BYTES?: string; ACP_INTERACTION_FORM_SCHEMA_MAX_BYTES?: string; ACP_INTERACTION_FORM_SCHEMA_MAX_PROPERTIES?: string; ACP_INTERACTION_FORM_SCHEMA_MAX_ENUM?: string; diff --git a/apps/api/src/routes/projects/acp-interaction-callback.ts b/apps/api/src/routes/projects/acp-interaction-callback.ts index 4f2b503705..14ef572217 100644 --- a/apps/api/src/routes/projects/acp-interaction-callback.ts +++ b/apps/api/src/routes/projects/acp-interaction-callback.ts @@ -64,14 +64,21 @@ async function verifyWorkspaceCallback(c: { }; } -async function assertAgentSessionCurrent(env: Env, workspaceId: string, agentSessionId: string) { +async function assertAgentSessionExists( + env: Env, + workspaceId: string, + agentSessionId: string, + requireRunning: boolean +) { const row = await env.DATABASE.prepare( `SELECT id, status FROM agent_sessions WHERE id = ? AND workspace_id = ? LIMIT 1` ) .bind(agentSessionId, workspaceId) .first<{ id: string; status: string }>(); if (!row) throw errors.notFound('Agent session'); - if (row.status !== 'running') throw errors.conflict(`Agent session is ${row.status}`); + if (requireRunning && row.status !== 'running') { + throw errors.conflict(`Agent session is ${row.status}`); + } } function settleStatusCode(status: string): 200 | 404 | 409 { @@ -90,7 +97,7 @@ acpInteractionCallbackRoute.post( async (c) => { const identity = await verifyWorkspaceCallback(c); const body = c.req.valid('json'); - await assertAgentSessionCurrent(c.env, identity.workspaceId, body.agentSessionId); + await assertAgentSessionExists(c.env, identity.workspaceId, body.agentSessionId, true); const result = await createInteraction(c.env, { ...body, projectId: identity.projectId, @@ -122,7 +129,11 @@ acpInteractionCallbackRoute.post( if (body.interactionId !== c.req.param('interactionId')) { throw errors.badRequest('interactionId route/body mismatch'); } - await assertAgentSessionCurrent(c.env, identity.workspaceId, body.agentSessionId); + // A terminal session may race the runtime's final settle callback. The + // InteractionStore still fences settlement by agentSessionId, generation, + // and runtimeIdentity, so accepting the existing row cannot settle another + // runtime's interaction. + await assertAgentSessionExists(c.env, identity.workspaceId, body.agentSessionId, false); const result = await settleInteraction(c.env, { ...body, projectId: identity.projectId, diff --git a/apps/api/src/services/acp-interaction-config.ts b/apps/api/src/services/acp-interaction-config.ts index 4282578e0c..076a428396 100644 --- a/apps/api/src/services/acp-interaction-config.ts +++ b/apps/api/src/services/acp-interaction-config.ts @@ -3,6 +3,7 @@ import { DEFAULT_ACP_INTERACTION_ALARM_WALL_TIME_MS, DEFAULT_ACP_INTERACTION_ANSWER_MAX_BYTES, DEFAULT_ACP_INTERACTION_ANSWER_STRING_MAX_BYTES, + DEFAULT_ACP_INTERACTION_DEADLINE_MARGIN_MS, DEFAULT_ACP_INTERACTION_DELIVERY_BATCH_SIZE, DEFAULT_ACP_INTERACTION_DELIVERY_WINDOW_MS, DEFAULT_ACP_INTERACTION_EXPIRY_BATCH_SIZE, @@ -11,12 +12,17 @@ import { DEFAULT_ACP_INTERACTION_FORM_SCHEMA_MAX_PROPERTIES, DEFAULT_ACP_INTERACTION_MAX_DEADLINE_MS, DEFAULT_ACP_INTERACTION_MAX_PENDING_PER_SESSION, + DEFAULT_ACP_INTERACTION_OPTION_ID_MAX_CHARS, DEFAULT_ACP_INTERACTION_OPTION_NAME_MAX_CHARS, DEFAULT_ACP_INTERACTION_OPTIONS_MAX_COUNT, DEFAULT_ACP_INTERACTION_OUTBOX_BATCH_SIZE, + DEFAULT_ACP_INTERACTION_PERMISSION_CONVERSATION_DEADLINE_MS, + DEFAULT_ACP_INTERACTION_PERMISSION_TASK_DEADLINE_MS, DEFAULT_ACP_INTERACTION_REQUEST_MAX_BYTES, DEFAULT_ACP_INTERACTION_RETRY_DELAYS_MS, DEFAULT_ACP_INTERACTION_RETRY_STEADY_MS, + DEFAULT_ACP_INTERACTION_RUNTIME_RECEIPT_LIMIT, + DEFAULT_ACP_INTERACTION_RUNTIME_RESPONSE_MAX_BYTES, DEFAULT_ACP_INTERACTION_SENSITIVE_PURGE_MS, DEFAULT_ACP_INTERACTION_SNAPSHOT_LAST_SETTLED, DEFAULT_ACP_INTERACTION_SUMMARY_LAST_SETTLED, @@ -26,11 +32,17 @@ import { export interface AcpInteractionConfigEnv { ACP_INTERACTIONS_ENABLED?: string; + ACP_INTERACTION_PERMISSION_TASK_DEADLINE_MS?: string; + ACP_INTERACTION_PERMISSION_CONVERSATION_DEADLINE_MS?: string; ACP_INTERACTION_MAX_DEADLINE_MS?: string; + ACP_INTERACTION_DEADLINE_MARGIN_MS?: string; ACP_INTERACTION_MAX_PENDING_PER_SESSION?: string; ACP_INTERACTION_REQUEST_MAX_BYTES?: string; ACP_INTERACTION_OPTIONS_MAX_COUNT?: string; + ACP_INTERACTION_OPTION_ID_MAX_CHARS?: string; ACP_INTERACTION_OPTION_NAME_MAX_CHARS?: string; + ACP_INTERACTION_RUNTIME_RECEIPT_LIMIT?: string; + ACP_INTERACTION_RUNTIME_RESPONSE_MAX_BYTES?: string; ACP_INTERACTION_FORM_SCHEMA_MAX_BYTES?: string; ACP_INTERACTION_FORM_SCHEMA_MAX_PROPERTIES?: string; ACP_INTERACTION_FORM_SCHEMA_MAX_ENUM?: string; @@ -52,11 +64,17 @@ export interface AcpInteractionConfigEnv { export interface AcpInteractionConfig { enabled: boolean; + permissionTaskDeadlineMs: number; + permissionConversationDeadlineMs: number; maxDeadlineMs: number; + deadlineMarginMs: number; maxPendingPerSession: number; requestMaxBytes: number; optionsMaxCount: number; + optionIdMaxChars: number; optionNameMaxChars: number; + runtimeReceiptLimit: number; + runtimeResponseMaxBytes: number; formSchemaMaxBytes: number; formSchemaMaxProperties: number; formSchemaMaxEnum: number; @@ -99,10 +117,22 @@ function positiveIntList(value: string | undefined, fallback: readonly number[]) export function getAcpInteractionConfig(env: AcpInteractionConfigEnv): AcpInteractionConfig { return { enabled: envFlag(env.ACP_INTERACTIONS_ENABLED, DEFAULT_ACP_INTERACTIONS_ENABLED), + permissionTaskDeadlineMs: positiveInt( + env.ACP_INTERACTION_PERMISSION_TASK_DEADLINE_MS, + DEFAULT_ACP_INTERACTION_PERMISSION_TASK_DEADLINE_MS + ), + permissionConversationDeadlineMs: positiveInt( + env.ACP_INTERACTION_PERMISSION_CONVERSATION_DEADLINE_MS, + DEFAULT_ACP_INTERACTION_PERMISSION_CONVERSATION_DEADLINE_MS + ), maxDeadlineMs: positiveInt( env.ACP_INTERACTION_MAX_DEADLINE_MS, DEFAULT_ACP_INTERACTION_MAX_DEADLINE_MS ), + deadlineMarginMs: positiveInt( + env.ACP_INTERACTION_DEADLINE_MARGIN_MS, + DEFAULT_ACP_INTERACTION_DEADLINE_MARGIN_MS + ), maxPendingPerSession: positiveInt( env.ACP_INTERACTION_MAX_PENDING_PER_SESSION, DEFAULT_ACP_INTERACTION_MAX_PENDING_PER_SESSION @@ -115,10 +145,22 @@ export function getAcpInteractionConfig(env: AcpInteractionConfigEnv): AcpIntera env.ACP_INTERACTION_OPTIONS_MAX_COUNT, DEFAULT_ACP_INTERACTION_OPTIONS_MAX_COUNT ), + optionIdMaxChars: positiveInt( + env.ACP_INTERACTION_OPTION_ID_MAX_CHARS, + DEFAULT_ACP_INTERACTION_OPTION_ID_MAX_CHARS + ), optionNameMaxChars: positiveInt( env.ACP_INTERACTION_OPTION_NAME_MAX_CHARS, DEFAULT_ACP_INTERACTION_OPTION_NAME_MAX_CHARS ), + runtimeReceiptLimit: positiveInt( + env.ACP_INTERACTION_RUNTIME_RECEIPT_LIMIT, + DEFAULT_ACP_INTERACTION_RUNTIME_RECEIPT_LIMIT + ), + runtimeResponseMaxBytes: positiveInt( + env.ACP_INTERACTION_RUNTIME_RESPONSE_MAX_BYTES, + DEFAULT_ACP_INTERACTION_RUNTIME_RESPONSE_MAX_BYTES + ), formSchemaMaxBytes: positiveInt( env.ACP_INTERACTION_FORM_SCHEMA_MAX_BYTES, DEFAULT_ACP_INTERACTION_FORM_SCHEMA_MAX_BYTES diff --git a/apps/api/src/services/acp-interaction-delivery.ts b/apps/api/src/services/acp-interaction-delivery.ts index e294455a19..9af1c918fb 100644 --- a/apps/api/src/services/acp-interaction-delivery.ts +++ b/apps/api/src/services/acp-interaction-delivery.ts @@ -129,6 +129,7 @@ export async function deliverAcpInteractionAnswer( workspaceId: target.workspaceId, requestTimeoutMs, recoverContainerOnTimeout: false, + noWakeContainer: true, } ) ); @@ -153,6 +154,7 @@ export async function deliverAcpInteractionAnswer( workspaceId: target.workspaceId, requestTimeoutMs, recoverContainerOnTimeout: false, + noWakeContainer: true, body: JSON.stringify({ protocolVersion: 1, interactionId: input.interactionId, diff --git a/apps/api/src/services/acp-interaction-runtime-config.ts b/apps/api/src/services/acp-interaction-runtime-config.ts index 6a2adb3d5e..b9f1e60689 100644 --- a/apps/api/src/services/acp-interaction-runtime-config.ts +++ b/apps/api/src/services/acp-interaction-runtime-config.ts @@ -1,11 +1,6 @@ import { ACP_INTERACTION_PROTOCOL_VERSION, type AcpInteractionRuntimeConfig, - DEFAULT_ACP_INTERACTION_DEADLINE_MARGIN_MS, - DEFAULT_ACP_INTERACTION_OPTION_ID_MAX_CHARS, - DEFAULT_ACP_INTERACTION_PERMISSION_CONVERSATION_DEADLINE_MS, - DEFAULT_ACP_INTERACTION_PERMISSION_TASK_DEADLINE_MS, - DEFAULT_ACP_INTERACTION_RUNTIME_RECEIPT_LIMIT, } from '@simple-agent-manager/shared'; import type { Env } from '../env'; @@ -21,15 +16,16 @@ export function buildAcpInteractionRuntimeConfig( protocolVersion: ACP_INTERACTION_PROTOCOL_VERSION, permissionDeadlineMs: taskMode === 'conversation' - ? DEFAULT_ACP_INTERACTION_PERMISSION_CONVERSATION_DEADLINE_MS - : DEFAULT_ACP_INTERACTION_PERMISSION_TASK_DEADLINE_MS, + ? config.permissionConversationDeadlineMs + : config.permissionTaskDeadlineMs, maxDeadlineMs: config.maxDeadlineMs, - deadlineMarginMs: DEFAULT_ACP_INTERACTION_DEADLINE_MARGIN_MS, + deadlineMarginMs: config.deadlineMarginMs, requestMaxBytes: config.requestMaxBytes, optionsMaxCount: config.optionsMaxCount, - optionIdMaxChars: DEFAULT_ACP_INTERACTION_OPTION_ID_MAX_CHARS, + optionIdMaxChars: config.optionIdMaxChars, optionNameMaxChars: config.optionNameMaxChars, - receiptLimit: DEFAULT_ACP_INTERACTION_RUNTIME_RECEIPT_LIMIT, + receiptLimit: config.runtimeReceiptLimit, + responseMaxBytes: config.runtimeResponseMaxBytes, settleRetryDelaysMs: config.retryDelaysMs, settleRetrySteadyMs: config.retrySteadyMs, }; diff --git a/apps/api/src/services/node-agent.ts b/apps/api/src/services/node-agent.ts index 2fdd9eab56..8d354f3b4b 100644 --- a/apps/api/src/services/node-agent.ts +++ b/apps/api/src/services/node-agent.ts @@ -24,6 +24,7 @@ import { import { recordNodeRoutingMetric } from './telemetry'; import { fetchVmAgentContainer, + fetchVmAgentContainerNoWake, getVmAgentContainerConfig, markVmAgentContainerActiveWorkEndedBestEffort, markVmAgentContainerActiveWorkStarted, @@ -54,12 +55,15 @@ interface NodeAgentRequestOptions extends RequestInit { beforeExternalMutation?: () => Promise; /** Whether a cf-container timeout may initiate normal runtime recovery. */ recoverContainerOnTimeout?: boolean; + /** Route an Instant request only to an already-running container. */ + noWakeContainer?: boolean; } interface FetchNodeAgentControls { sourceTaskGuard?: VmAgentContainerRequestGuard; beforeExternalMutation?: () => Promise; recoverContainerOnTimeout?: boolean; + noWakeContainer?: boolean; } export interface GuardedNodeAgentMutationOptions { @@ -183,6 +187,7 @@ export async function nodeAgentRequest( sourceTaskGuard, beforeExternalMutation, recoverContainerOnTimeout = true, + noWakeContainer = false, ...requestOptions } = options; const { token } = await signNodeManagementToken(userId, nodeId, workspaceId, env); @@ -216,7 +221,7 @@ export async function nodeAgentRequest( url, { ...requestOptions, headers }, requestTimeoutMs, - { sourceTaskGuard, beforeExternalMutation, recoverContainerOnTimeout } + { sourceTaskGuard, beforeExternalMutation, recoverContainerOnTimeout, noWakeContainer } ); recordNodeRoutingMetric( @@ -283,7 +288,12 @@ export async function fetchNodeAgent( requestTimeoutMs: number, controls: FetchNodeAgentControls = {} ): Promise { - const { sourceTaskGuard, beforeExternalMutation, recoverContainerOnTimeout = true } = controls; + const { + sourceTaskGuard, + beforeExternalMutation, + recoverContainerOnTimeout = true, + noWakeContainer = false, + } = controls; if (!env.DATABASE || typeof env.DATABASE.prepare !== 'function') { await beforeExternalMutation?.(); return fetchWithTimeout(url, options, requestTimeoutMs); @@ -331,13 +341,20 @@ export async function fetchNodeAgent( try { await beforeExternalMutation?.(); const response = await Promise.race([ - fetchVmAgentContainer( - env, - nodeId, - new Request(containerUrl.toString(), requestInitWithoutSignal(options)), - vmAgentPort, - sourceTaskGuard - ), + noWakeContainer + ? fetchVmAgentContainerNoWake( + env, + nodeId, + new Request(containerUrl.toString(), requestInitWithoutSignal(options)), + vmAgentPort + ) + : fetchVmAgentContainer( + env, + nodeId, + new Request(containerUrl.toString(), requestInitWithoutSignal(options)), + vmAgentPort, + sourceTaskGuard + ), new Promise((_resolve, reject) => { timeoutHandle = setTimeout( () => reject(new Error(`Request timed out after ${requestTimeoutMs}ms`)), diff --git a/apps/api/src/services/vm-agent-container.ts b/apps/api/src/services/vm-agent-container.ts index 6ecae47d09..1bc664b710 100644 --- a/apps/api/src/services/vm-agent-container.ts +++ b/apps/api/src/services/vm-agent-container.ts @@ -87,6 +87,15 @@ export async function fetchVmAgentContainer( return container.proxyHttp(request, port); } +export async function fetchVmAgentContainerNoWake( + env: Env, + nodeId: string, + request: Request, + port?: number +): Promise { + return getVmAgentContainer(env, nodeId).proxyHttpNoWake(request, port); +} + export async function resumeVmAgentContainer( env: Env, nodeId: string, diff --git a/apps/api/tests/acp-interaction-delivery.test.ts b/apps/api/tests/acp-interaction-delivery.test.ts index 53bc228788..3da9f7b8f4 100644 --- a/apps/api/tests/acp-interaction-delivery.test.ts +++ b/apps/api/tests/acp-interaction-delivery.test.ts @@ -67,31 +67,29 @@ describe('ACP interaction answer delivery', () => { ['vm', 'duplicate'], ['cf-container', 'consumed'], ['cf-container', 'duplicate'], - ] as const)( - 'confirms %s %s runtime receipts without recovery', - async (runtime, status) => { - nodeAgentRequest.mockResolvedValueOnce(capabilities()).mockResolvedValueOnce({ - status, - interactionId: input.interactionId, - generation: input.generation, - runtimeIdentity: input.runtimeIdentity, - }); + ] as const)('confirms %s %s runtime receipts without recovery', async (runtime, status) => { + nodeAgentRequest.mockResolvedValueOnce(capabilities()).mockResolvedValueOnce({ + status, + interactionId: input.interactionId, + generation: input.generation, + runtimeIdentity: input.runtimeIdentity, + }); - await expect( - deliverAcpInteractionAnswer({} as never, { ...target, runtime }, input) - ).resolves.toMatchObject({ outcome: 'confirmed', runtimeStatus: status }); - expect(nodeAgentRequest).toHaveBeenCalledWith( - 'node-1', - expect.anything(), - '/workspaces/workspace-1/agent-sessions/agent-session-1/interactions/11111111-1111-4111-8111-111111111111/answer', - expect.objectContaining({ - recoverContainerOnTimeout: false, - method: 'POST', - requestTimeoutMs: 5_000, - }) - ); - } - ); + await expect( + deliverAcpInteractionAnswer({} as never, { ...target, runtime }, input) + ).resolves.toMatchObject({ outcome: 'confirmed', runtimeStatus: status }); + expect(nodeAgentRequest).toHaveBeenCalledWith( + 'node-1', + expect.anything(), + '/workspaces/workspace-1/agent-sessions/agent-session-1/interactions/11111111-1111-4111-8111-111111111111/answer', + expect.objectContaining({ + recoverContainerOnTimeout: false, + noWakeContainer: true, + method: 'POST', + requestTimeoutMs: 5_000, + }) + ); + }); it.each(['stale_generation', 'no_waiter', 'conflict'] as const)( 'interrupts on %s runtime receipts', @@ -131,6 +129,7 @@ describe('ACP interaction answer delivery', () => { '/workspaces/workspace-1/agent-capabilities', expect.objectContaining({ recoverContainerOnTimeout: false, + noWakeContainer: true, method: 'GET', requestTimeoutMs: 5_000, }) diff --git a/apps/api/tests/acp-interaction-runtime-config.test.ts b/apps/api/tests/acp-interaction-runtime-config.test.ts index 1bfd721d52..594813342b 100644 --- a/apps/api/tests/acp-interaction-runtime-config.test.ts +++ b/apps/api/tests/acp-interaction-runtime-config.test.ts @@ -13,15 +13,35 @@ describe('ACP interaction runtime start config', () => { deadlineMarginMs: 60 * 1000, optionsMaxCount: 16, receiptLimit: 256, + responseMaxBytes: 64 * 1024, }); }); - it('uses the conversation deadline only for conversation sessions', () => { + it('serializes operator overrides into the trusted runtime contract', () => { expect( buildAcpInteractionRuntimeConfig( - { ACP_INTERACTIONS_ENABLED: 'true' } as Env, - 'conversation' + { + ACP_INTERACTION_PERMISSION_TASK_DEADLINE_MS: '120000', + ACP_INTERACTION_PERMISSION_CONVERSATION_DEADLINE_MS: '240000', + ACP_INTERACTION_DEADLINE_MARGIN_MS: '5000', + ACP_INTERACTION_OPTION_ID_MAX_CHARS: '96', + ACP_INTERACTION_RUNTIME_RECEIPT_LIMIT: '64', + ACP_INTERACTION_RUNTIME_RESPONSE_MAX_BYTES: '8192', + } as Env, + 'task' ) + ).toMatchObject({ + permissionDeadlineMs: 120000, + deadlineMarginMs: 5000, + optionIdMaxChars: 96, + receiptLimit: 64, + responseMaxBytes: 8192, + }); + }); + + it('uses the conversation deadline only for conversation sessions', () => { + expect( + buildAcpInteractionRuntimeConfig({ ACP_INTERACTIONS_ENABLED: 'true' } as Env, 'conversation') ).toMatchObject({ enabled: true, permissionDeadlineMs: 2 * 60 * 60 * 1000, diff --git a/apps/api/tests/unit/durable-objects/vm-agent-container-wake-state.test.ts b/apps/api/tests/unit/durable-objects/vm-agent-container-wake-state.test.ts index 66419f5263..870700cbc9 100644 --- a/apps/api/tests/unit/durable-objects/vm-agent-container-wake-state.test.ts +++ b/apps/api/tests/unit/durable-objects/vm-agent-container-wake-state.test.ts @@ -27,6 +27,14 @@ function callProxyHttp(fake: unknown, request: Request): Promise { ).proxyHttp.call(fake, request); } +function callProxyHttpNoWake(fake: unknown, request: Request): Promise { + return ( + VmAgentContainer.prototype as unknown as { + proxyHttpNoWake: (this: unknown, request: Request, port?: number) => Promise; + } + ).proxyHttpNoWake.call(fake, request); +} + function callProxyHttpGuarded( fake: unknown, request: Request, @@ -70,6 +78,44 @@ function makeProxyFake(input: { } describe('VmAgentContainer proxy recovery boundaries', () => { + it.each(['sleeping', 'error', 'stopped'])( + 'never wakes or recovers a %s runtime for a no-wake request', + async (status) => { + const containerFetch = vi.fn(); + const ensureAwake = vi.fn(); + const beginUnexpectedRecovery = vi.fn(); + const response = await callProxyHttpNoWake( + { + ctx: { storage: { get: vi.fn().mockResolvedValue(status) } }, + defaultPort: 8080, + containerFetch, + ensureAwake, + beginUnexpectedRecovery, + }, + new Request('http://container/capabilities') + ); + + expect(response.status).toBe(410); + expect(containerFetch).not.toHaveBeenCalled(); + expect(ensureAwake).not.toHaveBeenCalled(); + expect(beginUnexpectedRecovery).not.toHaveBeenCalled(); + } + ); + + it('forwards a no-wake request only when the runtime is already running', async () => { + const containerFetch = vi.fn().mockResolvedValue(new Response('live')); + const response = await callProxyHttpNoWake( + { + ctx: { storage: { get: vi.fn().mockResolvedValue('running') } }, + defaultPort: 8080, + containerFetch, + }, + new Request('http://container/capabilities') + ); + + expect(await response.text()).toBe('live'); + expect(containerFetch).toHaveBeenCalledOnce(); + }); it('rejects a revoked source guard before proxy preparation can wake compute', async () => { const proxyHttp = vi.fn(); const first = vi.fn().mockResolvedValue(null); diff --git a/apps/api/tests/unit/routes/acp-interaction-callback.test.ts b/apps/api/tests/unit/routes/acp-interaction-callback.test.ts index 2a85185c4c..2c26e1edf2 100644 --- a/apps/api/tests/unit/routes/acp-interaction-callback.test.ts +++ b/apps/api/tests/unit/routes/acp-interaction-callback.test.ts @@ -161,7 +161,7 @@ describe('ACP interaction callback routes', () => { reason: 'completed', }), }, - env() + env('stopped') ); expect(settle.status).toBe(200); expect(mocks.settleInteraction).toHaveBeenCalledWith( diff --git a/apps/api/tests/unit/services/vm-agent-container-guard.test.ts b/apps/api/tests/unit/services/vm-agent-container-guard.test.ts index cbc1d95b13..48978dd8a4 100644 --- a/apps/api/tests/unit/services/vm-agent-container-guard.test.ts +++ b/apps/api/tests/unit/services/vm-agent-container-guard.test.ts @@ -8,6 +8,7 @@ function containerEnv() { fetch: vi.fn(), proxyHttp: vi.fn().mockResolvedValue(new Response('plain')), proxyHttpGuarded: vi.fn().mockResolvedValue(new Response('guarded')), + proxyHttpNoWake: vi.fn().mockResolvedValue(new Response('no-wake')), }; const get = vi.fn(() => stub); const idFromName = vi.fn(() => ({ toString: () => 'container-id' })); @@ -28,14 +29,27 @@ describe('fetchVmAgentContainer source-task guard', () => { chatSessionId: 'chat-1', }; - await expect(fetchVmAgentContainer(env, 'node-1', request, 8080, guard)).resolves.toBeInstanceOf( - Response - ); + await expect( + fetchVmAgentContainer(env, 'node-1', request, 8080, guard) + ).resolves.toBeInstanceOf(Response); expect(stub.proxyHttpGuarded).toHaveBeenCalledWith(request, 8080, guard); expect(stub.proxyHttp).not.toHaveBeenCalled(); }); + it('selects the dedicated no-wake DO RPC without entering the ordinary proxy', async () => { + const { env, stub } = containerEnv(); + const request = new Request('http://localhost/capabilities'); + + const { fetchVmAgentContainerNoWake } = + await import('../../../src/services/vm-agent-container'); + await fetchVmAgentContainerNoWake(env, 'node-1', request, 8080); + + expect(stub.proxyHttpNoWake).toHaveBeenCalledWith(request, 8080); + expect(stub.proxyHttp).not.toHaveBeenCalled(); + expect(stub.proxyHttpGuarded).not.toHaveBeenCalled(); + }); + it('preserves the ordinary proxy path when no source guard exists', async () => { const { env, stub } = containerEnv(); const request = new Request('http://localhost/capabilities'); diff --git a/packages/shared/src/acp-interactions.ts b/packages/shared/src/acp-interactions.ts index 0a9cc0dd38..31f4054d65 100644 --- a/packages/shared/src/acp-interactions.ts +++ b/packages/shared/src/acp-interactions.ts @@ -64,6 +64,7 @@ export const DEFAULT_ACP_INTERACTION_DELIVERY_BATCH_SIZE = 1; export const DEFAULT_ACP_INTERACTION_ALARM_WALL_TIME_MS = 15_000; export const DEFAULT_ACP_INTERACTION_ALARM_REARM_DELAY_MS = 1_000; export const DEFAULT_ACP_INTERACTION_RUNTIME_RECEIPT_LIMIT = 256; +export const DEFAULT_ACP_INTERACTION_RUNTIME_RESPONSE_MAX_BYTES = 64 * 1024; export const AcpInteractionIdSchema = v.pipe(v.string(), v.uuid()); export const AcpInteractionGenerationSchema = v.pipe(v.string(), v.uuid()); @@ -174,7 +175,8 @@ export const AcpInteractionRuntimeConfigSchema = v.object({ optionIdMaxChars: v.pipe(v.number(), v.integer(), v.minValue(1)), optionNameMaxChars: v.pipe(v.number(), v.integer(), v.minValue(1)), receiptLimit: v.pipe(v.number(), v.integer(), v.minValue(1)), - settleRetryDelaysMs: v.array(v.pipe(v.number(), v.integer(), v.minValue(0))), + responseMaxBytes: v.pipe(v.number(), v.integer(), v.minValue(1)), + settleRetryDelaysMs: v.array(v.pipe(v.number(), v.integer(), v.minValue(1))), settleRetrySteadyMs: v.pipe(v.number(), v.integer(), v.minValue(1)), }); @@ -187,9 +189,7 @@ export type AcpInteractionAnswerDecision = v.InferOutput; export type AcpRuntimeAnswerRequest = v.InferOutput; export type AcpRuntimeAnswerResponse = v.InferOutput; -export type AcpInteractionRuntimeConfig = v.InferOutput< - typeof AcpInteractionRuntimeConfigSchema ->; +export type AcpInteractionRuntimeConfig = v.InferOutput; export interface AcpInteractionCapabilities { version: typeof ACP_INTERACTION_CAPABILITY_VERSION; diff --git a/packages/vm-agent/internal/acp/session_host_interaction_transport.go b/packages/vm-agent/internal/acp/session_host_interaction_transport.go index 818ea2ca38..373f623506 100644 --- a/packages/vm-agent/internal/acp/session_host_interaction_transport.go +++ b/packages/vm-agent/internal/acp/session_host_interaction_transport.go @@ -13,9 +13,8 @@ import ( "time" ) -const maxAcpInteractionResponseBytes = 64 * 1024 - func (h *SessionHost) createAcpInteraction(ctx context.Context, request acpInteractionCreateRequest) error { + config := h.acpInteractionConfigSnapshot() body, err := json.Marshal(request) if err != nil { return fmt.Errorf("marshal ACP interaction create: %w", err) @@ -37,7 +36,7 @@ func (h *SessionHost) createAcpInteraction(ctx context.Context, request acpInter var result struct { Status string `json:"status"` } - if err := json.NewDecoder(io.LimitReader(response.Body, maxAcpInteractionResponseBytes)).Decode(&result); err != nil { + if err := json.NewDecoder(io.LimitReader(response.Body, config.ResponseMaxBytes)).Decode(&result); err != nil { return fmt.Errorf("decode ACP interaction create response: %w", err) } if response.StatusCode < 200 || response.StatusCode >= 300 || @@ -93,7 +92,7 @@ func (h *SessionHost) settleAcpInteraction(request acpInteractionSettleRequest, httpRequest.Header.Set("Content-Type", "application/json") response, sendErr := h.httpClient().Do(httpRequest) if sendErr == nil { - _, _ = io.Copy(io.Discard, io.LimitReader(response.Body, maxAcpInteractionResponseBytes)) + _, _ = io.Copy(io.Discard, io.LimitReader(response.Body, config.ResponseMaxBytes)) response.Body.Close() if response.StatusCode >= 200 && response.StatusCode < 300 { return diff --git a/packages/vm-agent/internal/acp/session_host_interactions.go b/packages/vm-agent/internal/acp/session_host_interactions.go index 5d7f9f15c2..c52fd7cf43 100644 --- a/packages/vm-agent/internal/acp/session_host_interactions.go +++ b/packages/vm-agent/internal/acp/session_host_interactions.go @@ -32,6 +32,7 @@ type AcpInteractionRuntimeConfig struct { OptionIDMaxChars int `json:"optionIdMaxChars"` OptionNameMaxChars int `json:"optionNameMaxChars"` ReceiptLimit int `json:"receiptLimit"` + ResponseMaxBytes int64 `json:"responseMaxBytes"` SettleRetryDelaysMs []int `json:"settleRetryDelaysMs"` SettleRetrySteadyMs int `json:"settleRetrySteadyMs"` } @@ -48,7 +49,8 @@ func (c AcpInteractionRuntimeConfig) validate() error { return errors.New("invalid ACP interaction deadline configuration") } if c.RequestMaxBytes <= 0 || c.OptionsMaxCount <= 0 || c.OptionIDMaxChars <= 0 || - c.OptionNameMaxChars <= 0 || c.ReceiptLimit <= 0 || c.SettleRetrySteadyMs <= 0 { + c.OptionNameMaxChars <= 0 || c.ReceiptLimit <= 0 || c.ResponseMaxBytes <= 0 || + c.SettleRetrySteadyMs <= 0 { return errors.New("invalid ACP interaction bounds") } for _, delay := range c.SettleRetryDelaysMs { diff --git a/packages/vm-agent/internal/acp/session_host_interactions_test.go b/packages/vm-agent/internal/acp/session_host_interactions_test.go index ac3b60ee76..8fcd99a572 100644 --- a/packages/vm-agent/internal/acp/session_host_interactions_test.go +++ b/packages/vm-agent/internal/acp/session_host_interactions_test.go @@ -20,6 +20,7 @@ func testInteractionConfig() AcpInteractionRuntimeConfig { PermissionDeadlineMs: 2_000, MaxDeadlineMs: 4_000, DeadlineMarginMs: 10, RequestMaxBytes: 32 * 1024, OptionsMaxCount: 16, OptionIDMaxChars: 128, OptionNameMaxChars: 200, ReceiptLimit: 2, + ResponseMaxBytes: 64 * 1024, SettleRetryDelaysMs: []int{1, 5}, SettleRetrySteadyMs: 10, } } @@ -282,6 +283,24 @@ func TestRequestPermissionFeatureOffAndCreateFailureNeverSelect(t *testing.T) { waitCreate(t, rejected) } +func TestAcpInteractionRuntimeConfigRejectsNonPositiveRuntimeBounds(t *testing.T) { + tests := map[string]func(*AcpInteractionRuntimeConfig){ + "response bytes": func(config *AcpInteractionRuntimeConfig) { config.ResponseMaxBytes = 0 }, + "retry delay": func(config *AcpInteractionRuntimeConfig) { + config.SettleRetryDelaysMs = []int{0} + }, + } + for name, mutate := range tests { + t.Run(name, func(t *testing.T) { + config := testInteractionConfig() + mutate(&config) + if err := ValidateAcpInteractionRuntimeConfig(config); err == nil { + t.Fatal("invalid runtime contract was accepted") + } + }) + } +} + func TestAcpConnectionGenerationsAreOpaqueAndUniqueAcrossHosts(t *testing.T) { first := NewSessionHost(SessionHostConfig{}) second := NewSessionHost(SessionHostConfig{}) diff --git a/packages/vm-agent/internal/server/execution_protocol_test.go b/packages/vm-agent/internal/server/execution_protocol_test.go index 8e692cd017..4ce6561418 100644 --- a/packages/vm-agent/internal/server/execution_protocol_test.go +++ b/packages/vm-agent/internal/server/execution_protocol_test.go @@ -30,6 +30,16 @@ func TestLegacyPromptRequestRemainsCompatibleWithoutDeliveryFields(t *testing.T) } } +func TestAgentStartDeliveryFingerprintFencesInteractionContractChanges(t *testing.T) { + disabled := acp.AcpInteractionRuntimeConfig{ProtocolVersion: 1} + enabled := disabled + enabled.Enabled = true + if agentStartDeliveryFingerprint(1, "message", "prompt", "instructions", disabled) == + agentStartDeliveryFingerprint(1, "message", "prompt", "instructions", enabled) { + t.Fatal("ACP interaction contract change did not change delivery fingerprint") + } +} + func TestExecutionProtocolRoutesRequireNodeManagementBearerToken(t *testing.T) { s := newContractTestServer() tests := []struct { diff --git a/packages/vm-agent/internal/server/workspaces.go b/packages/vm-agent/internal/server/workspaces.go index 98646bee6c..c70322b5bc 100644 --- a/packages/vm-agent/internal/server/workspaces.go +++ b/packages/vm-agent/internal/server/workspaces.go @@ -1370,16 +1370,14 @@ func (s *Server) handleStartAgentSession(w http.ResponseWriter, r *http.Request) writeError(w, http.StatusConflict, "workspace snapshot restore is in progress") return } - host.ConfigureAcpInteractions(body.AcpInteractions) - s.appendNodeEvent(workspaceID, "info", "agent_session.starting", "Starting agent with initial prompt", map[string]interface{}{ "sessionId": sessionID, "agentType": body.AgentType, }) if body.DeliveryID != "" { - hash := promptDeliveryFingerprint(body.ProtocolVersion, body.MessageID, - body.InitialPrompt+"\x00"+body.InjectedInstructions) + hash := agentStartDeliveryFingerprint(body.ProtocolVersion, body.MessageID, + body.InitialPrompt, body.InjectedInstructions, body.AcpInteractions) receipt, _, conflict, receiptErr := s.store.AcceptPromptDelivery(workspaceID, sessionID, body.DeliveryID, body.ProtocolVersion, hash) if receiptErr != nil { @@ -1412,6 +1410,7 @@ func (s *Server) handleStartAgentSession(w http.ResponseWriter, r *http.Request) }) return } + host.ConfigureAcpInteractions(body.AcpInteractions) observer := s.promptReceiptObserver(workspaceID, sessionID, body.DeliveryID) go s.startAgentWithPromptObserved(host, workspaceID, sessionID, body.AgentType, body.InitialPrompt, body.InjectedInstructions, body.MessageID, observer) @@ -1423,6 +1422,7 @@ func (s *Server) handleStartAgentSession(w http.ResponseWriter, r *http.Request) // Start agent and send initial prompt in a background goroutine. // The endpoint returns 202 immediately — the agent runs asynchronously. + host.ConfigureAcpInteractions(body.AcpInteractions) go s.startAgentWithPrompt(host, workspaceID, sessionID, body.AgentType, body.InitialPrompt, body.InjectedInstructions) writeJSON(w, http.StatusAccepted, map[string]interface{}{ @@ -1654,6 +1654,18 @@ func promptDeliveryFingerprint(protocolVersion int, messageID, prompt string) st return fmt.Sprintf("%x", sum[:]) } +func agentStartDeliveryFingerprint( + protocolVersion int, + messageID string, + prompt string, + injectedInstructions string, + interactions acp.AcpInteractionRuntimeConfig, +) string { + encodedInteractions, _ := json.Marshal(interactions) + return promptDeliveryFingerprint(protocolVersion, messageID, + prompt+"\x00"+injectedInstructions+"\x00"+string(encodedInteractions)) +} + type versionedPromptResponse struct { Status string `json:"status"` SessionID string `json:"sessionId"` diff --git a/specs/001-mvp/contracts/api.md b/specs/001-mvp/contracts/api.md index 3cb75e4c14..14d29d889e 100644 --- a/specs/001-mvp/contracts/api.md +++ b/specs/001-mvp/contracts/api.md @@ -43,6 +43,32 @@ The Worker includes a versioned `acpInteractions` object in every agent-session VM agent only creates permission interactions when that per-session contract is enabled; missing, disabled, invalid, or unsupported contracts cancel the ACP permission request explicitly. +The start request uses this additive runtime contract (values shown are the centralized defaults): + +```json +{ + "acpInteractions": { + "enabled": false, + "protocolVersion": 1, + "permissionDeadlineMs": 1800000, + "maxDeadlineMs": 14400000, + "deadlineMarginMs": 60000, + "requestMaxBytes": 32768, + "optionsMaxCount": 16, + "optionIdMaxChars": 128, + "optionNameMaxChars": 200, + "receiptLimit": 256, + "responseMaxBytes": 65536, + "settleRetryDelaysMs": [1000, 5000, 30000, 120000, 300000], + "settleRetrySteadyMs": 300000 + } +} +``` + +Conversation-mode sessions use the separately configured conversation deadline in the serialized +`permissionDeadlineMs` field. The global default remains disabled. The VM rejects unknown protocol +versions and invalid or incomplete enabled contracts rather than inferring local defaults. + - `POST /api/projects/:projectId/workspaces/:workspaceId/acp-interactions` creates an interaction. It requires a workspace-scoped callback JWT; project, workspace, chat-session, and running agent-session identity are resolved server-side. @@ -55,6 +81,13 @@ disabled, invalid, or unsupported contracts cancel the ACP permission request ex - `POST /api/projects/:projectId/sessions/:sessionId/interactions/:interactionId/answer` requires `task:write`, session-creator ownership, and the exact configured app Origin. The accepted decision is committed before no-wake delivery is attempted. +- `POST /workspaces/:workspaceId/agent-sessions/:sessionId/interactions/:interactionId/answer` + is the dedicated runtime answer endpoint. It requires a node-management JWT whose workspace and + node claims match the route and active runtime. Its JSON body contains `protocolVersion`, the same + UUID `interactionId` as the route, UUID `generation`, `runtimeIdentity`, a `decision` of + `selected_option`, `declined`, or `cancelled`, an exact `optionId` only for `selected_option`, and + the durable `answerHash`. It returns a structural receipt status of `consumed`, `duplicate`, + `conflict`, `stale_generation`, or `no_waiter`. Runtime create/settle routes return `404` for mismatched workspace/project/session binding, `409` for stale or conflicting state, and `410` for terminal workspaces. @@ -66,7 +99,8 @@ persists bounded option labels and structural tool metadata through the callback the durable answer, and accepts only an exact option ID. Raw tool input/content is never sent on the viewer WebSocket or copied into the interaction payload. Runtime answers use the dedicated node-management-JWT endpoint and an in-memory waiter/receipt registry; missing or stale runtimes -return `no_waiter`/`stale_generation` and are never woken or recreated to consume an answer. +are never woken or recreated to consume an answer. A live runtime with no matching registry entry +returns `no_waiter`; an absent or stopped runtime returns an HTTP error before registry lookup. ### GET /projects/:projectId/library/:fileId/preview From b3ac376fb9a8a1296b32f97a4957f0104bbba48c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 30 Sep 2026 08:19:44 +0000 Subject: [PATCH 07/13] test(acp): fence duplicate start configuration --- .../internal/acp/session_host_interactions.go | 6 +++ .../server/execution_protocol_test.go | 52 +++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/packages/vm-agent/internal/acp/session_host_interactions.go b/packages/vm-agent/internal/acp/session_host_interactions.go index c52fd7cf43..2363d8ecc8 100644 --- a/packages/vm-agent/internal/acp/session_host_interactions.go +++ b/packages/vm-agent/internal/acp/session_host_interactions.go @@ -162,6 +162,12 @@ func (h *SessionHost) ConfigureAcpInteractions(config AcpInteractionRuntimeConfi h.configureAcpInteractions(config) } +// AcpInteractionBridgeEnabled reports whether the trusted session-start +// contract enabled permission interactions for this host. +func (h *SessionHost) AcpInteractionBridgeEnabled() bool { + return h.acpInteractionConfigSnapshot().Enabled +} + func (h *SessionHost) acpInteractionConfigSnapshot() AcpInteractionRuntimeConfig { h.interactionMu.Lock() defer h.interactionMu.Unlock() diff --git a/packages/vm-agent/internal/server/execution_protocol_test.go b/packages/vm-agent/internal/server/execution_protocol_test.go index 4ce6561418..1095df199c 100644 --- a/packages/vm-agent/internal/server/execution_protocol_test.go +++ b/packages/vm-agent/internal/server/execution_protocol_test.go @@ -40,6 +40,58 @@ func TestAgentStartDeliveryFingerprintFencesInteractionContractChanges(t *testin } } +func TestStartAgentSessionDuplicateDoesNotMutateInteractionContract(t *testing.T) { + s, _ := newRestoreRetryTestServer(t) + validator, privateKey := newWorkspaceCreateJWTValidator(t, "node-test") + s.jwtValidator = validator + s.executionRuntimeID = "runtime-1" + host := acp.NewSessionHost(acp.SessionHostConfig{}) + t.Cleanup(host.Stop) + s.sessionHosts["ws:session"] = host + + interactionConfig := acp.AcpInteractionRuntimeConfig{ + Enabled: true, ProtocolVersion: 1, + PermissionDeadlineMs: 1_000, MaxDeadlineMs: 2_000, DeadlineMarginMs: 100, + RequestMaxBytes: 32 * 1024, OptionsMaxCount: 16, OptionIDMaxChars: 128, + OptionNameMaxChars: 200, ReceiptLimit: 16, ResponseMaxBytes: 64 * 1024, + SettleRetryDelaysMs: []int{10}, SettleRetrySteadyMs: 100, + } + hash := agentStartDeliveryFingerprint(1, "message-1", "prompt", "", interactionConfig) + if _, _, conflict, err := s.store.AcceptPromptDelivery("ws", "session", "delivery-1", 1, hash); err != nil || conflict { + t.Fatalf("seed prompt delivery: conflict=%v err=%v", conflict, err) + } + if _, claimed, err := s.store.ClaimPromptDelivery("ws", "session", "delivery-1", s.executionRuntimeID); err != nil || !claimed { + t.Fatalf("claim prompt delivery: claimed=%v err=%v", claimed, err) + } + + body, err := json.Marshal(map[string]any{ + "protocolVersion": 1, + "deliveryId": "delivery-1", + "messageId": "message-1", + "agentType": "claude-code", + "initialPrompt": "prompt", + "acpInteractions": interactionConfig, + }) + if err != nil { + t.Fatal(err) + } + req := httptest.NewRequest(http.MethodPost, "/workspaces/ws/agent-sessions/session/start", strings.NewReader(string(body))) + req.SetPathValue("workspaceId", "ws") + req.SetPathValue("sessionId", "session") + req.Header.Set("Authorization", "Bearer "+signWorkspaceCreateNodeToken(t, privateKey, "node-test", "ws")) + req.Header.Set("X-SAM-Node-Id", "node-test") + req.Header.Set("X-SAM-Workspace-Id", "ws") + rec := httptest.NewRecorder() + s.handleStartAgentSession(rec, req) + + if rec.Code != http.StatusOK || !strings.Contains(rec.Body.String(), `"status":"duplicate"`) { + t.Fatalf("duplicate response = %d %s", rec.Code, rec.Body.String()) + } + if host.AcpInteractionBridgeEnabled() { + t.Fatal("duplicate delivery mutated the live SessionHost interaction contract") + } +} + func TestExecutionProtocolRoutesRequireNodeManagementBearerToken(t *testing.T) { s := newContractTestServer() tests := []struct { From ea3dbad82a6aeac4e668176227ad0a1bd8952905 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 30 Sep 2026 08:25:33 +0000 Subject: [PATCH 08/13] test(acp): prove permission failures stay closed --- .../acp/session_host_interactions_test.go | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/packages/vm-agent/internal/acp/session_host_interactions_test.go b/packages/vm-agent/internal/acp/session_host_interactions_test.go index 8fcd99a572..089b0f1d76 100644 --- a/packages/vm-agent/internal/acp/session_host_interactions_test.go +++ b/packages/vm-agent/internal/acp/session_host_interactions_test.go @@ -283,6 +283,74 @@ func TestRequestPermissionFeatureOffAndCreateFailureNeverSelect(t *testing.T) { waitCreate(t, rejected) } +func TestRequestPermissionInvalidContractsAndRequestsFailClosedBeforeCreate(t *testing.T) { + tests := map[string]func(*AcpInteractionRuntimeConfig, *acpsdk.RequestPermissionRequest, *string){ + "missing contract": func(config *AcpInteractionRuntimeConfig, _ *acpsdk.RequestPermissionRequest, _ *string) { + *config = AcpInteractionRuntimeConfig{} + }, + "protocol version skew": func(config *AcpInteractionRuntimeConfig, _ *acpsdk.RequestPermissionRequest, _ *string) { + config.ProtocolVersion++ + }, + "missing generation": func(_ *AcpInteractionRuntimeConfig, _ *acpsdk.RequestPermissionRequest, generation *string) { + *generation = "" + }, + "unsupported option kind": func(_ *AcpInteractionRuntimeConfig, request *acpsdk.RequestPermissionRequest, _ *string) { + request.Options[0].Kind = acpsdk.PermissionOptionKind("unsupported") + }, + "empty options": func(_ *AcpInteractionRuntimeConfig, request *acpsdk.RequestPermissionRequest, _ *string) { + request.Options = nil + }, + "too many options": func(config *AcpInteractionRuntimeConfig, _ *acpsdk.RequestPermissionRequest, _ *string) { + config.OptionsMaxCount = 1 + }, + "oversized option id": func(config *AcpInteractionRuntimeConfig, _ *acpsdk.RequestPermissionRequest, _ *string) { + config.OptionIDMaxChars = 3 + }, + "oversized option name": func(config *AcpInteractionRuntimeConfig, _ *acpsdk.RequestPermissionRequest, _ *string) { + config.OptionNameMaxChars = 3 + }, + "duplicate option ids": func(_ *AcpInteractionRuntimeConfig, request *acpsdk.RequestPermissionRequest, _ *string) { + request.Options[1].OptionId = request.Options[0].OptionId + }, + "oversized encoded detail": func(config *AcpInteractionRuntimeConfig, _ *acpsdk.RequestPermissionRequest, _ *string) { + config.RequestMaxBytes = 1 + }, + } + + for name, mutate := range tests { + t.Run(name, func(t *testing.T) { + recorder := newInteractionRecorder(t, http.StatusCreated) + host := NewSessionHost(SessionHostConfig{GatewayConfig: GatewayConfig{ + ControlPlaneURL: recorder.server.URL, + ProjectID: "project-1", + WorkspaceID: "workspace-1", + SessionID: "agent-session-1", + RuntimeIdentity: "runtime-1", + CallbackToken: "callback-token", + HTTPClient: recorder.server.Client(), + }}) + t.Cleanup(host.Stop) + config := testInteractionConfig() + request := permissionRequest() + generation := host.attachAcpInteractionGeneration() + mutate(&config, &request, &generation) + host.ConfigureAcpInteractions(config) + + response, err := (&sessionHostClient{host: host, interactionGeneration: generation}).RequestPermission( + context.Background(), request, + ) + if err != nil || response.Outcome.Cancelled == nil || response.Outcome.Selected != nil { + t.Fatalf("response = %+v err=%v", response, err) + } + select { + case created := <-recorder.creates: + t.Fatalf("fail-closed path created durable interaction: %+v", created) + default: + } + }) + } +} + func TestAcpInteractionRuntimeConfigRejectsNonPositiveRuntimeBounds(t *testing.T) { tests := map[string]func(*AcpInteractionRuntimeConfig){ "response bytes": func(config *AcpInteractionRuntimeConfig) { config.ResponseMaxBytes = 0 }, From 22647ccd6c2512aa8ad64a288fe7f55225f2ca0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 30 Sep 2026 08:29:04 +0000 Subject: [PATCH 09/13] docs(task): record ACP bridge delivery --- .../2026-09-30-acp-runtime-permission-bridge.md | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tasks/active/2026-09-30-acp-runtime-permission-bridge.md b/tasks/active/2026-09-30-acp-runtime-permission-bridge.md index 61220b6a42..f48d04f021 100644 --- a/tasks/active/2026-09-30-acp-runtime-permission-bridge.md +++ b/tasks/active/2026-09-30-acp-runtime-permission-bridge.md @@ -39,8 +39,8 @@ Slice B connects ACP `RequestPermission` to the shipped Cloudflare create/answer - [x] Add deterministic real ACP fixture behavior with reversed safety options for the coordinator's final staged roundtrip. - [x] Add contract and race tests for callback JWT workspace/session identity, recreated `SessionHost` generation, reversed options, duplicate/conflicting answer, process loss, explicit Stop, deadlines/cancellation, feature-off/unsupported behavior, and VM/Instant no-wake transport. - [x] Update narrow API/runtime contract documentation and fixtures without claiming unproven form/URL capability. -- [ ] Run package and repository validation, task-completion validation, and relevant Go, Cloudflare, security, constitution, test, and documentation reviews. -- [ ] Open an implementation-ready draft PR and record exact branch/contracts/test/review evidence for the coordinator. +- [x] Run package and repository validation, task-completion validation, and relevant Go, Cloudflare, security, constitution, test, and documentation reviews. +- [x] Open an implementation-ready draft PR and record exact branch/contracts/test/review evidence for the coordinator. ## Acceptance Criteria @@ -68,3 +68,10 @@ Slice B connects ACP `RequestPermission` to the shipped Cloudflare create/answer - `.claude/rules/34-vm-agent-callback-auth.md` - `packages/vm-agent/.claude/rules/54-vm-agent-rollout-compatibility.md` - `packages/vm-agent/.claude/rules/71-request-context-must-not-outlive-its-request.md` + +## Delivery + +- Branch: `sam/execute-task-using-skill-zafwb6` +- Draft PR: `#2201` +- Final implementation commit: `ea3dbad82` +- Integrated staging: explicitly deferred to coordinator before readiness or merge From d7439c91b305cc1f3dc1d18f0b97c4ba40e0e104 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 30 Sep 2026 09:01:45 +0000 Subject: [PATCH 10/13] test(acp): advertise permission bridge in vertical fixture --- .../tests/workers/acp-interaction-vertical-slice.test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/apps/api/tests/workers/acp-interaction-vertical-slice.test.ts b/apps/api/tests/workers/acp-interaction-vertical-slice.test.ts index f3a7e7f60f..c520786adf 100644 --- a/apps/api/tests/workers/acp-interaction-vertical-slice.test.ts +++ b/apps/api/tests/workers/acp-interaction-vertical-slice.test.ts @@ -143,6 +143,12 @@ describe('ACP interaction cross-boundary vertical slice', () => { lookup: true, states: ['accepted', 'in_flight', 'completed', 'not_found', 'ambiguous'], }, + interactions: { + supported: true, + version: 1, + answerEndpoint: true, + permissionBridge: true, + }, checkpointRollover: { supported: true, automatic: false, From d3e2b3f8c182bd1cfb76da826fc19277012bafbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 30 Sep 2026 10:07:25 +0000 Subject: [PATCH 11/13] fix(acp): bind permission bridge lifecycle --- .../src/durable-objects/vm-agent-container.ts | 12 +- .../vm-agent-container-wake-state.test.ts | 81 ++++++- .../internal/acp/session_host_attempt.go | 4 + .../acp/session_host_interaction_transport.go | 29 ++- .../internal/acp/session_host_interactions.go | 115 +++++++--- .../acp/session_host_interactions_sdk_test.go | 205 ++++++++++++++++++ .../acp/session_host_interactions_test.go | 194 ++++++++++++++++- .../internal/acp/session_host_prompt.go | 2 +- .../internal/acp/session_host_prompt_state.go | 14 +- specs/001-mvp/contracts/api.md | 15 +- ...026-09-30-acp-runtime-permission-bridge.md | 26 +++ 11 files changed, 641 insertions(+), 56 deletions(-) create mode 100644 packages/vm-agent/internal/acp/session_host_interactions_sdk_test.go diff --git a/apps/api/src/durable-objects/vm-agent-container.ts b/apps/api/src/durable-objects/vm-agent-container.ts index 145c5cfbc7..71a1a974d9 100644 --- a/apps/api/src/durable-objects/vm-agent-container.ts +++ b/apps/api/src/durable-objects/vm-agent-container.ts @@ -175,8 +175,18 @@ export class VmAgentContainer extends Container { if (status !== 'running') { return recoveryResponse('RUNTIME_STOPPED', RUNTIME_STOPPED_MESSAGE, 410); } + const container = this.ctx.container; + if (!container?.running) { + return recoveryResponse('RUNTIME_STOPPED', RUNTIME_STOPPED_MESSAGE, 410); + } try { - return await this.containerFetch(request, port ?? this.defaultPort); + // Container.containerFetch() is intentionally forbidden here: the pinned + // SDK starts compute when either the real runtime is stopped or its own + // persisted health state is stale. The direct port primitive never starts + // a container; if stop/crash wins after the running check, fetch rejects. + const tcpPort = container.getTcpPort(port ?? this.defaultPort); + const containerUrl = request.url.replace('https:', 'http:'); + return await tcpPort.fetch(containerUrl, request); } catch { return interruptedRequestResponse(request); } diff --git a/apps/api/tests/unit/durable-objects/vm-agent-container-wake-state.test.ts b/apps/api/tests/unit/durable-objects/vm-agent-container-wake-state.test.ts index 870700cbc9..2cfae48d17 100644 --- a/apps/api/tests/unit/durable-objects/vm-agent-container-wake-state.test.ts +++ b/apps/api/tests/unit/durable-objects/vm-agent-container-wake-state.test.ts @@ -86,7 +86,10 @@ describe('VmAgentContainer proxy recovery boundaries', () => { const beginUnexpectedRecovery = vi.fn(); const response = await callProxyHttpNoWake( { - ctx: { storage: { get: vi.fn().mockResolvedValue(status) } }, + ctx: { + storage: { get: vi.fn().mockResolvedValue(status) }, + container: { running: false }, + }, defaultPort: 8080, containerFetch, ensureAwake, @@ -102,19 +105,85 @@ describe('VmAgentContainer proxy recovery boundaries', () => { } ); - it('forwards a no-wake request only when the runtime is already running', async () => { - const containerFetch = vi.fn().mockResolvedValue(new Response('live')); + it.each([ + ['capability', new Request('https://container/capabilities')], + ['answer', new Request('https://container/interactions/id/answer', { method: 'POST' })], + ] as const)( + 'never starts a stale persisted-running container for the %s path', + async (_path, request) => { + const start = vi.fn(); + const getTcpPort = vi.fn(); + const containerFetch = vi.fn(); + const response = await callProxyHttpNoWake( + { + ctx: { + storage: { get: vi.fn().mockResolvedValue('running') }, + container: { running: false, start, getTcpPort }, + }, + defaultPort: 8080, + containerFetch, + }, + request + ); + + expect(response.status).toBe(410); + expect(getTcpPort).not.toHaveBeenCalled(); + expect(containerFetch).not.toHaveBeenCalled(); + expect(start).not.toHaveBeenCalled(); + } + ); + + it.each([ + ['capability', new Request('https://container/capabilities'), 503], + ['answer', new Request('https://container/interactions/id/answer', { method: 'POST' }), 409], + ] as const)( + 'does not restart when stop wins at the %s forwarding boundary', + async (_path, request, expectedStatus) => { + const start = vi.fn(); + const directFetch = vi.fn().mockRejectedValue(new Error('container stopped')); + const getTcpPort = vi.fn(() => ({ fetch: directFetch })); + const containerFetch = vi.fn(); + const response = await callProxyHttpNoWake( + { + ctx: { + storage: { get: vi.fn().mockResolvedValue('running') }, + container: { running: true, start, getTcpPort }, + }, + defaultPort: 8080, + containerFetch, + }, + request + ); + + expect(response.status).toBe(expectedStatus); + expect(getTcpPort).toHaveBeenCalledWith(8080); + expect(directFetch).toHaveBeenCalledWith(request.url.replace('https:', 'http:'), request); + expect(containerFetch).not.toHaveBeenCalled(); + expect(start).not.toHaveBeenCalled(); + } + ); + + it('forwards a no-wake request only through the already-running container port', async () => { + const directFetch = vi.fn().mockResolvedValue(new Response('live')); + const getTcpPort = vi.fn(() => ({ fetch: directFetch })); + const containerFetch = vi.fn(); + const request = new Request('https://container/capabilities'); const response = await callProxyHttpNoWake( { - ctx: { storage: { get: vi.fn().mockResolvedValue('running') } }, + ctx: { + storage: { get: vi.fn().mockResolvedValue('running') }, + container: { running: true, getTcpPort }, + }, defaultPort: 8080, containerFetch, }, - new Request('http://container/capabilities') + request ); expect(await response.text()).toBe('live'); - expect(containerFetch).toHaveBeenCalledOnce(); + expect(getTcpPort).toHaveBeenCalledWith(8080); + expect(directFetch).toHaveBeenCalledWith('http://container/capabilities', request); + expect(containerFetch).not.toHaveBeenCalled(); }); it('rejects a revoked source guard before proxy preparation can wake compute', async () => { const proxyHttp = vi.fn(); diff --git a/packages/vm-agent/internal/acp/session_host_attempt.go b/packages/vm-agent/internal/acp/session_host_attempt.go index da639dfb5f..e06eb0306f 100644 --- a/packages/vm-agent/internal/acp/session_host_attempt.go +++ b/packages/vm-agent/internal/acp/session_host_attempt.go @@ -14,6 +14,10 @@ type PromptTerminalObserver func(stopReason string, promptErr error) type promptAttempt struct { id uint64 + // ctx is the exact outgoing Prompt attempt lifetime. The ACP SDK creates + // inbound RequestPermission contexts from the connection instead, so nested + // permission work must explicitly inherit this context. + ctx context.Context // deliveryID is the control-plane prompt delivery that created this // attempt, empty for viewer prompts. Immutable after beginPrompt. deliveryID string diff --git a/packages/vm-agent/internal/acp/session_host_interaction_transport.go b/packages/vm-agent/internal/acp/session_host_interaction_transport.go index 373f623506..1d70dbc4e5 100644 --- a/packages/vm-agent/internal/acp/session_host_interaction_transport.go +++ b/packages/vm-agent/internal/acp/session_host_interaction_transport.go @@ -13,37 +13,48 @@ import ( "time" ) -func (h *SessionHost) createAcpInteraction(ctx context.Context, request acpInteractionCreateRequest) error { +type acpInteractionCreateOutcome string + +const ( + acpInteractionCreateAcknowledged acpInteractionCreateOutcome = "acknowledged" + acpInteractionCreateRejected acpInteractionCreateOutcome = "rejected" + acpInteractionCreateUnknown acpInteractionCreateOutcome = "unknown" +) + +func (h *SessionHost) createAcpInteraction(ctx context.Context, request acpInteractionCreateRequest) (acpInteractionCreateOutcome, error) { config := h.acpInteractionConfigSnapshot() body, err := json.Marshal(request) if err != nil { - return fmt.Errorf("marshal ACP interaction create: %w", err) + return acpInteractionCreateRejected, fmt.Errorf("marshal ACP interaction create: %w", err) } endpoint := strings.TrimRight(h.config.ControlPlaneURL, "/") + "/api/projects/" + url.PathEscape(h.config.ProjectID) + "/workspaces/" + url.PathEscape(h.config.WorkspaceID) + "/acp-interactions" httpRequest, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(body)) if err != nil { - return fmt.Errorf("build ACP interaction create: %w", err) + return acpInteractionCreateRejected, fmt.Errorf("build ACP interaction create: %w", err) } httpRequest.Header.Set("Authorization", "Bearer "+h.config.CallbackToken) httpRequest.Header.Set("Content-Type", "application/json") response, err := h.httpClient().Do(httpRequest) if err != nil { - return fmt.Errorf("send ACP interaction create: %w", err) + return acpInteractionCreateUnknown, fmt.Errorf("send ACP interaction create: %w", err) } defer response.Body.Close() + if response.StatusCode < 200 || response.StatusCode >= 300 { + _, _ = io.Copy(io.Discard, io.LimitReader(response.Body, config.ResponseMaxBytes)) + return acpInteractionCreateRejected, fmt.Errorf("ACP interaction create rejected with status %d", response.StatusCode) + } var result struct { Status string `json:"status"` } if err := json.NewDecoder(io.LimitReader(response.Body, config.ResponseMaxBytes)).Decode(&result); err != nil { - return fmt.Errorf("decode ACP interaction create response: %w", err) + return acpInteractionCreateUnknown, fmt.Errorf("decode ACP interaction create response: %w", err) } - if response.StatusCode < 200 || response.StatusCode >= 300 || - (result.Status != "created" && result.Status != "existing") { - return fmt.Errorf("ACP interaction create rejected with status %d (%s)", response.StatusCode, result.Status) + if result.Status != "created" && result.Status != "existing" { + return acpInteractionCreateUnknown, fmt.Errorf("ACP interaction create returned unknown status %q", result.Status) } - return nil + return acpInteractionCreateAcknowledged, nil } func (h *SessionHost) settleAcpInteraction(request acpInteractionSettleRequest, deadline time.Time) { diff --git a/packages/vm-agent/internal/acp/session_host_interactions.go b/packages/vm-agent/internal/acp/session_host_interactions.go index 2363d8ecc8..87b7f20ee1 100644 --- a/packages/vm-agent/internal/acp/session_host_interactions.go +++ b/packages/vm-agent/internal/acp/session_host_interactions.go @@ -140,9 +140,11 @@ type acpInteractionWaitResult struct { } type acpInteractionWaiter struct { - generation string - options map[string]struct{} - result chan acpInteractionWaitResult + generation string + attemptID uint64 + options map[string]struct{} + result chan acpInteractionWaitResult + cancelRequest context.CancelFunc } type acpInteractionReceipt struct { @@ -150,6 +152,11 @@ type acpInteractionReceipt struct { decisionHash string } +type acpInteractionCreateResult struct { + outcome acpInteractionCreateOutcome + err error +} + func (h *SessionHost) configureAcpInteractions(config AcpInteractionRuntimeConfig) { h.interactionMu.Lock() defer h.interactionMu.Unlock() @@ -194,6 +201,7 @@ func (h *SessionHost) cancelInteractionWaiters(reason string) { func (h *SessionHost) cancelInteractionWaitersLocked(reason string) { for interactionID, waiter := range h.interactionWaiters { delete(h.interactionWaiters, interactionID) + waiter.cancelRequest() waiter.result <- acpInteractionWaitResult{cancel: true, reason: reason} } } @@ -201,7 +209,9 @@ func (h *SessionHost) cancelInteractionWaitersLocked(reason string) { func (h *SessionHost) registerInteractionWaiter( interactionID string, generation string, + attemptID uint64, options map[string]struct{}, + cancelRequest context.CancelFunc, ) (*acpInteractionWaiter, error) { h.interactionMu.Lock() defer h.interactionMu.Unlock() @@ -212,9 +222,11 @@ func (h *SessionHost) registerInteractionWaiter( return nil, errors.New("ACP interaction waiter already exists") } waiter := &acpInteractionWaiter{ - generation: generation, - options: options, - result: make(chan acpInteractionWaitResult, 1), + generation: generation, + attemptID: attemptID, + options: options, + result: make(chan acpInteractionWaitResult, 1), + cancelRequest: cancelRequest, } h.interactionWaiters[interactionID] = waiter return waiter, nil @@ -230,6 +242,25 @@ func (h *SessionHost) cancelAcpInteractionWaiter(interactionID, generation, reas return false } delete(h.interactionWaiters, interactionID) + waiter.cancelRequest() + waiter.result <- acpInteractionWaitResult{cancel: true, reason: reason} + return true +} + +func (h *SessionHost) cancelAcpInteractionWaiterForAttempt( + interactionID string, + generation string, + attemptID uint64, + reason string, +) bool { + h.interactionMu.Lock() + defer h.interactionMu.Unlock() + waiter, ok := h.interactionWaiters[interactionID] + if !ok || waiter.generation != generation || waiter.attemptID != attemptID { + return false + } + delete(h.interactionWaiters, interactionID) + waiter.cancelRequest() waiter.result <- acpInteractionWaitResult{cancel: true, reason: reason} return true } @@ -302,6 +333,7 @@ func (h *SessionHost) ResolveAcpInteractionAnswer( return "conflict" } delete(h.interactionWaiters, interactionID) + waiter.cancelRequest() h.rememberInteractionReceiptLocked(interactionID, acpInteractionReceipt{ generation: generation, decisionHash: decisionHash, }) @@ -309,7 +341,7 @@ func (h *SessionHost) ResolveAcpInteractionAnswer( return "consumed" } -func (h *SessionHost) permissionDeadline(ctx context.Context, config AcpInteractionRuntimeConfig) (time.Time, bool) { +func (h *SessionHost) permissionDeadline(promptCtx context.Context, config AcpInteractionRuntimeConfig) (time.Time, bool) { now := h.now() duration := time.Duration(config.PermissionDeadlineMs) * time.Millisecond maxDuration := time.Duration(config.MaxDeadlineMs) * time.Millisecond @@ -317,7 +349,7 @@ func (h *SessionHost) permissionDeadline(ctx context.Context, config AcpInteract duration = maxDuration } deadline := now.Add(duration) - if promptDeadline, ok := ctx.Deadline(); ok { + if promptDeadline, ok := promptCtx.Deadline(); ok { promptDeadline = promptDeadline.Add(-time.Duration(config.DeadlineMarginMs) * time.Millisecond) if !promptDeadline.After(now) { return time.Time{}, false @@ -380,7 +412,7 @@ func cancelledPermissionResponse() acpsdk.RequestPermissionResponse { } func (h *SessionHost) requestPermission( - ctx context.Context, + inboundCtx context.Context, generation string, params acpsdk.RequestPermissionRequest, ) (acpsdk.RequestPermissionResponse, error) { @@ -391,7 +423,12 @@ func (h *SessionHost) requestPermission( slog.Info("acp_interaction.permission_cancelled", "reason", "unsupported") return cancelledPermissionResponse(), nil } - deadline, ok := h.permissionDeadline(ctx, config) + attempt, ok := h.activePromptAttempt() + if !ok { + slog.Info("acp_interaction.permission_cancelled", "reason", "prompt_unavailable") + return cancelledPermissionResponse(), nil + } + deadline, ok := h.permissionDeadline(attempt.ctx, config) if !ok { slog.Info("acp_interaction.permission_cancelled", "reason", "deadline_elapsed") return cancelledPermissionResponse(), nil @@ -421,7 +458,13 @@ func (h *SessionHost) requestPermission( } payloadHash := sha256.Sum256(canonical) request.PayloadHash = hex.EncodeToString(payloadHash[:]) - waiter, err := h.registerInteractionWaiter(interactionID, generation, optionIDs) + requestCtx, cancelRequest := context.WithDeadline(attempt.ctx, deadline) + stopInboundCancel := context.AfterFunc(inboundCtx, cancelRequest) + defer stopInboundCancel() + defer cancelRequest() + waiter, err := h.registerInteractionWaiter( + interactionID, generation, attempt.id, optionIDs, cancelRequest, + ) if err != nil { slog.Info("acp_interaction.permission_cancelled", "reason", "generation_unavailable") return cancelledPermissionResponse(), nil @@ -436,25 +479,43 @@ func (h *SessionHost) requestPermission( Reason: reason, }, deadline) } - if err := h.createAcpInteraction(ctx, request); err != nil { - h.cancelAcpInteractionWaiter(interactionID, generation, "wrapper_cancelled") - settle("wrapper_cancelled") - slog.Warn("acp_interaction.create_failed", "interactionId", interactionID, - "reason", "control_plane_rejected") - return cancelledPermissionResponse(), nil - } - - timer := time.NewTimer(deadline.Sub(h.now())) - defer timer.Stop() var result acpInteractionWaitResult + var createResult acpInteractionCreateResult + createDone := make(chan acpInteractionCreateResult, 1) + go func() { + outcome, err := h.createAcpInteraction(requestCtx, request) + createDone <- acpInteractionCreateResult{outcome: outcome, err: err} + }() + waitForResult := func() acpInteractionWaitResult { + select { + case waiterResult := <-waiter.result: + return waiterResult + case <-requestCtx.Done(): + reason := "wrapper_cancelled" + if errors.Is(requestCtx.Err(), context.DeadlineExceeded) { + reason = "expired" + } + h.cancelAcpInteractionWaiterForAttempt(interactionID, generation, attempt.id, reason) + return <-waiter.result + } + } select { case result = <-waiter.result: - case <-ctx.Done(): - h.cancelAcpInteractionWaiter(interactionID, generation, "wrapper_cancelled") - result = <-waiter.result - case <-timer.C: - h.cancelAcpInteractionWaiter(interactionID, generation, "expired") - result = <-waiter.result + case createResult = <-createDone: + if createResult.outcome == acpInteractionCreateRejected { + h.cancelAcpInteractionWaiter(interactionID, generation, "wrapper_cancelled") + } + result = waitForResult() + case <-requestCtx.Done(): + result = waitForResult() + } + if createResult.err != nil { + if result.cancel { + slog.Warn("acp_interaction.create_failed", "interactionId", interactionID, + "outcome", createResult.outcome, "reason", "control_plane_unconfirmed") + } else { + slog.Info("acp_interaction.create_ack_unconfirmed_answered", "interactionId", interactionID) + } } if result.cancel { reason := result.reason diff --git a/packages/vm-agent/internal/acp/session_host_interactions_sdk_test.go b/packages/vm-agent/internal/acp/session_host_interactions_sdk_test.go new file mode 100644 index 0000000000..e13f893aed --- /dev/null +++ b/packages/vm-agent/internal/acp/session_host_interactions_sdk_test.go @@ -0,0 +1,205 @@ +package acp + +import ( + "context" + "io" + "net/http" + "testing" + "time" + + acpsdk "github.com/coder/acp-go-sdk" +) + +type sdkPermissionContextObservation struct { + hasDeadline bool + err error +} + +type sdkPermissionObservingClient struct { + *sessionHostClient + observed chan sdkPermissionContextObservation +} + +func (c *sdkPermissionObservingClient) RequestPermission( + ctx context.Context, + params acpsdk.RequestPermissionRequest, +) (acpsdk.RequestPermissionResponse, error) { + _, hasDeadline := ctx.Deadline() + c.observed <- sdkPermissionContextObservation{hasDeadline: hasDeadline, err: ctx.Err()} + return c.sessionHostClient.RequestPermission(ctx, params) +} + +type sdkPermissionFixtureAgent struct { + acpsdk.Agent + conn *acpsdk.AgentSideConnection +} + +func (a *sdkPermissionFixtureAgent) Initialize( + context.Context, + acpsdk.InitializeRequest, +) (acpsdk.InitializeResponse, error) { + return acpsdk.InitializeResponse{ + ProtocolVersion: acpsdk.ProtocolVersionNumber, + AgentCapabilities: acpsdk.AgentCapabilities{}, + }, nil +} + +func (a *sdkPermissionFixtureAgent) Cancel(context.Context, acpsdk.CancelNotification) error { + return nil +} + +func (a *sdkPermissionFixtureAgent) Prompt( + ctx context.Context, + params acpsdk.PromptRequest, +) (acpsdk.PromptResponse, error) { + request := permissionRequest() + request.SessionId = params.SessionId + _, _ = a.conn.RequestPermission(ctx, request) + return acpsdk.PromptResponse{StopReason: acpsdk.StopReasonEndTurn}, nil +} + +type sdkPermissionFixture struct { + host *SessionHost + clientConn *acpsdk.ClientSideConnection + observed chan sdkPermissionContextObservation +} + +func newSDKPermissionFixture(t *testing.T, recorder *interactionRecorder) *sdkPermissionFixture { + t.Helper() + host := NewSessionHost(SessionHostConfig{GatewayConfig: GatewayConfig{ + ControlPlaneURL: recorder.server.URL, + ProjectID: "project-1", + WorkspaceID: "workspace-1", + SessionID: "agent-session-1", + RuntimeIdentity: "runtime-1", + CallbackToken: "callback-token", + HTTPClient: recorder.server.Client(), + }}) + host.ConfigureAcpInteractions(testInteractionConfig()) + generation := host.attachAcpInteractionGeneration() + + clientToAgentReader, clientToAgentWriter := io.Pipe() + agentToClientReader, agentToClientWriter := io.Pipe() + t.Cleanup(func() { + host.Stop() + _ = clientToAgentReader.Close() + _ = clientToAgentWriter.Close() + _ = agentToClientReader.Close() + _ = agentToClientWriter.Close() + }) + + agent := &sdkPermissionFixtureAgent{} + agentConn := acpsdk.NewAgentSideConnection(agent, agentToClientWriter, clientToAgentReader) + agent.conn = agentConn + observed := make(chan sdkPermissionContextObservation, 4) + client := &sdkPermissionObservingClient{ + sessionHostClient: &sessionHostClient{host: host, interactionGeneration: generation}, + observed: observed, + } + return &sdkPermissionFixture{ + host: host, + clientConn: acpsdk.NewClientSideConnection(client, clientToAgentWriter, agentToClientReader), + observed: observed, + } +} + +func (f *sdkPermissionFixture) startPrompt( + t *testing.T, + ctx context.Context, + cancel context.CancelFunc, + deliveryID string, +) (*promptAttempt, <-chan error) { + t.Helper() + attempt, ok := f.host.beginPromptForDelivery(ctx, cancel, deliveryID, nil) + if !ok { + t.Fatal("SDK fixture prompt was not accepted") + } + done := make(chan error, 1) + go func() { + _, err := f.clientConn.Prompt(ctx, acpsdk.PromptRequest{ + SessionId: "sdk-session", + Prompt: []acpsdk.ContentBlock{acpsdk.TextBlock(deliveryID)}, + }) + attempt.complete(f.host, "fixture_complete", err) + done <- err + }() + return attempt, done +} + +func TestSDKPermissionUsesPromptAttemptDeadlineAndKeepsConnectionAlive(t *testing.T) { + recorder := newInteractionRecorder(t, http.StatusCreated) + fixture := newSDKPermissionFixture(t, recorder) + promptCtx, promptCancel := context.WithTimeout(fixture.host.lifecycleContext(), 80*time.Millisecond) + defer promptCancel() + _, promptDone := fixture.startPrompt(t, promptCtx, promptCancel, "deadline-attempt") + created := waitCreate(t, recorder) + + observation := <-fixture.observed + if observation.hasDeadline || observation.err != nil { + t.Fatalf("SDK inbound permission context = %+v, want live context without prompt deadline", observation) + } + if settle := waitSettle(t, recorder); settle.InteractionID != created.InteractionID || settle.Reason != "expired" { + t.Fatalf("deadline settle = %+v, want matching expired permission", settle) + } + select { + case <-promptDone: + case <-time.After(time.Second): + t.Fatal("prompt deadline did not close the SDK prompt") + } + + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + if _, err := fixture.clientConn.Initialize(ctx, acpsdk.InitializeRequest{ProtocolVersion: acpsdk.ProtocolVersionNumber}); err != nil { + t.Fatalf("SDK connection did not survive permission deadline: %v", err) + } +} + +func TestSDKPermissionCancelIsAttemptScopedAcrossNextPrompt(t *testing.T) { + recorder := newInteractionRecorder(t, http.StatusCreated) + fixture := newSDKPermissionFixture(t, recorder) + + firstCtx, firstCancel := context.WithCancel(fixture.host.lifecycleContext()) + firstAttempt, firstDone := fixture.startPrompt(t, firstCtx, firstCancel, "cancelled-attempt") + first := waitCreate(t, recorder) + <-fixture.observed + fixture.host.CancelPrompt() + if settle := waitSettle(t, recorder); settle.InteractionID != first.InteractionID || settle.Reason != "wrapper_cancelled" { + t.Fatalf("cancel settle = %+v, want matching wrapper_cancelled permission", settle) + } + select { + case <-firstDone: + case <-time.After(time.Second): + t.Fatal("prompt cancel did not close the first SDK prompt") + } + + secondCtx, secondCancel := context.WithCancel(fixture.host.lifecycleContext()) + defer secondCancel() + _, secondDone := fixture.startPrompt(t, secondCtx, secondCancel, "next-attempt") + second := waitCreate(t, recorder) + <-fixture.observed + if fixture.host.cancelAcpInteractionWaiterForAttempt( + second.InteractionID, + second.Generation, + firstAttempt.id, + "wrapper_cancelled", + ) { + t.Fatal("stale first-attempt cancellation claimed the next permission") + } + decision := AcpInteractionAnswerDecision{ + Kind: "selected_option", OptionID: "allow", AnswerHash: "second-answer", + } + if status := fixture.host.ResolveAcpInteractionAnswer(second.InteractionID, second.Generation, decision); status != "consumed" { + t.Fatalf("second answer receipt = %q, want consumed", status) + } + if settle := waitSettle(t, recorder); settle.InteractionID != second.InteractionID || settle.Reason != "completed" { + t.Fatalf("second settle = %+v, want matching completed permission", settle) + } + select { + case err := <-secondDone: + if err != nil { + t.Fatalf("next SDK prompt failed after stale cancellation: %v", err) + } + case <-time.After(time.Second): + t.Fatal("next SDK prompt did not complete") + } +} diff --git a/packages/vm-agent/internal/acp/session_host_interactions_test.go b/packages/vm-agent/internal/acp/session_host_interactions_test.go index 089b0f1d76..7afc96a527 100644 --- a/packages/vm-agent/internal/acp/session_host_interactions_test.go +++ b/packages/vm-agent/internal/acp/session_host_interactions_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "net/http" "net/http/httptest" "strings" @@ -14,6 +15,12 @@ import ( acpsdk "github.com/coder/acp-go-sdk" ) +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(request *http.Request) (*http.Response, error) { + return f(request) +} + func testInteractionConfig() AcpInteractionRuntimeConfig { return AcpInteractionRuntimeConfig{ Enabled: true, ProtocolVersion: acpInteractionProtocolVersion, @@ -44,9 +51,10 @@ func permissionRequest() acpsdk.RequestPermissionRequest { } type interactionRecorder struct { - server *httptest.Server - creates chan acpInteractionCreateRequest - settles chan acpInteractionSettleRequest + server *httptest.Server + creates chan acpInteractionCreateRequest + settles chan acpInteractionSettleRequest + createBlock <-chan struct{} } func newInteractionRecorder(t *testing.T, createStatus int) *interactionRecorder { @@ -67,6 +75,13 @@ func newInteractionRecorder(t *testing.T, createStatus int) *interactionRecorder t.Errorf("decode create: %v", err) } recorder.creates <- request + if recorder.createBlock != nil { + select { + case <-recorder.createBlock: + case <-r.Context().Done(): + return + } + } w.Header().Set("Content-Type", "application/json") w.WriteHeader(createStatus) status := "created" @@ -104,9 +119,66 @@ func newInteractionHost(recorder *interactionRecorder) (*SessionHost, *sessionHo }}) host.ConfigureAcpInteractions(testInteractionConfig()) generation := host.attachAcpInteractionGeneration() + promptCtx, promptCancel := context.WithCancel(host.lifecycleContext()) + if _, ok := host.beginPromptForDelivery(promptCtx, promptCancel, "test-prompt", nil); !ok { + panic("test prompt was not accepted") + } return host, &sessionHostClient{host: host, interactionGeneration: generation} } +func newBlockedCreateInteractionHost(t *testing.T) ( + *SessionHost, + *sessionHostClient, + <-chan acpInteractionCreateRequest, + <-chan acpInteractionSettleRequest, + chan struct{}, +) { + t.Helper() + creates := make(chan acpInteractionCreateRequest, 1) + settles := make(chan acpInteractionSettleRequest, 1) + releaseCreate := make(chan struct{}) + httpClient := &http.Client{Transport: roundTripFunc(func(request *http.Request) (*http.Response, error) { + if strings.HasSuffix(request.URL.Path, "/settle") { + var settle acpInteractionSettleRequest + if err := json.NewDecoder(request.Body).Decode(&settle); err != nil { + t.Errorf("decode settle: %v", err) + } + settles <- settle + return &http.Response{ + StatusCode: http.StatusOK, + Body: http.NoBody, + Header: make(http.Header), + }, nil + } + var create acpInteractionCreateRequest + if err := json.NewDecoder(request.Body).Decode(&create); err != nil { + t.Errorf("decode create: %v", err) + } + creates <- create + // Deliberately ignore request.Context(). This models an injected client + // with no transport timeout and proves the permission callback itself is + // bounded even if the transport does not return on cancellation. + <-releaseCreate + return nil, errors.New("create acknowledgement lost") + })} + host := NewSessionHost(SessionHostConfig{GatewayConfig: GatewayConfig{ + ControlPlaneURL: "https://control-plane.test", + ProjectID: "project-1", + WorkspaceID: "workspace-1", + SessionID: "agent-session-1", + RuntimeIdentity: "runtime-1", + CallbackToken: "callback-token", + HTTPClient: httpClient, + }}) + host.ConfigureAcpInteractions(testInteractionConfig()) + generation := host.attachAcpInteractionGeneration() + promptCtx, promptCancel := context.WithCancel(host.lifecycleContext()) + if _, ok := host.beginPromptForDelivery(promptCtx, promptCancel, "blocked-create-test", nil); !ok { + t.Fatal("test prompt was not accepted") + } + return host, &sessionHostClient{host: host, interactionGeneration: generation}, creates, settles, releaseCreate +} + func waitCreate(t *testing.T, recorder *interactionRecorder) acpInteractionCreateRequest { t.Helper() select { @@ -256,6 +328,114 @@ func TestRequestPermissionContextCancelAndDeadlineFailClosed(t *testing.T) { }) } +func TestRequestPermissionDelayedCreateObeysDeadlineAndLifecycle(t *testing.T) { + tests := []struct { + name string + cancel func(*SessionHost) + wantReason string + wantReceipt string + deadlineMs int64 + }{ + {name: "deadline", wantReason: "expired", wantReceipt: "no_waiter", deadlineMs: 30}, + { + name: "Stop", + cancel: func(host *SessionHost) { host.Stop() }, + wantReason: "session_stopped", wantReceipt: "no_waiter", + }, + { + name: "connection replacement", + cancel: func(host *SessionHost) { host.attachAcpInteractionGeneration() }, + wantReason: "connection_replaced", wantReceipt: "stale_generation", + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + host, client, creates, settles, releaseCreate := newBlockedCreateInteractionHost(t) + defer close(releaseCreate) + defer host.Stop() + if test.deadlineMs > 0 { + config := testInteractionConfig() + config.PermissionDeadlineMs = test.deadlineMs + host.ConfigureAcpInteractions(config) + } + + done := make(chan acpsdk.RequestPermissionResponse, 1) + go func() { + response, _ := client.RequestPermission(context.Background(), permissionRequest()) + done <- response + }() + var created acpInteractionCreateRequest + select { + case created = <-creates: + case <-time.After(time.Second): + t.Fatal("timed out waiting for blocked durable create") + } + if test.cancel != nil { + test.cancel(host) + } + select { + case response := <-done: + if response.Outcome.Cancelled == nil || response.Outcome.Selected != nil { + t.Fatalf("outcome = %+v, want cancelled", response.Outcome) + } + case <-time.After(time.Second): + t.Fatal("permission remained blocked behind delayed create") + } + select { + case settle := <-settles: + if settle.Reason != test.wantReason { + t.Fatalf("settle reason = %q, want %q", settle.Reason, test.wantReason) + } + case <-time.After(time.Second): + t.Fatal("timed out waiting for durable settle") + } + decision := AcpInteractionAnswerDecision{ + Kind: "selected_option", OptionID: "allow", AnswerHash: "late-answer", + } + if status := host.ResolveAcpInteractionAnswer(created.InteractionID, created.Generation, decision); status != test.wantReceipt { + t.Fatalf("late answer receipt = %q, want %q", status, test.wantReceipt) + } + }) + } +} + +func TestRequestPermissionAnswerWinsBeforeLostCreateAcknowledgement(t *testing.T) { + blockCreate := make(chan struct{}) + recorder := newInteractionRecorder(t, http.StatusCreated) + recorder.createBlock = blockCreate + host, client := newInteractionHost(recorder) + defer host.Stop() + + type result struct { + response acpsdk.RequestPermissionResponse + err error + } + done := make(chan result, 1) + go func() { + response, err := client.RequestPermission(context.Background(), permissionRequest()) + done <- result{response: response, err: err} + }() + created := waitCreate(t, recorder) + decision := AcpInteractionAnswerDecision{ + Kind: "selected_option", OptionID: "allow", AnswerHash: "durable-answer", + } + if status := host.ResolveAcpInteractionAnswer(created.InteractionID, created.Generation, decision); status != "consumed" { + t.Fatalf("answer receipt = %q, want consumed", status) + } + + permissionResult := <-done + if permissionResult.err != nil || permissionResult.response.Outcome.Selected == nil || + permissionResult.response.Outcome.Selected.OptionId != "allow" { + t.Fatalf("permission result = %+v err=%v", permissionResult.response, permissionResult.err) + } + if settle := waitSettle(t, recorder); settle.Reason != "completed" { + t.Fatalf("settle reason = %q, want completed", settle.Reason) + } + if status := host.ResolveAcpInteractionAnswer(created.InteractionID, created.Generation, decision); status != "duplicate" { + t.Fatalf("duplicate receipt = %q, want duplicate", status) + } +} + func TestRequestPermissionFeatureOffAndCreateFailureNeverSelect(t *testing.T) { recorder := newInteractionRecorder(t, http.StatusCreated) host, client := newInteractionHost(recorder) @@ -330,6 +510,10 @@ func TestRequestPermissionInvalidContractsAndRequestsFailClosedBeforeCreate(t *t HTTPClient: recorder.server.Client(), }}) t.Cleanup(host.Stop) + promptCtx, promptCancel := context.WithCancel(host.lifecycleContext()) + if _, ok := host.beginPromptForDelivery(promptCtx, promptCancel, "invalid-request-test", nil); !ok { + t.Fatal("test prompt was not accepted") + } config := testInteractionConfig() request := permissionRequest() generation := host.attachAcpInteractionGeneration() @@ -390,7 +574,7 @@ func TestInteractionAnswerRaceAndReceiptEvictionAreBounded(t *testing.T) { host.ConfigureAcpInteractions(config) generation := host.attachAcpInteractionGeneration() options := map[string]struct{}{"allow": {}} - first, err := host.registerInteractionWaiter("first", generation, options) + first, err := host.registerInteractionWaiter("first", generation, 1, options, func() {}) if err != nil { t.Fatal(err) } @@ -425,7 +609,7 @@ func TestInteractionAnswerRaceAndReceiptEvictionAreBounded(t *testing.T) { t.Fatalf("waiter resolved %d times", len(first.result)) } - second, err := host.registerInteractionWaiter("second", generation, options) + second, err := host.registerInteractionWaiter("second", generation, 2, options, func() {}) if err != nil { t.Fatal(err) } diff --git a/packages/vm-agent/internal/acp/session_host_prompt.go b/packages/vm-agent/internal/acp/session_host_prompt.go index 4e5948f562..addf50d3ad 100644 --- a/packages/vm-agent/internal/acp/session_host_prompt.go +++ b/packages/vm-agent/internal/acp/session_host_prompt.go @@ -67,7 +67,7 @@ func (h *SessionHost) AcceptPrompt( } promptCtx, promptCancel, promptTimeout := h.newPromptContext(ctx) - attempt, ok := h.beginPromptForDelivery(promptCancel, deliveryID, observer) + attempt, ok := h.beginPromptForDelivery(promptCtx, promptCancel, deliveryID, observer) if !ok { promptCancel() h.sendJSONRPCErrorToViewer(viewerID, reqID, -32603, "Prompt already in progress") diff --git a/packages/vm-agent/internal/acp/session_host_prompt_state.go b/packages/vm-agent/internal/acp/session_host_prompt_state.go index 0f11d64fad..e738989cfe 100644 --- a/packages/vm-agent/internal/acp/session_host_prompt_state.go +++ b/packages/vm-agent/internal/acp/session_host_prompt_state.go @@ -31,10 +31,10 @@ func (h *SessionHost) promptCancelGracePeriod() time.Duration { } func (h *SessionHost) beginPrompt(cancel context.CancelFunc, observer PromptTerminalObserver) (*promptAttempt, bool) { - return h.beginPromptForDelivery(cancel, "", observer) + return h.beginPromptForDelivery(context.Background(), cancel, "", observer) } -func (h *SessionHost) beginPromptForDelivery(cancel context.CancelFunc, deliveryID string, observer PromptTerminalObserver) (*promptAttempt, bool) { +func (h *SessionHost) beginPromptForDelivery(ctx context.Context, cancel context.CancelFunc, deliveryID string, observer PromptTerminalObserver) (*promptAttempt, bool) { h.promptMu.Lock() defer h.promptMu.Unlock() if h.promptInFlight { @@ -44,6 +44,7 @@ func (h *SessionHost) beginPromptForDelivery(cancel context.CancelFunc, delivery promptID := atomic.AddUint64(&h.promptSeq, 1) attempt := &promptAttempt{ id: promptID, + ctx: ctx, startedAt: h.now(), cancel: cancel, deliveryID: deliveryID, @@ -61,6 +62,15 @@ func (h *SessionHost) beginPromptForDelivery(cancel context.CancelFunc, delivery return attempt, true } +func (h *SessionHost) activePromptAttempt() (*promptAttempt, bool) { + h.promptMu.Lock() + defer h.promptMu.Unlock() + if !h.promptInFlight || h.promptAttempt == nil { + return nil, false + } + return h.promptAttempt, true +} + func (h *SessionHost) releasePrompt(attempt *promptAttempt) { h.promptMu.Lock() if h.promptAttempt == attempt { diff --git a/specs/001-mvp/contracts/api.md b/specs/001-mvp/contracts/api.md index 14d29d889e..d808c6342b 100644 --- a/specs/001-mvp/contracts/api.md +++ b/specs/001-mvp/contracts/api.md @@ -96,11 +96,16 @@ normal authenticated browser session in addition to the Origin check. Permission requests use a fresh UUID generation for every ACP connection attachment. The VM agent persists bounded option labels and structural tool metadata through the callback route, waits for -the durable answer, and accepts only an exact option ID. Raw tool input/content is never sent on the -viewer WebSocket or copied into the interaction payload. Runtime answers use the dedicated -node-management-JWT endpoint and an in-memory waiter/receipt registry; missing or stale runtimes -are never woken or recreated to consume an answer. A live runtime with no matching registry entry -returns `no_waiter`; an absent or stopped runtime returns an HTTP error before registry lookup. +the durable answer, and accepts only an exact option ID. Create and wait share the exact outgoing +prompt attempt's cancellation/deadline even though the ACP SDK gives inbound permission callbacks +an independent connection context. An ambiguous create acknowledgement keeps waiting because the +durable create may already have committed. Raw tool input/content is never sent on the viewer +WebSocket or copied into the interaction payload. Runtime answers use the dedicated +node-management-JWT endpoint and an attempt-bound in-memory waiter/receipt registry; missing or stale +runtimes are never woken or recreated to consume an answer. Instant delivery forwards through the +already-running Durable Object container TCP port and never calls an SDK helper that can start the +container. A live runtime with no matching registry entry returns `no_waiter`; an absent or stopped +runtime returns an HTTP error before registry lookup. ### GET /projects/:projectId/library/:fileId/preview diff --git a/tasks/active/2026-09-30-acp-runtime-permission-bridge.md b/tasks/active/2026-09-30-acp-runtime-permission-bridge.md index f48d04f021..c1093005fa 100644 --- a/tasks/active/2026-09-30-acp-runtime-permission-bridge.md +++ b/tasks/active/2026-09-30-acp-runtime-permission-bridge.md @@ -75,3 +75,29 @@ Slice B connects ACP `RequestPermission` to the shipped Cloudflare create/answer - Draft PR: `#2201` - Final implementation commit: `ea3dbad82` - Integrated staging: explicitly deferred to coordinator before readiness or merge + +## Coordinator Review Follow-up (2026-09-30) + +### Findings + +- The pinned `@cloudflare/containers@0.3.7` implementation of `containerFetch()` calls `startAndWaitForPorts()` whenever the actual container is stopped or its SDK state is not healthy. The persisted lifecycle guard therefore cannot make `proxyHttpNoWake` non-starting. +- Permission create currently runs before the deadline/lifecycle select. A slow create can outlive expiry, Stop, or connection replacement, and an answer committed before a lost create acknowledgement is discarded. +- Pinned `acp-go-sdk@v0.13.5` creates inbound request contexts from its connection context, independently of the outgoing Prompt context. Permission ownership must therefore come from SAM's active prompt attempt. + +### Follow-up Checklist + +- [x] Forward Instant no-wake requests through the direct Durable Object container TCP port without calling an auto-starting SDK helper. +- [x] Test persisted-running/actual-stopped and stop-between-check-and-forward races for both capability and answer request shapes. +- [x] Bind create and wait to the permission deadline, prompt attempt, Stop, process loss, and connection generation. +- [x] Preserve a valid answer that wins before an ambiguous/lost create acknowledgement and record its normal duplicate receipt. +- [x] Test delayed-create expiry, Stop, replacement, and durable-create/lost-response outcomes with exact settle reasons and receipts. +- [x] Add a real pinned-SDK fixture proving prompt deadline/cancel settles the matching permission while the connection remains usable, including a stale-cancel/new-attempt control. +- [ ] Re-run focused race/contract tests, repository checks, specialist review, and CI; update the draft PR evidence without staging, readiness, or merge. + +### Follow-up Acceptance Criteria + +- Capability and answer delivery cannot start or wake Instant compute even when persisted lifecycle state is stale or the container stops during forwarding. +- Permission create cannot hold the callback beyond its bounded attempt/deadline, and lifecycle cancellation retains its exact durable settle reason. +- Ambiguous create acknowledgement does not override an answer already consumed by the runtime; the matching receipt remains duplicate-safe. +- Permission cancellation is owned by the exact prompt attempt. An old attempt's cancellation cannot cancel a newer attempt's permission. +- The real `acp-go-sdk@v0.13.5` connection remains usable after prompt deadline/cancel closes the matching permission. From 7e60301748da0958c85ce2ccc823e6661a4fa4df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 30 Sep 2026 10:34:18 +0000 Subject: [PATCH 12/13] fix(acp): retry settle after create race --- .../acp/session_host_interaction_transport.go | 8 +- .../acp/session_host_interactions_test.go | 93 +++++++++++++++++++ 2 files changed, 99 insertions(+), 2 deletions(-) diff --git a/packages/vm-agent/internal/acp/session_host_interaction_transport.go b/packages/vm-agent/internal/acp/session_host_interaction_transport.go index 1d70dbc4e5..25afef098d 100644 --- a/packages/vm-agent/internal/acp/session_host_interaction_transport.go +++ b/packages/vm-agent/internal/acp/session_host_interaction_transport.go @@ -108,9 +108,13 @@ func (h *SessionHost) settleAcpInteraction(request acpInteractionSettleRequest, if response.StatusCode >= 200 && response.StatusCode < 300 { return } + // A create request can commit after its response is lost or after local + // cancellation wins. In that ordering the first settle may observe 404 + // before the create becomes visible, so retry not-found within this + // already-bounded settle context. if response.StatusCode == http.StatusBadRequest || response.StatusCode == http.StatusUnauthorized || - response.StatusCode == http.StatusForbidden || response.StatusCode == http.StatusNotFound || - response.StatusCode == http.StatusConflict || response.StatusCode == http.StatusGone { + response.StatusCode == http.StatusForbidden || response.StatusCode == http.StatusConflict || + response.StatusCode == http.StatusGone { return } } diff --git a/packages/vm-agent/internal/acp/session_host_interactions_test.go b/packages/vm-agent/internal/acp/session_host_interactions_test.go index 7afc96a527..a13fbd9301 100644 --- a/packages/vm-agent/internal/acp/session_host_interactions_test.go +++ b/packages/vm-agent/internal/acp/session_host_interactions_test.go @@ -5,10 +5,12 @@ import ( "context" "encoding/json" "errors" + "io" "net/http" "net/http/httptest" "strings" "sync" + "sync/atomic" "testing" "time" @@ -436,6 +438,97 @@ func TestRequestPermissionAnswerWinsBeforeLostCreateAcknowledgement(t *testing.T } } +func TestRequestPermissionSettleRetriesNotFoundUntilDelayedCreateCommits(t *testing.T) { + createStarted := make(chan acpInteractionCreateRequest, 1) + commitCreate := make(chan struct{}) + settles := make(chan acpInteractionSettleRequest, 2) + var settleCalls atomic.Int32 + httpClient := &http.Client{Transport: roundTripFunc(func(request *http.Request) (*http.Response, error) { + if strings.HasSuffix(request.URL.Path, "/settle") { + var settle acpInteractionSettleRequest + if err := json.NewDecoder(request.Body).Decode(&settle); err != nil { + t.Errorf("decode settle: %v", err) + } + settles <- settle + if settleCalls.Add(1) == 1 { + close(commitCreate) + return &http.Response{ + StatusCode: http.StatusNotFound, + Body: http.NoBody, + Header: make(http.Header), + }, nil + } + return &http.Response{ + StatusCode: http.StatusOK, + Body: http.NoBody, + Header: make(http.Header), + }, nil + } + var create acpInteractionCreateRequest + if err := json.NewDecoder(request.Body).Decode(&create); err != nil { + t.Errorf("decode create: %v", err) + } + createStarted <- create + // Deliberately let the first settle observe not-found before this durable + // create commits and returns its acknowledgement. + <-commitCreate + return &http.Response{ + StatusCode: http.StatusCreated, + Body: io.NopCloser(strings.NewReader(`{"status":"created"}`)), + Header: make(http.Header), + }, nil + })} + host := NewSessionHost(SessionHostConfig{GatewayConfig: GatewayConfig{ + ControlPlaneURL: "https://control-plane.test", + ProjectID: "project-1", + WorkspaceID: "workspace-1", + SessionID: "agent-session-1", + RuntimeIdentity: "runtime-1", + CallbackToken: "callback-token", + HTTPClient: httpClient, + }}) + host.ConfigureAcpInteractions(testInteractionConfig()) + generation := host.attachAcpInteractionGeneration() + promptCtx, promptCancel := context.WithCancel(host.lifecycleContext()) + if _, ok := host.beginPromptForDelivery(promptCtx, promptCancel, "delayed-commit-test", nil); !ok { + t.Fatal("test prompt was not accepted") + } + client := &sessionHostClient{host: host, interactionGeneration: generation} + done := make(chan acpsdk.RequestPermissionResponse, 1) + go func() { + response, _ := client.RequestPermission(context.Background(), permissionRequest()) + done <- response + }() + + select { + case <-createStarted: + case <-time.After(time.Second): + t.Fatal("timed out waiting for delayed create") + } + host.Stop() + select { + case response := <-done: + if response.Outcome.Cancelled == nil || response.Outcome.Selected != nil { + t.Fatalf("outcome = %+v, want cancelled", response.Outcome) + } + case <-time.After(time.Second): + t.Fatal("permission remained blocked behind delayed create") + } + for attempt := 1; attempt <= 2; attempt++ { + select { + case settle := <-settles: + if settle.Reason != "session_stopped" { + t.Fatalf("settle attempt %d reason = %q, want session_stopped", attempt, settle.Reason) + } + case <-time.After(time.Second): + t.Fatalf("timed out waiting for settle attempt %d", attempt) + } + } + if got := settleCalls.Load(); got != 2 { + t.Fatalf("settle calls = %d, want 2", got) + } +} + func TestRequestPermissionFeatureOffAndCreateFailureNeverSelect(t *testing.T) { recorder := newInteractionRecorder(t, http.StatusCreated) host, client := newInteractionHost(recorder) From 96df904f7f847b84a2cb86df98439a19634a8612 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 30 Sep 2026 10:35:01 +0000 Subject: [PATCH 13/13] docs(task): record ACP follow-up validation --- tasks/active/2026-09-30-acp-runtime-permission-bridge.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tasks/active/2026-09-30-acp-runtime-permission-bridge.md b/tasks/active/2026-09-30-acp-runtime-permission-bridge.md index c1093005fa..74145bb2a3 100644 --- a/tasks/active/2026-09-30-acp-runtime-permission-bridge.md +++ b/tasks/active/2026-09-30-acp-runtime-permission-bridge.md @@ -73,7 +73,7 @@ Slice B connects ACP `RequestPermission` to the shipped Cloudflare create/answer - Branch: `sam/execute-task-using-skill-zafwb6` - Draft PR: `#2201` -- Final implementation commit: `ea3dbad82` +- Original implementation commit: `ea3dbad82`; coordinator follow-up fixes: `d3e2b3f8c`, `7e6030174`. - Integrated staging: explicitly deferred to coordinator before readiness or merge ## Coordinator Review Follow-up (2026-09-30) @@ -92,7 +92,7 @@ Slice B connects ACP `RequestPermission` to the shipped Cloudflare create/answer - [x] Preserve a valid answer that wins before an ambiguous/lost create acknowledgement and record its normal duplicate receipt. - [x] Test delayed-create expiry, Stop, replacement, and durable-create/lost-response outcomes with exact settle reasons and receipts. - [x] Add a real pinned-SDK fixture proving prompt deadline/cancel settles the matching permission while the connection remains usable, including a stale-cancel/new-attempt control. -- [ ] Re-run focused race/contract tests, repository checks, specialist review, and CI; update the draft PR evidence without staging, readiness, or merge. +- [x] Re-run focused race/contract tests, repository checks, specialist review, and CI; update the draft PR evidence without staging, readiness, or merge. ### Follow-up Acceptance Criteria