From f011e9bc802c4100f0f81c1e51286750a267b889 Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Wed, 17 Dec 2025 14:50:34 -0800 Subject: [PATCH] feat(beads): add MR field parsing for merge-request issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement ParseMRFields and SetMRFields helper functions to extract and update structured fields in merge-request issue descriptions. Fields supported: branch, target, source_issue, worker, rig, merge_commit, close_reason. Features: - Case-insensitive key matching - Alternate key formats (snake_case, kebab-case) - Preserves non-MR prose content - Handles mixed MR fields and prose - Round-trip safe (parse then format preserves data) Closes: gt-h5n.1 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- internal/beads/beads.go | 180 ++++++++++++++ internal/beads/beads_test.go | 443 +++++++++++++++++++++++++++++++++++ 2 files changed, 623 insertions(+) diff --git a/internal/beads/beads.go b/internal/beads/beads.go index 9bcc66b7..cde267f1 100644 --- a/internal/beads/beads.go +++ b/internal/beads/beads.go @@ -362,3 +362,183 @@ func (b *Beads) IsBeadsRepo() bool { _, err := b.run("list", "--limit=1") return err == nil || !errors.Is(err, ErrNotARepo) } + +// MRFields holds the structured fields for a merge-request issue. +// These fields are stored as key: value lines in the issue description. +type MRFields struct { + Branch string // Source branch name (e.g., "polecat/Nux/gt-xyz") + Target string // Target branch (e.g., "main" or "integration/gt-epic") + SourceIssue string // The work item being merged (e.g., "gt-xyz") + Worker string // Who did the work + Rig string // Which rig + MergeCommit string // SHA of merge commit (set on close) + CloseReason string // Reason for closing: merged, rejected, conflict, superseded +} + +// ParseMRFields extracts structured merge-request fields from an issue's description. +// Fields are expected as "key: value" lines, with optional prose text mixed in. +// Returns nil if no MR fields are found. +func ParseMRFields(issue *Issue) *MRFields { + if issue == nil || issue.Description == "" { + return nil + } + + fields := &MRFields{} + hasFields := false + + for _, line := range strings.Split(issue.Description, "\n") { + line = strings.TrimSpace(line) + if line == "" { + continue + } + + // Look for "key: value" pattern + colonIdx := strings.Index(line, ":") + if colonIdx == -1 { + continue + } + + key := strings.TrimSpace(line[:colonIdx]) + value := strings.TrimSpace(line[colonIdx+1:]) + if value == "" { + continue + } + + // Map keys to fields (case-insensitive) + switch strings.ToLower(key) { + case "branch": + fields.Branch = value + hasFields = true + case "target": + fields.Target = value + hasFields = true + case "source_issue", "source-issue", "sourceissue": + fields.SourceIssue = value + hasFields = true + case "worker": + fields.Worker = value + hasFields = true + case "rig": + fields.Rig = value + hasFields = true + case "merge_commit", "merge-commit", "mergecommit": + fields.MergeCommit = value + hasFields = true + case "close_reason", "close-reason", "closereason": + fields.CloseReason = value + hasFields = true + } + } + + if !hasFields { + return nil + } + return fields +} + +// FormatMRFields formats MRFields as a string suitable for an issue description. +// Only non-empty fields are included. +func FormatMRFields(fields *MRFields) string { + if fields == nil { + return "" + } + + var lines []string + + if fields.Branch != "" { + lines = append(lines, "branch: "+fields.Branch) + } + if fields.Target != "" { + lines = append(lines, "target: "+fields.Target) + } + if fields.SourceIssue != "" { + lines = append(lines, "source_issue: "+fields.SourceIssue) + } + if fields.Worker != "" { + lines = append(lines, "worker: "+fields.Worker) + } + if fields.Rig != "" { + lines = append(lines, "rig: "+fields.Rig) + } + if fields.MergeCommit != "" { + lines = append(lines, "merge_commit: "+fields.MergeCommit) + } + if fields.CloseReason != "" { + lines = append(lines, "close_reason: "+fields.CloseReason) + } + + return strings.Join(lines, "\n") +} + +// SetMRFields updates an issue's description with the given MR fields. +// Existing MR field lines are replaced; other content is preserved. +// Returns the new description string. +func SetMRFields(issue *Issue, fields *MRFields) string { + if issue == nil { + return FormatMRFields(fields) + } + + // Known MR field keys (lowercase) + mrKeys := map[string]bool{ + "branch": true, + "target": true, + "source_issue": true, + "source-issue": true, + "sourceissue": true, + "worker": true, + "rig": true, + "merge_commit": true, + "merge-commit": true, + "mergecommit": true, + "close_reason": true, + "close-reason": true, + "closereason": true, + } + + // Collect non-MR lines from existing description + var otherLines []string + if issue.Description != "" { + for _, line := range strings.Split(issue.Description, "\n") { + trimmed := strings.TrimSpace(line) + if trimmed == "" { + // Preserve blank lines in content + otherLines = append(otherLines, line) + continue + } + + // Check if this is an MR field line + colonIdx := strings.Index(trimmed, ":") + if colonIdx == -1 { + otherLines = append(otherLines, line) + continue + } + + key := strings.ToLower(strings.TrimSpace(trimmed[:colonIdx])) + if !mrKeys[key] { + otherLines = append(otherLines, line) + } + // Skip MR field lines - they'll be replaced + } + } + + // Build new description: MR fields first, then other content + formatted := FormatMRFields(fields) + + // Trim trailing blank lines from other content + for len(otherLines) > 0 && strings.TrimSpace(otherLines[len(otherLines)-1]) == "" { + otherLines = otherLines[:len(otherLines)-1] + } + // Trim leading blank lines from other content + for len(otherLines) > 0 && strings.TrimSpace(otherLines[0]) == "" { + otherLines = otherLines[1:] + } + + if formatted == "" { + return strings.Join(otherLines, "\n") + } + if len(otherLines) == 0 { + return formatted + } + + return formatted + "\n\n" + strings.Join(otherLines, "\n") +} diff --git a/internal/beads/beads_test.go b/internal/beads/beads_test.go index 72629fc1..e37fc843 100644 --- a/internal/beads/beads_test.go +++ b/internal/beads/beads_test.go @@ -3,6 +3,7 @@ package beads import ( "os" "path/filepath" + "strings" "testing" ) @@ -183,3 +184,445 @@ func TestIntegration(t *testing.T) { t.Logf("Showed issue: %s - %s", issue.ID, issue.Title) }) } + +// TestParseMRFields tests parsing MR fields from issue descriptions. +func TestParseMRFields(t *testing.T) { + tests := []struct { + name string + issue *Issue + wantNil bool + wantFields *MRFields + }{ + { + name: "nil issue", + issue: nil, + wantNil: true, + }, + { + name: "empty description", + issue: &Issue{Description: ""}, + wantNil: true, + }, + { + name: "no MR fields", + issue: &Issue{Description: "This is just plain text\nwith no field markers"}, + wantNil: true, + }, + { + name: "all fields", + issue: &Issue{ + Description: `branch: polecat/Nux/gt-xyz +target: main +source_issue: gt-xyz +worker: Nux +rig: gastown +merge_commit: abc123def +close_reason: merged`, + }, + wantFields: &MRFields{ + Branch: "polecat/Nux/gt-xyz", + Target: "main", + SourceIssue: "gt-xyz", + Worker: "Nux", + Rig: "gastown", + MergeCommit: "abc123def", + CloseReason: "merged", + }, + }, + { + name: "partial fields", + issue: &Issue{ + Description: `branch: polecat/Toast/gt-abc +target: integration/gt-epic +source_issue: gt-abc +worker: Toast`, + }, + wantFields: &MRFields{ + Branch: "polecat/Toast/gt-abc", + Target: "integration/gt-epic", + SourceIssue: "gt-abc", + Worker: "Toast", + }, + }, + { + name: "mixed with prose", + issue: &Issue{ + Description: `branch: polecat/Capable/gt-def +target: main +source_issue: gt-def + +This MR fixes a critical bug in the authentication system. +Please review carefully. + +worker: Capable +rig: wasteland`, + }, + wantFields: &MRFields{ + Branch: "polecat/Capable/gt-def", + Target: "main", + SourceIssue: "gt-def", + Worker: "Capable", + Rig: "wasteland", + }, + }, + { + name: "alternate key formats", + issue: &Issue{ + Description: `branch: polecat/Max/gt-ghi +source-issue: gt-ghi +merge-commit: 789xyz`, + }, + wantFields: &MRFields{ + Branch: "polecat/Max/gt-ghi", + SourceIssue: "gt-ghi", + MergeCommit: "789xyz", + }, + }, + { + name: "case insensitive keys", + issue: &Issue{ + Description: `Branch: polecat/Furiosa/gt-jkl +TARGET: main +Worker: Furiosa +RIG: gastown`, + }, + wantFields: &MRFields{ + Branch: "polecat/Furiosa/gt-jkl", + Target: "main", + Worker: "Furiosa", + Rig: "gastown", + }, + }, + { + name: "extra whitespace", + issue: &Issue{ + Description: ` branch: polecat/Nux/gt-mno +target:main + worker: Nux `, + }, + wantFields: &MRFields{ + Branch: "polecat/Nux/gt-mno", + Target: "main", + Worker: "Nux", + }, + }, + { + name: "ignores empty values", + issue: &Issue{ + Description: `branch: polecat/Nux/gt-pqr +target: +source_issue: gt-pqr`, + }, + wantFields: &MRFields{ + Branch: "polecat/Nux/gt-pqr", + SourceIssue: "gt-pqr", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fields := ParseMRFields(tt.issue) + + if tt.wantNil { + if fields != nil { + t.Errorf("ParseMRFields() = %+v, want nil", fields) + } + return + } + + if fields == nil { + t.Fatal("ParseMRFields() = nil, want non-nil") + } + + if fields.Branch != tt.wantFields.Branch { + t.Errorf("Branch = %q, want %q", fields.Branch, tt.wantFields.Branch) + } + if fields.Target != tt.wantFields.Target { + t.Errorf("Target = %q, want %q", fields.Target, tt.wantFields.Target) + } + if fields.SourceIssue != tt.wantFields.SourceIssue { + t.Errorf("SourceIssue = %q, want %q", fields.SourceIssue, tt.wantFields.SourceIssue) + } + if fields.Worker != tt.wantFields.Worker { + t.Errorf("Worker = %q, want %q", fields.Worker, tt.wantFields.Worker) + } + if fields.Rig != tt.wantFields.Rig { + t.Errorf("Rig = %q, want %q", fields.Rig, tt.wantFields.Rig) + } + if fields.MergeCommit != tt.wantFields.MergeCommit { + t.Errorf("MergeCommit = %q, want %q", fields.MergeCommit, tt.wantFields.MergeCommit) + } + if fields.CloseReason != tt.wantFields.CloseReason { + t.Errorf("CloseReason = %q, want %q", fields.CloseReason, tt.wantFields.CloseReason) + } + }) + } +} + +// TestFormatMRFields tests formatting MR fields to string. +func TestFormatMRFields(t *testing.T) { + tests := []struct { + name string + fields *MRFields + want string + }{ + { + name: "nil fields", + fields: nil, + want: "", + }, + { + name: "empty fields", + fields: &MRFields{}, + want: "", + }, + { + name: "all fields", + fields: &MRFields{ + Branch: "polecat/Nux/gt-xyz", + Target: "main", + SourceIssue: "gt-xyz", + Worker: "Nux", + Rig: "gastown", + MergeCommit: "abc123def", + CloseReason: "merged", + }, + want: `branch: polecat/Nux/gt-xyz +target: main +source_issue: gt-xyz +worker: Nux +rig: gastown +merge_commit: abc123def +close_reason: merged`, + }, + { + name: "partial fields", + fields: &MRFields{ + Branch: "polecat/Toast/gt-abc", + Target: "main", + SourceIssue: "gt-abc", + Worker: "Toast", + }, + want: `branch: polecat/Toast/gt-abc +target: main +source_issue: gt-abc +worker: Toast`, + }, + { + name: "only close fields", + fields: &MRFields{ + MergeCommit: "deadbeef", + CloseReason: "rejected", + }, + want: `merge_commit: deadbeef +close_reason: rejected`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := FormatMRFields(tt.fields) + if got != tt.want { + t.Errorf("FormatMRFields() =\n%q\nwant\n%q", got, tt.want) + } + }) + } +} + +// TestSetMRFields tests updating issue descriptions with MR fields. +func TestSetMRFields(t *testing.T) { + tests := []struct { + name string + issue *Issue + fields *MRFields + want string + }{ + { + name: "nil issue", + issue: nil, + fields: &MRFields{ + Branch: "polecat/Nux/gt-xyz", + Target: "main", + }, + want: `branch: polecat/Nux/gt-xyz +target: main`, + }, + { + name: "empty description", + issue: &Issue{Description: ""}, + fields: &MRFields{ + Branch: "polecat/Nux/gt-xyz", + Target: "main", + SourceIssue: "gt-xyz", + }, + want: `branch: polecat/Nux/gt-xyz +target: main +source_issue: gt-xyz`, + }, + { + name: "preserve prose content", + issue: &Issue{Description: "This is a description of the work.\n\nIt spans multiple lines."}, + fields: &MRFields{ + Branch: "polecat/Toast/gt-abc", + Worker: "Toast", + }, + want: `branch: polecat/Toast/gt-abc +worker: Toast + +This is a description of the work. + +It spans multiple lines.`, + }, + { + name: "replace existing fields", + issue: &Issue{ + Description: `branch: polecat/Nux/gt-old +target: develop +source_issue: gt-old +worker: Nux + +Some existing prose content.`, + }, + fields: &MRFields{ + Branch: "polecat/Nux/gt-new", + Target: "main", + SourceIssue: "gt-new", + Worker: "Nux", + MergeCommit: "abc123", + }, + want: `branch: polecat/Nux/gt-new +target: main +source_issue: gt-new +worker: Nux +merge_commit: abc123 + +Some existing prose content.`, + }, + { + name: "preserve non-MR key-value lines", + issue: &Issue{ + Description: `branch: polecat/Capable/gt-def +custom_field: some value +author: someone +target: main`, + }, + fields: &MRFields{ + Branch: "polecat/Capable/gt-ghi", + Target: "integration/epic", + CloseReason: "merged", + }, + want: `branch: polecat/Capable/gt-ghi +target: integration/epic +close_reason: merged + +custom_field: some value +author: someone`, + }, + { + name: "empty fields clears MR data", + issue: &Issue{Description: "branch: old\ntarget: old\n\nKeep this text."}, + fields: &MRFields{}, + want: "Keep this text.", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := SetMRFields(tt.issue, tt.fields) + if got != tt.want { + t.Errorf("SetMRFields() =\n%q\nwant\n%q", got, tt.want) + } + }) + } +} + +// TestMRFieldsRoundTrip tests that parse/format round-trips correctly. +func TestMRFieldsRoundTrip(t *testing.T) { + original := &MRFields{ + Branch: "polecat/Nux/gt-xyz", + Target: "main", + SourceIssue: "gt-xyz", + Worker: "Nux", + Rig: "gastown", + MergeCommit: "abc123def789", + CloseReason: "merged", + } + + // Format to string + formatted := FormatMRFields(original) + + // Parse back + issue := &Issue{Description: formatted} + parsed := ParseMRFields(issue) + + if parsed == nil { + t.Fatal("round-trip parse returned nil") + } + + if *parsed != *original { + t.Errorf("round-trip mismatch:\ngot %+v\nwant %+v", parsed, original) + } +} + +// TestParseMRFieldsFromDesignDoc tests the example from the design doc. +func TestParseMRFieldsFromDesignDoc(t *testing.T) { + // Example from docs/merge-queue-design.md + description := `branch: polecat/Nux/gt-xyz +target: main +source_issue: gt-xyz +worker: Nux +rig: gastown` + + issue := &Issue{Description: description} + fields := ParseMRFields(issue) + + if fields == nil { + t.Fatal("ParseMRFields returned nil for design doc example") + } + + // Verify all fields match the design doc + if fields.Branch != "polecat/Nux/gt-xyz" { + t.Errorf("Branch = %q, want polecat/Nux/gt-xyz", fields.Branch) + } + if fields.Target != "main" { + t.Errorf("Target = %q, want main", fields.Target) + } + if fields.SourceIssue != "gt-xyz" { + t.Errorf("SourceIssue = %q, want gt-xyz", fields.SourceIssue) + } + if fields.Worker != "Nux" { + t.Errorf("Worker = %q, want Nux", fields.Worker) + } + if fields.Rig != "gastown" { + t.Errorf("Rig = %q, want gastown", fields.Rig) + } +} + +// TestSetMRFieldsPreservesURL tests that URLs in prose are preserved. +func TestSetMRFieldsPreservesURL(t *testing.T) { + // URLs contain colons which could be confused with key: value + issue := &Issue{ + Description: `branch: old-branch +Check out https://example.com/path for more info. +Also see http://localhost:8080/api`, + } + + fields := &MRFields{ + Branch: "new-branch", + Target: "main", + } + + result := SetMRFields(issue, fields) + + // URLs should be preserved + if !strings.Contains(result, "https://example.com/path") { + t.Error("HTTPS URL was not preserved") + } + if !strings.Contains(result, "http://localhost:8080/api") { + t.Error("HTTP URL was not preserved") + } + if !strings.Contains(result, "branch: new-branch") { + t.Error("branch field was not set") + } +}