From 999bfe48a1dadee77e5681b8248f516f50ffc7b6 Mon Sep 17 00:00:00 2001 From: satyaborg Date: Mon, 17 Aug 2026 16:56:07 +1000 Subject: [PATCH 1/2] feat: replace per-round PR comments with one living review status Every pass previously appended a full review dump, and the run appended a final report on top, so a four-pass run buried the reader under five walls of text. The PR now carries exactly one status comment, found by a marker and edited in place. The comment leads with the current state, a two-row quality gate table, and open findings. Review history, gate matrices, the full review, and run details move into collapsed sections, so a human sees the verdict first and opens only what they need. --- README.md | 2 +- devloop | 327 ++++++++++++++++++++++++++-------------- scripts/devloop_test.sh | 129 +++++++++++++++- 3 files changed, 343 insertions(+), 115 deletions(-) diff --git a/README.md b/README.md index ec15425..1204bf7 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,7 @@ Uninstall with `./scripts/uninstall.sh` (`--dry-run` to preview). | `devloop nightshift` | Survey configured repos, run selected specs headlessly, and write a morning digest | | `devloop ` | Run a spec | | `devloop --no-tmux ` | Run a spec in the current foreground terminal | -| `devloop --create-pr ` | Run a spec and maintain a draft PR (requires `gh`) | +| `devloop --create-pr ` | Run a spec and maintain a draft PR, then mark it ready once the accepted checkpoint verifies (requires `gh`) | | `devloop update` | Install the latest released Devloop | | `devloop continue` | Resume a tracked run | | `devloop status` | Show run status | diff --git a/devloop b/devloop index 5a87af5..ff0e150 100755 --- a/devloop +++ b/devloop @@ -76,6 +76,7 @@ REVIEWED_HEAD="" ACCEPTED_HEAD="" REMOTE_HEAD="" CHECKS_STATE="" +STATUS_COMMENT_ID="" READINESS="" READINESS_DETAIL="" RUN_START_PASS=1 @@ -2791,7 +2792,7 @@ EOF is_devloop_runtime_artifact_path() { case "$1" in - .devloop/specs/*|.devloop/tracks/*|.devloop/reviews/*|.devloop/reports/*|.devloop/logs/*|.devloop/sessions/*) return 0 ;; + .devloop/specs/*|.devloop/tracks/*|.devloop/reviews/*|.devloop/reports/*|.devloop/logs/*|.devloop/sessions/*|.devloop/status/*) return 0 ;; esac return 1 } @@ -3174,6 +3175,7 @@ run_devloop() { fi local coder_session=".devloop/sessions/$slug-coder-$coder.id" local reviewer_session=".devloop/sessions/$slug-reviewer-$reviewer.id" + local status_history=".devloop/status/$slug-history.tsv" init_track "$repo/$TRACK" "$run_spec" "$spec" "$PWD" "$SOURCE_REPO" "$repo" "$base" "$source_branch" "$run_branch" "$max" "$report_format" "$strict" "$coder" "$reviewer" "$WORK_TYPE" "$WORK_BREAKING" "$create_pr" "$timeout_minutes" @@ -3306,20 +3308,24 @@ run_devloop() { break fi + local verdict + verdict="$(parse_verdict "$repo/$review")" + append_status_history "$repo/$status_history" "$pass" \ + "$(matrix_result "$repo/$review" "Acceptance matrix")" \ + "$(matrix_result "$repo/$review" "Engineering quality matrix")" \ + "$REVIEWED_HEAD" "${verdict:-no verdict}" + if [ "$create_pr" = true ] && [ -n "$PULL_REQUEST" ]; then local pr_comment_id="pr-review-comment-$pass" - event_step "$pr_comment_id" "posting review to PR" - if post_pr_round_review "$repo" "$PULL_REQUEST" "$repo/$review" "$pass"; then - event_done "$pr_comment_id" true "comment posted" + event_step "$pr_comment_id" "updating PR review status" + if post_status_comment "$repo" "$PULL_REQUEST" "$repo/$review" "$repo/$status_history" "running" "$REVIEWED_HEAD" "$FINAL_BRANCH"; then + event_done "$pr_comment_id" true "status updated" else STATUS="pr-error" event_done "$pr_comment_id" false "$PULL_REQUEST_ERROR" break fi fi - - local verdict - verdict="$(parse_verdict "$repo/$review")" if [ "$verdict" = "ACCEPT" ]; then event_gate "pass $pass review verdict" 1 "$verdict" if head_drifted "$repo" "$REVIEWED_HEAD"; then @@ -3360,16 +3366,6 @@ run_devloop() { REVIEWER_SESSION_ID="$(read_first_line "$repo/$reviewer_session")" synthesize_report "$repo" "$slug" "$reviewer" "$run_spec" "$spec" "$spec_text" "$SOURCE_REPO" "$repo" "$TRACK" "$REPORT" "$STATUS" "$PASSES" "$max" "$base" "$source_branch" "$FINAL_BRANCH" "$FINAL_COMMIT" "$FINAL_COMMIT_MESSAGE" "$PULL_REQUEST" "$PULL_REQUEST_ERROR" "$coder" "$repo/$reviewer_session" "$CODER_SESSION_ID" "$REVIEWER_SESSION_ID" "$report_format" - if [ "$create_pr" = true ] && [ -n "$PULL_REQUEST" ] && [ "$STATUS" != "pr-error" ]; then - event_step "pr-final-report" "posting final report to PR" - if post_pr_final_report "$repo" "$PULL_REQUEST" "$slug" "$STATUS" "$PASSES" "$max" "$FINAL_BRANCH"; then - event_done "pr-final-report" true "comment posted" - else - STATUS="pr-error" - event_done "pr-final-report" false "$PULL_REQUEST_ERROR" - 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 @@ -3387,6 +3383,16 @@ run_devloop() { fi fi + if [ "$create_pr" = true ] && [ -n "$PULL_REQUEST" ] && [ "$STATUS" != "pr-error" ]; then + event_step "pr-final-status" "publishing final review status" + if post_status_comment "$repo" "$PULL_REQUEST" "$repo/.devloop/reviews/$slug-r$PASSES.md" "$repo/$status_history" "$STATUS" "${ACCEPTED_HEAD:-$REVIEWED_HEAD}" "$FINAL_BRANCH"; then + event_done "pr-final-status" true "status published" + else + STATUS="pr-error" + event_done "pr-final-status" false "$PULL_REQUEST_ERROR" + fi + fi + print_result rm -f "$criteria_file" "$obligations_file" "$initial_dirty" return 0 @@ -5102,141 +5108,236 @@ ensure_pull_request() { create_draft_pull_request "$repo" "$branch" "$base" "$spec" "$criteria_file" "$commit" "$source_repo" "$title" } -round_review_comment_body() { - local review_file="$1" - local pass="$2" - local verdict - verdict="$(parse_verdict "$review_file")" - if [ -z "$verdict" ]; then verdict="UNCLEAR"; fi - cat < "$body_file" - if ! out="$(cd "$repo" >/dev/null 2>&1 && gh pr comment "$pr" --body-file "$body_file" 2>&1)"; then - PULL_REQUEST_ERROR="PR comment failed: $(gh_error_detail "$out")" - rm -f "$body_file" - return 1 - fi - rm -f "$body_file" +matrix_evidence() { + local file="$1" + local heading="$2" + local total passed + [ -f "$file" ] || { printf '%s\n' "no review recorded"; return 0; } + total="$(review_section "$file" "$heading" | grep -cE '^\|[[:space:]]*(AC|F|I)[0-9]+[[:space:]]*\||^\|[[:space:]]*(Correctness|Test quality|Maintainability|Architecture boundaries|Simplicity|Security|Operational safety)[[:space:]]*\|' || true)" + if [ "${total:-0}" -eq 0 ]; then printf '%s\n' "no rows reported"; return 0; fi + passed="$(review_section "$file" "$heading" | grep -ciE '\|[[:space:]]*(PASS|N/A)[[:space:]]*\|' || true)" + printf '%s of %s rows passed\n' "${passed:-0}" "$total" } -latest_pr_review_comment() { - local repo="$1" - local pr="$2" - local out - local query='[.comments[].body | select(contains("# Devloop Review Round "))] | last // ""' - if ! out="$(cd "$repo" >/dev/null 2>&1 && gh pr view "$pr" --json comments --jq "$query" 2>&1)"; then - PULL_REQUEST_ERROR="PR review lookup failed: $(gh_error_detail "$out")" - return 1 - fi - printf '%s\n' "$out" +append_status_history() { + local history_file="$1" + local pass="$2" + local specification="$3" + local engineering="$4" + local checkpoint="$5" + local outcome="$6" + mkdir -p "$(dirname "$history_file")" 2>/dev/null || true + printf '%s\t%s\t%s\t%s\t%s\n' "$pass" "$specification" "$engineering" "$(short_sha "$checkpoint")" "$outcome" >> "$history_file" } -review_section() { - local file="$1" - local heading="$2" - [ -f "$file" ] || return 1 - awk -v heading="$heading" ' - BEGIN { inside = 0 } - $0 == "## " heading { inside = 1; print; next } - inside && /^##[[:space:]]+/ { exit } - inside { print } - ' "$file" +render_status_history() { + local history_file="$1" + [ -f "$history_file" ] || return 0 + awk -F'\t' '{ printf "| %s | %s | %s | `%s` | %s |\n", $1, $2, $3, $4, $5 }' "$history_file" } -final_pr_report_body() { - local repo="$1" - local slug="$2" +status_history_count() { + local history_file="$1" + if [ -f "$history_file" ]; then wc -l < "$history_file" | tr -d ' '; else printf '%s\n' "0"; fi +} + +status_comment_body() { + local review_file="$1" + local history_file="$2" local status="$3" - local pass="$4" - local max="$5" - local pr="$6" + local readiness="$4" + local readiness_detail="$5" + local checkpoint="$6" local branch="$7" - local latest_review="$repo/.devloop/reviews/$slug-r$pass.md" - local final_verdict acceptance engineering risk - final_verdict="" - if [ -f "$latest_review" ]; then final_verdict="$(parse_verdict "$latest_review")"; fi - if [ -z "$final_verdict" ]; then final_verdict="none"; fi - acceptance="$(review_section "$latest_review" "Acceptance matrix" || true)" - engineering="$(review_section "$latest_review" "Engineering quality matrix" || true)" - if [ -z "$acceptance" ]; then acceptance="- Not available."; fi - if [ -z "$engineering" ]; then engineering="- Not available."; fi - case "$status" in - accepted) risk="No blocking residual risk was recorded by the final review." ;; - *) risk="The run ended as $status. Inspect the latest findings and missing tests before merging." ;; + local verdict label specification engineering findings passes summary + verdict="$(if [ -f "$review_file" ]; then parse_verdict "$review_file"; fi)" + label="$(review_state_label "$status" "$readiness" "$verdict")" + specification="$(matrix_result "$review_file" "Acceptance matrix")" + engineering="$(matrix_result "$review_file" "Engineering quality matrix")" + passes="$(status_history_count "$history_file")" + findings="" + if [ "$verdict" != "ACCEPT" ] && [ -f "$review_file" ]; then + findings="$(review_section "$review_file" "Findings" | sed '1d' | sed '/^[[:space:]]*$/d')" + fi + case "$label" in + "Ready for human review") summary="The accepted checkpoint matches the pull-request head and required checks passed." ;; + "Final verification") summary="The checkpoint was accepted. ${readiness_detail:-Final checks are running.}" ;; + "Changes required") summary="The current checkpoint has open findings." ;; + "Review unavailable") summary="The review did not produce a usable verdict. Run status: $status." ;; + *) summary="A checkpoint was pushed and is awaiting a completed review. Run status: $status." ;; esac cat < +Review history: $passes $(if [ "$passes" = "1" ]; then printf 'pass'; else printf 'passes'; fi) -## Acceptance Matrix Summary +| Pass | Specification | Engineering | Checkpoint | Outcome | +| ---: | --- | --- | --- | --- | +$(render_status_history "$history_file") -$acceptance + -## Engineering Quality Summary +
+Gate details -$engineering +$(review_section "$review_file" "Acceptance matrix" 2>/dev/null || printf '%s\n' "No acceptance matrix recorded.") -## Implementation Summary +$(review_section "$review_file" "Engineering quality matrix" 2>/dev/null || printf '%s\n' "No engineering quality matrix recorded.") -- Final branch: \`$branch\` -- Final commit: \`${FINAL_COMMIT:-none}\` -- Commit message: ${FINAL_COMMIT_MESSAGE:-none} +
-## Commit References +
+Latest review detail -$(commit_lines) +$(if [ -f "$review_file" ]; then cat "$review_file"; else printf '%s\n' "No review recorded."; fi) + +
+ +
+Run details -## Tests Run +- Branch: \`$branch\` +- Checkpoint: \`${checkpoint:-none}\` +- Verification: ${VERIFY_LOG:-not configured} +- Readiness: ${readiness:-not evaluated}${readiness_detail:+ ($readiness_detail)} -- Verification hook log: ${VERIFY_LOG:-not configured} -- Review test evidence: see the acceptance matrix summary above. +
-## Residual Risk +--- -- $risk +Generated by [devloop.sh](https://devloop.sh) EOF } -post_pr_final_report() { +find_status_comment() { local repo="$1" local pr="$2" - local slug="$3" - local status="$4" - local pass="$5" - local max="$6" - local branch="$7" - local body_file out - body_file="$(mktemp "${TMPDIR:-/tmp}/devloop-pr-final.XXXXXX")" - final_pr_report_body "$repo" "$slug" "$status" "$pass" "$max" "$pr" "$branch" > "$body_file" + local out query + STATUS_COMMENT_ID="" + query="[.comments[] | select(.body | contains(\"$STATUS_COMMENT_MARKER\"))] | last | .url // \"\"" + if ! out="$(cd "$repo" >/dev/null 2>&1 && gh pr view "$pr" --json comments --jq "$query" 2>&1)"; then + PULL_REQUEST_ERROR="PR status comment lookup failed: $(gh_error_detail "$out")" + return 1 + fi + out="$(printf '%s\n' "$out" | sed '/^[[:space:]]*$/d' | head -n 1)" + case "$out" in + *\#issuecomment-*) STATUS_COMMENT_ID="${out##*\#issuecomment-}" ;; + *) STATUS_COMMENT_ID="" ;; + esac +} + +upsert_status_comment() { + local repo="$1" + local pr="$2" + local body_file="$3" + local out slug + if ! find_status_comment "$repo" "$pr"; then return 1; fi + if [ -n "$STATUS_COMMENT_ID" ]; then + slug="$(cd "$repo" >/dev/null 2>&1 && gh repo view --json nameWithOwner -q .nameWithOwner 2>/dev/null | head -n 1)" + if [ -n "$slug" ]; then + if ! out="$(cd "$repo" >/dev/null 2>&1 && gh api --method PATCH "repos/$slug/issues/comments/$STATUS_COMMENT_ID" -f body="$(cat "$body_file")" 2>&1)"; then + PULL_REQUEST_ERROR="PR status comment update failed: $(gh_error_detail "$out")" + return 1 + fi + return 0 + fi + fi if ! out="$(cd "$repo" >/dev/null 2>&1 && gh pr comment "$pr" --body-file "$body_file" 2>&1)"; then - PULL_REQUEST_ERROR="PR final report comment failed: $(gh_error_detail "$out")" + PULL_REQUEST_ERROR="PR status comment failed: $(gh_error_detail "$out")" + return 1 + fi +} + +post_status_comment() { + local repo="$1" + local pr="$2" + local review_file="$3" + local history_file="$4" + local status="$5" + local checkpoint="$6" + local branch="$7" + local body_file + body_file="$(mktemp "${TMPDIR:-/tmp}/devloop-pr-status.XXXXXX")" + status_comment_body "$review_file" "$history_file" "$status" "$READINESS" "$READINESS_DETAIL" "$checkpoint" "$branch" > "$body_file" + if ! upsert_status_comment "$repo" "$pr" "$body_file"; then rm -f "$body_file" return 1 fi rm -f "$body_file" } +latest_pr_review_comment() { + local repo="$1" + local pr="$2" + local out query + query="[.comments[].body | select(contains(\"$STATUS_COMMENT_MARKER\"))] | last // \"\"" + if ! out="$(cd "$repo" >/dev/null 2>&1 && gh pr view "$pr" --json comments --jq "$query" 2>&1)"; then + PULL_REQUEST_ERROR="PR review lookup failed: $(gh_error_detail "$out")" + return 1 + fi + printf '%s\n' "$out" +} + +review_section() { + local file="$1" + local heading="$2" + [ -f "$file" ] || return 1 + awk -v heading="$heading" ' + BEGIN { inside = 0 } + $0 == "## " heading { inside = 1; print; next } + inside && /^##[[:space:]]+/ { exit } + inside { print } + ' "$file" +} + synthesize_report() { local repo="$1" local slug="$2" diff --git a/scripts/devloop_test.sh b/scripts/devloop_test.sh index 8a008a1..29047b9 100755 --- a/scripts/devloop_test.sh +++ b/scripts/devloop_test.sh @@ -173,7 +173,7 @@ contains "$review_skill_text" "Do not flag work required solely by an invariant ok "spec Mermaid diagram guidance" contains "$(cat "$REPO_ROOT/README.md")" "\`devloop --create-pr \`" "README PR mode" -contains "$(cat "$REPO_ROOT/README.md")" "maintain a draft PR (requires \`gh\`)" "README PR mode" +contains "$(cat "$REPO_ROOT/README.md")" "maintain a draft PR, then mark it ready once the accepted checkpoint verifies (requires \`gh\`)" "README PR mode" ok "README PR guidance" for skill in "$REPO_ROOT"/skills/*/SKILL.md; do @@ -1181,6 +1181,133 @@ READINESS_DETAIL="" PULL_REQUEST_ERROR="" ok "final verification gates" +status_dir="$work/status-comment" +mkdir -p "$status_dir" +status_review="$status_dir/review.md" +cat > "$status_review" <<'MARKDOWN' +# Review 2 + +Verdict: REJECT + +## Acceptance matrix + +| Obligation | Status | Implementation evidence | Test evidence | +| --- | --- | --- | --- | +| AC1 | PASS | handler wired | unit test | +| AC2 | FAIL | not implemented | none | + +## Engineering quality matrix + +| Area | Status | Evidence | +| --- | --- | --- | +| Correctness | PASS | traced | +| Test quality | PASS | covered | +| Maintainability | PASS | small diff | +| Architecture boundaries | N/A | no boundary change | +| Simplicity | PASS | direct | +| Security | PASS | no new surface | +| Operational safety | PASS | recoverable | + +## Findings + +1. [P0] lib/send.ts:14 - retry never fires. Root cause: guard inverted. Principle: fail closed. + +## Notes + +- None +MARKDOWN + +equals "$(matrix_result "$status_review" "Acceptance matrix")" "Changes required" "matrix_result detects a failing obligation" +equals "$(matrix_result "$status_review" "Engineering quality matrix")" "Passed" "matrix_result passes a clean quality matrix" +equals "$(matrix_result "$status_dir/absent.md" "Acceptance matrix")" "Unavailable" "matrix_result handles a missing review" +equals "$(matrix_evidence "$status_review" "Acceptance matrix")" "1 of 2 rows passed" "matrix_evidence counts obligations" +equals "$(matrix_evidence "$status_review" "Engineering quality matrix")" "7 of 7 rows passed" "matrix_evidence counts quality rows" +equals "$(matrix_evidence "$status_dir/absent.md" "Acceptance matrix")" "no review recorded" "matrix_evidence handles a missing review" + +equals "$(review_state_label accepted ready ACCEPT)" "Ready for human review" "review_state_label ready" +equals "$(review_state_label accepted blocked ACCEPT)" "Final verification" "review_state_label pending verification" +equals "$(review_state_label max-turns "" REJECT)" "Changes required" "review_state_label rejected" +equals "$(review_state_label head-drift "" ACCEPT)" "Review pending" "review_state_label head drift" +equals "$(review_state_label review-missing "" "")" "Review unavailable" "review_state_label unavailable" +equals "$(review_state_label running "" "")" "Review pending" "review_state_label running" + +status_history="$status_dir/history.tsv" +append_status_history "$status_history" 1 "Changes required" "Passed" "1111111111111111111111111111111111111111" "REJECT" +append_status_history "$status_history" 2 "Passed" "Passed" "2222222222222222222222222222222222222222" "ACCEPT" +equals "$(status_history_count "$status_history")" "2" "status_history_count" +equals "$(status_history_count "$status_dir/absent.tsv")" "0" "status_history_count with no history" +contains "$(render_status_history "$status_history")" "| 1 | Changes required | Passed | \`1111111\` | REJECT |" "render_status_history first row" +contains "$(render_status_history "$status_history")" "| 2 | Passed | Passed | \`2222222\` | ACCEPT |" "render_status_history second row" +equals "$(render_status_history "$status_dir/absent.tsv")" "" "render_status_history with no history" + +status_body="$(status_comment_body "$status_review" "$status_history" running "" "" "2222222222222222222222222222222222222222" "feat/chat-retry")" +contains "$status_body" "" "status comment marker" +contains "$status_body" "**Review pending**" "status comment state" +contains "$status_body" "| Specification | Changes required | 1 of 2 rows passed |" "status comment specification gate" +contains "$status_body" "| Engineering | Passed | 7 of 7 rows passed |" "status comment engineering gate" +contains "$status_body" "### Open findings" "status comment findings" +contains "$status_body" "retry never fires" "status comment finding detail" +contains "$status_body" "Review history: 2 passes" "status comment history summary" +contains "$status_body" "Gate details" "status comment gate details" +contains "$status_body" "Run details" "status comment run details" +contains "$status_body" "feat/chat-retry" "status comment branch" + +status_accept_review="$status_dir/accepted.md" +sed 's/^Verdict: REJECT/Verdict: ACCEPT/; s/| AC2 | FAIL |/| AC2 | PASS |/' "$status_review" > "$status_accept_review" +status_ready_body="$(status_comment_body "$status_accept_review" "$status_history" accepted ready "checkpoint verified" "2222222222222222222222222222222222222222" "feat/chat-retry")" +contains "$status_ready_body" "**Ready for human review**" "accepted status comment state" +not_contains "$status_ready_body" "### Open findings" "accepted status comment omits findings" +contains "$status_ready_body" "| Specification | Passed | 2 of 2 rows passed |" "accepted status comment specification gate" + +status_empty_body="$(status_comment_body "$status_dir/absent.md" "$status_dir/absent.tsv" running "" "" "" "feat/chat-retry")" +contains "$status_empty_body" "Review history: 0 passes" "empty status comment history" +contains "$status_empty_body" "No review recorded." "empty status comment review detail" + +status_comment_url="https://github.com/o/r/pull/5#issuecomment-98765" +gh() { + case "$*" in + *"--json comments"*) printf '%s\n' "$GH_STUB_COMMENT" ;; + *"nameWithOwner"*) printf '%s\n' "o/r" ;; + *"--method PATCH"*) printf '%s\n' "patched" >> "$status_dir/calls.log" ;; + *"pr comment"*) printf '%s\n' "created" >> "$status_dir/calls.log" ;; + esac + return "$GH_STUB_CODE" +} +GH_STUB_CODE=0 +GH_STUB_COMMENT="$status_comment_url" +find_status_comment "$status_dir" "https://pr/5" || fail "find_status_comment failed" +equals "$STATUS_COMMENT_ID" "98765" "find_status_comment extracts the comment id" +GH_STUB_COMMENT="" +find_status_comment "$status_dir" "https://pr/5" || fail "find_status_comment failed on an absent comment" +equals "$STATUS_COMMENT_ID" "" "find_status_comment with no existing comment" + +: > "$status_dir/calls.log" +printf 'body\n' > "$status_dir/body.md" +upsert_status_comment "$status_dir" "https://pr/5" "$status_dir/body.md" || fail "upsert_status_comment failed to create" +contains "$(cat "$status_dir/calls.log")" "created" "upsert_status_comment creates when absent" +: > "$status_dir/calls.log" +GH_STUB_COMMENT="$status_comment_url" +upsert_status_comment "$status_dir" "https://pr/5" "$status_dir/body.md" || fail "upsert_status_comment failed to update" +contains "$(cat "$status_dir/calls.log")" "patched" "upsert_status_comment edits the existing comment" + +post_status_comment "$status_dir" "https://pr/5" "$status_review" "$status_history" running "$checkpoint_accepted" "feat/chat-retry" || fail "post_status_comment failed" +contains "$(latest_pr_review_comment "$status_dir" "https://pr/5")" "issuecomment" "latest_pr_review_comment reads the living comment" + +GH_STUB_CODE=1 +if find_status_comment "$status_dir" "https://pr/5"; then fail "find_status_comment ignored a gh failure"; fi +contains "$PULL_REQUEST_ERROR" "PR status comment lookup failed" "find_status_comment error" +if upsert_status_comment "$status_dir" "https://pr/5" "$status_dir/body.md"; then fail "upsert_status_comment ignored a lookup failure"; fi +if post_status_comment "$status_dir" "https://pr/5" "$status_review" "$status_history" running "" "feat/chat-retry"; then fail "post_status_comment ignored a gh failure"; fi +if latest_pr_review_comment "$status_dir" "https://pr/5" >/dev/null 2>&1; then fail "latest_pr_review_comment ignored a gh failure"; fi +contains "$PULL_REQUEST_ERROR" "PR review lookup failed" "latest_pr_review_comment error" +GH_STUB_CODE=0 +unset -f gh +PULL_REQUEST_ERROR="" +STATUS_COMMENT_ID="" +ok "living status comment" + +is_devloop_runtime_artifact_path ".devloop/status/slug-history.tsv" || fail "status history is not treated as a runtime artifact" + : > "$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" From 5baba9b0e104b09791d1430d423eac682649eb74 Mon Sep 17 00:00:00 2001 From: satyaborg Date: Mon, 17 Aug 2026 17:31:27 +1000 Subject: [PATCH 2/2] test: assert the living status comment end to end The fake gh now stores the marked comment and answers gh api PATCH, so the loop tests prove a two-pass run creates exactly one comment, edits it on the later pass, and keeps one history row per pass. --- scripts/devloop_test.sh | 115 ++++++++++++++++++++++++---------------- 1 file changed, 70 insertions(+), 45 deletions(-) diff --git a/scripts/devloop_test.sh b/scripts/devloop_test.sh index 7468cef..6ebd7d6 100755 --- a/scripts/devloop_test.sh +++ b/scripts/devloop_test.sh @@ -2924,13 +2924,11 @@ case "${1:-}" in count="$(find "$state/comments" -type f | wc -l | tr -d ' ')" body="$state/comments/comment-$((count + 1)).md" cp "$body_file" "$body" - if grep -q '^# Devloop Review Round ' "$body"; then - round_count="$(find "$state/comments" -name 'round-*.md' | wc -l | tr -d ' ')" - cp "$body" "$state/comments/round-$((round_count + 1)).md" - cp "$body" "$state/latest_round_comment" - elif grep -q '^# Devloop Final Report' "$body"; then - final_count="$(find "$state/comments" -name 'final-*.md' | wc -l | tr -d ' ')" - cp "$body" "$state/comments/final-$((final_count + 1)).md" + if grep -q '' "$body"; then + status_count="$(find "$state/comments" -name 'status-*.md' | wc -l | tr -d ' ')" + cp "$body" "$state/comments/status-$((status_count + 1)).md" + cp "$body" "$state/status_comment" + printf '%s\n' "https://github.com/satyaborg/devloop/pull/123#issuecomment-4242" > "$state/status_comment_url" fi printf '%s\n' "commented" ;; @@ -2958,8 +2956,18 @@ case "${1:-}" in *statusCheckRollup*) printf '%s\n' "${DEVLOOP_GH_CHECKS:-none}" ;; + *comments*) + case "$*" in + *".url"*) + if [ -f "$state/status_comment_url" ]; then cat "$state/status_comment_url"; fi + ;; + *) + if [ -f "$state/status_comment" ]; then cat "$state/status_comment"; fi + ;; + esac + ;; *) - if [ -f "$state/latest_round_comment" ]; then cat "$state/latest_round_comment"; fi + if [ -f "$state/status_comment" ]; then cat "$state/status_comment"; fi ;; esac ;; @@ -2968,6 +2976,30 @@ case "${1:-}" in ;; esac ;; + api) + if [ "${DEVLOOP_GH_PATCH_FAIL:-0}" = "1" ]; then + printf '%s\n' "gh api exploded" >&2 + exit 1 + fi + body="" + while [ "$#" -gt 0 ]; do + case "$1" in + -f) + shift + case "${1:-}" in + body=*) body="${1#body=}" ;; + esac + ;; + esac + shift || true + done + if [ -n "$body" ]; then + printf '%s\n' "$body" > "$state/status_comment" + status_count="$(find "$state/comments" -name 'status-*.md' | wc -l | tr -d ' ')" + printf '%s\n' "$body" > "$state/comments/status-$((status_count + 1)).md" + fi + printf '%s\n' "patched" + ;; *) exit 1 ;; @@ -3151,7 +3183,7 @@ mode="${DEVLOOP_FAKE_MODE:-accept}" if [ -n "${DEVLOOP_AGENT_LOG:-}" ]; then printf 'agent coder %s\n' "${pass:-1}" >> "$DEVLOOP_AGENT_LOG" if [ "${pass:-1}" = "2" ]; then - if printf '%s\n' "$prompt" | grep -q "# Devloop Review Round 1"; then + if printf '%s\n' "$prompt" | grep -q ''; then printf '%s\n' "coder-pr-prior:yes" >> "$DEVLOOP_AGENT_LOG" else printf '%s\n' "coder-pr-prior:no" >> "$DEVLOOP_AGENT_LOG" @@ -3605,36 +3637,25 @@ pr_body_footer="$(printf '%s\n' "$pr_body" | awk 'NF { line = $0 } END { print l equals "$pr_body_footer" "Generated by [devloop.sh](https://devloop.sh)" "created PR body footer" if printf '%s\n' "$pr_body" | grep -Eq '^[0-9a-f]{7,40}$'; then fail "created PR body leaked bare commit hash"; fi if printf '%s\n' "$pr_body" | grep -q '/Users/'; then fail "created PR body leaked absolute local path"; fi -equals "$(find "$pr_state/comments" -name 'round-*.md' | wc -l | tr -d ' ')" "1" "one round PR comment" -equals "$(find "$pr_state/comments" -name 'final-*.md' | wc -l | tr -d ' ')" "1" "one final PR comment" -round_body="$(cat "$pr_state/comments/round-1.md")" -contains "$round_body" "# Devloop Review Round 1" "round PR comment" -contains "$round_body" "Verdict: ACCEPT" "round PR comment" -contains "$round_body" "## Acceptance matrix" "round PR comment" -contains "$round_body" "| AC1 | PASS |" "round PR comment" -contains "$round_body" "## Engineering quality matrix" "round PR comment" -contains "$round_body" "| Security | N/A |" "round PR comment" -contains "$round_body" "## Review flags" "round PR comment" -contains "$round_body" "## Findings" "round PR comment" -contains "$round_body" "## Missing tests" "round PR comment" -contains "$round_body" "## Fix instructions" "round PR comment" -contains "$round_body" "## Notes" "round PR comment" -final_body="$(cat "$pr_state/comments/final-1.md")" -contains "$final_body" "# Devloop Final Report" "final PR comment" -contains "$final_body" "Final status" "final PR comment" -contains "$final_body" "Pass count" "final PR comment" -contains "$final_body" "Final verdict" "final PR comment" -contains "$final_body" "Acceptance Matrix Summary" "final PR comment" -contains "$final_body" "Engineering Quality Summary" "final PR comment" -contains "$final_body" "Implementation Summary" "final PR comment" -contains "$final_body" "Tests Run" "final PR comment" -contains "$final_body" "Residual Risk" "final PR comment" -contains "$final_body" "PR URL" "final PR comment" -contains "$final_body" "Branch" "final PR comment" -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 +[[ -s "$pr_state/status_comment" ]] || fail "living status comment missing" +status_body="$(cat "$pr_state/status_comment")" +contains "$status_body" "" "status comment marker" +contains "$status_body" "## Review status" "status comment heading" +contains "$status_body" "### Quality gates" "status comment gates" +contains "$status_body" "| Specification |" "status comment specification row" +contains "$status_body" "| Engineering |" "status comment engineering row" +contains "$status_body" "Review history:" "status comment history block" +contains "$status_body" "Gate details" "status comment gate details block" +contains "$status_body" "Latest review detail" "status comment review detail block" +contains "$status_body" "Run details" "status comment run details block" +contains "$status_body" "Verdict: ACCEPT" "status comment carries the verdict" +contains "$status_body" "| AC1 | PASS |" "status comment carries the acceptance matrix" +contains "$status_body" "| Security | N/A |" "status comment carries the quality matrix" +not_contains "$status_body" "### Open findings" "accepted status comment omits findings" +equals "$(printf '%s\n' "$status_body" | grep -c '')" "1" "status comment marker appears once" +if printf '%s\n' "$status_body" | grep -q '/Users/'; then fail "status comment leaked absolute local path"; fi +if printf '%s\n' "$status_body" | grep -q 'Local cache'; then fail "status comment leaked local cache path"; fi +if printf '%s\n' "$status_body" | grep -Eq '<(html|script|style)'; then fail "status 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" @@ -3655,9 +3676,8 @@ if pr_terminal_output="$(run_loop "$pr_repo" "e2e-pr-terminal" bad-ac 1 "--creat fail "PR terminal failure loop unexpectedly passed" fi 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" +[[ -s "$pr_state/status_comment" ]] || fail "terminal status comment missing" +contains "$(cat "$pr_state/status_comment")" "Run status: unclear" "terminal status comment reports the run status" 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" @@ -3681,7 +3701,7 @@ fi contains "$pr_existing_output" "https://github.com/satyaborg/devloop/pull/456" "existing PR loop" if grep -q 'gh pr create' "$pr_log"; then fail "existing PR loop created a duplicate PR"; fi contains "$(cat "$pr_log")" "gh pr list" "existing PR lookup" -equals "$(find "$pr_state/comments" -name 'round-*.md' | wc -l | tr -d ' ')" "1" "existing PR round comment" +[[ -s "$pr_state/status_comment" ]] || fail "existing PR status comment missing" unset DEVLOOP_GH_STATE DEVLOOP_GH_LOG DEVLOOP_AGENT_LOG ok "PR-backed existing PR reuse" @@ -3702,7 +3722,12 @@ if ! pr_retry_output="$(run_loop "$pr_repo" "e2e-pr-retry" reject-then-accept 2 fi contains "$pr_retry_output" "accepted" "PR retry loop" contains "$(cat "$pr_log")" "coder-pr-prior:yes" "PR retry prior review" -equals "$(find "$pr_state/comments" -name 'round-*.md' | wc -l | tr -d ' ')" "2" "two round PR comments" +equals "$(grep -c 'gh pr comment' "$pr_log")" "1" "two passes create exactly one PR comment" +if ! grep -q 'gh api --method PATCH' "$pr_log"; then fail "later passes did not edit the living comment"; fi +pr_retry_status="$(cat "$pr_state/status_comment")" +contains "$pr_retry_status" "Review history: 2 passes" "living comment records both passes" +equals "$(printf '%s\n' "$pr_retry_status" | grep -cE '^\| [12] \|')" "2" "living comment keeps one history row per pass" +contains "$pr_retry_status" "REJECT" "living comment retains the earlier rejection" unset DEVLOOP_GH_STATE DEVLOOP_GH_LOG DEVLOOP_AGENT_LOG ok "PR-backed retry uses durable PR review" @@ -3723,7 +3748,7 @@ if pr_comment_fail_output="$(run_loop "$pr_repo" "e2e-pr-comment-fail" accept 1 fail "PR comment failure loop unexpectedly passed" fi contains "$pr_comment_fail_output" "pr-error" "PR comment failure" -contains "$pr_comment_fail_output" "PR comment failed: gh comment exploded" "PR comment failure" +contains "$pr_comment_fail_output" "PR status comment failed: gh comment exploded" "PR comment failure" unset DEVLOOP_GH_STATE DEVLOOP_GH_LOG DEVLOOP_AGENT_LOG DEVLOOP_GH_COMMENT_FAIL ok "PR comment failure handling"