feat(client): minimal response-cache substrate (ResponseCacheStore + aggregate-then-write list*()) - #2336
Merged
Claude / Claude Code Review
completed
Jun 22, 2026 in 14m 22s
Code review found 2 potential issues
Found 5 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 2 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | packages/client/src/client/client.ts:1738-1745 |
Eviction comment references nonexistent _cacheListResult |
| 🟡 Nit | packages/client/src/client/client.ts:273-288 |
responseCacheStore JSDoc tense now inverted vs implementation; SEP-2243 mirroring still claimed in present tense elsewhe |
Annotations
Check warning on line 1745 in packages/client/src/client/client.ts
claude / Claude Code Review
Eviction comment references nonexistent _cacheListResult
The new eviction comment in `_onnotification` says "the `_cacheListResult` race guard relies on the bump", but no `_cacheListResult` method exists anywhere in the codebase — the generation race guard actually lives in `_listAllPages` (`captureGeneration`) and `ClientResponseCache.write()`. Update the reference so the comment points at code that exists.
Check warning on line 288 in packages/client/src/client/client.ts
claude / Claude Code Review
responseCacheStore JSDoc tense now inverted vs implementation; SEP-2243 mirroring still claimed in present tense elsewhere
The `ClientOptions.responseCacheStore` JSDoc says the cached `tools/list` result is what callTool's output validation "will read once the stacked SEP-2243 PR lands; this commit ships only the seam", but in this revision `callTool()` already reads the cache via `this._cache.outputValidator(...)` (the `_cache` field doc even calls callTool "the substrate's first production caller"), while the changeset, `docs/migration.md`, and the `listTools()` JSDoc still describe SEP-2243 `Mcp-Param-*` mirrorin
Loading