diff --git a/bots/gsbot/Cargo.lock b/bots/gsbot/Cargo.lock index 13f49e0e..3df9dcc1 100644 --- a/bots/gsbot/Cargo.lock +++ b/bots/gsbot/Cargo.lock @@ -1650,9 +1650,9 @@ dependencies = [ [[package]] name = "rustls" -version = "0.23.45" +version = "0.23.46" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0d41d731c7d2f962d1ccc364cec258de3c0e93b38c2fb3ba97ac74513048d634" +checksum = "48e13bd8c0e9365c43cfa5c9e8f9ad49d3c8444926c9aac819e0e4dc503c8fdf" dependencies = [ "once_cell", "ring", diff --git a/docs/BOT-OPERATIONS.adoc b/docs/BOT-OPERATIONS.adoc index 52afba49..4fb8acc3 100644 --- a/docs/BOT-OPERATIONS.adoc +++ b/docs/BOT-OPERATIONS.adoc @@ -88,6 +88,45 @@ Findings → observed-patterns.jsonl → threshold (5+) → propose_new_rule() → Human reviews PR → Approved rule deployed to hypatia .... +==== Review-Thread Stewardship (metarule) + +Reviewer bots (CodeRabbit, Codeac, and friends) post review threads on +PRs; a finding addressed by a fix commit is not *closed* until the thread +carries the fix and is resolved. 2026-10-15's estate sweep addressed +seven such threads by hand — reply + resolve are fleet actions, and the +permissions for them live in link:TOKEN-ARCHITECTURE.adoc[`echidnabot`]. + +The metarule: + +.... +1. Use the same PR and file/line class only to filter candidate unresolved + review threads for a Hypatia finding (or a fleet fix). Verify the + finding-to-thread match and require exactly one match before replying + or resolving. Leave all threads unchanged if zero or multiple matches + remain. +2. The fix lands on the PR branch (robot-repo-automaton; identity-class + findings like HYP-S010 are auto_fixable=false — the bot proposes and + replies, the owner rules on direction). +3. echidnabot replies to the thread with the fix commit and reasoning: + POST /repos/{owner}/{repo}/pulls/{pr}/comments/{id}/replies + where {id} is the top-level review-comment ID. +4. echidnabot resolves an addressed finding's thread only after the + required fix lands and Step 3's reply is posted: + GraphQL mutation resolveReviewThread (thread id from the PR's + reviewThreads). + Identity-class findings such as HYP-S010 stay open until the owner + directs and approves a fix and that fix lands. For verified-invalid + findings, follow Step 5 instead. +5. Never resolve a thread whose finding is not addressed. For findings + verified as no-longer-valid, post a "skipped with a brief reason" + reply explaining why and leave the thread unresolved. +.... + +Corollary learned the hard way (odraz #19): when a reviewer bot's finding +says "align X to the declared Y", check which side is the residue before +automating — a reviewer bot can confidently align to template residue. +Auto-remediation of identity-class findings stays owner-ruled. + ==== Maintenance Release Gate Routing Use this routing model for repository maintenance and release diff --git a/docs/TOKEN-ARCHITECTURE.adoc b/docs/TOKEN-ARCHITECTURE.adoc index 56fe262c..1340e3ad 100644 --- a/docs/TOKEN-ARCHITECTURE.adoc +++ b/docs/TOKEN-ARCHITECTURE.adoc @@ -165,6 +165,8 @@ This bot triggers Echidna workflows for proof execution, which requires the | Trigger Echidna workflow | `POST /repos/{owner}/{repo}/actions/workflows/echidna-scan.yml/dispatches` | Read ProofTrace data | `GET /repos/{owner}/{repo}/contents/.prooftrace/` | Write ProofTrace data | `PUT /repos/{owner}/{repo}/contents/.prooftrace/` +| Reply to a review thread | `POST /repos/{owner}/{repo}/pulls/{pr}/comments/{comment_id}/replies` — `{comment_id}` is the top-level review-comment ID, distinct from the GraphQL review-thread ID used by `resolveReviewThread`; review-thread stewardship (BOT-OPERATIONS.adoc); no new scope, `repo` covers it +| Resolve a review thread | GraphQL `mutation resolveReviewThread(input: {threadId})` — same stewardship; resolves only threads whose finding the fleet addressed, with the fix-commit reply posted first |=== === 4. sustainabot diff --git a/robot-repo-automaton/.claude/CLAUDE.md b/robot-repo-automaton/.claude/CLAUDE.md index 59527dd5..c2af6102 100644 --- a/robot-repo-automaton/.claude/CLAUDE.md +++ b/robot-repo-automaton/.claude/CLAUDE.md @@ -8,6 +8,7 @@ |---------------|----------|-------| | **AffineScript** | Primary application code | Compiles to JS/WASM, type-safe with affine types | | **Bun** | JS runtime & package management (tier 1) | Default for all new work. Runs compiled ESM/JS directly — no bundler step. Uses an npm-compatible `package.json` plus `bun.lock` — both are expected, not anti-patterns. | +| **Deno** | JS runtime only (tier 2) | Secondary to Bun; limited to running compiled ESM/JS. Use Bun for all JavaScript dependency installation and management. | | **Rust** | Performance-critical, systems, WASM | Preferred for CLI tools | | **Tauri 2.0+** | Mobile apps (iOS/Android) | Rust backend + web UI | | **Dioxus** | Mobile apps (native UI) | Pure Rust, React-like | @@ -26,7 +27,6 @@ |--------|-------------| | TypeScript | AffineScript | | ReScript | AffineScript | -| Deno | Bun | | Node.js | Bun | | npm | Bun | | pnpm/yarn | Bun | diff --git a/tests/e2e.sh b/tests/e2e.sh index 9cfbf016..2ea4efde 100644 --- a/tests/e2e.sh +++ b/tests/e2e.sh @@ -211,6 +211,20 @@ done echo "" +# ─── Section 7: Documentation policy contracts ────────────────────── +bold "Section 7: Documentation policy contracts" + +for policy_test in review-thread-stewardship-test.sh runtime-policy-test.sh; do + if bash "$SCRIPT_DIR/$policy_test"; then + PASS=$((PASS + 1)) + else + red " FAIL: $policy_test" + FAIL=$((FAIL + 1)) + fi +done + +echo "" + # ═══════════════════════════════════════════════════════════════════════ # Summary # ═══════════════════════════════════════════════════════════════════════ diff --git a/tests/review-thread-stewardship-test.sh b/tests/review-thread-stewardship-test.sh new file mode 100644 index 00000000..f68867b2 --- /dev/null +++ b/tests/review-thread-stewardship-test.sh @@ -0,0 +1,108 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: MPL-2.0 +# Acceptance checks for the documented contract, not runtime enforcement. +# Offline: reads documentation only; never calls GitHub or enables the fleet. +# Run with: bash tests/review-thread-stewardship-test.sh +set -euo pipefail +root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" + +# Limit assertions to the relevant section so another bot or metarule cannot +# accidentally satisfy them. Collapse wrapping without snapshotting whole docs. +section() { + local file="$1" heading="$2" level="$3" + awk -v heading="$heading" -v level="$level" ' + $0 == heading { found = 1; next } + found && $0 ~ "^" level " " { exit } + found { print } + ' "$file" +} +normalize() { awk '{$1=$1; printf "%s ", $0}'; } +require() { + local label="$1" pattern="$2" content="$3" + if ! grep -Eq -- "$pattern" <<< "$content"; then + printf 'FAIL: %s\n' "$label" >&2 + exit 1 + fi + printf 'PASS: %s\n' "$label" +} + +operations="$(section "$root/docs/BOT-OPERATIONS.adoc" \ + '==== Review-Thread Stewardship (metarule)' '====')" +tokens="$(section "$root/docs/TOKEN-ARCHITECTURE.adoc" \ + '=== 3. echidnabot' '===')" + +# Keep each guard in its own numbered step; a disclaimer elsewhere must not +# conceal a weakened matching, resolution, or invalid-finding instruction. +step() { + local number="$1" + awk -v number="$number" ' + /^[0-9]+\. / { + if (found) exit + if ($1 == number ".") found = 1 + } + found && /^\.\.\.\.$/ { exit } + found { print } + ' <<< "$operations" | normalize +} +match_step="$(step 1)" +fix_step="$(step 2)" +reply_step="$(step 3)" +resolve_step="$(step 4)" +invalid_step="$(step 5)" + +require 'candidate filtering uses the same PR, file/line class and unresolved threads' \ + 'same PR and file/line class only to filter candidate unresolved' "$match_step" +require 'candidate filtering requires a verified finding-to-thread match' \ + 'Verify the finding-to-thread match' "$match_step" +require 'reply and resolution require exactly one match' \ + 'exactly one match before replying or resolving' "$match_step" +require 'zero or ambiguous matches leave every thread unchanged' \ + 'Leave all threads unchanged if zero or multiple matches' "$match_step" +require 'the executor lands the fix on the PR branch' \ + 'fix lands on the PR branch \(robot-repo-automaton' "$fix_step" +require 'identity findings cannot be automatically fixed' \ + 'HYP-S010 are auto_fixable=false' "$fix_step" +require 'identity direction belongs to the owner' \ + 'bot proposes and replies, the owner rules on direction' "$fix_step" +require 'echidnabot replies with both the fix commit and reasoning' \ + 'echidnabot replies to the thread with the fix commit and reasoning' "$reply_step" + +# REST takes a top-level comment ID; GraphQL takes a review-thread node ID. +reply_endpoint='POST /repos/{owner}/{repo}/pulls/{pr}/comments/{id}/replies' +require 'operations document the REST reply endpoint' \ + "$(sed 's/[][\\.^$*+?(){}|]/\\&/g' <<< "$reply_endpoint")" "$reply_step" +require 'REST replies use the top-level review-comment ID' \ + 'where \{id\} is the top-level review-comment ID' "$reply_step" +require 'resolution follows both a landed fix and a posted reply' \ + "only after the required fix lands and Step 3's reply is posted" "$resolve_step" +require 'resolution uses a GraphQL thread ID from the PR' \ + "GraphQL mutation resolveReviewThread \(thread id from the PR's reviewThreads\)" "$resolve_step" +require 'identity threads wait for owner direction, approval and a landed fix' \ + 'HYP-S010 stay open until the owner directs and approves a fix and that fix lands' "$resolve_step" +require 'invalid findings take the skip path instead of normal resolution' \ + 'For verified-invalid findings, follow Step 5 instead' "$resolve_step" +require 'unaddressed findings must never be resolved' \ + 'Never resolve a thread whose finding is not addressed' "$invalid_step" +require 'invalid findings receive a skip explanation and remain unresolved' \ + 'verified as no-longer-valid, post a "skipped with a brief reason" reply explaining why and leave the thread unresolved' "$invalid_step" +require 'alignment findings require checking for template residue first' \ + 'check which side is the residue before automating' "$(normalize <<< "$operations")" + +reply_row="$(awk '/^\| Reply to a review thread \|/ {print}' <<< "$tokens")" +resolve_row="$(awk '/^\| Resolve a review thread \|/ {print}' <<< "$tokens")" +require 'echidnabot owns exactly one reply action and one resolve action' \ + '^1 1$' "$(grep -c '^| Reply to a review thread |' <<< "$tokens") $(grep -c '^| Resolve a review thread |' <<< "$tokens")" +require 'token docs use the same REST endpoint as operations' \ + "$(sed 's/[][\\.^$*+?(){}|]/\\&/g' <<< "${reply_endpoint//\{id\}/\{comment_id\}}")" "$reply_row" +require 'token docs distinguish REST comment IDs from GraphQL thread IDs' \ + 'top-level review-comment ID, distinct from the GraphQL review-thread ID' "$reply_row" +require 'review replies require no additional token scope' \ + 'no new scope, `repo` covers it' "$reply_row" +require 'token docs name the GraphQL mutation and threadId argument' \ + 'mutation resolveReviewThread\(input: \{threadId\}\)' "$resolve_row" +require 'token docs restrict resolution to addressed findings after the reply' \ + 'resolves only threads whose finding the fleet addressed, with the fix-commit reply posted first' "$resolve_row" +require 'operations link to the token permission document' \ + 'link:TOKEN-ARCHITECTURE\.adoc\[' "$(normalize <<< "$operations")" +require 'token reply action refers back to the operations contract' \ + 'BOT-OPERATIONS\.adoc' "$reply_row" diff --git a/tests/runtime-policy-test.sh b/tests/runtime-policy-test.sh new file mode 100644 index 00000000..57c79a5f --- /dev/null +++ b/tests/runtime-policy-test.sh @@ -0,0 +1,45 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: MPL-2.0 +# Acceptance check for RRA's documented runtime policy. No runtime is started. +# Run with: bash tests/runtime-policy-test.sh +set -euo pipefail +root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +policy="$root/robot-repo-automaton/.claude/CLAUDE.md" + +section() { + local heading="$1" + awk -v heading="$heading" ' + $0 == heading { found = 1; next } + found && /^### / { exit } + found { print } + ' "$policy" +} +allowed="$(section '### ALLOWED Languages & Tools')" +banned="$(section '### BANNED - Do Not Use')" + +if [[ -z "$allowed" || -z "$banned" ]]; then + echo 'FAIL: runtime policy must retain its allowed and banned sections' >&2 + exit 1 +fi +if [[ "$(grep -cE '^\| \*\*Deno\*\* \|' <<< "$allowed")" != 1 ]] || + ! grep -Eq '^\| \*\*Deno\*\* \| JS runtime only \(tier 2\) \| Secondary to Bun; limited to running compiled ESM/JS\.' <<< "$allowed"; then + echo 'FAIL: Deno must be allowed once as the secondary tier 2 runtime' >&2 + exit 1 +fi +echo 'PASS: Deno is allowed as the secondary tier 2 runtime' +if ! grep -Eq '^\| \*\*Deno\*\* \|[^|]*\|[^|]*Use Bun for all JavaScript dependency installation and management\.' <<< "$allowed"; then + echo 'FAIL: Deno must delegate JavaScript dependency installation and management to Bun' >&2 + exit 1 +fi +echo 'PASS: JavaScript dependency installation and management remain with Bun' +if ! grep -Eq '^\| \*\*Bun\*\* \| JS runtime & package management \(tier 1\) \|' <<< "$allowed"; then + echo 'FAIL: Bun must remain the primary tier 1 runtime' >&2 + exit 1 +fi +echo 'PASS: Bun remains the primary tier 1 runtime' +# Regression: adding Deno to ALLOWED alone leaves contradictory instructions. +if grep -Eiq '^\|[[:space:]*`]*Deno[[:space:]*`]*\|' <<< "$banned"; then + echo 'FAIL: Deno must not also appear in the banned runtime table' >&2 + exit 1 +fi +echo 'PASS: the banned table does not contradict the Deno allowance'