Add automated Wasp code review - #4554
Conversation
GitHub Actions
Inactive
@wasp.sh/spec
@wasp.sh/wasp-cli
@wasp.sh/wasp-cli-darwin-arm64-unknown
@wasp.sh/wasp-cli-darwin-x64-unknown
@wasp.sh/wasp-cli-linux-arm64-glibc
@wasp.sh/wasp-cli-linux-x64-glibc
@wasp.sh/wasp-cli-linux-x64-musl
commit: |
Deploying wasp-docs-on-main with
|
| Latest commit: |
317ab30
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://41b0fc72.wasp-docs-on-main.pages.dev |
| Branch Preview URL: | https://miho-wasp-review-skill-only.wasp-docs-on-main.pages.dev |
GitHub Actions
Inactive
|
Testing the review action by marking the PR ready for review. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Wasp reviewThe workflow has two trust-boundary flaws and does not rerun reviews when a PR’s base changes. |
GitHub Actions
Inactive
| env: | ||
| REVIEW_JSON: ${{ needs.review.outputs.review }} | ||
| run: | | ||
| printf '%s' "$REVIEW_JSON" | node .github/wasp-review/to-rdjson.mjs \ |
There was a problem hiding this comment.
🚫 [wasp-review] reported by reviewdog 🐶
Do not execute PR-controlled code in the publisher
This executes the converter from the PR checkout in a job that later exposes a write-scoped token. Run the converter from the trusted base commit or an immutable action outside the privileged job.
| safety-strategy: drop-sudo | ||
| output-schema-file: .github/wasp-review/output-schema.json | ||
| prompt: | | ||
| Use $wasp-review to review pull request |
There was a problem hiding this comment.
Keep review instructions outside the reviewed tree
The invoked skill comes from the PR checkout, allowing a change to suppress or reshape its own review. Load the skill and output schema from the trusted base revision while exposing the PR only as review data.
|
|
||
| on: | ||
| pull_request: | ||
| types: [opened, synchronize, reopened, ready_for_review] |
There was a problem hiding this comment.
Rerun reviews after base-branch changes
Changing a PR’s base emits pull_request.edited, which this trigger excludes, leaving the review based on the old diff. Add edited and gate it on github.event.changes.base.
GitHub Actions
Inactive
There was a problem hiding this comment.
Why not part of the existing ci workflow?
| contains( | ||
| fromJSON('["OWNER", "MEMBER", "COLLABORATOR"]'), | ||
| github.event.pull_request.author_association | ||
| ) |
There was a problem hiding this comment.
In the "label PRs as external", I tried doing stuff with these tags but they don't straightforwardly signal what I think we'd like them to. I ended up using only the github.event.pull_request.head.repo.full_name == github.repository check, and I think here you can do it too.
| review: ${{ steps.review.outputs.final-message }} | ||
| steps: | ||
| - name: Checkout pull request | ||
| uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 |
There was a problem hiding this comment.
We don't use sha-pinned actions anywhere else. (Yes they are good for security but they need an autoupdater too so we don't miss out on important improvements)
| - name: Checkout pull request | ||
| uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| with: | ||
| ref: refs/pull/${{ github.event.pull_request.number }}/merge |
There was a problem hiding this comment.
I'm quite sure this is already the ref when using the checkout action inside a PR trigger, i think
| ref: refs/pull/${{ github.event.pull_request.number }}/merge | ||
| persist-credentials: false | ||
|
|
||
| - name: Fetch pull request refs |
There was a problem hiding this comment.
If it's to bound the agent to review base and head refs, why don't we use the same SHAs here as you do in the message to the agent?
| publish: | ||
| name: Publish review | ||
| needs: review | ||
| if: ${{ needs.review.result == 'success' }} |
There was a problem hiding this comment.
| if: ${{ needs.review.result == 'success' }} |
This is already the default
| with: | ||
| github-token: ${{ github.token }} | ||
| script: | |
There was a problem hiding this comment.
you're looking for this instead gh pr comment --edit-last --create-if-none ...
| env: | ||
| REVIEW_JSON: ${{ needs.review.outputs.review }} | ||
| run: | | ||
| printf '%s' "$REVIEW_JSON" | node .github/wasp-review/to-rdjson.mjs \ |
There was a problem hiding this comment.
is there a meaningful difference?
| printf '%s' "$REVIEW_JSON" | node .github/wasp-review/to-rdjson.mjs \ | |
| echo "$REVIEW_JSON" | node .github/wasp-review/to-rdjson.mjs \ |
There was a problem hiding this comment.
I think this might be much simpler if we define the schema in zod and then write it to this file with z.toJsonSchema(), so we can use the same values and types everywhere
There was a problem hiding this comment.
you're lucky i didn't find any process.exit() here 🫡
2 participants
Description
Adds automated PR review.
The workflow runs
/wasp-reviewskill withgpt-5.6-sol. It reviews trusted same-repository PRs in a read-only Codex job, converts structured output to RDJSON, publishes inline findings through Reviewdog, and updates one summary comment. Draft and fork PRs are skipped.Alternative
CodeRabbit is a reasonable replacement if we no longer want to maintain this workflow. It handles incremental reviews and comment cleanup for us, and it is free for public open-source repositories. The tradeoff is less control: it can read our review skill as instructions, but it cannot run the skill directly or let us choose the exact model.
Type of change
Checklist
I tested my change in a Wasp app to verify that it works as intended.
🧪 Tests and apps:
examples/kitchen-sink/e2e-tests.waspc/data/Cli/templates, as needed.examples/, as needed.examples/tutorials) I updated the tutorial in the docs (and vice versa).📜 Documentation:
web/docs/.🆕 Changelog: (if change is more than just code/docs improvement)
waspc/ChangeLog.mdwith a user-friendly description of the change.web/docs/migration-guides/.versioninwaspc/waspc.cabalto reflect the changes I introduced.No Wasp app, e2e, starter, documentation, changelog, or version changes are needed for this repository automation.