From 4a3f3af296212d7275fbb5f80e37c3b1428c4551 Mon Sep 17 00:00:00 2001
From: Tianyi Cui <53024+tianyicui@users.noreply.github.com>
Date: Fri, 19 Jun 2026 00:37:12 +0800
Subject: [PATCH] Address Claude review follow-ups

---
 examples/acp-agent/start.ts                   |  2 ++
 packages/acp/src/index.ts                     | 21 ++++++++----
 packages/acp/tests/load.spec.ts               |  9 ++++-
 packages/agent-loop/src/loop.ts               |  9 +++++
 packages/agent-loop/tests/loop.spec.ts        | 34 ++++++++++++++++++-
 .../session-persistence-sqlite/src/index.ts   | 16 +++++++--
 packages/system-prompt/src/index.ts           |  9 +++--
 packages/tools/src/index.ts                   | 11 +++---
 8 files changed, 92 insertions(+), 19 deletions(-)

diff --git a/examples/acp-agent/start.ts b/examples/acp-agent/start.ts
index 7c21ed9262..61a7241c80 100644
--- a/examples/acp-agent/start.ts
+++ b/examples/acp-agent/start.ts
@@ -18,6 +18,8 @@ try {
   // ENOENT (no .env) is fine — rely on the ambient environment.
 }
 
+// Resolve relative cordis.yml paths from the repo root no matter where the
+// editor launches this demo command.
 process.chdir(fileURLToPath(new URL('../..', import.meta.url)))
 
 const ctx = new Context()
diff --git a/packages/acp/src/index.ts b/packages/acp/src/index.ts
index c24b294d32..97a3d5b0a2 100644
--- a/packages/acp/src/index.ts
+++ b/packages/acp/src/index.ts
@@ -100,6 +100,10 @@ function internalError(detail: string): RequestError {
   return RequestError.internalError(undefined, detail)
 }
 
