Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .claude-plugin/marketplace.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
"name": "pre-push-review",
"source": "./plugins/pre-push-review",
"description": "git push 前に 3 つのレビュー (code review / codex review / security review) の完了を強制するプラグイン。レビュー済みマーカーと「commit 列 (HEAD / merge-base の OID) + ブランチ全差分」の同一性検証により、未レビューの commit が remote に到達するのを構造的にブロックする。",
"version": "5.1.1",
"version": "5.2.0",
"keywords": ["push", "review", "quality", "codex", "code-review", "security-review", "subagent", "branch-diff", "pr-diff", "parallel"]
},
{
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ Claude Code 側で fable-risk-labeler を install していた場合は、plugin
| [git-guardrails](#git-guardrails) | 0.6.1 | GitHub Flow を構造強制するプラグイン。デフォルトブランチ (master/main) への直接書き込み経路 (commit / push / master/main を head とする PR 作成) を PreToolUse hook で deny し、変更を GitHub 上の PR merge 経由のみに限定する |
| [enforce-draft-pr](#enforce-draft-pr) | 0.5.0 | `gh pr create` に `--draft` を自動付与する PreToolUse hook プラグイン (任意導入)。PR を常に draft として作成させ、レビューを経て ready 化する運用を支える |
| [auto-lint-check](#auto-lint-check) | 0.6.0 | 編集後の自動フォーマット適用、git commit 直前の staged ファイル lint、commit 直後の HEAD 再 lint を行うプラグイン。lint の ignore コメント挿入も編集時に禁止する |
| [pre-push-review](#pre-push-review) | 5.1.1 | `git push` 前に 3 つのレビュー (code review / codex review / security review) の完了を強制するプラグイン。レビュー済みマーカーと「commit 列 (HEAD / merge-base の OID) + ブランチ全差分」の同一性検証により、未レビューの commit が remote に到達するのを構造的にブロックする |
| [pre-push-review](#pre-push-review) | 5.2.0 | `git push` 前に 3 つのレビュー (code review / codex review / security review) の完了を強制するプラグイン。レビュー済みマーカーと「commit 列 (HEAD / merge-base の OID) + ブランチ全差分」の同一性検証により、未レビューの commit が remote に到達するのを構造的にブロックする |
| [update-default-branch](#update-default-branch) | 0.4.0 | PR マージ報告を契機にデフォルトブランチを最新化し、追跡先が消えたローカルブランチを片付けるプラグイン |
| [natsuume-statusline](#natsuume-statusline) | 0.10.0 | Claude Code の statusLine 表示 (パス / repo / branch / 変更量 / context 使用量 / レートリミット) を提供するプラグイン。`/natsuume-statusline:setup` で `~/.claude/settings.json` に登録する |
| [agent-discipline](#agent-discipline) | 0.22.1 | 作業規律を SessionStart / SubagentStart prompt で配送し、gh issue/pr body の未決定事項を PreToolUse で検知するプラグイン |
Expand Down
2 changes: 1 addition & 1 deletion plugins/pre-push-review/.claude-plugin/plugin.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "pre-push-review",
"version": "5.1.1",
"version": "5.2.0",
"author": {"name": "natsuume", "url": "https://github.com/natsuume"},
"description": "git push 前に 3 つのレビュー (code review / codex review / security review) の完了を強制するプラグイン。レビュー済みマーカーと「commit 列 (HEAD / merge-base の OID) + ブランチ全差分」の同一性検証により、未レビューの commit が remote に到達するのを構造的にブロックする。"
}
18 changes: 15 additions & 3 deletions plugins/pre-push-review/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,21 @@ Linked worktree では marker、launch attestation、tombstone を main `.git`

## バージョン

v5.1.1
v5.2.0

(前身: `pre-commit-review` v0.4.0)

### v5.1.1 → v5.2.0 の変更点 (issue #337)

codex-reviewer が Bash timeout の background 自動移行後に review 結果を回収できるようになった。従来は `tools` が `Bash` のみだったため、Bash tool が timeout してラッパー実行を background へ自動移行させると codex review 自体は完走していても subagent 側に回収手段が無く、正規の parent-safe report を返せず push gate が詰まっていた。

- `agents/codex-reviewer.md` の frontmatter `tools` を `Bash, TaskOutput, Read` に拡張した (TaskOutput / Read は下記回収専用)
- `## Background-move recovery` セクションを新設した: Bash 結果が background 移行を報告した直後に task ID と output file path を記録し、同一 task ID に対して `TaskOutput (block=true)` で terminal state まで bounded に poll する (初回自動回収は TaskOutput 呼び出し 5 回・各呼び出しはツールの最大 timeout までを予算とする)。report が truncated な場合のみ記録済み output file path を Read で補完する。回収成功時は既存の parent-safe report 契約 (`Status: pass` / `Status: findings` / `Status: execution-failed`) へ normalize する
- 境界条件 3 種で `Status: execution-failed` を返す: task ID 喪失、回収予算超過 (この場合は codex review が background で継続中である旨と、診断専用の単発 status check のためにこの同じ subagent を resume できる旨を report に記載する。resume は marker を昇格できず、push gate を満たすには新規 reviewer run が必要)、TaskOutput が task を見失った場合
- background 移行はそれ自体では wrapper failure (non-zero exit) ではないことを明記し、両者を排他的な扱いに固定した
- `## Constraints` の旧 `Bash` のみ前提の文言 2 箇所を、`Bash, TaskOutput, Read` 構成 (TaskOutput / Read は上記回収専用、Edit / Write / Skill / Task は引き続き非許可) に合わせて改訂した
- 後方互換のある機能追加 (tools 拡張は既存の wrapper-only 実行サーフェスを維持したままの回収経路追加) のため minor bump

### v5.1.0 → v5.1.1 の変更点

code-reviewer / security-reviewer subagent の report contract に、自由記述フィールド (Cause class / Violated invariant / Impact / Fix direction) を 1 文に制限する長さ較正を追加した。
Expand Down Expand Up @@ -170,7 +181,7 @@ plugin description (plugin.json / marketplace.json) を人間向けの簡潔な
### v2.0.1 → v3.0.0 の変更点 (互換破壊あり)

- **`agents/code-reviewer.md` を新設**: v2.x の Skill `/code-review` (Anthropic bundled skill / read-only correctness バグ検出) に相当する self-contained subagent。 prompt は標準 skill と独立に管理 (security-reviewer と同じ理由: 主 session から直接 skill を呼ぶと turn が終了、 subagent 内から呼んでも nested subagent 制約で sub-task が動かないため)。 tools は `Bash, Read, Glob, Grep, LS` で `Skill` / `Task` を含まない (= 標準 skill を invoke できない構造的防御)。
- **`agents/codex-reviewer.md` を新設**: codex review wrapper (`run-codex-review.sh`) を foreground で 1 回起動するだけの最小 subagent。 tools は `Bash` のみで、 wrapper の output (codex review の verdict / findings) を markdown report として親 session に返す。v3.0.0 当時は wrapper が codex-reviewed marker を書いたが、v4.0.1 で pending attestation + auto-mark 昇格へ変更した。
- **`agents/codex-reviewer.md` を新設**: codex review wrapper (`run-codex-review.sh`) を foreground で 1 回起動するだけの最小 subagent。 tools は `Bash` のみで、 wrapper の output (codex review の verdict / findings) を markdown report として親 session に返す。v3.0.0 当時は wrapper が codex-reviewed marker を書いたが、v4.0.1 で pending attestation + auto-mark 昇格へ変更した。v5.2.0 で tools を `Bash, TaskOutput, Read` に拡張し、Bash timeout による background 移行後の回収経路 (詳細は上記バージョン節) を追加した。
- **`commands/review.md` を 3 subagent 並列発出に書き換え**: Skill (`code-review`) + Bash (codex wrapper) + Agent (security-reviewer) の 3 経路混在を、 Agent x 3 (code-reviewer + codex-reviewer + security-reviewer) に統一しました。
- **`auto-mark.sh` の検知ロジックを Skill → Agent に移行 + name-only 受理を廃止**: PRECHECK\_RE と case 文から Skill `code-review` / `security-review` の検知を全廃。v3.0.0 当時は namespace 付き code/security reviewer のみを検知し、codex marker は wrapper が書いた。v4.0.1 では正規 report Status を検証できるようになったため namespace 付き codex-reviewer も検知対象へ加え、pending attestation を final marker へ昇格する。name-only (`code-reviewer` / `security-reviewer` / `codex-reviewer`) は他 plugin の同名 agent との衝突を避けるため引き続き受理しない。
- **`block-pre-push.sh` の deny メッセージを 3 Agent 案内に書き換え**: Skill (`code-review`) と Bash (codex wrapper) の fallback 起動コマンドを Agent x 3 に置換。 wrapper の絶対パス埋め込み (CODEX_WRAPPER_PATH) も削除しました (subagent 経由で起動するため不要)。
Expand Down Expand Up @@ -359,7 +370,8 @@ codex review wrapper (`hooks/scripts/run-codex-review.sh`) を foreground で 1

**動作**:

- tools は `Bash` のみ (Read / Edit / Write / Skill / Task はすべて非許可)。 wrapper-only な実行サーフェスを構造的に強制
- tools は `Bash, TaskOutput, Read` (v5.2.0 で拡張。 Edit / Write / Skill / Task はすべて非許可)。 TaskOutput / Read は Bash timeout による background 移行後の回収専用 (下記) で、 wrapper-only な実行サーフェスは構造的に維持される
- **v5.2.0 で background-move recovery を追加 (issue #337)**: Bash tool が timeout してラッパー実行を background へ自動移行させても、 codex review 自体は完走していることが多い。 subagent は Bash 結果が background 移行を報告した直後に task ID と output file path を記録し、 同一 task ID に対して `TaskOutput (block=true)` で terminal state まで bounded に poll する (初回自動回収は TaskOutput 呼び出し 5 回・各呼び出しはツールの最大 timeout までを予算とする)。 report が truncated な場合のみ、 記録済み output file path に限定して Read で補完する。 回収成功時は既存の parent-safe report 契約へ normalize し、 task ID 喪失・回収予算超過・task 見失いの 3 境界では `Status: execution-failed` を返す (予算超過時は診断専用の resume 経路を report に案内するが、 resume は marker を昇格できず push gate を満たすには新規 reviewer run が必要)。 background 移行はそれ自体では wrapper failure (non-zero exit) 扱いにしない
- subagent body は wrapper を `run_in_background: false` で 1 回起動し、raw output を final reply へコピーせず parent-safe report に変換する
- 親 session は finding の priority / location / impact / verification / fix direction / disposition を受け取る。実行可能な command、payload、環境値、段階的な再現・回避手順、raw stdout / stderr は subagent context に閉じ込められる
- exact detail を使った追加確認が必要な場合は同一 codex-reviewer を resume し、検証結果だけを再度 parent-safe report で受け取る
Expand Down
49 changes: 46 additions & 3 deletions plugins/pre-push-review/agents/codex-reviewer.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
name: codex-reviewer
description: pre-push-review の codex review 専用 subagent。 `git push` 前のレビューループで block-pre-push.sh の deny メッセージが「independent review」 (codex review) のマーカーを「未実行」 または「失効」 と指摘したときに、 Agent / Task tool の subagent_type="pre-push-review:codex-reviewer" で呼び出す。 codex review の実行手順と report 形式は本 subagent の body に定義されている。 完了すると codex-reviewed marker が更新され、 markdown report が親 session に返る。
tools: Bash
tools: Bash, TaskOutput, Read
model: sonnet
color: blue
---
Expand Down Expand Up @@ -32,6 +32,49 @@ You are the codex review runner for the pre-push-review plugin. Your only job is

4. If the wrapper exited non-zero (`tool_response.is_error` true), do not retry. Return `Status: execution-failed`, the numeric exit status when available, a normalized failure class (`wrapper-path`, `codex-unavailable`, `dirty-tree`, `base-resolution`, or `other`), and a conceptual recovery direction. Do not include the failing command, raw error, or command trace.

## Background-move recovery

The Bash tool may time out and move the wrapper run to the background instead
of returning its result. A background move is not by itself a wrapper failure:
when the Bash result reports the run was moved to the background, follow this
recovery section instead of the non-zero-exit path. When the Bash result
reports that the wrapper run was moved to the background, immediately record
the task ID and the output file path that the background-move result
surfaces. Do not start a second wrapper run — the moved run is still the
single authorized wrapper run.

Recover with TaskOutput (block=true) against the same task ID, repeating
until the task reaches a terminal state or the recovery budget is exhausted.
For the initial automatic recovery, make at most five TaskOutput calls, each
with the tool's maximum timeout. Once the recovered run reaches a terminal
state, normalize its output through the existing report contract: a
successful review yields `Status: pass` or `Status: findings`, and a failed
wrapper run yields `Status: execution-failed`.

Normalize only from the complete recovered output: treat the recovered report
as truncated whenever its completeness cannot be established — for example
when a retrieval call returns only a tail chunk instead of the whole report.
If the recovered report is truncated, complete it by Reading the recorded
output file path; if that path was not captured or cannot be read, return
`Status: execution-failed`. Use Read only when the recovered report is
truncated, and only on the same background task's recorded output file path;
do not read other files or independently re-review the diff.

Recovery boundaries:

- If you lost the task ID, return `Status: execution-failed` (failure class
`other`).
- If the recovery budget is exhausted before the task reaches a terminal
state, return `Status: execution-failed`, state in the recovery direction
that the codex review is likely still running in the background, and note
that the parent may resume this same subagent for a diagnostic status check
only. A resumed status check is a single bounded TaskOutput call outside
the initial recovery budget; it is diagnostic only and can never promote
the codex-reviewed marker — satisfying the push gate requires a fresh
reviewer run.
- If TaskOutput reports that the task can no longer be found, return
`Status: execution-failed` (failure class `other`).

## Parent-safe report contract

Allowed status values are `Status: pass | findings | execution-failed`.
Expand Down Expand Up @@ -103,7 +146,7 @@ Keep exact mechanics in this subagent's context:
## Constraints

- **Run the wrapper exactly once.** Do not re-run on failure; failure recovery is the parent's responsibility. The wrapper itself is a sequential, deterministic script — there is no value in retrying it from inside the subagent. (The CLAUDE_PLUGIN_ROOT fallback above is one replacement attempt with a different path — not a retry — and is allowed.)
- **Do not invoke other tools.** Only the `Bash` tool to start the wrapper. Do not read files or independently analyze the diff — the wrapper's codex companion already does the review. Your role is to launch it and normalize its result without changing the verdict.
- **Start the wrapper with the `Bash` tool only.** TaskOutput and Read exist solely for the Background-move recovery section above — TaskOutput to collect a backgrounded run, Read within the scope defined there. Do not use any tool to independently analyze or re-review the diff — the wrapper's codex companion already does the review. Your role is to launch it and normalize its result without changing the verdict.
- **Do not run the wrapper in background.** `run_in_background: true` (Bash option) and shell-level `&` / `|` are deny'd by the plugin's `block-bg-codex-wrapper.sh` PreToolUse hook regardless, but as a discipline always start the wrapper as a plain foreground command. Both forms above (the env-var canonical `bash "${CLAUDE_PLUGIN_ROOT}/hooks/scripts/run-codex-review.sh"` and the fallback `$(...find...) && bash "$WRAPPER"` chain) are plain foreground commands — the `$(...)` substitution is a path-resolution step, not a background process — and either may be used so this subagent can inspect the completed output before producing the parent-safe report.
- **Do not edit the wrapper or any other file.** This subagent's `tools` field grants `Bash` only — Read / Edit / Write / Skill / Task are all disallowed. The intent is to enforce the "wrapper-only" execution surface.
- **Do not edit the wrapper or any other file.** This subagent's `tools` field grants `Bash, TaskOutput, Read` — Edit / Write / Skill / Task remain unavailable. The intent is to keep the execution surface wrapper-only: TaskOutput and Read are recovery instruments for the single authorized wrapper run, not general-purpose tools.
- **Return the parent-safe report as your final reply.** No follow-up actions or text outside the contract. The parent session will decide the next step from the structured summary.
Loading
Loading