docs(todo): #369/#370 feedback の採用 3 件を登録し narrow-fix 教訓を順位 375 へ補強 - #372
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughTODO 文書に、 ChangesTODO 文書の検証タスク
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/todo20.md`:
- Around line 323-325: Update the work-plan and completion-criteria references
near the five-item checklist so they consistently state that there are five
items, including the newly added narrow-fix adjacent-edge check. Ensure the
completion flow requires verification of all five items before marking the entry
complete.
In `@docs/todo21.md`:
- Around line 135-141: docs/todo21.md の「完了基準」に、設定内の Write(...) 検出を必須 CI check
の失敗条件として明記してください。Write(...) が検出された場合は warning のみで通過させず、CI
を失敗させてマージを阻止することを完了条件に含め、Edit(...) と Read(...) は引き続き許可される状態を維持してください。
- Around line 162-179: docs/dev-conventions.md の「jj 運用の落とし穴と復旧」チェックリストに、jj
バージョン、リモート有無、初期コミットグラフ、bookmark 位置、ワークツリー状態を明記した最小の再現手順を追加してください。jj squash
--into と jj new master の操作例は、その初期状態から実行・結果確認できる形にし、既存のコミット確定・squash 方向・bookmark
復旧の指針を維持してください。
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: bb2373f6-635f-41ac-aaaa-00577dbdc992
📒 Files selected for processing (3)
docs/todo-summary2.mddocs/todo20.mddocs/todo21.md
| > 5. **narrow な修正を入れたら、隣接エッジに穴が残っていないか確認する** (#369/#370 で複数回再演、memory `dont-trust-takt-fix-output` と同根)。fix step / 自分の修正が「指摘された 1 点」だけを塞ぐと、同じクラスの入力空間の別の点が素通りになる。実例: 不可視文字の除去を公開面だけに入れ parse 側の枠検査を素通りさせた / 出力先を 1 つ (PR 本文) 塞いで step ログを見落とした。**入力空間・出力経路を「点」ではなく「クラス / 経路の集合」として一度に固める**。 | ||
| > | ||
| > **参照**: [ADR-042](adr/adr-042-rule-vs-mechanism-boundary.md) (ルール vs 仕組みの線引き — 4 件とも機械 lint 化が難しくルール側)、[ADR-068](adr/adr-068-fix-step-authority-boundary.md) (fix step の権限境界)、[ADR-048](adr/adr-048-facet-findings-handoff-markdown-contract.md) (findings handoff の contract)。 | ||
| > **参照**: [ADR-042](adr/adr-042-rule-vs-mechanism-boundary.md) (ルール vs 仕組みの線引き — いずれも機械 lint 化が難しくルール側)、[ADR-068](adr/adr-068-fix-step-authority-boundary.md) (fix step の権限境界)、[ADR-048](adr/adr-048-facet-findings-handoff-markdown-contract.md) (findings handoff の contract)、memory `dont-trust-takt-fix-output` (narrow 修正の隣接穴)。 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
作業計画と完了基準を5項目に更新してください。
Line [323] で5項目目を追加していますが、Line [331] と Line [338] はまだ「4項目」と記載しています。
このままでは、5項目目を確認せずにエントリを完了扱いにできます。
- - [ ] `docs/dev-conventions.md` にチェックリストを 1 本追記 (4 項目、由来 PR 付き)
+ - [ ] `docs/dev-conventions.md` にチェックリストを 1 本追記 (5 項目、由来 PR 付き)
- - 4 項目がいずれも「由来 (どの PR のどの指摘か)」付きで dev-conventions.md に存在すること。
+ - 5 項目がいずれも「由来 (どの PR のどの指摘か)」付きで dev-conventions.md に存在すること。🤖 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/todo20.md` around lines 323 - 325, Update the work-plan and
completion-criteria references near the five-item checklist so they consistently
state that there are five items, including the newly added narrow-fix
adjacent-edge check. Ensure the completion flow requires verification of all
five items before marking the entry complete.
| - [ ] `Write(...)` 指定子を検出する regex 層ルールを custom-lint-rules.toml に追加 | ||
| - [ ] `.claude/settings*.json` と `.github/workflows/*.yml` の `claude_args` を検査対象に含める | ||
| - [ ] 現行リポジトリで false positive が出ないことを確認 (Edit/Read 指定子は許可) | ||
|
|
||
| #### 完了基準 | ||
|
|
||
| - `Write(path)` 指定子を含む設定が検知され、`Edit(path)` / `Read(path)` は検知されないこと。 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Write(...) 検出を必須 CI 失敗として完了条件にしてください。
この項目は Write(...) を silent security failure と定義しています。一方、作業計画と完了基準は「warning」と「検知」だけを要求しています。警告が非ブロッキングなら、無効な deny 設定を残したまま merge できます。Write(...) を検出した場合に必須 check を失敗させる条件を明記してください。
提案する完了条件
- [ ] `Write(path)` 指定子を含む設定が検知され、`Edit(path)` / `Read(path)` は検知されないこと。
+ [ ] `Write(path)` 指定子を含む設定が検知され、必須 CI check が失敗すること。
+ [ ] `Edit(path)` / `Read(path)` は検知されないこと。📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - [ ] `Write(...)` 指定子を検出する regex 層ルールを custom-lint-rules.toml に追加 | |
| - [ ] `.claude/settings*.json` と `.github/workflows/*.yml` の `claude_args` を検査対象に含める | |
| - [ ] 現行リポジトリで false positive が出ないことを確認 (Edit/Read 指定子は許可) | |
| #### 完了基準 | |
| - `Write(path)` 指定子を含む設定が検知され、`Edit(path)` / `Read(path)` は検知されないこと。 | |
| - [ ] `Write(...)` 指定子を検出する regex 層ルールを custom-lint-rules.toml に追加 | |
| - [ ] `.claude/settings*.json` と `.github/workflows/*.yml` の `claude_args` を検査対象に含める | |
| - [ ] 現行リポジトリで false positive が出ないことを確認 (Edit/Read 指定子は許可) | |
| #### 完了基準 | |
| - `Write(path)` 指定子を含む設定が検知され、必須 CI check が失敗すること。 | |
| - `Edit(path)` / `Read(path)` は検知されないこと。 |
🤖 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/todo21.md` around lines 135 - 141, docs/todo21.md の「完了基準」に、設定内の
Write(...) 検出を必須 CI check の失敗条件として明記してください。Write(...) が検出された場合は warning
のみで通過させず、CI を失敗させてマージを阻止することを完了条件に含め、Edit(...) と Read(...)
は引き続き許可される状態を維持してください。
| ### jj の落とし穴 (squash の方向・空コミットでの bookmark ずれ) を dev-conventions へ | ||
|
|
||
| > **動機**: 本セッション (#364〜#371) で jj 運用の落とし穴を繰り返し踏んだ。(a) `jj squash --into <bookmark>` の方向が直感と逆で、ターゲットが description なしの新コミットへ移動する / (b) `jj bookmark set` の後退拒否と `jj abandon` による bookmark 消失・復旧 / (c) `jj new master` を「コミット確定」のつもりで実行して変更を前コミットに取り残す。いずれも復旧に op log 参照が要った。memory には別の jj squash gotcha (headless editor hang) が既に記録済みで、jj 運用の落とし穴は systemic に再発している。 | ||
| > | ||
| > **対処案**: [dev-conventions.md](dev-conventions.md) に「jj 運用の落とし穴と復旧」チェックリストを 1 本追加する。**コミット確定は `jj describe` + `jj bookmark set/create`、`jj new` は新しい作業を始めるときだけ**、`jj squash` は方向を確認、bookmark がずれたら `jj edit <bookmark>` で戻す、を明文化。順位 386 (空コミットでの bookmark ずれ) の機構側対処とは別に、運用ルール側で人間 / agent を守る。 | ||
| > | ||
| > **参照**: `.claude/feedback-reports/369.md` Tier 3 #1、memory `jj-squash-editor-hang-headless` / `jj-concurrent-session-op-divergence`、順位 386 (機構側対処)、[ADR-021](adr/adr-021-jj-change-detection-principles.md)。 | ||
| > | ||
| > **実行優先度**: 🔧 Tier 3 — Severity Medium (作業の取り残し・bookmark 消失。ただし loud で復旧可) / Frequency High (本セッションで複数回) / Effort S / Adoption Risk None (docs-only)。 | ||
|
|
||
| #### 作業計画 | ||
|
|
||
| - [ ] `docs/dev-conventions.md` に「jj 運用の落とし穴と復旧」チェックリストを追記 (由来セッション付き) | ||
| - [ ] 既存 memory (squash hang / op divergence) と重複せず補完する形にする | ||
|
|
||
| #### 完了基準 | ||
|
|
||
| - コミット確定・squash 方向・bookmark ずれ復旧の 3 点が根拠つきで dev-conventions に存在すること。 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- jj version ---'
jj --version
printf '%s\n' '--- squash semantics ---'
jj help squash | rg -n -C 3 -- '--into|working copy|description' || true
printf '%s\n' '--- related project guidance ---'
rg -n -C 3 'jj squash --into|jj new master|jj bookmark set|jj abandon' \
docs/todo21.md docs/dev-conventions.md || trueRepository: aloekun/claude-code-hook-test
Length of output: 226
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- files ---'
git ls-files | rg '(^|/)(docs/todo21.md|docs/dev-conventions.md|README|.*\.md)$|(^|/)jj|memory|todo' || true
printf '%s\n' '--- todo21 snippet ---'
sed -n '150,185p' docs/todo21.md 2>/dev/null || true
printf '%s\n' '--- existing guidance snippets ---'
sed -n '1,220p' docs/dev-conventions.md 2>/dev/null || true
printf '%s\n' '--- references to jj guidance / lock files ---'
git ls-files | rg 'jj|dev-conventions|requirements|poetry|pnpm|package|Makefile|README|docs/todo21.md|memory' || true
rg -n -C 3 '\bjj\b|jj squash|jj new|jj bookmark|dev-conventions|memo|memory' . \
--glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' || trueRepository: aloekun/claude-code-hook-test
Length of output: 50388
jj の操作例を再現可能な初期状態で記録してください。
jj squash --into や jj new master の結果は jj のバージョン、git リモートの有無、コミットグラフ、bookmark の場所、ワークツリーの状態に依存します。docs/dev-conventions.md はいずれかを実際に実行・検証できる最小の条件で追記してください。
🤖 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/todo21.md` around lines 162 - 179, docs/dev-conventions.md の「jj
運用の落とし穴と復旧」チェックリストに、jj バージョン、リモート有無、初期コミットグラフ、bookmark
位置、ワークツリー状態を明記した最小の再現手順を追加してください。jj squash --into と jj new master
の操作例は、その初期状態から実行・結果確認できる形にし、既存のコミット確定・squash 方向・bookmark 復旧の指針を維持してください。
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
このPRは Applicable Findings (Critical / High / Major)
Applicable Findings (Medium 以下)
Filtered (not applicable)該当なし 次のアクション
|
…Phase 2 C) WP-18 の prompt injection 対策 PR (#369/#370) の post-merge feedback の採用候補の うち、セッション中に未対応で価値の高い分をユーザー承認 (2026-08-09) のうえ登録する。 ## 新規登録 (todo21.md、389-391) - 389 (T1): Write(path) tool-scope 指定子の no-op を検出する settings validator。 CLI 2.1.218 で Write() は no-op = deny の silent 無効化。順位 379 で実際に踏んだ。 **検知は warning ではなく error (必須 CI 失敗) にする** — silent security failure は ADR-043 の fail-closed 対象 (#372 CodeRabbit 指摘) - 390 (T2): 台帳 framing 区切りの定数 (LEDGER_DATA_FRAME_MARKER) と workflow リテラル (===BEGIN/END_LEDGER_DATA===) の cross-file 一致を CI 検証 - 391 (T3): jj の落とし穴 (squash 方向・空コミットでの bookmark ずれ) を dev-conventions へ。本セッションで複数回踏んだ。**操作例は再現可能な最小の初期状態 つきで書く** (jj バージョン・リモート有無・コミットグラフ・bookmark 位置に依存する ため、断定形でなく前提つきで、#372 CodeRabbit 指摘) ## 順位 375 補強 (todo20.md) narrow-fix が隣接エッジに穴を作る教訓を 5 項目目として追加 (#369/#370 で複数回 再演、memory dont-trust-takt-fix-output と同根)。新規 rank は立てず既存 375 を編集。 ## 見送り (ユーザー非選択) Cf カテゴリ網羅テスト (#369 T1#3) / GITHUB_TOKEN vs App token 挙動差テスト (#364 T2#3) / pre-push warning 対応方針 (#370 T3#4) は今回登録しない。既に セッション中に対応済みの候補も再登録しない。 ## 検証 pnpm lint:docs OK / markdownlint 0 error。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
c1b981e to
47845a5
Compare
Summary
Context
Why: 3 回分の feedback(#364/#369/#370)の採用候補をトリアージし、(1) セッション中に既に対応済みのもの、(2) ユーザーが非選択のもの、を除いた a/b/d/e の 4 件をユーザー承認(2026-08-09)のうえ反映する。
新規登録(389-391):
Write(path)tool-scope 指定子の no-op を検出する settings validator。CLI 2.1.218 でWrite()は no-op(Edit()のみ機能)= deny の silent 無効化。順位 379 で実際に踏んだLEDGER_DATA_FRAME_MARKER)と workflow リテラル(===BEGIN/END_LEDGER_DATA===)の cross-file 一致を CI 検証。片側変更で決定 13 の framing が破れるsquash方向・空コミットでの bookmark ずれ)を dev-conventions へ。本セッションで複数回踏んだ順位 375 補強(e): narrow-fix の教訓を 5 項目目として追加。#369/#370 で複数回再演し、memory
dont-trust-takt-fix-outputと同根。見送り: Cf カテゴリ網羅テスト(#369 T1#3)/
GITHUB_TOKENvs App token 挙動差テスト(#364 T2#3)/ pre-push warning 対応方針(#370 T3#4)は今回登録しない(ユーザー非選択)。既にセッション中に対応済みの候補(秘密値非記録・toggle 受け入れ基準・tee 露出・output-contract regression 等)も再登録しない。Validation
pnpm lint:docs: OK(preamble + cross-ref + priority-inversion)markdownlint: 0 errorpnpm pushpre-push review: verdict=APPROVE(両 facet)References
.claude/feedback-reports/364.md/369.md/370.md(採用候補の出所)Summary by CodeRabbit