diff --git a/apps/backend/internal/agentctl/server/process/workspace_git_diff.go b/apps/backend/internal/agentctl/server/process/workspace_git_diff.go index 75533d9666..dc52dde01b 100644 --- a/apps/backend/internal/agentctl/server/process/workspace_git_diff.go +++ b/apps/backend/internal/agentctl/server/process/workspace_git_diff.go @@ -70,13 +70,16 @@ func (wt *WorkspaceTracker) enrichWithDiffData( if err := wt.enrichWithUnstagedDiffBudget(ctx, update, "HEAD", prior, &budget); err != nil { return err } + if err := wt.enrichMixedUnstagedDiffsBudget(ctx, update, prior, &budget); err != nil { + return err + } if err := wt.enrichWithStagedDiffBudget(ctx, update, "HEAD", prior, &budget); err != nil { return err } if err := wt.enrichUntrackedFileDiffsBudget(ctx, update, &budget); err != nil { return err } - return carryForwardFileDiffs(ctx, update, prior) + return carryForwardFileDiffsBudget(ctx, update, prior, &budget) } // enrichWithBranchDiff computes the total additions/deletions for the entire branch @@ -168,6 +171,12 @@ func totalDiffBytes(ctx context.Context, update *types.GitStatusUpdate) (int64, return 0, err } total += int64(len(fi.Diff)) + if fi.StagedChange != nil { + total += int64(len(fi.StagedChange.Diff)) + } + if fi.UnstagedChange != nil { + total += int64(len(fi.UnstagedChange.Diff)) + } } return total, nil } @@ -212,6 +221,19 @@ func carryBranchDiff(update *types.GitStatusUpdate, prior types.GitStatusUpdate) // last known diff data visible until the next successful poll. HEAD is // required to be unchanged so we don't show stale diffs after a reset/commit. func carryForwardFileDiffs(ctx context.Context, update *types.GitStatusUpdate, prior types.GitStatusUpdate) error { + budget, err := newDiffBudget(ctx, update) + if err != nil { + return err + } + return carryForwardFileDiffsBudget(ctx, update, prior, &budget) +} + +func carryForwardFileDiffsBudget( + ctx context.Context, + update *types.GitStatusUpdate, + prior types.GitStatusUpdate, + budget *diffBudget, +) error { if prior.HeadCommit == "" || prior.HeadCommit != update.HeadCommit { return nil } @@ -219,37 +241,79 @@ func carryForwardFileDiffs(ctx context.Context, update *types.GitStatusUpdate, p if err := ctx.Err(); err != nil { return err } - // A non-empty Diff means an earlier enrichment pass already populated - // this entry (e.g. unstaged numstat succeeded, then staged numstat - // failed). Leave the freshly-computed entry alone — only fill in - // completely-missing diff data. - if fi.Diff != "" { - continue - } - // A skip reason set this poll (budget_exceeded, too_large, binary) - // is an intentional decision to not surface diff content; restoring - // a prior diff would defeat that skip and could show stale content - // for a file that has since grown past the size or budget cap. - if fi.DiffSkipReason != "" { - continue - } priorFi, ok := prior.Files[path] - if !ok || priorFi.Diff == "" { + if !ok { continue } - fi.Diff = priorFi.Diff - fi.DiffSkipReason = priorFi.DiffSkipReason - if fi.Additions == 0 { - fi.Additions = priorFi.Additions - } - if fi.Deletions == 0 { - fi.Deletions = priorFi.Deletions + // A skip reason set this poll is intentional and must not be replaced by + // cached content. Otherwise fill each missing representation + // independently; a fresh combined diff must not suppress facet carry. + if fi.Diff == "" && fi.DiffSkipReason == "" && priorFi.Diff != "" { + if fi.Additions == 0 { + fi.Additions = priorFi.Additions + } + if fi.Deletions == 0 { + fi.Deletions = priorFi.Deletions + } + if budget.exhausted() { + fi.DiffSkipReason = diffSkipReasonBudgetExceeded + } else { + fi.Diff = priorFi.Diff + fi.DiffSkipReason = priorFi.DiffSkipReason + budget.replace("", fi.Diff) + } } + fi.StagedChange = carryForwardChangeFacetBudget(fi.StagedChange, priorFi.StagedChange, budget) + fi.UnstagedChange = carryForwardChangeFacetBudget(fi.UnstagedChange, priorFi.UnstagedChange, budget) update.Files[path] = fi } return nil } +func carryForwardChangeFacetBudget( + current *types.FileChangeFacet, + previous *types.FileChangeFacet, + budget *diffBudget, +) *types.FileChangeFacet { + if current == nil || previous == nil || current.Diff != "" || current.DiffSkipReason != "" || previous.Diff == "" { + return current + } + carried := *current + if carried.Additions == 0 { + carried.Additions = previous.Additions + } + if carried.Deletions == 0 { + carried.Deletions = previous.Deletions + } + if budget.exhausted() { + carried.DiffSkipReason = diffSkipReasonBudgetExceeded + return &carried + } + carried.Diff = previous.Diff + carried.DiffSkipReason = previous.DiffSkipReason + budget.replace("", carried.Diff) + return &carried +} + +func carryForwardChangeFacet( + current *types.FileChangeFacet, + previous *types.FileChangeFacet, +) *types.FileChangeFacet { + if current == nil || previous == nil || current.Diff != "" || current.DiffSkipReason != "" || previous.Diff == "" { + return current + } + carried := *current + carried.Diff = previous.Diff + carried.DiffSkipReason = previous.DiffSkipReason + if carried.Additions == 0 { + carried.Additions = previous.Additions + } + if carried.Deletions == 0 { + carried.Deletions = previous.Deletions + } + return &carried +} + // carryForwardFileDiff fills fi.Diff/DiffSkipReason from prior for a single // file when HEAD matches and prior had diff content. Used when numstat // succeeded (so Additions/Deletions are fresh) but the per-file capDiffOutput @@ -424,6 +488,97 @@ func (wt *WorkspaceTracker) enrichUnstagedFileDiff(ctx context.Context, update * return nil } +// enrichMixedUnstagedDiffsBudget fills the index-to-working-tree facet for +// paths whose porcelain status contains both layers. The flattened legacy diff +// above remains HEAD-to-working-tree for compatibility. +func (wt *WorkspaceTracker) enrichMixedUnstagedDiffsBudget( + ctx context.Context, + update *types.GitStatusUpdate, + prior types.GitStatusUpdate, + budget *diffBudget, +) error { + if err := ctx.Err(); err != nil { + return err + } + hasMixedFacet := false + for _, fileInfo := range update.Files { + if fileInfo.UnstagedChange != nil { + hasMixedFacet = true + break + } + } + if !hasMixedFacet { + return nil + } + numstatOut, err := wt.runGitOutput(ctx, "diff", "--numstat") + if err != nil { + if ctxErr := ctx.Err(); ctxErr != nil { + return ctxErr + } + wt.logger.Debug("enrichMixedUnstagedDiffs: numstat failed", zap.Error(err)) + return nil + } + for _, line := range strings.Split(string(numstatOut), "\n") { + if err := ctx.Err(); err != nil { + return err + } + entry, ok := parseNumstatEntry(line) + if !ok { + continue + } + if err := wt.enrichMixedUnstagedFileDiff(ctx, update, prior, budget, entry); err != nil { + return err + } + } + return ctx.Err() +} + +func (wt *WorkspaceTracker) enrichMixedUnstagedFileDiff( + ctx context.Context, + update *types.GitStatusUpdate, + prior types.GitStatusUpdate, + budget *diffBudget, + entry numstatEntry, +) error { + fileInfo, exists := update.Files[entry.path] + if !exists || fileInfo.UnstagedChange == nil { + return nil + } + if err := ctx.Err(); err != nil { + return err + } + change := *fileInfo.UnstagedChange + change.Additions = entry.additions + change.Deletions = entry.deletions + if budget.exhausted() { + change.DiffSkipReason = diffSkipReasonBudgetExceeded + fileInfo.UnstagedChange = &change + update.Files[entry.path] = fileInfo + return nil + } + + previousDiff := change.Diff + diffOut, truncated := capDiffOutput(ctx, wt.workDir, "diff", "--", entry.path) + if err := ctx.Err(); err != nil { + return err + } + if diffOut != "" { + change.Diff = diffOut + if truncated { + change.DiffSkipReason = diffSkipReasonTruncated + } + } else if prior.HeadCommit != "" && prior.HeadCommit == update.HeadCommit { + if priorFile, ok := prior.Files[entry.path]; ok { + carried := carryForwardChangeFacet(&change, priorFile.UnstagedChange) + change = *carried + } + } + budget.replace(previousDiff, change.Diff) + fileInfo.UnstagedChange = &change + update.Files[entry.path] = fileInfo + return nil +} + // enrichWithStagedDiff populates additions/deletions and diff content for staged files // that have no additional unstaged changes, using git diff --cached. On --cached // numstat failure the function returns early; carry-forward happens once at the @@ -483,6 +638,9 @@ func (wt *WorkspaceTracker) enrichStagedFileDiff(ctx context.Context, update *ty if err := ctx.Err(); err != nil { return err } + if fileInfo.StagedChange != nil { + return wt.enrichMixedStagedFileDiff(ctx, update, baseRef, prior, budget, entry, fileInfo) + } if fileInfo.Additions == 0 && fileInfo.Deletions == 0 { fileInfo.Additions = entry.additions fileInfo.Deletions = entry.deletions @@ -514,6 +672,62 @@ func (wt *WorkspaceTracker) enrichStagedFileDiff(ctx context.Context, update *ty return nil } +func (wt *WorkspaceTracker) enrichMixedStagedFileDiff( + ctx context.Context, + update *types.GitStatusUpdate, + baseRef string, + prior types.GitStatusUpdate, + budget *diffBudget, + entry numstatEntry, + fileInfo types.FileInfo, +) error { + change := *fileInfo.StagedChange + change.Additions = entry.additions + change.Deletions = entry.deletions + if budget.exhausted() { + change.DiffSkipReason = diffSkipReasonBudgetExceeded + fileInfo.StagedChange = &change + update.Files[entry.path] = fileInfo + return nil + } + + previousDiff := change.Diff + diffOut, truncated := capDiffOutput(ctx, wt.workDir, "diff", "--cached", baseRef, "--", entry.path) + if err := ctx.Err(); err != nil { + return err + } + if diffOut != "" { + change.Diff = diffOut + } + if diffOut != "" && truncated { + change.DiffSkipReason = diffSkipReasonTruncated + } + if diffOut == "" { + change = carryPriorStagedFacet(change, entry.path, update, prior) + } + + budget.replace(previousDiff, change.Diff) + fileInfo.StagedChange = &change + update.Files[entry.path] = fileInfo + return nil +} + +func carryPriorStagedFacet( + current types.FileChangeFacet, + path string, + update *types.GitStatusUpdate, + prior types.GitStatusUpdate, +) types.FileChangeFacet { + if prior.HeadCommit == "" || prior.HeadCommit != update.HeadCommit { + return current + } + priorFile, ok := prior.Files[path] + if !ok { + return current + } + return *carryForwardChangeFacet(¤t, priorFile.StagedChange) +} + // isBinaryContent checks for null bytes in the data, same heuristic git uses. func isBinaryContent(data []byte) bool { return bytes.IndexByte(data, 0) != -1 diff --git a/apps/backend/internal/agentctl/server/process/workspace_git_mixed_changes_test.go b/apps/backend/internal/agentctl/server/process/workspace_git_mixed_changes_test.go new file mode 100644 index 0000000000..6bb681d475 --- /dev/null +++ b/apps/backend/internal/agentctl/server/process/workspace_git_mixed_changes_test.go @@ -0,0 +1,270 @@ +package process + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/kandev/kandev/internal/agentctl/types" +) + +func TestEnrichMixedUnstagedDiffsSkipsGitWithoutMixedFacets(t *testing.T) { + repoDir, cleanup := setupTestRepo(t) + defer cleanup() + + tracker := NewWorkspaceTracker(repoDir, newTestLogger(t)) + tracePath := filepath.Join(t.TempDir(), "git.trace") + t.Setenv("GIT_TRACE", tracePath) + update := &types.GitStatusUpdate{Files: map[string]types.FileInfo{ + "plain.txt": {Path: "plain.txt", Status: fileStatusModified}, + }} + budget := diffBudget{} + + if err := tracker.enrichMixedUnstagedDiffsBudget( + context.Background(), update, types.GitStatusUpdate{}, &budget, + ); err != nil { + t.Fatalf("enrichMixedUnstagedDiffsBudget() error = %v", err) + } + if _, err := os.Stat(tracePath); !os.IsNotExist(err) { + t.Fatalf("git trace exists after non-mixed update: %v", err) + } +} + +type mixedChangeFacetWire struct { + Status string `json:"status"` + Additions int `json:"additions"` + Deletions int `json:"deletions"` + Diff string `json:"diff"` +} + +// TestWorkspaceGitStatusPreservesMixedChangeFacets covers +// AC-PLATFORM-WORKSPACE-GIT-STATUS-001.9. +func TestWorkspaceGitStatusPreservesMixedChangeFacets(t *testing.T) { + repoDir, cleanup := setupTestRepo(t) + defer cleanup() + + const filePath = "mixed-change.txt" + writeFile(t, repoDir, filePath, "base\n") + runGit(t, repoDir, "add", filePath) + runGit(t, repoDir, "commit", "-m", "Add mixed change fixture") + + writeFile(t, repoDir, filePath, "base\nSTAGED_LAYER_MARKER\n") + runGit(t, repoDir, "add", filePath) + writeFile(t, repoDir, filePath, "base\nSTAGED_LAYER_MARKER\nUNSTAGED_LAYER_MARKER\n") + + tracker := NewWorkspaceTracker(repoDir, newTestLogger(t)) + status, err := tracker.getGitStatus(context.Background()) + if err != nil { + t.Fatalf("getGitStatus() error = %v", err) + } + + file, ok := status.Files[filePath] + if !ok { + t.Fatalf("status.Files missing %q", filePath) + } + wire, err := json.Marshal(file) + if err != nil { + t.Fatalf("marshal FileInfo: %v", err) + } + + var encoded map[string]json.RawMessage + if err := json.Unmarshal(wire, &encoded); err != nil { + t.Fatalf("unmarshal FileInfo wire shape: %v", err) + } + staged := decodeMixedChangeFacet(t, encoded, "staged_change") + unstaged := decodeMixedChangeFacet(t, encoded, "unstaged_change") + + if staged.Status != fileStatusModified || staged.Additions != 1 || staged.Deletions != 0 { + t.Errorf("staged facet = %+v, want modified +1 -0", staged) + } + if !strings.Contains(staged.Diff, "STAGED_LAYER_MARKER") { + t.Errorf("staged diff missing staged marker: %q", staged.Diff) + } + if strings.Contains(staged.Diff, "UNSTAGED_LAYER_MARKER") { + t.Errorf("staged diff contains unstaged marker: %q", staged.Diff) + } + if unstaged.Status != fileStatusModified || unstaged.Additions != 1 || unstaged.Deletions != 0 { + t.Errorf("unstaged facet = %+v, want modified +1 -0", unstaged) + } + if !strings.Contains(unstaged.Diff, "UNSTAGED_LAYER_MARKER") { + t.Errorf("unstaged diff missing unstaged marker: %q", unstaged.Diff) + } +} + +// TestWorkspaceGitStatusPreservesAddedMixedChangeFacets covers +// AC-PLATFORM-WORKSPACE-GIT-STATUS-001.9 for an AM path. +func TestWorkspaceGitStatusPreservesAddedMixedChangeFacets(t *testing.T) { + repoDir, cleanup := setupTestRepo(t) + defer cleanup() + + const filePath = "mixed-added.txt" + writeFile(t, repoDir, filePath, "STAGED_ADDED_MARKER\n") + runGit(t, repoDir, "add", filePath) + writeFile(t, repoDir, filePath, "STAGED_ADDED_MARKER\nUNSTAGED_ADDED_MARKER\n") + + tracker := NewWorkspaceTracker(repoDir, newTestLogger(t)) + status, err := tracker.getGitStatus(context.Background()) + if err != nil { + t.Fatalf("getGitStatus() error = %v", err) + } + + file, ok := status.Files[filePath] + if !ok { + t.Fatalf("status.Files missing %q", filePath) + } + wire, err := json.Marshal(file) + if err != nil { + t.Fatalf("marshal FileInfo: %v", err) + } + var encoded map[string]json.RawMessage + if err := json.Unmarshal(wire, &encoded); err != nil { + t.Fatalf("unmarshal FileInfo wire shape: %v", err) + } + + staged := decodeMixedChangeFacet(t, encoded, "staged_change") + unstaged := decodeMixedChangeFacet(t, encoded, "unstaged_change") + if staged.Status != "added" || staged.Additions != 1 || staged.Deletions != 0 { + t.Errorf("staged facet = %+v, want added +1 -0", staged) + } + if strings.Contains(staged.Diff, "UNSTAGED_ADDED_MARKER") { + t.Errorf("staged diff contains unstaged marker: %q", staged.Diff) + } + if unstaged.Status != fileStatusModified || unstaged.Additions != 1 || unstaged.Deletions != 0 { + t.Errorf("unstaged facet = %+v, want modified +1 -0", unstaged) + } +} + +// TestMixedChangeFacetDiffBudgetCountsAllRepresentations covers +// AC-PLATFORM-WORKSPACE-GIT-STATUS-001.7. +func TestMixedChangeFacetDiffBudgetCountsAllRepresentations(t *testing.T) { + file := fileInfoFromWire(t, `{ + "path":"mixed.txt", + "status":"modified", + "staged":false, + "diff":"base", + "staged_change":{"status":"modified","diff":"staged"}, + "unstaged_change":{"status":"modified","diff":"unstaged"} + }`) + update := &types.GitStatusUpdate{Files: map[string]types.FileInfo{"mixed.txt": file}} + + total, err := totalDiffBytes(context.Background(), update) + if err != nil { + t.Fatalf("totalDiffBytes() error = %v", err) + } + if total != int64(len("base")+len("staged")+len("unstaged")) { + t.Fatalf("totalDiffBytes() = %d, want all three representations", total) + } +} + +// TestCarryForwardMixedChangeFacets covers AC-PLATFORM-WORKSPACE-GIT-STATUS-001.9. +func TestCarryForwardMixedChangeFacets(t *testing.T) { + current := fileInfoFromWire(t, `{ + "path":"mixed.txt", + "status":"modified", + "staged":false, + "diff":"fresh combined", + "staged_change":{"status":"modified"}, + "unstaged_change":{"status":"modified"} + }`) + priorFile := fileInfoFromWire(t, `{ + "path":"mixed.txt", + "status":"modified", + "staged":false, + "diff":"old combined", + "staged_change":{"status":"modified","additions":1,"diff":"cached staged"}, + "unstaged_change":{"status":"modified","additions":1,"diff":"cached unstaged"} + }`) + update := &types.GitStatusUpdate{ + HeadCommit: "same-head", + Files: map[string]types.FileInfo{"mixed.txt": current}, + } + prior := types.GitStatusUpdate{ + HeadCommit: "same-head", + Files: map[string]types.FileInfo{"mixed.txt": priorFile}, + } + + if err := carryForwardFileDiffs(context.Background(), update, prior); err != nil { + t.Fatalf("carryForwardFileDiffs() error = %v", err) + } + wire, err := json.Marshal(update.Files["mixed.txt"]) + if err != nil { + t.Fatalf("marshal carried FileInfo: %v", err) + } + var encoded map[string]json.RawMessage + if err := json.Unmarshal(wire, &encoded); err != nil { + t.Fatalf("unmarshal carried FileInfo: %v", err) + } + if got := decodeMixedChangeFacet(t, encoded, "staged_change").Diff; got != "cached staged" { + t.Errorf("staged carried diff = %q, want cached staged", got) + } + if got := decodeMixedChangeFacet(t, encoded, "unstaged_change").Diff; got != "cached unstaged" { + t.Errorf("unstaged carried diff = %q, want cached unstaged", got) + } +} + +func TestCarryForwardMixedChangeFacetsRespectsBudget(t *testing.T) { + update := &types.GitStatusUpdate{ + HeadCommit: "same-head", + Files: map[string]types.FileInfo{ + "mixed.txt": { + Path: "mixed.txt", + Status: fileStatusModified, + Diff: strings.Repeat("x", maxTotalDiffBytes), + StagedChange: &types.FileChangeFacet{Status: fileStatusModified}, + UnstagedChange: &types.FileChangeFacet{Status: fileStatusModified}, + }, + }, + } + prior := types.GitStatusUpdate{ + HeadCommit: "same-head", + Files: map[string]types.FileInfo{ + "mixed.txt": { + Path: "mixed.txt", + Status: fileStatusModified, + StagedChange: &types.FileChangeFacet{Status: fileStatusModified, Diff: "cached staged"}, + UnstagedChange: &types.FileChangeFacet{Status: fileStatusModified, Diff: "cached unstaged"}, + }, + }, + } + + if err := carryForwardFileDiffs(context.Background(), update, prior); err != nil { + t.Fatalf("carryForwardFileDiffs() error = %v", err) + } + file := update.Files["mixed.txt"] + if file.StagedChange.Diff != "" || file.StagedChange.DiffSkipReason != diffSkipReasonBudgetExceeded { + t.Errorf("staged facet = %+v, want budget-exceeded without cached diff", file.StagedChange) + } + if file.UnstagedChange.Diff != "" || file.UnstagedChange.DiffSkipReason != diffSkipReasonBudgetExceeded { + t.Errorf("unstaged facet = %+v, want budget-exceeded without cached diff", file.UnstagedChange) + } +} + +func fileInfoFromWire(t *testing.T, wire string) types.FileInfo { + t.Helper() + var file types.FileInfo + if err := json.Unmarshal([]byte(wire), &file); err != nil { + t.Fatalf("unmarshal FileInfo fixture: %v", err) + } + return file +} + +func decodeMixedChangeFacet( + t *testing.T, + encoded map[string]json.RawMessage, + name string, +) mixedChangeFacetWire { + t.Helper() + raw, ok := encoded[name] + if !ok { + full, _ := json.Marshal(encoded) + t.Fatalf("FileInfo missing %s facet: %s", name, full) + } + var facet mixedChangeFacetWire + if err := json.Unmarshal(raw, &facet); err != nil { + t.Fatalf("unmarshal %s facet: %v", name, err) + } + return facet +} diff --git a/apps/backend/internal/agentctl/server/process/workspace_git_status.go b/apps/backend/internal/agentctl/server/process/workspace_git_status.go index 89964f5b01..aa8370b77e 100644 --- a/apps/backend/internal/agentctl/server/process/workspace_git_status.go +++ b/apps/backend/internal/agentctl/server/process/workspace_git_status.go @@ -847,12 +847,16 @@ func (wt *WorkspaceTracker) applyPorcelainLine(line string, update *types.GitSta // For renames the format is "old -> new" (each part may be independently // quoted), so we must split first and unquote each part separately. - filePath := rawPath - if indexStatus != 'R' { - filePath = unquoteGitPath(rawPath) + filePath := unquoteGitPath(rawPath) + oldPath := "" + if indexStatus == 'R' { + if idx := strings.Index(rawPath, " -> "); idx != -1 { + oldPath = unquoteGitPath(rawPath[:idx]) + filePath = unquoteGitPath(rawPath[idx+4:]) + } } - fileInfo := types.FileInfo{Path: filePath} + fileInfo := types.FileInfo{Path: filePath, OldPath: oldPath} // Determine staged status based on index and worktree status. // Prioritize worktree changes as they represent the current state. @@ -889,14 +893,35 @@ func (wt *WorkspaceTracker) applyPorcelainLine(line string, update *types.GitSta case indexStatus == 'R': fileInfo.Status = "renamed" fileInfo.Staged = true - // Renamed files have format "old -> new"; each part may be quoted independently. - if idx := strings.Index(rawPath, " -> "); idx != -1 { - fileInfo.OldPath = unquoteGitPath(rawPath[:idx]) - filePath = unquoteGitPath(rawPath[idx+4:]) - fileInfo.Path = filePath - } update.Renamed = append(update.Renamed, filePath) } + // A path can have both an index and a working-tree change (for example MM + // or AM). Keep the flattened compatibility projection above, but preserve + // both independently for consumers that understand mixed paths. + if stagedChange := porcelainChangeFacet(indexStatus, oldPath); stagedChange != nil { + if unstagedChange := porcelainChangeFacet(workTreeStatus, ""); unstagedChange != nil { + fileInfo.StagedChange = stagedChange + fileInfo.UnstagedChange = unstagedChange + } + } + update.Files[filePath] = fileInfo } + +func porcelainChangeFacet(status byte, oldPath string) *types.FileChangeFacet { + change := &types.FileChangeFacet{OldPath: oldPath} + switch status { + case 'M': + change.Status = fileStatusModified + case 'A': + change.Status = "added" + case 'D': + change.Status = fileStatusDeleted + case 'R': + change.Status = "renamed" + default: + return nil + } + return change +} diff --git a/apps/backend/internal/agentctl/types/streams/git.go b/apps/backend/internal/agentctl/types/streams/git.go index 974270033c..212ff87b5c 100644 --- a/apps/backend/internal/agentctl/types/streams/git.go +++ b/apps/backend/internal/agentctl/types/streams/git.go @@ -97,6 +97,29 @@ type GitStatusUpdate struct { BranchDeletions int `json:"branch_deletions,omitempty"` } +// FileChangeFacet is one layer of a mixed file change. A mixed path can have +// one change between HEAD and the index plus another between the index and the +// working tree. +type FileChangeFacet struct { + // Status indicates the layer status: "modified", "added", "deleted", or "renamed". + Status string `json:"status"` + + // Additions is the number of added lines in this layer. + Additions int `json:"additions,omitempty"` + + // Deletions is the number of deleted lines in this layer. + Deletions int `json:"deletions,omitempty"` + + // OldPath is the original path for a rename in this layer. + OldPath string `json:"old_path,omitempty"` + + // Diff contains the unified diff content for this layer. + Diff string `json:"diff,omitempty"` + + // DiffSkipReason explains why this layer's diff was omitted or truncated. + DiffSkipReason string `json:"diff_skip_reason,omitempty"` +} + // FileInfo represents detailed information about a file's git status. type FileInfo struct { // Path is the file path relative to workspace root. @@ -124,6 +147,12 @@ type FileInfo struct { // DiffSkipReason explains why diff content was omitted or truncated. // Values: "too_large", "binary", "truncated", "budget_exceeded". DiffSkipReason string `json:"diff_skip_reason,omitempty"` + + // StagedChange and UnstagedChange preserve the two layers when the same + // path has both index and working-tree changes. Single-layer paths keep the + // compact legacy fields above and omit both facets. + StagedChange *FileChangeFacet `json:"staged_change,omitempty"` + UnstagedChange *FileChangeFacet `json:"unstaged_change,omitempty"` } // GitCommitNotification is sent when a new commit is detected in the workspace. diff --git a/apps/backend/internal/agentctl/types/types.go b/apps/backend/internal/agentctl/types/types.go index 88a91d3658..46d5b584e1 100644 --- a/apps/backend/internal/agentctl/types/types.go +++ b/apps/backend/internal/agentctl/types/types.go @@ -31,6 +31,7 @@ type ( GitCommitNotification = streams.GitCommitNotification GitResetNotification = streams.GitResetNotification GitBranchSwitchNotification = streams.GitBranchSwitchNotification + FileChangeFacet = streams.FileChangeFacet FileInfo = streams.FileInfo // File stream types diff --git a/apps/web/components/review/review-diff-list.tsx b/apps/web/components/review/review-diff-list.tsx index b33c93b352..2318176038 100644 --- a/apps/web/components/review/review-diff-list.tsx +++ b/apps/web/components/review/review-diff-list.tsx @@ -90,8 +90,8 @@ export const ReviewDiffList = memo(function ReviewDiffList({ // All in-memory state (selectedFile, reviewedFiles, staleFiles, fileRefs) // is keyed by `reviewFileKey(file)` so two files at the same path in // different repos (e.g. `kandev/README.md` + `lvc/README.md`) get - // distinct rows. Without this every multi-repo review with same-named - // files would mark them all reviewed when one is checked. + // distinct rows. Layer-qualified mixed changes use the same key helper so + // reviewing one facet does not mark the other facet reviewed. const selectedIndex = selectedFile ? files.findIndex((f) => reviewFileKey(f) === selectedFile) : -1; diff --git a/apps/web/components/review/types.multi-repo.test.ts b/apps/web/components/review/types.multi-repo.test.ts index 8bb03a598f..4f9bea66ee 100644 --- a/apps/web/components/review/types.multi-repo.test.ts +++ b/apps/web/components/review/types.multi-repo.test.ts @@ -163,6 +163,14 @@ describe("reviewFileKey", () => { const b = reviewFileKey({ path: "README.md", repository_name: "backend" }); expect(a).not.toBe(b); }); + + it("keeps mixed layers distinct without colliding with repository keys", () => { + const staged = reviewFileKey({ path: "mixed.ts", change_layer: "staged" }); + const repositoryFile = reviewFileKey({ path: "staged", repository_name: "mixed.ts" }); + + expect(staged).toBe(`mixed.ts${SEP}${SEP}staged`); + expect(staged).not.toBe(repositoryFile); + }); }); describe("resolvePRReviewRepositoryName", () => { @@ -240,6 +248,20 @@ describe("splitReviewFileKey", () => { }); }); + it("round-trips a layer-qualified repository key", () => { + const key = reviewFileKey({ + path: APP_PATH, + repository_name: "frontend", + change_layer: "unstaged", + }); + + expect(splitReviewFileKey(key)).toEqual({ + repositoryName: "frontend", + path: APP_PATH, + changeLayer: "unstaged", + }); + }); + it("keeps a legacy bare-path key without a repository scope", () => { expect(splitReviewFileKey(APP_PATH)).toEqual({ repositoryName: "", path: APP_PATH }); }); diff --git a/apps/web/components/review/types.ts b/apps/web/components/review/types.ts index 604520da60..224b80d604 100644 --- a/apps/web/components/review/types.ts +++ b/apps/web/components/review/types.ts @@ -1,6 +1,16 @@ import { djb2Hash } from "@/lib/utils/hash"; import { t } from "@/lib/i18n"; import type { FileChangeStatus } from "@/lib/utils/file-change-status"; +import type { GitChangeLayer } from "@/lib/state/slices/session-runtime/types"; + +export type ReviewChangeFacet = { + diff: string; + status: FileChangeStatus; + additions: number; + deletions: number; + old_path?: string; + diff_skip_reason?: "too_large" | "binary" | "truncated" | "budget_exceeded"; +}; export type ReviewFile = { path: string; @@ -12,6 +22,10 @@ export type ReviewFile = { source: "uncommitted" | "committed" | "pr"; old_path?: string; diff_skip_reason?: "too_large" | "binary" | "truncated" | "budget_exceeded"; + staged_change?: ReviewChangeFacet; + unstaged_change?: ReviewChangeFacet; + /** Frontend-only layer selected from a mixed uncommitted file. */ + change_layer?: GitChangeLayer; /** * Repository this file belongs to. Set on multi-repo task changes so the * file tree groups files under per-repo top-level nodes. Optional for @@ -35,13 +49,22 @@ export type ReviewFile = { * keep the legacy bare-path key for backwards compatibility with existing * `session_file_reviews` rows. An explicit empty name represents the real * workspace root in a multi-repo payload and therefore keeps the separator. + * Layer-qualified mixed changes append a double-NUL marker plus the layer; + * the doubled marker cannot collide with a legacy repository/path key. */ const FILE_KEY_SEP = "\u0000"; +const FILE_LAYER_KEY_SEP = `${FILE_KEY_SEP}${FILE_KEY_SEP}`; -export function reviewFileKey(file: { path: string; repository_name?: string }): string { - return file.repository_name === undefined - ? file.path - : `${file.repository_name}${FILE_KEY_SEP}${file.path}`; +export function reviewFileKey(file: { + path: string; + repository_name?: string; + change_layer?: GitChangeLayer; +}): string { + const baseKey = + file.repository_name === undefined + ? file.path + : `${file.repository_name}${FILE_KEY_SEP}${file.path}`; + return file.change_layer ? `${baseKey}${FILE_LAYER_KEY_SEP}${file.change_layer}` : baseKey; } /** Mirrors backend `worktree.SanitizeRepoDirName`, which defines the @@ -124,10 +147,21 @@ export function getCumulativeReviewRepositoryNames( return Array.from(names); } -export function splitReviewFileKey(key: string): { repositoryName: string; path: string } { - const sep = key.indexOf(FILE_KEY_SEP); - if (sep < 0) return { repositoryName: "", path: key }; - return { repositoryName: key.slice(0, sep), path: key.slice(sep + 1) }; +export function splitReviewFileKey(key: string): { + repositoryName: string; + path: string; + changeLayer?: GitChangeLayer; +} { + const layerSep = key.lastIndexOf(FILE_LAYER_KEY_SEP); + const layer = layerSep < 0 ? undefined : key.slice(layerSep + FILE_LAYER_KEY_SEP.length); + const changeLayer = layer === "staged" || layer === "unstaged" ? layer : undefined; + const baseKey = changeLayer ? key.slice(0, layerSep) : key; + const sep = baseKey.indexOf(FILE_KEY_SEP); + const identity = + sep < 0 + ? { repositoryName: "", path: baseKey } + : { repositoryName: baseKey.slice(0, sep), path: baseKey.slice(sep + 1) }; + return changeLayer ? { ...identity, changeLayer } : identity; } export function diffSkipReasonLabel(reason?: string): string { diff --git a/apps/web/components/task/changes-diff-target.ts b/apps/web/components/task/changes-diff-target.ts index 5c8f5b714f..ff400a2d2c 100644 --- a/apps/web/components/task/changes-diff-target.ts +++ b/apps/web/components/task/changes-diff-target.ts @@ -1,4 +1,7 @@ +import type { GitChangeLayer } from "@/lib/state/slices/session-runtime/types"; + export type DiffSource = "uncommitted" | "committed" | "pr"; +export type ChangeLayer = GitChangeLayer; export type LocalCommitDetailTarget = { source: "local"; @@ -23,6 +26,7 @@ export type OpenDiffOptions = { source?: DiffSource; repositoryName?: string; prKey?: string; + changeLayer?: ChangeLayer; }; export type DiffSheetMode = @@ -33,5 +37,6 @@ export type DiffSheetMode = sourceFilter?: "all" | DiffSource; repositoryName?: string; prKey?: string; + changeLayer?: ChangeLayer; } | { kind: "commit"; target: CommitDetailTarget }; diff --git a/apps/web/components/task/changes-panel-banner.tsx b/apps/web/components/task/changes-panel-banner.tsx index fd4753264f..6a971ac11f 100644 --- a/apps/web/components/task/changes-panel-banner.tsx +++ b/apps/web/components/task/changes-panel-banner.tsx @@ -1,6 +1,7 @@ import { useMemo } from "react"; import { IconAlertTriangle } from "@tabler/icons-react"; import { reviewFileKey } from "@/components/review/types"; +import type { GitChangeLayer } from "@/lib/state/slices/session-runtime/types"; import { useTranslation } from "react-i18next"; /** Composite key for the file currently selected in single-file mode. */ @@ -8,13 +9,18 @@ export function useSelectedFileKey( mode: "all" | "file", filePath: string | undefined, fileRepositoryName: string | undefined, + changeLayer?: GitChangeLayer, ): string | undefined { return useMemo( () => mode === "file" && filePath - ? reviewFileKey({ path: filePath, repository_name: fileRepositoryName }) + ? reviewFileKey({ + path: filePath, + repository_name: fileRepositoryName, + change_layer: changeLayer, + }) : undefined, - [mode, filePath, fileRepositoryName], + [mode, filePath, fileRepositoryName, changeLayer], ); } diff --git a/apps/web/components/task/changes-panel-file-row.test.tsx b/apps/web/components/task/changes-panel-file-row.test.tsx index c00397731a..63666d80a9 100644 --- a/apps/web/components/task/changes-panel-file-row.test.tsx +++ b/apps/web/components/task/changes-panel-file-row.test.tsx @@ -96,7 +96,12 @@ describe("FileRow truncation (regression: path overlaps diff stats in narrow pan