diff --git a/app/cli/internal/trace/README.md b/app/cli/internal/trace/README.md index 90fd6aac8..7f65c055d 100644 --- a/app/cli/internal/trace/README.md +++ b/app/cli/internal/trace/README.md @@ -53,10 +53,14 @@ Which lines of a commit the AI wrote. | Partial commits | A commit that holds only part of the AI lines of a file credits the session with those lines. The other AI lines stay pending, and the next commit with them is credited to the session too (spec issue-3639). | Yes | Partial | Yes | Yes | | Generated line counts | The trace state keeps the lines that each agent edit added and removed, also the lines that never reach a commit. The evidence does not report them yet (spec issue-3639). | Yes | Partial | Yes | Yes | | Shell changes attribution | Changes that the agent makes with shell commands (for example `sed`) are attributed to the AI too, not only changes made with the file tools. | Partial | Partial | Partial | Partial | +| Net contribution | The added lines of a session are the lines that its commits added and that are still in the commit that the push sends. A line that a later commit replaced does not count, and each line counts one time. The removed lines are the sum of the removals of the session's commits (spec issue-3639). | Partial | Partial | Partial | Partial | +| Session updates | A later commit that replaces lines of a session sends the evidence of the session again at the next push, with the new numbers. A commit in no session that replaces lines of a session joins it: the commit list of the session holds the commit, and its replacing lines count as human (spec issue-3639). | Partial | Partial | Partial | Partial | Why not Yes: - **Line attribution, partial commits and generated line counts, Cursor:** Cursor has only the `afterFileEdit` hook, which gives the edits but no snapshot before the edit. The provider rebuilds the content before the edit from the edits. +- **Net contribution, all agents:** the push finds the last writer of each line with `git blame`. A later commit that only re-indents a line takes the line, because blame sees a change. For Cursor, the limits of line attribution apply too. +- **Session updates, all agents:** the update works only in the clone where the work happened, while the CLI keeps the trace state of the session (30 days), and only when the commits of the session are on the branch that the push sends. - **Shell changes attribution, all agents:** the hook before a shell command records only a hash of each file, not its content. The file at the `HEAD` commit from before the command stands in for the content before the command. Uncommitted lines that an earlier agent edit recorded are not recorded again, but uncommitted human lines of a file that the command changes count as AI. - **Shell changes attribution, Cursor:** the provider installs the `preToolUse` and `postToolUse` hooks for the `Shell` tool. These hooks are verified for the Cursor IDE only. It is not verified that the Cursor CLI sends them. diff --git a/app/cli/internal/trace/attribution/enrich.go b/app/cli/internal/trace/attribution/enrich.go index 0d5fed4ae..2628d7408 100644 --- a/app/cli/internal/trace/attribution/enrich.go +++ b/app/cli/internal/trace/attribution/enrich.go @@ -25,8 +25,14 @@ import ( // CommitLines is one commit of a session: the lines it changed and the // sessions that its record names. type CommitLines struct { + SHA string SessionIDs []string Files []tracegit.FileLines + // Joined is set on a commit in no session that replaced lines of the + // session and so joined it (R-010). It holds, for each file, the + // fingerprints of the lines that its replacing hunks added. Only these + // lines count for the session, and they are human. + Joined map[string][]string } // Enrich adds AI/human attribution to the code changes of a session. @@ -43,15 +49,31 @@ type CommitLines struct { // another session of the commit counts for neither side: it is that session's // work. All other lines are human. // +// tip holds, for each file, its lines at the pushed commit with the commit +// that last wrote each one. The added lines of a file in tip are the net +// contribution of the session (R-003): the lines that its commits added and +// that no later commit replaced, each one counted once. A file that tip holds +// with no lines is gone, and adds nothing. A file that tip does not hold falls +// back to the sum of the added lines of each commit. Removed lines are always +// the sum over the session's own commits. +// // A file with an edit recorded before line fingerprints existed keeps the // earlier rule: AI lines are the recorded ranges, capped at the file's diff. -func Enrich(sessionID string, ledgers map[string]*state.AILineAttribution, commits []CommitLines, changes *aicodingsession.CodeChanges) { +func Enrich(sessionID string, ledgers map[string]*state.AILineAttribution, commits []CommitLines, tip map[string][]tracegit.BlameLine, changes *aicodingsession.CodeChanges) { self := ledgers[sessionID] if self == nil { self = &state.AILineAttribution{SessionID: sessionID} } byPath := splitByPath(sessionID, self, ledgers, commits) + for path, net := range netAdded(sessionID, self, ledgers, commits, tip) { + s, ok := byPath[path] + if !ok { + s = &fileSplit{} + byPath[path] = s + } + s.aiAdded, s.humanAdded = net.aiAdded, net.humanAdded + } changes.AILinesAdded, changes.AILinesRemoved = 0, 0 changes.HumanLinesAdded, changes.HumanLinesRemoved = 0, 0 @@ -103,12 +125,12 @@ func splitByPath(sessionID string, self *state.AILineAttribution, ledgers map[st out := make(map[string]*fileSplit) for _, commit := range commits { - var others []*state.AILineAttribution - for _, id := range commit.SessionIDs { - if l := ledgers[id]; id != sessionID && l != nil { - others = append(others, l) - } + // A joined commit counts only through the lines that are still in + // the pushed commit (netAdded). + if commit.Joined != nil { + continue } + others := otherLedgers(sessionID, ledgers, commit.SessionIDs) for _, fl := range commit.Files { s, ok := out[fl.Path] @@ -143,6 +165,72 @@ func splitByPath(sessionID string, self *state.AILineAttribution, ledgers map[st return out } +// netAdded splits the lines of each file in tip that the session's commits +// wrote. A line from a commit outside the session is not the session's. A +// line from a joined commit counts as human when its replacing hunks added +// it, and not at all otherwise. +func netAdded(sessionID string, self *state.AILineAttribution, ledgers map[string]*state.AILineAttribution, commits []CommitLines, tip map[string][]tracegit.BlameLine) map[string]*fileSplit { + bySHA := make(map[string]CommitLines, len(commits)) + for _, c := range commits { + bySHA[c.SHA] = c + } + + written := linePool{pick: func(a *state.AILineAttribution) map[string]state.LineCounts { return a.Written }} + joined := make(map[[2]string]state.LineCounts) + + out := make(map[string]*fileSplit, len(tip)) + for path, lines := range tip { + s := &fileSplit{} + out[path] = s + + for _, l := range lines { + commit, ok := bySHA[l.SHA] + if !ok { + continue + } + fp := Fingerprint(l.Text) + + if commit.Joined != nil { + key := [2]string{commit.SHA, path} + pool, ok := joined[key] + if !ok { + pool = state.LineCounts{} + pool.Add(commit.Joined[path]...) + joined[key] = pool + } + if pool.Take(fp) { + s.humanAdded++ + } + + continue + } + + switch { + case written.take(self, path, fp): + s.aiAdded++ + case written.takeAny(otherLedgers(sessionID, ledgers, commit.SessionIDs), path, fp): + default: + s.humanAdded++ + } + } + } + + return out +} + +// otherLedgers returns the ledgers of the sessions in ids other than +// sessionID. +func otherLedgers(sessionID string, ledgers map[string]*state.AILineAttribution, ids []string) []*state.AILineAttribution { + var out []*state.AILineAttribution + for _, id := range ids { + if l := ledgers[id]; id != sessionID && l != nil { + out = append(out, l) + } + } + + return out +} + // linePool hands out the lines of the ledgers, one copy per match. Each // session's lines of a file are copied on first use and drawn down across // the commits, so a line matches at most as often as the ledger holds it. diff --git a/app/cli/internal/trace/attribution/enrich_test.go b/app/cli/internal/trace/attribution/enrich_test.go index 745a8fae3..e7d38db62 100644 --- a/app/cli/internal/trace/attribution/enrich_test.go +++ b/app/cli/internal/trace/attribution/enrich_test.go @@ -16,6 +16,7 @@ package attribution import ( + "strings" "testing" "github.com/chainloop-dev/chainloop/app/cli/internal/trace" @@ -67,7 +68,7 @@ func TestEnrich_Legacy(t *testing.T) { }, } - Enrich(testSession, nil, nil, changes) + Enrich(testSession, nil, nil, nil, changes) assert.Equal(t, 10, changes.HumanLinesAdded) assert.Equal(t, 4, changes.HumanLinesRemoved) @@ -95,7 +96,7 @@ func TestEnrich_Legacy(t *testing.T) { }, } - Enrich(testSession, ledgers, nil, changes) + Enrich(testSession, ledgers, nil, nil, changes) assert.Equal(t, trace.AttributionAI, changes.Files[0].Attribution) assert.Equal(t, trace.AttributionAI, changes.Files[1].Attribution) @@ -115,7 +116,7 @@ func TestEnrich_Legacy(t *testing.T) { }, } - Enrich(testSession, ledgers, nil, changes) + Enrich(testSession, ledgers, nil, nil, changes) assert.Equal(t, trace.AttributionAI, changes.Files[0].Attribution) assert.Equal(t, []string{testSession}, changes.Files[0].SessionIDs) @@ -141,7 +142,7 @@ func TestEnrich_Legacy(t *testing.T) { }, } - Enrich(testSession, ledgers, nil, changes) + Enrich(testSession, ledgers, nil, nil, changes) assert.Equal(t, trace.AttributionAI, changes.Files[0].Attribution) assert.Equal(t, []string{testSession}, changes.Files[0].SessionIDs) @@ -164,7 +165,7 @@ func TestEnrich_Legacy(t *testing.T) { }, } - Enrich(testSession, ledgers, nil, changes) + Enrich(testSession, ledgers, nil, nil, changes) assert.Equal(t, trace.AttributionAI, changes.Files[0].Attribution) assert.Equal(t, trace.AttributionHuman, changes.Files[1].Attribution) @@ -258,7 +259,7 @@ func TestEnrich_Lines(t *testing.T) { t.Run(tc.name, func(t *testing.T) { changes := &aicodingsession.CodeChanges{Files: []aicodingsession.FileChange{{Path: testFile}}} - Enrich(testSession, tc.ledgers, tc.commits, changes) + Enrich(testSession, tc.ledgers, tc.commits, nil, changes) f := changes.Files[0] assert.Equal(t, tc.want, split{f.AILinesAdded, f.AILinesRemoved, f.HumanLinesAdded, f.HumanLinesRemoved}, "file split") @@ -270,6 +271,103 @@ func TestEnrich_Lines(t *testing.T) { } } +func TestEnrich_Net(t *testing.T) { + type split struct{ aiAdded, aiRemoved, humanAdded, humanRemoved int } + + const c1, c2, c3 = "c1", "c2", "c3" + tipOf := func(lines ...string) []tracegit.BlameLine { + // Each entry is "sha:text". + out := make([]tracegit.BlameLine, 0, len(lines)) + for _, l := range lines { + sha, text, _ := strings.Cut(l, ":") + out = append(out, tracegit.BlameLine{SHA: sha, Text: text}) + } + + return out + } + five := []string{"a", "b", "c", "d", "e"} + ownCommit := func(sha string, added, removed []string) CommitLines { + return CommitLines{SHA: sha, SessionIDs: []string{testSession}, Files: []tracegit.FileLines{{Path: testFile, Added: added, Removed: removed}}} + } + + testCases := []struct { + name string + ledgers map[string]*state.AILineAttribution + commits []CommitLines + tip map[string][]tracegit.BlameLine + want split + }{ + { + // Case 3 of the spec: a later commit of the session replaces an + // AI line by hand. Each line counts once (R-003). + name: "a replaced line of the session counts once", + ledgers: map[string]*state.AILineAttribution{testSession: ledger(testSession, testFile, five, nil)}, + commits: []CommitLines{ + ownCommit(c1, five, nil), + ownCommit(c2, []string{"C"}, []string{"c"}), + }, + tip: map[string][]tracegit.BlameLine{testFile: tipOf(c1+":a", c1+":b", c2+":C", c1+":d", c1+":e")}, + want: split{aiAdded: 4, humanAdded: 1, humanRemoved: 1}, + }, + { + // Case 2 of the spec: a commit in no session replaces an AI line + // and joins the session (R-010). Its replacing lines are human, + // and its removals of the session's lines do not count again. + name: "a joined commit adds human lines of its replacing hunks", + ledgers: map[string]*state.AILineAttribution{testSession: ledger(testSession, testFile, five, nil)}, + commits: []CommitLines{ + ownCommit(c1, five, nil), + {SHA: c2, Files: []tracegit.FileLines{{Path: testFile, Added: []string{"C", "x"}, Removed: []string{"c"}}}, + Joined: map[string][]string{testFile: fps("C")}}, + }, + tip: map[string][]tracegit.BlameLine{testFile: tipOf(c1+":a", c1+":b", c2+":C", c1+":d", c1+":e", c2+":x")}, + want: split{aiAdded: 4, humanAdded: 1}, + }, + { + // Case 4 of the spec: a commit of another session replaces the + // line. The session keeps only what is still there. + name: "lines that a later commit outside the session replaced are gone", + ledgers: map[string]*state.AILineAttribution{testSession: ledger(testSession, testFile, five, nil)}, + commits: []CommitLines{ownCommit(c1, five, nil)}, + tip: map[string][]tracegit.BlameLine{testFile: tipOf(c1+":a", c1+":b", c3+":C", c1+":d", c1+":e")}, + want: split{aiAdded: 4}, + }, + { + name: "a line of another session of the commit counts for neither", + ledgers: map[string]*state.AILineAttribution{ + testSession: ledger(testSession, testFile, []string{"a"}, nil), + otherSession: ledger(otherSession, testFile, []string{"b"}, nil), + }, + commits: []CommitLines{{SHA: c1, SessionIDs: []string{testSession, otherSession}, + Files: []tracegit.FileLines{{Path: testFile, Added: []string{"a", "b", "h"}}}}}, + tip: map[string][]tracegit.BlameLine{testFile: tipOf(c1+":a", c1+":b", c1+":h")}, + want: split{aiAdded: 1, humanAdded: 1}, + }, + { + name: "a file that the pushed commit does not hold adds nothing", + ledgers: map[string]*state.AILineAttribution{testSession: ledger(testSession, testFile, five, []string{"z"})}, + commits: []CommitLines{ + ownCommit(c1, five, nil), + ownCommit(c2, nil, append([]string{"z"}, five...)), + }, + tip: map[string][]tracegit.BlameLine{testFile: nil}, + want: split{aiRemoved: 1, humanRemoved: 5}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + changes := &aicodingsession.CodeChanges{Files: []aicodingsession.FileChange{{Path: testFile}}} + + Enrich(testSession, tc.ledgers, tc.commits, tc.tip, changes) + + f := changes.Files[0] + assert.Equal(t, tc.want, split{f.AILinesAdded, f.AILinesRemoved, f.HumanLinesAdded, f.HumanLinesRemoved}) + assert.Equal(t, tc.want, split{changes.AILinesAdded, changes.AILinesRemoved, changes.HumanLinesAdded, changes.HumanLinesRemoved}) + }) + } +} + func TestSettleCommit(t *testing.T) { open := func(added, removed []string) *state.OpenLines { o := &state.OpenLines{Added: state.LineCounts{}, Removed: state.LineCounts{}} diff --git a/app/cli/internal/trace/attribution/generated_test.go b/app/cli/internal/trace/attribution/generated_test.go index fccd28800..1ac57be91 100644 --- a/app/cli/internal/trace/attribution/generated_test.go +++ b/app/cli/internal/trace/attribution/generated_test.go @@ -17,6 +17,7 @@ package attribution import ( "os" + "os/exec" "path/filepath" "testing" @@ -83,6 +84,10 @@ func TestFilterGenerated(t *testing.T) { t.Run("ent gitattributes rules drop generated ent code but keep schema and migrations", func(t *testing.T) { dir := t.TempDir() + // The exec client reads attributes with `git check-attr`, which needs + // a repository. + out, err := exec.Command("git", "init", "-q", dir).CombinedOutput() + require.NoError(t, err, string(out)) // Order matches the project .gitattributes: general rule first, then // more-specific overrides (last match wins, per gitattributes spec). writeGitAttributes(t, dir, @@ -91,7 +96,7 @@ func TestFilterGenerated(t *testing.T) { "backend/internal/data/ent/schema/* linguist-generated=false\n", ) - isGenerated := tracegit.NewGoGitClient().GeneratedMatcher(dir) + isGenerated := tracegit.NewGitClient().GeneratedMatcher(dir) changes := &aicodingsession.CodeChanges{ FilesCreated: 3, diff --git a/app/cli/internal/trace/git/git.go b/app/cli/internal/trace/git/git.go index 1b3a04c28..2f73c2cc3 100644 --- a/app/cli/internal/trace/git/git.go +++ b/app/cli/internal/trace/git/git.go @@ -16,11 +16,26 @@ package git import ( + "os/exec" "time" "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/materials/aicodingsession" ) +// lookPath finds a binary on the PATH. Tests replace it to pick a client. +var lookPath = exec.LookPath + +// NewGitClient returns the client that runs the git binary when one is on +// the PATH, and the go-git client otherwise. The git binary is many times +// faster on a long history, for example for blame. +func NewGitClient() Client { + if _, err := lookPath("git"); err == nil { + return NewExecClient() + } + + return NewGoGitClient() +} + // File statuses reported in aicodingsession.FileChange.Status. The control // plane buckets on these exact strings when it ingests the evidence, so they // are a wire contract, not free-form labels. @@ -37,6 +52,24 @@ type FileLines struct { Path string Added []string Removed []string + // Hunks holds the same lines, split into the hunks of the diff. + Hunks []Hunk +} + +// Hunk is one run of removed and added lines of a diff. +type Hunk struct { + // OldStart is the line of the parent's file where the hunk starts, + // counting from one: the first removed line, or the line that the added + // lines go before. Removed line i is line OldStart+i of the parent. + OldStart int + Removed []string + Added []string +} + +// BlameLine is one line of a file, with the commit that last wrote it. +type BlameLine struct { + SHA string + Text string } // Client abstracts git operations so the implementation can be swapped @@ -63,6 +96,10 @@ type Client interface { // FileAt returns the content of a repo-relative path at a commit, or nil // when the commit does not hold the path. FileAt(repoRoot, sha, path string) ([]byte, error) + // Blame returns each line of a repo-relative path at a revision, with the + // commit that last wrote it, or nil when the revision does not hold the + // path. + Blame(repoRoot, rev, path string) ([]BlameLine, error) // CommitHeadInfo returns the SHA and message of HEAD. CommitHeadInfo(repoRoot string) (sha, message string, err error) // HeadAuthorTime returns the author date of HEAD. diff --git a/app/cli/internal/trace/git/lines.go b/app/cli/internal/trace/git/lines.go index dd40e652a..bc0cdb6d3 100644 --- a/app/cli/internal/trace/git/lines.go +++ b/app/cli/internal/trace/git/lines.go @@ -20,8 +20,10 @@ import ( "fmt" "io" "os/exec" + "strconv" "strings" + gogit "github.com/go-git/go-git/v6" "github.com/go-git/go-git/v6/plumbing" fdiff "github.com/go-git/go-git/v6/plumbing/format/diff" "github.com/go-git/go-git/v6/plumbing/object" @@ -80,15 +82,39 @@ func (c *GoGitClient) CommitLines(repoRoot, sha string) ([]FileLines, error) { fl.Path = from.Path() } + // A hunk is a run of removed and added chunks between two equal ones. + // oldLine is the line of the parent's file that the next chunk starts + // at. + oldLine := 1 + var cur *Hunk + closeHunk := func() { + if cur != nil { + fl.Hunks = append(fl.Hunks, *cur) + cur = nil + } + } for _, chunk := range fp.Chunks() { + lines := splitLines(chunk.Content()) switch chunk.Type() { - case fdiff.Add: - fl.Added = append(fl.Added, splitLines(chunk.Content())...) - case fdiff.Delete: - fl.Removed = append(fl.Removed, splitLines(chunk.Content())...) case fdiff.Equal: + closeHunk() + oldLine += len(lines) + case fdiff.Delete: + if cur == nil { + cur = &Hunk{OldStart: oldLine} + } + cur.Removed = append(cur.Removed, lines...) + fl.Removed = append(fl.Removed, lines...) + oldLine += len(lines) + case fdiff.Add: + if cur == nil { + cur = &Hunk{OldStart: oldLine} + } + cur.Added = append(cur.Added, lines...) + fl.Added = append(fl.Added, lines...) } } + closeHunk() out = append(out, fl) } @@ -140,8 +166,16 @@ func (c *ExecClient) CommitLines(repoRoot, sha string) ([]FileLines, error) { var out []FileLines var cur *FileLines + var hunk *Hunk var binary, inHunk bool + closeHunk := func() { + if cur != nil && hunk != nil { + cur.Hunks = append(cur.Hunks, *hunk) + } + hunk = nil + } flush := func() { + closeHunk() if cur != nil && !binary { out = append(out, *cur) } @@ -172,6 +206,7 @@ func (c *ExecClient) CommitLines(repoRoot, sha string) ([]FileLines, error) { binary = true case strings.HasPrefix(line, "@@"): inHunk = true + hunk = &Hunk{OldStart: hunkOldStart(line)} } continue @@ -179,10 +214,14 @@ func (c *ExecClient) CommitLines(repoRoot, sha string) ([]FileLines, error) { switch { case strings.HasPrefix(line, "@@"): + closeHunk() + hunk = &Hunk{OldStart: hunkOldStart(line)} case strings.HasPrefix(line, "+"): cur.Added = append(cur.Added, line[1:]) + hunk.Added = append(hunk.Added, line[1:]) case strings.HasPrefix(line, "-"): cur.Removed = append(cur.Removed, line[1:]) + hunk.Removed = append(hunk.Removed, line[1:]) } } flush() @@ -190,14 +229,33 @@ func (c *ExecClient) CommitLines(repoRoot, sha string) ([]FileLines, error) { return out, nil } +// hunkOldStart returns the first line of the parent's file that a hunk +// header "@@ -a,b +c,d @@" covers. A hunk that removes nothing gives a as the +// line before the insertion, so the hunk starts at a+1. +func hunkOldStart(header string) int { + fields := strings.Fields(header) + if len(fields) < 2 { + return 0 + } + + start, count, hasCount := strings.Cut(strings.TrimPrefix(fields[1], "-"), ",") + a, err := strconv.Atoi(start) + if err != nil { + return 0 + } + if hasCount && count == "0" { + return a + 1 + } + + return a +} + // FileAt returns the content of path at the commit, or nil when the commit // does not hold it. func (c *ExecClient) FileAt(repoRoot, sha, path string) ([]byte, error) { - if _, err := gitRawOutput(repoRoot, "rev-parse", "--verify", "--quiet", sha+"^{commit}"); err != nil { - return nil, fmt.Errorf("resolve %s: %w", sha, err) - } - if _, err := gitRawOutput(repoRoot, "cat-file", "-e", sha+":"+path); err != nil { - return nil, nil //nolint:nilerr // the commit resolves, so a failure here means the path is absent + exists, err := pathAt(repoRoot, sha, path) + if err != nil || !exists { + return nil, err } out, err := gitRawOutput(repoRoot, "cat-file", "blob", sha+":"+path) @@ -208,6 +266,19 @@ func (c *ExecClient) FileAt(repoRoot, sha, path string) ([]byte, error) { return out, nil } +// pathAt reports whether the commit holds path, without reading its content. +// An error means the commit does not resolve. +func pathAt(repoRoot, sha, path string) (bool, error) { + if _, err := gitRawOutput(repoRoot, "rev-parse", "--verify", "--quiet", sha+"^{commit}"); err != nil { + return false, fmt.Errorf("resolve %s: %w", sha, err) + } + if _, err := gitRawOutput(repoRoot, "cat-file", "-e", sha+":"+path); err != nil { + return false, nil //nolint:nilerr // the commit resolves, so a failure here means the path is absent + } + + return true, nil +} + // gitRawOutput runs git and returns its output unchanged. gitOutput trims // it, which would change file content. func gitRawOutput(repoRoot string, args ...string) ([]byte, error) { @@ -234,3 +305,89 @@ func splitLines(text string) []string { return lines } + +// Blame returns the last writer of each line of path at rev. On a long history +// it is many times slower than the git binary, which is why NewGitClient +// prefers the exec client. +func (c *GoGitClient) Blame(repoRoot, rev, path string) ([]BlameLine, error) { + return goGitBlame(repoRoot, rev, path) +} + +// Blame returns the last writer of each line of path at rev. +func (c *ExecClient) Blame(repoRoot, rev, path string) ([]BlameLine, error) { + return blamePorcelain(repoRoot, rev, path) +} + +// blamePorcelain runs `git blame --porcelain` and reads the commit and the +// text of each line. A path that rev does not hold has no lines. +func blamePorcelain(repoRoot, rev, path string) ([]BlameLine, error) { + exists, err := pathAt(repoRoot, rev, path) + if err != nil || !exists { + return nil, err + } + + raw, err := gitRawOutput(repoRoot, "blame", "--porcelain", rev, "--", path) + if err != nil { + return nil, fmt.Errorf("blame %s at %s: %w", path, rev, err) + } + + // Each line starts with a header " []", + // followed on the first use of a commit by key-value lines, and ends with + // its text after a tab. + var out []BlameLine + var sha string + for _, line := range strings.Split(string(raw), "\n") { + if text, ok := strings.CutPrefix(line, "\t"); ok { + out = append(out, BlameLine{SHA: sha, Text: text}) + continue + } + if fields := strings.Fields(line); len(fields) >= 3 && len(fields[0]) == 40 && isHex(fields[0]) { + sha = fields[0] + } + } + + return out, nil +} + +// goGitBlame blames with go-git. +func goGitBlame(repoRoot, rev, path string) ([]BlameLine, error) { + repo, err := openRepo(repoRoot) + if err != nil { + return nil, fmt.Errorf("open repo: %w", err) + } + + hash, err := repo.ResolveRevision(plumbing.Revision(rev)) + if err != nil { + return nil, fmt.Errorf("resolve %s: %w", rev, err) + } + commit, err := repo.CommitObject(*hash) + if err != nil { + return nil, fmt.Errorf("get commit %s: %w", rev, err) + } + if _, err := commit.File(path); errors.Is(err, object.ErrFileNotFound) { + return nil, nil + } + + res, err := gogit.Blame(commit, path) + if err != nil { + return nil, fmt.Errorf("blame %s at %s: %w", path, rev, err) + } + + out := make([]BlameLine, 0, len(res.Lines)) + for _, l := range res.Lines { + out = append(out, BlameLine{SHA: l.Hash.String(), Text: l.Text}) + } + + return out, nil +} + +// isHex reports whether s holds only lowercase hexadecimal digits. +func isHex(s string) bool { + for _, r := range s { + if (r < '0' || r > '9') && (r < 'a' || r > 'f') { + return false + } + } + + return true +} diff --git a/app/cli/internal/trace/git/lines_test.go b/app/cli/internal/trace/git/lines_test.go index d72068de9..43cb36a0f 100644 --- a/app/cli/internal/trace/git/lines_test.go +++ b/app/cli/internal/trace/git/lines_test.go @@ -17,6 +17,7 @@ package git import ( "os" + "os/exec" "path/filepath" "sort" "testing" @@ -25,6 +26,12 @@ import ( "github.com/stretchr/testify/require" ) +const ( + lineOne = "one" + lineTwo = "two" + lineFour = "four" +) + func TestCommitLinesAndFileAt(t *testing.T) { dir := t.TempDir() run := func(args ...string) string { return gitOutput(dir, args...) } @@ -60,9 +67,10 @@ func TestCommitLinesAndFileAt(t *testing.T) { got, err := c.CommitLines(dir, root) require.NoError(t, err) sortFileLines(got) + rootA, rootGone := []string{lineOne, lineTwo, "three"}, []string{"bye"} assert.Equal(t, []FileLines{ - {Path: "a.txt", Added: []string{"one", "two", "three"}}, - {Path: "gone.txt", Added: []string{"bye"}}, + {Path: "a.txt", Added: rootA, Hunks: []Hunk{{OldStart: 1, Added: rootA}}}, + {Path: "gone.txt", Added: rootGone, Hunks: []Hunk{{OldStart: 1, Added: rootGone}}}, }, got, "binary files are left out") }) @@ -70,11 +78,15 @@ func TestCommitLinesAndFileAt(t *testing.T) { got, err := c.CommitLines(dir, second) require.NoError(t, err) sortFileLines(got) + two, upperTwo, bye := []string{lineTwo}, []string{"TWO"}, []string{"bye"} assert.Equal(t, []FileLines{ - {Path: "a.txt", Added: []string{"TWO", "four"}, Removed: []string{"two"}}, - {Path: "gone.txt", Removed: []string{"bye"}}, - {Path: "new.txt", Added: []string{"hello"}}, - {Path: "with space.txt", Added: []string{"spaced"}}, + {Path: "a.txt", Added: []string{upperTwo[0], lineFour}, Removed: two, Hunks: []Hunk{ + {OldStart: 2, Removed: two, Added: upperTwo}, + {OldStart: 4, Added: []string{lineFour}}, + }}, + {Path: "gone.txt", Removed: bye, Hunks: []Hunk{{OldStart: 1, Removed: bye}}}, + {Path: "new.txt", Added: []string{"hello"}, Hunks: []Hunk{{OldStart: 1, Added: []string{"hello"}}}}, + {Path: "with space.txt", Added: []string{"spaced"}, Hunks: []Hunk{{OldStart: 1, Added: []string{"spaced"}}}}, }, got) }) @@ -99,6 +111,25 @@ func TestCommitLinesAndFileAt(t *testing.T) { _, err := c.CommitLines(dir, "deadbeef00000000000000000000000000000000") assert.Error(t, err) }) + + t.Run("blame names the last writer of each line", func(t *testing.T) { + got, err := c.Blame(dir, second, "a.txt") + require.NoError(t, err) + assert.Equal(t, []BlameLine{ + {SHA: root, Text: lineOne}, + {SHA: second, Text: "TWO"}, + {SHA: root, Text: "three"}, + {SHA: second, Text: lineFour}, + }, got) + + got, err = c.Blame(dir, second+"^", "a.txt") + require.NoError(t, err) + assert.Len(t, got, 3, "a revision name resolves") + + got, err = c.Blame(dir, second, "gone.txt") + require.NoError(t, err) + assert.Nil(t, got, "a file the commit does not hold has no lines") + }) }) } } @@ -106,3 +137,41 @@ func TestCommitLinesAndFileAt(t *testing.T) { func sortFileLines(fl []FileLines) { sort.Slice(fl, func(i, j int) bool { return fl[i].Path < fl[j].Path }) } + +func TestNewGitClient(t *testing.T) { + testCases := []struct { + name string + lookErr error + want Client + }{ + {name: "git on the PATH", want: &ExecClient{}}, + {name: "no git binary", lookErr: exec.ErrNotFound, want: &GoGitClient{}}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + orig := lookPath + t.Cleanup(func() { lookPath = orig }) + lookPath = func(string) (string, error) { return "/usr/bin/git", tc.lookErr } + + assert.IsType(t, tc.want, NewGitClient()) + }) + } +} + +func TestGoGitBlameFallback(t *testing.T) { + dir := t.TempDir() + run := func(args ...string) string { return gitOutput(dir, args...) } + run("init", "-b", "main") + run("config", "user.email", "test@test.com") + run("config", "user.name", "Test") + run("config", "commit.gpgsign", "false") + require.NoError(t, os.WriteFile(filepath.Join(dir, "a.txt"), []byte("one\ntwo\n"), 0600)) + run("add", ".") + run("commit", "-m", "root") + root := run("rev-parse", "HEAD") + + got, err := goGitBlame(dir, "HEAD", "a.txt") + require.NoError(t, err) + assert.Equal(t, []BlameLine{{SHA: root, Text: lineOne}, {SHA: root, Text: lineTwo}}, got) +} diff --git a/app/cli/internal/trace/state/commit.go b/app/cli/internal/trace/state/commit.go index 37b6715b2..546da6162 100644 --- a/app/cli/internal/trace/state/commit.go +++ b/app/cli/internal/trace/state/commit.go @@ -35,6 +35,33 @@ type CommitRecord struct { // the last push (e.g. a bare `git push` with no new commits, or a // `git tag && git push --tags` over already-pushed commits). Tracked bool `json:"tracked,omitempty"` + // Joined is set on a commit in no session that replaced lines of a + // session, and so joined that session (spec issue-3639 R-010). It maps + // each joined session ID to the files of the commit, and each file to the + // fingerprints of the lines that the replacing hunks added. The push + // fills it in. + Joined map[string]map[string][]string `json:"joined,omitempty"` +} + +// AllSessionIDs returns the sessions of the commit and the sessions that it +// joined, without duplicates, in no fixed order. +func (r *CommitRecord) AllSessionIDs() []string { + out := make([]string, 0, len(r.SessionIDs)+len(r.Joined)) + seen := make(map[string]struct{}, cap(out)) + for _, id := range r.SessionIDs { + if _, dup := seen[id]; !dup { + seen[id] = struct{}{} + out = append(out, id) + } + } + for id := range r.Joined { + if _, dup := seen[id]; !dup { + seen[id] = struct{}{} + out = append(out, id) + } + } + + return out } // SaveCommitRecord writes a commit record to /chainloop-trace/commits/.json. diff --git a/app/cli/internal/trace/state/state.go b/app/cli/internal/trace/state/state.go index 5c5525e4f..b4f625f0f 100644 --- a/app/cli/internal/trace/state/state.go +++ b/app/cli/internal/trace/state/state.go @@ -68,8 +68,10 @@ const ( // staleSessionRetention is how long a session's records are kept once // everything it produced has been attested. Reachability alone never // expires them: a session referenced by one surviving commit pins itself - // into the repository for good. - staleSessionRetention = 7 * 24 * time.Hour + // into the repository for good. Within this time, a later commit that + // replaces lines of the session sends its evidence again (spec + // issue-3639 R-004 and D-012). + staleSessionRetention = 30 * 24 * time.Hour ) // InitTraceDir creates the /chainloop-trace/ directory structure. @@ -251,7 +253,9 @@ func (s *Store) GCOrphans(liveSHAs map[string]bool) error { _ = os.Remove(path) continue } - for _, sid := range rec.SessionIDs { + // A session that a commit joined is live too: the next push of that + // commit counts its replacing lines for the session. + for _, sid := range rec.AllSessionIDs() { liveSessions[sanitizeID(sid)] = struct{}{} if !rec.Tracked { unattestedSessions[sanitizeID(sid)] = struct{}{} @@ -318,13 +322,13 @@ func (s *Store) GCOrphans(liveSHAs map[string]bool) error { // Liveness is deliberately not consulted: session-end does not always run, so // a leaked record stays Active forever and would never expire. // -// Uncommitted attribution is deliberately not spared either, so a week-old -// edit that is committed later is attributed to nobody rather than to the -// session that made it. That is the safe direction: keeping the entry means a -// session nothing has heard from in a week can still claim a file and pull the -// whole commit's diff into its evidence, and by then its transcript is usually -// gone — raw/ is wiped on every push — so the attestation it wins yields -// nothing anyway. Under-crediting beats crediting the wrong session. +// Uncommitted attribution is deliberately not spared either, so an edit older +// than the retention that is committed later is attributed to nobody rather +// than to the session that made it. That is the safe direction: keeping the +// entry means a session nothing has heard from in that time can still claim a +// file and pull the commit into its evidence, and by then its transcript is +// usually gone — raw/ is wiped on every push — so the attestation it wins +// yields nothing anyway. Under-crediting beats crediting the wrong session. func (s *Store) staleSessionIDs(unattested map[string]struct{}) map[string]struct{} { records, err := s.LoadAllSessionRecords() if err != nil { diff --git a/app/cli/internal/trace/state/state_test.go b/app/cli/internal/trace/state/state_test.go index 8ac3f7605..282e4788d 100644 --- a/app/cli/internal/trace/state/state_test.go +++ b/app/cli/internal/trace/state/state_test.go @@ -651,7 +651,7 @@ func TestGCOrphans_StaleSessions(t *testing.T) { store := NewGitStore(t.TempDir()) require.NoError(t, store.InitTraceDir()) - old := time.Now().UTC().Add(-30 * 24 * time.Hour).Format(time.RFC3339) + old := time.Now().UTC().Add(-31 * 24 * time.Hour).Format(time.RFC3339) require.NoError(t, store.SaveCommitRecord(&CommitRecord{ SHA: "sha-1", SessionIDs: []string{sessOld}, Timestamp: old, Tracked: true, })) @@ -670,7 +670,7 @@ func TestGCOrphans_StaleSessions(t *testing.T) { store := NewGitStore(t.TempDir()) require.NoError(t, store.InitTraceDir()) - old := time.Now().UTC().Add(-30 * 24 * time.Hour).Format(time.RFC3339) + old := time.Now().UTC().Add(-31 * 24 * time.Hour).Format(time.RFC3339) require.NoError(t, store.SaveCommitRecord(&CommitRecord{ SHA: "sha-1", SessionIDs: []string{sessOld}, Timestamp: old, Tracked: false, })) @@ -700,6 +700,43 @@ func TestGCOrphans_StaleSessions(t *testing.T) { assert.NotEmpty(t, store.LoadAILineAttribution("sess-new").Files) }) + // A later commit can still replace the lines of a pushed session and + // send it again (spec issue-3639 R-004), so its state outlives a week. + t.Run("keeps an attested session for 30 days", func(t *testing.T) { + store := NewGitStore(t.TempDir()) + require.NoError(t, store.InitTraceDir()) + + tenDays := time.Now().UTC().Add(-10 * 24 * time.Hour).Format(time.RFC3339) + require.NoError(t, store.SaveCommitRecord(&CommitRecord{ + SHA: "sha-1", SessionIDs: []string{sessOld}, Timestamp: tenDays, Tracked: true, + })) + require.NoError(t, store.SaveSessionRecord(&SessionRecord{SessionID: sessOld, StartedAt: tenDays})) + require.NoError(t, store.RecordLineRanges(sessOld, fileA, []aicodingsession.LineRange{{Start: 1, End: 5}})) + + require.NoError(t, store.GCOrphans(map[string]bool{"sha-1": true})) + + assert.True(t, store.SessionRecordExists(sessOld)) + assert.NotEmpty(t, store.LoadAILineAttribution(sessOld).Files) + }) + + t.Run("a commit that joined a session keeps the session", func(t *testing.T) { + store := NewGitStore(t.TempDir()) + require.NoError(t, store.InitTraceDir()) + + recent := time.Now().UTC().Add(-1 * time.Hour).Format(time.RFC3339) + require.NoError(t, store.SaveCommitRecord(&CommitRecord{ + SHA: "sha-human", Timestamp: recent, Tracked: true, + Joined: map[string]map[string][]string{"sess-joined": {fileA: {"fp"}}}, + })) + require.NoError(t, store.SaveSessionRecord(&SessionRecord{SessionID: "sess-joined", StartedAt: recent})) + require.NoError(t, store.RecordLineRanges("sess-joined", fileA, []aicodingsession.LineRange{{Start: 1, End: 5}})) + + require.NoError(t, store.GCOrphans(map[string]bool{"sha-human": true})) + + assert.True(t, store.SessionRecordExists("sess-joined")) + assert.NotEmpty(t, store.LoadAILineAttribution("sess-joined").Files) + }) + t.Run("an unparseable StartedAt is never treated as stale", func(t *testing.T) { store := NewGitStore(t.TempDir()) require.NoError(t, store.InitTraceDir()) diff --git a/app/cli/pkg/action/trace_agent_hook.go b/app/cli/pkg/action/trace_agent_hook.go index 50bca42b6..aaea0f63b 100644 --- a/app/cli/pkg/action/trace_agent_hook.go +++ b/app/cli/pkg/action/trace_agent_hook.go @@ -428,7 +428,7 @@ func shellCallKey(input *trace.HookInput) state.ShellCallKey { // run in the session's own checkout. Best-effort: failures are logged and // never block the agent. func captureWorktreeSnapshot(store *state.Store, repoRoot string, key state.ShellCallKey, log zerolog.Logger) { - client := tracegit.NewGoGitClient() + client := tracegit.NewGitClient() sigs := make(state.WorktreeSignatures) heads := make(map[string]string) for _, root := range shellSnapshotRoots(store, repoRoot, key.SessionID) { @@ -896,7 +896,7 @@ func recordCommandLineRanges(provider trace.Provider, input *trace.HookInput, st // ones, such as uncommitted human lines, are claimed. An untracked file counts // as new. func recordRootChanges(store *state.Store, repoRoot, base string, before map[string]string, sessionID string, log zerolog.Logger) { - client := tracegit.NewGoGitClient() + client := tracegit.NewGitClient() after, err := client.SnapshotWorktree(repoRoot) if err != nil { log.Debug().Err(err).Str("root", repoRoot).Msg("post-command: worktree snapshot failed") diff --git a/app/cli/pkg/action/trace_hook_handler.go b/app/cli/pkg/action/trace_hook_handler.go index bc16bbc22..3c82cfe17 100644 --- a/app/cli/pkg/action/trace_hook_handler.go +++ b/app/cli/pkg/action/trace_hook_handler.go @@ -25,6 +25,7 @@ import ( "maps" "os" "path/filepath" + "slices" "sort" "strconv" "strings" @@ -72,7 +73,7 @@ func handleCommitMsg(msgFilePath string, log zerolog.Logger) error { return nil } - client := tracegit.NewGoGitClient() + client := tracegit.NewGitClient() // An amend with a new message replaces HEAD's trailer, and its sessions // cannot be matched again because HEAD already consumed their pending @@ -248,7 +249,7 @@ func handlePostCommit(_ context.Context, log zerolog.Logger) error { log.Debug().Str("git_dir", gitDir).Msg("post-commit hook invoked") - client := tracegit.NewGoGitClient() + client := tracegit.NewGitClient() sha, message, err := client.CommitHeadInfo(repoRoot) if err != nil { return fmt.Errorf("get commit info: %w", err) @@ -749,14 +750,26 @@ func RunTracePush(ctx context.Context, log zerolog.Logger, opts RunTracePushOpts // they produced no commit on this branch (the trace-run and local-store paths). // A session that has ended without a commit is left out: assembling it on every // later push would republish the same stale evidence. +// +// A commit that joined a session (spec issue-3639 R-010) is in the group of +// that session too. A session with joined commits only is left out: its own +// commits are not on this branch, so the push cannot rebuild it (D-013). func sessionCommitGroups(aiCommits []*state.CommitRecord, sessionRecords map[string]*state.SessionRecord, allowEmpty bool) map[string][]*state.CommitRecord { groups := make(map[string][]*state.CommitRecord) + own := make(map[string]bool) for _, c := range aiCommits { - for _, sid := range c.SessionIDs { + for _, sid := range c.AllSessionIDs() { groups[sid] = append(groups[sid], c) } + for _, sid := range c.SessionIDs { + own[sid] = true + } } - for _, commits := range groups { + for sid, commits := range groups { + if !own[sid] { + delete(groups, sid) + continue + } sort.Slice(commits, func(i, j int) bool { return commits[i].Timestamp < commits[j].Timestamp }) @@ -790,19 +803,33 @@ func gatherPushSessions(ctx context.Context, store *state.Store, repoRoot, sessi } log.Debug().Int("commit_count", len(allCommits)).Msg("loaded commit records") - gitClient := tracegit.NewGoGitClient() + gitClient := tracegit.NewGitClient() // Drop orphan records (SHAs no longer in the branch — typically left over // by a rebase) and any record that turns out to belong to a merge commit. allCommits = filterCurrentBranchCommits(gitClient, repoRoot, allCommits, log) - // Discard commits without any session IDs (no AI agent involved). + // A new commit can replace lines of a session that a previous push sent. + // That session is sent again, with its new net numbers (spec issue-3639 + // R-004), and a commit in no session joins it (R-010). + ledgers := loadLedgers(store, log) + affected, failed := scanReplacements(gitClient, store, repoRoot, allCommits, ledgers, log) + // A commit in no session that could not be read stays untracked, so the + // next push reads it again. It is never sent itself, so the retry does not + // send a session twice. A commit of a session is sent with its session, and + // is tracked like any other: a replacement in it that the scan missed is + // not found later. + retry := func(c *state.CommitRecord) bool { return failed[c.SHA] && len(c.SessionIDs) == 0 } + scanned := slices.DeleteFunc(slices.Clone(allCommits), retry) + + // Discard commits that neither belong to nor joined a session (no AI + // agent involved). var aiCommits []*state.CommitRecord var untrackedAICommits int for _, c := range allCommits { - if len(c.SessionIDs) > 0 { + if len(c.AllSessionIDs()) > 0 { aiCommits = append(aiCommits, c) - if !c.Tracked { + if !c.Tracked && !retry(c) { untrackedAICommits++ } } @@ -810,15 +837,18 @@ func gatherPushSessions(ctx context.Context, store *state.Store, repoRoot, sessi if len(aiCommits) == 0 && !allowEmpty { log.Debug().Msg("no AI-assisted commits") + markScanned(store, scanned, log) return nil, nil, nil } // If every candidate AI commit has already been recorded by a previous - // successful run, there is nothing new. Short-circuit so a no-op `git push` - // does not re-record the same evidence. - if len(aiCommits) > 0 && untrackedAICommits == 0 && !allowEmpty { + // successful run, and no new commit replaced lines of a session, there is + // nothing new. Short-circuit so a no-op `git push` does not re-record the + // same evidence. + if len(aiCommits) > 0 && untrackedAICommits == 0 && len(affected) == 0 && !allowEmpty { log.Debug().Int("ai_commit_count", len(aiCommits)).Msg("all AI-assisted commits already recorded") + markScanned(store, scanned, log) return nil, aiCommits, nil } @@ -836,22 +866,55 @@ func gatherPushSessions(ctx context.Context, store *state.Store, repoRoot, sessi } log.Debug().Int("session_count", len(sessionCommits)).Msg("grouped commits by session") - sessions := buildSessionEvidence(ctx, store, repoRoot, gitClient, sessionCommits, sessionRecords, sessionMode, log) + sessions := buildSessionEvidence(ctx, store, repoRoot, gitClient, sessionCommits, sessionRecords, ledgers, sessionMode, log) if len(sessions) == 0 { log.Debug().Msg("no session evidence could be generated") } - return sessions, aiCommits, nil + // Every commit of the branch that the scan could read is marked tracked + // once the evidence is out. + return sessions, scanned, nil +} + +// markScanned marks tracked the commits in no session that scanReplacements +// read, when the push has nothing to send. They replaced no line of a session, +// and reading them again on each push would only repeat the blame. +func markScanned(store *state.Store, records []*state.CommitRecord, log zerolog.Logger) { + for _, c := range records { + if c.Tracked || len(c.AllSessionIDs()) > 0 { + continue + } + c.Tracked = true + if err := store.SaveCommitRecord(c); err != nil { + log.Debug().Err(err).Str("sha", c.SHA).Msg("could not mark commit record as tracked") + } + } } -// finalizePush marks the AI commits recorded by this run as tracked and prunes -// the single-use trace state, so a later `git push` with no new commits +// loadLedgers returns the AI line ledger of every session, by session ID. +func loadLedgers(store *state.Store, log zerolog.Logger) map[string]*state.AILineAttribution { + ledgers := make(map[string]*state.AILineAttribution) + attrs, err := store.LoadAllAILineAttributions() + if err != nil { + log.Debug().Err(err).Msg("could not load AI line attributions") + + return ledgers + } + for _, a := range attrs { + ledgers[a.SessionID] = a + } + + return ledgers +} + +// finalizePush marks the commits that this run read as tracked and prunes the +// single-use trace state, so a later `git push` with no new commits // short-circuits in gatherPushSessions rather than re-recording. Every step is // best-effort: the evidence is already out, and at worst the next run repeats // the work. It is shared by the control-plane push and the local store, so both // get the same idempotency. -func finalizePush(store *state.Store, repoRoot string, aiCommits []*state.CommitRecord, log zerolog.Logger) { - for _, c := range aiCommits { +func finalizePush(store *state.Store, repoRoot string, commits []*state.CommitRecord, log zerolog.Logger) { + for _, c := range commits { if c.Tracked { continue } @@ -863,7 +926,7 @@ func finalizePush(store *state.Store, repoRoot string, aiCommits []*state.Commit _ = store.WipeTraceDir() - gitClient := tracegit.NewGoGitClient() + gitClient := tracegit.NewGitClient() if liveSHAs, err := gitClient.LocalReachableSHAs(repoRoot); err == nil { if err := store.GCOrphans(liveSHAs); err != nil { log.Debug().Err(err).Msg("orphan GC failed; trace state is intact but not pruned") @@ -887,7 +950,7 @@ type sessionEvidence struct { // buildSessionEvidence parses each session of the push into its evidence, // with the specs and the skills that go into the attestation next to it. A // session that cannot be read is left out, with a warning in the log. -func buildSessionEvidence(ctx context.Context, store *state.Store, repoRoot string, gitClient tracegit.Client, sessionCommits map[string][]*state.CommitRecord, sessionRecords map[string]*state.SessionRecord, sessionMode string, log zerolog.Logger) []sessionEvidence { +func buildSessionEvidence(ctx context.Context, store *state.Store, repoRoot string, gitClient tracegit.Client, sessionCommits map[string][]*state.CommitRecord, sessionRecords map[string]*state.SessionRecord, ledgers map[string]*state.AILineAttribution, sessionMode string, log zerolog.Logger) []sessionEvidence { // Collect repo-level context once (same for all sessions) rawDir := store.RawSessionDir() @@ -898,17 +961,9 @@ func buildSessionEvidence(ctx context.Context, store *state.Store, repoRoot stri isGenerated := gitClient.GeneratedMatcher(repoRoot) - // Every ledger, because a commit can hold the lines of more than one - // session, and the lines of each commit, read once for all sessions. - ledgers := make(map[string]*state.AILineAttribution) - if attrs, err := store.LoadAllAILineAttributions(); err != nil { - log.Debug().Err(err).Msg("could not load AI line attributions") - } else { - for _, a := range attrs { - ledgers[a.SessionID] = a - } - } - commitLines := make(map[string][]tracegit.FileLines) + // The lines of each commit and the blame of each file at the pushed + // commit, read once for all sessions. + lineCache := newLineCache(gitClient, repoRoot, log) var sessions []sessionEvidence @@ -978,23 +1033,8 @@ func buildSessionEvidence(ctx context.Context, store *state.Store, repoRoot stri result.Data.GitContext = &sessionCtx } - // Collect code changes scoped to this session's commits. Use a SET-based - // API rather than a SHA range: when the session's commits are non- - // contiguous on the branch (e.g. interleaved with another session or - // human commits), a range diff would over-count by including the gap. if len(commits) > 0 { - shas := make([]string, 0, len(commits)) - for _, c := range commits { - shas = append(shas, c.SHA) - } - codeChanges, err := gitClient.CodeChangesForCommits(repoRoot, shas) - if err != nil { - log.Debug().Err(err).Msg("could not collect code changes") - } else { - attribution.FilterGenerated(codeChanges, isGenerated) - attribution.Enrich(sessionID, ledgers, sessionCommitLines(gitClient, repoRoot, commits, commitLines, log), codeChanges) - result.Data.CodeChanges = codeChanges - } + result.Data.CodeChanges = sessionCodeChanges(gitClient, repoRoot, sessionID, commits, ledgers, lineCache, isGenerated, log) } log.Debug().Str("session", sessionID).Msg("generated evidence") @@ -1011,27 +1051,130 @@ func buildSessionEvidence(ctx context.Context, store *state.Store, repoRoot stri return sessions } +// sessionCodeChanges collects the code changes of one session and attributes +// them. The diff stats come from the session's own commits. Use a SET-based +// API rather than a SHA range: when the session's commits are non-contiguous +// on the branch (e.g. interleaved with another session or human commits), a +// range diff would over-count by including the gap. The AI/human split of the +// added lines is the net contribution at the pushed commit (spec issue-3639 +// R-003), with the human lines of the commits that joined the session (R-010). +// It returns nil when the diff cannot be read. +func sessionCodeChanges(gitClient tracegit.Client, repoRoot, sessionID string, commits []*state.CommitRecord, ledgers map[string]*state.AILineAttribution, cache *lineCache, isGenerated func(string) bool, log zerolog.Logger) *aicodingsession.CodeChanges { + var shas []string + for _, c := range commits { + if slices.Contains(c.SessionIDs, sessionID) { + shas = append(shas, c.SHA) + } + } + + codeChanges, err := gitClient.CodeChangesForCommits(repoRoot, shas) + if err != nil { + log.Debug().Err(err).Msg("could not collect code changes") + + return nil + } + + attribution.FilterGenerated(codeChanges, isGenerated) + + // A legacy file keeps the range rule, which needs no blame. + tip := make(map[string][]tracegit.BlameLine, len(codeChanges.Files)) + for _, f := range codeChanges.Files { + if l := ledgers[sessionID]; l != nil && l.Legacy[f.Path] { + continue + } + if lines, ok := cache.blame(f.Path); ok { + tip[f.Path] = lines + } + } + + attribution.Enrich(sessionID, ledgers, sessionCommitLines(sessionID, commits, cache), tip, codeChanges) + + return codeChanges +} + // sessionCommitLines returns the changed lines of each commit of a session, -// oldest first, with the sessions that the commit's record names. cache holds -// the lines already read, by SHA. A commit that cannot be read is left out. -func sessionCommitLines(gitClient tracegit.Client, repoRoot string, commits []*state.CommitRecord, cache map[string][]tracegit.FileLines, log zerolog.Logger) []attribution.CommitLines { +// oldest first, with the sessions that the commit's record names. A commit +// that joined the session carries the lines that its replacing hunks added. A +// commit that cannot be read is left out. +func sessionCommitLines(sessionID string, commits []*state.CommitRecord, cache *lineCache) []attribution.CommitLines { out := make([]attribution.CommitLines, 0, len(commits)) for _, c := range commits { - lines, ok := cache[c.SHA] + lines, ok := cache.commit(c.SHA) if !ok { - var err error - if lines, err = gitClient.CommitLines(repoRoot, c.SHA); err != nil { - log.Debug().Err(err).Str("sha", c.SHA).Msg("could not read the lines of a commit") - continue + continue + } + + cl := attribution.CommitLines{SHA: c.SHA, SessionIDs: c.SessionIDs, Files: lines} + if !slices.Contains(c.SessionIDs, sessionID) { + // A joined commit with no added line still has to read as joined. + cl.Joined = c.Joined[sessionID] + if cl.Joined == nil { + cl.Joined = map[string][]string{} } - cache[c.SHA] = lines } - out = append(out, attribution.CommitLines{SessionIDs: c.SessionIDs, Files: lines}) + out = append(out, cl) } return out } +// lineCache reads the lines of commits and the blame of files at HEAD, the +// commit that the push sends, each one time for all the sessions of a push. +type lineCache struct { + client tracegit.Client + repoRoot string + log zerolog.Logger + commits map[string][]tracegit.FileLines + blames map[string][]tracegit.BlameLine +} + +func newLineCache(client tracegit.Client, repoRoot string, log zerolog.Logger) *lineCache { + return &lineCache{ + client: client, + repoRoot: repoRoot, + log: log, + commits: make(map[string][]tracegit.FileLines), + blames: make(map[string][]tracegit.BlameLine), + } +} + +// commit returns the lines that a commit changed, and false when the commit +// cannot be read. +func (c *lineCache) commit(sha string) ([]tracegit.FileLines, bool) { + if lines, ok := c.commits[sha]; ok { + return lines, true + } + + lines, err := c.client.CommitLines(c.repoRoot, sha) + if err != nil { + c.log.Debug().Err(err).Str("sha", sha).Msg("could not read the lines of a commit") + + return nil, false + } + c.commits[sha] = lines + + return lines, true +} + +// blame returns the lines of a file at HEAD with the commit that last wrote +// each, nil for a file that HEAD does not hold, and false when the blame +// fails. +func (c *lineCache) blame(path string) ([]tracegit.BlameLine, bool) { + if lines, ok := c.blames[path]; ok { + return lines, true + } + + lines, err := c.client.Blame(c.repoRoot, "HEAD", path) + if err != nil { + c.log.Debug().Err(err).Str("file", path).Msg("could not blame a file at HEAD; its net numbers fall back to the sum of its commits") + + return nil, false + } + c.blames[path] = lines + + return lines, true +} + // attachSessionEvidence adds the materials of each session to the attestation: // its spec and skill materials first, so that the session material can record // their digests, then the session itself. It returns the sessions that were diff --git a/app/cli/pkg/action/trace_hook_handler_test.go b/app/cli/pkg/action/trace_hook_handler_test.go index d07b2a3d1..be430c55e 100644 --- a/app/cli/pkg/action/trace_hook_handler_test.go +++ b/app/cli/pkg/action/trace_hook_handler_test.go @@ -27,7 +27,6 @@ import ( "time" "github.com/chainloop-dev/chainloop/app/cli/internal/trace" - "github.com/chainloop-dev/chainloop/app/cli/internal/trace/attribution" tracegit "github.com/chainloop-dev/chainloop/app/cli/internal/trace/git" "github.com/chainloop-dev/chainloop/app/cli/internal/trace/state" "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/materials/aicodingsession" @@ -301,6 +300,10 @@ func (f *fakeBranchClient) CommitLines(string, string) ([]tracegit.FileLines, er func (f *fakeBranchClient) FileAt(string, string, string) ([]byte, error) { return nil, nil } +func (f *fakeBranchClient) Blame(string, string, string) ([]tracegit.BlameLine, error) { + return nil, nil +} + func (f *fakeBranchClient) CommitHeadInfo(string) (string, string, error) { return "", "", nil } func (f *fakeBranchClient) HeadAuthorTime(string) (time.Time, error) { return time.Time{}, nil } func (f *fakeBranchClient) GeneratedMatcher(string) func(string) bool { @@ -775,7 +778,7 @@ func TestRebaseLifecycle(t *testing.T) { assert.Len(t, all, 2) // --- 3. The pre-push filter should drop the orphan and keep the new SHA. --- - client := tracegit.NewGoGitClient() + client := tracegit.NewGitClient() filtered := filterCurrentBranchCommits(client, dir, all, zerolog.Nop()) require.Len(t, filtered, 1) assert.Equal(t, newSHA, filtered[0].SHA, "only the in-branch SHA should survive the filter") @@ -851,7 +854,7 @@ func TestCommitLifecycle_AmendKeepsSessions(t *testing.T) { all, err := store.LoadAllCommitRecords() require.NoError(t, err) - filtered := filterCurrentBranchCommits(tracegit.NewGoGitClient(), dir, all, zerolog.Nop()) + filtered := filterCurrentBranchCommits(tracegit.NewGitClient(), dir, all, zerolog.Nop()) require.Len(t, filtered, 1) assert.Equal(t, newSHA, filtered[0].SHA) } @@ -1169,7 +1172,7 @@ func TestPushRebasePushPreservesAIAttribution(t *testing.T) { // end of runTracePush. ai-lines/ and commits/ for sessionID must // survive because the SHA is reachable from feature. --- require.NoError(t, store.WipeTraceDir()) - gitClient := tracegit.NewGoGitClient() + gitClient := tracegit.NewGitClient() liveSHAs, err := gitClient.LocalReachableSHAs(dir) require.NoError(t, err) require.NotEmpty(t, liveSHAs, "post-push GC must see reachable SHAs") @@ -1216,20 +1219,15 @@ func TestPushRebasePushPreservesAIAttribution(t *testing.T) { // Returns the resulting CodeChanges so the caller can assert on it. func buildSessionCodeChanges(t *testing.T, store *state.Store, dir, sha, sessionID string) *aicodingsession.CodeChanges { t.Helper() - client := tracegit.NewGoGitClient() + client := tracegit.NewGitClient() cc, err := client.CodeChangesForCommits(dir, []string{sha}) require.NoError(t, err) - attrs, err := store.LoadAllAILineAttributions() - require.NoError(t, err) - ledgers := make(map[string]*state.AILineAttribution, len(attrs)) - for _, a := range attrs { - ledgers[a.SessionID] = a - } commits := []*state.CommitRecord{{SHA: sha, SessionIDs: []string{sessionID}}} - lines := sessionCommitLines(client, dir, commits, map[string][]tracegit.FileLines{}, zerolog.Nop()) - attribution.Enrich(sessionID, ledgers, lines, cc) + got := sessionCodeChanges(client, dir, sessionID, commits, loadLedgers(store, zerolog.Nop()), newLineCache(client, dir, zerolog.Nop()), func(string) bool { return false }, zerolog.Nop()) + require.NotNil(t, got) + require.Equal(t, cc.LinesAdded, got.LinesAdded) - return cc + return got } // requireFileAttribution finds path in changes.Files and asserts attribution diff --git a/app/cli/pkg/action/trace_line_attribution_test.go b/app/cli/pkg/action/trace_line_attribution_test.go index c5bf8fbe0..2d35ca355 100644 --- a/app/cli/pkg/action/trace_line_attribution_test.go +++ b/app/cli/pkg/action/trace_line_attribution_test.go @@ -16,12 +16,12 @@ package action import ( + "errors" "os" "path/filepath" "strings" "testing" - "github.com/chainloop-dev/chainloop/app/cli/internal/trace/attribution" tracegit "github.com/chainloop-dev/chainloop/app/cli/internal/trace/git" "github.com/chainloop-dev/chainloop/app/cli/internal/trace/state" "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/materials/aicodingsession" @@ -102,32 +102,68 @@ func (r *lineRepo) commit(subject string, files ...string) string { return string(got) } -// sessionChanges runs the push pipeline for one session over the commits that -// name it, and returns its code changes. -func (r *lineRepo) sessionChanges(sessionID string) *aicodingsession.CodeChanges { +// pushed is what a push sends for one session: its code changes and its +// commit list. +type pushed struct { + changes *aicodingsession.CodeChanges + commits []string +} + +// push runs the push pipeline of the git hook on the recorded commits, and +// marks them tracked as a successful push does. It returns what each session +// sends. +func (r *lineRepo) push() map[string]pushed { r.t.Helper() records, err := r.store.LoadAllCommitRecords() require.NoError(r.t, err) - commits := sessionCommitGroups(records, nil, false)[sessionID] - shas := make([]string, 0, len(commits)) - for _, c := range commits { - shas = append(shas, c.SHA) + client := tracegit.NewGitClient() + ledgers := loadLedgers(r.store, zerolog.Nop()) + affected, failed := scanReplacements(client, r.store, r.dir, records, ledgers, zerolog.Nop()) + require.Empty(r.t, failed) + + var aiCommits []*state.CommitRecord + untracked := 0 + for _, c := range records { + if len(c.AllSessionIDs()) > 0 { + aiCommits = append(aiCommits, c) + if !c.Tracked { + untracked++ + } + } } - client := tracegit.NewGoGitClient() - cc, err := client.CodeChangesForCommits(r.dir, shas) - require.NoError(r.t, err) + out := make(map[string]pushed) + if untracked > 0 || len(affected) > 0 { + cache := newLineCache(client, r.dir, zerolog.Nop()) + for sid, commits := range sessionCommitGroups(aiCommits, nil, false) { + var shas []string + for _, c := range commits { + shas = append(shas, c.SHA) + } + cc := sessionCodeChanges(client, r.dir, sid, commits, ledgers, cache, func(string) bool { return false }, zerolog.Nop()) + require.NotNil(r.t, cc) + out[sid] = pushed{changes: cc, commits: shas} + } + } - attrs, err := r.store.LoadAllAILineAttributions() - require.NoError(r.t, err) - ledgers := make(map[string]*state.AILineAttribution, len(attrs)) - for _, a := range attrs { - ledgers[a.SessionID] = a + for _, c := range records { + if !c.Tracked { + c.Tracked = true + require.NoError(r.t, r.store.SaveCommitRecord(c)) + } } - attribution.Enrich(sessionID, ledgers, sessionCommitLines(client, r.dir, commits, map[string][]tracegit.FileLines{}, zerolog.Nop()), cc) - return cc + return out +} + +// sessionChanges pushes and returns the code changes that a session sends. +func (r *lineRepo) sessionChanges(sessionID string) *aicodingsession.CodeChanges { + r.t.Helper() + got, ok := r.push()[sessionID] + require.True(r.t, ok, "the push sends session %s", sessionID) + + return got.changes } type lineSplit struct{ aiAdded, aiRemoved, humanAdded, humanRemoved int } @@ -185,10 +221,10 @@ func TestLineAttribution_HumanChangeBetweenAgentEdits(t *testing.T) { assert.Equal(t, lineSplit{aiAdded: 3, humanAdded: 1}, fileSplitOf(t, r.sessionChanges(lineSession), "a.go")) } -// Case 3 of the spec, as far as milestone 1 goes: a later commit of the same -// session replaces AI lines by hand. The removed AI lines are human removals -// (R-005). The net numbers (R-003) come with milestone 2. -func TestLineAttribution_HumanRemovalIsHuman(t *testing.T) { +// Case 3 of the spec: a later commit of the same session replaces AI lines by +// hand. Each line counts once (R-003), and the removed AI lines are human +// removals (R-005). +func TestLineAttribution_Case3SameSessionLaterCommit(t *testing.T) { r := newLineRepo(t) lines := numbered("ai", 5) @@ -200,7 +236,129 @@ func TestLineAttribution_HumanRemovalIsHuman(t *testing.T) { r.agentWrite(lineSession, "b.go", "package b\n") assert.Contains(t, r.commit("feat: b", "a.go", "b.go"), "Chainloop-Trace-Sessions: "+lineSession) - assert.Equal(t, lineSplit{aiAdded: 5, humanAdded: 1, humanRemoved: 1}, fileSplitOf(t, r.sessionChanges(lineSession), "a.go")) + assert.Equal(t, lineSplit{aiAdded: 4, humanAdded: 1, humanRemoved: 1}, fileSplitOf(t, r.sessionChanges(lineSession), "a.go")) +} + +// Cases 2 and 5 of the spec: a human commit in no session replaces AI lines +// after the session's push. The next push sends the session again (R-004), +// and the commit joins it with the human lines of its replacing hunks only +// (R-010). +func TestLineAttribution_Case2HumanCommitJoinsSession(t *testing.T) { + r := newLineRepo(t) + + lines := numbered("ai", 5) + r.agentWrite(lineSession, "a.go", content(lines)) + r.commit("feat: a", "a.go") + first := r.push() + require.Contains(t, first, lineSession) + assert.Equal(t, lineSplit{aiAdded: 5}, fileSplitOf(t, first[lineSession].changes, "a.go")) + + // The human replaces one AI line and also edits an unrelated file. + lines[1] = "human" + r.humanWrite("a.go", content(lines)) + r.humanWrite("other.go", "package other\n") + assert.NotContains(t, r.commit("fix: by hand", "a.go", "other.go"), "Chainloop-Trace-Sessions") + human := headSHA(t, r.dir) + + second := r.push() + require.Contains(t, second, lineSession, "the session is sent again") + got := second[lineSession] + assert.Equal(t, lineSplit{aiAdded: 4, humanAdded: 1}, fileSplitOf(t, got.changes, "a.go")) + assert.Contains(t, got.commits, human, "the commit list holds the joined commit") + for _, f := range got.changes.Files { + assert.NotEqual(t, "other.go", f.Path, "the unrelated file of the joined commit is not the session's") + } + + // A third push with nothing new sends nothing. + assert.Empty(t, r.push()) +} + +// A human commit that replaces the lines of an earlier human commit joins the +// sessions that the earlier commit joined, in whatever order the records are +// read. +func TestLineAttribution_ChainedHumanCommitsJoin(t *testing.T) { + r := newLineRepo(t) + + lines := numbered("ai", 5) + r.agentWrite(lineSession, "a.go", content(lines)) + r.commit("feat: a", "a.go") + r.push() + + lines[1] = "human one" + r.humanWrite("a.go", content(lines)) + r.commit("fix: one", "a.go") + first := headSHA(t, r.dir) + + lines[1] = "human two" + r.humanWrite("a.go", content(lines)) + r.commit("fix: two", "a.go") + second := headSHA(t, r.dir) + + got := r.push() + require.Contains(t, got, lineSession) + assert.Equal(t, lineSplit{aiAdded: 4, humanAdded: 1}, fileSplitOf(t, got[lineSession].changes, "a.go")) + assert.Contains(t, got[lineSession].commits, first) + assert.Contains(t, got[lineSession].commits, second, "the second commit joins through the first one") +} + +// failingLinesClient fails to read the lines of one commit. +type failingLinesClient struct { + tracegit.Client + sha string +} + +func (c failingLinesClient) CommitLines(repoRoot, sha string) ([]tracegit.FileLines, error) { + if sha == c.sha { + return nil, errors.New("read failed") + } + + return c.Client.CommitLines(repoRoot, sha) +} + +// A commit that the scan cannot read is reported, so that the push keeps it +// untracked and reads it again. +func TestScanReplacements_ReportsFailedCommits(t *testing.T) { + r := newLineRepo(t) + + r.agentWrite(lineSession, "a.go", content(numbered("ai", 3))) + r.commit("feat: a", "a.go") + r.humanWrite("a.go", "changed\n") + r.commit("fix: by hand", "a.go") + human := headSHA(t, r.dir) + + records, err := r.store.LoadAllCommitRecords() + require.NoError(t, err) + client := failingLinesClient{Client: tracegit.NewGitClient(), sha: human} + _, failed := scanReplacements(client, r.store, r.dir, records, loadLedgers(r.store, zerolog.Nop()), zerolog.Nop()) + + assert.Equal(t, map[string]bool{human: true}, failed) +} + +// Case 4 of the spec: a commit of another session replaces lines of the first +// session. The first session is sent again with what is still there (R-004), +// and the commit does not join it. +func TestLineAttribution_Case4OtherSessionReplaces(t *testing.T) { + r := newLineRepo(t) + const other = "sess-other" + + lines := numbered("ai", 5) + r.agentWrite(lineSession, "a.go", content(lines)) + r.commit("feat: a", "a.go") + r.push() + + lines[1] = "human in b" + r.humanWrite("a.go", content(lines)) + r.agentWrite(other, "b.go", "package b\n") + assert.Contains(t, r.commit("feat: b", "a.go", "b.go"), "Chainloop-Trace-Sessions: "+other) + commitB := headSHA(t, r.dir) + + got := r.push() + require.Contains(t, got, lineSession, "the replaced session is sent again") + assert.Equal(t, lineSplit{aiAdded: 4}, fileSplitOf(t, got[lineSession].changes, "a.go")) + assert.NotContains(t, got[lineSession].commits, commitB, "a commit of another session does not join") + + require.Contains(t, got, other) + assert.Equal(t, lineSplit{humanAdded: 1, humanRemoved: 1}, fileSplitOf(t, got[other].changes, "a.go")) } // R-005: lines that the agent removed are AI removals. @@ -288,7 +446,7 @@ func TestLineAttribution_SurvivesCherryPick(t *testing.T) { // Drop the feature branch and its record, as the post-push GC does. runGit(t, r.dir, "branch", "-D", "feature") - live, err := tracegit.NewGoGitClient().LocalReachableSHAs(r.dir) + live, err := tracegit.NewGitClient().LocalReachableSHAs(r.dir) require.NoError(t, err) require.NoError(t, r.store.GCOrphans(live)) diff --git a/app/cli/pkg/action/trace_replacements.go b/app/cli/pkg/action/trace_replacements.go new file mode 100644 index 000000000..a64c6c994 --- /dev/null +++ b/app/cli/pkg/action/trace_replacements.go @@ -0,0 +1,213 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package action + +import ( + "reflect" + "slices" + "sort" + + "github.com/chainloop-dev/chainloop/app/cli/internal/trace/attribution" + tracegit "github.com/chainloop-dev/chainloop/app/cli/internal/trace/git" + "github.com/chainloop-dev/chainloop/app/cli/internal/trace/state" + "github.com/rs/zerolog" +) + +// scanReplacements finds the sessions whose lines the commits not pushed yet +// removed or replaced, so that the push sends their evidence again with the +// new net numbers (spec issue-3639 R-004). records are the commit records of +// the branch. A commit in no session that replaced lines of a session joins +// that session (R-010): its record gets the lines that its replacing hunks +// added, and is saved. +// +// A join can depend on another one: a human commit that replaces the lines of +// an earlier human commit joins the sessions that the earlier commit joined. +// So the commits are read oldest first, and read again until no join changes. +// +// Only files with ledger entries are blamed, because only those can hold the +// lines of a session. It also returns the commits that it could not read, +// which must stay untracked so that the next push reads them again. +func scanReplacements(client tracegit.Client, store *state.Store, repoRoot string, records []*state.CommitRecord, ledgers map[string]*state.AILineAttribution, log zerolog.Logger) (affected map[string]struct{}, failed map[string]bool) { + bySHA := make(map[string]*state.CommitRecord, len(records)) + var untracked []*state.CommitRecord + for _, r := range records { + bySHA[r.SHA] = r + if !r.Tracked { + untracked = append(untracked, r) + } + } + sort.SliceStable(untracked, func(i, j int) bool { + return untracked[i].Timestamp < untracked[j].Timestamp + }) + + ledgerFiles := make(map[string]bool) + for _, l := range ledgers { + for path := range l.Files { + ledgerFiles[path] = true + } + } + + scan := &replacementScan{ + client: client, + repoRoot: repoRoot, + bySHA: bySHA, + ledgerFiles: ledgerFiles, + lines: make(map[string][]tracegit.FileLines), + blames: make(map[[2]string][]tracegit.BlameLine), + } + + affected = make(map[string]struct{}) + failed = make(map[string]bool) + changed := make(map[string]*state.CommitRecord) + // Each pass can add one link to a chain of joins, so the number of + // commits bounds the passes. + for range len(untracked) + 1 { + again := false + for _, rec := range untracked { + if failed[rec.SHA] { + continue + } + + joined, err := scan.replacedSessions(rec, affected) + if err != nil { + log.Debug().Err(err).Str("sha", rec.SHA).Msg("could not read the lines of a commit; the next push reads it again") + failed[rec.SHA] = true + continue + } + + // Only a commit in no session joins one. It is recomputed on each + // push until the commit is tracked, so that a failed push leaves + // no stale join behind. + if len(rec.SessionIDs) > 0 || reflect.DeepEqual(joined, rec.Joined) { + continue + } + rec.Joined = joined + changed[rec.SHA] = rec + again = true + } + if !again { + break + } + } + + for _, rec := range changed { + if err := store.SaveCommitRecord(rec); err != nil { + log.Debug().Err(err).Str("sha", rec.SHA).Msg("could not save the sessions that a commit joined") + } + } + + return affected, failed +} + +// replacementScan reads the lines of commits and the blame of their parents, +// each one time for all the passes of scanReplacements. +type replacementScan struct { + client tracegit.Client + repoRoot string + bySHA map[string]*state.CommitRecord + ledgerFiles map[string]bool + lines map[string][]tracegit.FileLines + blames map[[2]string][]tracegit.BlameLine +} + +// replacedSessions adds to affected each session that owns a line that the +// commit removed. It returns, for a commit in no session, the sessions that +// the commit joins, with the fingerprints of the lines that its replacing +// hunks added, per file. An error means the commit could not be read. +func (s *replacementScan) replacedSessions(rec *state.CommitRecord, affected map[string]struct{}) (map[string]map[string][]string, error) { + lines, ok := s.lines[rec.SHA] + if !ok { + var err error + if lines, err = s.client.CommitLines(s.repoRoot, rec.SHA); err != nil { + return nil, err + } + s.lines[rec.SHA] = lines + } + + var joined map[string]map[string][]string + for _, fl := range lines { + // A root commit removes nothing, so its parent is never blamed. + if len(fl.Removed) == 0 || !s.ledgerFiles[fl.Path] { + continue + } + + // The removed lines are lines of the parent, so the parent's blame + // names the commit that last wrote each one. + key := [2]string{rec.SHA, fl.Path} + blame, ok := s.blames[key] + if !ok { + var err error + if blame, err = s.client.Blame(s.repoRoot, rec.SHA+"^", fl.Path); err != nil { + return nil, err + } + s.blames[key] = blame + } + + for _, h := range fl.Hunks { + for _, sid := range hunkOwners(h, blame, s.bySHA, rec.SHA, fl.Path) { + affected[sid] = struct{}{} + if len(rec.SessionIDs) > 0 { + continue + } + + if joined == nil { + joined = make(map[string]map[string][]string) + } + if joined[sid] == nil { + joined[sid] = make(map[string][]string) + } + for _, l := range h.Added { + joined[sid][fl.Path] = append(joined[sid][fl.Path], attribution.Fingerprint(l)) + } + } + } + } + + return joined, nil +} + +// hunkOwners returns the sessions that own a line that the hunk removed, by +// the record of the commit that last wrote the line. A commit owns its line +// for each of its sessions, and for each session it joined when its +// replacing hunks added that line. +func hunkOwners(h tracegit.Hunk, blame []tracegit.BlameLine, bySHA map[string]*state.CommitRecord, self, path string) []string { + var owners []string + for i, text := range h.Removed { + idx := h.OldStart - 1 + i + if idx < 0 || idx >= len(blame) { + continue + } + owner := bySHA[blame[idx].SHA] + if owner == nil || owner.SHA == self { + continue + } + + for _, sid := range owner.SessionIDs { + if !slices.Contains(owners, sid) { + owners = append(owners, sid) + } + } + fp := attribution.Fingerprint(text) + for sid, files := range owner.Joined { + if slices.Contains(files[path], fp) && !slices.Contains(owners, sid) { + owners = append(owners, sid) + } + } + } + slices.Sort(owners) + + return owners +} diff --git a/app/cli/pkg/action/trace_skills_scenario_test.go b/app/cli/pkg/action/trace_skills_scenario_test.go index cc12448d9..a4ca30711 100644 --- a/app/cli/pkg/action/trace_skills_scenario_test.go +++ b/app/cli/pkg/action/trace_skills_scenario_test.go @@ -151,8 +151,8 @@ func TestSkillScenario(t *testing.T) { records, err := store.LoadAllSessionRecords() require.NoError(t, err) - sessions := buildSessionEvidence(ctx, store, repoRoot, tracegit.NewGoGitClient(), - map[string][]*state.CommitRecord{sessionID: nil}, records, aicodingsession.ModeCoding, zerolog.Nop()) + sessions := buildSessionEvidence(ctx, store, repoRoot, tracegit.NewGitClient(), + map[string][]*state.CommitRecord{sessionID: nil}, records, nil, aicodingsession.ModeCoding, zerolog.Nop()) require.Len(t, sessions, 1) adder := &fakeMaterialAdder{realDigests: true} diff --git a/docs/specs/issue-3639-trace-line-attribution.md b/docs/specs/issue-3639-trace-line-attribution.md index bc8b5b37e..c4b7334d3 100644 --- a/docs/specs/issue-3639-trace-line-attribution.md +++ b/docs/specs/issue-3639-trace-line-attribution.md @@ -209,10 +209,25 @@ The ledger keeps the added and removed counts of every agent edit (R-011). Repor - Do nothing now: the follow-up would need a second ledger change. - Source: https://github.com/chainloop-dev/chainloop/pull/3640#discussion_r4296029633 +### D-012: Keep the trace state of a pushed session for 30 days +The CLI keeps the ledger and the commit records of a session for 30 days after the session starts. The time counts only when a push attested all the work of the session. A local branch must still reach the session's commits. After that, a replacement of its lines does not update the session (R-004). +- Why: a later human edit often comes days after the push. Thirty days covers a normal review cycle, and the state stays small. +- Alternatives considered: + - Keep the earlier 7 days: a fix after a long review does not update the session. +- Source: decision of the spec owner on the open question, during milestone 2 (PR #3658) + +### D-013: The push sends a session again only from the branch with its commits +The push sends a session again only when its own commits are on the branch that the push sends. Take a session whose commits are only on another branch, for example the default branch. A commit that replaces its lines does not update it. +- Why: the push already reads only the commits of its branch. Finding sessions on other branches needs a walk of the history at each push. +- Alternatives considered: + - Any session whose commits HEAD reaches: more complete, but slower pushes. +- Source: milestone 2 implementation + ## Open Questions -- [ ] **How long does the CLI keep the trace state of a pushed session?** +- [x] **How long does the CLI keep the trace state of a pushed session?** Proposed: keep its ledger and commit records for 30 days at most. A local branch must still reach the session's commits. After that, a replacement of these lines does not update the session. + Answered by D-012. ## Milestones