fix(agents): honor cacheRetention for custom anthropic providers by vincentkoc · Pull Request #59049 · openclaw/openclaw

vincentkoc · 2026-04-01T13:53:27Z

Summary

Problem: custom providers using the anthropic-messages API never honored explicit cacheRetention settings because retention was gated on provider id instead of model API.
Why it matters: Anthropic-compatible proxy/custom-provider users pay full prompt cost and lose cache behavior even when they explicitly configure retention.
What changed: thread model.api into cache-retention resolution and allow explicit retention for custom anthropic-messages providers; add targeted regression tests.
What did NOT change (scope boundary): direct Anthropic defaults stay the same, Bedrock behavior stays the same, and custom providers still do not get implicit caching unless they opt in.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Closes [Feature]: Support prompt caching for custom providers using anthropic-messages API #37325
Closes [Feature]: Prompt caching does not work for custom providers using anthropic-messages API #37733
Related feat: enable prompt caching for custom providers using anthropic-mess… #37418
This PR fixes a bug or regression

Root Cause / Regression History (if applicable)

Root cause: resolveCacheRetention keyed behavior off provider id and treated only direct Anthropic or Anthropic Bedrock as cache-capable paths.
Missing detection / guardrail: there was no test covering a custom provider with model.api === "anthropic-messages" plus explicit retention config.
Prior context (git blame, prior PR, issue, or refactor if known): user reports in [Feature]: Support prompt caching for custom providers using anthropic-messages API #37325 and [Feature]: Prompt caching does not work for custom providers using anthropic-messages API #37733 plus prior attempt in fix: respect cacheRetention for custom providers using anthropic-messages API #58756.
Why this regressed now: custom provider support grew around provider plumbing, but retention logic stayed tied to vendor ids.
If unknown, what was ruled out: N/A.

Regression Test Plan (if applicable)

Coverage level that should have caught this:
- Unit test
- Seam / integration test
- End-to-end test
- Existing coverage already sufficient
Target test or file: src/agents/pi-embedded-runner/extra-params.cache-retention-default.test.ts
Scenario the test should lock in: explicit cacheRetention is passed through for custom providers using anthropic-messages, while the no-config default remains disabled.
Why this is the smallest reliable guardrail: the regression lives entirely in pre-stream extra-param resolution.
Existing test that already covers this (if any): existing direct Anthropic and Bedrock coverage in the same file.
If no new test is added, why not: N/A.

User-visible / Behavior Changes

Custom providers using anthropic-messages now honor explicit cacheRetention values. No behavior change for direct Anthropic or Bedrock users.

Diagram (if applicable)

Before:
custom anthropic-messages provider + cacheRetention=long -> provider-id gate fails -> no cache retention sent

After:
custom anthropic-messages provider + cacheRetention=long -> model-api gate passes -> cache retention sent

Security Impact (required)

New permissions/capabilities? (No)
Secrets/tokens handling changed? (No)
New/changed network calls? (No)
Command/tool execution surface changed? (No)
Data access scope changed? (No)
If any Yes, explain risk + mitigation: N/A

Repro + Verification

Environment

OS: macOS
Runtime/container: Node 22 / pnpm
Model/provider: custom provider with anthropic-messages
Integration/channel (if any): N/A
Relevant config (redacted): provider configured with explicit cacheRetention

Steps

Configure a custom provider whose model reports api: "anthropic-messages".
Set explicit cacheRetention in effective extra params.
Run the stream path through pre-plugin wrappers.

Expected

Explicit cache retention is preserved for the request.

Actual

Before this change, cache retention was dropped for custom providers.

Evidence

Attach at least one:

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

Human Verification (required)

Verified scenarios: custom anthropic-messages provider with explicit short and long retention; custom provider without explicit config stays unset.
Edge cases checked: direct Anthropic default path still uses short retention; explicit direct-Anthropic none/short/long behavior still covered.
What you did not verify: live provider round-trip against an external Anthropic-compatible proxy.

Review Conversations

I replied to or resolved every bot review conversation I addressed in this PR.
I left unresolved only the conversations that still need reviewer or maintainer judgment.

If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.

Compatibility / Migration

