diff --git a/agentkit/.claude-plugin/plugin.json b/agentkit/.claude-plugin/plugin.json index 9ea636ca..8ec78423 100644 --- a/agentkit/.claude-plugin/plugin.json +++ b/agentkit/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "agentkit", - "version": "0.9.11", + "version": "0.9.12", "description": "Board-aware parallel issue and PR review skills, with lifecycle hooks and a per-repository contract.", "author": { "name": "wrzonance", diff --git a/agentkit/.codex-plugin/plugin.json b/agentkit/.codex-plugin/plugin.json index 57947170..fe92f129 100644 --- a/agentkit/.codex-plugin/plugin.json +++ b/agentkit/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "agentkit", - "version": "0.9.11", + "version": "0.9.12", "description": "Board-aware parallel issue and PR review skills, with lifecycle hooks and a per-repository contract.", "author": { "name": "wrzonance", diff --git a/agentkit/skills/.shared/scripts/agent-run.sh b/agentkit/skills/.shared/scripts/agent-run.sh index 9d27fda0..d5324d94 100755 --- a/agentkit/skills/.shared/scripts/agent-run.sh +++ b/agentkit/skills/.shared/scripts/agent-run.sh @@ -14,19 +14,75 @@ if [[ -z ${BASH_VERSION:-} || ${BASH_VERSINFO[0]:-0} -lt 4 ]]; then exit 2 fi +current_process_start() { + local pid=$1 identity + if [[ -r /proc/$pid/stat ]] && identity=$(awk '{print $22}' "/proc/$pid/stat" 2> /dev/null) && + [[ $identity =~ ^[0-9]+$ ]]; then + printf '%s' "$identity" + return 0 + fi + identity=$(LC_ALL=C ps -o lstart= -p "$pid" 2>/dev/null | tr -d '[:space:]') || return 1 + [[ $identity =~ ^[[:alnum:]:]+$ ]] || return 1 + printf '%s' "$identity" +} + +epoch_seconds() { date -u +%s 2>/dev/null || printf '0'; } + +status_agent_log() { + local requested=$1 log fallback_dir last header pid start epoch current elapsed now + [[ -f $requested && ! -L $requested && -O $requested ]] || { + printf 'agent-run: error: status requires an owned regular log: %s\n' "$requested" >&2; exit 2; + } + log=$(realpath -e -- "$requested") || exit 2 + case $log in + */.agent/logs/*.log) ;; + *) + fallback_dir=$(realpath -e -- "${TMPDIR:-/tmp}/agent-logs-$(id -u)" 2>/dev/null || true) + [[ -n $fallback_dir && $log == "$fallback_dir/"*.log ]] || { + printf 'agent-run: error: status path is not an agent log: %s\n' "$requested" >&2; exit 2; + } + ;; + esac + last=$(tail -n 1 -- "$log") + if [[ $last =~ ^===\ agent-run\ exited\ rc=([0-9]+)\ after\ [0-9]+s$ ]]; then + ((BASH_REMATCH[1] == 0)) && printf 'pass\n' || printf 'fail rc=%s\n' "${BASH_REMATCH[1]}" + exit 0 + fi + [[ $last != '=== agent-run interrupted by '* ]] || { printf 'interrupted\n'; exit 0; } + header=$(sed -n '2p' -- "$log") + pid=$(sed -n 's/.* pid=\([0-9][0-9]*\) .*/\1/p' <<< "$header") + start=$(sed -n 's/.* process-start=\([^ ]*\) .*/\1/p' <<< "$header") + epoch=$(sed -n 's/.* epoch=\([0-9][0-9]*\) .*/\1/p' <<< "$header") + current=$(current_process_start "$pid" 2>/dev/null || true) + if [[ -n $pid && -n $start && $current == "$start" && $epoch =~ ^[0-9]+$ ]]; then + now=$(epoch_seconds); elapsed=$((now - epoch)); ((elapsed >= 0)) || elapsed=0 + printf 'running pid=%s elapsed=%ss\n' "$pid" "$elapsed" + else + printf 'interrupted\n' + fi + exit 0 +} + +if [[ ${1:-} == status ]]; then + (($# == 2)) || { printf 'agent-run: error: usage: agent-run.sh status LOG\n' >&2; exit 2; } + status_agent_log "$2" +fi + usage() { cat <<'EOF' -Usage: agent-run.sh [--dir PATH] [--label NAME] [--resolve NAME] [--force] [--summary] [--only NAME[,NAME...]] +Usage: agent-run.sh status LOG + agent-run.sh [--dir PATH] [--label NAME] [--resolve NAME] [--force] [--summary] [--only NAME[,NAME...]] [--baseline-ref REF --baseline-path PATH --baseline-id ID] (--cmd NAME | [--] ...) Runs one command with a sandbox-safe environment and a compact result summary. + status LOG Print running, pass, fail, or interrupted for an agent log. --dir PATH Working directory for the command (default: current directory). --repo-root PATH is a silent alias, accepted for compatibility with the kit's other checkout-path helpers. --label NAME Label used in the log file name (default: the command's basename). --force Require fresh execution, including recovery of unknown evidence. - An identical in-flight local command still returns its handle. + An identical in-flight command is still refused with its log. --summary End with status, exit code, duration, log path, digest, and receipt. --verification-key Read-only query for one local, generic, full-checkout command. Prints only its current fingerprint; creates no execution @@ -41,8 +97,8 @@ Runs one command with a sandbox-safe environment and a compact result summary. --if-declared With --cmd, exit 0 quietly when the repository declares no such command. For a command a skill treats as optional. --resolve NAME Query a named command without executing it. Prints declared, - runner, or unresolved and exits 0, 4, or 3 respectively; exit 2 - is reserved for a fatal unsupported-interpreter guard. + runner, or unresolved and exits 0, 4, or 3 respectively. The + fatal unsupported-interpreter guard exits 2. --cmd NAME Run the command this repository declares under that name, instead of spelling one out. Repeatable: each --cmd runs only after the previous one exits 0 (re-execs itself for the rest); --if-declared @@ -93,7 +149,8 @@ Output: verification miss: no reusable evidence; a following command executes freshly, or a completed result was not stored for later reuse verification current/reused: prior evidence, never a fresh PASS - verification running/unknown: durable handle and exit 75; inspect before retry + active duplicate exits 2 with its original log; wait instead of relaunching + verification unknown: unknown abandoned handle exits 75; inspect before retry Examples: agent-run.sh --cmd test @@ -117,6 +174,7 @@ failure_result() { "${failure_evidence:-${log_file:-stderr}}" "$failure_state" "$failure_action" >&2 fi if declare -F cleanup_suite_run >/dev/null; then cleanup_suite_run; fi + if declare -F cleanup_active_run >/dev/null; then cleanup_active_run; fi if ((summary_ready)); then printf 'agent-run-summary status=%s rc=%s duration_seconds=%s log=%q log-sha256=%s receipt=%q\n' \ "$summary_status" "$status" "$elapsed" "$log_file" "$log_sha256" "$log_sha256_receipt" @@ -162,6 +220,7 @@ finish() { [[ -z ${verification_fd:-} ]] || exec {verification_fd}>&- if ((rc == 0)) && ((${#remaining_queue[@]})); then if declare -F cleanup_suite_run >/dev/null; then cleanup_suite_run; fi + if declare -F cleanup_active_run >/dev/null; then cleanup_active_run; fi build_chain_argv exec "$0" "${chain_argv[@]}" fi @@ -1051,7 +1110,8 @@ apply_test_focus() { # --------------------------------------------------------------------- logs --- choose_log() { local log_dir stamp log - if [[ -n $git_top ]] && dir_writable "$git_top/.agent/logs"; then + if [[ -n $git_top && ! -L $git_top/.agent && ! -L $git_top/.agent/logs ]] && + dir_writable "$git_top/.agent/logs"; then log_dir=$git_top/.agent/logs else # Failing commands routinely echo tokens and credentialed URLs into these @@ -1083,22 +1143,10 @@ concurrent_suites=1 timeout_scale=1 suite_marker_dir=${TMPDIR:-/tmp}/agent-run-suites-$(id -u) -current_process_start() { - local pid=$1 - if [[ -r /proc/$pid/stat ]]; then - awk '{print $22}' "/proc/$pid/stat" 2> /dev/null - else - # macOS has no /proc; kill -0 is the portable liveness fallback. The - # marker is still short-lived and is removed by the EXIT trap. - kill -0 "$pid" 2> /dev/null || return 1 - printf 'alive' - fi -} - suite_marker_live() { local marker=$1 pid start current read -r pid start < "$marker" 2> /dev/null || return 1 - [[ $pid =~ ^[0-9]+$ && ($start =~ ^[0-9]+$ || $start == alive) ]] || return 1 + [[ $pid =~ ^[0-9]+$ && $start =~ ^[[:alnum:]:]+$ ]] || return 1 current=$(current_process_start "$pid") [[ -n $current && $current == "$start" ]] } @@ -1116,7 +1164,7 @@ register_suite_run() { die "cannot create active-suite marker in $suite_marker_dir" pid=$$ start=$(current_process_start "$pid") - [[ $start =~ ^[0-9]+$ || $start == alive ]] || { + [[ $start =~ ^[[:alnum:]:]+$ ]] || { rm -f -- "$marker" die "cannot identify active-suite process $pid" } @@ -1146,6 +1194,40 @@ cleanup_suite_run() { suite_marker='' } +active_run_handle='' active_run_fd='' active_run_owned=0 +claim_active_run() { + local root key prior + [[ -n ${git_top:-} && -z $verification_handle ]] || return 0 + command -v flock >/dev/null || return 0 + [[ ! -L $git_top/.agent ]] || return 0 + root=$git_top/.agent/run-records + assert_private_dir "$root" + key=$(printf '%s\0' "$work_dir" "${cmd[@]}" | sha256sum | awk '{print $1}') + active_run_handle=$root/$key + assert_private_dir "$active_run_handle" + [[ ! -L $active_run_handle/lock && ! -L $active_run_handle/running ]] || + refuse_boundary "active run record is a symlink: $active_run_handle" + exec {active_run_fd}>"$active_run_handle/lock" || refuse_boundary "cannot open active run lock: $active_run_handle/lock" + if ! flock -n "$active_run_fd"; then + IFS= read -r prior < "$active_run_handle/running" 2>/dev/null || true + [[ -n $prior ]] || prior=$active_run_handle + failure_class=usage failure_state=command-already-running failure_action=wait-for-existing-log + failure_evidence=$prior + printf 'agent-run: already running: %s; wait for its exited line or run status on it\n' "$prior" >&2 + finish 2 + fi + printf '%s\n' "$log_file" > "$active_run_handle/running" + active_run_owned=1 +} + +# shellcheck disable=SC2329 # Invoked indirectly by the EXIT trap. +cleanup_active_run() { + ((active_run_owned)) || return 0 + rm -f -- "$active_run_handle/running" 2>/dev/null || true + [[ -z $active_run_fd ]] || exec {active_run_fd}>&- + active_run_owned=0 +} + # A worker may ask for one failed verification to be checked against the chain # base. The source path must be the same blob at both commits, and the base # checkout must produce matching failure evidence. Deliberately opt-in: ordinary @@ -1647,9 +1729,16 @@ claim_verification() { [[ ! -L $verification_handle/running ]] || refuse_boundary "verification running record is a symlink: $verification_handle/running" exec {verification_fd}>"$verification_handle/lock" || refuse_boundary "cannot open verification lock: $verification_handle/lock" if ! flock -n "$verification_fd"; then + local running_log='' + IFS= read -r running_log < "$verification_handle/running" 2>/dev/null || true failure_evidence=$verification_handle - failure_state=verification-running - failure_action=inspect-running-handle-before-retry + failure_state=command-already-running + failure_action=wait-for-existing-log + if [[ $running_log == "$git_top/.agent/logs/"*.log ]]; then + failure_evidence=$running_log + printf 'agent-run: already running: %s; wait for its exited line or run status on it\n' "$running_log" >&2 + finish 2 + fi printf 'agent-run: verification running: handle=%s\n' "$verification_handle" finish 75 fi @@ -1822,25 +1911,27 @@ fi log_file=$(choose_log) [[ -z $verification_handle ]] || printf '%s\n' "$log_file" > "$verification_handle/running" +claim_active_run register_suite_run trap failure_result EXIT +process_start=$(current_process_start "$$") || die "cannot identify active process $$" # Announce the log before captured output makes a long run look hung. -if ((summary_cmd)); then - printf 'running: %s (wait for the terminal agent-run-summary marker)\n' "$cmd_str" >&2 -else - printf 'running: %s\n log: %s (grows while this runs; tail it instead of waiting blind)\n' \ - "$cmd_str" "$log_file" >&2 -fi -printf ' a log with no "=== agent-run exited" line has NOT finished\n' >&2 +printf 'running: %s\n log: %s\n' "$cmd_str" "$log_file" >&2 +printf ' if this call returns before "=== agent-run exited", the run is still going: resume this same call; never relaunch; status: %q status %q\n' \ + "$0" "$log_file" >&2 # The closing marker distinguishes completed logs; exclude bookkeeping lines. 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 { printf '=== agent-run %s\n' "$cmd_str" - printf '=== started %s pid=%s cwd=%s concurrent-suites=%s\n' \ - "$(date -u +%Y-%m-%dT%H:%M:%SZ 2> /dev/null || printf 'unknown')" "$$" "$work_dir" \ - "$concurrent_suites" + printf '=== started %s pid=%s process-start=%s epoch=%s cwd=%s concurrent-suites=%s head=%s tracked-clean=%s\n' \ + "$(date -u +%Y-%m-%dT%H:%M:%SZ 2> /dev/null || printf 'unknown')" "$$" \ + "$process_start" "$(epoch_seconds)" "$work_dir" "$concurrent_suites" \ + "$log_head" "$log_clean" } > "$log_file" started_at=$SECONDS @@ -1859,7 +1950,10 @@ trap 'log_interrupted SIGINT' INT trap 'log_interrupted SIGTERM' TERM rc=0 attempt_start_line=3 -(cd -- "$work_dir" && exec "${cmd[@]}") >> "$log_file" 2>&1 || rc=$? +( + [[ -z $active_run_fd ]] || exec {active_run_fd}>&- + cd -- "$work_dir" && exec "${cmd[@]}" +) >> "$log_file" 2>&1 || rc=$? load_flake_retry=0 if ((rc != 0)) && probe_timeout_load_flake "$log_file"; then load_flake_retry=1 @@ -1867,7 +1961,10 @@ if ((rc != 0)) && probe_timeout_load_flake "$log_file"; then "$concurrent_suites" >> "$log_file" attempt_start_line=$(($(wc -l < "$log_file" | tr -d '[:space:]') + 1)) rc=0 - (cd -- "$work_dir" && exec "${cmd[@]}") >> "$log_file" 2>&1 || rc=$? + ( + [[ -z $active_run_fd ]] || exec {active_run_fd}>&- + cd -- "$work_dir" && exec "${cmd[@]}" + ) >> "$log_file" 2>&1 || rc=$? fi trap - INT TERM elapsed=$((SECONDS - started_at)) diff --git a/agentkit/skills/.shared/scripts/bootstrap-repo.sh b/agentkit/skills/.shared/scripts/bootstrap-repo.sh index 0e2cced5..06501654 100755 --- a/agentkit/skills/.shared/scripts/bootstrap-repo.sh +++ b/agentkit/skills/.shared/scripts/bootstrap-repo.sh @@ -389,9 +389,8 @@ done # infer from the forge. Keep the proposal visible until a valid declaration is # selected; a carried declaration then appears once as active config below. review_providers_declared=0 -if [[ -f $repo_root/.agent/config.env && $reset -eq 0 ]]; then - if grep -qE '^[[:space:]]*AGENT_REVIEW_PROVIDERS=(coderabbit|github-code-quality|none|coderabbit,github-code-quality|github-code-quality,coderabbit)[[:space:]]*$' \ - "$repo_root/.agent/config.env" 2> /dev/null; then +if [[ -f $repo_root/.agent/config.env && $reset -eq 0 && -x $resolver ]]; then + if "$resolver" --repo-root "$repo_root" --get AGENT_REVIEW_PROVIDERS > /dev/null 2>&1; then review_providers_declared=1 fi fi @@ -488,7 +487,8 @@ fi fi if ((review_providers_declared == 0)); then printf '\n# Automated review providers expected on pull requests. Choose one or more.\n' - printf '# Supported choices: coderabbit, github-code-quality, or none (none is exclusive).\n' + printf '# Known providers get provider-specific settlement. Any other valid name is observed\n' + printf '# and triaged on the merits, and is never triggered. Use none alone to disable reviews.\n' printf '# AGENT_REVIEW_PROVIDERS=\n' fi if [[ -n $proposal_inventory ]]; then diff --git a/agentkit/skills/.shared/scripts/lib/review-provider-catalog.sh b/agentkit/skills/.shared/scripts/lib/review-provider-catalog.sh index e45d2130..88d632fd 100644 --- a/agentkit/skills/.shared/scripts/lib/review-provider-catalog.sh +++ b/agentkit/skills/.shared/scripts/lib/review-provider-catalog.sh @@ -1,9 +1,8 @@ #!/usr/bin/env bash # Single source of review-provider capabilities and canonical identities. -# Canonical accepted provider identities, in display order. Every function in -# this file recognizes exactly these values; review_provider_names() is what -# a rejection message names instead of leaving a caller to read this file. +# Provider-specific identities, in display order. Names outside this list use +# the generic observe-only defaults when they match the declaration grammar. # # Not `readonly`: this file is sourced more than once within a single process # in practice (the pr-to-green transition engine sources it directly, then again @@ -19,12 +18,41 @@ review_provider_names() { printf '%s\n' "$out" } +review_provider_name_valid() { + [[ ${1:-} =~ ^[a-z][a-z0-9-]*$ && $1 != coderabbitai ]] +} + +review_provider_login_alias_reserved() { + case ${1,,} in + coderabbitai|github-code-quality) return 0 ;; + *) return 1 ;; + esac +} + +review_provider_known() { + case ${1:-} in + coderabbit|github-code-quality|none) return 0 ;; + *) return 1 ;; + esac +} + +review_provider_override_login() { + local provider=$1 suffix key value + suffix=${provider^^} + suffix=${suffix//-/_} + key=AGENT_REVIEW_PROVIDER_${suffix}_LOGIN + value=${!key-} + [[ -n $value && $value =~ ^[A-Za-z0-9]([A-Za-z0-9_.-]{0,37}[A-Za-z0-9])?$ ]] || return 1 + review_provider_login_alias_reserved "$value" && return 1 + printf '%s\n' "${value,,}" +} + review_provider_mode() { case ${1:-} in coderabbit) printf '%s\n' triggerable ;; github-code-quality) printf '%s\n' observe-only ;; none) printf '%s\n' disabled ;; - *) return 1 ;; + *) review_provider_name_valid "${1:-}" && printf '%s\n' observe-only || return 1 ;; esac } @@ -33,7 +61,7 @@ review_provider_lifecycle() { coderabbit) printf '%s\n' reply-settlement ;; github-code-quality) printf '%s\n' provider-rescan ;; none) printf '%s\n' disabled ;; - *) return 1 ;; + *) review_provider_name_valid "${1:-}" && printf '%s\n' generic-settlement || return 1 ;; esac } @@ -42,18 +70,44 @@ review_provider_login() { coderabbit) printf '%s\n' coderabbitai ;; github-code-quality) printf '%s\n' github-code-quality ;; none) printf '%s\n' none ;; - *) return 1 ;; + *) + review_provider_name_valid "${1:-}" || return 1 + review_provider_override_login "$1" 2>/dev/null || printf '%s\n' "$1" + ;; esac } review_provider_from_login() { - case ${1,,} in + local login=${1,,} provider expected bot_suffix=0 + local -a declared_providers=() + case $login in coderabbitai|coderabbitai\[bot\]) printf '%s\n' coderabbit ;; github-code-quality|github-code-quality\[bot\]) printf '%s\n' github-code-quality ;; - *) return 1 ;; + *) + [[ $login == *'[bot]' ]] && bot_suffix=1 + login=${login%\[bot\]} + IFS=, read -r -a declared_providers <<< "${AGENT_REVIEW_PROVIDERS:-}" + for provider in "${declared_providers[@]}"; do + review_provider_name_valid "$provider" || continue + expected=$(review_provider_login "$provider" 2>/dev/null) || continue + [[ $login == "$expected" ]] || continue + printf '%s\n' "$provider" + return 0 + done + ((bot_suffix)) || return 1 + review_provider_name_valid "$login" || return 1 + [[ $login != none ]] || return 1 + printf '%s\n' "$login" + ;; esac } +review_provider_lane() { + review_provider_name_valid "${1:-}" || return 1 + review_provider_known "$1" && return 1 + printf '%s\n' generic-automated +} + review_provider_request_marker() { case ${1:-} in coderabbit) printf '%s\n' '' ;; diff --git a/agentkit/skills/.shared/scripts/onboard-state.sh b/agentkit/skills/.shared/scripts/onboard-state.sh index fccaebbb..288f09a9 100755 --- a/agentkit/skills/.shared/scripts/onboard-state.sh +++ b/agentkit/skills/.shared/scripts/onboard-state.sh @@ -22,16 +22,24 @@ done [[ -n $repo_root && -d $repo_root ]] || usage (( want_report || want_next || want_preflight || want_next_steps )) || usage repo_root=$(cd -- "$repo_root" && pwd -P) || die "cannot resolve repository root: $repo_root" +self_dir=$(dirname -- "$(readlink -f -- "${BASH_SOURCE[0]}")") state='not onboarded'; next='discover' config=$repo_root/.agent/config.env; board=$repo_root/.agent/board.json marker=$repo_root/.agent/cache/onboarding-stage +config_valid=yes +if [[ -r $config && -x $self_dir/repo-config.sh ]] && + ! "$self_dir/repo-config.sh" --repo-root "$repo_root" --validate >/dev/null 2>&1; then + config_valid=no +fi if [[ ! -r $config ]]; then if [[ $(head -n 1 "$marker" 2> /dev/null || true) == discovered ]]; then state=discovered; next=declare fi elif [[ ! -r $board ]]; then state=discovered; next=declare +elif [[ $config_valid == no ]]; then + state=declared; next=verify else command_declared=no grep -qE '^AGENT_CMD_[A-Z][A-Z0-9_]*=' "$config" 2> /dev/null && command_declared=yes @@ -75,8 +83,6 @@ else fi fi -self_dir=$(dirname -- "$(readlink -f -- "${BASH_SOURCE[0]}")") - # --next is the bare stage/next line, exclusive of every other section. if ((want_next)); then printf 'stage=%s next=%s repo-root=%s\n' "$state" "$next" "$repo_root" diff --git a/agentkit/skills/.shared/scripts/repo-config.sh b/agentkit/skills/.shared/scripts/repo-config.sh index 23b893b2..9affec56 100755 --- a/agentkit/skills/.shared/scripts/repo-config.sh +++ b/agentkit/skills/.shared/scripts/repo-config.sh @@ -91,6 +91,7 @@ readonly BASE_TRUSTED_KEYS=( # The NAME is what `agent-run.sh --cmd ` takes, so it is constrained to a # shape that survives being lowercased into a filename and an argument. readonly CMD_KEY_PATTERN='^AGENT_CMD_[A-Z][A-Z0-9_]*$' +readonly PROVIDER_LOGIN_KEY_PATTERN='^AGENT_REVIEW_PROVIDER_[A-Z][A-Z0-9_]*_LOGIN$' # Mirrors codex-adversarial-review.sh / claude-adversarial-review.sh's own # `--effort` enum (no `ultra`): a value AGENT_WORKER_EFFORT would accept but @@ -212,6 +213,7 @@ done if [[ $mode == keys ]]; then printf '%s\n' "${ACCEPTED_KEYS[@]}" printf 'AGENT_CMD_\n' + printf 'AGENT_REVIEW_PROVIDER__LOGIN\n' printf 'AGENT_RUNDIR_\n' printf 'AGENT_VERIFY__MODE\nAGENT_VERIFY__INPUTS\nAGENT_VERIFY__TOOLCHAIN\n' exit 0 @@ -274,6 +276,7 @@ is_accepted() { # here, and only a test that asked for the hole by name caught it. [[ ! $candidate =~ $SECRET_PATTERN ]] || return 1 [[ ! $candidate =~ $CMD_KEY_PATTERN ]] || return 0 + [[ ! $candidate =~ $PROVIDER_LOGIN_KEY_PATTERN ]] || return 0 [[ ! $candidate =~ $RUNDIR_KEY_PATTERN ]] || return 0 [[ ! $candidate =~ $VERIFY_KEY_PATTERN ]] || return 0 for key in "${ACCEPTED_KEYS[@]}"; do @@ -405,42 +408,40 @@ generated_paths_valid() { done } -# Mirrors lib/review-provider-catalog.sh's REVIEW_PROVIDER_NAMES, for warning -# text only. Kept as a local literal rather than sourcing that file: this +# Mirrors lib/review-provider-catalog.sh's provider-specific names. Kept as a +# local literal rather than sourcing that file: this # parser is deliberately self-contained (see the file header) so a missing or # broken lib file can never turn one bad declaration into a hard failure for # every accepted key. test-repo-config.sh pins this against the catalog. +# shellcheck disable=SC2034 # test-repo-config.sh reads this schema mirror. readonly REVIEW_PROVIDER_ACCEPTED_NAMES=(coderabbit github-code-quality none) -providers_display() { - local out='' name - for name in "${REVIEW_PROVIDER_ACCEPTED_NAMES[@]}"; do - out+="${out:+, }$name" - done - printf '%s' "$out" +provider_login_alias_reserved() { + case ${1,,} in + coderabbitai|github-code-quality) return 0 ;; + *) return 1 ;; + esac } providers_valid() { - local item saw_none=0 seen_coderabbit=0 seen_code_quality=0 + local item saw_none=0 + local -A seen=() [[ -n $1 && $1 != ,* && $1 != *, && $1 != *,,* ]] || return 1 local -a items=() IFS=, read -ra items <<< "$1" ((${#items[@]})) || return 1 for item in "${items[@]}"; do case $item in - coderabbit) - ((saw_none == 0 && seen_coderabbit == 0)) || return 1 - seen_coderabbit=1 - ;; - github-code-quality) - ((saw_none == 0 && seen_code_quality == 0)) || return 1 - seen_code_quality=1 - ;; none) ((saw_none == 0 && ${#items[@]} == 1)) || return 1 saw_none=1 ;; - *) return 1 ;; + *) + [[ $item =~ ^[a-z][a-z0-9-]*$ && $item != coderabbitai ]] || return 1 + ((saw_none == 0)) || return 1 + [[ -z ${seen[$item]+yes} ]] || return 1 + seen[$item]=1 + ;; esac done return 0 @@ -839,6 +840,12 @@ validate() { ;; AGENT_GENERATED_PATHS) generated_paths_valid "$value" ;; AGENT_REVIEW_PROVIDERS) providers_valid "$value" ;; + AGENT_REVIEW_PROVIDER_*_LOGIN) + [[ $key =~ $PROVIDER_LOGIN_KEY_PATTERN && + ! $key =~ ^AGENT_REVIEW_PROVIDER_(CODERABBIT|GITHUB_CODE_QUALITY|NONE)_LOGIN$ && + $value =~ ^[A-Za-z0-9]([A-Za-z0-9_.-]{0,37}[A-Za-z0-9])?$ ]] && + ! provider_login_alias_reserved "$value" + ;; AGENT_WORKER_MODEL | AGENT_WORKER_MODEL_FALLBACK) worker_model_valid "$value" ;; AGENT_WORKER_MODELS | AGENT_WORKER_MODELS_FALLBACK) worker_models_roster_valid "$value" ;; AGENT_WORKER_EFFORT) @@ -975,7 +982,11 @@ while IFS= read -r line || [[ -n $line ]]; do if [[ -z $value ]]; then warn "empty value for $key on line $lineno, ignoring -- to record that this repository has none, comment the line out instead" elif [[ $key == AGENT_REVIEW_PROVIDERS ]]; then - warn "invalid value for $key on line $lineno, ignoring -- accepted: $(providers_display)" + if [[ ,$value, == *,coderabbitai,* ]]; then + warn "invalid value for $key on line $lineno, ignoring -- coderabbitai is a reserved built-in login alias; declare coderabbit instead" + else + warn "invalid value for $key on line $lineno, ignoring -- each name must match [a-z][a-z0-9-]*; none is exclusive; empty and duplicate items are forbidden" + fi elif [[ $key == AGENT_ADVERSARIAL_REVIEWER ]]; then warn "invalid value for $key on line $lineno, ignoring -- accepted: $(names_display "${ADVERSARIAL_REVIEWER_ACCEPTED_NAMES[@]}")" elif [[ $key == AGENT_ADVERSARIAL_REVIEW_EFFORT ]]; then diff --git a/agentkit/skills/.shared/scripts/review-provider-config.sh b/agentkit/skills/.shared/scripts/review-provider-config.sh index f20ff116..58b48bbe 100755 --- a/agentkit/skills/.shared/scripts/review-provider-config.sh +++ b/agentkit/skills/.shared/scripts/review-provider-config.sh @@ -158,5 +158,10 @@ for provider in "${providers[@]}"; do ;; esac fi - printf 'provider=%s mode=%s source=declared\n' "$provider" "$mode" + lane=$(review_provider_lane "$provider" 2>/dev/null || true) + if [[ -n $lane ]]; then + printf 'provider=%s mode=%s source=declared lane=%s\n' "$provider" "$mode" "$lane" + else + printf 'provider=%s mode=%s source=declared\n' "$provider" "$mode" + fi done diff --git a/agentkit/skills/.shared/scripts/run-state.sh b/agentkit/skills/.shared/scripts/run-state.sh index 7a9d5ebd..d93f71f9 100755 --- a/agentkit/skills/.shared/scripts/run-state.sh +++ b/agentkit/skills/.shared/scripts/run-state.sh @@ -234,6 +234,8 @@ print_summary() { positive_ids("queued"; true) as $queued | positive_ids("receipt_prs"; true) as $receipts | positive_ids("skipped_prs"; true) as $skipped | + has("auto_review") as $has_auto_review | + .auto_review as $auto_review | has("root_turns") as $has_root_turns | has("first_completion") as $has_first_completion | .root_turns as $root_turns | @@ -242,6 +244,8 @@ print_summary() { elif (($skipped - $prs) | length) > 0 then error("skipped_prs must be a subset of opened_prs") elif (($receipts + $skipped | length) != ($receipts + $skipped | unique | length)) then error("receipt_prs and skipped_prs must be disjoint") + elif $has_auto_review and ($auto_review | type) != "boolean" + then error("auto_review must be a boolean") elif $has_root_turns and (($has_first_completion | not) or ($root_turns | type) != "array" or any($root_turns[]; . != true) or ($first_completion | type) != "boolean") then error("invalid root-turn summary evidence") @@ -249,7 +253,10 @@ print_summary() { then error("invalid first-completion evidence") else (if $has_root_turns | not then "unavailable" elif $first_completion then ($root_turns | length | tostring) else "unlatched" end) as $telemetry | - [($prs | length), ($receipts | length), ($skipped | length), ($queued | length), $telemetry] | @tsv end + ($prs - ($receipts + $skipped)) as $missing | + [($prs | length), ($receipts | length), ($skipped | length), ($queued | length), + (if $has_auto_review then ($auto_review | tostring) else "false" end), + ($missing | if length == 0 then "-" else map(tostring) | join(",") end), $telemetry] | @tsv end ' <<<"$STATE" 2>/dev/null) || die 'summary state requires valid opened_prs, queued, receipt_prs, and skipped_prs collections' [[ ! -L $LEDGER && -f $LEDGER && -r $LEDGER && -O $LEDGER ]] || @@ -271,22 +278,35 @@ print_summary() { then . else error("invalid handback evidence") end] | sort_by(.issue) ' "$LEDGER" 2>/dev/null) || die "unparseable active-workers evidence: $LEDGER" parked_count=$(jq 'length' <<<"$parked_rows") - local prs receipts skipped queued root_turns - IFS=$'\t' read -r prs receipts skipped queued root_turns <<<"$counts" + local prs receipts skipped queued auto_review missing_review_prs root_turns review_resume coverage_failure='' + IFS=$'\t' read -r prs receipts skipped queued auto_review missing_review_prs root_turns <<<"$counts" + if [[ $auto_review == true && $missing_review_prs != - ]]; then + review_resume="/review-remote-pr --auto-review ${missing_review_prs//,/; /review-remote-pr --auto-review }" + coverage_failure="auto-review coverage missing for PRs: $missing_review_prs; resume: $review_resume" + fi printf 'coverage= prs=%s receipts=%s skipped=%s parked=%s queued=%s root-turns-before-first-completion=%s\n' \ "$prs" "$receipts" "$skipped" "$parked_count" "$queued" "$root_turns" jq -r '.[] | "blocked=\(.issue):\(.evidence)"' <<<"$parked_rows" - [[ -n $REPORTS_DIR ]] || return 0 + if [[ -z $REPORTS_DIR ]]; then + [[ -z $coverage_failure ]] || die "$coverage_failure" + return 0 + fi [[ ! -L $REPORTS_DIR ]] || die "verification reports directory must not be a symlink: $REPORTS_DIR" [[ ! -e $REPORTS_DIR || (-d $REPORTS_DIR && -O $REPORTS_DIR) ]] || die "verification reports must be an owned directory: $REPORTS_DIR" - [[ -e $REPORTS_DIR ]] || return 0 + if [[ ! -e $REPORTS_DIR ]]; then + [[ -z $coverage_failure ]] || die "$coverage_failure" + return 0 + fi local reports_mode report report_mode report_text report_issue content_issue reports_mode=$(stat -c %a -- "$REPORTS_DIR") || die "could not inspect verification reports: $REPORTS_DIR" (( (8#$reports_mode & 8#077) == 0 )) || die "verification reports directory must be owner-private: $REPORTS_DIR" local -a reports=("$REPORTS_DIR"/issue-*.report) - [[ -e ${reports[0]} ]] || return 0 + if [[ ! -e ${reports[0]} ]]; then + [[ -z $coverage_failure ]] || die "$coverage_failure" + return 0 + fi for report in "${reports[@]}"; do [[ ${report##*/} =~ ^issue-([1-9][0-9]*)\.report$ ]] || die "verification report filename must be issue-POSITIVE_INTEGER.report: $report" @@ -304,6 +324,7 @@ print_summary() { die "verification report filename issue does not match content issue: $report" cat -- "$report" done + [[ -z $coverage_failure ]] || die "$coverage_failure" } main() { diff --git a/agentkit/skills/parallel-issues/SKILL.md b/agentkit/skills/parallel-issues/SKILL.md index 66251cd1..0d29bbf7 100755 --- a/agentkit/skills/parallel-issues/SKILL.md +++ b/agentkit/skills/parallel-issues/SKILL.md @@ -117,7 +117,7 @@ The scope, flags, repository, and base are fixed before the first receipt and su changes after compaction/resume: `scope=57,54` and `scope=57,62` cannot share an ID, nor can `auto-review=false` and `auto-review=true`; the same exact tuple may intentionally resume. Reuse this `RUN_ID` for all issues; never use a worker-local value. Immediately append each grant, steer, or board adjudication with `printf '%s' "$QUOTE" | "$agentkit/.shared/scripts/session-ledger.sh" append --ledger "$LEDGER" --run-id "$RUN_ID" --skills-path "$agentkit" --procedure-set parallel-issues --decision "$DECISION" --scope "$SCOPE" --quote-stdin || exit 1`. -After establishing `RUN_ID`, run `"$agentkit/.shared/scripts/run-state.sh" init-summary --run-id "$RUN_ID" --repo-root "$repository_root"`; it preserves existing records. +After establishing `RUN_ID`, run `"$agentkit/.shared/scripts/run-state.sh" init-summary --run-id "$RUN_ID" --repo-root "$repository_root"`; it preserves existing records. Persist the fixed invocation fact, never an unset shell default: when the invocation carried `--auto-review`, run `"$agentkit/.shared/scripts/run-state.sh" set --run-id "$RUN_ID" --repo-root "$repository_root" --path auto_review --json true`; otherwise run `"$agentkit/.shared/scripts/run-state.sh" set --run-id "$RUN_ID" --repo-root "$repository_root" --path auto_review --json false`. The handoff summary can then enforce review coverage after compaction. `QUOTE` is the human's verbatim quote; never put secrets or credentials in any field. After any compaction/resume, before taking another action, run `"$agentkit/.shared/scripts/session-ledger.sh" read --ledger "$LEDGER" --run-id "$RUN_ID"` and treat its output as the durable decision state. @@ -510,7 +510,7 @@ Composer publishes once; root installs and verifies its hashed `uncoveredVerific ### Collect (per-completion — never wait for the slowest issue) `worker-result=PATH` uses the [result contract](references/worker-prompts.md#structured-result-contract): validate dispatch, ownership, Git and logs before accepting. Keep root CI/review obligations; unknown or blocked evidence is never green; unchanged accepted receipts resume without repeated work. Text fallbacks stay unknown. -`agentkit activation-blocked: {...}` keeps ownership. Validate worker, worktree and workflow, then follow `.shared/spawn-contract.md` once to redeliver current bytes to the same context. The leaf acknowledges and resumes; unavailable or repeated delivery parks with work preserved. +`agentkit activation-blocked: {...}` keeps ownership. Validate worker, worktree and workflow, then follow `.shared/spawn-contract.md` once to redeliver current bytes to the same context. The leaf acknowledges and resumes; unavailable or repeated delivery parks with work preserved. Before either PR-open path, including after a resumed Collect, restore the fixed invocation fact with `auto_review_state=$("$agentkit/.shared/scripts/run-state.sh" get --run-id "$RUN_ID" --repo-root "$repository_root" --path auto_review) || exit 1`; validate it with `case $auto_review_state in true|false) ;; *) printf 'invalid durable auto_review: %s\n' "$auto_review_state" >&2; exit 1 ;; esac`. - **Cross-write check first** → run the root-checkout Collect check against the immutable dispatch snapshot before trusting the worker's handback. Keep the helper's incident line, @@ -518,8 +518,8 @@ Composer publishes once; root installs and verifies its hashed `uncoveredVerific worker's evidence. A dirty path is never an "unrelated local change" until the check proves otherwise. -- **Completion report (branch + pushed SHA)** → review pushed diff; run `$agentkit/parallel-issues/scripts/compose-pr-body.sh`, then `"$agentkit/.shared/scripts/gh-body.sh" pr create --draft`; record with `"$agentkit/.shared/scripts/run-state.sh" record-summary --run-id "$RUN_ID" --repo-root "$repository_root" --path opened_prs --json "$pr"`; move issue to `In review`; start Phase 3. Diff size is never a reason to withhold this; see Diff-size facts. -- **BLOCKED** → preserve the text handback, set `blocker_file="$worktree/.agent/logs/partial-blockers.list"`, and run `"$agentkit/.shared/scripts/validate-handback.sh" --classify-completion --worktree "$worktree" --handback-file "$completion_file" --blocker-file "$blocker_file"`. `disposition=partial-pushed pr=open blocker-file=written verification=unbound` proves the queried remote HEAD, not log attribution: review the diff, open the draft, record it with `"$agentkit/.shared/scripts/run-state.sh" record-summary --run-id "$RUN_ID" --repo-root "$repository_root" --path opened_prs --json "$pr"`, and pass `--blocker-file "$blocker_file"` to `$agentkit/parallel-issues/scripts/compose-pr-body.sh`; its `## Operator action required` section preserves blocker paths and discloses limited verification. Dispatch chained successors from the pushed SHA on both completion paths. Otherwise gate redrive on `"$agentkit/.shared/scripts/run-state.sh" get --run-id "$RUN_ID" --path redrive.` and proceed only on exit 11 (absent); clear the blocker (`write-set`: widen the fence, recheck every active worker); only after the blocker clears, run one `tools.send`, then record `"$agentkit/.shared/scripts/run-state.sh" set --run-id "$RUN_ID" --path redrive.`. If the same lead is unavailable, give a fresh lead the exact resume command; other blockers park. `baseline-red` gets one automatic re-drive. A sole `needs-paths: [,...]` drives that recheck; otherwise preserve the worktree and blocker evidence. +- **Completion report (branch + pushed SHA)** → review pushed diff; run `$agentkit/parallel-issues/scripts/compose-pr-body.sh`, then `"$agentkit/.shared/scripts/gh-body.sh" pr create --draft`; record with `"$agentkit/.shared/scripts/run-state.sh" record-summary --run-id "$RUN_ID" --repo-root "$repository_root" --path opened_prs --json "$pr"`; print `printf 'next: dispatch draft-phase loop for #%s (Step 3a); auto-review=%s\n' "$pr" "$auto_review_state"`; move issue to `In review`; start Phase 3. Diff size is never a reason to withhold this; see Diff-size facts. +- **BLOCKED** → preserve the text handback, set `blocker_file="$worktree/.agent/logs/partial-blockers.list"`, and run `"$agentkit/.shared/scripts/validate-handback.sh" --classify-completion --worktree "$worktree" --handback-file "$completion_file" --blocker-file "$blocker_file"`. `disposition=partial-pushed pr=open blocker-file=written verification=unbound` proves the queried remote HEAD, not log attribution: review the diff, open the draft, record it with `"$agentkit/.shared/scripts/run-state.sh" record-summary --run-id "$RUN_ID" --repo-root "$repository_root" --path opened_prs --json "$pr"`, print `printf 'next: dispatch draft-phase loop for #%s (Step 3a); auto-review=%s\n' "$pr" "$auto_review_state"`, and pass `--blocker-file "$blocker_file"` to `$agentkit/parallel-issues/scripts/compose-pr-body.sh`; its `## Operator action required` section preserves blocker paths and discloses limited verification. Dispatch chained successors from the pushed SHA on both completion paths. Otherwise gate redrive on `"$agentkit/.shared/scripts/run-state.sh" get --run-id "$RUN_ID" --path redrive.` and proceed only on exit 11 (absent); clear the blocker (`write-set`: widen the fence, recheck every active worker); only after the blocker clears, run one `tools.send`, then record `"$agentkit/.shared/scripts/run-state.sh" set --run-id "$RUN_ID" --path redrive.`. If the same lead is unavailable, give a fresh lead the exact resume command; other blockers park. `baseline-red` gets one automatic re-drive. A sole `needs-paths: [,...]` drives that recheck; otherwise preserve the worktree and blocker evidence. - **Queued issue** → spawn it immediately into the freed slot. **Stall detection:** record the next check at last progress + `STALL_THRESHOLD_MINUTES` (default 12 minutes). Before the threshold elapses, do not call diff --git a/agentkit/skills/parallel-issues/references/worker-prompts.md b/agentkit/skills/parallel-issues/references/worker-prompts.md index 7a834889..7d550db8 100644 --- a/agentkit/skills/parallel-issues/references/worker-prompts.md +++ b/agentkit/skills/parallel-issues/references/worker-prompts.md @@ -544,12 +544,13 @@ metadata, comments, replies, board moves, ready-flips — stays with the root. silent. 3. Follow this composed verification runbook: __VERIFY_RUNBOOK__ - Run every focused and full verification command through `agent-run.sh`; retain the fresh - green marker-bearing log path and do not rerun a failed command outside the wrapper. -4. When verification is green, commit with `"$shared/worktree-commit.sh"` (explicit file + Run every verification command through `agent-run.sh`; use focused checks during TDD, but + commit the repair before the final unfocused run. Do not rerun a failed command outside the wrapper. +4. When focused verification is green, commit with `"$shared/worktree-commit.sh"` (explicit file operands, Conventional Commit subject, the expanded `--trailer "$worker_attribution"` - -- or omitted, letting the helper derive it from the contract), then - push the branch. If unrelated dirt appears, stop and surface its files, diffstat, and + -- or omitted, letting the helper derive it from the contract). Run the unfocused full command + through `agent-run.sh` on that clean commit, retain its green marker-bearing log, and + push the branch only after that clean committed-HEAD run passes. If unrelated dirt appears, stop and surface its files, diffstat, and whether the checkpoint manifest explains it — never commit it. 5. Return a completion report: branch, full commit SHA from the helper's success line, diffstat, and the green verification log path. If the helper exits 2 (nothing diff --git a/agentkit/skills/pr-to-green/scripts/review-transition.sh b/agentkit/skills/pr-to-green/scripts/review-transition.sh index 3a2f69c5..7affb98e 100755 --- a/agentkit/skills/pr-to-green/scripts/review-transition.sh +++ b/agentkit/skills/pr-to-green/scripts/review-transition.sh @@ -305,9 +305,11 @@ trap cleanup EXIT HUP INT TERM [[ -s $work_dir/providers.txt ]] || die 'provider capability resolver returned an empty plan' while IFS= read -r line; do - if [[ $line =~ ^provider=([a-z0-9-]+)[[:space:]]mode=([a-z-]+)[[:space:]]source=([a-z]+)$ ]]; then + lane='' + if [[ $line =~ ^provider=([a-z0-9-]+)[[:space:]]mode=([a-z-]+)[[:space:]]source=([a-z]+)([[:space:]]lane=([a-z-]+))?$ ]]; then provider=${BASH_REMATCH[1]} mode=${BASH_REMATCH[2]} + lane=${BASH_REMATCH[5]-} else die 'provider capability resolver returned a malformed record' fi @@ -316,6 +318,9 @@ while IFS= read -r line; do die "provider capability plan contains unsupported provider: $provider" [[ $mode == "$expected_mode" ]] || die "provider capability plan contains unsupported capability: $provider:$mode" + expected_lane=$(review_provider_lane "$provider" 2>/dev/null || true) + [[ $lane == "$expected_lane" ]] || + die "provider capability plan contains unsupported lane: $provider:${lane:-none}" providers+=("$provider") modes[$provider]=$mode done <"$work_dir/providers.txt" diff --git a/agentkit/skills/review-remote-pr/SKILL.md b/agentkit/skills/review-remote-pr/SKILL.md index 306f1afc..442ab9e5 100755 --- a/agentkit/skills/review-remote-pr/SKILL.md +++ b/agentkit/skills/review-remote-pr/SKILL.md @@ -335,24 +335,25 @@ failure; stop with evidence unavailable. A receipt marker is authoritative from ## Step 2: Fix CI Failures -**Step 1c — batch pushes:** review behavior after a push is provider configuration, not a -workflow guarantee — still batch each cycle's fixes into **one** push; never post -`@coderabbitai pause`/`resume`. +**Step 1c — batch pushes:** batch each cycle's fixes into **one** push; never post `@coderabbitai pause`/`resume`. Provider behavior after a push is configuration, not a guarantee. -Diagnose the causal failure (`gh run view --log-failed "$run_id" | grep -E "FAIL|error|Error"`, -run ID from the `gh pr checks` URL column), then run the **Implementation-worker gate** above. -The worker verifies independently before its cycle push, through `agent-run.sh`: +Diagnose the causal failure from the run ID in `gh pr checks`, then run the **Implementation-worker gate** above; the worker verifies before its cycle push: ```bash [ -d "${agentkit:-}/.shared/scripts" ] && [ "${agentkit_provenance:-}" = ok ] || { printf "%s\n" "agentkit unresolved: prepend THE CACHE REHYDRATION block" >&2; exit 1; } agent_run="$agentkit/.shared/scripts/agent-run.sh" -"$agent_run" --cmd lint --if-declared --cmd test +"$agent_run" --cmd lint --if-declared ``` -For red/green iterations the worker uses `"$agent_run" --cmd test --only NAME[,NAME...]` (forwards through the -repo's `AGENT_CMD_TEST_FOCUS` declaration); after the final tree change, the worker must run the unfocused `"$agent_run" --cmd test` once for the full-suite verdict -before worker publication. A successful run prints one `PASS:` line; a failure prints `FAIL(rc=N):`, -context, `note:` lines, matched errors, and the log path. **Never push without local verification passing** — on `FAIL`, having set `check`, `log`, and `failing_paths` from its output: +Commit the repair through the worker gate. After the worker-gate commit and before push, run the full test on clean committed HEAD: + +```bash +[ -d "${agentkit:-}/.shared/scripts" ] && [ "${agentkit_provenance:-}" = ok ] || { printf "%s\n" "agentkit unresolved: prepend THE CACHE REHYDRATION block" >&2; exit 1; } +agent_run="$agentkit/.shared/scripts/agent-run.sh" +"$agent_run" --cmd test +``` + +For red/green iterations use `"$agent_run" --cmd test --only NAME[,NAME...]` through `AGENT_CMD_TEST_FOCUS`. After the final edit, commit, then run the unfocused `"$agent_run" --cmd test` once on clean committed HEAD for the full-suite verdict; push only after `PASS:`. On `FAIL`, having set `check`, `log`, and `failing_paths` from its output: ```bash [ -d "${agentkit:-}/.shared/scripts" ] && [ "${agentkit_provenance:-}" = ok ] || { printf "%s\n" "agentkit unresolved: prepend THE CACHE REHYDRATION block" >&2; exit 1; } diff --git a/agentkit/skills/review-remote-pr/references/adversarial-review.md b/agentkit/skills/review-remote-pr/references/adversarial-review.md index f26b01fc..8a2ffec6 100644 --- a/agentkit/skills/review-remote-pr/references/adversarial-review.md +++ b/agentkit/skills/review-remote-pr/references/adversarial-review.md @@ -445,7 +445,8 @@ covered rather than `stale` with zero additional review spends — see Update the same title after repair. Produce its evidence with `finding-ledger.sh evidence --title TITLE --path AFFECTED_PATH --log GREEN_LOG --repo-root WORKTREE --repair-sha REPAIR_SHA > FILE`: the log must be the green, unfocused `agent-run.sh --cmd test` run (a focused `--only` or red log is -refused), `--head` defaults to the checkout's HEAD, and `REPAIR_SHA` is the commit that changed that +refused) made after the repair commit and before push. Its header binds the tested head and tracked-tree cleanliness; +that head must be the checkout's clean current HEAD. `REPAIR_SHA` is the commit that changed the path (not a later formatting-only commit). Then record it with `add --verdict fixed --sha "$(jq -r .repairSha FILE)" --evidence FILE --repo-root WORKTREE --head CURRENT_SHA` (also supply title and severity). One evidence file per finding. The helper checks commit ancestry, diff --git a/agentkit/skills/review-remote-pr/references/worker-gate.md b/agentkit/skills/review-remote-pr/references/worker-gate.md index 7e6f8682..84bf109b 100644 --- a/agentkit/skills/review-remote-pr/references/worker-gate.md +++ b/agentkit/skills/review-remote-pr/references/worker-gate.md @@ -52,11 +52,11 @@ grep -Fq -- '--cmd test' "$repair_prompt" || exit 1 ## Worker-owned publication -Workers commit and push their own branch after focused/full verification and a completion report -(branch, full SHA, diffstat, and the path of the green unfocused `agent-run.sh --cmd test` log); -`worktree-commit.sh` uses explicit files and trailer. Root turns that log into repair evidence with -`finding-ledger.sh evidence`, which refuses a focused or red log -- send the worker back to verify -rather than accepting the handback. +Workers commit and push their own branch. Between those actions, run unfocused `agent-run.sh --cmd test` +after the repair commit and before push. Use focused checks while editing; commit via `worktree-commit.sh` +with explicit files and a trailer; push only after the clean committed HEAD passes. Return a completion report +with branch, full SHA, diffstat, and green log. Root passes it to `finding-ledger.sh evidence`, which refuses +focused, red, dirty, unbound, or different-HEAD logs; send the worker back to verify. The root owns the pushed `base...HEAD` review, PR metadata, board, replies, and next cycle. ## Environment-refusal fallback diff --git a/agentkit/skills/review-remote-pr/scripts/finding-ledger.sh b/agentkit/skills/review-remote-pr/scripts/finding-ledger.sh index d0612e47..1856ef16 100755 --- a/agentkit/skills/review-remote-pr/scripts/finding-ledger.sh +++ b/agentkit/skills/review-remote-pr/scripts/finding-ledger.sh @@ -7,6 +7,7 @@ readonly PROGNAME=${0##*/} readonly RECEIPT_MARKER='' readonly DOC_MARKER='' readonly SHA_RE='^[[:xdigit:]]{7,64}(,[[:xdigit:]]{7,64})*$' +readonly FULL_SHA_RE='^([[:xdigit:]]{40}|[[:xdigit:]]{64})$' readonly ORDER_RC=13 readonly FINDING_SLUG_JQ='ascii_downcase | gsub("[^a-z0-9]+"; "-") | ltrimstr("-") | rtrimstr("-") | if . == "" then "finding" else . end' SCRIPT_DIR=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd -P) @@ -32,7 +33,8 @@ Usage: $PROGNAME add --title TITLE --severity P1|P2 --verdict fixed --sha SHA $PROGNAME ids --file FILE (prints IDTITLE; review-ledger.sh cover --reason fix:ID names one) $PROGNAME evidence --title T --path P --log LOG --repo-root DIR --repair-sha SHA [--head SHA] (prints fixed-verdict evidence JSON; LOG must be a green unfocused agent-run.sh --cmd test - log; head defaults to DIR's HEAD; SHA is the commit that changed P) + log run on DIR's clean committed HEAD; head defaults to that HEAD; SHA is the + commit that changed P) Terminal evidence: --evidence FILE --repo-root DIR --head SHA. Evidence JSON binds finding (title) to decision rejected|accepted-risk and rationale, or to @@ -361,7 +363,7 @@ validate_repairs() { log=$(jq -r .evidence.log <<<"$row") digest=$(jq -r .evidence.logSha256 <<<"$row") command=$(jq -r .evidence.command <<<"$row") - [[ $head =~ ^[0-9a-f]{40}$ && $sha =~ ^[0-9a-f]{40}$ && $tested =~ ^[0-9a-f]{40}$ && $digest =~ ^[0-9a-f]{64}$ ]] || + [[ $head =~ $FULL_SHA_RE && $sha =~ $FULL_SHA_RE && $tested =~ $FULL_SHA_RE && $digest =~ ^[0-9a-f]{64}$ ]] || die_evidence 'repair evidence requires full commit and log hashes' if ! git -C "$root" merge-base --is-ancestor "$sha" "$tested" 2>/dev/null || ! git -C "$root" merge-base --is-ancestor "$tested" "$head" 2>/dev/null; then @@ -441,11 +443,26 @@ resolve_commit() { git -C "$1" rev-parse --verify -q "$2^{commit}" 2>/dev/null || die_evidence "not a commit in $1: $2" } +require_tested_head() { + local log=$1 current=$2 header tested clean + header=$(sed -n '2p' "$log") + if [[ $header =~ ' head='([[:xdigit:]]{40}|[[:xdigit:]]{64})' tracked-clean='(yes|no)$ ]]; then + tested=${BASH_REMATCH[1]} + clean=${BASH_REMATCH[2]} + else + die_evidence 'verification log has no tested-head metadata; commit the repair, then run agent-run.sh --cmd test' + fi + [[ $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' +} + # Emit fixed-verdict evidence for one finding, refusing anything add would # later reject: the log must be the green, unfocused declared test run, and the # named repair commit must change the finding's path. cmd_evidence() { - local title='' path='' log='' root='' repair_sha='' head='' declared command digest row + local title='' path='' log='' root='' repair_sha='' head='' actual_head='' declared command digest row shift while (($#)); do case $1 in @@ -464,7 +481,11 @@ cmd_evidence() { [[ $path != /* && $path != -* && $path != *'..'* ]] || die_evidence 'repair path must be repository relative' [[ -f $log && ! -L $log ]] || die_evidence "verification log is unavailable: $log" log=$(cd -- "$(dirname -- "$log")" && pwd -P)/${log##*/} - head=$(resolve_commit "$root" "${head:-HEAD}") && repair_sha=$(resolve_commit "$root" "$repair_sha") || exit 1 + head=$(resolve_commit "$root" "${head:-HEAD}") && + actual_head=$(resolve_commit "$root" HEAD) && + repair_sha=$(resolve_commit "$root" "$repair_sha") || exit 1 + [[ $head == "$actual_head" ]] || + die_evidence "evidence head $head is not the current head $actual_head" 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' @@ -472,6 +493,9 @@ cmd_evidence() { [[ -n $declared ]] || die_evidence 'the repository declares no AGENT_CMD_TEST' [[ -n $command && $command == "$declared" ]] || die_evidence "log is not the unfocused declared test run (log: ${command:-}; declared: $declared); run agent-run.sh --cmd test without --only" + [[ $(tail -n 1 -- "$log") == '=== agent-run exited rc=0 '* ]] || + die_evidence 'verification log has no final successful agent-run result' + require_tested_head "$log" "$head" digest=$(verification_digest "$log") || die_evidence 'verification log digest unavailable (requires sha256sum or shasum)' row=$(jq -cn --arg finding "$title" --arg sha "$repair_sha" --arg head "$head" \ --arg path "$path" --arg command "$command" --arg log "$log" --arg digest "${digest%% *}" \ diff --git a/opencode/package-lock.json b/opencode/package-lock.json index 7b62caf6..cb349ad8 100644 --- a/opencode/package-lock.json +++ b/opencode/package-lock.json @@ -1,12 +1,12 @@ { "name": "@wrzonance/agentkit-opencode", - "version": "0.9.11", + "version": "0.9.12", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@wrzonance/agentkit-opencode", - "version": "0.9.11", + "version": "0.9.12", "license": "MIT", "devDependencies": { "@opencode-ai/plugin": "1.18.18" diff --git a/opencode/package.json b/opencode/package.json index 3f9f077c..c47d56bf 100644 --- a/opencode/package.json +++ b/opencode/package.json @@ -1,6 +1,6 @@ { "name": "@wrzonance/agentkit-opencode", - "version": "0.9.11", + "version": "0.9.12", "description": "Agent Kit plugin for OpenCode CLI: injects the environment contract into the model's system prompt at session start.", "type": "module", "main": "./index.js", diff --git a/plugin/agentkit/.claude-plugin/plugin.json b/plugin/agentkit/.claude-plugin/plugin.json index 9ea636ca..8ec78423 100644 --- a/plugin/agentkit/.claude-plugin/plugin.json +++ b/plugin/agentkit/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "agentkit", - "version": "0.9.11", + "version": "0.9.12", "description": "Board-aware parallel issue and PR review skills, with lifecycle hooks and a per-repository contract.", "author": { "name": "wrzonance", diff --git a/plugin/agentkit/.codex-plugin/plugin.json b/plugin/agentkit/.codex-plugin/plugin.json index 57947170..fe92f129 100644 --- a/plugin/agentkit/.codex-plugin/plugin.json +++ b/plugin/agentkit/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "agentkit", - "version": "0.9.11", + "version": "0.9.12", "description": "Board-aware parallel issue and PR review skills, with lifecycle hooks and a per-repository contract.", "author": { "name": "wrzonance", diff --git a/plugin/opencode/package.json b/plugin/opencode/package.json index 3f9f077c..c47d56bf 100644 --- a/plugin/opencode/package.json +++ b/plugin/opencode/package.json @@ -1,6 +1,6 @@ { "name": "@wrzonance/agentkit-opencode", - "version": "0.9.11", + "version": "0.9.12", "description": "Agent Kit plugin for OpenCode CLI: injects the environment contract into the model's system prompt at session start.", "type": "module", "main": "./index.js", diff --git a/tests/lint-helper-size.sh b/tests/lint-helper-size.sh index 9ebc983a..8f07f61e 100755 --- a/tests/lint-helper-size.sh +++ b/tests/lint-helper-size.sh @@ -26,12 +26,15 @@ declare -A KNOWN_OVERSIZE=( # #777: complete Step 0 recipe moved from injected prose into --help. # #777 absolute-path guard + #778 harness-bound runtime-tool record. [skills/.shared/scripts/agent-preflight.sh]="1401:17157:800" - # #731/#732/#776/#809: records, summaries, and truthful reuse diagnostics. - [skills/.shared/scripts/agent-run.sh]="1920:20761:800" + # #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" # #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. - [skills/.shared/scripts/repo-config.sh]="1154:12008:800" + # #876 review repair: reject normalized aliases of built-in provider logins. + [skills/.shared/scripts/repo-config.sh]="1165:12203:800" [skills/.shared/scripts/worktree-commit.sh]="847:8772:800" # #852: distinguish configured missing scans at the retarget boundary. [skills/parallel-issues/scripts/chain-advance.sh]="1086:13108:800" @@ -183,7 +186,17 @@ readonly MAX_HELPER_TOKENS=10000 # #834 review repair: exact combined helper tree measurement. # #865 reference-use clauses and bounded no-run diagnostics: 1,891,483 bytes / 4. # #873 (trimmed): IDs, evidence producer, cover preconditions: 1,898,236 bytes / 4. -readonly MAX_TREE_TOKENS=474559 +# #875 review: coverage failures preserve handoff evidence across every report path. +# #874 PR #877: portable status and bounded active-run lease inheritance. +# #873 chain: tested-head metadata and repair handback validation. +# #873 review repair: real SHA-256 evidence fixture and split verification recipe. +# #873/#874 + #875 final merge-down: exact combined helper tree measurement. +# #874 review repair: stable identity and symlink-safe log selection. +# #874 review + #875 merge: exact combined helper tree measurement. +# #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 violations=0 checked=0 diff --git a/tests/lint-skill-size.sh b/tests/lint-skill-size.sh index acc9a74c..e6beaef7 100755 --- a/tests/lint-skill-size.sh +++ b/tests/lint-skill-size.sh @@ -46,7 +46,10 @@ declare -A KNOWN_OVERSIZE=( # #845 + #865: state the cold-start/reference-use boundary at workflow activation. # #873: Step 0 names each helper's flags separately, with a canonical preflight path. # #873 review: preflight's flag set includes --activation-origin. - [parallel-issues]="761:16265:500" + # #875 review: preserve summary evidence and persist an invocation-derived review mode. + # #875 follow-up: Collect restores that durable review mode in resumed shells. + # #875 integration: retain the inline restore across the final predecessor merge. + [parallel-issues]="772:16666:500" ) readonly MAX_BODY_LINES=500 diff --git a/tests/test-agent-run-cmd.sh b/tests/test-agent-run-cmd.sh index 6ec82364..3e979c0b 100755 --- a/tests/test-agent-run-cmd.sh +++ b/tests/test-agent-run-cmd.sh @@ -394,7 +394,29 @@ assert_contains "$log" '=== agent-run echo hello' 'the log names the command it assert_contains "$log" '=== started' 'and when it started' assert_contains "$log" 'concurrent-suites=1' 'the log records the active full-suite count' assert_contains "$log" '=== agent-run exited rc=0' 'and terminates with the verdict' -assert_contains "$out" 'has NOT finished' 'and the caller is told what an unterminated log means' +assert_contains "$out" 'resume this same call; never relaunch' \ + 'and the caller is told how to continue an unterminated run' + +# Issue #873: a repair handback must prove which committed head the full run +# 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/\n' >"$bound_repo/.gitignore" +printf 'a\n' >"$bound_repo/tracked.txt" +git -C "$bound_repo" add .gitignore tracked.txt +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 '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) +log=$(cat "$bound_repo"/.agent/logs/*-ok.log) +assert_contains "$log" "head=$bound_sha tracked-clean=no" \ + 'the log header records uncommitted tracked changes' # 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' @@ -414,12 +436,19 @@ cat >"$marker_repo/bin/awk" <<'EOF' #!/usr/bin/env bash set -uo pipefail if [[ ${1:-} == '{print $22}' ]]; then - printf 'alive\n' + exit 1 else exec /usr/bin/awk "$@" fi EOF chmod +x -- "$marker_repo/bin/awk" +cat >"$marker_repo/bin/ps" <<'EOF' +#!/usr/bin/env bash +set -uo pipefail +[[ ${*: -1} != 999999 ]] || exit 1 +printf 'Tue Sep 22 12:34:56 2026\n' +EOF +chmod +x -- "$marker_repo/bin/ps" cat >"$marker_repo/bin/mktemp" <<'EOF' #!/usr/bin/env bash set -uo pipefail @@ -438,9 +467,12 @@ printf '999999 1\n' >"$marker_suite_dir/agent-run.stale" marker_out=$(cd "$marker_repo" && PATH="$marker_repo/bin:$PATH" \ TMPDIR="$tmp" "$real_run_sh" --force --cmd test 2>&1) marker_log=$(find "$marker_repo/.agent/logs" -type f -name '*-test.log' -print -quit) -assert_contains "$marker_out" 'marker-ok' 'a no-proc marker accepts the alive fallback' +assert_contains "$marker_out" 'PASS: printf marker-ok' \ + 'a failed proc read uses a stable process-start fallback' assert_not_contains "$marker_out" 'marker template is not BSD-compatible' \ 'the active marker uses a BSD-compatible trailing-X template' +assert_not_contains "$(cat "$marker_log")" 'process-start=alive' \ + 'the process identity never degrades to a liveness sentinel' assert_contains "$(cat "$marker_log")" 'concurrent-suites=1' \ 'a stale new-format marker is removed before counting active suites' @@ -740,7 +772,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 1920 ]] && printf yes || printf no)" \ - 'agent-run.sh stays at or under 1920 lines (#809 reuse diagnostics)' +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)' finish diff --git a/tests/test-agent-run-verification-cache.sh b/tests/test-agent-run-verification-cache.sh index 66309e9f..6f58e72c 100755 --- a/tests/test-agent-run-verification-cache.sh +++ b/tests/test-agent-run-verification-cache.sh @@ -564,15 +564,17 @@ printf 'AGENT_CMD_TEST=tools/run\nAGENT_VERIFY_TEST_MODE=local\nAGENT_VERIFY_TES printf '%s\n' '#!/bin/sh' 'echo run >> "$COUNT_FILE"' 'sleep 2' 'exit 0' > "$local_repo/tools/run" local_run > "$tmp/owner-output" & owner=$! for ((attempt=0; attempt<100; attempt++)); do - [[ -d $local_repo/.agent/verification-records ]] && - find "$local_repo/.agent/verification-records" -name running -print | grep -q . && break + running_record=$(find "$local_repo/.agent/verification-records" -name running -print -quit 2>/dev/null || true) + [[ -n $running_record ]] && grep -q '/.agent/logs/.*\.log$' "$running_record" && break sleep 0.05 done out=$(local_run); local_rc=$? -assert_eq '75' "$local_rc" 'running identical command returns non-success status' -assert_contains "$out" 'verification running: handle=' 'running identical command returns existing handle' -running_handle=$(printf '%s\n' "$out" | sed -n 's/^agent-run: verification running: handle=//p') +assert_eq '2' "$local_rc" 'running identical command is refused before it can duplicate work' +assert_contains "$out" 'already running:' 'running identical command returns the existing log' wait "$owner" +# An unrelated record proves later checks never rediscover the handle by directory order. +mkdir -p "$local_repo/.agent/verification-records/000-decoy" +running_handle=${running_record%/running} out=$(local_run) assert_contains "$out" 'verification current:' 'a completed concurrent owner is reusable' record=$running_handle/result diff --git a/tests/test-agent-run-yield.sh b/tests/test-agent-run-yield.sh new file mode 100755 index 00000000..f19c8737 --- /dev/null +++ b/tests/test-agent-run-yield.sh @@ -0,0 +1,174 @@ +#!/usr/bin/env bash +# Suite: yielded agent-run calls remain observable and cannot be duplicated. +set -uo pipefail + +TEST_NAME='agent-run-yield' +here=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) +root=$(dirname -- "$here") +# shellcheck source=lib/assert.sh +source "$here/lib/assert.sh" + +run_sh="$root/agentkit/skills/.shared/scripts/agent-run.sh" +tmp=$(mktemp -d) +trap 'rm -rf -- "$tmp"' EXIT + +help=$($run_sh --help) +assert_contains "$help" 'active duplicate exits 2' \ + 'help documents the accepted duplicate-run exit status' +assert_contains "$help" 'unknown abandoned handle exits 75' \ + 'help distinguishes abandoned verification evidence from a live duplicate' +assert_not_contains "$help" 'exit 2 is reserved' \ + 'help no longer reserves duplicate-run status for interpreter errors' +assert_contains "$help" 'in-flight command is still refused with its log' \ + '--force documentation does not promise a duplicate-run bypass' + +epoch_repo=$tmp/epoch-repo +git -C "$tmp" init -q epoch-repo +mkdir -p "$epoch_repo/.agent" "$epoch_repo/tools" +cat > "$epoch_repo/tools/check" <<'EOF' +#!/usr/bin/env bash +set -uo pipefail +printf 'started\n' > "${STARTED_FILE:?}" +for ((attempt=0; attempt<500; attempt++)); do + [[ -e ${RELEASE_FILE:?} ]] && exit 0 + sleep 0.02 +done +exit 1 +EOF +chmod +x -- "$epoch_repo/tools/check" +printf 'AGENT_CMD_TEST=tools/check\n' > "$epoch_repo/.agent/config.env" +printf 'unset EPOCHSECONDS\n' > "$tmp/unset-epoch" +BASH_ENV=$tmp/unset-epoch STARTED_FILE=$tmp/epoch-started RELEASE_FILE=$tmp/epoch-release \ + "$run_sh" --dir "$epoch_repo" --cmd test > "$tmp/epoch.out" 2>&1 & +epoch_owner=$! +for ((attempt=0; attempt<100; attempt++)); do + epoch_log=$(find "$epoch_repo/.agent/logs" -type f -name '*-test.log' -print -quit 2>/dev/null || true) + [[ -n ${epoch_log:-} && -f $tmp/epoch-started ]] && break + sleep 0.02 +done +epoch_status=$(BASH_ENV=$tmp/unset-epoch "$run_sh" status "$epoch_log" 2>&1) +assert_contains "$epoch_status" 'running pid=' \ + 'a live run remains observable when EPOCHSECONDS is unavailable' +printf 'release\n' > "$tmp/epoch-release" +wait "$epoch_owner" +assert_eq 0 "$?" 'a declared command runs when EPOCHSECONDS is unavailable' + +repo=$tmp/repo +git -C "$tmp" init -q repo +mkdir -p "$repo/.agent" "$repo/tools" +cat > "$repo/tools/slow-check" <<'EOF' +#!/usr/bin/env bash +set -uo pipefail +printf 'started\n' > "${STARTED_FILE:?}" +for ((attempt=0; attempt<500; attempt++)); do + [[ -e ${RELEASE_FILE:?} ]] && { printf 'finished\n'; exit 0; } + sleep 0.02 +done +exit 1 +EOF +chmod +x -- "$repo/tools/slow-check" +printf 'AGENT_CMD_TEST=tools/slow-check\n' > "$repo/.agent/config.env" + +owner_out=$tmp/owner.out +STARTED_FILE=$tmp/started RELEASE_FILE=$tmp/release \ + "$run_sh" --dir "$repo" --cmd test > "$owner_out" 2>&1 & +owner=$! +for ((attempt=0; attempt<100; attempt++)); do + log=$(find "$repo/.agent/logs" -type f -name '*-test.log' -print -quit 2>/dev/null || true) + [[ -n ${log:-} && -f $tmp/started ]] && break + sleep 0.02 +done + +assert_contains "$(tail -n 1 -- "$owner_out")" 'resume this same call; never relaunch' \ + 'the final pre-block line tells a yielded caller to resume the same call' +status=$($run_sh status "$log" 2>&1) +assert_contains "$status" 'running pid=' 'status identifies an unfinished live run' +assert_contains "$status" 'elapsed=' 'running status includes elapsed seconds' + +duplicate='' +duplicate_rc=0 +duplicate=$(STARTED_FILE=$tmp/duplicate RELEASE_FILE=$tmp/release \ + "$run_sh" --dir "$repo" --cmd test 2>&1) || duplicate_rc=$? +assert_eq 2 "$duplicate_rc" 'an identical active launch is refused as usage' +assert_contains "$duplicate" "already running: $log" 'duplicate refusal names the original log' +assert_eq no "$([[ -e $tmp/duplicate ]] && printf yes || printf no)" \ + 'the refused duplicate never starts the declared command' + +printf 'release\n' > "$tmp/release" +wait "$owner" +assert_eq pass "$($run_sh status "$log")" 'status reports pass after the owner completes' +assert_eq pass "$(TMPDIR=$tmp/nonexistent-fallback "$run_sh" status "$log")" \ + 'repository-log status does not require a fallback directory' + +printf 'AGENT_CMD_TEST=false\n' > "$repo/.agent/config.env" +"$run_sh" --dir "$repo" --label failing --cmd test > /dev/null 2>&1 || true +fail_log=$(find "$repo/.agent/logs" -type f -name '*-failing.log' -print -quit) +assert_eq 'fail rc=1' "$($run_sh status "$fail_log")" 'status preserves a terminal failure code' + +background_repo=$tmp/background-repo +git -C "$tmp" init -q background-repo +mkdir -p "$background_repo/.agent" "$background_repo/tools" +cat > "$background_repo/tools/check" <<'EOF' +#!/usr/bin/env bash +set -uo pipefail +printf 'run\n' >> "${COUNT_FILE:?}" +( + for ((attempt=0; attempt<500; attempt++)); do + [[ -e ${DESCENDANT_RELEASE:?} ]] && exit 0 + sleep 0.02 + done + exit 1 +) & +printf '%s\n' "$!" > "${DESCENDANT_FILE:?}" +EOF +chmod +x -- "$background_repo/tools/check" +printf 'AGENT_CMD_TEST=tools/check\n' > "$background_repo/.agent/config.env" +COUNT_FILE=$tmp/background-count DESCENDANT_FILE=$tmp/descendant DESCENDANT_RELEASE=$tmp/descendant-release \ + "$run_sh" --dir "$background_repo" --cmd test > /dev/null 2>&1 +descendant=$(<"$tmp/descendant") +assert_eq yes "$([[ $descendant =~ ^[0-9]+$ ]] && kill -0 "$descendant" 2>/dev/null && printf yes || printf no)" \ + 'fixture leaves a live background descendant after agent-run completes' +second_rc=0 +COUNT_FILE=$tmp/background-count DESCENDANT_FILE=$tmp/descendant-2 DESCENDANT_RELEASE=$tmp/descendant-release \ + "$run_sh" --dir "$background_repo" --cmd test > /dev/null 2>&1 || second_rc=$? +assert_eq 0 "$second_rc" 'a completed command descendant does not retain the active-run lease' +assert_eq 2 "$(wc -l < "$tmp/background-count" | tr -d '[:space:]')" \ + 'the identical command executes again after its prior wrapper completes' +descendant_two=$(<"$tmp/descendant-2") +assert_eq yes "$([[ $descendant_two =~ ^[0-9]+$ ]] && kill -0 "$descendant_two" 2>/dev/null && printf yes || printf no)" \ + 'the second completed wrapper also leaves its descendant alive until release' +printf 'release\n' > "$tmp/descendant-release" +for ((attempt=0; attempt<100; attempt++)); do + kill -0 "$descendant" 2>/dev/null || { kill -0 "$descendant_two" 2>/dev/null || break; } + sleep 0.02 +done +kill "$descendant" "$descendant_two" 2>/dev/null || true + +symlink_repo=$tmp/symlink-repo +symlink_agent=$tmp/symlink-agent +git -C "$tmp" init -q symlink-repo +mkdir -p "$symlink_agent" +ln -s "$symlink_agent" "$symlink_repo/.agent" +"$run_sh" --dir "$symlink_repo" -- true > /dev/null 2>&1 +assert_eq no "$([[ -e $symlink_agent/run-records ]] && printf yes || printf no)" \ + 'a symlinked .agent parent receives no active-run records' +assert_eq no "$([[ -e $symlink_agent/logs ]] && printf yes || printf no)" \ + 'a symlinked .agent parent receives no command logs' + +logs_repo=$tmp/logs-repo +logs_target=$tmp/logs-target +git -C "$tmp" init -q logs-repo +mkdir -p "$logs_repo/.agent" "$logs_target" +ln -s "$logs_target" "$logs_repo/.agent/logs" +"$run_sh" --dir "$logs_repo" -- true > /dev/null 2>&1 +assert_eq '' "$(find "$logs_target" -mindepth 1 -print -quit)" \ + 'a symlinked .agent/logs directory receives no command log' + +mkdir -p "$tmp/relative-tmp" +(cd "$tmp" && TMPDIR=relative-tmp/// "$run_sh" --dir "$symlink_repo" -- true > /dev/null 2>&1) +fallback_log=$(find "$tmp/relative-tmp/agent-logs-$(id -u)" -type f -name '*-true.log' -print -quit) +fallback_status=$(cd "$tmp" && TMPDIR=relative-tmp/// "$run_sh" status "$fallback_log" 2>&1) +assert_eq pass "$fallback_status" \ + 'status accepts a canonical fallback log with relative trailing-slash TMPDIR' + +finish diff --git a/tests/test-bootstrap-repo.sh b/tests/test-bootstrap-repo.sh index 85ec5e45..263f4f10 100755 --- a/tests/test-bootstrap-repo.sh +++ b/tests/test-bootstrap-repo.sh @@ -108,8 +108,11 @@ assert_contains "$listed" 'AGENT_STATUS_VOCAB=Backlog,Ready,In progress,In revie assert_contains "$(cat "$repo/.agent/config.env")" '# AGENT_REVIEW_PROVIDERS=' \ 'generated config proposes an explicit automated review provider choice' assert_contains "$(cat "$repo/.agent/config.env")" \ - 'coderabbit, github-code-quality, or none' \ - 'provider proposal explains the supported choices' + 'Known providers get provider-specific settlement' \ + 'provider proposal explains provider-specific handling' +assert_contains "$(cat "$repo/.agent/config.env")" \ + 'Any other valid name is observed' \ + 'provider proposal explains the generic observe-only default' expected_generator=$(jq -r .version "$root/agentkit/.codex-plugin/plugin.json") assert_contains "$(cat "$repo/.agent/config.env")" "AGENT_ONBOARDED_BY=agentkit/$expected_generator" \ 'generated config records the installed generator version' @@ -490,6 +493,17 @@ assert_eq '1' "$(grep -c '^AGENT_REVIEW_PROVIDERS=none$' "$repo/.agent/config.en assert_eq '0' "$(grep -c '^# AGENT_REVIEW_PROVIDERS=' "$repo/.agent/config.env" || true)" \ 'refresh does not leave a proposal beside a selected provider' +# Arbitrary valid names are declarations too; refresh must preserve the choice +# and must not reintroduce a commented proposal beside it. +sed -i 's/^AGENT_REVIEW_PROVIDERS=none$/AGENT_REVIEW_PROVIDERS=chatgpt-codex-connector/' \ + "$repo/.agent/config.env" +assert_rc 0 'refresh accepts an unknown observe-only provider choice' -- run_bs \ + --repo-root "$repo" --project 7 --force +assert_eq '1' "$(grep -c '^AGENT_REVIEW_PROVIDERS=chatgpt-codex-connector$' "$repo/.agent/config.env")" \ + 'refresh preserves one unknown provider declaration' +assert_eq '0' "$(grep -c '^# AGENT_REVIEW_PROVIDERS=' "$repo/.agent/config.env" || true)" \ + 'refresh does not propose a replacement for an unknown provider' + # A declared worker model is likewise a declaration, not a proposal: refresh # must not nag once a repository has already made the choice. Uses --refresh # (not --force) since that is what this case actually describes and exercises diff --git a/tests/test-finding-ledger.sh b/tests/test-finding-ledger.sh index 7f99efd5..ee7a6916 100755 --- a/tests/test-finding-ledger.sh +++ b/tests/test-finding-ledger.sh @@ -316,8 +316,9 @@ assert_rc 1 'ids refuses a missing findings file as unavailable evidence' -- \ # --- evidence producer (issue #873) ------------------------------------------- # Plan-level strictness: the log must be the green, unfocused declared test run -# and the named repair commit must change the finding's path. The log is not -# bound to a commit; forge-verifiable evidence is a separate follow-up. +# on the clean current head, and the named repair commit must change the +# finding's path. Existing ledgers written before tested-head metadata remain +# readable, but new evidence cannot be produced from an unbound log. ev_repo="$tmp/ev-repo" git init -q "$ev_repo" git -C "$ev_repo" config user.name Test @@ -338,9 +339,16 @@ printf 'tidy\n' >"$ev_repo/other.txt" git -C "$ev_repo" add other.txt git -C "$ev_repo" commit -qm 'format follow-up' ev_head=$(git -C "$ev_repo" rev-parse HEAD) -printf '=== agent-run tests/regression.sh\n=== agent-run exited rc=0 after 1s\n' >"$tmp/ev-full.log" -printf '=== agent-run tests/regression.sh --only one\n=== agent-run exited rc=0 after 1s\n' >"$tmp/ev-focused.log" -printf '=== agent-run tests/regression.sh\n=== agent-run exited rc=1 after 1s\n' >"$tmp/ev-red.log" +agent_log() { + printf '=== agent-run %s\n=== started 2026-09-22T00:00:00Z pid=1 cwd=%s concurrent-suites=1 head=%s tracked-clean=%s\n=== agent-run exited rc=%s after 1s\n' \ + "$2" "$ev_repo" "$3" "$4" "$5" >"$1" +} +agent_log "$tmp/ev-full.log" tests/regression.sh "$ev_head" yes 0 +agent_log "$tmp/ev-focused.log" 'tests/regression.sh --only one' "$ev_head" yes 0 +agent_log "$tmp/ev-red.log" tests/regression.sh "$ev_head" yes 1 +agent_log "$tmp/ev-other-head.log" tests/regression.sh "$ev_repair" yes 0 +agent_log "$tmp/ev-dirty.log" tests/regression.sh "$ev_head" no 0 +printf '=== agent-run tests/regression.sh\n=== agent-run exited rc=0 after 1s\n' >"$tmp/ev-unbound.log" evidence() { "$script" evidence --title 'Guard input' --path affected.sh --repo-root "$ev_repo" "$@" } @@ -361,7 +369,7 @@ assert_rc 0 'producer output is accepted by add --verdict fixed unmodified' -- r --repo-root "$ev_repo" --head "$ev_head" assert_eq complete "$("$script" status --file "$ev_run/findings.ndjson" --repo-root "$ev_repo" \ --head "$ev_head" | jq -r .remediation)" \ - 'evidence from a header-less agent-run log validates as complete (existing ledgers keep working)' + 'fresh tested-head evidence validates as complete' assert_rc 2 'evidence requires an explicit --repair-sha' -- evidence --log "$tmp/ev-full.log" assert_rc 1 'a focused log is refused as repair evidence' -- \ @@ -370,10 +378,33 @@ assert_rc 1 'a red log is refused as repair evidence' -- \ evidence --log "$tmp/ev-red.log" --repair-sha "$ev_repair" assert_rc 1 'a repair SHA that does not change the path is refused' -- \ evidence --log "$tmp/ev-full.log" --repair-sha "$ev_head" +stale_override_rc=0 +evidence --head "$ev_repair" --log "$tmp/ev-other-head.log" --repair-sha "$ev_repair" \ + >/dev/null 2>"$tmp/ev-stale-override.err" || stale_override_rc=$? +assert_eq 1 "$stale_override_rc" \ + 'an explicit reachable old head cannot replace the checkout HEAD for new evidence' +assert_contains "$(cat "$tmp/ev-stale-override.err")" \ + "evidence head $ev_repair is not the current head $ev_head" \ + 'the stale override refusal names both the requested and actual heads' +assert_rc 1 'a log from another head cannot certify the current pushed head' -- \ + evidence --log "$tmp/ev-other-head.log" --repair-sha "$ev_repair" +assert_rc 1 'a log from a dirty tree cannot certify the committed head' -- \ + evidence --log "$tmp/ev-dirty.log" --repair-sha "$ev_repair" +assert_rc 1 'new repair evidence requires tested-head metadata' -- \ + evidence --log "$tmp/ev-unbound.log" --repair-sha "$ev_repair" focused_err=$(evidence --log "$tmp/ev-focused.log" --repair-sha "$ev_repair" 2>&1 >/dev/null || true) assert_contains "$focused_err" 'without --only' 'the focused-log refusal says how to produce valid evidence' red_err=$(evidence --log "$tmp/ev-red.log" --repair-sha "$ev_repair" 2>&1 >/dev/null || true) assert_contains "$red_err" 'no final successful agent-run result' 'the red-log refusal names the failed run' +other_head_err=$(evidence --log "$tmp/ev-other-head.log" --repair-sha "$ev_repair" 2>&1 >/dev/null || true) +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' +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' abs_err=$("$script" evidence --title 'Guard input' --path "$ev_repo/affected.sh" --log "$tmp/ev-full.log" \ --repo-root "$ev_repo" --repair-sha "$ev_repair" 2>&1 >/dev/null; printf 'rc=%s' "$?") assert_contains "$abs_err" 'repair path must be repository relative' 'an absolute --path is refused by name' @@ -390,6 +421,43 @@ real_rc=0 evidence --log "$real_log" --repair-sha "$ev_repair" >/dev/null 2>"$tmp/real.err" || real_rc=$? assert_eq 0 "$real_rc" "a real agent-run.sh log certifies the repair ($(cat "$tmp/real.err"))" +# Git repositories can use 64-character SHA-256 object IDs. When this Git +# supports that object format, exercise the real runner and evidence producer +# together so the log-header contract stays aligned with Git's full IDs. +sha256_repo="$tmp/ev-sha256-repo" +if git init -q --object-format=sha256 "$sha256_repo" 2>/dev/null; then + git -C "$sha256_repo" config user.name Test + git -C "$sha256_repo" config user.email test@example.invalid + mkdir -p "$sha256_repo/.agent" "$sha256_repo/tests" + printf 'AGENT_CMD_TEST=tests/regression.sh\n' >"$sha256_repo/.agent/config.env" + printf '.agent/\n' >"$sha256_repo/.gitignore" + printf '#!/bin/sh\necho regression ok\n' >"$sha256_repo/tests/regression.sh" + chmod +x "$sha256_repo/tests/regression.sh" + printf 'broken\n' >"$sha256_repo/affected.sh" + git -C "$sha256_repo" add .gitignore affected.sh tests/regression.sh + git -C "$sha256_repo" commit -qm baseline + printf 'repaired\n' >"$sha256_repo/affected.sh" + git -C "$sha256_repo" commit -qam repair + sha256_repair=$(git -C "$sha256_repo" rev-parse HEAD) + assert_eq 64 "${#sha256_repair}" 'the regression fixture uses a full SHA-256 object ID' + (cd -- "$sha256_repo" && "$agent_run" --cmd test >/dev/null 2>&1) + sha256_log=$(find "$sha256_repo/.agent/logs" -name '*-test.log' -type f -print -quit) + sha256_rc=0 + "$script" evidence --title 'Guard input' --path affected.sh --repo-root "$sha256_repo" \ + --log "$sha256_log" --repair-sha "$sha256_repair" >/dev/null 2>"$tmp/sha256.err" || sha256_rc=$? + assert_eq 0 "$sha256_rc" \ + "a real SHA-256 repository log certifies the repair ($(cat "$tmp/sha256.err"))" +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%% *} +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" \ + --head "$ev_head" | jq -r .remediation)" \ + 'existing header-less repair evidence remains readable' + undeclared_repo="$tmp/ev-undeclared" git clone -q "$ev_repo" "$undeclared_repo" undeclared_err=$("$script" evidence --title 'Guard input' --path affected.sh --log "$tmp/ev-full.log" \ diff --git a/tests/test-onboard-refresh.sh b/tests/test-onboard-refresh.sh index 2c3fa9d6..02b65d6d 100755 --- a/tests/test-onboard-refresh.sh +++ b/tests/test-onboard-refresh.sh @@ -66,7 +66,7 @@ assert_contains "$out" 'toolchains=-1' \ assert_contains "$out" 'paths=drift' \ 'summary preserves path drift alongside generator drift' -printf '%s\n' 'AGENT_REVIEW_PROVIDERS=not-a-provider' >> "$repo/.agent/config.env" +printf '%s\n' 'AGENT_REVIEW_PROVIDERS=X-1' >> "$repo/.agent/config.env" out=$(bash "$refresh_sh" --repo-root "$repo" --summary 2>&1) assert_contains "$out" 'review-providers=invalid' \ 'report identifies an invalid provider choice as an advisory gap' diff --git a/tests/test-onboard-state.sh b/tests/test-onboard-state.sh index be779498..4eaf79f3 100755 --- a/tests/test-onboard-state.sh +++ b/tests/test-onboard-state.sh @@ -49,6 +49,16 @@ out=$($state_sh --repo-root "$repo" --report) assert_contains "$out" 'stage=armed' 'ignored local declarations arm after verification' assert_contains "$out" 'next=none' 'the local model has no commit step' +# An invalid declaration cannot be called armed merely because it is locally +# ignored and has a command. It must return to declaration repair first. +printf 'AGENT_REPO_SLUG=o/r\nAGENT_BASE_BRANCH=main\nAGENT_CMD_VERIFY=true\nAGENT_REVIEW_PROVIDERS=X-1\n' \ + > "$repo/.agent/config.env" +out=$("$state_sh" --repo-root "$repo" --report) +assert_not_contains "$out" 'stage=armed' 'invalid config never reports the repository as armed' +assert_contains "$out" 'stage=declared' 'invalid config returns to the declaration stage' +assert_contains "$out" 'next=verify' 'invalid config asks for verification after repair' +printf 'AGENT_REPO_SLUG=o/r\nAGENT_BASE_BRANCH=main\nAGENT_CMD_VERIFY=true\n' > "$repo/.agent/config.env" + printf '.agent/*\n!.agent/config.env\n!.agent/board.json\n' > "$repo/.gitignore" git -C "$repo" add -- .agent/config.env .agent/board.json .gitignore out=$("$state_sh" --repo-root "$repo" --report) diff --git a/tests/test-repo-config.sh b/tests/test-repo-config.sh index 93cddee8..7f60caef 100755 --- a/tests/test-repo-config.sh +++ b/tests/test-repo-config.sh @@ -86,16 +86,14 @@ repo=$(make_repo config-bad.env) out=$("$rc_sh" --repo-root "$repo" --export 2> /dev/null) err=$("$rc_sh" --repo-root "$repo" --export 2>&1 > /dev/null) for key in AGENT_REPO_SLUG AGENT_BASE_BRANCH AGENT_PROJECT_NUMBER \ - AGENT_ADR_DIR AGENT_WORKTREE_ROOT AGENT_BRANCH_PREFIXES AGENT_REVIEW_PROVIDERS; do + AGENT_ADR_DIR AGENT_WORKTREE_ROOT AGENT_BRANCH_PREFIXES; do assert_not_contains "$out" "export $key=" "rejects invalid $key" done assert_contains "$err" 'AGENT_UNKNOWN_KEY' 'warns about an unknown key' assert_contains "$err" '--list-keys' 'the unknown-key warning names a discoverable way to list accepted keys' assert_contains "$err" 'no equals sign' 'warns about a malformed line' -assert_contains "$err" 'invalid value for AGENT_REVIEW_PROVIDERS' 'warns about the invalid provider declaration' -for provider in coderabbit github-code-quality none; do - assert_contains "$err" "$provider" "the provider rejection names $provider as accepted" -done +assert_contains "$out" "export AGENT_REVIEW_PROVIDERS='coderabbit,not-a-provider'" \ + 'accepts arbitrary syntactically valid provider names in a mixed declaration' assert_rc 0 'a fully invalid config still exits 0' -- "$rc_sh" --repo-root "$repo" --export # Unknown declarations are onboarding drift, not a per-line emergency. One @@ -139,6 +137,8 @@ for key in AGENT_REPO_SLUG AGENT_BASE_BRANCH AGENT_REVIEW_PROVIDERS AGENT_WORKER assert_contains "$list_out" "$key" "--list-keys names the accepted literal key $key" done assert_contains "$list_out" 'AGENT_CMD_' '--list-keys documents the open-ended command pattern' +assert_contains "$list_out" 'AGENT_REVIEW_PROVIDER__LOGIN' \ + '--list-keys documents the open-ended provider login pattern' assert_contains "$list_out" 'AGENT_RUNDIR_' '--list-keys documents the open-ended rundir pattern' list_out_repo=$("$rc_sh" --repo-root "$repo" --list-keys 2> /dev/null) assert_eq "$list_out" "$list_out_repo" '--list-keys is schema, not affected by which repo it is pointed at' @@ -152,6 +152,76 @@ assert_eq 'yes' "$([[ -n $repo_config_providers ]] && printf yes || printf no)" assert_eq "$catalog_providers" "$repo_config_providers" \ 'repo-config.sh provider display list matches lib/review-provider-catalog.sh exactly' +# --- open provider declarations and optional login overrides --------------- +repo=$(mktemp -d "$tmp/repo.XXXXXX") +mkdir -p "$repo/.agent" +printf '%s\n' \ + 'AGENT_REVIEW_PROVIDERS=chatgpt-codex-connector' \ + 'AGENT_REVIEW_PROVIDER_CHATGPT_CODEX_CONNECTOR_LOGIN=codex-review-bot' \ + > "$repo/.agent/config.env" +assert_rc 0 'a valid unknown provider and login override validate' -- \ + "$rc_sh" --repo-root "$repo" --validate +out=$("$rc_sh" --repo-root "$repo" --export 2> /dev/null) +assert_contains "$out" "export AGENT_REVIEW_PROVIDER_CHATGPT_CODEX_CONNECTOR_LOGIN='codex-review-bot'" \ + 'the optional provider login override is exported as data' + +for alias in coderabbitai CodeRabbitAI github-code-quality GITHUB-CODE-QUALITY; do + printf '%s\n' \ + 'AGENT_REVIEW_PROVIDERS=chatgpt-codex-connector' \ + "AGENT_REVIEW_PROVIDER_CHATGPT_CODEX_CONNECTOR_LOGIN=$alias" \ + > "$repo/.agent/config.env" + set +e + invalid_out=$("$rc_sh" --repo-root "$repo" --validate 2>&1) + invalid_rc=$? + set -e + assert_eq 1 "$invalid_rc" "reserved login alias '$alias' is rejected after normalization" + assert_contains "$invalid_out" 'invalid value for AGENT_REVIEW_PROVIDER_CHATGPT_CODEX_CONNECTOR_LOGIN' \ + "reserved login alias '$alias' identifies the rejected override key" +done + +for key in AGENT_REVIEW_PROVIDER_CODERABBIT_LOGIN \ + AGENT_REVIEW_PROVIDER_GITHUB_CODE_QUALITY_LOGIN \ + AGENT_REVIEW_PROVIDER_NONE_LOGIN; do + printf '%s=%s\n' "$key" custom-review-bot > "$repo/.agent/config.env" + set +e + invalid_out=$("$rc_sh" --repo-root "$repo" --validate 2>&1) + invalid_rc=$? + set -e + assert_eq 1 "$invalid_rc" "$key is rejected because built-in identities are fixed" + assert_contains "$invalid_out" "invalid value for $key" \ + "$key reports the dead override instead of silently accepting it" + out=$("$rc_sh" --repo-root "$repo" --export 2> /dev/null) + assert_not_contains "$out" "export $key=" "$key is never exported as live configuration" +done + +for providers in 'coderabbit,chatgpt-codex-connector' 'x-y,z9'; do + printf 'AGENT_REVIEW_PROVIDERS=%s\n' "$providers" > "$repo/.agent/config.env" + assert_rc 0 "valid provider list '$providers' validates" -- \ + "$rc_sh" --repo-root "$repo" --validate +done +for providers in 'none,x' 'x,x' 'x,,y' 'X-1' 'coderabbitai'; do + printf 'AGENT_REVIEW_PROVIDERS=%s\n' "$providers" > "$repo/.agent/config.env" + set +e + invalid_out=$("$rc_sh" --repo-root "$repo" --validate 2>&1) + invalid_rc=$? + set -e + assert_eq 1 "$invalid_rc" "invalid provider list '$providers' is rejected" + if [[ $providers == coderabbitai ]]; then + assert_contains "$invalid_out" 'reserved built-in login alias' \ + "invalid provider list '$providers' names the alias collision" + else + assert_contains "$invalid_out" '[a-z][a-z0-9-]*' \ + "invalid provider list '$providers' names the provider-name rule" + fi +done + +printf '%s\n' \ + 'AGENT_REVIEW_PROVIDERS=chatgpt-codex-connector' \ + 'AGENT_REVIEW_PROVIDER_CHATGPT_CODEX_CONNECTOR_LOGIN=bad login' \ + > "$repo/.agent/config.env" +assert_rc 1 'an unsafe provider login override is rejected' -- \ + "$rc_sh" --repo-root "$repo" --validate + # --- secret rejection ------------------------------------------------------ repo=$(make_repo config-secrets.env) out=$("$rc_sh" --repo-root "$repo" --export 2> /dev/null) @@ -740,8 +810,8 @@ assert_rc 0 '--validate still accepts launchable codex/claude reviewer compounds # reviewer_roster_entry_valid now refuses an OpenCode-family compound, which # model_family itself recognizes but adversarial-run.sh cannot launch (+4 # lines). Measured. -assert_eq yes "$([[ $(wc -l < "$root/agentkit/skills/.shared/scripts/repo-config.sh") -le 1154 ]] && printf yes || printf no)" \ - 'repo-config.sh stays at or under 1154 lines (issue #777 helper-owned recipe)' +assert_eq yes "$([[ $(wc -l < "$root/agentkit/skills/.shared/scripts/repo-config.sh") -le 1165 ]] && printf yes || printf no)" \ + 'repo-config.sh stays at or under 1165 lines (issue #876 review repair)' verify_repo=$(mktemp -d "$tmp/verification.XXXXXX") mkdir -p "$verify_repo/.agent" diff --git a/tests/test-review-provider-catalog.sh b/tests/test-review-provider-catalog.sh index 3b74bb2c..ebd9dc47 100755 --- a/tests/test-review-provider-catalog.sh +++ b/tests/test-review-provider-catalog.sh @@ -38,8 +38,45 @@ for provider in coderabbit github-code-quality none; do assert_contains "$names_out" "$provider" \ "review_provider_names names $provider as accepted" done +assert_eq observe-only "$(catalog_call review_provider_mode chatgpt-codex-connector)" \ + 'a syntactically valid unknown provider is observe-only' +assert_eq generic-settlement "$(catalog_call review_provider_lifecycle chatgpt-codex-connector)" \ + 'an unknown provider uses generic settlement' +assert_eq chatgpt-codex-connector \ + "$(catalog_call review_provider_login chatgpt-codex-connector)" \ + 'an unknown provider defaults its login to its declared name' +assert_eq chatgpt-codex-connector \ + "$(AGENT_REVIEW_PROVIDERS=chatgpt-codex-connector \ + catalog_call review_provider_from_login chatgpt-codex-connector)" \ + 'an unknown provider login round-trips without a bot suffix' +assert_eq chatgpt-codex-connector \ + "$(catalog_call review_provider_from_login 'chatgpt-codex-connector[bot]')" \ + 'an unknown provider login round-trips with a bot suffix' # shellcheck disable=SC2016 # The inner shell expands its own positional parameter. -assert_rc 1 'unknown providers have no capability entry' -- bash -c \ - 'source "$1"; review_provider_mode unexpected' bash "$catalog" +assert_rc 1 'an unknown provider has no request marker' -- bash -c \ + 'source "$1"; review_provider_request_marker chatgpt-codex-connector' bash "$catalog" +assert_eq codex-review-bot "$(AGENT_REVIEW_PROVIDERS=chatgpt-codex-connector \ + AGENT_REVIEW_PROVIDER_CHATGPT_CODEX_CONNECTOR_LOGIN=codex-review-bot \ + catalog_call review_provider_login chatgpt-codex-connector)" \ + 'a declared unknown provider accepts an explicit login override' +assert_eq chatgpt-codex-connector "$(AGENT_REVIEW_PROVIDERS=chatgpt-codex-connector \ + AGENT_REVIEW_PROVIDER_CHATGPT_CODEX_CONNECTOR_LOGIN=codex-review-bot \ + catalog_call review_provider_from_login 'codex-review-bot[bot]')" \ + 'an explicit login override round-trips to its declared provider' +for alias in coderabbitai CodeRabbitAI github-code-quality GITHUB-CODE-QUALITY; do + # shellcheck disable=SC2016 # The inner shell expands its own positional parameters. + assert_rc 1 "reserved login alias $alias is rejected by the catalog override boundary" -- \ + env AGENT_REVIEW_PROVIDER_CHATGPT_CODEX_CONNECTOR_LOGIN="$alias" \ + bash -c 'source "$1"; review_provider_override_login chatgpt-codex-connector' bash "$catalog" +done +# shellcheck disable=SC2016 # The inner shell expands its own positional parameter. +assert_rc 1 'a built-in login alias cannot become an implicit generic provider name' -- bash -c \ + 'source "$1"; review_provider_mode coderabbitai' bash "$catalog" +# shellcheck disable=SC2016 # The inner shell expands its own positional parameter. +assert_rc 1 'an undeclared human-shaped login is not promoted to a provider' -- bash -c \ + 'source "$1"; review_provider_from_login ordinary-human' bash "$catalog" +# shellcheck disable=SC2016 # The inner shell expands its own positional parameter. +assert_rc 1 'malformed provider names have no capability entry' -- bash -c \ + 'source "$1"; review_provider_mode X-1' bash "$catalog" finish diff --git a/tests/test-review-provider-classification.sh b/tests/test-review-provider-classification.sh new file mode 100755 index 00000000..496ac1a3 --- /dev/null +++ b/tests/test-review-provider-classification.sh @@ -0,0 +1,23 @@ +#!/usr/bin/env bash +# Unknown forge bots stay in the generic automated lane regardless of declaration. +set -uo pipefail + +TEST_NAME='review-provider-classification' +here=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) +root=$(dirname -- "$here") +# shellcheck source=lib/assert.sh +source "$here/lib/assert.sh" + +classifier="$root/agentkit/skills/review-remote-pr/scripts/classify-author.sh" +author='{"login":"chatgpt-codex-connector[bot]","type":"Bot"}' + +out=$(printf '%s\n' "$author" | "$classifier") +assert_contains "$out" '"lane":"generic-automated"' \ + 'an undeclared unknown bot uses the generic automated lane' + +out=$(AGENT_REVIEW_PROVIDERS=chatgpt-codex-connector \ + bash -c 'printf "%s\n" "$1" | "$2"' bash "$author" "$classifier") +assert_contains "$out" '"lane":"generic-automated"' \ + 'declaring an unknown provider does not grant the known-provider lane' + +finish diff --git a/tests/test-review-provider-config.sh b/tests/test-review-provider-config.sh index 72487c55..ce8f7090 100755 --- a/tests/test-review-provider-config.sh +++ b/tests/test-review-provider-config.sh @@ -67,24 +67,21 @@ assert_eq 'provider=none mode=disabled source=invalid' "$out" \ repo=$(make_repo 'unknown-provider') out=$(bash "$resolver" --repo-root "$repo" 2> "$tmp/err") +assert_eq 'provider=unknown-provider mode=observe-only source=declared lane=generic-automated' "$out" \ + 'unknown providers use the observe-only generic bot lane' +assert_eq '' "$(<"$tmp/err")" 'a valid unknown provider declaration is silent' + +repo=$(make_repo 'coderabbitai') +out=$(bash "$resolver" --repo-root "$repo" 2> "$tmp/err") assert_eq 'provider=none mode=disabled source=invalid' "$out" \ - 'unknown providers cannot select a trigger' -unrecognized_source=$out -for provider in coderabbit github-code-quality none; do - assert_contains "$(<"$tmp/err")" "$provider" \ - "an unrecognized declared provider names $provider as accepted" -done + 'a built-in login alias cannot declare an implicit generic provider' +assert_contains "$(<"$tmp/err")" 'reserved built-in login alias' \ + 'the implicit-alias rejection names the collision' repo=$(make_repo __missing__) out=$(bash "$resolver" --repo-root "$repo" 2> "$tmp/err") assert_eq 'provider=none mode=disabled source=missing' "$out" \ 'missing configuration uses the safe disabled plan' -# A declared-but-unrecognized provider (source=invalid, asserted above) and no -# declaration at all (source=missing, just above) must never collapse to the -# same reported source -- that would quietly hide an explicit bad choice -# behind "nothing was asked for". -assert_not_contains "$unrecognized_source" 'source=missing' \ - 'a declared-but-unrecognized provider never reports as an undeclared repository' assert_contains "$(<"$tmp/err")" 'using effective none' \ 'missing configuration warns without blocking' diff --git a/tests/test-review-transition.sh b/tests/test-review-transition.sh index 00971d3a..e551a9e0 100755 --- a/tests/test-review-transition.sh +++ b/tests/test-review-transition.sh @@ -21,6 +21,7 @@ printf '%s\n' provider-resolve >>"$TRANSITION_LOG" case ${PROVIDER_MODE:-coderabbit} in coderabbit) printf '%s\n' 'provider=coderabbit mode=triggerable source=declared' ;; observe) printf '%s\n' 'provider=github-code-quality mode=observe-only source=declared' ;; + generic) printf '%s\n' 'provider=chatgpt-codex-connector mode=observe-only source=declared lane=generic-automated' ;; none) printf '%s\n' 'provider=none mode=disabled source=declared' ;; pair) printf '%s\n' 'provider=coderabbit mode=triggerable source=declared' @@ -343,6 +344,14 @@ assert_contains "$out" 'provider=github-code-quality result=OBSERVE_ONLY' \ assert_eq '0' "$(grep -c '^comment ' "$tmp/transition.log" || true)" \ 'observe-only provider cannot reach comment posting' +write_auth '[{"name":"chatgpt-codex-connector","action":"observe","source":"capability-default"}]' +: >"$tmp/transition.log" +out=$(PROVIDER_MODE=generic run_transition) +assert_contains "$out" 'provider=chatgpt-codex-connector result=OBSERVE_ONLY' \ + 'an unknown declared provider remains observe-only during ready transition' +assert_eq '0' "$(grep -c '^comment ' "$tmp/transition.log" || true)" \ + 'an unknown declared provider cannot reach comment posting' + write_auth "$(trigger_action coderabbit)" rm -f "$tmp/missing-auth.json" set +e diff --git a/tests/test-rrp-remediation-contract.sh b/tests/test-rrp-remediation-contract.sh index 2e6cab64..33c09223 100755 --- a/tests/test-rrp-remediation-contract.sh +++ b/tests/test-rrp-remediation-contract.sh @@ -67,6 +67,49 @@ assert_contains "$(cat -- "$rrp_skill")" 'finding-ledger.sh" evidence' \ worker_gate="$skills/review-remote-pr/references/worker-gate.md" assert_contains "$(cat -- "$worker_gate")" 'unfocused' \ 'the worker completion report names the unfocused test log' +assert_contains "$(cat -- "$worker_gate")" 'after the repair commit and before push' \ + 'the repair handback orders full verification on the commit that will be pushed' +assert_contains "$(cat -- "$worker_gate")" 'clean committed HEAD' \ + 'the repair handback requires the log to bind the committed head' +fix_prompt=$(sed -n '/## PR-fix-batch worker prompt/,/## Exit Report/p' \ + "$skills/parallel-issues/references/worker-prompts.md") +assert_contains "$fix_prompt" 'commit the repair before the final unfocused run' \ + 'the composed fix-worker prompt commits before full verification' +assert_contains "$fix_prompt" 'push the branch only after that clean committed-HEAD run passes' \ + 'the composed fix-worker prompt cannot push an unverified commit' + +# Step 2's runnable phases must not execute the full suite before the worker +# commits. Execute its first fence with a recording runner, then ensure the +# committed-head test is a separate fence after an explicit commit boundary. +ci_fix_section=$(sed -n '/^## Step 2: Fix CI Failures/,/^For red\/green iterations/p' "$rrp_skill") +fence() { + local number=$1 + awk -v wanted="$number" ' + /^```bash$/ { count++; capture=(count==wanted); next } + /^```$/ { if (capture) exit } + capture + ' <<<"$ci_fix_section" +} +precommit_fence=$(fence 1) +postcommit_fence=$(fence 2) +recipe_kit="$tmp/recipe-kit" +recipe_calls="$tmp/recipe-calls" +mkdir -p "$recipe_kit/.shared/scripts" +cat >"$recipe_kit/.shared/scripts/agent-run.sh" <<'EOF' +#!/usr/bin/env bash +printf '%s\n' "$*" >>"$AGENTKIT_RECIPE_CALLS" +EOF +chmod +x "$recipe_kit/.shared/scripts/agent-run.sh" +AGENTKIT_RECIPE_CALLS="$recipe_calls" agentkit="$recipe_kit" agentkit_provenance=ok \ + bash -c "$precommit_fence" +assert_eq '--cmd lint --if-declared' "$(cat "$recipe_calls")" \ + 'executing the precommit fence cannot run the full test on a dirty repair' +assert_contains "$postcommit_fence" '"$agent_run" --cmd test' \ + 'committed-head verification has its own runnable fence' +commit_line=$(grep -nF 'Commit the repair' <<<"$ci_fix_section" | cut -d: -f1 | head -n1) +test_line=$(grep -nF '"$agent_run" --cmd test' <<<"$ci_fix_section" | cut -d: -f1 | head -n1) +assert_eq yes "$([[ -n $commit_line && -n $test_line && $commit_line -lt $test_line ]] && printf yes || printf no)" \ + 'the actual commit step precedes the runnable full-test phase' # --- item 6: the spawn contract names the primary checkout's ledger ---------- assert_contains "$(cat -- "$skills/.shared/spawn-contract.md")" "primary checkout's \`.agent/runs/active-workers.ndjson\`" \ @@ -123,8 +166,10 @@ assert_contains "$after_repair" 'RUN_DIR="$RUN_DIR" "$agentkit/review-remote-pr/ assert_contains "$after_repair" '--repair-sha' 'the evidence step names the repair commit' assert_not_contains "$after_repair" '--reviewed-head' 'the evidence step needs no reviewed head' assert_not_contains "$(cat -- "$adv_ref")" '--reviewed-head' 'the evidence contract needs no reviewed head' -assert_not_contains "$(cat -- "$worker_gate")" 'after the commit, so its header' \ - 'workers are not told to re-run the suite after committing' +assert_contains "$(cat -- "$adv_ref")" 'after the repair commit and before push' \ + 'the evidence recipe says when the binding full run occurs' +assert_contains "$(cat -- "$adv_ref")" 'tested head and tracked-tree cleanliness' \ + 'the evidence recipe explains what the log binding proves' assert_not_contains "$(cat -- "$adv_ref")" 'defaults to the last commit' \ 'the evidence contract no longer promises a guessed repair commit' diff --git a/tests/test-run-state-summary.sh b/tests/test-run-state-summary.sh index 7ff41df3..63898aac 100755 --- a/tests/test-run-state-summary.sh +++ b/tests/test-run-state-summary.sh @@ -38,6 +38,60 @@ printf '%s\n' \ >"$ledger" chmod 600 -- "$ledger" +# Auto-review handoff cannot succeed until every opened PR has either an +# adversarial receipt or a verified skip. Non-auto-review runs keep the same +# summary behavior. +printf '%s\n' \ + '{"opened_prs":[594,595],"queued":[],"receipt_prs":[594],"skipped_prs":[],"auto_review":false}' >"$state" +assert_rc 0 'non-auto-review summary permits opened PRs without review coverage' -- \ + "$script" summary --run-id wave --repo-root "$repo" + +printf '%s\n' \ + '{"opened_prs":[594,595],"queued":[],"receipt_prs":[594],"skipped_prs":[],"auto_review":true}' >"$state" +missing_review_rc=0 +missing_review_out="$tmp/missing-review.out" +missing_review_err="$tmp/missing-review.err" +"$script" summary --run-id wave --repo-root "$repo" --reports-dir "$reports" \ + >"$missing_review_out" 2>"$missing_review_err" || missing_review_rc=$? +assert_eq 1 "$missing_review_rc" 'auto-review summary refuses uncovered opened PRs' +assert_contains "$(cat "$missing_review_err")" '595' 'auto-review refusal names every uncovered PR' +assert_contains "$(cat "$missing_review_err")" '/review-remote-pr --auto-review 595' \ + 'auto-review refusal prints the exact review resume command' +assert_contains "$(cat "$missing_review_out")" 'coverage= prs=2 receipts=1 skipped=0 parked=2 queued=0' \ + 'auto-review refusal preserves coverage output' +assert_contains "$(cat "$missing_review_out")" 'blocked=101:src/one.sh' \ + 'auto-review refusal preserves parked-worker evidence' +assert_contains "$(cat "$missing_review_out")" 'spec-verification= issue=103' \ + 'auto-review refusal preserves durable verification reports' + +for report_case in omitted absent; do + early_out="$tmp/missing-review-$report_case.out" + early_err="$tmp/missing-review-$report_case.err" + early_rc=0 + early_args=() + [[ $report_case == omitted ]] || early_args=(--reports-dir "$tmp/absent-reports") + "$script" summary --run-id wave --repo-root "$repo" "${early_args[@]}" \ + >"$early_out" 2>"$early_err" || early_rc=$? + assert_eq 1 "$early_rc" "auto-review refusal survives the $report_case reports early-return path" + assert_contains "$(cat "$early_out")" 'blocked=101:src/one.sh' \ + "auto-review refusal preserves parked evidence with $report_case reports" +done + +printf '%s\n' \ + '{"opened_prs":[594,595],"queued":[],"receipt_prs":[],"skipped_prs":[],"auto_review":true}' >"$state" +multiple_missing_err=$("$script" summary --run-id wave --repo-root "$repo" 2>&1 >/dev/null) || true +assert_contains "$multiple_missing_err" \ + '/review-remote-pr --auto-review 594; /review-remote-pr --auto-review 595' \ + 'auto-review refusal prints one exact review invocation per uncovered PR' + +printf '%s\n' \ + '{"opened_prs":[594,595],"queued":[],"receipt_prs":[594],"skipped_prs":[595],"auto_review":true}' >"$state" +assert_rc 0 'auto-review summary accepts complete receipt and skip coverage' -- \ + "$script" summary --run-id wave --repo-root "$repo" + +printf '%s\n' \ + '{"opened_prs":[],"queued":[103],"receipt_prs":[],"skipped_prs":[],"root_turns":[true,true,true,true,true,true,true],"first_completion":true}' >"$state" + expected=$'coverage= prs=0 receipts=0 skipped=0 parked=2 queued=1 root-turns-before-first-completion=7\nblocked=101:src/one.sh\nblocked=102:src/two.sh,tests/two.sh\nspec-verification= issue=103 steps=4 covered=1 uncovered=3 uncovered-steps=2,3,4 coverage=1/4 classification=majority-uncovered' assert_eq "$expected" \ "$(cd -- "$tmp" && "$script" summary --run-id wave --repo-root "$repo" --reports-dir "$reports")" \ @@ -176,4 +230,35 @@ large_output=$(PATH="$fake_bin:$PATH" SUMMARY_REAL_GIT="$(command -v git)" SUMMA assert_eq 0 "$large_rc" 'primary worktree selection consumes a large porcelain stream without SIGPIPE' assert_contains "$large_output" 'coverage= prs=0' 'large worktree selection still resolves the primary ledger' +skill_text=$(tr '\n' ' ' <"$root/agentkit/skills/parallel-issues/SKILL.md" | tr -s '[:space:]' ' ') +# shellcheck disable=SC2016 +auto_review_get_recipe='auto_review_state=$("$agentkit/.shared/scripts/run-state.sh" get --run-id "$RUN_ID" --repo-root "$repository_root" --path auto_review) || exit 1' +# shellcheck disable=SC2016 +auto_review_case_recipe='case $auto_review_state in true|false)' +# shellcheck disable=SC2016 +auto_review_value='"$auto_review_state"' +# shellcheck disable=SC2016 +auto_review_default='${auto_review:-false}' +assert_contains "$skill_text" \ + "$auto_review_get_recipe" \ + 'Collect restores the durable auto-review mode before either PR-open path' +assert_contains "$skill_text" "$auto_review_case_recipe" \ + 'Collect refuses a restored auto-review value outside the boolean boundary' +completion_recipe=$(grep -F -- '- **Completion report (branch + pushed SHA)**' "$root/agentkit/skills/parallel-issues/SKILL.md") +blocked_recipe=$(grep -F -- '- **BLOCKED**' "$root/agentkit/skills/parallel-issues/SKILL.md") +assert_contains "$completion_recipe" "$auto_review_value" \ + 'normal PR-open completion prints the restored auto-review mode' +assert_contains "$blocked_recipe" "$auto_review_value" \ + 'partial-pushed PR-open completion prints the restored auto-review mode' +assert_not_contains "$completion_recipe$blocked_recipe" "$auto_review_default" \ + 'neither Collect completion path can default a resumed auto-review run to false' +# The literal expansion is the unsafe recipe under test. +# shellcheck disable=SC2016 +assert_not_contains "$skill_text" '--path auto_review --json "${auto_review:-false}"' \ + 'auto-review persistence never defaults an unset shell variable to false' +assert_contains "$skill_text" '--path auto_review --json true' \ + 'auto-review invocation facts persist the literal true value' +assert_contains "$skill_text" '--path auto_review --json false' \ + 'non-auto-review invocation facts persist the literal false value' + finish diff --git a/tests/test-skill-size.sh b/tests/test-skill-size.sh index aa2cebbf..940c7092 100755 --- a/tests/test-skill-size.sh +++ b/tests/test-skill-size.sh @@ -220,9 +220,9 @@ mkdir -p "$root/parallel-issues" } > "$root/parallel-issues/SKILL.md" run_lint "$root" assert_eq '1' "$LINT_RC" 'the parallel-issues ratchet fixture exceeds its measured ceiling' -assert_contains "$LINT_OUT" 'past its ratcheted ceiling of 761 lines' \ +assert_contains "$LINT_OUT" 'past its ratcheted ceiling of 772 lines' \ 'the parallel-issues line ratchet pins the extracted-recipe ceiling' -assert_contains "$LINT_OUT" 'past its ratcheted ceiling of 16265 tokens' \ +assert_contains "$LINT_OUT" 'past its ratcheted ceiling of 16666 tokens' \ 'the parallel-issues token ratchet pins the extracted-recipe ceiling' # A bad allowlist field must be named, never evaluated. Under `set -u` these