From 5c6434d7d815459f4abc43d9ea22df7f8f45797e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Mon, 14 Sep 2026 09:28:23 +0100 Subject: [PATCH] feat: explain divergent contribution histories (#3627) --- .../handlers/git_contribution_history_test.go | 63 +++ .../internal/agent/handlers/git_handlers.go | 42 ++ .../agent/handlers/git_handlers_test.go | 1 + .../internal/agent/runtime/agentctl/git.go | 61 +++ .../agentctl/git_contribution_history_test.go | 55 +++ .../internal/agentctl/server/api/git.go | 46 ++ .../api/git_contribution_history_test.go | 59 +++ .../agentctl/server/api/git_handlers_test.go | 2 + .../internal/agentctl/server/api/server.go | 1 + .../internal/agentctl/server/process/git.go | 9 +- .../process/git_contribution_history.go | 459 ++++++++++++++++++ .../git_contribution_history_edge_test.go | 384 +++++++++++++++ .../process/git_contribution_history_test.go | 170 +++++++ .../agentctl/server/process/testmain_test.go | 33 ++ apps/backend/internal/backendapp/e2e_reset.go | 84 ++++ .../internal/backendapp/e2e_reset_test.go | 35 ++ .../internal/common/securityutil/git.go | 1 + apps/backend/pkg/websocket/actions.go | 31 +- .../components/task/changes-panel-body.tsx | 7 + .../task/changes-panel-branch-rows.ts | 32 ++ .../components/task/changes-panel-data.tsx | 12 +- .../components/task/changes-panel-header.tsx | 38 +- .../changes-panel-timeline-grouping.test.tsx | 20 + .../task/changes-panel-timeline.tsx | 30 +- apps/web/components/task/changes-panel.tsx | 37 +- .../task/mobile/mobile-changes-panel.test.tsx | 142 ++++++ .../task/mobile/mobile-changes-panel.tsx | 85 +++- .../task/remote-contribution-action-items.tsx | 217 +++++++-- .../task/remote-contribution-comparison.ts | 41 ++ ...emote-contribution-header-actions.test.tsx | 73 ++- .../remote-contribution-header-actions.tsx | 456 ++++++++++++++--- ...te-contribution-resolution-dialog.test.tsx | 8 +- .../remote-contribution-resolution-dialog.tsx | 6 +- .../use-remote-contribution-resolution.ts | 9 +- apps/web/components/vcs-multi-repo-menu.tsx | 7 + .../components/vcs-split-button-dropdown.tsx | 26 + .../web/components/vcs-split-button-parts.tsx | 64 +-- apps/web/components/vcs-split-button.test.ts | 70 ++- apps/web/components/vcs-split-button.tsx | 74 ++- apps/web/e2e/helpers/api-client.ts | 83 +++- ...-session-transcript-reconciliation.spec.ts | 5 +- .../e2e/tests/chat/mobile-busy-signal.spec.ts | 6 +- .../e2e/tests/git/git-changes-panel.spec.ts | 444 ++++++++++++++++- .../git/mobile-pr-checkout-drift.spec.ts | 410 +++++++++++++++- ...-contribution-history-explanation.test.tsx | 241 +++++++++ .../use-contribution-history-explanation.ts | 255 ++++++++++ .../use-remote-contribution-relation.test.tsx | 31 +- .../use-remote-contribution-relation.ts | 55 ++- .../session/use-session-messages.test.ts | 85 ++++ .../domains/session/use-session-messages.ts | 29 +- apps/web/src/locales/en/task.json | 22 +- apps/web/src/locales/pseudo/task.json | 22 +- apps/web/src/locales/pt-pt/task.json | 22 +- apps/web/src/locales/zh-cn/task.json | 22 +- apps/web/src/locales/zh-hk/task.json | 22 +- apps/web/src/locales/zh-tw/task.json | 22 +- .../plans/branch-history-explanations/plan.md | 216 +++++++++ .../task-01-observe-local-rebase.md | 93 ++++ .../task-02-present-history-choices.md | 181 +++++++ .../task-03-prove-history-flows.md | 181 +++++++ docs/public/git-operations.md | 16 + docs/specs/tasks/README.md | 1 + .../requirements/remote-contribution-tasks.md | 40 +- .../branch-history-explanations.md | 195 ++++++++ .../remote-contribution-tasks.md | 43 +- 65 files changed, 5376 insertions(+), 356 deletions(-) create mode 100644 apps/backend/internal/agent/handlers/git_contribution_history_test.go create mode 100644 apps/backend/internal/agent/runtime/agentctl/git_contribution_history_test.go create mode 100644 apps/backend/internal/agentctl/server/api/git_contribution_history_test.go create mode 100644 apps/backend/internal/agentctl/server/process/git_contribution_history.go create mode 100644 apps/backend/internal/agentctl/server/process/git_contribution_history_edge_test.go create mode 100644 apps/backend/internal/agentctl/server/process/git_contribution_history_test.go create mode 100644 apps/web/components/task/changes-panel-branch-rows.ts create mode 100644 apps/web/components/task/mobile/mobile-changes-panel.test.tsx create mode 100644 apps/web/components/task/remote-contribution-comparison.ts create mode 100644 apps/web/hooks/domains/session/use-contribution-history-explanation.test.tsx create mode 100644 apps/web/hooks/domains/session/use-contribution-history-explanation.ts create mode 100644 docs/plans/branch-history-explanations/plan.md create mode 100644 docs/plans/branch-history-explanations/task-01-observe-local-rebase.md create mode 100644 docs/plans/branch-history-explanations/task-02-present-history-choices.md create mode 100644 docs/plans/branch-history-explanations/task-03-prove-history-flows.md create mode 100644 docs/specs/tasks/system-design/branch-history-explanations.md diff --git a/apps/backend/internal/agent/handlers/git_contribution_history_test.go b/apps/backend/internal/agent/handlers/git_contribution_history_test.go new file mode 100644 index 0000000000..78541c872f --- /dev/null +++ b/apps/backend/internal/agent/handlers/git_contribution_history_test.go @@ -0,0 +1,63 @@ +package handlers + +import ( + "context" + "encoding/json" + "net/http" + "testing" + + ws "github.com/kandev/kandev/pkg/websocket" +) + +func TestContributionHistoryHandler(t *testing.T) { + h, server := gitHandlerServer(t, func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/git/contribution/history-explanation" { + t.Errorf("path = %s, want history explanation endpoint", r.URL.Path) + } + var body map[string]any + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Errorf("decode request: %v", err) + } + if body["branch"] != "feature/history" || body["repo"] != "svc" { + t.Errorf("body = %#v, want branch and repo", body) + } + _, _ = w.Write([]byte(`{ + "repo":"svc", + "branch":"feature/history", + "expected_local_head":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "expected_remote_head":"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + "kind":"unexplained", + "reason":"no_matching_reflog" + }`)) + }) + defer server.Close() + + msg, err := ws.NewRequest("id", ws.ActionWorktreeContributionHistoryExplanation, GitContributionHistoryExplanationRequest{ + SessionID: "s", + Repo: "svc", + Branch: "feature/history", + ExpectedLocalHead: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + ExpectedRemoteHead: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + }) + if err != nil { + t.Fatal(err) + } + response, err := h.wsContributionHistoryExplanation(context.Background(), msg) + if err != nil { + t.Fatalf("handler: %v", err) + } + if response == nil || !json.Valid(response.Payload) { + t.Fatalf("response = %#v, want JSON payload", response) + } +} + +func TestContributionHistoryHandlerRejectsMissingIdentity(t *testing.T) { + h := NewGitHandlers(nil, nil, newTestLogger()) + msg, err := ws.NewRequest("id", ws.ActionWorktreeContributionHistoryExplanation, GitContributionHistoryExplanationRequest{SessionID: "s"}) + if err != nil { + t.Fatal(err) + } + if _, err := h.wsContributionHistoryExplanation(context.Background(), msg); err == nil { + t.Fatal("expected missing identity validation error") + } +} diff --git a/apps/backend/internal/agent/handlers/git_handlers.go b/apps/backend/internal/agent/handlers/git_handlers.go index 0030e4daa5..0dff4eed94 100644 --- a/apps/backend/internal/agent/handlers/git_handlers.go +++ b/apps/backend/internal/agent/handlers/git_handlers.go @@ -153,6 +153,7 @@ func (h *GitHandlers) RegisterHandlers(d *ws.Dispatcher) { d.RegisterFunc(ws.ActionWorktreePush, h.wsPush) d.RegisterFunc(ws.ActionWorktreeReplaceContribution, h.wsReplaceContribution) d.RegisterFunc(ws.ActionWorktreeUseContribution, h.wsUseContribution) + d.RegisterFunc(ws.ActionWorktreeContributionHistoryExplanation, h.wsContributionHistoryExplanation) d.RegisterFunc(ws.ActionWorktreeRebase, h.wsRebase) d.RegisterFunc(ws.ActionWorktreeMerge, h.wsMerge) d.RegisterFunc(ws.ActionWorktreeAbort, h.wsAbort) @@ -198,6 +199,16 @@ type GitContributionRequest struct { Repo string `json:"repo,omitempty"` } +// GitContributionHistoryExplanationRequest carries the selected branch and +// both immutable heads for a read-only contribution history observation. +type GitContributionHistoryExplanationRequest struct { + SessionID string `json:"session_id"` + Repo string `json:"repo,omitempty"` + Branch string `json:"branch"` + ExpectedLocalHead string `json:"expected_local_head"` + ExpectedRemoteHead string `json:"expected_remote_head"` +} + // GitRebaseRequest for worktree.rebase action. // Repo is the multi-repo subpath (e.g. "kandev"); empty for single-repo workspaces. type GitRebaseRequest struct { @@ -367,6 +378,37 @@ func (h *GitHandlers) wsUseContribution(ctx context.Context, msg *ws.Message) (* }) } +func (h *GitHandlers) wsContributionHistoryExplanation(ctx context.Context, msg *ws.Message) (*ws.Message, error) { + var req GitContributionHistoryExplanationRequest + if err := msg.ParsePayload(&req); err != nil { + return nil, fmt.Errorf("invalid payload: %w", err) + } + if req.SessionID == "" { + return nil, fmt.Errorf("session_id is required") + } + for field, value := range map[string]string{ + "branch": req.Branch, + "expected_local_head": req.ExpectedLocalHead, + "expected_remote_head": req.ExpectedRemoteHead, + } { + if value == "" { + return nil, fmt.Errorf("%s is required", field) + } + } + + agentClient, releaseClient, err := h.getAgentCtlClient(ctx, req.SessionID) + defer releaseClient() + if err != nil { + return nil, err + } + result, err := agentClient.GitContributionHistoryExplanation( + ctx, req.Branch, req.ExpectedLocalHead, req.ExpectedRemoteHead, req.Repo) + if err != nil { + return nil, fmt.Errorf("contribution history explanation failed: %w", err) + } + return ws.NewResponse(msg.ID, msg.Action, result) +} + func (h *GitHandlers) wsContribution(ctx context.Context, msg *ws.Message, operation string, action func(*client.Client, string, string) (*client.GitOperationResult, error)) (*ws.Message, error) { var req GitContributionRequest if err := msg.ParsePayload(&req); err != nil { diff --git a/apps/backend/internal/agent/handlers/git_handlers_test.go b/apps/backend/internal/agent/handlers/git_handlers_test.go index d3249935bb..e78c6ac2a9 100644 --- a/apps/backend/internal/agent/handlers/git_handlers_test.go +++ b/apps/backend/internal/agent/handlers/git_handlers_test.go @@ -424,6 +424,7 @@ func TestRegisterGitHandlers(t *testing.T) { actions := []string{ ws.ActionWorktreePull, ws.ActionWorktreePush, + ws.ActionWorktreeContributionHistoryExplanation, ws.ActionWorktreeRebase, ws.ActionWorktreeMerge, ws.ActionWorktreeAbort, diff --git a/apps/backend/internal/agent/runtime/agentctl/git.go b/apps/backend/internal/agent/runtime/agentctl/git.go index 56b696382c..ffb5337c3f 100644 --- a/apps/backend/internal/agent/runtime/agentctl/git.go +++ b/apps/backend/internal/agent/runtime/agentctl/git.go @@ -47,6 +47,21 @@ type PushOptions struct { ExpectedBranch string } +// ContributionHistoryExplanationResult mirrors the bounded, read-only Git +// observation returned by the agentctl server. +type ContributionHistoryExplanationResult struct { + Repo string `json:"repo,omitempty"` + Branch string `json:"branch"` + ExpectedLocalHead string `json:"expected_local_head"` + ExpectedRemoteHead string `json:"expected_remote_head"` + Kind string `json:"kind"` + Reason string `json:"reason"` + OntoHead string `json:"onto_head,omitempty"` + TaskCommitCount *int `json:"task_commit_count,omitempty"` + PublishedCommitCount *int `json:"published_commit_count,omitempty"` + NewBaseCommitCount *int `json:"new_base_commit_count,omitempty"` +} + // PRCreateResult represents the result of a PR creation operation. // This matches the server-side process.PRCreateResult. type PRCreateResult struct { @@ -143,6 +158,52 @@ func (c *Client) GitUseRemoteContribution(ctx context.Context, expectedRemoteHea return c.gitOperation(ctx, "/api/v1/git/contribution/use", payload) } +// GitContributionHistoryExplanation observes the selected branch and heads +// without fetching, rewriting, or publishing any Git refs. +func (c *Client) GitContributionHistoryExplanation( + ctx context.Context, branch, expectedLocalHead, expectedRemoteHead, repo string, +) (*ContributionHistoryExplanationResult, error) { + payload := struct { + Branch string `json:"branch"` + ExpectedLocalHead string `json:"expected_local_head"` + ExpectedRemoteHead string `json:"expected_remote_head"` + Repo string `json:"repo,omitempty"` + }{ + Branch: branch, + ExpectedLocalHead: expectedLocalHead, + ExpectedRemoteHead: expectedRemoteHead, + Repo: repo, + } + body, err := json.Marshal(payload) + if err != nil { + return nil, fmt.Errorf("failed to marshal request: %w", err) + } + req, err := http.NewRequestWithContext(ctx, http.MethodPost, + c.baseURL+"/api/v1/git/contribution/history-explanation", bytes.NewReader(body)) + if err != nil { + return nil, fmt.Errorf("failed to create request: %w", err) + } + req.Header.Set("Content-Type", "application/json") + resp, err := c.httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("failed to execute request: %w", err) + } + defer func() { _ = resp.Body.Close() }() + responseBody, err := readResponseBody(resp) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + var result ContributionHistoryExplanationResult + if err := json.Unmarshal(responseBody, &result); err != nil { + return nil, fmt.Errorf("failed to parse response (status %d, body: %s): %w", + resp.StatusCode, truncateBody(responseBody), err) + } + if resp.StatusCode >= 400 { + return &result, fmt.Errorf("git contribution history explanation failed with status %d", resp.StatusCode) + } + return &result, nil +} + // GitRebase rebases the worktree branch onto the specified base branch. // It first fetches origin/, then rebases onto it. // repo is the multi-repo subpath (e.g. "kandev"); empty for single-repo workspaces. diff --git a/apps/backend/internal/agent/runtime/agentctl/git_contribution_history_test.go b/apps/backend/internal/agent/runtime/agentctl/git_contribution_history_test.go new file mode 100644 index 0000000000..4460b4fb51 --- /dev/null +++ b/apps/backend/internal/agent/runtime/agentctl/git_contribution_history_test.go @@ -0,0 +1,55 @@ +package client + +import ( + "context" + "encoding/json" + "net/http" + "testing" +) + +func TestContributionHistoryClient(t *testing.T) { + localHead := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + remoteHead := "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + srv, got := captureServer(t, jsonResponder(http.StatusOK, `{ + "repo":"svc", + "branch":"feature/history", + "expected_local_head":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "expected_remote_head":"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + "kind":"local_rebase", + "reason":"matched_reflog", + "onto_head":"cccccccccccccccccccccccccccccccccccccccc", + "task_commit_count":5, + "published_commit_count":5, + "new_base_commit_count":29 + }`)) + + result, err := newHTTPOnlyClient(srv.URL).GitContributionHistoryExplanation( + context.Background(), "feature/history", localHead, remoteHead, "svc") + if err != nil { + t.Fatalf("GitContributionHistoryExplanation: %v", err) + } + if got.Method != http.MethodPost || got.Path != "/api/v1/git/contribution/history-explanation" { + t.Fatalf("request = %s %s, want POST /api/v1/git/contribution/history-explanation", got.Method, got.Path) + } + if got.ContentType != "application/json" { + t.Fatalf("content-type = %q, want application/json", got.ContentType) + } + var body map[string]any + if err := json.Unmarshal(got.Body, &body); err != nil { + t.Fatalf("decode request body: %v", err) + } + for key, want := range map[string]any{ + "repo": "svc", + "branch": "feature/history", + "expected_local_head": localHead, + "expected_remote_head": remoteHead, + } { + if body[key] != want { + t.Errorf("body[%q] = %#v, want %#v", key, body[key], want) + } + } + if result.Kind != "local_rebase" || result.Reason != "matched_reflog" || + result.TaskCommitCount == nil || *result.TaskCommitCount != 5 { + t.Fatalf("result = %+v, want decoded explanation", result) + } +} diff --git a/apps/backend/internal/agentctl/server/api/git.go b/apps/backend/internal/agentctl/server/api/git.go index 3e4c0aabe8..ce3dcd02bc 100644 --- a/apps/backend/internal/agentctl/server/api/git.go +++ b/apps/backend/internal/agentctl/server/api/git.go @@ -80,6 +80,15 @@ type GitContributionRequest struct { Repo string `json:"repo,omitempty"` } +// GitContributionHistoryExplanationRequest describes the selected local and +// published heads for a read-only history observation. +type GitContributionHistoryExplanationRequest struct { + Branch string `json:"branch"` + ExpectedLocalHead string `json:"expected_local_head"` + ExpectedRemoteHead string `json:"expected_remote_head"` + Repo string `json:"repo,omitempty"` +} + // GitRebaseRequest for POST /api/v1/git/rebase type GitRebaseRequest struct { BaseBranch string `json:"base_branch"` @@ -278,6 +287,43 @@ func (s *Server) handleGitUseContribution(c *gin.Context) { }) } +func (s *Server) handleGitContributionHistoryExplanation(c *gin.Context) { + var req GitContributionHistoryExplanationRequest + if err := c.ShouldBindJSON(&req); err != nil { + c.JSON(http.StatusBadRequest, process.GitOperationResult{ + Success: false, Operation: "contribution_history_explanation", Error: "invalid request: " + err.Error(), + }) + return + } + for _, required := range []struct { + name string + value string + }{ + {name: "branch", value: req.Branch}, + {name: "expected_local_head", value: req.ExpectedLocalHead}, + {name: "expected_remote_head", value: req.ExpectedRemoteHead}, + } { + if required.value == "" { + c.JSON(http.StatusBadRequest, process.GitOperationResult{ + Success: false, Operation: "contribution_history_explanation", Error: required.name + " is required", + }) + return + } + } + + gitOp := s.gitOpForRepo(c, "contribution_history_explanation", req.Repo) + if gitOp == nil { + return + } + result, err := gitOp.ExplainContributionHistory( + c.Request.Context(), req.Branch, req.ExpectedLocalHead, req.ExpectedRemoteHead) + if err != nil { + s.handleGitError(c, "contribution_history_explanation", err) + return + } + c.JSON(http.StatusOK, result) +} + func (s *Server) handleGitContribution(c *gin.Context, operation string, action func(*process.GitOperator, string) (*process.GitOperationResult, error)) { var req GitContributionRequest if err := c.ShouldBindJSON(&req); err != nil { diff --git a/apps/backend/internal/agentctl/server/api/git_contribution_history_test.go b/apps/backend/internal/agentctl/server/api/git_contribution_history_test.go new file mode 100644 index 0000000000..05480d90c7 --- /dev/null +++ b/apps/backend/internal/agentctl/server/api/git_contribution_history_test.go @@ -0,0 +1,59 @@ +package api + +import ( + "encoding/json" + "net/http" + "strings" + "testing" + + "github.com/kandev/kandev/internal/agentctl/server/process" +) + +func TestContributionHistoryAPI(t *testing.T) { + fixture := newGitAPIFixture(t) + localHead := fixture.head(t) + remoteHead := strings.TrimSpace(runGitAPI(t, fixture.repo, "rev-parse", "origin/main")) + + rec := postGitAPI(t, fixture.server, "/api/v1/git/contribution/history-explanation", map[string]any{ + "branch": fixture.branch(t), + "expected_local_head": localHead, + "expected_remote_head": remoteHead, + }) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", rec.Code, rec.Body.String()) + } + var result process.ContributionHistoryExplanationResult + if err := json.Unmarshal(rec.Body.Bytes(), &result); err != nil { + t.Fatalf("decode response: %v", err) + } + if result.Branch != fixture.branch(t) || result.ExpectedLocalHead != localHead || + result.ExpectedRemoteHead != remoteHead { + t.Fatalf("result identity = %+v, want request identity", result) + } + if result.Kind != "unexplained" || result.Reason != "no_matching_reflog" { + t.Fatalf("result = %+v, want neutral no-matching-reflog response", result) + } +} + +func TestContributionHistoryAPIRejectsMissingFields(t *testing.T) { + fixture := newGitAPIFixture(t) + for _, field := range []string{"branch", "expected_local_head", "expected_remote_head"} { + t.Run(field, func(t *testing.T) { + body := map[string]any{ + "branch": fixture.branch(t), + "expected_local_head": localHeadForAPI(t, fixture), + "expected_remote_head": strings.TrimSpace(runGitAPI(t, fixture.repo, "rev-parse", "origin/main")), + } + delete(body, field) + rec := postGitAPI(t, fixture.server, "/api/v1/git/contribution/history-explanation", body) + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 (body %s)", rec.Code, rec.Body.String()) + } + }) + } +} + +func localHeadForAPI(t *testing.T, fixture *gitAPIFixture) string { + t.Helper() + return fixture.head(t) +} diff --git a/apps/backend/internal/agentctl/server/api/git_handlers_test.go b/apps/backend/internal/agentctl/server/api/git_handlers_test.go index eebdcce994..9dd5f08269 100644 --- a/apps/backend/internal/agentctl/server/api/git_handlers_test.go +++ b/apps/backend/internal/agentctl/server/api/git_handlers_test.go @@ -132,6 +132,7 @@ func TestGitHandlers_RejectMalformedBody(t *testing.T) { {"/api/v1/git/push-preflight", "push_preflight"}, {"/api/v1/git/contribution/replace", "replace_remote_contribution"}, {"/api/v1/git/contribution/use", "use_remote_contribution"}, + {"/api/v1/git/contribution/history-explanation", "contribution_history_explanation"}, {"/api/v1/git/rebase", "rebase"}, {"/api/v1/git/merge", "merge"}, {"/api/v1/git/abort", "abort"}, @@ -204,6 +205,7 @@ func TestGitHandlers_RejectMissingRequiredFields(t *testing.T) { {"reset without sha", "/api/v1/git/reset", GitResetRequest{}, "reset", "commit_sha is required"}, {"replace without expected head", "/api/v1/git/contribution/replace", GitContributionRequest{}, "replace_remote_contribution", "expected_remote_head is required"}, {"use without expected head", "/api/v1/git/contribution/use", GitContributionRequest{}, "use_remote_contribution", "expected_remote_head is required"}, + {"history explanation without branch", "/api/v1/git/contribution/history-explanation", GitContributionHistoryExplanationRequest{}, "contribution_history_explanation", "branch is required"}, { "reset with unknown mode", "/api/v1/git/reset", diff --git a/apps/backend/internal/agentctl/server/api/server.go b/apps/backend/internal/agentctl/server/api/server.go index fada65c7d5..c678790aae 100644 --- a/apps/backend/internal/agentctl/server/api/server.go +++ b/apps/backend/internal/agentctl/server/api/server.go @@ -215,6 +215,7 @@ func (s *Server) setupRoutes() { api.POST("/git/push-preflight", s.handleGitPushPreflight) api.POST("/git/contribution/replace", s.handleGitReplaceContribution) api.POST("/git/contribution/use", s.handleGitUseContribution) + api.POST("/git/contribution/history-explanation", s.handleGitContributionHistoryExplanation) api.POST("/git/rebase", s.handleGitRebase) api.POST("/git/merge", s.handleGitMerge) api.POST("/git/abort", s.handleGitAbort) diff --git a/apps/backend/internal/agentctl/server/process/git.go b/apps/backend/internal/agentctl/server/process/git.go index 2a489e41e6..b1177019bc 100644 --- a/apps/backend/internal/agentctl/server/process/git.go +++ b/apps/backend/internal/agentctl/server/process/git.go @@ -66,10 +66,11 @@ type GitOperationResult struct { // GitOperator executes git operations in a workspace directory. type GitOperator struct { - workDir string - logger *logger.Logger - workspaceTracker *WorkspaceTracker - environment func() []string + workDir string + logger *logger.Logger + workspaceTracker *WorkspaceTracker + environment func() []string + contributionHistoryCommandOverride func(context.Context, ...string) (string, error) // repoName is the multi-repo subpath this operator runs in (e.g. "kandev"). // Empty for the workspace-root operator. Stamped on emitted commit // notifications so the frontend can group commits per repo. diff --git a/apps/backend/internal/agentctl/server/process/git_contribution_history.go b/apps/backend/internal/agentctl/server/process/git_contribution_history.go new file mode 100644 index 0000000000..96b1606134 --- /dev/null +++ b/apps/backend/internal/agentctl/server/process/git_contribution_history.go @@ -0,0 +1,459 @@ +package process + +import ( + "bytes" + "context" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "sync" + "time" + + "github.com/kandev/kandev/internal/common/securityutil" + "github.com/kandev/kandev/internal/common/subproc" + "go.uber.org/zap" +) + +const ( + contributionHistoryObservationTimeout = 2 * time.Second + contributionHistoryReflogLimit = 200 + contributionHistoryCommitLimit = 2000 + contributionHistoryOutputLimit = 1 << 20 + contributionHistoryKindLocalRebase = "local_rebase" + contributionHistoryKindUnexplained = "unexplained" + contributionHistoryReasonMatched = "matched_reflog" + contributionHistoryReasonNoMatch = "no_matching_reflog" + contributionHistoryReasonMissing = "missing_objects" + contributionHistoryReasonAmbiguous = "ambiguous" + contributionHistoryReasonStale = "stale" + contributionHistoryReasonLimit = "limit" + contributionHistoryReasonUnavailable = "unavailable" +) + +var ( + errContributionHistoryMissingObjects = errors.New("contribution history objects are unavailable") + errContributionHistoryAmbiguous = errors.New("contribution history is ambiguous") + errContributionHistoryLimit = errors.New("contribution history observation limit reached") + errContributionHistoryStale = errors.New("contribution history identity is stale") +) + +// ContributionHistoryExplanationResult is a bounded, read-only explanation of +// a task branch and its published contribution head. +type ContributionHistoryExplanationResult struct { + Repo string `json:"repo,omitempty"` + Branch string `json:"branch"` + ExpectedLocalHead string `json:"expected_local_head"` + ExpectedRemoteHead string `json:"expected_remote_head"` + Kind string `json:"kind"` + Reason string `json:"reason"` + OntoHead string `json:"onto_head,omitempty"` + TaskCommitCount *int `json:"task_commit_count,omitempty"` + PublishedCommitCount *int `json:"published_commit_count,omitempty"` + NewBaseCommitCount *int `json:"new_base_commit_count,omitempty"` +} + +type contributionHistoryReflogEntry struct { + sha string + subject string +} + +type contributionHistoryCountResult struct { + count *int + limited bool + failed bool +} + +// contributionHistoryOutputWriter captures command output without allowing a +// reflog or commit walk to grow the agentctl process without bound. +type contributionHistoryOutputWriter struct { + mu sync.Mutex + buffer bytes.Buffer + limit int + truncated bool +} + +func newContributionHistoryOutputWriter(limit int) *contributionHistoryOutputWriter { + return &contributionHistoryOutputWriter{limit: limit} +} + +func (w *contributionHistoryOutputWriter) Write(data []byte) (int, error) { + w.mu.Lock() + defer w.mu.Unlock() + + remaining := w.limit - w.buffer.Len() + if remaining > 0 { + keep := len(data) + if keep > remaining { + keep = remaining + } + _, _ = w.buffer.Write(data[:keep]) + } + if len(data) > remaining { + w.truncated = true + } + return len(data), nil +} + +func (w *contributionHistoryOutputWriter) String() string { + w.mu.Lock() + defer w.mu.Unlock() + return w.buffer.String() +} + +func (w *contributionHistoryOutputWriter) Truncated() bool { + w.mu.Lock() + defer w.mu.Unlock() + return w.truncated +} + +// ExplainContributionHistory observes local Git state for a selected +// contribution identity. It never fetches, rewrites, or publishes refs. +func (g *GitOperator) ExplainContributionHistory( + ctx context.Context, + branch, expectedLocalHead, expectedRemoteHead string, +) (*ContributionHistoryExplanationResult, error) { + result := newContributionHistoryResult(g.repoName, branch, expectedLocalHead, expectedRemoteHead) + if !validContributionHistoryRequest(branch, expectedLocalHead, expectedRemoteHead) { + return result, nil + } + + observeCtx, cancel := context.WithTimeout(ctx, contributionHistoryObservationTimeout) + defer cancel() + + if err := g.requireNoContributionHistoryRebase(observeCtx); err != nil { + result.Reason = contributionHistoryFailureReason(err, contributionHistoryReasonAmbiguous) + return result, nil + } + if err := g.requireContributionHistoryIdentity(observeCtx, branch, expectedLocalHead); err != nil { + result.Reason = contributionHistoryFailureReason(err, contributionHistoryReasonUnavailable) + return result, nil + } + if err := g.requireContributionHistoryObjects(observeCtx, expectedLocalHead, expectedRemoteHead); err != nil { + result.Reason = contributionHistoryFailureReason(err, contributionHistoryReasonMissing) + return result, nil + } + + ontoHead, err := g.findContributionHistoryRebase(observeCtx, branch, expectedLocalHead, expectedRemoteHead) + if err != nil { + result.Reason = contributionHistoryFailureReason(err, contributionHistoryReasonNoMatch) + return result, nil + } + if err := g.requireContributionHistoryObject(observeCtx, ontoHead); err != nil { + result.Reason = contributionHistoryFailureReason(err, contributionHistoryReasonMissing) + return result, nil + } + ancestor, err := g.contributionHistoryIsAncestor(observeCtx, ontoHead, expectedLocalHead) + if err != nil { + result.Reason = contributionHistoryFailureReason(err, contributionHistoryReasonUnavailable) + return result, nil + } + if !ancestor { + result.Reason = contributionHistoryReasonAmbiguous + return result, nil + } + + result.Kind = contributionHistoryKindLocalRebase + result.Reason = contributionHistoryReasonMatched + result.OntoHead = ontoHead + if limited, failed := g.populateContributionHistoryCounts(observeCtx, result); failed { + result.Kind = contributionHistoryKindUnexplained + result.Reason = contributionHistoryReasonUnavailable + result.OntoHead = "" + } else if limited { + result.Reason = contributionHistoryReasonLimit + } + if err := g.requireContributionHistoryIdentity(observeCtx, branch, expectedLocalHead); err != nil { + result = newContributionHistoryResult(g.repoName, branch, expectedLocalHead, expectedRemoteHead) + result.Reason = contributionHistoryFailureReason(err, contributionHistoryReasonStale) + return result, nil + } + return result, nil +} + +func newContributionHistoryResult(repo, branch, localHead, remoteHead string) *ContributionHistoryExplanationResult { + return &ContributionHistoryExplanationResult{ + Repo: repo, + Branch: branch, + ExpectedLocalHead: localHead, + ExpectedRemoteHead: remoteHead, + Kind: contributionHistoryKindUnexplained, + Reason: contributionHistoryReasonUnavailable, + } +} + +func validContributionHistoryRequest(branch, localHead, remoteHead string) bool { + return securityutil.IsValidBranchName(branch) && !securityutil.IsGitSymbolicRef(branch) && + isFullCommitSHA(localHead) && isFullCommitSHA(remoteHead) +} + +func contributionHistoryFailureReason(err error, fallback string) string { + switch { + case errors.Is(err, errContributionHistoryLimit): + return contributionHistoryReasonLimit + case errors.Is(err, errContributionHistoryMissingObjects): + return contributionHistoryReasonMissing + case errors.Is(err, errContributionHistoryAmbiguous): + return contributionHistoryReasonAmbiguous + case errors.Is(err, errContributionHistoryStale): + return contributionHistoryReasonStale + case errors.Is(err, context.Canceled), errors.Is(err, context.DeadlineExceeded): + return contributionHistoryReasonUnavailable + default: + return fallback + } +} + +func (g *GitOperator) requireContributionHistoryIdentity(ctx context.Context, branch, expectedHead string) error { + currentBranch, err := g.contributionHistoryValue(ctx, "rev-parse", "--abbrev-ref", "HEAD") + if err != nil { + return err + } + currentHead, err := g.contributionHistoryValue(ctx, "rev-parse", "HEAD") + if err != nil { + return err + } + if currentBranch != branch || !strings.EqualFold(currentHead, expectedHead) { + return fmt.Errorf("%w: selected branch or head changed", errContributionHistoryStale) + } + return nil +} + +func (g *GitOperator) requireContributionHistoryObjects(ctx context.Context, heads ...string) error { + for _, head := range heads { + if err := g.requireContributionHistoryObject(ctx, head); err != nil { + return err + } + } + return nil +} + +func (g *GitOperator) requireContributionHistoryObject(ctx context.Context, head string) error { + resolved, err := g.contributionHistoryValue(ctx, "rev-parse", "--verify", head+"^{commit}") + if err != nil { + return fmt.Errorf("%w: %v", errContributionHistoryMissingObjects, err) + } + if !strings.EqualFold(resolved, head) { + return fmt.Errorf("%w: resolved head differs", errContributionHistoryMissingObjects) + } + return nil +} + +func (g *GitOperator) requireNoContributionHistoryRebase(ctx context.Context) error { + for _, state := range []string{"rebase-merge", "rebase-apply"} { + path, err := g.contributionHistoryValue(ctx, "rev-parse", "--git-path", state) + if err != nil { + return err + } + if !filepath.IsAbs(path) { + path = filepath.Join(g.workDir, path) + } + if _, err := os.Stat(path); err == nil { + return errContributionHistoryAmbiguous + } else if !errors.Is(err, os.ErrNotExist) { + return err + } + } + return nil +} + +func (g *GitOperator) findContributionHistoryRebase( + ctx context.Context, branch, localHead, remoteHead string, +) (string, error) { + branchOutput, err := g.contributionHistoryValue(ctx, "reflog", "show", + "--format=%H%x1f%gs", fmt.Sprintf("-n%d", contributionHistoryReflogLimit), branch) + if err != nil { + return "", err + } + branchEntries, err := parseContributionHistoryReflog(branchOutput) + if err != nil { + return "", err + } + ontoHead, err := matchContributionHistoryBranchReflog(branchEntries, branch, localHead, remoteHead) + if err != nil { + return "", err + } + + headOutput, err := g.contributionHistoryValue(ctx, "reflog", "show", + "--format=%H%x1f%gs", fmt.Sprintf("-n%d", contributionHistoryReflogLimit), "HEAD") + if err != nil { + return "", err + } + headEntries, err := parseContributionHistoryReflog(headOutput) + if err != nil { + return "", err + } + if err := matchContributionHistoryHeadReflog(headEntries, branch, localHead, ontoHead); err != nil { + return "", err + } + return ontoHead, nil +} + +func parseContributionHistoryReflog(output string) ([]contributionHistoryReflogEntry, error) { + var entries []contributionHistoryReflogEntry + for _, line := range strings.Split(strings.TrimSpace(output), "\n") { + line = strings.TrimSpace(line) + if line == "" { + continue + } + sha, subject, ok := strings.Cut(line, "\x1f") + if !ok || !isFullCommitSHA(sha) { + return nil, errContributionHistoryAmbiguous + } + entries = append(entries, contributionHistoryReflogEntry{sha: sha, subject: strings.TrimSpace(subject)}) + if len(entries) > contributionHistoryReflogLimit { + return nil, errContributionHistoryLimit + } + } + return entries, nil +} + +func matchContributionHistoryBranchReflog( + entries []contributionHistoryReflogEntry, branch, localHead, remoteHead string, +) (string, error) { + if len(entries) < 2 || !strings.EqualFold(entries[0].sha, localHead) || + !strings.EqualFold(entries[1].sha, remoteHead) { + return "", errors.New("no matching branch reflog") + } + prefix := "rebase (finish): refs/heads/" + branch + " onto " + if !strings.HasPrefix(entries[0].subject, prefix) { + return "", errors.New("no matching branch reflog") + } + ontoHead := strings.TrimSpace(strings.TrimPrefix(entries[0].subject, prefix)) + if !isFullCommitSHA(ontoHead) { + return "", errContributionHistoryAmbiguous + } + return ontoHead, nil +} + +func matchContributionHistoryHeadReflog( + entries []contributionHistoryReflogEntry, branch, localHead, ontoHead string, +) error { + if len(entries) < 3 || !strings.EqualFold(entries[0].sha, localHead) { + return errors.New("no matching HEAD reflog") + } + if entries[0].subject != "rebase (finish): returning to refs/heads/"+branch { + return errors.New("no matching HEAD reflog") + } + + startPrefix := "rebase (start):" + replayCount := 0 + for _, entry := range entries[1:] { + switch { + case strings.HasPrefix(entry.subject, "rebase (pick):"), + strings.HasPrefix(entry.subject, "rebase (continue):"), + strings.HasPrefix(entry.subject, "rebase (reword):"), + strings.HasPrefix(entry.subject, "rebase (edit):"): + replayCount++ + case strings.HasPrefix(entry.subject, startPrefix): + if replayCount == 0 || !strings.EqualFold(entry.sha, ontoHead) { + return errContributionHistoryAmbiguous + } + return nil + default: + return errContributionHistoryAmbiguous + } + } + return errContributionHistoryAmbiguous +} + +func (g *GitOperator) contributionHistoryIsAncestor(ctx context.Context, ancestor, descendant string) (bool, error) { + _, err := g.contributionHistoryCommand(ctx, "merge-base", "--is-ancestor", ancestor, descendant) + if err == nil { + return true, nil + } + var exitErr *exec.ExitError + if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 { + return false, nil + } + return false, err +} + +func (g *GitOperator) populateContributionHistoryCounts( + ctx context.Context, result *ContributionHistoryExplanationResult, +) (limited, failed bool) { + base, err := g.contributionHistoryMergeBase(ctx, result.ExpectedRemoteHead, result.OntoHead) + if err != nil { + return errors.Is(err, errContributionHistoryLimit), errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) + } + + task := g.contributionHistoryCount(ctx, result.OntoHead, result.ExpectedLocalHead) + published := g.contributionHistoryCount(ctx, base, result.ExpectedRemoteHead) + newBase := g.contributionHistoryCount(ctx, base, result.OntoHead) + result.TaskCommitCount = task.count + result.PublishedCommitCount = published.count + result.NewBaseCommitCount = newBase.count + return task.limited || published.limited || newBase.limited, + task.failed || published.failed || newBase.failed +} + +func (g *GitOperator) contributionHistoryMergeBase(ctx context.Context, left, right string) (string, error) { + output, err := g.contributionHistoryValue(ctx, "merge-base", "--all", left, right) + if err != nil { + return "", err + } + lines := strings.Fields(output) + if len(lines) != 1 || !isFullCommitSHA(lines[0]) { + return "", errContributionHistoryAmbiguous + } + return lines[0], nil +} + +func (g *GitOperator) contributionHistoryCount(ctx context.Context, from, to string) contributionHistoryCountResult { + rangeRef := from + ".." + to + merges, err := g.contributionHistoryValue(ctx, "rev-list", "--merges", "-n1", rangeRef) + if err != nil { + return contributionHistoryCountResult{failed: errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded)} + } + if strings.TrimSpace(merges) != "" { + return contributionHistoryCountResult{} + } + output, err := g.contributionHistoryValue(ctx, "rev-list", "--count", + fmt.Sprintf("-n%d", contributionHistoryCommitLimit+1), rangeRef) + if err != nil { + return contributionHistoryCountResult{failed: errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded)} + } + count, err := strconv.Atoi(strings.TrimSpace(output)) + if err != nil { + return contributionHistoryCountResult{failed: true} + } + if count > contributionHistoryCommitLimit { + return contributionHistoryCountResult{limited: true} + } + return contributionHistoryCountResult{count: &count} +} + +func (g *GitOperator) contributionHistoryValue(ctx context.Context, args ...string) (string, error) { + output, err := g.contributionHistoryCommand(ctx, args...) + return strings.TrimSpace(output), err +} + +func (g *GitOperator) contributionHistoryCommand(ctx context.Context, args ...string) (string, error) { + if err := validateGitCommandArgs(args); err != nil { + return "", err + } + if g.contributionHistoryCommandOverride != nil { + return g.contributionHistoryCommandOverride(ctx, args...) + } + cmd := subproc.NewGitCommand(ctx, args...) + cmd.Dir = g.workDir + cmd.Env = filterGitEnv(g.environmentValues()) + output := newContributionHistoryOutputWriter(contributionHistoryOutputLimit) + cmd.Stdout = output + cmd.Stderr = output + g.logger.Debug("executing bounded contribution history command", zap.Strings("args", args)) + err := subproc.RunGitClass(ctx, subproc.GitInteractive, cmd) + if ctxErr := ctx.Err(); ctxErr != nil { + return output.String(), ctxErr + } + if output.Truncated() { + return output.String(), errContributionHistoryLimit + } + if err != nil { + return output.String(), fmt.Errorf("git command failed: %w: %s", err, strings.TrimSpace(output.String())) + } + return output.String(), nil +} diff --git a/apps/backend/internal/agentctl/server/process/git_contribution_history_edge_test.go b/apps/backend/internal/agentctl/server/process/git_contribution_history_edge_test.go new file mode 100644 index 0000000000..5c375630a4 --- /dev/null +++ b/apps/backend/internal/agentctl/server/process/git_contribution_history_edge_test.go @@ -0,0 +1,384 @@ +package process + +import ( + "context" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "runtime" + "strconv" + "strings" + "testing" + "time" +) + +func TestContributionHistoryExpiredReflogFallsBackToNeutral(t *testing.T) { + _, linkedDir, localHead, remoteHead, _, cleanup := seedContributionHistoryRepo(t) + defer cleanup() + + runGit(t, linkedDir, "reflog", "expire", "--expire=now", "--all") + result, err := NewGitOperator(linkedDir, newTestLogger(t), nil).ExplainContributionHistory( + context.Background(), "feature/history", localHead, remoteHead) + if err != nil { + t.Fatalf("ExplainContributionHistory returned error: %v", err) + } + assertContributionHistoryNeutral(t, result, contributionHistoryReasonNoMatch) +} + +func TestContributionHistoryMissingObjectsFallsBackToNeutral(t *testing.T) { + repoDir, cleanup := setupTestRepo(t) + defer cleanup() + + localHead := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD")) + missingHead := strings.Repeat("0", 40) + result, err := NewGitOperator(repoDir, newTestLogger(t), nil).ExplainContributionHistory( + context.Background(), "main", localHead, missingHead) + if err != nil { + t.Fatalf("ExplainContributionHistory returned error: %v", err) + } + assertContributionHistoryNeutral(t, result, contributionHistoryReasonMissing) +} + +func TestContributionHistoryPostRebaseCommitFallsBackToNeutral(t *testing.T) { + _, linkedDir, _, remoteHead, _, cleanup := seedContributionHistoryRepo(t) + defer cleanup() + + writeFile(t, linkedDir, "after-rebase.txt", "post-rebase change\n") + runGit(t, linkedDir, "add", "after-rebase.txt") + runGit(t, linkedDir, "commit", "-m", "post-rebase commit") + localHead := strings.TrimSpace(runGit(t, linkedDir, "rev-parse", "HEAD")) + result, err := NewGitOperator(linkedDir, newTestLogger(t), nil).ExplainContributionHistory( + context.Background(), "feature/history", localHead, remoteHead) + if err != nil { + t.Fatalf("ExplainContributionHistory returned error: %v", err) + } + assertContributionHistoryNeutral(t, result, contributionHistoryReasonNoMatch) +} + +func TestContributionHistoryInProgressRebaseIsAmbiguous(t *testing.T) { + _, linkedDir, currentHead, remoteHead, _, cleanup := seedContributionHistoryConflictRepo(t) + defer cleanup() + + result, err := NewGitOperator(linkedDir, newTestLogger(t), nil).ExplainContributionHistory( + context.Background(), "feature/history", currentHead, remoteHead) + if err != nil { + t.Fatalf("ExplainContributionHistory returned error: %v", err) + } + assertContributionHistoryNeutral(t, result, contributionHistoryReasonAmbiguous) + runGit(t, linkedDir, "rebase", "--abort") +} + +func TestContributionHistoryConflictResolutionKeepsRebaseEvidence(t *testing.T) { + _, linkedDir, _, remoteHead, ontoHead, cleanup := seedContributionHistoryConflictRepo(t) + defer cleanup() + + writeFile(t, linkedDir, "conflict.txt", "resolved after rebase\n") + runGit(t, linkedDir, "add", "conflict.txt") + runGit(t, linkedDir, "-c", "core.editor=true", "rebase", "--continue") + localHead := strings.TrimSpace(runGit(t, linkedDir, "rev-parse", "HEAD")) + result, err := NewGitOperator(linkedDir, newTestLogger(t), nil).ExplainContributionHistory( + context.Background(), "feature/history", localHead, remoteHead) + if err != nil { + t.Fatalf("ExplainContributionHistory returned error: %v", err) + } + if result.Kind != contributionHistoryKindLocalRebase || result.Reason != contributionHistoryReasonMatched { + t.Fatalf("result = %+v, want matched conflict-resolved rebase", result) + } + if result.OntoHead != ontoHead { + t.Fatalf("onto_head = %q, want %q", result.OntoHead, ontoHead) + } + if result.TaskCommitCount == nil || *result.TaskCommitCount != 1 { + t.Fatalf("task_commit_count = %v, want 1", result.TaskCommitCount) + } + if result.PublishedCommitCount == nil || *result.PublishedCommitCount != 1 { + t.Fatalf("published_commit_count = %v, want 1", result.PublishedCommitCount) + } + if result.NewBaseCommitCount == nil || *result.NewBaseCommitCount != 1 { + t.Fatalf("new_base_commit_count = %v, want 1", result.NewBaseCommitCount) + } +} + +func TestContributionHistoryMergeRangeOmitsPublishedCount(t *testing.T) { + _, linkedDir, localHead, remoteHead, ontoHead, cleanup := seedContributionHistoryMergeRepo(t) + defer cleanup() + + result, err := NewGitOperator(linkedDir, newTestLogger(t), nil).ExplainContributionHistory( + context.Background(), "feature/history", localHead, remoteHead) + if err != nil { + t.Fatalf("ExplainContributionHistory returned error: %v", err) + } + if result.Kind != contributionHistoryKindLocalRebase || result.Reason != contributionHistoryReasonMatched { + t.Fatalf("result = %+v, want matched rebase with a merge range", result) + } + if result.OntoHead != ontoHead { + t.Fatalf("onto_head = %q, want %q", result.OntoHead, ontoHead) + } + if result.PublishedCommitCount != nil { + t.Fatalf("published_commit_count = %v, want omitted for a merge range", result.PublishedCommitCount) + } +} + +func TestContributionHistoryReflogObservationUsesTwoHundredEntryBound(t *testing.T) { + repoDir, cleanup := setupTestRepo(t) + defer cleanup() + + localHead := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD")) + argsFile := filepath.Join(t.TempDir(), "git-args") + operator := NewGitOperator(repoDir, newTestLogger(t), nil) + operator.contributionHistoryCommandOverride = contributionHistoryGitCommandOverride(t, repoDir, + func(args []string) (string, error) { + if err := os.WriteFile(argsFile, []byte(strings.Join(args, "\x1f")), 0o600); err != nil { + return "", err + } + return "", errors.New("synthetic reflog failure") + }) + + result, err := operator.ExplainContributionHistory( + context.Background(), "main", localHead, localHead) + if err != nil { + t.Fatalf("ExplainContributionHistory returned error: %v", err) + } + assertContributionHistoryNeutral(t, result, contributionHistoryReasonNoMatch) + args, err := os.ReadFile(argsFile) + if err != nil { + t.Fatalf("read captured git arguments: %v", err) + } + if !strings.Contains(string(args), "-n200") { + t.Fatalf("reflog command arguments = %q, want -n200", args) + } +} + +func TestContributionHistoryReflogOverflowFallsBackToBoundedNeutral(t *testing.T) { + repoDir, cleanup := setupTestRepo(t) + defer cleanup() + + head := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD")) + operator := NewGitOperator(repoDir, newTestLogger(t), nil) + operator.contributionHistoryCommandOverride = contributionHistoryGitCommandOverride(t, repoDir, + func(args []string) (string, error) { + return strings.Repeat(fmt.Sprintf("%s\x1fnoise\n", head), contributionHistoryReflogLimit+1), nil + }) + + result, err := operator.ExplainContributionHistory( + context.Background(), "main", head, head) + if err != nil { + t.Fatalf("ExplainContributionHistory returned error: %v", err) + } + assertContributionHistoryNeutral(t, result, contributionHistoryReasonLimit) +} + +func TestContributionHistoryCommitCountBoundary(t *testing.T) { + repoDir, cleanup := setupTestRepo(t) + defer cleanup() + + base := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD")) + runGit(t, repoDir, "checkout", "-b", "feature/count-limit") + createContributionHistoryCommitChain(t, repoDir, "feature/count-limit", base, contributionHistoryCommitLimit+1) + head := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD")) + operator := NewGitOperator(repoDir, newTestLogger(t), nil) + limited := operator.contributionHistoryCount(context.Background(), base, head) + if !limited.limited || limited.count != nil { + t.Fatalf("count at limit = %+v, want limited with no count", limited) + } + + underLimitHead := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD~1")) + underLimit := operator.contributionHistoryCount(context.Background(), base, underLimitHead) + if underLimit.limited || underLimit.count == nil || *underLimit.count != contributionHistoryCommitLimit { + t.Fatalf("count below limit = %+v, want exact count %d", underLimit, contributionHistoryCommitLimit) + } +} + +func createContributionHistoryCommitChain(t *testing.T, repoDir, branch, base string, count int) { + t.Helper() + var input strings.Builder + for index := 0; index < count; index++ { + input.WriteString("commit refs/heads/") + input.WriteString(branch) + input.WriteByte('\n') + input.WriteString("mark :") + input.WriteString(strconv.Itoa(index + 1)) + input.WriteByte('\n') + input.WriteString("author Kandev Test 1700000000 +0000\n") + input.WriteString("committer Kandev Test 1700000000 +0000\n") + message := fmt.Sprintf("count commit %d", index) + input.WriteString("data ") + input.WriteString(strconv.Itoa(len(message))) + input.WriteByte('\n') + input.WriteString(message) + input.WriteByte('\n') + if index == 0 { + input.WriteString("from ") + input.WriteString(base) + input.WriteByte('\n') + } else { + input.WriteString("from :") + input.WriteString(strconv.Itoa(index)) + input.WriteByte('\n') + } + } + + cmd := exec.Command("git", "-C", repoDir, "fast-import") + cmd.Env = filterTestGitEnv(os.Environ()) + cmd.Stdin = strings.NewReader(input.String()) + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git fast-import failed: %v\nOutput: %s", err, output) + } +} + +func TestContributionHistoryRunningCommandStopsAtObservationDeadline(t *testing.T) { + repoDir, cleanup := setupTestRepo(t) + defer cleanup() + + head := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD")) + installContributionHistoryGitTimeoutShim(t) + + started := time.Now() + result, err := NewGitOperator(repoDir, newTestLogger(t), nil).ExplainContributionHistory( + context.Background(), "main", head, head) + if err != nil { + t.Fatalf("ExplainContributionHistory returned error: %v", err) + } + if elapsed := time.Since(started); elapsed > 4*time.Second { + t.Fatalf("observation took %s, want it to stop near the 2-second deadline", elapsed) + } + assertContributionHistoryNeutral(t, result, contributionHistoryReasonUnavailable) +} + +func contributionHistoryGitCommandOverride( + t *testing.T, + repoDir string, + reflogOverride func([]string) (string, error), +) func(context.Context, ...string) (string, error) { + t.Helper() + realGit, err := exec.LookPath("git") + if err != nil { + t.Fatalf("exec.LookPath(git) returned error: %v", err) + } + return func(ctx context.Context, args ...string) (string, error) { + if len(args) >= 2 && args[0] == "reflog" && args[1] == "show" { + return reflogOverride(args) + } + cmd := exec.CommandContext(ctx, realGit, args...) + cmd.Dir = repoDir + cmd.Env = filterTestGitEnv(os.Environ()) + output, err := cmd.CombinedOutput() + return string(output), err + } +} + +func installContributionHistoryGitTimeoutShim(t *testing.T) { + t.Helper() + realGit, err := exec.LookPath("git") + if err != nil { + t.Fatalf("exec.LookPath(git) returned error: %v", err) + } + shimDir := t.TempDir() + shimName := "git" + if runtime.GOOS == "windows" { + shimName += ".exe" + } + shimPath := filepath.Join(shimDir, shimName) + executable, err := os.Executable() + if err != nil { + t.Fatalf("find test executable: %v", err) + } + contents, err := os.ReadFile(executable) + if err != nil { + t.Fatalf("read test executable: %v", err) + } + if err := os.WriteFile(shimPath, contents, 0o755); err != nil { + t.Fatalf("write Git shim: %v", err) + } + if runtime.GOOS != "windows" { + if err := os.Chmod(shimPath, 0o755); err != nil { + t.Fatalf("make Git shim executable: %v", err) + } + } + t.Setenv("PATH", shimDir+string(os.PathListSeparator)+os.Getenv("PATH")) + t.Setenv(contributionHistoryGitShimModeEnv, contributionHistoryGitShimTimeout) + t.Setenv(contributionHistoryGitShimRealGitEnv, realGit) +} + +func assertContributionHistoryNeutral(t *testing.T, result *ContributionHistoryExplanationResult, reason string) { + t.Helper() + if result.Kind != contributionHistoryKindUnexplained || result.Reason != reason { + t.Fatalf("result = %+v, want unexplained/%s", result, reason) + } + if result.OntoHead != "" || result.TaskCommitCount != nil || result.PublishedCommitCount != nil || result.NewBaseCommitCount != nil { + t.Fatalf("result = %+v, want no inferred rebase details", result) + } +} + +func seedContributionHistoryConflictRepo(t *testing.T) (repoDir, linkedDir, currentHead, remoteHead, ontoHead string, cleanup func()) { + t.Helper() + repoDir, cleanup = setupTestRepo(t) + runGit(t, repoDir, "checkout", "-b", "feature/history") + writeFile(t, repoDir, "conflict.txt", "feature version\n") + runGit(t, repoDir, "add", "conflict.txt") + runGit(t, repoDir, "commit", "-m", "feature conflict commit") + runGit(t, repoDir, "push", "-u", "origin", "feature/history") + remoteHead = strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD")) + + runGit(t, repoDir, "checkout", "main") + writeFile(t, repoDir, "conflict.txt", "base version\n") + runGit(t, repoDir, "add", "conflict.txt") + runGit(t, repoDir, "commit", "-m", "base conflict commit") + runGit(t, repoDir, "push", "origin", "main") + ontoHead = strings.TrimSpace(runGit(t, repoDir, "rev-parse", "origin/main")) + + linkedDir = filepath.Join(t.TempDir(), "linked-feature") + runGit(t, repoDir, "worktree", "add", linkedDir, "feature/history") + runGitExpectFailure(t, linkedDir, "rebase", "origin/main") + currentHead = strings.TrimSpace(runGit(t, linkedDir, "rev-parse", "HEAD")) + return repoDir, linkedDir, currentHead, remoteHead, ontoHead, cleanup +} + +func seedContributionHistoryMergeRepo(t *testing.T) (repoDir, linkedDir, localHead, remoteHead, ontoHead string, cleanup func()) { + t.Helper() + repoDir, cleanup = setupTestRepo(t) + runGit(t, repoDir, "checkout", "-b", "feature/history") + writeFile(t, repoDir, "feature-one.txt", "feature one\n") + runGit(t, repoDir, "add", "feature-one.txt") + runGit(t, repoDir, "commit", "-m", "feature one") + + runGit(t, repoDir, "checkout", "main") + writeFile(t, repoDir, "base-one.txt", "base one\n") + runGit(t, repoDir, "add", "base-one.txt") + runGit(t, repoDir, "commit", "-m", "base one") + runGit(t, repoDir, "push", "origin", "main") + + runGit(t, repoDir, "checkout", "feature/history") + writeFile(t, repoDir, "feature-two.txt", "feature two\n") + runGit(t, repoDir, "add", "feature-two.txt") + runGit(t, repoDir, "commit", "-m", "feature two") + runGit(t, repoDir, "merge", "--no-ff", "main", "-m", "merge main into feature") + runGit(t, repoDir, "push", "-u", "origin", "feature/history") + remoteHead = strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD")) + + runGit(t, repoDir, "checkout", "main") + writeFile(t, repoDir, "base-two.txt", "base two\n") + runGit(t, repoDir, "add", "base-two.txt") + runGit(t, repoDir, "commit", "-m", "base two") + runGit(t, repoDir, "push", "origin", "main") + ontoHead = strings.TrimSpace(runGit(t, repoDir, "rev-parse", "origin/main")) + + linkedDir = filepath.Join(t.TempDir(), "linked-feature") + runGit(t, repoDir, "worktree", "add", linkedDir, "feature/history") + runGit(t, linkedDir, "rebase", "origin/main") + localHead = strings.TrimSpace(runGit(t, linkedDir, "rev-parse", "HEAD")) + return repoDir, linkedDir, localHead, remoteHead, ontoHead, cleanup +} + +func runGitExpectFailure(t *testing.T, dir string, args ...string) string { + t.Helper() + fullArgs := append([]string{"-C", dir, "-c", "commit.gpgsign=false", "-c", "tag.gpgsign=false"}, args...) + cmd := exec.Command("git", fullArgs...) + cmd.Env = filterTestGitEnv(os.Environ()) + out, err := cmd.CombinedOutput() + if err == nil { + t.Fatalf("git %v succeeded, want failure", args) + } + return string(out) +} diff --git a/apps/backend/internal/agentctl/server/process/git_contribution_history_test.go b/apps/backend/internal/agentctl/server/process/git_contribution_history_test.go new file mode 100644 index 0000000000..2b69c239ec --- /dev/null +++ b/apps/backend/internal/agentctl/server/process/git_contribution_history_test.go @@ -0,0 +1,170 @@ +package process + +import ( + "context" + "fmt" + "path/filepath" + "strings" + "testing" + "time" +) + +func TestContributionHistoryLocalRebase(t *testing.T) { + repoDir, linkedDir, localHead, remoteHead, onto, cleanup := seedContributionHistoryRepo(t) + defer cleanup() + + operator := NewGitOperator(linkedDir, newTestLogger(t), nil) + result, err := operator.ExplainContributionHistory( + context.Background(), "feature/history", localHead, remoteHead) + if err != nil { + t.Fatalf("ExplainContributionHistory returned error: %v", err) + } + if result.Kind != "local_rebase" || result.Reason != "matched_reflog" { + t.Fatalf("result = %+v, want a matched local rebase", result) + } + if result.Branch != "feature/history" || result.ExpectedLocalHead != localHead || + result.ExpectedRemoteHead != remoteHead { + t.Fatalf("result identity = %+v, want branch and both requested heads", result) + } + if result.OntoHead != onto { + t.Fatalf("onto_head = %q, want %q", result.OntoHead, onto) + } + if result.TaskCommitCount == nil || *result.TaskCommitCount != 5 { + t.Fatalf("task_commit_count = %v, want 5", result.TaskCommitCount) + } + if result.PublishedCommitCount == nil || *result.PublishedCommitCount != 5 { + t.Fatalf("published_commit_count = %v, want 5", result.PublishedCommitCount) + } + if result.NewBaseCommitCount == nil || *result.NewBaseCommitCount != 29 { + t.Fatalf("new_base_commit_count = %v, want 29", result.NewBaseCommitCount) + } + + if got := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "--abbrev-ref", "HEAD")); got != "main" { + t.Fatalf("fixture root branch = %q, want main", got) + } +} + +func TestContributionHistoryUnknown(t *testing.T) { + repoDir, cleanup := setupTestRepo(t) + defer cleanup() + + runGit(t, repoDir, "checkout", "-b", "feature/history") + writeFile(t, repoDir, "feature.txt", "feature\n") + runGit(t, repoDir, "add", "feature.txt") + runGit(t, repoDir, "commit", "-m", "feature commit") + localHead := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD")) + remoteHead := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "origin/main")) + + operator := NewGitOperator(repoDir, newTestLogger(t), nil) + result, err := operator.ExplainContributionHistory( + context.Background(), "feature/history", localHead, remoteHead) + if err != nil { + t.Fatalf("ExplainContributionHistory returned error: %v", err) + } + if result.Kind != "unexplained" || result.Reason != "no_matching_reflog" { + t.Fatalf("result = %+v, want a neutral no-matching-reflog result", result) + } + if result.OntoHead != "" || result.TaskCommitCount != nil || + result.PublishedCommitCount != nil || result.NewBaseCommitCount != nil { + t.Fatalf("result = %+v, want no inferred rebase details", result) + } + + stale, err := operator.ExplainContributionHistory( + context.Background(), "feature/history", strings.Repeat("a", 40), remoteHead) + if err != nil { + t.Fatalf("stale ExplainContributionHistory returned error: %v", err) + } + if stale.Kind != "unexplained" || stale.Reason != "stale" { + t.Fatalf("stale result = %+v, want a stale neutral result", stale) + } +} + +func TestContributionHistoryCounts(t *testing.T) { + _, linkedDir, localHead, remoteHead, _, cleanup := seedContributionHistoryRepo(t) + defer cleanup() + + operator := NewGitOperator(linkedDir, newTestLogger(t), nil) + result, err := operator.ExplainContributionHistory( + context.Background(), "feature/history", localHead, remoteHead) + if err != nil { + t.Fatalf("ExplainContributionHistory returned error: %v", err) + } + for name, value := range map[string]*int{ + "task": result.TaskCommitCount, + "published": result.PublishedCommitCount, + "new base": result.NewBaseCommitCount, + } { + if value == nil { + t.Errorf("%s count is nil, want an exact count", name) + } + } +} + +func TestContributionHistoryBounds(t *testing.T) { + writer := newContributionHistoryOutputWriter(8) + written, err := writer.Write([]byte("123456789")) + if err != nil { + t.Fatalf("Write returned error: %v", err) + } + if written != 9 { + t.Fatalf("Write returned %d, want the complete input length", written) + } + if got := writer.String(); got != "12345678" { + t.Fatalf("captured output = %q, want the configured bound", got) + } + if !writer.Truncated() { + t.Fatal("Truncated = false, want true after exceeding the bound") + } +} + +func TestContributionHistoryCancellation(t *testing.T) { + repoDir, cleanup := setupTestRepo(t) + defer cleanup() + + branch := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "--abbrev-ref", "HEAD")) + head := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD")) + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + started := time.Now() + result, err := NewGitOperator(repoDir, newTestLogger(t), nil).ExplainContributionHistory( + ctx, branch, head, head) + if err != nil { + t.Fatalf("cancelled ExplainContributionHistory returned error: %v", err) + } + if elapsed := time.Since(started); elapsed > time.Second { + t.Fatalf("cancelled observation took %s, want prompt return", elapsed) + } + if result.Kind != "unexplained" || result.Reason != "unavailable" { + t.Fatalf("result = %+v, want an unavailable neutral result", result) + } +} + +func seedContributionHistoryRepo(t *testing.T) (repoDir, linkedDir, localHead, remoteHead, onto string, cleanup func()) { + t.Helper() + repoDir, cleanup = setupTestRepo(t) + + runGit(t, repoDir, "checkout", "-b", "feature/history") + for i := 1; i <= 5; i++ { + writeFile(t, repoDir, fmt.Sprintf("feature-commit-%d.txt", i), "feature\n") + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "feature commit") + } + runGit(t, repoDir, "push", "-u", "origin", "feature/history") + remoteHead = strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD")) + + runGit(t, repoDir, "checkout", "main") + for i := 1; i <= 29; i++ { + writeFile(t, repoDir, fmt.Sprintf("base-commit-%d.txt", i), "base\n") + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "base commit") + } + runGit(t, repoDir, "push", "origin", "main") + onto = strings.TrimSpace(runGit(t, repoDir, "rev-parse", "origin/main")) + + linkedDir = filepath.Join(t.TempDir(), "linked-feature") + runGit(t, repoDir, "worktree", "add", linkedDir, "feature/history") + runGit(t, linkedDir, "rebase", "origin/main") + localHead = strings.TrimSpace(runGit(t, linkedDir, "rev-parse", "HEAD")) + return repoDir, linkedDir, localHead, remoteHead, onto, cleanup +} diff --git a/apps/backend/internal/agentctl/server/process/testmain_test.go b/apps/backend/internal/agentctl/server/process/testmain_test.go index 3ea182cdec..e1e476b0f6 100644 --- a/apps/backend/internal/agentctl/server/process/testmain_test.go +++ b/apps/backend/internal/agentctl/server/process/testmain_test.go @@ -26,6 +26,9 @@ import ( const kandevTestFixtureEnv = "KANDEV_TEST_FIXTURE" const gitFetchRaceRealGitEnv = "KANDEV_GIT_FETCH_RACE_REAL_GIT" const gitFetchRaceWorktreeEnv = "KANDEV_GIT_FETCH_RACE_WORKTREE" +const contributionHistoryGitShimModeEnv = "KANDEV_TEST_CONTRIBUTION_HISTORY_GIT_SHIM_MODE" +const contributionHistoryGitShimRealGitEnv = "KANDEV_TEST_CONTRIBUTION_HISTORY_GIT_SHIM_REAL_GIT" +const contributionHistoryGitShimTimeout = "reflog-timeout" const legacyGitLabHostEnv = "GITLAB_HOST" // TestMain branches into fixture-binary mode when the activation env var @@ -37,6 +40,10 @@ func TestMain(m *testing.M) { runFixture(spec) return } + if mode := os.Getenv(contributionHistoryGitShimModeEnv); mode != "" { + runContributionHistoryGitShim(mode) + return + } // Tests build GitOperator with a nil environment (NewGitOperator(dir, log, // nil)), which falls back to os.Environ() (environmentValues in git.go). // An inherited KANDEV_GITLAB_HOST / GITLAB_HOST / GITLAB_TOKEN then leaks @@ -248,6 +255,32 @@ func runGitFetchRaceFixture() { os.Exit(1) } +func runContributionHistoryGitShim(mode string) { + args := os.Args[1:] + if len(args) >= 2 && args[0] == "reflog" && args[1] == "show" && mode == contributionHistoryGitShimTimeout { + time.Sleep(time.Hour) + os.Exit(0) + } + + realGit := os.Getenv(contributionHistoryGitShimRealGitEnv) + if realGit == "" { + fmt.Fprintln(os.Stderr, "contribution history shim: missing real Git path") + os.Exit(2) + } + cmd := exec.Command(realGit, args...) + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + if err := cmd.Run(); err == nil { + os.Exit(0) + } else if exitErr, ok := err.(*exec.ExitError); ok { + os.Exit(exitErr.ExitCode()) + } else { + fmt.Fprintf(os.Stderr, "contribution history shim: run Git: %v\n", err) + os.Exit(1) + } +} + // fixtureExec returns the (Command argv, Env) pair tests pass to runners that // take a Command []string and an Env map (e.g. InteractiveStartRequest). The // runner spawns this test binary with the env var set, TestMain dispatches diff --git a/apps/backend/internal/backendapp/e2e_reset.go b/apps/backend/internal/backendapp/e2e_reset.go index b6ddcb8060..180f2d4e8c 100644 --- a/apps/backend/internal/backendapp/e2e_reset.go +++ b/apps/backend/internal/backendapp/e2e_reset.go @@ -53,6 +53,9 @@ func registerE2EResetRoutes( api := router.Group("/api/v1/e2e") api.DELETE("/reset/:workspaceId", handleE2EReset(repo, taskSvc, automationSvc, githubSvc, gitlabSvc, log)) + if githubSvc != nil { + api.POST("/tasks/:id/remote-contribution", handleE2EAttachGitHubContribution(repo, taskSvc, githubSvc, log)) + } // Hidden-workflow factory: lets E2E tests cover the system-only // workflow path (e.g. improve-kandev) without depending on the real // bootstrap endpoint, which clones from GitHub and shells out to gh. @@ -90,6 +93,87 @@ func registerE2EResetRoutes( log.Info("registered E2E endpoints (test-only)") } +type e2eAttachGitHubContributionRequest struct { + PRURL string `json:"pr_url"` +} + +type e2eAttachGitHubContributionResponse struct { + Binding taskmodels.RemoteContribution `json:"binding"` + RemoteName string `json:"remote_name"` +} + +type e2eTaskAccessAuthorizer interface { + AuthorizeTaskAccess(context.Context, string) error +} + +// handleE2EAttachGitHubContribution resolves a mock-provider PR and persists +// the same server-authored binding that a remote-contribution task carries at +// launch. It exists only in the mock-agent E2E surface so tests can prepare a +// task before opening its session; callers cannot submit a writable binding. +func handleE2EAttachGitHubContribution( + repo *sqliterepo.Repository, + authorizer e2eTaskAccessAuthorizer, + githubSvc *github.Service, + log *logger.Logger, +) gin.HandlerFunc { + return func(c *gin.Context) { + var req e2eAttachGitHubContributionRequest + if err := c.ShouldBindJSON(&req); err != nil || strings.TrimSpace(req.PRURL) == "" { + c.JSON(http.StatusBadRequest, gin.H{errKey: "pr_url is required"}) + return + } + + ctx := c.Request.Context() + taskID := c.Param("id") + if err := authorizer.AuthorizeTaskAccess(ctx, taskID); err != nil { + c.JSON(http.StatusNotFound, gin.H{errKey: "task not found"}) + return + } + task, err := repo.GetTask(ctx, taskID) + if err != nil || task == nil { + c.JSON(http.StatusNotFound, gin.H{errKey: "task not found"}) + return + } + resolution, err := githubSvc.ResolveRemoteContributionForWorkspace( + ctx, task.WorkspaceID, github.DefaultUserID, req.PRURL, + ) + if err != nil { + log.Warn("e2e remote contribution resolution failed", zap.String("task_id", task.ID), zap.Error(err)) + c.JSON(http.StatusUnprocessableEntity, gin.H{errKey: err.Error()}) + return + } + + link, err := repo.GetPrimaryTaskRepository(ctx, task.ID) + if err != nil || link == nil { + c.JSON(http.StatusBadRequest, gin.H{errKey: "task has no primary repository"}) + return + } + metadata := make(map[string]interface{}, len(link.Metadata)+1) + for key, value := range link.Metadata { + metadata[key] = value + } + binding := resolution.Binding + if err := taskmodels.PutRemoteContribution(metadata, &binding); err != nil { + log.Error("e2e remote contribution binding validation failed", zap.String("task_id", task.ID), zap.Error(err)) + c.JSON(http.StatusInternalServerError, gin.H{errKey: "failed to persist remote contribution"}) + return + } + link.Metadata = metadata + link.BaseBranch = binding.BaseBranch + link.CheckoutBranch = binding.HeadBranch + if err := repo.UpdateTaskRepository(ctx, link); err != nil { + log.Error("e2e remote contribution binding persistence failed", zap.String("task_id", task.ID), zap.Error(err)) + c.JSON(http.StatusInternalServerError, gin.H{errKey: "failed to persist remote contribution"}) + return + } + + c.JSON(http.StatusOK, e2eAttachGitHubContributionResponse{ + Binding: binding, + RemoteName: binding.ContributionRemoteName(), + }) + } +} + func handleE2EReset( repo *sqliterepo.Repository, taskSvc *taskservice.Service, diff --git a/apps/backend/internal/backendapp/e2e_reset_test.go b/apps/backend/internal/backendapp/e2e_reset_test.go index 6f1647f449..78bb47e3f0 100644 --- a/apps/backend/internal/backendapp/e2e_reset_test.go +++ b/apps/backend/internal/backendapp/e2e_reset_test.go @@ -3,6 +3,7 @@ package backendapp import ( "context" "errors" + "net/http/httptest" "path/filepath" "strconv" "strings" @@ -10,6 +11,7 @@ import ( "testing" "time" + "github.com/gin-gonic/gin" "github.com/jmoiron/sqlx" "github.com/kandev/kandev/internal/db" @@ -22,6 +24,16 @@ type e2eResetTaskDeleterStub struct { options taskservice.DeleteTaskOptions } +type e2eAttachTaskAuthorizerStub struct { + err error + taskID string +} + +func (s *e2eAttachTaskAuthorizerStub) AuthorizeTaskAccess(_ context.Context, taskID string) error { + s.taskID = taskID + return s.err +} + func (s *e2eResetTaskDeleterStub) DeleteTaskWithOptions( _ context.Context, taskID string, @@ -46,6 +58,29 @@ func TestDeleteTaskForE2EResetDiscardsWorktreeChanges(t *testing.T) { } } +func TestE2EAttachGitHubContributionRejectsUnauthorizedTask(t *testing.T) { + gin.SetMode(gin.TestMode) + authorizer := &e2eAttachTaskAuthorizerStub{err: errors.New("task is not visible")} + response := httptest.NewRecorder() + context, _ := gin.CreateTestContext(response) + context.Params = gin.Params{{Key: "id", Value: "other-task"}} + context.Request = httptest.NewRequest( + "POST", + "/api/v1/e2e/tasks/other-task/remote-contribution", + strings.NewReader(`{"pr_url":"https://github.com/testorg/testrepo/pull/1"}`), + ) + context.Request.Header.Set("Content-Type", "application/json") + + handleE2EAttachGitHubContribution(nil, authorizer, nil, nil)(context) + + if response.Code != 404 { + t.Fatalf("status = %d, want 404", response.Code) + } + if authorizer.taskID != "other-task" { + t.Fatalf("authorized task ID = %q, want other-task", authorizer.taskID) + } +} + func TestE2EResetDeletesWorkspaceGitHubAuthentication(t *testing.T) { raw, err := db.OpenSQLite(filepath.Join(t.TempDir(), "e2e-reset.db")) if err != nil { diff --git a/apps/backend/internal/common/securityutil/git.go b/apps/backend/internal/common/securityutil/git.go index 20fa92280b..5f6559f72b 100644 --- a/apps/backend/internal/common/securityutil/git.go +++ b/apps/backend/internal/common/securityutil/git.go @@ -127,6 +127,7 @@ func IsKnownSafeGitFlag(arg string) bool { "--amend", "--allow-empty", "--soft", "--mixed", "--hard", "--cached", "--force", "--source=HEAD", "--staged", "--worktree", "--dry-run", "--no-verify", "--get-all", "--first-parent", "--is-ancestor", + "--git-path", "--count", "--merges", "--refs", "--src-prefix=", "--dst-prefix=", } diff --git a/apps/backend/pkg/websocket/actions.go b/apps/backend/pkg/websocket/actions.go index f6849d1a1b..857c1fd036 100644 --- a/apps/backend/pkg/websocket/actions.go +++ b/apps/backend/pkg/websocket/actions.go @@ -368,21 +368,22 @@ const ( ActionSessionProcessStatus = "session.process.status" // Git worktree actions - ActionWorktreePull = "worktree.pull" // Pull from remote - ActionWorktreePush = "worktree.push" // Push to remote - ActionWorktreeReplaceContribution = "worktree.replace_contribution" // Replace the bound contribution branch - ActionWorktreeUseContribution = "worktree.use_contribution" // Adopt the bound contribution version - ActionWorktreeRebase = "worktree.rebase" // Rebase onto base branch - ActionWorktreeMerge = "worktree.merge" // Merge base branch into worktree - ActionWorktreeAbort = "worktree.abort" // Abort in-progress merge or rebase - ActionWorktreeCommit = "worktree.commit" // Commit changes - ActionWorktreeStage = "worktree.stage" // Stage files for commit - ActionWorktreeUnstage = "worktree.unstage" // Unstage files from index - ActionWorktreeDiscard = "worktree.discard" // Discard changes to files - ActionWorktreeCreatePR = "worktree.create_pr" // Create a pull request - ActionWorktreeRevertCommit = "worktree.revert_commit" // Revert a commit (staged, no new commit) - ActionWorktreeRenameBranch = "worktree.rename_branch" // Rename the current branch - ActionWorktreeReset = "worktree.reset" // Reset HEAD to a commit (soft/hard) + ActionWorktreePull = "worktree.pull" // Pull from remote + ActionWorktreePush = "worktree.push" // Push to remote + ActionWorktreeReplaceContribution = "worktree.replace_contribution" // Replace the bound contribution branch + ActionWorktreeUseContribution = "worktree.use_contribution" // Adopt the bound contribution version + ActionWorktreeContributionHistoryExplanation = "worktree.contribution_history_explanation" // Observe contribution history evidence + ActionWorktreeRebase = "worktree.rebase" // Rebase onto base branch + ActionWorktreeMerge = "worktree.merge" // Merge base branch into worktree + ActionWorktreeAbort = "worktree.abort" // Abort in-progress merge or rebase + ActionWorktreeCommit = "worktree.commit" // Commit changes + ActionWorktreeStage = "worktree.stage" // Stage files for commit + ActionWorktreeUnstage = "worktree.unstage" // Unstage files from index + ActionWorktreeDiscard = "worktree.discard" // Discard changes to files + ActionWorktreeCreatePR = "worktree.create_pr" // Create a pull request + ActionWorktreeRevertCommit = "worktree.revert_commit" // Revert a commit (staged, no new commit) + ActionWorktreeRenameBranch = "worktree.rename_branch" // Rename the current branch + ActionWorktreeReset = "worktree.reset" // Reset HEAD to a commit (soft/hard) // User actions ActionUserGet = "user.get" diff --git a/apps/web/components/task/changes-panel-body.tsx b/apps/web/components/task/changes-panel-body.tsx index a1dcd9e759..1a7199b87d 100644 --- a/apps/web/components/task/changes-panel-body.tsx +++ b/apps/web/components/task/changes-panel-body.tsx @@ -127,6 +127,7 @@ type TimelineProps = Pick< | "repoDisplayName" | "perRepoStatus" | "prByRepo" + | "comparisonRequestToken" >; type WorkingTreeProps = Pick< @@ -213,12 +214,14 @@ function CommitHistorySections({ defaultCollapsed, mergedCommits, separated, + comparisonRequestToken, }: { props: TimelineProps; isDiverged: boolean; defaultCollapsed: boolean; mergedCommits: ReturnType; separated: ReturnType; + comparisonRequestToken?: number; }) { const { t } = useTranslation(); if (isDiverged) { @@ -230,6 +233,7 @@ function CommitHistorySections({ label={t("task:localCheckoutCommits")} testId="local-checkout-commits-section" defaultCollapsed={defaultCollapsed} + expandOnRequestToken={comparisonRequestToken} pushDisabled={props.pushDisabled} onOpenCommitDetail={props.onOpenCommitDetail} onRevertCommit={props.onRevertCommit} @@ -248,6 +252,8 @@ function CommitHistorySections({ label={t("task:prNumberVersion", { number: props.providerPRNumber ?? "" })} testId="current-pr-commits-section" defaultCollapsed + expandOnRequestToken={comparisonRequestToken} + focusOnExpand showActions={false} onOpenCommitDetail={props.onOpenCommitDetail} repoDisplayName={props.repoDisplayName} @@ -344,6 +350,7 @@ function ChangesPanelTimeline(props: TimelineProps) { defaultCollapsed={firstSection !== "commits"} mergedCommits={mergedCommits} separated={separated} + comparisonRequestToken={props.comparisonRequestToken} /> )} diff --git a/apps/web/components/task/changes-panel-branch-rows.ts b/apps/web/components/task/changes-panel-branch-rows.ts new file mode 100644 index 0000000000..74f7548448 --- /dev/null +++ b/apps/web/components/task/changes-panel-branch-rows.ts @@ -0,0 +1,32 @@ +export type PerRepoStatus = { + repository_name: string; + branch: string | null; + ahead: number; + behind: number; + pullBehind?: number; + hasStaged: boolean; + hasUnstaged: boolean; +}; + +export type BranchRow = { + repoLabel: string | null; + branch: string; + baseBranch: string; + repositoryName: string; +}; + +export function buildBranchRows( + perRepoStatus: PerRepoStatus[], + baseBranchByRepo: Record | undefined, + baseBranchFallback: string, + repoDisplayName: ((name: string) => string | undefined) | undefined, +): BranchRow[] { + const named = perRepoStatus.filter((s) => s.repository_name !== "" && s.branch); + if (named.length <= 1) return []; + return named.map((s) => ({ + repoLabel: repoDisplayName?.(s.repository_name) || s.repository_name, + branch: s.branch ?? "", + baseBranch: baseBranchByRepo?.[s.repository_name] || baseBranchFallback, + repositoryName: s.repository_name, + })); +} diff --git a/apps/web/components/task/changes-panel-data.tsx b/apps/web/components/task/changes-panel-data.tsx index fca7e05057..80e5f7bd49 100644 --- a/apps/web/components/task/changes-panel-data.tsx +++ b/apps/web/components/task/changes-panel-data.tsx @@ -161,6 +161,8 @@ export type ChangesPanelBodyProps = { workspaceRestoration?: WorkspaceRestorationAttempt | null; onRestoreWorkspace?: () => void; restoreWorkspaceDisabled?: boolean; + /** Monotonic token used to expand both histories after comparison navigation. */ + comparisonRequestToken?: number; }; function usePerRepoCallbacks( @@ -340,9 +342,11 @@ function useChangesPanelPRData(repositoryNames: string[], sessionId: string | nu useRepositoryKeys, relation: relationState.relation, prs, + repositoryScope: relationState.repositoryScope, repositoryName: relationState.repositoryName, selectedPR: taskPR, refreshProviderEvidence: relationState.refreshProviderEvidence, + contributionHistoryTarget: relationState.contributionHistoryTarget, }; } @@ -352,7 +356,7 @@ function hasCumulativeFiles(files: Record | null | undefined): function useChangesPanelResolutionTarget( relation: RemoteContributionRelation, - repositoryName: string | undefined, + repositoryScope: string, selectedPR: TaskPR | null | undefined, t: (key: string) => string, ) { @@ -361,11 +365,11 @@ function useChangesPanelResolutionTarget( () => buildRemoteContributionResolutionTarget( relation, - repositoryName, + repositoryScope, selectedPR, remoteRepositoryLabel, ), - [relation, repositoryName, selectedPR, remoteRepositoryLabel], + [relation, repositoryScope, selectedPR, remoteRepositoryLabel], ); } @@ -389,7 +393,7 @@ export function useChangesPanelData() { ); const resolutionTarget = useChangesPanelResolutionTarget( prData.relation, - prData.repositoryName, + prData.repositoryScope, prData.selectedPR, t, ); diff --git a/apps/web/components/task/changes-panel-header.tsx b/apps/web/components/task/changes-panel-header.tsx index 6ea5f99a34..77488da0cb 100644 --- a/apps/web/components/task/changes-panel-header.tsx +++ b/apps/web/components/task/changes-panel-header.tsx @@ -34,51 +34,22 @@ import { useTouchDrawer } from "@/hooks/use-compact-task-chrome"; import { useTranslation } from "react-i18next"; import { PerRepoPullMenu } from "./changes-panel-per-repo-menu"; import type { RemoteContributionRelation } from "@/hooks/domains/session/remote-contribution-relation"; +import type { ContributionHistoryExplanationTarget } from "@/hooks/domains/session/use-contribution-history-explanation"; import { type RemoteContributionResolutionTarget, type useRemoteContributionResolution, } from "./use-remote-contribution-resolution"; import { RemoteContributionHeaderActions } from "./remote-contribution-header-actions"; import { ComparisonTargetDisplay } from "./changes-panel-comparison-target"; +import { buildBranchRows, type BranchRow, type PerRepoStatus } from "./changes-panel-branch-rows"; -export type PerRepoStatus = { - repository_name: string; - branch: string | null; - ahead: number; - behind: number; - pullBehind?: number; - hasStaged: boolean; - hasUnstaged: boolean; -}; - -type BranchRow = { - repoLabel: string | null; - branch: string; - baseBranch: string; - repositoryName: string; -}; +export type { PerRepoStatus } from "./changes-panel-branch-rows"; type RenameBranchResult = { success: boolean; error?: string; }; -function buildBranchRows( - perRepoStatus: PerRepoStatus[], - baseBranchByRepo: Record | undefined, - baseBranchFallback: string, - repoDisplayName: ((name: string) => string | undefined) | undefined, -): BranchRow[] { - const named = perRepoStatus.filter((s) => s.repository_name !== "" && s.branch); - if (named.length <= 1) return []; - return named.map((s) => ({ - repoLabel: repoDisplayName?.(s.repository_name) || s.repository_name, - branch: s.branch ?? "", - baseBranch: baseBranchByRepo?.[s.repository_name] || baseBranchFallback, - repositoryName: s.repository_name, - })); -} - function RenameBranchButton({ branch, repositoryName, @@ -525,6 +496,7 @@ type ChangesPanelHeaderProps = { * hydrating — the picker falls back to a static label. */ taskId: string | null; relation?: RemoteContributionRelation; + contributionHistoryTarget?: ContributionHistoryExplanationTarget | null; resolution?: ReturnType; resolutionTarget?: RemoteContributionResolutionTarget | null; remoteContributionUrl?: string; @@ -559,6 +531,7 @@ export function ChangesPanelHeader(props: ChangesPanelHeaderProps) { credentialDisplay, comparisonTargets, relation, + contributionHistoryTarget, resolution, resolutionTarget, remoteContributionUrl, @@ -597,6 +570,7 @@ export function ChangesPanelHeader(props: ChangesPanelHeaderProps) { )} { expect(toggle.getAttribute(ARIA_EXPANDED)).toBe("true"); expect(screen.getByTestId(COMMIT_ROW_TID)).toBeTruthy(); }); + + it("keeps a comparison-targeted disclosure in keyboard tab order", () => { + render( + <> + + + , + ); + + const toggle = screen.getByTestId(COMMITS_SECTION_TOGGLE_TID); + screen.getByTestId("before-disclosure").focus(); + expect(toggle.tabIndex).toBe(0); + toggle.focus(); + expect(document.activeElement).toBe(toggle); + + fireEvent.click(toggle); + expect(toggle.getAttribute(ARIA_EXPANDED)).toBe("true"); + }); }); diff --git a/apps/web/components/task/changes-panel-timeline.tsx b/apps/web/components/task/changes-panel-timeline.tsx index f229f1df7a..8ffcaf59e9 100644 --- a/apps/web/components/task/changes-panel-timeline.tsx +++ b/apps/web/components/task/changes-panel-timeline.tsx @@ -1,6 +1,6 @@ "use client"; -import { useState, useMemo, useCallback } from "react"; +import { useState, useMemo, useCallback, useEffect, useRef } from "react"; import { IconChevronDown, IconChevronRight } from "@tabler/icons-react"; import { useMultiSelect } from "@/hooks/use-multi-select"; import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; @@ -47,6 +47,8 @@ function TimelineSection({ children, collapsible = true, defaultCollapsed = false, + expandOnRequestToken, + focusOnExpand, "data-testid": testId, }: { dotColor: string; @@ -56,6 +58,8 @@ function TimelineSection({ children?: React.ReactNode; collapsible?: boolean; defaultCollapsed?: boolean; + expandOnRequestToken?: number; + focusOnExpand?: boolean; "data-testid": string; }) { const [collapsed, setCollapsed] = useState(defaultCollapsed); @@ -68,10 +72,25 @@ function TimelineSection({ // rules. const [userToggled, setUserToggled] = useState(false); const [prevDefaultCollapsed, setPrevDefaultCollapsed] = useState(defaultCollapsed); + const [lastExpandOnRequestToken, setLastExpandOnRequestToken] = useState(); + const headingRef = useRef(null); if (prevDefaultCollapsed !== defaultCollapsed) { setPrevDefaultCollapsed(defaultCollapsed); if (!userToggled) setCollapsed(defaultCollapsed); } + if (expandOnRequestToken !== undefined && expandOnRequestToken !== lastExpandOnRequestToken) { + setLastExpandOnRequestToken(expandOnRequestToken); + setCollapsed(false); + setUserToggled(false); + } + useEffect(() => { + if (!focusOnExpand || expandOnRequestToken === undefined) return; + // The request closes a Radix menu/drawer before this section expands. Defer + // one frame so the disclosure remains the final focus target after the + // primitive's close and focus-management callbacks have run. + const frame = requestAnimationFrame(() => headingRef.current?.focus()); + return () => cancelAnimationFrame(frame); + }, [expandOnRequestToken, focusOnExpand]); const canCollapse = collapsible && !!label; return ( @@ -96,6 +115,7 @@ function TimelineSection({ }} aria-expanded={!collapsed} data-testid={`${testId}-collapse-toggle`} + ref={headingRef} > {label} {typeof count === "number" && ( @@ -151,6 +171,10 @@ type CommitsSectionProps = { prByRepo?: Record; /** Initial collapse state. Defaults to collapsed; the panel expands it when it is the first visible section. */ defaultCollapsed?: boolean; + /** Expands this section when a comparison request targets the Changes panel. */ + expandOnRequestToken?: number; + /** Moves focus to this section heading after a comparison request. */ + focusOnExpand?: boolean; label?: string; testId?: string; pushDisabled?: boolean; @@ -172,6 +196,8 @@ export function CommitsSection({ perRepoStatus, prByRepo, defaultCollapsed = true, + expandOnRequestToken, + focusOnExpand, label, testId = "commits-section", pushDisabled = false, @@ -204,6 +230,8 @@ export function CommitsSection({ label={label ?? t("task:commits")} count={commits.length} defaultCollapsed={defaultCollapsed} + expandOnRequestToken={expandOnRequestToken} + focusOnExpand={focusOnExpand} data-testid={testId} action={sectionAction} > diff --git a/apps/web/components/task/changes-panel.tsx b/apps/web/components/task/changes-panel.tsx index 83f3fa27e3..7328c41c71 100644 --- a/apps/web/components/task/changes-panel.tsx +++ b/apps/web/components/task/changes-panel.tsx @@ -1,6 +1,6 @@ "use client"; -import { memo } from "react"; +import { memo, useEffect, useMemo } from "react"; import { PanelRoot } from "./panel-primitives"; import { useIsTaskArchived, ArchivedPanelPlaceholder } from "./task-archived-context"; import { ChangesPanelHeader } from "./changes-panel-header"; @@ -13,6 +13,11 @@ import { useChangesPanelData, buildChangesPanelBodyProps } from "./changes-panel import { ChangesPanelBody } from "./changes-panel-body"; import type { CommitDetailTarget, OpenDiffOptions } from "./changes-diff-target"; import { useRequestChangesWalkthrough } from "@/hooks/domains/session/use-request-changes-walkthrough"; +import { + consumeContributionComparisonRequest, + useContributionComparisonRequest, +} from "./remote-contribution-comparison"; +import { contributionHistoryExplanationKey } from "@/hooks/domains/session/use-contribution-history-explanation"; export { filterUnpushedCommits, mergeCommits, separateCommitHistories }; @@ -32,6 +37,30 @@ const ChangesPanel = memo(function ChangesPanel(props: ChangesPanelProps) { sessionId: data.activeSessionId, ready: data.walkthroughRequestReady, }); + const comparisonRequest = useContributionComparisonRequest(); + const contributionKey = useMemo( + () => contributionHistoryExplanationKey(data.contributionHistoryTarget), + [data.contributionHistoryTarget], + ); + const comparisonRequestToken = + comparisonRequest?.key === contributionKey ? comparisonRequest.token : undefined; + useEffect(() => { + if (comparisonRequestToken !== undefined) { + // Let the targeted disclosure's focus effect run before consuming the + // one-shot request. The second frame also gives the menu/drawer close + // primitive time to finish its own focus bookkeeping. + let consumeFrame = 0; + const settleFrame = requestAnimationFrame(() => { + consumeFrame = requestAnimationFrame(() => { + consumeContributionComparisonRequest(comparisonRequestToken); + }); + }); + return () => { + cancelAnimationFrame(settleFrame); + if (consumeFrame) cancelAnimationFrame(consumeFrame); + }; + } + }, [comparisonRequestToken]); if (isArchived) return ; return ( @@ -62,12 +91,16 @@ const ChangesPanel = memo(function ChangesPanel(props: ChangesPanelProps) { credentialDisplay={data.gitCredentialDisplay} comparisonTargets={data.git.comparisonTargets} relation={data.relation} + contributionHistoryTarget={data.contributionHistoryTarget} resolution={data.resolution} resolutionTarget={data.resolutionTarget} remoteContributionUrl={data.selectedPR?.pr_url ?? data.existingPrUrl} remoteContributionNumber={data.selectedPR?.pr_number} /> - + ); }); diff --git a/apps/web/components/task/mobile/mobile-changes-panel.test.tsx b/apps/web/components/task/mobile/mobile-changes-panel.test.tsx new file mode 100644 index 0000000000..770266aea3 --- /dev/null +++ b/apps/web/components/task/mobile/mobile-changes-panel.test.tsx @@ -0,0 +1,142 @@ +import type { ReactNode } from "react"; +import { act, cleanup, render, screen } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { + contributionHistoryExplanationKey, + resetContributionHistoryExplanationForTests, +} from "@/hooks/domains/session/use-contribution-history-explanation"; +import { requestContributionComparison } from "../remote-contribution-comparison"; +import { MobileChangesPanel } from "./mobile-changes-panel"; + +const mocks = vi.hoisted(() => ({ + data: null as Record | null, + headerProps: null as Record | null, + comparisonTokens: [] as Array, +})); + +vi.mock("@/components/state-provider", () => ({ + useAppStore: (selector: (state: unknown) => unknown) => + selector({ tasks: { activeSessionId: "session-1" } }), +})); + +vi.mock("@/hooks/domains/session/use-review-sources", () => ({ + useReviewSources: () => ({ sourceCounts: {} }), +})); + +vi.mock("@/hooks/domains/session/use-request-changes-walkthrough", () => ({ + useRequestChangesWalkthrough: () => vi.fn(), +})); + +vi.mock("../changes-panel", () => ({ + ChangesPanelBody: (props: { comparisonRequestToken?: number }) => { + mocks.comparisonTokens.push(props.comparisonRequestToken); + return
; + }, + ChangesPanelHeader: (props: Record) => { + mocks.headerProps = props; + return
; + }, + useChangesPanelData: () => mocks.data, + buildChangesPanelBodyProps: () => ({}), +})); + +vi.mock("../changes-panel-header", () => ({ + ChangesPanelHeader: (props: Record) => { + mocks.headerProps = props; + return
; + }, +})); + +vi.mock("../panel-primitives", () => ({ + PanelRoot: ({ children, ...props }: { children: ReactNode }) => ( +
{children}
+ ), +})); + +vi.mock("./mobile-diff-sheet", () => ({ + MobileDiffSheet: () => null, +})); + +const target = { + sessionId: "session-1", + workspaceId: "workspace-1", + repositoryScope: "frontend", + branch: "feature/one", + selectedPRKey: "acme/frontend/42", + expectedLocalHead: "a".repeat(40), + expectedRemoteHead: "b".repeat(40), +}; + +function makeData() { + return { + activeTaskId: "task-1", + activeSessionId: "session-1", + relation: { kind: "diverged", action: "diverged_replace" }, + contributionHistoryTarget: target, + resolution: {}, + resolutionTarget: {}, + selectedPR: { pr_url: "https://github.com/acme/frontend/pull/42", pr_number: 42 }, + existingPrUrl: undefined, + git: { + hasChanges: false, + hasCommits: true, + hasAnything: true, + hasUnstaged: false, + hasStaged: false, + pullBehind: 0, + isLoading: false, + loadingOperation: null, + repoNames: ["frontend"], + perRepoStatus: [], + branch: "feature/one", + comparisonTargets: [], + cumulativeDiff: null, + statusLoaded: true, + unstagedFiles: [], + stagedFiles: [], + allFiles: [], + }, + baseBranchDisplay: "main", + baseBranchByRepo: {}, + pullDisabled: false, + pullDisabledReason: undefined, + gitCredentialDisplay: undefined, + repoCallbacks: { + onRepoPull: vi.fn(), + onRepoRebase: vi.fn(), + onRepoMerge: vi.fn(), + }, + repoDisplayName: vi.fn(), + walkthroughRequestReady: false, + }; +} + +afterEach(() => { + cleanup(); + resetContributionHistoryExplanationForTests(); +}); + +beforeEach(() => { + mocks.data = makeData(); + mocks.headerProps = null; + mocks.comparisonTokens = []; +}); + +describe("MobileChangesPanel contribution comparison", () => { + it("passes the selected contribution identity to the header and opens both histories", () => { + render( + , + ); + + expect(mocks.headerProps?.contributionHistoryTarget).toEqual(target); + + const key = contributionHistoryExplanationKey(target); + expect(key).not.toBeNull(); + act(() => { + requestContributionComparison(key!); + }); + + expect(screen.getByTestId("mobile-changes-panel")).toBeTruthy(); + expect(mocks.comparisonTokens).toContainEqual(expect.any(Number)); + }); +}); diff --git a/apps/web/components/task/mobile/mobile-changes-panel.tsx b/apps/web/components/task/mobile/mobile-changes-panel.tsx index 38b3aa491c..d796773bdf 100644 --- a/apps/web/components/task/mobile/mobile-changes-panel.tsx +++ b/apps/web/components/task/mobile/mobile-changes-panel.tsx @@ -13,6 +13,11 @@ import { useReviewSources } from "@/hooks/domains/session/use-review-sources"; import { useAppStore } from "@/components/state-provider"; import { getWebSocketClient } from "@/lib/ws/connection"; import { useRequestChangesWalkthrough } from "@/hooks/domains/session/use-request-changes-walkthrough"; +import { + consumeContributionComparisonRequest, + useContributionComparisonRequest, +} from "../remote-contribution-comparison"; +import { contributionHistoryExplanationKey } from "@/hooks/domains/session/use-contribution-history-explanation"; import type { SelectedDiff } from "../task-layout"; import type { OpenDiffOptions, DiffSheetMode } from "../changes-diff-target"; @@ -25,6 +30,7 @@ type MobileChangesPanelProps = { function buildContributionHeaderProps(data: ReturnType) { return { relation: data.relation, + contributionHistoryTarget: data.contributionHistoryTarget, resolution: data.resolution, resolutionTarget: data.resolutionTarget, remoteContributionUrl: data.selectedPR?.pr_url ?? data.existingPrUrl, @@ -39,6 +45,59 @@ function useRefreshMobileSessionData(sessionId: string | null | undefined) { }, [sessionId]); } +function useContributionComparisonRequestToken( + target: Parameters[0], +): number | undefined { + const comparisonRequest = useContributionComparisonRequest(); + const contributionKey = contributionHistoryExplanationKey(target); + const comparisonRequestToken = + comparisonRequest?.key === contributionKey ? comparisonRequest.token : undefined; + + useEffect(() => { + if (comparisonRequestToken === undefined) return; + // Let the targeted disclosure schedule its focus before consuming the + // one-shot request. The second frame also lets the Drawer finish its + // focus bookkeeping on touch browsers. + let consumeFrame = 0; + const settleFrame = requestAnimationFrame(() => { + consumeFrame = requestAnimationFrame(() => { + consumeContributionComparisonRequest(comparisonRequestToken); + }); + }); + return () => { + cancelAnimationFrame(settleFrame); + if (consumeFrame) cancelAnimationFrame(consumeFrame); + }; + }, [comparisonRequestToken]); + + return comparisonRequestToken; +} + +function useOpenSelectedDiff( + selectedDiff: SelectedDiff | null, + onClearSelected: () => void, + setDiffSheet: (mode: DiffSheetMode | null) => void, +) { + const prevSelectedDiffRef = useRef(null); + + useEffect(() => { + if (!selectedDiff?.path) { + prevSelectedDiffRef.current = selectedDiff; + return; + } + + const prevPath = prevSelectedDiffRef.current?.path; + prevSelectedDiffRef.current = selectedDiff; + if (prevPath === selectedDiff.path) return; + + // queueMicrotask satisfies react-hooks/set-state-in-effect; executes before next paint. + queueMicrotask(() => { + setDiffSheet({ kind: "file", path: selectedDiff.path }); + onClearSelected(); + }); + }, [selectedDiff, onClearSelected, setDiffSheet]); +} + /** * Mobile Changes panel — renders the same timeline summary surface as desktop. * Reuses ChangesPanelBody + ChangesPanelHeader from desktop. @@ -61,26 +120,10 @@ export const MobileChangesPanel = memo(function MobileChangesPanel({ sessionId: data.activeSessionId, ready: data.walkthroughRequestReady, }); - - // Track the previous selectedDiff to detect changes - const prevSelectedDiffRef = useRef(null); - useEffect(() => { - // Only open the sheet if selectedDiff changed from null/undefined to a new path - if (!selectedDiff?.path) { - prevSelectedDiffRef.current = selectedDiff; - return; - } - - const prevPath = prevSelectedDiffRef.current?.path; - prevSelectedDiffRef.current = selectedDiff; - if (prevPath === selectedDiff.path) return; - - // queueMicrotask satisfies react-hooks/set-state-in-effect; executes before next paint. - queueMicrotask(() => { - setDiffSheet({ kind: "file", path: selectedDiff.path }); - onClearSelected(); - }); - }, [selectedDiff, onClearSelected]); + const comparisonRequestToken = useContributionComparisonRequestToken( + data.contributionHistoryTarget, + ); + useOpenSelectedDiff(selectedDiff, onClearSelected, setDiffSheet); const handleOpenDiffAll = useCallback(() => { setDiffSheet({ kind: "all" }); @@ -145,7 +188,7 @@ export const MobileChangesPanel = memo(function MobileChangesPanel({ comparisonTargets={data.git.comparisonTargets} {...buildContributionHeaderProps(data)} /> - + void; onUseContribution?: () => void; onViewPRVersion?: () => void; + onCompareVersions?: () => void; + compareDisabled?: boolean; descriptionMode?: DescriptionMode; + surface?: ActionSurface; + replaceLabelKey?: string; + useLabelKey?: string; + replaceDescriptionKey?: string; + useDescriptionKey?: string; + compareDescriptionKey?: string; itemClassName?: string; testIdPrefix?: string; prNumber?: number; + viewLabelKey?: string; }; function actionHandler( kind: ActionKind, props: RemoteContributionActionItemsProps, ): (() => void) | undefined { + if (kind === "compare") return props.onCompareVersions; if (kind === "replace") return props.onReplaceContribution; if (kind === "use") return props.onUseContribution; return props.onViewPRVersion; } function actionDisabled(kind: ActionKind, props: RemoteContributionActionItemsProps): boolean { + if (kind === "compare") return Boolean(props.compareDisabled) || !props.onCompareVersions; if (kind === "replace") return Boolean(props.replaceDisabled) || !props.onReplaceContribution; if (kind === "use") return Boolean(props.useDisabled) || !props.onUseContribution; return !props.onViewPRVersion; @@ -72,6 +97,7 @@ function actionDisabled(kind: ActionKind, props: RemoteContributionActionItemsPr function actionTestId(prefix: string | undefined, kind: ActionKind): string | undefined { if (!prefix) return undefined; + if (kind === "compare") return `${prefix}-compare-versions`; if (kind === "replace") return `${prefix}-replace-pr-branch`; if (kind === "use") return `${prefix}-use-pr-version`; return `${prefix}-view-pr-version`; @@ -100,6 +126,104 @@ function ActionInfo({ description }: { description: string }) { ); } +type ActionItemProps = { + definition: ActionDefinition; + disabled: boolean; + descriptionMode: DescriptionMode; + itemClassName?: string; + testIdPrefix?: string; + onSelect?: () => void; + prNumber?: number; + surface: ActionSurface; + viewLabelKey?: string; + replaceLabelKey?: string; + useLabelKey?: string; + replaceDescriptionKey?: string; + useDescriptionKey?: string; + compareDescriptionKey?: string; +}; + +function actionLabelKey( + definition: ActionDefinition, + props: Pick, +): string { + switch (definition.kind) { + case "view": + return props.viewLabelKey ?? definition.labelKey; + case "replace": + return props.replaceLabelKey ?? definition.labelKey; + case "use": + return props.useLabelKey ?? definition.labelKey; + default: + return definition.labelKey; + } +} + +function actionDescriptionKey( + definition: ActionDefinition, + props: Pick< + ActionItemProps, + "replaceDescriptionKey" | "useDescriptionKey" | "compareDescriptionKey" + >, +): string { + switch (definition.kind) { + case "replace": + return props.replaceDescriptionKey ?? definition.descriptionKey; + case "use": + return props.useDescriptionKey ?? definition.descriptionKey; + case "compare": + return props.compareDescriptionKey ?? definition.descriptionKey; + default: + return definition.descriptionKey; + } +} + +function ActionItemContent({ + definition, + descriptionMode, + label, + description, +}: Pick & { + label: string; + description: string; +}) { + const Icon = definition.icon; + return ( + <> + + + {label} + {descriptionMode === "inline" && ( + + {description} + + )} + + {descriptionMode === "tooltip" && } + + ); +} + +function actionItemClassName( + definition: ActionDefinition, + descriptionMode: DescriptionMode, + surface: ActionSurface, + itemClassName?: string, +): string { + return cn( + "cursor-pointer gap-3", + descriptionMode === "inline" && "min-h-11", + surface === "drawer" && "flex w-full items-start rounded-md px-3 py-2.5 text-left", + definition.kind === "replace" && "text-destructive", + itemClassName, + ); +} + function ActionItem({ definition, disabled, @@ -108,59 +232,69 @@ function ActionItem({ testIdPrefix, onSelect, prNumber, -}: { - definition: ActionDefinition; - disabled: boolean; - descriptionMode: DescriptionMode; - itemClassName?: string; - testIdPrefix?: string; - onSelect?: () => void; - prNumber?: number; -}) { + surface, + viewLabelKey, + replaceLabelKey, + useLabelKey, + replaceDescriptionKey, + useDescriptionKey, + compareDescriptionKey, +}: ActionItemProps) { const { t } = useTranslation(); - const Icon = definition.icon; - const label = t(definition.labelKey, { number: prNumber ?? "" }); - const description = t(definition.descriptionKey, { number: prNumber ?? "" }); - const labelContent = ( - - {label} - {descriptionMode === "inline" && ( - - {description} - - )} - + const translationOptions = { number: prNumber ?? "" }; + const label = t( + actionLabelKey(definition, { viewLabelKey, replaceLabelKey, useLabelKey }), + translationOptions, ); - + const descriptionKey = actionDescriptionKey(definition, { + replaceDescriptionKey, + useDescriptionKey, + compareDescriptionKey, + }); + const description = t(descriptionKey, { number: prNumber ?? "" }); + const className = actionItemClassName(definition, descriptionMode, surface, itemClassName); + const content = ( + + ); + if (surface === "drawer") { + return ( + + ); + } return ( - - {labelContent} - {descriptionMode === "tooltip" && } + {content} ); } export function RemoteContributionActionItems(props: RemoteContributionActionItemsProps) { const descriptionMode = props.descriptionMode ?? "tooltip"; + const surface = props.surface ?? "menu"; return ( <> - {ACTION_DEFINITIONS.map((definition) => ( + {ACTION_DEFINITIONS.filter( + (definition) => definition.kind !== "compare" || props.onCompareVersions, + ).map((definition) => ( ))} diff --git a/apps/web/components/task/remote-contribution-comparison.ts b/apps/web/components/task/remote-contribution-comparison.ts new file mode 100644 index 0000000000..91a2fd1316 --- /dev/null +++ b/apps/web/components/task/remote-contribution-comparison.ts @@ -0,0 +1,41 @@ +import { useSyncExternalStore } from "react"; + +export type ContributionComparisonRequest = { + key: string; + token: number; +}; + +let nextToken = 0; +let currentRequest: ContributionComparisonRequest | null = null; +const listeners = new Set<() => void>(); + +function notify() { + for (const listener of listeners) listener(); +} + +function subscribe(listener: () => void) { + listeners.add(listener); + return () => listeners.delete(listener); +} + +function getSnapshot() { + return currentRequest; +} + +export function requestContributionComparison(key: string) { + currentRequest = { key, token: ++nextToken }; + if (typeof window !== "undefined") { + window.dispatchEvent(new CustomEvent("switch-to-changes-tab")); + } + notify(); +} + +export function consumeContributionComparisonRequest(token: number) { + if (currentRequest?.token !== token) return; + currentRequest = null; + notify(); +} + +export function useContributionComparisonRequest() { + return useSyncExternalStore(subscribe, getSnapshot, () => null); +} diff --git a/apps/web/components/task/remote-contribution-header-actions.test.tsx b/apps/web/components/task/remote-contribution-header-actions.test.tsx index baa5a03c2c..03fbbc775b 100644 --- a/apps/web/components/task/remote-contribution-header-actions.test.tsx +++ b/apps/web/components/task/remote-contribution-header-actions.test.tsx @@ -58,6 +58,13 @@ const relation: RemoteContributionRelation = { canUseRemote: true, }; +const HEADER_ACTION_TEST_IDS = { + compare: "header-compare-versions", + replace: "header-replace-pr-branch", + use: "header-use-pr-version", + view: "header-view-pr-version", +} as const; + function makeResolution() { return { isLoading: false, @@ -88,19 +95,25 @@ describe("RemoteContributionHeaderActions", () => { />, ); + const warning = screen.getByTestId("header-remote-contribution-warning"); + expect(warning.getAttribute("aria-label")).toBe("Task and PR histories differ"); + expect(warning.getAttribute("title")).toBe( + "The task and published PR histories differ. Compare before choosing a version.", + ); + expect(screen.getByTestId(HEADER_ACTION_TEST_IDS.replace)).toBeTruthy(); + expect(screen.getByTestId(HEADER_ACTION_TEST_IDS.use)).toBeTruthy(); + expect(screen.getByTestId(HEADER_ACTION_TEST_IDS.compare)).toBeTruthy(); + expect(screen.getByTestId(HEADER_ACTION_TEST_IDS.compare)).toHaveProperty("disabled", true); + expect(screen.getByTestId(HEADER_ACTION_TEST_IDS.view).textContent).toContain( + "Open PR on GitHub", + ); expect( - screen.getByTestId("header-remote-contribution-warning").getAttribute("aria-label"), - ).toBe("PR branch changed"); - expect(screen.getByTestId("header-replace-pr-branch")).toBeTruthy(); - expect(screen.getByTestId("header-use-pr-version")).toBeTruthy(); - expect(screen.getByTestId("header-view-pr-version").textContent).toContain("PR #901 version"); - expect( - screen.getByRole("img", { name: /Replace the published PR branch/ }).getAttribute("title"), + screen.getByRole("img", { name: /Replace the published PR history/ }).getAttribute("title"), ).toBeNull(); expect(screen.getAllByRole("tooltip").map((tooltip) => tooltip.textContent)).toEqual( expect.arrayContaining([ - expect.stringContaining("Replace the published PR branch"), - expect.stringContaining("Use the current PR version"), + expect.stringContaining("Replace the published PR history"), + expect.stringContaining("Replace the task checkout history"), expect.stringContaining("Open PR #901 version"), ]), ); @@ -110,4 +123,46 @@ describe("RemoteContributionHeaderActions", () => { expect(resolution.requestReplace).toHaveBeenCalledOnce(); expect(resolution.requestUse).toHaveBeenCalledOnce(); }); + + it("puts comparison before the version changing actions", () => { + render( + , + ); + const menu = screen.getByTestId("header-remote-contribution-menu"); + const itemIds = Array.from(menu.querySelectorAll("[data-testid]")).map((item) => + item.getAttribute("data-testid"), + ); + expect(itemIds.indexOf(HEADER_ACTION_TEST_IDS.compare)).toBeLessThan( + itemIds.indexOf(HEADER_ACTION_TEST_IDS.replace), + ); + }); + + it("keeps unavailable replacement actions disabled in the history menu", () => { + render( + , + ); + + expect(screen.getByTestId(HEADER_ACTION_TEST_IDS.replace)).toHaveProperty("disabled", true); + expect(screen.getByTestId(HEADER_ACTION_TEST_IDS.use)).toHaveProperty("disabled", true); + }); }); diff --git a/apps/web/components/task/remote-contribution-header-actions.tsx b/apps/web/components/task/remote-contribution-header-actions.tsx index 9cc0f78265..35efff58fd 100644 --- a/apps/web/components/task/remote-contribution-header-actions.tsx +++ b/apps/web/components/task/remote-contribution-header-actions.tsx @@ -1,7 +1,17 @@ "use client"; +import { useCallback, useRef, useState, type ReactNode } from "react"; import { IconAlertTriangle } from "@tabler/icons-react"; import { Button } from "@kandev/ui/button"; +import { + Drawer, + DrawerClose, + DrawerContent, + DrawerDescription, + DrawerHeader, + DrawerTitle, + DrawerTrigger, +} from "@kandev/ui/drawer"; import { DropdownMenu, DropdownMenuContent, @@ -14,6 +24,13 @@ import { remoteContributionActionPolicy, type RemoteContributionRelation, } from "@/hooks/domains/session/remote-contribution-relation"; +import { + contributionHistoryExplanationKey, + useContributionHistoryExplanation, + type ContributionHistoryExplanation, + type ContributionHistoryExplanationTarget, +} from "@/hooks/domains/session/use-contribution-history-explanation"; +import { useTouchDrawer } from "@/hooks/use-compact-task-chrome"; import { openExternalLink } from "@/lib/desktop/external-links"; import { type RemoteContributionResolutionTarget, @@ -22,74 +39,395 @@ import { } from "./use-remote-contribution-resolution"; import { RemoteContributionActionItems } from "./remote-contribution-action-items"; import { RemoteContributionResolutionDialog } from "./remote-contribution-resolution-dialog"; +import { requestContributionComparison } from "./remote-contribution-comparison"; + +function isConfirmedLocalRebase(explanation: ContributionHistoryExplanation | null) { + return explanation?.kind === "local_rebase" && explanation.reason === "matched_reflog"; +} + +function HistoryExplanationBody({ + status, + explanation, +}: { + status: "idle" | "loading" | "ready" | "unavailable"; + explanation: ContributionHistoryExplanation | null; +}) { + const { t } = useTranslation(); + const localRebase = isConfirmedLocalRebase(explanation); + return ( +
+

+ {localRebase + ? t("task:remoteContributionHistoryLocalRebase") + : t("task:remoteContributionHistoryBody")} +

+ {status === "loading" && ( +

+ {t("task:remoteContributionHistoryLoading")} +

+ )} + {localRebase && explanation && ( +
+ {typeof explanation.task_commit_count === "number" && ( +

+ {t("task:remoteContributionTaskCommits", { + count: explanation.task_commit_count, + })} +

+ )} + {typeof explanation.published_commit_count === "number" && ( +

+ {t("task:remoteContributionPublishedCommits", { + count: explanation.published_commit_count, + })} +

+ )} + {typeof explanation.new_base_commit_count === "number" && ( +

+ {t("task:remoteContributionNewBaseCommits", { + count: explanation.new_base_commit_count, + })} +

+ )} +
+ )} + {localRebase && ( +

+ {t("task:remoteContributionHistoryGuidance")} +

+ )} +
+ ); +} + +type HistoryMenuBodyProps = { + status: "idle" | "loading" | "ready" | "unavailable"; + explanation: ContributionHistoryExplanation | null; + disabled: boolean; + replaceDisabled: boolean; + useDisabled: boolean; + compareDisabled: boolean; + descriptionMode: "tooltip" | "inline"; + surface: "menu" | "drawer"; + onCompare: () => void; + onReplace: () => void; + onUse: () => void; + onView?: () => void; + prNumber?: number; + onCloseAutoFocus: (event: Event) => void; +}; + +type HistoryMenuActionsProps = Pick< + HistoryMenuBodyProps, + | "disabled" + | "replaceDisabled" + | "useDisabled" + | "compareDisabled" + | "descriptionMode" + | "surface" + | "onCompare" + | "onReplace" + | "onUse" + | "onView" + | "prNumber" +>; + +function HistoryMenuActions({ + disabled, + replaceDisabled, + useDisabled, + compareDisabled, + descriptionMode, + surface, + onCompare, + onReplace, + onUse, + onView, + prNumber, +}: HistoryMenuActionsProps) { + return ( + + ); +} + +function HistoryMenuBody({ + status, + explanation, + disabled, + replaceDisabled, + useDisabled, + compareDisabled, + descriptionMode, + surface, + onCompare, + onReplace, + onUse, + onView, + prNumber, + onCloseAutoFocus, +}: HistoryMenuBodyProps) { + const { t } = useTranslation(); + const actions = ( + + ); + if (surface === "drawer") { + return ( +
+ + + {t("task:remoteContributionHistoryTitle")} + + + + + + {t("task:remoteContributionHistoryBody")} + + +
+ +
{actions}
+
+
+ ); + } + return ( + + +
+ +
+

+ {t("task:remoteContributionHistoryTitle")} +

+ +
+
+ + + {actions} + + ); +} + +type RemoteContributionHeaderActionsProps = { + relation?: RemoteContributionRelation; + resolution?: ReturnType; + resolutionTarget?: RemoteContributionResolutionTarget | null; + contributionHistoryTarget?: ContributionHistoryExplanationTarget | null; + prUrl?: string; + prNumber?: number; +}; + +function useRemoteContributionMenu({ + comparisonKey, + resolution, + resolutionTarget, + policy, + prUrl, +}: { + comparisonKey: string | null; + resolution?: ReturnType; + resolutionTarget?: RemoteContributionResolutionTarget | null; + policy: ReturnType | null; + prUrl?: string; +}) { + const [open, setOpen] = useState(false); + const triggerRef = useRef(null); + const restoreFocusOnCloseRef = useRef(true); + const closeMenu = useCallback((restoreFocus: boolean) => { + restoreFocusOnCloseRef.current = restoreFocus; + setOpen(false); + }, []); + const handleCloseAutoFocus = useCallback((event: Event) => { + event.preventDefault(); + if (restoreFocusOnCloseRef.current) { + queueMicrotask(() => triggerRef.current?.focus()); + } + restoreFocusOnCloseRef.current = true; + }, []); + const handleOpenChange = useCallback((nextOpen: boolean) => { + if (nextOpen) restoreFocusOnCloseRef.current = true; + setOpen(nextOpen); + }, []); + const handleCompare = useCallback(() => { + if (comparisonKey) requestContributionComparison(comparisonKey); + closeMenu(false); + }, [closeMenu, comparisonKey]); + const handleReplace = useCallback(() => { + if (!resolution || !resolutionTarget || policy?.replaceDisabled) return; + closeMenu(true); + resolution.requestReplace(resolutionTarget); + }, [closeMenu, policy?.replaceDisabled, resolution, resolutionTarget]); + const handleUse = useCallback(() => { + if (!resolution || !resolutionTarget || policy?.useDisabled) return; + closeMenu(true); + resolution.requestUse(resolutionTarget); + }, [closeMenu, policy?.useDisabled, resolution, resolutionTarget]); + const handleView = useCallback(() => { + closeMenu(true); + if (prUrl) void openExternalLink(prUrl).catch(() => undefined); + }, [closeMenu, prUrl]); + + return { + open, + triggerRef, + handleCloseAutoFocus, + handleOpenChange, + handleCompare, + handleReplace, + handleUse, + handleView, + }; +} + +function RemoteContributionSurface({ + usesTouchDrawer, + trigger, + menuBody, + open, + onOpenChange, + onCloseAutoFocus, +}: { + usesTouchDrawer: boolean; + trigger: ReactNode; + menuBody: ReactNode; + open: boolean; + onOpenChange: (open: boolean) => void; + onCloseAutoFocus: (event: Event) => void; +}) { + if (usesTouchDrawer) { + return ( + + {trigger} + + {menuBody} + + + ); + } + return ( + + {trigger} + {menuBody} + + ); +} export function RemoteContributionHeaderActions({ relation, resolution, resolutionTarget, + contributionHistoryTarget, prUrl, prNumber, -}: { - relation?: RemoteContributionRelation; - resolution?: ReturnType; - resolutionTarget?: RemoteContributionResolutionTarget | null; - prUrl?: string; - prNumber?: number; -}) { +}: RemoteContributionHeaderActionsProps) { const { t } = useTranslation(); + const usesTouchDrawer = useTouchDrawer(); const confirmResolution = useRemoteContributionResolutionConfirmation(resolution); - if (relation?.kind !== "diverged" || !resolution || !resolutionTarget) return null; - const policy = remoteContributionActionPolicy(relation); + const policy = relation ? remoteContributionActionPolicy(relation) : null; + const comparisonKey = contributionHistoryExplanationKey(contributionHistoryTarget ?? null); + const menu = useRemoteContributionMenu({ + comparisonKey, + resolution, + resolutionTarget, + policy, + prUrl, + }); + const history = useContributionHistoryExplanation(contributionHistoryTarget ?? null, menu.open); + + if (relation?.kind !== "diverged" || !resolution || !resolutionTarget || !policy) return null; + const trigger = ( + + ); + const menuBody = ( + + ); return (
- - - - - - -
- -
-

- {t("task:remoteContributionChangedTitle")} -

-

- {t("task:remoteContributionChangedBody")} -

-
-
- - - resolution.requestReplace(resolutionTarget)} - onUseContribution={() => resolution.requestUse(resolutionTarget)} - onViewPRVersion={ - prUrl ? () => void openExternalLink(prUrl).catch(() => undefined) : undefined - } - testIdPrefix="header" - prNumber={prNumber} - /> - - + {resolution.pending && ( { - if (!open) resolution.cancel(); + onOpenChange={(isOpen) => { + if (!isOpen) resolution.cancel(); }} onConfirm={() => { void confirmResolution(); diff --git a/apps/web/components/task/remote-contribution-resolution-dialog.test.tsx b/apps/web/components/task/remote-contribution-resolution-dialog.test.tsx index fdd7170ce3..785abf600c 100644 --- a/apps/web/components/task/remote-contribution-resolution-dialog.test.tsx +++ b/apps/web/components/task/remote-contribution-resolution-dialog.test.tsx @@ -33,10 +33,12 @@ describe("RemoteContributionResolutionDialog", () => { />, ); - expect(screen.getByRole("heading", { name: "Replace PR branch" })).toBeTruthy(); + expect(screen.getByRole("heading", { name: "Publish task version..." })).toBeTruthy(); expect(screen.getByText(/frontend/)).toBeTruthy(); expect(screen.getByText(/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/)).toBeTruthy(); - expect(screen.getByText(/provider branch/)).toBeTruthy(); + expect( + screen.getByText(/replaces the published PR history with the task history/), + ).toBeTruthy(); fireEvent.click(screen.getByTestId("remote-contribution-confirm")); expect(onConfirm).toHaveBeenCalledOnce(); }); @@ -54,7 +56,7 @@ describe("RemoteContributionResolutionDialog", () => { />, ); - expect(screen.getByRole("heading", { name: "Use PR version" })).toBeTruthy(); + expect(screen.getByRole("heading", { name: "Restore published PR version..." })).toBeTruthy(); expect(screen.getByText(/recovery branch/)).toBeTruthy(); expect(screen.getByText(/working tree must be clean/)).toBeTruthy(); }); diff --git a/apps/web/components/task/remote-contribution-resolution-dialog.tsx b/apps/web/components/task/remote-contribution-resolution-dialog.tsx index f4c60fd6d7..810b838c8f 100644 --- a/apps/web/components/task/remote-contribution-resolution-dialog.tsx +++ b/apps/web/components/task/remote-contribution-resolution-dialog.tsx @@ -35,13 +35,13 @@ export function RemoteContributionResolutionDialog({ }: RemoteContributionResolutionDialogProps) { const { t } = useTranslation(); const isReplace = action === "replace"; - const title = t(isReplace ? "task:replacePRBranch" : "task:usePRVersion"); + const title = t(isReplace ? "task:publishTaskVersion" : "task:restorePublishedPRVersion"); const description = isReplace - ? t("task:replacePRBranchConfirmation", { + ? t("task:remoteContributionPublishConfirmation", { repository: repositoryName, head: expectedRemoteHead, }) - : t("task:usePRVersionConfirmation", { head: expectedRemoteHead }); + : t("task:remoteContributionRestoreConfirmation", { head: expectedRemoteHead }); return ( diff --git a/apps/web/components/task/use-remote-contribution-resolution.ts b/apps/web/components/task/use-remote-contribution-resolution.ts index 4028174bb3..c1c1e2ca61 100644 --- a/apps/web/components/task/use-remote-contribution-resolution.ts +++ b/apps/web/components/task/use-remote-contribution-resolution.ts @@ -22,19 +22,18 @@ export type PendingRemoteContributionResolution = RemoteContributionResolutionTa export function buildRemoteContributionResolutionTarget( relation: RemoteContributionRelation, - repositoryName: string | undefined, + repositoryScope: string | undefined, selectedPR: TaskPR | null | undefined, remoteRepositoryLabel: string, ): RemoteContributionResolutionTarget | null { const providerHead = relation.providerHead; if (!providerHead || (!relation.canReplaceRemote && !relation.canUseRemote)) return null; - const repositoryScope = repositoryName ?? ""; + const scope = repositoryScope ?? ""; return { expectedRemoteHead: providerHead, - repo: repositoryScope, + repo: scope, repositoryName: - repositoryScope || - (selectedPR ? `${selectedPR.owner}/${selectedPR.repo}` : remoteRepositoryLabel), + scope || (selectedPR ? `${selectedPR.owner}/${selectedPR.repo}` : remoteRepositoryLabel), }; } diff --git a/apps/web/components/vcs-multi-repo-menu.tsx b/apps/web/components/vcs-multi-repo-menu.tsx index fc38045922..5937073734 100644 --- a/apps/web/components/vcs-multi-repo-menu.tsx +++ b/apps/web/components/vcs-multi-repo-menu.tsx @@ -49,6 +49,7 @@ export type PerRepoCallbacks = { onReplaceContribution: (repo: string) => void; onUseContribution: (repo: string) => void; onViewContribution: (repo: string) => void; + onCompareContribution: (repo: string) => void; }; export type PrimaryButtonConfig = { @@ -382,7 +383,13 @@ function MultiRepoVcsDropdown({ onReplaceContribution={() => callbacks.onReplaceContribution(blockedRepositoryName)} onUseContribution={() => callbacks.onUseContribution(blockedRepositoryName)} onViewPRVersion={() => callbacks.onViewContribution(blockedRepositoryName)} + onCompareVersions={() => callbacks.onCompareContribution(blockedRepositoryName)} prNumber={prNumber} + replaceLabelKey="task:publishTaskVersion" + useLabelKey="task:restorePublishedPRVersion" + viewLabelKey="task:openPROnGitHub" + replaceDescriptionKey="task:remoteContributionPublishDescription" + useDescriptionKey="task:remoteContributionRestoreDescription" /> )} diff --git a/apps/web/components/vcs-split-button-dropdown.tsx b/apps/web/components/vcs-split-button-dropdown.tsx index 3925ff8a73..df708dac12 100644 --- a/apps/web/components/vcs-split-button-dropdown.tsx +++ b/apps/web/components/vcs-split-button-dropdown.tsx @@ -83,6 +83,12 @@ function ContributionDropdownItems({ onReplaceContribution, onUseContribution, onViewPRVersion, + onCompareVersions, + replaceLabelKey, + useLabelKey, + viewLabelKey, + replaceDescriptionKey, + useDescriptionKey, prNumber, }: { disabled: boolean; @@ -91,6 +97,12 @@ function ContributionDropdownItems({ onReplaceContribution: () => void; onUseContribution: () => void; onViewPRVersion: () => void; + onCompareVersions?: () => void; + replaceLabelKey?: string; + useLabelKey?: string; + viewLabelKey?: string; + replaceDescriptionKey?: string; + useDescriptionKey?: string; prNumber?: number; }) { return ( @@ -103,7 +115,13 @@ function ContributionDropdownItems({ onReplaceContribution={onReplaceContribution} onUseContribution={onUseContribution} onViewPRVersion={onViewPRVersion} + onCompareVersions={onCompareVersions} prNumber={prNumber} + replaceLabelKey={replaceLabelKey} + useLabelKey={useLabelKey} + viewLabelKey={viewLabelKey} + replaceDescriptionKey={replaceDescriptionKey} + useDescriptionKey={useDescriptionKey} /> ); @@ -128,6 +146,7 @@ export type VcsDropdownItemsProps = { onReplaceContribution: () => void; onUseContribution: () => void; onViewPRVersion: () => void; + onCompareVersions?: () => void; prNumber?: number; onRebase: () => void; onMerge: () => void; @@ -152,6 +171,7 @@ export function VcsDropdownItems({ onReplaceContribution, onUseContribution, onViewPRVersion, + onCompareVersions, prNumber, onRebase, onMerge, @@ -199,7 +219,13 @@ export function VcsDropdownItems({ onReplaceContribution={onReplaceContribution} onUseContribution={onUseContribution} onViewPRVersion={onViewPRVersion} + onCompareVersions={onCompareVersions} prNumber={prNumber} + replaceLabelKey="task:publishTaskVersion" + useLabelKey="task:restorePublishedPRVersion" + viewLabelKey="task:openPROnGitHub" + replaceDescriptionKey="task:remoteContributionPublishDescription" + useDescriptionKey="task:remoteContributionRestoreDescription" /> )} diff --git a/apps/web/components/vcs-split-button-parts.tsx b/apps/web/components/vcs-split-button-parts.tsx index 93de0766f0..1569bf0af9 100644 --- a/apps/web/components/vcs-split-button-parts.tsx +++ b/apps/web/components/vcs-split-button-parts.tsx @@ -98,6 +98,7 @@ type SingleRepoVcsButtonProps = { onReplaceContribution: () => void; onUseContribution: () => void; onViewPRVersion: () => void; + onCompareContribution: () => void; prNumber?: number; onRebase: () => void; onMerge: () => void; @@ -128,6 +129,7 @@ function SingleRepoVcsButton({ onReplaceContribution, onUseContribution, onViewPRVersion, + onCompareContribution, prNumber, onRebase, onMerge, @@ -195,6 +197,7 @@ function SingleRepoVcsButton({ onReplaceContribution={onReplaceContribution} onUseContribution={onUseContribution} onViewPRVersion={onViewPRVersion} + onCompareVersions={onCompareContribution} prNumber={prNumber} onRebase={onRebase} onMerge={onMerge} @@ -214,19 +217,22 @@ export type VcsSplitButtonCallbacks = { onReplaceContribution: (repo?: string) => void; onUseContribution: (repo?: string) => void; onViewContribution: (repo?: string) => void; + onCompareContribution: (repo?: string) => void; }; export function buildSingleRepoContributionCallbacks( callbacks: Pick< VcsSplitButtonCallbacks, - "onReplaceContribution" | "onUseContribution" | "onViewContribution" + "onReplaceContribution" | "onUseContribution" | "onViewContribution" | "onCompareContribution" >, blockedRepositoryName?: string, ) { + const repositoryScope = blockedRepositoryName ?? ""; return { - onReplaceContribution: () => callbacks.onReplaceContribution(blockedRepositoryName), - onUseContribution: () => callbacks.onUseContribution(blockedRepositoryName), - onViewContribution: () => callbacks.onViewContribution(blockedRepositoryName), + onReplaceContribution: () => callbacks.onReplaceContribution(repositoryScope), + onUseContribution: () => callbacks.onUseContribution(repositoryScope), + onViewContribution: () => callbacks.onViewContribution(repositoryScope), + onCompareContribution: () => callbacks.onCompareContribution(repositoryScope), }; } @@ -261,6 +267,11 @@ type VcsSplitButtonContentProps = { prNumber?: number; }; +type SingleRepoVcsContentProps = Omit< + VcsSplitButtonContentProps, + "isMultiRepo" | "repoNames" | "perRepoStatus" | "repoDisplayName" +>; + function ContributionResolutionDialog({ resolution, resolutionTarget, @@ -341,6 +352,7 @@ function MultiRepoVcsContent({ onReplaceContribution: (repo) => callbacks.onReplaceContribution(repo), onUseContribution: (repo) => callbacks.onUseContribution(repo), onViewContribution: (repo) => callbacks.onViewContribution(repo), + onCompareContribution: (repo) => callbacks.onCompareContribution(repo), }; return ( <> @@ -373,8 +385,7 @@ function MultiRepoVcsContent({ ); } -export function VcsSplitButtonContent({ - isMultiRepo, +function SingleRepoVcsContent({ primaryButtonConfig, primaryAction, isDisabled, @@ -390,10 +401,7 @@ export function VcsSplitButtonContent({ showContributionResolution, replaceDisabled, useDisabled, - repoNames, - perRepoStatus, blockedRepositoryName, - repoDisplayName, buttonSize, className, showDivergencePills, @@ -402,35 +410,7 @@ export function VcsSplitButtonContent({ resolutionTarget, confirmResolution, prNumber, -}: VcsSplitButtonContentProps) { - if (isMultiRepo) { - return ( - - ); - } - +}: SingleRepoVcsContentProps) { const singleRepoContributionCallbacks = buildSingleRepoContributionCallbacks( callbacks, blockedRepositoryName, @@ -463,6 +443,7 @@ export function VcsSplitButtonContent({ onReplaceContribution={singleRepoContributionCallbacks.onReplaceContribution} onUseContribution={singleRepoContributionCallbacks.onUseContribution} onViewPRVersion={singleRepoContributionCallbacks.onViewContribution} + onCompareContribution={singleRepoContributionCallbacks.onCompareContribution} prNumber={prNumber} onRebase={() => callbacks.onRebase()} onMerge={() => callbacks.onMerge()} @@ -475,3 +456,10 @@ export function VcsSplitButtonContent({ ); } + +export function VcsSplitButtonContent(props: VcsSplitButtonContentProps) { + if (props.isMultiRepo) { + return ; + } + return ; +} diff --git a/apps/web/components/vcs-split-button.test.ts b/apps/web/components/vcs-split-button.test.ts index 3d36013eaf..3747ee0cb3 100644 --- a/apps/web/components/vcs-split-button.test.ts +++ b/apps/web/components/vcs-split-button.test.ts @@ -8,6 +8,8 @@ import { } from "./vcs-split-button"; import { buildSingleRepoContributionCallbacks } from "./vcs-split-button-parts"; +const SELECTED_PR_URL = "https://github.com/acme/widget-a/pull/42"; + /** * The VCS tooltips used to build their plural by hand: * @@ -111,7 +113,7 @@ describe("vcs split-button remote action semantics", () => { handleMerge: vi.fn(), resolution: { requestReplace, requestUse }, resolutionTarget, - selectedPR: { pr_url: "https://github.com/acme/widget-a/pull/42" }, + selectedPR: { pr_url: SELECTED_PR_URL }, openExternalLinkFn, }); @@ -122,25 +124,87 @@ describe("vcs split-button remote action semantics", () => { expect(requestReplace).toHaveBeenCalledWith(resolutionTarget); expect(requestUse).toHaveBeenCalledWith(resolutionTarget); - expect(openExternalLinkFn).toHaveBeenCalledWith("https://github.com/acme/widget-a/pull/42"); + expect(openExternalLinkFn).toHaveBeenCalledWith(SELECTED_PR_URL); }); it("scopes the single-repository menu callbacks to the blocked repository", () => { const onReplaceContribution = vi.fn(); const onUseContribution = vi.fn(); const onViewContribution = vi.fn(); + const onCompareContribution = vi.fn(); const callbacks = buildSingleRepoContributionCallbacks( - { onReplaceContribution, onUseContribution, onViewContribution }, + { onReplaceContribution, onUseContribution, onViewContribution, onCompareContribution }, "widget-a", ); callbacks.onReplaceContribution(); callbacks.onUseContribution(); callbacks.onViewContribution(); + callbacks.onCompareContribution(); expect(onReplaceContribution).toHaveBeenCalledWith("widget-a"); expect(onUseContribution).toHaveBeenCalledWith("widget-a"); expect(onViewContribution).toHaveBeenCalledWith("widget-a"); + expect(onCompareContribution).toHaveBeenCalledWith("widget-a"); + }); + + it("normalizes a root checkout scope for single-repository menu callbacks", () => { + const onReplaceContribution = vi.fn(); + const onUseContribution = vi.fn(); + const onViewContribution = vi.fn(); + const onCompareContribution = vi.fn(); + const callbacks = buildSingleRepoContributionCallbacks( + { onReplaceContribution, onUseContribution, onViewContribution, onCompareContribution }, + undefined, + ); + + callbacks.onReplaceContribution(); + callbacks.onUseContribution(); + callbacks.onViewContribution(); + callbacks.onCompareContribution(); + + expect(onReplaceContribution).toHaveBeenCalledWith(""); + expect(onUseContribution).toHaveBeenCalledWith(""); + expect(onViewContribution).toHaveBeenCalledWith(""); + expect(onCompareContribution).toHaveBeenCalledWith(""); + }); +}); + +describe("vcs split-button root repository scope", () => { + it("matches the root checkout scope when callbacks receive no repository name", async () => { + const requestReplace = vi.fn(); + const requestUse = vi.fn(); + const openExternalLinkFn = vi.fn().mockResolvedValue("browser"); + const onCompareContribution = vi.fn(); + const resolutionTarget = { + expectedRemoteHead: "a".repeat(40), + repo: "", + repositoryName: "testorg/testrepo", + }; + const callbacks = buildVcsSplitCallbacks({ + openCommitDialog: vi.fn(), + openPRDialog: vi.fn(), + handlePull: vi.fn(), + handlePush: vi.fn(), + handleRebase: vi.fn(), + handleMerge: vi.fn(), + resolution: { requestReplace, requestUse }, + resolutionTarget, + selectedPR: { pr_url: SELECTED_PR_URL }, + openExternalLinkFn, + onCompareContribution, + }); + + callbacks.onReplaceContribution(); + callbacks.onUseContribution(); + callbacks.onViewContribution(); + callbacks.onCompareContribution(); + await Promise.resolve(); + + expect(requestReplace).toHaveBeenCalledWith(resolutionTarget); + expect(requestUse).toHaveBeenCalledWith(resolutionTarget); + expect(openExternalLinkFn).toHaveBeenCalledWith(SELECTED_PR_URL); + expect(onCompareContribution).toHaveBeenCalledWith(""); }); }); diff --git a/apps/web/components/vcs-split-button.tsx b/apps/web/components/vcs-split-button.tsx index c31f990127..27b3bba01f 100644 --- a/apps/web/components/vcs-split-button.tsx +++ b/apps/web/components/vcs-split-button.tsx @@ -30,6 +30,12 @@ import { type RemoteContributionResolutionTarget, } from "@/components/task/use-remote-contribution-resolution"; import { openExternalLink } from "@/lib/desktop/external-links"; +import { usePanelActions } from "@/hooks/use-panel-actions"; +import { + contributionHistoryExplanationKey, + type ContributionHistoryExplanationTarget, +} from "@/hooks/domains/session/use-contribution-history-explanation"; +import { requestContributionComparison } from "@/components/task/remote-contribution-comparison"; import { VcsSplitButtonContent } from "./vcs-split-button-parts"; import { DEFAULT_BASE_BRANCH } from "./vcs-constants"; @@ -225,7 +231,7 @@ function useGitActions(git: ReturnType, baseBranch?: strin function useVcsContributionResolution( sessionId: string | null, relation: ReturnType["relation"], - repositoryName: string | undefined, + repositoryScope: string, selectedPR: TaskPR | null, refreshProviderEvidence: ReturnType< typeof useRemoteContributionRelation @@ -239,11 +245,11 @@ function useVcsContributionResolution( () => buildRemoteContributionResolutionTarget( relation, - repositoryName, + repositoryScope, selectedPR, remoteRepositoryLabel, ), - [relation, repositoryName, selectedPR, remoteRepositoryLabel], + [relation, repositoryScope, selectedPR, remoteRepositoryLabel], ); return { resolution, confirmResolution, resolutionTarget }; } @@ -254,7 +260,7 @@ function useVcsContributionState(sessionId: string | null) { const resolutionState = useVcsContributionResolution( sessionId, contribution.relation, - contribution.repositoryName, + contribution.repositoryScope, contribution.selectedPR, contribution.refreshProviderEvidence, ); @@ -266,6 +272,34 @@ type VcsResolutionActions = Pick< "requestReplace" | "requestUse" >; +function requestContributionComparisonForRepo( + repo: string | undefined, + resolutionTarget: RemoteContributionResolutionTarget | null, + contributionHistoryTarget: ContributionHistoryExplanationTarget | null, + addChanges: () => void, +): void { + const repositoryScope = repo ?? ""; + if (!resolutionTarget || resolutionTarget.repo !== repositoryScope) return; + const key = contributionHistoryExplanationKey(contributionHistoryTarget); + if (!key) return; + requestContributionComparison(key); + addChanges(); +} + +function buildContributionComparisonCallback( + resolutionTarget: RemoteContributionResolutionTarget | null, + contributionHistoryTarget: ContributionHistoryExplanationTarget | null, + addChanges: () => void, +) { + return (repo?: string) => + requestContributionComparisonForRepo( + repo, + resolutionTarget, + contributionHistoryTarget, + addChanges, + ); +} + export function buildVcsSplitCallbacks({ openCommitDialog, openPRDialog, @@ -277,6 +311,7 @@ export function buildVcsSplitCallbacks({ resolutionTarget, selectedPR, openExternalLinkFn, + onCompareContribution, }: { openCommitDialog: (repo?: string) => void; openPRDialog: (repo?: string) => void; @@ -288,6 +323,7 @@ export function buildVcsSplitCallbacks({ resolutionTarget: RemoteContributionResolutionTarget | null; selectedPR: Pick | null; openExternalLinkFn?: typeof openExternalLink; + onCompareContribution?: (repo?: string) => void; }) { const openLink = openExternalLinkFn ?? openExternalLink; return { @@ -298,19 +334,29 @@ export function buildVcsSplitCallbacks({ onRebase: handleRebase, onMerge: handleMerge, onReplaceContribution: (repo?: string) => { - if (resolutionTarget && resolutionTarget.repo === repo) { + const repositoryScope = repo ?? ""; + if (resolutionTarget && resolutionTarget.repo === repositoryScope) { resolution.requestReplace(resolutionTarget); } }, onUseContribution: (repo?: string) => { - if (resolutionTarget && resolutionTarget.repo === repo) { + const repositoryScope = repo ?? ""; + if (resolutionTarget && resolutionTarget.repo === repositoryScope) { resolution.requestUse(resolutionTarget); } }, onViewContribution: (repo?: string) => { - if (resolutionTarget?.repo !== repo || !selectedPR?.pr_url) return; + const repositoryScope = repo ?? ""; + if (!resolutionTarget || resolutionTarget.repo !== repositoryScope || !selectedPR?.pr_url) { + return; + } void openLink(selectedPR.pr_url).catch(() => undefined); }, + onCompareContribution: (repo?: string) => { + const repositoryScope = repo ?? ""; + if (!resolutionTarget || resolutionTarget.repo !== repositoryScope) return; + onCompareContribution?.(repositoryScope); + }, }; } @@ -327,6 +373,7 @@ const VcsSplitButton = memo(function VcsSplitButton({ relation, repositoryName, selectedPR, + contributionHistoryTarget, remoteActionPolicy, resolution, confirmResolution, @@ -335,13 +382,12 @@ const VcsSplitButton = memo(function VcsSplitButton({ const hasOpenPR = useHasOpenChangeRequest(selectedPR?.state); const { handlePull, handlePush, handleRebase, handleMerge } = useGitActions(git, baseBranch); const repoDisplayName = useRepoDisplayName(sessionId); + const { addChanges } = usePanelActions(); - const remoteBranch = git.remoteBranch; - const hasUpstream = Boolean(remoteBranch); + const hasUpstream = Boolean(git.remoteBranch); const uncommittedFileCount = git.allFiles.length; const aheadCount = remoteActionPolicy.pushDisabled ? 0 : git.pushAhead; - const behindCount = git.behind; - const pullCount = git.pullBehind; + const { behind: behindCount, pullBehind: pullCount } = git; const isDisabled = git.isLoading || !sessionId; const showContributionResolution = relation.action === "diverged_replace"; const pushDisabledReasonKey = remoteContributionActionReasonKey(relation, "push"); @@ -365,6 +411,11 @@ const VcsSplitButton = memo(function VcsSplitButton({ handleRebase, }); const showDivergencePills = primaryAction !== "commit"; + const onCompareContribution = buildContributionComparisonCallback( + resolutionTarget, + contributionHistoryTarget, + addChanges, + ); const callbacks = buildVcsSplitCallbacks({ openCommitDialog, openPRDialog, @@ -375,6 +426,7 @@ const VcsSplitButton = memo(function VcsSplitButton({ resolution, resolutionTarget, selectedPR, + onCompareContribution, }); return ( diff --git a/apps/web/e2e/helpers/api-client.ts b/apps/web/e2e/helpers/api-client.ts index afd51a4929..a9575caf7d 100644 --- a/apps/web/e2e/helpers/api-client.ts +++ b/apps/web/e2e/helpers/api-client.ts @@ -165,6 +165,15 @@ export type MockGitHubPersonalConnection = { access_expires_at?: string; }; +export type E2ERemoteContributionBinding = { + source_repository: { + remote_url: string; + }; + base_branch: string; + head_branch: string; + head_sha: string; +}; + export type MockGitHubCLIAccount = { host: string; login: string; @@ -235,6 +244,37 @@ function buildWorkflowStepCreateBody( return body; } +type CreateRepositoryOpts = { + name?: string; + provider?: string; + provider_host?: string; + provider_owner?: string; + provider_name?: string; + remote_url?: string; + pull_before_worktree?: boolean; +}; + +function buildRepositoryCreateBody( + localPath: string, + defaultBranch: string, + opts?: CreateRepositoryOpts, +): Record { + return { + name: opts?.name ?? "E2E Repo", + source_type: "local", + local_path: localPath, + default_branch: defaultBranch, + ...(opts?.provider ? { provider: opts.provider } : {}), + ...(opts?.provider_host ? { provider_host: opts.provider_host } : {}), + ...(opts?.provider_owner ? { provider_owner: opts.provider_owner } : {}), + ...(opts?.provider_name ? { provider_name: opts.provider_name } : {}), + ...(opts?.remote_url ? { remote_url: opts.remote_url } : {}), + ...(opts?.pull_before_worktree !== undefined + ? { pull_before_worktree: opts.pull_before_worktree } + : {}), + }; +} + type CreateTaskOpts = { description?: string; workflow_id?: string; @@ -359,6 +399,8 @@ type OptionalAgentTaskOpts = { blocked_by?: string[]; /** Force the start-when-unblocked intent on or off; defaults from start_agent. */ start_when_unblocked?: boolean; + /** Prepare the task without launching its session so E2E fixtures can bind it first. */ + start_agent?: boolean; }; /** `repositories` (with per-entry branches) takes precedence over the shorthand @@ -387,6 +429,7 @@ function buildOptionalAgentTaskFields(opts?: OptionalAgentTaskOpts): Record { return this.request("POST", "/api/v1/tasks", { workspace_id: workspaceId, title, description: opts?.description ?? "", - start_agent: true, + start_agent: opts?.start_agent ?? true, agent_profile_id: agentProfileId, ...buildOptionalAgentTaskFields(opts), }); @@ -847,28 +892,13 @@ export class ApiClient { workspaceId: string, localPath: string, defaultBranch = "main", - opts?: { - name?: string; - provider?: string; - provider_host?: string; - provider_owner?: string; - provider_name?: string; - pull_before_worktree?: boolean; - }, + opts?: CreateRepositoryOpts, ): Promise<{ id: string }> { - return this.request("POST", `/api/v1/workspaces/${workspaceId}/repositories`, { - name: opts?.name ?? "E2E Repo", - source_type: "local", - local_path: localPath, - default_branch: defaultBranch, - ...(opts?.provider ? { provider: opts.provider } : {}), - ...(opts?.provider_host ? { provider_host: opts.provider_host } : {}), - ...(opts?.provider_owner ? { provider_owner: opts.provider_owner } : {}), - ...(opts?.provider_name ? { provider_name: opts.provider_name } : {}), - ...(opts?.pull_before_worktree !== undefined - ? { pull_before_worktree: opts.pull_before_worktree } - : {}), - }); + return this.request( + "POST", + `/api/v1/workspaces/${workspaceId}/repositories`, + buildRepositoryCreateBody(localPath, defaultBranch, opts), + ); } /** @@ -1683,6 +1713,15 @@ export class ApiClient { await this.seedMockGitHubRepositoryAccess(prs); } + async attachE2EGitHubContribution( + taskId: string, + prUrl: string, + ): Promise<{ binding: E2ERemoteContributionBinding; remote_name: string }> { + return this.request("POST", `/api/v1/e2e/tasks/${taskId}/remote-contribution`, { + pr_url: prUrl, + }); + } + async mockGitHubSetMergeOutcome( owner: string, repo: string, diff --git a/apps/web/e2e/tests/chat/inactive-session-transcript-reconciliation.spec.ts b/apps/web/e2e/tests/chat/inactive-session-transcript-reconciliation.spec.ts index 5a49a4489e..02c16a28af 100644 --- a/apps/web/e2e/tests/chat/inactive-session-transcript-reconciliation.spec.ts +++ b/apps/web/e2e/tests/chat/inactive-session-transcript-reconciliation.spec.ts @@ -190,11 +190,14 @@ test.describe("inactive session transcript reconciliation", () => { await expect(chat.getByText(RECENT_RECEIVER_MARKER, { exact: true })).toBeVisible({ timeout: 15_000, }); + await expect(chat.getByTestId("conversation-loading-state")).toHaveCount(0, { + timeout: 30_000, + }); await scrollToOldestLoadedEdge(list, RECENT_RECEIVER_MARKER); const peerRow = chat.locator("[id^='msg-']").filter({ hasText: PEER_PROMPT_MARKER }); - await expect(peerRow).toHaveCount(1); + await expect(peerRow).toHaveCount(1, { timeout: 15_000 }); await expect(peerRow.getByText(PEER_PROMPT_MARKER, { exact: true })).toBeVisible({ timeout: 15_000, }); diff --git a/apps/web/e2e/tests/chat/mobile-busy-signal.spec.ts b/apps/web/e2e/tests/chat/mobile-busy-signal.spec.ts index 82e2be18af..3563bf9cd8 100644 --- a/apps/web/e2e/tests/chat/mobile-busy-signal.spec.ts +++ b/apps/web/e2e/tests/chat/mobile-busy-signal.spec.ts @@ -1,6 +1,6 @@ import { test, expect } from "../../fixtures/test-base"; import { waitForActiveSessionForegroundActivity } from "../../helpers/session-store"; -import { typeWhileBusy } from "../../helpers/type-while-busy"; +import { typeWhileBusy, waitForComposerQueueMode } from "../../helpers/type-while-busy"; import { SessionPage } from "../../pages/session-page"; test.describe("Mobile coarse RUNNING busy signal", () => { @@ -39,7 +39,7 @@ test.describe("Mobile coarse RUNNING busy signal", () => { await waitForActiveSessionForegroundActivity(testPage, "generating"); await expect(session.idleInput()).not.toBeVisible(); - await expect(testPage.locator('[data-placeholder^="Queue"]')).toBeVisible(); + await waitForComposerQueueMode(testPage); const editor = session.activeChat().locator(".tiptap.ProseMirror:visible"); await typeWhileBusy(testPage, editor, "queue this mobile follow-up"); @@ -51,7 +51,7 @@ test.describe("Mobile coarse RUNNING busy signal", () => { await expect(session.agentStatus()).toBeVisible(); await waitForActiveSessionForegroundActivity(testPage, "generating"); await expect(session.idleInput()).not.toBeVisible(); - await expect(testPage.locator('[data-placeholder^="Queue"]')).toBeVisible(); + await waitForComposerQueueMode(testPage); }); }); diff --git a/apps/web/e2e/tests/git/git-changes-panel.spec.ts b/apps/web/e2e/tests/git/git-changes-panel.spec.ts index 391494efa6..d51ac39634 100644 --- a/apps/web/e2e/tests/git/git-changes-panel.spec.ts +++ b/apps/web/e2e/tests/git/git-changes-panel.spec.ts @@ -1,9 +1,10 @@ -import { test, expect } from "../../fixtures/test-base"; +import { test, expect, restoreSeedRepositoryOrigin, type SeedData } from "../../fixtures/test-base"; import { dwell } from "../../helpers/causal-waits"; import type { ApiClient } from "../../helpers/api-client"; import { waitForSessionState } from "../../helpers/session"; import { KanbanPage } from "../../pages/kanban-page"; import { SessionPage } from "../../pages/session-page"; +import { waitForFiniteAnimations } from "../../helpers/animations"; import type { Page, WebSocketRoute } from "@playwright/test"; import fs from "node:fs"; import path from "node:path"; @@ -312,11 +313,153 @@ function createRewrittenProviderHistory( return { head: commits[commits.length - 1].sha, commits }; } +function createLocalRebasedProviderHistory( + git: GitHelper, + branch: string, +): { + localHead: string; + publishedHead: string; + newBaseCommitCount: number; + commits: Array<{ + sha: string; + message: string; + author_login: string; + author_date: string; + stats_available: boolean; + }>; +} { + const publishedBranch = "kandev-e2e-published-history"; + git.exec(`git checkout -B ${publishedBranch} origin/main`); + const commits: Array<{ + sha: string; + message: string; + author_login: string; + author_date: string; + stats_available: boolean; + }> = []; + for (let index = 1; index <= 5; index += 1) { + const message = `Published contribution commit ${index}`; + git.createFile(`published-contribution-${index}.txt`, message); + git.stageFile(`published-contribution-${index}.txt`); + const sha = git.commit(message); + commits.push({ + sha, + message, + author_login: "local-rebase-contributor", + author_date: `2026-08-${String(index).padStart(2, "0")}T12:00:00Z`, + stats_available: false, + }); + } + const publishedHead = git.getCurrentSha(); + git.exec(`git push --force origin HEAD:refs/heads/${branch}`); + + git.exec("git checkout -B main origin/main"); + const newBaseCommitCount = 3; + for (let index = 1; index <= newBaseCommitCount; index += 1) { + git.createFile(`newer-base-${index}.txt`, `newer base commit ${index}`); + git.stageFile(`newer-base-${index}.txt`); + git.commit(`Newer base commit ${index}`); + } + git.exec("git push --force origin main"); + + // Leave the branch at the completed rebase head. The branch and HEAD + // reflogs therefore retain the adjacent published head and rebase sequence + // that the production observer requires. + git.exec(`git checkout -B ${branch} ${publishedHead}`); + git.exec("git rebase origin/main"); + const localHead = git.getCurrentSha(); + git.exec(`git branch --set-upstream-to=origin/${branch} ${branch}`); + return { localHead, publishedHead, newBaseCommitCount, commits }; +} + +function advanceProviderHead(git: GitHelper, previousHead: string, branch: string): string { + const tree = git.exec(`git rev-parse ${previousHead}^{tree}`).trim(); + const nextHead = git + .exec( + `git -c commit.gpgsign=false commit-tree ${tree} -p ${previousHead} -m "Provider head advanced"`, + ) + .trim(); + git.exec(`git push --force origin ${nextHead}:refs/heads/${branch}`); + return nextHead; +} + +function configureContributionRemote( + git: GitHelper, + remoteName: string, + sourceRemoteURL: string, + localRemoteURL: string, +): void { + const remotes = git.exec("git remote").split(/\r?\n/); + if (remotes.includes(remoteName)) { + git.exec(`git remote set-url ${remoteName} "${sourceRemoteURL}"`); + } else { + git.exec(`git remote add ${remoteName} "${sourceRemoteURL}"`); + } + // Keep the production binding's exact credential-free URL while routing + // the disposable E2E source through its local bare origin. + git.exec(`git config url."${localRemoteURL}".insteadOf "${sourceRemoteURL}"`); +} + +function recoveryBranches(git: GitHelper): string[] { + return git + .exec('git for-each-ref --format="%(refname:short)" refs/heads/kandev') + .split(/\r?\n/) + .map((branch) => branch.trim()) + .filter(Boolean); +} + +function restoreContributionHistoryRepository( + seedData: SeedData, + backend: { tmpDir: string }, +): void { + const git = new GitHelper(seedData.repositoryPath, { + ...process.env, + HOME: backend.tmpDir, + GIT_AUTHOR_NAME: "E2E Test", + GIT_AUTHOR_EMAIL: "e2e@test.local", + GIT_COMMITTER_NAME: "E2E Test", + GIT_COMMITTER_EMAIL: "e2e@test.local", + }); + for (const command of ["git rebase --abort", "git merge --abort"]) { + try { + git.exec(command); + } catch { + // The fixture is normally idle when the test finishes. + } + } + git.exec("git checkout -f main"); + git.exec("git clean -fd"); + restoreSeedRepositoryOrigin(seedData); + git.exec(`git push --force origin ${seedData.repositoryBaselineOID}:refs/heads/main`); + git.exec("git fetch --no-tags origin main"); + git.exec(`git reset --hard ${seedData.repositoryBaselineOID}`); + git.exec("git clean -fd"); + for (const branch of ["feature/local-rebase-explanation", "kandev-e2e-published-history"]) { + try { + git.exec(`git branch -D ${branch}`); + } catch { + // Setup may have failed before creating this branch. + } + try { + git.exec(`git push origin --delete ${branch}`); + } catch { + // The branch may only exist locally. + } + } +} + // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- test.describe("Git Changes Panel", () => { + test.afterEach(({ backend, seedData }) => { + // Local-rebase scenarios rewrite origin/main and create temporary refs. + // Restore both sides after every test, including assertion failures, so a + // later worker test cannot inherit the synthetic provider history. + restoreContributionHistoryRepository(seedData, backend); + }); + /** * Verifies that modified files appear in the unstaged section of the Changes panel. * Creates a task, modifies a file in the repository, and verifies the Changes panel @@ -2128,6 +2271,19 @@ test.describe("Git Changes Panel", () => { const localHead = git.getCurrentSha(); const providerBranch = "feature/rewritten-contribution"; const providerHistory = createRewrittenProviderHistory(git, providerBranch); + const contributionRepository = await apiClient.createRepository( + seedData.workspaceId, + repoDir, + "main", + { + name: "E2E GitHub Contribution Repository", + provider: "github", + provider_host: "https://github.com", + provider_owner: "testorg", + provider_name: "testrepo", + remote_url: "https://github.com/testorg/testrepo.git", + }, + ); const task = await apiClient.createTaskWithAgent( seedData.workspaceId, @@ -2137,7 +2293,8 @@ test.describe("Git Changes Panel", () => { description: "/e2e:simple-message", workflow_id: seedData.workflowId, workflow_step_id: seedData.startStepId, - repository_ids: [seedData.repositoryId], + repository_ids: [contributionRepository.id], + start_agent: false, }, ); @@ -2154,8 +2311,23 @@ test.describe("Git Changes Panel", () => { repo_owner: "testorg", repo_name: "testrepo", head_sha: providerHistory.head, + head_repo_owner: "testorg", + head_repo_name: "testrepo", + base_repo_owner: "testorg", + base_repo_name: "testrepo", + base_default_branch: "main", }, ]); + const contribution = await apiClient.attachE2EGitHubContribution( + task.id, + "https://github.com/testorg/testrepo/pull/901", + ); + configureContributionRemote( + git, + contribution.remote_name, + contribution.binding.source_repository.remote_url, + seedData.repositoryRemoteURL, + ); await apiClient.mockGitHubAddPRCommits("testorg", "testrepo", 901, providerHistory.commits); await apiClient.mockGitHubAssociateTaskPR({ task_id: task.id, @@ -2168,6 +2340,7 @@ test.describe("Git Changes Panel", () => { base_branch: "main", author_login: "remote-contributor", }); + await apiClient.ensureTaskSession(task.id); await testPage.goto(`/t/${task.id}`); const session = new SessionPage(testPage); @@ -2221,27 +2394,79 @@ test.describe("Git Changes Panel", () => { await driftWarning.click(); const driftMenu = testPage.getByTestId("header-remote-contribution-menu"); await expect(driftMenu).toBeVisible(); - await expect(driftMenu.getByTestId("header-replace-pr-branch")).toBeVisible(); - await expect(driftMenu.getByTestId("header-use-pr-version")).toBeVisible(); - await expect(driftMenu.getByTestId("header-view-pr-version")).toContainText("PR #901 version"); - const replaceInfo = driftMenu.getByRole("img", { - name: /Replace the published PR branch/, + await waitForFiniteAnimations(driftMenu); + await prCapture.screenshot("remote-contribution-menu-desktop", { + caption: "Desktop history choices explain the preserved checkout and published PR history", }); - await replaceInfo.hover(); + await expect(driftMenu.getByTestId("header-compare-versions")).toBeVisible(); + await driftMenu.getByTestId("header-compare-versions").click(); + await expect(driftMenu).toBeHidden(); + await expect( + providerSection.getByTestId("current-pr-commits-section-collapse-toggle"), + ).toHaveAttribute("aria-expanded", "true"); + expect(git.getCurrentSha()).toBe(localHead); + + await driftWarning.click(); + const reopenedDriftMenu = testPage.getByTestId("header-remote-contribution-menu"); + await expect(reopenedDriftMenu).toBeVisible(); + await expect(reopenedDriftMenu.getByTestId("header-replace-pr-branch")).toBeVisible(); + await expect(reopenedDriftMenu.getByTestId("header-use-pr-version")).toBeVisible(); + await expect(reopenedDriftMenu.getByTestId("header-view-pr-version")).toContainText( + "Open PR on GitHub", + ); + const replaceInfo = reopenedDriftMenu.getByRole("img", { + name: /Replace the published PR history/, + }); + await replaceInfo.focus(); await expect(replaceInfo).not.toHaveAttribute("title"); const openTooltip = testPage.locator( '[data-slot="tooltip-content"]:not([data-state="closed"])', ); - await expect(openTooltip).toContainText("Replace the published PR branch"); - const useInfo = driftMenu.getByRole("img", { name: /Use the current PR version/ }); - await useInfo.hover(); - await expect(openTooltip).toContainText("Use the current PR version"); - await testPage.mouse.move(0, 0); + await expect(openTooltip).toContainText("Replace the published PR history"); + const useInfo = reopenedDriftMenu.getByRole("img", { + name: /Replace the task checkout history/, + }); + await expect(useInfo).toHaveAttribute( + "aria-label", + "Replace the task checkout history with the PR history. Kandev creates a recovery branch first and requires a clean working tree.", + ); + await replaceInfo.evaluate((element) => (element as HTMLElement).blur()); await expect(openTooltip).toBeHidden({ timeout: 5_000 }); - await driftMenu.getByTestId("header-replace-pr-branch").click(); + await reopenedDriftMenu.getByTestId("header-replace-pr-branch").click(); const resolutionDialog = testPage.getByTestId("remote-contribution-resolution-dialog"); await expect(resolutionDialog).toBeVisible(); await expect(resolutionDialog).toContainText(providerHistory.head); + const changedProviderHead = advanceProviderHead(git, providerHistory.head, providerBranch); + await apiClient.mockGitHubAddPRs([ + { + number: 901, + title: "Rewritten contribution", + state: "open", + head_branch: providerBranch, + base_branch: "main", + author_login: "remote-contributor", + repo_owner: "testorg", + repo_name: "testrepo", + head_sha: changedProviderHead, + }, + ]); + await apiClient.mockGitHubAddPRCommits("testorg", "testrepo", 901, [ + { + sha: changedProviderHead, + message: "Provider head advanced", + author_login: "remote-contributor", + author_date: "2026-08-20T12:00:00Z", + stats_available: false, + }, + ]); + await resolutionDialog.getByTestId("remote-contribution-confirm").click(); + await expect(resolutionDialog).toContainText("The PR changed again"); + await expect(resolutionDialog).toContainText(changedProviderHead); + expect(git.getCurrentSha()).toBe(localHead); + expect(git.exec("git status --porcelain").trim()).toBe(""); + expect(git.exec(`git ls-remote origin refs/heads/${providerBranch}`)).toContain( + changedProviderHead, + ); await resolutionDialog.getByRole("button", { name: "Cancel" }).click(); await expect(resolutionDialog).toBeHidden(); await prCapture.screenshot("remote-contribution-drift-desktop", { @@ -2249,6 +2474,197 @@ test.describe("Git Changes Panel", () => { }); }); + test("explains a completed local rebase before offering history changes", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + test.setTimeout(120_000); + + const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); + const git = new GitHelper(repoDir, { + ...process.env, + HOME: backend.tmpDir, + GIT_AUTHOR_NAME: "E2E Test", + GIT_AUTHOR_EMAIL: "e2e@test.local", + GIT_COMMITTER_NAME: "E2E Test", + GIT_COMMITTER_EMAIL: "e2e@test.local", + }); + const providerBranch = "feature/local-rebase-explanation"; + const history = createLocalRebasedProviderHistory(git, providerBranch); + // Let session preparation start from the published branch. Recreate the + // local rebase after the session owns the contribution binding so the + // startup push does not race a deliberately divergent checkout. + git.exec(`git checkout -B ${providerBranch} ${history.publishedHead}`); + git.exec(`git branch --set-upstream-to=origin/${providerBranch} ${providerBranch}`); + const contributionRepository = await apiClient.createRepository( + seedData.workspaceId, + repoDir, + "main", + { + name: "E2E GitHub Contribution Repository", + provider: "github", + provider_host: "https://github.com", + provider_owner: "testorg", + provider_name: "testrepo", + remote_url: "https://github.com/testorg/testrepo.git", + }, + ); + await apiClient.mockGitHubReset(); + await apiClient.mockGitHubSetUser("local-rebase-contributor"); + await apiClient.mockGitHubAddPRs([ + { + number: 904, + title: "Local rebase explanation", + state: "open", + head_branch: providerBranch, + base_branch: "main", + author_login: "local-rebase-contributor", + repo_owner: "testorg", + repo_name: "testrepo", + head_sha: history.publishedHead, + head_repo_owner: "testorg", + head_repo_name: "testrepo", + base_repo_owner: "testorg", + base_repo_name: "testrepo", + base_default_branch: "main", + }, + ]); + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Git Local Rebase Explanation", + seedData.agentProfileId, + { + description: "/e2e:simple-message", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [contributionRepository.id], + start_agent: false, + }, + ); + + const contribution = await apiClient.attachE2EGitHubContribution( + task.id, + "https://github.com/testorg/testrepo/pull/904", + ); + configureContributionRemote( + git, + contribution.remote_name, + contribution.binding.source_repository.remote_url, + seedData.repositoryRemoteURL, + ); + await apiClient.mockGitHubAddPRCommits("testorg", "testrepo", 904, history.commits); + await apiClient.mockGitHubAssociateTaskPR({ + task_id: task.id, + owner: "testorg", + repo: "testrepo", + pr_number: 904, + pr_url: "https://github.com/testorg/testrepo/pull/904", + pr_title: "Local rebase explanation", + head_branch: providerBranch, + base_branch: "main", + author_login: "local-rebase-contributor", + }); + await apiClient.ensureTaskSession(task.id); + + await testPage.goto(`/t/${task.id}`); + const session = new SessionPage(testPage); + await session.waitForLoad(); + await session.waitForChatIdle({ timeout: 45_000 }); + + // Recreate the rebase after the one-time session preparation. The active + // session already owns the server-authored binding, so this leaves the + // production observer's latest HEAD reflog transition as the rebase. + git.exec(`git checkout -B ${providerBranch} ${history.publishedHead}`); + git.exec("git rebase origin/main"); + history.localHead = git.getCurrentSha(); + git.exec(`git branch --set-upstream-to=origin/${providerBranch} ${providerBranch}`); + + await testPage.reload(); + await session.waitForLoad(); + await session.waitForChatIdle({ timeout: 45_000 }); + await session.clickTab("Changes"); + + const changes = testPage.getByTestId("changes-panel"); + const providerSection = changes.getByTestId("current-pr-commits-section"); + const localSection = changes.getByTestId("local-checkout-commits-section"); + await expect(providerSection).toBeVisible({ timeout: 15_000 }); + await expect(localSection).toBeVisible({ timeout: 15_000 }); + + const warning = changes.getByTestId("header-remote-contribution-warning"); + await warning.click(); + const menu = testPage.getByTestId("header-remote-contribution-menu"); + await expect(menu).toBeVisible(); + await expect(menu).toContainText("Task and PR histories differ"); + await expect(menu).toContainText("The task branch was rebased locally"); + await expect(menu).toContainText("Task commits: 5"); + await expect(menu).toContainText("Published commits: 5"); + await expect(menu).toContainText("Newer base commits: 3"); + await expect(menu.getByTestId("header-compare-versions")).toBeVisible(); + + await menu.getByTestId("header-compare-versions").click(); + await expect(menu).toBeHidden(); + const providerToggle = providerSection.getByTestId( + "current-pr-commits-section-collapse-toggle", + ); + const localToggle = localSection.getByTestId("local-checkout-commits-section-collapse-toggle"); + await expect(providerToggle).toHaveAttribute("aria-expanded", "true"); + await expect(providerToggle).toBeFocused(); + await expect(providerToggle).toHaveJSProperty("tabIndex", 0); + await testPage.keyboard.press("Enter"); + await expect(providerToggle).toHaveAttribute("aria-expanded", "false"); + await localToggle.focus(); + for (let index = 0; index < 8; index += 1) { + if (await providerToggle.evaluate((element) => document.activeElement === element)) break; + await testPage.keyboard.press("Tab"); + } + await expect(providerToggle).toBeFocused(); + await testPage.keyboard.press("Enter"); + await expect(providerToggle).toHaveAttribute("aria-expanded", "true"); + await expect(providerSection.locator('[data-testid^="commit-row-"]')).toHaveCount(5); + await expect(localSection.locator('[data-testid^="commit-row-"]')).toHaveCount(5); + expect(git.getCurrentSha()).toBe(history.localHead); + expect(git.exec("git status --porcelain").trim()).toBe(""); + expect(git.exec(`git rev-parse refs/remotes/origin/${providerBranch}`).trim()).toBe( + history.publishedHead, + ); + + await warning.click(); + const reopenedMenu = testPage.getByTestId("header-remote-contribution-menu"); + await expect(reopenedMenu).toBeVisible(); + await testPage.keyboard.press("Escape"); + await expect(reopenedMenu).toBeHidden(); + await expect(warning).toBeFocused(); + await warning.click(); + await expect(reopenedMenu).toBeVisible(); + const recoveryBefore = recoveryBranches(git); + git.createFile("local-rebase-dirty.txt", "uncommitted local change"); + await reopenedMenu.getByTestId("header-use-pr-version").click(); + const dialog = testPage.getByTestId("remote-contribution-resolution-dialog"); + await expect(dialog).toContainText("replaces the current task checkout history"); + await expect(dialog).toContainText(history.publishedHead); + await dialog.getByTestId("remote-contribution-confirm").click(); + await expect(dialog).toContainText("Commit or discard your local changes"); + expect(git.getCurrentSha()).toBe(history.localHead); + expect(git.exec("git status --porcelain").trim()).toContain("local-rebase-dirty.txt"); + expect(recoveryBranches(git)).toEqual(recoveryBefore); + + git.deleteFile("local-rebase-dirty.txt"); + await dialog.getByTestId("remote-contribution-confirm").click(); + await expect(dialog).toBeHidden(); + expect(git.getCurrentSha()).toBe(history.publishedHead); + expect(git.exec("git status --porcelain").trim()).toBe(""); + const recoveryAfter = recoveryBranches(git); + const createdRecoveryBranches = recoveryAfter.filter( + (branch) => !recoveryBefore.includes(branch), + ); + expect(createdRecoveryBranches).toHaveLength(1); + expect(git.exec(`git rev-parse refs/heads/${createdRecoveryBranches[0]}`).trim()).toBe( + history.localHead, + ); + }); + test("uses the PR for the checked-out branch when an older PR is merged", async ({ testPage, apiClient, diff --git a/apps/web/e2e/tests/git/mobile-pr-checkout-drift.spec.ts b/apps/web/e2e/tests/git/mobile-pr-checkout-drift.spec.ts index 73c617dac5..06e45e66c5 100644 --- a/apps/web/e2e/tests/git/mobile-pr-checkout-drift.spec.ts +++ b/apps/web/e2e/tests/git/mobile-pr-checkout-drift.spec.ts @@ -1,6 +1,7 @@ import type { Locator, Page } from "@playwright/test"; -import { test, expect } from "../../fixtures/test-base"; +import { test, expect, restoreSeedRepositoryOrigin, type SeedData } from "../../fixtures/test-base"; import { GitHelper, makeGitEnv } from "../../helpers/git-helper"; +import { waitForFiniteAnimations } from "../../helpers/animations"; import { SessionPage } from "../../pages/session-page"; import path from "node:path"; @@ -62,6 +63,127 @@ function createRewrittenProviderHistory( return { head: commits[commits.length - 1].sha, commits }; } +function createLocalRebasedProviderHistory( + git: GitHelper, + branch: string, +): { + localHead: string; + publishedHead: string; + commits: Array<{ + sha: string; + message: string; + author_login: string; + author_date: string; + stats_available: boolean; + }>; +} { + const publishedBranch = "kandev-e2e-mobile-published-history"; + git.exec(`git checkout -B ${publishedBranch} origin/main`); + const commits: Array<{ + sha: string; + message: string; + author_login: string; + author_date: string; + stats_available: boolean; + }> = []; + for (let index = 1; index <= 5; index += 1) { + const message = `Mobile published contribution commit ${index}`; + git.createFile(`mobile-published-contribution-${index}.txt`, message); + git.stageFile(`mobile-published-contribution-${index}.txt`); + const sha = git.commit(message); + commits.push({ + sha, + message, + author_login: "mobile-local-rebase-contributor", + author_date: `2026-08-${String(index).padStart(2, "0")}T12:00:00Z`, + stats_available: false, + }); + } + const publishedHead = git.getCurrentSha(); + git.exec(`git push --force origin HEAD:refs/heads/${branch}`); + + git.exec("git checkout -B main origin/main"); + for (let index = 1; index <= 3; index += 1) { + git.createFile(`mobile-newer-base-${index}.txt`, `mobile newer base commit ${index}`); + git.stageFile(`mobile-newer-base-${index}.txt`); + git.commit(`Mobile newer base commit ${index}`); + } + git.exec("git push --force origin main"); + + git.exec(`git checkout -B ${branch} ${publishedHead}`); + git.exec("git rebase origin/main"); + const localHead = git.getCurrentSha(); + git.exec(`git branch --set-upstream-to=origin/${branch} ${branch}`); + return { localHead, publishedHead, commits }; +} + +function configureContributionRemote( + git: GitHelper, + remoteName: string, + sourceRemoteURL: string, + localRemoteURL: string, +): void { + const remotes = git.exec("git remote").split(/\r?\n/); + if (remotes.includes(remoteName)) { + git.exec(`git remote set-url ${remoteName} "${sourceRemoteURL}"`); + } else { + git.exec(`git remote add ${remoteName} "${sourceRemoteURL}"`); + } + git.exec(`git config url."${localRemoteURL}".insteadOf "${sourceRemoteURL}"`); +} + +function advanceProviderHead(git: GitHelper, previousHead: string, branch: string): string { + const tree = git.exec(`git rev-parse ${previousHead}^{tree}`).trim(); + const nextHead = git + .exec( + `git -c commit.gpgsign=false commit-tree ${tree} -p ${previousHead} -m "Mobile provider head advanced"`, + ) + .trim(); + git.exec(`git push --force origin ${nextHead}:refs/heads/${branch}`); + return nextHead; +} + +function recoveryBranches(git: GitHelper): string[] { + return git + .exec('git for-each-ref --format="%(refname:short)" refs/heads/kandev') + .split(/\r?\n/) + .map((branch) => branch.trim()) + .filter(Boolean); +} + +function restoreContributionHistoryRepository( + seedData: SeedData, + backend: { tmpDir: string }, +): void { + const git = new GitHelper(seedData.repositoryPath, makeGitEnv(backend.tmpDir)); + for (const command of ["git rebase --abort", "git merge --abort"]) { + try { + git.exec(command); + } catch { + // The fixture is normally idle when the test finishes. + } + } + git.exec("git checkout -f main"); + git.exec("git clean -fd"); + restoreSeedRepositoryOrigin(seedData); + git.exec(`git push --force origin ${seedData.repositoryBaselineOID}:refs/heads/main`); + git.exec("git fetch --no-tags origin main"); + git.exec(`git reset --hard ${seedData.repositoryBaselineOID}`); + git.exec("git clean -fd"); + for (const branch of ["feature/mobile-local-rebase", "kandev-e2e-mobile-published-history"]) { + try { + git.exec(`git branch -D ${branch}`); + } catch { + // Setup may have failed before creating this branch. + } + try { + git.exec(`git push origin --delete ${branch}`); + } catch { + // The branch may only exist locally. + } + } +} + async function swipeUpOnElement(page: Page, element: Locator): Promise { const box = await element.boundingBox(); if (!box) throw new Error("Changes scroll container has no bounding box"); @@ -86,6 +208,13 @@ async function swipeUpOnElement(page: Page, element: Locator): Promise { test.describe("Mobile rewritten contribution history", () => { test.describe.configure({ retries: 1, timeout: 120_000 }); + test.afterEach(({ backend, seedData }) => { + // Local-rebase scenarios rewrite origin/main and create temporary refs. + // Restore both sides after every test, including assertion failures, so a + // later worker test cannot inherit the synthetic provider history. + restoreContributionHistoryRepository(seedData, backend); + }); + test.beforeEach(({ backend, seedData }) => { const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); const git = new GitHelper(repoDir, makeGitEnv(backend.tmpDir)); @@ -108,14 +237,24 @@ test.describe("Mobile rewritten contribution history", () => { backend, prCapture, }) => { - // Phone composition extends through 767px. Exercise the range where an - // `sm:` reset would otherwise shrink touch controls before `md`. - await testPage.setViewportSize({ width: 700, height: 500 }); const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); const git = new GitHelper(repoDir, makeGitEnv(backend.tmpDir)); const localHead = seedStaleCheckout(git, seedData.repositoryRemoteURL); const providerBranch = "feature/mobile-rewritten"; const providerHistory = createRewrittenProviderHistory(git, providerBranch); + const contributionRepository = await apiClient.createRepository( + seedData.workspaceId, + repoDir, + "main", + { + name: "E2E GitHub Contribution Repository", + provider: "github", + provider_host: "https://github.com", + provider_owner: "testorg", + provider_name: "testrepo", + remote_url: "https://github.com/testorg/testrepo.git", + }, + ); const task = await apiClient.createTaskWithAgent( seedData.workspaceId, "Mobile Rewritten Contribution History", @@ -124,7 +263,8 @@ test.describe("Mobile rewritten contribution history", () => { description: "/e2e:simple-message", workflow_id: seedData.workflowId, workflow_step_id: seedData.startStepId, - repository_ids: [seedData.repositoryId], + repository_ids: [contributionRepository.id], + start_agent: false, }, ); await apiClient.mockGitHubReset(); @@ -140,8 +280,23 @@ test.describe("Mobile rewritten contribution history", () => { repo_owner: "testorg", repo_name: "testrepo", head_sha: providerHistory.head, + head_repo_owner: "testorg", + head_repo_name: "testrepo", + base_repo_owner: "testorg", + base_repo_name: "testrepo", + base_default_branch: "main", }, ]); + const contribution = await apiClient.attachE2EGitHubContribution( + task.id, + "https://github.com/testorg/testrepo/pull/902", + ); + configureContributionRemote( + git, + contribution.remote_name, + contribution.binding.source_repository.remote_url, + seedData.repositoryRemoteURL, + ); await apiClient.mockGitHubAddPRCommits("testorg", "testrepo", 902, providerHistory.commits); await apiClient.mockGitHubAssociateTaskPR({ task_id: task.id, @@ -154,6 +309,7 @@ test.describe("Mobile rewritten contribution history", () => { base_branch: "main", author_login: "mobile-remote-contributor", }); + await apiClient.ensureTaskSession(task.id); // The first provider-history request fails. The resource must retry it // without dropping the local checkout history from the final panel. await apiClient.mockGitHubSetPRCommitsFailures("testorg", "testrepo", 902, 1); @@ -180,7 +336,27 @@ test.describe("Mobile rewritten contribution history", () => { ).toHaveAttribute("aria-expanded", "false"); await expect(localSection.locator('[data-testid^="commit-row-"]')).toHaveCount(6); await expect(localSection.locator('[data-commit-provenance="local_checkout"]')).toHaveCount(6); - await providerSection.getByTestId("current-pr-commits-section-collapse-toggle").tap(); + const contributionWarning = changes.getByTestId("header-remote-contribution-warning"); + await expect(contributionWarning).toBeVisible(); + await contributionWarning.tap(); + const comparisonMenu = testPage.getByTestId("header-remote-contribution-menu"); + await expect(comparisonMenu).toBeVisible(); + const canonicalDrawer = testPage.locator('[data-slot="drawer-content"]'); + const canonicalDrawerBox = await canonicalDrawer.boundingBox(); + expect(canonicalDrawerBox).not.toBeNull(); + const canonicalViewport = await testPage.evaluate(() => ({ height: innerHeight })); + expect(canonicalDrawerBox!.y).toBeGreaterThan(0); + expect(canonicalDrawerBox!.height).toBeLessThan(canonicalViewport.height); + await expect(comparisonMenu.getByRole("button", { name: "Close" })).toBeVisible(); + await expect(comparisonMenu.getByTestId("header-compare-versions")).toBeVisible(); + await comparisonMenu.getByTestId("header-compare-versions").tap(); + await expect(comparisonMenu).toHaveCount(0); + const providerToggle = providerSection.getByTestId( + "current-pr-commits-section-collapse-toggle", + ); + await expect(providerToggle).toHaveAttribute("aria-expanded", "true"); + await expect(providerToggle).toBeFocused(); + expect(git.getCurrentSha()).toBe(localHead); await expect(providerSection.locator('[data-commit-provenance="current_pr"]')).toHaveCount(15); await expect( providerSection.locator('[data-commit-provenance="current_pr"]').first(), @@ -207,18 +383,27 @@ test.describe("Mobile rewritten contribution history", () => { await swipeUpOnElement(testPage, scroller); await expect.poll(() => scroller.evaluate((element) => element.scrollTop)).toBeGreaterThan(0); - const contributionWarning = changes.getByTestId("header-remote-contribution-warning"); - await expect(contributionWarning).toBeVisible(); const warningBox = await contributionWarning.boundingBox(); expect(warningBox).not.toBeNull(); expect(Math.round(warningBox!.width)).toBeGreaterThanOrEqual(44); expect(Math.round(warningBox!.height)).toBeGreaterThanOrEqual(44); - await contributionWarning.tap(); + // Phone composition extends through 767px. Exercise the wider touch range + // where an `sm:` reset could otherwise shrink touch controls before `md`. + await testPage.setViewportSize({ width: 700, height: 500 }); + await contributionWarning.click(); const openMenu = testPage.getByTestId("header-remote-contribution-menu"); await expect(openMenu).toHaveCount(1); + const wideDrawer = testPage.locator('[data-slot="drawer-content"]'); + const wideDrawerBox = await wideDrawer.boundingBox(); + expect(wideDrawerBox).not.toBeNull(); + expect(wideDrawerBox!.y).toBeGreaterThan(0); + expect(wideDrawerBox!.height).toBeLessThan(500); + await expect(openMenu.getByRole("button", { name: "Close" })).toBeVisible(); await expect(openMenu.getByTestId("header-replace-pr-branch")).toBeVisible(); await expect(openMenu.getByTestId("header-use-pr-version")).toBeVisible(); - await expect(openMenu.getByTestId("header-view-pr-version")).toContainText("PR #902 version"); + await expect(openMenu.getByTestId("header-view-pr-version")).toContainText("Open PR on GitHub"); + await expect(openMenu).toContainText("Replace the published PR history"); + await expect(openMenu).toContainText("Replace the task checkout history"); await expect(openMenu.locator('[data-slot="dropdown-menu-sub-trigger"]')).toHaveCount(0); await openMenu.getByTestId("header-replace-pr-branch").tap(); const dialog = testPage.getByTestId("remote-contribution-resolution-dialog"); @@ -232,7 +417,44 @@ test.describe("Mobile rewritten contribution history", () => { await expect .poll(async () => Math.round((await confirm.boundingBox())?.height ?? 0)) .toBeGreaterThanOrEqual(44); + const changedProviderHead = advanceProviderHead(git, providerHistory.head, providerBranch); + await apiClient.mockGitHubAddPRs([ + { + number: 902, + title: "Mobile rewritten contribution", + state: "open", + head_branch: providerBranch, + base_branch: "main", + author_login: "mobile-remote-contributor", + repo_owner: "testorg", + repo_name: "testrepo", + head_sha: changedProviderHead, + head_repo_owner: "testorg", + head_repo_name: "testrepo", + base_repo_owner: "testorg", + base_repo_name: "testrepo", + base_default_branch: "main", + }, + ]); + await apiClient.mockGitHubAddPRCommits("testorg", "testrepo", 902, [ + { + sha: changedProviderHead, + message: "Mobile provider head advanced", + author_login: "mobile-remote-contributor", + author_date: "2026-08-20T12:00:00Z", + stats_available: false, + }, + ]); + await confirm.tap(); + await expect(dialog).toContainText("The PR changed again"); + await expect(dialog).toContainText(changedProviderHead); + expect(git.getCurrentSha()).toBe(localHead); + expect(git.exec("git status --porcelain").trim()).toBe(""); + expect(git.exec(`git ls-remote origin refs/heads/${providerBranch}`)).toContain( + changedProviderHead, + ); await cancel.tap(); + await expect(dialog).toHaveCount(0); expect(git.getCurrentSha()).toBe(localHead); expect(git.exec("git status --porcelain").trim()).toBe(""); @@ -246,4 +468,172 @@ test.describe("Mobile rewritten contribution history", () => { "700px touch Changes preserves the local checkout and offers provider version choices", }); }); + + test("shows a real local-rebase explanation on the canonical phone viewport", async ({ + testPage, + apiClient, + seedData, + backend, + prCapture, + }) => { + const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); + const git = new GitHelper(repoDir, makeGitEnv(backend.tmpDir)); + const providerBranch = "feature/mobile-local-rebase"; + const history = createLocalRebasedProviderHistory(git, providerBranch); + // Start session preparation from the published branch. The real rebase is + // recreated after the session owns the contribution binding. + git.exec(`git checkout -B ${providerBranch} ${history.publishedHead}`); + git.exec(`git branch --set-upstream-to=origin/${providerBranch} ${providerBranch}`); + const contributionRepository = await apiClient.createRepository( + seedData.workspaceId, + repoDir, + "main", + { + name: "E2E GitHub Contribution Repository", + provider: "github", + provider_host: "https://github.com", + provider_owner: "testorg", + provider_name: "testrepo", + remote_url: "https://github.com/testorg/testrepo.git", + }, + ); + await apiClient.mockGitHubReset(); + await apiClient.mockGitHubSetUser("mobile-local-rebase-contributor"); + await apiClient.mockGitHubAddPRs([ + { + number: 905, + title: "Mobile local rebase explanation", + state: "open", + head_branch: providerBranch, + base_branch: "main", + author_login: "mobile-local-rebase-contributor", + repo_owner: "testorg", + repo_name: "testrepo", + head_sha: history.publishedHead, + head_repo_owner: "testorg", + head_repo_name: "testrepo", + base_repo_owner: "testorg", + base_repo_name: "testrepo", + base_default_branch: "main", + }, + ]); + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Mobile Local Rebase Explanation", + seedData.agentProfileId, + { + description: "/e2e:simple-message", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [contributionRepository.id], + start_agent: false, + }, + ); + + const contribution = await apiClient.attachE2EGitHubContribution( + task.id, + "https://github.com/testorg/testrepo/pull/905", + ); + configureContributionRemote( + git, + contribution.remote_name, + contribution.binding.source_repository.remote_url, + seedData.repositoryRemoteURL, + ); + await apiClient.mockGitHubAddPRCommits("testorg", "testrepo", 905, history.commits); + await apiClient.mockGitHubAssociateTaskPR({ + task_id: task.id, + owner: "testorg", + repo: "testrepo", + pr_number: 905, + pr_url: "https://github.com/testorg/testrepo/pull/905", + pr_title: "Mobile local rebase explanation", + head_branch: providerBranch, + base_branch: "main", + author_login: "mobile-local-rebase-contributor", + }); + await apiClient.ensureTaskSession(task.id); + + await testPage.goto(`/t/${task.id}`); + const session = new SessionPage(testPage); + await session.waitForLoad(); + await session.waitForChatIdle({ timeout: 45_000 }); + + git.exec(`git checkout -B ${providerBranch} ${history.publishedHead}`); + git.exec("git rebase origin/main"); + history.localHead = git.getCurrentSha(); + git.exec(`git branch --set-upstream-to=origin/${providerBranch} ${providerBranch}`); + + await testPage.reload(); + await session.waitForLoad(); + await session.waitForChatIdle({ timeout: 45_000 }); + await testPage.getByRole("button", { name: "Changes" }).tap(); + + const changes = testPage.getByTestId("mobile-changes-panel"); + const providerSection = changes.getByTestId("current-pr-commits-section"); + const localSection = changes.getByTestId("local-checkout-commits-section"); + await expect(providerSection).toBeVisible({ timeout: 15_000 }); + await expect(localSection).toBeVisible({ timeout: 15_000 }); + + const warning = changes.getByTestId("header-remote-contribution-warning"); + await warning.tap(); + const menu = testPage.getByTestId("header-remote-contribution-menu"); + await expect(menu).toBeVisible(); + await expect(menu).toContainText("Task and PR histories differ"); + await expect(menu).toContainText("The task branch was rebased locally"); + await expect(menu).toContainText("Task commits: 5"); + await expect(menu).toContainText("Published commits: 5"); + await expect(menu).toContainText("Newer base commits: 3"); + await expect(menu.getByRole("button", { name: "Close" })).toBeVisible(); + await waitForFiniteAnimations(menu); + await prCapture.screenshot("remote-contribution-menu-mobile", { + caption: "Phone history drawer explains the local rebase and version choices", + }); + + await menu.getByTestId("header-compare-versions").tap(); + await expect(menu).toHaveCount(0); + await expect( + providerSection.getByTestId("current-pr-commits-section-collapse-toggle"), + ).toHaveAttribute("aria-expanded", "true"); + await expect( + providerSection.getByTestId("current-pr-commits-section-collapse-toggle"), + ).toBeFocused(); + await expect(providerSection.locator('[data-testid^="commit-row-"]')).toHaveCount(5); + expect(git.getCurrentSha()).toBe(history.localHead); + expect(git.exec("git status --porcelain").trim()).toBe(""); + await expect( + testPage.evaluate( + () => document.documentElement.scrollWidth <= document.documentElement.clientWidth, + ), + ).resolves.toBe(true); + + await warning.tap(); + const resolutionMenu = testPage.getByTestId("header-remote-contribution-menu"); + await expect(resolutionMenu).toBeVisible(); + const recoveryBefore = recoveryBranches(git); + git.createFile("mobile-local-rebase-dirty.txt", "uncommitted mobile local change"); + await resolutionMenu.getByTestId("header-use-pr-version").tap(); + const dialog = testPage.getByTestId("remote-contribution-resolution-dialog"); + await expect(dialog).toContainText("replaces the current task checkout history"); + await expect(dialog).toContainText(history.publishedHead); + await dialog.getByTestId("remote-contribution-confirm").tap(); + await expect(dialog).toContainText("Commit or discard your local changes"); + expect(git.getCurrentSha()).toBe(history.localHead); + expect(git.exec("git status --porcelain").trim()).toContain("mobile-local-rebase-dirty.txt"); + expect(recoveryBranches(git)).toEqual(recoveryBefore); + + git.deleteFile("mobile-local-rebase-dirty.txt"); + await dialog.getByTestId("remote-contribution-confirm").tap(); + await expect(dialog).toHaveCount(0); + expect(git.getCurrentSha()).toBe(history.publishedHead); + expect(git.exec("git status --porcelain").trim()).toBe(""); + const recoveryAfter = recoveryBranches(git); + const createdRecoveryBranches = recoveryAfter.filter( + (branch) => !recoveryBefore.includes(branch), + ); + expect(createdRecoveryBranches).toHaveLength(1); + expect(git.exec(`git rev-parse refs/heads/${createdRecoveryBranches[0]}`).trim()).toBe( + history.localHead, + ); + }); }); diff --git a/apps/web/hooks/domains/session/use-contribution-history-explanation.test.tsx b/apps/web/hooks/domains/session/use-contribution-history-explanation.test.tsx new file mode 100644 index 0000000000..5b4857df14 --- /dev/null +++ b/apps/web/hooks/domains/session/use-contribution-history-explanation.test.tsx @@ -0,0 +1,241 @@ +import { render, renderHook, waitFor } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { + resetContributionHistoryExplanationForTests, + useContributionHistoryExplanation, + type ContributionHistoryExplanationTarget, +} from "./use-contribution-history-explanation"; + +const requestMock = vi.hoisted(() => vi.fn()); + +vi.mock("@/lib/ws/connection", () => ({ + getWebSocketClient: () => ({ request: requestMock }), +})); + +type Deferred = { + promise: Promise; + resolve: (value: T) => void; + reject: (error: unknown) => void; +}; + +function deferred(): Deferred { + let resolve!: (value: T) => void; + let reject!: (error: unknown) => void; + const promise = new Promise((resolvePromise, rejectPromise) => { + resolve = resolvePromise; + reject = rejectPromise; + }); + return { promise, resolve, reject }; +} + +const target: ContributionHistoryExplanationTarget = { + sessionId: "session-1", + workspaceId: "workspace-1", + repositoryScope: "frontend", + branch: "feature/one", + selectedPRKey: "acme/frontend/42", + expectedLocalHead: "a".repeat(40), + expectedRemoteHead: "b".repeat(40), +}; + +const explanation = { + repo: "frontend", + branch: "feature/one", + expected_local_head: target.expectedLocalHead, + expected_remote_head: target.expectedRemoteHead, + kind: "local_rebase", + reason: "matched_reflog", + onto_head: "c".repeat(40), + task_commit_count: 5, + published_commit_count: 5, + new_base_commit_count: 29, +} as const; + +function SnapshotProbe({ + target: currentTarget, + enabled, + onRender, +}: { + target: ContributionHistoryExplanationTarget; + enabled: boolean; + onRender: (snapshot: ReturnType) => void; +}) { + const snapshot = useContributionHistoryExplanation(currentTarget, enabled); + onRender(snapshot); + return null; +} + +beforeEach(() => { + requestMock.mockReset(); + resetContributionHistoryExplanationForTests(); +}); + +afterEach(() => { + resetContributionHistoryExplanationForTests(); +}); + +describe("useContributionHistoryExplanation", () => { + it("does not observe history until a confirmed explanation surface opens", () => { + const hook = renderHook(({ enabled }) => useContributionHistoryExplanation(target, enabled), { + initialProps: { enabled: false }, + }); + + expect(requestMock).not.toHaveBeenCalled(); + + hook.rerender({ enabled: true }); + + expect(requestMock).toHaveBeenCalledOnce(); + expect(requestMock).toHaveBeenCalledWith( + "worktree.contribution_history_explanation", + { + session_id: "session-1", + repo: "frontend", + branch: "feature/one", + expected_local_head: target.expectedLocalHead, + expected_remote_head: target.expectedRemoteHead, + }, + expect.any(Number), + ); + }); + + it("shares one request and result across mounted consumers with the same identity", async () => { + const pending = deferred(); + requestMock.mockReturnValue(pending.promise); + const first = renderHook(() => useContributionHistoryExplanation(target, true)); + const second = renderHook(() => useContributionHistoryExplanation(target, true)); + + expect(requestMock).toHaveBeenCalledOnce(); + pending.resolve(explanation); + + await waitFor(() => { + expect(first.result.current.explanation).toEqual(explanation); + expect(second.result.current.explanation).toEqual(explanation); + expect(first.result.current.status).toBe("ready"); + expect(second.result.current.status).toBe("ready"); + }); + }); + + it("releases the result when the last consumer dismisses the explanation", async () => { + requestMock.mockResolvedValue(explanation); + const hook = renderHook(({ enabled }) => useContributionHistoryExplanation(target, enabled), { + initialProps: { enabled: true }, + }); + + await waitFor(() => expect(hook.result.current.status).toBe("ready")); + hook.rerender({ enabled: false }); + hook.rerender({ enabled: true }); + + await waitFor(() => expect(requestMock).toHaveBeenCalledTimes(2)); + }); + + it("discards a response after the requested head identity changes", async () => { + const oldRequest = deferred(); + const newRequest = deferred(); + requestMock.mockReturnValueOnce(oldRequest.promise).mockReturnValueOnce(newRequest.promise); + const hook = renderHook( + ({ currentTarget }) => useContributionHistoryExplanation(currentTarget, true), + { initialProps: { currentTarget: target } }, + ); + + const nextTarget = { ...target, expectedLocalHead: "d".repeat(40) }; + hook.rerender({ currentTarget: nextTarget }); + oldRequest.resolve(explanation); + + await waitFor(() => expect(requestMock).toHaveBeenCalledTimes(2)); + expect(hook.result.current.explanation).toBeNull(); + + newRequest.resolve({ + ...explanation, + expected_local_head: nextTarget.expectedLocalHead, + }); + await waitFor(() => + expect(hook.result.current.explanation?.expected_local_head).toBe( + nextTarget.expectedLocalHead, + ), + ); + }); + + it("returns a neutral snapshot during a provider refresh render", async () => { + const pending = deferred(); + requestMock.mockReturnValueOnce(pending.promise).mockResolvedValueOnce({ + ...explanation, + expected_remote_head: "e".repeat(40), + }); + const renders: Array> = []; + const view = render( + renders.push(snapshot)} />, + ); + pending.resolve(explanation); + + await waitFor(() => expect(renders.at(-1)?.status).toBe("ready")); + const nextTarget = { ...target, expectedRemoteHead: "e".repeat(40) }; + const renderCountBeforeRefresh = renders.length; + view.rerender( + renders.push(snapshot)} />, + ); + + expect(renders[renderCountBeforeRefresh]?.status).toBe("idle"); + expect(renders[renderCountBeforeRefresh]?.explanation).toBeNull(); + await waitFor(() => expect(renders.at(-1)?.status).toBe("ready")); + expect(renders.at(-1)?.explanation?.expected_remote_head).toBe(nextTarget.expectedRemoteHead); + }); +}); + +describe("useContributionHistoryExplanation identity reuse", () => { + it("does not restart an identical request for a new target object", () => { + const pending = deferred(); + requestMock.mockReturnValue(pending.promise); + const hook = renderHook( + ({ currentTarget }) => useContributionHistoryExplanation(currentTarget, true), + { initialProps: { currentTarget: target } }, + ); + + hook.rerender({ currentTarget: { ...target } }); + + expect(requestMock).toHaveBeenCalledOnce(); + }); + + it("shares an observation when only the selected PR identity changes", () => { + const pending = deferred(); + requestMock.mockReturnValue(pending.promise); + const hook = renderHook( + ({ currentTarget }) => useContributionHistoryExplanation(currentTarget, true), + { initialProps: { currentTarget: target } }, + ); + + hook.rerender({ + currentTarget: { ...target, selectedPRKey: "acme/frontend/43" }, + }); + + expect(requestMock).toHaveBeenCalledOnce(); + }); + + it("keeps separate repository and PR identities isolated", () => { + requestMock.mockResolvedValue(explanation); + const otherTarget = { + ...target, + repositoryScope: "backend", + selectedPRKey: "acme/backend/42", + }; + + renderHook(() => useContributionHistoryExplanation(target, true)); + renderHook(() => useContributionHistoryExplanation(otherTarget, true)); + + expect(requestMock).toHaveBeenCalledTimes(2); + expect(requestMock.mock.calls.map(([action, payload]) => [action, payload])).toEqual([ + ["worktree.contribution_history_explanation", expect.objectContaining({ repo: "frontend" })], + ["worktree.contribution_history_explanation", expect.objectContaining({ repo: "backend" })], + ]); + }); + + it("does not publish a late result after all consumers cancel", async () => { + const pending = deferred(); + requestMock.mockReturnValue(pending.promise); + const hook = renderHook(() => useContributionHistoryExplanation(target, true)); + hook.unmount(); + pending.resolve(explanation); + + await Promise.resolve(); + expect(requestMock).toHaveBeenCalledOnce(); + }); +}); diff --git a/apps/web/hooks/domains/session/use-contribution-history-explanation.ts b/apps/web/hooks/domains/session/use-contribution-history-explanation.ts new file mode 100644 index 0000000000..d7d9f58a88 --- /dev/null +++ b/apps/web/hooks/domains/session/use-contribution-history-explanation.ts @@ -0,0 +1,255 @@ +"use client"; + +import { useEffect, useMemo, useState } from "react"; +import { getWebSocketClient } from "@/lib/ws/connection"; +import { isFullCommitSHA } from "./remote-contribution-relation"; + +export type ContributionHistoryExplanationKind = "local_rebase" | "unexplained"; + +export type ContributionHistoryExplanationReason = + | "matched_reflog" + | "no_matching_reflog" + | "missing_objects" + | "ambiguous" + | "stale" + | "limit" + | "unavailable"; + +export type ContributionHistoryExplanation = { + repo?: string; + branch: string; + expected_local_head: string; + expected_remote_head: string; + kind: ContributionHistoryExplanationKind; + reason: ContributionHistoryExplanationReason; + onto_head?: string; + task_commit_count?: number; + published_commit_count?: number; + new_base_commit_count?: number; +}; + +export type ContributionHistoryExplanationTarget = { + sessionId: string; + workspaceId: string; + repositoryScope: string; + branch: string; + selectedPRKey: string; + expectedLocalHead: string; + expectedRemoteHead: string; +}; + +type ContributionHistoryRequestTarget = Omit; + +export type ContributionHistoryExplanationStatus = "idle" | "loading" | "ready" | "unavailable"; + +type ExplanationSnapshot = { + status: ContributionHistoryExplanationStatus; + explanation: ContributionHistoryExplanation | null; + error: unknown; +}; + +type ExplanationEntry = ExplanationSnapshot & { + listeners: Set<() => void>; +}; + +type BoundExplanationSnapshot = { + key: string | null; + snapshot: ExplanationSnapshot; +}; + +const ACTION = "worktree.contribution_history_explanation"; +const REQUEST_TIMEOUT_MS = 2_500; +const entries = new Map(); + +function emptySnapshot(): ExplanationSnapshot { + return { status: "idle", explanation: null, error: null }; +} + +function notify(entry: ExplanationEntry) { + for (const listener of entry.listeners) listener(); +} + +function targetIsRequestable( + target: ContributionHistoryExplanationTarget | null, +): target is ContributionHistoryExplanationTarget { + return Boolean( + target?.sessionId && + target.workspaceId && + target.repositoryScope !== undefined && + target.branch && + target.selectedPRKey && + isFullCommitSHA(target.expectedLocalHead) && + isFullCommitSHA(target.expectedRemoteHead), + ); +} + +export function contributionHistoryExplanationKey( + target: ContributionHistoryExplanationTarget | null, +): string | null { + if (!targetIsRequestable(target)) return null; + return JSON.stringify([ + target.workspaceId, + target.sessionId, + target.repositoryScope, + target.branch, + target.expectedLocalHead, + target.expectedRemoteHead, + ]); +} + +function responseMatchesTarget( + response: ContributionHistoryExplanation, + target: ContributionHistoryRequestTarget, +): boolean { + return ( + (response.repo ?? "") === target.repositoryScope && + response.branch === target.branch && + response.expected_local_head === target.expectedLocalHead && + response.expected_remote_head === target.expectedRemoteHead + ); +} + +function startEntry(key: string, target: ContributionHistoryRequestTarget): ExplanationEntry { + const entry: ExplanationEntry = { + ...emptySnapshot(), + status: "loading", + listeners: new Set(), + }; + entries.set(key, entry); + + const client = getWebSocketClient(); + if (!client) { + entry.status = "unavailable"; + return entry; + } + + try { + const request = client.request( + ACTION, + { + session_id: target.sessionId, + repo: target.repositoryScope, + branch: target.branch, + expected_local_head: target.expectedLocalHead, + expected_remote_head: target.expectedRemoteHead, + }, + REQUEST_TIMEOUT_MS, + ); + void request.then( + (response) => { + if (entries.get(key) !== entry) return; + if (!responseMatchesTarget(response, target)) { + entry.status = "unavailable"; + entry.error = new Error("contribution history explanation identity changed"); + } else { + entry.status = "ready"; + entry.explanation = response; + } + notify(entry); + }, + (error: unknown) => { + if (entries.get(key) !== entry) return; + entry.status = "unavailable"; + entry.error = error; + notify(entry); + }, + ); + } catch (error) { + entry.status = "unavailable"; + entry.error = error; + } + return entry; +} + +function getOrStartEntry(key: string, target: ContributionHistoryRequestTarget): ExplanationEntry { + return entries.get(key) ?? startEntry(key, target); +} + +function snapshotOf(entry: ExplanationEntry): ExplanationSnapshot { + return { + status: entry.status, + explanation: entry.explanation, + error: entry.error, + }; +} + +export function useContributionHistoryExplanation( + target: ContributionHistoryExplanationTarget | null, + enabled: boolean, +) { + const requestTarget = useMemo( + () => ({ + sessionId: target?.sessionId ?? "", + workspaceId: target?.workspaceId ?? "", + repositoryScope: target?.repositoryScope ?? "", + branch: target?.branch ?? "", + selectedPRKey: target?.selectedPRKey ?? "", + expectedLocalHead: target?.expectedLocalHead ?? "", + expectedRemoteHead: target?.expectedRemoteHead ?? "", + }), + [ + target?.sessionId, + target?.workspaceId, + target?.repositoryScope, + target?.branch, + target?.selectedPRKey, + target?.expectedLocalHead, + target?.expectedRemoteHead, + ], + ); + const requestFields = useMemo( + () => ({ + sessionId: requestTarget.sessionId, + workspaceId: requestTarget.workspaceId, + repositoryScope: requestTarget.repositoryScope, + branch: requestTarget.branch, + expectedLocalHead: requestTarget.expectedLocalHead, + expectedRemoteHead: requestTarget.expectedRemoteHead, + }), + [ + requestTarget.sessionId, + requestTarget.workspaceId, + requestTarget.repositoryScope, + requestTarget.branch, + requestTarget.expectedLocalHead, + requestTarget.expectedRemoteHead, + ], + ); + const key = enabled ? contributionHistoryExplanationKey(requestTarget) : null; + const [boundSnapshot, setBoundSnapshot] = useState(() => ({ + key: null, + snapshot: emptySnapshot(), + })); + const snapshot = boundSnapshot.key === key ? boundSnapshot.snapshot : emptySnapshot(); + + useEffect(() => { + if (!key) { + setBoundSnapshot({ key: null, snapshot: emptySnapshot() }); + return; + } + + const entry = getOrStartEntry(key, requestFields); + let active = true; + const sync = () => { + if (active) setBoundSnapshot({ key, snapshot: snapshotOf(entry) }); + }; + entry.listeners.add(sync); + sync(); + + return () => { + active = false; + entry.listeners.delete(sync); + if (entry.listeners.size === 0) entries.delete(key); + }; + }, [key, requestFields]); + + return { + ...snapshot, + isLoading: snapshot.status === "loading", + }; +} + +/** Test-only registry cleanup. Mounted consumers keep production entries alive. */ +export function resetContributionHistoryExplanationForTests() { + entries.clear(); +} diff --git a/apps/web/hooks/domains/session/use-remote-contribution-relation.test.tsx b/apps/web/hooks/domains/session/use-remote-contribution-relation.test.tsx index 950a43f9ac..06a7b6eb51 100644 --- a/apps/web/hooks/domains/session/use-remote-contribution-relation.test.tsx +++ b/apps/web/hooks/domains/session/use-remote-contribution-relation.test.tsx @@ -123,6 +123,26 @@ function status( }; } +function expectSelectedRepositoryState(current: ReturnType) { + expect(current.relation).toMatchObject({ + kind: "aligned", + canPush: false, + canPull: false, + canReplaceRemote: false, + canUseRemote: false, + }); + expect(current.contributionHistoryTarget).toMatchObject({ + sessionId: "session-1", + workspaceId: "workspace-1", + repositoryScope: "frontend", + branch: "feature", + selectedPRKey: "acme/frontend/2", + expectedLocalHead: mocks.providerHead, + expectedRemoteHead: mocks.providerHead, + }); + expect(current.repositoryScope).toBe("frontend"); +} + describe("useRemoteContributionRelation repository scoping", () => { beforeEach(() => { mocks.selectedPR = selectedPR; @@ -151,14 +171,7 @@ describe("useRemoteContributionRelation repository scoping", () => { ); const { result } = renderHook(() => useRemoteContributionRelation("session-1")); - - expect(result.current.relation).toMatchObject({ - kind: "aligned", - canPush: false, - canPull: false, - canReplaceRemote: false, - canUseRemote: false, - }); + expectSelectedRepositoryState(result.current); }); it("uses the empty-key status for a single-repository session", () => { @@ -173,6 +186,8 @@ describe("useRemoteContributionRelation repository scoping", () => { canReplaceRemote: false, canUseRemote: false, }); + expect(result.current.contributionHistoryTarget?.repositoryScope).toBe(""); + expect(result.current.repositoryScope).toBe(""); }); // @covers AC-TASKS-REMOTE-CONTRIBUTION-TASKS-001.7 diff --git a/apps/web/hooks/domains/session/use-remote-contribution-relation.ts b/apps/web/hooks/domains/session/use-remote-contribution-relation.ts index 0c57bc18b8..fb098b42dd 100644 --- a/apps/web/hooks/domains/session/use-remote-contribution-relation.ts +++ b/apps/web/hooks/domains/session/use-remote-contribution-relation.ts @@ -8,22 +8,60 @@ import { usePRReviewRepositoryIdentityResolver } from "@/hooks/domains/github/us import { useReviewPRSelection } from "@/hooks/domains/github/use-review-pr-selection"; import { useSessionGitStatusByRepo } from "./use-session-git-status"; import { resolveBranchScopedTaskPRs, selectBranchScopedTaskPR } from "./branch-scoped-task-pr"; +import { prTaskKey } from "@/components/github/pr-utils"; import { classifyRemoteContribution, type RemoteContributionRelation, } from "./remote-contribution-relation"; +import type { ContributionHistoryExplanationTarget } from "./use-contribution-history-explanation"; export type RemoteContributionRelationState = { prs: TaskPR[]; selectedPR: TaskPR | null; + /** The repository key used by Git operations. Empty means the single-repo root. */ + repositoryScope: string; repositoryName: string | undefined; commits: PRCommitInfo[]; loading: boolean; error: string | null; relation: RemoteContributionRelation; refreshProviderEvidence: () => Promise; + contributionHistoryTarget: ContributionHistoryExplanationTarget | null; }; +function buildContributionHistoryTarget( + sessionId: string | null | undefined, + selectedPR: TaskPR | null, + gitStatus: ReturnType[number]["status"] | undefined, + providerHead: string | null | undefined, +): ContributionHistoryExplanationTarget | null { + if (!sessionId || !selectedPR || !gitStatus?.branch) return null; + return { + sessionId, + workspaceId: selectedPR.workspace_id, + repositoryScope: gitStatus.repository_name ?? "", + branch: gitStatus.branch, + selectedPRKey: prTaskKey(selectedPR), + expectedLocalHead: gitStatus.head_commit ?? "", + expectedRemoteHead: providerHead ?? "", + }; +} + +function classifyContributionRelation( + input: Parameters[0], +): RemoteContributionRelation { + return classifyRemoteContribution(input); +} + +function useSelectedPRCommits(selectedPR: TaskPR | null) { + return usePRCommits( + selectedPR?.owner ?? null, + selectedPR?.repo ?? null, + selectedPR?.pr_number ?? null, + selectedPR?.last_synced_at ?? null, + ); +} + export function useRemoteContributionRelation( sessionId: string | null | undefined, ): RemoteContributionRelationState { @@ -47,22 +85,23 @@ export function useRemoteContributionRelation( ); const scopedPRs = useMemo(() => branchScopedPRs.map((entry) => entry.pr), [branchScopedPRs]); const selectedPR = selection?.pr ?? null; - const commitsState = usePRCommits( - selectedPR?.owner ?? null, - selectedPR?.repo ?? null, - selectedPR?.pr_number ?? null, - selectedPR?.last_synced_at ?? null, - ); + const commitsState = useSelectedPRCommits(selectedPR); const repositoryName = selection?.repositoryName; + const repositoryScope = selection?.gitStatus.repository_name ?? ""; const refreshProviderEvidence = useCallback(async () => { const refreshed = await commitsState.refresh(); return refreshed?.providerHead ?? null; }, [commitsState.refresh]); const gitStatus = selection?.gitStatus; + const contributionHistoryTarget = useMemo( + () => + buildContributionHistoryTarget(sessionId, selectedPR, gitStatus, commitsState.providerHead), + [sessionId, selectedPR, gitStatus, commitsState.providerHead], + ); const relation = useMemo( () => - classifyRemoteContribution({ + classifyContributionRelation({ hasSelectedPR: Boolean(selectedPR), providerCommits: commitsState.authoritativeCommits, providerHead: commitsState.providerHead, @@ -90,11 +129,13 @@ export function useRemoteContributionRelation( return { prs: scopedPRs, selectedPR, + repositoryScope, repositoryName, commits: commitsState.commits, loading: commitsState.loading, error: commitsState.error, relation, refreshProviderEvidence, + contributionHistoryTarget, }; } diff --git a/apps/web/hooks/domains/session/use-session-messages.test.ts b/apps/web/hooks/domains/session/use-session-messages.test.ts index 47a729f07a..1a718ddc4f 100644 --- a/apps/web/hooks/domains/session/use-session-messages.test.ts +++ b/apps/web/hooks/domains/session/use-session-messages.test.ts @@ -371,6 +371,91 @@ describe("cached session entry history readiness", () => { await response.promise; }); expect(result.current.historyRefreshPending).toBe(false); + expect(result.current.isLoading).toBe(false); + unmount(); + }); +}); + +describe("session entry loading state", () => { + it("clears local loading after a live row rerenders the entry effect", async () => { + const response = deferred<{ messages: Message[]; has_more: boolean }>(); + const subscriptionReadiness = deferred(); + mockWebSocketClient.getSessionSubscriptionReadiness.mockReturnValue(Promise.resolve()); + mockWebSocketClient.subscribeSessionWithReady.mockReturnValue({ + ready: subscriptionReadiness.promise, + unsubscribe: vi.fn(), + }); + mockWebSocketClient.request.mockReturnValue(response.promise); + mockState.messages.bySession["sess-1"] = [makeMessage({ id: "cached" })]; + + const { result, rerender, unmount } = renderHook(() => useSessionMessages("sess-1")); + await waitFor(() => expect(mockWebSocketClient.request).toHaveBeenCalledTimes(1)); + + mockState.messages.bySession["sess-1"] = [ + makeMessage({ id: "cached" }), + makeMessage({ id: "live" }), + ]; + rerender(); + + await act(async () => { + response.resolve({ messages: [], has_more: false }); + await response.promise; + }); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + unmount(); + }); + + it("keeps a cached session switch fetch current across live message updates", async () => { + const sessionAResponse = deferred<{ messages: Message[]; has_more: boolean }>(); + const sessionBResponse = deferred<{ messages: Message[]; has_more: boolean }>(); + const sessionBSubscriptionReadiness = deferred(); + const sessionAReadiness = Promise.resolve(); + const messagesBySession = mockState.messages.bySession as Record; + const sessions = mockState.taskSessions.items as Record; + const sessionAUser = makeMessage({ id: "session-a-user", session_id: sessionId("sess-a") }); + const sessionBUser = makeMessage({ id: "session-b-user", session_id: sessionId("sess-b") }); + messagesBySession["sess-a"] = [sessionAUser]; + messagesBySession["sess-b"] = [sessionBUser]; + sessions["sess-a"] = { state: "RUNNING" }; + sessions["sess-b"] = { state: "RUNNING" }; + mockWebSocketClient.getSessionSubscriptionReadiness.mockImplementation((id: string) => + id === "sess-a" ? sessionAReadiness : Promise.resolve(), + ); + mockWebSocketClient.subscribeSessionWithReady.mockImplementation((id: string) => ({ + ready: id === "sess-a" ? sessionAReadiness : sessionBSubscriptionReadiness.promise, + unsubscribe: vi.fn(), + })); + mockWebSocketClient.request.mockImplementation( + (_action: string, payload: { session_id: string }) => + payload.session_id === "sess-a" ? sessionAResponse.promise : sessionBResponse.promise, + ); + + const { result, rerender, unmount } = renderHook( + ({ activeSessionId }: { activeSessionId: string }) => useSessionMessages(activeSessionId), + { initialProps: { activeSessionId: "sess-a" } }, + ); + await waitFor(() => expect(mockWebSocketClient.request).toHaveBeenCalledTimes(1)); + await act(async () => { + sessionAResponse.resolve({ messages: [sessionAUser], has_more: false }); + await sessionAResponse.promise; + }); + + rerender({ activeSessionId: "sess-b" }); + await waitFor(() => expect(mockWebSocketClient.request).toHaveBeenCalledTimes(2)); + + messagesBySession["sess-b"] = [ + sessionBUser, + makeMessage({ id: "session-b-live", session_id: sessionId("sess-b") }), + ]; + rerender({ activeSessionId: "sess-b" }); + + await act(async () => { + sessionBResponse.resolve({ messages: [sessionBUser], has_more: false }); + await sessionBResponse.promise; + }); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); unmount(); }); }); diff --git a/apps/web/hooks/domains/session/use-session-messages.ts b/apps/web/hooks/domains/session/use-session-messages.ts index bbe85a1c65..6b22fb4f8b 100644 --- a/apps/web/hooks/domains/session/use-session-messages.ts +++ b/apps/web/hooks/domains/session/use-session-messages.ts @@ -381,7 +381,8 @@ function useTerminalStateFetch( useEffect(() => { let active = true; const generation = sessionFetchGenerationRef.current; - const isActive = () => active && sessionFetchGenerationRef.current === generation; + const isCurrentGeneration = () => sessionFetchGenerationRef.current === generation; + const isActive = () => active && isCurrentGeneration(); const deactivate = () => { active = false; }; @@ -397,7 +398,7 @@ function useTerminalStateFetch( ...refs, fetchAndStoreMessages, isActive, - canFinalizeLoading: isActive, + canFinalizeLoading: isCurrentGeneration, onError: (error) => console.error("Failed to fetch messages after state change:", error), }); return deactivate; @@ -508,13 +509,13 @@ function useSessionSubscription({ void subscription.ready .then(() => { if (!active) return; - const isCurrentGeneration = () => - active && sessionFetchGenerationRef.current === generation; + const isCurrentGeneration = () => sessionFetchGenerationRef.current === generation; + const isActive = () => active && isCurrentGeneration(); return doFetchMessages({ taskSessionId, ...fetchRefs, fetchAndStoreMessages, - isActive: isCurrentGeneration, + isActive, canFinalizeLoading: isCurrentGeneration, hydrationRef, hydrationKey, @@ -771,14 +772,9 @@ function useSessionEntryMessageFetch(params: SessionEntryFetchParams): void { refs: fetchRefs, } = fetchState; useEffect(() => { - let active = true; const generation = sessionFetchGenerationRef.current; const isCurrentGeneration = () => sessionFetchGenerationRef.current === generation; - const isActive = () => active && isCurrentGeneration(); - const deactivate = () => { - active = false; - }; - if (!taskSessionId || connectionStatus !== "connected") return deactivate; + if (!taskSessionId || connectionStatus !== "connected") return; const isFreshMount = prevSessionIdRef.current === null; const sessionChanged = prevSessionIdRef.current !== null && prevSessionIdRef.current !== taskSessionId; @@ -792,7 +788,7 @@ function useSessionEntryMessageFetch(params: SessionEntryFetchParams): void { lastFetchedSessionIdRef.current = taskSessionId; setIsWaitingForInitialMessages(false); if (historyInitialized) fetchRefs.setHistoryStatus("ready"); - return deactivate; + return; } if (isFreshMount && messagesLength > 0) { lastFetchedSessionIdRef.current = taskSessionId; @@ -815,19 +811,18 @@ function useSessionEntryMessageFetch(params: SessionEntryFetchParams): void { setIsCachedHistoryRefreshPending(false); } }); - return deactivate; + return; } - if (lastFetchedSessionIdRef.current === taskSessionId) return deactivate; + if (lastFetchedSessionIdRef.current === taskSessionId) return; void doFetchMessages({ taskSessionId, ...fetchRefs, fetchAndStoreMessages, - isActive, - canFinalizeLoading: isActive, + isActive: isCurrentGeneration, + canFinalizeLoading: isCurrentGeneration, hydrationRef, hydrationKey, }); - return deactivate; }, [ taskSessionId, connectionStatus, diff --git a/apps/web/src/locales/en/task.json b/apps/web/src/locales/en/task.json index b70d9a4133..b94a846695 100644 --- a/apps/web/src/locales/en/task.json +++ b/apps/web/src/locales/en/task.json @@ -1234,9 +1234,29 @@ "relatedTaskCount_other": "{{count}} related tasks", "relatedTasks": "Related tasks", "remote": "Remote", + "remoteContributionHistoryTitle": "Task and PR histories differ", + "remoteContributionHistoryTooltip": "The task and published PR histories differ. Compare before choosing a version.", + "remoteContributionHistoryBody": "The task and published PR contain different histories. Compare them before choosing a version.", + "remoteContributionHistoryLocalRebase": "The task branch was rebased locally. The PR still contains the earlier history.", + "remoteContributionHistoryLoading": "Checking local history...", + "remoteContributionHistoryGuidance": "Publish the task version after validation finishes.", + "compareContributionVersions": "Compare versions", + "remoteContributionCompareDescription": "Show both histories without changing Git.", + "publishTaskVersion": "Publish task version...", + "restorePublishedPRVersion": "Restore published PR version...", + "openPROnGitHub": "Open PR on GitHub", + "remoteContributionPublishDescription": "Replace the published PR history with the task history. Requires confirmation and an exact provider-head match.", + "remoteContributionRestoreDescription": "Replace the task checkout history with the PR history. Kandev creates a recovery branch first and requires a clean working tree.", + "remoteContributionPublishConfirmation": "Publish the task version for {{repository}} at {{head}}? This replaces the published PR history with the task history.", + "remoteContributionRestoreConfirmation": "Restore the published PR version at {{head}}? This replaces the current task checkout history. Kandev will create a recovery branch first, and the working tree must be clean.", + "remoteContributionTaskCommits_one": "Task commit: {{count}}", + "remoteContributionTaskCommits_other": "Task commits: {{count}}", + "remoteContributionPublishedCommits_one": "Published commit: {{count}}", + "remoteContributionPublishedCommits_other": "Published commits: {{count}}", + "remoteContributionNewBaseCommits_one": "Newer base commit: {{count}}", + "remoteContributionNewBaseCommits_other": "Newer base commits: {{count}}", "remoteContributionChangedTitle": "PR branch changed", "remoteContributionChangedBody": "Your task version is unchanged. When you are ready, choose which version to use.", - "remoteContributionChangedTooltip": "The PR branch changed after this task started. Your task checkout is unchanged.", "replacePRBranch": "Replace PR branch", "usePRVersion": "Use PR version", "prNumberVersion": "PR #{{number}} version", diff --git a/apps/web/src/locales/pseudo/task.json b/apps/web/src/locales/pseudo/task.json index 102f951d35..7b78b00c1f 100644 --- a/apps/web/src/locales/pseudo/task.json +++ b/apps/web/src/locales/pseudo/task.json @@ -1234,9 +1234,29 @@ "relatedTaskCount_other": "{{count}} ŕēĺàţēď ţàśķś", "relatedTasks": "Ŕēĺàţēď ţàśķś", "remote": "Ŕēḿōţē", + "remoteContributionHistoryTitle": "Ţàśķ àńď ƤŔ ĥĩśţōŕĩēś ďĩƒƒēŕ", + "remoteContributionHistoryTooltip": "Ţĥē ţàśķ àńď ƥũƀĺĩśĥēď ƤŔ ĥĩśţōŕĩēś ďĩƒƒēŕ. Ćōḿƥàŕē ƀēƒōŕē ćĥōōśĩńĝ à vēŕśĩōń.", + "remoteContributionHistoryBody": "Ţĥē ţàśķ àńď ƥũƀĺĩśĥēď ƤŔ ćōńţàĩń ďĩƒƒēŕēńţ ĥĩśţōŕĩēś. Ćōḿƥàŕē ţĥēḿ ƀēƒōŕē ćĥōōśĩńĝ à vēŕśĩōń.", + "remoteContributionHistoryLocalRebase": "Ţĥē ţàśķ ƀŕàńćĥ ŵàś ŕēƀàśēď ĺōćàĺĺŷ. Ţĥē ƤŔ śţĩĺĺ ćōńţàĩńś ţĥē ēàŕĺĩēŕ ĥĩśţōŕŷ.", + "remoteContributionHistoryLoading": "Ćĥēćķĩńĝ ĺōćàĺ ĥĩśţōŕŷ...", + "remoteContributionHistoryGuidance": "Ƥũƀĺĩśĥ ţĥē ţàśķ vēŕśĩōń àƒţēŕ vàĺĩďàţĩōń ƒĩńĩśĥēś.", + "compareContributionVersions": "Ćōḿƥàŕē vēŕśĩōńś", + "remoteContributionCompareDescription": "Śĥōŵ ƀōţĥ ĥĩśţōŕĩēś ŵĩţĥōũţ ćĥàńĝĩńĝ Ĝĩţ.", + "publishTaskVersion": "Ƥũƀĺĩśĥ ţàśķ vēŕśĩōń...", + "restorePublishedPRVersion": "Ŕēśţōŕē ƥũƀĺĩśĥēď ƤŔ vēŕśĩōń...", + "openPROnGitHub": "Ōƥēń ƤŔ ōń ĜĩţĤũƀ", + "remoteContributionPublishDescription": "Ŕēƥĺàćē ţĥē ƥũƀĺĩśĥēď ƤŔ ĥĩśţōŕŷ ŵĩţĥ ţĥē ţàśķ ĥĩśţōŕŷ. Ŕēqũĩŕēś ćōńƒĩŕḿàţĩōń àńď àń ēxàćţ ƥŕōvĩďēŕ-ĥēàď ḿàţćĥ.", + "remoteContributionRestoreDescription": "Ŕēƥĺàćē ţĥē ţàśķ ćĥēćķōũţ ĥĩśţōŕŷ ŵĩţĥ ţĥē ƤŔ ĥĩśţōŕŷ. Ķàńďēv ćŕēàţēś à ŕēćōvēŕŷ ƀŕàńćĥ ƒĩŕśţ àńď ŕēqũĩŕēś à ćĺēàń ŵōŕķĩńĝ ţŕēē.", + "remoteContributionPublishConfirmation": "Ƥũƀĺĩśĥ ţĥē ţàśķ vēŕśĩōń ƒōŕ {{repository}} àţ {{head}}? Ţĥĩś ŕēƥĺàćēś ţĥē ƥũƀĺĩśĥēď ƤŔ ĥĩśţōŕŷ ŵĩţĥ ţĥē ţàśķ ĥĩśţōŕŷ.", + "remoteContributionRestoreConfirmation": "Ŕēśţōŕē ţĥē ƥũƀĺĩśĥēď ƤŔ vēŕśĩōń àţ {{head}}? Ţĥĩś ŕēƥĺàćēś ţĥē ćũŕŕēńţ ţàśķ ćĥēćķōũţ ĥĩśţōŕŷ. Ķàńďēv ŵĩĺĺ ćŕēàţē à ŕēćōvēŕŷ ƀŕàńćĥ ƒĩŕśţ, àńď ţĥē ŵōŕķĩńĝ ţŕēē ḿũśţ ƀē ćĺēàń.", + "remoteContributionTaskCommits_one": "Ţàśķ ćōḿḿĩţ: {{count}}", + "remoteContributionTaskCommits_other": "Ţàśķ ćōḿḿĩţś: {{count}}", + "remoteContributionPublishedCommits_one": "Ƥũƀĺĩśĥēď ćōḿḿĩţ: {{count}}", + "remoteContributionPublishedCommits_other": "Ƥũƀĺĩśĥēď ćōḿḿĩţś: {{count}}", + "remoteContributionNewBaseCommits_one": "Ńēŵēŕ ƀàśē ćōḿḿĩţ: {{count}}", + "remoteContributionNewBaseCommits_other": "Ńēŵēŕ ƀàśē ćōḿḿĩţś: {{count}}", "remoteContributionChangedTitle": "ƤŔ ƀŕàńćĥ ćĥàńĝēď", "remoteContributionChangedBody": "Ŷōũŕ ţàśķ vēŕśĩōń ĩś ũńćĥàńĝēď. Ŵĥēń ŷōũ àŕē ŕēàďŷ, ćĥōōśē ŵĥĩćĥ vēŕśĩōń ţō ũśē.", - "remoteContributionChangedTooltip": "Ţĥē ƤŔ ƀŕàńćĥ ćĥàńĝēď àƒţēŕ ţĥĩś ţàśķ śţàŕţēď. Ŷōũŕ ţàśķ ćĥēćķōũţ ĩś ũńćĥàńĝēď.", "replacePRBranch": "Ŕēƥĺàćē ƤŔ ƀŕàńćĥ", "usePRVersion": "Ũśē ƤŔ vēŕśĩōń", "prNumberVersion": "ƤŔ #{{number}} vēŕśĩōń", diff --git a/apps/web/src/locales/pt-pt/task.json b/apps/web/src/locales/pt-pt/task.json index b0f488b63f..898ef18d9c 100644 --- a/apps/web/src/locales/pt-pt/task.json +++ b/apps/web/src/locales/pt-pt/task.json @@ -1177,9 +1177,29 @@ "relatedTaskCount_other": "{{count}} tarefas relacionadas", "relatedTasks": "Tarefas relacionadas", "remote": "Remoto", + "remoteContributionHistoryTitle": "Os históricos da tarefa e do PR são diferentes", + "remoteContributionHistoryTooltip": "Os históricos da tarefa e do PR publicado são diferentes. Compare-os antes de escolher uma versão.", + "remoteContributionHistoryBody": "A tarefa e o PR publicado têm históricos diferentes. Compare-os antes de escolher uma versão.", + "remoteContributionHistoryLocalRebase": "O branch da tarefa foi rebaseado localmente. O PR ainda contém o histórico anterior.", + "remoteContributionHistoryLoading": "A verificar o histórico local...", + "remoteContributionHistoryGuidance": "Publique a versão da tarefa depois de concluir a validação.", + "compareContributionVersions": "Comparar versões", + "remoteContributionCompareDescription": "Mostrar os dois históricos sem alterar o Git.", + "publishTaskVersion": "Publicar versão da tarefa...", + "restorePublishedPRVersion": "Restaurar versão publicada do PR...", + "openPROnGitHub": "Abrir PR no GitHub", + "remoteContributionPublishDescription": "Substitui o histórico publicado do PR pelo histórico da tarefa. Requer confirmação e correspondência exata do commit de referência do fornecedor.", + "remoteContributionRestoreDescription": "Substitui o histórico do checkout da tarefa pelo histórico do PR. O Kandev cria primeiro um branch de recuperação e requer uma árvore de trabalho limpa.", + "remoteContributionPublishConfirmation": "Publicar a versão da tarefa para {{repository}} em {{head}}? Isto substitui o histórico publicado do PR pelo histórico da tarefa.", + "remoteContributionRestoreConfirmation": "Restaurar a versão publicada do PR em {{head}}? Isto substitui o histórico atual do checkout da tarefa. O Kandev vai criar primeiro um branch de recuperação e a árvore de trabalho tem de estar limpa.", + "remoteContributionTaskCommits_one": "Commit da tarefa: {{count}}", + "remoteContributionTaskCommits_other": "Commits da tarefa: {{count}}", + "remoteContributionPublishedCommits_one": "Commit publicado: {{count}}", + "remoteContributionPublishedCommits_other": "Commits publicados: {{count}}", + "remoteContributionNewBaseCommits_one": "Commit de base mais recente: {{count}}", + "remoteContributionNewBaseCommits_other": "Commits de base mais recentes: {{count}}", "remoteContributionChangedTitle": "O branch do PR mudou", "remoteContributionChangedBody": "A versão da tarefa não mudou. Quando estiver pronto, escolha qual versão quer usar.", - "remoteContributionChangedTooltip": "O branch do PR mudou depois de esta tarefa começar. O checkout da tarefa não mudou.", "replacePRBranch": "Substituir branch do PR", "usePRVersion": "Usar versão do PR", "prNumberVersion": "Versão do PR #{{number}}", diff --git a/apps/web/src/locales/zh-cn/task.json b/apps/web/src/locales/zh-cn/task.json index cff8a8103a..9a7a65ecff 100644 --- a/apps/web/src/locales/zh-cn/task.json +++ b/apps/web/src/locales/zh-cn/task.json @@ -1176,9 +1176,29 @@ "relatedTaskCount_other": "{{count}} 个相关任务", "relatedTasks": "相关任务", "remote": "远程", + "remoteContributionHistoryTitle": "任务和 PR 的历史记录不同", + "remoteContributionHistoryTooltip": "任务和已发布 PR 的历史记录不同。请先比较，再选择版本。", + "remoteContributionHistoryBody": "任务和已发布的 PR 包含不同的历史记录。请先比较，再选择版本。", + "remoteContributionHistoryLocalRebase": "任务分支已在本地变基。PR 仍包含较早的历史记录。", + "remoteContributionHistoryLoading": "正在检查本地历史记录...", + "remoteContributionHistoryGuidance": "验证完成后，再发布任务版本。", + "compareContributionVersions": "比较版本", + "remoteContributionCompareDescription": "显示两个历史记录，不更改 Git。", + "publishTaskVersion": "发布任务版本...", + "restorePublishedPRVersion": "恢复已发布的 PR 版本...", + "openPROnGitHub": "在 GitHub 上打开 PR", + "remoteContributionPublishDescription": "用任务历史记录替换已发布的 PR 历史记录。需要确认，并且提供商的最新提交必须完全匹配。", + "remoteContributionRestoreDescription": "用 PR 历史记录替换任务工作区历史记录。Kandev 会先创建恢复分支，并且工作区必须干净。", + "remoteContributionPublishConfirmation": "要在 {{repository}} 的 {{head}} 发布任务版本吗？这会用任务历史记录替换已发布的 PR 历史记录。", + "remoteContributionRestoreConfirmation": "要恢复 {{head}} 的已发布 PR 版本吗？这会替换当前任务工作区历史记录。Kandev 会先创建恢复分支，并且工作区必须干净。", + "remoteContributionTaskCommits_one": "任务提交：{{count}}", + "remoteContributionTaskCommits_other": "任务提交：{{count}}", + "remoteContributionPublishedCommits_one": "已发布提交：{{count}}", + "remoteContributionPublishedCommits_other": "已发布提交：{{count}}", + "remoteContributionNewBaseCommits_one": "较新基础提交：{{count}}", + "remoteContributionNewBaseCommits_other": "较新基础提交：{{count}}", "remoteContributionChangedTitle": "PR 分支已更改", "remoteContributionChangedBody": "你的任务版本未更改。准备好后，选择要使用的版本。", - "remoteContributionChangedTooltip": "PR 分支在任务开始后发生了更改。任务工作区未更改。", "replacePRBranch": "替换 PR 分支", "usePRVersion": "使用 PR 版本", "prNumberVersion": "PR #{{number}} 版本", diff --git a/apps/web/src/locales/zh-hk/task.json b/apps/web/src/locales/zh-hk/task.json index 09e0bd0286..f7e5c97a7c 100644 --- a/apps/web/src/locales/zh-hk/task.json +++ b/apps/web/src/locales/zh-hk/task.json @@ -1176,9 +1176,29 @@ "relatedTaskCount_other": "{{count}} 個相關任務", "relatedTasks": "相關任務", "remote": "遠端", + "remoteContributionHistoryTitle": "任務和 PR 的歷史記錄不同", + "remoteContributionHistoryTooltip": "任務和已發佈 PR 的歷史記錄不同。請先比較，再選擇版本。", + "remoteContributionHistoryBody": "任務和已發佈的 PR 包含不同的歷史記錄。請先比較，再選擇版本。", + "remoteContributionHistoryLocalRebase": "任務分支已在本地變基。PR 仍包含較早的歷史記錄。", + "remoteContributionHistoryLoading": "正在檢查本地歷史記錄...", + "remoteContributionHistoryGuidance": "驗證完成後，再發布任務版本。", + "compareContributionVersions": "比較版本", + "remoteContributionCompareDescription": "顯示兩個歷史記錄，不更改 Git。", + "publishTaskVersion": "發佈任務版本...", + "restorePublishedPRVersion": "恢復已發佈的 PR 版本...", + "openPROnGitHub": "在 GitHub 上開啟 PR", + "remoteContributionPublishDescription": "用任務歷史記錄替換已發佈的 PR 歷史記錄。需要確認，並且提供者的最新提交必須完全匹配。", + "remoteContributionRestoreDescription": "用 PR 歷史記錄替換任務工作區歷史記錄。Kandev 會先建立恢復分支，並且工作區必須乾淨。", + "remoteContributionPublishConfirmation": "要在 {{repository}} 的 {{head}} 發佈任務版本嗎？這會用任務歷史記錄替換已發佈的 PR 歷史記錄。", + "remoteContributionRestoreConfirmation": "要恢復 {{head}} 的已發佈 PR 版本嗎？這會替換目前任務工作區歷史記錄。Kandev 會先建立恢復分支，並且工作區必須乾淨。", + "remoteContributionTaskCommits_one": "任務提交：{{count}}", + "remoteContributionTaskCommits_other": "任務提交：{{count}}", + "remoteContributionPublishedCommits_one": "已發佈提交：{{count}}", + "remoteContributionPublishedCommits_other": "已發佈提交：{{count}}", + "remoteContributionNewBaseCommits_one": "較新基礎提交：{{count}}", + "remoteContributionNewBaseCommits_other": "較新基礎提交：{{count}}", "remoteContributionChangedTitle": "PR 分支已更改", "remoteContributionChangedBody": "你的任務版本未更改。準備好後，選擇要使用的版本。", - "remoteContributionChangedTooltip": "PR 分支在任務開始後發生了更改。任務工作區未更改。", "replacePRBranch": "替換 PR 分支", "usePRVersion": "使用 PR 版本", "prNumberVersion": "PR #{{number}} 版本", diff --git a/apps/web/src/locales/zh-tw/task.json b/apps/web/src/locales/zh-tw/task.json index 6b92026087..cde4f29d0c 100644 --- a/apps/web/src/locales/zh-tw/task.json +++ b/apps/web/src/locales/zh-tw/task.json @@ -1176,9 +1176,29 @@ "relatedTaskCount_other": "{{count}} 個相關任務", "relatedTasks": "相關任務", "remote": "遠端", + "remoteContributionHistoryTitle": "任務和 PR 的歷史記錄不同", + "remoteContributionHistoryTooltip": "任務和已釋出 PR 的歷史記錄不同。請先比較，再選擇版本。", + "remoteContributionHistoryBody": "任務和已釋出的 PR 包含不同的歷史記錄。請先比較，再選擇版本。", + "remoteContributionHistoryLocalRebase": "任務分支已在本地變基。PR 仍包含較早的歷史記錄。", + "remoteContributionHistoryLoading": "正在檢查本地歷史記錄...", + "remoteContributionHistoryGuidance": "驗證完成後，再發布任務版本。", + "compareContributionVersions": "比較版本", + "remoteContributionCompareDescription": "顯示兩個歷史記錄，不更改 Git。", + "publishTaskVersion": "釋出任務版本...", + "restorePublishedPRVersion": "恢復已釋出的 PR 版本...", + "openPROnGitHub": "在 GitHub 上開啟 PR", + "remoteContributionPublishDescription": "用任務歷史記錄替換已釋出的 PR 歷史記錄。需要確認，並且提供者的最新提交必須完全匹配。", + "remoteContributionRestoreDescription": "用 PR 歷史記錄替換任務工作區歷史記錄。Kandev 會先建立恢復分支，並且工作區必須乾淨。", + "remoteContributionPublishConfirmation": "要在 {{repository}} 的 {{head}} 發佈任務版本嗎？這會用任務歷史記錄替換已釋出的 PR 歷史記錄。", + "remoteContributionRestoreConfirmation": "要恢復 {{head}} 的已釋出 PR 版本嗎？這會替換目前任務工作區歷史記錄。Kandev 會先建立恢復分支，並且工作區必須乾淨。", + "remoteContributionTaskCommits_one": "任務提交：{{count}}", + "remoteContributionTaskCommits_other": "任務提交：{{count}}", + "remoteContributionPublishedCommits_one": "已釋出提交：{{count}}", + "remoteContributionPublishedCommits_other": "已釋出提交：{{count}}", + "remoteContributionNewBaseCommits_one": "較新基礎提交：{{count}}", + "remoteContributionNewBaseCommits_other": "較新基礎提交：{{count}}", "remoteContributionChangedTitle": "PR 分支已更改", "remoteContributionChangedBody": "你的任務版本未更改。準備好後，選擇要使用的版本。", - "remoteContributionChangedTooltip": "PR 分支在任務開始後發生了更改。任務工作區未更改。", "replacePRBranch": "替換 PR 分支", "usePRVersion": "使用 PR 版本", "prNumberVersion": "PR #{{number}} 版本", diff --git a/docs/plans/branch-history-explanations/plan.md b/docs/plans/branch-history-explanations/plan.md new file mode 100644 index 0000000000..2232589564 --- /dev/null +++ b/docs/plans/branch-history-explanations/plan.md @@ -0,0 +1,216 @@ +--- +created: 2026-09-12 +status: completed +requirements: + - REQ-TASKS-REMOTE-CONTRIBUTION-TASKS-003 +system_design: + - ../../specs/tasks/system-design/branch-history-explanations.md +legacy_specs: [] +--- + +# Implementation Plan: Branch History Explanations + +## Overview + +Explain task/PR divergence accurately, including a completed local rebase that +is not yet published. Implement bounded evidence first, responsive UI second, +and rendered regression coverage with public documentation third. + +The task system owns checkout preservation and contribution version choices. +The [requirement amendment](../../specs/tasks/requirements/remote-contribution-tasks.md#amendment-branch-history-explanations) +is implemented. Existing requirements 001 and 002 remain active. + +## Scope + +### In scope + +- Neutral divergence wording and optional evidence of a completed local rebase. +- Separate task, published, and base commit counts when graph evidence is complete. +- Comparison-first actions and existing safe resolution operations. +- Desktop menus, phone drawers, all supported locales, and focused regression tests. + +### Out of scope + +- Automatic fetch, rebase, reset, publication, or agent coordination. +- Determining whether CI or local validation passed. +- Patch-based equality, commit deduplication, and a new comparison editor. +- New providers, GitLab Changes presentation, persistence, and feature flags. + +## Technical approach + +Add the read-only request described in the [system design](../../specs/tasks/system-design/branch-history-explanations.md). +Use `GitHandlers`, the runtime agentctl client, and `GitOperator` in the current +executor. Add the WS action constant in `apps/backend/pkg/websocket/actions.go`. +The implementation must use existing session authorization and repository routing. + +Keep `classifyRemoteContribution` and `remoteContributionActionPolicy` separate +from the explanation. Add a shared, cancellable explanation hook scoped to the +selected repository, branch, PR, and heads. Discard stale results. +Reuse existing Changes histories for comparison. Replace the external-link-only +view action with comparison navigation and retain an explicit provider link. + +Update `RemoteContributionHeaderActions`, shared action items, confirmations, +`VcsSplitButton`, and phone Git controls together. A shared view model supplies +both responsive surfaces. Localized labels describe replacement consequences. + +## Related delivery records + +The completed packages remain historical evidence: + +- [Head drift](../remote-contribution-head-drift/plan.md) +- [Local-first choices](../remote-contribution-local-first/plan.md), including task 06 +- [History reconciliation](../remote-contribution-history-reconciliation/plan.md) +- [Resume recovery](../contribution-resume-recovery/plan.md) + +Do not reopen their completed tasks or replace their recorded test counts. +This package supersedes their warning-copy and action-order expectations only. +During implementation, update affected existing tests as part of Task 02 and 03. + +## ASCII UI preview + +UI-01: Desktop Changes toolbar, diverged state. The source shows a yellow +warning menu titled “Task and PR histories differ”, with comparison first. +The menu keeps one toolbar entry and makes comparison primary. + +```text +Changes [histories differ] + +----------------------------------------+ + | Task and PR histories differ | + | The task branch was rebased locally. | + | The PR still has the earlier history. | + | Task: 5 Published: 5 Newer base: 29 | + | [Compare versions] | + | Publish task version... | + | Restore published PR version... | + | Open PR on GitHub | + +----------------------------------------+ +``` + +UI-02: Phone Changes header entry, same state. A short choice deserves an +inset bottom drawer. Comparison dismisses this drawer and opens Changes. + +```text ++-----------------------------------+ +| Changes [History] | +| Task history | +| ... | +| +-----------------------------+ | +| | Task and PR histories differ| | +| | Local rebase detected. | | +| | Task commits: 5 | | +| | Published commits: 5 | | +| | Newer base commits: 29 | | +| | [Compare versions] | | +| | Publish task version... | | +| | Replaces published history.| | +| | Restore published version..| | +| | Creates a recovery branch. | | +| | Open PR on GitHub | | +| | [Close] | | +| +-----------------------------+ | ++-----------------------------------+ +``` + +UI-03: Both surfaces, evidence states. These replace only the explanation body. + +```text +Loading: Task and published histories differ. + Checking local history... +Unknown: Task and published histories differ. + Compare them before choosing a version. +Stale: Discard the old explanation and return to neutral wording. + Existing current-provider action gates remain in force. +``` + +UI-04: Compare versions, existing Changes view, desktop and phone. + +```text +Task version [current local head] + local commits and existing local actions +PR #3587 version (expanded) [current published head] + published commits, read-only +``` + +The first comparison heading receives focus. The Changes view owns scrolling. +The drawer has one internal scroll region and safe-area padding. Its touch +controls have at least 44px hit targets. Desktop keeps compact controls. +Action order, version identity, and phone composition are required. +Spacing, counts, and abbreviated drawing text are illustrative. Product copy +uses the full localized labels from the design. These views cover AC 003.1, +003.3, 003.4, 003.6, and 003.8. + +## Tests + +| Criteria | Targeted evidence | +| --- | --- | +| 003.2, 003.3, 003.7 | New `git_contribution_history_test.go`: `TestContributionHistoryLocalRebase`, `TestContributionHistoryUnknown`, `TestContributionHistoryCounts`, `TestContributionHistoryBounds` | +| 003.7, 003.8 | API, client, and handler `TestContributionHistory` tests for routing, authorization, validation, stale heads, and unavailable executors | +| 003.2, 003.5, 003.7, 003.8 | New `use-contribution-history-explanation.test.tsx`: shared reads, stale responses, cancellation, identity isolation, and policy independence | +| 003.1, 003.3, 003.4, 003.5, 003.6 | Existing header, resolution, and Changes suites, plus new `mobile-changes-panel.test.tsx` | +| 003.8 | Locale completeness, plural rules, pseudo catalog, and typecheck | + +Test names for new suites are proposed. Existing paths are listed in work orders. +Task 01 uses real temporary Git repositories, including a linked worktree. +It reproduces a five-commit rebase onto 29 newer base commits with conflict resolution. +Assertions distinguish operation evidence from patch equality. +Include expired reflog, shallow history, missing objects, changed provider head, +post-rebase commit, branch switch, merges, and process cancellation. + +## E2E tests + +Add scenarios to `e2e/tests/git/git-changes-panel.spec.ts` (`chromium`) and +`e2e/tests/git/mobile-pr-checkout-drift.spec.ts` (`mobile-chrome`). +Use a real disposable repository and the existing mock-provider fixture. +The evidence request traverses the production WS and agentctl path. +Mock provider data describes actual original commit SHAs. + +| Flow | Criteria | +| --- | --- | +| Local rebase shows neutral title, correct cause and separate counts | 003.1, 003.2, 003.3 | +| Compare expands both histories without Git writes | 003.4, 003.7 | +| Publish confirmation and stale lease rejection preserve remote history | 003.5 | +| Restore requires clean files and reports a recovery branch | 003.5 | +| Missing explanation evidence leaves neutral usable UI | 003.2, 003.8 | +| Phone drawer, inline consequences, comparison, focus return and containment | 003.4, 003.6 | +| Provider moves or repository switches during evidence request | 003.7 | +| Existing aligned and linear-history behavior remains intact | 003.8 | + +## Work orders + +- [x] [Task 01: Observe local rebase evidence](task-01-observe-local-rebase.md) +- [x] [Task 02: Present branch history choices](task-02-present-history-choices.md) +- [x] [Task 03: Prove rendered history flows](task-03-prove-history-flows.md) + +Execute sequentially: 01 -> 02 -> 03. No subagents are authorized. +Each work order follows TDD and records its exact checks after implementation. +Install dependencies once with `(cd apps && pnpm install --frozen-lockfile)`. +Production and permanent test changes belong in the work orders. + +## Verification results + +Implementation and design-package validation on 2026-09-12: + +- `python3 scripts/lint-spec-files.test.py`: passed, 36 tests. +- `python3 scripts/lint-spec-files.py --all`: passed. +- Local Markdown links and work-order acceptance references: all resolve. +- `git diff --check -- docs/specs docs/plans/branch-history-explanations`: passed. +- `git status --short -- docs/specs docs/plans/branch-history-explanations`: all four package files and specification edits accounted for. + +Task 01 and Task 02 results record their focused backend and frontend checks. +Task 03 rendered-flow checks passed for desktop and phone, and public Git +documentation was updated. + +## Risks + +- Git reflog formats and expiration limit local-rebase detection. Neutral wording is the required fallback. +- Conflict resolution changes patches. Rebase detection cannot claim identical content. +- Concurrent agent work can invalidate a snapshot. Exact head checks and browser ownership keys prevent stale explanations. +- A new diagnostic endpoint must work through remote executors without backend-local filesystem assumptions. +- Existing mobile menu and Changes tests contain old labels and action order. +- A publication label can hide destructive effects unless confirmation and inline consequences remain explicit. + +## Documentation impact + +This implementation updates `docs/public/git-operations.md` with the neutral +fallback, comparison-first path, and replacement consequences. No ADR change is +required because mutation policy and commit identity remain unchanged. diff --git a/docs/plans/branch-history-explanations/task-01-observe-local-rebase.md b/docs/plans/branch-history-explanations/task-01-observe-local-rebase.md new file mode 100644 index 0000000000..173e15ace0 --- /dev/null +++ b/docs/plans/branch-history-explanations/task-01-observe-local-rebase.md @@ -0,0 +1,93 @@ +--- +id: "01-observe-local-rebase" +title: "Observe local rebase evidence" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-TASKS-REMOTE-CONTRIBUTION-TASKS-003 +acceptance_criteria: + - AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.2 + - AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.3 + - AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.7 + - AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.8 +system_design: + - ../../specs/tasks/system-design/branch-history-explanations.md +--- + +# Task 01: Observe local rebase evidence + +## Summary + +Provide bounded, read-only local-rebase evidence through the existing executor Git path. Preserve existing relation and mutation operations. + +## In scope + +- Implement the typed WS, handler, agentctl client, API, and Git observation contract. +- Resolve linked-worktree reflogs and exact graph counts with conservative fallback. +- Add temporary-repository tests and transport validation tests, including cancellation and repository isolation. + +## Out of scope + +Automatic reconciliation, provider writes outside existing confirmed operations, +and unrelated Git or UI refactors. + +## Acceptance + +- Matching completed rebase evidence returns the exact identity and independent counts. +- Missing, ambiguous, stale, or bounded-out evidence returns neutral results without Git mutation. +- Authorized session routing reaches the selected executor repository. Requests cannot supply arbitrary paths or commands. + +## Verification + +Run from the repository root. Install workspace dependencies once before pnpm. +Add failing tests first, implement, then rerun these exact checks. + +```bash +(cd apps/backend && go test ./internal/agentctl/server/process -run 'TestContributionHistory' -count=1) +(cd apps/backend && go test ./internal/agentctl/server/api ./internal/agent/runtime/agentctl ./internal/agent/handlers -run 'TestContributionHistory' -count=1) +(cd apps/backend && go test -race ./internal/agentctl/server/process -run 'TestContributionHistory' -count=1) +git diff --check +``` + +## Files likely touched + +- New `apps/backend/internal/agentctl/server/process/git_contribution_history.go` and `_test.go` +- `apps/backend/internal/agentctl/server/api/git.go`, `server.go`, and focused tests +- `apps/backend/internal/agent/runtime/agentctl/git.go` and tests +- `apps/backend/internal/agent/handlers/git_handlers.go` and tests +- `apps/backend/pkg/websocket/actions.go` + +## Dependencies + +None. + +## Risks + +Unsupported reflog formats must not produce a guessed cause. A slow Git process must terminate within the observation budget. + +## Parallelism + +sequential + +## Inputs + +- [Requirements](../../specs/tasks/requirements/remote-contribution-tasks.md#amendment-branch-history-explanations) +- [System design](../../specs/tasks/system-design/branch-history-explanations.md) +- [Plan](plan.md) +- Existing contribution resolution tests and branch-scoped selection patterns. + +## Results + +Implemented the bounded, read-only contribution history observer and wired it +through the existing WS, agentctl, executor, and Git operation paths. Matching +linked-worktree rebases return separate task, published, and newer-base counts; +ambiguous, stale, unavailable, and bounded observations remain neutral. + +Verification passed: + +- `(cd apps/backend && go test ./internal/agentctl/server/process -count=1)` (full package, including expired and ambiguous reflogs, missing objects, post-rebase and conflict-resolution histories, merge ranges, 200-entry and 2000-commit bounds, and a running-process timeout) +- `(cd apps/backend && go test ./internal/agentctl/server/api ./internal/agent/runtime/agentctl ./internal/agent/handlers -run 'TestContributionHistory' -count=1)` +- `(cd apps/backend && go test -race ./internal/agentctl/server/process -run 'TestContributionHistory' -count=1)` +- `git diff --check` diff --git a/docs/plans/branch-history-explanations/task-02-present-history-choices.md b/docs/plans/branch-history-explanations/task-02-present-history-choices.md new file mode 100644 index 0000000000..f7cf4fccf2 --- /dev/null +++ b/docs/plans/branch-history-explanations/task-02-present-history-choices.md @@ -0,0 +1,181 @@ +--- +id: "02-present-history-choices" +title: "Present branch history choices" +status: done +wave: 2 +depends_on: ['01-observe-local-rebase'] +plan: "plan.md" +requirements: + - REQ-TASKS-REMOTE-CONTRIBUTION-TASKS-003 +acceptance_criteria: + - AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.1 + - AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.2 + - AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.3 + - AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.4 + - AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.5 + - AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.6 + - AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.7 + - AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.8 +system_design: + - ../../specs/tasks/system-design/branch-history-explanations.md +--- + +# Task 02: Present branch history choices + +## Summary + +Connect evidence to the existing contribution UI. Deliver neutral wording, readable version choices, and a phone drawer through shared state. + +## In scope + +- Add the explanation client and hook with complete identity keys and stale-result rejection. +- Wire compare navigation to existing task and expanded PR histories. +- Update header, VCS, phone actions, confirmations, and all locale catalogs. +- Preserve current action policy and resolution handlers. + +## Out of scope + +Automatic reconciliation, provider writes outside existing confirmed operations, +and unrelated Git or UI refactors. + +## Acceptance + +- Explanation loading, success, fallback, and head changes produce the defined wording without changing permissions. +- All entry points offer comparison first and clearly describe publication and restoration. +- Desktop and phone follow UI-01 through UI-04 with shared behavior and keyboard/touch accessibility. + +## ASCII UI preview + +UI-01: Desktop Changes toolbar, diverged state. Current source shows a yellow +warning menu titled “Task and PR histories differ”, with comparison first. +The menu keeps one toolbar entry and makes comparison primary. + +```text +Changes [histories differ] + +----------------------------------------+ + | Task and PR histories differ | + | The task branch was rebased locally. | + | The PR still has the earlier history. | + | Task: 5 Published: 5 Newer base: 29 | + | [Compare versions] | + | Publish task version... | + | Restore published PR version... | + | Open PR on GitHub | + +----------------------------------------+ +``` + +UI-02: Phone Changes header entry, same state. A short choice deserves an +inset bottom drawer. Comparison dismisses this drawer and opens Changes. + +```text ++-----------------------------------+ +| Changes [History] | +| Task history | +| ... | +| +-----------------------------+ | +| | Task and PR histories differ| | +| | Local rebase detected. | | +| | Task commits: 5 | | +| | Published commits: 5 | | +| | Newer base commits: 29 | | +| | [Compare versions] | | +| | Publish task version... | | +| | Replaces published history.| | +| | Restore published version..| | +| | Creates a recovery branch. | | +| | Open PR on GitHub | | +| | [Close] | | +| +-----------------------------+ | ++-----------------------------------+ +``` + +UI-03: Both surfaces, evidence states. These replace only the explanation body. + +```text +Loading: Task and published histories differ. + Checking local history... +Unknown: Task and published histories differ. + Compare them before choosing a version. +Stale: Discard the old explanation and return to neutral wording. + Existing current-provider action gates remain in force. +``` + +UI-04: Compare versions, existing Changes view, desktop and phone. + +```text +Task version [current local head] + local commits and existing local actions +PR #3587 version (expanded) [current published head] + published commits, read-only +``` + +The first comparison heading receives focus. The Changes view owns scrolling. +The drawer has one internal scroll region and safe-area padding. Its touch +controls have at least 44px hit targets. Desktop keeps compact controls. +Action order, version identity, and phone composition are required. +Spacing, counts, and abbreviated drawing text are illustrative. Product copy +uses the full localized labels from the design. These views cover AC 003.1, +003.3, 003.4, 003.6, and 003.8. + +Full combined preview: [plan](plan.md#ascii-ui-preview). + +## Verification + +Run from the repository root. Install workspace dependencies once before pnpm. +Add failing tests first, implement, then rerun these exact checks. + +```bash +(cd apps/web && pnpm exec vitest run hooks/domains/session/use-contribution-history-explanation.test.tsx hooks/domains/session/remote-contribution-relation.test.ts hooks/domains/session/use-remote-contribution-relation.test.tsx components/task/remote-contribution-header-actions.test.tsx components/task/remote-contribution-resolution-dialog.test.tsx components/task/use-remote-contribution-resolution.test.tsx components/task/mobile/mobile-changes-panel.test.tsx components/task/changes-panel-remote.test.ts components/vcs-split-button.test.ts) +(cd apps/web && pnpm run typecheck) +(cd apps/web && pnpm run i18n:check) +(cd apps/web && pnpm run i18n:ratchet) +git diff --check +``` + +## Files likely touched + +- New `apps/web/hooks/domains/session/use-contribution-history-explanation.ts` and `.test.tsx` +- `apps/web/hooks/domains/session/use-remote-contribution-relation.ts` +- `apps/web/components/task/remote-contribution-header-actions.tsx` +- `apps/web/components/task/remote-contribution-action-items.tsx` +- `apps/web/components/task/remote-contribution-resolution-dialog.tsx` +- `apps/web/components/task/changes-panel-data.tsx` and relevant history/navigation components +- `apps/web/components/task/mobile/mobile-changes-panel.tsx` +- `apps/web/components/vcs-split-button.tsx` and related multi-repository consumers +- New `apps/web/components/task/mobile/mobile-changes-panel.test.tsx` +- Existing WebSocket client helpers and `apps/web/src/locales/` catalogs + +## Dependencies + +Task 01. + +## Risks + +Comparison must target the selected repository without resetting desktop preferences. Long translations must wrap inside the phone drawer. + +## Parallelism + +sequential + +## Inputs + +- [Requirements](../../specs/tasks/requirements/remote-contribution-tasks.md#amendment-branch-history-explanations) +- [System design](../../specs/tasks/system-design/branch-history-explanations.md) +- [Plan](plan.md) +- Existing contribution resolution tests and branch-scoped selection patterns. + +## Results + +Implemented the comparison-first divergence menu, shared explanation hook, +desktop and phone surfaces, selected-repository comparison navigation, updated +confirmations, and all supported locale catalogs. The explanation cache is +scoped to mounted consumers and exact identity/head keys, and stale responses +are discarded. + +Verification passed: + +- `(cd apps/web && pnpm exec vitest run hooks/domains/session/use-contribution-history-explanation.test.tsx hooks/domains/session/remote-contribution-relation.test.ts hooks/domains/session/use-remote-contribution-relation.test.tsx components/task/remote-contribution-header-actions.test.tsx components/task/remote-contribution-resolution-dialog.test.tsx components/task/use-remote-contribution-resolution.test.tsx components/task/mobile/mobile-changes-panel.test.tsx components/task/changes-panel-remote.test.ts components/vcs-split-button.test.ts)` (9 files, 75 tests) +- `(cd apps/web && pnpm run typecheck)` +- `(cd apps/web && pnpm run i18n:check)` +- `(cd apps/web && pnpm run i18n:ratchet)` +- `git diff --check` diff --git a/docs/plans/branch-history-explanations/task-03-prove-history-flows.md b/docs/plans/branch-history-explanations/task-03-prove-history-flows.md new file mode 100644 index 0000000000..e239cebcc6 --- /dev/null +++ b/docs/plans/branch-history-explanations/task-03-prove-history-flows.md @@ -0,0 +1,181 @@ +--- +id: "03-prove-history-flows" +title: "Prove rendered history flows" +status: done +wave: 3 +depends_on: ['02-present-history-choices'] +plan: "plan.md" +requirements: + - REQ-TASKS-REMOTE-CONTRIBUTION-TASKS-003 +acceptance_criteria: + - AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.1 + - AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.2 + - AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.3 + - AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.4 + - AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.5 + - AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.6 + - AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.7 + - AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.8 +system_design: + - ../../specs/tasks/system-design/branch-history-explanations.md +--- + +# Task 03: Prove rendered history flows + +## Summary + +Prove the integrated divergence flows in disposable desktop and phone sessions. Document the shipped behavior after the focused scenarios pass. + +## In scope + +- Add the scenario matrix from the plan to existing desktop and mobile Git suites. +- Use real local rebase evidence and mock-provider original SHAs. +- Verify no Git writes on comparison, stale lease protection, clean-tree restoration, and recovery results. +- Add the public Git explanation and synchronize specification delivery status after all work orders pass. + +## Out of scope + +Automatic reconciliation, provider writes outside existing confirmed operations, +and unrelated Git or UI refactors. + +## Acceptance + +- Both projects pass the planned scenarios with fresh production builds and correct repository identity. +- Phone screenshots and geometry checks match UI-02, with touch targets, focus return, safe areas, and no horizontal overflow. +- Public docs explain the neutral fallback and destructive choices. All package results record actual commands and outcomes. + +## ASCII UI preview + +UI-01: Desktop Changes toolbar, diverged state. The source shows a yellow +warning menu titled “Task and PR histories differ”, with comparison first. +The menu keeps one toolbar entry and makes comparison primary. + +```text +Changes [histories differ] + +----------------------------------------+ + | Task and PR histories differ | + | The task branch was rebased locally. | + | The PR still has the earlier history. | + | Task: 5 Published: 5 Newer base: 29 | + | [Compare versions] | + | Publish task version... | + | Restore published PR version... | + | Open PR on GitHub | + +----------------------------------------+ +``` + +UI-02: Phone Changes header entry, same state. A short choice deserves an +inset bottom drawer. Comparison dismisses this drawer and opens Changes. + +```text ++-----------------------------------+ +| Changes [History] | +| Task history | +| ... | +| +-----------------------------+ | +| | Task and PR histories differ| | +| | Local rebase detected. | | +| | Task commits: 5 | | +| | Published commits: 5 | | +| | Newer base commits: 29 | | +| | [Compare versions] | | +| | Publish task version... | | +| | Replaces published history.| | +| | Restore published version..| | +| | Creates a recovery branch. | | +| | Open PR on GitHub | | +| | [Close] | | +| +-----------------------------+ | ++-----------------------------------+ +``` + +UI-03: Both surfaces, evidence states. These replace only the explanation body. + +```text +Loading: Task and published histories differ. + Checking local history... +Unknown: Task and published histories differ. + Compare them before choosing a version. +Stale: Discard the old explanation and return to neutral wording. + Existing current-provider action gates remain in force. +``` + +UI-04: Compare versions, existing Changes view, desktop and phone. + +```text +Task version [current local head] + local commits and existing local actions +PR #3587 version (expanded) [current published head] + published commits, read-only +``` + +The first comparison heading receives focus. The Changes view owns scrolling. +The drawer has one internal scroll region and safe-area padding. Its touch +controls have at least 44px hit targets. Desktop keeps compact controls. +Action order, version identity, and phone composition are required. +Spacing, counts, and abbreviated drawing text are illustrative. Product copy +uses the full localized labels from the design. These views cover AC 003.1, +003.3, 003.4, 003.6, and 003.8. + +Full combined preview: [plan](plan.md#ascii-ui-preview). + +## Verification + +Run from the repository root. Install workspace dependencies once before pnpm. +Add failing tests first, implement, then rerun these exact checks. + +```bash +(cd apps/web && pnpm e2e:run --project chromium tests/git/git-changes-panel.spec.ts) +(cd apps/web && pnpm e2e:run --project mobile-chrome tests/git/mobile-pr-checkout-drift.spec.ts) +node --test scripts/validate-public-docs.test.mjs +node scripts/validate-public-docs.mjs +python3 scripts/lint-spec-files.py --all +git diff --check +``` + +## Files likely touched + +- `apps/web/e2e/tests/git/git-changes-panel.spec.ts` +- `apps/web/e2e/tests/git/mobile-pr-checkout-drift.spec.ts` +- Shared Git fixtures/helpers used by those suites +- `docs/public/git-operations.md` +- Requirements, paired designs, and this plan package for completed delivery records + +## Dependencies + +Task 02. + +## Risks + +Run projects sequentially with managed runners. Do not use the live diagnostic task or overlap full suites. Rebase fixtures must exercise production evidence rather than a fabricated explanation response. + +## Parallelism + +sequential + +## Inputs + +- [Requirements](../../specs/tasks/requirements/remote-contribution-tasks.md#amendment-branch-history-explanations) +- [System design](../../specs/tasks/system-design/branch-history-explanations.md) +- [Plan](plan.md) +- Existing contribution resolution tests and branch-scoped selection patterns. + +## Results + +Implemented the rendered desktop and phone divergence flows with real disposable +Git history evidence. The desktop scenario verifies local-rebase explanation and +separate counts, comparison without Git writes, stale-lease rejection, and the +existing confirmation lease. The phone scenarios verify the inset Drawer, inline +consequences, a single contained scroll owner, touch target sizing, comparison, +stale-lease rejection, clean-tree restoration rejection, and recovery-branch +results. Updated the public Git operations guide and synchronized the requirement +and system-design delivery status. + +Verification passed: + +- `(cd apps/web && E2E_DEBUG=1 pnpm e2e:run --host --no-build --project chromium tests/git/git-changes-panel.spec.ts)` (25 passed; the preceding E2E run completed the fresh backend and Vite build) +- `(cd apps/web && E2E_DEBUG=1 pnpm e2e:run --host --project mobile-chrome tests/git/mobile-pr-checkout-drift.spec.ts)` (fresh backend and Vite build, 2 passed) +- `node --test scripts/validate-public-docs.test.mjs` (62 passed) +- `node scripts/validate-public-docs.mjs` (46 published docs pages validated) +- `python3 scripts/lint-spec-files.py --all` +- `git diff --check` diff --git a/docs/public/git-operations.md b/docs/public/git-operations.md index 85365b85c8..631c23be11 100644 --- a/docs/public/git-operations.md +++ b/docs/public/git-operations.md @@ -24,6 +24,22 @@ Use the task's **Changes** panel to inspect, stage, discard, commit, push, reset The state transitions are separate operations. Inspect the diff before staging, verify checks before pushing, and decide whether cleanup may remove a worktree or other local data. +### When task and PR histories differ + +For an associated pull request, Kandev keeps the task checkout and the published PR history +separate when their commit histories differ. It may identify a completed local rebase when the +current repository evidence supports that explanation. If the evidence is missing or incomplete, +Kandev uses neutral wording instead of guessing which history changed. + +Choose **Compare versions** first. Kandev opens **Changes** with the task and PR histories visible; +the comparison does not fetch, rewrite, or publish Git history. Review the selected repository, +branch, pull request, and both displayed heads before choosing a replacement action. + +**Publish task version...** replaces the published PR history after an exact provider-head check and +confirmation. **Restore published PR version...** replaces the task checkout history, creates a +recovery branch at the current task head first, and requires a clean working tree. If the provider +head changes before confirmation, Kandev leaves both versions unchanged and asks for a fresh review. + ## Prerequisites and trust boundary The repository must be a valid Git checkout in the executor workspace and the session's `agentctl` must be reachable. Remote commands use the remote named `origin`; configure its URL and credentials in the executor where the command runs before relying on Pull, Push, or change-request creation. Rebase and Merge use `origin` when it exists, or a local base branch when it does not. The workspace's provider automation identity does not replace the task's Git credential policy or executor-local SSH setup; see [Executors](executors.md#workspace-automation-identity-and-task-git-transport). diff --git a/docs/specs/tasks/README.md b/docs/specs/tasks/README.md index fa58cae8fe..a547b05308 100644 --- a/docs/specs/tasks/README.md +++ b/docs/specs/tasks/README.md @@ -164,6 +164,7 @@ signals, and task-scoped scheduling contracts. - [MCP Tool Name Stability](system-design/mcp-tool-name-stability.md) - [MCP workspace modes](system-design/mcp-workspace-mode.md) - [Remote Contribution Tasks](system-design/remote-contribution-tasks.md) +- [Branch history explanations](system-design/branch-history-explanations.md) - [Passthrough Queued Prompt Dispatch](system-design/passthrough-queued-prompt-dispatch.md) - [Saved Prompt Delivery](system-design/saved-prompt-delivery.md) - [Passthrough Initial Prompt Turn Boundary](system-design/passthrough-initial-prompt-turn-boundary.md) diff --git a/docs/specs/tasks/requirements/remote-contribution-tasks.md b/docs/specs/tasks/requirements/remote-contribution-tasks.md index 453dd82a12..09e83439ed 100644 --- a/docs/specs/tasks/requirements/remote-contribution-tasks.md +++ b/docs/specs/tasks/requirements/remote-contribution-tasks.md @@ -2,7 +2,7 @@ status: active system: tasks created: 2026-08-04 -updated: 2026-09-11 +updated: 2026-09-12 owners: - product --- @@ -47,8 +47,46 @@ user intent for destructive replacement, and evidence-based version comparison. This amendment is draft. The [startup recovery fix package](../../../plans/startup-recovery-scroll-timeout/plan.md) owns implementation of criterion 002.6. +## Amendment: branch history explanations + +Requirement 003 is implemented. Requirements 001 and 002 retain their current status. +The task system owns this amendment because it owns checkout preservation and +published-version choices across repositories and executors. + +### REQ-TASKS-REMOTE-CONTRIBUTION-TASKS-003: Branch history explanations + +**Intent:** Explain differing histories without falsely attributing a change to +the published branch or implying that a local rebase lost work. + +#### Acceptance criteria + +- **AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.1:** When histories diverge, the system shall show “Task and PR histories differ” without assuming which side changed. +- **AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.2:** When current evidence identifies a completed local rebase from the published head, the explanation shall identify that local rebase. Missing, stale, ambiguous, or incomplete evidence shall retain neutral wording. +- **AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.3:** When exact counts are available, the explanation shall distinguish task commits, published commits, and newer base commits. Counts shall not imply content equivalence or lost work. Unavailable counts shall be omitted. +- **AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.4:** The first action shall be “Compare versions”. It shall reveal both version histories without changing Git state. Secondary actions shall be “Publish task version...” and “Restore published PR version...”. +- **AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.5:** Publication and restoration shall retain criteria 001.5 and 001.6. Confirmations shall explain replacement effects. A local-rebase explanation shall not relax authorization or claim that validation passed. +- **AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.6:** Desktop and phone users shall receive the same explanation, comparison, and resolution choices. Phone explanations shall be touch-accessible without hover. Dismissal shall restore focus. Long content shall remain inside the viewport. +- **AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.7:** An explanation shall belong to one selected repository, checkout branch, and PR. Changes to that identity or either head shall invalidate it. Explanations shall not fetch, rewrite, or publish Git history automatically. +- **AC-TASKS-REMOTE-CONTRIBUTION-TASKS-003.8:** All changed copy shall be localized. Loading or failed explanation requests shall preserve the usable Changes view and existing action policy. Aligned and linear histories shall retain their behavior. + +#### Exclusions + +Automatic reconciliation, validation orchestration, agent intent inference, +content-based commit deduplication, and new provider support are excluded. +Existing GitLab presentation exclusions remain in force. + +#### Design and delivery + +- [Branch history explanations design](../system-design/branch-history-explanations.md) +- [Branch history explanations package](../../../plans/branch-history-explanations/plan.md) + ## Delivery Requirement 002 is implemented. See the [contribution resume recovery package](../../../plans/contribution-resume-recovery/plan.md) for the admission tests and verification evidence. + +Requirement 003 is implemented. See the +[branch history explanations package](../../../plans/branch-history-explanations/plan.md) +for the bounded local-rebase observation, comparison-first UI, mobile parity, and rendered-flow +verification evidence. diff --git a/docs/specs/tasks/system-design/branch-history-explanations.md b/docs/specs/tasks/system-design/branch-history-explanations.md new file mode 100644 index 0000000000..50068b2901 --- /dev/null +++ b/docs/specs/tasks/system-design/branch-history-explanations.md @@ -0,0 +1,195 @@ +--- +status: current +system: tasks +requirements: + - REQ-TASKS-REMOTE-CONTRIBUTION-TASKS-003 +created: 2026-09-12 +updated: 2026-09-12 +owners: + - product +--- + +# Branch History Explanations System Design + +## Purpose and boundaries + +This design extends [remote contribution tasks](remote-contribution-tasks.md). +It covers ordinary tasks with a matching PR and imported contribution tasks. +The existing relation classifier and replacement operations retain their authority. +This design replaces the older warning wording and action labels in that design. +Other contribution contracts remain unchanged. + +The diagnostic case was a local rebase with five rewritten commits and 29 newer +base commits. The published branch retained the original five commits. +The current warning incorrectly attributed the change to the PR branch. + +## Requirement mapping + +| Requirement criteria | Design section | +| --- | --- | +| 003.1, 003.2, 003.3 | Evidence and explanation | +| 003.4, 003.6 | Responsive presentation | +| 003.5, 003.7 | Identity and safety | +| 003.8 | Failure and compatibility | + +All criteria belong to REQ-TASKS-REMOTE-CONTRIBUTION-TASKS-003. + +## Existing components + +- `classifyRemoteContribution` in `apps/web/hooks/domains/session/remote-contribution-relation.ts` owns the relation and action capabilities. +- `useRemoteContributionRelation` selects a branch-scoped PR and combines provider evidence with session Git status. +- `RemoteContributionHeaderActions`, `RemoteContributionActionItems`, and `RemoteContributionResolutionDialog` own current warning and resolution UI. +- `apps/web/components/task/mobile/mobile-changes-panel.tsx` wires the shared Changes header and history into the phone surface. +- `GitHandlers` in `apps/backend/internal/agent/handlers/git_handlers.go` routes session Git requests through the runtime agentctl client. +- `GitOperator` in `apps/backend/internal/agentctl/server/process/git.go` performs Git work in the selected executor repository. + +## Evidence and explanation + +Add a read-only `worktree.contribution_history_explanation` WebSocket request. +Use the existing session authorization and agentctl forwarding path. +Add `POST /api/v1/git/contribution/history-explanation` at agentctl. +These names and the typed payload below are proposed additions. +Neither endpoint belongs in the agent MCP catalog. + +The request contains `session_id`, `repo`, `branch`, `expected_local_head`, +and `expected_remote_head`. Heads must be full hexadecimal commit IDs. +The selected branch and repository come from existing branch-scoped selection. +The executor resolves the attached repository through `gitOpForRepo`. +No caller-supplied filesystem path, remote URL, or shell expression is accepted. + +The response contains the same identity and heads, plus: + +| Field | Values or meaning | +| --- | --- | +| `kind` | `local_rebase` or `unexplained` | +| `reason` | `matched_reflog`, `no_matching_reflog`, `missing_objects`, `ambiguous`, `stale`, `limit`, or `unavailable` | +| `onto_head` | Optional resolved commit onto which the rebase finished | +| `task_commit_count` | Optional commits after `onto_head` in the current task history | +| `published_commit_count` | Optional commits after the old merge base in the published history | +| `new_base_commit_count` | Optional commits between the old merge base and `onto_head` | + +Counts are independent exact graph counts. They do not attest patch equivalence. +The response contains no raw reflog messages, author identities, diffs, or paths. + +### Bounded local observation + +Run only when a confirmed diverged explanation surface opens. Share one in-flight +request for an identical identity and head pair across mounted consumers. +Do not add work to Git status polling or perform a network fetch. + +Use structured argument arrays, a controlled locale, and the existing owned Git +process path. Bound the operation to two seconds, 200 reflog entries per log, +1 MiB of captured output, and 2,000 enumerated commits per count. +If a bound is reached, return `unexplained` or omit the affected count. +Cancellation must terminate the owned process and return promptly. + +A local-rebase explanation requires all of these facts: + +1. The executor branch and HEAD match the request at the start and end of the read. +2. The latest branch transition ends at the requested local head and identifies a completed rebase. +3. The transition starts at the requested published head, using the adjacent reflog identity. +4. The worktree HEAD reflog contains the corresponding completed rebase sequence and a resolvable onto commit. +5. The onto commit is an ancestor of the new head. All referenced commits exist locally. +6. No intervening branch transition, unfinished rebase, or ambiguous sequence contradicts that observation. + +Read reflogs through Git, including linked-worktree HEAD reflogs. Do not assume +that `.git` is a directory. Unsupported Git reflog forms use neutral wording. +Reflog evidence describes an observed local operation. It cannot prove user +intent, validation status, or content equivalence. + +For counts, require a unique old merge base between the published head and onto +commit. Require linear old and new topic ranges and an ancestor relationship +from that merge base to onto. Omit counts for merges or ambiguous ranges. +Count the topic ranges separately and the base range separately. +Conflict resolution can change patches while the local-rebase explanation remains valid. +No subject matching or range-diff similarity threshold participates in permissions. + +Conservative first-release exclusions include rebase followed by another commit, +expired reflogs, squash operations, and missing published objects. These cases +still receive the neutral explanation and all existing resolution capabilities. + +## Identity and safety + +The browser accepts a result only while workspace, session, repository, branch, +selected PR, local head, and current provider head still match its request key. +A new provider sync invalidates the result until current evidence returns. +The backend checks executor branch and head again before returning evidence. +The browser discards a late response after navigation or head movement. + +Do not persist explanation evidence or modify task contribution metadata. +Cache only within mounted consumers. Dismissal releases the result and cancellation +subscription. A new opening starts a fresh observation. +Older executors without the endpoint return the neutral explanation. + +`remoteContributionActionPolicy` remains independent of explanation fields. +Exact provider-head leases, recovery branches, clean-tree checks, and explicit +confirmation retain their existing implementations. A detected rebase never +changes the relation to aligned or removes either version from the history. +The UI never claims that an agent is validating solely because its session runs. + +## Responsive presentation + +The common title is “Task and PR histories differ”. Neutral body copy is +“The task and published PR contain different histories. Compare them before choosing a version.” +The local-rebase body is “The task branch was rebased locally. The PR still contains the earlier history.” +Guidance is “Publish the task version after validation finishes.” This is advice, +not a claim about current validation progress. + +Exact count rows use separate pluralized translation keys: +“Task commits: 5”, “Published commits: 5”, and “Newer base commits: 29”. +Do not substitute upstream ahead/behind totals into these rows. +During observation, retain the neutral body with “Checking local history...”. +On failure, remove the progress text and retain the neutral body without another alert. + +`Compare versions` is first and visually primary. It closes the menu, opens the +existing Changes surface, expands the provider-version disclosure, and focuses +the version heading. Keep task history visible with its existing local actions. +The provider history remains read-only. This is a history comparison, not a new +patch-matching editor. Preserve the existing provider link as “Open PR on GitHub”. + +`Publish task version...` and `Restore published PR version...` retain destructive +consequence text. Publication replaces published history. Restoration creates a +recovery branch and replaces the current checkout history. Show those effects +inside confirmations and inline on touch surfaces. + +Desktop uses the existing compact toolbar trigger and anchored menu. Phone and +coarse-pointer users receive an inset bottom `Drawer` with the same view model. +The nearest shipped exemplar is `MobileMenuSheet` in +`apps/web/components/kanban/mobile-menu-sheet.tsx`. Reuse its inset drawer treatment. +The existing phone Changes header supplies the entry point and repository context. +The drawer closes before comparison navigation or confirmation opens. + +The drawer has one internal vertical scroll owner, safe-area padding, and a +maximum height based on `dvh`. Touch actions have at least 44px hit targets. +Desktop controls retain existing compact dimensions. Labels wrap on phones. +Escape, dismiss, and Back return focus to the initiating trigger. +Comparison uses the existing phone Changes view with vertically ordered histories. +No side-by-side desktop pane is mounted and hidden on phones. + +All copy uses `t()` or ``. Update English, Portuguese, and Simplified +Chinese, then generate Traditional Chinese and pseudo catalogs through repo scripts. + +## Failure and compatibility + +Unknown evidence does not alter Git action permissions or create a second warning. +Missing endpoint, timeout, cancellation, shallow history, and offline executors +preserve the existing Changes view. There is no automatic fetch or retry loop. +Confirmed divergence still offers comparison when optional explanation evidence fails. +Provider evidence failures continue to obey the existing unknown-relation policy. +A branch without a matching PR receives no contribution explanation. +Aligned, provider-ahead, and local-ahead relations retain existing UI behavior. + +## Observability and decisions + +Use existing request diagnostics with duration and the bounded reason enum. +Do not log reflog text, credentials, patches, or user-entered branch descriptions. +No new metric family, database schema, feature flag, or background worker is needed. + +The [local-first decision](../../../decisions/2026-08-12-local-first-contribution-replacement.md) +and [head-drift decision](../../../decisions/2026-08-10-remote-contribution-head-drift.md) +retain their mutation and commit-identity rules. This design adds diagnostic +explanation only. Its rationale fits this feature design, so no new ADR is required. + +## Implementation plans + +- [Branch history explanations](../../../plans/branch-history-explanations/plan.md) diff --git a/docs/specs/tasks/system-design/remote-contribution-tasks.md b/docs/specs/tasks/system-design/remote-contribution-tasks.md index ee872dc201..7014e93b76 100644 --- a/docs/specs/tasks/system-design/remote-contribution-tasks.md +++ b/docs/specs/tasks/system-design/remote-contribution-tasks.md @@ -21,6 +21,7 @@ This design record preserves the technical source for the capability mapped to R | --- | --- | | REQ-TASKS-REMOTE-CONTRIBUTION-TASKS-001 | Migrated legacy design detail below | | REQ-TASKS-REMOTE-CONTRIBUTION-TASKS-002 | Contribution resume preflight | +| REQ-TASKS-REMOTE-CONTRIBUTION-TASKS-003 | [Branch history explanations](branch-history-explanations.md) | ## Contribution resume preflight @@ -159,18 +160,18 @@ Kandev must preserve both versions and ask for user intent before one version re - When the provider history no longer contains local HEAD, the Changes panel keeps the task version as the primary version. A yellow warning icon in the Changes toolbar opens the available version actions; the panel body does not repeat the warning. The current provider history remains collapsed - behind a **PR # version** disclosure. + behind a **PR # version** disclosure. The warning title is **Task and PR histories differ**. - A diverged task keeps local edit, commit, amend, reset, and review actions available. Generic Pull is - unavailable because it requires a merge strategy. Push becomes an explicit **Replace PR branch** + unavailable because it requires a merge strategy. Push becomes an explicit **Publish task version...** action instead of a disabled control. -- **Replace PR branch** requires a direct user action and an explicit destructive confirmation. The +- **Publish task version...** requires a direct user action and an explicit destructive confirmation. The confirmation identifies the selected repository and the current provider version. Kandev replaces the remote branch only when its head still equals the confirmed provider head. - The managed replacement action uses an exact force-with-lease condition. If the provider head moves after confirmation, Kandev changes neither version and asks the user to review the new state. -- The user can select **Use PR version** instead. Kandev requires a clean working tree, creates a local - recovery branch at the current task HEAD, fetches the confirmed provider head, and resets the task - branch to that head. The result shows the recovery branch name. +- The user can select **Restore published PR version...** instead. Kandev requires a clean working tree, + creates a local recovery branch at the current task HEAD, fetches the confirmed provider head, and + resets the task branch to that head. The result shows the recovery branch name. - Kandev exposes no replacement action through agent MCP tools or automatic Git operations. Generic contribution force-push requests remain rejected. Direct terminal commands remain outside this UI approval boundary. @@ -300,11 +301,11 @@ recovery branch name after a successful reset. Neither action appears in the age | Task persists but the existing-change association fails | Kandev compensates the newly created task and returns failure; it does not launch an agent. | | Checkout SHA no longer matches the source branch during preparation | Launch fails without checking out or pushing a different revision; retry resolves fresh provider state. | | Provider branch advances after launch and still contains local HEAD | Kandev identifies a provider-ahead fast-forward, shows the current provider history, and offers Pull instead of Push. | -| Provider branch is rewritten after launch and no longer contains local HEAD | Kandev preserves the task version, shows a compact remote-change status, and offers user-controlled version choices. | +| Provider branch is rewritten after launch and no longer contains local HEAD | Kandev preserves the task version, shows a history-difference warning, and offers comparison and user-controlled version choices. | | Provider head changes after the replacement confirmation opens | The exact lease fails. Kandev does not change the remote branch and refreshes the shown provider state. | | Provider rejects the leased replacement | The task version remains unchanged. Kandev shows the provider or Git error. | -| The user selects **Use PR version** with local file changes | Kandev does not reset the checkout. It asks the user to commit or discard the file changes first. | -| The user selects **Use PR version** and the fetch does not match the confirmed provider head | Kandev does not create a recovery branch or reset the checkout. It refreshes the provider state. | +| The user selects **Restore published PR version...** with local file changes | Kandev does not reset the checkout. It asks the user to commit or discard the file changes first. | +| The user selects **Restore published PR version...** and the fetch does not match the confirmed provider head | Kandev does not create a recovery branch or reset the checkout. It refreshes the provider state. | | Current provider commits cannot be loaded | Kandev retries once, keeps the checkout history without a warning, and derives remote actions only from sufficient evidence. | | A same-contribution provider refresh is pending or fails after a successful read | Changes retains the last confirmed commit provenance for display, while current-evidence action gates remain closed until refresh succeeds. | | Effective Git credentials cannot dry-run a push to the source branch | The task remains durable, but the session does not start and exposes an actionable credential/collaboration error. | @@ -329,7 +330,7 @@ The frontend provider-history resource can retain the latest successful commit l same-contribution evidence versions. This display cache is not persisted. It does not retain provider-head authority. The resource discards the cache when it evicts the bounded entry. -A recovery branch created by **Use PR version** remains in the task repository across backend +A recovery branch created by **Restore published PR version...** remains in the task repository across backend restarts. Kandev does not persist confirmation dialogs, provider snapshots, or replacement leases. Repository-qualified comparison targets survive backend and executor restarts. Their deterministic @@ -384,8 +385,11 @@ GIVEN a running contribution task whose provider branch is force-pushed and no l HEAD WHEN Kandev loads the current provider commits THEN the Changes panel keeps the task version primary, shows one warning icon in its toolbar, and offers -**Replace PR branch**, **Use PR version**, and **PR # version** from the warning menu -AND each action explanation appears in an immediate tooltip on pointer hover +**Compare versions**, **Publish task version...**, **Restore published PR version...**, and +**Open PR on GitHub** from the warning menu +AND the warning title is **Task and PR histories differ** +AND comparison is the first action and opens both histories without changing Git state +AND desktop action consequences are available through tooltips while phone action consequences appear inline AND the panel body does not repeat the warning ### Ignore a historical pull request after the checkout changes branch @@ -396,30 +400,30 @@ WHEN Kandev computes the Changes relation and provider history THEN it uses only the newer pull request AND the historical pull request remains available in Review without producing a Changes warning -### Replace the PR branch with the task version +### Publish the task version to the PR branch GIVEN a diverged contribution and a confirmed provider head -WHEN the user confirms **Replace PR branch** +WHEN the user confirms **Publish task version...** THEN Kandev replaces the remote branch only when the exact provider-head lease still matches ### Reject a stale replacement lease GIVEN the replacement confirmation names provider head A AND the provider branch advances to head B -WHEN the user confirms **Replace PR branch** +WHEN the user confirms **Publish task version...** THEN Kandev leaves head B unchanged and shows the refreshed remote-change state -### Use the current PR version with recovery +### Restore the published PR version with recovery GIVEN a diverged contribution with a clean working tree -WHEN the user confirms **Use PR version** +WHEN the user confirms **Restore published PR version...** THEN Kandev creates a local recovery branch at the prior task HEAD and moves the task branch to the confirmed provider head ### Preserve local file changes GIVEN a diverged contribution with staged, unstaged, or untracked file changes -WHEN the user selects **Use PR version** +WHEN the user selects **Restore published PR version...** THEN Kandev keeps the checkout unchanged and asks the user to commit or discard those changes ### Provide the same choice on mobile @@ -427,7 +431,8 @@ THEN Kandev keeps the checkout unchanged and asks the user to commit or discard GIVEN a diverged contribution on a phone viewport WHEN the user opens Changes and its remote-contribution actions THEN the user can replace the PR branch, use the PR version, or inspect the PR version without a desktop -workflow +workflow through **Publish task version...**, **Restore published PR version...**, and +**Open PR on GitHub**, with **Compare versions** available first ### Keep ordinary local-ahead work pushable