Backward compatible? (Yes)
Config/env changes? (No)
Migration needed? (No)
If yes, exact upgrade steps: N/A

Risks and Mitigations

Risk: a custom provider may advertise anthropic-messages but reject Anthropic cache fields.
- Mitigation: custom providers only get retention when explicitly configured; there is no new implicit default for them.

vincentkoc · 2026-04-01T13:53:48Z

Supersedes #58756. Refreshed onto current main, revalidated locally, and kept the fix scoped to explicit cache retention for custom anthropic-messages providers.

greptile-apps · 2026-04-01T14:04:18Z

Greptile Summary

This PR fixes a targeted bug where custom providers using the anthropic-messages API were silently ignored by resolveCacheRetention, causing explicitly-configured cacheRetention values to be dropped. The fix threads model.api through the call chain and adds a new isCustomAnthropicApi guard that activates cache retention for any provider reporting api === "anthropic-messages" when explicit config is present — without introducing implicit defaults.

Key changes:

resolveCacheRetention gains an optional modelApi parameter; the new isCustomAnthropicApi branch requires both modelApi === "anthropic-messages" and an explicit cacheRetention/cacheControlTtl value, so no implicit default is added for custom providers.
createStreamFnWithExtraParams and applyPrePluginStreamWrappers are updated to forward ctx.model?.api — all existing call sites that omit model are unaffected via optional chaining.
Direct Anthropic and Bedrock paths are unchanged, satisfying the stated scope boundary.
Three new test cases are added; the direct unit test for the "no default for custom provider" case is well-targeted. The two integration-style tests only assert streamFn is defined (always true due to unconditional wrappers), so they don't tightly lock in the cacheRetention injection itself — direct resolveCacheRetention unit tests for the \"long\"/\"short\" positive cases would provide stronger regression coverage.

Confidence Score: 5/5

Safe to merge; the logic change is minimal and well-scoped, with no behavior change for direct Anthropic or Bedrock users.

All findings are P2 style/test-coverage suggestions. The core fix in anthropic-cache-retention.ts is correct: the new guard requires both anthropic-messages API and explicit config, preserving existing defaults for all other providers. The optional chaining in extra-params.ts ensures backward compatibility. No P0 or P1 issues found.

No files require special attention; the only suggestions are about adding tighter direct unit tests in extra-params.cache-retention-default.test.ts.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/agents/pi-embedded-runner/extra-params.cache-retention-default.test.ts
Line: 154-173

Comment:
**Integration tests only verify `streamFn` is defined, not that `cacheRetention` was applied**

`applyAndExpectWrapped` asserts `expect(agent.streamFn).toBeDefined()`, but `createAnthropicToolPayloadCompatibilityWrapper` is unconditionally applied inside `applyPrePluginStreamWrappers`, meaning `streamFn` will always be defined regardless of whether the `cacheRetention` path was exercised. The new tests at lines 154–173 and 179–198 would both pass even if `resolveCacheRetention` returned `undefined` for the custom provider path.

The one direct unit test (`resolveCacheRetention(undefined, "litellm", "anthropic-messages")` at line 175–177) correctly validates the no-default path. Consider adding similar direct-unit-test coverage for the positive cases (`"long"` / `"short"`) to lock in the actual regression scenario without relying on indirect `streamFn` presence:

```ts
it("passes cacheRetention 'long' through for custom anthropic-messages provider", () => {
  expect(
    resolveCacheRetention({ cacheRetention: "long" }, "litellm", "anthropic-messages"),
  ).toBe("long");
});

it("passes cacheRetention 'short' through for custom anthropic-messages provider", () => {
  expect(
    resolveCacheRetention({ cacheRetention: "short" }, "litellm", "anthropic-messages"),
  ).toBe("short");
});
```

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

---

This is a comment left during a code review.
Path: src/agents/pi-embedded-runner/extra-params.cache-retention-default.test.ts
Line: 175-177

Comment:
**`cacheRetention: "none"` for custom provider not covered**

The new tests cover `"long"` and `"short"` for custom `anthropic-messages` providers, but there is no test that a custom provider with `cacheRetention: "none"` actually passes `"none"` through (rather than being dropped). Given that `"none"` is the explicit opt-out value, a missed regression here could silently re-enable caching for providers that opted out. A direct unit test is straightforward:

