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
42 changes: 39 additions & 3 deletions runtime/bin/pr-gate.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2224,9 +2224,30 @@ else
def finding_contract:
.findings | type == "array" and all(.[]; finding) and
([.[].id] | length) == ([.[].id] | unique | length);
def findings_array:
if (.findings | type) == "array" then .findings else [] end;
def blocking_severity_violation:
[findings_array[] |
select(
(.hard_gate_class | IN("soft_block","hard_block")) and
((.severity | IN("critical","high")) | not)
)
] | first;
def blocking_origin_violation:
[findings_array[] |
select(
(.hard_gate_class | IN("soft_block","hard_block")) and
((.origin | IN("diff_caused","uncertain")) | not)
)
] | first;
def display:
if . == null
then "<missing>"
else (tostring | tojson | .[1:-1])
end;
def verdict_contract:
(.verdict | IN("approve","advise","block-soft","block")) and
(.rationale | nonempty) and
(.rationale | nonempty) and
(if .verdict == "block-soft"
then any(.findings[]; .hard_gate_class == "soft_block")
elif .verdict == "block"
Expand All @@ -2245,6 +2266,18 @@ else
then "invalid top-level or binding contract"
elif (coverage_contract | not)
then "invalid coverage contract"
elif (blocking_severity_violation != null)
then (blocking_severity_violation as $invalid |
"invalid finding contract: " + ($invalid.id | display) +
" hard_gate_class=" + ($invalid.hard_gate_class | display) +
" requires severity=critical|high (got " +
($invalid.severity | display) + ")")
elif (blocking_origin_violation != null)
then (blocking_origin_violation as $invalid |
"invalid finding contract: " + ($invalid.id | display) +
" hard_gate_class=" + ($invalid.hard_gate_class | display) +
" requires origin=diff_caused|uncertain (got " +
($invalid.origin | display) + ")")
elif (finding_contract | not)
then "invalid finding contract"
elif $references != null and (evidence_reference_contract | not)
Expand Down Expand Up @@ -5285,8 +5318,11 @@ printf -v REVIEWER_PROTOCOL_INSTRUCTIONS \
' origin=diff_caused|pre_existing|uncertain|caution, source={path,line,symbol},' \
' affected_behavior, why_it_matters, failure_mode, minimum_fix_boundary, and' \
' verification_expectation. source needs path plus line or symbol.' \
' - pre_existing/caution findings are non-blocking. block-soft needs a soft_block' \
' finding; block needs a hard_block finding; approve/advise may contain only none.' \
' - soft_block/hard_block findings require severity=critical|high and' \
' origin=diff_caused|uncertain. medium/low and pre_existing/caution findings' \
' must use hard_gate_class=none.' \
' - block-soft needs a soft_block finding; block needs a hard_block finding;' \
' approve/advise may contain only hard_gate_class=none.' \
' - verdict is exactly approve|advise|block-soft|block. Map legacy pass and' \
' pass-not-applicable to approve; map needs-tests to block. Never put pass,' \
' pass-not-applicable, needs-tests, or prose in verdict; prose belongs in rationale.' \
Expand Down
35 changes: 34 additions & 1 deletion runtime/lib/gate-result-verify.sh
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,30 @@ _gate_reviewer_protocol_document_verify() {
def finding_contract:
.findings | type == "array" and all(.[]; finding) and
([.[].id] | length) == ([.[].id] | unique | length);
def findings_array:
if (.findings | type) == "array" then .findings else [] end;
def blocking_severity_violation:
[findings_array[] |
select(
(.hard_gate_class | IN("soft_block","hard_block")) and
((.severity | IN("critical","high")) | not)
)
] | first;
def blocking_origin_violation:
[findings_array[] |
select(
(.hard_gate_class | IN("soft_block","hard_block")) and
((.origin | IN("diff_caused","uncertain")) | not)
)
] | first;
def display:
if . == null
then "<missing>"
else (tostring | tojson | .[1:-1])
end;
def verdict_contract:
(.verdict | IN("approve","advise","block-soft","block")) and
(.rationale | nonempty) and
(.rationale | nonempty) and
(if .verdict == "block-soft"
then any(.findings[]; .hard_gate_class == "soft_block")
elif .verdict == "block"
Expand All @@ -211,6 +232,18 @@ _gate_reviewer_protocol_document_verify() {
then "invalid top-level or binding contract"
elif (coverage_contract | not)
then "invalid coverage contract"
elif (blocking_severity_violation != null)
then (blocking_severity_violation as $invalid |
"invalid finding contract: " + ($invalid.id | display) +
" hard_gate_class=" + ($invalid.hard_gate_class | display) +
" requires severity=critical|high (got " +
($invalid.severity | display) + ")")
elif (blocking_origin_violation != null)
then (blocking_origin_violation as $invalid |
"invalid finding contract: " + ($invalid.id | display) +
" hard_gate_class=" + ($invalid.hard_gate_class | display) +
" requires origin=diff_caused|uncertain (got " +
($invalid.origin | display) + ")")
elif (finding_contract | not)
then "invalid finding contract"
elif $references != null and (evidence_reference_contract | not)
Expand Down
10 changes: 10 additions & 0 deletions tests/lib/test-pr-gate-fixture.sh
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,16 @@ pr_gate_fixture_write_reviewer_protocol() {
minimum_fix_boundary:"Use the complete reviewer role as the prefix.",
verification_expectation:"Validate the corrected finding ID."
}]
elif $mutation == "blocking-medium-severity" and
$reviewer == "architecture-reviewer"
then .findings[0].severity = "medium"
elif $mutation == "blocking-pre-existing-origin" and
$reviewer == "architecture-reviewer"
then .findings[0].origin = "pre_existing"
elif $mutation == "blocking-terminal-escape" and
$reviewer == "architecture-reviewer"
then .findings[0].id = "architecture-reviewer-F001\u001b[31m"
| .findings[0].severity = "medium"
else .
end
' | {
Expand Down
116 changes: 116 additions & 0 deletions tests/shell/test-pr-gate.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8696,6 +8696,10 @@ test_parallel_reviewer_protocol_preserves_session_topology() {
"reference_index.entries[]" || return
assert_file_contains "$name" "$reviewer_brief" \
"out-of-scope repository paths make the protocol INCOMPLETE" || return
assert_file_contains "$name" "$reviewer_brief" \
"soft_block/hard_block findings require severity=critical|high" || return
assert_file_contains "$name" "$reviewer_brief" \
"medium/low and pre_existing/caution findings" || return
pass "$name"
}

Expand Down Expand Up @@ -8879,6 +8883,115 @@ test_reviewer_protocol_abbreviated_finding_id_is_incomplete() {
pass "$name"
}

# Behavior: a blocking finding with medium severity is rejected with the exact
# field-level contract diagnostic that the reviewer brief now documents.
# Steps: emit the observed architecture-reviewer combination, run the parallel
# gate, then assert the finding ID, blocking class, and required severity.
test_reviewer_protocol_blocking_medium_severity_is_diagnosed() {
local name="reviewer-protocol/blocking-medium-severity-diagnostic"
should_run "$name" || return 0
local dir="$TMP_ROOT/$name" home="$TMP_ROOT/$name/home"
local repo="$TMP_ROOT/$name/repo" runner="$TMP_ROOT/$name/runner"
local out="$TMP_ROOT/$name/out" err="$TMP_ROOT/$name/err" code
mkdir -p "$dir"
create_runner "$runner"
create_agents "$home" critic qa-tester architecture-reviewer
create_repo "$repo" docs
set +e
CODEX_GATE_STUB_VERDICT=block-soft \
CODEX_GATE_STUB_PROTOCOL_MUTATION=blocking-medium-severity \
run_gate "$home" "$runner" "$repo" "$out" "$err" \
--base main --reviewers critic,qa-tester,architecture-reviewer --mode parallel
code=$?
set -e
[[ "$code" -ne 0 ]] || {
fail "$name" "medium-severity blocker unexpectedly passed"
return
}
if ! grep -Fq -- \
"architecture-reviewer-F001 hard_gate_class=soft_block requires severity=critical|high (got medium)" \
"$err"; then
fail "$name" "precise blocking-severity diagnostic missing: $(cat "$err" 2>/dev/null)"
return
fi
assert_not_contains "$name" "$out" "[synthesis]" || return
pass "$name"
}

# Behavior: a blocking finding with a pre-existing origin is rejected before
# synthesis with a precise origin-contract diagnostic.
# Steps: mutate the architecture-reviewer origin, run the parallel gate, then
# assert the offending value, permitted origins, and absence of synthesis.
test_reviewer_protocol_blocking_pre_existing_origin_is_diagnosed() {
local name="reviewer-protocol/blocking-pre-existing-origin-diagnostic"
should_run "$name" || return 0
local dir="$TMP_ROOT/$name" home="$TMP_ROOT/$name/home"
local repo="$TMP_ROOT/$name/repo" runner="$TMP_ROOT/$name/runner"
local out="$TMP_ROOT/$name/out" err="$TMP_ROOT/$name/err" code
mkdir -p "$dir"
create_runner "$runner"
create_agents "$home" critic qa-tester architecture-reviewer
create_repo "$repo" docs
set +e
CODEX_GATE_STUB_VERDICT=block-soft \
CODEX_GATE_STUB_PROTOCOL_MUTATION=blocking-pre-existing-origin \
run_gate "$home" "$runner" "$repo" "$out" "$err" \
--base main --reviewers critic,qa-tester,architecture-reviewer --mode parallel
code=$?
set -e
[[ "$code" -ne 0 ]] || {
fail "$name" "pre-existing-origin blocker unexpectedly passed"
return
}
if ! grep -Fq -- \
"architecture-reviewer-F001 hard_gate_class=soft_block requires origin=diff_caused|uncertain (got pre_existing)" \
"$err"; then
fail "$name" "precise blocking-origin diagnostic missing: $(cat "$err" 2>/dev/null)"
return
fi
assert_not_contains "$name" "$out" "[synthesis]" || return
pass "$name"
}

# Behavior: reviewer-controlled control characters are JSON-escaped before a
# field-level protocol diagnostic reaches the terminal.
# Steps: inject ESC into a rejected finding ID, run the parallel gate, then
# assert literal JSON escaping, no raw ESC byte, and no synthesis.
test_reviewer_protocol_diagnostic_terminal_escapes_control_characters() {
local name="reviewer-protocol/diagnostic-terminal-control-escape"
should_run "$name" || return 0
local dir="$TMP_ROOT/$name" home="$TMP_ROOT/$name/home"
local repo="$TMP_ROOT/$name/repo" runner="$TMP_ROOT/$name/runner"
local out="$TMP_ROOT/$name/out" err="$TMP_ROOT/$name/err" code
mkdir -p "$dir"
create_runner "$runner"
create_agents "$home" critic qa-tester architecture-reviewer
create_repo "$repo" docs
set +e
CODEX_GATE_STUB_VERDICT=block-soft \
CODEX_GATE_STUB_PROTOCOL_MUTATION=blocking-terminal-escape \
run_gate "$home" "$runner" "$repo" "$out" "$err" \
--base main --reviewers critic,qa-tester,architecture-reviewer --mode parallel
code=$?
set -e
[[ "$code" -ne 0 ]] || {
fail "$name" "control-character finding unexpectedly passed"
return
}
if ! grep -Fq -- \
'architecture-reviewer-F001\u001b[31m hard_gate_class=soft_block requires severity=critical|high (got medium)' \
"$err"; then
fail "$name" "JSON-escaped diagnostic missing: $(cat "$err" 2>/dev/null)"
return
fi
if grep -q $'\033' "$err"; then
fail "$name" "raw terminal ESC byte leaked into diagnostic"
return
fi
assert_not_contains "$name" "$out" "[synthesis]" || return
pass "$name"
}

# Behavior: a parallel reviewer cannot cite a syntactically valid repository
# path that is absent from the scope manifest reference index.
# Steps: mutate one coverage reference to an out-of-scope path, run the
Expand Down Expand Up @@ -9039,6 +9152,9 @@ run_test test_reviewer_protocol_evidence_less_blocker_is_incomplete
run_test test_reviewer_protocol_legacy_pass_reports_verdict_contract
run_test test_reviewer_protocol_extra_role_field_reports_top_level_contract
run_test test_reviewer_protocol_abbreviated_finding_id_is_incomplete
run_test test_reviewer_protocol_blocking_medium_severity_is_diagnosed
run_test test_reviewer_protocol_blocking_pre_existing_origin_is_diagnosed
run_test test_reviewer_protocol_diagnostic_terminal_escapes_control_characters
run_test test_parallel_reviewer_protocol_out_of_scope_reference_is_incomplete
run_test test_sequential_reviewer_protocol_out_of_range_line_is_incomplete
run_test test_reviewer_protocol_duplicate_heading_uses_json_verdict
Expand Down