Skip to content

xreview: remove dry-run - #50

Merged
bdchatham merged 1 commit into
mainfrom
chore/xreview-remove-dry-run
Jul 30, 2026
Merged

xreview: remove dry-run#50
bdchatham merged 1 commit into
mainfrom
chore/xreview-remove-dry-run

Conversation

@bdchatham

Copy link
Copy Markdown
Contributor

What

Removes dry-run from seidroid xreview entirely. Posting the verdict to the PR becomes the only behaviour.

Why

Dry-run suppressed the driver's verdict post and nothing else. It still claimed a runner and created a full sandbox session, so it was never a cheap preview — and because the repo variable defaults to dry (${REPO_DRY_RUN:-true}), every adopting repo needed a per-repo opt-in before it could do the one thing the feature is for.

It also read as a safety mode without being one. policy.dry_run_guard's own docstring said so:

It does NOT make the agent read-only: Bash is permitted in both modes, and dry-run gates only the driver's verdict post — not the agent's own gh/git writes through its vended token.

The prompt did append a DRY RUN: do not post ... do not push, merge, or otherwise mutate anything instruction, but that is a request to a model rather than a control, and it never applied in the mode repos actually run in.

What changed

Removed Where
SEIDROID_XREVIEW_DRY_RUN repo variable + ${REPO_DRY_RUN:-true} resolution .github/workflows/seidroid-xreview.yml
--dry-run command flag in the trigger regex same
guard's dry_run output, drive step's DRY_RUN env + --dry-run arg same
"Dry-run summary (posts nothing)" step same
--dry-run CLI arg, ReviewRequest.dry_run driver/__main__.py, driver/driver.py
DRY RUN: prompt instruction driver/driver.py
dry_run_guard driver/policy.py

Posting is now gated on steps.drive.outputs.verdict_produced == 'true' alone, which it already was in addition to the mode.

Two properties deliberately preserved

The exact-line trigger match. Previously described in terms of stopping prose from forcing dry-run; it stands on its own and is kept, so a comment that quotes or discusses seidroid xreview still does not trigger a review.

The untrusted-content preconditions. These never depended on dry-run — the risk belongs to running the agent at all — so they are kept and promoted rather than trimmed alongside the mode. The section is retitled "Before pointing this at untrusted content" and now states plainly that no review mode mitigates it, which the old framing arguably obscured by offering dry-run as a first step. The precondition that was purely about enabling posts ("a successful dry run has completed") is the only one dropped.

The safety-properties list loses the "Dry-run is the default" bullet and gains the one that is now true: a run that produces no verdict posts nothing.

Breaking

The driver CLI drops --dry-run and the workflow drops the guard's dry_run output, so this wants a version bump. sei-protocol/platform pins uci v0.0.14 by commit SHA in two places (uses: and with: uci-ref) and needs a companion bump once this is tagged — I have that PR ready to open against the new tag.

Verification

  • Driver selftest: ALL PASS (18 checks)
  • ruff check driver/: clean
  • ruff format --check: clean on the three files touched. driver/tests/selftest.py has a pre-existing deviation on main and is left alone.
  • Both workflow YAMLs parse, jobs intact (guard, xreview).

Neither the selftest nor ruff is currently wired into CI, so these were run locally.

Dry-run suppressed the driver's verdict post and nothing else. It cost a full
runner and a full sandbox session, so it was not a cheap preview, and it required
a per-repo opt-in before any adopter could get a verdict onto a PR, which is the
only thing the feature exists to do.

It also read as a safety mode without being one. policy.dry_run_guard's own
docstring said so: Bash is permitted in both modes and the agent keeps its vended
gh/git token, so dry-run gated the driver's post and not the agent's writes. The
prompt did carry a DRY RUN instruction asking the agent not to mutate anything,
but that is a request to a model rather than a control, and it never applied in
the mode everyone actually runs.

Removed: the SEIDROID_XREVIEW_DRY_RUN repo variable, the --dry-run command flag,
the dry-run job-summary step, the driver's --dry-run argument and ReviewRequest
field, the prompt instruction, and policy.dry_run_guard. Posting is now gated on a
real verdict having been produced, which it already was.

The trigger match is now an exact whole line, so a comment that quotes or
discusses the command still does not trigger a review. That property used to be
described in terms of not letting prose force dry-run, and it survives on its own.

The untrusted-content preconditions in the README are kept and promoted. They
never depended on dry-run: the risk belongs to running the agent at all, and
dry-run was arguably making it easier to overlook. The section now says there is
no review mode that mitigates it. The safety-properties list loses the dry-run
bullet and gains the one that is actually true, that a run producing no verdict
posts nothing.

Breaking for consumers: the driver CLI drops --dry-run and the workflow drops the
guard's dry_run output, so this wants a version bump and a consumer ref bump.

Verified: driver selftest ALL PASS (18 checks), ruff check clean, ruff format
clean on the three files touched (selftest.py's pre-existing deviation left
alone), both workflow YAMLs parse.
@cursor

cursor Bot commented Jul 30, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Behavior change: adopting repos that relied on default dry-run will now post verdicts to PRs once they bump the pin; security posture for untrusted fork PRs is unchanged but no longer obscured by a false “safe mode.”

Overview
Removes dry-run from seidroid xreview so a successful review always upserts the sticky PR comment when the driver writes a real verdict—no repo variable, CLI flag, or job-summary-only path.

The reusable workflow drops SEIDROID_XREVIEW_DRY_RUN, guard dry_run output, --dry-run in the trigger regex, the dry-run summary step, and the post-step condition that required dry_run == 'false'. Posting is gated only on verdict_produced. The Python driver loses --dry-run, ReviewRequest.dry_run, the DRY RUN prompt line, and policy.dry_run_guard; runs always use best_effort_readonly_policy.

Docs are updated to state that dry-run was never a cheap or safe preview (full sandbox + vended gh write risk unchanged) and to retitle the untrusted-content checklist accordingly. The exact whole-line seidroid xreview trigger match is unchanged.

Reviewed by Cursor Bugbot for commit 7a218f3. Bugbot is set up for automated code reviews on this repo. Configure here.

@bdchatham
bdchatham merged commit 6590124 into main Jul 30, 2026
1 check passed
@bdchatham
bdchatham deleted the chore/xreview-remove-dry-run branch July 30, 2026 20:37
bdchatham added a commit that referenced this pull request Jul 30, 2026
Two documentation/organization tidy-ups in `.github/seidroid/`, no
behavior change.

## 1. Rename `auto-review/` → `ai-review/`

The folder holding the seidroid[bot] workflow prompts is renamed to
match the `ai-review` workflow name. Updated every reference so the live
prompt fetches keep resolving:
- `ai-review.yml` / `ai-assistant.yml`: the `sparse-checkout` path and
the `cat .../scout|review|assistant.md`, plus the `see …/README.md` doc
pointers.
- the seidroid index table row + link.

The folder contents (README, scout.md, review.md, assistant.md) are
unchanged; the workflows fetch the same files from the new path.

## 2. Trim the xreview README to xreview only

`xreview/README.md` no longer documents `ai-review`/`ai-assistant` —
dropped the cross-capability comparison table and the "difference is the
engine" prose, and added a pointer to the seidroid index
(`../README.md`) for how the capabilities relate. Also fixed a stray
`job summary)` fragment the dry-run removal (#50) left behind.

Verified: no `auto-review` references remain anywhere; both workflows
parse; the xreview README has no `ai-review`/`ai-assistant` mentions.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant