Merge branch 'codex/invariant-service-seam' into codex/invariant-package-registration-gate
This commit is contained in:
@@ -114,7 +114,7 @@ When optional consumers are loaded, ACP form answers become the exact JSON shape
|
||||
|
||||
#### Token effect
|
||||
|
||||
Answer, error, and denial text enters context only through the owning tool result; presentation metadata adds zero model tokens.
|
||||
Answer, error, and denial text enters context only through the owning tool result; presentation metadata adds zero model tokens. A replacement `tool/result` still changes the model-facing session surface, but live and replayed ACP feeds ignore it as an execution update so the original terminal or diff completion is not overwritten.
|
||||
|
||||
#### KV Cache effect
|
||||
|
||||
|
||||
@@ -82,7 +82,7 @@ These are capabilities the bridge would *drive* on the editor. The harness runs
|
||||
| `agent_thought_chunk` | S | ✅ | ✅ | ✅ | From `assistant/chunk` reasoning-delta. |
|
||||
| `user_message_chunk` | S | ✅ | ✅ | ✅ | Emitted during `session/load` replay to reconstruct the user side. |
|
||||
| `tool_call` | S | ✅ | ✅ | ✅ | Tool-owned presentation (`presentCall`); see [§5](#5-tool-call-rendering). |
|
||||
| `tool_call_update` | S | ✅ | ✅ | ✅ | From `tool/result` via `presentResult`. |
|
||||
| `tool_call_update` | S | ✅ | ✅ | ✅ | From appended `tool/result` via `presentResult`; replacement results rewrite model context and do not duplicate or overwrite execution presentation. |
|
||||
| `plan` | S | ❌ | ✅ | ✅ | No agent plan emitted. Both adapters emit real plan entries (Codex's `CodexEventHandler.updatePlan` maps `turn/plan/updated` → `{ sessionUpdate: 'plan', entries }`). |
|
||||
| `available_commands_update` | S | ❌ | ✅ | ✅ | No slash commands advertised. |
|
||||
| `current_mode_update` | S | ❌ | ✅ | ✅ | No session modes. |
|
||||
|
||||
@@ -827,10 +827,11 @@ export function apply(ctx: Context, config: AcpConfig): void {
|
||||
// session/cancel maps to the queue-aware agent.cancel(reason): it aborts
|
||||
// a RUNNING step, clears the queued + steering FIFOs, and drops a
|
||||
// turn that is about to start (the pre-step window) — so a queued-but-
|
||||
// not-yet-started prompt never runs, and a prompt accepted right after
|
||||
// cannot be batched into the cancelled turn. Scoped to THIS session's
|
||||
// not-yet-started prompt never runs, while a prompt accepted afterward
|
||||
// remains a separate queued turn. Scoped to THIS session's
|
||||
// agent — a cancel in one session never touches another's stream or
|
||||
// pending prompt (RFC 011 isolation). We ALSO settle the in-flight prompt
|
||||
// pending prompt (multi-session isolation).
|
||||
// We ALSO settle the in-flight prompt
|
||||
// as cancelled directly here: do NOT rely on the resulting turn/end to
|
||||
// settle it, because cancel() may drop the turn before any turn/end is
|
||||
// emitted, and removing this direct settle would move the RPC's
|
||||
@@ -1039,7 +1040,8 @@ function validateMcpServers(params: { mcpServers?: unknown[] }): void {
|
||||
* loaded transcript reconstructs the USER side of each turn without echoing
|
||||
* a live `session/prompt` back to the client
|
||||
* - `tool/call` → `tool_call` (pending)
|
||||
* - `tool/result` → `tool_call_update` (completed/failed)
|
||||
* - appended `tool/result` → `tool_call_update` (completed/failed)
|
||||
* - replacement `tool/result` → no update (context rewrite, not execution)
|
||||
*
|
||||
* Tool-call presentation (title/kind/rawInput, and the completed-state content)
|
||||
* is owned by each TOOL via `presentCall`/`presentResult` — the bridge never
|
||||
@@ -1100,6 +1102,10 @@ export function streamSessionEventUpdate(
|
||||
return
|
||||
}
|
||||
case 'tool/result': {
|
||||
// Replacements (for example model-free pruning) are transcript rewrites,
|
||||
// not repeated tool executions. Re-presenting one would consume no
|
||||
// pending call and could clobber the original terminal/diff completion.
|
||||
if (event.surfaceOp !== undefined && event.surfaceOp !== 'append') return
|
||||
const view = presenter.result(event.data.callId, event.data.content, event.data.isError, event.data.meta)
|
||||
notify({ sessionId, update: toolResultUpdate(event.data.callId, view, event.data.isError, terminal) })
|
||||
return
|
||||
|
||||
@@ -209,12 +209,12 @@ describe('acp bridge — session config options', () => {
|
||||
expect(after.configOptions).toEqual(optionsWithPermission('danger-full-access'))
|
||||
|
||||
const session = h.ctx.agents.list()[0]?.session
|
||||
expect(session?.events.some(e => e.type === 'permission/preset' || e.type === 'bash/sandbox-mode' || e.type === 'approval/policy')).toBe(false)
|
||||
expect(session?.events.some(e => e.type === 'permission/preset' || e.type === 'sandbox/mode' || e.type === 'approval/policy')).toBe(false)
|
||||
|
||||
await h.client.prompt({ sessionId, prompt: [{ type: 'text', text: 'anchor' }] })
|
||||
const events = session?.events ?? []
|
||||
expect(events.filter(e => e.type === 'permission/preset').map(e => e.data)).toEqual([{ preset: 'danger-full-access' }])
|
||||
expect(events.filter(e => e.type === 'bash/sandbox-mode').map(e => e.data)).toEqual([{ mode: 'danger-full-access' }])
|
||||
expect(events.filter(e => e.type === 'sandbox/mode').map(e => e.data)).toEqual([{ mode: 'danger-full-access' }])
|
||||
expect(events.filter(e => e.type === 'approval/policy').map(e => e.data)).toEqual([{ policy: 'never' }])
|
||||
const turnStart = events.findIndex(e => e.type === 'turn/start')
|
||||
const anchored = events.findIndex(e => e.type === 'permission/preset')
|
||||
@@ -244,7 +244,7 @@ describe('acp bridge — session config options', () => {
|
||||
expect(back.configOptions).toEqual(optionsWithPermission('workspace-write'))
|
||||
await h.client.prompt({ sessionId, prompt: [{ type: 'text', text: 'anchor' }] })
|
||||
const events = h.ctx.agents.list()[0]?.session.events ?? []
|
||||
expect(events.some(e => e.type === 'permission/preset' || e.type === 'bash/sandbox-mode' || e.type === 'approval/policy')).toBe(false)
|
||||
expect(events.some(e => e.type === 'permission/preset' || e.type === 'sandbox/mode' || e.type === 'approval/policy')).toBe(false)
|
||||
})
|
||||
|
||||
it('a no-op switch (the value already shown) records nothing and keeps a live pending', async () => {
|
||||
@@ -272,7 +272,7 @@ describe('acp bridge — session config options', () => {
|
||||
const anchored = events.findIndex(e => e.type === 'permission/preset')
|
||||
expect(turnStart).toBeGreaterThanOrEqual(0)
|
||||
expect(anchored).toBeGreaterThan(turnStart)
|
||||
expect(events.some(e => e.type === 'bash/sandbox-mode')).toBe(true)
|
||||
expect(events.some(e => e.type === 'sandbox/mode')).toBe(true)
|
||||
expect(events.some(e => e.type === 'approval/policy')).toBe(true)
|
||||
await h.client.cancel({ sessionId })
|
||||
await hung
|
||||
@@ -342,7 +342,7 @@ describe('acp bridge — session config options', () => {
|
||||
const agent = h.ctx.agents.list()[0]
|
||||
if (agent === undefined) throw new Error('expected an agent')
|
||||
agent.session.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } })
|
||||
agent.session.append('bash/sandbox-mode', { mode: 'read-only' })
|
||||
agent.session.append('sandbox/mode', { mode: 'read-only' })
|
||||
agent.session.append('turn/end', { turn: 1, reason: { kind: 'completed' } })
|
||||
const echo = await h.client.setSessionConfigOption({ sessionId, configId: 'permission', value: 'custom' })
|
||||
const option = echo.configOptions?.find(entry => entry.id === 'permission')
|
||||
|
||||
@@ -223,7 +223,8 @@ describe('acp bridge — disposal & HMR safety', () => {
|
||||
|
||||
it('per-session AgentHandle dispose leaves sibling agents untouched', async () => {
|
||||
// The factory returns a per-agent AgentHandle whose dispose() tears down
|
||||
// EXACTLY that agent + its session — RFC 011 isolation. Create two agents
|
||||
// EXACTLY that agent + its session — the registry's per-handle isolation
|
||||
// contract. Create two agents
|
||||
// directly through the registry factory (the same path the ACP bridge uses),
|
||||
// dispose one handle, and assert the other survives, registered and
|
||||
// queryable, with its session still in the store.
|
||||
|
||||
@@ -161,6 +161,53 @@ describe('acp bridge — session/load replay', () => {
|
||||
expect(meta.terminal_exit?.exit_code).toBe(0)
|
||||
})
|
||||
|
||||
it('keeps one terminal completion live and on replay when a pruning replacement is logged', async () => {
|
||||
live = await makeBridgeHarness({
|
||||
storageDir,
|
||||
withBash: true,
|
||||
script: [toolCallResponse('c1', 'bash', { command: 'echo full', description: 'Print full output' }), textResponse('done')],
|
||||
})
|
||||
await live.client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: { _meta: { terminal_output: true } } })
|
||||
const { sessionId } = await live.client.newSession({ cwd: process.cwd(), mcpServers: [] })
|
||||
await live.client.prompt({ sessionId, prompt: [{ type: 'text', text: 'run it' }] })
|
||||
|
||||
const session = live.ctx.agents.get(SessionId(sessionId))!.session
|
||||
const original = session.events.find(event => event.type === 'tool/result')
|
||||
if (original?.type !== 'tool/result') throw new Error('expected original tool/result')
|
||||
const liveCompletions = () => live!.updates.filter(update =>
|
||||
update.sessionUpdate === 'tool_call_update' && update.toolCallId === 'c1')
|
||||
expect(liveCompletions()).toHaveLength(1)
|
||||
expect((liveCompletions()[0] as { _meta?: { terminal_output?: { data: string } } })._meta?.terminal_output?.data)
|
||||
.toBe('full\n')
|
||||
|
||||
session.append('turn/start', { turn: 2, trigger: { kind: 'message', source: { kind: 'user' } } })
|
||||
session.append('tool/result', {
|
||||
...original.data,
|
||||
content: [{ type: 'text', text: '[... tool result middle pruned ...]' }],
|
||||
}, {
|
||||
surfaceOp: { op: 'replace', start: original.seq, end: original.seq },
|
||||
sourceEventSeqs: [original.seq],
|
||||
})
|
||||
session.append('turn/end', { turn: 2, reason: { kind: 'completed' } })
|
||||
|
||||
// The replacement is durable but is not another live completion.
|
||||
expect(session.events.filter(event => event.type === 'tool/result')).toHaveLength(2)
|
||||
expect(JSON.stringify(session.deriveMessages())).toContain('tool result middle pruned')
|
||||
expect(liveCompletions()).toHaveLength(1)
|
||||
await live.dispose()
|
||||
live = undefined
|
||||
|
||||
loader = await makeBridgeHarness({ storageDir, withBash: true, script: [] })
|
||||
await loader.client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: { _meta: { terminal_output: true } } })
|
||||
await loader.client.loadSession({ sessionId, cwd: process.cwd(), mcpServers: [] })
|
||||
|
||||
const replayed = loader.updates.filter(update =>
|
||||
update.sessionUpdate === 'tool_call_update' && update.toolCallId === 'c1')
|
||||
expect(replayed).toHaveLength(1)
|
||||
expect((replayed[0] as { _meta?: { terminal_output?: { data: string } } })._meta?.terminal_output?.data)
|
||||
.toBe('full\n')
|
||||
})
|
||||
|
||||
it('a load whose resume finishes after a client disconnect leaks no live session', async () => {
|
||||
// Stall persistence so transport closes while resume is pending. Whether the SDK rejects first
|
||||
// or the bridge's post-await guard fires, no agent may survive for the dead connection.
|
||||
|
||||
@@ -14,7 +14,7 @@ function messageTextFor(updates: { sessionId?: string; update: CapturedUpdate }[
|
||||
.join('')
|
||||
}
|
||||
|
||||
describe('acp bridge — RFC 011 multi-session isolation', () => {
|
||||
describe('acp bridge — multi-session isolation', () => {
|
||||
let storageDir: string
|
||||
let harness: BridgeHarness | undefined
|
||||
|
||||
|
||||
@@ -105,6 +105,22 @@ describe('streamSessionEventUpdate', () => {
|
||||
expect((failed[0] as { status: string }).status).toBe('failed')
|
||||
})
|
||||
|
||||
it('emits no execution update for a tool-result surface replacement', () => {
|
||||
const replacement = {
|
||||
...evt('tool/result', {
|
||||
turn: 1,
|
||||
step: 1,
|
||||
callId: CallId('c1'),
|
||||
content: [{ type: 'text', text: '[... tool result middle pruned ...]' }],
|
||||
isError: false,
|
||||
}),
|
||||
seq: 2,
|
||||
surfaceOp: { op: 'replace', start: 1, end: 1 },
|
||||
sourceEventSeqs: [1],
|
||||
} as SessionEvent
|
||||
expect(updatesFor(replacement)).toEqual([])
|
||||
})
|
||||
|
||||
it('drops non-text tool-result content (text-only)', () => {
|
||||
const update = updatesFor(evt('tool/result', {
|
||||
turn: 1, step: 1, callId: CallId('c1'),
|
||||
@@ -450,6 +466,16 @@ describe('terminal-card mapping (capability-gated)', () => {
|
||||
|
||||
const callEvent = evt('tool/call', { turn: 1, step: 1, callId: CallId('c1'), name: 'bash', arguments: JSON.stringify({ command: 'echo hi', description: 'Greet' }) })
|
||||
const resultEvent = evt('tool/result', { turn: 1, step: 1, callId: CallId('c1'), content: [{ type: 'text', text: 'hi\n' }], isError: false })
|
||||
const prunedResultEvent = {
|
||||
...resultEvent,
|
||||
seq: 2,
|
||||
data: {
|
||||
...resultEvent.data,
|
||||
content: [{ type: 'text', text: '[... tool result middle pruned ...]' }],
|
||||
},
|
||||
surfaceOp: { op: 'replace', start: 1, end: 1 },
|
||||
sourceEventSeqs: [1],
|
||||
} as SessionEvent
|
||||
|
||||
function termUpdates(tool: ToolDefinition, enabled: boolean, cwd: string | undefined, ...events: SessionEvent[]): SessionNotification['update'][] {
|
||||
const presenter = new ToolPresenter(registryOf(tool))
|
||||
@@ -477,6 +503,27 @@ describe('terminal-card mapping (capability-gated)', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('live/replay translation preserves the original terminal completion across a pruning rewrite', () => {
|
||||
const updates = termUpdates(
|
||||
termTool({ card: 'terminal' }, { output: 'hi\n', exitCode: 0 }),
|
||||
true,
|
||||
'/work/proj',
|
||||
callEvent,
|
||||
resultEvent,
|
||||
prunedResultEvent,
|
||||
)
|
||||
expect(updates).toHaveLength(2)
|
||||
expect(updates[1]).toEqual({
|
||||
sessionUpdate: 'tool_call_update',
|
||||
toolCallId: 'c1',
|
||||
status: 'completed',
|
||||
_meta: {
|
||||
terminal_output: { terminal_id: 'c1', data: 'hi\n' },
|
||||
terminal_exit: { terminal_id: 'c1', exit_code: 0 },
|
||||
},
|
||||
})
|
||||
})
|
||||
|
||||
it('capability ON: an ABSOLUTE tool cwd wins; a RELATIVE one resolves against the session cwd', () => {
|
||||
const [absCall] = termUpdates(termTool({ card: 'terminal', cwd: '/explicit/abs' }, { output: 'x' }), true, '/work/proj', callEvent)
|
||||
expect((absCall as unknown as { _meta: { terminal_info: { cwd: string } } })._meta.terminal_info.cwd).toBe('/explicit/abs')
|
||||
@@ -633,17 +680,38 @@ describe('result-time diff card (REAL fs edit tool → tool_call_update diff blo
|
||||
// call-time snippet, then the tool/result carries the tool's computed applied-hunk `meta`,
|
||||
// which presentResult narrows into a `diff` result card the bridge forwards as `{ type:
|
||||
// 'diff' }` content blocks. The real tool is required because its result metadata is the contract.
|
||||
it('forwards the applied-hunk meta onto the wire as tool_call_update diff content', async () => {
|
||||
it('live/replay translation keeps the applied diff when a pruning rewrite follows', async () => {
|
||||
const ctx = await fsCtx()
|
||||
const presenter = new ToolPresenter(ctx.tools)
|
||||
const args = JSON.stringify({ file_path: 'src/b.ts', old_string: 'OLD', new_string: 'NEW' })
|
||||
// The applied hunk the tool would compute and persist on the result meta.
|
||||
const meta = { diffs: [{ path: 'src/b.ts', oldText: 'a\nOLD\nb', newText: 'a\nNEW\nb' }] }
|
||||
const [, resultUpdate] = updatesWith(
|
||||
const originalResult = evt('tool/result', {
|
||||
turn: 1,
|
||||
step: 1,
|
||||
callId: CallId('e1'),
|
||||
content: [{ type: 'text', text: 'ok' }],
|
||||
isError: false,
|
||||
meta,
|
||||
})
|
||||
const replacement = {
|
||||
...originalResult,
|
||||
seq: 3,
|
||||
data: {
|
||||
...originalResult.data,
|
||||
content: [{ type: 'text', text: '[... tool result middle pruned ...]' }],
|
||||
},
|
||||
surfaceOp: { op: 'replace', start: 2, end: 2 },
|
||||
sourceEventSeqs: [2],
|
||||
} as SessionEvent
|
||||
const updates = updatesWith(
|
||||
presenter,
|
||||
evt('tool/call', { turn: 1, step: 1, callId: CallId('e1'), name: 'edit', arguments: args }),
|
||||
evt('tool/result', { turn: 1, step: 1, callId: CallId('e1'), content: [{ type: 'text', text: 'ok' }], isError: false, meta }),
|
||||
originalResult,
|
||||
replacement,
|
||||
)
|
||||
expect(updates).toHaveLength(2)
|
||||
const resultUpdate = updates[1]
|
||||
expect(resultUpdate).toEqual({
|
||||
sessionUpdate: 'tool_call_update',
|
||||
toolCallId: 'e1',
|
||||
|
||||
Reference in New Issue
Block a user