diff --git a/.agents/skills/launch-openshell-gator/SKILL.md b/.agents/skills/launch-openshell-gator/SKILL.md index 8b25760698..c8e9fc9489 100644 --- a/.agents/skills/launch-openshell-gator/SKILL.md +++ b/.agents/skills/launch-openshell-gator/SKILL.md @@ -28,6 +28,7 @@ For gator's PR/issue validation policy, load `gator-gate` inside the launched sa | `scripts/agents/gator/policy.yaml` | Sandbox policy for the gator agent. | | `scripts/agents/gator/bin/gh` | Gator-specific `gh` wrapper and same-SHA duplicate-post guard. | | `scripts/agents/gator/bin/review-feedback-ledger` | Builds tree-aware review scope, durable findings, convergence telemetry, and review-budget state. | +| `scripts/agents/gator/bin/resolve-gator-review-threads` | Resolves addressed Gator-owned inline review threads by ledger finding ID. | | `scripts/agents/gator/bin/validate-review-findings` | Enforces the blocker evidence schema and downgrades unsupported hypotheses. | | `scripts/agents/gator/prompts/gator.md` | Rendered top-level prompt template baked into the payload. | | `scripts/agents/gator/skills/gator-gate/SKILL.md` | In-sandbox gator state-machine skill. | @@ -157,7 +158,7 @@ sandbox_name="gator-pr-${pr_number}-supervised" "Review and monitor PR #${pr_number} through the gator-gate workflow. Scope this invocation only to PR #${pr_number}." ``` -The launcher builds the gator sandbox image when needed, stages the immutable payload, imports provider profiles, configures provider credentials and refresh, creates the sandbox, and writes a background log under `scripts/agents/gator/logs/`. +The launcher builds the gator sandbox image when needed, stages the immutable payload, imports provider profiles, configures provider credentials and refresh, creates and uploads the sandbox payload, then starts the agent supervisor with `sandbox exec`. It writes a background log under `scripts/agents/gator/logs/`. ### Launch An Issue Or Issue/PR Pair @@ -317,10 +318,10 @@ Restart when the payload must change, the sandbox is wedged without a sentinel, Increment `payload_version` in `scripts/agents/gator/agent.yaml` whenever a merged change alters the Gator prompt, gate skill, reviewer contract, write -guard, ledger, or bundled validator. Existing immutable watchers cannot replace -their own payload. New-version watchers detect later published versions and -stop with `stale_gator_payload`; relaunch every still-active older watcher after -the version bump is published. +guard, ledger, thread resolver, or bundled validator. Existing immutable +watchers cannot replace their own payload. New-version watchers detect later +published versions and stop with `stale_gator_payload`; relaunch every +still-active older watcher after the version bump is published. Before deleting, check that the sandbox is truly stale or that the operator asked for a restart. If a bounded review cycle is actively running and still producing useful output, prefer leaving it alone. @@ -368,7 +369,9 @@ Symptoms: host `gh` auth fails, Codex refresh fails, in-sandbox GitHub calls rep Actions: - Re-run the GitHub and Codex preflight checks. +- Existing refresh-managed providers are reused without an ordinary credential update; the launcher rotates their gateway-managed credential instead. - If host Codex auth changed, relaunch with `--reset-refresh` once. +- `--reset-refresh` removes the old refresh ownership before rediscovering host credentials, then configures and rotates the replacement refresh state. - If Entra or Microsoft auth is involved in a future provider, use the relevant auth skill. Gator's default providers are GitHub and Codex. ### Unsupported `gh pr view --json` Field diff --git a/scripts/agents/gator/Dockerfile b/scripts/agents/gator/Dockerfile index 5cf2616b7e..6484746dbb 100644 --- a/scripts/agents/gator/Dockerfile +++ b/scripts/agents/gator/Dockerfile @@ -85,10 +85,12 @@ RUN mkdir -p /etc/openshell COPY policy.yaml /etc/openshell/policy.yaml COPY bin/gh /usr/local/bin/gh-gator COPY bin/review-feedback-ledger /usr/local/bin/review-feedback-ledger +COPY bin/resolve-gator-review-threads /usr/local/bin/resolve-gator-review-threads COPY bin/validate-review-findings /usr/local/bin/validate-review-findings RUN rm -f /usr/local/bin/gh && \ cp /usr/local/bin/gh-gator /usr/local/bin/gh && \ chmod 755 /usr/local/bin/gh /usr/local/bin/review-feedback-ledger \ + /usr/local/bin/resolve-gator-review-threads \ /usr/local/bin/validate-review-findings RUN printf 'export PATH="/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin:/bin:/sbin"\nexport PS1="\\u@\\h:\\w\\$ "\n' \ diff --git a/scripts/agents/gator/README.md b/scripts/agents/gator/README.md index 5773d08251..3bdb4f490a 100644 --- a/scripts/agents/gator/README.md +++ b/scripts/agents/gator/README.md @@ -29,7 +29,7 @@ The manifest-driven launcher at `scripts/agents/run.sh` reads `agent.yaml`, whic The launcher: -- Scans `profile_paths` in manifest order and imports `providers/github-gator.yaml`. +- Scans `profile_paths` in manifest order and imports or updates `providers/github-gator.yaml`. - Creates or updates the `github-gator` provider from `gh auth token`. - Selects the requested harness and bakes the common runtime into the immutable sandbox payload. - For `--harness codex`, imports `providers/codex-gator.yaml`, creates or updates the `codex-gator` provider from `$HOME/.codex/auth.json`, and stores the refresh token as gateway-only refresh material. @@ -38,14 +38,18 @@ The launcher: - Uses the gator image policy copied to `/etc/openshell/policy.yaml`. - Installs the gator-specific `gh` wrapper from `gator/bin/gh` as `/usr/local/bin/gh` to fail closed when same-head-SHA history cannot be checked, prevent duplicate dispositions, and require versioned review payloads. - Installs `gator/bin/review-feedback-ledger` as `/usr/local/bin/review-feedback-ledger` so reviews receive tree- and patch-aware scope, prior summaries and findings, resolution state, convergence telemetry, and the three-round Warning budget. +- Installs `gator/bin/resolve-gator-review-threads` so a follow-up commit that + demonstrably fixes a Gator inline finding can resolve the corresponding + Gator-owned GitHub review thread without touching human review threads. - Installs `gator/bin/validate-review-findings` to downgrade blockers that lack the required reachability, ownership, base-vs-head, impact, and reproducer evidence. +- Keeps that normalized evidence as Gator's internal review contract, then renders validated blockers for people as a read-aloud `Summary`, an actionable `Fix`, and a deterministic `Verify`. Exact paths and only the additional provenance an implementation agent needs appear in collapsed `Agent context`; raw evidence headings such as `Base` and `Head` are not posted publicly. Review-process provenance, docs and E2E disposition, SHAs, and state codes appear at the end of the summary in collapsed `Gator metadata`, while required human actions remain visible. - Bakes `scripts/agents/gator/skills/gator-gate/SKILL.md` into `/etc/openshell/agent-payload`. - Bakes `.claude/agents/principal-engineer-reviewer.md` so the selected harness can run a deterministic independent reviewer execution through `/etc/openshell/agent-payload/runtime/subagent.sh principal-engineer-reviewer < task.md`. - For `--harness codex`, optionally bakes a host Codex executable as `/etc/openshell/agent-payload/runtime/harnesses/codex/codex`. - Starts the selected harness without a TTY. - Runs gator in `watch` mode by default. The sandbox stays alive while the supervisor sleeps between bounded Codex cycles, so Codex is not connected during passive PR waits. The supervisor prints periodic heartbeat lines during active cycles and passive sleeps. - Makes each watch cycle compare its immutable payload version with the version published on the default branch. A stale watcher stops without GitHub writes and must be relaunched. -The GitHub provider profile allows read-only GraphQL queries on `api.github.com/graphql` so `gh` read paths can use GraphQL when needed. Write operations remain REST-only and scoped to the two allowed repositories. +The GitHub provider profile allows read-only GraphQL queries on `api.github.com/graphql` so `gh` read paths can use GraphQL when needed. Its only GraphQL mutation is the named `ResolveGatorReviewThread` operation, restricted to the `resolveReviewThread` root field. All other writes remain REST-only and scoped to the two allowed repositories. Set `GATOR_CODEX_ACCESS_CREDENTIAL_KEY` or pass `--codex-access-key` if the gator Codex profile uses a credential key other than `CODEX_AUTH_ACCESS_TOKEN` for the short-lived access token. @@ -58,4 +62,6 @@ The launcher preserves existing gateway-owned Codex refresh material by default ```shell bash scripts/agents/gator/bin/gh_guard_test.sh bash scripts/agents/gator/bin/review_feedback_ledger_test.sh +bash scripts/agents/gator/bin/resolve_gator_review_threads_test.sh +bash scripts/agents/runtime/harnesses/codex/exec_test.sh ``` diff --git a/scripts/agents/gator/agent.yaml b/scripts/agents/gator/agent.yaml index 209c709b51..f4eb082ac1 100644 --- a/scripts/agents/gator/agent.yaml +++ b/scripts/agents/gator/agent.yaml @@ -2,7 +2,7 @@ # SPDX-License-Identifier: Apache-2.0 id: gator -payload_version: 4 +payload_version: 7 display_name: Gator Gate Agent description: Validate and monitor OpenShell GitHub issues and pull requests through the gator state machine. diff --git a/scripts/agents/gator/bin/gh b/scripts/agents/gator/bin/gh index bda573d795..d65159ff7b 100755 --- a/scripts/agents/gator/bin/gh +++ b/scripts/agents/gator/bin/gh @@ -7,7 +7,7 @@ set -euo pipefail REAL_GH="${OPENSHELL_REAL_GH:-/usr/bin/gh}" GATOR_MARKER='> **gator-agent**' -GATOR_PAYLOAD_VERSION="${OPENSHELL_AGENT_PAYLOAD_VERSION:-4}" +GATOR_PAYLOAD_VERSION="${OPENSHELL_AGENT_PAYLOAD_VERSION:-5}" if [[ $# -lt 1 || "$1" != "api" ]]; then exec "$REAL_GH" "$@" diff --git a/scripts/agents/gator/bin/gh_guard_test.sh b/scripts/agents/gator/bin/gh_guard_test.sh index 35cc5e161d..c1571866c1 100755 --- a/scripts/agents/gator/bin/gh_guard_test.sh +++ b/scripts/agents/gator/bin/gh_guard_test.sh @@ -111,8 +111,15 @@ run_review_case() { ## PR Review Status -Head SHA: `0e4d7af7722fbedce2307d571b0c937a1eb3250f`' \ - --arg payload 'Gator payload: `4`' \ +The review is ready for author follow-up. + +
+Gator metadata + +- Head SHA: `0e4d7af7722fbedce2307d571b0c937a1eb3250f`' \ + --arg payload '- Gator payload: `5` + +
' \ --arg inline_body '> **gator-agent** **Warning:** Keep this validation bound to the accepted value.' \ @@ -142,7 +149,7 @@ same_sha_body='> **gator-agent** ## PR Review Status Head SHA: `0e4d7af7722fbedce2307d571b0c937a1eb3250f` -Gator payload: `4`' +Gator payload: `5`' run_case "blocks duplicate marked comment" \ "$same_sha_body" \ @@ -169,7 +176,7 @@ run_case "allows first versioned review disposition" \ ## PR Review Status Head SHA: `0e4d7af7722fbedce2307d571b0c937a1eb3250f` -Gator payload: `4`' \ +Gator payload: `5`' \ 0 run_case "allows unmarked comment" \ @@ -224,7 +231,7 @@ Gator is blocked from completing the required independent re-review for current ## PR Review Status Head SHA: `0e4d7af7722fbedce2307d571b0c937a1eb3250f` -Gator payload: `4`' \ +Gator payload: `5`' \ 0 draft_blocked_body='> **gator-agent** @@ -244,7 +251,7 @@ run_case "ignores draft blocker after PR is ready" \ ## PR Review Status Head SHA: `0e4d7af7722fbedce2307d571b0c937a1eb3250f` -Gator payload: `4`' \ +Gator payload: `5`' \ 0 \ false diff --git a/scripts/agents/gator/bin/resolve-gator-review-threads b/scripts/agents/gator/bin/resolve-gator-review-threads new file mode 100755 index 0000000000..1ed2d39bf4 --- /dev/null +++ b/scripts/agents/gator/bin/resolve-gator-review-threads @@ -0,0 +1,106 @@ +#!/usr/bin/env bash + +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +set -euo pipefail + +usage() { + cat >&2 <<'EOF' +Usage: + resolve-gator-review-threads LEDGER.json FINDING_ID [FINDING_ID ...] +EOF +} + +[[ "$#" -ge 2 && -r "$1" ]] || { + usage + exit 2 +} + +ledger="$1" +shift + +jq -e ' + .schema_version == 4 and + (.threads | type == "array") +' "$ledger" >/dev/null || { + echo "invalid gator review feedback ledger" >&2 + exit 2 +} + +mutation='mutation ResolveGatorReviewThread($threadId: ID!) { + resolveReviewThread(input: {threadId: $threadId}) { + thread { + id + isResolved + } + } +}' + +finding_ids=("$@") +thread_ids=() +resolution_states=() +declare -A seen_finding_ids=() + +# Validate the complete request before performing the first GitHub write so a +# malformed or ambiguous later ID cannot leave a partially resolved batch. +for finding_id in "${finding_ids[@]}"; do + [[ "$finding_id" =~ ^(GATOR-[0-9A-Fa-f]{8}-[0-9]{2}|gator-inline-[0-9]+)$ ]] || { + echo "invalid gator finding ID: $finding_id" >&2 + exit 2 + } + [[ -z "${seen_finding_ids[$finding_id]:-}" ]] || { + echo "duplicate gator finding ID: $finding_id" >&2 + exit 2 + } + seen_finding_ids["$finding_id"]=1 + + matching_threads="$(jq -c --arg finding_id "$finding_id" ' + [ + .threads[] + | select(.finding_id == $finding_id) + ] + ' "$ledger")" + match_count="$(jq 'length' <<< "$matching_threads")" + [[ "$match_count" -eq 1 ]] || { + echo "expected exactly one Gator thread for $finding_id; found $match_count" >&2 + exit 1 + } + + first_body="$(jq -r '.[0].comments[0].body // empty' <<< "$matching_threads")" + [[ "$first_body" == '> **gator-agent**'* ]] || { + echo "refusing to resolve non-Gator thread for $finding_id" >&2 + exit 1 + } + + thread_id="$(jq -r '.[0].thread_id // empty' <<< "$matching_threads")" + [[ -n "$thread_id" ]] || { + echo "Gator thread ID missing for $finding_id" >&2 + exit 1 + } + + thread_ids+=("$thread_id") + resolution_states+=("$(jq -r '.[0].is_resolved' <<< "$matching_threads")") +done + +for ((i = 0; i < ${#finding_ids[@]}; i++)); do + finding_id="${finding_ids[$i]}" + thread_id="${thread_ids[$i]}" + if [[ "${resolution_states[$i]}" == "true" ]]; then + echo "Gator review thread already resolved: $finding_id" + continue + fi + + response="$(gh api graphql \ + -f query="$mutation" \ + -f threadId="$thread_id")" + jq -e --arg thread_id "$thread_id" ' + .data.resolveReviewThread.thread + | .id == $thread_id and .isResolved == true + ' <<< "$response" >/dev/null || { + echo "GitHub did not confirm resolution for $finding_id ($thread_id)" >&2 + exit 1 + } + + echo "Resolved Gator review thread: $finding_id" +done diff --git a/scripts/agents/gator/bin/resolve_gator_review_threads_test.sh b/scripts/agents/gator/bin/resolve_gator_review_threads_test.sh new file mode 100755 index 0000000000..1e38947d9a --- /dev/null +++ b/scripts/agents/gator/bin/resolve_gator_review_threads_test.sh @@ -0,0 +1,112 @@ +#!/usr/bin/env bash + +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +GATOR_DIR="$(cd "$SCRIPT_DIR/.." && pwd)" +RESOLVER="$SCRIPT_DIR/resolve-gator-review-threads" + +tmp="$(mktemp -d)" +trap 'rm -rf "$tmp"' EXIT +mkdir -p "$tmp/bin" + +cat > "$tmp/bin/gh" <<'SH' +#!/usr/bin/env bash +set -euo pipefail + +thread_id="" +previous="" +for arg in "$@"; do + if [[ "$previous" == "-f" && "$arg" == threadId=* ]]; then + thread_id="${arg#threadId=}" + fi + previous="$arg" +done + +printf '%s\n' "$*" >> "${MOCK_GH_LOG:?}" +if [[ "$thread_id" == "${MOCK_FAIL_THREAD:-}" ]]; then + jq -n --arg id "$thread_id" '{data: {resolveReviewThread: {thread: {id: $id, isResolved: false}}}}' +else + jq -n --arg id "$thread_id" '{data: {resolveReviewThread: {thread: {id: $id, isResolved: true}}}}' +fi +SH +chmod +x "$tmp/bin/gh" + +cat > "$tmp/ledger.json" <<'JSON' +{ + "schema_version": 4, + "threads": [ + { + "thread_id": "thread-open", + "finding_id": "GATOR-11111111-01", + "is_resolved": false, + "comments": [{"body": "> **gator-agent**\n\nOpen finding"}] + }, + { + "thread_id": "thread-resolved", + "finding_id": "GATOR-11111111-02", + "is_resolved": true, + "comments": [{"body": "> **gator-agent**\n\nResolved finding"}] + }, + { + "thread_id": "thread-legacy", + "finding_id": "gator-inline-1234", + "is_resolved": false, + "comments": [{"body": "> **gator-agent**\n\nLegacy finding"}] + }, + { + "thread_id": "thread-human", + "finding_id": "GATOR-22222222-01", + "is_resolved": false, + "comments": [{"body": "Human review thread"}] + } + ] +} +JSON + +export MOCK_GH_LOG="$tmp/gh.log" +PATH="$tmp/bin:$PATH" "$RESOLVER" "$tmp/ledger.json" \ + GATOR-11111111-01 GATOR-11111111-02 gator-inline-1234 \ + > "$tmp/success.out" + +[[ "$(grep -c 'threadId=thread-' "$tmp/gh.log")" -eq 2 ]] +grep -q 'ResolveGatorReviewThread' "$tmp/gh.log" +grep -q 'threadId=thread-open' "$tmp/gh.log" +grep -q 'threadId=thread-legacy' "$tmp/gh.log" +grep -q 'already resolved: GATOR-11111111-02' "$tmp/success.out" + +if PATH="$tmp/bin:$PATH" "$RESOLVER" "$tmp/ledger.json" GATOR-33333333-01 >/dev/null 2>&1; then + echo "expected unknown finding ID to fail" >&2 + exit 1 +fi + +if PATH="$tmp/bin:$PATH" "$RESOLVER" "$tmp/ledger.json" GATOR-22222222-01 >/dev/null 2>&1; then + echo "expected human-owned thread resolution to fail" >&2 + exit 1 +fi + +if MOCK_FAIL_THREAD=thread-open PATH="$tmp/bin:$PATH" \ + "$RESOLVER" "$tmp/ledger.json" GATOR-11111111-01 >/dev/null 2>&1; then + echo "expected unconfirmed GitHub resolution to fail" >&2 + exit 1 +fi + +ruby -ryaml -e ' + profile = YAML.load_file(ARGV.fetch(0)) + endpoint = profile.fetch("endpoints").find { + |entry| entry["path"] == "/graphql" + } + abort unless endpoint.fetch("rules").any? { + |rule| + allow = rule["allow"] + allow && + allow["operation_type"] == "mutation" && + allow["operation_name"] == "ResolveGatorReviewThread" && + allow["fields"] == ["resolveReviewThread"] + } +' "$GATOR_DIR/providers/github-gator.yaml" + +echo "resolve-gator-review-threads tests passed" diff --git a/scripts/agents/gator/bin/review_feedback_ledger_test.sh b/scripts/agents/gator/bin/review_feedback_ledger_test.sh index 4ac64ca95d..7c5708cc8a 100755 --- a/scripts/agents/gator/bin/review_feedback_ledger_test.sh +++ b/scripts/agents/gator/bin/review_feedback_ledger_test.sh @@ -156,7 +156,7 @@ cat > "$tmp/reviews.json" <<'JSON' "login": "drew" }, "author_association": "MEMBER", - "body": "> **gator-agent**\n\n## PR Review Status\n\nHead SHA: `1111111111111111111111111111111111111111`\nBase SHA: `aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa`\nMerge base SHA: `bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb`\nPatch ID: `cccccccccccccccccccccccccccccccccccccccc`\nGator payload: `2`\n\nGeneral findings:\n- Finding ID: GATOR-11111111-01 — Keep package verification.", + "body": "> **gator-agent**\n\n## PR Review Status\n\nGeneral findings:\n- Finding ID: GATOR-11111111-01 — Keep package verification.\n\n
\nGator metadata\n\n- Head SHA: `1111111111111111111111111111111111111111`\n- Base SHA: `aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa`\n- Merge base SHA: `bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb`\n- Patch ID: `cccccccccccccccccccccccccccccccccccccccc`\n- Gator payload: `2`\n\n
", "state": "COMMENTED", "submitted_at": "2026-07-28T19:53:23Z", "commit_id": "1111111111111111111111111111111111111111" @@ -342,12 +342,14 @@ if "$LEDGER" --input "$tmp/missing-pr.json" >/dev/null 2>&1; then fi rg -q 'COPY bin/review-feedback-ledger /usr/local/bin/review-feedback-ledger' \ - "$GATOR_DIR/Dockerfile" + "$GATOR_DIR/Dockerfile" +rg -q 'COPY bin/resolve-gator-review-threads /usr/local/bin/resolve-gator-review-threads' \ + "$GATOR_DIR/Dockerfile" rg -q 'COPY bin/validate-review-findings /usr/local/bin/validate-review-findings' \ - "$GATOR_DIR/Dockerfile" + "$GATOR_DIR/Dockerfile" ruby -ryaml -e ' manifest = YAML.load_file(ARGV.fetch(0)) - abort unless manifest.fetch("payload_version") == 4 + abort unless manifest.fetch("payload_version") == 7 resource = manifest.fetch("resources").find { |entry| entry.fetch("id") == "gator-review-findings-schema" } @@ -384,5 +386,13 @@ rg -q '`test_dispatch_required`' \ "$GATOR_DIR/skills/gator-gate/SKILL.md" rg -q 'attacker_or_operator_prerequisite' \ "$GATOR_DIR/skills/gator-gate/references/review-findings-schema.md" +rg -Fq 'Write `Summary` as natural prose that can be read aloud' \ + "$GATOR_DIR/skills/gator-gate/SKILL.md" +rg -Fq 'Agent context' \ + "$GATOR_DIR/skills/gator-gate/SKILL.md" +rg -Fq 'Gator metadata' \ + "$GATOR_DIR/skills/gator-gate/SKILL.md" +rg -Fq 'Those fields validate the finding;' \ + "$GATOR_DIR/skills/gator-gate/SKILL.md" printf 'PASS: gator review feedback ledger tests\n' diff --git a/scripts/agents/gator/prompts/gator.md b/scripts/agents/gator/prompts/gator.md index 96d8dff5ec..07055a29cc 100644 --- a/scripts/agents/gator/prompts/gator.md +++ b/scripts/agents/gator/prompts/gator.md @@ -11,7 +11,10 @@ Load and follow this skill exactly: Important sandbox constraints: - GitHub REST write access is scoped to NVIDIA/OpenShell and NVIDIA/OpenShell-Community. -- GitHub GraphQL access is read-only. Prefer REST endpoints for write actions and use GraphQL-backed `gh` reads when useful. +- GitHub GraphQL access is read-only except for the named + `ResolveGatorReviewThread` mutation used by + `resolve-gator-review-threads`. Prefer REST endpoints for all other write + actions and use GraphQL-backed `gh` reads when useful. - Keep watching active PRs until they close, merge, or the operator stops the sandbox. - At the start of every watch cycle, read `payload_version` from `scripts/agents/gator/agent.yaml` on the default branch through the GitHub diff --git a/scripts/agents/gator/providers/github-gator.yaml b/scripts/agents/gator/providers/github-gator.yaml index 654254971f..d0c1f32d49 100644 --- a/scripts/agents/gator/providers/github-gator.yaml +++ b/scripts/agents/gator/providers/github-gator.yaml @@ -65,6 +65,10 @@ endpoints: rules: - allow: operation_type: query + - allow: + operation_type: mutation + operation_name: ResolveGatorReviewThread + fields: [resolveReviewThread] - host: github.com port: 443 protocol: rest diff --git a/scripts/agents/gator/skills/gator-gate/SKILL.md b/scripts/agents/gator/skills/gator-gate/SKILL.md index c7a1dcab4b..99ab8304f5 100644 --- a/scripts/agents/gator/skills/gator-gate/SKILL.md +++ b/scripts/agents/gator/skills/gator-gate/SKILL.md @@ -138,6 +138,11 @@ Treat the ledger as required reviewer input, not optional background: - Carry every still-open finding forward as an existing obligation. Do not post a new thread or semantically equivalent general finding for it. - A Gator thread resolved by a verified maintainer is addressed. If the resolver is only the PR author, inspect the trusted reply and latest diff to decide whether the finding was fixed; resolution alone does not grant a non-maintainer author waiver authority. +- When a later commit demonstrably fixes an open inline Gator finding, resolve + that Gator-owned GitHub review thread as part of the same reconciliation + cycle. Also resolve it after a verified maintainer explicitly waives the + finding. Do not resolve a thread merely because it is outdated, the author + says it is fixed, or a new review omits it. - Preserve a verified maintainer's reply as the rationale. An explicit rejection such as "invalid", "intentional", "fine as implemented", or "won't fix" is a waiver, not an unanswered request. - An unresolved thread with an explicit verified-maintainer waiver is also waived. A non-maintainer author's disagreement remains context for review but does not override a maintainer-required change. - Preserve each `GATOR--` finding ID across later @@ -152,6 +157,33 @@ Treat the ledger as required reviewer input, not optional background: nonzero duplicate finding-ID count, a waived finding reappearing, or an unchanged-code proposal as a reviewer-quality signal, not an author defect. +### Resolve addressed Gator threads + +After checking the new diff and trusted replies, partition every open ledger +finding into still-open, fixed by the current head, or explicitly waived by a +verified maintainer. Before posting the current-head disposition or advancing +out of review, resolve each fixed or waived inline Gator thread by stable +finding ID: + +```bash +resolve-gator-review-threads \ + /tmp/gator-review-feedback-ledger.json \ + GATOR-- [...] +``` + +The resolver accepts only exact threads from the feedback ledger whose first +comment carries the Gator marker. Never resolve a human-owned review thread, a +still-open finding, or a thread whose disposition is ambiguous. General +findings and issue comments have no review thread to resolve; retain their +durable disposition in the ledger. + +Thread resolution is required housekeeping for an addressed inline finding, +not a new review disposition, and does not consume the one-disposition-per-head +SHA slot. If GitHub does not confirm every requested resolution, do not advance +the Gator state or post the follow-up disposition. Return +`OPENSHELL_AGENT_RESULT {"status":"transient_failure","next_poll_seconds":120,"reason":"review_thread_resolution_failed"}` +and retry the reconciliation later. + ## Labels There must be at most one `gator:*` label on an issue or PR at any time. @@ -746,10 +778,50 @@ Post findings using these rules: - Submit all inline comments for a head SHA together in one `COMMENT` review. The review summary plus its complete inline-comment batch is the single gator disposition for that SHA. - Begin the review summary and each inline body with `> **gator-agent**`. Put the current head SHA in the summary using the canonical `Head SHA: ` field. - Put the stable finding ID in every blocking summary item and inline comment. -- In each blocker, state reachability, impact, why the PR owns the problem, and - the proportionate requested change. Also state the prerequisite, supported - entry point and sink, base-vs-head behavior, and deterministic reproducer - from the validated evidence contract. +- Keep the normalized evidence contract in `review-findings.json`; do not copy + its field labels into the public comment. Render each new blocker as the + human-first card below. +- Write `Summary` as natural prose that can be read aloud to someone who has + not seen the code. In one compact paragraph, explain who encounters the + problem, what they do, what goes wrong, and the concrete consequence. Prefer + product concepts over internal identifiers unless an identifier is needed + to understand the issue. +- Write `Fix` as a proportionate imperative derived from `requested_change`. + Retain every operative constraint and required regression-test outcome. +- Write `Verify` as a deterministic setup, action, and current-versus-expected + observation derived from `reproducer`. Preserve identity and equality + qualifiers and distinguish concurrent calls, staged objects, and durable + state when those distinctions matter. +- Across the visible card and its agent context, preserve the operative facts + needed to understand, fix, and verify the blocker: exact trigger conditions, + actor or permission scope, supported-path qualifiers, causal direction, + observable impact, relevant identifiers, and negations. Never move a read, + transform, store, forward, reject, or delete operation to a neighboring + component for brevity. Do not infer missing facts. +- Do not expose `Invariant`, `Prerequisite`, `Entry point`, `Sink`, `Base`, + `Head`, or `PR ownership` as headings. Those fields validate the finding; + they are not the public explanation. +- Add collapsed `Agent context` after the visible card. Always include the + validated `changed_location` as `path:line`. Include the exact execution + path, PR ownership, minimal historical contrast, or concrete sibling sites + only when that information is not already clear above and helps an agent act + or establishes otherwise-unclear PR causality. Do not add empty rows or + duplicate the visible prose. Most findings need no historical comparison. +- Target roughly 130 total words and about half the legacy evidence-dump + length. Operative fidelity is the escape hatch: exceed the target rather + than make the defect, fix, or verification ambiguous. +- Put review-process bookkeeping at the end of the review summary in one + collapsed `
Gator metadata` block. This includes + validation provenance, docs and E2E disposition, check status, head/base/ + merge-base SHAs, patch ID, payload version, review mode, previous reviewed + SHA, review-budget state, maintainer-decision state, next Gator state, and a + machine-readable blocked reason. Keep the canonical `Head SHA: ` and + `Gator payload: ` text intact inside the block so the write guard + and feedback ledger can parse them. +- Do not hide an action a person must take. When blocked, approval-needed, or + awaiting clarification, put one plain-language `Action required:` sentence + in the visible summary. Keep the redundant state code and operational detail + in `Gator metadata`. - Use the review summary for blocking design concerns, missing tests, cross-file findings, and blockers that cannot be anchored because the relevant line is outside the mode-appropriate diff. For an unanchored @@ -770,13 +842,13 @@ Build the batch as one REST request. Verify every requested line appears in the { "commit_id": "", "event": "COMMENT", - "body": "> **gator-agent**\n\n## PR Review Status\n\nHead SHA: ``\nBase SHA: ``\nMerge base SHA: ``\nPatch ID: ``\nGator payload: ``\n\n", + "body": "> **gator-agent**\n\n## PR Review Status\n\n\n\n
\nGator metadata\n\n- Head SHA: ``\n- Base SHA: ``\n- Merge base SHA: ``\n- Patch ID: ``\n- Gator payload: ``\n\n
", "comments": [ { "path": "crates/example/src/lib.rs", "line": 123, "side": "RIGHT", - "body": "> **gator-agent**\n\n**Warning — GATOR-12345678-01**\n\nInvariant: \n\nPrerequisite: \n\nEntry point → sink: → \n\nBase → head: → \n\nImpact: \n\nReproducer: \n\nPR ownership: \n\nRequested change: " + "body": "> **gator-agent**\n\n**Warning — GATOR-12345678-01 · Authorization happens after lookup**\n\n**Summary:** A user who can request a workspace can make the server look up another workspace before checking access. That discloses whether the workspace exists.\n\n**Fix:** Restore the authorization check before the workspace lookup and cover cross-workspace requests.\n\n**Verify:** Request another workspace as an unauthorized user; the server must return 404 without performing the lookup.\n\n
\nAgent context\n\n- **Agent path:** `GET /workspaces/{name}` → workspace record lookup\n- **Ownership:** This PR reordered the authorization check and lookup.\n- **Location:** `server.rs:123`\n\n
" } ] } @@ -804,12 +876,14 @@ determine which Gator findings or trusted maintainer comments are still unanswered. Ignore unacknowledged third-party comments and reviews. If the PR author has pushed commits and `review_scope.mode` is `follow_up`, review only the unresolved obligations plus `..HEAD`, carrying all -other dispositions without duplicating them. If the author replied without -pushing a new commit, do not re-review, repost findings, or post a same-SHA -disposition; inspect the response internally and wait for a new commit or -maintainer override. If CI changes state without a new commit, do not post a -same-SHA CI update. A due TTL author nudge remains allowed when the unresolved -feedback still requires an author action. +other dispositions without duplicating them. Resolve the Gator-owned inline +threads for obligations that the follow-up confirms are fixed or explicitly +waived before posting the new-head disposition or advancing the label. If the +author replied without pushing a new commit, do not re-review, repost findings, +or post a same-SHA disposition; inspect the response internally and wait for a +new commit or maintainer override. If CI changes state without a new commit, do +not post a same-SHA CI update. A due TTL author nudge remains allowed when the +unresolved feedback still requires an author action. If review feedback is waiting on the PR author for more than 48 business hours, post a single author nudge. Use the latest of these timestamps as the TTL start: @@ -986,16 +1060,9 @@ Recommended next step: . ## PR Review Status -Validation: -Head SHA: `` -Base SHA: `` -Merge base SHA: `` -Patch ID: `` -Gator payload: `` -Review mode: `` -Previous reviewed SHA: `` -Review budget exhausted: `` -Maintainer decision required: `` + + +Action required: Blocking findings: - ``: @@ -1006,9 +1073,26 @@ Carried findings: Non-blocking suggestions: - -Docs: - -Next state: `` +
+Gator metadata + +- Validation: +- Docs: +- Checks: +- E2E: +- Head SHA: `` +- Base SHA: `` +- Merge base SHA: `` +- Patch ID: `` +- Gator payload: `` +- Review mode: `` +- Previous reviewed SHA: `` +- Review budget exhausted: `` +- Maintainer decision required: `` +- Next state: `` +- Blocked reason: `` + +
``` ### Maintainer Convergence Decision @@ -1018,12 +1102,6 @@ Next state: `` -Base SHA: `` -Merge base SHA: `` -Patch ID: `` -Gator payload: `` - The autonomous Warning budget is exhausted, and a specific maintainer decision is required before review can proceed. @@ -1039,8 +1117,18 @@ Reviewer-quality signals: Maintainer action: -Next state: `gator:blocked` -Blocked reason: `review_convergence_decision_required` +
+Gator metadata + +- Head SHA: `` +- Base SHA: `` +- Merge base SHA: `` +- Patch ID: `` +- Gator payload: `` +- Next state: `gator:blocked` +- Blocked reason: `review_convergence_decision_required` + +
``` ### Human Response Disposition @@ -1054,12 +1142,6 @@ Post this as a new comment after a substantive author, maintainer, or reviewer r Thanks . I re-evaluated latest head `` after your comment about . -Head SHA: `` -Base SHA: `` -Merge base SHA: `` -Patch ID: `` -Gator payload: `` - What I checked: . Disposition: . @@ -1067,7 +1149,20 @@ Disposition: -Next state: `` +Action required: + +
+Gator metadata + +- Head SHA: `` +- Base SHA: `` +- Merge base SHA: `` +- Patch ID: `` +- Gator payload: `` +- Next state: `` +- Blocked reason: `` + +
``` ### Approval Needed @@ -1079,13 +1174,20 @@ Next state: ` Review: -Docs: -Checks: -E2E: Human maintainer approval is now required. + +
+Gator metadata + +- Validation: +- Docs: +- Checks: +- E2E: +- Next state: `gator:approval-needed` + +
``` ### Merge Ready @@ -1097,14 +1199,21 @@ Human maintainer approval is now required. Gator validation and PR monitoring are complete, and maintainer approval is present. -Validation: Review: Approval: -Docs: -Checks: -E2E: Human maintainer merge or close decision is now required. + +
+Gator metadata + +- Validation: +- Docs: +- Checks: +- E2E: +- Next state: `gator:merge-ready` + +
``` ### Monitoring Complete diff --git a/scripts/agents/gator/skills/gator-gate/references/review-findings-schema.md b/scripts/agents/gator/skills/gator-gate/references/review-findings-schema.md index b9e267ae2e..b65f40f5be 100644 --- a/scripts/agents/gator/skills/gator-gate/references/review-findings-schema.md +++ b/scripts/agents/gator/skills/gator-gate/references/review-findings-schema.md @@ -60,6 +60,11 @@ Critical or Warning missing any evidence field becomes a non-blocking list sibling sites on the first finding instead. Suggestions always remain non-blocking. +These fields are Gator's internal evidence contract, not the public comment +format. After validation, render blockers using the human-first +`Summary`/`Fix`/`Verify` card and conditional `Agent context` defined in the +gator-gate skill. Do not expose the schema's evidence labels as prose headings. + For a finite family, put every known member in `sibling_sites` under one invariant and one finding ID. On later rounds, update that finding instead of creating a sibling finding. diff --git a/scripts/agents/run.sh b/scripts/agents/run.sh index 8c5fd6e1ae..4bd868ea3d 100755 --- a/scripts/agents/run.sh +++ b/scripts/agents/run.sh @@ -324,18 +324,51 @@ upsert_provider() { fi } +provider_exists() { + openshell_cmd provider get "$1" >/dev/null 2>&1 +} + import_provider_profile() { local profile_id="$1" local profile_file="$2" - local import_output + local import_output current_profile resource_version update_dir update_file openshell_cmd provider profile delete "$profile_id" >/dev/null 2>&1 || true if import_output="$(openshell_cmd provider profile import --file "$profile_file" 2>&1)"; then return 0 fi if [[ "$import_output" == *"already exists"* ]]; then - echo "Provider profile already exists: $profile_file" - return 0 + if ! current_profile="$(openshell_cmd provider profile export \ + --output json "$profile_id")"; then + echo "failed to export existing provider profile: $profile_id" >&2 + return 1 + fi + resource_version="$(printf '%s' "$current_profile" | \ + jq -r '.resource_version // 0')" + [[ "$resource_version" =~ ^[1-9][0-9]*$ ]] || { + echo "existing provider profile has no resource version: $profile_id" >&2 + return 1 + } + + update_dir="$(mktemp -d "${TMPDIR:-/tmp}/openshell-provider-profile-XXXXXX")" + update_file="$update_dir/profile.yaml" + ruby -ryaml - "$profile_file" "$resource_version" "$update_file" <<'RUBY' +profile_file, resource_version, update_file = ARGV +profile = YAML.load_file(profile_file) || {} +profile["resource_version"] = Integer(resource_version, 10) +File.write(update_file, YAML.dump(profile)) +RUBY + if openshell_cmd provider profile update "$profile_id" \ + --file "$update_file" >/dev/null; then + rm -f "$update_file" + rmdir "$update_dir" + echo "Updated provider profile: $profile_file" + return 0 + fi + rm -f "$update_file" + rmdir "$update_dir" + echo "failed to update existing provider profile: $profile_id" >&2 + return 1 fi printf '%s\n' "$import_output" >&2 @@ -670,6 +703,7 @@ for ((provider_index = 0; provider_index < PROVIDER_COUNT; provider_index++)); d mode_var="PROVIDER_${provider_index}_CREDENTIAL_MODE" credential_count_var="PROVIDER_${provider_index}_CREDENTIAL_COUNT" refresh_enabled_var="PROVIDER_${provider_index}_REFRESH_ENABLED" + refresh_key_var="PROVIDER_${provider_index}_REFRESH_CREDENTIAL_KEY" provider_name="${!name_var}" profile_id="${!profile_var}" credential_mode="${!mode_var}" @@ -700,18 +734,32 @@ for ((provider_index = 0; provider_index < PROVIDER_COUNT; provider_index++)); d fi done + provider_credential_args=() case "$credential_mode" in explicit) - upsert_provider "$provider_name" "$profile_id" "${credential_args[@]}" + provider_credential_args=("${credential_args[@]}") ;; from_existing) - upsert_provider "$provider_name" "$profile_id" --from-existing + provider_credential_args=(--from-existing) ;; *) fail "unsupported credential_mode for $provider_name: $credential_mode" ;; esac + if [[ "${!refresh_enabled_var}" == "true" ]] && provider_exists "$provider_name"; then + if [[ "$RESET_REFRESH" == "1" ]]; then + log "Resetting refresh-owned provider credential '$provider_name/${!refresh_key_var}'." + openshell_cmd provider refresh delete "$provider_name" \ + --credential-key "${!refresh_key_var}" >/dev/null + upsert_provider "$provider_name" "$profile_id" "${provider_credential_args[@]}" + else + log "Reusing provider '$provider_name' because its credentials are managed by gateway refresh." + fi + else + upsert_provider "$provider_name" "$profile_id" "${provider_credential_args[@]}" + fi + if [[ "${!refresh_enabled_var}" == "true" ]]; then log "Refreshing provider credential '$provider_name/${!refresh_key_var}'." configure_provider_refresh "$provider_index" @@ -737,7 +785,7 @@ case "$HARNESS" in ;; esac -SANDBOX_CMD=( +SANDBOX_CREATE_CMD=( env -u OPENSHELL_SANDBOX_POLICY "$OPENSHELL_BIN" --gateway "$GATEWAY" sandbox create --name "$SANDBOX_NAME" @@ -747,11 +795,35 @@ SANDBOX_CMD=( --no-git-ignore --no-auto-providers --no-tty + --detach ) -if [[ "$KEEP_SANDBOX" != "1" ]]; then - SANDBOX_CMD+=(--no-keep) -fi -SANDBOX_CMD+=(-- env "${HARNESS_ENV_ARGS[@]}" bash "$PAYLOAD_IMAGE_DIR/runtime/entrypoint.sh") + +SANDBOX_EXEC_CMD=( + "$OPENSHELL_BIN" --gateway "$GATEWAY" sandbox exec + --name "$SANDBOX_NAME" + --no-tty + -- + env + "${HARNESS_ENV_ARGS[@]}" + bash "$PAYLOAD_IMAGE_DIR/runtime/entrypoint.sh" +) + +run_agent_sandbox() { + local exec_status=0 + local cleanup_status=0 + + "${SANDBOX_CREATE_CMD[@]}" + "${SANDBOX_EXEC_CMD[@]}" || exec_status=$? + + if [[ "$KEEP_SANDBOX" != "1" ]]; then + openshell_cmd sandbox delete "$SANDBOX_NAME" >/dev/null || cleanup_status=$? + fi + + if [[ "$exec_status" -ne 0 ]]; then + return "$exec_status" + fi + return "$cleanup_status" +} log "Launching $AGENT_DISPLAY_NAME sandbox '$SANDBOX_NAME' on gateway '$GATEWAY'." if [[ "$BACKGROUND" == "1" ]]; then @@ -761,9 +833,9 @@ if [[ "$BACKGROUND" == "1" ]]; then trap - EXIT ( trap 'cleanup_config; cleanup_payload' EXIT - "${SANDBOX_CMD[@]}" + run_agent_sandbox ) >"$LOG_FILE" 2>&1 & echo "Started in background. Log: $LOG_FILE" else - "${SANDBOX_CMD[@]}" + run_agent_sandbox fi diff --git a/scripts/agents/runtime/harnesses/codex/exec.sh b/scripts/agents/runtime/harnesses/codex/exec.sh index a2eb5be909..ef92258795 100755 --- a/scripts/agents/runtime/harnesses/codex/exec.sh +++ b/scripts/agents/runtime/harnesses/codex/exec.sh @@ -26,7 +26,7 @@ export GH_NO_UPDATE_NOTIFIER=1 export GH_NO_EXTENSION_UPDATE_NOTIFIER=1 export GH_TELEMETRY=false export DO_NOT_TRACK=1 -export HOME=/sandbox/home +export HOME="${OPENSHELL_AGENT_HOME:-/sandbox/home}" echo "openshell-agent: preparing Codex harness auth and workspace" >&2 mkdir -p "$HOME/.codex" @@ -34,13 +34,9 @@ node - <<'NODE' const fs = require("fs"); const path = `${process.env.HOME}/.codex/auth.json`; const b64u = (obj) => Buffer.from(JSON.stringify(obj)).toString("base64url"); -const providerPlaceholder = (envName) => { - const value = process.env[envName]; - if (value && value.startsWith("openshell:resolve:env:")) { - return `openshell:resolve:env:${envName}`; - } - return value; -}; +// Preserve gateway-issued revision and stable-handle placeholders verbatim. +// Endpoint-bound credentials reject identityless aliases by design. +const providerValue = (envName) => process.env[envName]; const now = Math.floor(Date.now() / 1000); const fallbackIdToken = [ b64u({ alg: "none", typ: "JWT" }), @@ -59,10 +55,10 @@ fs.writeFileSync(path, JSON.stringify({ auth_mode: "chatgpt", OPENAI_API_KEY: null, tokens: { - id_token: providerPlaceholder("CODEX_AUTH_ID_TOKEN") || fallbackIdToken, - access_token: providerPlaceholder("CODEX_AUTH_ACCESS_TOKEN"), - refresh_token: providerPlaceholder("CODEX_AUTH_REFRESH_TOKEN") || "gateway-managed-refresh-token", - account_id: providerPlaceholder("CODEX_AUTH_ACCOUNT_ID"), + id_token: providerValue("CODEX_AUTH_ID_TOKEN") || fallbackIdToken, + access_token: providerValue("CODEX_AUTH_ACCESS_TOKEN"), + refresh_token: providerValue("CODEX_AUTH_REFRESH_TOKEN") || "gateway-managed-refresh-token", + account_id: providerValue("CODEX_AUTH_ACCOUNT_ID"), }, last_refresh: new Date().toISOString(), }, null, 2)); diff --git a/scripts/agents/runtime/harnesses/codex/exec_test.sh b/scripts/agents/runtime/harnesses/codex/exec_test.sh new file mode 100755 index 0000000000..6f1b998217 --- /dev/null +++ b/scripts/agents/runtime/harnesses/codex/exec_test.sh @@ -0,0 +1,39 @@ +#!/usr/bin/env bash + +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +TMP_DIR="$(mktemp -d)" +trap 'rm -rf "$TMP_DIR"' EXIT + +ACCESS_PLACEHOLDER="openshell:resolve:env:s$(printf 'a%.0s' {1..64})_CODEX_AUTH_ACCESS_TOKEN" +ACCOUNT_PLACEHOLDER="openshell:resolve:env:v42_CODEX_AUTH_ACCOUNT_ID" + +cat > "$TMP_DIR/codex" <<'MOCK' +#!/usr/bin/env bash +set -euo pipefail + +if [[ "${1:-}" == "exec" && "${2:-}" == "--help" ]]; then + exit 0 +fi + +access="$(jq -r '.tokens.access_token' "$HOME/.codex/auth.json")" +account="$(jq -r '.tokens.account_id' "$HOME/.codex/auth.json")" +[[ "$access" == "$EXPECTED_ACCESS_PLACEHOLDER" ]] +[[ "$account" == "$EXPECTED_ACCOUNT_PLACEHOLDER" ]] +printf '%s\n' 'ok - Codex auth preserves opaque provider placeholders' +MOCK +chmod +x "$TMP_DIR/codex" +printf '%s\n' 'test prompt' > "$TMP_DIR/prompt.md" + +OPENSHELL_AGENT_HOME="$TMP_DIR/home" \ +CODEX_BIN="$TMP_DIR/codex" \ +CODEX_AUTH_ACCESS_TOKEN="$ACCESS_PLACEHOLDER" \ +CODEX_AUTH_ACCOUNT_ID="$ACCOUNT_PLACEHOLDER" \ +GITHUB_TOKEN="openshell:resolve:env:v42_GITHUB_TOKEN" \ +EXPECTED_ACCESS_PLACEHOLDER="$ACCESS_PLACEHOLDER" \ +EXPECTED_ACCOUNT_PLACEHOLDER="$ACCOUNT_PLACEHOLDER" \ + bash "$SCRIPT_DIR/exec.sh" "$TMP_DIR/prompt.md"