diff --git a/app/cli/internal/trace/README.md b/app/cli/internal/trace/README.md index 5c7699cbe..60220254a 100644 --- a/app/cli/internal/trace/README.md +++ b/app/cli/internal/trace/README.md @@ -64,6 +64,7 @@ What the session was asked to build, and which skills it used. |---------|---------------|-------------|--------|--------------|------------| | Spec capture instruction | At session start, the agent is told to capture the specs of the session (tickets, documents, plans, images) in the spec folder. | Yes | Yes | Yes | Yes | | Spec capture reminder | At each user prompt, the agent is reminded to capture new or changed specs. | Yes | No | Yes | Yes | +| Local spec sources read at push | A capture whose source is a local file holds only a header and a placeholder. Each push reads the file and records its current content, so the evidence follows the edits of the file. Only a regular text file of up to 1 MiB is read. Otherwise the push keeps the body that the agent wrote, or drops the capture with a warning when there is no body (spec issue-3561). | Yes | Yes | Yes | Yes | | Skills tracking | The evidence lists the skills that the session used, with a copy of each skill as it ran. | Yes | No | Partial | Partial | | Spec source pointers | Exact copies of spec sources in the transcript (full file reads and file writes) are replaced with a pointer to the spec material, so the evidence does not hold the same content two or three times (spec issue-3556). | Yes | No | No | No | diff --git a/app/cli/internal/trace/spec/local.go b/app/cli/internal/trace/spec/local.go new file mode 100644 index 000000000..b93103ff7 --- /dev/null +++ b/app/cli/internal/trace/spec/local.go @@ -0,0 +1,193 @@ +// +// 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 spec + +import ( + "fmt" + "io" + "net/url" + "os" + "path/filepath" + "strings" + "time" + + "github.com/chainloop-dev/chainloop/app/cli/internal/trace/pointer" +) + +// Placeholder is the body that the agent writes for a capture whose source is +// a local file (spec issue-3561). It tells that the push fills the body from +// the file, and that the push has not done so yet. It is never recorded as +// content. +const Placeholder = "" + +// maxLocalSourceSize is the largest local file that the push reads as the body +// of a capture. A spec is text and small: a larger file is more likely a +// mistake than a spec. +const maxLocalSourceSize = 1 << 20 + +// readText converts one text file of the spec folder into a capture, and +// returns a warning when the file asked for content that the push could not +// get. +// +// When the header names a local file as the source, the body is the content +// of that file now, whatever the agent wrote below the header: the agent can +// edit the file many turns after it wrote the capture, and the evidence must +// hold the current version. The header the agent wrote is kept as it is. When +// the file cannot be used, the body the agent wrote stands, as for any other +// capture. The placeholder alone is never recorded. +func readText(repoRoot, name string, doc []byte, modTime time.Time) (*Capture, string) { + header, body := split(string(doc)) + path, local := localPath(repoRoot, homeDir(), parseHeader(header).URI) + + var entry *Capture + raw := doc + if local { + if content, fileTime, ok := readLocalSource(path); ok { + // The body is a suffix of the document, so what comes before + // it is the header block as the agent wrote it. + filled := fill(doc[:len(doc)-len(body)], content) + // An empty file gives no capture, and the fallback below + // decides whether that is worth a warning. + if entry = Parse(filled, fileTime); entry != nil { + raw = filled + entry.SourceDigest = pointer.Digest(content) + } + } + } + + if entry == nil { + entry = Parse(doc, modTime) + } + + if entry != nil { + entry.FileName = name + entry.Raw = raw + + return entry, "" + } + + var reason string + switch { + case local && (isPlaceholder(body) || strings.TrimSpace(body) == ""): + reason = "its source file could not be read" + case isPlaceholder(body): + reason = "it holds only the placeholder, and its source is not a local file" + default: + // A file with nothing in it and no local source is what "nothing + // to capture" leaves behind. + return nil, "" + } + + return nil, fmt.Sprintf("spec file %q was not recorded: %s", name, reason) +} + +// homeDir returns the home directory, or "" when it is not known. +func homeDir() string { + home, err := os.UserHomeDir() + if err != nil { + return "" + } + + return home +} + +// localPath returns the path of the local file that a capture names as its +// source, and whether it names one. A local file is an absolute path, a path +// in the home directory (~/), a file:// URI, or a path relative to the +// repository root. Anything with a scheme, such as a web address, is not. +func localPath(repoRoot, home, uri string) (string, bool) { + uri = strings.TrimSpace(uri) + + switch { + case uri == "": + return "", false + case strings.HasPrefix(uri, "~/"): + if home == "" { + return "", false + } + return filepath.Join(home, uri[2:]), true + case strings.HasPrefix(uri, "~"): + // Another user's home directory is not a source of this session. + return "", false + case strings.HasPrefix(uri, "file://"): + u, err := url.Parse(uri) + if err != nil || (u.Host != "" && u.Host != "localhost") || !filepath.IsAbs(u.Path) { + return "", false + } + return filepath.Clean(u.Path), true + case filepath.IsAbs(uri): + return filepath.Clean(uri), true + case hasScheme(uri): + return "", false + } + + return filepath.Join(repoRoot, uri), true +} + +// hasScheme reports whether uri starts with a URI scheme, such as https: or +// mailto:, rather than with a path. +func hasScheme(uri string) bool { + u, err := url.Parse(uri) + if err != nil { + // A path that net/url rejects, for example "1:x", is not a web + // address either, but a colon before any slash is a scheme in all + // but name. + before, _, found := strings.Cut(uri, ":") + return found && !strings.Contains(before, "/") + } + + return u.Scheme != "" +} + +// readLocalSource reads a local source file, and returns its content and its +// modification time. It reports false for anything other than a regular text +// file under maxLocalSourceSize. +func readLocalSource(path string) ([]byte, time.Time, bool) { + f, err := os.Open(path) + if err != nil { + return nil, time.Time{}, false + } + defer f.Close() + + info, err := f.Stat() + if err != nil || !info.Mode().IsRegular() || info.Size() > maxLocalSourceSize { + return nil, time.Time{}, false + } + + // The file can grow between the stat and the read. + content, err := io.ReadAll(io.LimitReader(f, maxLocalSourceSize+1)) + if err != nil || len(content) > maxLocalSourceSize { + return nil, time.Time{}, false + } + + if !isText(path, content) { + return nil, time.Time{}, false + } + + return content, info.ModTime(), true +} + +// fill joins the header block that the agent wrote and the content of the +// source file into the capture that the push records. +func fill(header, content []byte) []byte { + out := make([]byte, 0, len(header)+1+len(content)) + out = append(out, header...) + if len(out) > 0 && out[len(out)-1] != '\n' { + out = append(out, '\n') + } + + return append(out, content...) +} diff --git a/app/cli/internal/trace/spec/local_test.go b/app/cli/internal/trace/spec/local_test.go new file mode 100644 index 000000000..35953bbe4 --- /dev/null +++ b/app/cli/internal/trace/spec/local_test.go @@ -0,0 +1,347 @@ +// +// 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 spec + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/chainloop-dev/chainloop/app/cli/internal/trace/pointer" + "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/materials/aicodingsession" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// writeSource writes a local source file at path with a known modification +// time, creating its parent directory. +func writeSource(t *testing.T, path, content string, modTime time.Time) { + t.Helper() + + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) + require.NoError(t, os.WriteFile(path, []byte(content), 0o600)) + require.NoError(t, os.Chtimes(path, modTime, modTime)) +} + +// TestReadAllLocalSource covers spec issue-3561: a capture whose source is a +// local file is recorded with the content of that file at push time. +func TestReadAllLocalSource(t *testing.T) { + const ( + current = "# Spec foo\n\nThe last edit.\n" + header = "---\nkind: document\nuri: %s\nrole: spec\ntitle: \"Spec foo\"\n---\n" + fooPath = "docs/specs/foo.md" + gonePath = "docs/specs/gone.md" + bigPath = "big.md" + agentBody = "the copy the agent wrote\n" + ticketURL = "https://tracker.example.com/issue/1" + ) + + // uri returns a fixed uri, whatever the repository root. + uri := func(u string) func(_, _ string) string { return func(_, _ string) string { return u } } + + captureTime := time.Date(2026, 10, 1, 9, 0, 0, 0, time.UTC) + fileTime := time.Date(2026, 10, 2, 15, 30, 0, 0, time.UTC) + largeContent := strings.Repeat("a", maxLocalSourceSize+1) + binaryContent := "\x00\x01\xff\xfe not text" + + testCases := []struct { + name string + // uri returns the uri of the capture, given the repository root and + // the home directory of the test. + uri func(root, home string) string + // source is where the local file is written, relative to the + // repository root, or empty for no file. + source string + sourceContent string + // sourceDir makes the source a directory rather than a file. + sourceDir bool + body string + kind string + // wantBody is the body the push records, or empty for no entry. + wantBody string + wantFromFile bool + wantWarning bool + }{ + { + name: "a relative path with the placeholder", + uri: uri(fooPath), + source: fooPath, + sourceContent: current, + body: Placeholder + "\n", + wantBody: current, + wantFromFile: true, + }, + { + // R-001: the agent copied the spec, then edited it three times + // and did not touch the capture again. + name: "a stale copy is replaced", + uri: uri(fooPath), + source: fooPath, + sourceContent: current, + body: "# Spec foo\n\nThe first draft.\n", + wantBody: current, + wantFromFile: true, + }, + { + name: "no body at all", + uri: uri(fooPath), + source: fooPath, + sourceContent: current, + wantBody: current, + wantFromFile: true, + }, + { + name: "an absolute path", + uri: func(root, _ string) string { return filepath.Join(root, "notes", "design.md") }, + source: "notes/design.md", + sourceContent: current, + body: Placeholder, + wantBody: current, + wantFromFile: true, + }, + { + name: "a path in the home directory", + uri: uri("~/notes/design.md"), + source: "home/notes/design.md", + sourceContent: current, + body: Placeholder, + wantBody: current, + wantFromFile: true, + }, + { + name: "a file URI", + uri: func(root, _ string) string { return "file://" + filepath.Join(root, "notes", "design.md") }, + source: "notes/design.md", + sourceContent: current, + body: Placeholder, + wantBody: current, + wantFromFile: true, + }, + { + // The rule applies to each kind whose capture file is text. + name: "a capture of kind text", + uri: uri("plan.md"), + source: "plan.md", + sourceContent: current, + kind: aicodingsession.SpecKindText, + body: Placeholder, + wantBody: current, + wantFromFile: true, + }, + { + // R-003: the file was deleted before the push. + name: "a missing file with the placeholder", + uri: uri(gonePath), + body: Placeholder + "\n", + wantWarning: true, + }, + { + name: "a missing file with no body", + uri: uri(gonePath), + wantWarning: true, + }, + { + name: "a missing file with a body keeps the body", + uri: uri(gonePath), + body: agentBody, + wantBody: agentBody, + }, + { + name: "a file over the limit keeps the body", + uri: uri(bigPath), + source: bigPath, + sourceContent: largeContent, + body: agentBody, + wantBody: agentBody, + }, + { + name: "a file over the limit with the placeholder", + uri: uri(bigPath), + source: bigPath, + sourceContent: largeContent, + body: Placeholder, + wantWarning: true, + }, + { + name: "a file that is not text", + uri: uri("blob.bin"), + source: "blob.bin", + sourceContent: binaryContent, + body: Placeholder, + wantWarning: true, + }, + { + // NUL is valid UTF-8, but no text file holds it. + name: "a valid UTF-8 file with a NUL byte", + uri: uri("blob.dat"), + source: "blob.dat", + sourceContent: "header\x00binary payload", + body: Placeholder, + wantWarning: true, + }, + { + name: "a directory is not a source", + uri: uri("docs"), + source: "docs", + sourceDir: true, + body: Placeholder, + wantWarning: true, + }, + { + name: "an empty file with the placeholder", + uri: uri("empty.md"), + source: "empty.md", + sourceContent: " \n", + body: Placeholder, + wantWarning: true, + }, + { + name: "an empty file keeps the body", + uri: uri("empty.md"), + source: "empty.md", + sourceContent: " \n", + body: agentBody, + wantBody: agentBody, + }, + { + // A remote source is copied by the agent, as before. + name: "a URL is not a local file", + uri: uri(ticketURL), + body: "the ticket\n", + wantBody: "the ticket\n", + }, + { + // The push never records the placeholder as content, whatever + // the source. + name: "the placeholder for a URL", + uri: uri(ticketURL), + body: Placeholder, + wantWarning: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + root := t.TempDir() + home := filepath.Join(root, "home") + t.Setenv("HOME", home) + + if tc.source != "" { + path := filepath.Join(root, tc.source) + if tc.sourceDir { + require.NoError(t, os.MkdirAll(path, 0o755)) + } else { + writeSource(t, path, tc.sourceContent, fileTime) + } + } + + kind := tc.kind + if kind == "" { + kind = aicodingsession.SpecKindDocument + } + head := strings.Replace(header, "kind: document", "kind: "+kind, 1) + head = strings.Replace(head, "%s", tc.uri(root, home), 1) + writeSpec(t, root, sessionID, "spec-foo.md", head+tc.body, captureTime) + + entries, warnings, err := ReadAll(root, sessionID, nil) + require.NoError(t, err, "a capture never fails the push") + + if tc.wantWarning { + assert.Empty(t, entries, "the placeholder is never recorded as content") + require.Len(t, warnings, 1) + assert.Contains(t, warnings[0], "spec-foo.md", "the warning names the capture") + assert.NotContains(t, warnings[0], root, "a warning never carries a local path") + + return + } + + assert.Empty(t, warnings) + require.Len(t, entries, 1) + got := entries[0] + + assert.Equal(t, kind, got.Kind) + assert.Equal(t, head+tc.wantBody, string(got.Raw), "the header the agent wrote stays, and the body is replaced") + + if tc.wantFromFile { + assert.Equal(t, fileTime.Format(time.RFC3339), got.CapturedAt, "the capture time is the time of the file") + assert.Equal(t, pointer.Digest([]byte(tc.sourceContent)), got.SourceDigest, "the content read is a source for the pointers") + } else { + assert.Equal(t, captureTime.Format(time.RFC3339), got.CapturedAt) + assert.Empty(t, got.SourceDigest) + } + }) + } +} + +// TestReadAllLocalSourceFollowsEdits checks the case of spec issue-3561: the +// agent captures a spec, edits it, and does not touch the capture again. +// Each push records the version on disk. +func TestReadAllLocalSourceFollowsEdits(t *testing.T) { + root := t.TempDir() + source := filepath.Join(root, "docs", "specs", "foo.md") + writeSpec(t, root, sessionID, "spec-foo.md", "---\nkind: document\nuri: docs/specs/foo.md\n---\n"+Placeholder+"\n", time.Now()) + + for _, version := range []string{"first draft", "second draft", "third draft"} { + writeSource(t, source, version, time.Now()) + + entries, warnings, err := ReadAll(root, sessionID, nil) + require.NoError(t, err) + assert.Empty(t, warnings) + require.Len(t, entries, 1) + assert.True(t, strings.HasSuffix(string(entries[0].Raw), "---\n"+version), "the push records %q", version) + } +} + +func TestLocalPath(t *testing.T) { + const ( + root = "/repo" + home = "/home/dev" + abs = "/abs/notes/design.md" + ) + + testCases := []struct { + uri string + want string + wantOK bool + }{ + {uri: "docs/specs/foo.md", want: "/repo/docs/specs/foo.md", wantOK: true}, + {uri: "./docs/specs/foo.md", want: "/repo/docs/specs/foo.md", wantOK: true}, + {uri: abs, want: abs, wantOK: true}, + {uri: "~/notes/design.md", want: "/home/dev/notes/design.md", wantOK: true}, + {uri: "file://" + abs, want: abs, wantOK: true}, + {uri: "file:///abs/notes/my%20design.md", want: "/abs/notes/my design.md", wantOK: true}, + {uri: "file://localhost" + abs, want: abs, wantOK: true}, + {uri: "file://otherhost/abs/notes/design.md"}, + {uri: "file:relative.md"}, + {uri: "https://tracker.example.com/issue/1"}, + {uri: "http://example.com/doc"}, + {uri: "mailto:someone@example.com"}, + {uri: "urn:isbn:0451450523"}, + {uri: "~someone/notes.md"}, + {uri: ""}, + {uri: " "}, + } + + for _, tc := range testCases { + t.Run(tc.uri, func(t *testing.T) { + got, ok := localPath(root, home, tc.uri) + assert.Equal(t, tc.wantOK, ok) + assert.Equal(t, tc.want, got) + }) + } +} diff --git a/app/cli/internal/trace/spec/parse.go b/app/cli/internal/trace/spec/parse.go index d9873cb33..3da6410ae 100644 --- a/app/cli/internal/trace/spec/parse.go +++ b/app/cli/internal/trace/spec/parse.go @@ -16,10 +16,12 @@ package spec import ( + "bytes" "net/http" "path/filepath" "strings" "time" + "unicode/utf8" "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/materials/aicodingsession" "gopkg.in/yaml.v3" @@ -80,6 +82,11 @@ type Capture struct { // MetaRaw is the companion file of a verbatim file as the agent wrote it, // or nil when there is none. It is redacted before ParseMeta reads it. MetaRaw []byte + // SourceDigest is the digest of the local file that the push read the + // body from, in the form of pointer.Digest, or empty when the body is + // what the agent wrote. A full copy of that file in the transcript is a + // copy of this source. + SourceDigest string } // verbatimCapture describes a file that is stored as it is. Its kind comes @@ -123,7 +130,9 @@ func Parse(doc []byte, capturedAt time.Time) *Capture { meta := parseHeader(header) - if strings.TrimSpace(body) == "" { + // The placeholder only asks the push to fill the body from a local file. + // It is never content. + if strings.TrimSpace(body) == "" || isPlaceholder(body) { return nil } @@ -277,6 +286,7 @@ func truncate(s string, limit int) string { // split separates a leading frontmatter block from the body. A document with no // well-formed block is all body, so that a missing or unterminated header // cannot swallow the text it was supposed to introduce. +// The body is always a suffix of doc. func split(doc string) (header, body string) { rest := strings.TrimLeft(doc, "\r\n\t ") if !strings.HasPrefix(rest, delimiter) { @@ -299,12 +309,24 @@ func split(doc string) (header, body string) { return "", doc } +// isPlaceholder reports whether a body holds only the placeholder line. +func isPlaceholder(body string) bool { + return strings.TrimSpace(body) == Placeholder +} + // isDelimiter reports whether a line is a frontmatter fence, tolerating the // carriage return a CRLF document leaves behind and any trailing whitespace. func isDelimiter(line string) bool { return strings.TrimRight(line, "\r\t ") == delimiter } +// isText reports whether a file is text that a spec can hold: valid UTF-8 +// with no NUL byte, and not an image. NUL is valid UTF-8, but no text file +// holds it. +func isText(name string, doc []byte) bool { + return utf8.Valid(doc) && bytes.IndexByte(doc, 0) < 0 && !isImage(name, doc) +} + // isImage reports whether a spec file is an image. Content sniffing finds the // binary formats. An SVG is text, so only its extension tells it apart from a // spec the agent wrote. diff --git a/app/cli/internal/trace/spec/parse_test.go b/app/cli/internal/trace/spec/parse_test.go index 6d2ef44c5..81312316e 100644 --- a/app/cli/internal/trace/spec/parse_test.go +++ b/app/cli/internal/trace/spec/parse_test.go @@ -268,6 +268,10 @@ func TestParse(t *testing.T) { {name: "frontmatter with no body", doc: "---\nkind: ticket\n---\n", wantNil: true}, {name: "a whitespace-only body", doc: "---\nkind: ticket\n---\n \n\n\t\n", wantNil: true}, {name: "a document of only whitespace", doc: " \n\n ", wantNil: true}, + // The push fills the placeholder from a local file. It is never + // recorded as content (spec issue-3561, R-002). + {name: "only the placeholder", doc: "---\nkind: document\nuri: docs/foo.md\n---\n" + Placeholder + "\n", wantNil: true}, + {name: "the placeholder with padding", doc: "---\nkind: document\n---\n\n " + Placeholder + " \n\n", wantNil: true}, } for _, tc := range testCases { diff --git a/app/cli/internal/trace/spec/spec.go b/app/cli/internal/trace/spec/spec.go index 85fc0faef..7052f512a 100644 --- a/app/cli/internal/trace/spec/spec.go +++ b/app/cli/internal/trace/spec/spec.go @@ -41,7 +41,6 @@ import ( "sort" "strings" "time" - "unicode/utf8" "github.com/chainloop-dev/chainloop/app/cli/internal/trace/state" ) @@ -227,8 +226,8 @@ func ReadAll(repoRoot, sessionID string, recorded []string) ([]Capture, []string // An image, or a file that is not text, is something the agent copied // in rather than wrote. Parsing it as text would only mangle it. - if image := isImage(c.name, doc); image || !utf8.Valid(doc) { - capture := verbatimCapture(c.name, doc, c.modTime, image) + if !isText(c.name, doc) { + capture := verbatimCapture(c.name, doc, c.modTime, isImage(c.name, doc)) if path, ok := companions[c.name]; ok { // A companion file we cannot read costs its values only. if meta, err := os.ReadFile(path); err == nil { @@ -240,9 +239,11 @@ func ReadAll(repoRoot, sessionID string, recorded []string) ([]Capture, []string continue } - if entry := Parse(doc, c.modTime); entry != nil { - entry.FileName = c.name - entry.Raw = doc + entry, warning := readText(repoRoot, c.name, doc, c.modTime) + if warning != "" { + warnings = append(warnings, warning) + } + if entry != nil { entries = append(entries, *entry) } } diff --git a/app/cli/pkg/action/trace_spec.go b/app/cli/pkg/action/trace_spec.go index f63a61bff..d37ab226a 100644 --- a/app/cli/pkg/action/trace_spec.go +++ b/app/cli/pkg/action/trace_spec.go @@ -78,8 +78,17 @@ func sessionSpecReminder(repoRoot, sessionID string, log zerolog.Logger) string // transcript already holds it. And it says plainly that some sessions have // nothing to capture, because a model handed a mandatory-sounding instruction // will otherwise manufacture a spec for a typo fix. +// +// It states the stakes, because an agent that reads the captures as +// bookkeeping skips the update after an edit (spec issue-3561). A local file +// gets the placeholder instead of its text: each push reads the file, so the +// evidence follows its edits with no help from the agent. Only a remote +// source still depends on the agent, so the rules to keep it current are +// about that one. func specCaptureInstruction(specDir string) string { - return fmt.Sprintf(`Write the specification this session is working from into %s now, in your next tool call, before you start the work. + return fmt.Sprintf(`Write the specification this session is working from into %[1]s now, in your next tool call, before you start the work. + +These files are attested evidence of what this session was asked to build, like a commit. Each push of this session records them. A partial, outdated or summarized capture is a false record. Use your file-writing tool, one call per file — not a shell redirect or heredoc, which a sandboxed or worktree-isolated session refuses. It creates the directory for you. @@ -102,13 +111,19 @@ Set role to the purpose of the source. If a source has more than one purpose, pi - reference: supporting material, such as a screenshot or a background document. Set title to a short name for the source, such as the ticket title. Set description to one or two sentences that tell what the source holds. +A local text file is the exception to writing text. When the source is a file on disk — a spec in this repository, or a design note outside the repository — set uri to its path (absolute, ~/, or relative to the repository root) and write only this line below the frontmatter, not the text of the file: + +%[2]s + +Each push reads the file and records its content at that time, so a later edit of the file needs no new capture. + The transcript of this session already holds the conversation, so the user's request prompt is not a spec: do not capture it. Spec text that the user pastes, such as a ticket or a design document, is a spec. -An image is the one exception to writing text. If you can reach the image as a file — on disk, or at a URL you can download — copy the file itself into the folder with a copy or download command (cp, curl -o), keeping its extension and adding no frontmatter. Then write the role, title and description lines in a second file next to it. Give that file the name of the image plus .meta.yaml, for example mockup.png.meta.yaml. Do not capture an image pasted into this conversation: you have no file for it. +An image is the other exception to writing text. If you can reach the image as a file — on disk, or at a URL you can download — copy the file itself into the folder with a copy or download command (cp, curl -o), keeping its extension and adding no frontmatter. Then write the role, title and description lines in a second file next to it. Give that file the name of the image plus .meta.yaml, for example mockup.png.meta.yaml. Do not capture an image pasted into this conversation: you have no file for it. -If the task changes, or a spec changes — also one that this session writes, such as a design note outside the repository — overwrite its file with the current content, or add another. If there is nothing to capture — a one-line request, a typo fix, a question, a passing remark — write nothing at all. +For any other source — a ticket, a web page, pasted text, an approved plan — write its full text. If the task changes, or such a source changes, overwrite its file with the current content in the same turn as the change, or add another. Before you push, check that each of these files holds the current full text of its source. If there is nothing to capture — a one-line request, a typo fix, a question, a passing remark — write nothing at all. -These files are recorded as part of this session's evidence each time it is pushed, and removed when the session ends. They are git-ignored and will never appear in a commit. There is no need to mention any of this to the user.`, specDir) +The files are removed when the session ends. They are git-ignored and will never appear in a commit.`, specDir, spec.Placeholder) } // specCaptureReminder is the one place the wording of the reminder lives. @@ -117,15 +132,17 @@ These files are recorded as part of this session's evidence each time it is push // cases only: a broader rule would capture too much, and a long one would // distract the agent from the task. It repeats the folder and the header // format, because a resumed session may not hold the full instruction any -// more. +// more. Like the instruction, it states the stakes and gives a local file the +// placeholder instead of its text (spec issue-3561). func specCaptureReminder(specDir string) string { return fmt.Sprintf(`Chainloop spec capture. Folder: %s -Capture only in these cases, one file per source. Write each text file with your file-writing tool, starting with frontmatter (kind: ticket, document, image or text; uri: the source, when there is one). Copy an image file as it is, with no frontmatter: -- The user pasted or gave a new spec: write its actual text. +These files are attested evidence: a partial or outdated capture is a false record. Capture only in these cases, one file per source. Write each text file with your file-writing tool, starting with frontmatter (kind: ticket, document, image or text; uri: the source, when there is one). Copy an image file as it is, with no frontmatter: +- The user pasted or gave a new spec: write its full text. - The user gave an image as a file path or a URL: copy the file. - The user approved a plan: write it as kind text. -- A spec changed, also one this session edits: overwrite its file with the current content. A local file has its path as uri. -Do not capture the user's request prompt or a pasted image. If no case applies, do nothing and say nothing about it.`, specDir) +- A spec is a local text file, also one this session writes: set uri to its path. Each push reads the file. Write only this line below the frontmatter: %s +- A spec that is not a local file changed: overwrite its file with the current full text in the same turn. Check these files before a push. +Do not capture the user's request prompt or a pasted image. If no case applies, do nothing and say nothing about it.`, specDir, spec.Placeholder) } // readSessionSpecs reads the spec files of a session for a push. Most diff --git a/app/cli/pkg/action/trace_spec_materials.go b/app/cli/pkg/action/trace_spec_materials.go index 7cf23a77f..39c3d01aa 100644 --- a/app/cli/pkg/action/trace_spec_materials.go +++ b/app/cli/pkg/action/trace_spec_materials.go @@ -71,8 +71,9 @@ type specMaterialAdder interface { // // It also returns the names of the files it stored, in order, so that a // later push of the session keeps them first, and the sources that the -// transcript can hold copies of: the digest of each stored file as the agent -// wrote it, mapped to the digest of its material. +// transcript can hold copies of: the digest of each stored file as the push +// read it, and of the local file its body came from, mapped to the digest of +// its material. func attachSpecs(ctx context.Context, adder specMaterialAdder, redactor *specRedactor, names *materials.NameAllocator, sessionID string, captures []spec.Capture, log zerolog.Logger) (entries []aicodingsession.SpecEntry, warnings, stored []string, sources pointer.Sources) { if len(captures) == 0 { return nil, nil, nil, nil @@ -103,6 +104,12 @@ func attachSpecs(ctx context.Context, adder specMaterialAdder, redactor *specRed sources = make(pointer.Sources) } sources[pointer.Digest(c.Raw)] = entry.Digest + // A body the push read from a local file is a source too: a full + // read or write of that file holds the same content (spec + // issue-3561, R-005). + if c.SourceDigest != "" { + sources[c.SourceDigest] = entry.Digest + } } return entries, warnings, stored, sources diff --git a/app/cli/pkg/action/trace_spec_materials_test.go b/app/cli/pkg/action/trace_spec_materials_test.go index a912d5d4b..63f455980 100644 --- a/app/cli/pkg/action/trace_spec_materials_test.go +++ b/app/cli/pkg/action/trace_spec_materials_test.go @@ -25,6 +25,7 @@ import ( "testing" "time" + "github.com/chainloop-dev/chainloop/app/cli/internal/trace/pointer" "github.com/chainloop-dev/chainloop/app/cli/internal/trace/spec" "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/materials" "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/materials/aicodingsession" @@ -157,6 +158,24 @@ func TestAttachSpecs(t *testing.T) { }, entries) }) + t.Run("the content of a local source is a source for the pointers", func(t *testing.T) { + // Spec issue-3561, R-005: the push read the body from a local file, + // so a full copy of that file in the transcript is a copy of the + // material, as is the capture as recorded. + const fileContent = "# Spec foo\n\nThe last edit.\n" + local := specCapture(t, "spec-foo.md", "---\nkind: document\nuri: docs/specs/foo.md\n---\n"+fileContent, "2026-10-02T15:30:00Z") + local.SourceDigest = pointer.Digest([]byte(fileContent)) + + _, warnings, _, sources := attachSpecs(context.Background(), &fakeMaterialAdder{}, newSpecRedactor(t.TempDir()), materials.NewNameAllocator(nil), sessionID, []spec.Capture{local, ticket}, zerolog.Nop()) + + assert.Empty(t, warnings) + assert.Equal(t, pointer.Sources{ + pointer.Digest(local.Raw): "sha256:spec-7412a0-spec-foo", + pointer.Digest([]byte(fileContent)): "sha256:spec-7412a0-spec-foo", + pointer.Digest(ticket.Raw): "sha256:spec-7412a0-ticket-pfm-7289", + }, sources) + }) + t.Run("secrets are removed from the text and the source address before upload", func(t *testing.T) { adder := &fakeMaterialAdder{} withSecret := specCapture(t, "ticket.md", diff --git a/app/cli/pkg/action/trace_spec_pointers_test.go b/app/cli/pkg/action/trace_spec_pointers_test.go index c423665e4..ab04557eb 100644 --- a/app/cli/pkg/action/trace_spec_pointers_test.go +++ b/app/cli/pkg/action/trace_spec_pointers_test.go @@ -146,6 +146,60 @@ func TestAttachSessionEvidenceSpecPointers(t *testing.T) { assert.JSONEq(t, `{"type":"chainloop.replaced","digest":"`+ticketDigest+`"}`, string(writeResult.ToolUseResult[pointer.Marker])) } +// TestAttachSessionEvidenceLocalSourcePointers follows R-005 of spec +// issue-3561: the agent writes a design note in one full write, reads it in +// full, and captures it by its path with the placeholder. The push records +// the note, and both copies in the transcript become pointers to it. +func TestAttachSessionEvidenceLocalSourcePointers(t *testing.T) { + const sessionID = "9a8b7c-session" + const note = "# Design note\n\nThe export runs in the background.\n" + + repoRoot := t.TempDir() + notePath := filepath.Join(t.TempDir(), "notes", "design.md") + require.NoError(t, os.MkdirAll(filepath.Dir(notePath), 0o755)) + require.NoError(t, os.WriteFile(notePath, []byte(note), 0o600)) + + require.NoError(t, os.MkdirAll(spec.SessionDir(repoRoot, sessionID), 0o755)) + capture := "---\nkind: document\nuri: " + notePath + "\nrole: spec\ntitle: Design note\n---\n" + spec.Placeholder + "\n" + require.NoError(t, os.WriteFile(filepath.Join(spec.SessionDir(repoRoot, sessionID), "design-note.md"), []byte(capture), 0o600)) + + captures, warnings, err := spec.ReadAll(repoRoot, sessionID, nil) + require.NoError(t, err) + require.Empty(t, warnings) + require.Len(t, captures, 1) + + numLines := strconv.Itoa(strings.Count(note, "\n")) + raw := []json.RawMessage{ + json.RawMessage(`{"type":"assistant","message":{"role":"assistant","content":[{"type":"tool_use","id":"toolu_w","name":"Write","input":{"file_path":` + strconv.Quote(notePath) + `,"content":` + strconv.Quote(note) + `}}]}}`), + json.RawMessage(`{"type":"user","message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"toolu_w","content":"File created successfully"}]},"toolUseResult":{"type":"create","filePath":` + strconv.Quote(notePath) + `,"content":` + strconv.Quote(note) + `,"structuredPatch":[],"originalFile":null}}`), + json.RawMessage(`{"type":"assistant","message":{"role":"assistant","content":[{"type":"tool_use","id":"toolu_r","name":"Read","input":{"file_path":` + strconv.Quote(notePath) + `}}]}}`), + json.RawMessage(`{"type":"user","message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"toolu_r","content":` + strconv.Quote(note) + `}]},"toolUseResult":{"type":"text","file":{"filePath":` + strconv.Quote(notePath) + `,"content":` + strconv.Quote(note) + `,"numLines":` + numLines + `,"startLine":1,"totalLines":` + numLines + `}}}`), + } + + evidence := aicodingsession.NewEvidence(aicodingsession.Data{ + SchemaVersion: "0.1", + RawSession: map[string][]json.RawMessage{mainStream: raw}, + }) + + adder := &fakeMaterialAdder{realDigests: true} + attested, _ := attachSessionEvidence(context.Background(), adder, state.NewGitStore(t.TempDir()), []sessionEvidence{{ + sessionID: sessionID, + provider: claude.New(), + evidence: evidence, + specs: captures, + }}, zerolog.Nop()) + require.Equal(t, []string{sessionID}, attested) + + material := adder.byName("spec-9a8b7c-design-note") + assert.Contains(t, material.content, note, "the material holds the note, not the placeholder") + assert.NotContains(t, material.content, spec.Placeholder) + + session := adder.byName("ai-coding-session-9a8b7c") + assert.NotContains(t, session.content, "The export runs in the background", "no inline copy of the note") + // Two copies in the write, two in the read, and the spec entry. + assert.Equal(t, 5, strings.Count(session.content, material.digest), "each copy points to the material") +} + // TestReplaceSpecCopiesNeedsAFinder checks that the transcript of an agent // that cannot find copies keeps them. func TestReplaceSpecCopiesNeedsAFinder(t *testing.T) { diff --git a/app/cli/pkg/action/trace_spec_test.go b/app/cli/pkg/action/trace_spec_test.go index 72ba2d35a..a831974dc 100644 --- a/app/cli/pkg/action/trace_spec_test.go +++ b/app/cli/pkg/action/trace_spec_test.go @@ -75,7 +75,16 @@ func TestSpecCaptureInstruction(t *testing.T) { {name: "a pasted spec still counts", want: "Spec text that the user pastes", why: "a ticket or design document pasted into the chat is still a spec"}, {name: "a changed spec", want: "overwrite its file with the current content", why: "the push records what is on disk, so the final version replaces its drafts"}, {name: "a spec the session writes", want: "a design note outside the repository", why: "a plan the session keeps outside the working tree is a spec that changes"}, - {name: "each push", want: "each time it is pushed", why: "the files stay for the whole session, and every push records them"}, + {name: "each push", want: "Each push of this session records them", why: "the files stay for the whole session, and every push records them"}, + {name: "the stakes", want: "attested evidence", why: "an agent that reads the captures as bookkeeping skips the update after an edit (spec issue-3561, R-006)"}, + {name: "a wrong capture is false", want: "is a false record", why: "a partial or outdated capture is not a harmless shortcut"}, + {name: "a local file has the placeholder", want: spec.Placeholder, why: "the push fills the body of a local file, so the agent writes the placeholder only (R-002)"}, + {name: "a local file has its path", want: "set uri to its path", why: "the push reads the file at that path"}, + {name: "no text of a local file", want: "not the text of the file", why: "a copy of a local file is a duplicate that the push discards"}, + {name: "a local file is read at each push", want: "Each push reads the file", why: "a later edit of the file needs no new capture"}, + {name: "a remote source in full", want: "write its full text", why: "the push cannot read a remote source"}, + {name: "a remote change in the same turn", want: "in the same turn as the change", why: "an edit can be many turns before the push"}, + {name: "a check before a push", want: "Before you push", why: "nothing else checks a remote capture"}, {name: "the way out", want: "write nothing at all", why: "without it a model invents a spec for a typo fix"}, // A real session was lost exactly here: the agent resolved both sources // correctly and then wrote them with `mkdir -p X && cat > Y <