diff --git a/agentkit/skills/.shared/scripts/agent-run.sh b/agentkit/skills/.shared/scripts/agent-run.sh index d5324d94..10dc4045 100755 --- a/agentkit/skills/.shared/scripts/agent-run.sh +++ b/agentkit/skills/.shared/scripts/agent-run.sh @@ -1576,16 +1576,16 @@ report_failure() { } # ---------------------------------------------------------------- verification cache --- +verification_command_name() { + case ${cmd_name:-} in test|lint|typecheck|coverage|verify|check|*-test|*-lint|*-typecheck|*-check) return 0 ;; *) return 1 ;; esac +} verification_cache_eligible() { verification_ineligible_reason='' [[ $cmd_declared == yes ]] || { verification_ineligible_reason='not-declared'; return 1; } [[ $verification_mode == local ]] || { verification_ineligible_reason='mode-not-local'; return 1; } [[ -n $verification_tools ]] || { verification_ineligible_reason='no-toolchain'; return 1; } [[ -z $baseline_ref ]] || { verification_ineligible_reason='baseline-run'; return 1; } - case ${cmd_name:-} in - test|lint|typecheck|coverage|verify|check|*-test|*-lint|*-typecheck|*-check) return 0 ;; - *) verification_ineligible_reason='name-not-verification'; return 1 ;; - esac + verification_command_name || { verification_ineligible_reason='name-not-verification'; return 1; } } hash_untracked_files() { @@ -1925,7 +1925,12 @@ printf ' if this call returns before "=== agent-run exited", the run is still g readonly LOG_HEADER_LINES=2 log_head=none log_clean=no [[ -z $git_top ]] || log_head=$(git -C "$git_top" rev-parse --verify -q HEAD 2> /dev/null) || log_head=none -[[ -z $git_top || -n $(git -C "$git_top" status --porcelain --untracked-files=no 2> /dev/null || printf x) ]] || log_clean=yes +git_worktree_status() { + git -C "$git_top" status --porcelain --untracked-files=normal 2> /dev/null +} +if [[ -n $git_top ]] && log_status=$(git_worktree_status); then + [[ -n $log_status ]] || log_clean=yes +fi { printf '=== agent-run %s\n' "$cmd_str" printf '=== started %s pid=%s process-start=%s epoch=%s cwd=%s concurrent-suites=%s head=%s tracked-clean=%s\n' \ @@ -1987,6 +1992,15 @@ fi if ((rc != 0)) && compose_dependency_start_collision "$log_file"; then printf '=== finding environment-retry-eligible: compose dependency-start collision (not a code regression)\n' >> "$log_file" fi +if ((rc == 0)) && [[ $log_clean == yes ]] && verification_command_name; then + if ! log_status=$(git_worktree_status); then + rc=1 baseline_excluded=no + printf '=== finding checkout-dirty: checkout status unavailable after the command\n' >> "$log_file" + elif [[ -n $log_status ]]; then + rc=1 baseline_excluded=no + printf '=== finding checkout-dirty: checkout became dirty after the command\n' >> "$log_file" + fi +fi printf '=== agent-run exited rc=%s after %ss\n' "$rc" "$elapsed" >> "$log_file" receipt_failure=no if ! publish_log_sha256_receipt; then diff --git a/agentkit/skills/review-remote-pr/scripts/finding-ledger.sh b/agentkit/skills/review-remote-pr/scripts/finding-ledger.sh index 1856ef16..fc750c08 100755 --- a/agentkit/skills/review-remote-pr/scripts/finding-ledger.sh +++ b/agentkit/skills/review-remote-pr/scripts/finding-ledger.sh @@ -443,6 +443,14 @@ resolve_commit() { git -C "$1" rev-parse --verify -q "$2^{commit}" 2>/dev/null || die_evidence "not a commit in $1: $2" } +require_clean_checkout() { + local status + status=$(git -C "$1" status --porcelain --untracked-files=normal 2>/dev/null) || + die_evidence 'could not inspect checkout status; evidence unavailable' + [[ -z $status ]] || + die_evidence 'checkout has staged, unstaged, or untracked changes; commit or remove them before evidence' +} + require_tested_head() { local log=$1 current=$2 header tested clean header=$(sed -n '2p' "$log") @@ -455,7 +463,7 @@ require_tested_head() { [[ $tested == "$current" ]] || die_evidence "verification log tested $tested, not the current head $current" [[ $clean == yes ]] || - die_evidence 'verification log ran with uncommitted tracked changes; commit the repair, then run agent-run.sh --cmd test' + die_evidence 'verification log began with staged, unstaged, or untracked changes; clean the checkout, then run agent-run.sh --cmd test' } # Emit fixed-verdict evidence for one finding, refusing anything add would @@ -486,6 +494,7 @@ cmd_evidence() { repair_sha=$(resolve_commit "$root" "$repair_sha") || exit 1 [[ $head == "$actual_head" ]] || die_evidence "evidence head $head is not the current head $actual_head" + require_clean_checkout "$root" command=$(sed -n '1s/^=== agent-run //p' "$log") declared=$("$SCRIPT_DIR/../../.shared/scripts/repo-config.sh" --repo-root "$root" \ --get-argv AGENT_CMD_TEST | tr '\0' ' ') || die_evidence 'the repository declares no AGENT_CMD_TEST' diff --git a/tests/lint-helper-size.sh b/tests/lint-helper-size.sh index 8f07f61e..5d2fbe81 100755 --- a/tests/lint-helper-size.sh +++ b/tests/lint-helper-size.sh @@ -29,7 +29,8 @@ declare -A KNOWN_OVERSIZE=( # #731/#732/#776/#809/#874/PR #877: yielded-run status and lease lifecycle. # #873: bind repair evidence to the clean committed head tested before push. # #874 review repair: stable process identity and canonical fallback boundaries. - [skills/.shared/scripts/agent-run.sh]="2017:21859:800" + # #873 PR repair: fail closed when a clean-start verification dirties its checkout. + [skills/.shared/scripts/agent-run.sh]="2031:22008:800" # #865: scope the generated regeneration hint to plugin-backed onboarding. [skills/.shared/scripts/bootstrap-repo.sh]="818:10363:800" # #777: repository-facts recipe moved from injected prose into --help. @@ -196,7 +197,8 @@ readonly MAX_HELPER_TOKENS=10000 # #873 PR repair: bind evidence overrides to the checkout's actual HEAD. # #873 evidence repair + #874/#875 merge: exact combined helper tree measurement. # #873/#874 + #875 + #876 provider-alias repair: exact combined helper tree measurement. -readonly MAX_TREE_TOKENS=477181 +# #873 PR repair: verification-scoped post-run cleanliness and setup compatibility. +readonly MAX_TREE_TOKENS=477429 violations=0 checked=0 diff --git a/tests/test-agent-run-cmd.sh b/tests/test-agent-run-cmd.sh index 3e979c0b..9074f130 100755 --- a/tests/test-agent-run-cmd.sh +++ b/tests/test-agent-run-cmd.sh @@ -401,22 +401,80 @@ assert_contains "$out" 'resume this same call; never relaunch' \ # tested. The header records HEAD and tracked-tree cleanliness so a stale or # dirty log cannot certify the branch tip that was pushed. bound_repo=$(make_repo) -printf 'AGENT_CMD_OK=echo hello\n' >"$bound_repo/.agent/config.env" +printf 'AGENT_CMD_OK=echo hello\nAGENT_CMD_SETUP=./dirty-setup.sh\nAGENT_CMD_DIRTY_TEST=./dirty-tracked.sh\nAGENT_CMD_DIRTY_CHECK=./dirty-untracked.sh\n' \ + >"$bound_repo/.agent/config.env" printf '.agent/\n' >"$bound_repo/.gitignore" printf 'a\n' >"$bound_repo/tracked.txt" -git -C "$bound_repo" add .gitignore tracked.txt +printf '#!/bin/sh\nprintf "changed\\n" >tracked.txt\n' >"$bound_repo/dirty-tracked.sh" +printf '#!/bin/sh\nprintf "generated\\n" >verification-input.txt\n' >"$bound_repo/dirty-untracked.sh" +printf '#!/bin/sh\nprintf "configured\\n" >setup-output.txt\n' >"$bound_repo/dirty-setup.sh" +chmod +x "$bound_repo/dirty-tracked.sh" "$bound_repo/dirty-untracked.sh" "$bound_repo/dirty-setup.sh" +git -C "$bound_repo" add .gitignore tracked.txt dirty-tracked.sh dirty-untracked.sh dirty-setup.sh git -C "$bound_repo" -c user.name=Test -c user.email=test@example.invalid commit -qm init bound_sha=$(git -C "$bound_repo" rev-parse HEAD) (cd "$bound_repo" && "$real_run_sh" --cmd ok >/dev/null 2>&1) log=$(cat "$bound_repo"/.agent/logs/*-ok.log) assert_contains "$log" "head=$bound_sha tracked-clean=yes" \ 'the log header binds a clean run to its committed head' +printf 'input\n' >"$bound_repo/untracked-test.conf" +rm -f -- "$bound_repo"/.agent/logs/*-ok.log* +untracked_dev_rc=0 +(cd "$bound_repo" && "$real_run_sh" --cmd ok >/dev/null 2>&1) || untracked_dev_rc=$? +log=$(cat "$bound_repo"/.agent/logs/*-ok.log) +assert_eq 0 "$untracked_dev_rc" 'an ordinary command keeps its native status in a dirty development tree' +assert_contains "$log" "head=$bound_sha tracked-clean=no" \ + 'a nonignored untracked verification input makes the run non-certifiable' +rm "$bound_repo/untracked-test.conf" +status_bin="$tmp/status-bin" +mkdir "$status_bin" +cat >"$status_bin/git" <<'EOF' +#!/bin/sh +for arg do + [ "$arg" != status ] || exit 7 +done +exec "$AGENTKIT_REAL_GIT" "$@" +EOF +chmod +x "$status_bin/git" +rm -f -- "$bound_repo"/.agent/logs/*-ok.log* +status_dev_rc=0 +(cd "$bound_repo" && PATH="$status_bin:$PATH" AGENTKIT_REAL_GIT=$(command -v git) \ + "$real_run_sh" --cmd ok >/dev/null 2>&1) || status_dev_rc=$? +log=$(cat "$bound_repo"/.agent/logs/*-ok.log) +assert_eq 0 "$status_dev_rc" 'a status-read error does not block an ordinary development command' +assert_contains "$log" "head=$bound_sha tracked-clean=no" \ + 'a status-read error fails closed instead of certifying an empty clean status' printf 'b\n' >"$bound_repo/tracked.txt" rm -f -- "$bound_repo"/.agent/logs/*-ok.log* -(cd "$bound_repo" && "$real_run_sh" --cmd ok >/dev/null 2>&1) +dirty_dev_rc=0 +(cd "$bound_repo" && "$real_run_sh" --cmd ok >/dev/null 2>&1) || dirty_dev_rc=$? log=$(cat "$bound_repo"/.agent/logs/*-ok.log) +assert_eq 0 "$dirty_dev_rc" 'preexisting tracked dirt does not change an ordinary command status' assert_contains "$log" "head=$bound_sha tracked-clean=no" \ 'the log header records uncommitted tracked changes' +git -C "$bound_repo" checkout -q -- tracked.txt +rm -f -- "$bound_repo"/.agent/logs/*-setup.log* +setup_rc=0 +(cd "$bound_repo" && "$real_run_sh" --cmd setup >/dev/null 2>&1) || setup_rc=$? +assert_eq 0 "$setup_rc" 'a setup command keeps its native success after creating an untracked output' +rm -f -- "$bound_repo/setup-output.txt" +rm -f -- "$bound_repo"/.agent/logs/*-dirty-test.log* +dirty_tracked_rc=0 +(cd "$bound_repo" && "$real_run_sh" --cmd dirty-test >/dev/null 2>&1) || dirty_tracked_rc=$? +dirty_tracked_log=$(cat "$bound_repo"/.agent/logs/*-dirty-test.log) +assert_eq 1 "$dirty_tracked_rc" 'a clean-start command cannot pass after dirtying a tracked file' +assert_contains "$dirty_tracked_log" 'checkout became dirty after the command' \ + 'the tracked post-run refusal names the reproducibility failure' +assert_contains "$dirty_tracked_log" '=== agent-run exited rc=1' \ + 'a tracked post-run change cannot leave a green terminal marker' +git -C "$bound_repo" checkout -q -- tracked.txt +rm -f -- "$bound_repo"/.agent/logs/*-dirty-check.log* +dirty_untracked_rc=0 +(cd "$bound_repo" && "$real_run_sh" --cmd dirty-check >/dev/null 2>&1) || dirty_untracked_rc=$? +dirty_untracked_log=$(cat "$bound_repo"/.agent/logs/*-dirty-check.log) +assert_eq 1 "$dirty_untracked_rc" 'a clean-start command cannot pass after creating nonignored untracked input' +assert_contains "$dirty_untracked_log" 'checkout became dirty after the command' \ + 'the untracked post-run refusal names the reproducibility failure' +rm "$bound_repo/verification-input.txt" # The suppressed-line count must report the command output, not the markers. assert_contains "$out" '(1 lines suppressed' 'the line count excludes the log bookkeeping' @@ -772,7 +830,7 @@ assert_contains "$out" 'declared-test-ran' \ # runner-resolved link; finding 2 carries --force into build_chain_argv. Both # were offset by further comment trims elsewhere, holding the line count at 1627. # #612 adds paired formatter resolution and bounded cargo failure summaries. -assert_eq yes "$([[ $(wc -l < "$root/agentkit/skills/.shared/scripts/agent-run.sh") -le 2017 ]] && printf yes || printf no)" \ - 'agent-run.sh stays at or under 2017 lines (#873 metadata + #874 review repair)' +assert_eq yes "$([[ $(wc -l < "$root/agentkit/skills/.shared/scripts/agent-run.sh") -le 2031 ]] && printf yes || printf no)" \ + 'agent-run.sh stays at or under 2031 lines (#873 evidence cleanliness)' finish diff --git a/tests/test-finding-ledger.sh b/tests/test-finding-ledger.sh index ee7a6916..0b35d1d0 100755 --- a/tests/test-finding-ledger.sh +++ b/tests/test-finding-ledger.sh @@ -7,6 +7,16 @@ here=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) root=$(dirname -- "$here") source "$here/lib/assert.sh" +fixture_digest() { + if command -v sha256sum >/dev/null 2>&1; then + sha256sum -- "$1" + elif command -v shasum >/dev/null 2>&1; then + shasum -a 256 -- "$1" + else + return 1 + fi +} + script="$root/agentkit/skills/review-remote-pr/scripts/finding-ledger.sh" tmp=$(mktemp -d) trap 'rm -rf -- "$tmp"' EXIT @@ -198,7 +208,7 @@ printf 'repaired\n' >"$repair_repo/affected.sh" git -C "$repair_repo" commit -qam repair repair_sha=$(git -C "$repair_repo" rev-parse HEAD) printf '=== agent-run tests/regression.sh\n=== agent-run exited rc=0 after 1s\n' >"$tmp/verification.log" -log_hash=$(sha256sum "$tmp/verification.log"); log_hash=${log_hash%% *} +log_hash=$(fixture_digest "$tmp/verification.log"); log_hash=${log_hash%% *} for n in {1..8}; do jq -n --arg finding "confirmed-$n" --arg sha "$repair_sha" --arg log "$tmp/verification.log" --arg digest "$log_hash" \ '{finding:$finding,repairSha:$sha,head:$sha,path:"affected.sh",command:"tests/regression.sh",status:"passed",log:$log,logSha256:$digest}' >"$tmp/repair.json" @@ -222,6 +232,9 @@ for tool in bash dirname jq git grep tail; do ln -s "$(command -v "$tool")" "$portable_bin/$tool" done ln -s "$(command -v shasum)" "$portable_bin/shasum" +portable_fixture_digest=$(PATH="$portable_bin" fixture_digest "$tmp/verification.log") +assert_eq "$log_hash" "${portable_fixture_digest%% *}" \ + 'test fixture hashing supports shasum without GNU sha256sum' portable_rc=0 portable_out=$(PATH="$portable_bin" "$script" status --file "$open_run/findings.ndjson" \ --repo-root "$repair_repo" --head "$repair_sha" 2>&1) || portable_rc=$? @@ -358,6 +371,40 @@ assert_eq "$ev_repair" "$(jq -r .repairSha <<<"$ev_out")" 'evidence records the assert_eq "$ev_head" "$(jq -r .head <<<"$ev_out")" 'evidence head defaults to the checkout HEAD' assert_eq 'tests/regression.sh' "$(jq -r .command <<<"$ev_out")" 'evidence records the logged command' assert_eq false "$(jq 'has("reviewedHead")' <<<"$ev_out")" 'evidence carries no reviewedHead' +printf 'dirty\n' >>"$ev_repo/other.txt" +dirty_checkout_rc=0 +evidence --log "$tmp/ev-full.log" --repair-sha "$ev_repair" \ + >/dev/null 2>"$tmp/ev-dirty-checkout.err" || dirty_checkout_rc=$? +assert_eq 1 "$dirty_checkout_rc" 'evidence refuses current tracked checkout changes' +assert_contains "$(cat "$tmp/ev-dirty-checkout.err")" 'checkout has staged, unstaged, or untracked changes' \ + 'the current tracked-dirt refusal names every unsupported change class' +git -C "$ev_repo" checkout -q -- other.txt +printf 'input\n' >"$ev_repo/untracked-test.conf" +untracked_checkout_rc=0 +evidence --log "$tmp/ev-full.log" --repair-sha "$ev_repair" \ + >/dev/null 2>"$tmp/ev-untracked-checkout.err" || untracked_checkout_rc=$? +assert_eq 1 "$untracked_checkout_rc" 'evidence refuses current nonignored untracked inputs' +assert_contains "$(cat "$tmp/ev-untracked-checkout.err")" 'checkout has staged, unstaged, or untracked changes' \ + 'the current untracked refusal uses the same fail-closed boundary' +rm "$ev_repo/untracked-test.conf" +status_bin="$tmp/evidence-status-bin" +mkdir "$status_bin" +cat >"$status_bin/git" <<'EOF' +#!/bin/sh +for arg do + [ "$arg" != status ] || exit 7 +done +exec "$AGENTKIT_REAL_GIT" "$@" +EOF +chmod +x "$status_bin/git" +status_error_rc=0 +PATH="$status_bin:$PATH" AGENTKIT_REAL_GIT=$(command -v git) \ + "$script" evidence --title 'Guard input' --path affected.sh --repo-root "$ev_repo" \ + --log "$tmp/ev-full.log" --repair-sha "$ev_repair" \ + >/dev/null 2>"$tmp/ev-status-error.err" || status_error_rc=$? +assert_eq 1 "$status_error_rc" 'evidence fails closed when checkout status is unavailable' +assert_contains "$(cat "$tmp/ev-status-error.err")" 'could not inspect checkout status' \ + 'the status error is not mistaken for an empty clean checkout' printf '%s\n' "$ev_out" >"$tmp/ev.json" ev_run="$tmp/ev-run" mkdir -m 700 "$ev_run" @@ -400,8 +447,8 @@ other_head_err=$(evidence --log "$tmp/ev-other-head.log" --repair-sha "$ev_repai assert_contains "$other_head_err" "tested $ev_repair, not the current head $ev_head" \ 'the stale-log refusal names both the tested and current heads' dirty_err=$(evidence --log "$tmp/ev-dirty.log" --repair-sha "$ev_repair" 2>&1 >/dev/null || true) -assert_contains "$dirty_err" 'uncommitted tracked changes' \ - 'the dirty-log refusal tells the worker to commit before full verification' +assert_contains "$dirty_err" 'staged, unstaged, or untracked changes' \ + 'the dirty-log refusal names every change class that prevents reproducible verification' unbound_err=$(evidence --log "$tmp/ev-unbound.log" --repair-sha "$ev_repair" 2>&1 >/dev/null || true) assert_contains "$unbound_err" 'no tested-head metadata' \ 'the unbound-log refusal asks for a current agent-run log' @@ -451,7 +498,7 @@ fi # Validation remains backward-compatible with records produced before the # tested-head header existed. Only evidence creation requires the new binding. -legacy_digest=$(sha256sum "$tmp/ev-unbound.log"); legacy_digest=${legacy_digest%% *} +legacy_digest=$(fixture_digest "$tmp/ev-unbound.log"); legacy_digest=${legacy_digest%% *} jq -c --arg log "$tmp/ev-unbound.log" --arg digest "$legacy_digest" \ '.evidence.log=$log | .evidence.logSha256=$digest' "$ev_run/findings.ndjson" >"$tmp/legacy-repair.ndjson" assert_eq complete "$("$script" status --file "$tmp/legacy-repair.ndjson" --repo-root "$ev_repo" \