Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions internal/api/jira.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
23 changes: 18 additions & 5 deletions internal/api/markdown.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
}

Expand Down
85 changes: 85 additions & 0 deletions internal/api/markdown_indented_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}
19 changes: 14 additions & 5 deletions internal/cmd/issue/field_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
128 changes: 128 additions & 0 deletions internal/cmd/issue/field_util_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
Loading