build: cache RocksDB C/C++ compile via sccache + add PRD docs - #388
Merged
Conversation
- Wrap CC/CXX with sccache in CI (clippy + build-and-test) and dev.sh so librocksdb-sys C++ compile is actually cached (was bypassing RUSTC_WRAPPER, ~18min cold build) - Add target/ to CI cargo cache path - Add docs/prd.md + docs/personas-and-user-stories.md synthesized from .planning/ governance docs
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe PR updates CI and local builds to use ChangesBuild caching
Product planning documentation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/prd.md (1)
59-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the canonical hot-tier terminology consistently.
Replace shorthand such as “未来热层” and other standalone “热层” references throughout this document with
Embedded Redis Hot Tieror内嵌 Redis 8.8.1 原生内存热数据层.As per coding guidelines: only those two terms are canonical terminology.
🤖 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/prd.md` at line 59, Update docs/prd.md to replace every standalone hot-tier reference, including “未来热层”, with one of the two canonical terms: “Embedded Redis Hot Tier” or “内嵌 Redis 8.8.1 原生内存热数据层”. Preserve the existing meaning and use terminology consistently throughout the document.Source: Coding guidelines
🤖 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/personas-and-user-stories.md`:
- Around line 104-111: Update the persona-and-user-story coverage matrix and
closing note to match the mappings defined by the individual stories: either add
explicit stories for REQ-STABILITY-003/004/006 and account for REQ-HOT-001, or
narrow the summary claims to the requirements actually covered. Ensure the
active-story and P2-freeze statements are mutually consistent.
In `@docs/prd.md`:
- Around line 63-68: Define one consistent hot-tier authorization boundary
across docs/prd.md lines 63-68, 174-196, and docs/personas-and-user-stories.md
lines 19-20: state that G1–G7 stability-gate passage plus renewed user approval
only unfreezes planning and authorizes a separately approved implementation
task, never production implementation automatically. Update the Frozen → Ready
and M7 wording and the SRE user story to preserve this distinction, including
that Redis-derived production integration requires explicit approval of the
separate task.
---
Nitpick comments:
In `@docs/prd.md`:
- Line 59: Update docs/prd.md to replace every standalone hot-tier reference,
including “未来热层”, with one of the two canonical terms: “Embedded Redis Hot Tier”
or “内嵌 Redis 8.8.1 原生内存热数据层”. Preserve the existing meaning and use terminology
consistently throughout the document.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5e008f03-bcf3-4b64-b8cf-490d9f763501
📒 Files selected for processing (4)
.github/workflows/ci.ymldocs/personas-and-user-stories.mddocs/prd.mdscripts/dev.sh
AlexStocks
commented
Jul 30, 2026
| if command -v sccache &> /dev/null; then | ||
| export RUSTC_WRAPPER=sccache | ||
| export CC="sccache cc" | ||
| export CXX="sccache c++" |
Contributor
Author
There was a problem hiding this comment.
[P1] 不要在 Windows 构建中把 MSVC 编译器改成 GNU c++
Windows 的 make build 同样会经 Makefile 的 Bash 进入此脚本，因此 workflow 中跳过 Windows 的 CC/CXX step 并不能阻止这里生效。当前 Head 的 Windows job 90812427321 实际执行 "sccache" "c++"，随后把 MSVC 参数 -EHsc、-std:c++20 传给 GNU c++.exe 并失败；exact Base 的 同一 job 成功完成，所以这是本 PR 引入的回归。该失败还使 integration test 因 needs: [build-and-test] 被跳过。请在 Windows 保留 cc-rs 对 cl.exe 的选择，或使用经过验证的 sccache cl.exe 配置，并重跑 Windows build/test 与 integration test。
AlexStocks
commented
Jul 30, 2026
|
|
||
| > 综合来源：`.planning/PROJECT.md`（项目宪法）、`.planning/REQUIREMENTS.md`（可验收需求 v2）、`.planning/ROADMAP.md`（唯一路线图）、`.planning/STATE.md`（当前状态）、`.planning/DECISIONS.md`（已批准决定）。 | ||
| > 稳定性门禁 G1–G7 引用：`docs/quality/system-stability-gate.md`。 | ||
| > 文档性质：纯本地规划文档，不修改 git、CI 或生产源码。 |
Contributor
Author
There was a problem hiding this comment.
[P1] 先同步 implementation task 的项目真相
这里仍把内容描述成 planning-only 的本地文档，但本 PR 同时修改了 CI 和 scripts/dev.sh，而当前 Head 的 .planning/STATE.md / KANBAN.md 仍指向已经合入 Base 的 #383 planning task，并明确禁止在该任务修改 CI/build scripts。CONTRIBUTING.md 又要求每个 implementation PR 引用相关 REQ-*，并用精确验证命令更新 STATE.md 与 KANBAN.md；当前 Diff 没有完成这些门禁。合并后版本化项目真相会与实际实现冲突，恢复流程无法判断应继续还是冻结哪条任务线。请拆分 planning 文档与构建实现，或先建立新的 implementation task，并同步相关 REQ、状态/Kanban 和本次验证结果。
AlexStocks
commented
Jul 30, 2026
| | 反推画像 | 反推依据（来源） | | ||
| |---|---| | ||
| | 平台 / SRE 工程师 | 可观测（REQ-OBS-001/002）、运维演练（G7, ROADMAP M9）、稳定门禁（REQ-STABILITY-001~006） | | ||
| | Redis 迁移用户 | RESP2/3 字节级兼容（REQ-COMPAT-002）、兼容矩阵（REQ-COMPAT-003）、客户端零改造（REQ-COMPAT-005/006） | |
Contributor
Author
There was a problem hiding this comment.
[P1] 不要把测试边界写成“客户端零改造”承诺
REQ-COMPAT-005 只限制 redis-rs 的测试依赖边界，REQ-COMPAT-006 只要求 Pipeline/连接行为回归；两者都没有保证任意现有客户端可以零改造切换，REQ-COMPAT-003 还明确允许兼容矩阵登记 known difference。这里把它们映射成“客户端零改造”，会新增一个没有客户端集合、命令 Profile、配置范围和差异容忍度的产品承诺，也没有可判定的验收标准。请改为有现有证据支持的 raw-frame differential/兼容矩阵表述；若确实要求零改造，则先新增有明确范围的 Requirement，并同步修正 personas-and-user-stories.md:39 的同根表述。
AlexStocks
commented
Jul 30, 2026
| path: | | ||
| ~/.cargo/registry | ||
| ~/.cargo/git | ||
| target |
Contributor
Author
There was a problem hiding this comment.
[P2] 不要用两套 action 重复缓存同一个 target
前面的 setup-rust-toolchain 默认已经调用 Swatinem/rust-cache，当前 Linux build job 明确显示 cache-targets: true 并恢复了包含 target 的约 597 MB；这里的 actions/cache 随后又恢复了包含同一路径的约 656 MB，同一 job 为重叠缓存下载约 1.2 GB。手写 key 还只包含 OS/Cargo.lock，精确命中后不会保存因 compiler、toolchain 或 workflow 配置变化而重建的新产物。请为 target 选择一个 cache owner：移除这里新增的 target，或关闭/配置 rust-cache 后维护一套能区分 toolchain、target、profile、compiler 和 feature 的 key；clippy job 的同类改动也应一并处理。
Preserve MSVC selection on Windows while retaining native sccache defaults on Unix. Remove duplicate target cache ownership and align the PRD, user stories, State, and Kanban with the validated review findings. Constraint: Limit this commit to the seven PR #388 review-fix paths for compiler caching, regression coverage, product documentation, and task state. Confidence: High; the Windows regression is reproduced by the new shell probe and all seven platform/compiler cases pass after the fix. Scope-risk: Build wrapper selection, CI cache ownership, and versioned documentation only; no Rust production code, Cargo dependency, Oracle implementation, or Hot Tier implementation changes. Tested: TDD red reproduced forced GNU CC/CXX on MINGW; green shell probe passed 7/7 cases; Git blob Bash syntax, workflow YAML/cache assertions, documentation ID/table checks, and git diff --cached --check passed. Not-tested: shellcheck and actionlint are unavailable; full Rust/RocksDB build and new remote Head CI require a later push. Co-authored-by: OmX <omx@oh-my-codex.dev>
Give the shell probe the repository ASF license header and preserve its executable bit so Linux can execute the symlinked cargo and sccache test doubles. Constraint: Limit the follow-up to scripts/tests/test-dev-sccache-env.sh and the two failures reported by new-Head CI. Confidence: High; the license content matches .licenserc.yaml and the Git index records mode 100755. Scope-risk: Test harness metadata and header only; no production code, compiler selection logic, workflow topology, or dependency changes. Tested: shell regression probe passed 7/7 cases; normalized Bash syntax passed; git diff --cached --check passed; staged mode verified as 100755. Not-tested: GitHub license-eye and hosted Linux execution require the follow-up remote CI run. Co-authored-by: OmX <omx@oh-my-codex.dev>
AlexStocks
added a commit
that referenced
this pull request
Jul 31, 2026
* docs(planning): define RESP allocation guardrails Record the approved parser resource-limit design, TDD execution steps, and the transition from merged PR #388 to the RESP-LIMITS-001 implementation card. Constraint: This commit changes planning and recovery-facing documentation only; runtime behavior remains unchanged. Confidence: High; the scope follows exact main source inspection and Redis 8.8.1 parser evidence. Scope-risk: Low; only task-specific design, plan, STATE, and KANBAN files are included. Tested: git diff --cached --check; placeholder scan; Requirement and task identifier scan. Not-tested: Runtime tests are deferred to the implementation commit. Co-authored-by: OmX <omx@oh-my-codex.dev> * fix(resp): bound aggregate parser allocations Reject aggregate declarations above the Redis 8.8.1 INT_MAX boundary and cap initial Array, Map, Set, and Push capacity at 1024 entries so unauthenticated input cannot directly request an unbounded Vec allocation. Add regression coverage for exact boundary overflow, i64 capacity overflow, and the maximum accepted declaration without changing existing null or incomplete-frame semantics. Constraint: This commit changes only the RESP parser and its in-file tests; bulk payload and connection buffer limits remain separate work. Confidence: High; the original test failed with capacity overflow and passed after the shared capacity guard was applied. Scope-risk: Low; normal aggregate parsing is unchanged and all four allocation sites use one private helper. Tested: Windows and WSL cargo test -p resp; Windows and WSL target Clippy; cargo fmt --all -- --check; git diff --check. Not-tested: Full workspace and process-level network suites were not rerun because no network, storage, Cargo, or server code changed. Co-authored-by: OmX <omx@oh-my-codex.dev> * fix(resp): bound aggregate nesting depth Reject aggregate nesting beyond 128 levels before entering Array, Map, Set, or Push parsing so capped per-layer allocations and the recursive call stack cannot still grow without bound. Add deterministic coverage for the 1024-entry capacity policy and the 128/129 nesting boundary, and correct the implementation plan exact-test commands and validation record. Constraint: Bulk payload limits and total per-connection parser buffer limits remain separate work; no network, authentication, Cargo, storage, or command behavior is changed. Confidence: High; the new nesting regression failed on the previous PR Head, passed after the shared depth guard, and an independent second review found no remaining finding. Scope-risk: Low; the depth is threaded only through the four existing recursive aggregate parsers and the guard returns through the existing protocol-error path. Tested: Windows and WSL cargo test -p resp (65 unit, 20 integration); Windows and WSL target Clippy; cargo fmt --all -- --check; git diff --check. Not-tested: Full workspace and process-level network suites were not rerun because no network, storage, Cargo, server, or authentication code changed. Co-authored-by: OmX <omx@oh-my-codex.dev> * fix(resp): avoid declaration-driven aggregate allocation Start aggregate containers empty so incomplete untrusted declarations cannot reserve element storage. Keep the Redis INT_MAX length check and the 128-level nesting guard, and measure parser-phase allocations in regression tests. Constraint: Scope remains limited to Issue #395 B1 and the five authorized RESP task paths. Confidence: High; the allocation regressions failed on the previous Head and pass with zero parser-phase allocation on Windows and WSL. Scope-risk: Actual payload size and total connection-buffer limits remain separate follow-up work. Tested: Windows and WSL cargo test -p resp; Windows and WSL cargo clippy -p resp --all-targets -- -D warnings -D clippy::unwrap_used; cargo fmt --all -- --check; git diff --check. Not-tested: Full workspace and native RocksDB suites because this change is confined to the resp crate. Co-authored-by: OmX <omx@oh-my-codex.dev> * docs(planning): record independent RESP allocation PR Update the active branch, PR number, publication history, and final-state instructions after the narrow verified fix moved to PR #406 instead of overwriting externally updated PR #404. Constraint: This commit changes only STATE and KANBAN publication records. Confidence: High; PR #406 and its initial Head were queried directly from GitHub. Scope-risk: None for runtime behavior; the two overlapping PRs still require maintainer reconciliation. Tested: cargo fmt --all -- --check; git diff --check; gh pr view 406. Not-tested: Runtime suites were not rerun because this commit changes planning documents only. Co-authored-by: OmX <omx@oh-my-codex.dev> --------- Co-authored-by: OmX <omx@oh-my-codex.dev>
AlexStocks
added a commit
that referenced
this pull request
Jul 31, 2026
* docs(planning): define RESP allocation guardrails Record the approved parser resource-limit design, TDD execution steps, and the transition from merged PR #388 to the RESP-LIMITS-001 implementation card. Constraint: This commit changes planning and recovery-facing documentation only; runtime behavior remains unchanged. Confidence: High; the scope follows exact main source inspection and Redis 8.8.1 parser evidence. Scope-risk: Low; only task-specific design, plan, STATE, and KANBAN files are included. Tested: git diff --cached --check; placeholder scan; Requirement and task identifier scan. Not-tested: Runtime tests are deferred to the implementation commit. Co-authored-by: OmX <omx@oh-my-codex.dev> * fix(resp): bound aggregate parser allocations Reject aggregate declarations above the Redis 8.8.1 INT_MAX boundary and cap initial Array, Map, Set, and Push capacity at 1024 entries so unauthenticated input cannot directly request an unbounded Vec allocation. Add regression coverage for exact boundary overflow, i64 capacity overflow, and the maximum accepted declaration without changing existing null or incomplete-frame semantics. Constraint: This commit changes only the RESP parser and its in-file tests; bulk payload and connection buffer limits remain separate work. Confidence: High; the original test failed with capacity overflow and passed after the shared capacity guard was applied. Scope-risk: Low; normal aggregate parsing is unchanged and all four allocation sites use one private helper. Tested: Windows and WSL cargo test -p resp; Windows and WSL target Clippy; cargo fmt --all -- --check; git diff --check. Not-tested: Full workspace and process-level network suites were not rerun because no network, storage, Cargo, or server code changed. Co-authored-by: OmX <omx@oh-my-codex.dev> * fix(resp): bound aggregate nesting depth Reject aggregate nesting beyond 128 levels before entering Array, Map, Set, or Push parsing so capped per-layer allocations and the recursive call stack cannot still grow without bound. Add deterministic coverage for the 1024-entry capacity policy and the 128/129 nesting boundary, and correct the implementation plan exact-test commands and validation record. Constraint: Bulk payload limits and total per-connection parser buffer limits remain separate work; no network, authentication, Cargo, storage, or command behavior is changed. Confidence: High; the new nesting regression failed on the previous PR Head, passed after the shared depth guard, and an independent second review found no remaining finding. Scope-risk: Low; the depth is threaded only through the four existing recursive aggregate parsers and the guard returns through the existing protocol-error path. Tested: Windows and WSL cargo test -p resp (65 unit, 20 integration); Windows and WSL target Clippy; cargo fmt --all -- --check; git diff --check. Not-tested: Full workspace and process-level network suites were not rerun because no network, storage, Cargo, server, or authentication code changed. Co-authored-by: OmX <omx@oh-my-codex.dev> * fix(security): bound RESP processing and pipeline queues Constraint: Bound unauthenticated RESP parsing and optional pipeline resources without changing Redis command, storage, or Raft semantics. Confidence: High. Parser budgets, every active network consumer, legacy queue draining, and bounded pipeline admission have direct regression coverage. Scope-risk: Oversized or excessively expensive RESP frames now fail closed; queue admission shares the existing 30-second timeout. Red: Baseline parsing trusted attacker-controlled lengths, replayed incomplete aggregates without a cumulative budget, retained legacy command copies, and used an unbounded pipeline channel. Green: Resource-limit, parser-consumer, pipeline, and Linux TCP isolation regressions pass. Tested: Windows MSVC cargo test -p resp --all-features --locked; cargo test -p net --lib --all-features --locked; strict resp/net Clippy; fmt check; diff check. WSL/Linux exact oversized unauthenticated request and protocol-error isolation tests. Not-tested: Full WSL workspace suite. The earlier Windows workspace run reached all unit suites but retained the repository baseline of 19 TCP startup failures. Co-authored-by: OmX <omx@oh-my-codex.dev> * docs(planning): record RESP limits publication Record implementation commit b260188 and the fast-forward update of PR #404. Keep final-Head checks as the remaining live verification step. Co-authored-by: OmX <omx@oh-my-codex.dev> * test(net): tolerate early oversized-request closure Allow the unauthenticated buffer-limit regression to accept an expected mid-write disconnect, clarify the frozen planning scope, and keep related status notes in valid Markdown blockquotes. Constraint: Preserve cumulative parser replay-work accounting and its CPU-denial-of-service boundary Tested: WSL oversized-request exact test; Windows target Clippy; cargo fmt and diff checks; parser replay-work exact test Co-authored-by: OmX <omx@oh-my-codex.dev> --------- Co-authored-by: OmX <omx@oh-my-codex.dev>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
背景
审计发现 sccache 此前只包 rustc，而真正耗时的 librocksdb-sys C++ 编译经 cc crate 直调 gcc/clang，绕过了 RUSTC_WRAPPER，首编约 18 分钟无任何缓存。
改动
Configure C/C++ compiler caching（CC=sccache cc/CXX=sccache c++，Windows 跳过）；并把target/加入 Cargo 缓存 path。CC/CXX包裹，本地开发同享加速。prd.md与personas-and-user-stories.md，由.planning/治理文档综合生成，补齐此前缺失的 PRD 单文档与用户画像。验证
yaml.safe_load(ci.yml)合法；bash -n dev.sh语法 OK。注意
kiwi-github/下存在与本项目无关的 C++ "quantclaw" CI，建议另行清理（治理噪音）。Summary by CodeRabbit
Documentation
Developer Experience
Chores