Skip to content

Commit e09e3e6

Browse files
committed
feat(cli): record the role, title and description of each spec source
Implements spec 004. The agent can set role, title and description in the header of a spec file, or in a .meta.yaml companion file for a binary file. The CLI records the values in the spec entry of the session material and in chainloop.spec.* annotations on the spec material, after secret redaction. Refs #3531 Assisted-by: Claude Code Signed-off-by: Jose I. Paris <jiparis@chainloop.dev> Chainloop-Trace-Sessions: 902bc2fc-3616-4feb-8ebb-35d423e26806
1 parent a2dc7a4 commit e09e3e6

14 files changed

Lines changed: 440 additions & 17 deletions

File tree

‎app/cli/internal/trace/spec/parse.go‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,31 @@ import (
2828
// delimiter opens and closes a frontmatter block.
2929
const delimiter = "---"
3030

31+
// The limits of a title and a description, in characters. A model sometimes
32+
// writes a paragraph where a label is asked for, and a cut keeps a usable one.
33+
const (
34+
MaxTitleLen = 120
35+
MaxDescriptionLen = 300
36+
)
37+
38+
// Meta holds what the agent states about a source next to its kind: its
39+
// purpose, a short name, and what it holds. A text file carries it in its
40+
// header, and a binary file in a companion file.
41+
type Meta struct {
42+
// Role is one of the aicodingsession.SpecRole* constants, or empty.
43+
Role string `yaml:"role"`
44+
// Title is a short name for the source, or empty.
45+
Title string `yaml:"title"`
46+
// Description says what the source holds, or is empty.
47+
Description string `yaml:"description"`
48+
}
49+
3150
// frontmatter is the header a spec document carries. It is decoded into a typed
3251
// struct rather than a map so an unexpected key cannot reach the evidence.
3352
type frontmatter struct {
3453
Kind string `yaml:"kind"`
3554
URI string `yaml:"uri"`
55+
Meta `yaml:",inline"`
3656
}
3757

3858
// Capture is one spec document as read from the session folder: what the agent
@@ -45,6 +65,9 @@ type Capture struct {
4565
Kind string
4666
// URI is where the text came from, empty for a spec stated in the session.
4767
URI string
68+
// Meta holds the role, the title and the description of the source. For
69+
// a binary file they stay empty until its companion file is redacted.
70+
Meta
4871
// CapturedAt is the file's modification time, RFC3339.
4972
CapturedAt string
5073
// Raw is the file as the agent wrote it, header included. It is what
@@ -54,6 +77,9 @@ type Capture struct {
5477
// image, or any file that is not text. The agent copied it into the
5578
// folder, so it has no header and nothing in it is rewritten.
5679
Verbatim bool
80+
// MetaRaw is the companion file of a verbatim file as the agent wrote it,
81+
// or nil when there is none. It is redacted before ParseMeta reads it.
82+
MetaRaw []byte
5783
}
5884

5985
// 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
86112
// - Frontmatter that is not valid YAML: the body after the block is content,
87113
// again with kind "text" and no URI.
88114
// - A kind outside the vocabulary: normalised to "text".
115+
// - A role outside the vocabulary: no role.
116+
// - A title or a description over its limit: cut to the limit.
89117
//
90118
// It returns nil when nothing is left once the body is trimmed. That is the
91119
// 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 {
110138
return &Capture{
111139
Kind: aicodingsession.ResolveSpecKind(meta.Kind),
112140
URI: strings.TrimSpace(meta.URI),
141+
Meta: meta.normalize(),
113142
CapturedAt: capturedAt.UTC().Format(time.RFC3339),
114143
}
115144
}
116145

146+
// ParseMeta reads the companion file of a binary file. It never fails: a file
147+
// that is not valid YAML gives no values, and costs the binary file nothing.
148+
func ParseMeta(doc []byte) Meta {
149+
var meta Meta
150+
if err := yaml.Unmarshal(doc, &meta); err != nil {
151+
return Meta{}
152+
}
153+
154+
return meta.normalize()
155+
}
156+
157+
// normalize drops a role outside the vocabulary, trims the title and the
158+
// description, and cuts them to their limits.
159+
func (m Meta) normalize() Meta {
160+
return Meta{
161+
Role: aicodingsession.ResolveSpecRole(m.Role),
162+
Title: truncate(strings.TrimSpace(m.Title), MaxTitleLen),
163+
Description: truncate(strings.TrimSpace(m.Description), MaxDescriptionLen),
164+
}
165+
}
166+
167+
// truncate keeps the first limit characters of s, so a cut never splits one.
168+
func truncate(s string, limit int) string {
169+
runes := []rune(s)
170+
if len(runes) <= limit {
171+
return s
172+
}
173+
174+
return strings.TrimSpace(string(runes[:limit]))
175+
}
176+
117177
// split separates a leading frontmatter block from the body. A document with no
118178
// well-formed block is all body, so that a missing or unterminated header
119179
// cannot swallow the text it was supposed to introduce.

‎app/cli/internal/trace/spec/parse_test.go‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ func TestParse(t *testing.T) {
4040
wantKind string
4141
wantURI string
4242
wantContent string
43+
wantRole string
44+
wantTitle string
45+
wantDesc string
4346
}{
4447
{
4548
name: "a full frontmatter block",
@@ -118,6 +121,51 @@ func TestParse(t *testing.T) {
118121
wantKind: aicodingsession.SpecKindImage,
119122
wantContent: "what the mockup shows",
120123
},
124+
{
125+
name: "a role, a title and a description",
126+
doc: "---\nkind: ticket\nrole: task\ntitle: \"ENG-1234: Add an export button\"\ndescription: The ticket that the session implements.\n---\n" + specBody,
127+
wantKind: aicodingsession.SpecKindTicket,
128+
wantRole: aicodingsession.SpecRoleTask,
129+
wantTitle: "ENG-1234: Add an export button",
130+
wantDesc: "The ticket that the session implements.",
131+
wantContent: specBody,
132+
},
133+
{
134+
name: "a role with case and padding",
135+
doc: "---\nrole: \" Reference \"\n---\n" + specBody,
136+
wantKind: aicodingsession.SpecKindText,
137+
wantRole: aicodingsession.SpecRoleReference,
138+
wantContent: specBody,
139+
},
140+
{
141+
// A bad role costs the role only: never the kind, and never a
142+
// role guessed from the kind.
143+
name: "a role outside the vocabulary",
144+
doc: "---\nkind: document\nrole: design\n---\n" + specBody,
145+
wantKind: aicodingsession.SpecKindDocument,
146+
wantContent: specBody,
147+
},
148+
{
149+
name: "a title and a description of only whitespace",
150+
doc: "---\ntitle: \" \"\ndescription: \"\\t\"\n---\n" + specBody,
151+
wantKind: aicodingsession.SpecKindText,
152+
wantContent: specBody,
153+
},
154+
{
155+
// The limit counts characters, so a cut never splits one.
156+
name: "a title over the limit is cut",
157+
doc: "---\ntitle: " + strings.Repeat("é", MaxTitleLen+5) + "\n---\n" + specBody,
158+
wantKind: aicodingsession.SpecKindText,
159+
wantTitle: strings.Repeat("é", MaxTitleLen),
160+
wantContent: specBody,
161+
},
162+
{
163+
name: "a description over the limit is cut",
164+
doc: "---\ndescription: " + strings.Repeat("a", MaxDescriptionLen+1) + "\n---\n" + specBody,
165+
wantKind: aicodingsession.SpecKindText,
166+
wantDesc: strings.Repeat("a", MaxDescriptionLen),
167+
wantContent: specBody,
168+
},
121169
{name: "an empty document", doc: "", wantNil: true},
122170
{name: "frontmatter with no body", doc: "---\nkind: ticket\n---\n", wantNil: true},
123171
{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) {
136184
require.NotNil(t, got)
137185
assert.Equal(t, tc.wantKind, got.Kind)
138186
assert.Equal(t, tc.wantURI, got.URI)
187+
assert.Equal(t, tc.wantRole, got.Role)
188+
assert.Equal(t, tc.wantTitle, got.Title)
189+
assert.Equal(t, tc.wantDesc, got.Description)
139190
// What follows the header is the text: split decides where the
140191
// header ends, which is also what decides the kind and the URI.
141192
_, body := split(tc.doc)
@@ -144,3 +195,28 @@ func TestParse(t *testing.T) {
144195
})
145196
}
146197
}
198+
199+
func TestParseMeta(t *testing.T) {
200+
testCases := []struct {
201+
name string
202+
doc string
203+
want Meta
204+
}{
205+
{
206+
name: "every field",
207+
doc: "role: reference\ntitle: Export button mockup\ndescription: Where the button goes.\n",
208+
want: Meta{Role: aicodingsession.SpecRoleReference, Title: "Export button mockup", Description: "Where the button goes."},
209+
},
210+
{name: "a role outside the vocabulary", doc: "role: mockup\ntitle: x\n", want: Meta{Title: "x"}},
211+
{name: "a title over the limit is cut", doc: "title: " + strings.Repeat("a", MaxTitleLen+1), want: Meta{Title: strings.Repeat("a", MaxTitleLen)}},
212+
// A file we cannot read costs its values, and nothing else.
213+
{name: "not valid YAML", doc: "role: plan\ntitle: [unclosed\n", want: Meta{}},
214+
{name: "an empty file", doc: "", want: Meta{}},
215+
}
216+
217+
for _, tc := range testCases {
218+
t.Run(tc.name, func(t *testing.T) {
219+
assert.Equal(t, tc.want, ParseMeta([]byte(tc.doc)))
220+
})
221+
}
222+
}

‎app/cli/internal/trace/spec/spec.go‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,11 @@ const (
6262
// the oldest new ones, since what the session started from is the last
6363
// thing worth dropping.
6464
MaxEntries = 25
65+
66+
// MetaSuffix names the companion file of a binary file: the name of the
67+
// binary file plus this suffix. It holds the role, the title and the
68+
// description that a binary file has no header for.
69+
MetaSuffix = ".meta.yaml"
6570
)
6671

6772
// Dir returns the directory holding every session's captured specs.
@@ -156,12 +161,21 @@ func ReadAll(repoRoot, sessionID string, recorded []string) ([]Capture, []string
156161

157162
var warnings []string
158163

164+
// A companion file is never a spec of its own. It is read only for the
165+
// binary file it is named for, and ignored when there is none.
166+
companions := make(map[string]string)
167+
159168
candidates := make([]candidate, 0, len(dirEntries))
160169
for _, e := range dirEntries {
161170
if !isCandidate(e) {
162171
continue
163172
}
164173

174+
if base, ok := strings.CutSuffix(e.Name(), MetaSuffix); ok {
175+
companions[base] = filepath.Join(dir, e.Name())
176+
continue
177+
}
178+
165179
info, err := e.Info()
166180
if err != nil {
167181
warnings = append(warnings, notRecorded(e.Name(), err))
@@ -214,7 +228,15 @@ func ReadAll(repoRoot, sessionID string, recorded []string) ([]Capture, []string
214228
// An image, or a file that is not text, is something the agent copied
215229
// in rather than wrote. Parsing it as text would only mangle it.
216230
if image := isImage(c.name, doc); image || !utf8.Valid(doc) {
217-
entries = append(entries, verbatimCapture(c.name, doc, c.modTime, image))
231+
capture := verbatimCapture(c.name, doc, c.modTime, image)
232+
if path, ok := companions[c.name]; ok {
233+
// A companion file we cannot read costs its values only.
234+
if meta, err := os.ReadFile(path); err == nil {
235+
capture.MetaRaw = meta
236+
}
237+
}
238+
239+
entries = append(entries, capture)
218240
continue
219241
}
220242

‎app/cli/internal/trace/spec/spec_test.go‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,43 @@ func TestReadAll(t *testing.T) {
224224
assert.True(t, entries[1].Verbatim)
225225
})
226226

227+
t.Run("a companion file goes with its binary file and is no spec of its own", func(t *testing.T) {
228+
root := t.TempDir()
229+
pngBytes := []byte("\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\xff\xfe")
230+
meta := "role: reference\ntitle: Export button mockup\n"
231+
writeSpec(t, root, sessionID, "mockup.png", string(pngBytes), time.Now())
232+
writeSpec(t, root, sessionID, "mockup.png"+MetaSuffix, meta, time.Now())
233+
234+
entries, warnings, err := ReadAll(root, sessionID, nil)
235+
236+
require.NoError(t, err)
237+
assert.Empty(t, warnings)
238+
require.Len(t, entries, 1)
239+
assert.Equal(t, "mockup.png", entries[0].FileName)
240+
assert.Equal(t, []byte(meta), entries[0].MetaRaw)
241+
// The values are read only after the companion file is redacted.
242+
assert.Empty(t, entries[0].Title)
243+
assert.Empty(t, entries[0].Role)
244+
})
245+
246+
t.Run("a companion file with no binary file next to it is ignored", func(t *testing.T) {
247+
root := t.TempDir()
248+
writeSpec(t, root, sessionID, "ticket.md", "the ticket", time.Now())
249+
// One with no file at all, and one next to a text file, which has a
250+
// header of its own.
251+
writeSpec(t, root, sessionID, "gone.png"+MetaSuffix, "role: reference\n", time.Now())
252+
writeSpec(t, root, sessionID, "ticket.md"+MetaSuffix, "role: task\n", time.Now())
253+
254+
entries, warnings, err := ReadAll(root, sessionID, nil)
255+
256+
require.NoError(t, err)
257+
assert.Empty(t, warnings)
258+
require.Len(t, entries, 1)
259+
assert.Equal(t, "ticket.md", entries[0].FileName)
260+
assert.Nil(t, entries[0].MetaRaw)
261+
assert.Empty(t, entries[0].Role)
262+
})
263+
227264
t.Run("an image in a text format is kept as it is too", func(t *testing.T) {
228265
// An SVG is valid UTF-8, so only its type tells it apart from a spec
229266
// the agent wrote. Parsing and redacting it as text could break it.

‎app/cli/pkg/action/trace_spec.go‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,13 +88,23 @@ Resolve whatever the task points at — a ticket, a design document, an image fi
8888
---
8989
kind: ticket
9090
uri: https://tracker.example.com/issue/ENG-1234
91+
role: task
92+
title: "ENG-1234: Add an export button"
93+
description: The ticket that this session implements.
9194
---
9295
9396
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.
9497
98+
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:
99+
- task: the item that states the work to do.
100+
- spec: a document that defines what to build.
101+
- plan: a plan that the user approved.
102+
- reference: supporting material, such as a screenshot or a background document.
103+
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.
104+
95105
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.
96106
97-
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.
107+
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.
98108
99109
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.
100110

0 commit comments

Comments
 (0)