Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions internal/cli/postreview.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,11 @@ has moved, a stale-head failure is posted instead.`,
RunE: func(cmd *cobra.Command, args []string) error {
printer := ui.New(os.Stdout)

if token == "" {
token = os.Getenv("GITHUB_TOKEN")
}
if token == "" {
return fmt.Errorf("--token or GITHUB_TOKEN required")
resolved, err := resolveReviewToken(token)
if err != nil {
return err
}
token = resolved

if pr <= 0 {
return fmt.Errorf("--pr must be a positive integer, got %d", pr)
Expand Down Expand Up @@ -137,7 +136,7 @@ has moved, a stale-head failure is posted instead.`,
cmd.Flags().StringVar(&repo, "repo", "", "repository in owner/repo format (required)")
cmd.Flags().IntVar(&pr, "pr", 0, "pull request number (required)")
cmd.Flags().StringVar(&result, "result", "-", "path to review result file, or '-' for stdin")
cmd.Flags().StringVar(&token, "token", "", "GitHub token (default: $GITHUB_TOKEN)")
cmd.Flags().StringVar(&token, "token", "", "GitHub token (default: $REVIEW_TOKEN)")
cmd.Flags().StringVar(&headSHA, "head-sha", "", "expected PR HEAD SHA (skips review if HEAD has moved)")
cmd.Flags().BoolVar(&dryRun, "dry-run", false, "print what would be posted without making API calls")
_ = cmd.MarkFlagRequired("repo")
Expand All @@ -146,6 +145,21 @@ has moved, a stale-head failure is posted instead.`,
return cmd
}

// resolveReviewToken returns the token to use for review submission.
// Priority: explicit flag value > REVIEW_TOKEN env var > error.
// GITHUB_TOKEN is deliberately not used as a fallback because it
// inherits the workflow initiator's identity, causing 422 self-review
// errors when the PR author matches the workflow identity.
func resolveReviewToken(flagValue string) (string, error) {
if flagValue != "" {
return flagValue, nil
}
if t := os.Getenv("REVIEW_TOKEN"); t != "" {
return t, nil
}
return "", fmt.Errorf("--token or $REVIEW_TOKEN required; GITHUB_TOKEN is not used as a fallback because it causes 422 self-review errors when the workflow identity matches the PR author")
}

// ReviewResult represents a parsed review result file.
type ReviewResult struct {
Body string `json:"body"`
Expand Down
48 changes: 48 additions & 0 deletions internal/cli/postreview_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1001,3 +1001,51 @@ func TestPostApprovedFollowUpIssues_DisabledIsNoop(t *testing.T) {
err := postApprovedFollowUpIssues(context.Background(), "acme", "repo", 9, parsed, printer)
require.NoError(t, err)
}

func TestResolveReviewToken(t *testing.T) {
tests := []struct {
name string
flagValue string
envToken string
envGH string
want string
wantErr bool
}{
{
name: "flag value takes priority",
flagValue: "flag-token",
envToken: "env-token",
want: "flag-token",
},
{
name: "falls back to REVIEW_TOKEN",
envToken: "env-token",
want: "env-token",
},
{
name: "does not fall back to GITHUB_TOKEN",
envGH: "gh-token",
wantErr: true,
},
{
name: "errors when no token available",
wantErr: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Setenv("REVIEW_TOKEN", tt.envToken)
t.Setenv("GITHUB_TOKEN", tt.envGH)

got, err := resolveReviewToken(tt.flagValue)
if tt.wantErr {
require.Error(t, err)
assert.Contains(t, err.Error(), "REVIEW_TOKEN")
return
}
require.NoError(t, err)
assert.Equal(t, tt.want, got)
})
}
}
Loading