Skip to content

Commit 0c379e5

Browse files
7418claude
andcommitted
fix: address tool_result dedup, SDK task ID collision, and doc accuracy
P1: tool_result dedup changed from "first wins" to "last wins" - claude-client.ts: remove emittedToolResultIds Set, both PostToolUse and user message handler emit freely. PostToolUse provides immediate UI feedback; user handler's potentially more complete result arrives second. - route.ts: collectStreamResponse replaces existing contentBlock entry when duplicate tool_use_id arrives (last wins for DB persistence). - ChatView.tsx: onToolResult replaces existing state entry by tool_use_id instead of skipping, ensuring most complete result is displayed. P2: SDK task ID collision risk eliminated - db.ts: syncSdkTasks now uses full sessionId in task ID (`sdk-{fullSessionId}-{index}`) instead of truncated 8-char prefix, eliminating cross-session primary key collisions. P2: Todo ordering stabilized - db.ts: add sort_order column migration, syncSdkTasks writes array index as sort_order. getTasksBySession sorts by sort_order ASC, created_at ASC so tasks with identical timestamps maintain insertion order. P3: Documentation field mapping corrected - agent-tooling-todo-bridge.md: fix field mapping table — actual implementation maps activeForm (not priority) to description column. Updated dedup strategy docs from "three-layer skip" to "two-layer last-wins replace". Lint: fix prefer-const for finalPrompt in claude-client.ts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent c13ff54 commit 0c379e5

File tree

5 files changed

+64
-54
lines changed

5 files changed

+64
-54
lines changed

docs/agent-tooling-todo-bridge.md

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,13 @@ SDK TodoWrite tool_use
1616

1717
## TodoWrite Field Mapping
1818

19-
| SDK TodoWrite field | Local tasks table column | Notes |
20-
|---------------------|-------------------------|-------|
21-
| `id` | `id` (prefixed `sdk-`) | Prefixed to avoid collision with user tasks |
22-
| `content` | `title` | Main display text |
23-
| `status` | `status` | Mapped via `mapStatus()` |
24-
| `priority` | `description` | Optional context |
19+
| SDK TodoWriteInput field | Local tasks table column | Notes |
20+
|--------------------------|-------------------------|-------|
21+
| (array index) | `id` (prefixed `sdk-{sessionId}-{index}`) | Full sessionId avoids collision |
22+
| `content` | `title` | Main display text |
23+
| `status` | `status` | Mapped via `mapStatus()` |
24+
| `activeForm` | `description` | Present-continuous form label |
25+
| (array index) | `sort_order` | Preserves original order |
2526

2627
### Status Mapping
2728

@@ -40,19 +41,20 @@ SDK TodoWrite tool_use
4041

4142
This is naturally idempotent — calling it multiple times with the same data produces identical results. User-created tasks (`source = 'user'`) are never affected.
4243

43-
## tool_result Three-Layer Dedup
44+
## tool_result Two-Layer Last-Wins Dedup
4445

45-
Both `PostToolUse` hook and `user` message handler in the SDK can emit the same `tool_result`. Three layers prevent duplicates:
46+
Both `PostToolUse` hook and `user` message handler in the SDK can emit the same `tool_result`.
47+
PostToolUse fires first (for immediate UI feedback); the user handler's result may be more
48+
complete/canonical. Strategy: **both sources emit freely; consumption layers use "last wins"
49+
(replace, not skip)** so the most complete result is always kept.
4650

47-
### Layer 1: claude-client.ts (SSE emission)
48-
- `emittedToolResultIds` Set — checks before enqueueing any `tool_result` SSE event
49-
- Both PostToolUse and user message handler check this Set
51+
### Layer 1: route.ts (DB persistence)
52+
- `seenToolResultIds` Set in `collectStreamResponse()` — when a duplicate `tool_use_id`
53+
arrives, the existing `contentBlock` entry is **replaced** with the newer result.
5054

