From 81c239d364ac06f85f42036c773441f48be70605 Mon Sep 17 00:00:00 2001 From: Rob Berwick Date: Sat, 31 Jan 2026 16:51:24 +0000 Subject: [PATCH 01/22] feat: add support for dynamic task finished text Add FinishedText field to TaskFinishedMsg to allow tasks to override their completion message based on the actual outcome. This enables more accurate feedback when task results vary (e.g., merge vs queue). --- internal/tui/constants/progressMsg.go | 6 ++++++ internal/tui/ui.go | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/internal/tui/constants/progressMsg.go b/internal/tui/constants/progressMsg.go index 864c17891..2c284a4f6 100644 --- a/internal/tui/constants/progressMsg.go +++ b/internal/tui/constants/progressMsg.go @@ -8,6 +8,12 @@ type TaskFinishedMsg struct { SectionType string Err error Msg tea.Msg + // FinishedText overrides the FinishedText set on the Task at creation time + // when non-empty. Only applied on successful completion (Err == nil). + // Use this when the task's outcome determines the displayed message + // (e.g. "merged" vs "auto-merge enabled" vs "queued") and the final text + // cannot be known until the task completes. + FinishedText string } type ClearTaskMsg struct { diff --git a/internal/tui/ui.go b/internal/tui/ui.go index a0a0f39bb..57cd55170 100644 --- a/internal/tui/ui.go +++ b/internal/tui/ui.go @@ -658,6 +658,10 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { task.Error = msg.Err } else { task.State = context.TaskFinished + // Use override finished text if provided + if msg.FinishedText != "" { + task.FinishedText = msg.FinishedText + } } now := time.Now() task.FinishedTime = &now From 09fb9287a8271e3cf6c286dfa233c47f68554c2e Mon Sep 17 00:00:00 2001 From: Rob Berwick Date: Sat, 31 Jan 2026 16:51:29 +0000 Subject: [PATCH 02/22] feat: add merge-method fields to Repository, node ID and AutoMergeRequest type to PullRequestData - Add AllowMergeCommit, AllowSquashMerge, AllowRebaseMerge to Repository so the preferred merge method can be determined without an extra query - Add ID string to PullRequestData to carry the GraphQL node ID required as pullRequestId input for merge mutations - Add AutoMergeRequest struct and AutoMergeRequest field to PullRequestData to represent auto-merge configuration and support the visual indicator --- internal/data/prapi.go | 11 +++++++++++ internal/data/repository.go | 3 +++ 2 files changed, 14 insertions(+) diff --git a/internal/data/prapi.go b/internal/data/prapi.go index da2ff88d2..808e4b4dc 100644 --- a/internal/data/prapi.go +++ b/internal/data/prapi.go @@ -17,6 +17,14 @@ import ( "github.com/dlvhdr/gh-dash/v4/internal/tui/theme" ) +type AutoMergeRequest struct { + EnabledAt time.Time `graphql:"enabledAt"` + MergeMethod string `graphql:"mergeMethod"` + EnabledBy struct { + Login string `graphql:"login"` + } `graphql:"enabledBy"` +} + type SuggestedReviewer struct { IsAuthor bool IsCommenter bool @@ -64,6 +72,7 @@ type EnrichedPullRequestData struct { } type PullRequestData struct { + ID string Number int Title string Body string @@ -96,6 +105,8 @@ type PullRequestData struct { Files ChangedFiles `graphql:"files(first: 5)"` IsDraft bool IsInMergeQueue bool + AutoMergeEnabled bool // Set locally when auto-merge is enabled via the UI; AutoMergeRequest holds the real API data when populated from a fetch. + AutoMergeRequest *AutoMergeRequest Commits Commits `graphql:"commits(last: 1)"` Labels PRLabels `graphql:"labels(first: 6)"` MergeStateStatus MergeStateStatus `graphql:"mergeStateStatus"` diff --git a/internal/data/repository.go b/internal/data/repository.go index 72c9d9a49..e6e11b111 100644 --- a/internal/data/repository.go +++ b/internal/data/repository.go @@ -18,5 +18,8 @@ type Repository struct { Name string NameWithOwner string IsArchived bool + AllowMergeCommit bool `graphql:"mergeCommitAllowed"` + AllowSquashMerge bool `graphql:"squashMergeAllowed"` + AllowRebaseMerge bool `graphql:"rebaseMergeAllowed"` BranchProtectionRules BranchProtectionRules `graphql:"branchProtectionRules(first: 1)"` } From 9fbabed6d57714af518d2ec5333c36aa8e17a0a1 Mon Sep 17 00:00:00 2001 From: Rob Berwick Date: Sat, 28 Feb 2026 14:54:36 +0000 Subject: [PATCH 03/22] feat: add AutoMergeIcon constant and visual indicator for auto-merge enabled PRs - Add AutoMergeIcon constant (timer with checkmark) - Update PR row to show auto-merge icon when AutoMergeRequest is set or AutoMergeEnabled flag is true - Update RenderState() to show "Auto-merge" text in PR details --- internal/tui/components/prrow/prrow.go | 6 ++++++ internal/tui/constants/constants.go | 1 + 2 files changed, 7 insertions(+) diff --git a/internal/tui/components/prrow/prrow.go b/internal/tui/components/prrow/prrow.go index c325f75fb..2858d5491 100644 --- a/internal/tui/components/prrow/prrow.go +++ b/internal/tui/components/prrow/prrow.go @@ -80,6 +80,9 @@ func (pr *PullRequest) renderState() string { return mergeCellStyle.Foreground(pr.Ctx.Theme.WarningText). Render(constants.MergeQueueIcon) } + if pr.Data.Primary.AutoMergeRequest != nil || pr.Data.Primary.AutoMergeEnabled { + return mergeCellStyle.Foreground(pr.Ctx.Theme.WarningText).Render(constants.AutoMergeIcon) + } if pr.Data.Primary.IsDraft { return mergeCellStyle.Foreground(pr.Ctx.Theme.FaintText).Render(constants.DraftIcon) } else { @@ -302,6 +305,9 @@ func (pr *PullRequest) RenderState() string { if pr.Data.Primary.IsInMergeQueue { return constants.MergeQueueIcon + " Queued" } + if pr.Data.Primary.AutoMergeRequest != nil || pr.Data.Primary.AutoMergeEnabled { + return constants.AutoMergeIcon + " Auto-merge" + } if pr.Data.Primary.IsDraft { return constants.DraftIcon + " Draft" } else { diff --git a/internal/tui/constants/constants.go b/internal/tui/constants/constants.go index c29c5de5e..a9c548a66 100644 --- a/internal/tui/constants/constants.go +++ b/internal/tui/constants/constants.go @@ -54,6 +54,7 @@ const ( LabelsIcon = "󰌖" MergedIcon = "" MergeQueueIcon = "" // \uf4db nf-oct-git_merge_queue + AutoMergeIcon = "󰦼" // \uF09BC nf-md-arrow_decision_auto OpenIcon = "" SelectionIcon = "❯" From e6b8d5ef7750713a7c1b2b1b9198438f575f89cf Mon Sep 17 00:00:00 2001 From: Rob Berwick Date: Sat, 28 Feb 2026 14:54:41 +0000 Subject: [PATCH 04/22] feat: add MergePullRequest GraphQL mutation and mergeMethodForRepo helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the two-step approach (subprocess + follow-up query) with a single GraphQL mutation whose response directly reveals the outcome. - Add mergeMethodForRepo() to select MERGE > SQUASH > REBASE based on what the repository allows - Add MergePullRequest() which picks the correct mutation based on mergeStateStatus: CLEAN/UNSTABLE → mergePullRequest (direct merge) BLOCKED → addPullRequestToMergeQueue default → enablePullRequestAutoMerge and returns PRMergeStatus directly from the mutation response --- internal/data/prapi.go | 136 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) diff --git a/internal/data/prapi.go b/internal/data/prapi.go index 808e4b4dc..f845e9b48 100644 --- a/internal/data/prapi.go +++ b/internal/data/prapi.go @@ -584,3 +584,139 @@ func FetchPullRequest(prUrl string) (EnrichedPullRequestData, error) { return queryResult.Resource.PullRequest, nil } + +// PRMergeStatus represents the merge/queue state of a PR +type PRMergeStatus struct { + State string // OPEN, CLOSED, MERGED + IsInMergeQueue bool + HasAutoMerge bool +} + +// mergeMethodForRepo returns the GraphQL merge method string to use based on +// which methods the repository allows, in priority order: MERGE > SQUASH > REBASE. +// +// NOTE: Future enhancement — when multiple merge methods are allowed, consider: +// - Prompting the user to select from the available options before merging +// - Splitting MergePR into separate MergePR / SquashPR / RebasePR task functions +// so the user can invoke them with explicit keybindings +func mergeMethodForRepo(repo Repository) (graphql.String, error) { + switch { + case repo.AllowMergeCommit: + return "MERGE", nil + case repo.AllowSquashMerge: + return "SQUASH", nil + case repo.AllowRebaseMerge: + return "REBASE", nil + default: + return "", fmt.Errorf("repository has no allowed merge methods") + } +} + +// MergePullRequest performs the appropriate merge action via a single GraphQL +// mutation, returning the outcome directly from the mutation response without +// a follow-up query. +// +// The mutation chosen depends on the PR's current mergeStateStatus: +// - CLEAN or UNSTABLE → mergePullRequest (direct merge) +// - BLOCKED → addPullRequestToMergeQueue (repo has a merge queue) +// - anything else → enablePullRequestAutoMerge (checks still pending) +func MergePullRequest(prNodeID string, mergeStateStatus MergeStateStatus, repo Repository) (PRMergeStatus, error) { + var err error + if cachedClient == nil { + cachedClient, err = gh.NewGraphQLClient(gh.ClientOptions{EnableCache: false}) + if err != nil { + return PRMergeStatus{}, err + } + } + + log.Debug("Performing PR merge via GraphQL", "nodeID", prNodeID, "mergeStateStatus", mergeStateStatus) + + switch mergeStateStatus { + case "CLEAN", "UNSTABLE": + mergeMethod, err := mergeMethodForRepo(repo) + if err != nil { + return PRMergeStatus{}, err + } + + var mutation struct { + MergePullRequest struct { + PullRequest struct { + State string `graphql:"state"` + } `graphql:"pullRequest"` + } `graphql:"mergePullRequest(input: $input)"` + } + type MergePullRequestInput struct { + PullRequestID string `json:"pullRequestId"` + MergeMethod graphql.String `json:"mergeMethod"` + } + variables := map[string]any{ + "input": MergePullRequestInput{ + PullRequestID: prNodeID, + MergeMethod: mergeMethod, + }, + } + if err := cachedClient.Mutate("MergePullRequest", &mutation, variables); err != nil { + return PRMergeStatus{}, err + } + log.Info("PR merged directly", "nodeID", prNodeID, "state", mutation.MergePullRequest.PullRequest.State) + return PRMergeStatus{State: mutation.MergePullRequest.PullRequest.State}, nil + + case "BLOCKED": + var mutation struct { + AddPullRequestToMergeQueue struct { + MergeQueueEntry struct { + ID string `graphql:"id"` + } `graphql:"mergeQueueEntry"` + } `graphql:"addPullRequestToMergeQueue(input: $input)"` + } + type AddPullRequestToMergeQueueInput struct { + PullRequestID string `json:"pullRequestId"` + } + variables := map[string]any{ + "input": AddPullRequestToMergeQueueInput{ + PullRequestID: prNodeID, + }, + } + if err := cachedClient.Mutate("AddPullRequestToMergeQueue", &mutation, variables); err != nil { + return PRMergeStatus{}, err + } + log.Info("PR added to merge queue", "nodeID", prNodeID) + return PRMergeStatus{State: "OPEN", IsInMergeQueue: true}, nil + + default: + mergeMethod, err := mergeMethodForRepo(repo) + if err != nil { + return PRMergeStatus{}, err + } + + var mutation struct { + EnablePullRequestAutoMerge struct { + PullRequest struct { + State string `graphql:"state"` + AutoMergeRequest *struct { + EnabledAt time.Time `graphql:"enabledAt"` + } `graphql:"autoMergeRequest"` + } `graphql:"pullRequest"` + } `graphql:"enablePullRequestAutoMerge(input: $input)"` + } + type EnablePullRequestAutoMergeInput struct { + PullRequestID string `json:"pullRequestId"` + MergeMethod graphql.String `json:"mergeMethod"` + } + variables := map[string]any{ + "input": EnablePullRequestAutoMergeInput{ + PullRequestID: prNodeID, + MergeMethod: mergeMethod, + }, + } + if err := cachedClient.Mutate("EnablePullRequestAutoMerge", &mutation, variables); err != nil { + return PRMergeStatus{}, err + } + hasAutoMerge := mutation.EnablePullRequestAutoMerge.PullRequest.AutoMergeRequest != nil + log.Info("Auto-merge enabled for PR", "nodeID", prNodeID, "hasAutoMerge", hasAutoMerge) + return PRMergeStatus{ + State: mutation.EnablePullRequestAutoMerge.PullRequest.State, + HasAutoMerge: hasAutoMerge, + }, nil + } +} From c9fda6aebe25d8d155d6b12278e2fa114f1dfba3 Mon Sep 17 00:00:00 2001 From: Rob Berwick Date: Sat, 28 Feb 2026 14:54:48 +0000 Subject: [PATCH 05/22] feat: replace gh pr merge subprocess with MergePullRequest GraphQL mutation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MergePR now calls data.MergePullRequest() directly via a goroutine instead of spawning a 'gh pr merge' subprocess followed by a follow-up status query. The mutation response is authoritative — no fallback 'assume merged' path. - Rewrite MergePR to use a plain func() tea.Msg goroutine - Add extractPullRequestData() helper and primaryPRDataProvider interface to retrieve the underlying *data.PullRequestData from the RowData value (handles both prrow.Data from the PR list and *data.PullRequestData from the branch section) - Add GetPrimaryPRData() to prrow.Data to satisfy the interface - Add IsQueued and AutoMergeEnabled fields to UpdatePRMsg --- internal/tui/components/prrow/data.go | 11 ++++ internal/tui/components/tasks/pr.go | 92 +++++++++++++++++++++------ 2 files changed, 84 insertions(+), 19 deletions(-) diff --git a/internal/tui/components/prrow/data.go b/internal/tui/components/prrow/data.go index b35e0d8d4..888003c1c 100644 --- a/internal/tui/components/prrow/data.go +++ b/internal/tui/components/prrow/data.go @@ -35,3 +35,14 @@ func (data Data) GetUpdatedAt() time.Time { func (data Data) GetCreatedAt() time.Time { return data.Primary.CreatedAt } + +// GetPrimaryPRData returns the underlying PullRequestData, satisfying the +// primaryPRDataProvider interface consumed by tasks.extractPullRequestData +// (see internal/tui/components/tasks/pr.go). The method is on a pointer +// receiver, so only *Data satisfies primaryPRDataProvider — a caller holding +// a plain Data value (non-pointer) will not match the interface case and +// extractPullRequestData will return nil. All prrow rows are stored and +// passed as *Data throughout the codebase, so this is the expected usage. +func (d *Data) GetPrimaryPRData() *data.PullRequestData { + return d.Primary +} diff --git a/internal/tui/components/tasks/pr.go b/internal/tui/components/tasks/pr.go index bb8bc127c..d0b9d6620 100644 --- a/internal/tui/components/tasks/pr.go +++ b/internal/tui/components/tasks/pr.go @@ -26,6 +26,8 @@ type UpdatePRMsg struct { NewComment *data.Comment ReadyForReview *bool IsMerged *bool + IsQueued *bool // PR was added to merge queue + AutoMergeEnabled *bool // Auto-merge was enabled (waiting for checks) AddedAssignees *data.Assignees RemovedAssignees *data.Assignees } @@ -165,14 +167,10 @@ func PRReady(ctx *context.ProgramContext, section SectionIdentifier, pr data.Row func MergePR(ctx *context.ProgramContext, section SectionIdentifier, pr data.RowData) tea.Cmd { prNumber := pr.GetNumber() - c := exec.Command( - "gh", - "pr", - "merge", - fmt.Sprint(prNumber), - "-R", - pr.GetRepoNameWithOwner(), - ) + + // Extract the underlying PullRequestData to obtain the node ID, merge state + // status, and repository merge-method settings required for the GraphQL mutation. + prData := extractPullRequestData(pr) taskId := fmt.Sprintf("merge_%d", prNumber) task := context.Task{ @@ -184,20 +182,76 @@ func MergePR(ctx *context.ProgramContext, section SectionIdentifier, pr data.Row } startCmd := ctx.StartTask(task) - return tea.Batch(startCmd, tea.ExecProcess(c, func(err error) tea.Msg { - isMerged := err == nil && c.ProcessState.ExitCode() == 0 + return tea.Batch(startCmd, func() tea.Msg { + if prData == nil { + return constants.TaskFinishedMsg{ + SectionId: section.Id, + SectionType: section.Type, + TaskId: taskId, + Err: fmt.Errorf("could not resolve PR data for PR #%d", prNumber), + Msg: UpdatePRMsg{PrNumber: prNumber}, + } + } + + status, err := data.MergePullRequest(prData.ID, prData.MergeStateStatus, prData.Repository) + if err != nil { + log.Error("Failed to merge PR via GraphQL", "pr", prNumber, "err", err) + return constants.TaskFinishedMsg{ + SectionId: section.Id, + SectionType: section.Type, + TaskId: taskId, + Err: err, + Msg: UpdatePRMsg{PrNumber: prNumber}, + } + } + + var finishedText string + updateMsg := UpdatePRMsg{PrNumber: prNumber} + + if status.State == "MERGED" { + isMerged := true + updateMsg.IsMerged = &isMerged + finishedText = fmt.Sprintf("PR #%d has been merged", prNumber) + } else if status.IsInMergeQueue { + isQueued := true + updateMsg.IsQueued = &isQueued + finishedText = fmt.Sprintf("PR #%d has been added to merge queue", prNumber) + } else if status.HasAutoMerge { + autoMergeEnabled := true + updateMsg.AutoMergeEnabled = &autoMergeEnabled + finishedText = fmt.Sprintf("Auto-merge enabled for PR #%d", prNumber) + } else { + finishedText = fmt.Sprintf("PR #%d merge action completed", prNumber) + } return constants.TaskFinishedMsg{ - SectionId: section.Id, - SectionType: section.Type, - TaskId: taskId, - Err: err, - Msg: UpdatePRMsg{ - PrNumber: prNumber, - IsMerged: &isMerged, - }, + SectionId: section.Id, + SectionType: section.Type, + TaskId: taskId, + Err: nil, + FinishedText: finishedText, + Msg: updateMsg, } - })) + }) +} + +// primaryPRDataProvider is implemented by row types that wrap a PullRequestData +// (e.g. prrow.Data in the PR list section). +type primaryPRDataProvider interface { + GetPrimaryPRData() *data.PullRequestData +} + +// extractPullRequestData retrieves the underlying *data.PullRequestData from a +// RowData value. RowData is implemented by both prrow.Data (PR list rows) and +// *data.PullRequestData (branch section rows), so we handle both cases. +func extractPullRequestData(pr data.RowData) *data.PullRequestData { + switch v := pr.(type) { + case *data.PullRequestData: + return v + case primaryPRDataProvider: + return v.GetPrimaryPRData() + } + return nil } func CreatePR( From 4577d88b13c214922c6f357f73e5eb165e44e03f Mon Sep 17 00:00:00 2001 From: Rob Berwick Date: Sat, 28 Feb 2026 14:55:05 +0000 Subject: [PATCH 06/22] feat: harden MergePullRequest with mutex, FF_MOCK_DATA support, and enableAutoMerge helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Use sync.Mutex (clientMu) for goroutine-safe client initialisation - Respect FF_MOCK_DATA feature flag to point the GraphQL client at localhost:3000 - Extract enableAutoMerge() helper encapsulating the enablePullRequestAutoMerge mutation (reused by BLOCKED, BEHIND, and UNKNOWN/default cases) - Expand mergeStateStatus switch: CLEAN, UNSTABLE, HAS_HOOKS → mergePullRequest (direct merge) DIRTY → error (merge conflicts) BLOCKED, BEHIND → enableAutoMerge (enable auto-merge) UNKNOWN → log warning, attempt enableAutoMerge default → log warning, attempt enableAutoMerge - Use package-level client (not cachedClient) for all mutations --- internal/data/prapi.go | 133 +++++++++++++++++++++++------------------ 1 file changed, 76 insertions(+), 57 deletions(-) diff --git a/internal/data/prapi.go b/internal/data/prapi.go index f845e9b48..c95961d66 100644 --- a/internal/data/prapi.go +++ b/internal/data/prapi.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "net/url" + "sync" "time" "charm.land/log/v2" @@ -476,6 +477,7 @@ type PullRequestsResponse struct { } var ( + clientMu sync.Mutex client *gh.GraphQLClient cachedClient *gh.GraphQLClient ) @@ -588,7 +590,7 @@ func FetchPullRequest(prUrl string) (EnrichedPullRequestData, error) { // PRMergeStatus represents the merge/queue state of a PR type PRMergeStatus struct { State string // OPEN, CLOSED, MERGED - IsInMergeQueue bool + IsInMergeQueue bool // TODO: not yet populated; reserved for future merge-queue mutation support HasAutoMerge bool } @@ -612,27 +614,76 @@ func mergeMethodForRepo(repo Repository) (graphql.String, error) { } } +// enableAutoMerge calls the enablePullRequestAutoMerge GraphQL mutation and +// returns the resulting PRMergeStatus. +func enableAutoMerge(prNodeID string, mergeMethod graphql.String) (PRMergeStatus, error) { + var mutation struct { + EnablePullRequestAutoMerge struct { + PullRequest struct { + State string `graphql:"state"` + AutoMergeRequest *struct { + EnabledAt time.Time `graphql:"enabledAt"` + } `graphql:"autoMergeRequest"` + } `graphql:"pullRequest"` + } `graphql:"enablePullRequestAutoMerge(input: $input)"` + } + type EnablePullRequestAutoMergeInput struct { + PullRequestID string `json:"pullRequestId"` + MergeMethod graphql.String `json:"mergeMethod"` + } + variables := map[string]any{ + "input": EnablePullRequestAutoMergeInput{ + PullRequestID: prNodeID, + MergeMethod: mergeMethod, + }, + } + if err := client.Mutate("EnablePullRequestAutoMerge", &mutation, variables); err != nil { + return PRMergeStatus{}, err + } + hasAutoMerge := mutation.EnablePullRequestAutoMerge.PullRequest.AutoMergeRequest != nil + log.Info("Auto-merge enabled for PR", "nodeID", prNodeID, "hasAutoMerge", hasAutoMerge) + return PRMergeStatus{ + State: mutation.EnablePullRequestAutoMerge.PullRequest.State, + HasAutoMerge: hasAutoMerge, + }, nil +} + // MergePullRequest performs the appropriate merge action via a single GraphQL // mutation, returning the outcome directly from the mutation response without // a follow-up query. // -// The mutation chosen depends on the PR's current mergeStateStatus: -// - CLEAN or UNSTABLE → mergePullRequest (direct merge) -// - BLOCKED → addPullRequestToMergeQueue (repo has a merge queue) -// - anything else → enablePullRequestAutoMerge (checks still pending) +// The mutation chosen depends on the PR's current mergeStateStatus +// https://docs.github.com/en/graphql/reference/enums#mergestatestatus +// - CLEAN, UNSTABLE, or HAS_HOOKS → mergePullRequest +// - BLOCKED → enablePullRequestAutoMerge +// - BEHIND → enablePullRequestAutoMerge +// - DIRTY → error +// - UNKNOWN → log warning, attempt enablePullRequestAutoMerge +// +// Note: draft PRs are not handled here; callers should check IsDraft before +// invoking this function. func MergePullRequest(prNodeID string, mergeStateStatus MergeStateStatus, repo Repository) (PRMergeStatus, error) { - var err error - if cachedClient == nil { - cachedClient, err = gh.NewGraphQLClient(gh.ClientOptions{EnableCache: false}) + clientMu.Lock() + if client == nil { + var err error + if config.IsFeatureEnabled(config.FF_MOCK_DATA) { + log.Info("using mock data", "server", "https://localhost:3000") + http.DefaultTransport.(*http.Transport).TLSClientConfig = &tls.Config{InsecureSkipVerify: true} + client, err = gh.NewGraphQLClient(gh.ClientOptions{Host: "localhost:3000", AuthToken: "fake-token"}) + } else { + client, err = gh.DefaultGraphQLClient() + } if err != nil { + clientMu.Unlock() return PRMergeStatus{}, err } } + clientMu.Unlock() log.Debug("Performing PR merge via GraphQL", "nodeID", prNodeID, "mergeStateStatus", mergeStateStatus) switch mergeStateStatus { - case "CLEAN", "UNSTABLE": + case "CLEAN", "UNSTABLE", "HAS_HOOKS": mergeMethod, err := mergeMethodForRepo(repo) if err != nil { return PRMergeStatus{}, err @@ -655,68 +706,36 @@ func MergePullRequest(prNodeID string, mergeStateStatus MergeStateStatus, repo R MergeMethod: mergeMethod, }, } - if err := cachedClient.Mutate("MergePullRequest", &mutation, variables); err != nil { + if err := client.Mutate("MergePullRequest", &mutation, variables); err != nil { return PRMergeStatus{}, err } log.Info("PR merged directly", "nodeID", prNodeID, "state", mutation.MergePullRequest.PullRequest.State) return PRMergeStatus{State: mutation.MergePullRequest.PullRequest.State}, nil - case "BLOCKED": - var mutation struct { - AddPullRequestToMergeQueue struct { - MergeQueueEntry struct { - ID string `graphql:"id"` - } `graphql:"mergeQueueEntry"` - } `graphql:"addPullRequestToMergeQueue(input: $input)"` - } - type AddPullRequestToMergeQueueInput struct { - PullRequestID string `json:"pullRequestId"` - } - variables := map[string]any{ - "input": AddPullRequestToMergeQueueInput{ - PullRequestID: prNodeID, - }, - } - if err := cachedClient.Mutate("AddPullRequestToMergeQueue", &mutation, variables); err != nil { + case "DIRTY": + return PRMergeStatus{}, fmt.Errorf("PR has merge conflicts, please resolve locally") + + case "BLOCKED", "BEHIND": + mergeMethod, err := mergeMethodForRepo(repo) + if err != nil { return PRMergeStatus{}, err } - log.Info("PR added to merge queue", "nodeID", prNodeID) - return PRMergeStatus{State: "OPEN", IsInMergeQueue: true}, nil + return enableAutoMerge(prNodeID, mergeMethod) - default: + case "UNKNOWN": + log.Warn("Unknown merge state status, attempting auto-merge", "status", mergeStateStatus) mergeMethod, err := mergeMethodForRepo(repo) if err != nil { return PRMergeStatus{}, err } + return enableAutoMerge(prNodeID, mergeMethod) - var mutation struct { - EnablePullRequestAutoMerge struct { - PullRequest struct { - State string `graphql:"state"` - AutoMergeRequest *struct { - EnabledAt time.Time `graphql:"enabledAt"` - } `graphql:"autoMergeRequest"` - } `graphql:"pullRequest"` - } `graphql:"enablePullRequestAutoMerge(input: $input)"` - } - type EnablePullRequestAutoMergeInput struct { - PullRequestID string `json:"pullRequestId"` - MergeMethod graphql.String `json:"mergeMethod"` - } - variables := map[string]any{ - "input": EnablePullRequestAutoMergeInput{ - PullRequestID: prNodeID, - MergeMethod: mergeMethod, - }, - } - if err := cachedClient.Mutate("EnablePullRequestAutoMerge", &mutation, variables); err != nil { + default: + log.Warn("Unrecognised merge state status, attempting auto-merge", "status", mergeStateStatus) + mergeMethod, err := mergeMethodForRepo(repo) + if err != nil { return PRMergeStatus{}, err } - hasAutoMerge := mutation.EnablePullRequestAutoMerge.PullRequest.AutoMergeRequest != nil - log.Info("Auto-merge enabled for PR", "nodeID", prNodeID, "hasAutoMerge", hasAutoMerge) - return PRMergeStatus{ - State: mutation.EnablePullRequestAutoMerge.PullRequest.State, - HasAutoMerge: hasAutoMerge, - }, nil + return enableAutoMerge(prNodeID, mergeMethod) } } From 3213d3cf5819dead042b376dbb01c70f54ec5b6f Mon Sep 17 00:00:00 2001 From: Rob Berwick Date: Sat, 28 Feb 2026 14:55:10 +0000 Subject: [PATCH 07/22] feat: handle AutoMergeEnabled in prssection and UpdatePRMsg in reposection - prssection: on UpdatePRMsg with AutoMergeEnabled=true, set a non-nil AutoMergeRequest sentinel so the auto-merge icon renders immediately without waiting for a full data refresh - reposection: add tasks.UpdatePRMsg handler to Update() so merge results are reflected in the branch section - reposection/commands.go: update UpdatePRMsg doc comment --- internal/tui/components/prssection/prssection.go | 5 +++++ internal/tui/components/reposection/commands.go | 4 ++++ .../tui/components/reposection/reposection.go | 16 ++++++++++++++++ 3 files changed, 25 insertions(+) diff --git a/internal/tui/components/prssection/prssection.go b/internal/tui/components/prssection/prssection.go index e834a45c4..828fb2504 100644 --- a/internal/tui/components/prssection/prssection.go +++ b/internal/tui/components/prssection/prssection.go @@ -187,6 +187,11 @@ func (m *Model) Update(msg tea.Msg) (section.Section, tea.Cmd) { currPr.Primary.State = "MERGED" currPr.Primary.Mergeable = "" } + if msg.AutoMergeEnabled != nil && *msg.AutoMergeEnabled { + // Set a local flag so the auto-merge icon renders immediately + // without waiting for a full refresh. + currPr.Primary.AutoMergeEnabled = true + } m.Prs[i] = currPr m.SetIsLoading(false) m.Table.SetRows(m.BuildRows()) diff --git a/internal/tui/components/reposection/commands.go b/internal/tui/components/reposection/commands.go index 308b2a5b4..efee9da5f 100644 --- a/internal/tui/components/reposection/commands.go +++ b/internal/tui/components/reposection/commands.go @@ -16,6 +16,10 @@ import ( "github.com/dlvhdr/gh-dash/v4/internal/tui/context" ) +// UpdatePRMsg carries PR state changes emitted by repo-section-specific +// commands (e.g. inline comment, assignee edits). Tasks that are shared +// between the PR-list section and the repo section (such as MergePR, ClosePR) +// emit tasks.UpdatePRMsg instead, which is handled directly in Update(). type UpdatePRMsg struct { PrNumber int IsClosed *bool diff --git a/internal/tui/components/reposection/reposection.go b/internal/tui/components/reposection/reposection.go index e91de1924..4ee6ab01b 100644 --- a/internal/tui/components/reposection/reposection.go +++ b/internal/tui/components/reposection/reposection.go @@ -169,6 +169,22 @@ func (m *Model) Update(msg tea.Msg) (section.Section, tea.Cmd) { m.Prs = append(m.Prs, *msg.NewPr) } + case tasks.UpdatePRMsg: + for i, pr := range m.Prs { + if pr.Number != msg.PrNumber { + continue + } + if msg.IsMerged != nil && *msg.IsMerged { + m.Prs[i].State = "MERGED" + m.Prs[i].Mergeable = "" + } + if msg.AutoMergeEnabled != nil && *msg.AutoMergeEnabled { + m.Prs[i].AutoMergeEnabled = true + } + m.Table.SetRows(m.BuildRows()) + break + } + case repoMsg: m.repo = msg.repo m.SetIsLoading(false) From 156cf591e79666ed189042233d60c5d051165d1b Mon Sep 17 00:00:00 2001 From: Rob Berwick Date: Sat, 28 Feb 2026 14:55:17 +0000 Subject: [PATCH 08/22] fix: add IsDraft guard to MergePR and remove unused IsQueued field - Reject draft PRs before the network call with a clear error message - Remove IsQueued field from UpdatePRMsg (merge-queue support is not yet implemented; reserved for a future commit) --- internal/tui/components/tasks/pr.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/internal/tui/components/tasks/pr.go b/internal/tui/components/tasks/pr.go index d0b9d6620..6a9b9f32b 100644 --- a/internal/tui/components/tasks/pr.go +++ b/internal/tui/components/tasks/pr.go @@ -26,7 +26,6 @@ type UpdatePRMsg struct { NewComment *data.Comment ReadyForReview *bool IsMerged *bool - IsQueued *bool // PR was added to merge queue AutoMergeEnabled *bool // Auto-merge was enabled (waiting for checks) AddedAssignees *data.Assignees RemovedAssignees *data.Assignees @@ -193,6 +192,16 @@ func MergePR(ctx *context.ProgramContext, section SectionIdentifier, pr data.Row } } + if prData.IsDraft { + return constants.TaskFinishedMsg{ + SectionId: section.Id, + SectionType: section.Type, + TaskId: taskId, + Err: fmt.Errorf("cannot merge a draft PR, please publish it first"), + Msg: UpdatePRMsg{PrNumber: prNumber}, + } + } + status, err := data.MergePullRequest(prData.ID, prData.MergeStateStatus, prData.Repository) if err != nil { log.Error("Failed to merge PR via GraphQL", "pr", prNumber, "err", err) @@ -212,10 +221,6 @@ func MergePR(ctx *context.ProgramContext, section SectionIdentifier, pr data.Row isMerged := true updateMsg.IsMerged = &isMerged finishedText = fmt.Sprintf("PR #%d has been merged", prNumber) - } else if status.IsInMergeQueue { - isQueued := true - updateMsg.IsQueued = &isQueued - finishedText = fmt.Sprintf("PR #%d has been added to merge queue", prNumber) } else if status.HasAutoMerge { autoMergeEnabled := true updateMsg.AutoMergeEnabled = &autoMergeEnabled From bc7c533036bb8645591a72d3ad737840eaeae11f Mon Sep 17 00:00:00 2001 From: Rob Berwick Date: Sat, 28 Feb 2026 14:56:09 +0000 Subject: [PATCH 09/22] test: add shared GraphQL mock client test helper package - Add internal/testhelpers/graphql package with LocalRoundTripper and NewMockGraphQLClient for use across test packages - Refactor internal/data/graphql_test_helpers_test.go to delegate to the new shared helpers, removing the inline localRoundTripper duplicate --- internal/data/graphql_test_helpers_test.go | 18 ++++++++++ internal/testhelpers/graphql/helpers.go | 39 ++++++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 internal/data/graphql_test_helpers_test.go create mode 100644 internal/testhelpers/graphql/helpers.go diff --git a/internal/data/graphql_test_helpers_test.go b/internal/data/graphql_test_helpers_test.go new file mode 100644 index 000000000..c4f28eec0 --- /dev/null +++ b/internal/data/graphql_test_helpers_test.go @@ -0,0 +1,18 @@ +package data + +import ( + "net/http" + "testing" + + gh "github.com/cli/go-gh/v2/pkg/api" + graphqltest "github.com/dlvhdr/gh-dash/v4/internal/testhelpers/graphql" +) + +// newMockGraphQLClient creates a GraphQL client backed by the given handler. +// It is a package-local wrapper around graphqltest.NewMockGraphQLClient so +// that internal/data tests can call it without importing the test-helper +// package name explicitly. +func newMockGraphQLClient(t *testing.T, handler http.Handler) *gh.GraphQLClient { + t.Helper() + return graphqltest.NewMockGraphQLClient(t, handler) +} diff --git a/internal/testhelpers/graphql/helpers.go b/internal/testhelpers/graphql/helpers.go new file mode 100644 index 000000000..4d621afae --- /dev/null +++ b/internal/testhelpers/graphql/helpers.go @@ -0,0 +1,39 @@ +// Package graphqltest provides shared test helpers for creating mock GraphQL +// clients backed by in-process HTTP handlers. +package graphqltest + +import ( + "net/http" + "net/http/httptest" + "testing" + + gh "github.com/cli/go-gh/v2/pkg/api" + "github.com/stretchr/testify/require" +) + +// LocalRoundTripper is an http.RoundTripper that routes requests directly to +// an in-process handler, avoiding a real network connection. +type LocalRoundTripper struct { + Handler http.Handler +} + +// RoundTrip implements http.RoundTripper. +func (l LocalRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + w := httptest.NewRecorder() + l.Handler.ServeHTTP(w, req) + return w.Result(), nil +} + +// NewMockGraphQLClient creates a *gh.GraphQLClient backed by the given handler. +// It is intended for use in tests that need to intercept GraphQL mutations +// without a real network connection. +func NewMockGraphQLClient(t *testing.T, handler http.Handler) *gh.GraphQLClient { + t.Helper() + c, err := gh.NewGraphQLClient(gh.ClientOptions{ + Transport: LocalRoundTripper{Handler: handler}, + Host: "localhost:3000", + AuthToken: "fake-token", + }) + require.NoError(t, err) + return c +} From e82449ba432b9378421d50229a599f090afccb3c Mon Sep 17 00:00:00 2001 From: Rob Berwick Date: Sat, 28 Feb 2026 14:56:15 +0000 Subject: [PATCH 10/22] test: add unit tests for mergeMethodForRepo and MergePullRequest - TestMergeMethodForRepo: priority order (MERGE > SQUASH > REBASE) and error for no allowed methods - TestMergePullRequest_*: direct merge (CLEAN/UNSTABLE/HAS_HOOKS), auto-merge (BLOCKED/BEHIND), DIRTY conflict error, and GraphQL error propagation - TestFF_MOCK_DATA_*: FF_MOCK_DATA feature flag routes to localhost:3000 --- internal/data/prapi_test.go | 243 ++++++++++++++++++++++++++++++++++++ 1 file changed, 243 insertions(+) diff --git a/internal/data/prapi_test.go b/internal/data/prapi_test.go index 79af55c19..1fbb1b245 100644 --- a/internal/data/prapi_test.go +++ b/internal/data/prapi_test.go @@ -1,9 +1,13 @@ package data import ( + "io" + "net/http" + "strings" "testing" gh "github.com/cli/go-gh/v2/pkg/api" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -78,3 +82,242 @@ func TestSetClient(t *testing.T) { require.True(t, IsEnrichmentCacheCleared()) }) } + +// setMockMergeClient installs a GraphQL client backed by the given handler and +// resets the package-level clients afterwards. +func setMockMergeClient(t *testing.T, handler http.Handler) { + t.Helper() + SetClient(newMockGraphQLClient(t, handler)) + t.Cleanup(func() { + client = nil + cachedClient = nil + }) +} + +// mergePullRequestResponse returns a JSON body for a successful mergePullRequest mutation. +func mergePullRequestResponse(state string) string { + return `{"data":{"mergePullRequest":{"pullRequest":{"state":"` + state + `"}}}}` +} + +// enableAutoMergeResponse returns a JSON body for a successful enablePullRequestAutoMerge mutation. +func enableAutoMergeResponse(hasAutoMerge bool) string { + autoMergeValue := "null" + if hasAutoMerge { + autoMergeValue = `{"enabledAt":"2024-01-01T00:00:00Z"}` + } + return `{"data":{"enablePullRequestAutoMerge":{"pullRequest":{"state":"OPEN","autoMergeRequest":` + autoMergeValue + `}}}}` +} + +// graphqlErrorResponse returns a JSON body representing a GraphQL-level error. +func graphqlErrorResponse(message string) string { + return `{"errors":[{"message":"` + message + `","locations":[],"path":[]}]}` +} + +// mergeMockHandler returns an http.Handler that serves controlled responses for +// each of the merge-related GraphQL mutations. Each response body is +// keyed by the operation name embedded in the request body by the GraphQL client. +func mergeMockHandler(t *testing.T, responses map[string]string) http.Handler { + t.Helper() + return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + body, err := io.ReadAll(req.Body) + require.NoError(t, err) + bodyStr := string(body) + + w.Header().Set("Content-Type", "application/json") + switch { + case strings.Contains(bodyStr, "mutation MergePullRequest"): + resp, ok := responses["MergePullRequest"] + if !ok { + t.Error("unexpected call to MergePullRequest mutation") + w.WriteHeader(http.StatusInternalServerError) + return + } + _, _ = io.WriteString(w, resp) + case strings.Contains(bodyStr, "mutation EnablePullRequestAutoMerge"): + resp, ok := responses["EnablePullRequestAutoMerge"] + if !ok { + t.Error("unexpected call to EnablePullRequestAutoMerge mutation") + w.WriteHeader(http.StatusInternalServerError) + return + } + _, _ = io.WriteString(w, resp) + default: + t.Errorf("unexpected GraphQL request body: %s", bodyStr) + w.WriteHeader(http.StatusInternalServerError) + } + }) +} + +// --- mergeMethodForRepo tests --- + +func TestMergeMethodForRepo(t *testing.T) { + tests := []struct { + name string + repo Repository + expectedMethod string + expectError bool + }{ + { + name: "only merge commits allowed", + repo: Repository{AllowMergeCommit: true}, + expectedMethod: "MERGE", + }, + { + name: "only squash merge allowed", + repo: Repository{AllowSquashMerge: true}, + expectedMethod: "SQUASH", + }, + { + name: "only rebase merge allowed", + repo: Repository{AllowRebaseMerge: true}, + expectedMethod: "REBASE", + }, + { + name: "all three allowed - MERGE wins", + repo: Repository{AllowMergeCommit: true, AllowSquashMerge: true, AllowRebaseMerge: true}, + expectedMethod: "MERGE", + }, + { + name: "merge and squash allowed - MERGE wins", + repo: Repository{AllowMergeCommit: true, AllowSquashMerge: true}, + expectedMethod: "MERGE", + }, + { + name: "squash and rebase allowed - SQUASH wins", + repo: Repository{AllowSquashMerge: true, AllowRebaseMerge: true}, + expectedMethod: "SQUASH", + }, + { + name: "no methods allowed returns error", + repo: Repository{}, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + method, err := mergeMethodForRepo(tt.repo) + if tt.expectError { + require.Error(t, err) + assert.Empty(t, method) + } else { + require.NoError(t, err) + assert.Equal(t, tt.expectedMethod, string(method)) + } + }) + } +} + +// --- MergePullRequest tests --- + +func TestMergePullRequest_DirectMerge_Clean(t *testing.T) { + setMockMergeClient(t, mergeMockHandler(t, map[string]string{ + "MergePullRequest": mergePullRequestResponse("MERGED"), + })) + + repo := Repository{AllowMergeCommit: true} + status, err := MergePullRequest("PR_node123", "CLEAN", repo) + + require.NoError(t, err) + assert.Equal(t, "MERGED", status.State) + assert.False(t, status.IsInMergeQueue) + assert.False(t, status.HasAutoMerge) +} + +func TestMergePullRequest_DirectMerge_Unstable(t *testing.T) { + setMockMergeClient(t, mergeMockHandler(t, map[string]string{ + "MergePullRequest": mergePullRequestResponse("MERGED"), + })) + + repo := Repository{AllowSquashMerge: true} + status, err := MergePullRequest("PR_node456", "UNSTABLE", repo) + + require.NoError(t, err) + assert.Equal(t, "MERGED", status.State) + assert.False(t, status.IsInMergeQueue) + assert.False(t, status.HasAutoMerge) +} + +func TestMergePullRequest_Blocked_EnablesAutoMerge(t *testing.T) { + setMockMergeClient(t, mergeMockHandler(t, map[string]string{ + "EnablePullRequestAutoMerge": enableAutoMergeResponse(true), + })) + + repo := Repository{AllowMergeCommit: true} + status, err := MergePullRequest("PR_node789", "BLOCKED", repo) + + require.NoError(t, err) + assert.Equal(t, "OPEN", status.State) + assert.True(t, status.HasAutoMerge) + assert.False(t, status.IsInMergeQueue) +} + +func TestMergePullRequest_AutoMergeEnabled(t *testing.T) { + setMockMergeClient(t, mergeMockHandler(t, map[string]string{ + "EnablePullRequestAutoMerge": enableAutoMergeResponse(true), + })) + + repo := Repository{AllowMergeCommit: true} + // Any status other than CLEAN, UNSTABLE, or BLOCKED falls through to auto-merge. + status, err := MergePullRequest("PR_node999", "BEHIND", repo) + + require.NoError(t, err) + assert.True(t, status.HasAutoMerge) + assert.False(t, status.IsInMergeQueue) +} + +// TestMergePullRequest_MockDataFlag_UsesLocalServer verifies that when the +// FF_MOCK_DATA environment variable is set, MergePullRequest initialises the +// GraphQL client targeting localhost:3000 rather than calling +// gh.DefaultGraphQLClient(). Because the production code hardcodes the host +// and also replaces http.DefaultTransport's TLS config, we cannot intercept +// the connection via a test server. Instead we confirm the behaviour +// indirectly: with FF_MOCK_DATA set and client == nil, MergePullRequest must +// return an error that references localhost:3000 (connection refused), proving +// it attempted the mock-data host rather than the real GitHub API (which would +// produce a credentials error, not a dial error to localhost). +func TestMergePullRequest_MockDataFlag_UsesLocalServer(t *testing.T) { + // Ensure client starts nil so MergePullRequest performs lazy init. + originalClient := client + client = nil + t.Cleanup(func() { client = originalClient }) + + // Activate the feature flag. No server is listening on localhost:3000, + // so the call will fail — but with a dial error to localhost:3000, which + // is what we want to assert. + t.Setenv("FF_MOCK_DATA", "1") + + repo := Repository{AllowMergeCommit: true} + _, err := MergePullRequest("PR_mockflag", "CLEAN", repo) + + require.Error(t, err, "MergePullRequest should fail when no mock server is running") + assert.Contains(t, err.Error(), "localhost:3000", + "error should reference localhost:3000, confirming the mock-data init path was taken") +} + +func TestMergePullRequest_DirectMerge_HasHooks(t *testing.T) { + setMockMergeClient(t, mergeMockHandler(t, map[string]string{ + "MergePullRequest": mergePullRequestResponse("MERGED"), + })) + + repo := Repository{AllowMergeCommit: true} + status, err := MergePullRequest("PR_hooks", "HAS_HOOKS", repo) + + require.NoError(t, err) + assert.Equal(t, "MERGED", status.State) + assert.False(t, status.IsInMergeQueue) + assert.False(t, status.HasAutoMerge) +} + +func TestMergePullRequest_Behind_EnablesAutoMerge(t *testing.T) { + setMockMergeClient(t, mergeMockHandler(t, map[string]string{ + "EnablePullRequestAutoMerge": enableAutoMergeResponse(true), + })) + + repo := Repository{AllowMergeCommit: true} + status, err := MergePullRequest("PR_behind", "BEHIND", repo) + + require.NoError(t, err) + assert.True(t, status.HasAutoMerge) + assert.False(t, status.IsInMergeQueue) +} From fe92d518b671eebdacf3da051adb4e486cc6b916 Mon Sep 17 00:00:00 2001 From: Rob Berwick Date: Sat, 28 Feb 2026 14:56:21 +0000 Subject: [PATCH 11/22] test: add integration tests for MergePR task, extractPullRequestData, and AutoMergeEnabled - TestExtractPullRequestData_*: direct *PullRequestData, prrow.Data wrapper, nil primary provider, nil input, and unknown RowData types - TestMergePR_*: full MergePR task integration for direct merge, auto-merge (BLOCKED), auto-merge (BEHIND), draft PR guard, nil PR data error, and GraphQL error propagation - TestUpdatePRMsg_AutoMergeEnabled_SetsFlag: AutoMergeEnabled in UpdatePRMsg sets AutoMergeRequest sentinel in prssection so the icon renders immediately --- .../components/prssection/prssection_test.go | 28 +++ internal/tui/components/tasks/pr_test.go | 210 ++++++++++++++++++ 2 files changed, 238 insertions(+) diff --git a/internal/tui/components/prssection/prssection_test.go b/internal/tui/components/prssection/prssection_test.go index 0f6974371..80163679f 100644 --- a/internal/tui/components/prssection/prssection_test.go +++ b/internal/tui/components/prssection/prssection_test.go @@ -2,14 +2,17 @@ package prssection import ( "testing" + "time" tea "charm.land/bubbletea/v2" "github.com/stretchr/testify/require" + "github.com/dlvhdr/gh-dash/v4/internal/config" "github.com/dlvhdr/gh-dash/v4/internal/data" "github.com/dlvhdr/gh-dash/v4/internal/tui/components/prompt" "github.com/dlvhdr/gh-dash/v4/internal/tui/components/prrow" "github.com/dlvhdr/gh-dash/v4/internal/tui/components/section" + "github.com/dlvhdr/gh-dash/v4/internal/tui/components/tasks" "github.com/dlvhdr/gh-dash/v4/internal/tui/context" ) @@ -105,6 +108,31 @@ func TestConfirmation_CancelWithCtrlC(t *testing.T) { _ = cmd } +func TestUpdatePRMsg_AutoMergeEnabled_SetsFlag(t *testing.T) { + // Construct a minimal model with one PR whose AutoMergeEnabled starts false. + ctx := &context.ProgramContext{ + Config: &config.Config{Theme: &config.ThemeConfig{}}, + StartTask: func(task context.Task) tea.Cmd { + return func() tea.Msg { return nil } + }, + } + m := NewModel(0, ctx, config.PrsSectionConfig{}, time.Time{}, time.Time{}) + m.Prs = []prrow.Data{ + {Primary: &data.PullRequestData{Number: 42}}, + } + + autoMerge := true + m.Update(tasks.UpdatePRMsg{ + PrNumber: 42, + AutoMergeEnabled: &autoMerge, + }) + + require.True(t, m.Prs[0].Primary.AutoMergeEnabled, + "AutoMergeEnabled should be set to true after receiving AutoMergeEnabled update") + require.Nil(t, m.Prs[0].Primary.AutoMergeRequest, + "AutoMergeRequest should remain nil (only real API data should populate it)") +} + func TestConfirmation_AllActions(t *testing.T) { actions := []string{"close", "reopen", "ready", "merge", "update", "approveWorkflows"} diff --git a/internal/tui/components/tasks/pr_test.go b/internal/tui/components/tasks/pr_test.go index 322262d6d..186386eef 100644 --- a/internal/tui/components/tasks/pr_test.go +++ b/internal/tui/components/tasks/pr_test.go @@ -2,11 +2,19 @@ package tasks import ( "fmt" + "io" + "net/http" + "strings" "testing" + "time" tea "charm.land/bubbletea/v2" "github.com/stretchr/testify/require" + "github.com/dlvhdr/gh-dash/v4/internal/data" + graphqltest "github.com/dlvhdr/gh-dash/v4/internal/testhelpers/graphql" + "github.com/dlvhdr/gh-dash/v4/internal/tui/components/prrow" + "github.com/dlvhdr/gh-dash/v4/internal/tui/constants" "github.com/dlvhdr/gh-dash/v4/internal/tui/context" ) @@ -131,3 +139,205 @@ func TestApproveWorkflows_SectionIdentifierPropagation(t *testing.T) { }) } } + +// --- extractPullRequestData tests --- + +// unknownRowData is a RowData implementation that is neither *data.PullRequestData +// nor a primaryPRDataProvider — used to exercise the nil fallback branch. +type unknownRowData struct{} + +func (u unknownRowData) GetRepoNameWithOwner() string { return "owner/repo" } +func (u unknownRowData) GetTitle() string { return "title" } +func (u unknownRowData) GetNumber() int { return 1 } +func (u unknownRowData) GetUrl() string { return "https://github.com/owner/repo/pull/1" } +func (u unknownRowData) GetUpdatedAt() time.Time { return time.Time{} } + +func TestExtractPullRequestData_DirectPRData(t *testing.T) { + prData := &data.PullRequestData{ID: "PR_xyz"} + result := extractPullRequestData(prData) + assert.Same(t, prData, result, "should return the same *data.PullRequestData pointer") +} + +func TestExtractPullRequestData_PrrowData(t *testing.T) { + prData := &data.PullRequestData{ID: "PR_abc"} + row := &prrow.Data{Primary: prData} + result := extractPullRequestData(row) + assert.Same(t, prData, result, "should return the Primary field from prrow.Data") +} + +func TestExtractPullRequestData_PrrowData_NilPrimary(t *testing.T) { + row := &prrow.Data{Primary: nil} + result := extractPullRequestData(row) + assert.Nil(t, result) +} + +func TestExtractPullRequestData_NilInput(t *testing.T) { + // A bare nil becomes a nil interface value; the type switch has no + // matching case and falls through to return nil. + result := extractPullRequestData(nil) + assert.Nil(t, result) +} + +func TestExtractPullRequestData_UnknownRowDataType(t *testing.T) { + result := extractPullRequestData(unknownRowData{}) + assert.Nil(t, result, "unknown RowData type with no GetPrimaryPRData() method should return nil") +} + +// --- MergePR tests --- + +// setMockClient installs a mock GraphQL client backed by handler and resets it after the test. +func setMockClient(t *testing.T, handler http.Handler) { + t.Helper() + data.SetClient(graphqltest.NewMockGraphQLClient(t, handler)) + t.Cleanup(func() { data.SetClient(nil) }) +} + +// mergeMockHandler serves controlled responses keyed by GraphQL operation name. +func mergeMockHandler(t *testing.T, responses map[string]string) http.Handler { + t.Helper() + return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + body, err := io.ReadAll(req.Body) + require.NoError(t, err) + bodyStr := string(body) + + w.Header().Set("Content-Type", "application/json") + switch { + case strings.Contains(bodyStr, "mutation MergePullRequest"): + _, _ = io.WriteString(w, responses["MergePullRequest"]) + case strings.Contains(bodyStr, "mutation EnablePullRequestAutoMerge"): + _, _ = io.WriteString(w, responses["EnablePullRequestAutoMerge"]) + default: + t.Errorf("unexpected GraphQL request body: %s", bodyStr) + w.WriteHeader(http.StatusInternalServerError) + } + }) +} + +// noopCtx returns a minimal ProgramContext whose StartTask is a no-op. +// tea.Batch(nil, workFn) returns workFn directly, so calling the result of +// MergePR() will invoke the work function synchronously in tests. +func noopCtx() *context.ProgramContext { + return &context.ProgramContext{ + StartTask: func(task context.Task) tea.Cmd { return nil }, + } +} + +// prDataFor builds a *data.PullRequestData with the fields MergePR needs. +func prDataFor(id string, status data.MergeStateStatus, repo data.Repository) *data.PullRequestData { + return &data.PullRequestData{ + ID: id, + Number: 42, + MergeStateStatus: status, + Repository: repo, + } +} + +// runMergePR calls MergePR and immediately invokes the returned tea.Cmd to +// obtain the TaskFinishedMsg synchronously (no bubbletea runtime required). +func runMergePR(t *testing.T, pr data.RowData) constants.TaskFinishedMsg { + t.Helper() + ctx := noopCtx() + section := SectionIdentifier{Id: 1, Type: "prs"} + cmd := MergePR(ctx, section, pr) + require.NotNil(t, cmd, "MergePR should return a non-nil tea.Cmd") + msg := cmd() + finished, ok := msg.(constants.TaskFinishedMsg) + require.True(t, ok, "expected TaskFinishedMsg, got %T", msg) + return finished +} + +func TestMergePR_DirectMerge(t *testing.T) { + setMockClient(t, mergeMockHandler(t, map[string]string{ + "MergePullRequest": `{"data":{"mergePullRequest":{"pullRequest":{"state":"MERGED"}}}}`, + })) + + pr := prDataFor("PR_clean", "CLEAN", data.Repository{AllowMergeCommit: true}) + msg := runMergePR(t, pr) + + require.NoError(t, msg.Err) + assert.Equal(t, "PR #42 has been merged", msg.FinishedText) + update, ok := msg.Msg.(UpdatePRMsg) + require.True(t, ok) + require.NotNil(t, update.IsMerged) + assert.True(t, *update.IsMerged) + assert.Nil(t, update.AutoMergeEnabled) +} + +func TestMergePR_Blocked_EnablesAutoMerge(t *testing.T) { + setMockClient(t, mergeMockHandler(t, map[string]string{ + "EnablePullRequestAutoMerge": `{"data":{"enablePullRequestAutoMerge":{"pullRequest":{"state":"OPEN","autoMergeRequest":{"enabledAt":"2024-01-01T00:00:00Z"}}}}}`, + })) + + pr := prDataFor("PR_blocked", "BLOCKED", data.Repository{AllowMergeCommit: true}) + msg := runMergePR(t, pr) + + require.NoError(t, msg.Err) + assert.Equal(t, "Auto-merge enabled for PR #42", msg.FinishedText) + update, ok := msg.Msg.(UpdatePRMsg) + require.True(t, ok) + require.NotNil(t, update.AutoMergeEnabled) + assert.True(t, *update.AutoMergeEnabled) + assert.Nil(t, update.IsMerged) +} + +func TestMergePR_AutoMergeEnabled(t *testing.T) { + setMockClient(t, mergeMockHandler(t, map[string]string{ + "EnablePullRequestAutoMerge": `{"data":{"enablePullRequestAutoMerge":{"pullRequest":{"state":"OPEN","autoMergeRequest":{"enabledAt":"2024-01-01T00:00:00Z"}}}}}`, + })) + + pr := prDataFor("PR_behind", "BEHIND", data.Repository{AllowMergeCommit: true}) + msg := runMergePR(t, pr) + + require.NoError(t, msg.Err) + assert.Equal(t, "Auto-merge enabled for PR #42", msg.FinishedText) + update, ok := msg.Msg.(UpdatePRMsg) + require.True(t, ok) + require.NotNil(t, update.AutoMergeEnabled) + assert.True(t, *update.AutoMergeEnabled) + assert.Nil(t, update.IsMerged) +} + +func TestMergePR_NilPRData_ReturnsError(t *testing.T) { + // unknownRowData has no *data.PullRequestData and no GetPrimaryPRData(), + // so extractPullRequestData returns nil, triggering the nil-guard error path. + msg := runMergePR(t, unknownRowData{}) + + require.Error(t, msg.Err) + assert.Contains(t, msg.Err.Error(), "could not resolve PR data") +} + +func TestMergePR_DraftPR_ReturnsError(t *testing.T) { + // No mock client needed: the IsDraft guard fires before any network call. + pr := &data.PullRequestData{ + ID: "PR_draft", + Number: 42, + IsDraft: true, + MergeStateStatus: "CLEAN", + Repository: data.Repository{AllowMergeCommit: true}, + } + msg := runMergePR(t, pr) + + require.Error(t, msg.Err) + assert.Contains(t, msg.Err.Error(), "draft") +} + +func TestMergePR_GraphQLError_ReturnsError(t *testing.T) { + setMockClient(t, mergeMockHandler(t, map[string]string{ + "MergePullRequest": `{"errors":[{"message":"Pull request is not mergeable","locations":[],"path":[]}]}`, + })) + + pr := prDataFor("PR_clean2", "CLEAN", data.Repository{AllowMergeCommit: true}) + msg := runMergePR(t, pr) + + require.Error(t, msg.Err) + assert.Contains(t, msg.Err.Error(), "Pull request is not mergeable") +} + +func TestMergePR_RepoWithNoAllowedMethods_ReturnsError(t *testing.T) { + // Empty repository - no allowed merge methods + pr := prDataFor("PR_nomethods", "CLEAN", data.Repository{}) + msg := runMergePR(t, pr) + + require.Error(t, msg.Err) + assert.Contains(t, msg.Err.Error(), "no allowed merge methods") +} From b77eed21a18faec69443d6378b075938deeb881a Mon Sep 17 00:00:00 2001 From: Rob Berwick Date: Sat, 7 Mar 2026 14:31:10 +0000 Subject: [PATCH 12/22] refactor: move AutoMergeEnabled from API model to UI component wrapper The AutoMergeEnabled flag is a local UI state set when users enable auto-merge via the UI, not API-fetched data. Move it from PullRequestData to the prrow Data struct to properly separate concerns. --- internal/data/prapi.go | 1 - internal/tui/components/prrow/data.go | 7 ++++--- internal/tui/components/prrow/prrow.go | 4 ++-- internal/tui/components/prssection/prssection.go | 2 +- internal/tui/components/prssection/prssection_test.go | 2 +- internal/tui/components/reposection/reposection.go | 3 --- 6 files changed, 8 insertions(+), 11 deletions(-) diff --git a/internal/data/prapi.go b/internal/data/prapi.go index c95961d66..015cd403b 100644 --- a/internal/data/prapi.go +++ b/internal/data/prapi.go @@ -106,7 +106,6 @@ type PullRequestData struct { Files ChangedFiles `graphql:"files(first: 5)"` IsDraft bool IsInMergeQueue bool - AutoMergeEnabled bool // Set locally when auto-merge is enabled via the UI; AutoMergeRequest holds the real API data when populated from a fetch. AutoMergeRequest *AutoMergeRequest Commits Commits `graphql:"commits(last: 1)"` Labels PRLabels `graphql:"labels(first: 6)"` diff --git a/internal/tui/components/prrow/data.go b/internal/tui/components/prrow/data.go index 888003c1c..92e8b7ac0 100644 --- a/internal/tui/components/prrow/data.go +++ b/internal/tui/components/prrow/data.go @@ -7,9 +7,10 @@ import ( ) type Data struct { - Primary *data.PullRequestData - Enriched data.EnrichedPullRequestData - IsEnriched bool + Primary *data.PullRequestData + Enriched data.EnrichedPullRequestData + IsEnriched bool + AutoMergeEnabled bool // Set locally when auto-merge is enabled via the UI; not queried from the API. } func (data Data) GetTitle() string { diff --git a/internal/tui/components/prrow/prrow.go b/internal/tui/components/prrow/prrow.go index 2858d5491..1ee43c3ca 100644 --- a/internal/tui/components/prrow/prrow.go +++ b/internal/tui/components/prrow/prrow.go @@ -80,7 +80,7 @@ func (pr *PullRequest) renderState() string { return mergeCellStyle.Foreground(pr.Ctx.Theme.WarningText). Render(constants.MergeQueueIcon) } - if pr.Data.Primary.AutoMergeRequest != nil || pr.Data.Primary.AutoMergeEnabled { + if pr.Data.Primary.AutoMergeRequest != nil || pr.Data.AutoMergeEnabled { return mergeCellStyle.Foreground(pr.Ctx.Theme.WarningText).Render(constants.AutoMergeIcon) } if pr.Data.Primary.IsDraft { @@ -305,7 +305,7 @@ func (pr *PullRequest) RenderState() string { if pr.Data.Primary.IsInMergeQueue { return constants.MergeQueueIcon + " Queued" } - if pr.Data.Primary.AutoMergeRequest != nil || pr.Data.Primary.AutoMergeEnabled { + if pr.Data.Primary.AutoMergeRequest != nil || pr.Data.AutoMergeEnabled { return constants.AutoMergeIcon + " Auto-merge" } if pr.Data.Primary.IsDraft { diff --git a/internal/tui/components/prssection/prssection.go b/internal/tui/components/prssection/prssection.go index 828fb2504..c0e762e17 100644 --- a/internal/tui/components/prssection/prssection.go +++ b/internal/tui/components/prssection/prssection.go @@ -190,7 +190,7 @@ func (m *Model) Update(msg tea.Msg) (section.Section, tea.Cmd) { if msg.AutoMergeEnabled != nil && *msg.AutoMergeEnabled { // Set a local flag so the auto-merge icon renders immediately // without waiting for a full refresh. - currPr.Primary.AutoMergeEnabled = true + currPr.AutoMergeEnabled = true } m.Prs[i] = currPr m.SetIsLoading(false) diff --git a/internal/tui/components/prssection/prssection_test.go b/internal/tui/components/prssection/prssection_test.go index 80163679f..ae00fb539 100644 --- a/internal/tui/components/prssection/prssection_test.go +++ b/internal/tui/components/prssection/prssection_test.go @@ -127,7 +127,7 @@ func TestUpdatePRMsg_AutoMergeEnabled_SetsFlag(t *testing.T) { AutoMergeEnabled: &autoMerge, }) - require.True(t, m.Prs[0].Primary.AutoMergeEnabled, + require.True(t, m.Prs[0].AutoMergeEnabled, "AutoMergeEnabled should be set to true after receiving AutoMergeEnabled update") require.Nil(t, m.Prs[0].Primary.AutoMergeRequest, "AutoMergeRequest should remain nil (only real API data should populate it)") diff --git a/internal/tui/components/reposection/reposection.go b/internal/tui/components/reposection/reposection.go index 4ee6ab01b..a963abcd9 100644 --- a/internal/tui/components/reposection/reposection.go +++ b/internal/tui/components/reposection/reposection.go @@ -178,9 +178,6 @@ func (m *Model) Update(msg tea.Msg) (section.Section, tea.Cmd) { m.Prs[i].State = "MERGED" m.Prs[i].Mergeable = "" } - if msg.AutoMergeEnabled != nil && *msg.AutoMergeEnabled { - m.Prs[i].AutoMergeEnabled = true - } m.Table.SetRows(m.BuildRows()) break } From 3d1d4648e41d6653cfbec383030382ff57ab55e8 Mon Sep 17 00:00:00 2001 From: Rob Berwick Date: Sat, 14 Mar 2026 10:18:39 +0000 Subject: [PATCH 13/22] fix: remove IsInMergeQueue from PRMergeStatus and drop tautological assertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PRMergeStatus.IsInMergeQueue was always false — the enableAutoMerge mutation does not return queue membership, and merge-queue mutation support is not yet implemented. Keeping the field would mislead callers into thinking the status is tracked. Remove it now and re-add alongside the actual merge-queue mutation when that work lands. The six assert.False(t, status.IsInMergeQueue) calls in prapi_test.go were tautological (asserting a zero value that could never be anything else), so they are removed along with the field. --- internal/data/prapi.go | 7 +++---- internal/data/prapi_test.go | 6 ------ 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/internal/data/prapi.go b/internal/data/prapi.go index 015cd403b..34c525171 100644 --- a/internal/data/prapi.go +++ b/internal/data/prapi.go @@ -586,11 +586,10 @@ func FetchPullRequest(prUrl string) (EnrichedPullRequestData, error) { return queryResult.Resource.PullRequest, nil } -// PRMergeStatus represents the merge/queue state of a PR +// PRMergeStatus represents the outcome of a merge action. type PRMergeStatus struct { - State string // OPEN, CLOSED, MERGED - IsInMergeQueue bool // TODO: not yet populated; reserved for future merge-queue mutation support - HasAutoMerge bool + State string // OPEN, CLOSED, MERGED + HasAutoMerge bool } // mergeMethodForRepo returns the GraphQL merge method string to use based on diff --git a/internal/data/prapi_test.go b/internal/data/prapi_test.go index 1fbb1b245..1b8d1f95e 100644 --- a/internal/data/prapi_test.go +++ b/internal/data/prapi_test.go @@ -220,7 +220,6 @@ func TestMergePullRequest_DirectMerge_Clean(t *testing.T) { require.NoError(t, err) assert.Equal(t, "MERGED", status.State) - assert.False(t, status.IsInMergeQueue) assert.False(t, status.HasAutoMerge) } @@ -234,7 +233,6 @@ func TestMergePullRequest_DirectMerge_Unstable(t *testing.T) { require.NoError(t, err) assert.Equal(t, "MERGED", status.State) - assert.False(t, status.IsInMergeQueue) assert.False(t, status.HasAutoMerge) } @@ -249,7 +247,6 @@ func TestMergePullRequest_Blocked_EnablesAutoMerge(t *testing.T) { require.NoError(t, err) assert.Equal(t, "OPEN", status.State) assert.True(t, status.HasAutoMerge) - assert.False(t, status.IsInMergeQueue) } func TestMergePullRequest_AutoMergeEnabled(t *testing.T) { @@ -263,7 +260,6 @@ func TestMergePullRequest_AutoMergeEnabled(t *testing.T) { require.NoError(t, err) assert.True(t, status.HasAutoMerge) - assert.False(t, status.IsInMergeQueue) } // TestMergePullRequest_MockDataFlag_UsesLocalServer verifies that when the @@ -305,7 +301,6 @@ func TestMergePullRequest_DirectMerge_HasHooks(t *testing.T) { require.NoError(t, err) assert.Equal(t, "MERGED", status.State) - assert.False(t, status.IsInMergeQueue) assert.False(t, status.HasAutoMerge) } @@ -319,5 +314,4 @@ func TestMergePullRequest_Behind_EnablesAutoMerge(t *testing.T) { require.NoError(t, err) assert.True(t, status.HasAutoMerge) - assert.False(t, status.IsInMergeQueue) } From a24d7949750aa6f1f7f269e89ab17b720a321c67 Mon Sep 17 00:00:00 2001 From: Rob Berwick Date: Sat, 14 Mar 2026 10:19:26 +0000 Subject: [PATCH 14/22] fix: protect FetchPullRequests and FetchPullRequest lazy-init with clientMu Both functions used a client == nil check-then-set pattern without holding clientMu, creating a data race with MergePullRequest (which already used the mutex) when called concurrently from goroutines. Wrap the lazy-init block in each function with clientMu.Lock/Unlock, matching the existing pattern in MergePullRequest. Add TestClientInit_ConcurrentCallsAreSafe to verify the fix under -race. --- internal/data/prapi.go | 5 +++++ internal/data/prapi_test.go | 42 +++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/internal/data/prapi.go b/internal/data/prapi.go index 34c525171..570fd37f6 100644 --- a/internal/data/prapi.go +++ b/internal/data/prapi.go @@ -500,6 +500,7 @@ func IsEnrichmentCacheCleared() bool { func FetchPullRequests(query string, limit int, pageInfo *PageInfo) (PullRequestsResponse, error) { var err error + clientMu.Lock() if client == nil { if config.IsFeatureEnabled(config.FF_MOCK_DATA) { log.Info("using mock data", "server", "https://localhost:3000") @@ -513,6 +514,7 @@ func FetchPullRequests(query string, limit int, pageInfo *PageInfo) (PullRequest client, err = gh.DefaultGraphQLClient() } } + clientMu.Unlock() if err != nil { return PullRequestsResponse{}, err @@ -557,12 +559,15 @@ func FetchPullRequests(query string, limit int, pageInfo *PageInfo) (PullRequest func FetchPullRequest(prUrl string) (EnrichedPullRequestData, error) { var err error + clientMu.Lock() if client == nil { client, err = gh.DefaultGraphQLClient() if err != nil { + clientMu.Unlock() return EnrichedPullRequestData{}, err } } + clientMu.Unlock() var queryResult struct { Resource struct { diff --git a/internal/data/prapi_test.go b/internal/data/prapi_test.go index 1b8d1f95e..1abf2fcc4 100644 --- a/internal/data/prapi_test.go +++ b/internal/data/prapi_test.go @@ -315,3 +315,45 @@ func TestMergePullRequest_Behind_EnablesAutoMerge(t *testing.T) { require.NoError(t, err) assert.True(t, status.HasAutoMerge) } + +// TestClientInit_ConcurrentCallsAreSafe verifies that concurrent calls to +// FetchPullRequests, FetchPullRequest, and MergePullRequest do not race on the +// package-level client variable. With -race this test will fail unless every +// lazy-init path is protected by clientMu. +// +// The test intentionally leaves client == nil so each goroutine tries to +// initialise it. With FF_MOCK_DATA unset the real gh.DefaultGraphQLClient() +// is called, which will return an error (no credentials in CI) — that is fine; +// we only care that the race detector does not fire. +func TestClientInit_ConcurrentCallsAreSafe(t *testing.T) { + // Reset to nil so every goroutine races to initialise. + originalClient := client + client = nil + t.Cleanup(func() { client = originalClient }) + + const goroutines = 8 + done := make(chan struct{}) + go func() { + defer close(done) + errs := make(chan error, goroutines*3) + for i := range goroutines { + go func(i int) { + _, err := FetchPullRequests("repo:test/test", 1, nil) + errs <- err + }(i) + go func(i int) { + _, err := FetchPullRequest("https://github.com/test/test/pull/1") + errs <- err + }(i) + go func(i int) { + _, err := MergePullRequest("PR_race", "CLEAN", + Repository{AllowMergeCommit: true}) + errs <- err + }(i) + } + for range goroutines * 3 { + <-errs + } + }() + <-done +} From fe8aebd35fdac1621d7a935d8f2ddb9be6b3a346 Mon Sep 17 00:00:00 2001 From: Rob Berwick Date: Sat, 14 Mar 2026 10:19:52 +0000 Subject: [PATCH 15/22] feat: propagate AutoMergeEnabled flag to PullRequestData and reposection Add AutoMergeEnabled bool to data.PullRequestData as a local UI flag (not API-fetched) so the branch section can show the auto-merge icon immediately after the user triggers a merge, without waiting for a full refresh. Handle the field in reposection.Update's UpdatePRMsg case, mirroring the existing handler in prssection. Add reposection_test.go with three tests covering AutoMergeEnabled, IsMerged, and unknown-PR no-op cases. --- internal/data/prapi.go | 5 + .../tui/components/reposection/reposection.go | 5 + .../reposection/reposection_test.go | 99 +++++++++++++++++++ 3 files changed, 109 insertions(+) create mode 100644 internal/tui/components/reposection/reposection_test.go diff --git a/internal/data/prapi.go b/internal/data/prapi.go index 570fd37f6..09efea6d6 100644 --- a/internal/data/prapi.go +++ b/internal/data/prapi.go @@ -107,6 +107,11 @@ type PullRequestData struct { IsDraft bool IsInMergeQueue bool AutoMergeRequest *AutoMergeRequest + // AutoMergeEnabled is a local UI flag set when the user enables auto-merge + // via the TUI. It is NOT fetched from the API; it mirrors the same field + // on prrow.Data so that the branch section can show the auto-merge icon + // immediately without waiting for a full refresh. + AutoMergeEnabled bool Commits Commits `graphql:"commits(last: 1)"` Labels PRLabels `graphql:"labels(first: 6)"` MergeStateStatus MergeStateStatus `graphql:"mergeStateStatus"` diff --git a/internal/tui/components/reposection/reposection.go b/internal/tui/components/reposection/reposection.go index a963abcd9..b78d6d116 100644 --- a/internal/tui/components/reposection/reposection.go +++ b/internal/tui/components/reposection/reposection.go @@ -178,6 +178,11 @@ func (m *Model) Update(msg tea.Msg) (section.Section, tea.Cmd) { m.Prs[i].State = "MERGED" m.Prs[i].Mergeable = "" } + if msg.AutoMergeEnabled != nil && *msg.AutoMergeEnabled { + // Set a local flag so the auto-merge icon renders immediately + // without waiting for a full refresh. + m.Prs[i].AutoMergeEnabled = true + } m.Table.SetRows(m.BuildRows()) break } diff --git a/internal/tui/components/reposection/reposection_test.go b/internal/tui/components/reposection/reposection_test.go new file mode 100644 index 000000000..b04dbe5b4 --- /dev/null +++ b/internal/tui/components/reposection/reposection_test.go @@ -0,0 +1,99 @@ +package reposection + +import ( + "testing" + + tea "charm.land/bubbletea/v2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/dlvhdr/gh-dash/v4/internal/data" + "github.com/dlvhdr/gh-dash/v4/internal/git" + "github.com/dlvhdr/gh-dash/v4/internal/tui/components/section" + "github.com/dlvhdr/gh-dash/v4/internal/tui/components/tasks" + "github.com/dlvhdr/gh-dash/v4/internal/tui/context" +) + +// newTestRepoModel creates a minimal Model with a single PR in m.Prs so that +// the UpdatePRMsg handler can find and update it. +func newTestRepoModel(prNumber int) Model { + ctx := &context.ProgramContext{ + StartTask: func(task context.Task) tea.Cmd { + return func() tea.Msg { return nil } + }, + } + m := Model{ + BaseModel: section.BaseModel{ + Ctx: ctx, + }, + // repo must be non-nil; updateBranchesWithPrs ranges over repo.Branches. + repo: &git.Repo{Branches: []git.Branch{}}, + Prs: []data.PullRequestData{ + {Number: prNumber}, + }, + } + return m +} + +// TestUpdatePRMsg_AutoMergeEnabled_SetsFlag verifies that when an UpdatePRMsg +// with AutoMergeEnabled=true is processed, the matching PR's AutoMergeEnabled +// flag is set to true on data.PullRequestData. +func TestUpdatePRMsg_AutoMergeEnabled_SetsFlag(t *testing.T) { + m := newTestRepoModel(42) + + require.False(t, m.Prs[0].AutoMergeEnabled, "AutoMergeEnabled should start false") + + autoMerge := true + msg := tasks.UpdatePRMsg{ + PrNumber: 42, + AutoMergeEnabled: &autoMerge, + } + + result, _ := m.Update(msg) + updated := result.(*Model) + + assert.True(t, updated.Prs[0].AutoMergeEnabled, + "AutoMergeEnabled should be set to true after processing UpdatePRMsg") + assert.Nil(t, updated.Prs[0].AutoMergeRequest, + "AutoMergeRequest should remain nil (only real API data should populate it)") +} + +// TestUpdatePRMsg_IsMerged_SetsState verifies that when an UpdatePRMsg with +// IsMerged=true is processed, the PR state is set to "MERGED". +func TestUpdatePRMsg_IsMerged_SetsState(t *testing.T) { + m := newTestRepoModel(7) + m.Prs[0].State = "OPEN" + + isMerged := true + msg := tasks.UpdatePRMsg{ + PrNumber: 7, + IsMerged: &isMerged, + } + + result, _ := m.Update(msg) + updated := result.(*Model) + + assert.Equal(t, "MERGED", updated.Prs[0].State) + assert.Empty(t, updated.Prs[0].Mergeable) +} + +// TestUpdatePRMsg_UnknownPR_NoChange verifies that a message for an unknown +// PR number does not mutate any existing PR. +func TestUpdatePRMsg_UnknownPR_NoChange(t *testing.T) { + m := newTestRepoModel(1) + m.Prs[0].State = "OPEN" + + autoMerge := true + msg := tasks.UpdatePRMsg{ + PrNumber: 999, + AutoMergeEnabled: &autoMerge, + } + + result, _ := m.Update(msg) + updated := result.(*Model) + + assert.False(t, updated.Prs[0].AutoMergeEnabled, + "unrelated PR should not have AutoMergeEnabled set") + assert.Equal(t, "OPEN", updated.Prs[0].State, + "unrelated PR state should be unchanged") +} From 7a8054d34f0f70036b5b9338f65e08ff74178860 Mon Sep 17 00:00:00 2001 From: Rob Berwick Date: Sat, 14 Mar 2026 10:20:06 +0000 Subject: [PATCH 16/22] feat: show auto-merge icon in branch section and guard nil PR in RenderState Add auto-merge icon rendering to branch.renderState() and branch.RenderState(): when a PR is open and either AutoMergeRequest is non-nil (set by the API) or AutoMergeEnabled is true (set locally after the user triggers auto-merge), the branch cell shows the AutoMergeIcon in warning colour, and RenderState returns ' Auto-merge'. Also add a nil guard to RenderState so it returns an empty string when b.PR is nil, preventing a panic for branches without an associated PR. Add branch_test.go with 6 tests covering all RenderState paths including both auto-merge trigger sources. --- internal/tui/components/branch/branch.go | 9 +++ internal/tui/components/branch/branch_test.go | 78 +++++++++++++++++++ 2 files changed, 87 insertions(+) create mode 100644 internal/tui/components/branch/branch_test.go diff --git a/internal/tui/components/branch/branch.go b/internal/tui/components/branch/branch.go index 77cf1282d..19b353605 100644 --- a/internal/tui/components/branch/branch.go +++ b/internal/tui/components/branch/branch.go @@ -57,6 +57,9 @@ func (b *Branch) renderState() string { switch b.PR.State { case "OPEN": + if b.PR.AutoMergeRequest != nil || b.PR.AutoMergeEnabled { + return mergeCellStyle.Foreground(b.Ctx.Theme.WarningText).Render(constants.AutoMergeIcon) + } if b.PR.IsDraft { return mergeCellStyle.Foreground(b.Ctx.Theme.FaintText).Render(constants.DraftIcon) } else { @@ -207,8 +210,14 @@ func (b *Branch) renderBaseName() string { } func (b *Branch) RenderState() string { + if b.PR == nil { + return "" + } switch b.PR.State { case "OPEN": + if b.PR.AutoMergeRequest != nil || b.PR.AutoMergeEnabled { + return constants.AutoMergeIcon + " Auto-merge" + } if b.PR.IsDraft { return constants.DraftIcon + " Draft" } else { diff --git a/internal/tui/components/branch/branch_test.go b/internal/tui/components/branch/branch_test.go new file mode 100644 index 000000000..2dfdd75a0 --- /dev/null +++ b/internal/tui/components/branch/branch_test.go @@ -0,0 +1,78 @@ +package branch + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/dlvhdr/gh-dash/v4/internal/data" + "github.com/dlvhdr/gh-dash/v4/internal/tui/constants" +) + +// newTestBranch creates a minimal Branch with a PR having the given state and +// auto-merge configuration. +func newTestBranch(prState string, autoMergeRequest *data.AutoMergeRequest, autoMergeEnabled bool) Branch { + return Branch{ + PR: &data.PullRequestData{ + State: prState, + AutoMergeRequest: autoMergeRequest, + AutoMergeEnabled: autoMergeEnabled, + }, + } +} + +// TestRenderState_AutoMerge_ViaAutoMergeRequest verifies that RenderState +// returns the auto-merge icon when AutoMergeRequest is non-nil. +func TestRenderState_AutoMerge_ViaAutoMergeRequest(t *testing.T) { + b := newTestBranch("OPEN", &data.AutoMergeRequest{}, false) + state := b.RenderState() + assert.True(t, + strings.Contains(state, constants.AutoMergeIcon), + "RenderState should include AutoMergeIcon when AutoMergeRequest is set; got %q", state) +} + +// TestRenderState_AutoMerge_ViaLocalFlag verifies that RenderState returns the +// auto-merge icon when the local AutoMergeEnabled flag is true. +func TestRenderState_AutoMerge_ViaLocalFlag(t *testing.T) { + b := newTestBranch("OPEN", nil, true) + state := b.RenderState() + assert.True(t, + strings.Contains(state, constants.AutoMergeIcon), + "RenderState should include AutoMergeIcon when AutoMergeEnabled flag is true; got %q", state) +} + +// TestRenderState_Open_NoDraft verifies the normal open PR state. +func TestRenderState_Open_NoDraft(t *testing.T) { + b := newTestBranch("OPEN", nil, false) + state := b.RenderState() + assert.Contains(t, state, constants.OpenIcon) + assert.NotContains(t, state, constants.AutoMergeIcon) +} + +// TestRenderState_Draft verifies the draft PR state. +func TestRenderState_Draft(t *testing.T) { + b := Branch{ + PR: &data.PullRequestData{ + State: "OPEN", + IsDraft: true, + }, + } + state := b.RenderState() + assert.Contains(t, state, constants.DraftIcon) + assert.Contains(t, state, "Draft") +} + +// TestRenderState_Merged verifies the merged PR state. +func TestRenderState_Merged(t *testing.T) { + b := newTestBranch("MERGED", nil, false) + state := b.RenderState() + assert.Contains(t, state, constants.MergedIcon) +} + +// TestRenderState_Closed verifies the closed PR state. +func TestRenderState_Closed(t *testing.T) { + b := newTestBranch("CLOSED", nil, false) + state := b.RenderState() + assert.Contains(t, state, constants.ClosedIcon) +} From 4904d346ec275ff9acdb8b60fc4384eb13eb88f3 Mon Sep 17 00:00:00 2001 From: Rob Berwick Date: Sat, 14 Mar 2026 10:20:18 +0000 Subject: [PATCH 17/22] test: refactor TestUpdatePRMsg_AutoMergeEnabled_SetsFlag to use m.Update() end-to-end Replace the direct field-mutation approach with a call to m.Update() so the test exercises the full update path including BuildRows(). Introduce newFullTestModel() which uses the real NewModel() constructor to wire up Table, SearchBar, and PromptConfirmationBox with a fully-populated ProgramContext (Config, Theme, Styles), preventing panics during BuildRows(). Fix the type assertion from result.(Model) to result.(*Model): Update() returns section.Section wrapping a *Model (pointer receiver), so value-type assertion is a compile error. --- .../components/prssection/prssection_test.go | 66 ++++++++++++++----- 1 file changed, 50 insertions(+), 16 deletions(-) diff --git a/internal/tui/components/prssection/prssection_test.go b/internal/tui/components/prssection/prssection_test.go index ae00fb539..5ef52faae 100644 --- a/internal/tui/components/prssection/prssection_test.go +++ b/internal/tui/components/prssection/prssection_test.go @@ -14,6 +14,7 @@ import ( "github.com/dlvhdr/gh-dash/v4/internal/tui/components/section" "github.com/dlvhdr/gh-dash/v4/internal/tui/components/tasks" "github.com/dlvhdr/gh-dash/v4/internal/tui/context" + "github.com/dlvhdr/gh-dash/v4/internal/tui/theme" ) // newTestModel creates a minimal Model with the prompt confirmation box @@ -39,6 +40,39 @@ func newTestModel(action string) Model { return m } +// newFullTestModel creates a Model via the real NewModel constructor (so that +// all internal sub-models — Table, SearchBar, etc. — are properly wired) and +// then injects a single PR row with the given PR number. +// +// ctx.Config, ctx.Theme, and ctx.Styles are all populated so that +// m.Update() can invoke BuildRows() end-to-end without panicking. +func newFullTestModel(prNumber int) Model { + thm := *theme.DefaultTheme + s := context.InitStyles(thm) + cfg := &config.Config{ + // A non-nil ThemeConfig is required because table.NewModel and + // prrow.ToTableRow both dereference Config.Theme.Ui.Table fields. + Theme: &config.ThemeConfig{}, + } + + ctx := &context.ProgramContext{ + Config: cfg, + Theme: thm, + Styles: s, + StartTask: func(task context.Task) tea.Cmd { + return func() tea.Msg { return nil } + }, + } + + // Use the real constructor so the embedded Table, SearchBar, and + // PromptConfirmationBox are fully initialised with the right ctx. + m := NewModel(0, ctx, config.PrsSectionConfig{}, time.Time{}, time.Time{}) + m.Prs = []prrow.Data{ + {Primary: &data.PullRequestData{Number: prNumber}}, + } + return m +} + func TestConfirmation_AcceptWithEmptyInput(t *testing.T) { // Pressing Enter without typing anything should confirm, since the // prompt says (Y/n) indicating Y is the default. @@ -109,27 +143,27 @@ func TestConfirmation_CancelWithCtrlC(t *testing.T) { } func TestUpdatePRMsg_AutoMergeEnabled_SetsFlag(t *testing.T) { - // Construct a minimal model with one PR whose AutoMergeEnabled starts false. - ctx := &context.ProgramContext{ - Config: &config.Config{Theme: &config.ThemeConfig{}}, - StartTask: func(task context.Task) tea.Cmd { - return func() tea.Msg { return nil } - }, - } - m := NewModel(0, ctx, config.PrsSectionConfig{}, time.Time{}, time.Time{}) - m.Prs = []prrow.Data{ - {Primary: &data.PullRequestData{Number: 42}}, - } + // Test that when UpdatePRMsg with AutoMergeEnabled=true is processed via + // m.Update(), the AutoMergeEnabled flag on prrow.Data is set to true. + // + // This test uses newFullTestModel() which wires up ctx.Config, ctx.Theme, + // and ctx.Styles so that Update() can call BuildRows() end-to-end. + m := newFullTestModel(42) + + require.False(t, m.Prs[0].AutoMergeEnabled, "AutoMergeEnabled should start false") autoMerge := true - m.Update(tasks.UpdatePRMsg{ + msg := tasks.UpdatePRMsg{ PrNumber: 42, AutoMergeEnabled: &autoMerge, - }) + } + + result, _ := m.Update(msg) + updated := result.(*Model) - require.True(t, m.Prs[0].AutoMergeEnabled, - "AutoMergeEnabled should be set to true after receiving AutoMergeEnabled update") - require.Nil(t, m.Prs[0].Primary.AutoMergeRequest, + require.True(t, updated.Prs[0].AutoMergeEnabled, + "AutoMergeEnabled should be set to true after processing AutoMergeEnabled update") + require.Nil(t, updated.Prs[0].Primary.AutoMergeRequest, "AutoMergeRequest should remain nil (only real API data should populate it)") } From e20fa6f4a36854bb8382759ff693663e85b8a8e5 Mon Sep 17 00:00:00 2001 From: Rob Berwick Date: Sat, 14 Mar 2026 10:20:29 +0000 Subject: [PATCH 18/22] docs: clarify UNKNOWN and default fallback behaviour in MergePullRequest Both cases use enableAutoMerge as an optimistic fallback rather than hard-failing. Add comments explaining the rationale: UNKNOWN means GitHub cannot determine merge state yet (checks pending, transient error), so auto-merge will fire once the condition clears. The default branch catches any future MergeStateStatus values added by GitHub. In both cases the log warning is intentionally log-only; surfacing it as a user-visible notification is deferred. --- internal/data/prapi.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/internal/data/prapi.go b/internal/data/prapi.go index 09efea6d6..07fdc7055 100644 --- a/internal/data/prapi.go +++ b/internal/data/prapi.go @@ -731,6 +731,13 @@ func MergePullRequest(prNodeID string, mergeStateStatus MergeStateStatus, repo R return enableAutoMerge(prNodeID, mergeMethod) case "UNKNOWN": + // UNKNOWN is returned by GitHub when the merge state cannot be + // determined (e.g. checks are still pending or the state is + // temporarily unavailable). We optimistically attempt to enable + // auto-merge so that the PR merges automatically once the + // blocking condition clears. The warning is intentionally + // log-only; surfacing it to the user is left as a future + // enhancement (see: https://github.com/dlvhdr/gh-dash/discussions/546). log.Warn("Unknown merge state status, attempting auto-merge", "status", mergeStateStatus) mergeMethod, err := mergeMethodForRepo(repo) if err != nil { @@ -739,6 +746,12 @@ func MergePullRequest(prNodeID string, mergeStateStatus MergeStateStatus, repo R return enableAutoMerge(prNodeID, mergeMethod) default: + // Any future MergeStateStatus values not listed above fall here. + // We apply the same optimistic enableAutoMerge strategy rather + // than hard-failing, so that newly-introduced GitHub statuses + // don't silently break the merge workflow. As with UNKNOWN, + // the warning is log-only for now; a user-visible notification + // is a future enhancement. log.Warn("Unrecognised merge state status, attempting auto-merge", "status", mergeStateStatus) mergeMethod, err := mergeMethodForRepo(repo) if err != nil { From 5e77928c3d92867275f61490233376d255ea80ee Mon Sep 17 00:00:00 2001 From: Rob Berwick Date: Sat, 14 Mar 2026 10:20:32 +0000 Subject: [PATCH 19/22] test: add missing assert import to tasks/pr_test.go --- internal/tui/components/tasks/pr_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/tui/components/tasks/pr_test.go b/internal/tui/components/tasks/pr_test.go index 186386eef..0dbf46600 100644 --- a/internal/tui/components/tasks/pr_test.go +++ b/internal/tui/components/tasks/pr_test.go @@ -9,6 +9,7 @@ import ( "time" tea "charm.land/bubbletea/v2" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/dlvhdr/gh-dash/v4/internal/data" From 72522d2000d319f235adad546ffb2d63fafbc165 Mon Sep 17 00:00:00 2001 From: Rob Berwick Date: Sat, 14 Mar 2026 10:32:41 +0000 Subject: [PATCH 20/22] fix: exclude AutoMergeEnabled from GraphQL query with graphql:"-" tag shurcooL-graphql reflects all exported fields of PullRequestData into the API query. AutoMergeEnabled is a local UI flag, not a GitHub API field, so without a tag it generated an 'autoMergeEnabled' selector that GitHub rejected with 'field doesn't exist on type PullRequest'. Add `graphql:"-"` to opt the field out of query generation. Expand the comment to explain why the field lives on the data-layer struct (branch.Branch holds *data.PullRequestData directly, with no UI-wrapper to carry UI flags) and flag it as a candidate for a future refactor toward a dedicated wrapper type analogous to prrow.Data. --- internal/data/prapi.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/internal/data/prapi.go b/internal/data/prapi.go index 07fdc7055..f79532d36 100644 --- a/internal/data/prapi.go +++ b/internal/data/prapi.go @@ -111,7 +111,14 @@ type PullRequestData struct { // via the TUI. It is NOT fetched from the API; it mirrors the same field // on prrow.Data so that the branch section can show the auto-merge icon // immediately without waiting for a full refresh. - AutoMergeEnabled bool + // + // Note: this field lives on the data-layer struct for pragmatic reasons — + // branch.Branch holds *data.PullRequestData directly, so there is no + // UI-wrapper type here to carry the flag. The graphql:"-" tag prevents + // shurcooL-graphql from including it in the GitHub API query. + // A cleaner home would be a dedicated UI-wrapper type for PullRequestData + // (analogous to prrow.Data), which is left as a future refactor. + AutoMergeEnabled bool `graphql:"-"` Commits Commits `graphql:"commits(last: 1)"` Labels PRLabels `graphql:"labels(first: 6)"` MergeStateStatus MergeStateStatus `graphql:"mergeStateStatus"` From 297cdd702e4d65b6c4c78b8f26da4c7d060dcf1c Mon Sep 17 00:00:00 2001 From: Rob Berwick Date: Sat, 14 Mar 2026 10:44:52 +0000 Subject: [PATCH 21/22] fix: move AutoMergeEnabled off data.PullRequestData onto branch.Branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit shurcooL-graphql has no skip-field mechanism — the graphql:"-" tag caused GitHub to receive the literal string "-" as a field selector, rejecting the query at runtime. Remove AutoMergeEnabled from data.PullRequestData entirely. Instead: - Add AutoMergeEnabled bool to branch.Branch (UI-layer struct, never used as a GraphQL deserialization target). - Add autoMergeEnabledPRs map[int]bool to reposection.Model so the flag survives the scratch-rebuild inside updateBranchesWithPrs(). - UpdatePRMsg handler writes to the map; updateBranchesWithPrs re-applies it to each Branch after building from Prs. - Update branch.renderState/RenderState to read b.AutoMergeEnabled. - Update branch_test.go and reposection_test.go accordingly. --- internal/data/prapi.go | 12 ---- internal/tui/components/branch/branch.go | 10 ++- internal/tui/components/branch/branch_test.go | 2 +- .../tui/components/reposection/reposection.go | 24 ++++--- .../reposection/reposection_test.go | 70 +++++++++++++------ 5 files changed, 75 insertions(+), 43 deletions(-) diff --git a/internal/data/prapi.go b/internal/data/prapi.go index f79532d36..bfd1f064b 100644 --- a/internal/data/prapi.go +++ b/internal/data/prapi.go @@ -107,18 +107,6 @@ type PullRequestData struct { IsDraft bool IsInMergeQueue bool AutoMergeRequest *AutoMergeRequest - // AutoMergeEnabled is a local UI flag set when the user enables auto-merge - // via the TUI. It is NOT fetched from the API; it mirrors the same field - // on prrow.Data so that the branch section can show the auto-merge icon - // immediately without waiting for a full refresh. - // - // Note: this field lives on the data-layer struct for pragmatic reasons — - // branch.Branch holds *data.PullRequestData directly, so there is no - // UI-wrapper type here to carry the flag. The graphql:"-" tag prevents - // shurcooL-graphql from including it in the GitHub API query. - // A cleaner home would be a dedicated UI-wrapper type for PullRequestData - // (analogous to prrow.Data), which is left as a future refactor. - AutoMergeEnabled bool `graphql:"-"` Commits Commits `graphql:"commits(last: 1)"` Labels PRLabels `graphql:"labels(first: 6)"` MergeStateStatus MergeStateStatus `graphql:"mergeStateStatus"` diff --git a/internal/tui/components/branch/branch.go b/internal/tui/components/branch/branch.go index 19b353605..6f1ea70ab 100644 --- a/internal/tui/components/branch/branch.go +++ b/internal/tui/components/branch/branch.go @@ -20,6 +20,12 @@ type Branch struct { PR *data.PullRequestData Data git.Branch Columns []table.Column + // AutoMergeEnabled is a UI-layer flag set when the user enables auto-merge + // via the TUI. It is NOT fetched from the API; it allows the branch + // section to show the auto-merge icon immediately without waiting for a + // full refresh. The flag is stored on a map in reposection.Model and + // re-applied each time updateBranchesWithPrs() rebuilds this struct. + AutoMergeEnabled bool } func (b *Branch) getTextStyle() lipgloss.Style { @@ -57,7 +63,7 @@ func (b *Branch) renderState() string { switch b.PR.State { case "OPEN": - if b.PR.AutoMergeRequest != nil || b.PR.AutoMergeEnabled { + if b.PR.AutoMergeRequest != nil || b.AutoMergeEnabled { return mergeCellStyle.Foreground(b.Ctx.Theme.WarningText).Render(constants.AutoMergeIcon) } if b.PR.IsDraft { @@ -215,7 +221,7 @@ func (b *Branch) RenderState() string { } switch b.PR.State { case "OPEN": - if b.PR.AutoMergeRequest != nil || b.PR.AutoMergeEnabled { + if b.PR.AutoMergeRequest != nil || b.AutoMergeEnabled { return constants.AutoMergeIcon + " Auto-merge" } if b.PR.IsDraft { diff --git a/internal/tui/components/branch/branch_test.go b/internal/tui/components/branch/branch_test.go index 2dfdd75a0..67bc7f917 100644 --- a/internal/tui/components/branch/branch_test.go +++ b/internal/tui/components/branch/branch_test.go @@ -17,8 +17,8 @@ func newTestBranch(prState string, autoMergeRequest *data.AutoMergeRequest, auto PR: &data.PullRequestData{ State: prState, AutoMergeRequest: autoMergeRequest, - AutoMergeEnabled: autoMergeEnabled, }, + AutoMergeEnabled: autoMergeEnabled, } } diff --git a/internal/tui/components/reposection/reposection.go b/internal/tui/components/reposection/reposection.go index b78d6d116..0b8ec1acb 100644 --- a/internal/tui/components/reposection/reposection.go +++ b/internal/tui/components/reposection/reposection.go @@ -29,11 +29,15 @@ const SectionType = "repo" type Model struct { section.BaseModel - repo *git.Repo - Branches []branch.Branch - Prs []data.PullRequestData - isRefreshSetUp bool - refreshId int + repo *git.Repo + Branches []branch.Branch + Prs []data.PullRequestData + // autoMergeEnabledPRs tracks PR numbers for which the user has enabled + // auto-merge via the TUI. It persists across updateBranchesWithPrs() + // rebuilds, which recreate m.Branches from scratch on every Update call. + autoMergeEnabledPRs map[int]bool + isRefreshSetUp bool + refreshId int } func NewModel( @@ -60,6 +64,7 @@ func NewModel( m.repo = &git.Repo{Branches: []git.Branch{}} m.Branches = []branch.Branch{} m.Prs = []data.PullRequestData{} + m.autoMergeEnabledPRs = make(map[int]bool) m.isRefreshSetUp = false return m @@ -179,9 +184,9 @@ func (m *Model) Update(msg tea.Msg) (section.Section, tea.Cmd) { m.Prs[i].Mergeable = "" } if msg.AutoMergeEnabled != nil && *msg.AutoMergeEnabled { - // Set a local flag so the auto-merge icon renders immediately - // without waiting for a full refresh. - m.Prs[i].AutoMergeEnabled = true + // Record the flag in the persistent map so it survives + // updateBranchesWithPrs() rebuilds. + m.autoMergeEnabledPRs[msg.PrNumber] = true } m.Table.SetRows(m.BuildRows()) break @@ -384,6 +389,9 @@ func (m *Model) updateBranchesWithPrs() { for _, ref := range m.repo.Branches { b := branch.Branch{Ctx: m.Ctx, Data: ref, Columns: m.Table.Columns} b.PR = findPRForRef(m.Prs, ref.Name) + if b.PR != nil { + b.AutoMergeEnabled = m.autoMergeEnabledPRs[b.PR.Number] + } branches = append(branches, b) } diff --git a/internal/tui/components/reposection/reposection_test.go b/internal/tui/components/reposection/reposection_test.go index b04dbe5b4..2fe3a9415 100644 --- a/internal/tui/components/reposection/reposection_test.go +++ b/internal/tui/components/reposection/reposection_test.go @@ -2,46 +2,65 @@ package reposection import ( "testing" + "time" tea "charm.land/bubbletea/v2" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/dlvhdr/gh-dash/v4/internal/config" "github.com/dlvhdr/gh-dash/v4/internal/data" "github.com/dlvhdr/gh-dash/v4/internal/git" - "github.com/dlvhdr/gh-dash/v4/internal/tui/components/section" "github.com/dlvhdr/gh-dash/v4/internal/tui/components/tasks" "github.com/dlvhdr/gh-dash/v4/internal/tui/context" + "github.com/dlvhdr/gh-dash/v4/internal/tui/theme" ) -// newTestRepoModel creates a minimal Model with a single PR in m.Prs so that -// the UpdatePRMsg handler can find and update it. +// newTestRepoModel creates a Model via the real NewModel constructor so that +// all internal sub-models are properly wired. A single git branch and a +// matching PR are injected so that UpdatePRMsg can find the PR and +// updateBranchesWithPrs can populate m.Branches. func newTestRepoModel(prNumber int) Model { + thm := *theme.DefaultTheme + s := context.InitStyles(thm) + cfg := &config.Config{ + // A non-nil ThemeConfig is required because ToTableRow dereferences + // Config.Theme.Ui.Table fields. + Theme: &config.ThemeConfig{}, + } ctx := &context.ProgramContext{ + Config: cfg, + Theme: thm, + Styles: s, StartTask: func(task context.Task) tea.Cmd { return func() tea.Msg { return nil } }, } - m := Model{ - BaseModel: section.BaseModel{ - Ctx: ctx, - }, - // repo must be non-nil; updateBranchesWithPrs ranges over repo.Branches. - repo: &git.Repo{Branches: []git.Branch{}}, - Prs: []data.PullRequestData{ - {Number: prNumber}, + + t := config.RepoView + m := NewModel(0, ctx, config.PrsSectionConfig{Type: &t}, time.Time{}) + + const branchName = "feature/test" + // Inject a real git.Repo so updateBranchesWithPrs has something to iterate. + m.repo = &git.Repo{ + Branches: []git.Branch{ + {Name: branchName}, }, } + m.Prs = []data.PullRequestData{ + {Number: prNumber, HeadRefName: branchName}, + } return m } // TestUpdatePRMsg_AutoMergeEnabled_SetsFlag verifies that when an UpdatePRMsg -// with AutoMergeEnabled=true is processed, the matching PR's AutoMergeEnabled -// flag is set to true on data.PullRequestData. +// with AutoMergeEnabled=true is processed: +// - the PR number is recorded in the autoMergeEnabledPRs map, and +// - the corresponding Branch in m.Branches has AutoMergeEnabled set to true. func TestUpdatePRMsg_AutoMergeEnabled_SetsFlag(t *testing.T) { m := newTestRepoModel(42) - require.False(t, m.Prs[0].AutoMergeEnabled, "AutoMergeEnabled should start false") + require.False(t, m.autoMergeEnabledPRs[42], "autoMergeEnabledPRs should start empty") autoMerge := true msg := tasks.UpdatePRMsg{ @@ -52,9 +71,17 @@ func TestUpdatePRMsg_AutoMergeEnabled_SetsFlag(t *testing.T) { result, _ := m.Update(msg) updated := result.(*Model) - assert.True(t, updated.Prs[0].AutoMergeEnabled, - "AutoMergeEnabled should be set to true after processing UpdatePRMsg") - assert.Nil(t, updated.Prs[0].AutoMergeRequest, + assert.True(t, updated.autoMergeEnabledPRs[42], + "autoMergeEnabledPRs[42] should be true after processing UpdatePRMsg") + + // The flag must also be visible on the corresponding Branch so that + // renderState / RenderState can show the auto-merge icon immediately. + require.Len(t, updated.Branches, 1, "expected one branch after update") + assert.True(t, updated.Branches[0].AutoMergeEnabled, + "Branch.AutoMergeEnabled should be true after processing UpdatePRMsg") + + // AutoMergeRequest must not be touched — only real API data should set it. + assert.Nil(t, updated.Branches[0].PR.AutoMergeRequest, "AutoMergeRequest should remain nil (only real API data should populate it)") } @@ -78,7 +105,7 @@ func TestUpdatePRMsg_IsMerged_SetsState(t *testing.T) { } // TestUpdatePRMsg_UnknownPR_NoChange verifies that a message for an unknown -// PR number does not mutate any existing PR. +// PR number does not mutate any existing PR or branch. func TestUpdatePRMsg_UnknownPR_NoChange(t *testing.T) { m := newTestRepoModel(1) m.Prs[0].State = "OPEN" @@ -92,8 +119,11 @@ func TestUpdatePRMsg_UnknownPR_NoChange(t *testing.T) { result, _ := m.Update(msg) updated := result.(*Model) - assert.False(t, updated.Prs[0].AutoMergeEnabled, - "unrelated PR should not have AutoMergeEnabled set") + assert.False(t, updated.autoMergeEnabledPRs[1], + "unrelated PR should not have AutoMergeEnabled set in the map") assert.Equal(t, "OPEN", updated.Prs[0].State, "unrelated PR state should be unchanged") + require.Len(t, updated.Branches, 1) + assert.False(t, updated.Branches[0].AutoMergeEnabled, + "unrelated branch should not have AutoMergeEnabled set") } From 87ca472a46ac10711a94db3b30d6e8cc9d8865cb Mon Sep 17 00:00:00 2001 From: Rob Berwick Date: Sat, 14 Mar 2026 11:57:42 +0000 Subject: [PATCH 22/22] fix: guard against nil markdownStyle in GetMarkdownRenderer InitializeMarkdownStyle is only called when tea.BackgroundColorMsg arrives, which can happen after the first View() call that renders the PR sidebar. If GetMarkdownRenderer is called before that message arrives, dereferencing markdownStyle panics. Add a nil-check that falls back to the dark style, matching the behaviour InitializeMarkdownStyle would produce. The style will be overwritten as normal if BackgroundColorMsg subsequently arrives. --- internal/tui/markdown/markdownRenderer.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/tui/markdown/markdownRenderer.go b/internal/tui/markdown/markdownRenderer.go index 307c153ba..020d35077 100644 --- a/internal/tui/markdown/markdownRenderer.go +++ b/internal/tui/markdown/markdownRenderer.go @@ -20,6 +20,12 @@ func InitializeMarkdownStyle(hasDarkBackground bool) { } func GetMarkdownRenderer(width int) glamour.TermRenderer { + if markdownStyle == nil { + // BackgroundColorMsg has not arrived yet (e.g. the sidebar is rendered + // before the terminal reports its background colour). Fall back to the + // dark style so rendering does not panic. + InitializeMarkdownStyle(true) + } markdownRenderer, err := glamour.NewTermRenderer( glamour.WithStyles(*markdownStyle), glamour.WithWordWrap(width),