fullsend: review on-demand only - #3184
Conversation
|
🤖 Finished Review · ❌ Failure (creating sandbox: sandbox creation failed after 3 attempts: sandbox create failed: exit status 2 (output: error: unexpected argument '--detach' found tip: to pass '--detach' as a value, use '-- --deta…) · Started 4:29 PM UTC · Completed 4:30 PM UTC Commit: Effort: high |
|
Build succeeded. ✔️ pre-commit SUCCESS in 2m 00s |
|
🤖 Finished Review · ❌ Failure (creating sandbox: sandbox creation failed after 3 attempts: sandbox create failed: exit status 2 (output: error: unexpected argument '--detach' found tip: to pass '--detach' as a value, use '-- --deta…) · Started 4:36 PM UTC · Completed 4:36 PM UTC Commit: Effort: high |
|
Build succeeded. ✔️ pre-commit SUCCESS in 2m 18s |
| || github.event.comment.user.type != 'Bot' | ||
| (github.event_name != 'issue_comment' | ||
| || (github.event.comment.user.type != 'Bot' | ||
| && github.event.comment.body != '/fs-review')) |
There was a problem hiding this comment.
Now that I think about it, do we want to somehow restrict who can trigger a review from fullsend (similarly how it's done for stop-fix bellow)? I'm thinking how likely it is that someone would use their non-bot account to spam '/fs-review' across PRs in all our repos where fullsend is enabled (could cost us a lot).
There was a problem hiding this comment.
thank you for reminding me this Bety, it was definitely something I wanted to look into
There was a problem hiding this comment.
good news! fullsend 0.40 already has this built in (thanks to Codex for checking it):
Fullsend v0.40 already restricts slash commands based on GitHub repository permissions, so we do not need an additional GitHub Actions check to prevent outside contributors from invoking agents.
| Command | Required repository role |
|---|---|
/fs-review, /fs-triage |
triage or higher |
/fs-code, /fs-fix, /fs-retro, /fs-prioritize |
write, maintain, or admin |
Bot comments are rejected. The ready-for-review label trigger is safe as well: applying a PR label requires write access, and our workflow additionally ignores bot-applied labels.
One terminology caveat: Fullsend does not use GitHub’s CONTRIBUTOR association. That only indicates someone has contributed before; it is not an authorization role. Fullsend uses repository collaborator permissions instead, which is the appropriate boundary here.
b6ba43a to
65352ba
Compare
|
🤖 Finished Review · ❌ Failure (creating sandbox: sandbox creation failed after 3 attempts: sandbox create failed: exit status 2 (output: error: unexpected argument '--detach' found tip: to pass '--detach' as a value, use '-- --deta…) · Started 7:31 AM UTC · Completed 7:31 AM UTC Commit: Effort: high |
|
Build succeeded. ✔️ pre-commit SUCCESS in 2m 08s |
betulependule
left a comment
There was a problem hiding this comment.
Looks good to me, thanks 🙏
65352ba to
54eafa7
Compare
|
🤖 Finished Review · ❌ Failure (creating sandbox: sandbox creation failed after 3 attempts: sandbox create failed: exit status 2 (output: error: unexpected argument '--detach' found tip: to pass '--detach' as a value, use '-- --deta…) · Started 2:58 PM UTC · Completed 2:58 PM UTC Commit: Effort: high |
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 53s |
|
Build succeeded (gate pipeline). ✔️ pre-commit SUCCESS in 1m 56s |
|
Pull request merge failed: Resource not accessible by integration, You may need to manually rebase your PR and retry. |
Signed-off-by: Tomas Tomecek <ttomecek@redhat.com> Assisted-by: Codex
Signed-off-by: Tomas Tomecek <ttomecek@redhat.com>
54eafa7 to
2be6ec3
Compare
|
🤖 Finished Review · ❌ Failure (creating sandbox: sandbox creation failed after 3 attempts: sandbox create failed: exit status 2 (output: error: unexpected argument '--detach' found tip: to pass '--detach' as a value, use '-- --deta…) · Started 9:15 AM UTC · Completed 9:15 AM UTC Commit: Effort: high |
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 56s |
|
Build succeeded (gate pipeline). ✔️ pre-commit SUCCESS in 1m 54s |
|
Pull request merge failed: Resource not accessible by integration, You may need to manually rebase your PR and retry. |
mfocko
left a comment
There was a problem hiding this comment.
LGTM, thanks a lot.
You’re updating the workflows, need to merge manually.
TODO:
RELEASE NOTES BEGIN
Internal change
RELEASE NOTES END