Skip to content

fix: preserve thinking blocks when repairing tool pairs after compaction - #263

Merged
griffinmartin merged 3 commits into
griffinmartin:mainfrom
anneal-it:fix/tool-pair-repair-thinking-safe
Aug 3, 2026
Merged

fix: preserve thinking blocks when repairing tool pairs after compaction#263
griffinmartin merged 3 commits into
griffinmartin:mainfrom
anneal-it:fix/tool-pair-repair-thinking-safe

Conversation

@cdbattags

Copy link
Copy Markdown
Collaborator

Closes #261
Closes #212
Closes #226

Problem

OpenCode's automatic compaction can orphan a tool_use whose matching
tool_result is no longer in the immediately-following message.
repairToolPairs() (even with the adjacency fix from #250) mishandles two shapes:

  1. An orphaned tool_use in an assistant turn that also holds a thinking
    block → the function drops the tool_use but keeps the thinking, a partial
    rewrite Anthropic rejects with "thinking ... blocks ... cannot be modified" (repairToolPairs() corrupts thinking blocks → Anthropic 400 on long conversations #261).
  2. A duplicate/replayed tool_use id (first occurrence a valid pair, a later one
    orphaned) → the needsRepair gate only inspects first occurrences, so the
    orphan slips through as "tool_use ids ... without tool_result blocks
    immediately after" (Each tool_use block must have a corresponding tool_result block in the next message. #212/Sonnet rejects some /undo + /compact sessions that GPT still accepts #226).

Proven against the current release

Wire shape Result Error
thinking + orphaned tool_use, then assistant drops tool_use, keeps thinking #261
duplicate tool_use id (2nd orphaned) orphan survives (needsRepair=false) #212

Fix

Add OPENCODE_CLAUDE_AUTH_TOOL_REPAIR to select the strategy:

Emits a redacted repair_orphan_dropped / repair_orphan_synthesized debug event
(ids + indices only, no message content) under CLAUDE_AUTH_DEBUG.

Both shapes above now pass in both modes; new tests added, full suite green.

OpenCode's automatic compaction can orphan a tool_use whose matching
tool_result is no longer in the immediately-following message. On
thinking-enabled models every assistant turn also carries a thinking
block, so the existing drop-based repairToolPairs() partially rewrote
those turns and Anthropic rejected the request (issue griffinmartin#261), while other
compaction shapes slipped through unrepaired and produced the classic
"tool_use ids ... without tool_result blocks immediately after" 400
(issues griffinmartin#212/griffinmartin#226).

Add OPENCODE_CLAUDE_AUTH_TOOL_REPAIR to select the strategy:

- placeholder (default): synthesize a paired placeholder tool_result for
  every orphaned tool_use. Assistant content[] is never mutated, so
  thinking/redacted_thinking blocks stay byte-identical.
- drop: harden the previous behavior — evaluate adjacency per-occurrence
  (so a duplicate/replayed tool_use id no longer masks a later orphan),
  omit whole thinking turns instead of partially rewriting them, and
  iterate to a fixed point to reconcile cascades.

Emit a redacted repair_orphan_dropped / repair_orphan_synthesized debug
event (ids + indices only, no message content) when CLAUDE_AUTH_DEBUG is
enabled, so future occurrences are diagnosable.
@griffinmartin

Copy link
Copy Markdown
Owner

@greptileai

@greptile-apps

greptile-apps Bot commented Aug 2, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds a configurable OPENCODE_CLAUDE_AUTH_TOOL_REPAIR env-var to select between two strategies for fixing orphaned tool_use/tool_result adjacency after OpenCode auto-compaction: the new default placeholder mode synthesizes a paired error result without ever modifying assistant content (preserving thinking blocks byte-for-byte), and the hardened drop mode drops whole assistant thinking turns rather than partially rewriting them (avoiding Anthropic's thinking-preservation rejection) and evaluates adjacency per-occurrence to catch duplicate-id orphans.

  • synthesizeMissingToolResults (placeholder): two-pass approach — pass 1 strips orphaned tool_result blocks from user turns; pass 2 walks pass1 and inserts synthetic placeholder tool_result entries adjacent to any uncovered tool_use, merging into an existing user turn or inserting a new one.
  • repairToolPairs (drop): refactored into a dropPass helper iterated to a fixed point; each pass evaluates adjacency per-occurrence and drops whole thinking turns when any orphan is found, with cascade cleanup handled on subsequent passes.
  • New exports (applyToolRepair, resolveToolRepairMode, synthesizeMissingToolResults, TOOL_RESULT_PLACEHOLDER) are thoroughly covered by new tests including duplicate-id, thinking-block, plain-string-content, and diagnostic-logging cases.

Confidence Score: 5/5

Safe to merge — both repair strategies are logically correct and the thinking-block invariant is upheld in all tested shapes.

The two-pass placeholder strategy correctly never touches assistant content, preserving thinking blocks byte-for-byte. The drop strategy's per-occurrence adjacency evaluation closes the duplicate-id gap, and the fixed-point iteration cleanly handles cascade removals within the proven message-length bound. All described failure shapes (thinking+orphan, duplicate ID, cascade) are covered by new tests, and the existing suite was updated to match the new API. No correctness gaps were found.

Files Needing Attention: No files require special attention.

Important Files Changed

Filename Overview
src/transforms.ts Core repair logic rewritten into two well-separated strategies; adjacency helpers are clear and correct; the fixed-point iteration in repairToolPairs is properly bounded; thinking-block invariant is upheld by both paths.
src/transforms.test.ts Comprehensive new tests added for both modes, diagnostic logging, resolveToolRepairMode, and applyToolRepair dispatch; existing drop-mode test correctly updated to pass explicit mode argument.
README.md Documents the new OPENCODE_CLAUDE_AUTH_TOOL_REPAIR variable with accurate description and default value.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[transformBody called] --> B{mode?}
    B -->|placeholder default| C[synthesizeMissingToolResults]
    B -->|drop| D[repairToolPairs]

    C --> C1[Pass 1: scan user turns\nremove orphaned tool_result blocks\nusing original message indices]
    C1 --> C2[Pass 2: walk pass1\nfor each assistant tool_use\ncheck adjacent user turn for result]
    C2 --> C3{result present\nin next turn?}
    C3 -->|yes| C4[no-op, continue]
    C3 -->|no, user turn follows| C5[merge placeholder\ninto existing user turn\nskip that turn i++]
    C3 -->|no, no user turn| C6[insert new user turn\nwith placeholder result]
    C5 --> C7[log repair_orphan_synthesized]
    C6 --> C7
    C4 --> C8[return output\nassistant content NEVER modified]
    C7 --> C8

    D --> D1[dropPass on current messages]
    D1 --> D2{message has\nthinking block AND\norphaned tool_use?}
    D2 -->|yes| D3[drop entire turn\nlog omittedThinkingTurns]
    D2 -->|no| D4[filter per-block\nadjacency check per-occurrence\ncatches duplicate IDs]
    D3 --> D5{changed?}
    D4 --> D5
    D5 -->|yes, iterate| D1
    D5 -->|no, fixed point| D6[return result]
    D5 -->|maxIterations exceeded| D7[log repair_drop_max_iterations\nreturn current]
Loading

Reviews (4): Last reviewed commit: "fix: handle string-content adjacent user..." | Re-trigger Greptile

Comment thread src/transforms.ts
Comment thread src/transforms.ts
@cdbattags
cdbattags marked this pull request as ready for review August 3, 2026 15:27
…(review)

synthesizeMissingToolResults merged synthetic results by reassigning
pass1[i+1] while the loop still iterated pass1. It was correct (the write at i
always preceded the read at i+1), but the coupling was an implicit landmine.
Build `out` directly instead: push the merged user turn and skip its index.

Also log repair_drop_max_iterations before repairToolPairs returns after
exhausting its iteration cap, so a hypothetical non-converging shape surfaces
on the existing debug channel instead of returning silently.
…n loss (review)

Two spots Greptile flagged in synthesizeMissingToolResults / dropPass:

- Placeholder Pass 2: when the tool_use's adjacent turn is a plain-string user
  message, the synthetic result was emitted as a separate user message, leaving
  two consecutive user turns. Convert the string turn to blocks so the
  tool_result leads it as a single user turn instead. Added a test (and updated
  the transformBody placeholder test for the merged shape).

- dropPass omits an entire thinking turn when it holds an orphaned tool_use,
  which also discards any valid tool_use in that turn (its result is then pruned
  on the next fixed-point pass). This is inherent to griffinmartin#261 in `drop` mode;
  documented the trade-off and added tests covering the mixed valid+orphan turn
  in both modes (drop leaves no orphans; placeholder preserves the turn and
  synthesizes only the missing result).
@cdbattags

Copy link
Copy Markdown
Collaborator Author

Addressed the two spots from the review summary in 46b5176:

  • synthesizeMissingToolResults Pass 2 / string-content user turns — when the tool_use's adjacent turn is a plain-string user message, the synthetic result was emitted as a separate user message (two consecutive user turns). It now converts that turn to blocks so the tool_result leads it as a single user turn. Added a test + updated the transformBody placeholder test for the merged shape.
  • dropPass thinking-turn omission with a valid+orphaned mix — omitting the whole turn also discards any valid tool_use it carries (its result is pruned on the next fixed-point pass). This is inherent to the repairToolPairs() corrupts thinking blocks → Anthropic 400 on long conversations #261 no-partial-rewrite rule in drop mode; documented the trade-off and added tests for both modes (drop leaves no orphans; the default placeholder preserves the turn and synthesizes only the missing result).

@griffinmartin
griffinmartin merged commit 8de49c8 into griffinmartin:main Aug 3, 2026
5 checks passed
cdbattags added a commit to anneal-it/opencode-claude-auth that referenced this pull request Aug 3, 2026
…iagnostics

* upstream/main:
  fix: preserve thinking blocks when repairing tool pairs after compaction (griffinmartin#263)

# Conflicts:
#	README.md
griffinmartin pushed a commit that referenced this pull request Aug 3, 2026
🤖 I have created a release *beep* *boop*
---


##
[2.1.6](v2.1.5...v2.1.6)
(2026-08-03)


### Bug Fixes

* make OAuth refresh resilient to transient rate-limits (+ diagnostics)
([#264](#264))
([5532c37](5532c37))
* preserve thinking blocks when repairing tool pairs after compaction
([#263](#263))
([8de49c8](8de49c8))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
MaxAnderson95 added a commit to MaxAnderson95/opencode-claude-auth that referenced this pull request Aug 4, 2026
Brings in upstream v2.1.5 and v2.1.6: OAuth refresh over the runtime's native fetch instead of a spawned subprocess (griffinmartin#258), external Claude Code credential rotation handling with compare-and-swap writeback and a bounded 401 recovery loop (griffinmartin#260), thinking-block-safe tool-pair repair after compaction (griffinmartin#263), and transient-vs-terminal refresh classification with backoff plus a cross-process refresh lock (griffinmartin#264).

Two fork patches are dropped because upstream now solves the same problems better. `resolveJsRuntime` and the OAuth subprocess envelope are gone: upstream griffinmartin#258 removes the subprocess entirely, so there is no `process.execPath` runtime to resolve and the whole failure mode that patch existed for no longer exists. The slow-path credential store re-read in `refreshIfNeeded` is gone too: upstream griffinmartin#260 re-reads every source unconditionally and guards the writeback with a compare-and-swap, which is strictly stronger than re-reading only before spending a refresh. Dead `clearCredentialCache` is removed in favor of upstream's `invalidateCredentialCache`.

Five fork patches are retained. Billing identity stays `cli` rather than upstream's `sdk-cli`, in both the user-agent and the `CLAUDE_CODE_ENTRYPOINT` default, which is the reason this fork exists. Expiry timestamps are still floored to integers at the two sinks that write them (`syncToPath` and the auth loader callback), since credentials read straight from the keychain bypass `parseOAuthResponse`'s `Math.trunc`. SSE stream lifecycle instrumentation and the append-only debug log are re-applied on top of upstream's stream transform, which was unchanged by griffinmartin#263. The `claude` CLI is still resolved to an absolute path with stderr captured, because upstream reverted to a bare `execSync("claude ...")` that cannot find the binary under a launchd-managed server's minimal PATH.

The child-process test harness is updated for the retained CLI patch: it now rewrites the two-symbol `node:child_process` import, treats a spawned CLI as succeeding rather than throwing (nothing calls `execFileSync` for OAuth anymore), counts POSIX `execFileSync` and Windows `execSync` spawns together via `__getCliSpawnCount`, and pins `CLAUDE_CLI_PATH` so binary resolution never probes the host filesystem.

337 tests pass, typecheck and lint are clean, and the built plugin loads.

OpenCode session ID: ses_035a6417cffeukveuaZsQUEJFU
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants