Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions .github/seidroid/xreview/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
98 changes: 77 additions & 21 deletions .github/workflows/seidroid-xreview.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"

Expand All @@ -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; }
Expand Down Expand Up @@ -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
Expand All @@ -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"
Expand All @@ -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: "<!-- seidroid-xreview -->"
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}`);
}