fix(git): make archived-task git snapshot replay crash-proof and lifecycle-aware (#2851)

Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
Clément Fiere
2026-08-20 22:20:47 +02:00
committed by GitHub
parent 0243663ec6
commit 08f07b709e
23 changed files with 1634 additions and 61 deletions
@@ -6,6 +6,7 @@ import (
"fmt"
"os/exec"
"sort"
"strconv"
"strings"
"github.com/kandev/kandev/internal/common/unidiff"
@@ -270,7 +271,13 @@ func (g *GitOperator) GetCumulativeDiff(ctx context.Context, baseCommit string)
// `-` with nothing to compare — but both routes report 0/0 for them anyway,
// since a binary block carries no hunk.) Adding the flag would change a
// production git invocation for a difference no test can observe.
diffOutput, err := g.runGitCommand(ctx, "diff", baseCommit)
diffOutput, err := g.runGitCommand(
ctx,
"diff",
"--src-prefix=a/",
"--dst-prefix=b/",
baseCommit,
)
if err != nil {
result.Error = fmt.Sprintf("failed to get diff: %s", err.Error())
return result, nil
@@ -406,6 +413,8 @@ func (g *GitOperator) ShowCommit(ctx context.Context, commitSHA string) (*Commit
"--stat",
"--numstat",
"-p",
"--src-prefix=a/",
"--dst-prefix=b/",
commitSHA,
)
if err != nil {
@@ -447,45 +456,36 @@ func (g *GitOperator) parseCommitDiff(output string) map[string]interface{} {
func (g *GitOperator) parseCommitDiffWithOptions(output string, opts parseCommitDiffOptions) map[string]interface{} {
files := make(map[string]interface{})
// Split by "diff --git" to get individual file diffs
parts := strings.Split(output, "diff --git ")
if len(parts) <= 1 {
// Split into per-file sections. The split is line-aware: only a line that
// STARTS with `diff --git ` begins a new section, because git only emits
// that token at the start of a section header. Splitting the raw byte
// stream would truncate a real section when a patch body or filename
// contains the literal sequence (e.g. an added line
// `+diff --git a/example b/example`) and fabricate a bogus section.
prefix, sections := splitDiffSections(output)
if len(sections) == 0 {
return files
}
// When the caller's git command included --numstat, git printed authoritative
// per-file counts ahead of the first "diff --git" — exactly the text the split
// above discards. Recover them; files without a row fall back to counting
// discards. Recover them; files without a row fall back to counting
// their own patch body.
numstat := numstatByPath(parts[0])
numstat := numstatByPath(prefix)
totalDiffBytes := 0
for _, part := range parts[1:] {
if part == "" {
for _, diffContent := range sections {
// Extract the file path. git's per-file `+++ b/<path>` line is the
// authoritative source: it carries exactly one path, so spaces and
// literal ` b/` sequences inside a filename can't be confused with
// the old/new separator, and git C-quotes paths containing tabs,
// non-ASCII bytes, quotes, or backslashes (core.quotePath, default
// on) — a header-only split would drop those entirely.
filePath, ok := diffSectionPath(diffContent)
if !ok {
continue
}
// Re-add the "diff --git " prefix
diffContent := "diff --git " + part
// Extract file path from the diff header
// Format: diff --git a/path/to/file b/path/to/file
lines := strings.SplitN(diffContent, "\n", 2)
if len(lines) == 0 {
continue
}
header := lines[0]
// Extract path from "diff --git a/<path> b/<path>".
// We cannot split by space because paths may contain spaces.
// Instead, strip the known prefix and find the " b/" separator.
pathsPart := strings.TrimPrefix(header, "diff --git ")
bIdx := strings.Index(pathsPart, " b/")
if bIdx == -1 {
continue
}
filePath := pathsPart[bIdx+3:]
// Determine file status from diff content
status := fileStatusModified
switch {
@@ -504,6 +504,7 @@ func (g *GitOperator) parseCommitDiffWithOptions(output string, opts parseCommit
totalDiffBytes += len(diffOut)
fileEntry := map[string]interface{}{
"path": filePath,
"status": status,
"staged": false,
"additions": additions,
@@ -519,6 +520,308 @@ func (g *GitOperator) parseCommitDiffWithOptions(output string, opts parseCommit
return files
}
// diffSectionPath extracts the new-side path of one `diff --git` section.
// The `+++ b/<path>` file line is the authoritative source: git writes it
// once per section, exactly one path per line, so spaces and literal ` b/`
// sequences inside filenames can't confuse the old/new split, and git
// C-quotes paths containing special bytes (tabs, non-ASCII, quotes,
// backslashes) the same way it quotes the header. Sections that carry no
// file lines fall back to other unambiguous per-section lines before the
// `diff --git` header:
// - deleted files: `--- a/<path>` (the `+++` side is `/dev/null`);
// - pure renames without hunks: `rename to <path>`;
// - binary sections: `Binary files a/<old> and b/<new> differ`;
// - mode-only changes: the quoted/unquoted header itself.
//
// A single trailing tab on a file line is git's marker for paths that end in
// whitespace; it is stripped but any real trailing whitespace is kept.
func diffSectionPath(diffContent string) (string, bool) {
lines := strings.Split(diffContent, "\n")
// First pass prefers the `+++ b/<path>` line: in a rename section the
// `--- a/<old>` line precedes it, and the new-side path is what the
// changes panel shows. `+++ /dev/null` (deleted files) yields nothing,
// so the second pass falls back to the `--- a/<path>` old-side line.
for _, line := range lines {
if path, ok := diffFileLinePath("+++", line); ok {
return path, true
}
}
for _, line := range lines {
if path, ok := diffFileLinePath("---", line); ok {
return path, true
}
}
// Pure renames (similarity 100%, no hunks) emit rename from/to lines with
// no ---/+++ file lines; `rename to <path>` is one path per line.
for _, line := range lines {
if path, ok := diffWholeLinePath("rename to ", line); ok {
return path, true
}
}
// Binary sections emit `Binary files a/<old> and b/<new> differ` with no
// ---/+++ lines; parse the new-side path off that line.
for _, line := range lines {
if path, ok := diffBinaryFilesNewPath(line); ok {
return path, true
}
}
// Mode-only changes (old mode/new mode) have no path-bearing line at all;
// parse the header, handling git's C-quoted form.
return diffHeaderNewPath(lines[0])
}
// splitDiffSections splits git diff/show output into the pre-header prefix
// (numstat rows when --numstat was used) and one section per file, each
// section starting with its `diff --git ` header line. The split only breaks
// at lines that begin with the token — git's actual section-boundary
// grammar — so the token appearing inside a patch body or filename cannot
// truncate a real section or create a bogus one.
func splitDiffSections(output string) (prefix string, sections []string) {
first := firstDiffHeaderIndex(output)
prefix = output[:first]
rest := output[first:]
for rest != "" {
// rest starts at a `diff --git ` header line; the next section
// begins at the next line that starts with the token.
next := strings.Index(rest, "\ndiff --git ")
if next == -1 {
sections = append(sections, rest)
break
}
sections = append(sections, rest[:next+1])
rest = rest[next+1:]
}
return prefix, sections
}
// firstDiffHeaderIndex returns the byte offset of the first line that starts
// with `diff --git `, or len(s) when there is none.
func firstDiffHeaderIndex(s string) int {
for i := 0; i < len(s); {
lineEnd := i
for lineEnd < len(s) && s[lineEnd] != '\n' {
lineEnd++
}
if strings.HasPrefix(s[i:lineEnd], "diff --git ") {
return i
}
if lineEnd >= len(s) {
return len(s)
}
i = lineEnd + 1
}
return len(s)
}
// diffHeaderNewPath parses the new-side path out of a `diff --git` header,
// supporting git's C-quoted form. The unquoted form (`a/<old> b/<new>`) can
// only be split at the first ` b/` — a path that itself contains ` b/`
// renders unquoted and is inherently ambiguous in git's text output; the
// per-line sources above disambiguate every section that carries them.
func diffHeaderNewPath(header string) (string, bool) {
rest, ok := strings.CutPrefix(header, "diff --git ")
if !ok {
return "", false
}
if strings.HasPrefix(rest, `"`) {
// Quoted: `"a/<old>" "b/<new>"`. Both tokens are C-quoted, so scan
// the first token's closing quote escape-aware: a `\"` inside the
// path must not be read as the end of the token. The second token
// starts right after the closing quote and the separating space.
oldEnd := quotedTokenEnd(rest)
if oldEnd < 0 || oldEnd >= len(rest) || rest[oldEnd] != ' ' {
return "", false
}
after := rest[oldEnd+1:]
if !strings.HasPrefix(after, `"`) {
return "", false
}
newEnd := quotedTokenEnd(after)
if newEnd < 0 {
return "", false
}
newPath := unquoteGitPath(after[:newEnd])
return strings.TrimPrefix(newPath, "b/"), true
}
// Unquoted: `a/<old> b/<new>`. A path that itself contains ` b/` renders
// unquoted and the first occurrence splits inside the old path. Mode-only
// changes (the main consumer of this fallback) always carry identical
// old/new paths, so scan candidates and prefer the separator whose two
// sides unquote to equal paths — the same rule the binary parser uses.
// Fall back to the first candidate when none match (a hypothetical
// rename-without-rename-lines would land here).
firstSep := -1
search := rest
base := 0
for {
sep := strings.Index(search, " b/")
if sep < 0 {
break
}
// `abs` points at the space; the `b/` prefix of the new side starts
// one byte later, so the right side (prefix included) is rest[abs+1:].
abs := base + sep
if firstSep == -1 {
firstSep = abs
}
left, leftOK := headerSidePath(rest[:abs], "a/")
right, rightOK := headerSidePath(rest[abs+1:], "b/")
if leftOK && rightOK && left == right {
return right, true
}
base = abs + 3
search = rest[base:]
}
if firstSep == -1 {
return "", false
}
return unquoteGitPath(rest[firstSep+3:]), true
}
// headerSidePath strips a fixed a/ or b/ prefix (optionally inside git's
// C-quotes) off one side of a diff header or Binary files line, returning the
// unquoted path.
func headerSidePath(side, prefix string) (string, bool) {
if strings.HasPrefix(side, `"`) {
unquoted, err := strconv.Unquote(side)
if err != nil {
return "", false
}
side = unquoted
}
if !strings.HasPrefix(side, prefix) {
return "", false
}
return strings.TrimPrefix(side, prefix), true
}
// diffWholeLinePath reads a path off a line whose entire remainder is the
// path (`rename to <path>`, `rename from <path>`), C-unquoting when git
// quoted it. One path per line, so spaces inside the path are unambiguous.
func diffWholeLinePath(prefix, line string) (string, bool) {
rest, ok := strings.CutPrefix(line, prefix)
if !ok {
return "", false
}
if rest == "" {
return "", false
}
return unquoteGitPath(rest), true
}
// quotedTokenEnd returns the index just past the closing quote of the first
// C-quoted token in s (which must start with `"`), or -1 when unterminated.
// Escapes are honored: a `\"` inside the path is content, not a terminator.
func quotedTokenEnd(s string) int {
if !strings.HasPrefix(s, `"`) {
return -1
}
for i := 1; i < len(s); i++ {
switch s[i] {
case '\\':
i++ // skip the escaped byte
case '"':
return i + 1
}
}
return -1
}
// diffBinaryFilesNewPath parses the new-side path off a
// `Binary files a/<old> and b/<new> differ` line (git's shape for binary
// sections, which carry no ---/+++ file lines). Both sides may be C-quoted:
// `Binary files "a/<old>" and "b/<new>" differ`.
//
// For a binary content change git emits the SAME path on both sides, so the
// separator whose two sides unquote to equal paths is authoritative — this
// resolves paths that themselves contain ` and b/` (a valid filename; the
// first-candidate scan would split inside the old path). When no candidate
// matches (binary rename with differing paths), the first ` and ` whose
// right side starts the `b/` token wins.
func diffBinaryFilesNewPath(line string) (string, bool) {
rest, ok := strings.CutPrefix(line, "Binary files ")
if !ok {
return "", false
}
rest = strings.TrimSuffix(rest, " differ")
if rest == "" {
return "", false
}
firstSep := -1
search := rest
base := 0
for {
sep := strings.Index(search, " and ")
if sep < 0 {
break
}
abs := base + sep
right := search[sep+5:]
rightBare := strings.TrimPrefix(right, `"`)
if strings.HasPrefix(rightBare, "b/") {
if firstSep == -1 {
firstSep = abs
}
// Equal-paths check: the left side is everything before this
// candidate separator; both sides may be quoted.
leftPath, leftOK := binarySidePath(rest[:abs])
rightPath, rightOK := binarySidePath(right)
if leftOK && rightOK && leftPath == rightPath {
return rightPath, true
}
}
base = abs + 5
search = rest[base:]
}
if firstSep == -1 {
return "", false
}
newSide, ok := binarySidePath(rest[firstSep+5:])
if !ok {
return "", false
}
return newSide, true
}
// binarySidePath strips the `a/` or `b/` prefix (optionally inside git's
// C-quotes) off one side of a `Binary files` line, returning the unquoted
// path.
func binarySidePath(side string) (string, bool) {
if strings.HasPrefix(side, `"`) {
unquoted, err := strconv.Unquote(side)
if err != nil {
return "", false
}
side = unquoted
}
if !strings.HasPrefix(side, "a/") && !strings.HasPrefix(side, "b/") {
return "", false
}
return side[2:], true
}
// diffFileLinePath reads the path off one `+++ <path>` or `--- <path>` file
// line (per git's per-section file headers), returning ok=false for
// `/dev/null` sides and non-file lines. Quoted paths (git C-quoting) carry
// the `a/`/`b/` prefix inside the quotes; the trailing-tab whitespace marker
// sits outside them, so it is stripped before unquoting.
func diffFileLinePath(prefix, line string) (string, bool) {
rest, ok := strings.CutPrefix(line, prefix+" ")
if !ok {
return "", false
}
if strings.HasPrefix(rest, "/dev/null") {
return "", false
}
path := unquoteGitPath(strings.TrimSuffix(rest, "\t"))
if prefix == "+++" {
return strings.TrimPrefix(path, "b/"), true
}
return strings.TrimPrefix(path, "a/"), true
}
// numstatByPath indexes the `<adds>\t<dels>\t<path>` rows git writes for
// --numstat by their new-side path (parseNumstatEntry already expands git's
// `dir/{old => new}` rename notation, so the key matches the path
@@ -0,0 +1,313 @@
package process
// Special-path and section-boundary parser regressions for
// parseCommitDiffWithOptions. Kept in their own file (per the backend
// convention that new tests go in a new file rather than growing an
// already-large test file): the cases cover git's C-quoted paths (tab,
// non-ASCII octal, embedded quotes), ` b/`- and ` and b/`-containing names,
// binary/rename/mode-only sections, and header tokens inside patch bodies.
import (
"context"
"os"
"path/filepath"
"strings"
"testing"
)
// TestParseCommitDiffWithOptions_SpecialPaths guards path extraction against
// git's quoting and separator rules. git C-quotes paths containing tabs,
// non-ASCII bytes, quotes, or backslashes (core.quotePath, default on) —
// a naive first-` b/` split drops them entirely — and a path containing the
// literal sequence ` b/` mis-splits the header. The parser must read the
// path off the per-file `+++ b/<path>` line, which git writes once per
// section and is unambiguous.
func TestParseCommitDiffWithOptions_SpecialPaths(t *testing.T) {
tests := []struct {
name string
output string
wantPath string
}{
{
name: "tab in path is C-quoted by git",
output: "diff --git \"a/a\\tb.txt\" \"b/a\\tb.txt\"\n" +
"new file mode 100644\n" +
"index 0000000..e69de29\n" +
"--- /dev/null\n" +
"+++ \"b/a\\tb.txt\"\n" +
"@@ -0,0 +1 @@\n" +
"+content\n",
wantPath: "a\tb.txt",
},
{
name: "non-ASCII path is C-quoted with octal escapes",
output: "diff --git \"a/caf\\303\\251.txt\" \"b/caf\\303\\251.txt\"\n" +
"new file mode 100644\n" +
"--- /dev/null\n" +
"+++ \"b/caf\\303\\251.txt\"\n" +
"@@ -0,0 +1 @@\n" +
"+caf\u00e9\n",
wantPath: "caf\u00e9.txt",
},
{
name: "quote char in path is C-quoted",
output: "diff --git \"a/has\\\"quote.txt\" \"b/has\\\"quote.txt\"\n" +
"new file mode 100644\n" +
"--- /dev/null\n" +
"+++ \"b/has\\\"quote.txt\"\n" +
"@@ -0,0 +1 @@\n" +
"+content\n",
wantPath: "has\"quote.txt",
},
{
name: "path containing literal b-slash splits via the +++ line",
output: "diff --git a/name b/dir.txt b/name b/dir.txt\n" +
"new file mode 100644\n" +
"--- /dev/null\n" +
"+++ b/name b/dir.txt\n" +
"@@ -0,0 +1 @@\n" +
"+content\n",
wantPath: "name b/dir.txt",
},
{
name: "trailing-whitespace path keeps the tab marker stripped",
output: "diff --git a/trail.txt b/trail.txt \n" +
"index 111..222 100644\n" +
"--- a/trail.txt \t\n" +
"+++ b/trail.txt \t\n" +
"@@ -1 +1 @@\n" +
"-old\n" +
"+new\n",
wantPath: "trail.txt ",
},
{
name: "quoted path ending in whitespace strips the outer marker",
output: "diff --git \"a/weird \\\"quote.txt \" \"b/weird \\\"quote.txt \"\n" +
"index 111..222 100644\n" +
"--- \"a/weird \\\"quote.txt \"\t\n" +
"+++ \"b/weird \\\"quote.txt \"\t\n" +
"@@ -1 +1 @@\n" +
"-old\n" +
"+new\n",
wantPath: "weird \"quote.txt ",
},
{
name: "renamed file keeps the new path from the +++ line",
output: "diff --git a/apps/old/file.go b/apps/new/file.go\n" +
"similarity index 90%\n" +
"rename from apps/old/file.go\n" +
"rename to apps/new/file.go\n" +
"--- a/apps/old/file.go\n" +
"+++ b/apps/new/file.go\n" +
"@@ -1,2 +1,3 @@\n" +
" package p\n" +
"+added\n",
wantPath: "apps/new/file.go",
},
{
name: "pure rename without hunks keeps the rename to line path",
output: "diff --git a/old b.txt b/new b.txt\n" +
"similarity index 100%\n" +
"rename from old b.txt\n" +
"rename to new b.txt\n",
wantPath: "new b.txt",
},
{
name: "binary section with b-slash path parses the Binary files line",
output: "diff --git a/name b/binary.bin b/name b/binary.bin\n" +
"index 6735744..d7bf111 100644\n" +
"Binary files a/name b/binary.bin and b/name b/binary.bin differ\n",
wantPath: "name b/binary.bin",
},
{
name: "quoted binary section unquotes the Binary files line",
output: "diff --git \"a/caf\\303\\251 b.bin\" \"b/caf\\303\\251 b.bin\"\n" +
"index a6a3e7f..073ea92 100644\n" +
"Binary files \"a/caf\\303\\251 b.bin\" and \"b/caf\\303\\251 b.bin\" differ\n",
wantPath: "caf\u00e9 b.bin",
},
{
name: "mode-only change on a quoted path parses the quoted header",
output: "diff --git \"a/caf\\303\\251 b/mode.sh\" \"b/caf\\303\\251 b/mode.sh\"\n" +
"old mode 100644\n" +
"new mode 100755\n",
wantPath: "caf\u00e9 b/mode.sh",
},
{
name: "mode-only change with escaped quote in path parses the quoted header",
output: "diff --git \"a/has\\\"quote.sh\" \"b/has\\\"quote.sh\"\n" +
"old mode 100644\n" +
"new mode 100755\n",
wantPath: "has\"quote.sh",
},
{
name: "mode-only change with b-slash path splits on the equal-paths separator",
output: "diff --git a/mode name b/dir.sh b/mode name b/dir.sh\n" +
"old mode 100644\n" +
"new mode 100755\n",
wantPath: "mode name b/dir.sh",
},
{
name: "mode-only change with quote and b-slash in path",
output: "diff --git \"a/weird\\\"quote b/file.sh\" \"b/weird\\\"quote b/file.sh\"\n" +
"old mode 100644\n" +
"new mode 100755\n",
wantPath: "weird\"quote b/file.sh",
},
{
name: "binary path containing and b-slash splits on the equal-paths separator",
output: "diff --git a/old and b/thing.bin b/old and b/thing.bin\n" +
"index 6735744..d7bf111 100644\n" +
"Binary files a/old and b/thing.bin and b/old and b/thing.bin differ\n",
wantPath: "old and b/thing.bin",
},
{
name: "binary rename with and b-slash in new path keeps the new path",
output: "diff --git a/x.bin b/old and b/thing.bin\n" +
"similarity index 50%\n" +
"rename from x.bin\n" +
"rename to old and b/thing.bin\n" +
"Binary files a/x.bin and b/old and b/thing.bin differ\n",
wantPath: "old and b/thing.bin",
},
}
gitOp := &GitOperator{}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
files := gitOp.parseCommitDiffWithOptions(tt.output, parseCommitDiffOptions{})
entry, ok := files[tt.wantPath].(map[string]interface{})
if !ok {
t.Fatalf("no entry for %q; got keys %v", tt.wantPath, keysOf(files))
}
if got, _ := entry["path"].(string); got != tt.wantPath {
t.Errorf("entry path = %#v, want %#v", got, tt.wantPath)
}
})
}
}
// TestGetCumulativeDiff_ModeOnlyBSlashPath covers the round-5 blocker with
// real git output: a chmod-only change on a path containing the literal
// sequence ` b/` produces an unquoted header with no ---/+++ lines
// (`diff --git a/mode name b/dir.sh b/mode name b/dir.sh` + old mode/new
// mode). The first ` b/` lies inside the old path, so the equal-paths
// separator rule must select the correct split.
func TestGetCumulativeDiff_ModeOnlyBSlashPath(t *testing.T) {
repoDir, cleanup := setupTestRepo(t)
t.Cleanup(cleanup)
if err := os.MkdirAll(filepath.Join(repoDir, "mode name b"), 0o755); err != nil {
t.Fatalf("mkdir: %v", err)
}
writeFile(t, repoDir, "mode name b/dir.sh", "#!/bin/sh\necho hi\n")
runGit(t, repoDir, "add", ".")
runGit(t, repoDir, "commit", "-m", "seed: add script")
base := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD"))
// Pin core.filemode so the chmod below reliably produces a mode-only
// diff. Git's default (auto) disables executable-bit tracking on
// filesystems without chmod support; on those the test cannot produce
// the fixture it exists to prove, so skip with an explicit reason rather
// than fail.
runGit(t, repoDir, "config", "core.filemode", "true")
if err := os.Chmod(filepath.Join(repoDir, "mode name b/dir.sh"), 0o755); err != nil {
t.Fatalf("chmod: %v", err)
}
if diff := runGit(t, repoDir, "diff"); !strings.Contains(diff, "old mode") {
t.Skipf("filesystem does not track executable bits; skipping mode-only regression (diff=%q)", diff)
}
gitOp := NewGitOperator(repoDir, newTestLogger(t), nil)
result, err := gitOp.GetCumulativeDiff(context.Background(), base)
if err != nil {
t.Fatalf("GetCumulativeDiff returned error: %v", err)
}
if !result.Success {
t.Fatalf("GetCumulativeDiff failed: %s", result.Error)
}
const wantPath = "mode name b/dir.sh"
if _, ok := result.Files[wantPath].(map[string]interface{}); !ok {
t.Fatalf("no entry for %q; got keys %v", wantPath, keysOf(result.Files))
}
}
func TestGetCumulativeDiff_StablePrefixesIgnoreGitDiffConfig(t *testing.T) {
for _, diffConfig := range []struct {
name string
key string
value string
}{
{name: "no prefix", key: "diff.noprefix", value: "true"},
{name: "mnemonic prefix", key: "diff.mnemonicPrefix", value: "true"},
} {
t.Run(diffConfig.name, func(t *testing.T) {
repoDir, cleanup := setupTestRepo(t)
t.Cleanup(cleanup)
if err := os.MkdirAll(filepath.Join(repoDir, "src"), 0o755); err != nil {
t.Fatalf("mkdir src: %v", err)
}
writeFile(t, repoDir, "src/config.txt", "before\n")
runGit(t, repoDir, "add", ".")
runGit(t, repoDir, "commit", "-m", "seed config file")
base := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD"))
runGit(t, repoDir, "config", diffConfig.key, diffConfig.value)
writeFile(t, repoDir, "src/config.txt", "after\n")
result, err := NewGitOperator(repoDir, newTestLogger(t), nil).GetCumulativeDiff(
context.Background(), base,
)
if err != nil {
t.Fatalf("GetCumulativeDiff returned error: %v", err)
}
if !result.Success {
t.Fatalf("GetCumulativeDiff failed: %s", result.Error)
}
if _, ok := result.Files["src/config.txt"].(map[string]interface{}); !ok {
t.Fatalf("no entry for src/config.txt; got keys %v", keysOf(result.Files))
}
})
}
}
// TestParseCommitDiffWithOptions_HeaderTokenInsideBody guards the section
// boundary against a patch body (or filename) containing the literal
// `diff --git ` byte sequence. git only emits that token at the start of a
// section header line, so the split must be line-aware: an added line like
// `+diff --git a/example b/example` must not truncate the real section or
// fabricate a bogus one.
func TestParseCommitDiffWithOptions_HeaderTokenInsideBody(t *testing.T) {
output := "diff --git a/real.txt b/real.txt\n" +
"index 111..222 100644\n" +
"--- a/real.txt\n" +
"+++ b/real.txt\n" +
"@@ -1 +1,2 @@\n" +
"-old\n" +
"+diff --git a/example b/example\n" +
"+new\n"
gitOp := &GitOperator{}
files := gitOp.parseCommitDiffWithOptions(output, parseCommitDiffOptions{})
if len(files) != 1 {
t.Fatalf("got %d files, want exactly real.txt (%v)", len(files), keysOf(files))
}
entry, ok := files["real.txt"].(map[string]interface{})
if !ok {
t.Fatalf("no entry for real.txt; got keys %v", keysOf(files))
}
if got, _ := entry["path"].(string); got != "real.txt" {
t.Errorf("entry path = %#v, want real.txt", got)
}
diff, _ := entry["diff"].(string)
if !strings.Contains(diff, "+diff --git a/example b/example") || !strings.Contains(diff, "+new") {
t.Errorf("real.txt diff truncated by header-token line: %q", diff)
}
if got, _ := entry["additions"].(int); got != 2 {
t.Errorf("real.txt additions = %d, want 2 (the added header-token line counts)", got)
}
if _, ok := files["example"]; ok {
t.Errorf("bogus example entry created from patch-body token: %v", keysOf(files))
}
}
@@ -309,6 +309,44 @@ func TestShowCommit_CountsDashAndPlusPrefixedContent(t *testing.T) {
}
}
// TestParseCommitDiffWithOptions_EntriesCarryPath guards the file-entry
// contract the frontend's changes tree depends on: every entry must include
// its own `path` field, not just sit under the map key. The DB-snapshot
// fallback replays these entries as a git status_update, and the frontend
// builds its file tree from each entry's `path` — an entry without one
// crashes the changes panel (`Cannot read properties of undefined (reading
// 'split')`) for archived tasks whose live execution is gone.
func TestParseCommitDiffWithOptions_EntriesCarryPath(t *testing.T) {
output := "diff --git a/apps/a.go b/apps/a.go\n" +
"--- a/apps/a.go\n" +
"+++ b/apps/a.go\n" +
"@@ -1,3 +1,3 @@\n" +
" package a\n" +
"-old\n" +
"+new\n" +
"diff --git a/readme.md b/readme.md\n" +
"--- a/readme.md\n" +
"+++ b/readme.md\n" +
"@@ -1 +1 @@\n" +
"-hello\n" +
"+world\n"
gitOp := &GitOperator{}
files := gitOp.parseCommitDiffWithOptions(output, parseCommitDiffOptions{})
if len(files) != 2 {
t.Fatalf("got %d files, want 2 (%v)", len(files), keysOf(files))
}
for _, path := range []string{"apps/a.go", "readme.md"} {
entry, ok := files[path].(map[string]interface{})
if !ok {
t.Fatalf("no file entry for %q", path)
}
if got, _ := entry["path"].(string); got != path {
t.Errorf("entry %q path = %#v, want %q", path, entry["path"], path)
}
}
}
// TestGetCumulativeDiff_CountsDashAndPlusPrefixedContent covers the second
// caller, which builds its diff from `git diff <base>` rather than `git show`.
func TestGetCumulativeDiff_CountsDashAndPlusPrefixedContent(t *testing.T) {
@@ -405,8 +405,13 @@ func appendDBSnapshotGitStatus(ctx context.Context, taskRepo *sqliterepo.Reposit
"status": map[string]interface{}{
branchFieldKey: latestSnapshot.Branch,
"remote_branch": latestSnapshot.RemoteBranch,
"head_commit": latestSnapshot.HeadCommit,
"base_commit": latestSnapshot.BaseCommit,
"ahead": latestSnapshot.Ahead,
"behind": latestSnapshot.Behind,
"remote_ahead": metadata["remote_ahead"],
"remote_behind": metadata["remote_behind"],
"remote_head_commit": metadata["remote_head_commit"],
"files": latestSnapshot.Files,
"modified": metadata["modified"],
addedFieldKey: metadata[addedFieldKey],
@@ -77,6 +77,7 @@ func IsKnownSafeGitFlag(arg string) bool {
"--amend", "--allow-empty", "--soft", "--mixed", "--hard",
"--cached", "--force", "--source=HEAD", "--staged", "--worktree",
"--dry-run", "--get-all", "--first-parent", "--is-ancestor",
"--src-prefix=", "--dst-prefix=",
}
for _, safe := range safeFlags {
if strings.HasPrefix(arg, safe) {
@@ -2,6 +2,18 @@ package securityutil
import "testing"
// TestIsKnownSafeGitFlagAllowsDiffPrefixFlags pins the output-formatting flags
// GetCumulativeDiff and ShowCommit pass to force stable a/ and b/ path
// prefixes regardless of a user's diff.noprefix config. They only affect diff
// header formatting (never repository mutation), so any value form is safe.
func TestIsKnownSafeGitFlagAllowsDiffPrefixFlags(t *testing.T) {
for _, flag := range []string{"--src-prefix=a/", "--dst-prefix=b/", "--src-prefix=old/", "--dst-prefix=new/"} {
if !IsKnownSafeGitFlag(flag) {
t.Fatalf("IsKnownSafeGitFlag(%q) = false, want true — the cumulative-diff parser needs it", flag)
}
}
}
func TestIsValidBaseBranchRef(t *testing.T) {
cases := map[string]bool{
"main": true,
@@ -17,7 +17,7 @@ import (
"go.uber.org/zap"
agentruntime "github.com/kandev/kandev/internal/agent/runtime"
"github.com/kandev/kandev/internal/agent/runtime/agentctl"
client "github.com/kandev/kandev/internal/agent/runtime/agentctl"
"github.com/kandev/kandev/internal/agent/runtime/lifecycle"
"github.com/kandev/kandev/internal/agent/runtime/routingerr"
"github.com/kandev/kandev/internal/common/constants"
@@ -3435,13 +3435,27 @@ func (s *Service) captureArchiveDiff(ctx context.Context, sessionID, baseCommit
return
}
if err := s.repo.CreateGitSnapshot(ctx, &models.GitSnapshot{
snapshot := &models.GitSnapshot{
SessionID: sessionID,
SnapshotType: models.SnapshotTypeArchive,
HeadCommit: diffResult.HeadCommit,
BaseCommit: diffResult.BaseCommit,
Files: diffResult.Files,
}); err != nil {
}
status, statusErr := s.agentManager.GetGitStatusFresh(ctx, sessionID)
if statusErr != nil {
s.logger.Warn("failed to capture git status metadata for archive",
zap.String("session_id", sessionID),
zap.Error(statusErr))
} else if status != nil && status.Success {
snapshot.Branch = status.Branch
snapshot.RemoteBranch = status.RemoteBranch
snapshot.Ahead = status.Ahead
snapshot.Behind = status.Behind
snapshot.Metadata = archiveGitStatusMetadata(status, diffResult.Files)
}
if err := s.repo.CreateGitSnapshot(ctx, snapshot); err != nil {
s.logger.Warn("failed to save archive snapshot",
zap.String("session_id", sessionID),
zap.Error(err))
@@ -3454,6 +3468,29 @@ func (s *Service) captureArchiveDiff(ctx context.Context, sessionID, baseCommit
zap.Int("total_commits", diffResult.TotalCommits))
}
func archiveGitStatusMetadata(status *client.GitStatusResult, files map[string]interface{}) map[string]interface{} {
if status == nil {
return nil
}
return map[string]interface{}{
"timestamp": status.Timestamp,
// The archive's files map is a cumulative diff, while these status
// lists describe the working tree at capture time. Keep an explicit
// count for summary consumers so they do not add two different views.
"changed_files": len(files),
"modified": status.Modified,
"added": status.Added,
"deleted": status.Deleted,
"untracked": status.Untracked,
"renamed": status.Renamed,
"remote_ahead": status.RemoteAhead,
"remote_behind": status.RemoteBehind,
"remote_head_commit": status.RemoteHeadCommit,
"branch_additions": status.BranchAdditions,
"branch_deletions": status.BranchDeletions,
}
}
// parseCommitTime parses a commit timestamp from git log output.
// Returns UTC time to ensure consistent timestamps across environments.
func parseCommitTime(s string) time.Time {
@@ -0,0 +1,50 @@
package orchestrator
import (
"testing"
client "github.com/kandev/kandev/internal/agent/runtime/agentctl"
)
func TestArchiveGitStatusMetadataPreservesStatusSummary(t *testing.T) {
status := &client.GitStatusResult{
Timestamp: "2026-08-20T14:00:00Z",
Modified: []string{"src/app.ts"},
Added: []string{"src/new.ts"},
Deleted: []string{"src/old.ts"},
Untracked: []string{"tmp.txt"},
Renamed: []string{"src/old-name.ts -> src/new-name.ts"},
RemoteAhead: 2,
RemoteBehind: 1,
RemoteHeadCommit: "remote-head",
BranchAdditions: 8,
BranchDeletions: 3,
}
metadata := archiveGitStatusMetadata(status, map[string]interface{}{
"src/app.ts": map[string]interface{}{"status": "modified"},
"src/new.ts": map[string]interface{}{"status": "added"},
"src/old.ts": map[string]interface{}{"status": "deleted"},
"src/renamed": map[string]interface{}{"status": "renamed"},
})
if metadata["timestamp"] != status.Timestamp {
t.Errorf("timestamp = %#v, want %q", metadata["timestamp"], status.Timestamp)
}
if metadata["changed_files"] != 4 {
t.Errorf("changed_files = %#v, want 4", metadata["changed_files"])
}
if metadata["branch_additions"] != status.BranchAdditions {
t.Errorf("branch_additions = %#v, want %d", metadata["branch_additions"], status.BranchAdditions)
}
if metadata["branch_deletions"] != status.BranchDeletions {
t.Errorf("branch_deletions = %#v, want %d", metadata["branch_deletions"], status.BranchDeletions)
}
if metadata["remote_head_commit"] != status.RemoteHeadCommit {
t.Errorf("remote_head_commit = %#v, want %q", metadata["remote_head_commit"], status.RemoteHeadCommit)
}
for _, key := range []string{"modified", "added", "deleted", "untracked", "renamed"} {
if metadata[key] == nil {
t.Errorf("metadata[%q] is nil", key)
}
}
}
@@ -3,6 +3,7 @@ package repository
import (
"context"
"testing"
"time"
"github.com/kandev/kandev/internal/task/models"
"github.com/kandev/kandev/internal/task/repository/sqlite"
@@ -262,6 +263,448 @@ func TestGetLatestGitSnapshot_PrefersAgentCompletedOverNewerLiveMonitor(t *testi
}
}
// TestGetLatestGitSnapshot_PrefersArchiveOverAgentCompleted guards the
// terminal-replay invariant: while a task is still archived, its archive
// snapshot (final cumulative diff captured at archive time) must be selected
// over any agent_completed row. Before the ordering fix, GetLatestGitSnapshot
// preferred triggered_by='agent_completed' regardless of snapshot_type, so
// archived tasks that had completed a turn were replayed from a stale
// live-status row and the archive snapshot (with the cumulative-diff files
// carrying `path`) was never surfaced. See round-2 review blocker on
// d18c1a312.
func TestGetLatestGitSnapshot_PrefersArchiveOverAgentCompleted(t *testing.T) {
ctx := context.Background()
repo, cleanup := createTestSQLiteRepo(t)
defer cleanup()
const taskID = "task-arch"
const sessionID = "session-arch"
seedTaskAndSession(t, ctx, repo, taskID, sessionID)
// The task must be archived for the archive snapshot to be authoritative;
// an unarchived task's archive row is stale (see
// TestGetLatestGitSnapshot_UnarchivedResumePrefersCompleted).
if err := repo.ArchiveTask(ctx, taskID); err != nil {
t.Fatalf("archive task: %v", err)
}
// An agent_completed row exists from the last completed turn (older).
completed := &models.GitSnapshot{
SessionID: sessionID,
Branch: "feature",
HeadCommit: "completed-head",
TriggeredBy: "agent_completed",
Metadata: map[string]interface{}{"branch_additions": float64(5), "branch_deletions": float64(2)},
}
if err := repo.CreateGitSnapshot(ctx, completed); err != nil {
t.Fatalf("create agent_completed: %v", err)
}
// The archive snapshot carries the final cumulative diff (with path fields).
archive := &models.GitSnapshot{
SessionID: sessionID,
SnapshotType: models.SnapshotTypeArchive,
Branch: "feature",
HeadCommit: "archive-head",
BaseCommit: "base000",
Files: map[string]interface{}{
"file.txt": map[string]interface{}{
"path": "file.txt",
"status": "modified",
"additions": float64(3),
"deletions": float64(1),
},
},
}
if err := repo.CreateGitSnapshot(ctx, archive); err != nil {
t.Fatalf("create archive: %v", err)
}
t.Run("single query", func(t *testing.T) {
got, err := repo.GetLatestGitSnapshot(ctx, sessionID)
if err != nil {
t.Fatalf("GetLatestGitSnapshot: %v", err)
}
if got.HeadCommit != "archive-head" {
t.Errorf("expected archive-head, got %q — archive snapshot shadowed by agent_completed row", got.HeadCommit)
}
if got.SnapshotType != models.SnapshotTypeArchive {
t.Errorf("expected snapshot_type=archive, got %q", got.SnapshotType)
}
})
t.Run("batch query", func(t *testing.T) {
got, err := repo.GetLatestGitSnapshotsBySessionIDs(ctx, []string{sessionID})
if err != nil {
t.Fatalf("GetLatestGitSnapshotsBySessionIDs: %v", err)
}
if snap := got[sessionID]; snap == nil {
t.Fatal("expected a snapshot for session, got none")
} else if snap.HeadCommit != "archive-head" {
t.Errorf("expected archive-head, got %q — batch query shadowed by agent_completed row", snap.HeadCommit)
}
})
}
// TestGetLatestGitSnapshot_UnarchivedBeforeResumeKeepsArchive guards the
// unarchive-before-resume window: httpUnarchiveTask / UnarchiveTaskTree only
// clear archived_at and restore branches/workspaces — no snapshot is written
// until the session is resumed and the agent completes another turn. During
// that window the terminal archive row must remain authoritative; demoting it
// below an older agent_completed row would make a reload or task-list fallback
// show pre-archive files and diff counts. The archive row loses rank-0 only
// once a non-archive snapshot newer than it exists. Codex review P1 on PR
// #2851.
func TestGetLatestGitSnapshot_UnarchivedBeforeResumeKeepsArchive(t *testing.T) {
ctx := context.Background()
repo, cleanup := createTestSQLiteRepo(t)
defer cleanup()
const taskID = "task-unarchive-pending"
const sessionID = "session-unarchive-pending"
seedTaskAndSession(t, ctx, repo, taskID, sessionID)
// Older agent_completed row from a turn before the archive.
completed := &models.GitSnapshot{
SessionID: sessionID,
Branch: "feature",
HeadCommit: "completed-before-archive",
TriggeredBy: "agent_completed",
}
if err := repo.CreateGitSnapshot(ctx, completed); err != nil {
t.Fatalf("create agent_completed: %v", err)
}
// Task is archived; the terminal archive snapshot is captured.
if err := repo.ArchiveTask(ctx, taskID); err != nil {
t.Fatalf("archive task: %v", err)
}
archive := &models.GitSnapshot{
SessionID: sessionID,
SnapshotType: models.SnapshotTypeArchive,
Branch: "feature",
HeadCommit: "archive-head",
BaseCommit: "base000",
Files: map[string]interface{}{
"file.txt": map[string]interface{}{
"path": "file.txt",
"status": "modified",
"additions": float64(3),
"deletions": float64(1),
},
},
}
if err := repo.CreateGitSnapshot(ctx, archive); err != nil {
t.Fatalf("create archive: %v", err)
}
// Task is unarchived but NOT yet resumed: no newer snapshot exists.
if _, err := repo.UnarchiveTask(ctx, taskID); err != nil {
t.Fatalf("unarchive task: %v", err)
}
// The archive row must still win — it is the newest snapshot and the
// only terminal state; the older agent_completed row is pre-archive.
t.Run("single query", func(t *testing.T) {
got, err := repo.GetLatestGitSnapshot(ctx, sessionID)
if err != nil {
t.Fatalf("GetLatestGitSnapshot: %v", err)
}
if got.HeadCommit != "archive-head" {
t.Errorf("expected archive-head, got %q — pre-archive agent_completed row outranks terminal archive before resume", got.HeadCommit)
}
})
t.Run("batch query", func(t *testing.T) {
got, err := repo.GetLatestGitSnapshotsBySessionIDs(ctx, []string{sessionID})
if err != nil {
t.Fatalf("GetLatestGitSnapshotsBySessionIDs: %v", err)
}
if snap := got[sessionID]; snap == nil {
t.Fatal("expected a snapshot for session, got none")
} else if snap.HeadCommit != "archive-head" {
t.Errorf("expected archive-head in batch, got %q", snap.HeadCommit)
}
})
}
// TestGetLatestGitSnapshot_UnarchivedResumePrefersCompleted guards the
// lifecycle-conditional archive ranking: an archive snapshot is authoritative
// only while its owning task is still archived. ArchiveTask captures one, but
// UnarchiveTask does not invalidate it, and an archive-cancelled session
// resumes on the SAME session ID (task_operations.go: resume-reason path for
// archive-cancelled sessions). Once the task is unarchived and the agent runs
// again, newer agent_completed / live_monitor rows describe the current
// execution and must win over the stale archive row — otherwise the DB-fallback
// status and the sidebar badge (loadTaskGitObservations) show the pre-resume
// state forever. Round-3 review blocker on 62bd42f3a.
func TestGetLatestGitSnapshot_UnarchivedResumePrefersCompleted(t *testing.T) {
ctx := context.Background()
t.Run("completed row after unarchive wins over older archive row", func(t *testing.T) {
repo, cleanup := createTestSQLiteRepo(t)
defer cleanup()
const taskID = "task-resume"
const sessionID = "session-resume"
seedTaskAndSession(t, ctx, repo, taskID, sessionID)
// Task is archived; the archive snapshot is captured.
if err := repo.ArchiveTask(ctx, taskID); err != nil {
t.Fatalf("archive task: %v", err)
}
archive := &models.GitSnapshot{
SessionID: sessionID,
SnapshotType: models.SnapshotTypeArchive,
Branch: "feature",
HeadCommit: "archive-head",
BaseCommit: "base000",
}
if err := repo.CreateGitSnapshot(ctx, archive); err != nil {
t.Fatalf("create archive: %v", err)
}
// Task is unarchived and resumes; the agent completes a turn and
// writes a newer agent_completed row.
if _, err := repo.UnarchiveTask(ctx, taskID); err != nil {
t.Fatalf("unarchive task: %v", err)
}
completed := &models.GitSnapshot{
SessionID: sessionID,
Branch: "feature",
HeadCommit: "completed-after-resume",
TriggeredBy: "agent_completed",
Metadata: map[string]interface{}{"branch_additions": float64(7), "branch_deletions": float64(3)},
}
if err := repo.CreateGitSnapshot(ctx, completed); err != nil {
t.Fatalf("create agent_completed: %v", err)
}
// The resumed agent_completed row must win — the archive row is stale.
t.Run("single query", func(t *testing.T) {
got, err := repo.GetLatestGitSnapshot(ctx, sessionID)
if err != nil {
t.Fatalf("GetLatestGitSnapshot: %v", err)
}
if got.HeadCommit != "completed-after-resume" {
t.Errorf("expected completed-after-resume, got %q — stale archive row outranks resumed execution", got.HeadCommit)
}
})
t.Run("batch query", func(t *testing.T) {
got, err := repo.GetLatestGitSnapshotsBySessionIDs(ctx, []string{sessionID})
if err != nil {
t.Fatalf("GetLatestGitSnapshotsBySessionIDs: %v", err)
}
if snap := got[sessionID]; snap == nil {
t.Fatal("expected a snapshot for session, got none")
} else if snap.HeadCommit != "completed-after-resume" {
t.Errorf("expected completed-after-resume, got %q — batch query served stale archive row", snap.HeadCommit)
}
})
})
t.Run("live_monitor row after unarchive wins over older archive row", func(t *testing.T) {
repo, cleanup := createTestSQLiteRepo(t)
defer cleanup()
const taskID = "task-resume-live"
const sessionID = "session-resume-live"
seedTaskAndSession(t, ctx, repo, taskID, sessionID)
if err := repo.ArchiveTask(ctx, taskID); err != nil {
t.Fatalf("archive task: %v", err)
}
archive := &models.GitSnapshot{
SessionID: sessionID,
SnapshotType: models.SnapshotTypeArchive,
Branch: "feature",
HeadCommit: "archive-head",
}
if err := repo.CreateGitSnapshot(ctx, archive); err != nil {
t.Fatalf("create archive: %v", err)
}
if _, err := repo.UnarchiveTask(ctx, taskID); err != nil {
t.Fatalf("unarchive task: %v", err)
}
live := &models.GitSnapshot{
SessionID: sessionID,
Branch: "feature",
HeadCommit: "live-after-resume",
}
if err := repo.UpsertLatestLiveGitSnapshot(ctx, live); err != nil {
t.Fatalf("upsert live: %v", err)
}
got, err := repo.GetLatestGitSnapshot(ctx, sessionID)
if err != nil {
t.Fatalf("GetLatestGitSnapshot: %v", err)
}
if got.HeadCommit != "live-after-resume" {
t.Errorf("expected live-after-resume, got %q — stale archive row outranks resumed live status", got.HeadCommit)
}
// The batch selector has a separate ROW_NUMBER() expression; it must
// agree for the rank-2 live-monitor vs rank-3 stale-archive case too.
gotBatch, err := repo.GetLatestGitSnapshotsBySessionIDs(ctx, []string{sessionID})
if err != nil {
t.Fatalf("GetLatestGitSnapshotsBySessionIDs: %v", err)
}
if snap := gotBatch[sessionID]; snap == nil {
t.Fatal("expected a snapshot for session, got none")
} else if snap.HeadCommit != "live-after-resume" {
t.Errorf("expected live-after-resume in batch query, got %q — batch served stale archive row", snap.HeadCommit)
}
})
t.Run("archive still wins while the task remains archived", func(t *testing.T) {
repo, cleanup := createTestSQLiteRepo(t)
defer cleanup()
const taskID = "task-still-archived"
const sessionID = "session-still-archived"
seedTaskAndSession(t, ctx, repo, taskID, sessionID)
if err := repo.ArchiveTask(ctx, taskID); err != nil {
t.Fatalf("archive task: %v", err)
}
archive := &models.GitSnapshot{
SessionID: sessionID,
SnapshotType: models.SnapshotTypeArchive,
Branch: "feature",
HeadCommit: "archive-head",
}
if err := repo.CreateGitSnapshot(ctx, archive); err != nil {
t.Fatalf("create archive: %v", err)
}
// A turn completed before the archive; the archive must still win.
completed := &models.GitSnapshot{
SessionID: sessionID,
Branch: "feature",
HeadCommit: "completed-before-archive",
TriggeredBy: "agent_completed",
}
if err := repo.CreateGitSnapshot(ctx, completed); err != nil {
t.Fatalf("create agent_completed: %v", err)
}
got, err := repo.GetLatestGitSnapshot(ctx, sessionID)
if err != nil {
t.Fatalf("GetLatestGitSnapshot: %v", err)
}
if got.HeadCommit != "archive-head" {
t.Errorf("expected archive-head, got %q — archive lost while task is still archived", got.HeadCommit)
}
})
t.Run("current live row beats pre-archive completion row", func(t *testing.T) {
repo, cleanup := createTestSQLiteRepo(t)
defer cleanup()
const taskID = "task-resume-generation"
const sessionID = "session-resume-generation"
seedTaskAndSession(t, ctx, repo, taskID, sessionID)
base := time.Date(2026, 8, 20, 12, 0, 0, 0, time.UTC)
if err := repo.CreateGitSnapshot(ctx, &models.GitSnapshot{
SessionID: sessionID,
HeadCommit: "pre-archive-completed",
TriggeredBy: "agent_completed",
CreatedAt: base,
}); err != nil {
t.Fatalf("create pre-archive completion: %v", err)
}
if err := repo.ArchiveTask(ctx, taskID); err != nil {
t.Fatalf("archive task: %v", err)
}
if err := repo.CreateGitSnapshot(ctx, &models.GitSnapshot{
SessionID: sessionID,
SnapshotType: models.SnapshotTypeArchive,
HeadCommit: "archive-head",
CreatedAt: base.Add(time.Minute),
}); err != nil {
t.Fatalf("create archive: %v", err)
}
if _, err := repo.UnarchiveTask(ctx, taskID); err != nil {
t.Fatalf("unarchive task: %v", err)
}
if err := repo.UpsertLatestLiveGitSnapshot(ctx, &models.GitSnapshot{
SessionID: sessionID,
HeadCommit: "live-after-resume",
CreatedAt: base.Add(2 * time.Minute),
}); err != nil {
t.Fatalf("create resumed live snapshot: %v", err)
}
got, err := repo.GetLatestGitSnapshot(ctx, sessionID)
if err != nil {
t.Fatalf("GetLatestGitSnapshot: %v", err)
}
if got.HeadCommit != "live-after-resume" {
t.Errorf("head commit = %q, want current live row instead of pre-archive completion", got.HeadCommit)
}
gotBatch, err := repo.GetLatestGitSnapshotsBySessionIDs(ctx, []string{sessionID})
if err != nil {
t.Fatalf("GetLatestGitSnapshotsBySessionIDs: %v", err)
}
if gotBatch[sessionID] == nil || gotBatch[sessionID].HeadCommit != "live-after-resume" {
t.Errorf("batch head commit = %v, want current live row", gotBatch[sessionID])
}
})
}
// TestGetLatestGitSnapshot_TieBreakerIsDeterministic guards the equal-rank
// ordering: Postgres stores created_at at microsecond precision, so distinct
// writes (e.g. two archive rows from re-archive flows) can collapse to the
// same timestamp. Both selectors must break the tie identically via the ID
// column so the selected row is never database-plan dependent.
func TestGetLatestGitSnapshot_TieBreakerIsDeterministic(t *testing.T) {
ctx := context.Background()
repo, cleanup := createTestSQLiteRepo(t)
defer cleanup()
const taskID = "task-tie"
const sessionID = "session-tie"
seedTaskAndSession(t, ctx, repo, taskID, sessionID)
if err := repo.ArchiveTask(ctx, taskID); err != nil {
t.Fatalf("archive task: %v", err)
}
at := time.Date(2026, 3, 4, 5, 6, 7, 123456000, time.UTC)
for _, snap := range []*models.GitSnapshot{
{ID: "tie-aaa", SessionID: sessionID, SnapshotType: models.SnapshotTypeArchive,
Branch: "feature", HeadCommit: "older-id", CreatedAt: at},
{ID: "tie-bbb", SessionID: sessionID, SnapshotType: models.SnapshotTypeArchive,
Branch: "feature", HeadCommit: "newer-id", CreatedAt: at},
} {
if err := repo.CreateGitSnapshot(ctx, snap); err != nil {
t.Fatalf("create snapshot %s: %v", snap.ID, err)
}
}
// Both rows tie on created_at and rank; the id DESC tie-breaker picks
// tie-bbb deterministically in both selectors.
t.Run("single query", func(t *testing.T) {
got, err := repo.GetLatestGitSnapshot(ctx, sessionID)
if err != nil {
t.Fatalf("GetLatestGitSnapshot: %v", err)
}
if got.ID != "tie-bbb" {
t.Errorf("expected tie-bbb (id DESC tie-break), got %q", got.ID)
}
})
t.Run("batch query", func(t *testing.T) {
got, err := repo.GetLatestGitSnapshotsBySessionIDs(ctx, []string{sessionID})
if err != nil {
t.Fatalf("GetLatestGitSnapshotsBySessionIDs: %v", err)
}
if snap := got[sessionID]; snap == nil || snap.ID != "tie-bbb" {
t.Errorf("expected tie-bbb in batch, got %+v", got[sessionID])
}
})
}
func TestDeleteLiveMonitorSnapshots(t *testing.T) {
ctx := context.Background()
repo, cleanup := createTestSQLiteRepo(t)
@@ -17,6 +17,52 @@ import (
// UpsertLatestLiveGitSnapshot so we don't disturb archive/completion snapshots.
const TriggeredByLiveMonitor = "live_monitor"
// snapshotRankExpr is the ORDER BY ranking shared by GetLatestGitSnapshot and
// GetLatestGitSnapshotsBySessionIDs. Both selectors must pick the same row for
// a session, so the expression lives in one place:
//
// - rank 0: an archive snapshot that is still authoritative — either its
// owning task is still archived (terminal cumulative diff), or the task
// was unarchived but not yet resumed and no non-archive snapshot newer
// than this archive row exists yet (a reload in that window must not
// regress to pre-archive state).
// - rank 1: agent_completed snapshots from the current execution generation.
// - rank 2: live_monitor snapshots (periodic polls; may race completion).
// - rank 3: pre-archive completions and archive snapshots that are stale
// (task unarchived AND a newer non-archive snapshot exists).
//
// Ties within a rank break by created_at DESC, then id DESC so the selection
// is deterministic even when Postgres stores multiple writes at the same
// microsecond timestamp.
const snapshotRankExpr = `
CASE
WHEN snapshot_type = 'archive' AND (
EXISTS (
SELECT 1 FROM task_sessions ts
JOIN tasks t ON t.id = ts.task_id
WHERE ts.id = task_session_git_snapshots.session_id
AND t.archived_at IS NOT NULL
)
OR NOT EXISTS (
SELECT 1 FROM task_session_git_snapshots newer
WHERE newer.session_id = task_session_git_snapshots.session_id
AND newer.snapshot_type <> 'archive'
AND newer.created_at > task_session_git_snapshots.created_at
)
) THEN 0
WHEN triggered_by = 'agent_completed' AND EXISTS (
SELECT 1 FROM task_session_git_snapshots archive
WHERE archive.session_id = task_session_git_snapshots.session_id
AND archive.snapshot_type = 'archive'
AND archive.created_at > task_session_git_snapshots.created_at
) THEN 3
WHEN triggered_by = 'agent_completed' THEN 1
WHEN snapshot_type = 'archive' THEN 3
ELSE 2
END,
created_at DESC,
id DESC`
// UpsertLatestLiveGitSnapshot keeps at most one cached "live monitor" snapshot
// per session by deleting any previous live row and inserting the new one in a
// single transaction. This is the cache that backs the sidebar diff badge for
@@ -66,6 +112,10 @@ func (r *Repository) UpsertLatestLiveGitSnapshot(ctx context.Context, snapshot *
return tx.Commit()
}
// serializeSnapshotJSON marshals a snapshot's Files and Metadata maps into
// their JSON-text column forms. Nil maps serialize to "{}" (the sentinel
// that scans back to a nil map), preserving the row shape for both SQLite
// and Postgres.
func serializeSnapshotJSON(snapshot *models.GitSnapshot) (string, string, error) {
filesJSON := "{}"
if snapshot.Files != nil {
@@ -162,10 +212,11 @@ func (r *Repository) getGitSnapshotByOrder(ctx context.Context, sessionID, order
}
// GetLatestGitSnapshot retrieves the best git snapshot for a session.
// Prefers agent_completed snapshots (captured at exact completion time) over
// live_monitor snapshots (periodic polls that may contain stale data if the
// poll raced with agent completion). Falls back to most-recent-by-time when
// no agent_completed snapshot exists.
// An archive snapshot (terminal cumulative diff captured at archive time) is
// authoritative while its owning task is still archived, and also while the
// task is unarchived but not yet resumed (no newer non-archive snapshot
// exists yet); once a newer agent_completed / live_monitor row describes the
// current execution, it wins over the stale archive row.
// Returns sql.ErrNoRows if no snapshot is found.
func (r *Repository) GetLatestGitSnapshot(ctx context.Context, sessionID string) (*models.GitSnapshot, error) {
query := `
@@ -173,9 +224,7 @@ func (r *Repository) GetLatestGitSnapshot(ctx context.Context, sessionID string)
ahead, behind, files, triggered_by, metadata, created_at
FROM task_session_git_snapshots
WHERE session_id = ?
ORDER BY
CASE WHEN triggered_by = 'agent_completed' THEN 0 ELSE 1 END,
created_at DESC
ORDER BY` + snapshotRankExpr + `
LIMIT 1
`
return scanGitSnapshot(r.ro.QueryRowContext(ctx, r.ro.Rebind(query), sessionID))
@@ -183,7 +232,7 @@ func (r *Repository) GetLatestGitSnapshot(ctx context.Context, sessionID string)
// GetLatestGitSnapshotsBySessionIDs loads one authoritative snapshot per
// session in one query per placeholder-sized chunk. It mirrors
// GetLatestGitSnapshot's agent_completed preference without introducing an
// GetLatestGitSnapshot's ranking (snapshotRankExpr) without introducing an
// N+1 read when task-list summaries repair historical rows.
func (r *Repository) GetLatestGitSnapshotsBySessionIDs(
ctx context.Context,
@@ -203,8 +252,7 @@ func (r *Repository) GetLatestGitSnapshotsBySessionIDs(
ahead, behind, files, triggered_by, metadata, created_at,
ROW_NUMBER() OVER (
PARTITION BY session_id
ORDER BY CASE WHEN triggered_by = 'agent_completed' THEN 0 ELSE 1 END,
created_at DESC
ORDER BY` + snapshotRankExpr + `
) AS row_number
FROM task_session_git_snapshots
WHERE session_id IN (` + placeholders + `)
@@ -239,6 +287,9 @@ type gitSnapshotScanner interface {
Scan(dest ...interface{}) error
}
// scanGitSnapshot scans one task_session_git_snapshots row (all 13 columns)
// into a GitSnapshot, decoding the JSON-text files/metadata columns. The "{}"
// sentinel scans back to nil maps so empty rows round-trip cleanly.
func scanGitSnapshot(scanner gitSnapshotScanner) (*models.GitSnapshot, error) {
snapshot := &models.GitSnapshot{}
var snapshotType string
@@ -34,6 +34,20 @@ func TestPostgresGitSnapshotLifecycle(t *testing.T) {
}
}
// Archive the task so the archive snapshot's conditional rank-0 branch
// (`snapshot_type='archive' AND task archived`) is the one being tested —
// with the task unarchived the row would rank as a stale archive instead.
// Set archived_at directly rather than calling ArchiveTask: that method
// also purges the messagequeue in the same transaction, and on Postgres a
// missing queue table aborts the tx so the later Commit() fails with
// "commit unexpectedly resulted in rollback". The snapshot ordering only
// needs the archived_at flag.
if _, err := repo.db.Exec(repo.db.Rebind(
`UPDATE tasks SET archived_at = ?, updated_at = ? WHERE id = ?`),
time.Now().UTC(), time.Now().UTC(), "task-git-pg"); err != nil {
t.Fatalf("archive task: %v", err)
}
base := time.Date(2026, 3, 4, 5, 6, 7, 123456000, time.UTC)
want := fullGitSnapshot("session-git-pg-a", base)
if err := repo.CreateGitSnapshot(ctx, want); err != nil {
@@ -45,8 +59,8 @@ func TestPostgresGitSnapshotLifecycle(t *testing.T) {
}
assertGitSnapshotEqual(t, got, want)
// A newer live_monitor row must lose to the older agent_completed row in
// both the single-row read and the window-function batch read.
// A newer live_monitor row must lose to the older archive row (task still
// archived) in both the single-row read and the window-function batch read.
if err := repo.CreateGitSnapshot(ctx, &models.GitSnapshot{
ID: "snap-pg-live", SessionID: "session-git-pg-a",
SnapshotType: models.SnapshotTypeStatusUpdate, Branch: "main",
@@ -67,7 +81,7 @@ func TestPostgresGitSnapshotLifecycle(t *testing.T) {
t.Fatalf("GetLatestGitSnapshot: %v", err)
}
if latest.ID != "snapshot-full" {
t.Errorf("GetLatestGitSnapshot = %q, want the agent_completed snapshot-full", latest.ID)
t.Errorf("GetLatestGitSnapshot = %q, want the archive snapshot-full", latest.ID)
}
batch, err := repo.GetLatestGitSnapshotsBySessionIDs(ctx,
[]string{"session-git-pg-a", "session-git-pg-b", "session-git-pg-missing"})
@@ -86,6 +100,36 @@ func TestPostgresGitSnapshotLifecycle(t *testing.T) {
}
assertJSONMapEqual(t, "batch[a].Files", batch["session-git-pg-a"].Files, want.Files)
// Unarchive + resume: a newer agent_completed row must now outrank the
// stale archive row in both selectors (round-3 lifecycle fix parity).
if _, err := repo.db.Exec(repo.db.Rebind(
`UPDATE tasks SET archived_at = NULL, updated_at = ? WHERE id = ?`),
time.Now().UTC(), "task-git-pg"); err != nil {
t.Fatalf("unarchive task: %v", err)
}
if err := repo.CreateGitSnapshot(ctx, &models.GitSnapshot{
ID: "snap-pg-resumed", SessionID: "session-git-pg-a",
SnapshotType: models.SnapshotTypeStatusUpdate, Branch: "main",
TriggeredBy: "agent_completed", HeadCommit: "resumed-head", CreatedAt: base.Add(3 * time.Hour),
}); err != nil {
t.Fatalf("CreateGitSnapshot(resumed): %v", err)
}
latestResumed, err := repo.GetLatestGitSnapshot(ctx, "session-git-pg-a")
if err != nil {
t.Fatalf("GetLatestGitSnapshot(resumed): %v", err)
}
if latestResumed.ID != "snap-pg-resumed" {
t.Errorf("after unarchive GetLatestGitSnapshot = %q, want the resumed agent_completed row", latestResumed.ID)
}
batchResumed, err := repo.GetLatestGitSnapshotsBySessionIDs(ctx, []string{"session-git-pg-a"})
if err != nil {
t.Fatalf("GetLatestGitSnapshotsBySessionIDs(resumed): %v", err)
}
if batchResumed["session-git-pg-a"].ID != "snap-pg-resumed" {
t.Errorf("after unarchive batch[a] = %q, want the resumed agent_completed row",
batchResumed["session-git-pg-a"].ID)
}
// The single-live-row upsert transaction.
if err := repo.UpsertLatestLiveGitSnapshot(ctx, &models.GitSnapshot{
ID: "snap-pg-live-2", SessionID: "session-git-pg-a", Branch: "main",
@@ -100,16 +144,16 @@ func TestPostgresGitSnapshotLifecycle(t *testing.T) {
t.Errorf("live_monitor rows = %d, want exactly 1", liveRows)
}
if got := countRows(t, repo,
`SELECT COUNT(1) FROM task_session_git_snapshots WHERE session_id = ?`, "session-git-pg-a"); got != 2 {
t.Errorf("total rows = %d, want 2 (archive snapshot + one live row)", got)
`SELECT COUNT(1) FROM task_session_git_snapshots WHERE session_id = ?`, "session-git-pg-a"); got != 3 {
t.Errorf("total rows = %d, want 3 (archive + live + resumed agent_completed)", got)
}
if err := repo.DeleteLiveMonitorSnapshots(ctx, "session-git-pg-a"); err != nil {
t.Fatalf("DeleteLiveMonitorSnapshots: %v", err)
}
if got := countRows(t, repo,
`SELECT COUNT(1) FROM task_session_git_snapshots WHERE session_id = ?`, "session-git-pg-a"); got != 1 {
t.Errorf("rows after live delete = %d, want 1", got)
`SELECT COUNT(1) FROM task_session_git_snapshots WHERE session_id = ?`, "session-git-pg-a"); got != 2 {
t.Errorf("rows after live delete = %d, want 2 (archive + resumed agent_completed)", got)
}
if _, err := repo.GetLatestGitSnapshot(ctx, "session-git-pg-missing"); !errors.Is(err, sql.ErrNoRows) {
@@ -64,7 +64,11 @@ func fullGitSnapshot(sessionID string, createdAt time.Time) *models.GitSnapshot
"additions": float64(12),
},
},
TriggeredBy: "agent_completed",
// Production archive rows (captureArchiveDiff) carry no trigger; the
// trigger field must stay empty so ordering tests exercise the
// conditional `archive AND task archived` rank, not a
// triggered_by=agent_completed rank.
TriggeredBy: "",
Metadata: map[string]interface{}{
"reason": "task archived",
"attempt": float64(2),
@@ -152,6 +156,15 @@ func TestCreateGitSnapshotRoundTripsEveryField(t *testing.T) {
t.Fatalf("CreateGitSnapshot: %v", err)
}
// Archive the task so GetLatestGitSnapshot selects the archive row via
// its rank-0 branch (snapshot_type='archive' AND task archived) rather
// than merely being the only row — the fixture's intent is the
// authoritative-archive read, and the row is only the sole row here by
// construction. (Claude review note on PR #2851.)
if err := repo.ArchiveTask(ctx, "task-snap-full"); err != nil {
t.Fatalf("ArchiveTask: %v", err)
}
got, err := repo.GetLatestGitSnapshot(ctx, "session-snap-full")
if err != nil {
t.Fatalf("GetLatestGitSnapshot: %v", err)
@@ -613,6 +613,9 @@ func changedFilesFromSnapshot(snapshot *models.GitSnapshot) int {
if snapshot == nil {
return 0
}
if _, ok := snapshot.Metadata["changed_files"]; ok {
return nonNegative(snapshot.Metadata, "changed_files")
}
count := 0
for _, key := range []string{"modified", "added", "deleted", "untracked", "renamed"} {
count += collectionLength(snapshot.Metadata[key])
@@ -105,6 +105,16 @@ describe("ChangesTree", () => {
expect(isSelected).toHaveBeenCalledWith(FOO_TS);
});
it("skips a file entry without a path instead of crashing the route", () => {
// Regression: archived-task DB snapshots can replay cumulative-diff
// entries lacking a `path` field; the tree used to crash with
// "Cannot read properties of undefined (reading 'split')".
const pathless = { ...file(FOO_TS), path: undefined as unknown as string };
render(<ChangesTree {...baseProps} variant="unstaged" files={[pathless, file(BAR_TS)]} />);
expect(screen.getAllByTestId("file-row")).toHaveLength(1);
expect(screen.getByTestId("file-row").textContent).toBe(BAR_TS);
});
it("indents root files beneath a repository header", () => {
render(
<RepoTreeGroup
@@ -26,6 +26,11 @@ type TreeNode = {
function buildChangesTree(files: ChangedFile[]): TreeNode[] {
const root: TreeNode = { name: "", path: "", isDir: true, children: [] };
for (const file of files) {
// Defensive: a malformed entry (e.g. a legacy DB-snapshot replay without
// a path field) must not crash the whole route — skip it. Callers already
// backfill `path` from the map key, so this only fires on data that is
// broken at the source.
if (!file.path) continue;
const parts = file.path.split("/");
let current = root;
for (let i = 0; i < parts.length; i++) {
@@ -132,3 +132,37 @@ describe("buildReviewGitStatusFiles", () => {
expect(result.files?.["vendor/lib\u0000src/lib.ts"]?.repository_name).toBe("vendor/lib");
});
});
describe("legacy composite file replay", () => {
it("repairs pathless composite files before the Review dialog reads them", () => {
const result = buildReviewGitStatusFiles(
status({
"frontend\u0000src/app.ts": { status: "modified", staged: false } as FileInfo,
}),
[],
2,
);
expect(result.files?.["frontend\u0000src/app.ts"]).toMatchObject({
path: "src/app.ts",
repository_name: "frontend",
});
});
it("does not duplicate a composite key when a named status replays it", () => {
const result = buildReviewGitStatusFiles(
undefined,
[
{
repository_name: "frontend",
status: status({
"frontend\u0000src/app.ts": { status: "modified", staged: false } as FileInfo,
}),
},
],
2,
);
expect(Object.keys(result.files ?? {})).toEqual(["frontend\u0000src/app.ts"]);
});
});
@@ -23,6 +23,7 @@ import { generateUUID } from "@/lib/utils";
import { useToast } from "@/components/toast-provider";
import type { DiffComment } from "@/lib/diff/types";
import type { FileInfo, GitStatusEntry } from "@/lib/state/slices/session-runtime/types";
import { normalizeGitStatusFiles } from "@/lib/state/slices/session-runtime/git-status-normalizer";
import { t } from "@/lib/i18n";
type ReviewGitStatusFiles = {
@@ -36,7 +37,8 @@ function buildMultiRepoReviewFiles(
const files: Record<string, FileInfo> = {};
for (const { repository_name, status } of statusByRepo) {
if (!status?.files) continue;
for (const [path, file] of Object.entries(status.files)) {
for (const [keyPath, file] of Object.entries(normalizeGitStatusFiles(status.files) ?? {})) {
const path = file.path || keyPath;
const key = reviewFileKey({ path, repository_name });
files[key] = {
...file,
@@ -63,11 +65,12 @@ export function buildReviewGitStatusFiles(
);
if (!isMultiRepo) {
const sourceStatus = reviewGitStatus?.files ? reviewGitStatus : named[0]?.status;
const files = sourceStatus?.files
const sourceFiles = normalizeGitStatusFiles(sourceStatus?.files);
const files = sourceFiles
? Object.fromEntries(
Object.entries(sourceStatus.files).map(([path, file]) => [
Object.entries(sourceFiles).map(([path, file]) => [
path,
{ ...file, is_submodule: file.is_submodule ?? sourceStatus.is_submodule },
{ ...file, is_submodule: file.is_submodule ?? sourceStatus?.is_submodule },
]),
)
: null;
@@ -78,7 +81,7 @@ export function buildReviewGitStatusFiles(
}
if (named.length === 0) {
return {
files: reviewGitStatus?.files ?? null,
files: normalizeGitStatusFiles(reviewGitStatus?.files) ?? null,
isMultiRepo: true,
};
}
@@ -1,5 +1,75 @@
import { describe, it, expect } from "vitest";
import { groupPathsByRepoName, repositoryScopesForMutation } from "./use-session-git";
import {
aggregateFilesAcrossRepos,
groupPathsByRepoName,
repositoryScopesForMutation,
} from "./use-session-git";
describe("aggregateFilesAcrossRepos", () => {
it("stamps path from the map key when a status entry lacks its own path", () => {
// Archived-task DB snapshots can replay cumulative-diff entries whose
// older shape only sat under the key (no `path` field). The changes tree
// splits on file.path, so the fallback must not surface undefined.
const out = aggregateFilesAcrossRepos([], {
files: {
"apps/a.go": { path: "apps/a.go", status: "modified", staged: false },
"readme.md": { status: "modified", staged: false } as never,
},
} as never);
expect(out.map((f) => f.path).sort()).toEqual(["apps/a.go", "readme.md"]);
});
it("stamps path per repo in multi-repo statuses lacking it", () => {
const out = aggregateFilesAcrossRepos(
[
{
repository_name: "frontend",
status: {
files: { "app.tsx": { status: "modified", staged: false } as never },
},
} as never,
],
undefined,
);
expect(out).toEqual([
{ status: "modified", staged: false, path: "app.tsx", repository_name: "frontend" },
]);
});
it("preserves existing paths and falls back to empty list without status", () => {
expect(aggregateFilesAcrossRepos([], undefined)).toEqual([]);
});
it("restores path and repository from composite keys", () => {
// Multi-repo cumulative payloads key files as `<repo>\x00<path>`. If such
// an entry ever lacks the stamped `path`, the backfill must split the key
// (like splitReviewFileKey) instead of stamping the NUL composite into
// FileInfo.path — the changes tree splits on '/', so a composite path
// would surface as one bogus filename and break per-repo grouping.
const out = aggregateFilesAcrossRepos([], {
files: {
"repoA\u0000lib/util.ts": { status: "added", staged: false } as never,
},
} as never);
expect(out).toEqual([
{ status: "added", staged: false, path: "lib/util.ts", repository_name: "repoA" },
]);
});
it("keeps same relative paths distinct across repositories", () => {
const out = aggregateFilesAcrossRepos([], {
files: {
"repoA\u0000README.md": { status: "modified", staged: false } as never,
"repoB\u0000README.md": { status: "modified", staged: false } as never,
},
} as never);
expect(out).toEqual([
{ status: "modified", staged: false, path: "README.md", repository_name: "repoA" },
{ status: "modified", staged: false, path: "README.md", repository_name: "repoB" },
]);
});
});
describe("groupPathsByRepoName", () => {
it("splits paths by repository_name, preserving insertion order per bucket", () => {
@@ -23,6 +23,7 @@ import {
import { useMultiRepoSummary } from "./use-session-git-summary";
import { deriveSessionGitValues } from "./use-session-git-derived";
import { useScopedStageOperations } from "./use-scoped-stage-operations";
import { normalizeGitStatusFiles } from "@/lib/state/slices/session-runtime/git-status-normalizer";
/**
* Per-repo result emitted by frontend-side fan-outs (commit, push, pull,
@@ -209,8 +210,16 @@ export function repositoryScopesForMutation(
* Builds the SessionGit's flat file list. For multi-repo workspaces it
* stamps each FileInfo with its repository_name so consumers can group;
* for single-repo it returns the legacy single-status files unchanged.
*
* Each entry is stamped with `path` from the map key when the payload entry
* omits it. Git-status payloads always carry `path`, but the DB-snapshot
* fallback can replay archived cumulative-diff entries whose older shape only
* sat under the key (no `path` field) — the changes tree splits on
* `file.path` and crashes on `undefined`, so a missing path must never reach
* consumers. Multi-repo cumulative keys are `<repo>\x00<path>` composites;
* the normalizer restores both the path and repository scope from that key.
*/
function aggregateFilesAcrossRepos(
export function aggregateFilesAcrossRepos(
statusByRepo: ReturnType<typeof useSessionGitStatusByRepo>,
gitStatus: ReturnType<typeof useSessionGitStatus>,
): FileInfo[] {
@@ -218,13 +227,13 @@ function aggregateFilesAcrossRepos(
const out: FileInfo[] = [];
for (const { repository_name, status } of statusByRepo) {
if (!status?.files) continue;
for (const f of Object.values(status.files)) {
out.push(repository_name ? { ...f, repository_name } : f);
for (const file of Object.values(normalizeGitStatusFiles(status.files) ?? {})) {
out.push(repository_name ? { ...file, repository_name } : file);
}
}
return out;
}
return gitStatus?.files ? Object.values(gitStatus.files) : [];
return Object.values(normalizeGitStatusFiles(gitStatus?.files) ?? {});
}
type StageDispatchArgs = {
@@ -0,0 +1,56 @@
import { describe, expect, it } from "vitest";
import { normalizeGitStatusEntry, normalizeGitStatusFiles } from "./git-status-normalizer";
import type { FileInfo, GitStatusEntry } from "./types";
function file(overrides: Partial<FileInfo> = {}): FileInfo {
return { path: "src/app.ts", status: "modified", staged: false, ...overrides };
}
describe("normalizeGitStatusFiles", () => {
it("fills a missing path from a legacy map key", () => {
const files = { "src/app.ts": { status: "modified", staged: false } as FileInfo };
expect(normalizeGitStatusFiles(files)).toEqual({
"src/app.ts": file(),
});
});
it("fills both path and repository from a composite map key", () => {
const files = {
"frontend\u0000src/app.ts": { status: "modified", staged: false } as FileInfo,
"backend\u0000src/app.ts": { status: "added", staged: false } as FileInfo,
};
expect(normalizeGitStatusFiles(files)).toEqual({
"frontend\u0000src/app.ts": {
path: "src/app.ts",
repository_name: "frontend",
status: "modified",
staged: false,
},
"backend\u0000src/app.ts": {
path: "src/app.ts",
repository_name: "backend",
status: "added",
staged: false,
},
});
});
it("preserves explicit values, including an explicit root repository", () => {
const files = {
"frontend\u0000src/app.ts": file({ path: "renamed.ts", repository_name: "other" }),
"\u0000README.md": file({ path: "README.md", repository_name: "" }),
};
expect(normalizeGitStatusFiles(files)).toBe(files);
});
});
describe("normalizeGitStatusEntry", () => {
it("returns the same entry when all file records already use the current shape", () => {
const entry = { files: { "src/app.ts": file() } } as unknown as GitStatusEntry;
expect(normalizeGitStatusEntry(entry)).toBe(entry);
});
});
@@ -0,0 +1,52 @@
import type { FileInfo, GitStatusEntry } from "./types";
const FILE_KEY_SEPARATOR = "\u0000";
type ParsedFileKey = {
path: string;
repositoryName?: string;
};
function parseFileKey(key: string): ParsedFileKey {
const separator = key.indexOf(FILE_KEY_SEPARATOR);
if (separator < 0) return { path: key };
return {
repositoryName: key.slice(0, separator),
path: key.slice(separator + 1),
};
}
/**
* Repairs file records from older Git-status snapshots.
*
* New records carry `path` and `repository_name` on each value. Older
* cumulative-diff records stored those values only in the map key.
*/
export function normalizeGitStatusFiles(
files: Record<string, FileInfo> | undefined,
): Record<string, FileInfo> | undefined {
if (!files) return files;
let normalized: Record<string, FileInfo> | undefined;
for (const [key, file] of Object.entries(files)) {
const parsed = parseFileKey(key);
const path = typeof file.path === "string" && file.path.length > 0 ? file.path : parsed.path;
const repositoryName = file.repository_name ?? parsed.repositoryName;
if (path === file.path && repositoryName === file.repository_name) continue;
normalized ??= { ...files };
normalized[key] = {
...file,
path,
...(repositoryName === undefined ? {} : { repository_name: repositoryName }),
};
}
return normalized ?? files;
}
/** Normalize the file contract before a Git-status entry enters the store. */
export function normalizeGitStatusEntry(gitStatus: GitStatusEntry): GitStatusEntry {
const files = normalizeGitStatusFiles(gitStatus.files);
if (files === gitStatus.files) return gitStatus;
return { ...gitStatus, files: files ?? {} };
}
@@ -7,6 +7,7 @@ import type {
FileInfo,
} from "./types";
import { createDebugLogger, isDebug } from "@/lib/debug/log";
import { normalizeGitStatusEntry } from "./git-status-normalizer";
const debugGit = createDebugLogger("git-status:store");
@@ -546,7 +547,7 @@ export const createSessionRuntimeSlice: StateCreator<
setGitStatus: (sessionId, gitStatus) => {
let changed = false;
set((draft) => {
changed = applyGitStatus(draft, sessionId, gitStatus);
changed = applyGitStatus(draft, sessionId, normalizeGitStatusEntry(gitStatus));
});
return changed;
},
@@ -70,6 +70,26 @@ describe("setGitStatus change reporting (single deep compare)", () => {
expect(store.getState().setGitStatus(SESSION, status({ is_submodule: true }))).toBe(true);
});
it("normalizes legacy composite file keys before storing the status", () => {
store.getState().setGitStatus(
SESSION,
status({
files: {
"frontend\u0000src/app.ts": { status: "modified", staged: false } as never,
},
}),
);
expect(store.getState().gitStatus.byEnvironmentId[SESSION]?.files).toEqual({
"frontend\u0000src/app.ts": {
path: "src/app.ts",
repository_name: "frontend",
status: "modified",
staged: false,
},
});
});
it("returns false when the submodule marker is unchanged", () => {
store.getState().setGitStatus(SESSION, status({ is_submodule: true }));