diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 96ba0c7..47be519 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -8,7 +8,7 @@ { "name": "plan-review", "description": "Adversarial plan review via cross-model consultation (agy/Claude/Codex)", - "version": "1.7.1", + "version": "1.7.2", "author": { "name": "WooDragon" }, diff --git a/plugins/plan-review/.claude-plugin/plugin.json b/plugins/plan-review/.claude-plugin/plugin.json index f1af6dd..5fbe82b 100644 --- a/plugins/plan-review/.claude-plugin/plugin.json +++ b/plugins/plan-review/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "plan-review", "description": "Adversarial plan review via cross-model consultation (agy/Claude/Codex)", - "version": "1.7.1", + "version": "1.7.2", "author": { "name": "woodragon" } diff --git a/plugins/plan-review/README.md b/plugins/plan-review/README.md index d531172..bdca856 100644 --- a/plugins/plan-review/README.md +++ b/plugins/plan-review/README.md @@ -129,8 +129,9 @@ is written, separate from `plan-review.log`. A plan with dispatch keywords must include a Dispatch Manifest with at least one `agent` row. A plan without dispatch keywords, including Tier0 work, remains -Manifest-free. `plan-review.sh` stores only the approved v2 signature set. The -fixed columns are `step | location | subagent_type | model_source | model | +Manifest-free. `plan-review.sh` stores only the approved v2 signature set — the +set is written on APPROVE and on safety-valve escalation alike. The fixed +columns are `step | location | subagent_type | model_source | model | depends_on | parallel_with`. - `main` rows use `-` for `subagent_type`, `model_source`, and `model`. @@ -151,7 +152,11 @@ fails open. ExitPlanMode → hook intercepts → engine reviews ├─ APPROVE → ack-deny: present review + re-call ExitPlanMode → allow (user's native go/no-go) ├─ CONCERNS/REJECT → deny + feedback → Claude revises → re-submit - └─ max rounds reached → allow through (user decides) + └─ max rounds reached and the plan is the exact revision the last CONCERNS round + reviewed → allow through (user decides). Dispatch Manifest signatures are armed + exactly as on APPROVE, and a Manifest that cannot be armed denies instead of + escalating. Any other revision — edited after the last round, or state predating + this mechanism — gets one more review before it can be escalated. ``` An engine APPROVE is **not** authorization to start work — it only clears the plan diff --git a/plugins/plan-review/scripts/lib/verdict.sh b/plugins/plan-review/scripts/lib/verdict.sh index 11da826..ff6ecf6 100644 --- a/plugins/plan-review/scripts/lib/verdict.sh +++ b/plugins/plan-review/scripts/lib/verdict.sh @@ -80,6 +80,17 @@ render_concerns_or_reject_feedback() { if [ "$verdict" = "REJECT" ]; then feedback_header="Red Team Review — ${review_engine} — REJECT (Round ${total_rounds}/${review_max_total_rounds})" phase_msg="审阅引擎发现 Critical 级别问题。非 Critical 磋商计数已重置,解决 Critical 项后可重新获得 ${review_max_rounds} 轮磋商机会。" + elif [ "$attempt" -gt "$review_max_rounds" ]; then + # attempt is already post-increment here (caller bumps ATTEMPT before + # calling this function — see plan-review.sh's CONCERNS branch), so + # attempt > review_max_rounds means the round that just ran was a + # post-cap supplementary review: the non-Critical valve (ATTEMPT >= + # REVIEW_MAX_ROUNDS) had already armed, but the plan hash changed since + # the last reviewed revision, so the valve fell through to one more real + # review instead of escalating unreviewed. "Round N/MAX" and a negative + # remaining-rounds count would misreport this as a normal in-budget round. + feedback_header="Red Team Review — ${review_engine} — CONCERNS (Round ${attempt}, 补评审)" + phase_msg="本轮是 plan 改动后的补评审,非 Critical 磋商已达上限。同一修订再次调用 ExitPlanMode 将直接呈现给用户做最终裁决;再改 plan 则会再评审一轮。" else remaining=$((review_max_rounds - attempt)) feedback_header="Red Team Review — ${review_engine} — CONCERNS (Round ${attempt}/${review_max_rounds})" diff --git a/plugins/plan-review/scripts/plan-review.sh b/plugins/plan-review/scripts/plan-review.sh index ee6a15e..996ea34 100755 --- a/plugins/plan-review/scripts/plan-review.sh +++ b/plugins/plan-review/scripts/plan-review.sh @@ -271,6 +271,14 @@ CONV_FILE="$COUNTER_DIR/.conversation-${SESSION_ID}" # clearing it (see that branch below) — that is the highest-value moment for # cross-round memory, not a cycle end. HISTORY_FILE="$COUNTER_DIR/.review-history-${SESSION_ID}" +# Plan hash of the most recent CONCERNS/REJECT-reviewed revision. The +# non-Critical safety valve (below) only escalates a plan whose hash matches +# this file's content — an edit made after the last review round must get +# one more review, never ride the valve on the strength of a stale hash. +# Written on CONCERNS (A.4), deleted on REJECT (Critical resets the whole +# non-Critical cycle) and at every cycle-ending cleanup point alongside +# CONV_FILE/HISTORY_FILE. +REVIEWED_HASH_FILE="$COUNTER_DIR/.review-hash-${SESSION_ID}" # Dispatch state is session-scoped, but stale files are global debris. Clean # them for every valid plan-review invocation, including Tier0 plans that have # no Manifest and therefore never enter the approval serializer branch. @@ -320,7 +328,7 @@ if [ -z "$PLAN" ] || [ "$PLAN" = "null" ]; then TRANSCRIPT_PATH=$(echo "$INPUT" | jq -r '.transcript_path // ""' 2>/dev/null || echo "") CWD_VAL=$(echo "$INPUT" | jq -r '.cwd // ""' 2>/dev/null || echo "") log_decision "decision=deny reason=no-plan-content-fail-closed resolve=${RESOLVE_REASON:-none} resolvePath=${RESOLVE_PATH:-empty} planFilePath=${PLAN_FILE_PATH:-empty} top_keys=${TOP_KEYS} tool_input_keys=${TI_KEYS} transcript_path=${TRANSCRIPT_PATH:-empty} cwd=${CWD_VAL:-empty}" - rm -f "$APPROVE_MARKER" "$COUNTER_FILE" "${CONV_FILE:-}" "${HISTORY_FILE:-}" + rm -f "$APPROVE_MARKER" "$COUNTER_FILE" "${CONV_FILE:-}" "${HISTORY_FILE:-}" "${REVIEWED_HASH_FILE:-}" # Error message routed by the resolver's RESOLVE_REASON (see # lib/plan-source.sh: plan_source_error_reason) — sets $REASON directly. plan_source_error_reason @@ -337,7 +345,7 @@ if [ -f "$APPROVE_MARKER" ]; then if [ -z "$APPROVED_HASH" ] || [ "$CURRENT_HASH" = "$APPROVED_HASH" ]; then # True ack-round: plan unchanged (empty marker = legacy format, unconditional allow) log_decision "decision=allow reason=ack-round-approved" - rm -f "$APPROVE_MARKER" "$COUNTER_FILE" "${CONV_FILE:-}" "${HISTORY_FILE:-}" + rm -f "$APPROVE_MARKER" "$COUNTER_FILE" "${CONV_FILE:-}" "${HISTORY_FILE:-}" "${REVIEWED_HASH_FILE:-}" allow_with_reason "Red Team 审阅已通过,plan 放行。" else # Plan was modified after approve: marker invalid, delete and fall through to re-review. @@ -360,13 +368,109 @@ if [ -f "$APPROVE_MARKER" ]; then fi fi +# --- Safety-valve helper functions (defined here, before both valves use them) --- + +# arm_dispatch_state — parse the plan's Dispatch Manifest into dispatch-check's +# state file so subsequent Agent/Task calls are enforced against it. Shared by +# the APPROVE branch (below) and the non-Critical safety valve's escalation +# branch (A.3): a Manifest that APPROVE would have armed is armed identically +# when the round instead terminates via the valve — enforcement must not +# silently lapse just because the cycle ended by escalation instead of a +# clean APPROVE. +# +# Two landing checks beyond the pre-refactor inline version: +# - Pre-mv guard: if DISPATCH_FILE already exists and is NOT a regular file +# (e.g. pre-created as a directory), fail immediately WITHOUT attempting +# mv — `mv -f srcfile dir` does not error, it silently moves srcfile INTO +# the directory, which would both misreport success and leave a stray +# temp file behind. +# - Post-mv guard: after mv, re-require `-f` on DISPATCH_FILE AND a second +# dispatch_state_is_valid_v2 pass on the file actually sitting at that +# path (not the one validated before the move) — catches any other +# landing failure the pre-check does not. +# +# Returns 0 on success (state armed and re-verified), 1 on any failure. The +# APPROVE call site ignores the return (fail-silent, unchanged contract); the +# valve call site treats 1 as fail-closed (deny, do not escalate). +arm_dispatch_state() { + mkdir -p "$DISPATCH_DIR" 2>/dev/null || true + DISPATCH_FILE="$DISPATCH_DIR/.dispatch-${SESSION_ID}.json" + if [ -e "$DISPATCH_FILE" ] && [ ! -f "$DISPATCH_FILE" ]; then + log_decision "manifest-write-skipped reason=target-not-regular" + return 1 + fi + local dispatch_temp + dispatch_temp=$(mktemp "$DISPATCH_DIR/.dispatch-${SESSION_ID}.json.XXXXXX" 2>/dev/null || true) + if [ -z "$dispatch_temp" ] \ + || ! parse_manifest_to_json "$PLAN" "$(plan_hash "$PLAN")" > "$dispatch_temp" 2>/dev/null \ + || ! dispatch_state_is_valid_v2 "$dispatch_temp"; then + rm -f "${dispatch_temp:-}" "$DISPATCH_FILE" + log_decision "manifest-write-skipped reason=invalid-json" + return 1 + fi + mv -f "$dispatch_temp" "$DISPATCH_FILE" + if [ -f "$DISPATCH_FILE" ] && dispatch_state_is_valid_v2 "$DISPATCH_FILE"; then + local dispatch_bytes + dispatch_bytes=$(wc -c < "$DISPATCH_FILE" | tr -d ' ') + log_decision "manifest-written file=$DISPATCH_FILE bytes=$dispatch_bytes" + return 0 + fi + rm -f "${dispatch_temp:-}" "$DISPATCH_FILE" + log_decision "manifest-write-skipped reason=landing-failed" + return 1 +} + +# record_reviewed_hash — persist the plan hash this round's CONCERNS +# verdict reviewed, so the non-Critical safety valve can distinguish "this +# exact revision was just reviewed" from "the author edited the plan after +# the last review round." Same region as arm_dispatch_state: both are +# escalation-path helpers the valve branch consults. +# +# mktemp+mv (not a direct `printf > file`) for the same reason DISPATCH_FILE +# uses a temp-then-rename: the valve's next-round `cat` on REVIEWED_HASH_FILE +# must never observe a partially-written hash. +# +# Returns 0 on success, 1 on any failure (temp-file creation or write) — +# callers must guard with `|| true` under set -euo pipefail. A failed write +# degrades to "no hash recorded," which the valve already treats safely +# (empty REVIEWED_HASH_FILE → review again, never a false escalation). +# Same two landing checks as arm_dispatch_state: a pre-mv guard (fail before +# attempting mv if REVIEWED_HASH_FILE exists and isn't a regular file — `mv -f +# srcfile dir` doesn't error, it silently moves srcfile INTO the directory, +# which would both misreport success and leave a stray temp file behind) and +# a post-mv guard (re-require `-f` on REVIEWED_HASH_FILE after the move). +record_reviewed_hash() { + local hash="$1" + if [ -e "$REVIEWED_HASH_FILE" ] && [ ! -f "$REVIEWED_HASH_FILE" ]; then + log_decision "reviewed-hash-write-failed reason=target-not-regular" + return 1 + fi + local hash_temp + hash_temp=$(mktemp "$COUNTER_DIR/.review-hash-${SESSION_ID}.XXXXXX" 2>/dev/null || true) + if [ -z "$hash_temp" ] || ! printf '%s' "$hash" > "$hash_temp" 2>/dev/null; then + rm -f "${hash_temp:-}" + log_decision "reviewed-hash-write-failed" + return 1 + fi + if ! mv -f "$hash_temp" "$REVIEWED_HASH_FILE" 2>/dev/null; then + rm -f "${hash_temp:-}" + log_decision "reviewed-hash-write-failed reason=mv-failed" + return 1 + fi + if [ ! -f "$REVIEWED_HASH_FILE" ]; then + log_decision "reviewed-hash-write-failed reason=landing-failed" + return 1 + fi + return 0 +} + # --- Global safety valve: total rounds exhausted → hard deny (tombstone counter) --- # Never delete counter — tombstone blocks subsequent calls until human intervenes if [ "$TOTAL_ROUNDS" -ge "$REVIEW_MAX_TOTAL_ROUNDS" ]; then log_decision "decision=deny reason=global-safety-valve total=$TOTAL_ROUNDS" # Keep the COUNTER_FILE tombstone, but the review cycle is over — no further # engine call will happen, so drop the session ref to avoid an orphan CONV_FILE. - rm -f "${CONV_FILE:-}" "${HISTORY_FILE:-}" + rm -f "${CONV_FILE:-}" "${HISTORY_FILE:-}" "${REVIEWED_HASH_FILE:-}" BLOCK_MSG="## Red Team Review — HARD STOP 审阅已达全局上限(${TOTAL_ROUNDS}/${REVIEW_MAX_TOTAL_ROUNDS}),仍存在未解决的审阅意见。 @@ -378,18 +482,67 @@ EOF exit 0 fi -# --- Non-Critical safety valve: CONCERNS rounds exhausted → allow (escalate to user) --- +# --- Non-Critical safety valve: CONCERNS rounds exhausted → allow, but ONLY +# if this exact plan revision is the one the last CONCERNS round reviewed +# (A.3). Any other revision — edited after the last round, or a legacy +# counter state predating this hash mechanism — gets one more real review +# instead of riding the valve unreviewed. --- if [ "$ATTEMPT" -ge "$REVIEW_MAX_ROUNDS" ]; then - log_decision "decision=allow reason=non-critical-safety-valve round=$ATTEMPT total=$TOTAL_ROUNDS" - rm -f "$COUNTER_FILE" "${CONV_FILE:-}" "${HISTORY_FILE:-}" - VALVE_MSG="## Red Team Review — ${REVIEW_ENGINE} — ESCALATED + CURRENT_HASH=$(plan_hash "$PLAN") + REVIEWED_HASH=$(cat "$REVIEWED_HASH_FILE" 2>/dev/null || true) + if [ -z "$REVIEWED_HASH" ] || [ "$CURRENT_HASH" != "$REVIEWED_HASH" ]; then + # Empty REVIEWED_HASH covers every "not reviewed at this revision" case + # uniformly: hash file absent, a directory, or unreadable. Do NOT allow, + # do NOT clear the counter — fall through to pre-flight and the engine + # review below. A CONCERNS verdict there increments the counter as usual + # and records this revision's hash (A.4), so the next identical-revision + # ExitPlanMode call clears the valve. + log_decision "decision=review-again reason=plan-not-reviewed-at-this-revision round=$ATTEMPT total=$TOTAL_ROUNDS" + else + if has_manifest "$PLAN"; then + if arm_dispatch_state; then + log_decision "decision=allow reason=non-critical-safety-valve round=$ATTEMPT total=$TOTAL_ROUNDS" + rm -f "$COUNTER_FILE" "${CONV_FILE:-}" "${HISTORY_FILE:-}" "$REVIEWED_HASH_FILE" + VALVE_MSG="## Red Team Review — ${REVIEW_ENGINE} — ESCALATED + +非 Critical 磋商已达上限(${ATTEMPT}/${REVIEW_MAX_ROUNDS}),未能达成一致。Plan 直接呈现给用户做最终裁决。 +已按 plan 的 Dispatch Manifest 写入 dispatch-check 状态,Agent 调用须与 Manifest 签名一致。" + VALVE_JSON=$(printf '%s' "$VALVE_MSG" | jq -Rs .) + cat << EOF +{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"allow","permissionDecisionReason":${VALVE_JSON}}} +EOF + exit 0 + else + # Fail-closed: a Manifest that cannot be armed must not let the plan + # through un-enforced. Same accounting as a pre-flight rejection — + # TOTAL_ROUNDS increments, ATTEMPT is frozen — so persistent failure + # still eventually trips the global valve. arm_dispatch_state already + # cleaned up any dispatch temp/target file it touched internally. + TOTAL_ROUNDS=$((TOTAL_ROUNDS + 1)) + echo "${ATTEMPT}:${TOTAL_ROUNDS}" > "$COUNTER_FILE" + log_decision "decision=deny reason=valve-arm-failed round=$ATTEMPT total=$TOTAL_ROUNDS" + ARM_FAIL_MSG="## Red Team Pre-flight — DISPATCH STATE NOT ARMED + +Manifest 无法序列化为 dispatch 状态,请按 v2 七列修正 Manifest 后重试。" + ARM_FAIL_JSON=$(printf '%s' "$ARM_FAIL_MSG" | jq -Rs .) + cat << EOF +{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"deny","permissionDecisionReason":${ARM_FAIL_JSON}}} +EOF + exit 0 + fi + else + log_decision "decision=allow reason=non-critical-safety-valve round=$ATTEMPT total=$TOTAL_ROUNDS" + rm -f "$COUNTER_FILE" "${CONV_FILE:-}" "${HISTORY_FILE:-}" "$REVIEWED_HASH_FILE" + VALVE_MSG="## Red Team Review — ${REVIEW_ENGINE} — ESCALATED 非 Critical 磋商已达上限(${ATTEMPT}/${REVIEW_MAX_ROUNDS}),未能达成一致。Plan 直接呈现给用户做最终裁决。" - VALVE_JSON=$(printf '%s' "$VALVE_MSG" | jq -Rs .) - cat << EOF + VALVE_JSON=$(printf '%s' "$VALVE_MSG" | jq -Rs .) + cat << EOF {"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"allow","permissionDecisionReason":${VALVE_JSON}}} EOF - exit 0 + exit 0 + fi + fi fi # --- Pre-flight manifest checks (AFTER non-critical valve, not before) --- @@ -725,21 +878,12 @@ if [ "$VERDICT" = "APPROVE" ]; then # Write plan hash to marker — ack-round guard compares hash to detect post-approve edits printf '%s' "$(plan_hash "$PLAN")" > "$APPROVE_MARKER" - # Parse manifest → dispatch JSON for Layer 2 enforcement (fail-silent: Layer 2 self-disables) + # Parse manifest → dispatch JSON for Layer 2 enforcement (fail-silent: Layer 2 + # self-disables). arm_dispatch_state (defined above, shared with the + # non-Critical safety valve's escalation branch) does the parse/validate/ + # write/re-verify sequence; this call site ignores its return value. if has_manifest "$PLAN"; then - mkdir -p "$DISPATCH_DIR" 2>/dev/null || true - DISPATCH_FILE="$DISPATCH_DIR/.dispatch-${SESSION_ID}.json" - DISPATCH_TEMP=$(mktemp "$DISPATCH_DIR/.dispatch-${SESSION_ID}.json.XXXXXX" 2>/dev/null || true) - if [ -n "$DISPATCH_TEMP" ] \ - && parse_manifest_to_json "$PLAN" "$(plan_hash "$PLAN")" > "$DISPATCH_TEMP" 2>/dev/null \ - && dispatch_state_is_valid_v2 "$DISPATCH_TEMP"; then - mv -f "$DISPATCH_TEMP" "$DISPATCH_FILE" - dispatch_bytes=$(wc -c < "$DISPATCH_FILE" | tr -d ' ') - log_decision "manifest-written file=$DISPATCH_FILE bytes=$dispatch_bytes" - else - rm -f "${DISPATCH_TEMP:-}" "$DISPATCH_FILE" - log_decision "manifest-write-skipped reason=invalid-json" - fi + arm_dispatch_state || true fi # Emit deny so Claude presents the approval to the user (allow reasons are invisible) @@ -750,11 +894,22 @@ fi TOTAL_ROUNDS=$((TOTAL_ROUNDS + 1)) if [ "$VERDICT" = "REJECT" ]; then - # REJECT (Critical): reset ATTEMPT so subsequent non-Critical rounds restart fresh + # REJECT (Critical): reset ATTEMPT so subsequent non-Critical rounds restart fresh. + # Also drop the reviewed-hash marker (A.4) — a Critical reset restarts the + # whole non-Critical cycle, so no revision should be treated as "already + # reviewed" by the safety valve after this point. ATTEMPT=0 + rm -f "$REVIEWED_HASH_FILE" else - # CONCERNS: non-Critical, increment ATTEMPT + # CONCERNS: non-Critical, increment ATTEMPT. ATTEMPT=$((ATTEMPT + 1)) + # Record which revision this CONCERNS round reviewed (A.4) — the safety + # valve only escalates a revision whose hash matches this file's content. + # `|| true`: under set -euo pipefail a failed hash write must never abort + # the hook mid-deny; record_reviewed_hash's own cleanup already makes a + # write failure equivalent to "no hash recorded," which the valve already + # treats safely (empty file → review again, never a false escalation). + record_reviewed_hash "$(plan_hash "$PLAN")" || true fi echo "${ATTEMPT}:${TOTAL_ROUNDS}" > "$COUNTER_FILE" diff --git a/plugins/plan-review/tests/plan-review.bats b/plugins/plan-review/tests/plan-review.bats index b8a8a66..ad60876 100644 --- a/plugins/plan-review/tests/plan-review.bats +++ b/plugins/plan-review/tests/plan-review.bats @@ -85,12 +85,24 @@ teardown() { # ============================================================================= # 6. Non-Critical safety valve → allow JSON with ESCALATED +# NOTE: setup changed by the plan-not-reviewed-at-this-revision safety valve +# fix — the valve now only escalates a plan revision the last CONCERNS round +# actually reviewed (see plan-review.sh's REVIEWED_HASH_FILE mechanism), so +# hitting ATTEMPT>=MAX_ROUNDS with no reviewed hash on file no longer +# escalates on its own; it falls through to one more real review round +# first, which is what seeds a matching hash for the second call below. @test "counter: non-critical safety valve → allow JSON with ESCALATED" { set_counter_value 3 test-session 5 export REVIEW_MAX_ROUNDS=3 + create_mock_engine "agy" "CONCERNS +[Major] one more round needed." INPUT=$(build_input) run_hook + assert_deny_json + [ "$(get_counter_value)" -eq 4 ] + [ "$(get_total_rounds)" -eq 6 ] + run_hook assert_approve_json local reason reason=$(echo "$HOOK_STDOUT" | jq -r '.hookSpecificOutput.permissionDecisionReason') @@ -99,6 +111,254 @@ teardown() { [ ! -f "${REVIEW_COUNTER_DIR}/.review-count-test-session" ] } +# write_reviewed_hash [session] +# Seeds REVIEWED_HASH_FILE with plan_hash(plan) so a single run_hook call +# lands the non-Critical safety valve on its "this exact revision was +# already reviewed" branch instead of the mismatch/review-again branch — +# isolates a test on the valve's manifest handling from the separately +# covered mismatch mechanic (see the "valve: plan changed..." / +# "valve: legacy counter..." tests below). Mirrors create_approve_marker's +# own hash computation above. +write_reviewed_hash() { + local plan_content="$1" session="${2:-test-session}" + local marker="${REVIEW_COUNTER_DIR}/.review-hash-${session}" + if command -v sha256sum >/dev/null 2>&1; then + printf '%s' "$plan_content" | sha256sum | awk '{print $1}' > "$marker" + elif command -v shasum >/dev/null 2>&1; then + printf '%s' "$plan_content" | shasum -a 256 | awk '{print $1}' > "$marker" + else + printf '%s' "$plan_content" | cksum | awk '{print $1}' > "$marker" + fi +} + +@test "valve: escalation with agent-row manifest writes dispatch state (v2)" { + export REVIEW_MAX_ROUNDS=3 + local plan + plan=$(manifest_v2_plan '| 1 | main | - | - | - | - | - | +| 2 | agent | dev-econ | preset | - | 1 | - | +| 3 | agent | Explore | runtime | haiku | 1 | 2 |') + write_reviewed_hash "$plan" + set_counter_value 3 test-session 3 + INPUT=$(build_input "plan=$plan") + run_hook + + assert_approve_json + local reason + reason=$(echo "$HOOK_STDOUT" | jq -r '.hookSpecificOutput.permissionDecisionReason') + [[ "$reason" == *"ESCALATED"* ]] + [[ "$reason" == *"dispatch-check"* ]] + local dispatch_file="${REVIEW_COUNTER_DIR}/.dispatch-test-session.json" + [ -f "$dispatch_file" ] + jq -e '.schema_version == 2' "$dispatch_file" >/dev/null + jq -e '.allowed_signatures == [{"subagent_type":"dev-econ","model_source":"preset"},{"subagent_type":"Explore","model_source":"runtime","model":"haiku"}]' "$dispatch_file" >/dev/null + [ ! -f "${REVIEW_COUNTER_DIR}/.review-count-test-session" ] +} + +@test "valve: escalation without manifest writes no dispatch state (v2)" { + export REVIEW_MAX_ROUNDS=3 + write_reviewed_hash "Test plan content" + set_counter_value 3 test-session 3 + INPUT=$(build_input) + run_hook + + assert_approve_json + local reason + reason=$(echo "$HOOK_STDOUT" | jq -r '.hookSpecificOutput.permissionDecisionReason') + [[ "$reason" == *"ESCALATED"* ]] + [ ! -f "${REVIEW_COUNTER_DIR}/.dispatch-test-session.json" ] +} + +@test "valve: plan changed since last reviewed round is reviewed again (v2)" { + export REVIEW_MAX_ROUNDS=3 + set_counter_value 3 test-session 3 + printf '%s' "0000" > "${REVIEW_COUNTER_DIR}/.review-hash-test-session" + create_mock_engine "agy" "CONCERNS +[Major] still needs work." + INPUT=$(build_input) + run_hook + + assert_deny_json + local reason + reason=$(echo "$HOOK_STDOUT" | jq -r '.hookSpecificOutput.permissionDecisionReason') + [[ "$reason" == *"CONCERNS"* ]] + [[ "$reason" == *"补评审"* ]] + [[ "$reason" != *"-1"* ]] + [[ "$reason" != *"4/3"* ]] + [ "$(get_counter_value)" -eq 4 ] + [ "$(get_total_rounds)" -eq 4 ] + local hash_file="${REVIEW_COUNTER_DIR}/.review-hash-test-session" + [ -s "$hash_file" ] + [ "$(cat "$hash_file")" != "0000" ] + + run_hook + assert_approve_json + reason=$(echo "$HOOK_STDOUT" | jq -r '.hookSpecificOutput.permissionDecisionReason') + [[ "$reason" == *"ESCALATED"* ]] + [ ! -f "$hash_file" ] + [ ! -f "${REVIEW_COUNTER_DIR}/.review-count-test-session" ] +} + +@test "valve: legacy counter without hash file is reviewed once before escalation (v2)" { + export REVIEW_MAX_ROUNDS=3 + set_counter_value 3 test-session 3 + create_mock_engine "agy" "CONCERNS +[Major] still needs work." + INPUT=$(build_input) + run_hook + + assert_deny_json + local reason + reason=$(echo "$HOOK_STDOUT" | jq -r '.hookSpecificOutput.permissionDecisionReason') + [[ "$reason" == *"CONCERNS"* ]] + [[ "$reason" == *"补评审"* ]] + [[ "$reason" != *"-1"* ]] + [[ "$reason" != *"4/3"* ]] + [ "$(get_counter_value)" -eq 4 ] + [ "$(get_total_rounds)" -eq 4 ] + [ -s "${REVIEW_COUNTER_DIR}/.review-hash-test-session" ] + + run_hook + assert_approve_json + reason=$(echo "$HOOK_STDOUT" | jq -r '.hookSpecificOutput.permissionDecisionReason') + [[ "$reason" == *"ESCALATED"* ]] +} + +@test "hash: CONCERNS round records reviewed plan hash (v2)" { + create_mock_engine "agy" "CONCERNS +[Major] needs work." + INPUT=$(build_input plan="Plan A") + run_hook + assert_deny_json + local hash_file="${REVIEW_COUNTER_DIR}/.review-hash-test-session" + [ -s "$hash_file" ] + local hash_a + hash_a=$(cat "$hash_file") + + INPUT=$(build_input plan="Plan B") + run_hook + assert_deny_json + [ -s "$hash_file" ] + local hash_b + hash_b=$(cat "$hash_file") + [ "$hash_a" != "$hash_b" ] +} + +@test "hash: ack-round approval clears reviewed hash file (v2)" { + create_mock_engine "agy" "CONCERNS +[Major] needs work." + INPUT=$(build_input) + run_hook + assert_deny_json + local hash_file="${REVIEW_COUNTER_DIR}/.review-hash-test-session" + [ -s "$hash_file" ] + + create_mock_engine "agy" "APPROVE +Looks good now." + run_hook + assert_ack_approve_json + + run_hook + assert_approve_json + [ ! -f "$hash_file" ] +} + +@test "valve: REJECT after threshold resets cycle and does not escalate (v2)" { + export REVIEW_MAX_ROUNDS=3 + set_counter_value 3 test-session 3 + printf '%s' "0000" > "${REVIEW_COUNTER_DIR}/.review-hash-test-session" + create_mock_engine "agy" "REJECT +[Critical] broken." + INPUT=$(build_input) + run_hook + + assert_deny_json + local reason + reason=$(echo "$HOOK_STDOUT" | jq -r '.hookSpecificOutput.permissionDecisionReason') + [[ "$reason" == *"REJECT"* ]] + [ "$(get_counter_value)" -eq 0 ] + [ "$(get_total_rounds)" -eq 4 ] + [ ! -f "${REVIEW_COUNTER_DIR}/.review-hash-test-session" ] + + run_hook + assert_deny_json + reason=$(echo "$HOOK_STDOUT" | jq -r '.hookSpecificOutput.permissionDecisionReason') + [[ "$reason" == *"REJECT"* ]] + [[ "$reason" != *"ESCALATED"* ]] + [ ! -f "${REVIEW_COUNTER_DIR}/.dispatch-test-session.json" ] +} + +@test "valve: manifest present but dispatch state cannot be written denies (v2)" { + export REVIEW_MAX_ROUNDS=3 + local plan='## Plan +Use Task( for isolation. + +## Dispatch Manifest +| step | location | subagent_type | model_source | model | depends_on | +|------|----------|---------------|--------------|-------|------------| +| 1 | main | - | - | - | - |' + write_reviewed_hash "$plan" + set_counter_value 3 test-session 3 + INPUT=$(build_input "plan=$plan") + run_hook + + assert_deny_json + local reason + reason=$(echo "$HOOK_STDOUT" | jq -r '.hookSpecificOutput.permissionDecisionReason') + [[ "$reason" == *"dispatch 状态"* ]] + [ ! -f "${REVIEW_COUNTER_DIR}/.dispatch-test-session.json" ] + [ "$(get_counter_value)" -eq 3 ] + [ "$(get_total_rounds)" -eq 4 ] +} + +@test "valve: dispatch state landing failure denies (v2)" { + export REVIEW_MAX_ROUNDS=3 + local plan + plan=$(manifest_v2_plan '| 1 | main | - | - | - | - | - | +| 2 | agent | dev-econ | preset | - | 1 | - | +| 3 | agent | Explore | runtime | haiku | 1 | 2 |') + write_reviewed_hash "$plan" + set_counter_value 3 test-session 3 + mkdir "${REVIEW_COUNTER_DIR}/.dispatch-test-session.json" + INPUT=$(build_input "plan=$plan") + run_hook + + assert_deny_json + local reason + reason=$(echo "$HOOK_STDOUT" | jq -r '.hookSpecificOutput.permissionDecisionReason') + [[ "$reason" == *"dispatch 状态"* ]] + [ -d "${REVIEW_COUNTER_DIR}/.dispatch-test-session.json" ] + [ -z "$(ls -A "${REVIEW_COUNTER_DIR}/.dispatch-test-session.json")" ] + ! find "$REVIEW_COUNTER_DIR" -maxdepth 1 -name '.dispatch-test-session.json.*' -print -quit | grep -q . + [ "$(get_counter_value)" -eq 3 ] + [ "$(get_total_rounds)" -eq 4 ] +} + +@test "hash: unwritable hash path never breaks the hook nor escalates (v2)" { + export REVIEW_MAX_ROUNDS=3 + mkdir "${REVIEW_COUNTER_DIR}/.review-hash-test-session" + create_mock_engine "agy" "CONCERNS +[Major] needs work." + set_counter_value 2 test-session 2 + INPUT=$(build_input) + run_hook + + assert_deny_json + local reason + reason=$(echo "$HOOK_STDOUT" | jq -r '.hookSpecificOutput.permissionDecisionReason') + [[ "$reason" == *"CONCERNS"* ]] + [ "$(get_counter_value)" -eq 3 ] + [ "$(get_total_rounds)" -eq 3 ] + ! find "$REVIEW_COUNTER_DIR" -maxdepth 1 -name '.review-hash-test-session.*' -print -quit | grep -q . + [ -z "$(ls -A "${REVIEW_COUNTER_DIR}/.review-hash-test-session")" ] + + run_hook + assert_deny_json + reason=$(echo "$HOOK_STDOUT" | jq -r '.hookSpecificOutput.permissionDecisionReason') + [[ "$reason" == *"CONCERNS"* ]] + [[ "$reason" != *"ESCALATED"* ]] + [ -z "$(ls -A "${REVIEW_COUNTER_DIR}/.review-hash-test-session")" ] +} + # 7. Below max rounds → proceeds to review @test "counter: below max rounds → calls engine" { set_counter_value 2 test-session 3