diff --git a/app/cli/internal/trace/spec/parse.go b/app/cli/internal/trace/spec/parse.go index b5da2fdd3..b282ec61e 100644 --- a/app/cli/internal/trace/spec/parse.go +++ b/app/cli/internal/trace/spec/parse.go @@ -28,11 +28,31 @@ import ( // delimiter opens and closes a frontmatter block. const delimiter = "---" +// The limits of a title and a description, in characters. A model sometimes +// writes a paragraph where a label is asked for, and a cut keeps a usable one. +const ( + MaxTitleLen = 120 + MaxDescriptionLen = 300 +) + +// Meta holds what the agent states about a source next to its kind: its +// purpose, a short name, and what it holds. A text file carries it in its +// header, and a binary file in a companion file. +type Meta struct { + // Role is one of the aicodingsession.SpecRole* constants, or empty. + Role string `yaml:"role"` + // Title is a short name for the source, or empty. + Title string `yaml:"title"` + // Description says what the source holds, or is empty. + Description string `yaml:"description"` +} + // frontmatter is the header a spec document carries. It is decoded into a typed // struct rather than a map so an unexpected key cannot reach the evidence. type frontmatter struct { Kind string `yaml:"kind"` URI string `yaml:"uri"` + Meta `yaml:",inline"` } // Capture is one spec document as read from the session folder: what the agent @@ -45,6 +65,9 @@ type Capture struct { Kind string // URI is where the text came from, empty for a spec stated in the session. URI string + // Meta holds the role, the title and the description of the source. For + // a binary file they stay empty until its companion file is redacted. + Meta // CapturedAt is the file's modification time, RFC3339. CapturedAt string // Raw is the file as the agent wrote it, header included. It is what @@ -54,6 +77,9 @@ type Capture struct { // image, or any file that is not text. The agent copied it into the // folder, so it has no header and nothing in it is rewritten. Verbatim bool + // 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 } // verbatimCapture describes a file that is stored as it is. Its kind comes @@ -86,6 +112,8 @@ func verbatimCapture(name string, doc []byte, modTime time.Time, image bool) Cap // - Frontmatter that is not valid YAML: the body after the block is content, // again with kind "text" and no URI. // - A kind outside the vocabulary: normalised to "text". +// - A role outside the vocabulary: no role. +// - A title or a description over its limit: cut to the limit. // // It returns nil when nothing is left once the body is trimmed. That is the // common shape of a session with nothing to capture: the agent was told to @@ -110,10 +138,42 @@ func Parse(doc []byte, capturedAt time.Time) *Capture { return &Capture{ Kind: aicodingsession.ResolveSpecKind(meta.Kind), URI: strings.TrimSpace(meta.URI), + Meta: meta.normalize(), CapturedAt: capturedAt.UTC().Format(time.RFC3339), } } +// ParseMeta reads the companion file of a binary file. It never fails: a file +// that is not valid YAML gives no values, and costs the binary file nothing. +func ParseMeta(doc []byte) Meta { + var meta Meta + if err := yaml.Unmarshal(doc, &meta); err != nil { + return Meta{} + } + + return meta.normalize() +} + +// normalize drops a role outside the vocabulary, trims the title and the +// description, and cuts them to their limits. +func (m Meta) normalize() Meta { + return Meta{ + Role: aicodingsession.ResolveSpecRole(m.Role), + Title: truncate(strings.TrimSpace(m.Title), MaxTitleLen), + Description: truncate(strings.TrimSpace(m.Description), MaxDescriptionLen), + } +} + +// truncate keeps the first limit characters of s, so a cut never splits one. +func truncate(s string, limit int) string { + runes := []rune(s) + if len(runes) <= limit { + return s + } + + return strings.TrimSpace(string(runes[:limit])) +} + // 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. diff --git a/app/cli/internal/trace/spec/parse_test.go b/app/cli/internal/trace/spec/parse_test.go index 6d7866724..297f9aea1 100644 --- a/app/cli/internal/trace/spec/parse_test.go +++ b/app/cli/internal/trace/spec/parse_test.go @@ -40,6 +40,9 @@ func TestParse(t *testing.T) { wantKind string wantURI string wantContent string + wantRole string + wantTitle string + wantDesc string }{ { name: "a full frontmatter block", @@ -118,6 +121,51 @@ func TestParse(t *testing.T) { wantKind: aicodingsession.SpecKindImage, wantContent: "what the mockup shows", }, + { + name: "a role, a title and a description", + doc: "---\nkind: ticket\nrole: task\ntitle: \"ENG-1234: Add an export button\"\ndescription: The ticket that the session implements.\n---\n" + specBody, + wantKind: aicodingsession.SpecKindTicket, + wantRole: aicodingsession.SpecRoleTask, + wantTitle: "ENG-1234: Add an export button", + wantDesc: "The ticket that the session implements.", + wantContent: specBody, + }, + { + name: "a role with case and padding", + doc: "---\nrole: \" Reference \"\n---\n" + specBody, + wantKind: aicodingsession.SpecKindText, + wantRole: aicodingsession.SpecRoleReference, + wantContent: specBody, + }, + { + // A bad role costs the role only: never the kind, and never a + // role guessed from the kind. + name: "a role outside the vocabulary", + doc: "---\nkind: document\nrole: design\n---\n" + specBody, + wantKind: aicodingsession.SpecKindDocument, + wantContent: specBody, + }, + { + name: "a title and a description of only whitespace", + doc: "---\ntitle: \" \"\ndescription: \"\\t\"\n---\n" + specBody, + wantKind: aicodingsession.SpecKindText, + wantContent: specBody, + }, + { + // The limit counts characters, so a cut never splits one. + name: "a title over the limit is cut", + doc: "---\ntitle: " + strings.Repeat("é", MaxTitleLen+5) + "\n---\n" + specBody, + wantKind: aicodingsession.SpecKindText, + wantTitle: strings.Repeat("é", MaxTitleLen), + wantContent: specBody, + }, + { + name: "a description over the limit is cut", + doc: "---\ndescription: " + strings.Repeat("a", MaxDescriptionLen+1) + "\n---\n" + specBody, + wantKind: aicodingsession.SpecKindText, + wantDesc: strings.Repeat("a", MaxDescriptionLen), + wantContent: specBody, + }, {name: "an empty document", doc: "", wantNil: true}, {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}, @@ -136,6 +184,9 @@ func TestParse(t *testing.T) { require.NotNil(t, got) assert.Equal(t, tc.wantKind, got.Kind) assert.Equal(t, tc.wantURI, got.URI) + assert.Equal(t, tc.wantRole, got.Role) + assert.Equal(t, tc.wantTitle, got.Title) + assert.Equal(t, tc.wantDesc, got.Description) // What follows the header is the text: split decides where the // header ends, which is also what decides the kind and the URI. _, body := split(tc.doc) @@ -144,3 +195,28 @@ func TestParse(t *testing.T) { }) } } + +func TestParseMeta(t *testing.T) { + testCases := []struct { + name string + doc string + want Meta + }{ + { + name: "every field", + doc: "role: reference\ntitle: Export button mockup\ndescription: Where the button goes.\n", + want: Meta{Role: aicodingsession.SpecRoleReference, Title: "Export button mockup", Description: "Where the button goes."}, + }, + {name: "a role outside the vocabulary", doc: "role: mockup\ntitle: x\n", want: Meta{Title: "x"}}, + {name: "a title over the limit is cut", doc: "title: " + strings.Repeat("a", MaxTitleLen+1), want: Meta{Title: strings.Repeat("a", MaxTitleLen)}}, + // A file we cannot read costs its values, and nothing else. + {name: "not valid YAML", doc: "role: plan\ntitle: [unclosed\n", want: Meta{}}, + {name: "an empty file", doc: "", want: Meta{}}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, ParseMeta([]byte(tc.doc))) + }) + } +} diff --git a/app/cli/internal/trace/spec/spec.go b/app/cli/internal/trace/spec/spec.go index eee53fc39..85fc0faef 100644 --- a/app/cli/internal/trace/spec/spec.go +++ b/app/cli/internal/trace/spec/spec.go @@ -62,6 +62,11 @@ const ( // the oldest new ones, since what the session started from is the last // thing worth dropping. MaxEntries = 25 + + // MetaSuffix names the companion file of a binary file: the name of the + // binary file plus this suffix. It holds the role, the title and the + // description that a binary file has no header for. + MetaSuffix = ".meta.yaml" ) // Dir returns the directory holding every session's captured specs. @@ -156,12 +161,21 @@ func ReadAll(repoRoot, sessionID string, recorded []string) ([]Capture, []string var warnings []string + // A companion file is never a spec of its own. It is read only for the + // binary file it is named for, and ignored when there is none. + companions := make(map[string]string) + candidates := make([]candidate, 0, len(dirEntries)) for _, e := range dirEntries { if !isCandidate(e) { continue } + if base, ok := strings.CutSuffix(e.Name(), MetaSuffix); ok { + companions[base] = filepath.Join(dir, e.Name()) + continue + } + info, err := e.Info() if err != nil { warnings = append(warnings, notRecorded(e.Name(), err)) @@ -214,7 +228,15 @@ 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) { - entries = append(entries, verbatimCapture(c.name, doc, c.modTime, image)) + capture := verbatimCapture(c.name, doc, c.modTime, image) + 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 { + capture.MetaRaw = meta + } + } + + entries = append(entries, capture) continue } diff --git a/app/cli/internal/trace/spec/spec_test.go b/app/cli/internal/trace/spec/spec_test.go index ff9fabcd2..2c4d32130 100644 --- a/app/cli/internal/trace/spec/spec_test.go +++ b/app/cli/internal/trace/spec/spec_test.go @@ -224,6 +224,43 @@ func TestReadAll(t *testing.T) { assert.True(t, entries[1].Verbatim) }) + t.Run("a companion file goes with its binary file and is no spec of its own", func(t *testing.T) { + root := t.TempDir() + pngBytes := []byte("\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\xff\xfe") + meta := "role: reference\ntitle: Export button mockup\n" + writeSpec(t, root, sessionID, "mockup.png", string(pngBytes), time.Now()) + writeSpec(t, root, sessionID, "mockup.png"+MetaSuffix, meta, time.Now()) + + entries, warnings, err := ReadAll(root, sessionID, nil) + + require.NoError(t, err) + assert.Empty(t, warnings) + require.Len(t, entries, 1) + assert.Equal(t, "mockup.png", entries[0].FileName) + assert.Equal(t, []byte(meta), entries[0].MetaRaw) + // The values are read only after the companion file is redacted. + assert.Empty(t, entries[0].Title) + assert.Empty(t, entries[0].Role) + }) + + t.Run("a companion file with no binary file next to it is ignored", func(t *testing.T) { + root := t.TempDir() + writeSpec(t, root, sessionID, "ticket.md", "the ticket", time.Now()) + // One with no file at all, and one next to a text file, which has a + // header of its own. + writeSpec(t, root, sessionID, "gone.png"+MetaSuffix, "role: reference\n", time.Now()) + writeSpec(t, root, sessionID, "ticket.md"+MetaSuffix, "role: task\n", time.Now()) + + entries, warnings, err := ReadAll(root, sessionID, nil) + + require.NoError(t, err) + assert.Empty(t, warnings) + require.Len(t, entries, 1) + assert.Equal(t, "ticket.md", entries[0].FileName) + assert.Nil(t, entries[0].MetaRaw) + assert.Empty(t, entries[0].Role) + }) + t.Run("an image in a text format is kept as it is too", func(t *testing.T) { // An SVG is valid UTF-8, so only its type tells it apart from a spec // the agent wrote. Parsing and redacting it as text could break it. diff --git a/app/cli/pkg/action/trace_spec.go b/app/cli/pkg/action/trace_spec.go index 46ad6a892..f63a61bff 100644 --- a/app/cli/pkg/action/trace_spec.go +++ b/app/cli/pkg/action/trace_spec.go @@ -88,13 +88,23 @@ Resolve whatever the task points at — a ticket, a design document, an image fi --- kind: ticket uri: https://tracker.example.com/issue/ENG-1234 +role: task +title: "ENG-1234: Add an export button" +description: The ticket that this session implements. --- Set kind to one of ticket, document, image or text. An approved plan, or spec text that came from outside this conversation, is kind text. Set uri to the source URL or the path of a local file, or omit the uri line when the text has no other source. +Set role to the purpose of the source. If a source has more than one purpose, pick the role that tells why it is in this session. Use one of these values: +- task: the item that states the work to do. +- spec: a document that defines what to build. +- plan: a plan that the user approved. +- 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. + 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. Do not capture an image pasted into this conversation: you have no file for it. +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. 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. diff --git a/app/cli/pkg/action/trace_spec_materials.go b/app/cli/pkg/action/trace_spec_materials.go index a5f4e02b2..ab443b391 100644 --- a/app/cli/pkg/action/trace_spec_materials.go +++ b/app/cli/pkg/action/trace_spec_materials.go @@ -36,9 +36,12 @@ import ( // Annotations on each spec material, so that a policy or a reader can find // the spec of a session without opening the session material first. const ( - specAnnotationSession = "chainloop.spec.session_id" - specAnnotationKind = "chainloop.spec.kind" - specAnnotationURI = "chainloop.spec.uri" + specAnnotationSession = "chainloop.spec.session_id" + specAnnotationKind = "chainloop.spec.kind" + specAnnotationURI = "chainloop.spec.uri" + specAnnotationRole = "chainloop.spec.role" + specAnnotationTitle = "chainloop.spec.title" + specAnnotationDescription = "chainloop.spec.description" ) // specMaterialKind is the material type each captured spec is stored as. @@ -81,7 +84,7 @@ func attachSpecs(ctx context.Context, adder specMaterialAdder, redactor *specRed for i, c := range captures { name := names.AllocateNamed(specMaterialName(sessionID, c.FileName)) - entry, err := storeCapture(ctx, adder, redactor, filepath.Join(tmpDir, strconv.Itoa(i)), name, sessionID, c) + entry, err := storeCapture(ctx, adder, redactor, filepath.Join(tmpDir, strconv.Itoa(i)), name, sessionID, c, log) if err != nil { // The error stays in the local log. The warning goes into the // uploaded evidence, and error text can carry local paths. @@ -101,29 +104,50 @@ func attachSpecs(ctx context.Context, adder specMaterialAdder, redactor *specRed // storeCapture redacts one capture, adds it to the attestation, and returns // the reference the session material records for it: everything but the text, // which the digest points at. -func storeCapture(ctx context.Context, adder specMaterialAdder, redactor *specRedactor, dir, name, sessionID string, c spec.Capture) (aicodingsession.SpecEntry, error) { +func storeCapture(ctx context.Context, adder specMaterialAdder, redactor *specRedactor, dir, name, sessionID string, c spec.Capture, log zerolog.Logger) (aicodingsession.SpecEntry, error) { redacted, err := redactCapture(ctx, redactor, c) if err != nil { return aicodingsession.SpecEntry{}, err } + if redacted.MetaRaw != nil { + redacted.Meta = redactMeta(ctx, redactor, redacted.MetaRaw, log) + } + digest, err := addSpecMaterial(ctx, adder, dir, name, sessionID, redacted) if err != nil { return aicodingsession.SpecEntry{}, err } return aicodingsession.SpecEntry{ - Kind: redacted.Kind, - URI: redacted.URI, - Digest: digest, - CapturedAt: redacted.CapturedAt, + Kind: redacted.Kind, + Role: redacted.Role, + Title: redacted.Title, + Description: redacted.Description, + URI: redacted.URI, + Digest: digest, + CapturedAt: redacted.CapturedAt, }, nil } +// redactMeta redacts the companion file of a binary file and reads its values. +// A companion file that cannot be scanned gives no values: they are what could +// not be scanned, and the binary file itself is stored as it is anyway. +func redactMeta(ctx context.Context, redactor *specRedactor, raw []byte, log zerolog.Logger) spec.Meta { + doc, err := redactor.Redact(ctx, raw) + if err != nil { + log.Warn().Err(err).Msg("could not scan a spec companion file; its values are not recorded") + return spec.Meta{} + } + + return spec.ParseMeta(doc) +} + // redactCapture redacts the text file a capture was read from. The redacted file // is what gets stored, header included, so it is the file on disk with only -// its secrets taken out. It is parsed again for the source address that the -// annotation and the reference carry, so that address is redacted too. +// its secrets taken out. It is parsed again for the source address, the title +// and the description that the annotations and the reference carry, so they +// are redacted too. // // Redaction fails closed, as it does for the session material: a file that // could not be scanned is not uploaded at all. @@ -147,6 +171,7 @@ func redactCapture(ctx context.Context, redactor *specRedactor, c spec.Capture) } c.URI = parsed.URI + c.Meta = parsed.Meta c.Raw = doc return c, nil @@ -171,8 +196,15 @@ func addSpecMaterial(ctx context.Context, adder specMaterialAdder, dir, name, se specAnnotationSession: sessionID, specAnnotationKind: c.Kind, } - if c.URI != "" { - annotations[specAnnotationURI] = c.URI + for key, value := range map[string]string{ + specAnnotationURI: c.URI, + specAnnotationRole: c.Role, + specAnnotationTitle: c.Title, + specAnnotationDescription: c.Description, + } { + if value != "" { + annotations[key] = value + } } return adder.AddMaterial(ctx, name, path, specMaterialKind, annotations) diff --git a/app/cli/pkg/action/trace_spec_materials_test.go b/app/cli/pkg/action/trace_spec_materials_test.go index 29083093d..cc6030ec0 100644 --- a/app/cli/pkg/action/trace_spec_materials_test.go +++ b/app/cli/pkg/action/trace_spec_materials_test.go @@ -172,6 +172,95 @@ func TestAttachSpecs(t *testing.T) { assert.Equal(t, aicodingsession.SpecKindImage, entries[0].Kind) }) + t.Run("a role, a title and a description go into the annotations and the reference", func(t *testing.T) { + adder := &fakeMaterialAdder{} + described := specCapture(t, "ticket.md", + "---\nkind: ticket\nrole: task\ntitle: \"ENG-1234: Add an export button\"\ndescription: The ticket that the session implements.\n---\nthe ticket", + "2026-09-16T10:12:03Z") + + entries, warnings, _ := attachSpecs(context.Background(), adder, newSpecRedactor(t.TempDir()), materials.NewNameAllocator(nil), sessionID, []spec.Capture{described, plan}, zerolog.Nop()) + + assert.Empty(t, warnings) + require.Len(t, adder.added, 2) + assert.Equal(t, map[string]string{ + "chainloop.spec.session_id": sessionID, + "chainloop.spec.kind": aicodingsession.SpecKindTicket, + "chainloop.spec.role": aicodingsession.SpecRoleTask, + "chainloop.spec.title": "ENG-1234: Add an export button", + "chainloop.spec.description": "The ticket that the session implements.", + }, adder.added[0].annotations) + + require.Len(t, entries, 2) + assert.Equal(t, aicodingsession.SpecRoleTask, entries[0].Role) + assert.Equal(t, "ENG-1234: Add an export button", entries[0].Title) + assert.Equal(t, "The ticket that the session implements.", entries[0].Description) + + // Nothing stated, nothing recorded. + for _, key := range []string{specAnnotationRole, specAnnotationTitle, specAnnotationDescription} { + assert.NotContains(t, adder.added[1].annotations, key) + } + assert.Empty(t, entries[1].Role) + assert.Empty(t, entries[1].Title) + assert.Empty(t, entries[1].Description) + }) + + t.Run("secrets are removed from the title and the description", func(t *testing.T) { + adder := &fakeMaterialAdder{} + withSecret := specCapture(t, "ticket.md", + "---\nkind: ticket\ntitle: token "+pat+" fails\ndescription: uses "+pat+"\n---\nthe ticket", + "2026-09-16T10:12:03Z") + + entries, _, _ := attachSpecs(context.Background(), adder, newSpecRedactor(t.TempDir()), materials.NewNameAllocator(nil), sessionID, []spec.Capture{withSecret}, zerolog.Nop()) + + require.Len(t, entries, 1) + assert.Equal(t, "token [REDACTED:github-pat] fails", entries[0].Title) + assert.Equal(t, "uses [REDACTED:github-pat]", entries[0].Description) + assert.Equal(t, entries[0].Title, adder.added[0].annotations[specAnnotationTitle]) + assert.Equal(t, entries[0].Description, adder.added[0].annotations[specAnnotationDescription]) + }) + + t.Run("a binary file takes the redacted values of its companion file", func(t *testing.T) { + adder := &fakeMaterialAdder{} + pngBytes := []byte("\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\xff\xfe") + image := spec.Capture{ + FileName: "mockup.png", Kind: aicodingsession.SpecKindImage, + CapturedAt: "2026-09-16T10:31:40Z", Raw: pngBytes, Verbatim: true, + MetaRaw: []byte("role: reference\ntitle: mockup for " + pat + "\ndescription: Where the button goes.\n"), + } + + entries, warnings, _ := attachSpecs(context.Background(), adder, newSpecRedactor(t.TempDir()), materials.NewNameAllocator(nil), sessionID, []spec.Capture{image}, zerolog.Nop()) + + assert.Empty(t, warnings) + require.Len(t, adder.added, 1, "the companion file is no material of its own") + assert.Equal(t, string(pngBytes), adder.added[0].content, "the binary file is still stored as it is") + require.Len(t, entries, 1) + assert.Equal(t, aicodingsession.SpecRoleReference, entries[0].Role) + assert.Equal(t, "mockup for [REDACTED:github-pat]", entries[0].Title) + assert.Equal(t, "Where the button goes.", entries[0].Description) + assert.Equal(t, aicodingsession.SpecRoleReference, adder.added[0].annotations[specAnnotationRole]) + assert.Equal(t, entries[0].Title, adder.added[0].annotations[specAnnotationTitle]) + }) + + t.Run("a companion file that cannot be scanned costs its values, not the binary file", func(t *testing.T) { + adder := &fakeMaterialAdder{} + redactor := &specRedactor{dir: t.TempDir(), redact: func(context.Context, []byte) ([]byte, error) { + return nil, errors.New("scanner unavailable") + }} + image := spec.Capture{ + FileName: "mockup.png", Kind: aicodingsession.SpecKindImage, + CapturedAt: "2026-09-16T10:31:40Z", Raw: []byte("\x89PNG\r\n\x1a\n"), Verbatim: true, + MetaRaw: []byte("role: reference\ntitle: Export button mockup\n"), + } + + entries, warnings, _ := attachSpecs(context.Background(), adder, redactor, materials.NewNameAllocator(nil), sessionID, []spec.Capture{image}, zerolog.Nop()) + + assert.Empty(t, warnings) + require.Len(t, entries, 1) + assert.Empty(t, entries[0].Role) + assert.Empty(t, entries[0].Title) + assert.NotContains(t, adder.added[0].annotations, specAnnotationTitle) + }) + t.Run("a failed add drops that entry only, and says so", func(t *testing.T) { adder := &fakeMaterialAdder{failOn: map[string]bool{"spec-7412a0-ticket-pfm-7289": true}} diff --git a/app/cli/pkg/action/trace_spec_test.go b/app/cli/pkg/action/trace_spec_test.go index 61f573674..fea09d33b 100644 --- a/app/cli/pkg/action/trace_spec_test.go +++ b/app/cli/pkg/action/trace_spec_test.go @@ -57,6 +57,15 @@ func TestSpecCaptureInstruction(t *testing.T) { {name: "a document kind", want: "document", why: whyVocabulary}, {name: "an image kind", want: "image", why: whyVocabulary}, {name: "a text kind", want: "text", why: whyVocabulary}, + {name: "the role key", want: "role:", why: "the parser reads it"}, + {name: "the title key", want: "title:", why: "the parser reads it"}, + {name: "the description key", want: "description:", why: "the parser reads it"}, + {name: "a task role", want: "task:", why: whyVocabulary}, + {name: "a spec role", want: "spec:", why: whyVocabulary}, + {name: "a plan role", want: "plan:", why: whyVocabulary}, + {name: "a reference role", want: "reference:", why: whyVocabulary}, + {name: "the main purpose", want: "pick the role that tells why it is in this session", why: "a ticket can also hold the full spec, and the agent must pick one role"}, + {name: "the companion file", want: ".meta.yaml", why: "a binary file has no header, so its values go into a file next to it"}, {name: "the text, not a link", want: "Not a link to it, not your summary of it", why: "a link is worthless as evidence once the source moves"}, {name: "one file per source", want: "One file per source", why: "several sources become several entries, not one blob"}, {name: "an image file is copied", want: "copy the file itself into the folder", why: "an image the agent can reach as a file is stored as the image, not as a description"}, diff --git a/internal/schemavalidators/internal_schemas/aicodingsession/ai-coding-session-0.1.schema.json b/internal/schemavalidators/internal_schemas/aicodingsession/ai-coding-session-0.1.schema.json index b93f2bb84..70654051e 100644 --- a/internal/schemavalidators/internal_schemas/aicodingsession/ai-coding-session-0.1.schema.json +++ b/internal/schemavalidators/internal_schemas/aicodingsession/ai-coding-session-0.1.schema.json @@ -82,6 +82,18 @@ "type": "string", "description": "What this entry was resolved from: 'ticket', 'document', 'image', or 'text' for a spec stated in the session itself. Left unconstrained so further kinds need no schema version bump; a producer normalises anything it does not recognise to 'text'." }, + "role": { + "type": "string", + "description": "The purpose of this source, as the agent stated it: 'task', 'spec', 'plan', or 'reference'. Absent when the agent stated no role. Left unconstrained so further roles need no schema version bump." + }, + "title": { + "type": "string", + "description": "A short name for this source, as the agent wrote it" + }, + "description": { + "type": "string", + "description": "What this source holds, or why it is in the session, in one or two sentences, as the agent wrote it" + }, "uri": { "type": "string", "description": "Where the text came from, absent when the task was stated in the session itself rather than resolved from an external source" diff --git a/internal/schemavalidators/schemavalidators_test.go b/internal/schemavalidators/schemavalidators_test.go index 159e04de6..cca1936a7 100644 --- a/internal/schemavalidators/schemavalidators_test.go +++ b/internal/schemavalidators/schemavalidators_test.go @@ -350,6 +350,7 @@ func TestValidateAICodingSessionSpec(t *testing.T) { keyKind = "kind" keyDigest = "digest" keyURI = "uri" + keyRole = "role" digest = "sha256:3f786850e387550fdab836ed7e6dc881de23001b4a7b8f6d7e1a3d5c9b2e4f10" ) @@ -397,6 +398,11 @@ func TestValidateAICodingSessionSpec(t *testing.T) { // Kinds are open for the same reason modes are: one a newer CLI emits // must not be rejected by a control plane that predates it. {name: "a kind this schema version predates", spec: []any{entry(map[string]any{keyKind: "design"})}}, + {name: "an entry with a role, a title and a description", spec: []any{ + entry(map[string]any{keyKind: "ticket", keyRole: "task", "title": "PFM-7289: Capture the spec", "description": "The ticket that the session implements."}), + }}, + // Roles are open for the same reason kinds are. + {name: "a role this schema version predates", spec: []any{entry(map[string]any{keyRole: "rationale"})}}, } for _, tc := range accepted { @@ -420,6 +426,9 @@ func TestValidateAICodingSessionSpec(t *testing.T) { {name: "an entry with no kind", spec: []any{entry(map[string]any{keyKind: nil})}}, {name: "an entry with no captured_at", spec: []any{entry(map[string]any{"captured_at": nil})}}, {name: "a non-string kind", spec: []any{entry(map[string]any{keyKind: 3})}}, + {name: "a non-string role", spec: []any{entry(map[string]any{keyRole: 3})}}, + {name: "a non-string title", spec: []any{entry(map[string]any{"title": []any{"a"}})}}, + {name: "a non-string description", spec: []any{entry(map[string]any{"description": true})}}, // The stored file is the whole file, so nothing is ever cut. {name: "a truncated flag", spec: []any{entry(map[string]any{"truncated": true})}}, {name: "an unknown sibling within an entry", spec: []any{entry(map[string]any{"source": "linear"})}}, diff --git a/pkg/attestation/crafter/materials/aicodingsession/aicodingsession.go b/pkg/attestation/crafter/materials/aicodingsession/aicodingsession.go index ab233206e..58ca4ae9d 100644 --- a/pkg/attestation/crafter/materials/aicodingsession/aicodingsession.go +++ b/pkg/attestation/crafter/materials/aicodingsession/aicodingsession.go @@ -85,6 +85,32 @@ func ResolveSpecKind(kind string) string { } } +// The roles a spec source can have. The kind is the format of a source and the +// role is its purpose: a ticket can state the task, or be only background. +const ( + // SpecRoleTask is the item that states the work to do. + SpecRoleTask = "task" + // SpecRoleSpec is a document that defines what to build. + SpecRoleSpec = "spec" + // SpecRolePlan is a plan for the work that the user approved. + SpecRolePlan = "plan" + // SpecRoleReference is supporting material: a screenshot, a mockup, an + // example, or a background document. + SpecRoleReference = "reference" +) + +// ResolveSpecRole maps a captured role onto the vocabulary above. Unlike the +// kind, a role is never guessed: a value outside the vocabulary gives no role, +// so the evidence holds only what the agent stated. +func ResolveSpecRole(role string) string { + switch r := strings.ToLower(strings.TrimSpace(role)); r { + case SpecRoleTask, SpecRoleSpec, SpecRolePlan, SpecRoleReference: + return r + default: + return "" + } +} + // SpecEntry is one source a coding session was built from: the ticket, // document or prompt that set the task, resolved by the agent. It is what the // work gets judged against, which no amount of diff can answer on its own. @@ -95,6 +121,14 @@ func ResolveSpecKind(kind string) string { type SpecEntry struct { // Kind is one of the SpecKind* constants. Kind string `json:"kind"` + // Role is one of the SpecRole* constants. Empty when the agent stated no + // role, or one outside the vocabulary. + Role string `json:"role,omitempty"` + // Title is a short name for the source, as the agent wrote it. + Title string `json:"title,omitempty"` + // Description says in one or two sentences what the source holds, as the + // agent wrote it. + Description string `json:"description,omitempty"` // URI is where the text came from. Empty when the task was stated in the // session itself and there is no external source to point at. URI string `json:"uri,omitempty"` diff --git a/pkg/attestation/crafter/materials/aicodingsession/redact.go b/pkg/attestation/crafter/materials/aicodingsession/redact.go index 3a4620f2b..7f595a473 100644 --- a/pkg/attestation/crafter/materials/aicodingsession/redact.go +++ b/pkg/attestation/crafter/materials/aicodingsession/redact.go @@ -50,6 +50,7 @@ var protectedPaths = []string{ "/data/session/started_at", "/data/session/ended_at", "/data/spec/*/kind", + "/data/spec/*/role", "/data/spec/*/captured_at", "/data/spec/*/digest", "/data/git_context/branch", diff --git a/pkg/attestation/crafter/materials/aicodingsession/redact_test.go b/pkg/attestation/crafter/materials/aicodingsession/redact_test.go index 2863b207f..1620c39da 100644 --- a/pkg/attestation/crafter/materials/aicodingsession/redact_test.go +++ b/pkg/attestation/crafter/materials/aicodingsession/redact_test.go @@ -108,6 +108,10 @@ func TestEligible(t *testing.T) { {"/data/subagents/0/id", false}, {"/data/subagents/0/type", false}, {"/data/spec/0/kind", false}, + {"/data/spec/0/role", false}, + // The agent writes them, so they can hold a secret like the text. + {"/data/spec/0/title", true}, + {"/data/spec/0/description", true}, {"/data/spec/0/captured_at", false}, {"/data/spec/3/digest", false}, // A source URL is a common place for an embedded token. diff --git a/pkg/attestation/crafter/materials/aicodingsession/spec_test.go b/pkg/attestation/crafter/materials/aicodingsession/spec_test.go index 306c4abf5..376141a28 100644 --- a/pkg/attestation/crafter/materials/aicodingsession/spec_test.go +++ b/pkg/attestation/crafter/materials/aicodingsession/spec_test.go @@ -50,6 +50,30 @@ func TestResolveSpecKind(t *testing.T) { } } +func TestResolveSpecRole(t *testing.T) { + testCases := []struct { + name string + role string + want string + }{ + {name: "a task is kept", role: SpecRoleTask, want: SpecRoleTask}, + {name: "a spec is kept", role: SpecRoleSpec, want: SpecRoleSpec}, + {name: "a plan is kept", role: SpecRolePlan, want: SpecRolePlan}, + {name: "a reference is kept", role: SpecRoleReference, want: SpecRoleReference}, + {name: "case and padding are normalised", role: " Plan ", want: SpecRolePlan}, + // Unlike the kind, a role is never guessed: no role is a valid state, + // and a consumer applies its own rule to it. + {name: "an unset role is no role", role: "", want: ""}, + {name: "a role outside the vocabulary is no role", role: "design", want: ""}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, ResolveSpecRole(tc.role)) + }) + } +} + // TestSpecEntryMatchesSchema closes the loop between the struct tags here and // the property names in the schema. Each is tested on its own elsewhere, so a // field renamed on one side and not the other would otherwise only surface as a @@ -60,8 +84,12 @@ func TestSpecEntryMatchesSchema(t *testing.T) { entry SpecEntry }{ { - name: "every field populated", - entry: SpecEntry{Kind: SpecKindTicket, URI: "https://linear.app/chainloop/issue/PFM-7289", Digest: testSpecDigest, CapturedAt: "2026-09-16T10:12:03Z"}, + name: "every field populated", + entry: SpecEntry{ + Kind: SpecKindTicket, Role: SpecRoleTask, Title: "PFM-7289: Capture the spec", + Description: "The ticket that the session implements.", + URI: "https://linear.app/chainloop/issue/PFM-7289", Digest: testSpecDigest, CapturedAt: "2026-09-16T10:12:03Z", + }, }, { // What a spec written in the session itself looks like: no source