```ts
it("passes cacheRetention 'none' through for custom anthropic-messages provider", () => {
  expect(
    resolveCacheRetention({ cacheRetention: "none" }, "litellm", "anthropic-messages"),
  ).toBe("none");
});
```

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

_{Reviews (1): Last reviewed commit: "Merge branch 'main' into vk/pr-58756-cac..." | Re-trigger Greptile}

wwerst · 2026-04-01T19:42:04Z

@vincentkoc Thanks for picking this up! I fixed tests and addressed greptile comments in #59216

Anything I can help with to get this merged?

wwerst · 2026-04-01T19:42:59Z

#59216 tests passed. I rebased onto main, tests are running again now.

vincentkoc · 2026-04-02T05:28:31Z

Addressed Greptile test coverage feedback in 46c05e8.

Added direct unit assertions for custom anthropic-messages providers so the regression is locked in at the actual decision point:

cacheRetention: "long" -> "long"
cacheRetention: "none" -> "none"
cacheRetention: "short" -> "short"
no explicit config -> undefined remains covered

This avoids relying on streamFn wrapping as a proxy for retention behavior.

Local verification:

pnpm test -- src/agents/pi-embedded-runner/extra-params.cache-retention-default.test.ts
pnpm check

) * fix(agents): honor cacheRetention for custom anthropic providers * docs(changelog): add cache retention entry * Update CHANGELOG.md * test(agents): add direct cache retention assertions

…nclaw#59049) * fix(agents): honor cacheRetention for custom anthropic providers * docs(changelog): add cache retention entry * Update CHANGELOG.md * test(agents): add direct cache retention assertions

fix(agents): honor cacheRetention for custom anthropic providers

4a37c75

vincentkoc self-assigned this Apr 1, 2026

openclaw-barnacle Bot added agents Agent runtime and tooling size: S maintainer Maintainer-authored PR labels Apr 1, 2026

vincentkoc mentioned this pull request Apr 1, 2026

fix: respect cacheRetention for custom providers using anthropic-messages API #58756

Closed

5 tasks

vincentkoc added 3 commits April 1, 2026 22:59

docs(changelog): add cache retention entry

a64b1dd

Update CHANGELOG.md

10b2bd2

Merge branch 'main' into vk/pr-58756-cache-retention

37f61b7

vincentkoc marked this pull request as ready for review April 1, 2026 14:01

greptile-apps Bot reviewed Apr 1, 2026

View reviewed changes

Comment thread src/agents/pi-embedded-runner/extra-params.cache-retention-default.test.ts

Comment thread src/agents/pi-embedded-runner/extra-params.cache-retention-default.test.ts

This was referenced Apr 1, 2026

fix(agents): honor cacheRetention for custom anthropic providers CarbonCoreAI/openclaw#2

Open

fix(agents): honor cacheRetention for custom anthropic providers #59216

Closed

test(agents): add direct cache retention assertions

46c05e8

vincentkoc merged commit ed6012e into main Apr 2, 2026
135 of 136 checks passed

vincentkoc deleted the vk/pr-58756-cache-retention branch April 2, 2026 05:34

github-actions Bot mentioned this pull request Apr 2, 2026

📡 Upstream Digest — 2026-04-02 07:02 UTC curtismercier/openclaw-mods#440

Open

hxy91819 mentioned this pull request Apr 20, 2026

fix(agents): honor explicit long Anthropic cache TTL on custom hosts #67800

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(agents): honor cacheRetention for custom anthropic providers#59049

fix(agents): honor cacheRetention for custom anthropic providers#59049
vincentkoc merged 5 commits intomainfrom
vk/pr-58756-cache-retention

vincentkoc commented Apr 1, 2026

Uh oh!

vincentkoc commented Apr 1, 2026

Uh oh!

greptile-apps Bot commented Apr 1, 2026

Uh oh!

Uh oh!

Uh oh!

wwerst commented Apr 1, 2026

Uh oh!

wwerst commented Apr 1, 2026

Uh oh!

vincentkoc commented Apr 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants