Merge pull request #56 from tstromberg/gooselink

tstromberg · web-flow · commit 9971e2dfd228 · 2025-09-23T10:23:17.000-04:00
add next action to goose value in URL
diff --git a/cmd/goose/github.go b/cmd/goose/github.go
@@ -462,10 +462,12 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
 			needsReview := false
 			isBlocked := false
 			actionReason := ""
+			actionKind := ""
 			if action, exists := result.turnData.Analysis.NextAction[user]; exists {
 				needsReview = true
 				isBlocked = action.Critical // Only critical actions are blocking
 				actionReason = action.Reason
+				actionKind = string(action.Kind)
 				// Only log fresh API calls
 				if !result.wasFromCache {
 					slog.Debug("[TURN] NextAction", "url", result.url, "reason", action.Reason, "kind", action.Kind, "critical", action.Critical)
@@ -479,6 +481,7 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
 						(*outgoing)[i].NeedsReview = needsReview
 						(*outgoing)[i].IsBlocked = isBlocked
 						(*outgoing)[i].ActionReason = actionReason
+						(*outgoing)[i].ActionKind = actionKind
 						break
 					}
 				}
@@ -487,6 +490,7 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
 					if (*incoming)[i].URL == result.url {
 						(*incoming)[i].NeedsReview = needsReview
 						(*incoming)[i].ActionReason = actionReason
+						(*incoming)[i].ActionKind = actionKind
 						break
 					}
 				}
diff --git a/cmd/goose/main.go b/cmd/goose/main.go
@@ -57,6 +57,7 @@ type PR struct {
 	URL               string
 	Repository        string
 	ActionReason      string
+	ActionKind        string // The kind of action expected (review, merge, fix_tests, etc.)
 	Number            int
 	IsBlocked         bool
 	NeedsReview       bool
@@ -675,12 +676,12 @@ func (app *App) tryAutoOpenPR(ctx context.Context, pr PR, autoBrowserEnabled boo
 			slog.Warn("Auto-open strict validation failed", "url", sanitizeForLog(pr.URL), "error", err)
 			return
 		}
-		if err := openURL(ctx, pr.URL); err != nil {
+		if err := openURL(ctx, pr.URL, pr.ActionKind); err != nil {
 			slog.Error("[BROWSER] Failed to auto-open PR", "url", pr.URL, "error", err)
 		} else {
 			app.browserRateLimiter.RecordOpen(pr.URL)
 			slog.Info("[BROWSER] Successfully opened PR in browser",
-				"repo", pr.Repository, "number", pr.Number)
+				"repo", pr.Repository, "number", pr.Number, "action", pr.ActionKind)
 		}
 	}
 }
diff --git a/cmd/goose/security.go b/cmd/goose/security.go
@@ -124,17 +124,17 @@ func validateURL(rawURL string) error {
 // validateGitHubPRURL performs strict validation for GitHub PR URLs used in auto-opening.
 // This ensures the URL follows the exact pattern: https://github.com/{owner}/{repo}/pull/{number}
 // with no additional path segments, fragments, or suspicious characters.
-// The URL may optionally have ?goose=1 parameter which we add for tracking.
+// The URL may optionally have ?goose=<action> parameter which we add for tracking.
 func validateGitHubPRURL(rawURL string) error {
 	// First do basic URL validation
 	if err := validateURL(rawURL); err != nil {
 		return err
 	}
 
-	// Strip the ?goose=1 parameter if present for pattern validation
+	// Strip the ?goose parameter if present for pattern validation
 	urlToValidate := rawURL
-	if strings.HasSuffix(rawURL, "?goose=1") {
-		urlToValidate = strings.TrimSuffix(rawURL, "?goose=1")
+	if idx := strings.Index(rawURL, "?goose="); idx != -1 {
+		urlToValidate = rawURL[:idx]
 	}
 
 	// Check against strict GitHub PR URL pattern
@@ -149,19 +149,34 @@ func validateGitHubPRURL(rawURL string) error {
 	}
 
 	// Reject URLs with URL encoding (could hide malicious content)
-	// Exception: %3D which is = in URL encoding, only as part of ?goose=1
-	if strings.Contains(rawURL, "%") && !strings.HasSuffix(rawURL, "?goose%3D1") {
-		return errors.New("URL contains encoded characters")
+	// Exception: %3D which is = in URL encoding, only as part of ?goose parameter
+	if strings.Contains(rawURL, "%") {
+		// Allow URL encoding only in the goose parameter value
+		if idx := strings.Index(rawURL, "?goose="); idx != -1 {
+			// Check if encoding is only in the goose parameter
+			if strings.Contains(rawURL[:idx], "%") {
+				return errors.New("URL contains encoded characters outside goose parameter")
+			}
+		} else {
+			return errors.New("URL contains encoded characters")
+		}
 	}
 
 	// Reject URLs with fragments
 	if strings.Contains(rawURL, "#") {
 		return errors.New("URL contains fragments")
 	}
 
-	// Allow only ?goose=1 query parameter, nothing else
-	if strings.Contains(rawURL, "?") && !strings.HasSuffix(rawURL, "?goose=1") && !strings.HasSuffix(rawURL, "?goose%3D1") {
-		return errors.New("URL contains unexpected query parameters")
+	// Allow only ?goose=<value> query parameter, nothing else
+	if strings.Contains(rawURL, "?") {
+		// Check if it's the goose parameter
+		if idx := strings.Index(rawURL, "?goose="); idx == -1 {
+			return errors.New("URL contains unexpected query parameters")
+		}
+		// Ensure no additional parameters after goose
+		if strings.Contains(rawURL[strings.Index(rawURL, "?goose=")+7:], "&") {
+			return errors.New("URL contains additional query parameters")
+		}
 	}
 
 	// Reject URLs with double slashes (except after https:)
diff --git a/cmd/goose/security_test.go b/cmd/goose/security_test.go
@@ -21,6 +21,26 @@ func TestValidateGitHubPRURL(t *testing.T) {
 			url:     "https://github.com/owner/repo/pull/123?goose=1",
 			wantErr: false,
 		},
+		{
+			name:    "valid PR URL with goose=review param",
+			url:     "https://github.com/owner/repo/pull/123?goose=review",
+			wantErr: false,
+		},
+		{
+			name:    "valid PR URL with goose=merge param",
+			url:     "https://github.com/owner/repo/pull/123?goose=merge",
+			wantErr: false,
+		},
+		{
+			name:    "valid PR URL with goose=fix_tests param",
+			url:     "https://github.com/owner/repo/pull/123?goose=fix_tests",
+			wantErr: false,
+		},
+		{
+			name:    "valid PR URL with goose=resolve_comments param",
+			url:     "https://github.com/owner/repo/pull/123?goose=resolve_comments",
+			wantErr: false,
+		},
 		{
 			name:    "valid PR URL with hyphen in owner",
 			url:     "https://github.com/owner-name/repo/pull/1",
@@ -65,6 +85,11 @@ func TestValidateGitHubPRURL(t *testing.T) {
 		},
 
 		// Invalid URLs - wrong format
+		{
+			name:    "URL with multiple query parameters",
+			url:     "https://github.com/owner/repo/pull/123?goose=review&other=param",
+			wantErr: true,
+		},
 		{
 			name:    "not a PR URL",
 			url:     "https://github.com/owner/repo",
diff --git a/cmd/goose/ui.go b/cmd/goose/ui.go
@@ -20,7 +20,8 @@ import (
 var _ *systray.MenuItem = nil
 
 // openURL safely opens a URL in the default browser after validation.
-func openURL(ctx context.Context, rawURL string) error {
+// action parameter is optional and specifies the expected action type for tracking.
+func openURL(ctx context.Context, rawURL string, action string) error {
 	// Parse and validate the URL
 	u, err := url.Parse(rawURL)
 	if err != nil {
@@ -45,10 +46,15 @@ func openURL(ctx context.Context, rawURL string) error {
 		return errors.New("URLs with user info are not allowed")
 	}
 
-	// Add goose=1 parameter to track source for GitHub and dash URLs
+	// Add goose parameter to track source and action for GitHub and dash URLs
 	if u.Host == "github.com" || u.Host == "www.github.com" || u.Host == "dash.ready-to-review.dev" {
 		q := u.Query()
-		q.Set("goose", "1")
+		// Use action if provided, otherwise default to "1" for backward compatibility
+		if action != "" {
+			q.Set("goose", action)
+		} else {
+			q.Set("goose", "1")
+		}
 		u.RawQuery = q.Encode()
 		rawURL = u.String()
 	}
@@ -284,10 +290,11 @@ func (app *App) addPRSection(ctx context.Context, prs []PR, sectionTitle string,
 		// Create PR menu item
 		item := app.systrayInterface.AddMenuItem(title, tooltip)
 
-		// Capture URL to avoid loop variable capture bug
+		// Capture URL and action to avoid loop variable capture bug
 		prURL := sortedPRs[prIndex].URL
+		prAction := sortedPRs[prIndex].ActionKind
 		item.Click(func() {
-			if err := openURL(ctx, prURL); err != nil {
+			if err := openURL(ctx, prURL, prAction); err != nil {
 				slog.Error("failed to open url", "error", err)
 			}
 		})
@@ -426,7 +433,7 @@ func (app *App) rebuildMenu(ctx context.Context) {
 		// Add error details
 		errorMsg := app.systrayInterface.AddMenuItem(authError, "Click to see setup instructions")
 		errorMsg.Click(func() {
-			if err := openURL(ctx, "https://cli.github.com/manual/gh_auth_login"); err != nil {
+			if err := openURL(ctx, "https://cli.github.com/manual/gh_auth_login", ""); err != nil {
 				slog.Error("failed to open setup instructions", "error", err)
 			}
 		})
@@ -536,7 +543,7 @@ func (app *App) rebuildMenu(ctx context.Context) {
 	// Add Web Dashboard link
 	dashboardItem := app.systrayInterface.AddMenuItem("Web Dashboard", "")
 	dashboardItem.Click(func() {
-		if err := openURL(ctx, "https://dash.ready-to-review.dev/"); err != nil {
+		if err := openURL(ctx, "https://dash.ready-to-review.dev/", ""); err != nil {
 			slog.Error("failed to open dashboard", "error", err)
 		}
 	})
diff --git a/go.mod b/go.mod
@@ -13,7 +13,7 @@ require (
 
 require (
 	git.sr.ht/~jackmordaunt/go-toast v1.1.2 // indirect
-	github.com/codeGROOVE-dev/prx v0.0.0-20250908203157-0711b3ec5471 // indirect
+	github.com/codeGROOVE-dev/prx v0.0.0-20250923100916-d2b60be50274 // indirect
 	github.com/esiqveland/notify v0.13.3 // indirect
 	github.com/go-ole/go-ole v1.3.0 // indirect
 	github.com/godbus/dbus/v5 v5.1.0 // indirect
diff --git a/go.sum b/go.sum
@@ -2,6 +2,8 @@ git.sr.ht/~jackmordaunt/go-toast v1.1.2 h1:/yrfI55LRt1M7H1vkaw+NaH1+L1CDxrqDltwm
 git.sr.ht/~jackmordaunt/go-toast v1.1.2/go.mod h1:jA4OqHKTQ4AFBdwrSnwnskUIIS3HYzlJSgdzCKqfavo=
 github.com/codeGROOVE-dev/prx v0.0.0-20250908203157-0711b3ec5471 h1:CbUa70O+iNC9rPk5aoZGs/RZbpmPyfNydv5ncKLdOvk=
 github.com/codeGROOVE-dev/prx v0.0.0-20250908203157-0711b3ec5471/go.mod h1:7qLbi18baOyS8yO/6/64SBIqtyzSzLFdsDST15NPH3w=
+github.com/codeGROOVE-dev/prx v0.0.0-20250923100916-d2b60be50274 h1:9eLzQdOaQEn30279ai3YjNdJOM/efbcYanWC9juAJ+M=
+github.com/codeGROOVE-dev/prx v0.0.0-20250923100916-d2b60be50274/go.mod h1:7qLbi18baOyS8yO/6/64SBIqtyzSzLFdsDST15NPH3w=
 github.com/codeGROOVE-dev/retry v1.2.0 h1:xYpYPX2PQZmdHwuiQAGGzsBm392xIMl4nfMEFApQnu8=
 github.com/codeGROOVE-dev/retry v1.2.0/go.mod h1:8OgefgV1XP7lzX2PdKlCXILsYKuz6b4ZpHa/20iLi8E=
 github.com/codeGROOVE-dev/turnclient v0.0.0-20250922145707-664c2dcdf5b8 h1:3088TLJGgxzjM/bR1gafKQ609NMkBNlZe1Fd5SnRrrY=

Original file line number	Diff line number	Diff line change
`@@ -57,6 +57,7 @@ type PR struct {`
`57`	`57`	`URL string`
`58`	`58`	`Repository string`
`59`	`59`	`ActionReason string`
	`60`	`+ ActionKind string // The kind of action expected (review, merge, fix_tests, etc.)`
`60`	`61`	`Number int`
`61`	`62`	`IsBlocked bool`
`62`	`63`	`NeedsReview bool`
`@@ -675,12 +676,12 @@ func (app *App) tryAutoOpenPR(ctx context.Context, pr PR, autoBrowserEnabled boo`
`675`	`676`	`slog.Warn("Auto-open strict validation failed", "url", sanitizeForLog(pr.URL), "error", err)`
`676`	`677`	`return`
`677`	`678`	`}`
`678`		`- if err := openURL(ctx, pr.URL); err != nil {`
	`679`	`+ if err := openURL(ctx, pr.URL, pr.ActionKind); err != nil {`
`679`	`680`	`slog.Error("[BROWSER] Failed to auto-open PR", "url", pr.URL, "error", err)`
`680`	`681`	`} else {`
`681`	`682`	`app.browserRateLimiter.RecordOpen(pr.URL)`
`682`	`683`	`slog.Info("[BROWSER] Successfully opened PR in browser",`
`683`		`- "repo", pr.Repository, "number", pr.Number)`
	`684`	`+ "repo", pr.Repository, "number", pr.Number, "action", pr.ActionKind)`
`684`	`685`	`}`
`685`	`686`	`}`
`686`	`687`	`}`