diff --git a/internal/api/jira.go b/internal/api/jira.go index c47d61e..68fe956 100644 --- a/internal/api/jira.go +++ b/internal/api/jira.go @@ -996,6 +996,7 @@ type Field struct { // FieldSchema describes the type of a field. type FieldSchema struct { Type string `json:"type"` + Items string `json:"items,omitempty"` System string `json:"system,omitempty"` Custom string `json:"custom,omitempty"` CustomID int `json:"customId,omitempty"` diff --git a/internal/api/markdown.go b/internal/api/markdown.go index a865997..c707a01 100644 --- a/internal/api/markdown.go +++ b/internal/api/markdown.go @@ -55,8 +55,12 @@ func parseBlocks(lines []string) []ADFContent { continue } - // Fenced code block - if strings.HasPrefix(line, "```") { + // Fenced code block (trim to also catch indented fences inside lists + // or other contexts — without trimming, " ```" slipped past every + // dispatch branch and fell into parseParagraph, which broke on the + // trimmed "```" prefix and returned consumed=0, causing an infinite + // loop in this dispatch loop.) + if strings.HasPrefix(strings.TrimSpace(line), "```") { block, consumed := parseCodeBlock(lines, i) content = append(content, block) i += consumed @@ -412,6 +416,10 @@ func parseOrderedList(lines []string, start int) (ADFContent, int) { } // parseParagraph parses a paragraph (consecutive non-empty lines). +// +// Always consumes at least one line. Returning consumed=0 is forbidden because +// the parseBlocks dispatch loop relies on monotonic progress; a 0-consume +// return would spin forever. func parseParagraph(lines []string, start int) (ADFContent, int) { var paraLines []string i := start @@ -425,13 +433,18 @@ func parseParagraph(lines []string, start int) (ADFContent, int) { break } - // Special block elements end the paragraph - if strings.HasPrefix(trimmed, "#") || + // Special block elements end the paragraph — but only if we have + // already collected at least one paragraph line. Breaking on the + // first line would return consumed=0 and hang parseBlocks. The + // dispatcher in parseBlocks is responsible for routing block-shaped + // lines elsewhere; if one slips through to us, treat it as literal + // paragraph text. + if i > start && (strings.HasPrefix(trimmed, "#") || strings.HasPrefix(trimmed, "```") || strings.HasPrefix(trimmed, ">") || isBulletListItem(line) || isOrderedListItem(line) || - isHorizontalRule(line) { + isHorizontalRule(line)) { break } diff --git a/internal/api/markdown_indented_test.go b/internal/api/markdown_indented_test.go new file mode 100644 index 0000000..8d4e77a --- /dev/null +++ b/internal/api/markdown_indented_test.go @@ -0,0 +1,85 @@ +package api + +import ( + "testing" + "time" +) + +// TestMarkdownToADF_IndentedCodeFenceInList reproduces a hang where an indented +// fenced code block inside an ordered/bulleted list caused parseParagraph to +// return consumed=0, leading to an infinite loop in parseBlocks. +// +// The fix has two parts: +// 1. parseBlocks must trim before checking for "```" so indented fences are +// parsed as code blocks (not as paragraphs). +// 2. parseParagraph must guarantee consumed >= 1 as a defensive backstop. +func TestMarkdownToADF_IndentedCodeFenceInList(t *testing.T) { + input := "1. Verify the token works:\n" + + " ```\n" + + " curl https://api.example.com/v1/me\n" + + " ```\n" + + "2. Next step.\n" + + done := make(chan *ADF, 1) + go func() { + done <- MarkdownToADF(input) + }() + + select { + case adf := <-done: + if adf == nil || adf.Type != "doc" { + t.Fatalf("expected doc ADF, got %+v", adf) + } + case <-time.After(2 * time.Second): + t.Fatal("MarkdownToADF hung on indented code fence inside ordered list") + } +} + +// TestMarkdownToADF_IndentedCodeFenceTopLevel verifies that an indented code +// fence at the top level (4-space-indented ```) does not hang. Even if the +// dispatcher does not recognize it as a code block, parseParagraph must +// consume at least one line. +func TestMarkdownToADF_IndentedCodeFenceTopLevel(t *testing.T) { + input := "Header\n\n ```\n code\n ```\n\nFooter\n" + + done := make(chan *ADF, 1) + go func() { + done <- MarkdownToADF(input) + }() + + select { + case adf := <-done: + if adf == nil || adf.Type != "doc" { + t.Fatalf("expected doc ADF, got %+v", adf) + } + case <-time.After(2 * time.Second): + t.Fatal("MarkdownToADF hung on indented code fence") + } +} + +// TestParseParagraph_NeverReturnsZero is a defensive guarantee that +// parseParagraph always consumes at least one line. Otherwise the parseBlocks +// loop can spin forever. +func TestParseParagraph_NeverReturnsZero(t *testing.T) { + tests := []struct { + name string + input string + }{ + {"indented backticks", " ```"}, + {"indented heading marker", " # not a heading"}, + {"indented quote", " > not a quote"}, + {"indented bullet", " - not a bullet"}, + {"indented ordered", " 1. not a list"}, + {"indented hr", " ---"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + lines := []string{tt.input} + _, consumed := parseParagraph(lines, 0) + if consumed < 1 { + t.Errorf("parseParagraph returned consumed=%d for %q (must be >= 1 to prevent infinite loops)", consumed, tt.input) + } + }) + } +} diff --git a/internal/cmd/issue/field_util.go b/internal/cmd/issue/field_util.go index 8e4fcd4..c35a033 100644 --- a/internal/cmd/issue/field_util.go +++ b/internal/cmd/issue/field_util.go @@ -82,11 +82,20 @@ func coerceFieldValue(field *api.Field, value string) interface{} { value = strings.ReplaceAll(strings.ReplaceAll(strings.ReplaceAll(value, `\\`, "\x00"), `\n`, "\n"), "\x00", `\`) return api.TextToADF(value) } - if field.Schema.Type == "array" && field.Schema.Custom == "" { - // Labels-type array of strings. - vals := strings.Split(value, ",") - for i := range vals { - vals[i] = strings.TrimSpace(vals[i]) + // Labels-type fields: both the standard system "labels" field + // (Custom == "") and custom label fields (Custom contains "labels", + // e.g. "...customfieldtypes:labels"). Detected via Schema.Type=="array" + // with string items, or by the "labels" marker in Schema.Custom. + isLabelsCustom := strings.Contains(customType, "labels") + isStringArray := field.Schema.Type == "array" && field.Schema.Items == "string" + isUntypedArray := field.Schema.Type == "array" && field.Schema.Custom == "" + if isLabelsCustom || isStringArray || isUntypedArray { + raw := strings.Split(value, ",") + vals := make([]string, 0, len(raw)) + for _, v := range raw { + if trimmed := strings.TrimSpace(v); trimmed != "" { + vals = append(vals, trimmed) + } } return vals } diff --git a/internal/cmd/issue/field_util_test.go b/internal/cmd/issue/field_util_test.go new file mode 100644 index 0000000..5b7a3a2 --- /dev/null +++ b/internal/cmd/issue/field_util_test.go @@ -0,0 +1,128 @@ +package issue + +import ( + "reflect" + "testing" + + "github.com/enthus-appdev/atl-cli/internal/api" +) + +// TestCoerceFieldValue_LabelsCustomField reproduces a bug where label-typed +// custom fields (e.g. NX `Repo`, `Application`) were rejected by Jira because +// `--field "Repo=API"` sent the bare string "API" instead of the required +// `["API"]` array. The schema for label custom fields has: +// +// type = "array" +// items = "string" +// custom = "com.atlassian.jira.plugin.system.customfieldtypes:labels" +// +// The previous code only handled labels when Schema.Custom was empty, missing +// the custom-label case entirely. +func TestCoerceFieldValue_LabelsCustomField(t *testing.T) { + field := &api.Field{ + ID: "customfield_10410", + Name: "Repo", + Schema: &api.FieldSchema{ + Type: "array", + Items: "string", + Custom: "com.atlassian.jira.plugin.system.customfieldtypes:labels", + }, + } + + tests := []struct { + name string + value string + want []string + }{ + {"single label", "API", []string{"API"}}, + {"multiple labels comma-separated", "API,GUI,Portal", []string{"API", "GUI", "Portal"}}, + {"with spaces around commas", "API, GUI ,Portal", []string{"API", "GUI", "Portal"}}, + {"double commas dropped", "API,,GUI", []string{"API", "GUI"}}, + {"trailing comma dropped", "API,GUI,", []string{"API", "GUI"}}, + {"leading comma dropped", ",API,GUI", []string{"API", "GUI"}}, + {"whitespace-only entry dropped", "API, ,GUI", []string{"API", "GUI"}}, + {"all empty returns empty slice", ",,,", []string{}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := coerceFieldValue(field, tt.value) + gotSlice, ok := got.([]string) + if !ok { + t.Fatalf("expected []string, got %T (%v)", got, got) + } + if !reflect.DeepEqual(gotSlice, tt.want) { + t.Errorf("coerceFieldValue(%q) = %v, want %v", tt.value, gotSlice, tt.want) + } + }) + } +} + +// TestCoerceFieldValue_StandardLabelsField verifies the existing behavior for +// the standard system "labels" field (Schema.Custom == "") is preserved. +func TestCoerceFieldValue_StandardLabelsField(t *testing.T) { + field := &api.Field{ + ID: "labels", + Name: "Labels", + Schema: &api.FieldSchema{ + Type: "array", + Items: "string", + Custom: "", + }, + } + + got := coerceFieldValue(field, "bug,urgent") + want := []string{"bug", "urgent"} + gotSlice, ok := got.([]string) + if !ok { + t.Fatalf("expected []string, got %T", got) + } + if !reflect.DeepEqual(gotSlice, want) { + t.Errorf("got %v, want %v", gotSlice, want) + } +} + +// TestCoerceFieldValue_SelectStillWorks verifies select fields are not +// regressed by the new label detection. +func TestCoerceFieldValue_SelectStillWorks(t *testing.T) { + field := &api.Field{ + ID: "customfield_10412", + Name: "Ursprung des Fehlverhaltens", + Schema: &api.FieldSchema{ + Type: "option", + Custom: "com.atlassian.jira.plugin.system.customfieldtypes:select", + }, + } + + got := coerceFieldValue(field, "aktuell") + want := map[string]string{"value": "aktuell"} + gotMap, ok := got.(map[string]string) + if !ok { + t.Fatalf("expected map[string]string, got %T", got) + } + if !reflect.DeepEqual(gotMap, want) { + t.Errorf("got %v, want %v", gotMap, want) + } +} + +// TestCoerceFieldValue_RadioStillWorks verifies radiobutton fields are not regressed. +func TestCoerceFieldValue_RadioStillWorks(t *testing.T) { + field := &api.Field{ + ID: "customfield_10413", + Name: "Fehlverhalten", + Schema: &api.FieldSchema{ + Type: "option", + Custom: "com.atlassian.jira.plugin.system.customfieldtypes:radiobuttons", + }, + } + + got := coerceFieldValue(field, "Ja") + want := map[string]string{"value": "Ja"} + gotMap, ok := got.(map[string]string) + if !ok { + t.Fatalf("expected map[string]string, got %T", got) + } + if !reflect.DeepEqual(gotMap, want) { + t.Errorf("got %v, want %v", gotMap, want) + } +}