From a21ec9f02f65ce064c8504c8ddbc2f9aab779f97 Mon Sep 17 00:00:00 2001 From: satyaborg Date: Mon, 17 Aug 2026 16:51:49 +1000 Subject: [PATCH 1/3] feat: bind review acceptance to a verified checkpoint sha An ACCEPT verdict now binds to the exact commit the reviewer saw. Any commit landing between the review brief and the verdict invalidates acceptance and ends the run as head-drift instead of accepted. Accepted runs with a PR then pass a final verification gate before the draft is lifted: local head equals the accepted checkpoint, the worktree holds no uncommitted task changes, the remote PR head matches, and required checks are green for that same sha. Only then does the PR become ready for human review. --- devloop | 153 +++++++++++++++++++++++++++++++++++++++- scripts/devloop_test.sh | 105 +++++++++++++++++++++++++++ 2 files changed, 256 insertions(+), 2 deletions(-) diff --git a/devloop b/devloop index 904caad..5a87af5 100755 --- a/devloop +++ b/devloop @@ -72,6 +72,12 @@ CODER_SESSION_ID="" REVIEWER_SESSION_ID="" PULL_REQUEST="" PULL_REQUEST_ERROR="" +REVIEWED_HEAD="" +ACCEPTED_HEAD="" +REMOTE_HEAD="" +CHECKS_STATE="" +READINESS="" +READINESS_DETAIL="" RUN_START_PASS=1 VERIFY_LOG="" VERIFY_DETAIL="" @@ -3060,6 +3066,10 @@ run_devloop() { FINAL_COMMIT_MESSAGE="" PULL_REQUEST="" PULL_REQUEST_ERROR="" + REVIEWED_HEAD="" + ACCEPTED_HEAD="" + READINESS="" + READINESS_DETAIL="" COMMIT_PASSES=() COMMIT_HASHES=() COMMIT_MESSAGES=() @@ -3281,8 +3291,9 @@ run_devloop() { local review=".devloop/reviews/$slug-r$pass.md" local reviewer_log=".devloop/logs/$slug-r$pass-reviewer.log" local reviewer_id="reviewer-$pass" + REVIEWED_HEAD="$(head_sha "$repo")" event_step "$reviewer_id" "pass $pass/$max: $(agent_label "$reviewer") reviewing" - if run_agent "$reviewer" "$repo" "$repo/$reviewer_session" "$repo/$reviewer_log" "$(review_prompt "$coder" "$run_spec" "$TRACK" "$base" "$pass" "$review" "$slug" "$max" "$obligations_file" "$strict")" "$reviewer_id"; then + if run_agent "$reviewer" "$repo" "$repo/$reviewer_session" "$repo/$reviewer_log" "$(review_prompt "$coder" "$run_spec" "$TRACK" "$base" "$pass" "$review" "$slug" "$max" "$obligations_file" "$strict" "$REVIEWED_HEAD")" "$reviewer_id"; then event_done "$reviewer_id" true "done" else if [ "$RUN_TIMED_OUT" = true ]; then STATUS="timeout"; else STATUS="reviewer-error"; fi @@ -3311,11 +3322,15 @@ run_devloop() { verdict="$(parse_verdict "$repo/$review")" if [ "$verdict" = "ACCEPT" ]; then event_gate "pass $pass review verdict" 1 "$verdict" - if [ "$strict" = true ] && ! has_passing_matrix "$repo/$review" "$obligations_file"; then + if head_drifted "$repo" "$REVIEWED_HEAD"; then + event_gate "pass $pass checkpoint" 0 "head moved off reviewed $(short_sha "$REVIEWED_HEAD")" + STATUS="head-drift" + elif [ "$strict" = true ] && ! has_passing_matrix "$repo/$review" "$obligations_file"; then STATUS="unclear" elif [ "$strict" = true ] && ! has_passing_quality_matrix "$repo/$review"; then STATUS="unclear" else + ACCEPTED_HEAD="$REVIEWED_HEAD" STATUS="accepted" fi break @@ -3355,6 +3370,23 @@ run_devloop() { fi fi + if [ "$create_pr" = true ] && [ -n "$PULL_REQUEST" ] && [ "$STATUS" = "accepted" ]; then + event_step "final-verification" "verifying accepted checkpoint" + if final_verification "$repo" "$PULL_REQUEST" "$ACCEPTED_HEAD" "$initial_dirty"; then + event_done "final-verification" true "$READINESS_DETAIL" + event_step "pr-ready" "marking pull request ready for review" + if mark_pull_request_ready "$repo" "$PULL_REQUEST"; then + event_done "pr-ready" true "ready for human review" + else + READINESS="blocked" + READINESS_DETAIL="$PULL_REQUEST_ERROR" + event_done "pr-ready" false "$PULL_REQUEST_ERROR" + fi + else + event_done "final-verification" false "$READINESS_DETAIL" + fi + fi + print_result rm -f "$criteria_file" "$obligations_file" "$initial_dirty" return 0 @@ -4483,6 +4515,7 @@ review_prompt() { local max="$8" local obligations_file="$9" local strict="${10}" + local checkpoint="${11:-}" local obligations priors review_skill strict_rule obligations="$(cat "$obligations_file")" priors="$(list_reviews "$slug" "$pass" "$max")" @@ -4499,6 +4532,8 @@ Spec: $spec Track: $track Base: $base Pass: $pass +Checkpoint: ${checkpoint:-unknown} +Review exactly this checkpoint. Do not commit, amend, or push. Any new commit invalidates the verdict. Prior reviews: $priors Spec obligations (acceptance criteria, invariants, and failure modes): @@ -4712,6 +4747,118 @@ parse_verdict() { sed -nE 's/^Verdict:[[:space:]]+(ACCEPT|REJECT|UNCLEAR).*/\1/p' "$file" | head -n 1 } +head_sha() { + local repo="$1" + git -C "$repo" rev-parse HEAD 2>/dev/null +} + +short_sha() { + local sha="$1" + printf '%s\n' "${sha:0:7}" +} + +head_drifted() { + local repo="$1" + local reviewed="$2" + local current + [ -n "$reviewed" ] || return 1 + current="$(head_sha "$repo")" || return 1 + [ "$current" != "$reviewed" ] +} + +worktree_clean() { + local repo="$1" + local initial_dirty="$2" + local pending + pending="$(committable_paths "$repo" "$initial_dirty")" + [ -z "$pending" ] +} + +remote_pull_request_head() { + local repo="$1" + local pr="$2" + local out + REMOTE_HEAD="" + if ! out="$(cd "$repo" >/dev/null 2>&1 && gh pr view "$pr" --json headRefOid --jq '.headRefOid // ""' 2>&1)"; then + PULL_REQUEST_ERROR="PR head lookup failed: $(gh_error_detail "$out")" + return 1 + fi + REMOTE_HEAD="$(printf '%s\n' "$out" | sed '/^[[:space:]]*$/d' | head -n 1)" +} + +pull_request_checks_state() { + local repo="$1" + local pr="$2" + local query out + CHECKS_STATE="" + query='[.statusCheckRollup[]?] as $all + | if ($all | length) == 0 then "none" + elif ($all | map(select((.conclusion // .state // "") | ascii_downcase | test("^(success|neutral|skipped)$"))) | length) == ($all | length) then "green" + elif ($all | map(select((.conclusion // .state // "") == "")) | length) > 0 then "pending" + else "failing" end' + if ! out="$(cd "$repo" >/dev/null 2>&1 && gh pr view "$pr" --json statusCheckRollup --jq "$query" 2>&1)"; then + PULL_REQUEST_ERROR="PR checks lookup failed: $(gh_error_detail "$out")" + return 1 + fi + CHECKS_STATE="$(printf '%s\n' "$out" | sed '/^[[:space:]]*$/d' | head -n 1)" +} + +mark_pull_request_ready() { + local repo="$1" + local pr="$2" + local out + if ! run_compact_command "$repo" "mark pull request ready" gh pr ready "$pr"; then + out="$RUN_OUTPUT" + PULL_REQUEST_ERROR="PR ready failed: $(gh_error_detail "$out")" + return 1 + fi +} + +final_verification() { + local repo="$1" + local pr="$2" + local accepted="$3" + local initial_dirty="$4" + local local_head + READINESS="blocked" + READINESS_DETAIL="" + if [ -z "$accepted" ]; then + READINESS_DETAIL="no accepted checkpoint recorded" + return 1 + fi + local_head="$(head_sha "$repo")" + if [ "$local_head" != "$accepted" ]; then + READINESS_DETAIL="local head $(short_sha "$local_head") moved off accepted checkpoint $(short_sha "$accepted")" + return 1 + fi + if ! worktree_clean "$repo" "$initial_dirty"; then + READINESS_DETAIL="worktree has uncommitted task changes" + return 1 + fi + if ! remote_pull_request_head "$repo" "$pr"; then + READINESS_DETAIL="$PULL_REQUEST_ERROR" + return 1 + fi + if [ "$REMOTE_HEAD" != "$accepted" ]; then + READINESS_DETAIL="remote PR head $(short_sha "$REMOTE_HEAD") differs from accepted checkpoint $(short_sha "$accepted")" + return 1 + fi + if ! pull_request_checks_state "$repo" "$pr"; then + READINESS_DETAIL="$PULL_REQUEST_ERROR" + return 1 + fi + case "$CHECKS_STATE" in + green|none) + READINESS="ready" + READINESS_DETAIL="accepted checkpoint $(short_sha "$accepted") verified" + ;; + *) + READINESS_DETAIL="required checks are $CHECKS_STATE for $(short_sha "$accepted")" + return 1 + ;; + esac +} + has_passing_matrix() { local file="$1" local obligations_file="$2" @@ -5295,6 +5442,8 @@ print_result() { result_line "Branch" "$FINAL_BRANCH" result_line "Commit" "${FINAL_COMMIT:-none}" if [ "$WORKTREE_REPO" != "$SOURCE_REPO" ]; then result_line "Worktree" "$WORKTREE_REPO"; fi + if [ -n "$ACCEPTED_HEAD" ]; then result_line "Accepted" "$(short_sha "$ACCEPTED_HEAD")"; fi + if [ -n "$READINESS" ]; then result_line "Readiness" "$READINESS${READINESS_DETAIL:+ ($READINESS_DETAIL)}"; fi printf '\nOpen Next\n' if [ -n "$PULL_REQUEST" ]; then result_line "PR" "$PULL_REQUEST"; fi result_line "Report" "$(display_path "$REPORT")" diff --git a/scripts/devloop_test.sh b/scripts/devloop_test.sh index 4410895..8a008a1 100755 --- a/scripts/devloop_test.sh +++ b/scripts/devloop_test.sh @@ -1080,6 +1080,111 @@ PULL_REQUEST_ERROR="" if create_pull_request "$branch_repo" "feat/chat-retry" "main" >/dev/null 2>&1; then fail "pull request creation unexpectedly passed without remote"; fi contains "$PULL_REQUEST_ERROR" "branch push failed" "pull request push failure" contains "$PULL_REQUEST_ERROR" "repository exists" "pull request push failure" + +checkpoint_repo="$work/checkpoint-repo" +git init -q "$checkpoint_repo" +git -C "$checkpoint_repo" config user.email devloop-test@example.com +git -C "$checkpoint_repo" config user.name "devloop test" +printf 'one\n' > "$checkpoint_repo/file.txt" +git -C "$checkpoint_repo" add file.txt +git -C "$checkpoint_repo" commit -q -m init +checkpoint_head="$(head_sha "$checkpoint_repo")" +equals "$checkpoint_head" "$(git -C "$checkpoint_repo" rev-parse HEAD)" "head_sha resolves full sha" +equals "$(short_sha "$checkpoint_head")" "${checkpoint_head:0:7}" "short_sha truncates" + +if head_drifted "$checkpoint_repo" "$checkpoint_head"; then fail "head_drifted reported drift on an unchanged head"; fi +if head_drifted "$checkpoint_repo" ""; then fail "head_drifted reported drift without a reviewed checkpoint"; fi +printf 'two\n' >> "$checkpoint_repo/file.txt" +git -C "$checkpoint_repo" commit -q -am "second" +head_drifted "$checkpoint_repo" "$checkpoint_head" || fail "head_drifted missed a new commit" +ok "checkpoint drift detection" + +checkpoint_dirty="$work/checkpoint-dirty.txt" +: > "$checkpoint_dirty" +worktree_clean "$checkpoint_repo" "$checkpoint_dirty" || fail "worktree_clean rejected a clean worktree" +printf 'scratch\n' > "$checkpoint_repo/extra.txt" +if worktree_clean "$checkpoint_repo" "$checkpoint_dirty"; then fail "worktree_clean accepted an uncommitted change"; fi +printf 'extra.txt\n' > "$checkpoint_dirty" +worktree_clean "$checkpoint_repo" "$checkpoint_dirty" || fail "worktree_clean did not preserve pre-existing changes" +rm -f "$checkpoint_repo/extra.txt" +: > "$checkpoint_dirty" +ok "worktree_clean preserves pre-existing changes" + +checkpoint_accepted="$(head_sha "$checkpoint_repo")" +gh() { + case "$*" in + *"--json headRefOid"*) printf '%s\n' "$GH_STUB_HEAD" ;; + *"--json statusCheckRollup"*) printf '%s\n' "$GH_STUB_CHECKS" ;; + *"pr ready"*) [ "$GH_STUB_READY" = "ok" ] || { printf 'gh: not a draft\n'; return 1; }; printf 'marked ready\n' ;; + *) return "$GH_STUB_CODE" ;; + esac + return "$GH_STUB_CODE" +} +GH_STUB_CODE=0 +GH_STUB_HEAD="$checkpoint_accepted" +GH_STUB_CHECKS="green" +GH_STUB_READY="ok" + +remote_pull_request_head "$checkpoint_repo" "https://pr/1" || fail "remote_pull_request_head failed" +equals "$REMOTE_HEAD" "$checkpoint_accepted" "remote_pull_request_head" +pull_request_checks_state "$checkpoint_repo" "https://pr/1" || fail "pull_request_checks_state failed" +equals "$CHECKS_STATE" "green" "pull_request_checks_state" +mark_pull_request_ready "$checkpoint_repo" "https://pr/1" || fail "mark_pull_request_ready failed on a draft PR" + +final_verification "$checkpoint_repo" "https://pr/1" "$checkpoint_accepted" "$checkpoint_dirty" || fail "final_verification blocked a verified checkpoint" +equals "$READINESS" "ready" "final_verification ready" +contains "$READINESS_DETAIL" "$(short_sha "$checkpoint_accepted")" "final_verification detail names the checkpoint" + +if final_verification "$checkpoint_repo" "https://pr/1" "" "$checkpoint_dirty"; then fail "final_verification accepted an empty checkpoint"; fi +contains "$READINESS_DETAIL" "no accepted checkpoint" "final_verification empty checkpoint detail" +equals "$READINESS" "blocked" "final_verification blocked state" + +if final_verification "$checkpoint_repo" "https://pr/1" "$checkpoint_head" "$checkpoint_dirty"; then fail "final_verification accepted a stale local head"; fi +contains "$READINESS_DETAIL" "moved off accepted checkpoint" "final_verification stale local head detail" + +GH_STUB_HEAD="0000000000000000000000000000000000000000" +if final_verification "$checkpoint_repo" "https://pr/1" "$checkpoint_accepted" "$checkpoint_dirty"; then fail "final_verification accepted a mismatched remote head"; fi +contains "$READINESS_DETAIL" "remote PR head" "final_verification remote head detail" +GH_STUB_HEAD="$checkpoint_accepted" + +GH_STUB_CHECKS="pending" +if final_verification "$checkpoint_repo" "https://pr/1" "$checkpoint_accepted" "$checkpoint_dirty"; then fail "final_verification accepted pending checks"; fi +contains "$READINESS_DETAIL" "required checks are pending" "final_verification pending checks detail" + +GH_STUB_CHECKS="none" +final_verification "$checkpoint_repo" "https://pr/1" "$checkpoint_accepted" "$checkpoint_dirty" || fail "final_verification blocked a PR with no checks" +equals "$READINESS" "ready" "final_verification treats absent checks as ready" +GH_STUB_CHECKS="green" + +printf 'dirty\n' > "$checkpoint_repo/uncommitted.txt" +if final_verification "$checkpoint_repo" "https://pr/1" "$checkpoint_accepted" "$checkpoint_dirty"; then fail "final_verification accepted a dirty worktree"; fi +contains "$READINESS_DETAIL" "uncommitted task changes" "final_verification dirty worktree detail" +rm -f "$checkpoint_repo/uncommitted.txt" + +GH_STUB_CODE=1 +if remote_pull_request_head "$checkpoint_repo" "https://pr/1"; then fail "remote_pull_request_head ignored a gh failure"; fi +contains "$PULL_REQUEST_ERROR" "PR head lookup failed" "remote_pull_request_head error" +equals "$REMOTE_HEAD" "" "remote_pull_request_head clears its result on failure" +if pull_request_checks_state "$checkpoint_repo" "https://pr/1"; then fail "pull_request_checks_state ignored a gh failure"; fi +contains "$PULL_REQUEST_ERROR" "PR checks lookup failed" "pull_request_checks_state error" +if final_verification "$checkpoint_repo" "https://pr/1" "$checkpoint_accepted" "$checkpoint_dirty"; then fail "final_verification accepted a gh lookup failure"; fi +contains "$READINESS_DETAIL" "PR head lookup failed" "final_verification propagates lookup failure" +GH_STUB_CODE=0 + +GH_STUB_READY="fail" +if mark_pull_request_ready "$checkpoint_repo" "https://pr/1"; then fail "mark_pull_request_ready ignored a gh failure"; fi +contains "$PULL_REQUEST_ERROR" "PR ready failed" "mark_pull_request_ready error" +GH_STUB_READY="ok" +unset -f gh +READINESS="" +READINESS_DETAIL="" +PULL_REQUEST_ERROR="" +ok "final verification gates" + +: > "$work/empty-obligations.txt" +contains "$(review_prompt codex spec.md track.md main 2 out.md slug 5 "$work/empty-obligations.txt" false abc1234)" "Checkpoint: abc1234" "review prompt checkpoint" +contains "$(review_prompt codex spec.md track.md main 2 out.md slug 5 "$work/empty-obligations.txt" false abc1234)" "Do not commit, amend, or push" "review prompt checkpoint rule" +contains "$(review_prompt codex spec.md track.md main 2 out.md slug 5 "$work/empty-obligations.txt" false)" "Checkpoint: unknown" "review prompt missing checkpoint" mkdir -p "$branch_repo/.devloop/reports" "$branch_repo/.devloop/tracks" "$branch_repo/.devloop/reviews" printf '%s\n' "# Report" > "$branch_repo/.devloop/reports/chat-retry.md" branch_repo_real="$(cd "$branch_repo" && pwd -P)" From fbce89f5baadbd82f4be34f0d4afdb322ae0b2ba Mon Sep 17 00:00:00 2001 From: satyaborg Date: Mon, 17 Aug 2026 17:27:58 +1000 Subject: [PATCH 2/3] test: cover the readiness gate in the end-to-end PR loop The fake gh now answers headRefOid, statusCheckRollup, and pr ready, so the accept loop exercises final verification instead of stopping at the draft PR. Asserts an accepted run is marked ready and an unclear run is not. --- scripts/devloop_test.sh | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/scripts/devloop_test.sh b/scripts/devloop_test.sh index 8a008a1..6113dd8 100755 --- a/scripts/devloop_test.sh +++ b/scripts/devloop_test.sh @@ -2807,12 +2807,34 @@ case "${1:-}" in fi printf '%s\n' "commented" ;; + ready) + if [ "${DEVLOOP_GH_READY_FAIL:-0}" = "1" ]; then + printf '%s\n' "gh pr ready exploded" >&2 + exit 1 + fi + printf '%s\n' "ready" > "$state/pr_ready" + printf '%s\n' "marked ready" + ;; view) if [ "${DEVLOOP_GH_VIEW_FAIL:-0}" = "1" ]; then printf '%s\n' "gh pr view exploded" >&2 exit 1 fi - if [ -f "$state/latest_round_comment" ]; then cat "$state/latest_round_comment"; fi + case "$*" in + *headRefOid*) + if [ "${DEVLOOP_GH_REMOTE_HEAD:-}" = "drift" ]; then + printf '%s\n' "0000000000000000000000000000000000000000" + else + git rev-parse HEAD 2>/dev/null + fi + ;; + *statusCheckRollup*) + printf '%s\n' "${DEVLOOP_GH_CHECKS:-none}" + ;; + *) + if [ -f "$state/latest_round_comment" ]; then cat "$state/latest_round_comment"; fi + ;; + esac ;; *) exit 1 @@ -3486,6 +3508,7 @@ contains "$final_body" "Commit References" "final PR comment" if printf '%s\n' "$final_body" | grep -q '/Users/'; then fail "final PR comment leaked absolute local path"; fi if printf '%s\n' "$round_body" | grep -q 'Local cache'; then fail "round PR comment leaked local cache path"; fi if printf '%s\n' "$final_body" | grep -Eq '<(html|script|style)'; then fail "final PR comment embedded standalone HTML"; fi +equals "$(cat "$pr_state/pr_ready" 2>/dev/null || printf 'draft')" "ready" "accepted PR is marked ready" unset DEVLOOP_GH_STATE DEVLOOP_GH_LOG DEVLOOP_AGENT_LOG ok "PR-backed accept comments" @@ -3508,6 +3531,7 @@ contains "$pr_terminal_output" "unclear" "PR terminal failure" equals "$(find "$pr_state/comments" -name 'round-*.md' | wc -l | tr -d ' ')" "1" "terminal round PR comment" equals "$(find "$pr_state/comments" -name 'final-*.md' | wc -l | tr -d ' ')" "1" "terminal final PR comment" contains "$(cat "$pr_state/comments/final-1.md")" "| Final status | unclear |" "terminal final PR comment" +if [ -f "$pr_state/pr_ready" ]; then fail "unclear run marked the PR ready"; fi unset DEVLOOP_GH_STATE DEVLOOP_GH_LOG DEVLOOP_AGENT_LOG ok "PR-backed terminal final comment" From 6ed383a1a95d1ae819ecf7eeb1226362e8148372 Mon Sep 17 00:00:00 2001 From: satyaborg Date: Mon, 17 Aug 2026 17:40:15 +1000 Subject: [PATCH 3/3] fix: drop the jq variable binding from the checks query shellcheck flagged SC2016 on the single-quoted $all binding. The same classification works by piping the array directly, so the query needs no variable and no suppression comment. --- devloop | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/devloop b/devloop index 5a87af5..843a100 100755 --- a/devloop +++ b/devloop @@ -4791,10 +4791,10 @@ pull_request_checks_state() { local pr="$2" local query out CHECKS_STATE="" - query='[.statusCheckRollup[]?] as $all - | if ($all | length) == 0 then "none" - elif ($all | map(select((.conclusion // .state // "") | ascii_downcase | test("^(success|neutral|skipped)$"))) | length) == ($all | length) then "green" - elif ($all | map(select((.conclusion // .state // "") == "")) | length) > 0 then "pending" + query='[.statusCheckRollup[]?] + | if length == 0 then "none" + elif ([.[] | select((.conclusion // .state // "") | ascii_downcase | test("^(success|neutral|skipped)$"))] | length) == length then "green" + elif ([.[] | select((.conclusion // .state // "") == "")] | length) > 0 then "pending" else "failing" end' if ! out="$(cd "$repo" >/dev/null 2>&1 && gh pr view "$pr" --json statusCheckRollup --jq "$query" 2>&1)"; then PULL_REQUEST_ERROR="PR checks lookup failed: $(gh_error_detail "$out")"