-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix(web-search): replay executed bridge searches to the destination #4919
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
131 changes: 131 additions & 0 deletions
131
devlog/_plan/260918_ld_search_bridge_and_thinking_replay/010_roadmap.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| # LD — bridged web-search replay and Anthropic thinking replay | ||
|
|
||
| Delivery lane R3-LD. The completion bar for this lane is not "one turn | ||
| succeeded" but "the next turn inherits exactly the same result". Local test | ||
| execution is prohibited here: every claim below is backed by source reading and | ||
| by hosted CI at an exact head. | ||
|
|
||
| ## Units | ||
|
|
||
| | Unit | Item | Kind | Write scope | | ||
| |---|---|---|---| | ||
| | U1 | #4587 bridged hosted `web_search` result is not replayed to the destination | new implementation | `src/responses/bridge-search-replay-cache.ts` (new), `src/web-search/passthrough-bridge.ts`, `src/adapters/openai-responses/passthrough.ts`, `src/adapters/openai-responses/tool-output-recovery.ts`, `structure/` owner doc, one new test file plus its two layout entries | | ||
| | U2 | #4429 key-auth Responses gateway echoes hosted `web_search` as a client `function_call` | re-judge at HEAD, closure recommendation | none — assessment only | | ||
| | U3 | #3719 Anthropic thinking/`redacted_thinking` replay through proxy-auth translation | verify at HEAD, closure recommendation | none — assessment only | | ||
| | U4 | #3952, #4783, #4900 | review only, scoped | none | | ||
|
|
||
| U1 is the only unit that writes code. U2 and U3 are re-judged against current | ||
| source because both issue bodies predate the commits that changed the answer; | ||
| the host owns every close decision. | ||
|
|
||
| ## U1 — #4587 | ||
|
|
||
| ### What is actually broken | ||
|
|
||
| `src/web-search/passthrough-bridge.ts` intercepts the destination's | ||
| `function_call` named `web_search`, runs the search proxy-side, and shows the | ||
| caller a hosted `web_search_call` cell whose id is a proxy-minted | ||
| `ws_<uuid>`. Two paths reach that state: | ||
|
|
||
| - `endAfterSearch` (a leg mixing the search with a client-executed tool call) | ||
| ends the turn on that leg, so no continuation carries the result upstream. | ||
| - The ordinary continuation path does re-POST `function_call` + | ||
| `function_call_output` to the destination through `appendBridgeSearchTurn`, | ||
| but only inside that turn. | ||
|
|
||
| In both cases the caller's own history now holds a hosted `web_search_call` | ||
| item. On the next turn it replays that item, and the destination receives an | ||
| item type it never produced, with no result text and no matching | ||
| `function_call`/`function_call_output` pair. The observable effect is a wasted | ||
| round trip: the model usually searches again. | ||
|
|
||
| ### Where the fix has to live | ||
|
|
||
| Not in the bridge. By the time the bridge wraps a turn, that turn's first leg | ||
| is already on the wire, so the rewrite must happen before dispatch. The | ||
| existing pre-dispatch rewrites of outbound `input` are | ||
| `backfillWebSearchQueries` and `repairOrphanedInputItems` in | ||
| `src/adapters/openai-responses/tool-output-recovery.ts`, applied from | ||
| `src/adapters/openai-responses/passthrough.ts`. The restore joins them there. | ||
|
|
||
| ### The memo | ||
|
|
||
| A process-local, bounded, expiring memo records what the bridge executed: | ||
|
|
||
| - Key: the destination identity (the existing salted digest of the provider | ||
| base URL from `src/responses/reasoning-replay-cache.ts`) plus the synthesized | ||
| cell item id. The cell id is a v4 UUID this proxy mints, so it cannot collide | ||
| across conversations; the destination scope is what stops one provider's | ||
| executed call from being replayed into another provider's conversation. | ||
| - Value: the destination's `call_id`, its original item id, the original | ||
| arguments text, and the executed result text. | ||
| - Bounds: entry count, total bytes, and TTL, following the discipline already | ||
| established by the reasoning replay cache. Result text lives in memory only | ||
| and is never logged, serialized, or exported. | ||
|
|
||
| ### The invariant that matters most | ||
|
|
||
| **A memo miss is a no-op.** If the cell id is unknown, expired, or was recorded | ||
| against a different destination, the replayed `web_search_call` item is left | ||
| exactly as it is. The lane must never re-run the search to recover a lost | ||
| result, and must never synthesize result text. Re-running would bill a second | ||
| search the caller did not ask for and would answer the model with a different | ||
| search than the one its history claims; synthesizing would put words in the | ||
| destination's own mouth. Both are the easiest wrong fix available here, and | ||
| neither is permitted. | ||
|
|
||
| ### Regression pins | ||
|
|
||
| 1. A replayed `web_search_call` whose id is in the memo becomes the | ||
| destination's `function_call` followed by its `function_call_output`, in | ||
| that order, at the item's original position. | ||
| 2. A replayed `web_search_call` with no memo entry is byte-identical to the | ||
| input item. | ||
| 3. An entry recorded against one destination is not restored for another. | ||
| 4. An expired entry behaves exactly like a miss. | ||
| 5. Providers without `webSearchBridge.enabled` allocate nothing and their | ||
| outbound body keeps its original object identity. | ||
|
|
||
| ## U2 — #4429 | ||
|
|
||
| The issue asked for two things: a non-Ollama executor so a key-auth Responses | ||
| gateway can arm the bridge, and the intercepted call executed proxy-side with | ||
| the conversation continued upstream. Both shipped. The reporter's remaining | ||
| concern — the destination never learning the result — is #4587 and is U1 here. | ||
| The hosted/client distinction the issue turns on is `isWebSearchCallItem` | ||
| versus `isClientExecutedItem` in the bridge: a namespaced `ns__web_search` is | ||
| treated as a client-owned tool and is never intercepted, which is the boundary | ||
| that keeps the undeclared-tool guard's authority intact. | ||
|
|
||
| Closure recommendation and the evidence for it are recorded in `020`. | ||
|
|
||
| ## U3 — #3719 | ||
|
|
||
| The issue body states that the inbound translator drops assistant `thinking` | ||
| and `redacted_thinking` blocks. That is no longer true at HEAD: | ||
| `src/claude/inbound.ts` encodes the Anthropic signature and the opaque | ||
| redacted payloads into bounded `ocxr1` envelopes, and | ||
| `src/adapters/anthropic.ts` replays them as `redacted_thinking` blocks | ||
| followed by a signed `thinking` block. | ||
|
|
||
| This lane verifies that path and keeps it separate from the different question | ||
| of carrying signature data to other providers. The gate that enforces the | ||
| separation is `isLikelyRealAnthropicThinkingSignature`: a block is replayed | ||
| only when its signature looks like a real upstream-issued one, so a proxy-minted | ||
| continuity value or another provider's opaque blob is dropped rather than | ||
| forwarded as an Anthropic signature. Manufacturing a signature is out of scope | ||
| and stays that way. | ||
|
|
||
| The remainder of #3719 is measurement — a controlled cache creation/read | ||
| comparison across continuation turns — which this lane cannot produce under the | ||
| no-local-execution rule. | ||
|
|
||
| ## Operating constraints | ||
|
|
||
| - No local test, typecheck, install, or GUI build. Verification is source | ||
| reading plus hosted CI at the exact final head. | ||
| - No merging, no direct pushes to `dev`, no closing issues or pull requests. | ||
| The lane ends with an open PR and exact-head CI evidence, or with a written | ||
| closure recommendation handed to the host. | ||
| - No flake management: no widened timeouts, added retries, platform skips, or | ||
| masking. | ||
90 changes: 90 additions & 0 deletions
90
devlog/_plan/260918_ld_search_bridge_and_thinking_replay/020_closure_evidence.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| # LD — closure evidence for #4429 and #3719 | ||
|
|
||
| Both issue bodies predate the commits that changed the answer. Each claim below was re-judged | ||
| against `origin/dev` at `2f025814f3`, by reading the current source rather than the issue text. | ||
| The host owns every close decision; this unit only supplies the evidence. | ||
|
|
||
| ## #4429 — key-auth Responses gateway echoes hosted `web_search` as a client `function_call` | ||
|
|
||
| The report asked for two things, and named the shape of the fix itself: ship a non-Ollama executor | ||
| for `webSearchBridge` by reusing the existing sidecar executors, then have the intercepted call run | ||
| proxy-side and continue the conversation upstream so the caller sees a hosted `web_search_call` | ||
| cell. | ||
|
|
||
| | Ask | State at HEAD | Evidence | | ||
| |---|---|---| | ||
| | A non-Ollama key-auth gateway can arm the bridge | Delivered | `5b707d3a5c` | | ||
| | The intercepted call runs proxy-side and the conversation continues upstream | Delivered | `024e43ddf1`, `3557ada7de` | | ||
| | A leg mixing the search with a client-executed tool does not kill the turn | Delivered | `2e6a0316b9` (#4586) | | ||
| | The destination learns the executed result on the next turn | #4587, addressed by this lane's PR | — | | ||
|
|
||
| `planPassthroughWebSearchBridge` no longer returns early for a non-`ollama` backend. For | ||
| `openai`, `anthropic`, `xai`, `gemini` and `exa` it plans on the presence of that backend's own | ||
| resolved credential and never derives an Ollama Cloud origin, so an internal gateway on an | ||
| arbitrary base URL arms exactly as the reporter asked. The credential boundary #3761 called for | ||
| survives: only the `ollama` backend spends the passthrough provider's own API key, and a backend | ||
| whose credential is missing stays disarmed instead of falling through to a different paid search. | ||
|
|
||
| The hosted/client distinction the issue turns on is `isWebSearchCallItem` against | ||
| `isClientExecutedItem`. A namespaced `ns__web_search` is a tool identity the client declared and | ||
| executes itself, so it is never intercepted, and `web_search` is never added to the | ||
| undeclared-tool guard's allowed names — that would authorize a call nobody can execute rather than | ||
| removing it. The guard's authority over every other tool a destination emits is unchanged. | ||
|
|
||
| **Recommendation.** Closable once #4587 lands. One thing in the report is not resolved and is not | ||
| a code defect: whether the reporter's gateway auto-executes Kimi's builtin `$web_search` server | ||
| side and merely echoes the call. That needs a live probe the reporter offered to run. If the host | ||
| wants it tracked, it is a question to the reporter on the existing thread, not a separate defect. | ||
|
|
||
| ## #3719 — Anthropic thinking replay through proxy-auth translation | ||
|
|
||
| The body states that the inbound translator drops assistant `thinking` and `redacted_thinking` | ||
| blocks on replay. That is false at HEAD. | ||
|
|
||
| | Checkbox | State at HEAD | Evidence | | ||
| |---|---|---| | ||
| | Implement Anthropic-to-Anthropic replay fidelity, preserving upstream signatures and opaque redacted blocks | Delivered | `4a59dbc2b7`, `58fcb0961d` | | ||
| | Verify a multi-turn thinking/tool-result exchange with both block types | Covered by regression tests | `tests/adapters/anthropic/anthropic-thinking-signature.test.ts` | | ||
| | Compare cache creation/read usage across controlled continuation turns | Not established | — | | ||
| | Document native passthrough eligibility separately from translated-route cache support | Delivered | `docs-site/src/content/docs/guides/claude-code.md` | | ||
|
|
||
| `src/claude/inbound.ts` encodes the Anthropic signature as `{sig}` and each opaque | ||
| `redacted_thinking` payload as `{red:[...]}` inside a bounded `ocxr1` envelope carried in | ||
| `encrypted_content`. `src/adapters/anthropic.ts` replays the redacted blocks verbatim first, in | ||
| the original stream order, then the signed `thinking` block. | ||
|
|
||
| The separation this lane was asked to preserve is real and enforced in code, not by convention. | ||
| `isLikelyRealAnthropicThinkingSignature` gates the outbound `thinking` block, so a value that does | ||
| not look like an upstream-issued signature is dropped rather than sent as one. Two specific | ||
| leaks are refused rather than generalized: the inbound translator rejects an `ocxr1` envelope | ||
| carrying `sig` that arrives in an Anthropic `signature` field, because proxy-minted reasoning | ||
| continuity must never be replayed as an Anthropic signature; and a native OpenAI-encrypted blob | ||
| has no `ocxr1` prefix, so the decoder returns null and it keeps its placeholder rather than being | ||
| laundered into a signature. Carrying signature data to a different provider stays out of scope. | ||
|
|
||
| **Recommendation.** Only the third checkbox is unproven, and it is a measurement rather than an | ||
| implementation: a controlled cache creation/read comparison across continuation turns with a | ||
| stable model, credential scope, prompt prefix, tool set and retention setting. It needs live | ||
| Anthropic traffic, which this lane cannot produce under the no-local-execution rule, and neither | ||
| the per-turn cache-miss claim nor its attribution to dropped replay blocks was ever reproduced — | ||
| in #3646 or here. The host's options are to close #3719 on the implemented and documented scope | ||
| and treat the unreproduced cache claim as not established, or to keep it open solely as a | ||
| measurement task. The public guide already states the honest position: replay preserves non-hidden | ||
| signed blocks and opaque redacted blocks on the intended Anthropic adapter, and that this does not | ||
| establish live Anthropic acceptance or cache-hit improvements. No documentation change is needed | ||
| for a close. | ||
|
|
||
| ## Pull requests reviewed, not carried | ||
|
|
||
| - **#3952** mixes several changes. The part that touches this lane is a single guarded rewrite in | ||
| the Responses passthrough: when `provider.modelSuffixBracketStrip` is set, the outbound `model` | ||
| has its bracketed suffix stripped, detached before the write so the caller's raw body is not | ||
| mutated. That much is sound and independent of the freeform-tool and Moonshot work in the same | ||
| branch. It should be judged per change, not as one verdict. | ||
| - **#4783** targets `main` and is a draft. Within this lane's scope it adds an `openai-apikey` | ||
| bridge backend and routes the bridge's reasoning setting through `resolveSidecarReasoning` | ||
| instead of reading `sidecar.reasoning` directly. The backend addition is the same generalization | ||
| #4429 asked for, so it overlaps that area; it needs to be retargeted to `dev` before any of it | ||
| can be judged on merit. | ||
| - **#4900** belongs to another lane's Cursor work. Not duplicated here. Nothing in this lane's | ||
| history-replay scope depends on it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the result-text lifecycle claim.
entry.outputis serialized into the next scoped destination request asfunction_call_output. Do not state that it is never serialized or exported. State that it remains process-local between turns and is sent only during a matching destination-scoped replay.🤖 Prompt for AI Agents