From c4b4791de66bea1872fe2a02607a085f4a2b5ec3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Wed, 12 Aug 2026 08:12:53 +0100 Subject: [PATCH] fix(git): contain rewritten contribution histories (#2509) --- .../internal/agent/runtime/agentctl/git.go | 39 +-- .../agent/runtime/agentctl/git_test.go | 7 +- .../agent/runtime/lifecycle/event_types.go | 11 +- .../agent/runtime/lifecycle/events.go | 37 +-- .../lifecycle/events_git_status_test.go | 24 +- .../internal/agentctl/server/api/git.go | 113 ++++---- .../server/process/workspace_git_status.go | 45 +++- .../process/workspace_git_status_test.go | 68 +++++ .../server/process/workspace_tracker_test.go | 89 +++++++ .../internal/agentctl/types/streams/git.go | 5 + apps/backend/internal/backendapp/helpers.go | 31 ++- .../internal/backendapp/helpers_test.go | 34 +++ apps/backend/internal/github/handlers.go | 2 +- apps/backend/internal/github/models.go | 9 + apps/backend/internal/github/pat_client.go | 12 +- .../internal/github/pat_client_writes_test.go | 26 ++ apps/backend/internal/github/service_pr.go | 41 ++- .../internal/github/service_pr_test.go | 42 +++ .../components/task/changes-panel-body.tsx | 150 +++++++++-- .../components/task/changes-panel-data.tsx | 45 +++- .../task/changes-panel-header.test.tsx | 54 ++++ .../components/task/changes-panel-header.tsx | 53 ++-- .../components/task/changes-panel-helpers.ts | 60 +++++ .../task/changes-panel-per-repo-menu.tsx | 6 +- .../task/changes-panel-remote.test.ts | 39 ++- .../task/changes-panel-timeline.tsx | 28 +- apps/web/components/task/changes-panel.tsx | 11 +- apps/web/components/task/commit-row.test.tsx | 17 ++ apps/web/components/task/commit-row.tsx | 56 ++-- .../task/mobile/mobile-changes-panel.tsx | 3 +- .../task/mobile/mobile-git-push-submenu.tsx | 54 ++++ ...ssion-mobile-top-bar-git-controls.test.tsx | 70 +++++ .../session-mobile-top-bar-git-controls.tsx | 59 ++--- .../task/mobile/session-mobile-top-bar.tsx | 12 +- .../components/vcs-multi-repo-menu.test.ts | 22 ++ apps/web/components/vcs-multi-repo-menu.tsx | 64 ++++- apps/web/components/vcs-split-button.test.ts | 11 + apps/web/components/vcs-split-button.tsx | 66 +++-- .../e2e/tests/git/git-changes-panel.spec.ts | 247 ++++++++++++++++++ .../git/mobile-pr-checkout-drift.spec.ts | 184 +++++++++++++ .../domains/github/use-pr-commits.test.ts | 17 ++ .../hooks/domains/github/use-pr-commits.ts | 55 +++- .../remote-contribution-relation.test.ts | 215 +++++++++++++++ .../session/remote-contribution-relation.ts | 142 ++++++++++ .../use-remote-contribution-relation.test.tsx | 122 +++++++++ .../use-remote-contribution-relation.ts | 88 +++++++ .../use-scoped-stage-operations.test.ts | 51 ++++ .../session/use-scoped-stage-operations.ts | 32 +++ .../session/use-session-git-derived.test.ts | 72 +++++ .../session/use-session-git-derived.ts | 65 +++++ .../session/use-session-git-summary.test.ts | 47 ++++ .../session/use-session-git-summary.ts | 48 +++- .../hooks/domains/session/use-session-git.ts | 67 ++--- .../git-status-multi-repo.test.ts | 68 ++--- .../session-runtime/session-runtime-slice.ts | 5 + .../lib/state/slices/session-runtime/types.ts | 5 + apps/web/lib/types/git-events.ts | 3 + apps/web/lib/ws/handlers/git-status.test.ts | 27 ++ apps/web/lib/ws/handlers/git-status.ts | 10 + apps/web/src/locales/en/task.json | 7 + apps/web/src/locales/pseudo/task.json | 7 + apps/web/src/locales/pt-pt/task.json | 7 + apps/web/src/locales/zh-cn/task.json | 7 + ...26-08-10-remote-contribution-head-drift.md | 101 +++++++ docs/decisions/INDEX.md | 1 + .../remote-contribution-head-drift/plan.md | 185 +++++++++++++ .../task-01-upstream-status-contract.md | 88 +++++++ ...-checkout-relation-and-action-semantics.md | 110 ++++++++ ...3-diverged-changes-ui-and-mobile-parity.md | 124 +++++++++ ...04-responsive-force-push-regression-e2e.md | 96 +++++++ docs/specs/tasks/remote-contribution-tasks.md | 67 ++++- 71 files changed, 3535 insertions(+), 350 deletions(-) create mode 100644 apps/backend/internal/github/service_pr_test.go create mode 100644 apps/web/components/task/changes-panel-header.test.tsx create mode 100644 apps/web/components/task/mobile/mobile-git-push-submenu.tsx create mode 100644 apps/web/components/task/mobile/session-mobile-top-bar-git-controls.test.tsx create mode 100644 apps/web/components/vcs-multi-repo-menu.test.ts create mode 100644 apps/web/e2e/tests/git/mobile-pr-checkout-drift.spec.ts create mode 100644 apps/web/hooks/domains/session/remote-contribution-relation.test.ts create mode 100644 apps/web/hooks/domains/session/remote-contribution-relation.ts create mode 100644 apps/web/hooks/domains/session/use-remote-contribution-relation.test.tsx create mode 100644 apps/web/hooks/domains/session/use-remote-contribution-relation.ts create mode 100644 apps/web/hooks/domains/session/use-scoped-stage-operations.test.ts create mode 100644 apps/web/hooks/domains/session/use-scoped-stage-operations.ts create mode 100644 apps/web/hooks/domains/session/use-session-git-derived.test.ts create mode 100644 apps/web/hooks/domains/session/use-session-git-derived.ts create mode 100644 docs/decisions/2026-08-10-remote-contribution-head-drift.md create mode 100644 docs/plans/remote-contribution-head-drift/plan.md create mode 100644 docs/plans/remote-contribution-head-drift/task-01-upstream-status-contract.md create mode 100644 docs/plans/remote-contribution-head-drift/task-02-checkout-relation-and-action-semantics.md create mode 100644 docs/plans/remote-contribution-head-drift/task-03-diverged-changes-ui-and-mobile-parity.md create mode 100644 docs/plans/remote-contribution-head-drift/task-04-responsive-force-push-regression-e2e.md diff --git a/apps/backend/internal/agent/runtime/agentctl/git.go b/apps/backend/internal/agent/runtime/agentctl/git.go index 5bd8ff951f..c3ee913aad 100644 --- a/apps/backend/internal/agent/runtime/agentctl/git.go +++ b/apps/backend/internal/agent/runtime/agentctl/git.go @@ -498,24 +498,27 @@ func (c *Client) GetCumulativeDiff(ctx context.Context, baseCommit, targetBranch // GitStatusResult represents the result of a git status query. type GitStatusResult struct { - Success bool `json:"success"` - IsSubmodule bool `json:"is_submodule,omitempty"` - Branch string `json:"branch"` - RemoteBranch string `json:"remote_branch"` - HeadCommit string `json:"head_commit"` - BaseCommit string `json:"base_commit"` // Merge-base with origin branch - Ahead int `json:"ahead"` - Behind int `json:"behind"` - Modified []string `json:"modified"` - Added []string `json:"added"` - Deleted []string `json:"deleted"` - Untracked []string `json:"untracked"` - Renamed []string `json:"renamed"` - Files map[string]interface{} `json:"files"` - Timestamp string `json:"timestamp"` - BranchAdditions int `json:"branch_additions,omitempty"` - BranchDeletions int `json:"branch_deletions,omitempty"` - Error string `json:"error,omitempty"` + Success bool `json:"success"` + IsSubmodule bool `json:"is_submodule,omitempty"` + Branch string `json:"branch"` + RemoteBranch string `json:"remote_branch"` + HeadCommit string `json:"head_commit"` + BaseCommit string `json:"base_commit"` // Merge-base with origin branch + Ahead int `json:"ahead"` + Behind int `json:"behind"` + RemoteAhead int `json:"remote_ahead"` + RemoteBehind int `json:"remote_behind"` + RemoteHeadCommit string `json:"remote_head_commit,omitempty"` + Modified []string `json:"modified"` + Added []string `json:"added"` + Deleted []string `json:"deleted"` + Untracked []string `json:"untracked"` + Renamed []string `json:"renamed"` + Files map[string]interface{} `json:"files"` + Timestamp string `json:"timestamp"` + BranchAdditions int `json:"branch_additions,omitempty"` + BranchDeletions int `json:"branch_deletions,omitempty"` + Error string `json:"error,omitempty"` } // fetchJSONResult performs a GET against `path` and decodes the response into diff --git a/apps/backend/internal/agent/runtime/agentctl/git_test.go b/apps/backend/internal/agent/runtime/agentctl/git_test.go index 55c543358e..bbfa034441 100644 --- a/apps/backend/internal/agent/runtime/agentctl/git_test.go +++ b/apps/backend/internal/agent/runtime/agentctl/git_test.go @@ -611,7 +611,8 @@ const gitStatusBody = `{ "success":true,"is_submodule":true, "branch":"feature/x","remote_branch":"origin/feature/x", "head_commit":"head1","base_commit":"base1", - "ahead":3,"behind":1, + "ahead":3,"behind":1,"remote_ahead":4,"remote_behind":2, + "remote_head_commit":"remote1", "modified":["m.go"],"added":["a.go"],"deleted":["d.go"], "untracked":["u.go"],"renamed":["r.go"], "files":{"m.go":{"status":"M"}}, @@ -633,6 +634,10 @@ func assertFullGitStatus(t *testing.T, result *GitStatusResult) { if result.Ahead != 3 || result.Behind != 1 { t.Errorf("ahead/behind = %d / %d, want 3 / 1", result.Ahead, result.Behind) } + if result.RemoteAhead != 4 || result.RemoteBehind != 2 || result.RemoteHeadCommit != "remote1" { + t.Errorf("remote ancestry = %d / %d / %q, want 4 / 2 / remote1", + result.RemoteAhead, result.RemoteBehind, result.RemoteHeadCommit) + } for name, got := range map[string][]string{ "modified": result.Modified, "added": result.Added, diff --git a/apps/backend/internal/agent/runtime/lifecycle/event_types.go b/apps/backend/internal/agent/runtime/lifecycle/event_types.go index 979c9e5f5e..8d884b0511 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/event_types.go +++ b/apps/backend/internal/agent/runtime/lifecycle/event_types.go @@ -291,11 +291,12 @@ type GitStatusData struct { // (@{upstream}), unlike Ahead/Behind which are relative to the base // branch and never reach zero just because the branch was pushed. Push // detection (event_handlers_git.go) reads RemoteAhead, not Ahead. - RemoteAhead int `json:"remote_ahead"` - RemoteBehind int `json:"remote_behind"` - Files interface{} `json:"files,omitempty"` - BranchAdditions int `json:"branch_additions,omitempty"` - BranchDeletions int `json:"branch_deletions,omitempty"` + RemoteAhead int `json:"remote_ahead"` + RemoteBehind int `json:"remote_behind"` + RemoteHeadCommit string `json:"remote_head_commit,omitempty"` + Files interface{} `json:"files,omitempty"` + BranchAdditions int `json:"branch_additions,omitempty"` + BranchDeletions int `json:"branch_deletions,omitempty"` // RepositoryName identifies which repository this status belongs to in // multi-repo task workspaces. Empty for single-repo. Carried through to // the frontend so the Changes panel can render per-repo group headers. diff --git a/apps/backend/internal/agent/runtime/lifecycle/events.go b/apps/backend/internal/agent/runtime/lifecycle/events.go index c4817ec0b9..db7b9f67b6 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/events.go +++ b/apps/backend/internal/agent/runtime/lifecycle/events.go @@ -289,24 +289,25 @@ func (p *EventPublisher) PublishGitStatus(execution *AgentExecution, update *age AgentID: execution.ID, Timestamp: update.Timestamp.Format(time.RFC3339Nano), Status: &GitStatusData{ - Branch: update.Branch, - RemoteBranch: update.RemoteBranch, - HeadCommit: update.HeadCommit, - BaseCommit: update.BaseCommit, - Modified: update.Modified, - Added: update.Added, - Deleted: update.Deleted, - Untracked: update.Untracked, - Renamed: update.Renamed, - Ahead: update.Ahead, - Behind: update.Behind, - RemoteAhead: update.RemoteAhead, - RemoteBehind: update.RemoteBehind, - Files: update.Files, - BranchAdditions: update.BranchAdditions, - BranchDeletions: update.BranchDeletions, - RepositoryName: update.RepositoryName, - IsSubmodule: update.IsSubmodule, + Branch: update.Branch, + RemoteBranch: update.RemoteBranch, + HeadCommit: update.HeadCommit, + BaseCommit: update.BaseCommit, + Modified: update.Modified, + Added: update.Added, + Deleted: update.Deleted, + Untracked: update.Untracked, + Renamed: update.Renamed, + Ahead: update.Ahead, + Behind: update.Behind, + RemoteAhead: update.RemoteAhead, + RemoteBehind: update.RemoteBehind, + RemoteHeadCommit: update.RemoteHeadCommit, + Files: update.Files, + BranchAdditions: update.BranchAdditions, + BranchDeletions: update.BranchDeletions, + RepositoryName: update.RepositoryName, + IsSubmodule: update.IsSubmodule, }, }) } diff --git a/apps/backend/internal/agent/runtime/lifecycle/events_git_status_test.go b/apps/backend/internal/agent/runtime/lifecycle/events_git_status_test.go index 5033385a7f..8bfe224973 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/events_git_status_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/events_git_status_test.go @@ -37,12 +37,18 @@ func TestPublishGitStatus_PropagatesRepositoryName(t *testing.T) { SessionID: "sess-multi", } pub.PublishGitStatus(exec, &agentctl.GitStatusUpdate{ - Timestamp: time.Now(), - RepositoryName: "frontend", - IsSubmodule: true, - Branch: "feature/x", - Modified: []string{"src/app.tsx"}, - Files: map[string]agentctl.FileInfo{"src/app.tsx": {Path: "src/app.tsx"}}, + Timestamp: time.Now(), + RepositoryName: "frontend", + IsSubmodule: true, + Branch: "feature/x", + HeadCommit: "local-head", + BaseCommit: "base-head", + RemoteBranch: "contributor/feature/x", + RemoteHeadCommit: "upstream-head", + RemoteAhead: 2, + RemoteBehind: 1, + Modified: []string{"src/app.tsx"}, + Files: map[string]agentctl.FileInfo{"src/app.tsx": {Path: "src/app.tsx"}}, }) select { @@ -60,6 +66,12 @@ func TestPublishGitStatus_PropagatesRepositoryName(t *testing.T) { if !payload.Status.IsSubmodule { t.Error("is_submodule was dropped") } + if payload.Status.HeadCommit != "local-head" || payload.Status.BaseCommit != "base-head" { + t.Errorf("commit comparison SHAs were dropped: head=%q base=%q", payload.Status.HeadCommit, payload.Status.BaseCommit) + } + if payload.Status.RemoteHeadCommit != "upstream-head" || payload.Status.RemoteAhead != 2 || payload.Status.RemoteBehind != 1 { + t.Errorf("upstream evidence was dropped: head=%q ahead=%d behind=%d", payload.Status.RemoteHeadCommit, payload.Status.RemoteAhead, payload.Status.RemoteBehind) + } case <-time.After(2 * time.Second): t.Fatal("timed out waiting for git status event") } diff --git a/apps/backend/internal/agentctl/server/api/git.go b/apps/backend/internal/agentctl/server/api/git.go index 02f6f71e2c..269b8ad7fb 100644 --- a/apps/backend/internal/agentctl/server/api/git.go +++ b/apps/backend/internal/agentctl/server/api/git.go @@ -1246,24 +1246,27 @@ func mergeCumulativeFiles(dst, src map[string]interface{}, repo, baseRef string, // GitStatusResult represents the result of a git status query. type GitStatusResult struct { - Success bool `json:"success"` - IsSubmodule bool `json:"is_submodule,omitempty"` - Branch string `json:"branch"` - RemoteBranch string `json:"remote_branch"` - HeadCommit string `json:"head_commit"` - BaseCommit string `json:"base_commit"` // Merge-base with origin branch - Ahead int `json:"ahead"` - Behind int `json:"behind"` - Modified []string `json:"modified"` - Added []string `json:"added"` - Deleted []string `json:"deleted"` - Untracked []string `json:"untracked"` - Renamed []string `json:"renamed"` - Files map[string]interface{} `json:"files"` - Timestamp string `json:"timestamp"` - BranchAdditions int `json:"branch_additions,omitempty"` - BranchDeletions int `json:"branch_deletions,omitempty"` - Error string `json:"error,omitempty"` + Success bool `json:"success"` + IsSubmodule bool `json:"is_submodule,omitempty"` + Branch string `json:"branch"` + RemoteBranch string `json:"remote_branch"` + HeadCommit string `json:"head_commit"` + BaseCommit string `json:"base_commit"` // Merge-base with origin branch + Ahead int `json:"ahead"` + Behind int `json:"behind"` + RemoteAhead int `json:"remote_ahead"` + RemoteBehind int `json:"remote_behind"` + RemoteHeadCommit string `json:"remote_head_commit,omitempty"` + Modified []string `json:"modified"` + Added []string `json:"added"` + Deleted []string `json:"deleted"` + Untracked []string `json:"untracked"` + Renamed []string `json:"renamed"` + Files map[string]interface{} `json:"files"` + Timestamp string `json:"timestamp"` + BranchAdditions int `json:"branch_additions,omitempty"` + BranchDeletions int `json:"branch_deletions,omitempty"` + Error string `json:"error,omitempty"` } // PerRepoGitStatus pairs a repository_name with its current status. Used by @@ -1357,23 +1360,26 @@ func (s *Server) collectStatusForRepo(ctx context.Context, sub string, fresh boo return PerRepoGitStatus{ RepositoryName: sub, Status: GitStatusResult{ - Success: true, - IsSubmodule: status.IsSubmodule, - Branch: status.Branch, - RemoteBranch: status.RemoteBranch, - HeadCommit: status.HeadCommit, - BaseCommit: status.BaseCommit, - Ahead: status.Ahead, - Behind: status.Behind, - Modified: status.Modified, - Added: status.Added, - Deleted: status.Deleted, - Untracked: status.Untracked, - Renamed: status.Renamed, - Files: filesMap, - Timestamp: status.Timestamp.Format("2006-01-02T15:04:05.000Z07:00"), - BranchAdditions: status.BranchAdditions, - BranchDeletions: status.BranchDeletions, + Success: true, + IsSubmodule: status.IsSubmodule, + Branch: status.Branch, + RemoteBranch: status.RemoteBranch, + HeadCommit: status.HeadCommit, + BaseCommit: status.BaseCommit, + Ahead: status.Ahead, + Behind: status.Behind, + RemoteAhead: status.RemoteAhead, + RemoteBehind: status.RemoteBehind, + RemoteHeadCommit: status.RemoteHeadCommit, + Modified: status.Modified, + Added: status.Added, + Deleted: status.Deleted, + Untracked: status.Untracked, + Renamed: status.Renamed, + Files: filesMap, + Timestamp: status.Timestamp.Format("2006-01-02T15:04:05.000Z07:00"), + BranchAdditions: status.BranchAdditions, + BranchDeletions: status.BranchDeletions, }, } } @@ -1416,23 +1422,26 @@ func (s *Server) handleGitStatus(c *gin.Context) { } c.JSON(http.StatusOK, GitStatusResult{ - Success: true, - IsSubmodule: status.IsSubmodule, - Branch: status.Branch, - RemoteBranch: status.RemoteBranch, - HeadCommit: status.HeadCommit, - BaseCommit: status.BaseCommit, - Ahead: status.Ahead, - Behind: status.Behind, - Modified: status.Modified, - Added: status.Added, - Deleted: status.Deleted, - Untracked: status.Untracked, - Renamed: status.Renamed, - Files: filesMap, - Timestamp: status.Timestamp.Format("2006-01-02T15:04:05.000Z07:00"), - BranchAdditions: status.BranchAdditions, - BranchDeletions: status.BranchDeletions, + Success: true, + IsSubmodule: status.IsSubmodule, + Branch: status.Branch, + RemoteBranch: status.RemoteBranch, + HeadCommit: status.HeadCommit, + BaseCommit: status.BaseCommit, + Ahead: status.Ahead, + Behind: status.Behind, + RemoteAhead: status.RemoteAhead, + RemoteBehind: status.RemoteBehind, + RemoteHeadCommit: status.RemoteHeadCommit, + Modified: status.Modified, + Added: status.Added, + Deleted: status.Deleted, + Untracked: status.Untracked, + Renamed: status.Renamed, + Files: filesMap, + Timestamp: status.Timestamp.Format("2006-01-02T15:04:05.000Z07:00"), + BranchAdditions: status.BranchAdditions, + BranchDeletions: status.BranchDeletions, }) } diff --git a/apps/backend/internal/agentctl/server/process/workspace_git_status.go b/apps/backend/internal/agentctl/server/process/workspace_git_status.go index 301b8c20ef..7300364943 100644 --- a/apps/backend/internal/agentctl/server/process/workspace_git_status.go +++ b/apps/backend/internal/agentctl/server/process/workspace_git_status.go @@ -467,21 +467,41 @@ func (wt *WorkspaceTracker) getAheadBehindCounts(ctx context.Context, update *ty // (which would look like "just pushed" to a push-detection consumer). func (wt *WorkspaceTracker) getRemoteAheadBehindCounts(ctx context.Context, update *types.GitStatusUpdate, prior types.GitStatusUpdate) { if update.RemoteBranch == "" { + update.RemoteHeadCommit = "" update.RemoteAhead = 0 update.RemoteBehind = 0 return } - countOut, err := wt.runGitOutput(ctx, "rev-list", "--left-right", "--count", "HEAD..."+update.RemoteBranch) + // RemoteBranch came from Git, but keep the same inline ref boundary as the + // other status comparisons before using it in a command argument. + rest, hasOriginPrefix := strings.CutPrefix(update.RemoteBranch, "origin/") + check := update.RemoteBranch + if hasOriginPrefix { + check = rest + } + if !safeBranchRefPattern.MatchString(check) || strings.Contains(check, "..") || strings.HasSuffix(check, ".lock") { + carryRemoteSnapshot(update, prior) + return + } + remoteHeadOut, err := wt.runGitOutput(ctx, "rev-parse", "--verify", update.RemoteBranch+"^{commit}") + if err != nil { + wt.logger.Debug("getRemoteAheadBehindCounts: rev-parse failed, carrying forward", zap.Error(err)) + carryRemoteSnapshot(update, prior) + return + } + remoteHead := strings.TrimSpace(string(remoteHeadOut)) + countOut, err := wt.runGitOutput(ctx, "rev-list", "--left-right", "--count", "HEAD..."+remoteHead) if err != nil { wt.logger.Debug("getRemoteAheadBehindCounts: rev-list failed, carrying forward", zap.Error(err)) - carryRemoteAheadBehind(update, prior) + carryRemoteSnapshot(update, prior) return } parts := strings.Fields(string(countOut)) if len(parts) != 2 { - carryRemoteAheadBehind(update, prior) + carryRemoteSnapshot(update, prior) return } + update.RemoteHeadCommit = remoteHead update.RemoteAhead, _ = strconv.Atoi(parts[0]) update.RemoteBehind, _ = strconv.Atoi(parts[1]) } @@ -693,6 +713,25 @@ func carryRemoteAheadBehind(update *types.GitStatusUpdate, prior types.GitStatus update.RemoteBehind = prior.RemoteBehind } +// carryRemoteSnapshot keeps the upstream tip and divergence counts coherent +// when one of the secondary upstream observations fails. A changed local HEAD +// or tracking ref makes the previous snapshot unsafe, so the caller keeps the +// zero-value unknown state instead. +func carryRemoteSnapshot(update *types.GitStatusUpdate, prior types.GitStatusUpdate) { + update.RemoteHeadCommit = "" + update.RemoteAhead = 0 + update.RemoteBehind = 0 + if prior.HeadCommit == "" || prior.HeadCommit != update.HeadCommit { + return + } + if prior.RemoteBranch == "" || prior.RemoteBranch != update.RemoteBranch || prior.RemoteHeadCommit == "" { + return + } + update.RemoteHeadCommit = prior.RemoteHeadCommit + update.RemoteAhead = prior.RemoteAhead + update.RemoteBehind = prior.RemoteBehind +} + // parseGitStatusOutput runs git status --porcelain and populates the file lists and map. func (wt *WorkspaceTracker) parseGitStatusOutput(ctx context.Context, update *types.GitStatusUpdate) error { // --untracked-files=all shows all files in untracked directories, not just diff --git a/apps/backend/internal/agentctl/server/process/workspace_git_status_test.go b/apps/backend/internal/agentctl/server/process/workspace_git_status_test.go index 2d3ad81854..028ca3764c 100644 --- a/apps/backend/internal/agentctl/server/process/workspace_git_status_test.go +++ b/apps/backend/internal/agentctl/server/process/workspace_git_status_test.go @@ -480,3 +480,71 @@ func TestCarryRemoteAheadBehind(t *testing.T) { }) } } + +func TestCarryRemoteSnapshot(t *testing.T) { + const ( + localHead = "local-head" + remoteBranch = "origin/feature" + remoteHead = "remote-head" + ) + tests := []struct { + name string + prior types.GitStatusUpdate + update types.GitStatusUpdate + wantRemoteHead string + wantRemoteAhead int + wantRemoteBehind int + }{ + { + name: "same local and upstream identity preserves complete snapshot", + prior: types.GitStatusUpdate{ + HeadCommit: localHead, + RemoteBranch: remoteBranch, + RemoteHeadCommit: remoteHead, + RemoteAhead: 2, + RemoteBehind: 1, + }, + update: types.GitStatusUpdate{HeadCommit: localHead, RemoteBranch: remoteBranch}, + wantRemoteHead: remoteHead, + wantRemoteAhead: 2, + wantRemoteBehind: 1, + }, + { + name: "changed local head drops snapshot", + prior: types.GitStatusUpdate{ + HeadCommit: localHead, + RemoteBranch: remoteBranch, + RemoteHeadCommit: remoteHead, + RemoteAhead: 2, + RemoteBehind: 1, + }, + update: types.GitStatusUpdate{ + HeadCommit: "new-local-head", + RemoteBranch: remoteBranch, + }, + }, + { + name: "changed upstream identity drops snapshot", + prior: types.GitStatusUpdate{ + HeadCommit: localHead, + RemoteBranch: remoteBranch, + RemoteHeadCommit: remoteHead, + RemoteAhead: 2, + RemoteBehind: 1, + }, + update: types.GitStatusUpdate{ + HeadCommit: localHead, + RemoteBranch: "origin/other", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + update := tt.update + carryRemoteSnapshot(&update, tt.prior) + if update.RemoteHeadCommit != tt.wantRemoteHead || update.RemoteAhead != tt.wantRemoteAhead || update.RemoteBehind != tt.wantRemoteBehind { + t.Fatalf("remote snapshot = %q/%d/%d, want %q/%d/%d", update.RemoteHeadCommit, update.RemoteAhead, update.RemoteBehind, tt.wantRemoteHead, tt.wantRemoteAhead, tt.wantRemoteBehind) + } + }) + } +} diff --git a/apps/backend/internal/agentctl/server/process/workspace_tracker_test.go b/apps/backend/internal/agentctl/server/process/workspace_tracker_test.go index de9defc77b..5ea6761056 100644 --- a/apps/backend/internal/agentctl/server/process/workspace_tracker_test.go +++ b/apps/backend/internal/agentctl/server/process/workspace_tracker_test.go @@ -3,6 +3,7 @@ package process import ( "context" "encoding/base64" + "encoding/json" "os" "os/exec" "path/filepath" @@ -279,6 +280,94 @@ func TestGetGitStatus_AheadBehindWithoutUpstream(t *testing.T) { } } +func TestGetGitStatus_ReportsUpstreamHeadForHistoryStates(t *testing.T) { + repoDir, cleanup := setupTestRepo(t) + defer cleanup() + + log := newTestLogger(t) + wt := NewWorkspaceTracker(repoDir, log) + ctx := context.Background() + + runGit(t, repoDir, "checkout", "-b", "feature/pr") + runGit(t, repoDir, "push", "-u", "origin", "feature/pr") + pushedHead := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD")) + + status, err := wt.getGitStatus(ctx) + if err != nil { + t.Fatalf("getGitStatus (aligned): %v", err) + } + if got := remoteHeadFromWire(t, status); got != pushedHead { + t.Fatalf("remote_head_commit (aligned) = %q, want %q", got, pushedHead) + } + + writeFile(t, repoDir, "local.txt", "local") + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "local contribution") + status, err = wt.getGitStatus(ctx) + if err != nil { + t.Fatalf("getGitStatus (local ahead): %v", err) + } + if got := remoteHeadFromWire(t, status); got != pushedHead { + t.Fatalf("remote_head_commit (local ahead) = %q, want %q", got, pushedHead) + } + + remoteURL := strings.TrimSpace(runGit(t, repoDir, "remote", "get-url", "origin")) + providerRoot := t.TempDir() + runGit(t, providerRoot, "clone", remoteURL, "provider") + providerDir := filepath.Join(providerRoot, "provider") + runGit(t, providerDir, "config", "user.email", "provider@test.com") + runGit(t, providerDir, "config", "user.name", "Provider User") + runGit(t, providerDir, "checkout", "-b", "feature/pr", "origin/feature/pr") + writeFile(t, providerDir, "provider.txt", "provider") + runGit(t, providerDir, "add", ".") + runGit(t, providerDir, "commit", "-m", "provider contribution") + providerHead := strings.TrimSpace(runGit(t, providerDir, "rev-parse", "HEAD")) + runGit(t, providerDir, "push", "origin", "HEAD:feature/pr") + runGit(t, repoDir, "fetch", "origin") + + status, err = wt.getGitStatus(ctx) + if err != nil { + t.Fatalf("getGitStatus (provider ahead): %v", err) + } + if got := remoteHeadFromWire(t, status); got != providerHead { + t.Fatalf("remote_head_commit (provider ahead) = %q, want %q", got, providerHead) + } + + runGit(t, providerDir, "checkout", "--orphan", "rewritten") + runGit(t, providerDir, "rm", "-rf", ".") + writeFile(t, providerDir, "rewritten.txt", "rewritten") + runGit(t, providerDir, "add", ".") + runGit(t, providerDir, "commit", "-m", "rewritten contribution") + rewrittenHead := strings.TrimSpace(runGit(t, providerDir, "rev-parse", "HEAD")) + runGit(t, providerDir, "push", "--force", "origin", "HEAD:feature/pr") + runGit(t, repoDir, "fetch", "origin") + + status, err = wt.getGitStatus(ctx) + if err != nil { + t.Fatalf("getGitStatus (diverged): %v", err) + } + if got := remoteHeadFromWire(t, status); got != rewrittenHead { + t.Fatalf("remote_head_commit (diverged) = %q, want %q", got, rewrittenHead) + } +} + +func remoteHeadFromWire(t *testing.T, status types.GitStatusUpdate) string { + t.Helper() + wire, err := json.Marshal(status) + if err != nil { + t.Fatalf("marshal git status: %v", err) + } + var payload map[string]any + if err := json.Unmarshal(wire, &payload); err != nil { + t.Fatalf("unmarshal git status: %v", err) + } + value, ok := payload["remote_head_commit"].(string) + if !ok { + t.Fatalf("remote_head_commit missing from git status: %s", wire) + } + return value +} + // TestGetGitStatus_AheadBehindAfterRebase verifies that ahead/behind counts // reflect divergence from the base branch (origin/main), not the remote tracking // branch. After a rebase, the remote tracking branch has stale SHAs, so comparing diff --git a/apps/backend/internal/agentctl/types/streams/git.go b/apps/backend/internal/agentctl/types/streams/git.go index 8911abc31f..743a155595 100644 --- a/apps/backend/internal/agentctl/types/streams/git.go +++ b/apps/backend/internal/agentctl/types/streams/git.go @@ -65,6 +65,11 @@ type GitStatusUpdate struct { // empty. RemoteBehind int `json:"remote_behind"` + // RemoteHeadCommit is the locally observed tip of RemoteBranch. It is + // captured with RemoteAhead and RemoteBehind so consumers can compare the + // checkout with an upstream snapshot without confusing it with the base. + RemoteHeadCommit string `json:"remote_head_commit,omitempty"` + // HeadCommit is the current HEAD commit SHA. HeadCommit string `json:"head_commit,omitempty"` diff --git a/apps/backend/internal/backendapp/helpers.go b/apps/backend/internal/backendapp/helpers.go index 9f3becb9d2..905e03fc16 100644 --- a/apps/backend/internal/backendapp/helpers.go +++ b/apps/backend/internal/backendapp/helpers.go @@ -334,19 +334,24 @@ func tryGetLiveGitStatus(ctx context.Context, lifecycleMgr *lifecycle.Manager, s // stores it under byEnvironmentRepo[envKey][repository_name]. func buildGitStatusNotification(sessionID, repositoryName string, status client.GitStatusResult) *ws.Message { statusPayload := map[string]interface{}{ - "branch": status.Branch, - "remote_branch": status.RemoteBranch, - "ahead": status.Ahead, - "behind": status.Behind, - "files": status.Files, - "modified": status.Modified, - "added": status.Added, - "deleted": status.Deleted, - "untracked": status.Untracked, - "renamed": status.Renamed, - "branch_additions": status.BranchAdditions, - "branch_deletions": status.BranchDeletions, - "is_submodule": status.IsSubmodule, + "branch": status.Branch, + "remote_branch": status.RemoteBranch, + "head_commit": status.HeadCommit, + "base_commit": status.BaseCommit, + "ahead": status.Ahead, + "behind": status.Behind, + "remote_ahead": status.RemoteAhead, + "remote_behind": status.RemoteBehind, + "remote_head_commit": status.RemoteHeadCommit, + "files": status.Files, + "modified": status.Modified, + "added": status.Added, + "deleted": status.Deleted, + "untracked": status.Untracked, + "renamed": status.Renamed, + "branch_additions": status.BranchAdditions, + "branch_deletions": status.BranchDeletions, + "is_submodule": status.IsSubmodule, } if repositoryName != "" { statusPayload["repository_name"] = repositoryName diff --git a/apps/backend/internal/backendapp/helpers_test.go b/apps/backend/internal/backendapp/helpers_test.go index 67c34d71ec..1cd7988bcc 100644 --- a/apps/backend/internal/backendapp/helpers_test.go +++ b/apps/backend/internal/backendapp/helpers_test.go @@ -13,6 +13,7 @@ import ( "github.com/gin-gonic/gin" "github.com/jmoiron/sqlx" "github.com/kandev/kandev/internal/agent/executor" + client "github.com/kandev/kandev/internal/agent/runtime/agentctl" "github.com/kandev/kandev/internal/agent/runtime/lifecycle" "github.com/kandev/kandev/internal/agentctl/server/process" "github.com/kandev/kandev/internal/agentctl/types/streams" @@ -114,6 +115,39 @@ func decodePayload(t *testing.T, raw json.RawMessage) map[string]interface{} { return payload } +func TestBuildGitStatusNotificationIncludesAncestryEvidence(t *testing.T) { + msg := buildGitStatusNotification("session-1", "web", client.GitStatusResult{ + Branch: "feature/rewrite", + RemoteBranch: "origin/feature/rewrite", + HeadCommit: "local-head", + BaseCommit: "base-head", + Ahead: 5, + Behind: 1, + RemoteAhead: 2, + RemoteBehind: 3, + RemoteHeadCommit: "remote-head", + }) + if msg == nil { + t.Fatal("buildGitStatusNotification returned nil") + } + payload := decodePayload(t, msg.Payload) + status, ok := payload["status"].(map[string]interface{}) + if !ok { + t.Fatalf("status payload = %#v, want an object", payload["status"]) + } + for key, want := range map[string]interface{}{ + "head_commit": "local-head", + "base_commit": "base-head", + "remote_ahead": float64(2), + "remote_behind": float64(3), + "remote_head_commit": "remote-head", + } { + if got := status[key]; got != want { + t.Errorf("status[%q] = %#v, want %#v", key, got, want) + } + } +} + // TestAppendSessionStateMessage_IncludesTaskEnvironmentID asserts the snapshot // the WS hub sends on `session.subscribe` carries `task_environment_id`. // diff --git a/apps/backend/internal/github/handlers.go b/apps/backend/internal/github/handlers.go index 7331af0205..d9765aa7da 100644 --- a/apps/backend/internal/github/handlers.go +++ b/apps/backend/internal/github/handlers.go @@ -461,7 +461,7 @@ func wsGetPRCommits(svc *Service, _ *logger.Logger) func(ctx context.Context, ms if err != nil { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, err.Error(), nil) } - return ws.NewResponse(msg.ID, msg.Action, map[string]interface{}{"commits": commits}) + return ws.NewResponse(msg.ID, msg.Action, commits) } } diff --git a/apps/backend/internal/github/models.go b/apps/backend/internal/github/models.go index e4efdc4ed8..bd870372f4 100644 --- a/apps/backend/internal/github/models.go +++ b/apps/backend/internal/github/models.go @@ -830,6 +830,15 @@ type PRCommitInfo struct { StatsAvailable bool `json:"stats_available"` } +// PRCommitsResult contains the current provider head and the ancestry data +// used to classify a contribution checkout. Complete is true only when the +// provider client finished loading the full PR commit list. +type PRCommitsResult struct { + Commits []PRCommitInfo `json:"commits"` + HeadSHA string `json:"head_sha"` + Complete bool `json:"complete"` +} + // PRCommitDetail represents the metadata and changed files for one GitHub // commit. Unlike PRCommitInfo, its statistics come from GitHub's individual // commit endpoint and are always measured. diff --git a/apps/backend/internal/github/pat_client.go b/apps/backend/internal/github/pat_client.go index b6c0d2f599..cf1f937783 100644 --- a/apps/backend/internal/github/pat_client.go +++ b/apps/backend/internal/github/pat_client.go @@ -570,10 +570,16 @@ func (c *PATClient) ListPRFiles(ctx context.Context, owner, repo string, number } func (c *PATClient) ListPRCommits(ctx context.Context, owner, repo string, number int) ([]PRCommitInfo, error) { - var raw []ghPRCommit endpoint := fmt.Sprintf("/repos/%s/%s/pulls/%d/commits?per_page=100", owner, repo, number) - if err := c.get(ctx, endpoint, &raw); err != nil { - return nil, fmt.Errorf("list PR commits: %w", err) + var raw []ghPRCommit + for endpoint != "" { + var page []ghPRCommit + next, err := c.getPaginated(ctx, endpoint, &page) + if err != nil { + return nil, fmt.Errorf("list PR commits: %w", err) + } + raw = append(raw, page...) + endpoint = next } return convertRawPRCommits(raw), nil } diff --git a/apps/backend/internal/github/pat_client_writes_test.go b/apps/backend/internal/github/pat_client_writes_test.go index ba6bd2aa1a..a0f247eb8e 100644 --- a/apps/backend/internal/github/pat_client_writes_test.go +++ b/apps/backend/internal/github/pat_client_writes_test.go @@ -424,6 +424,32 @@ func TestPATClient_ListPRCommits(t *testing.T) { } } +func TestPATClient_ListPRCommits_FollowsLinkHeaderPages(t *testing.T) { + c, requests := newLinkPaginatedPATServer(t, "/repos/acme/widget/pulls/42/commits", []string{ + `[ + {"sha":"aaa","commit":{"message":"first","author":{"date":"2026-01-07T10:00:00Z"}}}, + {"sha":"bbb","commit":{"message":"second","author":{"date":"2026-01-08T10:00:00Z"}}} + ]`, + `[ + {"sha":"ccc","commit":{"message":"provider head","author":{"date":"2026-01-09T10:00:00Z"}}} + ]`, + }) + + commits, err := c.ListPRCommits(context.Background(), "acme", "widget", 42) + if err != nil { + t.Fatalf("ListPRCommits: %v", err) + } + if len(commits) != 3 { + t.Fatalf("commits = %d, want all pages (3)", len(commits)) + } + if commits[2].SHA != "ccc" { + t.Errorf("last commit = %q, want the second-page provider head", commits[2].SHA) + } + if len(*requests) != 2 { + t.Fatalf("requests = %d, want 2 pages", len(*requests)) + } +} + func TestPATClient_ListPRCommits_Error(t *testing.T) { c, _ := newRecordingPATServer(t, nil) if _, err := c.ListPRCommits(context.Background(), "acme", "widget", 42); err == nil || diff --git a/apps/backend/internal/github/service_pr.go b/apps/backend/internal/github/service_pr.go index f7d1ffa6e1..692528f942 100644 --- a/apps/backend/internal/github/service_pr.go +++ b/apps/backend/internal/github/service_pr.go @@ -546,17 +546,50 @@ func (s *Service) GetPRCommits(ctx context.Context, owner, repo string, number i return s.client.ListPRCommits(ctx, owner, repo, number) } +// GitHub's pull-request commit endpoint returns at most 250 commits, even +// when pagination has been requested. Treat a response at that limit as +// incomplete because the missing older ancestry can change a safe relation +// into an apparent divergence. +const githubPRCommitHistoryLimit = 250 + +func completePRCommitHistory(headSHA string, commits []PRCommitInfo) bool { + if headSHA == "" || len(commits) == 0 || len(commits) >= githubPRCommitHistoryLimit { + return false + } + for _, commit := range commits { + if commit.SHA == "" { + return false + } + } + return commits[len(commits)-1].SHA == headSHA +} + func (s *Service) GetPRCommitsForWorkspace( ctx context.Context, workspaceID, userID, owner, repo string, number int, -) ([]PRCommitInfo, error) { +) (PRCommitsResult, error) { if err := s.ensureRepositoryInWorkspaceScope(ctx, workspaceID, owner, repo); err != nil { - return nil, err + return PRCommitsResult{}, err } resolved, err := s.resolvePersonalReadClient(ctx, workspaceID, userID, owner, repo) if err != nil { - return nil, err + return PRCommitsResult{}, err } - return resolved.Client.ListPRCommits(ctx, owner, repo, number) + pr, err := resolved.Client.GetPR(ctx, owner, repo, number) + if err != nil { + return PRCommitsResult{}, fmt.Errorf("get PR #%d for commit history: %w", number, err) + } + if pr == nil { + return PRCommitsResult{}, fmt.Errorf("get PR #%d for commit history: empty response", number) + } + commits, err := resolved.Client.ListPRCommits(ctx, owner, repo, number) + if err != nil { + return PRCommitsResult{}, fmt.Errorf("list PR #%d commits: %w", number, err) + } + return PRCommitsResult{ + Commits: commits, + HeadSHA: pr.HeadSHA, + Complete: completePRCommitHistory(pr.HeadSHA, commits), + }, nil } // GetPRCommitDetailForWorkspace fetches one commit through the workspace's diff --git a/apps/backend/internal/github/service_pr_test.go b/apps/backend/internal/github/service_pr_test.go new file mode 100644 index 0000000000..994f6e1531 --- /dev/null +++ b/apps/backend/internal/github/service_pr_test.go @@ -0,0 +1,42 @@ +package github + +import ( + "fmt" + "testing" +) + +func TestCompletePRCommitHistoryRequiresACompleteProviderSnapshot(t *testing.T) { + complete := make([]PRCommitInfo, githubPRCommitHistoryLimit-1) + for index := range complete { + complete[index] = PRCommitInfo{SHA: fmt.Sprintf("commit-%d", index)} + } + complete[len(complete)-1] = PRCommitInfo{SHA: "provider-head"} + + truncated := make([]PRCommitInfo, githubPRCommitHistoryLimit) + for index := range truncated { + truncated[index] = PRCommitInfo{SHA: fmt.Sprintf("commit-%d", index)} + } + truncated[len(truncated)-1] = PRCommitInfo{SHA: "provider-head"} + + tests := []struct { + name string + head string + commits []PRCommitInfo + want bool + }{ + {name: "head terminates a paginated history", head: "provider-head", commits: complete, want: true}, + {name: "head is missing from the returned history", head: "provider-head", commits: []PRCommitInfo{{SHA: "older"}}, want: false}, + {name: "head does not terminate the returned history", head: "provider-head", commits: []PRCommitInfo{{SHA: "provider-head"}, {SHA: "newer"}}, want: false}, + {name: "empty history is incomplete", head: "provider-head", commits: nil, want: false}, + {name: "provider head is empty", head: "", commits: complete, want: false}, + {name: "provider endpoint cap is incomplete", head: "provider-head", commits: truncated, want: false}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if got := completePRCommitHistory(test.head, test.commits); got != test.want { + t.Fatalf("completePRCommitHistory() = %v, want %v", got, test.want) + } + }) + } +} diff --git a/apps/web/components/task/changes-panel-body.tsx b/apps/web/components/task/changes-panel-body.tsx index 8b1fc5eccc..d97ecd3acc 100644 --- a/apps/web/components/task/changes-panel-body.tsx +++ b/apps/web/components/task/changes-panel-body.tsx @@ -8,7 +8,11 @@ import { ReviewProgressBar, PRFilesSection, } from "./changes-panel-timeline"; -import { mergeCommits, firstVisibleSection } from "./changes-panel-helpers"; +import { + firstVisibleSection, + mergeCommits, + separateCommitHistories, +} from "./changes-panel-helpers"; import type { ChangesPanelBodyProps } from "./changes-panel-data"; import { useTranslation } from "react-i18next"; @@ -51,6 +55,11 @@ type TimelineProps = Pick< | "hasStaged" | "hasCommits" | "hasPRFiles" + | "relation" + | "providerCommitsLoading" + | "providerCommitsError" + | "pushDisabled" + | "pullDisabled" | "canPush" | "canCreatePR" | "existingPrUrl" @@ -165,18 +174,108 @@ function WorkingTreeSections(props: WorkingTreeProps) { ); } -function ChangesPanelTimeline(props: TimelineProps) { +function CommitHistorySections({ + props, + isDiverged, + defaultCollapsed, + mergedCommits, + separated, +}: { + props: TimelineProps; + isDiverged: boolean; + defaultCollapsed: boolean; + mergedCommits: ReturnType; + separated: ReturnType; +}) { const { t } = useTranslation(); - if (!props.hasAnything) { + if (isDiverged) { return ( -
- {t("task:yourChangedFilesWillAppearHere")} -
+ <> + {separated.providerCommits.length > 0 && ( + + )} + {separated.localCommits.length > 0 && ( + + )} + ); } - const mergedCommits = mergeCommits(props.commits, props.prCommits); - const hasMergedCommits = mergedCommits.length > 0; + return ( + + ); +} + +function EmptyChangesPanel({ providerCommitsError }: { providerCommitsError: string | null }) { + const { t } = useTranslation(); + if (providerCommitsError) { + return ( +
+ {t("task:providerHistoryUnavailable")} +
+ ); + } + return ( +
+ {t("task:yourChangedFilesWillAppearHere")} +
+ ); +} + +function ChangesPanelTimeline(props: TimelineProps) { + const { t } = useTranslation(); + if (!props.hasAnything) { + return ; + } + + const isDiverged = props.relation.presentation === "separate"; + const separated = isDiverged + ? separateCommitHistories(props.commits, props.prCommits) + : { providerCommits: [], localCommits: [] }; + const mergedCommits = isDiverged ? [] : mergeCommits(props.commits, props.prCommits); + const hasMergedCommits = isDiverged + ? separated.providerCommits.length > 0 || separated.localCommits.length > 0 + : mergedCommits.length > 0; const hasLocalChanges = props.hasUnstaged || props.hasStaged; const showCommitsList = props.hasStaged || hasMergedCommits; // Auto-expand the first (topmost) visible section so the panel never opens @@ -216,19 +315,32 @@ function ChangesPanelTimeline(props: TimelineProps) { )} + {props.providerCommitsError && ( +
+ {t("task:providerHistoryUnavailable")} +
+ )} + + {isDiverged && ( +
+ {t("task:remoteContributionHistoryChanged")} +
+ )} + {showCommitsList && ( - )} diff --git a/apps/web/components/task/changes-panel-data.tsx b/apps/web/components/task/changes-panel-data.tsx index e1e24f7d95..cbb8a124c3 100644 --- a/apps/web/components/task/changes-panel-data.tsx +++ b/apps/web/components/task/changes-panel-data.tsx @@ -11,12 +11,14 @@ import type { PRChangedFile } from "./changes-panel-timeline"; import { useChangesGitHandlers, useChangesDialogHandlers } from "./changes-panel-hooks"; import { useRepoDisplayName } from "@/hooks/domains/session/use-repo-display-name"; import { useBaseBranchByRepo } from "@/hooks/domains/session/use-base-branch-by-repo"; -import { useReviewPRSelection } from "@/hooks/domains/github/use-review-pr-selection"; +import { + useRemoteContributionRelation, + type RemoteContributionRelationState, +} from "@/hooks/domains/session/use-remote-contribution-relation"; import { prFetchKey, useActiveTaskPRsWithFiles, } from "@/hooks/domains/github/use-active-task-pr-files"; -import { usePRCommits } from "@/hooks/domains/github/use-pr-commits"; import { usePRReviewRepositoryIdentity } from "@/hooks/domains/github/use-pr-review-repository-identity"; import { getCumulativeReviewRepositoryNames, @@ -41,6 +43,8 @@ import { import type { CommitDetailTarget, OpenDiffOptions } from "./changes-diff-target"; import type { PRDiffFile, TaskPR } from "@/lib/types/github"; import { getGitCredentialDisplay } from "./changes-git-credential-display"; +import type { RemoteContributionRelation } from "@/hooks/domains/session/remote-contribution-relation"; +import { remoteContributionActionPolicy } from "@/hooks/domains/session/remote-contribution-relation"; function useChangesPanelStoreData() { const activeTaskId = useAppStore((state) => state.tasks.activeTaskId); @@ -90,6 +94,11 @@ export type ChangesPanelBodyProps = { hasCommits: boolean; hasPRFiles: boolean; hasPRCommits: boolean; + relation: RemoteContributionRelation; + providerCommitsLoading: boolean; + providerCommitsError: string | null; + pushDisabled: boolean; + pullDisabled: boolean; canPush: boolean; canCreatePR: boolean; existingPrUrl: string | undefined; @@ -135,7 +144,12 @@ export type ChangesPanelBodyProps = { onRepoPush?: (repo: string) => void; onRepoCreatePR?: (repo: string) => void; repoDisplayName?: (repositoryName: string) => string | undefined; - perRepoStatus?: Array<{ repository_name: string; ahead: number }>; + perRepoStatus?: Array<{ + repository_name: string; + ahead: number; + pushAhead: number; + pullBehind: number; + }>; prByRepo?: Record; }; @@ -251,7 +265,8 @@ function useChangesPanelPRData(repositoryNames: string[], sessionId: string | nu const { prs, filesByPRKey } = useActiveTaskPRsWithFiles(); const activeTaskId = useAppStore((state) => state.tasks.activeTaskId); const workspaceId = useAppStore((state) => state.workspaces.activeId); - const { selectedPR: taskPR } = useReviewPRSelection(activeTaskId); + const relationState: RemoteContributionRelationState = useRemoteContributionRelation(sessionId); + const taskPR = relationState.selectedPR; const reposByWorkspace = useAppStore((s) => s.repositories.itemsByWorkspaceId); const repoNameById = useMemo(() => buildRepoNameById(reposByWorkspace), [reposByWorkspace]); const taskRepositoryCount = useAppStore((s) => { @@ -263,13 +278,7 @@ function useChangesPanelPRData(repositoryNames: string[], sessionId: string | nu const taskHasMultipleRepos = taskRepositoryCount > 1; const useRepositoryKeys = isReviewMultiRepo(taskRepositoryCount, repositoryNames); const selectedPRRepositoryName = usePRReviewRepositoryIdentity(activeTaskId, sessionId, taskPR); - const refreshKey = taskPR?.last_synced_at ?? null; - const { commits: prCommitsList } = usePRCommits( - taskPR?.owner ?? null, - taskPR?.repo ?? null, - taskPR?.pr_number ?? null, - refreshKey, - ); + const prCommitsList = relationState.commits; const prCommits = useMemo(() => { if (!workspaceId?.trim() || !taskPR?.owner?.trim() || !taskPR.repo?.trim()) return []; return prCommitsList.map((commit) => ({ @@ -310,6 +319,9 @@ function useChangesPanelPRData(repositoryNames: string[], sessionId: string | nu hasPRCommits, prFiles, useRepositoryKeys, + relation: relationState.relation, + providerCommitsLoading: relationState.loading, + providerCommitsError: relationState.error, }; } @@ -329,6 +341,10 @@ export function useChangesPanelData() { [git.repoNames, git.cumulativeDiff], ); const prData = useChangesPanelPRData(reviewRepositoryNames, activeSessionId); + const remoteActionPolicy = useMemo( + () => remoteContributionActionPolicy(prData.relation), + [prData.relation], + ); const vcsDialogs = useVcsDialogs(); const baseBranchDisplay = useMemo(() => getBaseBranchDisplay(baseBranch), [baseBranch]); const unstagedFiles = useMemo(() => mapToChangedFiles(git.unstagedFiles), [git.unstagedFiles]); @@ -387,6 +403,8 @@ export function useChangesPanelData() { existingPrUrl, gitCredentialDisplay, walkthroughRequestReady, + pushDisabled: remoteActionPolicy.pushDisabled, + pullDisabled: remoteActionPolicy.pullDisabled, ...prData, }; } @@ -410,6 +428,11 @@ export function buildChangesPanelBodyProps( hasCommits: git.hasCommits, hasPRFiles: data.hasPRFiles, hasPRCommits: data.hasPRCommits, + relation: data.relation, + providerCommitsLoading: data.providerCommitsLoading, + providerCommitsError: data.providerCommitsError, + pushDisabled: data.pushDisabled, + pullDisabled: data.pullDisabled, canPush: git.canPush, canCreatePR: git.canCreatePR, existingPrUrl: data.existingPrUrl, diff --git a/apps/web/components/task/changes-panel-header.test.tsx b/apps/web/components/task/changes-panel-header.test.tsx new file mode 100644 index 0000000000..6e8183f067 --- /dev/null +++ b/apps/web/components/task/changes-panel-header.test.tsx @@ -0,0 +1,54 @@ +import { cleanup, render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { PullDropdown } from "./changes-panel-header"; + +vi.mock("@kandev/ui/button", () => ({ + Button: ({ children, ...props }: React.ButtonHTMLAttributes) => ( + + ), +})); + +vi.mock("@kandev/ui/dropdown-menu", () => ({ + DropdownMenu: ({ children }: { children: React.ReactNode }) =>
{children}
, + DropdownMenuContent: ({ children }: { children: React.ReactNode }) =>
{children}
, + DropdownMenuTrigger: ({ children }: { children: React.ReactNode }) => <>{children}, +})); + +vi.mock("@kandev/ui/tooltip", () => ({ + Tooltip: ({ children }: { children: React.ReactNode }) =>
{children}
, + TooltipContent: ({ children }: { children: React.ReactNode }) =>
{children}
, + TooltipTrigger: ({ children }: { children: React.ReactNode }) => <>{children}, +})); + +vi.mock("./changes-panel-per-repo-menu", () => ({ + PerRepoPullMenu: () => null, +})); + +afterEach(cleanup); + +describe("PullDropdown remote safety", () => { + it("keeps the history-change reason reachable while Pull is disabled", () => { + render( + , + ); + + const pullButton = screen.getByRole("button", { name: /Pull/ }); + expect(pullButton).toHaveProperty("disabled", true); + expect(pullButton.parentElement?.getAttribute("tabindex")).toBe("0"); + expect( + screen.getByText( + "Push, force-push, and Pull are disabled until the histories are reconciled.", + ), + ).toBeTruthy(); + }); +}); diff --git a/apps/web/components/task/changes-panel-header.tsx b/apps/web/components/task/changes-panel-header.tsx index c9f2182ded..601110dde9 100644 --- a/apps/web/components/task/changes-panel-header.tsx +++ b/apps/web/components/task/changes-panel-header.tsx @@ -39,6 +39,7 @@ export type PerRepoStatus = { branch: string | null; ahead: number; behind: number; + pullBehind?: number; hasStaged: boolean; hasUnstaged: boolean; }; @@ -335,8 +336,9 @@ function PullTriggerContent({ ); } -function PullDropdown({ +export function PullDropdown({ behindCount, + pullDisabled, isLoading, loadingOperation, repoNames, @@ -347,6 +349,7 @@ function PullDropdown({ repoDisplayName, }: { behindCount: number; + pullDisabled?: boolean; isLoading: boolean; loadingOperation: string | null; /** Always non-empty (single-repo includes the empty-name entry). */ @@ -358,6 +361,7 @@ function PullDropdown({ /** Maps a repository_name to its display label. */ repoDisplayName?: (repositoryName: string) => string | undefined; }) { + const { t } = useTranslation(); const isPulling = loadingOperation === "pull"; const isRebasing = loadingOperation === "rebase"; // For single-repo (empty repo entry), the trigger label uses the global @@ -365,24 +369,37 @@ function PullDropdown({ // labels and the trigger summarises with the max. const triggerBehind = perRepoStatus.length > 0 - ? Math.max(behindCount, ...perRepoStatus.map((s) => s.behind)) + ? Math.max(behindCount, ...perRepoStatus.map((s) => s.pullBehind ?? 0)) : behindCount; + const pullButton = ( + + ); + if (pullDisabled) { + return ( + + + + {pullButton} + + + {t("task:remoteActionsDisabledHistoryChanged")} + + ); + } return ( - - - + {pullButton} @@ -484,6 +502,7 @@ export function ChangesPanelHeader({ baseBranchDisplay, baseBranchByRepo, behindCount, + pullDisabled, isLoading, loadingOperation, onOpenDiffAll, @@ -509,6 +528,7 @@ export function ChangesPanelHeader({ * back to baseBranchDisplay. Empty/missing for single-repo workspaces. */ baseBranchByRepo?: Record; behindCount: number; + pullDisabled?: boolean; isLoading: boolean; loadingOperation: string | null; onOpenDiffAll?: () => void; @@ -562,6 +582,7 @@ export function ChangesPanelHeader({ )} { + const mapped = mapCurrentPRCommit(commit); + return mapped ? [mapped] : []; + }), + localCommits: localCommits.map(mapLocalCheckoutCommit), + }; +} + export function getBaseBranchDisplay(baseBranch: string | undefined): string { return baseBranch ? baseBranch.replace(/^origin\//, "") : "main"; } diff --git a/apps/web/components/task/changes-panel-per-repo-menu.tsx b/apps/web/components/task/changes-panel-per-repo-menu.tsx index b82b149fa5..c59b8c1aa6 100644 --- a/apps/web/components/task/changes-panel-per-repo-menu.tsx +++ b/apps/web/components/task/changes-panel-per-repo-menu.tsx @@ -15,6 +15,7 @@ export function PerRepoPullMenu({ onRepoPull, onRepoRebase, onRepoMerge, + pullDisabled, repoDisplayName, }: { repoNames: string[]; @@ -22,6 +23,7 @@ export function PerRepoPullMenu({ onRepoPull: (repo: string) => void; onRepoRebase: (repo: string) => void; onRepoMerge: (repo: string) => void; + pullDisabled?: boolean; repoDisplayName?: (repositoryName: string) => string | undefined; }) { const { t } = useTranslation(); @@ -30,7 +32,7 @@ export function PerRepoPullMenu({ <> {repoNames.map((repo, idx) => { const s = statusByName.get(repo); - const behind = s?.behind ?? 0; + const behind = s?.pullBehind ?? 0; const label = repoDisplayName?.(repo) || repo || t("task:repository2"); return (
@@ -46,6 +48,8 @@ export function PerRepoPullMenu({ onRepoPull(repo)} className="cursor-pointer text-xs gap-2" + disabled={pullDisabled} + title={pullDisabled ? t("task:remoteActionsDisabledHistoryChanged") : undefined} > {t("task:pull")} diff --git a/apps/web/components/task/changes-panel-remote.test.ts b/apps/web/components/task/changes-panel-remote.test.ts index ce9f9dd4b6..2f0f98ce94 100644 --- a/apps/web/components/task/changes-panel-remote.test.ts +++ b/apps/web/components/task/changes-panel-remote.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { mergeCommits } from "./changes-panel"; +import { mergeCommits, separateCommitHistories } from "./changes-panel"; const PR_DATE = "2026-03-29T00:00:00Z"; const SHARED_SHA = "ccc3333"; @@ -120,6 +120,43 @@ describe("mergeCommits source provenance", () => { }); }); +describe("separateCommitHistories", () => { + it("preserves rewritten provider and checkout rows without deduplication", () => { + const local = [makeLocal("old1111", "same message"), makeLocal("old2222", "local work")]; + const provider = [ + { + ...makePR("new1111", "same message"), + stats_available: false, + workspace_id: WORKSPACE_ID, + owner: "acme", + repo: WIDGET_REPO, + }, + { + ...makePR("new3333", "provider work"), + stats_available: false, + workspace_id: WORKSPACE_ID, + owner: "acme", + repo: WIDGET_REPO, + }, + ]; + + const result = separateCommitHistories(local, provider); + + expect(result.providerCommits.map((commit) => commit.commit_sha)).toEqual([ + "new1111", + "new3333", + ]); + expect(result.localCommits.map((commit) => commit.commit_sha)).toEqual(["old1111", "old2222"]); + expect(result.providerCommits.every((commit) => commit.presentation === "current_pr")).toBe( + true, + ); + expect(result.localCommits.every((commit) => commit.presentation === "local_checkout")).toBe( + true, + ); + expect(result.localCommits.every((commit) => commit.pushed === false)).toBe(true); + }); +}); + describe("mergeCommits repository identity", () => { it("does not create a PR-only target without complete provenance", () => { const pr = { diff --git a/apps/web/components/task/changes-panel-timeline.tsx b/apps/web/components/task/changes-panel-timeline.tsx index 7803dd8372..ca69a95c2d 100644 --- a/apps/web/components/task/changes-panel-timeline.tsx +++ b/apps/web/components/task/changes-panel-timeline.tsx @@ -145,12 +145,16 @@ type CommitsSectionProps = { repoDisplayName?: (repositoryName: string) => string | undefined; /** The session's base branch — passed through to the per-repo PR dialog. */ repoBaseBranch?: string; - /** Per-repo branch / ahead / behind summary; used for the "ahead" indicator. */ - perRepoStatus?: Array<{ repository_name: string; ahead: number }>; + /** Per-repo branch / upstream summary; used for the Push indicator. */ + perRepoStatus?: Array<{ repository_name: string; pushAhead?: number; ahead?: number }>; /** Existing PR URL keyed by repository_name; "" key for single-repo. */ prByRepo?: Record; /** Initial collapse state. Defaults to collapsed; the panel expands it when it is the first visible section. */ defaultCollapsed?: boolean; + label?: string; + testId?: string; + pushDisabled?: boolean; + showActions?: boolean; }; // Commits grouping shares the helper above with files — see @/lib/group-by-repo. @@ -168,10 +172,16 @@ export function CommitsSection({ perRepoStatus, prByRepo, defaultCollapsed = true, + label, + testId = "commits-section", + pushDisabled = false, + showActions = true, }: CommitsSectionProps) { const { t } = useTranslation(); const groups = groupByRepositoryName(commits, (c) => c.repository_name); - const aheadByRepo = new Map((perRepoStatus ?? []).map((s) => [s.repository_name, s.ahead])); + const aheadByRepo = new Map( + (perRepoStatus ?? []).map((s) => [s.repository_name, s.pushAhead ?? s.ahead ?? 0]), + ); // Single-repo: drop the per-repo sub-header (CommitsRepoGroup with // showHeader=false renders flat) and lift the Push / PR buttons into the // section header. @@ -182,8 +192,8 @@ export function CommitsSection({ aheadCount={aheadByRepo.get("") ?? 0} prExists={!!prByRepo?.[""]} canCreatePR={!!onRepoCreatePR && !prByRepo?.[""]} - onRepoPush={onRepoPush} - onRepoCreatePR={onRepoCreatePR} + onRepoPush={showActions && !pushDisabled ? onRepoPush : undefined} + onRepoCreatePR={showActions ? onRepoCreatePR : undefined} stop={(e) => e.stopPropagation()} /> ) : undefined; @@ -191,10 +201,10 @@ export function CommitsSection({ return (
    @@ -212,8 +222,8 @@ export function CommitsSection({ onAmendCommit={onAmendCommit} onRevertCommit={onRevertCommit} onResetToCommit={onResetToCommit} - onRepoPush={onRepoPush} - onRepoCreatePR={onRepoCreatePR} + onRepoPush={showActions && !pushDisabled ? onRepoPush : undefined} + onRepoCreatePR={showActions ? onRepoCreatePR : undefined} /> ))}
diff --git a/apps/web/components/task/changes-panel.tsx b/apps/web/components/task/changes-panel.tsx index d2b4162f58..829a85d4fe 100644 --- a/apps/web/components/task/changes-panel.tsx +++ b/apps/web/components/task/changes-panel.tsx @@ -4,13 +4,17 @@ import { memo } from "react"; import { PanelRoot } from "./panel-primitives"; import { useIsTaskArchived, ArchivedPanelPlaceholder } from "./task-archived-context"; import { ChangesPanelHeader } from "./changes-panel-header"; -import { filterUnpushedCommits, mergeCommits } from "./changes-panel-helpers"; +import { + filterUnpushedCommits, + mergeCommits, + separateCommitHistories, +} from "./changes-panel-helpers"; import { useChangesPanelData, buildChangesPanelBodyProps } from "./changes-panel-data"; import { ChangesPanelBody } from "./changes-panel-body"; import type { CommitDetailTarget, OpenDiffOptions } from "./changes-diff-target"; import { useRequestChangesWalkthrough } from "@/hooks/domains/session/use-request-changes-walkthrough"; -export { filterUnpushedCommits, mergeCommits }; +export { filterUnpushedCommits, mergeCommits, separateCommitHistories }; type ChangesPanelProps = { onOpenDiffFile: (path: string, options?: OpenDiffOptions) => void; @@ -38,7 +42,8 @@ const ChangesPanel = memo(function ChangesPanel(props: ChangesPanelProps) { displayBranch={data.git.branch} baseBranchDisplay={data.baseBranchDisplay} baseBranchByRepo={data.baseBranchByRepo} - behindCount={data.git.behind} + behindCount={data.git.pullBehind} + pullDisabled={data.pullDisabled} isLoading={data.git.isLoading} loadingOperation={data.git.loadingOperation} onOpenDiffAll={props.onOpenDiffAll} diff --git a/apps/web/components/task/commit-row.test.tsx b/apps/web/components/task/commit-row.test.tsx index ad045a954c..1c9a3b2e2d 100644 --- a/apps/web/components/task/commit-row.test.tsx +++ b/apps/web/components/task/commit-row.test.tsx @@ -73,4 +73,21 @@ describe("CommitRow", () => { fireEvent.click(screen.getByTestId("commit-row-remote1")); expect(onOpenCommitDetail).toHaveBeenCalledWith(commit.detailTarget); }); + + it("announces diverged row provenance without showing an unpushed marker", () => { + const localCheckoutCommit: CommitItem = { + ...remoteCommit(), + commit_sha: "local123456", + commit_message: "Superseded checkout commit", + pushed: false, + presentation: "local_checkout", + detailTarget: { source: "local", sha: "local123456" }, + }; + + render(); + + expect(screen.getByTitle("Local checkout commit")).toBeTruthy(); + expect(screen.getByText("Local checkout commit")).toBeTruthy(); + expect(screen.queryByTitle("Local commit (not yet pushed)")).toBeNull(); + }); }); diff --git a/apps/web/components/task/commit-row.tsx b/apps/web/components/task/commit-row.tsx index dbed881d75..63f20c2332 100644 --- a/apps/web/components/task/commit-row.tsx +++ b/apps/web/components/task/commit-row.tsx @@ -19,6 +19,8 @@ import { timeAgo } from "@/lib/utils/time"; import type { CommitDetailTarget } from "./changes-diff-target"; import { useTranslation } from "react-i18next"; +export type CommitPresentation = "current_pr" | "local_checkout"; + export type CommitItem = { commit_sha: string; commit_message: string; @@ -30,6 +32,8 @@ export type CommitItem = { /** Multi-repo: name of the repo this commit was made in. Empty for single-repo. */ repository_name?: string; committed_at?: string; + /** Explicit provenance used when provider and checkout histories diverge. */ + presentation?: CommitPresentation; }; /** Context menu for commit items */ @@ -170,6 +174,41 @@ function CommitRowActions({ ); } +function CommitStatusMarker({ commit }: { commit: CommitItem }) { + const { t } = useTranslation(); + let label: string; + let marker = - + {t("task:pull")} - + void; @@ -239,6 +246,7 @@ function MobileTopBarActions({ onMenuClick, }: MobileTopBarActionsProps) { const { t } = useTranslation(); + const remoteActionPolicy = useMobileRemoteActionPolicy(sessionId); return (
@@ -275,6 +283,8 @@ function MobileTopBarActions({ onPush={onPush} onRebase={onRebase} onMerge={onMerge} + pushDisabled={remoteActionPolicy.pushDisabled} + pullDisabled={remoteActionPolicy.pullDisabled} />