Repository navigation
claude/add-review-progress-logging-eq7PL - #13
Conversation
…mization - Change default execution mode from MCP to exec (CLI direct) - Progress now visible in STDERR during single /codex-review - Legacy MCP mode available via execution_mode: mcp setting - Parallel experts remain MCP (Claude's built-in parallel tool calls) - Add output constraints to all 8 expert prompts: - English only (saves tokens, Claude integrates in Japanese) - Max 500 chars per expert - Critical/High: all, Medium/Low: max 3 each - No issues → "Score: A / No issues."
ウォークスルーこのPRはCodexレビュー機能をMCPベースの呼び出しから直接Codex CLIの実行に移行します。プロンプト言語を日本語から英語に統一し、実行モード設定を導入し、複数の専門家プロンプトに出力制限(最大500文字、重要度別レポート制限)を追加します。 変更内容
推定コードレビュー労力🎯 2 (Simple) | ⏱️ ~12 分 関連する可能性のあるPR
詩
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@skills/codex-review/references/experts/accessibility-expert.md`:
- Around line 28-32: The OUTPUT FORMAT section expects a detailed table with
File/Line/Issue/WCAG/Fix entries which can exceed the current "English only, max
500 chars" constraint; update the guidance in accessibility-expert.md
(referencing the "OUTPUT FORMAT" block) to either raise the character limit
(suggest 700–800 chars) or explicitly allow a short summary (≤500 chars) plus an
expanded per-issue details block for each finding, and make the rule consistent
with the example output and WCAG requirement language so automated validators
and reviewers know which limit to enforce.
In `@skills/codex-review/references/experts/architect-expert.md`:
- Around line 29-32: Update the English length constraint for the architecture
expert: change the "English only, max 500 chars" rule to a higher limit (suggest
~1000 chars) or split the constraint so that the main summary remains concise
while the "Tradeoff Analysis" gets its own separate, larger limit; locate and
edit the rule text (the line containing "English only, max 500 chars") and the
OUTPUT FORMAT / "Tradeoff Analysis" guidance to reflect the new limits and
ensure they are consistent.
- Around line 29-34: Update the ambiguous language instruction line (- **English
only, max 500 chars** (Claude integrates in Japanese)) to a single, explicit
rule used across all expert files (see scope-analyst-expert.md for desired
format): state the primary response language ("English") and, if an alternate
integration language like Japanese is permitted for a specific model (Claude),
specify the exact condition and notation to use (e.g., "Primary: English; For
Claude integrations, responses may be in Japanese — annotate with '(Claude:
Japanese)'"). Apply this same explicit phrasing to the corresponding line in all
expert files to ensure consistency.
In `@skills/codex-review/references/experts/scope-analyst-expert.md`:
- Around line 29-33: The "English only, max 500 chars" constraint in
scope-analyst-expert.md is too strict for thorough requirement analysis; update
that directive to a higher limit (suggest 750–1000 characters) by replacing the
"English only, max 500 chars" line with "English only, max 750-1000 chars" and
adjust any related guidance or validation expectations in the same file (e.g.,
examples or scoring rules like "Score: A / No issues.") to reflect the new limit
so analysts can provide fuller findings, questions, and risk descriptions.
- Around line 29-31: The language instruction is ambiguous because the line
"**English only, max 500 chars** (Claude integrates in Japanese)" contradicts
itself; remove ambiguity by replacing that line with an explicit output-language
field such as "Output language: English — max 500 characters" or, if Japanese
output is intended, "Output language: Japanese — max 500 characters", and ensure
any parenthetical note about Claude integration is moved to a separate
clarification line like "Note: Claude may integrate with Japanese inputs" so the
directives in the header (the bolded language+length rule) and the explanatory
note are not conflicting.
In `@skills/codex-review/references/experts/security-expert.md`:
- Around line 27-31: The guidance line "**English only, max 500 chars**" imposes
an overly strict length limit for security-expert reports; update this rule to
either state "English only, max 1000+ chars" or add an explicit exemption for
security experts (e.g., "security-expert reports exempt from char limit") so
vulnerability findings can include detailed description, exploit vector, impact,
and remediation; make the change in security-expert.md by replacing the 500-char
rule with the relaxed/exception wording and keep the rest of the bullets
unchanged.
- Line 28: Update the vulnerability reporting guideline that currently reads
"Critical/High: report all, Medium/Low: max 3 each" so it no longer caps
Medium/Low findings; replace that phrase with wording that requires all
Critical/High and all Medium vulnerabilities to be reported (and remove or
revise the "max 3 each" constraint for Low to either report all or a justified
sampling policy). Locate the exact string "Critical/High: report all,
Medium/Low: max 3 each" in the document and change it to something like
"Critical/High: report all, Medium: report all, Low: [policy]" or an equivalent
that explicitly mandates reporting all Medium findings.
🧹 Nitpick comments (1)
commands/optional/codex-review.md (1)
123-142: マルチラインの prompt は heredoc 化が安全です。
現状の引用符付き文字列は改行/エスケープの影響を受けやすいので、可読性と安全性の面で heredoc を推奨します。♻️ 置き換え案
-codex exec "Review the following code changes and output issues and improvement suggestions: - -Files: {changed_files} - -{file_contents}" +codex exec <<'EOF' +Review the following code changes and output issues and improvement suggestions: + +Files: {changed_files} + +{file_contents} +EOF
| - **English only, max 500 chars** (Claude integrates in Japanese) | ||
| - Critical/High: report all, Medium/Low: max 3 each | ||
| - No issues → `Score: A / No issues.` | ||
| - WCAG 2.1 AA baseline | ||
| - Consider framework-specific patterns (React/Vue/Svelte) |
There was a problem hiding this comment.
アクセシビリティレビューの出力形式と文字数制限の整合性を確認してください。
OUTPUT FORMAT(50-66行目)で期待される詳細なテーブル(File、Line、Issue、WCAG、Fix列を含む)と500文字制限が矛盾する可能性があります。複数のa11y問題が検出された場合、WCAG基準への参照と具体的な修正案を含めると、制限を超過する可能性があります。
出力例をベースに実際の文字数を検証し、必要に応じて制限を調整してください(例: 700-800文字)。
🤖 Prompt for AI Agents
In `@skills/codex-review/references/experts/accessibility-expert.md` around lines
28 - 32, The OUTPUT FORMAT section expects a detailed table with
File/Line/Issue/WCAG/Fix entries which can exceed the current "English only, max
500 chars" constraint; update the guidance in accessibility-expert.md
(referencing the "OUTPUT FORMAT" block) to either raise the character limit
(suggest 700–800 chars) or explicitly allow a short summary (≤500 chars) plus an
expanded per-issue details block for each finding, and make the rule consistent
with the example output and WCAG requirement language so automated validators
and reviewers know which limit to enforce.
| - **English only, max 500 chars** (Claude integrates in Japanese) | ||
| - Critical/High: report all, Medium/Low: max 3 each | ||
| - No issues → `Score: A / No issues.` | ||
| - Avoid premature over-abstraction |
There was a problem hiding this comment.
アーキテクチャレビューでは500文字制限が不十分です。
アーキテクチャエキスパートは設計パターン、トレードオフ分析、スケーラビリティの評価を行いますが、500文字ではトレードオフの詳細な説明(現在のアプローチの長所/短所、推奨アプローチとその理由など)を提供することが困難です。OUTPUT FORMATセクション(76-80行目)で期待される「Tradeoff Analysis」の詳細度と矛盾しています。
アーキテクチャレビューの性質を考慮し、文字数制限を1000文字程度に緩和するか、トレードオフ分析を別枠として扱うことを検討してください。
🤖 Prompt for AI Agents
In `@skills/codex-review/references/experts/architect-expert.md` around lines 29 -
32, Update the English length constraint for the architecture expert: change the
"English only, max 500 chars" rule to a higher limit (suggest ~1000 chars) or
split the constraint so that the main summary remains concise while the
"Tradeoff Analysis" gets its own separate, larger limit; locate and edit the
rule text (the line containing "English only, max 500 chars") and the OUTPUT
FORMAT / "Tradeoff Analysis" guidance to reflect the new limits and ensure they
are consistent.
| - **English only, max 500 chars** (Claude integrates in Japanese) | ||
| - Critical/High: report all, Medium/Low: max 3 each | ||
| - No issues → `Score: A / No issues.` | ||
| - Avoid premature over-abstraction | ||
| - Base decisions on actual requirements | ||
|
|
There was a problem hiding this comment.
言語指示の一貫性を改善してください。
scope-analyst-expert.mdと同様、「English only」と「(Claude integrates in Japanese)」の関係が不明確です。全エキスパートファイルで統一された明確な言語指示が必要です。
🤖 Prompt for AI Agents
In `@skills/codex-review/references/experts/architect-expert.md` around lines 29 -
34, Update the ambiguous language instruction line (- **English only, max 500
chars** (Claude integrates in Japanese)) to a single, explicit rule used across
all expert files (see scope-analyst-expert.md for desired format): state the
primary response language ("English") and, if an alternate integration language
like Japanese is permitted for a specific model (Claude), specify the exact
condition and notation to use (e.g., "Primary: English; For Claude integrations,
responses may be in Japanese — annotate with '(Claude: Japanese)'"). Apply this
same explicit phrasing to the corresponding line in all expert files to ensure
consistency.
| - **English only, max 500 chars** (Claude integrates in Japanese) | ||
| - Critical/High: report all, Medium/Low: max 3 each | ||
| - No issues → `Score: A / No issues.` | ||
| - Evaluate from "can this actually be implemented?" perspective | ||
| - Avoid overly strict criteria |
There was a problem hiding this comment.
計画レビューの出力制約を見直してください。
計画レビューエキスパートは、実装をブロックする可能性のあるギャップ、曖昧さ、不足コンテキストを検出し、具体的な改善提案を提供する役割があります。500文字制限では、複数の問題点と改善提案を十分に説明できない可能性があります。特に、OUTPUT FORMAT(62-88行目)で期待される評価サマリーテーブルと詳細なFindings テーブルを含めると、制限を超過する可能性が高いです。
計画レビューの包括性を維持するため、文字数制限を800-1000文字に緩和することを推奨します。
| - **English only, max 500 chars** (Claude integrates in Japanese) | ||
| - Critical/High: report all, Medium/Low: max 3 each | ||
| - No issues → `Score: A / No issues.` | ||
| - Focus on real problems | ||
| - Avoid over-analysis |
There was a problem hiding this comment.
500文字制限が要件分析には厳しすぎる可能性があります。
スコープアナリストは隠れた要件、曖昧さ、依存関係、リスクを検出する役割を持っていますが、500文字の制限では詳細な分析結果や質問リストを十分に提供できない可能性があります。特に、複雑な要件や複数の曖昧さが存在する場合、制限により重要な情報が省略される恐れがあります。
要件分析の性質を考慮し、文字数制限を750-1000文字に緩和することを検討してください。
🤖 Prompt for AI Agents
In `@skills/codex-review/references/experts/scope-analyst-expert.md` around lines
29 - 33, The "English only, max 500 chars" constraint in scope-analyst-expert.md
is too strict for thorough requirement analysis; update that directive to a
higher limit (suggest 750–1000 characters) by replacing the "English only, max
500 chars" line with "English only, max 750-1000 chars" and adjust any related
guidance or validation expectations in the same file (e.g., examples or scoring
rules like "Score: A / No issues.") to reflect the new limit so analysts can
provide fuller findings, questions, and risk descriptions.
| - **English only, max 500 chars** (Claude integrates in Japanese) | ||
| - Critical/High: report all, Medium/Low: max 3 each | ||
| - No issues → `Score: A / No issues.` |
There was a problem hiding this comment.
言語指示の曖昧さを明確化してください。
「English only」という制約と「(Claude integrates in Japanese)」という注記が矛盾しています。エキスパートの出力言語が英語なのか日本語なのかが不明確です。
💡 提案される修正
出力言語を明確に指定してください:
-- **English only, max 500 chars** (Claude integrates in Japanese)
+- **Output in English only, max 500 chars** (main Claude system integrates responses in Japanese)または、日本語出力を意図している場合:
-- **English only, max 500 chars** (Claude integrates in Japanese)
+- **Output in Japanese, max 500 chars** (constraints documented in English for clarity)📝 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.
| - **English only, max 500 chars** (Claude integrates in Japanese) | |
| - Critical/High: report all, Medium/Low: max 3 each | |
| - No issues → `Score: A / No issues.` | |
| - **Output in English only, max 500 chars** (main Claude system integrates responses in Japanese) | |
| - Critical/High: report all, Medium/Low: max 3 each | |
| - No issues → `Score: A / No issues.` |
| - **English only, max 500 chars** (Claude integrates in Japanese) | |
| - Critical/High: report all, Medium/Low: max 3 each | |
| - No issues → `Score: A / No issues.` | |
| - **Output in Japanese, max 500 chars** (constraints documented in English for clarity) | |
| - Critical/High: report all, Medium/Low: max 3 each | |
| - No issues → `Score: A / No issues.` |
🤖 Prompt for AI Agents
In `@skills/codex-review/references/experts/scope-analyst-expert.md` around lines
29 - 31, The language instruction is ambiguous because the line "**English only,
max 500 chars** (Claude integrates in Japanese)" contradicts itself; remove
ambiguity by replacing that line with an explicit output-language field such as
"Output language: English — max 500 characters" or, if Japanese output is
intended, "Output language: Japanese — max 500 characters", and ensure any
parenthetical note about Claude integration is moved to a separate clarification
line like "Note: Claude may integrate with Japanese inputs" so the directives in
the header (the bolded language+length rule) and the explanatory note are not
conflicting.
| - **English only, max 500 chars** (Claude integrates in Japanese) | ||
| - Critical/High: report all, Medium/Low: max 3 each | ||
| - No issues → `Score: A / No issues.` | ||
| - Reduce false positives by considering context | ||
| - Consider framework-specific security features |
There was a problem hiding this comment.
セキュリティレビューでは500文字制限がリスクになります。
セキュリティエキスパートは脆弱性の詳細な説明、攻撃ベクトル、修正手順を提供する必要がありますが、500文字では不十分です。例えば、SQLインジェクションの脆弱性を報告する際には、脆弱なコード箇所、攻撃可能性、影響範囲、具体的な修正コードの提示が必要であり、これらを500文字に収めることは困難です。不完全なセキュリティレポートは、開発者が脆弱性の深刻さを理解できず、適切な対策を講じられないリスクがあります。
セキュリティドメインの特性を考慮し、文字数制限を1000文字以上に緩和するか、セキュリティエキスパートのみ制限を除外することを強く推奨します。
🤖 Prompt for AI Agents
In `@skills/codex-review/references/experts/security-expert.md` around lines 27 -
31, The guidance line "**English only, max 500 chars**" imposes an overly strict
length limit for security-expert reports; update this rule to either state
"English only, max 1000+ chars" or add an explicit exemption for security
experts (e.g., "security-expert reports exempt from char limit") so
vulnerability findings can include detailed description, exploit vector, impact,
and remediation; make the change in security-expert.md by replacing the 500-char
rule with the relaxed/exception wording and keep the rest of the bullets
unchanged.
| - false positive を減らすためコンテキストを考慮 | ||
| - フレームワーク固有のセキュリティ機能を考慮 | ||
| - **English only, max 500 chars** (Claude integrates in Japanese) | ||
| - Critical/High: report all, Medium/Low: max 3 each |
There was a problem hiding this comment.
Medium/Low脆弱性の報告制限を再考してください。
「Medium/Low: max 3 each」という制限により、複数の中程度のセキュリティ問題が隠される可能性があります。セキュリティの文脈では、複数のMedium脆弱性が組み合わさることでHigh/Criticalリスクに発展する場合があります。
セキュリティレビューでは少なくともMedium脆弱性は全件報告することを推奨します。
🤖 Prompt for AI Agents
In `@skills/codex-review/references/experts/security-expert.md` at line 28, Update
the vulnerability reporting guideline that currently reads "Critical/High:
report all, Medium/Low: max 3 each" so it no longer caps Medium/Low findings;
replace that phrase with wording that requires all Critical/High and all Medium
vulnerabilities to be reported (and remove or revise the "max 3 each" constraint
for Low to either report all or a justified sampling policy). Locate the exact
string "Critical/High: report all, Medium/Low: max 3 each" in the document and
change it to something like "Critical/High: report all, Medium: report all, Low:
[policy]" or an equivalent that explicitly mandates reporting all Medium
findings.
## 背景 v4.0.0 "Hokage" リリース直後の 2 日間で 13 件の v3 残骸バグが偶然発見された。 全て inclusion-based verification(「X が含まれるか」)の盲点であり、 exclusion-based verification(「削除済み X が残っていないか」)を systematic に追加するのが Phase 40 の目的。 ## Task 40.0.1: .claude/rules/deleted-concepts.yaml 削除済みパス・概念の SSOT カタログを新規作成。 - **deleted_paths** (5 件): - `core/src/guardrails` — TS guardrail engine 廃止 - `core/dist` — TS ビルド成果物廃止 - `hook-handlers/memory-bridge` — v3 bash shim 廃止 - `hook-handlers/permission-denied-handler` — 同上 - `hook-handlers/runtime-reactive` — 同上 - **deleted_concepts** (4 件, うち 1 件は scan_disabled): - `TypeScript guardrail engine` / `TypeScript ガードレールエンジン` - `Harness v3` - `(v3)` H1 サフィックス(check-residue.sh で特別処理) - `Node.js 18+` allowlist 設計: CHANGELOG.md・アーカイブ・worktrees・歴史ドキュメントを除外。 意図的な grep パターン文字列(setup-codex.sh 等)も明示的に除外。 ## Task 40.0.2: scripts/check-residue.sh Python3 を primary parser とした migration residue scanner を実装。 - `exec python3 - "$@"` パターンで bash はランチャーに徹する設計 - deleted_paths: `grep -rln -F` でリポジトリ全体スキャン(--exclude-dir=.git) - deleted_concepts: 英語・日本語 term の両方をスキャン - H1 (v3) サフィックス: `^# .*(v3)` の特別パターンマッチ - allowlist の prefix match で歴史ドキュメントを除外 - 違反あり → exit 1、0 件 → exit 0 ## DoD 検証結果 - YAML valid: `python3 -c "import yaml; yaml.safe_load(...)"` → exit 0 - YAML エントリ数: deleted_paths 5 + deleted_concepts 4 = 9 件(8-10 の範囲内) - 意図的混入テスト: README.md に `core/src/guardrails/rules.ts` を一時追加 → 検出 14 件(+1)→ revert → 13 件(正常復帰) - 現 HEAD での検出: 13 件(本物の v3 残骸。Phase 40.1 以降で順次修正予定) ## 期待検出件数(retroactive 手動カウント) v4.0.0 リリース commit (8d8ce3c) 直後の状態で scanner を実行した場合、 今回の 13 件に加え、README.md の残骸 (#10, #13) と SKILL.md frontmatter の "Harness v3" (#5, #6, #7) も検出されたはずで、 合計 **20 件前後**の検出が期待される。 c2af730 (docs: purge v3 residue from README) と他修正により README.md 関連 4-5 件が削減され現在の 13 件に至る。 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
redesign README.md は既に正直 (auto-approve 訂正=104.3 / badge=105.10 / 3画面=105.1)。 言行一致の差別化点 (wiring/台帳/binary drift gate が README 主張を機械検証) を明記。 判断カードは wired:no のため ✅ 主張しない (言行一致原則)。finding Chachamaru127#13 の HOTL 草稿自己矛盾は untracked draft の話で本 branch 対象外。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…mization
Summary by CodeRabbit
新機能
改善
✏️ Tip: You can customize this high-level summary in your review settings.