docs(todo): PR #222 post-merge-feedback T1-1 + T3-1 採用 entries 追加 (順位 223/224) - #223
Conversation
… 223/224) PR #222 post-merge-feedback (.claude/feedback-reports/222.md) で採用承認された 2 項目を docs/todo10.md + docs/todo-summary.md に登録。Bundle 222-FB として 1 PR 推奨 (T1 mechanical + T3 docs の 2 層防御を 1 PR で land)。 - 順位 223 (🔧 Tier 2、Effort M): ACTIVE_RUN_FRESH_THRESHOLD_SECS と ORPHAN_THRESHOLD_SECS の compile-time 同期。hooks-stop-quality と hooks-session-start で 1500s magic number を別 crate 独立に持ち、現状はコメント契約のみで mechanical enforcement が欠落。cli-merge-pipeline/src/feedback.rs:60 の derived const + 上流定数 precedent を cross-crate に拡張、Option C (const re-export + const _: () = assert!(...) compile-time check) を MVP 推奨。analyzer Tier 1 = mechanical enforcement → feedback_tier_classification per project Tier 2 に再分類。 - 順位 224 (💎 Tier 3、Effort XS): ADR-043 (Security/Quality Gate Fail-Closed) に hooks-stop-quality の error handling を具体例として追記。meta_is_fresh / meta_is_active_run / takt_subsession_active の 4 error path (mtime 取得失敗 / clock skew / malformed JSON / file read error) すべてが false 返却 → gate effective に倒れる fail-closed 構造を concrete instantiation として codify。 新 PC 環境でも同 task list を確認できるよう docs-only PR として分離 (実装は別 PR で着手)。
|
Warning Review limit reached
More reviews will be available in 45 minutes and 25 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate 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 see our Fair Usage Limits Policy for further information. 📝 WalkthroughWalkthrough
ChangesTODO文書更新
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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: 1
🧹 Nitpick comments (1)
docs/todo10.md (1)
973-979: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
takt_subsession_activeのread_dirエラーパスが欠落している。Line 955 で「
meta_is_fresh()/meta_is_active_run()/takt_subsession_active()はすべての error path でfalseを返却」と宣言しているが、Line 973-977 の列挙はmeta_is_freshとmeta_status_is_running（JSON/file read）のエラーのみで、takt_subsession_active内のstd::fs::read_dirエラー（runs_dir不存在/permission denied 時のErr(_) => return false）が含まれていない。具体例の網羅性を保つため、5 番目のパスとして追加すべき：
- runs_dir 読み取り失敗 (
std::fs::read_dir()error):Err(_) => return false（takt_subsession_active内）あるいは、列挙対象を「
meta_is_fresh/meta_is_active_runの内部エラーパス」と明確に限定し、takt_subsession_activeは「上位 caller として全 subsession 走査失敗時も false」を補足する。🤖 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/todo10.md` around lines 973 - 979, The error-path list is missing the `takt_subsession_active` case for `std::fs::read_dir` failures, so update the examples to include the `runs_dir` read failure path and its `false` return. In `takt_subsession_active`, document that a missing or inaccessible `runs_dir` (`read_dir` error) is also fail-closed and returns `false`, or alternatively narrow the scope of the list to only the internal error paths of `meta_is_fresh` and `meta_is_active_run` while clarifying the caller-level behavior of `takt_subsession_active`.
🤖 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/todo10.md`:
- Around line 923-925: Clarify the MVP recommendation so it matches the actual
implementation approach: the current “Option Cベース” wording conflicts with the
need to add a `hooks-session-start` dependency and make `ORPHAN_THRESHOLD_SECS`
public, which is a hybrid of `Option B` and `Option C`. Update the text around
the `const _: () = assert!(...)` plan to explicitly call it an “Option B+C
ハイブリッド” and reference the `stop-hook` and
`hooks-session-start::reaper::ORPHAN_THRESHOLD_SECS` symbols so readers
understand both the compile-time assertion and the cross-crate const exposure
are required.
---
Nitpick comments:
In `@docs/todo10.md`:
- Around line 973-979: The error-path list is missing the
`takt_subsession_active` case for `std::fs::read_dir` failures, so update the
examples to include the `runs_dir` read failure path and its `false` return. In
`takt_subsession_active`, document that a missing or inaccessible `runs_dir`
(`read_dir` error) is also fail-closed and returns `false`, or alternatively
narrow the scope of the list to only the internal error paths of `meta_is_fresh`
and `meta_is_active_run` while clarifying the caller-level behavior of
`takt_subsession_active`.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3bfe7c85-65d6-4a59-b8ed-3b60d6d4f530
📒 Files selected for processing (2)
docs/todo-summary.mddocs/todo10.md
Resolved findings: - [Major] docs/todo10.md:925 「MVP 推奨」の記述と実際のアプローチに齟齬がある。
1 participant
Summary
PR #222 (
fix(merge-pipeline, stop-hook): sync_local の stale master 解消...) の post-merge-feedback (.claude/feedback-reports/222.md) で ユーザー承認された 2 採用候補 をdocs/todo10.md+docs/todo-summary.mdに登録するだけの docs-only PR。新 PC 環境でも同 task list を確認できるよう docs を分離 (実装は別 PR で着手)。
採用 entries
ACTIVE_RUN_FRESH_THRESHOLD_SECSとORPHAN_THRESHOLD_SECSの compile-time 同期順位 223 (Tier 2 mechanical enforcement)
hooks-stop-quality::ACTIVE_RUN_FRESH_THRESHOLD_SECS = 1500とhooks-session-start::reaper::ORPHAN_THRESHOLD_SECS = 1500は 同値必須。非対称になると両防御層の隙間に挟まる run が漏れる。現状はhooks-stop-quality/src/main.rs:67のコメント (「reaper のORPHAN_THRESHOLD_SECS(= 1500s) と同値」) のみで mechanical enforcement が欠落。precedent:
cli-merge-pipeline/src/feedback.rs:60のpub const ORPHAN_THRESHOLD_SECS: u64 = TAKT_TIMEOUT_SECS + 300;(derived const + 上流定数 reference)。本 task は cross-crate 版に拡張。MVP 推奨: Option C (reaperconst を pub 昇格 → stop-hook から use →const _: () = assert!(...)で compile-time check)。analyzer の Tier 1 表記は memory
feedback_tier_classificationper project Tier 2 (🔧) に再分類。順位 224 (Tier 3 docs amendment)
PR #222 で hooks-stop-quality に追加した
meta_is_fresh()/meta_is_active_run()/takt_subsession_active()は、4 error path (mtime 取得失敗 / clock skew / malformed JSON / file read error) すべてがfalse返却 → gate effective に倒れる fail-closed pattern を踏襲。ADR-043 (Security/Quality Gate Fail-Closed) は試験運用 ADR として既存だが abstract な原則記述に留まっており、PR #222 実装は ADR-043 の concrete instantiation として追記価値あり。Bundle 222-FB 推奨
順位 223 (mechanical) + 順位 224 (docs) の 2 層防御を 1 PR で land。
cli-merge-pipeline/src/feedback.rs:60の precedent と同じ「derived const + 1 文書 cite」構造が 1 commit に揃い、reviewer の cross-reference 検証がjj diff内で完結する。Pre-push review result
pre-push-reviewworkflow: all("approved") (aggregate), 0 anomaly findings, 0 blocking issues.Test plan
git pull後docs/todo-summary.md順位 223/224 とdocs/todo10.md末尾 2 entries が同期されることを確認Summary by CodeRabbit