Fix orphan tool results - #845
Conversation
Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
|
Verifiable bug: The docs in {"type":"message","message":{"role":"assistant","content":[{"type":"toolCall","id":"t1"}]}}
{"type":"compaction","id":"c1"}
{"type":"message","message":{"role":"toolResult","toolCallId":"t1","content":"late result"}}Expected: Suggested fix: clear retained tool-call ids when encountering |
A `compaction` entry marks a fresh retained-context boundary — the assistant turns before it have been summarized away and are no longer in the retained context rehydrated for the provider. The sanitizer previously kept `seenToolCallIds` across the compaction marker, so a post-compaction `toolResult` could incorrectly match a pre-compaction assistant tool call and be kept as valid history, rehydrating an orphan `function_call_output` with no matching call. Treat the compaction marker as a fresh retained-context boundary by clearing `seenToolCallIds` when one is encountered. Add regression tests: - assistant tool call before compaction marker + matching toolResult after marker is dropped (the PR #845 scenario); - valid toolCall + toolResult pair both after the marker stays byte-identical. Preserves all existing behavior and diagnostics. Addresses PR #845 review comment. Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
|
Thanks, this was correct. I fixed to treat as a retained-context boundary by clearing tracked tool-call IDs when the marker is encountered. Added regressions for:
Validation passed:
Built dist/server/defaults/ from defaults/
[no-new-sleeps] 1 file(s) below baseline — run with --update to ratchet: Running 2 tests using 1 worker �[1A�[2K[1/2] [api] › tests/e2e/orphan-tool-results.spec.ts:158:2 › orphan tool-result restore lifecycle › restore repairs persisted orphan tool results, is idempotent, and leaves the session usable �[1A�[2KCreated .bobbit/ in /private/var/folders/qb/7js4lg914yqglb4_y8ctkz5r0000gn/T/bobbit-e2e/.e2e-inproc-37148-0-1782121902600. Customize roles, workflows, and system prompt in .bobbit/config/ �[1A�[2K[migration] Starting per-project state migration. Default project: "System" (system) �[1A�[2K[migration] Distributed 0 goals across 0 project(s) �[1A�[2K[migration] Distributed 0 sessions across 0 project(s) �[1A�[2K[migration] Distributed 0 tasks �[1A�[2K[migration] Distributed 0 teams �[1A�[2K[migration] Distributed 0 gate states �[1A�[2K[migration] Per-project state migration complete. Marker written. �[1A�[2K[trigger-engine] Started (60s poll interval) �[1A�[2K[aigw-manager] Wrote models.json to /private/var/folders/qb/7js4lg914yqglb4_y8ctkz5r0000gn/T/bobbit-e2e/.e2e-inproc-37148-0-1782121902600/agent/models.json �[1A�[2K[aigw-manager] Wrote 48 contextWindow overrides to models.json �[1A�[2K[team-manager] Re-subscribed to events for 0 team(s) �[1A�[2K[boot] sweeper start (0 projects) �[1A�[2K[boot] sweeper done in 10ms (reclaimed=0 cleaned=0 repaired=0) �[1A�[2K[boot] background tasks complete in 10ms �[1A�[2K[session-manager] Restoring 1 session(s) + 0 delegate(s) live... �[1A�[2K[transcript-sanitizer] dropped 1 orphan tool result row(s) in /private/var/folders/qb/7js4lg914yqglb4_y8ctkz5r0000gn/T/bobbit-e2e/.e2e-inproc-37148-0-1782121902600/agent/sessions/-private-var-folders-qb-7js4lg914yqglb4-y8ctkz5r0000gn-T-bob/2026-06-22T09-51-44-369Z_ef2118c6-8ebd-4a7b-8c4b-48fbc5e96072.jsonl �[1A�[2K[session-manager] Restoring 1 session(s) + 0 delegate(s) live... �[1A�[2K[2/2] [api] › tests/e2e/orphan-tool-results.spec.ts:206:2 › orphan tool-result restore lifecycle › generated OpenAI Responses guard hook drops only orphan function_call_output items �[1A�[2K[trigger-engine] Stopped �[1A�[2K[sandbox-manager] All 0 sandbox(es) shut down �[1A�[2K 2 passed (2.7s) |
|
Correction to my previous reply (shell quoting mangled the Markdown): the PR comment was correct. Fixed in
Added regressions:
Validation passed:
|
| function toolCallIdFromAssistantBlock(block: unknown): string | null { | ||
| if (!block || typeof block !== "object") return null; | ||
| const b = block as any; | ||
| if (b.type === "toolCall" || b.type === "tool_use") return stringField(b.id); |
There was a problem hiding this comment.
This extractor is narrower than the tolerant transcript shapes used elsewhere. For type: "toolCall" / "tool_use" it returns only b.id, so a valid block shaped like { type: "toolCall", toolCallId: "t1", ... } will not register as a producer and the matching role: "toolResult" row is dropped as orphaned. Existing tolerant code in src/server/extension-host/action-guard.ts uses b.id ?? b.toolCallId ?? b.tool_use_id; this should do the same before deciding the result is orphaned.
| // the retained tool-call ids so a post-compaction `toolResult` cannot | ||
| // incorrectly match a pre-compaction assistant tool call (which would | ||
| // rehydrate an orphan `function_call_output` with no matching call). | ||
| if (entry?.type === "compaction") seenToolCallIds.clear(); |
There was a problem hiding this comment.
Clearing every seen tool-call id at a compaction marker can drop valid retained history. pi compaction records firstKeptEntryId and rehydrates the summary plus messages from firstKeptEntryId through the compaction entry, then messages after it. If an assistant tool call before the compaction marker is still in that retained range, a later toolResult for it is still valid, but this reset makes the sanitizer drop it as orphaned. The sanitizer needs to model the retained range from the compaction entry, or otherwise avoid clearing IDs for retained pre-compaction calls.
Address PR #845 review comments (two parts): 1. toolCallIdFromAssistantBlock() now tolerantly resolves the id of typed toolCall/tool_use blocks via id ?? toolCallId ?? tool_use_id, matching src/server/extension-host/action-guard.ts. Previously it only accepted id for typed blocks, so a block carrying toolCallId or tool_use_id instead of id was skipped and its matching toolResult wrongly dropped as orphaned. Add regressions for typed blocks using toolCallId/tool_use_id. 2. Model retained ranges at compaction boundaries instead of blindly clearing seen tool-call ids. When a compaction entry carries a resolvable firstKeptEntryId (matches a kept assistant entry id seen before the marker), drop only the ids registered strictly before that kept entry; keep those at/after it. Fall back to clearing at the marker when firstKeptEntryId is absent or unresolvable. Add tests proving a kept-range assistant tool call before the marker validates a later result, while a call before firstKeptEntryId cannot. Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
Co-authored-by: bobbit-ai <bobbit@bobbit.ai>
|
Addressed the two latest sanitizer review comments in What changed:
Added regressions for:
Validation passed:
|
2 participants
Summary
Validation
🤖 Generated with Bobbit