Commit Graph

6 Commits

Author SHA1 Message Date
hugocasa bbef5c9dd4 ping PR author when auto-review verdict is not good to merge (#9101)
* feat(ci): ping PR author when auto-review verdict is not good to merge

* fix(ci): drop (unknown) author fallback and clarify verdict-line rule
2026-05-11 14:56:52 +00:00
Ruben Fiszel a73f0bb2c5 ci: move shared review policy from .github/ to REVIEW.md at repo root
Policy is not GitHub-specific (also used by local-review skill); .github/
keeps only CI-tool output-format shims (codex/pi/claude prompt files).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-05 07:32:29 +00:00
Ruben Fiszel 11b60e5b95 ci: share review policy across Claude/Codex/Pi via review-prompt-shared.md (#9035)
* ci: share review policy across Claude/Codex/Pi via review-prompt-shared.md

All three reviewers now consume a single canonical policy document
(.github/review-prompt-shared.md) covering AGENTS.md compliance,
severity triage (P0/P1/P2), and a checklist for new public surfaces
(auth contract, module placement, half-finished pub fns, input
validation). Each tool's own prompt file shrinks to just its
output-format quirks, and each workflow concatenates shared +
tool-specific at runtime before invoking the model.

Drops the suppressive "Prefer at most 10 findings" / "Keep the review
high signal. If there is no clear issue, return no findings" wording
from Codex and Pi, which was clipping P1 and P2 findings (e.g.
half-finished pub fn, blocking I/O, wrong module placement).
Replaces it with severity triage so both reviewers report all P0/P1
and surface P2 when the diff invites it. Also makes AGENTS.md
authoritative for Codex (was CLAUDE.md, which is just @AGENTS.md in
this repo) and adds an explicit "new public function" checklist that
covers the missing-auth-check failure mode none of the three reviewers
flagged on the test PR.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci: move test-coverage assessment to shared prompt, slim per-tool prompts

- Replace per-tool 'Reproduction instructions' with a single shared
  'Test coverage assessment' section that asks each reviewer to
  evaluate automated coverage (sufficient / thin / appropriate) and
  describe what manual verification remains, if any.
- Slim per-tool prompts to the absolute minimum: just where to read
  context, the comment header, severity tagging, and the Pi-only 'no
  preamble' constraint. Everything else lives in the shared policy.
- Drop the model name from Pi's title ('Pi Review (DeepSeek V4)' →
  'Pi Review') — the title's job is to let the bot find its own prior
  comment when re-reviewing; the model is irrelevant to the reader.

The titles ('## Codex Review', '## Pi Review') stay because Codex and
Pi both post as github-actions[bot], so the heading is the only
discriminator the bot can use to find its own past comment in the
prior-discussion context.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci: scope test-coverage assessment to layers actually changed

Don't ask reviewers about integration tests on a frontend-only diff or
about playwright tests on a backend-only diff. The shared 'Test
coverage' section now lists categories (backend / frontend / CI-docs)
and tells the reviewer to skip the ones the PR does not touch — only
ask about Rust integration tests when backend handlers/workers/queues
were modified, only ask about frontend tests when components or state
machines were touched, and explicitly call out 'no automated tests
expected' for CI/docs/config diffs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci: don't ask reviewers to flag missing frontend component tests

The Windmill frontend codebase doesn't generally test Svelte components
— existing tests cover pure-logic utilities only (flowDiff,
previousResults, copilot logic, dbtable queries, etc.). Asking
reviewers to flag every new component for lacking a test would produce
noise inconsistent with the established convention. Limit the
frontend test-coverage check to new pure-logic utilities (files that
would naturally have a sibling *.test.ts).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci: point local-review skill at the shared review policy

Codex flagged (and Pi confirmed on its second pass) that slimming
.claude/review-prompt.md to output-only broke the local-review skill
contract — the skill still told Claude to read only that file for the
review criteria, so /local-review would no longer apply severity
triage, the public-surface checklist, or AGENTS.md compliance.

Update the skill to read .github/review-prompt-shared.md as the policy
source and .claude/review-prompt.md only for Claude output preferences.
Also align the local output format with the severity-tag convention
used by the workflow reviewers, and replace the lingering
'CLAUDE.md compliance' wording with 'AGENTS.md compliance'.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-04 17:22:06 +00:00
Ruben Fiszel 505f78bd29 ci: re-review on push, thread prior PR comments into reviewer context (#9032)
* ci: re-review on push, thread prior PR comments into reviewer context

- Add 'synchronize' to all three review workflow triggers so each push
  to a PR branch re-runs Claude/Codex/Pi. Existing
  cancel-in-progress concurrency groups ensure only the latest push's
  review actually executes.
- Fetch the most recent up to 20 PR comments before each review and
  inject them into the prompt context so the reviewer can recognize
  its own previous review, focus on what changed, and avoid repeating
  findings the human already addressed.
- Update the three review prompts (Claude, Codex, Pi) to instruct the
  reviewer to honor the prior-discussion section when present.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci: bump codex CLI to 0.128.0 for gpt-5.5 support

Codex 0.117.0 rejects the gpt-5.5 model with 'requires a newer version
of Codex'. 0.128.0 is the current stable release on npm.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci: limit synchronize re-trigger to pi review only

Re-running Claude and Codex on every push gets expensive fast on busy
PRs. Pi (DeepSeek-V4) is cheap enough to re-run per push, while
Claude/Codex remain on opened/ready_for_review and re-trigger via
slash commands.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-04 16:53:20 +00:00
Ruben Fiszel 548794cdbe ci: pi progress streaming, codex gpt-5.5 + danger-full-access sandbox (#9030)
* ci: install bubblewrap for codex sandbox; stream pi progress in logs

- Codex's vendored bwrap fails to set up loopback on some ubicloud
  runners, leaving codex unable to read any local files. Install the
  system bubblewrap package before running codex so its read-only
  sandbox works reliably.
- Switch pi to --mode json and pipe events through jq to surface
  agent/turn boundaries and tool calls live in the GitHub Actions log,
  matching codex's progress visibility. Final assistant text is
  extracted from the saved event log into pi-final-message.md for the
  PR comment.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci: drop bubblewrap install, use codex -s danger-full-access

Codex's read-only sandbox uses bwrap which fails to set up loopback on
some ubicloud runners. Rather than apt-installing bubblewrap, switch to
the no-sandbox mode for parity with how Pi and Claude already operate
in the same workflow — runner is ephemeral and we trust the codex
prompt the same way.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci: bump codex review model from gpt-5.4 to gpt-5.5

gpt-5.5 is positioned as the agentic successor to gpt-5.4 — same
per-token latency, fewer tokens to complete Codex tasks, and
explicitly stronger at holding context across large systems and
multi-tool reasoning, which matches the PR review workload.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-04 16:44:53 +00:00
Ruben Fiszel 1ee73c51ba ci: multi-tool PR reviews (Codex/Pi/Claude) with slash commands (#9026)
* ci: add Pi+DeepSeek-V4 review and slash command dispatcher

Auto-reviews now fan out to Claude (Opus), Codex (gpt-5.4), and Pi
(DeepSeek-V4-Pro) on PR open/ready. PR comments support /review (all
three), /codex, /pi, /claude with optional extra context appended to
the prompt. All review workflows now substitute EE code before review
and gate the auto-trigger path on org membership of the PR author.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci: fix command parser whitespace, align checkout v5, broaden PR perms

- Trim leading/trailing whitespace from comment first line so /review
  with leading space parses correctly (caught by Pi review)
- Standardize EE checkout step on actions/checkout@v5 across all three
  review workflows (caught by Pi review)
- Bump pull-requests permission to write to satisfy GitHub's PR
  comment endpoint when issues=write alone is rejected

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-04 16:26:36 +00:00