refactor: remove UI identity translations

packages/ui/acp/README.md

@@ -8,7 +8,7 @@ It is a **client-driver / UI plugin**, the structured analogue of the readline `
 
 `apply(ctx, config)` — wires an `AgentSideConnection` (from `@agentclientprotocol/sdk`) to `process.stdin`/`process.stdout` and implements the ACP `Agent` method surface.
 
-`inject: ['agents', 'sessions', 'sessionPersistence', 'tools', 'userInteraction']` — programs against the interface packages only (never `dsh-agent-loop`). `sessionPersistence` is required because `initialize` advertises `loadSession: true`; `tools` lets a tool own how its calls render (`presentCall`/`presentResult`) — the bridge looks the definition up by name and falls back to a generic presentation when a tool declares none (see Tool-call presentation). `userInteraction` lets agent-owned `ask_user_question` calls become ACP form elicitations routed to the owning session.
+`inject: ['agents', 'sessionPersistence', 'tools', 'userInteraction']` — programs against the interface packages only (never `dsh-agent-loop`). `sessionPersistence` is required because `initialize` advertises `loadSession: true`; `tools` lets a tool own how its calls render (`presentCall`/`presentResult`) — the bridge looks the definition up by name and falls back to a generic presentation when a tool declares none (see Tool-call presentation). `userInteraction` lets agent-owned `ask_user_question` calls become ACP form elicitations routed to the owning session.
 
 ### Config
 
@@ -36,7 +36,7 @@ The `initialize` handshake reports a fixed server identity (`agentInfo: { name:
 
 ## Multi-session
 
-The bridge multiplexes N sessions over one connection. Live sessions are held in a `Map<sessionId, SessionRecord>` (forward) with a `WeakMap<Agent, sessionId>` reverse map so agent-scoped approval events demultiplex in O(1). Every `session/event` is routed strictly to its owning record, so concurrent sessions never cross-settle or interleave their `session/update` notifications. State is per session: one in-flight prompt each, `session/cancel` aborts and settles only its own agent/prompt, and disposal drains every live session in parallel to quiescence. Permission prompts follow the same ownership: the `approval/request` answerer resolves the owning session through the reverse map and prompts only there.
+The bridge multiplexes N sessions over one connection. Live sessions are held in a `Map<sessionId, SessionRecord>` keyed by the shared agent/session id. Agent-scoped events derive that id from `agent.session.id` and verify the record owns the exact agent object, so a foreign same-id object cannot claim the bridge's session. Every `session/event` is routed strictly to its owning record, so concurrent sessions never cross-settle or interleave their `session/update` notifications. State is per session: one in-flight prompt each, `session/cancel` aborts and settles only its own agent/prompt, and disposal drains every live session in parallel to quiescence. Permission prompts follow the same ownership and prompt only the matching session.
 
 ## Session config options
 

packages/ui/acp/src/index.ts

@@ -18,12 +18,11 @@
  *                      turn about to start) + settle the in-flight prompt
  *
  * Multi-session (RFC 011): N concurrent sessions per connection, each mapped to
- * its own `ReactLoopAgent`. Sessions are keyed by id in `sessions` (forward) with an
- * `agent→sessionId` reverse map for O(1) demux of `agent/*` events; every
- * `session/event` and `agent/*` event is routed strictly to its owning session
- * record, so two sessions streaming at once never interleave their
- * `session/update` notifications. Permission prompts ride the same ownership
- * map: the bridge answers `approval/request` for its own agents over
+ * its own `ReactLoopAgent`. Sessions are keyed by their shared agent/session id;
+ * every `session/event` and `agent/*` event is routed strictly to its owning
+ * session record, so two sessions streaming at once never interleave their
+ * `session/update` notifications. Permission prompts use the same identity: the
+ * bridge answers `approval/request` for its own agents over
  * `session/request_permission` (see the approval answerer below) — whether a
  * call ASKS is policy (a hook or plugin returning `ask`), not the bridge's.
  *
@@ -106,9 +105,7 @@ export const name = 'acp'
 // because `initialize` advertises `loadSession: true`. `tools` lets a tool own
 // how its calls render (`presentCall`/`presentResult`); the bridge looks up the
 // definition by name and falls back to a generic presentation when absent.
-// TODO(acp-session-inject): drop `sessions`; this bridge never reads
-// ctx.sessions, and agent/session ownership is already behind ctx.agents.
-export const inject = ['agents', 'sessions', 'sessionPersistence', 'tools', 'userInteraction']
+export const inject = ['agents', 'sessionPersistence', 'tools', 'userInteraction']
 
 /**
  * Build an ACP "invalid params" error whose human detail rides in the message.
@@ -268,7 +265,6 @@ export const Config: Schema<AcpConfig> = Schema.object({
  * map keyed by id (RFC 011 multi-session).
  */
 interface SessionRecord {
-  sessionId: SessionId
   agent: Agent
   /**
    * The owned-agent disposer (from the {@link AgentHandle} the factory returned).
@@ -353,15 +349,8 @@ export function apply(ctx: Context, config: AcpConfig): void {
   // this warn sink so a throwing tool presenter is logged, not propagated.
   const makePresenter = (agent?: Agent): ToolPresenter => new ToolPresenter(tools, (message) => { logger.warn(message) }, agent)
 
-  // TODO(derive-acp-session-id): derive an event's id from agent.session and
-  // verify sessions.get(id)?.agent === agent; then remove this reverse map and
-  // SessionRecord.sessionId, whose sole read duplicates the same identity.
-  // Live sessions keyed by id (RFC 011 multi-session), plus an agent→sessionId
-  // reverse map so `agent/*` events (which carry only the Agent) demux in O(1).
-  // The forward record and weak reverse entry are installed together; removing
-  // the record releases its strong Agent reference, so the WeakMap entry expires.
+  // Live sessions keyed by their shared agent/session id (RFC 011 multi-session).
   const sessions = new Map<SessionId, SessionRecord>()
-  const bySession = new WeakMap<Agent, SessionId>()
   // Session ids whose `session/load` is mid-`resume()` (the slot is reserved
   // before the async resume so a pipelined load/new for the SAME id can't create
   // two agents). Distinct ids load concurrently; a given id loads once at a time.
@@ -382,20 +371,26 @@ export function apply(ctx: Context, config: AcpConfig): void {
   // `notify` never observes it unset — no undefined guard needed.
   let conn: AgentSideConnection
 
+  /** Return the bridge-owned record for an agent, rejecting same-id impostors. */
+  const ownedRecord = (agent: Agent): SessionRecord | undefined => {
+    const rec = sessions.get(agent.session.id)
+    return rec?.agent === agent ? rec : undefined
+  }
+
   userInteraction.registerProvider({
     async ask(request: AskUserQuestionRequest): Promise<AskUserQuestionAnswer> {
       if (request.agent === undefined) {
         throw new UserInteractionError('ACP user questions must come from an agent-owned request', 'NO_AGENT')
       }
-      const sessionId = bySession.get(request.agent)
-      if (sessionId === undefined) {
+      const rec = ownedRecord(request.agent)
+      if (rec === undefined) {
         throw new UserInteractionError('ACP user question has no matching session', 'NO_SESSION')
       }
       const answers: AskUserQuestionAnswerItem[] = []
       for (const question of request.questions) {
         const options = question.options ?? []
         const response = await withAbort(conn.unstable_createElicitation(
-          elicitationForQuestion(sessionId, question, options),
+          elicitationForQuestion(rec.agent.session.id, question, options),
         ), request.signal).catch((error: unknown) => {
           if (error instanceof UserInteractionError) throw error
           throw new UserInteractionError('ACP elicitation request failed', 'ASK_FAILED', { cause: error })
@@ -496,7 +491,7 @@ export function apply(ctx: Context, config: AcpConfig): void {
     const rec = sessions.get(session.header.id)
     if (rec === undefined) return
     try {
-      streamSessionEventUpdate(rec.sessionId, event, notify, rec.presenter, {
+      streamSessionEventUpdate(rec.agent.session.id, event, notify, rec.presenter, {
         enabled: rec.terminalEnabled,
         cwd: session.header.cwd,
       }, { includeUserMessages: false })
@@ -527,12 +522,12 @@ export function apply(ctx: Context, config: AcpConfig): void {
   // allow_always is a grant-storage design the approval RFC defers, so the
   // prompt never offers a durable grant the harness could not honor.
   ctx.on('approval/request', (req, next) => {
-    const sessionId = bySession.get(req.agent)
+    const rec = ownedRecord(req.agent)
     // The protocol requires `toolCall` (the prompt renders attached to it), so
     // a request without a callId has nothing to attach to — delegate.
-    if (sessionId === undefined || req.callId === undefined) return next()
+    if (rec === undefined || req.callId === undefined) return next()
     return conn.requestPermission({
-      sessionId,
+      sessionId: rec.agent.session.id,
       toolCall: { toolCallId: req.callId },
       options: [
         { optionId: 'allow-once', name: 'Allow once', kind: 'allow_once' },
@@ -639,8 +634,7 @@ export function apply(ctx: Context, config: AcpConfig): void {
   // turn) leaves the switch pending — it runs no step, so nothing executes
   // or assembles under a stale value.
   ctx.on('agent/prompt-submit', (agent, _content, _source, next) => {
-    const sessionId = bySession.get(agent)
-    const rec = sessionId === undefined ? undefined : sessions.get(sessionId)
+    const rec = ownedRecord(agent)
     if (rec !== undefined) flushPendingSwitches(rec)
     return next()
   })
@@ -699,9 +693,7 @@ export function apply(ctx: Context, config: AcpConfig): void {
           await handle.dispose()
           throw internalError('connection closed during session/new')
         }
-        bySession.set(handle.agent, sessionId)
         sessions.set(sessionId, {
-          sessionId,
           agent: handle.agent,
           dispose: () => handle.dispose(),
           presenter: makePresenter(handle.agent),
@@ -773,13 +765,11 @@ export function apply(ctx: Context, config: AcpConfig): void {
             throw invalidParams('connection closed during session/load')
           }
           const agent = handle.agent
-          bySession.set(agent, sessionId)
           // Snapshot the terminal capability ONCE for this session (used by both
           // the replay below and the post-load live stream) so a later
           // `initialize` can't desync the call/result of a tool card.
           const terminalEnabled = terminalOutputCap
           const record: SessionRecord = {
-            sessionId,
             agent,
             dispose: () => handle.dispose(),
             presenter: makePresenter(agent),

packages/ui/acp/tests/approval.spec.ts

@@ -90,9 +90,12 @@ describe('acp bridge — approval answerer', () => {
     await harness.ctx.plugin(ApprovalService)
     harness.onPermission = () => ({ outcome: { outcome: 'selected', optionId: 'allow-once' } })
 
-    // Not created through the bridge: no bySession entry, so the answerer must
-    // call next() — nobody else answers, so the seam fails closed.
-    const foreign = { session: { events: [{ type: 'turn/start' }], append: () => ({}) } } as unknown as Agent
+    const { agent } = await ownedAgentRequest(harness)
+    // Even an impostor that claims the bridge-owned session id must delegate:
+    // ownership requires the exact Agent object stored in the session record.
+    const foreign = {
+      session: { id: agent.session.id, events: [{ type: 'turn/start' }], append: () => ({}) },
+    } as unknown as Agent
     await expect(harness.ctx.approval.request({ agent: foreign, toolName: 'echo', callId: CallId('c') }))
       .resolves.toBe('unavailable')
     expect(harness.permissionRequests).toHaveLength(0)

packages/ui/acp/tests/bridge.spec.ts

@@ -205,7 +205,8 @@ describe('acp bridge', () => {
 
     await expect(harness.ctx.userInteraction.ask({ questions: [{ id: 'x', question: 'No agent?' }] }))
       .rejects.toMatchObject({ name: 'UserInteractionError', code: 'NO_AGENT' })
-    await expect(harness.ctx.userInteraction.ask({ agent: { id: 'other' } as typeof agent, questions: [{ id: 'x', question: 'No session?' }] }))
+    const impostor = { session: { id: agent.session.id } } as typeof agent
+    await expect(harness.ctx.userInteraction.ask({ agent: impostor, questions: [{ id: 'x', question: 'No session?' }] }))
       .rejects.toMatchObject({ code: 'NO_SESSION' })
 
     harness.onElicitation = () => ({ action: 'cancel' })

packages/ui/jsonrpc/README.md

@@ -4,7 +4,7 @@ The **SDK server plugin** (`jsonrpc`): mounting it serves a stdio JSON-RPC serve
 
 ## Wiring
 
-`inject: ['agents']` — the server creates one agent per SDK `sessionId` (get-or-create on `session/prompt`) and demuxes `subagent/end` through the registry. The LLM seam is read opportunistically via `ctx.get('llm')` (not injected): when `initialize.model` has no registered adapter, the plugin mounts `dsh-llm-deepseek` for it (credentials from `$DEEPSEEK_API_KEY` / `$DEEPSEEK_BASE_URL`); a config-registered adapter for the model wins. Everything else — persistence, the tool stacks, the adapter set — comes from the surrounding `cordis.yml`.
+`inject: ['agents']` — the server creates one agent per SDK `sessionId` (get-or-create on `session/prompt`). A subagent's shared agent/session id supplies `subagent.finished.childSessionId` directly; the server caches only parent lineage because the child may be disposed before `subagent/end`. The LLM seam is read opportunistically via `ctx.get('llm')` (not injected): when `initialize.model` has no registered adapter, the plugin mounts `dsh-llm-deepseek` for it (credentials from `$DEEPSEEK_API_KEY` / `$DEEPSEEK_BASE_URL`); a config-registered adapter for the model wins. Everything else — persistence, the tool stacks, the adapter set — comes from the surrounding `cordis.yml`.
 
 ## Config
 

packages/ui/jsonrpc/src/server.ts

@@ -58,11 +58,6 @@ interface SessionRecord {
   activePrompt: boolean
 }
 
-interface SubagentRecord {
-  childSessionId: string
-  parentSessionId: string | undefined
-}
-
 /**
  * The SDK server over a booted harness context. Constructing it subscribes to
  * the context's `session/event`, `session/created`, `agent/created`, and
@@ -76,7 +71,7 @@ export class HarnessSdkServer {
   private llmFiber: { dispose(): Promise<void> } | undefined
   private readonly sessions = new Map<string, SessionRecord>()
   private readonly sessionCreations = new Map<string, Promise<SessionRecord>>()
-  private readonly subagentSessions = new Map<string, SubagentRecord>()
+  private readonly subagentParents = new Map<SessionId, SessionId>()
   private readonly disposers: (() => void)[] = []
   private shutdownTask: Promise<Record<string, never>> | undefined
   private shuttingDown = false
@@ -100,29 +95,22 @@ export class HarnessSdkServer {
         childSessionId: String(session.id),
       })
     }))
-    // Cache agent → session lineage on creation: by the time `subagent/end`
-    // fires the child agent may already be disposed and gone from the registry.
+    // Cache parent lineage on creation: by the time `subagent/end` fires the
+    // child agent may already be disposed and gone from the registry. The child
+    // session id needs no cache because it is the shared agent/session id.
     this.disposers.push(ctx.on('agent/created', (agent) => {
-      this.subagentSessions.set(String(agent.id), {
-        childSessionId: String(agent.session.id),
-        parentSessionId: agent.session.header.parentSession === undefined
-          ? undefined
-          : String(agent.session.header.parentSession),
-      })
+      const parentSessionId = agent.session.header.parentSession
+      if (parentSessionId !== undefined) this.subagentParents.set(agent.id, parentSessionId)
     }))
     this.disposers.push(ctx.on('subagent/end', (info: SubagentRunEndInfo) => {
-      const rec = this.subagentSessions.get(String(info.id))
       const agent = this.ctx.agents.get(info.id)
-      const childSessionId = rec?.childSessionId ?? (agent === undefined ? undefined : String(agent.session.id))
-      const parentSessionId = rec?.parentSessionId ?? (
-        agent?.session.header.parentSession === undefined ? undefined : String(agent.session.header.parentSession)
-      )
-      if (childSessionId === undefined) return
+      const parentSessionId = this.subagentParents.get(info.id) ?? agent?.session.header.parentSession
+      this.subagentParents.delete(info.id)
       this.transport.notify('subagent.finished', {
         provider: info.provider,
         agentId: String(info.id),
-        ...(parentSessionId === undefined ? {} : { parentSessionId }),
-        childSessionId,
+        ...(parentSessionId === undefined ? {} : { parentSessionId: String(parentSessionId) }),
+        childSessionId: String(info.id),
         status: info.stopReason === 'completed' ? 'ok' : 'error',
         stopReason: info.stopReason,
         ...(info.lastAssistantMessage === undefined ? {} : { lastAssistantMessage: info.lastAssistantMessage }),
@@ -195,7 +183,7 @@ export class HarnessSdkServer {
     this.sessionCreations.clear()
     const records = [...this.sessions.values()]
     this.sessions.clear()
-    this.subagentSessions.clear()
+    this.subagentParents.clear()
     const failures: unknown[] = []
     while (this.disposers.length > 0) {
       try {

packages/ui/jsonrpc/tests/server.spec.ts

@@ -272,6 +272,9 @@ describe('HarnessSdkServer', () => {
         meta: { cwd: storageDir, parentSession: SessionId('main') },
         agentOptions: { model: 'deepseek' },
       })
+      // The backend may dispose the child before publishing its run outcome;
+      // only the cached parent lineage should be needed at this point.
+      await handle.dispose()
       await settleSubagent(ctx, parentHandle.agent, {
         provider: 'spawn',
         id: SessionId('child-session'),
@@ -292,7 +295,6 @@ describe('HarnessSdkServer', () => {
         },
       })
 
-      await handle.dispose()
       await parentHandle.dispose()
       await server.shutdown()
     } finally {
@@ -301,7 +303,7 @@ describe('HarnessSdkServer', () => {
     }
   })
 
-  it('falls back to live agent lineage for uncached subagent end events', async () => {
+  it('falls back to live lineage and treats the shared id as the child session id', async () => {
     const storageDir = await mkdtemp(join(tmpdir(), 'dsh-jsonrpc-subagent-fallback-'))
     const ctx = await makeHarness(storageDir)
     let parentHandle: AgentHandle | undefined
@@ -365,10 +367,16 @@ describe('HarnessSdkServer', () => {
           stopReason: 'error',
         },
       })
-      expect(transport.notifications.some(n =>
-        n.method === 'subagent.finished'
-        && n.params?.agentId === 'missing-child-agent',
-      )).toBe(false)
+      expect(transport.notifications).toContainEqual({
+        method: 'subagent.finished',
+        params: {
+          provider: 'fork',
+          agentId: 'missing-child-agent',
+          childSessionId: 'missing-child-agent',
+          status: 'error',
+          stopReason: 'error',
+        },
+      })
 
       await server.shutdown()
     } finally {