Skip to content

perf(session): deduplicate Redis session response bodies (#1415) - #1417

Merged
ding113 merged 4 commits into
devfrom
perf/1415-session-response-dedup
Aug 12, 2026
Merged

perf(session): deduplicate Redis session response bodies (#1415)#1417
ding113 merged 4 commits into
devfrom
perf/1415-session-response-dedup

Conversation

@ding113

@ding113 ding113 commented Aug 11, 2026

Copy link
Copy Markdown
Owner

Summary

  • Store legacy, before, and after response body views in one request-scoped Redis Hash and deduplicate identical post-redaction values within that request sequence.
  • Replace independent body writes with atomic Lua generation switches across the bundle and the three legacy keys, including retry, rollback, and mixed-writer handling.
  • Read the bundle and the target legacy key in one Lua command so readers cannot observe a marker/body split while writers switch generations.
  • Keep headers and meta independent, preserve old-key reads during the TTL window, and retain STORE_SESSION_RESPONSE_BODY=false behavior.
  • Add SESSION_RESPONSE_BODY_DEDUP_ENABLED=false for a reader-first two-stage rollout. In dedup mode, SESSION_RESPONSE_BODY_MAX_BYTES bounds aggregate unique UTF-8 body bytes per request.

Problem

A single request's response body was written to up to three independent Redis keys (legacy response, before snapshot body, after snapshot body). When the three views held identical or overlapping content, Redis stored up to three full copies. #1414 capped each copy at 5 MiB, but the 3x amplification remained - the same amplifier behind the #1408 OOM incidents, where 64 completed SSE responses produced roughly 1.4 GiB of raw body values and pushed Redis peak to ~1.5 GiB during the RDB save window. This PR collapses the three views into one request-scoped Redis Hash so identical post-redaction bodies share a single physical value.

Related Issues

Tests and verification

  • Added unit coverage for all-identical, each pair-identical, all-distinct, post-redaction equal/different, UTF-8 limits, authoritative over-budget markers, old keys, retries, rollout switches, atomic reads, and disabled storage.
  • Added a six-case real Redis integration suite covering Hash storage, retry generations, rollback/forward rollout, mixed writers, legacy fallback, and TTL consistency.
  • Extended the Issue 物理真机卡死过2次 gpt5.6分析的结果 #1408 fixture with exact-byte completed SSE responses, manifest-based Redis inspection, BGSAVE, cgroup peaks, container OOM/exit state, and TTL cleanup evidence.
  • Final full Vitest, lint, typecheck, production build, script syntax checks, and diff checks passed.
  • Standards review and final Issue perf(session): 去重 Redis 中重复存储的 response body #1415 Spec review reported no remaining findings.

64 x 5 MiB acceptance

  • 64 completed responses produced 64 body:* fields totaling 335,544,320 bytes, exactly 64 x 5,242,880.
  • 64/64 requests shared one body field across the three views; stale legacy body keys and dangling refs were both zero.
  • Redis used_memory_peak was 437,018,776 bytes; Redis cgroup peak was 376,094,720 bytes; app cgroup peak was 417,484,800 bytes.
  • BGSAVE completed with rdb_last_bgsave_status=ok; Redis remained running with OOMKilled=false and ExitCode=0.
  • After SESSION_TTL, the same manifest had zero bundles and zero legacy response body keys.

Acceptance: https://app.lobehub.com/acceptance/d29406fe-a395-45bd-9b72-2229fa4d40c1

Final round: https://app.lobehub.com/verify/1f245416-a59f-40f2-bf45-9f345b18cea4


Description enhanced by Claude AI

Greptile Summary

The PR consolidates each request’s legacy, before, and after response-body views into a Redis hash with generation-fenced reads, writes, rollout compatibility, and termination cleanup.

  • Adds request-scoped body deduplication and aggregate unique-byte budgeting.
  • Coordinates dedup and legacy writers through atomic Lua generation switches.
  • Adds indexed bundle cleanup and late-writer fencing during full session termination.
  • Extends unit, integration, and load coverage for Redis storage, rollout, retries, TTLs, and memory behavior.

Confidence Score: 4/5

The PR is not yet safe to merge because full session termination can report success while the newly indexed response-body bundles remain undeleted.

The response-body cleanup is executed as one pipeline command, but its command-level error is ignored and a successful binding termination or unrelated deletion still makes terminateSession return true, leaving the memory-heavy artifacts until TTL expiry.

Files Needing Attention: src/lib/session-manager.ts

Important Files Changed

Filename Overview
src/lib/session-manager.ts Implements deduplicated response-body bundles and termination fencing, but full termination can report success when its new bundle-cleanup command fails.
src/app/v1/_lib/proxy/response-handler.ts Replaces independent response-body writes with complete three-view bundle writes across terminal response paths.
src/app/v1/_lib/proxy/warmup-guard.ts Coordinates response-body persistence with the new bundled storage path.
src/lib/config/env.schema.ts Adds the rollout-controlled response-body deduplication setting while preserving the existing storage controls.
tests/integration/session-response-body-dedup-redis.test.ts Exercises real-Redis bundle generations, rollout transitions, TTL behavior, and termination cleanup, but not a per-command cleanup failure.

Sequence Diagram

sequenceDiagram
  participant Request
  participant SessionManager
  participant Redis
  participant Admin
  Request->>SessionManager: Store legacy/before/after body set
  SessionManager->>Redis: Atomic generation switch
  Redis->>Redis: Store deduplicated hash and index entry
  Admin->>SessionManager: Terminate session
  SessionManager->>Redis: Terminate binding
  SessionManager->>Redis: Pipeline bundle cleanup and metadata deletion
  Note over SessionManager,Redis: Cleanup command errors must affect termination result
Loading
Prompt To Fix All With AI
### Issue 1
src/lib/session-manager.ts:3659-3669
**Bundle cleanup failures report success**

When the response-body Lua command fails after the binding has been terminated, `terminateSession` ignores that pipeline-command error and still returns success because `bindingTerminated` is true or another deletion succeeded, leaving the multi-megabyte response-body bundles in Redis until their TTL expires.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (2): Last reviewed commit: "test(load): stabilize replay-OOM fixture..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

Context used:

Replace three independent SETEX writes for legacy, before, and after
response bodies with a single atomic storeSessionResponseBodySet call
that writes one request-scoped Redis Hash. Identical views share a
single body field via intra-Hash refs, reducing Redis memory from 3x
the response size to 1x for the common case where all three views are
identical.

The new layout uses Lua scripts to atomically replace the prior
generation (Hash plus legacy keys) on every write, preventing stale
bodies from mixing across retries or concurrent writers. An
authoritative over-budget marker prevents fallback to stale legacy
keys when the aggregate unique-body byte count exceeds
SESSION_RESPONSE_BODY_MAX_BYTES.

A reader-first two-phase rollout is enforced via
SESSION_RESPONSE_BODY_DEDUP_ENABLED (default false). Phase A deploys
new readers while the flag stays false, writing legacy keys plus a
layout=legacy marker. Phase B flips the flag to switch writers to the
dedup Hash layout. Readers transparently handle both layouts and fall
back to legacy keys only when no bundle Hash exists.

Load fixture harness extended with exact-byte SSE sizing, complete
response mode, BGSAVE triggering, and a Redis inspection script that
validates bundle invariants, TTL cleanup, and container OOM state.

Closes #1415
…eration switches

The read Lua script previously declared only the dedup Hash key. When a
writer switched generations between the Hash read and a subsequent
legacy-key GET, the reader could observe a stale legacy marker pointing
at keys already deleted by the next generation, returning null instead
of the stored body.

The script now declares both the Hash key and the target view's legacy
key in a single Redis command, reading layout/present/ref/body and the
legacy body atomically. This eliminates the separate legacy fallback
GET entirely, removing the legacyFallback flag from the read result
type and simplifying both call sites in getSessionResponse and
getSessionDetailSnapshot.

The detail-snapshot test mock now simulates the legacy key store so the
atomic read returns the correct body. The dedup test adds a scenario
where a writer clears legacy keys and installs a dedup Hash after the
read fires, verifying the reader still returns the original legacy
generation.
@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

新增响应正文 bundle 去重存储。系统支持三种正文视图、旧键兼容、Lua 原子读写、UTF-8 聚合字节限制和 TTL 管理。代理路径、配置、测试及 5 MiB 负载验收工具同步更新。

Changes

会话响应正文去重

Layer / File(s) Summary
Bundle 存储、读取与清理
src/lib/session-manager.ts, src/lib/session-manager-response-body-dedup.test.ts, src/lib/session-manager-detail-snapshots.test.ts, tests/unit/lib/session-manager-terminate-session.test.ts
新增 storeSessionResponseBodySet、三视图 bundle、内容去重、UTF-8 预算、generation fencing、Lua 原子操作、旧键回退和终止清理。
配置与代理写入路径
.env.example, src/lib/config/env.schema.ts, src/app/v1/_lib/proxy/..., tests/unit/proxy/*, tests/unit/lib/env-store-session-response-body.test.ts, tests/integration/billing-model-source.test.ts
新增 SESSION_RESPONSE_BODY_DEDUP_ENABLED。非流式、流式、Gemini 和 Warmup 路径统一写入正文集合接口。相关 mock 和断言同步更新。
Redis 集成验证
tests/integration/session-response-body-dedup-redis.test.ts, tests/configs/integration.config.mts
验证去重布局、旧布局切换、并发写入、混合写入竞态、旧键回退、TTL 过期和终止竞态。
负载夹具与验收
tests/load/issue-1408-replay-oom/*, tests/unit/load/issue-1408-mock-upstream.test.ts, tests/unit/proxy/issue-1408-load-fixture.test.ts
负载夹具支持精确响应字节数、完成或断开模式、Redis/RDB/TTL 证据、cgroup 指标和失败诊断。
设计与验收记录
docs/research/issue-1415-session-response-body-dedup.md, docs/troubleshooting/issue-1408-replay-oom.md
新增 Redis Hash 布局、发布协调、验证矩阵和 64 个 5 MiB 响应的去重验收记录。

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed 实现覆盖 #1415 的去重、脱敏、字节预算、兼容读取、原子切换、清理、测试及负载验收要求。
Out of Scope Changes check ✅ Passed 代码、文档、测试和负载夹具变更均服务于 #1415 的实现、兼容性验证或 #1408 验收。
Title check ✅ Passed 标题准确概括了本次变更的主要内容,即对 Redis 会话响应正文进行去重。
Description check ✅ Passed 描述详细说明了响应正文去重、原子切换、兼容性、测试和验收结果,与变更内容直接相关。
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch perf/1415-session-response-dedup

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai
coderabbitai Bot requested a review from AptS-1547 August 11, 2026 19:13
@github-actions github-actions Bot added enhancement New feature or request area:session labels Aug 11, 2026
Comment on lines +186 to +188
function buildSessionResponseBodyBundleKey(sessionId: string, sequence: number): string {
return `session:${sessionId}:req:${sequence}:response-bodies:v1`;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Bundle keys bypass session cleanup

The new response-bodies:v1 keys are not included in terminateSession, so terminating a session leaves potentially multi-megabyte response bodies in Redis until their normal TTL expires, reducing the immediacy and memory effectiveness of administrative cleanup.

Knowledge Base Used: Redis Caching and Session Tracking

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/session-manager.ts
Line: 186-188

Comment:
**Bundle keys bypass session cleanup**

The new `response-bodies:v1` keys are not included in `terminateSession`, so terminating a session leaves potentially multi-megabyte response bodies in Redis until their normal TTL expires, reducing the immediacy and memory effectiveness of administrative cleanup.

**Knowledge Base Used:** [Redis Caching and Session Tracking](https://app.greptile.com/ygxz/-/custom-context/knowledge-base/ding113/claude-code-hub/-/docs/redis-caching-and-sessions.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (3)
docs/research/issue-1415-session-response-body-dedup.md (1)

46-47: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

缺少空格。

writer省略 之间缺少空格,与全文的中英混排风格不一致。

📝 建议修改
-如果新 writer 把旧 key 改写为引用描述符, 旧 reader 会把描述符当正文; 如果新 writer省略重复
+如果新 writer 把旧 key 改写为引用描述符, 旧 reader 会把描述符当正文; 如果新 writer 省略重复
 key, 旧 reader 会返回 `null`.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/research/issue-1415-session-response-body-dedup.md` around lines 46 -
47, 在文档描述中,将“writer省略”修改为“writer 省略”,补充中英文之间缺少的空格,并保持其余内容不变。
tests/load/issue-1408-replay-oom/mock-upstream.cjs (1)

186-193: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

每个请求都重新构建 5 MiB 帧数组。

buildSseFrames(responseBytes, responseMode) 的输入在整个进程生命周期内是常量。当前每个请求都重新分配约 5 MiB 的字符串数组,会增加 mock 自身的 CPU 与内存压力,并干扰负载观测。请在模块加载时构建一次并复用。

♻️ 建议重构
+const staticFrames = buildSseFrames(responseBytes, responseMode);
+
 // 在请求处理中:
-  const frames = buildSseFrames(responseBytes, responseMode);
+  const frames = staticFrames;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/load/issue-1408-replay-oom/mock-upstream.cjs` around lines 186 - 193, 将
buildSseFrames(responseBytes, responseMode) 从每次请求的处理流程移到模块加载阶段,仅构建一次并缓存结果;在
writeNext 及其所属请求逻辑中复用该模块级 frames,保持现有发送顺序与计数行为不变。
tests/load/issue-1408-replay-oom/run-wave.sh (1)

100-118: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

TTL 轮询会隐藏所有失败原因。

2>/dev/null 会丢弃 inspect-redis.cjs 的全部 stderr。如果失败原因不是 TTL 未过期(例如 docker 不可用或 manifest 解析失败),脚本会静默重试到超时。建议把 stderr 写入临时文件,并在超时时一并输出。

同时建议校验 CCH_TTL_CLEANUP_TIMEOUT_SECONDSCCH_RDB_DELAY_SECONDS 为整数,避免 [ ... -ge ]sleep 报错。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/load/issue-1408-replay-oom/run-wave.sh` around lines 100 - 118, 更新 TTL
轮询中 inspect-redis.cjs 的错误处理,将 stderr 重定向到独立临时文件而不是丢弃,并在超时时输出该文件内容。为
CCH_TTL_CLEANUP_TIMEOUT_SECONDS 和 CCH_RDB_DELAY_SECONDS 增加整数格式校验,在进入超时计算或 sleep
前拒绝无效值并返回清晰错误。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.env.example:
- Around line 108-113: Update the comments for
SESSION_RESPONSE_BODY_DEDUP_ENABLED to limit the atomic generation-marker
coordination guarantee to requests with a valid requestSequence. Explicitly note
that requests without a sequence use the compatibility legacy/before/after key
path and do not execute generation-marker Lua, while preserving the existing
deployment guidance.

In `@docs/research/issue-1415-session-response-body-dedup.md`:
- Around line 37-39: 修改文档中以 `#1408` 和 `#1415` 开头的行,避免将 issue 引用放在 Markdown
行首;将引用移入行内或改写为不触发标题解析的文本,同时保留原有内容和 issue 链接含义。

In `@tests/load/issue-1408-replay-oom/drive-disconnect-waves.cjs`:
- Around line 184-192: 为所有 handle 的 completion Promise 附加 no-op catch,不要仅在
requestMode 为 disconnect 时处理;同时更新 waitForCompletions 使用已附加 rejection 处理的 Promise
副本,确保 complete 模式及超时后 abortRequests 导致的后续 rejection 不会未处理。

---

Nitpick comments:
In `@docs/research/issue-1415-session-response-body-dedup.md`:
- Around line 46-47: 在文档描述中,将“writer省略”修改为“writer 省略”,补充中英文之间缺少的空格,并保持其余内容不变。

In `@tests/load/issue-1408-replay-oom/mock-upstream.cjs`:
- Around line 186-193: 将 buildSseFrames(responseBytes, responseMode)
从每次请求的处理流程移到模块加载阶段,仅构建一次并缓存结果;在 writeNext 及其所属请求逻辑中复用该模块级
frames,保持现有发送顺序与计数行为不变。

In `@tests/load/issue-1408-replay-oom/run-wave.sh`:
- Around line 100-118: 更新 TTL 轮询中 inspect-redis.cjs 的错误处理,将 stderr
重定向到独立临时文件而不是丢弃,并在超时时输出该文件内容。为 CCH_TTL_CLEANUP_TIMEOUT_SECONDS 和
CCH_RDB_DELAY_SECONDS 增加整数格式校验,在进入超时计算或 sleep 前拒绝无效值并返回清晰错误。
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9c674011-3aba-4215-8cec-5b7e81d6a48a

📥 Commits

Reviewing files that changed from the base of the PR and between f01f9f8 and b59cd8f.

📒 Files selected for processing (29)
  • .env.example
  • docs/research/issue-1415-session-response-body-dedup.md
  • docs/troubleshooting/issue-1408-replay-oom.md
  • src/app/v1/_lib/proxy/response-handler.ts
  • src/app/v1/_lib/proxy/warmup-guard.ts
  • src/lib/config/env.schema.ts
  • src/lib/session-manager-detail-snapshots.test.ts
  • src/lib/session-manager-response-body-dedup.test.ts
  • src/lib/session-manager.ts
  • tests/configs/integration.config.mts
  • tests/integration/billing-model-source.test.ts
  • tests/integration/session-response-body-dedup-redis.test.ts
  • tests/load/issue-1408-replay-oom/README.md
  • tests/load/issue-1408-replay-oom/drive-disconnect-waves.cjs
  • tests/load/issue-1408-replay-oom/inspect-redis.cjs
  • tests/load/issue-1408-replay-oom/mock-upstream.cjs
  • tests/load/issue-1408-replay-oom/run-wave.sh
  • tests/load/issue-1408-replay-oom/sample-container.sh
  • tests/load/issue-1408-replay-oom/start-mock-container.sh
  • tests/unit/lib/env-store-session-response-body.test.ts
  • tests/unit/load/issue-1408-mock-upstream.test.ts
  • tests/unit/proxy/pricing-no-price.test.ts
  • tests/unit/proxy/response-handler-abort-listener-cleanup.test.ts
  • tests/unit/proxy/response-handler-client-abort-drain.test.ts
  • tests/unit/proxy/response-handler-endpoint-circuit-isolation.test.ts
  • tests/unit/proxy/response-handler-gemini-stream-passthrough-timeouts.test.ts
  • tests/unit/proxy/response-handler-lease-decrement.test.ts
  • tests/unit/proxy/response-handler-non200.test.ts
  • tests/unit/proxy/warmup-guard.test.ts

Comment thread .env.example
Comment thread docs/research/issue-1415-session-response-body-dedup.md Outdated
Comment thread tests/load/issue-1408-replay-oom/drive-disconnect-waves.cjs
@github-actions

Copy link
Copy Markdown
Contributor

🧪 测试结果

测试类型 状态
代码质量
单元测试
集成测试
API 测试

总体结果: ✅ 所有测试通过

@github-actions github-actions Bot added the size/XL Extra Large PR (> 1000 lines) label Aug 11, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

No significant issues identified in this PR after reviewing the new Redis response-body dedup path, the rollout compatibility logic, and the added Redis/load-test coverage.
The main runtime changes stay aligned with the existing session snapshot consumers, and I did not find a diff-scoped defect that cleared the reporting threshold after validation.

PR Size: XL

  • Lines changed: 2363
  • Files changed: 29
  • Split suggestions: If follow-up work is needed, consider separating runtime dedup logic, Redis integration coverage, and load-fixture/docs evidence updates into smaller PRs.

Review Coverage

  • Logic and correctness - Clean
  • Security (OWASP Top 10) - Clean
  • Error handling - Clean
  • Type safety - Clean
  • Documentation accuracy - Clean
  • Test coverage - Adequate
  • Code clarity - Good

Automated review by Codex AI

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Reviewed PR #1417 across the requested perspectives, focused on the diffed runtime changes, rollout logic, and added Redis/load-test coverage.
  • Applied the size/XL label and posted the PR review summary via gh.
  • I did not find a diff-scoped issue that cleared the reporting threshold after full-context validation, so I submitted a no-findings summary and included split suggestions because the PR is XL.
  • I also tried to run targeted Vitest coverage for the new session-body paths, but this runner does not have bunx or a local vitest binary available.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

This PR implements Redis response body deduplication by collapsing legacy/before/after body views into a single request-scoped Redis Hash with atomic Lua-based generation switches. The implementation is architecturally sound: the Lua scripts have correct ARGV mappings, backward compatibility is carefully preserved through legacy key fallback in the reader, and the two-stage rollout (SESSION_RESPONSE_BODY_DEDUP_ENABLED) cleanly coordinates mixed-writer scenarios during rolling deploys.

PR Size: XL

  • Lines changed: 2363 (2195 additions, 168 deletions)
  • Files changed: 29 (5 source files, 24 test/doc/load-test files)

Issues Found

Category Critical High Medium Low
Logic/Bugs 0 0 0 0
Security 0 0 0 0
Error Handling 0 0 0 0
Types 0 0 0 0
Comments/Docs 0 0 0 0
Tests 0 0 0 0
Simplification 0 0 0 0

No issues met the reporting threshold (confidence >= 80).

Key verification areas

  • Lua ARGV/index mapping: Verified all three scripts (write:v1, write-legacy:v1, read:v1) against their JS call sites. KEYS and ARGV offsets are correct for both write paths (4 keys, 10+ ARGV) and the read path (2 keys, 1 ARGV).
  • Backward compatibility: The reader Lua falls back to legacy GET when the bundle key does not exist (EXISTS == 0), so sessions written by pre-bundle instances remain readable. The layout=legacy branch also reads from the legacy SETEX key, ensuring consistency when DEDUP_ENABLED=false.
  • Over-budget semantics: When aggregate unique bytes exceed the limit, all bodies are dropped and over_budget=1 is written with present:* flags but no refs. The reader correctly returns body: null for this case. The behavior difference in getSessionResponsePhaseSnapshot's null guard (returns a null-field snapshot instead of null when exists && present) is harmless — normalizeResponseSnapshot in active-sessions.ts handles both null and { body: null, ... } identically.
  • Error handling: All catch blocks log with appropriate context. The normalizeSessionResponseBody catch (non-JSON bodies like SSE streams) mirrors the existing pattern in storeSessionResponse. No silent failures.
  • Atomicity: DEL+HSET within a single Lua eval is atomic. The reader's EXISTS+GET/HGET sequence is also atomic within one eval, preventing marker/body splits during generation switches.
  • Test coverage: Unit tests cover all-identical, pair-identical, all-distinct, post-redaction, UTF-8 limits, over-budget markers, legacy fallback, retries, rollout switches, and disabled storage. Integration tests cover real Redis Hash storage, retry generations, rollback/forward rollout, and mixed writers.

Review Coverage

  • Logic and correctness - Clean
  • Security (OWASP Top 10) - Clean
  • Error handling - Clean
  • Type safety - Clean
  • Documentation accuracy - Clean
  • Test coverage - Comprehensive
  • Code clarity - Good

Automated review by Claude AI

Add a session-level response body generation counter and a per-request
generation snapshot taken when a request sequence is issued. The dedup
and legacy Lua writers now check that the request-scoped generation
matches the current session generation before writing, preventing a
late writer from recreating response bodies after terminateSession has
run.

Full termination advances the session generation atomically via a new
Lua script that also deletes every bundle registered in the session
ZSET index, their legacy view keys, and the sequence-1 fallback keys.
Provider-scoped termination is unchanged and does not touch shared
session artifacts.

Requests carrying a keyId but no valid sequence now fail closed instead
of falling through to the legacy compatibility path, avoiding any bypass
of the termination fence.
Pre-cache SSE frames once at mock-upstream startup instead of rebuilding
them per request, and always swallow completion rejections in the driver
so complete-mode failures surface through the controlled receipt
timeout rather than as unhandled rejections.

run-wave.sh now validates that CCH_RDB_DELAY_SECONDS and
CCH_TTL_CLEANUP_TIMEOUT_SECONDS are non-negative integers, uses separate
temp files for TTL inspection stdout/stderr so diagnostics are preserved
on timeout, and cleans up temp files via an EXIT trap with explicit INT
and TERM handlers.

The unit fixture test now strips all CCH_ environment variables before
applying test overrides, adds coverage for cached-frame reuse,
complete-mode receipt-timeout failure reporting, invalid integer
rejection, and TTL timeout diagnostic preservation.
Comment on lines +3659 to +3669
pipeline.eval(
DELETE_SESSION_RESPONSE_BODY_BUNDLES_LUA,
5,
buildSessionResponseBodyBundleIndexKey(sessionId),
buildSessionResponseBodyGenerationKey(sessionId),
`session:${sessionId}:response`,
buildLegacySessionResponseBodyViewKey(sessionId, 1, "before"),
buildLegacySessionResponseBodyViewKey(sessionId, 1, "after"),
SessionManager.SESSION_TTL,
crypto.randomUUID()
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Bundle cleanup failures report success

When the response-body Lua command fails after the binding has been terminated, terminateSession ignores that pipeline-command error and still returns success because bindingTerminated is true or another deletion succeeded, leaving the multi-megabyte response-body bundles in Redis until their TTL expires.

Knowledge Base Used: Redis Caching and Session Tracking

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/session-manager.ts
Line: 3659-3669

Comment:
**Bundle cleanup failures report success**

When the response-body Lua command fails after the binding has been terminated, `terminateSession` ignores that pipeline-command error and still returns success because `bindingTerminated` is true or another deletion succeeded, leaving the multi-megabyte response-body bundles in Redis until their TTL expires.

**Knowledge Base Used:** [Redis Caching and Session Tracking](https://app.greptile.com/ygxz/-/custom-context/knowledge-base/ding113/claude-code-hub/-/docs/redis-caching-and-sessions.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ef603e8048

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

local generation = redis.call('GET', KEYS[2])
if not generation then
generation = '0'
redis.call('SETEX', KEYS[2], ARGV[2], generation)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep response generations alive for long-running requests

When a streaming request remains open longer than SESSION_TTL (300 seconds by default), the global generation set here and the request generation set at line 696 both expire before the response finalizer writes its body. WRITE_SESSION_RESPONSE_BODY_BUNDLE_LUA requires both keys to exist and match, so it silently returns 0 and the completed response body is never persisted; this affects streams the response handler explicitly keeps alive beyond SESSION_TTL (response-handler.ts:406-412). Refresh these generation keys with the streaming heartbeat or otherwise retain them for the full request lifetime while preserving the termination fence.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/research/issue-1415-session-response-body-dedup.md`:
- Around line 115-121: 明确 requestOwnerKey 的一致性边界:若其 TTL 属于 dedup 原子写入契约,更新
storeSessionResponseBodySet 及对应 Lua 脚本,将 requestOwnerKey 的 TTL
刷新纳入同一原子执行;否则收窄并说明该契约不包含 requestOwnerKey,仅覆盖 Hash、ZSET 和 legacy key。
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e7d2776f-8b3e-4557-9b2a-432cc4b2af88

📥 Commits

Reviewing files that changed from the base of the PR and between b59cd8f and ef603e8.

📒 Files selected for processing (11)
  • .env.example
  • docs/research/issue-1415-session-response-body-dedup.md
  • src/lib/session-manager-detail-snapshots.test.ts
  • src/lib/session-manager-response-body-dedup.test.ts
  • src/lib/session-manager.ts
  • tests/integration/session-response-body-dedup-redis.test.ts
  • tests/load/issue-1408-replay-oom/drive-disconnect-waves.cjs
  • tests/load/issue-1408-replay-oom/mock-upstream.cjs
  • tests/load/issue-1408-replay-oom/run-wave.sh
  • tests/unit/lib/session-manager-terminate-session.test.ts
  • tests/unit/proxy/issue-1408-load-fixture.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/lib/session-manager-detail-snapshots.test.ts
  • .env.example
  • tests/load/issue-1408-replay-oom/run-wave.sh
  • tests/load/issue-1408-replay-oom/mock-upstream.cjs

Comment on lines +115 to +121
dedup 写入 Lua 同时声明 Hash, 三个旧正文 key 和 session-level bundle index:

1. 校验 request owner 和 request/session response body generation 一致.
2. 原子删除上一 generation 的 Hash 和旧正文 key.
3. 写入固定字段, present/ref 和唯一 body.
4. 对整个 Hash 和 generation marker 执行 `EXPIRE`.
5. 按 Redis `TIME` 的毫秒过期分数剪枝 ZSET, 登记当前 bundle, 并刷新 index TTL.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -i -C 12 \
  'storeSessionResponseBodySet|evalsha|eval|zadd|zremrangebyscore|expire|hset|del|unlink' \
  src/lib/session-manager.ts tests

Repository: ding113/claude-code-hub

Length of output: 50380


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)(session-manager|redis|session).*\.(ts|tsx|js|lua)$|issue-1415-session-response-body-dedup\.md$' | head -200

printf '%s\n' '--- exact symbols ---'
rg -n -S -C 8 \
  'storeSessionResponseBodySet|session-response|response.?body|legacy|zremrangebyscore|EVALSHA|evalsha|\.eval\(|\.multi\(|HSET|ZADD|EXPIRE|expire' \
  src tests \
  -g '*.ts' -g '*.tsx' -g '*.js' -g '*.lua' \
  | head -1000

Repository: ding113/claude-code-hub

Length of output: 50379


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- tracked candidate files ---'
git ls-files | rg -i 'session|redis' | rg '\.(ts|tsx|js|lua)$' | head -300

printf '%s\n' '--- exact symbol locations ---'
rg -n -S 'storeSessionResponseBodySet|storeSessionResponseBody|responseBodyGeneration|legacy.*key|bundle.*index' src tests \
  -g '*.ts' -g '*.tsx' -g '*.js' -g '*.lua' || true

printf '%s\n' '--- relevant Redis calls in session manager ---'
rg -n -S -C 18 'evalsha|eval|zadd|zremrangebyscore|expire|hset|del|unlink|multi|pipeline' \
  src/lib/session-manager.ts 2>/dev/null || true

Repository: ding113/claude-code-hub

Length of output: 50379


🏁 Script executed:

#!/bin/bash
set -euo pipefail

files="$(git ls-files | rg -i 'session|redis' | rg '\.(ts|tsx|js|lua)$' || true)"
printf '%s\n' "$files" | head -200

printf '%s\n' '--- matches across source ---'
rg -n -S -C 6 \
  'storeSessionResponseBodySet|responseBodyGeneration|zremrangebyscore|legacy.*key|bundle.*index|evalsha|eval\(' \
  src -g '*.ts' -g '*.tsx' -g '*.js' -g '*.lua' || true

Repository: ding113/claude-code-hub

Length of output: 50380


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
import re

patterns = re.compile(
    r"storeSessionResponseBodySet|responseBodyGeneration|"
    r"zremrangebyscore|zadd|hset|expire|unlink|evalsha|"
    r"redis\.eval|\.eval\("
    , re.I
)
for p in Path(".").rglob("*"):
    if not p.is_file() or any(part in {"node_modules", ".git", "dist", "build"} for part in p.parts):
        continue
    if p.suffix not in {".ts", ".tsx", ".js", ".lua"}:
        continue
    try:
        lines = p.read_text(errors="replace").splitlines()
    except OSError:
        continue
    hits = [i for i, line in enumerate(lines) if patterns.search(line)]
    if not hits:
        continue
    print(f"FILE {p} HITS {len(hits)}")
    for i in hits[:80]:
        lo, hi = max(0, i-2), min(len(lines), i+3)
        for j in range(lo, hi):
            print(f"{p}:{j+1}:{lines[j]}")
        print("--")
PY

Repository: ding113/claude-code-hub

Length of output: 50380


🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo='ding113/claude-code-hub'
for path in \
  src/lib/session-manager.ts \
  tests/unit/lib/session-manager.test.ts \
  tests/unit/session-manager.test.ts
do
  url="https://raw.githubusercontent.com/$repo/main/$path"
  body="$(curl -LfsS "$url" 2>/dev/null || true)"
  if [ -n "$body" ]; then
    printf '%s\n' "--- $path ---"
    printf '%s\n' "$body" | rg -n -S -C 10 \
      'storeSessionResponseBodySet|responseBodyGeneration|zremrangebyscore|zadd|hset|expire|unlink|evalsha|eval\(' \
      | head -800 || true
  fi
done

Repository: ding113/claude-code-hub

Length of output: 5821


🌐 Web query:

site:github.com/ding113/claude-code-hub "storeSessionResponseBodySet" OR "responseBodyGeneration"

💡 Result:

The terms storeSessionResponseBodySet and responseBodyGeneration are not standard API methods or formal technical concepts in general programming or the referenced "claude-code-hub" codebase [1][2][3][4]. Research indicates that the codebase associated with these terms uses explicit session management functions such as storeSessionResponse, storeSessionUpstreamResponseMeta, and storeSessionRequestBody [1][2][3]. Based on internal documentation and issue discussions within the claude-code-hub repository: 1. Session Data Handling: Request and response bodies are currently managed primarily as temporary data in Redis with a Time-To-Live (TTL) (typically 5 minutes), rather than being persisted in the primary database [3][5]. 2. Persistence Logic: Feature requests and architectural discussions confirm that there is no native storeSessionResponseBodySet method [5][6]. Developers have explored extending the database schema (e.g., adding request_body or response_body columns to a message_request table) to support persistence, but this is a custom implementation requirement rather than an existing API [5]. 3. Response Processing: Code related to response handling often focuses on streaming (SSE), filtering, and metadata extraction (such as actual-response-model.ts) rather than setting a generic "body generation" state [7][6][4]. If you encountered these specific terms in a codebase, they are likely internal custom identifiers, deprecated function names, or possibly typos for functions like storeSessionResponse [2][3]. It is recommended to search the local codebase directly for these strings to identify their specific implementation and context.

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- writer scripts and invocation ---'
sed -n '40,180p' src/lib/session-manager.ts
sed -n '2380,2525p' src/lib/session-manager.ts

printf '%s\n' '--- response-body key mutations outside writer method ---'
rg -n -S -C 5 \
  'response-bodies:v1|response-body-bundles:v1|response-body-generation:v1|snapshot:response:(before|after):body|:response`|:response"' \
  src/lib/session-manager.ts src/lib/redis src/app/v1/_lib/proxy tests/integration/session-response-body-dedup-redis.test.ts \
  -g '*.ts' | head -1200

Repository: ding113/claude-code-hub

Length of output: 20996


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- body-set remainder ---'
sed -n '2470,2605p' src/lib/session-manager.ts

printf '%s\n' '--- owner refresh implementation and callers ---'
rg -n -S -C 18 \
  'refreshSessionRequestOwner|REQUEST_OWNER|:owner|request owner' \
  src/lib/session-manager.ts src/lib/redis tests \
  -g '*.ts' | head -1200

Repository: ding113/claude-code-hub

Length of output: 50379


将 request owner 的 TTL 刷新纳入原子写入,或收窄原子性契约。

两个写入 Lua 脚本已在同一次执行中完成 HSET、legacy key 清理、ZREMRANGEBYSCOREZADD 和 bundle/index EXPIREstoreSessionResponseBodySet 仍先通过独立的 SETEX 刷新 requestOwnerKey。如果该 TTL 属于原子写入契约,请将刷新移入 Lua;否则明确该 key 不属于 Hash、ZSET 和 legacy key 的一致性范围。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/research/issue-1415-session-response-body-dedup.md` around lines 115 -
121, 明确 requestOwnerKey 的一致性边界:若其 TTL 属于 dedup 原子写入契约,更新
storeSessionResponseBodySet 及对应 Lua 脚本,将 requestOwnerKey 的 TTL
刷新纳入同一原子执行;否则收窄并说明该契约不包含 requestOwnerKey,仅覆盖 Hash、ZSET 和 legacy key。

@ding113
ding113 merged commit d6aa890 into dev Aug 12, 2026
13 of 16 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in Claude Code Hub Roadmap Aug 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:session enhancement New feature or request size/XL Extra Large PR (> 1000 lines)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants