Skip to content

Commit 9748ae7

Browse files
authored
Merge pull request #74 from tstromberg/main
improve state logging & tracking
2 parents aa73a35 + 2aa019b commit 9748ae7

File tree

3 files changed

+41
-9
lines changed

3 files changed

+41
-9
lines changed

cmd/goose/github.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,7 @@ func (app *App) fetchPRsInternal(ctx context.Context) (incoming []PR, outgoing [
482482
Repository: repo,
483483
Author: issue.GetUser().GetLogin(),
484484
Number: issue.GetNumber(),
485+
CreatedAt: issue.GetCreatedAt().Time,
485486
UpdatedAt: issue.GetUpdatedAt().Time,
486487
IsDraft: issue.GetDraft(),
487488
}
@@ -577,6 +578,7 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
577578
actionReason := ""
578579
actionKind := ""
579580
testState := result.turnData.PullRequest.TestState
581+
workflowState := result.turnData.Analysis.WorkflowState
580582
if action, exists := result.turnData.Analysis.NextAction[user]; exists {
581583
needsReview = true
582584
isBlocked = action.Critical // Only critical actions are blocking
@@ -599,6 +601,7 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
599601
(*outgoing)[i].ActionReason = actionReason
600602
(*outgoing)[i].ActionKind = actionKind
601603
(*outgoing)[i].TestState = testState
604+
(*outgoing)[i].WorkflowState = workflowState
602605
break
603606
}
604607
} else {
@@ -610,6 +613,7 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
610613
(*incoming)[i].ActionReason = actionReason
611614
(*incoming)[i].ActionKind = actionKind
612615
(*incoming)[i].TestState = testState
616+
(*incoming)[i].WorkflowState = workflowState
613617
break
614618
}
615619
}

cmd/goose/main.go

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ const (
5454
// PR represents a pull request with metadata.
5555
type PR struct {
5656
UpdatedAt time.Time
57+
CreatedAt time.Time
5758
TurnDataAppliedAt time.Time
5859
FirstBlockedAt time.Time // When this PR was first detected as blocked
5960
Title string
@@ -63,6 +64,7 @@ type PR struct {
6364
ActionReason string
6465
ActionKind string // The kind of action expected (review, merge, fix_tests, etc.)
6566
TestState string // Test state from Turn API: "running", "passing", "failing", etc.
67+
WorkflowState string // Workflow state from Turn API: "running_tests", "waiting_for_review", etc.
6668
Number int
6769
IsDraft bool
6870
IsBlocked bool
@@ -863,21 +865,42 @@ func (app *App) tryAutoOpenPR(ctx context.Context, pr *PR, autoBrowserEnabled bo
863865
return
864866
}
865867

868+
// Only auto-open if the PR is actually blocked or needs review
869+
// This ensures we have a valid NextAction before opening
870+
if !pr.IsBlocked && !pr.NeedsReview {
871+
slog.Debug("[BROWSER] Skipping auto-open for non-blocked PR",
872+
"repo", pr.Repository, "number", pr.Number,
873+
"is_blocked", pr.IsBlocked, "needs_review", pr.NeedsReview)
874+
return
875+
}
876+
866877
if app.browserRateLimiter.CanOpen(startTime, pr.URL) {
867878
slog.Info("[BROWSER] Auto-opening newly blocked PR",
868-
"repo", pr.Repository, "number", pr.Number, "url", pr.URL)
879+
"repo", pr.Repository,
880+
"number", pr.Number,
881+
"url", pr.URL,
882+
"workflow_state", pr.WorkflowState,
883+
"test_state", pr.TestState,
884+
"is_draft", pr.IsDraft,
885+
"age_since_creation", time.Since(pr.CreatedAt).Round(time.Second),
886+
"age_since_update", time.Since(pr.UpdatedAt).Round(time.Second))
869887
// Use strict validation for auto-opened URLs
870888
// Validate against strict GitHub PR URL pattern for auto-opening
871889
if err := validateGitHubPRURL(pr.URL); err != nil {
872890
slog.Warn("Auto-open strict validation failed", "url", sanitizeForLog(pr.URL), "error", err)
873891
return
874892
}
875-
if err := openURL(ctx, pr.URL); err != nil {
893+
// Use ActionKind as the goose parameter value, or "next_action" if not set
894+
gooseParam := pr.ActionKind
895+
if gooseParam == "" {
896+
gooseParam = "next_action"
897+
}
898+
if err := openURL(ctx, pr.URL, gooseParam); err != nil {
876899
slog.Error("[BROWSER] Failed to auto-open PR", "url", pr.URL, "error", err)
877900
} else {
878901
app.browserRateLimiter.RecordOpen(pr.URL)
879902
slog.Info("[BROWSER] Successfully opened PR in browser",
880-
"repo", pr.Repository, "number", pr.Number, "action", pr.ActionKind)
903+
"repo", pr.Repository, "number", pr.Number, "action", pr.ActionKind, "goose_param", gooseParam)
881904
}
882905
}
883906
}

cmd/goose/ui.go

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@ import (
2020
var _ *systray.MenuItem = nil
2121

2222
// openURL safely opens a URL in the default browser after validation.
23-
func openURL(ctx context.Context, rawURL string) error {
23+
// The gooseParam parameter specifies what value to use for the ?goose= query parameter.
24+
// If empty, defaults to "1" for menu clicks.
25+
func openURL(ctx context.Context, rawURL string, gooseParam string) error {
2426
// Parse and validate the URL
2527
u, err := url.Parse(rawURL)
2628
if err != nil {
@@ -45,10 +47,13 @@ func openURL(ctx context.Context, rawURL string) error {
4547
return errors.New("URLs with user info are not allowed")
4648
}
4749

48-
// Add goose=1 parameter to track source for GitHub and dash URLs
50+
// Add goose parameter to track source for GitHub and dash URLs
4951
if u.Host == "github.com" || u.Host == "www.github.com" || u.Host == "dash.ready-to-review.dev" {
5052
q := u.Query()
51-
q.Set("goose", "1")
53+
if gooseParam == "" {
54+
gooseParam = "1"
55+
}
56+
q.Set("goose", gooseParam)
5257
u.RawQuery = q.Encode()
5358
rawURL = u.String()
5459
}
@@ -423,7 +428,7 @@ func (app *App) addPRSection(ctx context.Context, prs []PR, sectionTitle string,
423428
// Capture URL to avoid loop variable capture bug
424429
prURL := sortedPRs[prIndex].URL
425430
item.Click(func() {
426-
if err := openURL(ctx, prURL); err != nil {
431+
if err := openURL(ctx, prURL, ""); err != nil {
427432
slog.Error("failed to open url", "error", err)
428433
}
429434
})
@@ -619,7 +624,7 @@ func (app *App) rebuildMenu(ctx context.Context) {
619624
// Add error details
620625
errorMsg := app.systrayInterface.AddMenuItem(authError, "Click to see setup instructions")
621626
errorMsg.Click(func() {
622-
if err := openURL(ctx, "https://cli.github.com/manual/gh_auth_login"); err != nil {
627+
if err := openURL(ctx, "https://cli.github.com/manual/gh_auth_login", ""); err != nil {
623628
slog.Error("failed to open setup instructions", "error", err)
624629
}
625630
})
@@ -729,7 +734,7 @@ func (app *App) rebuildMenu(ctx context.Context) {
729734
// Add Web Dashboard link
730735
dashboardItem := app.systrayInterface.AddMenuItem("Web Dashboard", "")
731736
dashboardItem.Click(func() {
732-
if err := openURL(ctx, "https://dash.ready-to-review.dev/"); err != nil {
737+
if err := openURL(ctx, "https://dash.ready-to-review.dev/", ""); err != nil {
733738
slog.Error("failed to open dashboard", "error", err)
734739
}
735740
})

0 commit comments

Comments
 (0)