+function sameWorkspaceCwd(left: string, right: string): boolean {
+  return resolvePath(left) === resolvePath(right)
+}
+
 /** Plugin config: the agent template ACP sessions are created from. */
 export interface AcpConfig {
   /** Model name for created agents (must have a registered adapter). */
@@ -466,13 +470,16 @@ export function apply(ctx: Context, config: AcpConfig): void {
           // (An id unknown to `list()` falls through to resume, which rejects with
           // the backend's not-found error.)
           const meta = (await sessionPersistence.list()).find(m => m.id === params.sessionId)
-          if (meta !== undefined && (meta.cwd === undefined || !isAbsolute(meta.cwd))) {
-            throw invalidParams(
-              `session ${params.sessionId} has no absolute persisted cwd; cannot determine its workspace (it predates per-session cwd, or was created without one)`,
-            )
-          }
-          if (meta !== undefined && meta.cwd !== params.cwd) {
-            throw invalidParams(`session ${params.sessionId} cwd mismatch: persisted ${meta.cwd}, requested ${params.cwd}`)
+          if (meta !== undefined) {
+            const persistedCwd = meta.cwd
+            if (persistedCwd === undefined || !isAbsolute(persistedCwd)) {
+              throw invalidParams(
+                `session ${params.sessionId} has no absolute persisted cwd; cannot determine its workspace (it predates per-session cwd, or was created without one)`,
+              )
+            }
+            if (!sameWorkspaceCwd(persistedCwd, params.cwd)) {
+              throw invalidParams(`session ${params.sessionId} cwd mismatch: persisted ${persistedCwd}, requested ${params.cwd}`)
+            }
           }
           const agent = await agents.resume({
             agentId: params.sessionId,
diff --git a/packages/acp/tests/load.spec.ts b/packages/acp/tests/load.spec.ts
index 55d56fa09c..f06c707d85 100644
--- a/packages/acp/tests/load.spec.ts
+++ b/packages/acp/tests/load.spec.ts
@@ -178,7 +178,7 @@ describe('acp bridge — session/load replay', () => {
       .rejects.toThrow(/cwd mismatch/)
     expect(loader.ctx.agents.get('elsewhere')).toBeUndefined()
 
-    const res = await loader.client.loadSession({ sessionId: 'elsewhere', cwd: otherCwd, mcpServers: [] })
+    const res = await loader.client.loadSession({ sessionId: 'elsewhere', cwd: `${otherCwd}/.`, mcpServers: [] })
     expect(res).toBeDefined()
     expect(loader.ctx.agents.get('elsewhere')!.session.header.cwd).toBe(otherCwd)
   })
@@ -190,6 +190,13 @@ describe('acp bridge — session/load replay', () => {
       .rejects.toThrow(/absolute/)
   })
 
+  it('lets persistence reject a load for an unknown id after metadata lookup misses', async () => {
+    loader = await makeBridgeHarness({ storageDir, script: [] })
+    await loader.client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: {} })
+    await expect(loader.client.loadSession({ sessionId: 'missing', cwd: process.cwd(), mcpServers: [] }))
+      .rejects.toThrow(/Internal error/)
+  })
+
   it('rejects loading a persisted session that has NO cwd (would silently run in the launch dir)', async () => {
     // A legacy / externally-created session log with no header.cwd. The bridge
     // must reject the load rather than accept it and let bash silently fall back
diff --git a/packages/agent-loop/src/loop.ts b/packages/agent-loop/src/loop.ts
index 51618585b0..d1ea117811 100644
--- a/packages/agent-loop/src/loop.ts
+++ b/packages/agent-loop/src/loop.ts
@@ -502,6 +502,11 @@ async function runStep(
   if (stepError) throw stepError
 
   if (assembler.finish.kind === 'max-tokens') {
+    let message: Message = withoutToolCalls(assembler.message())
+    message = withoutToolCalls(await ctx.waterfall('agent/step-result', agent, turn, step, message, () => Promise.resolve(message)))
+    if (message.content.length > 0) {
+      session.append('assistant/message', { turn, step, content: message.content })
+    }
     if (assembler.usage) {
       session.append('usage', { turn, step, usage: assembler.usage })
     }
@@ -566,6 +571,10 @@ async function runStep(
   return { hadToolCalls: toolCalls.length > 0, finish: assembler.finish }
 }
 
+function withoutToolCalls(message: Message): Message {
+  return { ...message, content: message.content.filter(block => block.type !== 'tool-call') }
+}
+
 /** The last turn number in a (possibly seeded) session log, or 0. */
 export function lastTurnNumber(session: Session): number {
   const lastStart = session.events.findLast(event => event.type === 'turn/start')
diff --git a/packages/agent-loop/tests/loop.spec.ts b/packages/agent-loop/tests/loop.spec.ts
index 3f3f52c48e..c4f09a24bf 100644
--- a/packages/agent-loop/tests/loop.spec.ts
+++ b/packages/agent-loop/tests/loop.spec.ts
@@ -385,6 +385,10 @@ describe('agent loop', () => {
 
     expect(steps).toBe(2)
     expect(adapter.requests).toHaveLength(2)
+    expect(adapter.requests[1]!.messages).toEqual([
+      { role: 'user', content: [{ type: 'text', text: 'go' }] },
+      { role: 'assistant', content: [{ type: 'text', text: 'first half' }] },
+    ])
     expect(reasons).toEqual([{ kind: 'max-tokens' }])
   })
 
@@ -436,10 +440,38 @@ describe('agent loop', () => {
 
     expect(executions).toBe(0)
     expect(agent.session.events.some(e => e.type === 'tool/call')).toBe(false)
+    expect(agent.session.deriveMessages()).toEqual([{ role: 'user', content: [{ type: 'text', text: 'go' }] }])
+    expect(reasons).toEqual([{ kind: 'max-tokens' }])
+  })
+
+  it('keeps safe max-tokens assistant content while dropping truncated tool calls', async () => {
+    const callId = CallId('c1')
+    const adapter = new MockAdapter([[
+      { type: 'block-start', index: 0, blockType: 'text' },
+      { type: 'text-delta', index: 0, text: 'partial text' },
+      { type: 'block-end', index: 0, block: { type: 'text', text: 'partial text' } },
+      { type: 'block-start', index: 1, blockType: 'tool-call' },
+      { type: 'tool-call-delta', index: 1, id: callId, name: 'echo', argumentsDelta: '{"text"' },
+      { type: 'finish', reason: { kind: 'max-tokens' } },
+    ]])
+    const ctx = await harness(adapter)
+    let stepResults = 0
+    ctx.on('agent/step-result', async (_agent, _turn, _step, message, next) => {
+      stepResults += 1
+      expect(message.content).toEqual([{ type: 'text', text: 'partial text' }])
+      return next()
+    })
+    const agent = ctx.agentLoop.create('a1', { model: 'mock' })
+
+    send(agent, 'go')
+    await waitForIdle(ctx, agent)
+
+    expect(stepResults).toBe(1)
+    expect(agent.session.events.some(e => e.type === 'tool/call')).toBe(false)
     expect(agent.session.deriveMessages()).toEqual([
       { role: 'user', content: [{ type: 'text', text: 'go' }] },
+      { role: 'assistant', content: [{ type: 'text', text: 'partial text' }] },
     ])
-    expect(reasons).toEqual([{ kind: 'max-tokens' }])
   })
 
   it('stops the turn when agent/step-end listener failure has recorded an error', async () => {
diff --git a/packages/session-persistence-sqlite/src/index.ts b/packages/session-persistence-sqlite/src/index.ts
index c791231221..7f59e3dd8a 100644
--- a/packages/session-persistence-sqlite/src/index.ts
+++ b/packages/session-persistence-sqlite/src/index.ts
@@ -448,6 +448,7 @@ export class SessionPersistenceSqlite extends SessionPersistence {
     // Dispose must reach quiescence: await every init + final drain, then close
     // the database, BEFORE returning, so no write lands after teardown.
     ctx.effect(() => async () => {
+      let disposeError: unknown
       try {
         const errors = [
           ...await settledErrors(this.inits.values()),
@@ -457,9 +458,20 @@ export class SessionPersistenceSqlite extends SessionPersistence {
         if (errors.length > 0) {
           throw new AggregateError(errors, 'session-persistence-sqlite dispose failed')
         }
+      } catch (error: unknown) {
+        disposeError = error
+        throw error
       } finally {
-        await this.ready
-        this.db.close()
+        try {
+          await this.ready
+          this.db.close()
+        } catch (error: unknown) {
+          /* v8 ignore next -- open/close failure racing disposal is a defensive teardown edge */
+          if (disposeError === undefined) throw error
+          // Opening/closing the database can only add teardown context here; keep
+          // the already-captured init/flush/chain AggregateError as the primary
+          // disposal failure instead of masking it from callers.
+        }
       }
     }, 'session-persistence-sqlite write path')
 
diff --git a/packages/system-prompt/src/index.ts b/packages/system-prompt/src/index.ts
index b2f3bd3db4..25d87d2194 100644
--- a/packages/system-prompt/src/index.ts
+++ b/packages/system-prompt/src/index.ts
@@ -116,9 +116,12 @@ export class SystemPrompt extends Service {
 
   /**
    * Assemble the current prompt (sections sorted by order, tools collected
-   * from all providers). Runs through the `system-prompt/assemble` waterfall,
-   * giving listeners the opportunity to mutate or replace the assembly before
-   * it reaches the model. Await the result before reading the assembly values —
+   * from all providers). Section records are top-level clones (the `text`
+   * provider may be a function and is intentionally shared); tool schemas are
+   * deep-cloned because adapters and request waterfalls may mutate schema
+   * objects. Runs through the `system-prompt/assemble` waterfall, giving
+   * listeners the opportunity to mutate or replace the assembly before it
+   * reaches the model. Await the result before reading the assembly values —
    * waterfall listeners may be async.
    */
   assemble(): Promise<PromptAssembly> {
diff --git a/packages/tools/src/index.ts b/packages/tools/src/index.ts
index aa2d5dccaf..eee77445eb 100644
--- a/packages/tools/src/index.ts
+++ b/packages/tools/src/index.ts
@@ -332,11 +332,12 @@ export class ToolRegistry extends Service {
   }
 
   /**
-   * Execute one tool call through the `tools/execute` waterfall. If the tool
-   * is not registered, the result is an `isError` carrying a `UNKNOWN_TOOL`
-   * structured error. If the tool throws, the error is caught and returned as
-   * an `isError` result so the loop never sees an uncaught exception; a thrown
-   * {@link HarnessError} surfaces its `{ name, code }` on the result.
+   * Execute one tool call through the `tools/execute` waterfall. If the tool is
+   * not registered, the result is an `isError` carrying a `UNKNOWN_TOOL`
+   * structured error. If the tool or a waterfall listener throws, the error is
+   * caught and returned as an `isError` result so the loop records a failed tool
+   * call instead of failing the whole turn; a thrown {@link HarnessError}
+   * surfaces its `{ name, code }` on the result.
    */
   async execute(exec: ToolExecution): Promise<ToolExecutionResult> {
     try {