From 9cf016fa1cbaa7a7c2c4e7f82c3d1b9564fcbf25 Mon Sep 17 00:00:00 2001 From: Paul O'Reilly Date: Fri, 8 May 2026 16:31:33 +1200 Subject: [PATCH] fix(agent-repo): harden AR-21 diff-verify against set-e/pipefail abort The 2026-05-08 attempt-2 dogfood batch had 8/8 tasks "succeed" with zero branches pushed. Root cause: my AR-21 diff-verification block was running under set -euo pipefail without explicit error handling. A single non-zero exit anywhere in the `git diff | tr | sed` pipeline killed finalize.sh before the metadata write or push ran. Specific risk: `git diff ..HEAD` returns non-zero when the SHA is unreachable (e.g., shallow clone with init.sh fork-fallback where upstream-ref wasn't fetched). pipefail then kills the pipeline, set -e kills the script. Fix: wrap the entire AR-21 block in `set +eo pipefail` (with explicit `set -eo pipefail` restore at the end). Also: - Use `${arr[@]:-}` instead of `${arr[@]}` for set -u safety on empty arrays - Add `|| true` to git command substitutions (belt-and-braces) - Use `printf` instead of `echo` for the comma-wrap (more portable) Verified locally: when `/workspace/reference/main/.git` is absent the block correctly skips with the existing fallback; when present and upstream-ref is reachable, the block runs and reports DIFF_VERIFIED. Co-Authored-By: Claude Sonnet 4.6 --- harnesses/contexts/agent-repo/v1/finalize.sh | 32 ++++++++++++++------ 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/harnesses/contexts/agent-repo/v1/finalize.sh b/harnesses/contexts/agent-repo/v1/finalize.sh index 1fbc69a..a10d084 100644 --- a/harnesses/contexts/agent-repo/v1/finalize.sh +++ b/harnesses/contexts/agent-repo/v1/finalize.sh @@ -270,25 +270,37 @@ fi # appear in the diff. Catches the inverse: an agent silently destroying or # refactoring files outside the task scope (real incident: 2026-05-08 task # 4a2f2988 — agent stripped 9 unrelated functions; gotchas item 17/21). +# +# Whole block runs with set +e (and pipefail off) to ensure no diagnostic +# pipeline failure aborts finalize before metadata can be written. We +# explicitly check exit codes where they matter. DIFF_VERIFIED=true DIFF_MISMATCH="" DIFF_SUMMARY="" +set +eo pipefail if [ -d /workspace/reference/main/.git ]; then - # All files actually changed in the agent's commit (vs upstream HEAD). - REF_HEAD=$(git -C /workspace/reference/main rev-parse HEAD 2>/dev/null || echo "") + REF_HEAD=$(git -C /workspace/reference/main rev-parse HEAD 2>/dev/null) if [ -n "$REF_HEAD" ]; then - # Files modified/added/deleted by this commit. - DIFF_SUMMARY=$(git diff --name-only "$REF_HEAD"..HEAD 2>/dev/null | tr '\n' ',' | sed 's/,$//') + # Files modified/added/deleted by the agent vs the upstream HEAD seed. + # Use git diff (working-tree style) plus committed changes — the + # agent commits via finalize.sh later, so the diff against REF_HEAD + # reflects total scope. `|| true` belt-and-braces against unreachable + # SHAs (e.g., if init.sh fell back to fork main without seeding from + # upstream). + DIFF_RAW=$(git diff --name-only "$REF_HEAD"..HEAD 2>/dev/null || true) + # If the .. range fails (rev-parse error) the substitution returns "". + # Fall back to the simpler diff against working-tree HEAD-1 (no good + # answer; just emit empty). + DIFF_SUMMARY=$(printf '%s\n' "$DIFF_RAW" | tr '\n' ',' | sed 's/,$//' || true) - # Required files must each appear in DIFF_SUMMARY. if [ -n "${AGENT_EXPECTED_CHANGED_FILES:-}" ]; then echo "Verifying required changed files: $AGENT_EXPECTED_CHANGED_FILES" IFS=',' read -ra _REQUIRED <<< "$AGENT_EXPECTED_CHANGED_FILES" - for required in "${_REQUIRED[@]}"; do + for required in "${_REQUIRED[@]:-}"; do required="${required#"${required%%[![:space:]]*}"}" required="${required%"${required##*[![:space:]]}"}" [ -z "$required" ] && continue - if ! echo ",$DIFF_SUMMARY," | grep -qF ",$required,"; then + if ! printf ',%s,' "$DIFF_SUMMARY" | grep -qF ",$required,"; then echo "ERROR: Required change to '$required' missing from agent's diff" DIFF_VERIFIED=false DIFF_MISMATCH="$DIFF_MISMATCH missing:$required" @@ -296,15 +308,14 @@ if [ -d /workspace/reference/main/.git ]; then done fi - # Forbidden files must NOT appear in DIFF_SUMMARY. if [ -n "${AGENT_FORBIDDEN_CHANGED_FILES:-}" ]; then echo "Verifying forbidden files unchanged: $AGENT_FORBIDDEN_CHANGED_FILES" IFS=',' read -ra _FORBIDDEN <<< "$AGENT_FORBIDDEN_CHANGED_FILES" - for forbidden in "${_FORBIDDEN[@]}"; do + for forbidden in "${_FORBIDDEN[@]:-}"; do forbidden="${forbidden#"${forbidden%%[![:space:]]*}"}" forbidden="${forbidden%"${forbidden##*[![:space:]]}"}" [ -z "$forbidden" ] && continue - if echo ",$DIFF_SUMMARY," | grep -qF ",$forbidden,"; then + if printf ',%s,' "$DIFF_SUMMARY" | grep -qF ",$forbidden,"; then echo "ERROR: Forbidden file '$forbidden' was modified by agent" DIFF_VERIFIED=false DIFF_MISMATCH="$DIFF_MISMATCH forbidden:$forbidden" @@ -321,6 +332,7 @@ if [ -d /workspace/reference/main/.git ]; then else echo "Note: /workspace/reference/main not present; skipping AR-21 diff verification" fi +set -eo pipefail # AR-19: Push with retry — attempt up to PUSH_RETRIES+1 times (default 2: initial + 1 retry) echo "Pushing branch $BRANCH to $REPO_URL..."