51-
### Layer 2: route.ts (DB persistence)
52-
- `seenToolResultIds` Set in `collectStreamResponse()` — skips duplicate `tool_use_id` before writing to `contentBlocks`
53-
54-
### Layer 3: ChatView.tsx (UI state)
55-
- `onToolResult` checks `prev.some(r => r.tool_use_id === id)` before adding to state
55+
### Layer 2: ChatView.tsx (UI state)
56+
- `onToolResult` finds existing entry by `tool_use_id` and **replaces** it instead of
57+
appending a duplicate.
5658

5759
## SSE Event Types
5860

src/app/api/chat/route.ts

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -272,15 +272,24 @@ async function collectStreamResponse(stream: ReadableStream<string>, sessionId:
272272
} else if (event.type === 'tool_result') {
273273
try {
274274
const resultData = JSON.parse(event.data);
275-
// Dedup: skip if already seen
276-
if (!seenToolResultIds.has(resultData.tool_use_id)) {
275+
const newBlock = {
276+
type: 'tool_result' as const,
277+
tool_use_id: resultData.tool_use_id,
278+
content: resultData.content,
279+
is_error: resultData.is_error || false,
280+
};
281+
// Last-wins: if same tool_use_id already exists, replace it
282+
// (user handler's result may be more complete than PostToolUse's)
283+
if (seenToolResultIds.has(resultData.tool_use_id)) {
284+
const idx = contentBlocks.findIndex(
285+
(b) => b.type === 'tool_result' && 'tool_use_id' in b && b.tool_use_id === resultData.tool_use_id
286+
);
287+
if (idx >= 0) {
288+
contentBlocks[idx] = newBlock;
289+
}
290+
} else {
277291
seenToolResultIds.add(resultData.tool_use_id);
278-
contentBlocks.push({
279-
type: 'tool_result',
280-
tool_use_id: resultData.tool_use_id,
281-
content: resultData.content,
282-
is_error: resultData.is_error || false,
283-
});
292+
contentBlocks.push(newBlock);
284293
}
285294
} catch {
286295
// skip malformed tool_result data

src/components/chat/ChatView.tsx

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -326,8 +326,14 @@ export function ChatView({ sessionId, initialMessages = [], initialHasMore = fal
326326
markActive();
327327
setStreamingToolOutput('');
328328
setToolResults((prev) => {
329-
// Dedup: skip if already received for this tool_use_id
330-
if (prev.some(r => r.tool_use_id === res.tool_use_id)) return prev;
329+
// Last-wins: if same tool_use_id exists, replace with newer (potentially more complete) result
330+
const existingIdx = prev.findIndex(r => r.tool_use_id === res.tool_use_id);
331+
if (existingIdx >= 0) {
332+
const next = [...prev];
333+
next[existingIdx] = res;
334+
toolResultsRef.current = next;
335+
return next;
336+
}
331337
const next = [...prev, res];
332338
toolResultsRef.current = next;
333339
return next;

src/lib/claude-client.ts

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -451,10 +451,6 @@ export function streamClaude(options: ClaudeStreamOptions): ReadableStream<strin
451451
queryOptions.resume = sdkSessionId;
452452
}
453453

454-
// Track emitted tool_result IDs to prevent duplicates
455-
// (PostToolUse hook and user message handler can both emit for the same tool_use_id)
456-
const emittedToolResultIds = new Set<string>();
457-
458454
// Permission handler: sends SSE event and waits for user response
459455
queryOptions.canUseTool = async (toolName, input, opts) => {
460456
const permissionRequestId = `perm-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`;
@@ -525,20 +521,16 @@ export function streamClaude(options: ClaudeStreamOptions): ReadableStream<strin
525521
hooks: [async (input) => {
526522
const toolEvent = input as PostToolUseHookInput;
527523
console.log('[claude-client] PostToolUse:', toolEvent.tool_name, 'id:', toolEvent.tool_use_id);
528-
// Dedup: only emit if not already seen
529-
if (!emittedToolResultIds.has(toolEvent.tool_use_id)) {
530-
emittedToolResultIds.add(toolEvent.tool_use_id);
531-
controller.enqueue(formatSSE({
532-
type: 'tool_result',
533-
data: JSON.stringify({
534-
tool_use_id: toolEvent.tool_use_id,
535-
content: typeof toolEvent.tool_response === 'string'
536-
? toolEvent.tool_response
537-
: JSON.stringify(toolEvent.tool_response),
538-
is_error: false,
539-
}),
540-
}));
541-
}
524+
controller.enqueue(formatSSE({
525+
type: 'tool_result',
526+
data: JSON.stringify({
527+
tool_use_id: toolEvent.tool_use_id,
528+
content: typeof toolEvent.tool_response === 'string'
529+
? toolEvent.tool_response
530+
: JSON.stringify(toolEvent.tool_response),
531+
is_error: false,
532+
}),
533+
}));
542534

543535
// Detect TodoWrite tool and emit task_update SSE for frontend sync
544536
if (toolEvent.tool_name === 'TodoWrite') {
@@ -665,7 +657,7 @@ export function streamClaude(options: ClaudeStreamOptions): ReadableStream<strin
665657
return textPrompt;
666658
}
667659

668-
let finalPrompt = buildFinalPrompt(!shouldResume);
660+
const finalPrompt = buildFinalPrompt(!shouldResume);
669661

670662
// Try to start the conversation. If resuming a previous session fails
671663
// (e.g. stale/corrupt session file, CLI version mismatch), automatically
@@ -759,9 +751,6 @@ export function streamClaude(options: ClaudeStreamOptions): ReadableStream<strin
759751
if (Array.isArray(content)) {
760752
for (const block of content) {
761753
if (block.type === 'tool_result') {
762-
// Dedup: skip if already emitted by PostToolUse hook
763-
if (emittedToolResultIds.has(block.tool_use_id)) continue;
764-
emittedToolResultIds.add(block.tool_use_id);
765754
const resultContent = typeof block.content === 'string'
766755
? block.content
767756
: Array.isArray(block.content)

src/lib/db.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,9 @@ function migrateDb(db: Database.Database): void {
298298
if (!taskColNames.includes('source')) {
299299
db.exec("ALTER TABLE tasks ADD COLUMN source TEXT NOT NULL DEFAULT 'user'");
300300
}
301+
if (!taskColNames.includes('sort_order')) {
302+
db.exec("ALTER TABLE tasks ADD COLUMN sort_order INTEGER NOT NULL DEFAULT 0");
303+
}
301304

302305
// Ensure api_providers table exists for databases created before this migration
303306
db.exec(`
@@ -717,7 +720,7 @@ export function updateSessionStatus(id: string, status: 'active' | 'archived'):
717720

718721
export function getTasksBySession(sessionId: string): TaskItem[] {
719722
const db = getDb();
720-
return db.prepare('SELECT * FROM tasks WHERE session_id = ? ORDER BY created_at ASC').all(sessionId) as TaskItem[];
723+
return db.prepare('SELECT * FROM tasks WHERE session_id = ? ORDER BY sort_order ASC, created_at ASC').all(sessionId) as TaskItem[];
721724
}
722725

723726
export function getTask(id: string): TaskItem | undefined {
@@ -788,13 +791,14 @@ export function syncSdkTasks(
788791
// Delete all SDK-sourced tasks for this session
789792
db.prepare("DELETE FROM tasks WHERE session_id = ? AND source = 'sdk'").run(sessionId);
790793

791-
// Insert new SDK tasks
794+
// Insert new SDK tasks with stable sort_order
792795
const insert = db.prepare(
793-
'INSERT INTO tasks (id, session_id, title, status, description, source, created_at, updated_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?)'
796+
'INSERT INTO tasks (id, session_id, title, status, description, source, sort_order, created_at, updated_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)'
794797
);
795-
for (const todo of todos) {
796-
const taskId = `sdk-${sessionId.slice(0, 8)}-${todo.id}`;
797-
insert.run(taskId, sessionId, todo.content, mapStatus(todo.status), todo.activeForm || null, 'sdk', now, now);
798+
for (let i = 0; i < todos.length; i++) {
799+
const todo = todos[i];
800+
const taskId = `sdk-${sessionId}-${todo.id}`;
801+
insert.run(taskId, sessionId, todo.content, mapStatus(todo.status), todo.activeForm || null, 'sdk', i, now, now);
798802
}
799803
});
800804
txn();

0 commit comments

Comments
 (0)