Skip to content

fix(openrouter): restore attribution wrapper in extra params#62677

Open
luoyanglang wants to merge 3 commits intoopenclaw:mainfrom
luoyanglang:wolf/openrouter-attribution-wrapper
Open

fix(openrouter): restore attribution wrapper in extra params#62677
luoyanglang wants to merge 3 commits intoopenclaw:mainfrom
luoyanglang:wolf/openrouter-attribution-wrapper

Conversation

@luoyanglang
Copy link
Copy Markdown
Contributor

Summary

  • restore the OpenRouter attribution wrapper in the applyExtraParamsToAgent() post-wrapper chain
  • keep the existing pi-embedded-runner regression test green for OpenRouter attribution headers on chat-path models
  • scope stays narrow: no changes to direct-completion attribution behavior, only the extra-params wrapper path

Validation

  • pnpm.cmd exec vitest run src/agents/pi-embedded-runner-extraparams.test.ts
  • pnpm.cmd exec oxlint src/agents/pi-embedded-runner/extra-params.ts src/agents/pi-embedded-runner-extraparams.test.ts

@openclaw-barnacle openclaw-barnacle bot added agents Agent runtime and tooling size: XS labels Apr 7, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 7, 2026

Greptile Summary

Restores createOpenRouterSystemCacheWrapper to the applyPostPluginStreamWrappers post-plugin chain in extra-params.ts, ensuring Anthropic ephemeral cache markers are correctly applied for OpenRouter-routed Anthropic model IDs on the chat-path. The production fix is accurate and well-scoped.

Confidence Score: 5/5

Safe to merge; the code fix is correct and all remaining findings are P2.

The single P2 finding is a test-mock inaccuracy that is harmless for current test cases (no Anthropic model refs exercised). The production wrapper chain is correct after the fix.

src/agents/pi-embedded-runner-extraparams-openrouter.test.ts — the wrapProviderStreamFn mock wraps with createOpenRouterSystemCacheWrapper, which the real plugin does not; consider trimming it to just createOpenRouterWrapper.

Comments Outside Diff (1)

  1. src/agents/pi-embedded-runner-extraparams-openrouter.test.ts, line 42 (link)

    P2 Test mock wraps with createOpenRouterSystemCacheWrapper but production does not

    The production OpenRouter plugin (extensions/openrouter/stream.tswrapOpenRouterProviderStream) never calls createOpenRouterSystemCacheWrapper; it only applies routing injection and the "openrouter-thinking" stream-family hook (createOpenRouterWrapper). The cache wrapper is exclusively post-plugin, applied once by applyPostPluginStreamWrappers at extra-params.ts:395 — the line restored by this PR.

    With this fix in place, for any Anthropic model ref routed through OpenRouter, applyAnthropicEphemeralCacheControlMarkers will run twice: once from inside this mock's plugin layer and once from applyPostPluginStreamWrappers. That function mutates the payload in-place and is not idempotent — a second application on an already-mutated payload can produce incorrect cache-control structure.

    The current test models (deepseek/deepseek-r1, openrouter/auto, x-ai/grok-4.1-fast) all return false from isAnthropicModelRef(), so the double-application is a no-op today. However, the mock misrepresents the real wrapper chain and could produce silently double-mutated payloads if an Anthropic-on-OpenRouter test case is added later.

    Consider removing createOpenRouterSystemCacheWrapper from the mock's return value:

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: src/agents/pi-embedded-runner-extraparams-openrouter.test.ts
    Line: 42
    
    Comment:
    **Test mock wraps with `createOpenRouterSystemCacheWrapper` but production does not**
    
    The production OpenRouter plugin (`extensions/openrouter/stream.ts``wrapOpenRouterProviderStream`) never calls `createOpenRouterSystemCacheWrapper`; it only applies routing injection and the `"openrouter-thinking"` stream-family hook (`createOpenRouterWrapper`). The cache wrapper is exclusively post-plugin, applied once by `applyPostPluginStreamWrappers` at `extra-params.ts:395` — the line restored by this PR.
    
    With this fix in place, for any Anthropic model ref routed through OpenRouter, `applyAnthropicEphemeralCacheControlMarkers` will run twice: once from inside this mock's plugin layer and once from `applyPostPluginStreamWrappers`. That function mutates the payload in-place and is not idempotent — a second application on an already-mutated payload can produce incorrect cache-control structure.
    
    The current test models (`deepseek/deepseek-r1`, `openrouter/auto`, `x-ai/grok-4.1-fast`) all return `false` from `isAnthropicModelRef()`, so the double-application is a no-op today. However, the mock misrepresents the real wrapper chain and could produce silently double-mutated payloads if an Anthropic-on-OpenRouter test case is added later.
    
    Consider removing `createOpenRouterSystemCacheWrapper` from the mock's return value:
    
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/agents/pi-embedded-runner-extraparams-openrouter.test.ts
Line: 42

Comment:
**Test mock wraps with `createOpenRouterSystemCacheWrapper` but production does not**

The production OpenRouter plugin (`extensions/openrouter/stream.ts``wrapOpenRouterProviderStream`) never calls `createOpenRouterSystemCacheWrapper`; it only applies routing injection and the `"openrouter-thinking"` stream-family hook (`createOpenRouterWrapper`). The cache wrapper is exclusively post-plugin, applied once by `applyPostPluginStreamWrappers` at `extra-params.ts:395` — the line restored by this PR.

With this fix in place, for any Anthropic model ref routed through OpenRouter, `applyAnthropicEphemeralCacheControlMarkers` will run twice: once from inside this mock's plugin layer and once from `applyPostPluginStreamWrappers`. That function mutates the payload in-place and is not idempotent — a second application on an already-mutated payload can produce incorrect cache-control structure.

The current test models (`deepseek/deepseek-r1`, `openrouter/auto`, `x-ai/grok-4.1-fast`) all return `false` from `isAnthropicModelRef()`, so the double-application is a no-op today. However, the mock misrepresents the real wrapper chain and could produce silently double-mutated payloads if an Anthropic-on-OpenRouter test case is added later.

Consider removing `createOpenRouterSystemCacheWrapper` from the mock's return value:
```suggestion
      return createOpenRouterWrapper(streamFn, thinkingLevel);
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "fix(openrouter): restore attribution wra..." | Re-trigger Greptile

@luoyanglang luoyanglang force-pushed the wolf/openrouter-attribution-wrapper branch from d6f634a to 9a2bb32 Compare April 7, 2026 19:50
@luoyanglang luoyanglang force-pushed the wolf/openrouter-attribution-wrapper branch from b0963b4 to 010ca1f Compare April 8, 2026 03:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling size: XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant