diff --git a/.github/AGENTS.md b/.github/AGENTS.md index 57f8a595..9e2e88d1 100644 --- a/.github/AGENTS.md +++ b/.github/AGENTS.md @@ -17,7 +17,7 @@ - Run offline routing, transport, input, and report tests, including the complete PR-flow integration, with `uv run --project projects/openshell-agent-runner pytest tests/test_ci_scope.py tests/test_github_api.py tests/test_*review*.py`. -- Keep the live pipeline smoke focused on execution contracts, not expected +- Keep the live integration focused on execution contracts, not expected reviewer verdicts. It never posts a PR assessment. - Run CI profile contract tests in the OAR environment. - See `docs/development/ci.md` for triggers, trust boundaries, and local checks. diff --git a/.github/actions/setup-review-gateway/action.yml b/.github/actions/setup-review-gateway/action.yml index 3908f7e9..5aab5741 100644 --- a/.github/actions/setup-review-gateway/action.yml +++ b/.github/actions/setup-review-gateway/action.yml @@ -16,7 +16,8 @@ runs: - name: Install OpenShell shell: bash run: | - curl -LsSf https://raw.githubusercontent.com/NVIDIA/OpenShell/v0.0.116/install.sh \ + # OpenShell v0.0.116, resolved to an immutable commit. + curl -LsSf https://raw.githubusercontent.com/NVIDIA/OpenShell/d1155aa70042d3e2ee49dbfa15346b108b7c1d92/install.sh \ | OPENSHELL_VERSION=v0.0.116 sh echo "$HOME/.local/bin" >> "$GITHUB_PATH" - name: Wait for the gateway diff --git a/.github/scripts/pr_review.py b/.github/scripts/pr_review.py index 90d29732..b1ca9a96 100644 --- a/.github/scripts/pr_review.py +++ b/.github/scripts/pr_review.py @@ -14,6 +14,7 @@ import yaml from ci_scope import new_project_paths, project_task from github_api import GitHub, GitHubError +from review_report import find_existing_report def resolve_request(github, context): @@ -24,12 +25,13 @@ def resolve_request(github, context): pr = github.request("GET", f"pulls/{number}") if ( pr["state"] != "open" - or pr["draft"] or pr["head"]["sha"] != payload_pr["head"]["sha"] or (pr["head"].get("repo") or {}).get("full_name") != github.repository or pr["user"]["login"] == "dependabot[bot]" ): return None + if pr["draft"]: + return _retirement_request(github, pr) files = github.paginate(f"pulls/{number}/files?per_page=100") if len(files) != pr["changed_files"]: @@ -56,7 +58,7 @@ def resolve_request(github, context): except ValueError as error: errors.append(str(error)) if not tasks and not errors: - return None + return _retirement_request(github, pr) return { "number": number, "head": pr["head"]["sha"], @@ -93,7 +95,10 @@ def main(): args.output.parent.mkdir(parents=True, exist_ok=True) args.output.write_text(json.dumps(request, indent=2) + "\n", encoding="utf-8") outputs = {key: request[key] for key in ("number", "head", "base")} - outputs.update(ready=str(not request["reason"]).lower(), tooling=tooling) + outputs.update( + ready=str(bool(request["tasks"]) and not request["reason"]).lower(), + tooling=tooling, + ) with Path(os.environ["GITHUB_OUTPUT"]).open("a", encoding="utf-8") as output: output.writelines(f"{key}={value}\n" for key, value in outputs.items()) if request["reason"]: @@ -122,5 +127,21 @@ def _read_metadata(github, path, revision): raise ValueError(f"Invalid YAML in {filename}.") from error +def _retirement_request(github, pr): + comments = github.paginate(f"issues/{pr['number']}/comments") + if find_existing_report(comments) is None: + return None + return { + "number": pr["number"], + "head": pr["head"]["sha"], + "base": pr["base"]["sha"], + "title": pr["title"], + "description": pr.get("body") or "", + "tasks": [], + "reason": "", + "retire": True, + } + + if __name__ == "__main__": main() diff --git a/.github/scripts/review_report.py b/.github/scripts/review_report.py index f478c48d..4f458cb1 100644 --- a/.github/scripts/review_report.py +++ b/.github/scripts/review_report.py @@ -8,9 +8,11 @@ import os import re import sys -from pathlib import Path +from pathlib import Path, PurePosixPath +from urllib.parse import quote REVIEW_MARKER = "" +REPORT_AUTHOR = "github-actions[bot]" VERDICTS = { "pass": "✅ Pass", "needs_changes": "⚠️ Needs changes", @@ -82,6 +84,7 @@ def read_results(directory, tasks): def render_report(*, request, reviews, run_url, run_id, outcome): reason = f" — {_escape_text(request['reason'])}" if request.get("reason") else "" + source_url = f"{run_url.partition('/actions/')[0]}/blob/{request['head']}" lines = [ REVIEW_MARKER, f"", @@ -89,7 +92,7 @@ def render_report(*, request, reviews, run_url, run_id, outcome): "", f"Revision: `{request['head']}` · [Workflow and result artifacts]({run_url})", "", - "Review findings and scores are advisory. Required checks remain separate merge gates.", + "Review findings are advisory. Required checks remain separate merge gates.", "", f"Execution: **{_escape_text(outcome)}**{reason}", "", @@ -101,17 +104,17 @@ def render_report(*, request, reviews, run_url, run_id, outcome): [ "### Reviews", "", - "| Project | Verdict | Guidelines | Score | Findings |", - "| --- | --- | --- | ---: | ---: |", + "| Project | Verdict | Guidelines | Findings |", + "| --- | --- | --- | ---: |", ] ) for review in reviews: label = _escape_text(review["label"]) result = review.get("result") lines.append( - f"| {label} | {VERDICTS[result['verdict']]} | {VERDICTS[result['guidelines_assessment']['verdict']]} | {result['overall_score']}/100 | {len(result['findings'])} |" + f"| {label} | {VERDICTS[result['verdict']]} | {VERDICTS[result['guidelines_assessment']['verdict']]} | {len(result['findings'])} |" if result - else f"| {label} | Not completed | — | — | — |" + else f"| {label} | Not completed | — | — |" ) for review in reviews: lines.extend( @@ -137,25 +140,19 @@ def render_report(*, request, reviews, run_url, run_id, outcome): f"**Project guidelines: {VERDICTS[result['guidelines_assessment']['verdict']]}**", "", _escape_text(result["guidelines_assessment"]["explanation"]), - "", - "| Criterion | Score | Rationale |", - "| --- | ---: | --- |", ] ) - for item in result["criterion_scores"]: - lines.append( - f"| {_escape_text(item.get('criterion'))} | {item['score']} | {_escape_text(item.get('explanation'))} |" - ) lines.extend(["", "#### Findings", ""]) if not result["findings"]: lines.append("No actionable findings.") for finding in result["findings"]: - location = finding.get("path") or review["label"] - line = f":{finding['line']}" if finding.get("line") else "" + location = _finding_location( + finding, review["label"], source_url=source_url + ) evidence = finding["evidence"] lines.extend( [ - f"- **{_escape_text(finding.get('severity'))}: {_escape_text(finding.get('title'))}** — {_escape_text(location)}{line}", + f"- **{_escape_text(finding.get('severity'))}: {_escape_text(finding.get('title'))}** — {location}", f" - Evidence: {_escape_text(evidence)}", f" - Recommendation: {_escape_text(finding.get('recommendation'))}", ] @@ -182,18 +179,14 @@ def render_report(*, request, reviews, run_url, run_id, outcome): def publish_report(github, request, body, run_id): number = request["number"] pr = github.request("GET", f"pulls/{number}") - if pr["state"] != "open" or pr["head"]["sha"] != request["head"]: + if ( + pr["state"] != "open" + or pr["head"]["sha"] != request["head"] + or (pr.get("draft") and not request.get("retire")) + ): return False comments = github.paginate(f"issues/{number}/comments") - existing = next( - ( - comment - for comment in comments - if (comment.get("user") or {}).get("type") == "Bot" - and REVIEW_MARKER in (comment.get("body") or "") - ), - None, - ) + existing = find_existing_report(comments) previous_run = ( re.search(r"", existing["body"]) if existing @@ -201,6 +194,11 @@ def publish_report(github, request, body, run_id): ) if previous_run and int(previous_run.group(1)) > int(run_id): return False + if request.get("retire"): + if not existing: + return False + github.request("DELETE", f"issues/comments/{existing['id']}") + return True if existing: github.request("PATCH", f"issues/comments/{existing['id']}", {"body": body}) else: @@ -208,6 +206,19 @@ def publish_report(github, request, body, run_id): return True +def find_existing_report(comments): + return next( + ( + comment + for comment in comments + if (comment.get("user") or {}).get("type") == "Bot" + and (comment.get("user") or {}).get("login") == REPORT_AUTHOR + and REVIEW_MARKER in (comment.get("body") or "") + ), + None, + ) + + def main(argv=None): parser = argparse.ArgumentParser(description=__doc__) commands = parser.add_subparsers(dest="command", required=True) @@ -280,5 +291,16 @@ def _escape_text(value): return re.sub(r"\r?\n", " ", text) +def _finding_location(finding, fallback, *, source_url): + path = finding.get("path") or fallback + line = finding.get("line") + display = f"{path}:{line}" if line else path + candidate = PurePosixPath(path) + if candidate.is_absolute() or ".." in candidate.parts or str(candidate) != path: + return _escape_text(display) + anchor = f"#L{line}" if line else "" + return f"[{_escape_text(display)}]({source_url}/{quote(path, safe='/')}{anchor})" + + if __name__ == "__main__": sys.exit(main()) diff --git a/.github/workflows/oar-checks.yml b/.github/workflows/oar-checks.yml index 3aedf4cb..8ffc7757 100644 --- a/.github/workflows/oar-checks.yml +++ b/.github/workflows/oar-checks.yml @@ -1,10 +1,12 @@ -name: OAR and PR review checks +name: OAR functional checks "on": pull_request: paths: - .pre-commit-config.yaml - - .github/workflows/** + - .github/workflows/oar-checks.yml + - .github/workflows/oar-integration.yml + - .github/workflows/pr-review.yml - .github/openshell-agents/** - .github/scripts/** - projects/PROJECT_GUIDELINES.md @@ -18,7 +20,9 @@ name: OAR and PR review checks - main paths: - .pre-commit-config.yaml - - .github/workflows/** + - .github/workflows/oar-checks.yml + - .github/workflows/oar-integration.yml + - .github/workflows/pr-review.yml - .github/openshell-agents/** - .github/scripts/** - projects/PROJECT_GUIDELINES.md @@ -37,13 +41,10 @@ concurrency: cancel-in-progress: true jobs: - check: - name: Check OAR and PR review (Python ${{ matrix.python-version }}) + checks: + name: Check OAR and review behavior runs-on: ubuntu-latest - strategy: - fail-fast: false - matrix: - python-version: ["3.12", "3.13", "3.14"] + timeout-minutes: 10 steps: - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -54,95 +55,118 @@ jobs: uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1 with: version: "0.12.5" - python-version: ${{ matrix.python-version }} + python-version: "3.12" + enable-cache: true + cache-dependency-glob: projects/openshell-agent-runner/uv.lock - - name: Configure isolated uv paths - run: | - echo "UV_CACHE_DIR=$RUNNER_TEMP/oar-checks-uv-cache" >> "$GITHUB_ENV" - echo "UV_PROJECT_ENVIRONMENT=$RUNNER_TEMP/oar-checks-venv" >> "$GITHUB_ENV" - - - name: Install locked dependencies - run: uv sync --project projects/openshell-agent-runner --locked - - - name: Validate agent profiles - run: | - uv run --project projects/openshell-agent-runner oar validate \ - .github/openshell-agents/profiles/ci-reviewer - for profile in code-reviewer technical-writing-reviewer; do - uv run --project projects/openshell-agent-runner oar validate \ - "projects/openshell-agent-runner/src/openshell_agent_runner/profiles/$profile" - done + - name: Isolate the project environment + run: echo "UV_PROJECT_ENVIRONMENT=$RUNNER_TEMP/oar-checks-venv" >> "$GITHUB_ENV" - - name: Preview agent execution + - name: Run project checks + working-directory: projects/openshell-agent-runner run: | - mkdir -p "$RUNNER_TEMP/review-context" - cp projects/PROJECT_GUIDELINES.md "$RUNNER_TEMP/review-context/PROJECT_GUIDELINES.md" - uv run --project projects/openshell-agent-runner oar run \ - .github/openshell-agents/profiles/ci-reviewer \ - --task review-tool \ - --gateway openshell \ - --input projects/openshell-agent-runner \ - --upload "$RUNNER_TEMP/review-context:/workspace" \ - --prompt-var guidelines_path=/workspace/review-context/PROJECT_GUIDELINES.md \ - --output "$RUNNER_TEMP/project-review.json" \ - --dry-run + uv run --frozen pre-commit validate-config ../../.pre-commit-config.yaml + make check - - name: Test CI profile, routing, and reports + - name: Test PR selection, review execution, and reporting run: | - uv run --project projects/openshell-agent-runner pytest \ + uv run --project projects/openshell-agent-runner --frozen pytest \ tests/test_ci_scope.py tests/test_github_api.py tests/test_*review*.py - - name: Validate agent workflow syntax - if: matrix.python-version == '3.12' + - name: Validate OAR workflow syntax run: | - curl -fsSL https://github.com/rhysd/actionlint/releases/download/v1.7.12/actionlint_1.7.12_linux_amd64.tar.gz \ - | tar -xz -C "$RUNNER_TEMP" actionlint + archive="$RUNNER_TEMP/actionlint.tar.gz" + curl -fsSL \ + https://github.com/rhysd/actionlint/releases/download/v1.7.12/actionlint_1.7.12_linux_amd64.tar.gz \ + -o "$archive" + echo "8aca8db96f1b94770f1b0d72b6dddcb1ebb8123cb3712530b08cc387b349a3d8 $archive" \ + | sha256sum --check + tar -xzf "$archive" -C "$RUNNER_TEMP" actionlint "$RUNNER_TEMP/actionlint" \ .github/workflows/oar-checks.yml \ - .github/workflows/oar-smoke.yml \ + .github/workflows/oar-integration.yml \ .github/workflows/pr-review.yml - - name: Run project checks - working-directory: projects/openshell-agent-runner - run: | - uv run pre-commit validate-config ../../.pre-commit-config.yaml - make check + package: + name: Build and exercise installable distributions + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - - name: Build distributions + - name: Set up uv and Python + uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1 + with: + version: "0.12.5" + python-version: "3.12" + enable-cache: true + cache-dependency-glob: projects/openshell-agent-runner/uv.lock + + - name: Build wheel and source distribution working-directory: projects/openshell-agent-runner run: | make build - wheel="$(find dist -name '*.whl' -print -quit)" - python -m zipfile -l "$wheel" | grep -F 'harnesses/pi/runtime/image/Dockerfile' - python -m zipfile -l "$wheel" | grep -F 'harnesses/pi/runtime/image/exec.sh' - python -m zipfile -l "$wheel" | grep -F 'harnesses/pi/runtime/extensions/submit-result.ts' - python -m zipfile -l "$wheel" | grep -F 'harnesses/pi/runtime/extensions/validate-tools.ts' - python -m zipfile -l "$wheel" | grep -F 'profiles/code-reviewer/profile.yaml' - python -m zipfile -l "$wheel" | grep -F 'profiles/technical-writing-reviewer/profile.yaml' - python -m zipfile -l "$wheel" | grep -F 'dist-info/licenses/LICENSE' - - - name: Verify the built wheel + shopt -s nullglob + wheels=(dist/*.whl) + sdists=(dist/*.tar.gz) + test "${#wheels[@]}" -eq 1 + test "${#sdists[@]}" -eq 1 + + - name: Install and exercise the wheel working-directory: projects/openshell-agent-runner run: | - wheel="$(find dist -name '*.whl' -print -quit)" - uvx --from "$wheel" oar init "$RUNNER_TEMP/profiles" \ - --model provider/model + wheel=(dist/*.whl) + uv venv "$RUNNER_TEMP/package-venv" + uv pip install --python "$RUNNER_TEMP/package-venv/bin/python" \ + "${wheel[0]}" "pytest>=8.4,<10" + python="$RUNNER_TEMP/package-venv/bin/python" + oar="$RUNNER_TEMP/package-venv/bin/oar" + "$python" -m pytest tests/test_installed_package.py + "$python" -m pytest tests/test_lifecycle.py -k cli + "$oar" init "$RUNNER_TEMP/profiles" --model provider/model for profile in code-reviewer technical-writing-reviewer; do - uvx --from "$wheel" oar validate \ - "$RUNNER_TEMP/profiles/$profile" + "$oar" validate "$RUNNER_TEMP/profiles/$profile" done printf '# Review me\n\nA short document.\n' > "$RUNNER_TEMP/review-input.md" - uvx --from "$wheel" oar run \ - "$RUNNER_TEMP/profiles/technical-writing-reviewer" \ + "$oar" run "$RUNNER_TEMP/profiles/technical-writing-reviewer" \ --task review-document \ --input "$RUNNER_TEMP/review-input.md" \ --output "$RUNNER_TEMP/review-output.json" \ --dry-run test ! -e "$RUNNER_TEMP/review-output.json" - uvx --from "$wheel" --with pytest pytest tests/test_lifecycle.py -k cli - - name: Build the Pi image - if: matrix.python-version == '3.12' + offline-runtime: + name: Exercise the Pi runtime without network access + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Set up uv and Python + uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1 + with: + version: "0.12.5" + python-version: "3.12" + enable-cache: true + cache-dependency-glob: projects/openshell-agent-runner/uv.lock + + - name: Isolate the project environment + run: echo "UV_PROJECT_ENVIRONMENT=$RUNNER_TEMP/oar-runtime-venv" >> "$GITHUB_ENV" + + - name: Install locked dependencies and initialize a profile + run: | + uv sync --project projects/openshell-agent-runner --locked + uv run --project projects/openshell-agent-runner --frozen oar init \ + "$RUNNER_TEMP/profiles" --profile code-reviewer \ + --model provider/model --thinking off + + - name: Build and inspect the Pi image run: | docker build \ --tag openshell-agent-runner-pi:ci \ @@ -156,7 +180,6 @@ jobs: | grep -F 'provider/model' - name: Validate the Pi extensions - if: matrix.python-version == '3.12' run: | docker run --rm --network none \ --entrypoint bash \ @@ -170,8 +193,7 @@ jobs: --experimental-strip-types --no-warnings \ /sandbox/validate-pi-extensions.mjs" - - name: Exercise the real Pi harness without network access - if: matrix.python-version == '3.12' + - name: Exercise isolated Pi sessions working-directory: projects/openshell-agent-runner env: OAR_PI_IMAGE: openshell-agent-runner-pi:ci diff --git a/.github/workflows/oar-smoke.yml b/.github/workflows/oar-integration.yml similarity index 67% rename from .github/workflows/oar-smoke.yml rename to .github/workflows/oar-integration.yml index 575fcdd1..ad73049f 100644 --- a/.github/workflows/oar-smoke.yml +++ b/.github/workflows/oar-integration.yml @@ -1,35 +1,35 @@ -name: OAR pipeline smoke +name: OAR live integration "on": pull_request: types: [opened, reopened, synchronize, ready_for_review] paths: - - .github/workflows/oar-smoke.yml + - .github/workflows/oar-integration.yml - .github/actions/setup-review-gateway/** - projects/openshell-agent-runner/src/** - projects/openshell-agent-runner/pyproject.toml - projects/openshell-agent-runner/uv.lock - - projects/openshell-agent-runner/tests/fixtures/pipeline-smoke/** + - projects/openshell-agent-runner/tests/fixtures/pipeline-integration/** push: branches: [main] paths: - - .github/workflows/oar-smoke.yml + - .github/workflows/oar-integration.yml - .github/actions/setup-review-gateway/** - projects/openshell-agent-runner/src/** - projects/openshell-agent-runner/pyproject.toml - projects/openshell-agent-runner/uv.lock - - projects/openshell-agent-runner/tests/fixtures/pipeline-smoke/** + - projects/openshell-agent-runner/tests/fixtures/pipeline-integration/** workflow_dispatch: permissions: {} concurrency: - group: oar-smoke-${{ github.ref }} + group: oar-integration-${{ github.ref }} cancel-in-progress: true jobs: - smoke: - name: Check installed OAR against a live gateway + integration: + name: Run installed OAR through OpenShell # Candidate OAR code runs on the host: restrict secrets to trusted branches. if: >- (github.event_name == 'pull_request' && @@ -39,7 +39,7 @@ jobs: (github.event_name != 'pull_request' && github.ref == format('refs/heads/{0}', github.event.repository.default_branch)) runs-on: ubuntu-latest - timeout-minutes: 15 + timeout-minutes: 10 permissions: contents: read steps: @@ -57,7 +57,7 @@ jobs: enable-cache: true cache-dependency-glob: projects/openshell-agent-runner/uv.lock - - name: Install the wheel and prepare one input + - name: Install the wheel and prepare the contract input working-directory: projects/openshell-agent-runner env: REVIEW_MODEL: ${{ secrets.MODEL_ID_TOP }} @@ -70,11 +70,11 @@ jobs: oar="$RUNNER_TEMP/oar-venv/bin/oar" "$oar" init "$RUNNER_TEMP/profiles" --profile code-reviewer \ --model "$REVIEW_MODEL" --thinking off - cp tests/fixtures/pipeline-smoke/* "$RUNNER_TEMP/profiles/code-reviewer/" + cp tests/fixtures/pipeline-integration/* "$RUNNER_TEMP/profiles/code-reviewer/" "$oar" validate "$RUNNER_TEMP/profiles/code-reviewer" - mkdir -p "$RUNNER_TEMP/oar-results" + mkdir -p "$RUNNER_TEMP/oar-integration" printf 'Input for workflow run %s, attempt %s.\n' "$GITHUB_RUN_ID" "$GITHUB_RUN_ATTEMPT" \ - > "$RUNNER_TEMP/oar-results/input.txt" + > "$RUNNER_TEMP/oar-integration/input.txt" - name: Start ephemeral gateway id: gateway @@ -84,23 +84,22 @@ jobs: base-url: ${{ secrets.INFERENCE_BASE_URL }} model: ${{ secrets.MODEL_ID_TOP }} - - name: Run the installed CLI - id: run + - name: Exercise the installed CLI run: | "$RUNNER_TEMP/oar-venv/bin/oar" run "$RUNNER_TEMP/profiles/code-reviewer" \ - --task echo-input --input "$RUNNER_TEMP/oar-results/input.txt" \ + --task echo-input --input "$RUNNER_TEMP/oar-integration/input.txt" \ --prompt-var "marker=$GITHUB_RUN_ID" --gateway openshell \ - --timeout-seconds 300 --output "$RUNNER_TEMP/oar-results/result.json" \ - > "$RUNNER_TEMP/oar-results/run.log" 2>&1 + --timeout-seconds 300 --output "$RUNNER_TEMP/oar-integration/result.json" \ + > "$RUNNER_TEMP/oar-integration/run.log" 2>&1 - - name: Verify transferred input and prompt variable + - name: Verify transfer, prompt substitution, and structured output run: | python3 - <<'PY' import json import os from pathlib import Path - results = Path(os.environ["RUNNER_TEMP"]) / "oar-results" + results = Path(os.environ["RUNNER_TEMP"]) / "oar-integration" result = json.loads((results / "result.json").read_text()) assert result["content"] == (results / "input.txt").read_text().strip(), result assert result["marker"] == os.environ["GITHUB_RUN_ID"], result @@ -109,32 +108,40 @@ jobs: - name: Verify sandbox cleanup if: ${{ !cancelled() && steps.gateway.outcome == 'success' }} run: | - openshell sandbox list --gateway openshell --names > "$RUNNER_TEMP/oar-results/remaining-sandboxes.txt" - test ! -s "$RUNNER_TEMP/oar-results/remaining-sandboxes.txt" + openshell sandbox list --gateway openshell --names \ + > "$RUNNER_TEMP/oar-integration/remaining-sandboxes.txt" + test ! -s "$RUNNER_TEMP/oar-integration/remaining-sandboxes.txt" + + - name: Capture safe diagnostics + if: always() && steps.gateway.outcome != 'skipped' + run: | + openshell --version > "$RUNNER_TEMP/oar-integration/openshell-version.txt" 2>&1 || true + openshell status > "$RUNNER_TEMP/oar-integration/gateway-status.txt" 2>&1 || true - name: Stop gateway if: always() && steps.gateway.outcome != 'skipped' run: systemctl --user stop openshell-gateway.service - - name: Upload evidence + - name: Upload integration evidence if: always() uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: - name: oar-pipeline-smoke - path: ${{ runner.temp }}/oar-results + name: oar-live-integration + path: ${{ runner.temp }}/oar-integration if-no-files-found: warn retention-days: 14 - - name: Summarize pipeline check + - name: Summarize integration check if: always() env: OUTCOME: ${{ job.status }} run: | { - echo '## OAR pipeline smoke' + echo '## OAR live integration' echo echo "Outcome: **$OUTCOME**" echo - echo 'One installed-CLI run checks input upload, prompt substitution, schema-validated output, and sandbox cleanup.' - echo 'This checks OAR execution, not review quality or the PR content. See the oar-pipeline-smoke artifact for evidence.' + echo 'An installed wheel ran through a real OpenShell gateway and inference route.' + echo 'The check verifies input transfer, prompt substitution, schema-validated output, and sandbox cleanup.' + echo 'It verifies execution contracts, not reviewer judgment or PR content.' } >> "$GITHUB_STEP_SUMMARY" diff --git a/.github/workflows/pr-review.yml b/.github/workflows/pr-review.yml index a8b24fd7..c55f1128 100644 --- a/.github/workflows/pr-review.yml +++ b/.github/workflows/pr-review.yml @@ -3,16 +3,22 @@ name: New project review "on": # Trusted workflow/tooling only. PR contents are uploaded as data, never run here. pull_request_target: - types: [opened, reopened, synchronize, ready_for_review] + types: [opened, reopened, synchronize, ready_for_review, converted_to_draft] permissions: {} +concurrency: + group: oar-pr-review-${{ github.event.pull_request.number }} + cancel-in-progress: true + jobs: request: name: Select new projects and validate their kinds runs-on: ubuntu-latest + timeout-minutes: 5 permissions: contents: read + issues: read pull-requests: read outputs: number: ${{ steps.request.outputs.number }} @@ -55,9 +61,6 @@ jobs: timeout-minutes: 45 permissions: contents: read - concurrency: - group: oar-pr-review-${{ needs.request.outputs.number }} - cancel-in-progress: true steps: - name: Checkout trusted tooling uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -160,6 +163,7 @@ jobs: needs: [request, review] if: always() && needs.request.outputs.number != '' runs-on: ubuntu-latest + timeout-minutes: 5 permissions: contents: read pull-requests: write diff --git a/docs/development/ci.md b/docs/development/ci.md index 1cd569a3..f6f3d2ee 100644 --- a/docs/development/ci.md +++ b/docs/development/ci.md @@ -9,13 +9,14 @@ description: New-project assessments through OAR, separate from deterministic ch | Workflow | Purpose | When it runs | | --- | --- | --- | -| OAR and PR review checks | Lint, types, tests, wheel installation, offline Pi sessions, and PR selection-to-report integration. No inference credentials. | OAR or review infrastructure changes. | -| OAR pipeline smoke | One installed CLI run through a real gateway; checks input transfer, prompt variables, structured output, and sandbox cleanup. | OAR source, dependencies, smoke fixture, or gateway setup changes on trusted branches. | +| OAR functional checks | Independent Python 3.12 jobs check deterministic behavior, installable distributions, and offline Pi sessions. No inference credentials. | OAR or review infrastructure changes. | +| OAR live integration | One installed CLI run through a real gateway checks input transfer, prompt variables, structured output, and sandbox cleanup. | OAR source, dependencies, integration fixture, or gateway setup changes on trusted branches. | | New project review | An advisory assessment of each new project against its purpose and project guidelines, in one updated PR comment. | Eligible new-project PRs, as described below. | | Existing repository checks | Dependency licenses, source headers, project tests, documentation builds, and previews. | Their existing workflow triggers; independent of OAR reviews. | -Model verdicts are not CI pass/fail expectations. The live smoke checks execution -contracts; it does not evaluate reviewer quality or assess the PR's content. +Model verdicts are not CI pass/fail expectations. The live integration checks +execution contracts; it does not evaluate reviewer quality or assess the PR's +content. The first iteration answers: **Does this new project deliver what it claims, follow our project guidelines, and use an appropriate level of engineering?** @@ -57,12 +58,14 @@ New-project PR └─ oar run once per project └─ Validated JSON → one current-revision PR comment -Native checks + offline integration ─── execution and policy checks -One live OAR pipeline check ─────────── Actions summary, not a PR assessment +Deterministic, package, and offline runtime checks ─── functional contracts +One live OAR integration ───────────────────────────── Actions summary, not a PR assessment ``` -The result includes an overall verdict, five fixed criterion scores (0–100, -100 best), their rounded mean, findings, strengths, and limitations. +The result artifact includes an overall verdict, five fixed criterion scores +(0–100, 100 best), their rounded mean, findings, strengths, and limitations. +The PR comment emphasizes verdicts and actionable findings rather than +displaying the numerical scores. `guidelines_assessment` separately records a verdict and evidence-based explanation covering applicable requirements and material verification gaps. Findings cite specific guideline violations; they do not invent requirements. @@ -74,7 +77,9 @@ findings are advisory; native checks remain separate merge gates. ## Execution and trust -`New project review` runs on PR opening, reopening, new commits, and readiness. +`New project review` runs on PR opening, reopening, new commits, readiness, and +conversion back to draft. When a new project is removed from the PR or the PR +returns to draft, the workflow removes its now-stale bot report. Draft, fork, and Dependabot PRs skip live review. There is no manual fork authorization or waiting for other workflows. @@ -98,8 +103,8 @@ The workflow must first land on the default branch before it can review actual project additions. Before that, offline integration tests exercise selection, snapshot preparation, the real OAR CLI, and report creation/update together, with simulated GitHub and OpenShell boundaries. This does not claim a live -GitHub comment has been tested. The separate smoke verifies real OpenShell -execution using the candidate wheel. +GitHub comment has been tested. The separate live integration verifies real +OpenShell execution using the candidate wheel. ## Setup @@ -127,20 +132,26 @@ uv run --project projects/openshell-agent-runner pytest \ ``` Run `make check` and `make build` from the OAR project directory. `make check` -includes CLI workflows against a simulated OpenShell; `make test-runtime` uses -Docker to exercise real Pi sessions against local scripted inference, with no -external network or credentials. CI runs both, plus the Pi extension SDK checks. - -The live smoke workflow (`.github/workflows/oar-smoke.yml`) installs the built -wheel, configures a small task from `tests/fixtures/pipeline-smoke/`, and calls -`oar run` directly. The agent reads an uploaded file and returns its contents -plus a runtime prompt variable in a schema-validated result. CI checks those -values and confirms that no sandbox remains on its dedicated gateway. - -The task has a five-minute timeout and the job has a 15-minute limit. +includes CLI workflows against a simulated OpenShell. CI runs deterministic +checks, distribution checks, and offline runtime checks as independent Python +3.12 jobs. The distribution job builds both artifacts, proves that the sdist can +build a wheel, installs the release wheel, and exercises its CLI and packaged +resources. The runtime job uses Docker to exercise real Pi sessions against +local scripted inference, with no external network or credentials, and checks +the Pi extensions directly. + +The live integration workflow (`.github/workflows/oar-integration.yml`) installs +the built wheel, configures a small task from +`tests/fixtures/pipeline-integration/`, and calls `oar run` directly. The agent +reads an uploaded file and returns its contents plus a runtime prompt variable +in a schema-validated result. CI checks those values and confirms that no +sandbox remains on its dedicated gateway. + +The task has a five-minute timeout and the job has a 10-minute limit. Input, result, log, and sandbox inventory are uploaded as -the `oar-pipeline-smoke` artifact. The workflow writes an Actions summary, -never a PR comment. No clean/flawed reviewer experiments run in CI. +the `oar-live-integration` artifact, together with safe OpenShell version and +gateway-status diagnostics. The workflow writes an Actions summary, never a PR +comment. No reviewer-opinion expectations run in CI. Keep team expectations in the project guidelines. Change review judgment in the common or kind-specific skill, and change selection/reporting only when the diff --git a/projects/openshell-agent-runner/README.md b/projects/openshell-agent-runner/README.md index 24b3be47..61f87845 100644 --- a/projects/openshell-agent-runner/README.md +++ b/projects/openshell-agent-runner/README.md @@ -103,8 +103,9 @@ make build `make check` includes CLI workflows against a simulated OpenShell. With Docker, run `make test-runtime` to exercise the real Pi harness without external inference. -For runtime changes, one live CI task checks input transfer, prompt variables, -structured output, and sandbox cleanup. It does not grade reviewer opinions. +For runtime changes, one live integration check runs the installed wheel through +OpenShell and verifies input transfer, prompt variables, structured output, and +sandbox cleanup. It does not grade reviewer opinions. Use `uv run --frozen oar` in this directory to run the checked-out code instead of the installed release. Run a focused test with diff --git a/projects/openshell-agent-runner/tests/fixtures/pipeline-smoke/output.schema.json b/projects/openshell-agent-runner/tests/fixtures/pipeline-integration/output.schema.json similarity index 100% rename from projects/openshell-agent-runner/tests/fixtures/pipeline-smoke/output.schema.json rename to projects/openshell-agent-runner/tests/fixtures/pipeline-integration/output.schema.json diff --git a/projects/openshell-agent-runner/tests/fixtures/pipeline-smoke/profile.yaml b/projects/openshell-agent-runner/tests/fixtures/pipeline-integration/profile.yaml similarity index 93% rename from projects/openshell-agent-runner/tests/fixtures/pipeline-smoke/profile.yaml rename to projects/openshell-agent-runner/tests/fixtures/pipeline-integration/profile.yaml index 61648459..1bfb3875 100644 --- a/projects/openshell-agent-runner/tests/fixtures/pipeline-smoke/profile.yaml +++ b/projects/openshell-agent-runner/tests/fixtures/pipeline-integration/profile.yaml @@ -1,4 +1,4 @@ -id: pipeline-smoke +id: pipeline-integration description: Verify the installed CLI can transfer input and return structured output. sandbox: policy: policy.yaml diff --git a/projects/openshell-agent-runner/tests/fixtures/pipeline-smoke/prompt.md b/projects/openshell-agent-runner/tests/fixtures/pipeline-integration/prompt.md similarity index 100% rename from projects/openshell-agent-runner/tests/fixtures/pipeline-smoke/prompt.md rename to projects/openshell-agent-runner/tests/fixtures/pipeline-integration/prompt.md diff --git a/projects/openshell-agent-runner/tests/test_installed_package.py b/projects/openshell-agent-runner/tests/test_installed_package.py new file mode 100644 index 00000000..3ff6b5a5 --- /dev/null +++ b/projects/openshell-agent-runner/tests/test_installed_package.py @@ -0,0 +1,41 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Contracts for the wheel installed by CI.""" + +from importlib.metadata import distribution +from importlib.resources import files + + +def test_distribution_exposes_both_cli_entry_points() -> None: + package = distribution("openshell-agent-runner") + + assert package.version + assert any( + str(path).endswith(".dist-info/licenses/LICENSE") + for path in package.files or () + ) + scripts = { + entry_point.name: entry_point.value + for entry_point in package.entry_points + if entry_point.group == "console_scripts" + } + assert scripts == { + "oar": "openshell_agent_runner.cli:app", + "openshell-agent-runner": "openshell_agent_runner.cli:app", + } + + +def test_distribution_contains_runtime_and_profile_resources() -> None: + package = files("openshell_agent_runner") + required_resources = ( + "harnesses/pi/runtime/image/Dockerfile", + "harnesses/pi/runtime/image/exec.sh", + "harnesses/pi/runtime/extensions/submit-result.ts", + "harnesses/pi/runtime/extensions/validate-tools.ts", + "profiles/code-reviewer/profile.yaml", + "profiles/technical-writing-reviewer/profile.yaml", + ) + + for relative_path in required_resources: + assert package.joinpath(relative_path).is_file(), relative_path diff --git a/projects/openshell-agent-runner/tests/test_lifecycle.py b/projects/openshell-agent-runner/tests/test_lifecycle.py index 1ead1224..88f5a83a 100644 --- a/projects/openshell-agent-runner/tests/test_lifecycle.py +++ b/projects/openshell-agent-runner/tests/test_lifecycle.py @@ -533,10 +533,12 @@ def test_cli_doctor_displays_configuration_without_claiming_inference_readiness( assert [command[0] for command in commands] == ["--version", "status", "inference"] -def test_cli_runs_the_live_smoke_profile(tmp_path: Path, monkeypatch) -> None: +def test_cli_runs_the_live_integration_profile(tmp_path: Path, monkeypatch) -> None: profile, _, state, log = prepare(tmp_path, monkeypatch) shutil.copytree( - Path(__file__).parent / "fixtures/pipeline-smoke", profile, dirs_exist_ok=True + Path(__file__).parent / "fixtures/pipeline-integration", + profile, + dirs_exist_ok=True, ) document = tmp_path / "input notes.txt" document.write_text("Uploaded content.\n") diff --git a/tests/test_pr_review.py b/tests/test_pr_review.py index c8e9638a..1041de4a 100644 --- a/tests/test_pr_review.py +++ b/tests/test_pr_review.py @@ -18,6 +18,7 @@ sys.path.insert(0, str(Path(__file__).resolve().parents[1] / ".github/scripts")) from github_api import GitHubError +from review_report import REVIEW_MARKER if importlib.util.find_spec("yaml") is None: raise unittest.SkipTest( @@ -51,6 +52,7 @@ def __init__(self): self.metadata = "kind: tool\n" self.metadata_type = "file" self.error = None + self.comments = [] self.calls = [] def request(self, method, path, data=None): @@ -77,6 +79,8 @@ def paginate(self, path, key=None): self.calls.append(("paginate", path, key)) if path == "pulls/7/files?per_page=100": return self.files + if path == "issues/7/comments": + return self.comments raise AssertionError(f"Unexpected pagination: {path}") @@ -116,7 +120,10 @@ def test_skip_drafts_forks_bots_closed_and_stale_events_before_reading_files(sel else: github.pr["head"]["repo"] = None self.assertIsNone(resolve_request(github, self.context)) - self.assertEqual(github.calls, [("GET", "pulls/7", None)]) + expected = [("GET", "pulls/7", None)] + if kind == "draft": + expected.append(("paginate", "issues/7/comments", None)) + self.assertEqual(github.calls, expected) def test_other_events_do_not_start_review(self): for event in ("workflow_dispatch", "workflow_run", "push"): @@ -127,6 +134,24 @@ def test_existing_project_needs_no_metadata(self): self.assertIsNone(resolve_request(self.github, self.context)) self.assertFalse(any("contents/" in path for _, path, _ in self.github.calls)) + def test_existing_report_is_retired_when_review_no_longer_applies(self): + self.github.files[0]["filename"] = "projects/existing/new-file.py" + self.github.comments = [ + { + "id": 9, + "user": {"type": "Bot", "login": "github-actions[bot]"}, + "body": REVIEW_MARKER, + } + ] + + request = resolve_request(self.github, self.context) + + self.assertTrue(request["retire"]) + self.assertEqual(request["tasks"], []) + self.github.pr["draft"] = True + request = resolve_request(self.github, self.context) + self.assertTrue(request["retire"]) + def test_no_base_projects_directory(self): self.github.base_tree = [] self.assertEqual( @@ -238,6 +263,49 @@ def test_cli_writes_pinned_request_and_reports_metadata_errors(self): ["git", "-C", "trusted tooling", "rev-parse", "HEAD"], ) + def test_cli_marks_report_retirement_as_not_ready_for_inference(self): + self.github.files[0]["filename"] = "projects/existing/new-file.py" + self.github.comments = [ + { + "id": 9, + "user": {"type": "Bot", "login": "github-actions[bot]"}, + "body": REVIEW_MARKER, + } + ] + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + event = root / "event.json" + event.write_text(json.dumps(self.context["payload"])) + request_file = root / "request/request.json" + output = root / "outputs" + environment = { + "GITHUB_EVENT_PATH": str(event), + "GITHUB_EVENT_NAME": "pull_request_target", + "GITHUB_OUTPUT": str(output), + } + with ( + patch.dict(os.environ, environment), + patch("pr_review.GitHub", return_value=self.github), + patch( + "sys.argv", + [ + "pr_review.py", + "--output", + str(request_file), + "--tooling", + "trusted tooling", + ], + ), + patch( + "pr_review.subprocess.run", + return_value=subprocess.CompletedProcess([], 0, "c" * 40 + "\n"), + ), + ): + main() + + self.assertTrue(json.loads(request_file.read_text())["retire"]) + self.assertIn("ready=false\n", output.read_text()) + if __name__ == "__main__": unittest.main() diff --git a/tests/test_pr_review_integration.py b/tests/test_pr_review_integration.py index 9287f3fb..ac459a16 100644 --- a/tests/test_pr_review_integration.py +++ b/tests/test_pr_review_integration.py @@ -38,7 +38,13 @@ def request(self, method, path, data=None): return self.responses[path] self.writes.append((method, path)) if (method, path) == ("POST", "issues/7/comments"): - self.comments.append({"id": 1, "user": {"type": "Bot"}, **data}) + self.comments.append( + { + "id": 1, + "user": {"type": "Bot", "login": "github-actions[bot]"}, + **data, + } + ) elif (method, path) == ("PATCH", "issues/comments/1"): self.comments[0].update(data) else: diff --git a/tests/test_review_report.py b/tests/test_review_report.py index f69515d8..23c8390d 100644 --- a/tests/test_review_report.py +++ b/tests/test_review_report.py @@ -79,6 +79,8 @@ def request(self, method, path, data=None): return self.pr if method in ("POST", "PATCH"): return data + if method == "DELETE" and path.startswith("issues/comments/"): + return None raise AssertionError(f"Unexpected request: {method} {path}") def paginate(self, path, key=None): @@ -171,7 +173,6 @@ def test_partial_results_preserve_successes(self): options.update(reviews=reviews, outcome="failed") body = render_report(**options) for expected in ( - "91/100", "Not completed", "Hardware not exercised.", "failed", @@ -188,6 +189,8 @@ def test_report_identifies_revision_guidelines_and_advisory_status(self): "c" * 40, ): self.assertIn(expected, normal) + for omitted in ("91/100", "Criterion", "Supported."): + self.assertNotIn(omitted, normal) def test_report_escapes_reviewer_data(self): options = report_options() @@ -195,7 +198,6 @@ def test_report_escapes_reviewer_data(self): review = options["reviews"][0] review["label"] = unsafe review["result"]["summary"] = unsafe - review["result"]["criterion_scores"][0]["explanation"] = unsafe review["result"]["findings"] = [ dict.fromkeys( ("title", "path", "evidence", "impact", "recommendation"), unsafe @@ -222,13 +224,33 @@ def test_report_retains_finding_evidence_and_impact(self): self.assertIn("Run missing-command", body) self.assertIn("That executable is not installed.", body) + def test_finding_locations_link_to_the_reviewed_revision(self): + options = report_options() + options["reviews"][0]["result"]["findings"] = [ + { + "severity": "medium", + "title": "Wrong command", + "path": "projects/new spike/README.md", + "line": 12, + "evidence": "The documented command is unavailable.", + "impact": "The first run fails.", + "recommendation": "Use the installed command.", + } + ] + + body = render_report(**options) + + self.assertIn( + f"https://github.com/example/research/blob/{HEAD}/projects/new%20spike/README.md#L12", + body, + ) + def test_large_details_keep_summary_and_artifact_link(self): options = report_options() options["reviews"][0]["result"]["summary"] = "Detailed evidence " * 5000 body = render_report(**options) self.assertLessEqual(len(body), 55000) self.assertIn(options["run_url"], body) - self.assertIn("91/100", body) def test_stale_and_closed_prs_do_not_publish(self): for change in ("head", "closed"): @@ -248,7 +270,7 @@ def test_older_runs_cannot_overwrite_newer_reports(self): github.comments = [ { "id": 5, - "user": {"type": "Bot"}, + "user": {"type": "Bot", "login": "github-actions[bot]"}, "body": f"{REVIEW_MARKER}\n", } ] @@ -259,6 +281,24 @@ def test_older_runs_cannot_overwrite_newer_reports(self): any(method in ("PATCH", "POST") for method, _, _ in github.calls) ) + def test_retirement_deletes_only_an_existing_current_report(self): + github = MockGitHub() + github.comments = [ + { + "id": 5, + "user": {"type": "Bot", "login": "github-actions[bot]"}, + "body": f"{REVIEW_MARKER}\n", + } + ] + + request = {"number": 7, "head": HEAD, "retire": True} + self.assertTrue(publish_report(github, request, "unused", 100)) + self.assertIn(("DELETE", "issues/comments/5", None), github.calls) + + github = MockGitHub() + self.assertFalse(publish_report(github, request, "unused", 100)) + self.assertFalse(any(method == "DELETE" for method, _, _ in github.calls)) + def test_sticky_comments_do_not_edit_human_or_unrelated_bot_comments(self): github = MockGitHub() github.comments = [ @@ -266,14 +306,19 @@ def test_sticky_comments_do_not_edit_human_or_unrelated_bot_comments(self): {"id": 2, "user": {"type": "Bot"}, "body": "Documentation preview"}, { "id": 3, - "user": {"type": "Bot"}, + "user": {"type": "Bot", "login": "other-app[bot]"}, + "body": f"{REVIEW_MARKER}\n", + }, + { + "id": 4, + "user": {"type": "Bot", "login": "github-actions[bot]"}, "body": f"{REVIEW_MARKER}\n", }, ] self.assertTrue( publish_report(github, {"number": 7, "head": HEAD}, "updated", 100) ) - self.assertIn(("PATCH", "issues/comments/3", {"body": "updated"}), github.calls) + self.assertIn(("PATCH", "issues/comments/4", {"body": "updated"}), github.calls) github.calls.clear() github.comments.pop() self.assertTrue(publish_report(github, {"number": 7, "head": HEAD}, "new", 101))