diff --git a/.github/seidroid/xreview/README.md b/.github/seidroid/xreview/README.md index 5213aea..f9e8822 100644 --- a/.github/seidroid/xreview/README.md +++ b/.github/seidroid/xreview/README.md @@ -145,8 +145,22 @@ until the items above hold. The driver has been exercised end-to-end against live PRs: mint, session create, sandbox launch, the agent reading the PR through the credential bridge, a structured verdict, and -teardown. The **reusable workflow and its thin caller** are being wired into their first repo -now; the first `seidroid xreview` comment there is the comment-trigger path's first real test. +teardown. The **reusable workflow and its thin caller** have carried a real +`seidroid xreview` comment as far as the in-cluster runner: the trusted-commenter guard +admitted it, the `xreview` job scheduled on `uci-default`, and the driver checked out at the +caller's `uci-ref`. The steps from the bearer mint onward have not yet run on that runner, +so treat the in-cluster half of the path as unproven until one run posts a verdict. + +The gaps that surfaced there were runner-image ones rather than logic ones, because the +in-cluster image is a minimal runner rather than the hosted image the driver was developed +against. It carries `curl` and `git`, and a `python3` with no venv module; it does not carry +the `gh` CLI. So the workflow now brings its own interpreter and posts through the API +client `github-script` provides. + +What it still expects from the image is `curl`, for the bearer mint, `git`, for the driver +checkout, and `bash` plus `tar` and the usual coreutils, which the interpreter setup needs +to unpack and install what it downloads. The node the three actions run on comes from the +runner rather than the image. Known gaps for a follow-up: the verdict currently posts as `github-actions[bot]` rather than `seidroid[bot]` (posting via the seidroid app token is a later change); and the diff --git a/.github/workflows/seidroid-xreview.yml b/.github/workflows/seidroid-xreview.yml index dc94e93..187f009 100644 --- a/.github/workflows/seidroid-xreview.yml +++ b/.github/workflows/seidroid-xreview.yml @@ -84,6 +84,10 @@ jobs: # The reviewed repo's runner label. Org ARC scale set by default, in-cluster, so it # reaches omnigent over the ClusterIP Service and mints its bearer there. runs-on: ${{ inputs.runs-on }} + # A stalled fetch, download, pip resolve, or mint would otherwise hold a runner (and a + # live credential) until the six-hour default. The driver bounds its own run; this + # bounds everything around it. + timeout-minutes: 30 permissions: pull-requests: write # upsert the one sticky verdict comment contents: read # read PR metadata @@ -104,15 +108,27 @@ jobs: sparse-checkout-cone-mode: false persist-credentials: false + # The interpreter comes from here rather than the runner image because an image is + # free to ship python3 without the venv module: the in-cluster ARC image does, and + # the venv below cannot bootstrap without ensurepip. This also fixes the version + # the driver runs under instead of inheriting whatever the image happens to carry. + - name: Set up Python + id: python + uses: actions/setup-python@v7 + with: + python-version: '3.12' + - name: Set up driver runtime id: runtime shell: bash + env: + SETUP_PYTHON: ${{ steps.python.outputs.python-path }} run: | set -euo pipefail # A pinned venv keeps the httpx version deterministic regardless of the image. - command -v python3 >/dev/null 2>&1 || { echo "python3 not found on runner" >&2; exit 1; } venv="$RUNNER_TEMP/seidroid-venv" - python3 -m venv "$venv" + # The interpreter the setup step resolved, not whatever PATH offers. + "$SETUP_PYTHON" -m venv "$venv" "$venv/bin/python" -m pip install --disable-pip-version-check --no-input --quiet "httpx==0.27.2" echo "python=$venv/bin/python" >> "$GITHUB_OUTPUT" @@ -128,6 +144,7 @@ jobs: client_id="${OMNIGENT_M2M_CLIENT_ID:-sei-droid}" : "${OMNIGENT_M2M_CLIENT_SECRET:?OMNIGENT_M2M_CLIENT_SECRET not provided by the caller}" resp="$(curl -sS -w $'\n%{http_code}' \ + --connect-timeout 5 --max-time 30 \ --config <(printf 'user = "%s:%s"\n' "$client_id" "$OMNIGENT_M2M_CLIENT_SECRET") \ -d grant_type=client_credentials \ "$BASE_URL/oauth/token")" || { echo "omnigent /oauth/token request failed" >&2; exit 1; } @@ -160,9 +177,8 @@ jobs: set -euo pipefail # Create exactly ONE managed sei-droid session, drive it, write the verdict # to verdict.md ONLY on a real verdict (no placeholder), and DELETE the - # session on exit AND on SIGTERM/SIGINT. Fail-closed: anything other than the - # exact opt-in 'false' runs dry. --trigger-id scopes the run key to this - # comment so a re-fire adopts rather than resurrects. + # session on exit AND on SIGTERM/SIGINT. --trigger-id scopes the run key to + # this comment so a re-fire adopts rather than resurrects. args=("$REPO" "$PR" --out verdict.md) if [ -n "${TRIGGER_ID:-}" ]; then args+=(--trigger-id "$TRIGGER_ID"); fi set +e @@ -173,7 +189,10 @@ jobs: # Downstream gates on whether a verdict was PRODUCED (verdict.md non-empty), # not on the exit code: a teardown-only failure still publishes, and a # no-verdict run never posts a placeholder. - if [ -s verdict.md ]; then + # Non-whitespace rather than non-empty: a final agent message with no text + # block writes a lone newline, which -s admits and which would post a comment + # carrying nothing but the marker. + if grep -q '[^[:space:]]' verdict.md 2>/dev/null; then echo "verdict_produced=true" >> "$GITHUB_OUTPUT" if [ "$rc" -ne 0 ]; then echo "::warning::driver exited $rc but produced a verdict (e.g. a teardown leak); see logs" @@ -188,22 +207,59 @@ jobs: # Post only when a real verdict was produced. Keying on verdict_produced, not # the exit code, so a teardown-only failure still posts a valid verdict and a # no-verdict run never upserts a placeholder. + # + # Posted through github-script rather than the gh CLI because the in-cluster + # runner image does not carry gh, and because the API client it hands the script + # is the same token this job already holds. The repo comes from the run context + # and the pr number from the guard's output, so neither is interpolated into a + # shell command. if: ${{ steps.drive.outputs.verdict_produced == 'true' }} - shell: bash + uses: actions/github-script@v9 env: - GH_TOKEN: ${{ github.token }} MARKER: "" - REPO: ${{ github.repository }} PR: ${{ needs.guard.outputs.pr_number }} - run: | - set -euo pipefail - body="$MARKER"$'\n'"$(cat verdict.md)" - # One bot comment per PR: find by marker -> PATCH, else POST. repo/pr from env, - # not template-interpolated into the script. - id="$(gh api "repos/$REPO/issues/$PR/comments" --paginate \ - --jq "map(select(.body | startswith(\"$MARKER\"))) | .[0].id // empty")" - if [ -n "$id" ]; then - gh api -X PATCH "repos/$REPO/issues/comments/$id" -f body="$body" >/dev/null - else - gh api -X POST "repos/$REPO/issues/$PR/comments" -f body="$body" >/dev/null - fi + with: + # A 429 or 5xx on this call would otherwise discard a completed review; the + # retry set excludes 4xx other than 429, so a permission or validation error + # still fails fast. + retries: 3 + script: | + const fs = require('fs'); + const path = require('path'); + const marker = process.env.MARKER; + const issue_number = Number(process.env.PR); + // Anchored to the workspace rather than the process cwd, which a later + // working-directory default or a step reorder could move out from under this. + const verdict = path.join(process.env.GITHUB_WORKSPACE, 'verdict.md'); + const body = `${marker}\n${fs.readFileSync(verdict, 'utf8')}`; + + const comments = await github.paginate(github.rest.issues.listComments, { + ...context.repo, + issue_number, + per_page: 100, + }); + // One bot comment per PR: edit the marked one, else create it. The author has + // to be a bot as well as the marker matching. Anyone who can comment on the PR + // can write the marker, including the untrusted PR author, whom the trigger + // guard does not cover because it gates who may run the bot rather than whose + // code is reviewed. Matching on the marker alone would let such a comment + // either capture the verdict slot, since this token can edit it, or fail the + // step and suppress the review. + const existing = comments.find( + (c) => c.body?.startsWith(marker) && c.user?.type === 'Bot', + ); + if (existing) { + await github.rest.issues.updateComment({ + ...context.repo, + comment_id: existing.id, + body, + }); + core.info(`updated verdict comment ${existing.id} by ${existing.user?.login}`); + } else { + const created = await github.rest.issues.createComment({ + ...context.repo, + issue_number, + body, + }); + core.info(`created verdict comment ${created.data.id}`); + }