From 07c50ba5c75798250542c7e987bda79cb4ebbb15 Mon Sep 17 00:00:00 2001 From: fullsend-code <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Fri, 10 Jul 2026 12:55:27 +0000 Subject: [PATCH 1/2] fix(#230): detect self-review 422 and degrade gracefully when REVIEW_TOKEN missing When REVIEW_TOKEN is undefined, post-review.sh hard-failed before reaching fullsend post-review. When the Go CLI fell back to GITHUB_TOKEN, the review submission hit a 422 because GitHub rejects self-reviews (PR author = token identity). Changes: - post-review.sh: fall back to GITHUB_TOKEN with a warning instead of hard-failing, so the agent's sticky comment and label work still completes - postreview.go: warn when falling back to GITHUB_TOKEN; detect self-review 422 errors and surface an actionable message pointing operators to set REVIEW_TOKEN via the mint-token action - Add isSelfReviewError helper with tests Closes #230 --- internal/cli/postreview.go | 40 +++++++++++++++++ internal/cli/postreview_test.go | 45 ++++++++++++++++++- .../fullsend-repo/scripts/post-review.sh | 7 ++- 3 files changed, 89 insertions(+), 3 deletions(-) diff --git a/internal/cli/postreview.go b/internal/cli/postreview.go index eb9be86eb..9991e1866 100644 --- a/internal/cli/postreview.go +++ b/internal/cli/postreview.go @@ -3,7 +3,9 @@ package cli import ( "context" "encoding/json" + "errors" "fmt" + "net/http" "os" "regexp" "strconv" @@ -61,12 +63,17 @@ has moved, a stale-head failure is posted instead.`, RunE: func(cmd *cobra.Command, args []string) error { printer := ui.New(os.Stdout) + tokenFallback := false if token == "" { token = os.Getenv("GITHUB_TOKEN") + tokenFallback = true } if token == "" { return fmt.Errorf("--token or GITHUB_TOKEN required") } + if tokenFallback { + printer.StepWarn("No --token provided, falling back to GITHUB_TOKEN; self-review 422 errors may occur if this token shares identity with the PR author") + } if pr <= 0 { return fmt.Errorf("--pr must be a positive integer, got %d", pr) @@ -357,12 +364,45 @@ func submitFormalReview(ctx context.Context, client forge.Client, owner, repo st printer.StepInfo(fmt.Sprintf("Attaching %d inline comment(s)", len(inlineComments))) } if err := client.CreatePullRequestReview(ctx, owner, repo, pr, event, reviewBody, commitSHA, inlineComments); err != nil { + if isSelfReviewError(err) { + return fmt.Errorf("submitting review: %w — the token identity matches the PR author; "+ + "set REVIEW_TOKEN to a minted token with pull-requests:write permission "+ + "(see the mint-token action) so the review is submitted as a different identity", err) + } return fmt.Errorf("submitting review: %w", err) } printer.StepDone("Review submitted") return nil } +// isSelfReviewError checks whether an error from CreatePullRequestReview is +// a 422 caused by the authenticated user attempting to review their own PR. +// GitHub rejects this with a validation error. Detecting it lets us surface +// a clear message pointing operators to use a minted REVIEW_TOKEN instead of +// the default GITHUB_TOKEN. +func isSelfReviewError(err error) bool { + var apiErr *gh.APIError + if !errors.As(err, &apiErr) { + return false + } + if apiErr.StatusCode != http.StatusUnprocessableEntity { + return false + } + // GitHub's error message for self-review attempts varies, but the + // combination of 422 + validation message mentioning "review" on a + // review submission call is sufficient. Check both the top-level + // message and detail messages. + combined := strings.ToLower(apiErr.Message) + for _, d := range apiErr.Errors { + combined += " " + strings.ToLower(d.Message) + } + // ponytail: matches GitHub's "Can not approve your own pull request" + // and similar phrasings. Broader match catches future wording changes. + return strings.Contains(combined, "your own") || + strings.Contains(combined, "can not") || + strings.Contains(combined, "cannot") +} + // findingsToReviewComments converts review findings with file and line // locations into inline review comments. Findings without a file path // or line number are omitted — they remain in the sticky comment body. diff --git a/internal/cli/postreview_test.go b/internal/cli/postreview_test.go index 05b7866ca..fb3e461e4 100644 --- a/internal/cli/postreview_test.go +++ b/internal/cli/postreview_test.go @@ -4,12 +4,14 @@ import ( "bytes" "context" "fmt" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "io" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/fullsend-ai/fullsend/internal/forge" + gh "github.com/fullsend-ai/fullsend/internal/forge/github" "github.com/fullsend-ai/fullsend/internal/sticky" "github.com/fullsend-ai/fullsend/internal/ui" ) @@ -1001,3 +1003,42 @@ func TestPostApprovedFollowUpIssues_DisabledIsNoop(t *testing.T) { err := postApprovedFollowUpIssues(context.Background(), "acme", "repo", 9, parsed, printer) require.NoError(t, err) } + +func TestIsSelfReviewError(t *testing.T) { + tests := []struct { + name string + err error + want bool + }{ + { + name: "GitHub self-review 422", + err: fmt.Errorf("create pull request review on #42: %w", &gh.APIError{StatusCode: 422, Message: "Validation Failed", Errors: []gh.APIErrorDetail{{Message: "Can not approve your own pull request"}}}), + want: true, + }, + { + name: "alternate wording cannot", + err: &gh.APIError{StatusCode: 422, Message: "You cannot review your own pull request"}, + want: true, + }, + { + name: "non-422 error", + err: &gh.APIError{StatusCode: 403, Message: "Resource not accessible by integration"}, + want: false, + }, + { + name: "422 unrelated validation", + err: &gh.APIError{StatusCode: 422, Message: "Validation Failed", Errors: []gh.APIErrorDetail{{Message: "commit_id is not part of the pull request"}}}, + want: false, + }, + { + name: "non-API error", + err: fmt.Errorf("network timeout"), + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, isSelfReviewError(tt.err)) + }) + } +} diff --git a/internal/scaffold/fullsend-repo/scripts/post-review.sh b/internal/scaffold/fullsend-repo/scripts/post-review.sh index 955c64de1..20c787b7e 100755 --- a/internal/scaffold/fullsend-repo/scripts/post-review.sh +++ b/internal/scaffold/fullsend-repo/scripts/post-review.sh @@ -18,7 +18,12 @@ # 1 — error (review not posted or fallback comment posted) set -euo pipefail -: "${REVIEW_TOKEN:?REVIEW_TOKEN is required}" +if [[ -z "${REVIEW_TOKEN:-}" ]]; then + # ponytail: fall back to GITHUB_TOKEN with a warning instead of hard-failing. + # The Go CLI detects self-review 422s and gives a clear error message. + echo "::warning::REVIEW_TOKEN is not set; falling back to GITHUB_TOKEN — self-review 422 errors may occur if this token shares identity with the PR author" + REVIEW_TOKEN="${GITHUB_TOKEN:?Neither REVIEW_TOKEN nor GITHUB_TOKEN is set}" +fi : "${PR_NUMBER:?PR_NUMBER is required}" if ! [[ "${PR_NUMBER}" =~ ^[0-9]+$ ]]; then echo "::error::PR_NUMBER must be a positive integer" From 599e2c4df35ad2acd5dc7913ed2db500bde000ec Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Sat, 11 Jul 2026 05:43:54 +0000 Subject: [PATCH 2/2] fix(#230): tighten isSelfReviewError matching and update stale docs - Narrow "can not"/"cannot" matches to "can not approve"/"cannot review" to avoid false positives on unrelated 422s (e.g. draft PR errors) - Add test case for 422 draft-PR false-positive scenario - Update error message to reference both REVIEW_TOKEN and --token - Mark REVIEW_TOKEN as optional in post-review.sh header comment Addresses review feedback on #271 --- internal/cli/postreview.go | 11 ++++++----- internal/cli/postreview_test.go | 5 +++++ .../scaffold/fullsend-repo/scripts/post-review.sh | 4 ++-- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/internal/cli/postreview.go b/internal/cli/postreview.go index 9991e1866..3257693fc 100644 --- a/internal/cli/postreview.go +++ b/internal/cli/postreview.go @@ -366,8 +366,8 @@ func submitFormalReview(ctx context.Context, client forge.Client, owner, repo st if err := client.CreatePullRequestReview(ctx, owner, repo, pr, event, reviewBody, commitSHA, inlineComments); err != nil { if isSelfReviewError(err) { return fmt.Errorf("submitting review: %w — the token identity matches the PR author; "+ - "set REVIEW_TOKEN to a minted token with pull-requests:write permission "+ - "(see the mint-token action) so the review is submitted as a different identity", err) + "set REVIEW_TOKEN (in post-review.sh) or pass --token with a minted token "+ + "that has pull-requests:write permission so the review is submitted as a different identity", err) } return fmt.Errorf("submitting review: %w", err) } @@ -397,10 +397,11 @@ func isSelfReviewError(err error) bool { combined += " " + strings.ToLower(d.Message) } // ponytail: matches GitHub's "Can not approve your own pull request" - // and similar phrasings. Broader match catches future wording changes. + // and similar phrasings. Tightened to avoid false positives on + // unrelated 422s (e.g. "cannot request changes on a draft"). return strings.Contains(combined, "your own") || - strings.Contains(combined, "can not") || - strings.Contains(combined, "cannot") + strings.Contains(combined, "can not approve") || + strings.Contains(combined, "cannot review") } // findingsToReviewComments converts review findings with file and line diff --git a/internal/cli/postreview_test.go b/internal/cli/postreview_test.go index fb3e461e4..29e58b4ac 100644 --- a/internal/cli/postreview_test.go +++ b/internal/cli/postreview_test.go @@ -1030,6 +1030,11 @@ func TestIsSelfReviewError(t *testing.T) { err: &gh.APIError{StatusCode: 422, Message: "Validation Failed", Errors: []gh.APIErrorDetail{{Message: "commit_id is not part of the pull request"}}}, want: false, }, + { + name: "422 draft PR cannot request changes", + err: &gh.APIError{StatusCode: 422, Message: "You cannot request changes on a draft pull request"}, + want: false, + }, { name: "non-API error", err: fmt.Errorf("network timeout"), diff --git a/internal/scaffold/fullsend-repo/scripts/post-review.sh b/internal/scaffold/fullsend-repo/scripts/post-review.sh index 20c787b7e..c05240e22 100755 --- a/internal/scaffold/fullsend-repo/scripts/post-review.sh +++ b/internal/scaffold/fullsend-repo/scripts/post-review.sh @@ -8,8 +8,8 @@ # if the PR touches sensitive paths, an "approve" action is downgraded # to "comment" so only a human can grant approval. # -# Required environment variables: -# REVIEW_TOKEN — token with pull-requests:write on the target repo +# Environment variables: +# REVIEW_TOKEN — (optional) token with pull-requests:write; falls back to GITHUB_TOKEN # PR_NUMBER — GitHub PR number # REPO_FULL_NAME — owner/repo (e.g. my-org/my-repo) #