From 794a8508f4c239fef83e363772a3c2d738013f02 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Thu, 8 Oct 2026 13:15:56 +0200 Subject: [PATCH 1/2] fix(cli): keep spec header values that contain '#' or ': ' Read spec source headers and .meta.yaml companion files line by line instead of with a YAML decoder. The value of a key is the rest of its line, so an unquoted value that holds ' #', ': ', a leading '[' or backticks is kept whole, and one bad line no longer clears the other keys. Quoted values and block scalars are still decoded as YAML. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: 36229b0e-92ef-46a9-9ee6-323b0dedac7f --- app/cli/internal/trace/spec/parse.go | 149 +++++++++++++++--- app/cli/internal/trace/spec/parse_test.go | 118 ++++++++++++-- .../pkg/action/trace_spec_materials_test.go | 44 ++++++ 3 files changed, 277 insertions(+), 34 deletions(-) diff --git a/app/cli/internal/trace/spec/parse.go b/app/cli/internal/trace/spec/parse.go index b282ec61e..14717f80f 100644 --- a/app/cli/internal/trace/spec/parse.go +++ b/app/cli/internal/trace/spec/parse.go @@ -40,19 +40,19 @@ const ( // 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"` + Role string // Title is a short name for the source, or empty. - Title string `yaml:"title"` + Title string // Description says what the source holds, or is empty. - Description string `yaml:"description"` + Description string } -// 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. +// frontmatter is the header a spec document carries. parseHeader fills only +// these fields, so an unexpected key cannot reach the evidence. type frontmatter struct { - Kind string `yaml:"kind"` - URI string `yaml:"uri"` - Meta `yaml:",inline"` + Kind string + URI string + Meta } // Capture is one spec document as read from the session folder: what the agent @@ -109,8 +109,8 @@ func verbatimCapture(name string, doc []byte, modTime time.Time, image bool) Cap // // - No frontmatter, or an unterminated block: the whole document is content, // with kind "text" and no URI. -// - Frontmatter that is not valid YAML: the body after the block is content, -// again with kind "text" and no URI. +// - A header line that is not a known key: skipped. The other lines keep +// their values. // - 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. @@ -121,15 +121,7 @@ func verbatimCapture(name string, doc []byte, modTime time.Time, image bool) Cap func Parse(doc []byte, capturedAt time.Time) *Capture { header, body := split(string(doc)) - var meta frontmatter - if header != "" { - // A header we cannot read costs the metadata, never the body. The - // decoder can fill some fields before it fails, so a failure clears - // what it filled. - if err := yaml.Unmarshal([]byte(header), &meta); err != nil { - meta = frontmatter{} - } - } + meta := parseHeader(header) if strings.TrimSpace(body) == "" { return nil @@ -143,15 +135,122 @@ func Parse(doc []byte, capturedAt time.Time) *Capture { } } -// 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. +// ParseMeta reads the companion file of a binary file. It never fails: a line +// it cannot read gives no value, 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 parseHeader(string(doc)).normalize() +} + +// parseHeader reads the known keys of a header line by line. It is not a YAML +// decoder: agents write values unquoted, and YAML cuts a value at " #" and +// rejects one that holds ": " or starts with "[". Here the value is the rest of +// the line after the first colon, kept as written. A value in quotes is +// unquoted, and a "|" or ">" block scalar is decoded, as YAML does. A blank +// line, a comment, an unknown key or a line with no colon is skipped. When a +// key repeats, the last value wins. +func parseHeader(header string) frontmatter { + var meta frontmatter + fields := map[string]*string{ + "kind": &meta.Kind, + "uri": &meta.URI, + "role": &meta.Role, + "title": &meta.Title, + "description": &meta.Description, + } + + lines := strings.Split(header, "\n") + for i := 0; i < len(lines); i++ { + line := strings.TrimRight(lines[i], "\r\t ") + // An indented line here has no key above it to continue. + if line == "" || isIndented(line) || line[0] == '#' || isDocumentMarker(line) { + continue + } + + key, value, found := strings.Cut(line, ":") + if !found { + continue + } + + // The key takes its continuation lines even when it is unknown, so + // that they cannot be read as keys of their own. + var continuation []string + for i+1 < len(lines) && continues(lines[i+1:]) { + i++ + continuation = append(continuation, strings.TrimRight(lines[i], "\r")) + } + + field, known := fields[strings.TrimSpace(key)] + if !known { + continue + } + + *field = decodeValue(strings.TrimSpace(value), continuation) + } + + return meta +} + +// decodeValue gives the value of one key from the rest of its line and the +// indented lines that follow it. +func decodeValue(value string, continuation []string) string { + if strings.HasPrefix(value, "|") || strings.HasPrefix(value, ">") { + if decoded, ok := decodeScalar(value + "\n" + strings.Join(continuation, "\n") + "\n"); ok { + return decoded + } + } + + // A quoted value with more text after its closing quote is not a YAML + // string, so it fails to decode and is kept as written. + if strings.HasPrefix(value, `"`) || strings.HasPrefix(value, "'") { + if decoded, ok := decodeScalar(strings.Join(append([]string{value}, continuation...), "\n")); ok { + return decoded + } } - return meta.normalize() + // A plain value that runs on over indented lines is folded into one line. + parts := []string{value} + for _, line := range continuation { + if line = strings.TrimSpace(line); line != "" { + parts = append(parts, line) + } + } + + return strings.TrimSpace(strings.Join(parts, " ")) +} + +// decodeScalar decodes one YAML scalar. It reports false when YAML cannot read +// the scalar as a string, so that the caller keeps the value as written. +func decodeScalar(scalar string) (string, bool) { + var decoded string + if err := yaml.Unmarshal([]byte(scalar), &decoded); err != nil { + return "", false + } + + return decoded, true +} + +// continues reports whether the first of lines belongs to the key above it: an +// indented line, or a blank line that an indented line follows, as in a block +// scalar with more than one paragraph. +func continues(lines []string) bool { + for _, line := range lines { + line = strings.TrimRight(line, "\r\t ") + if line != "" { + return isIndented(line) + } + } + + return false +} + +func isIndented(line string) bool { + return line != "" && (line[0] == ' ' || line[0] == '\t') +} + +// isDocumentMarker reports a YAML document start or end line. A companion file +// can carry one. +func isDocumentMarker(line string) bool { + return line == delimiter || line == "..." } // normalize drops a role outside the vocabulary, trims the title and the diff --git a/app/cli/internal/trace/spec/parse_test.go b/app/cli/internal/trace/spec/parse_test.go index 297f9aea1..49dea83cd 100644 --- a/app/cli/internal/trace/spec/parse_test.go +++ b/app/cli/internal/trace/spec/parse_test.go @@ -80,20 +80,109 @@ func TestParse(t *testing.T) { wantContent: specBody, }, { - // Frontmatter is used precisely because it degrades. A broken - // header costs the metadata, never the spec. - name: "frontmatter that is not valid YAML", + // Frontmatter is used precisely because it degrades. A value + // that is not a kind costs the kind, never the spec. + name: "a kind that is not valid YAML", doc: "---\nkind: [unclosed\n---\n" + specBody, wantKind: aicodingsession.SpecKindText, wantContent: specBody, }, { - // A decoder can fill some fields before it fails on a later - // line. The broken header must cost all of its metadata, not - // leave half of it behind. - name: "a header that breaks after a valid kind", + // A line that YAML rejects must not cost the other lines. + name: "a value that is not valid YAML after a valid kind", doc: "---\nkind: ticket\nuri: [unclosed\n---\n" + specBody, + wantKind: aicodingsession.SpecKindTicket, + wantURI: "[unclosed", + wantContent: specBody, + }, + { + // YAML reads " #" as the start of a comment and cuts the value. + name: "a description that holds a hash", + doc: "---\nkind: document\ndescription: The merged design spec (PR #3544) that defines R-001\n---\n" + specBody, + wantKind: aicodingsession.SpecKindDocument, + wantDesc: "The merged design spec (PR #3544) that defines R-001", + wantContent: specBody, + }, + { + // YAML rejects ": " in a plain value, which used to cost every key. + name: "an unquoted title that holds colons", + doc: "---\nkind: ticket\nuri: https://linear.app/chainloop/issue/PFM-1\nrole: task\ntitle: PFM-1: fix: the thing\ndescription: The ticket for the fix.\n---\n" + specBody, + wantKind: aicodingsession.SpecKindTicket, + wantURI: "https://linear.app/chainloop/issue/PFM-1", + wantRole: aicodingsession.SpecRoleTask, + wantTitle: "PFM-1: fix: the thing", + wantDesc: "The ticket for the fix.", + wantContent: specBody, + }, + { + // YAML reads a leading "[" as the start of a sequence. + name: "an unquoted title that starts with a bracket", + doc: "---\nkind: ticket\nuri: https://linear.app/chainloop/issue/PFM-2\nrole: task\ntitle: [WIP] something\ndescription: Work in progress.\n---\n" + specBody, + wantKind: aicodingsession.SpecKindTicket, + wantURI: "https://linear.app/chainloop/issue/PFM-2", + wantRole: aicodingsession.SpecRoleTask, + wantTitle: "[WIP] something", + wantDesc: "Work in progress.", + wantContent: specBody, + }, + { + name: "a description with backticks around a colon", + doc: "---\nkind: document\ndescription: uses `foo: bar` inline\n---\n" + specBody, + wantKind: aicodingsession.SpecKindDocument, + wantDesc: "uses `foo: bar` inline", + wantContent: specBody, + }, + { + // A header that is already quoted gives the same values as before. + name: "quoted values with escapes", + doc: "---\nkind: \"ticket\"\nuri: 'https://example.com/a'\nrole: \"spec\"\ntitle: 'it''s: quoted'\ndescription: \"line one\\nPR \\\"#1\\\"\"\n---\n" + specBody, + wantKind: aicodingsession.SpecKindTicket, + wantURI: "https://example.com/a", + wantRole: aicodingsession.SpecRoleSpec, + wantTitle: "it's: quoted", + wantDesc: "line one\nPR \"#1\"", + wantContent: specBody, + }, + { + // Text after the closing quote makes it a plain value. + name: "a value that only starts with a quote", + doc: "---\ntitle: \"Export\" button: v2\n---\n" + specBody, wantKind: aicodingsession.SpecKindText, + wantTitle: "\"Export\" button: v2", + wantContent: specBody, + }, + { + name: "comment lines, unknown keys and stray lines", + doc: "---\n# a comment\nkind: ticket\nowner: someone\n\nnot a key\n stray: indented\nrole: task\n---\n" + specBody, + wantKind: aicodingsession.SpecKindTicket, + wantRole: aicodingsession.SpecRoleTask, + wantContent: specBody, + }, + { + // The lines of an unknown key must not be read as known keys. + name: "an unknown key with indented lines", + doc: "---\ntags:\n title: not a title\nkind: ticket\n---\n" + specBody, + wantKind: aicodingsession.SpecKindTicket, + wantContent: specBody, + }, + { + name: "a literal block scalar", + doc: "---\nkind: document\ndescription: |\n first: line\n\n second #2\n---\n" + specBody, + wantKind: aicodingsession.SpecKindDocument, + wantDesc: "first: line\n\nsecond #2", + wantContent: specBody, + }, + { + name: "a plain value over more than one line", + doc: "---\ndescription: The ticket that\n the session implements.\nkind: ticket\n---\n" + specBody, + wantKind: aicodingsession.SpecKindTicket, + wantDesc: "The ticket that the session implements.", + wantContent: specBody, + }, + { + name: "a repeated key keeps the last value", + doc: "---\nkind: text\nkind: ticket\n---\n" + specBody, + wantKind: aicodingsession.SpecKindTicket, wantContent: specBody, }, { @@ -209,8 +298,19 @@ func TestParseMeta(t *testing.T) { }, {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{}}, + // A line that YAML rejects keeps its value, and costs the other lines nothing. + {name: "not valid YAML", doc: "role: plan\ntitle: [unclosed\n", want: Meta{Role: aicodingsession.SpecRolePlan, Title: "[unclosed"}}, + { + name: "unquoted values with colons, a bracket and a hash", + doc: "role: reference\ntitle: PFM-2: [WIP] fix #12\ndescription: The merged design spec (PR #3544) that defines R-001\n", + want: Meta{Role: aicodingsession.SpecRoleReference, Title: "PFM-2: [WIP] fix #12", Description: "The merged design spec (PR #3544) that defines R-001"}, + }, + { + name: "quoted values, a comment and a document marker", + doc: "---\n# the mockup\nrole: \"reference\"\ntitle: 'Export: mockup'\ndescription: \"Where the button goes.\"\n", + want: Meta{Role: aicodingsession.SpecRoleReference, Title: "Export: mockup", Description: "Where the button goes."}, + }, + {name: "keys a companion file does not hold", doc: "kind: ticket\nuri: https://example.com\ntitle: x\n", want: Meta{Title: "x"}}, {name: "an empty file", doc: "", want: Meta{}}, } diff --git a/app/cli/pkg/action/trace_spec_materials_test.go b/app/cli/pkg/action/trace_spec_materials_test.go index cc6030ec0..964651e72 100644 --- a/app/cli/pkg/action/trace_spec_materials_test.go +++ b/app/cli/pkg/action/trace_spec_materials_test.go @@ -204,6 +204,50 @@ func TestAttachSpecs(t *testing.T) { assert.Empty(t, entries[1].Description) }) + t.Run("unquoted values with a hash, colons or a bracket are kept whole", func(t *testing.T) { + adder := &fakeMaterialAdder{} + hash := specCapture(t, "design.md", + "---\nkind: document\nrole: spec\ndescription: The merged design spec (PR #3544) that defines R-001\n---\nthe spec", + "2026-09-16T10:12:03Z") + colons := specCapture(t, "ticket.md", + "---\nkind: ticket\nuri: "+ticketURI+"\nrole: task\ntitle: PFM-1: fix: the thing\ndescription: uses `foo: bar` inline\n---\nthe ticket", + "2026-09-16T10:12:03Z") + 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: PFM-2: [WIP] fix #12\n"), + } + + entries, warnings, _ := attachSpecs(context.Background(), adder, newSpecRedactor(t.TempDir()), materials.NewNameAllocator(nil), sessionID, []spec.Capture{hash, colons, image}, zerolog.Nop()) + + assert.Empty(t, warnings) + require.Len(t, adder.added, 3) + assert.Equal(t, map[string]string{ + "chainloop.spec.session_id": sessionID, + "chainloop.spec.kind": aicodingsession.SpecKindDocument, + "chainloop.spec.role": aicodingsession.SpecRoleSpec, + "chainloop.spec.description": "The merged design spec (PR #3544) that defines R-001", + }, adder.added[0].annotations) + assert.Equal(t, map[string]string{ + "chainloop.spec.session_id": sessionID, + "chainloop.spec.kind": aicodingsession.SpecKindTicket, + "chainloop.spec.uri": ticketURI, + "chainloop.spec.role": aicodingsession.SpecRoleTask, + "chainloop.spec.title": "PFM-1: fix: the thing", + "chainloop.spec.description": "uses `foo: bar` inline", + }, adder.added[1].annotations) + assert.Equal(t, "PFM-2: [WIP] fix #12", adder.added[2].annotations[specAnnotationTitle]) + + require.Len(t, entries, 3) + assert.Equal(t, "The merged design spec (PR #3544) that defines R-001", entries[0].Description) + assert.Equal(t, aicodingsession.SpecKindTicket, entries[1].Kind) + assert.Equal(t, ticketURI, entries[1].URI) + assert.Equal(t, aicodingsession.SpecRoleTask, entries[1].Role) + assert.Equal(t, "PFM-1: fix: the thing", entries[1].Title) + assert.Equal(t, "uses `foo: bar` inline", entries[1].Description) + assert.Equal(t, "PFM-2: [WIP] fix #12", entries[2].Title) + }) + t.Run("secrets are removed from the title and the description", func(t *testing.T) { adder := &fakeMaterialAdder{} withSecret := specCapture(t, "ticket.md", From 074031fb52703c6725651deecbce855edc2175f7 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Thu, 8 Oct 2026 13:23:16 +0200 Subject: [PATCH 2/2] fix(cli): skip comment lines when folding spec header values An indented line that starts with '#' is a YAML comment, so it is no longer folded into a plain header value. Also update the new attachSpecs test to the four return values that main now has. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: 36229b0e-92ef-46a9-9ee6-323b0dedac7f --- app/cli/internal/trace/spec/parse.go | 3 ++- app/cli/internal/trace/spec/parse_test.go | 9 +++++++++ app/cli/pkg/action/trace_spec_materials_test.go | 2 +- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/app/cli/internal/trace/spec/parse.go b/app/cli/internal/trace/spec/parse.go index 14717f80f..d9873cb33 100644 --- a/app/cli/internal/trace/spec/parse.go +++ b/app/cli/internal/trace/spec/parse.go @@ -208,9 +208,10 @@ func decodeValue(value string, continuation []string) string { } // A plain value that runs on over indented lines is folded into one line. + // An indented line that starts with "#" is a YAML comment, not part of it. parts := []string{value} for _, line := range continuation { - if line = strings.TrimSpace(line); line != "" { + if line = strings.TrimSpace(line); line != "" && !strings.HasPrefix(line, "#") { parts = append(parts, line) } } diff --git a/app/cli/internal/trace/spec/parse_test.go b/app/cli/internal/trace/spec/parse_test.go index 49dea83cd..6d2ef44c5 100644 --- a/app/cli/internal/trace/spec/parse_test.go +++ b/app/cli/internal/trace/spec/parse_test.go @@ -179,6 +179,15 @@ func TestParse(t *testing.T) { wantDesc: "The ticket that the session implements.", wantContent: specBody, }, + { + // An indented line that starts with "#" is a YAML comment. + name: "an indented comment line after a plain value", + doc: "---\ntitle: Foo\n # note\ndescription: Bar #1\n continued\n # another note\nkind: ticket\n---\n" + specBody, + wantKind: aicodingsession.SpecKindTicket, + wantTitle: "Foo", + wantDesc: "Bar #1 continued", + wantContent: specBody, + }, { name: "a repeated key keeps the last value", doc: "---\nkind: text\nkind: ticket\n---\n" + specBody, diff --git a/app/cli/pkg/action/trace_spec_materials_test.go b/app/cli/pkg/action/trace_spec_materials_test.go index 01f76ec7a..a912d5d4b 100644 --- a/app/cli/pkg/action/trace_spec_materials_test.go +++ b/app/cli/pkg/action/trace_spec_materials_test.go @@ -248,7 +248,7 @@ func TestAttachSpecs(t *testing.T) { MetaRaw: []byte("role: reference\ntitle: PFM-2: [WIP] fix #12\n"), } - entries, warnings, _ := attachSpecs(context.Background(), adder, newSpecRedactor(t.TempDir()), materials.NewNameAllocator(nil), sessionID, []spec.Capture{hash, colons, image}, zerolog.Nop()) + entries, warnings, _, _ := attachSpecs(context.Background(), adder, newSpecRedactor(t.TempDir()), materials.NewNameAllocator(nil), sessionID, []spec.Capture{hash, colons, image}, zerolog.Nop()) assert.Empty(t, warnings) require.Len(t, adder.added, 3)