Skip to content

Commit 36ab2a9

Browse files
authored
fix: allow converting string literal block with list (#3050)
This PR updates the following: * Fix YAML String Block Conversion: Resolved an issue where multiline strings (like tool descriptions) containing list syntax were being re-encoded as double-quoted strings with explicit \n characters. They now correctly use the | literal block style as expected. ``` description: | this is the description this tool uses the following parameter: - param_1 - param_2 # will turn into description: "this is the description\nthis tool uses the following parameter:\n-param_1\n-param_2" ``` * Updated the converter to identify and retain initial comment lines/license headers at the top of configuration files. * Updated migration completion status to reflect "ended" state. * Update to use "v1" -> "nested format" and "v2" -> "flat format" * Remove "authSources" when checking for keys. We had previously removed support for "authSources". Fixes #3023
1 parent 9859f4e commit 36ab2a9

File tree

4 files changed

+81
-64
lines changed

4 files changed

+81
-64
lines changed

cmd/internal/config.go

Lines changed: 53 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package internal
1616

1717
import (
18+
"bufio"
1819
"bytes"
1920
"context"
2021
"fmt"
@@ -120,14 +121,28 @@ func (p *ConfigParser) ParseConfig(ctx context.Context, raw []byte) (Config, err
120121

121122
// ConvertConfig converts configuration file to flat format.
122123
func ConvertConfig(raw []byte) ([]byte, error) {
124+
var buf bytes.Buffer
125+
// Manually copy top-level comments and empty lines from the source
126+
scanner := bufio.NewScanner(bytes.NewReader(raw))
127+
for scanner.Scan() {
128+
line := scanner.Text()
129+
trimmed := strings.TrimSpace(line)
130+
131+
// If the line is a comment or whitespace, preserve it
132+
if strings.HasPrefix(trimmed, "#") || trimmed == "" {
133+
buf.WriteString(line + "\n")
134+
} else {
135+
// Stop at the first line of actual data
136+
break
137+
}
138+
}
139+
140+
// convert configuration file to flat format
123141
var input yaml.MapSlice
124142
decoder := yaml.NewDecoder(bytes.NewReader(raw), yaml.UseOrderedMap())
143+
encoder := yaml.NewEncoder(&buf, yaml.UseLiteralStyleIfMultiline(true))
125144

126-
// convert to config file v2
127-
var buf bytes.Buffer
128-
encoder := yaml.NewEncoder(&buf)
129-
130-
v1keys := []string{"sources", "authServices", "embeddingModels", "tools", "toolsets", "prompts"}
145+
nestedFormatKey := []string{"sources", "authServices", "embeddingModels", "tools", "toolsets", "prompts"}
131146
docIndex := 0
132147
for {
133148
if err := decoder.Decode(&input); err != nil {
@@ -142,59 +157,48 @@ func ConvertConfig(raw []byte) ([]byte, error) {
142157
if !ok {
143158
return nil, fmt.Errorf("doc %d: unexpected non-string key in input: %v", docIndex, item.Key)
144159
}
145-
// check if the key is config file v1's key
146-
if slices.Contains(v1keys, key) {
147-
// check if value conversion to yaml.MapSlice successfully
148-
// fields such as "tools" in toolsets might pass the first check but
149-
// fail to convert to MapSlice
150-
if slice, ok := item.Value.(yaml.MapSlice); ok {
151-
// Deprecated: convert authSources to authServices
152-
switch key {
153-
case "authSources", "authServices":
154-
key = "authService"
155-
case "sources":
156-
key = "source"
157-
case "embeddingModels":
158-
key = "embeddingModel"
159-
case "tools":
160-
key = "tool"
161-
case "toolsets":
162-
key = "toolset"
163-
case "prompts":
164-
key = "prompt"
165-
}
166-
transformed, err := transformDocs(key, slice)
167-
if err != nil {
168-
return nil, fmt.Errorf("doc %d: invalid config format at key %q: %w", docIndex, key, err)
169-
}
170-
// encode per-doc
171-
for _, doc := range transformed {
172-
if err := encoder.Encode(doc); err != nil {
173-
return nil, err
174-
}
175-
}
176-
} else {
177-
if hasKindField(input) {
178-
// this doc is already v2, encode to buf
179-
if err := encoder.Encode(input); err != nil {
180-
return nil, err
181-
}
182-
break
183-
}
184-
return nil, fmt.Errorf("doc %d: invalid config format at key %q: expected map", docIndex, key)
185-
}
186-
} else {
187-
// this doc is already v2, encode to buf
160+
if hasKindField(input) {
161+
// this doc is already in flat format, encode to buf
188162
if err := encoder.Encode(input); err != nil {
189163
return nil, err
190164
}
191165
break
192166
}
167+
// check if value conversion to yaml.MapSlice successfully
168+
if slice, ok := item.Value.(yaml.MapSlice); slices.Contains(nestedFormatKey, key) && ok {
169+
switch key {
170+
case "authServices":
171+
key = "authService"
172+
case "sources":
173+
key = "source"
174+
case "embeddingModels":
175+
key = "embeddingModel"
176+
case "tools":
177+
key = "tool"
178+
case "toolsets":
179+
key = "toolset"
180+
case "prompts":
181+
key = "prompt"
182+
}
183+
transformed, err := transformDocs(key, slice)
184+
if err != nil {
185+
return nil, fmt.Errorf("doc %d: invalid config format at key %q: %w", docIndex, key, err)
186+
}
187+
// encode per-doc
188+
for _, doc := range transformed {
189+
if err := encoder.Encode(doc); err != nil {
190+
return nil, err
191+
}
192+
}
193+
} else {
194+
return nil, fmt.Errorf("doc %d: invalid config format at key %q: expected nested format keys and type map", docIndex, key)
195+
}
193196
}
194197
}
195198
return buf.Bytes(), nil
196199
}
197200

201+
// hasKindField is a helper function to check if an input is in flat format
198202
func hasKindField(input yaml.MapSlice) bool {
199203
for _, item := range input {
200204
if key, ok := item.Key.(string); ok && key == "kind" {
@@ -204,7 +208,7 @@ func hasKindField(input yaml.MapSlice) bool {
204208
return false
205209
}
206210

207-
// transformDocs transforms the configuration file from v1 format to v2
211+
// transformDocs transforms the configuration file from nested to flat format
208212
// yaml.MapSlice will preserve the order in a map
209213
func transformDocs(kind string, input yaml.MapSlice) ([]yaml.MapSlice, error) {
210214
var transformed []yaml.MapSlice

cmd/internal/config_test.go

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,8 @@ func TestConvertConfig(t *testing.T) {
184184
model: gemini-embedding-001
185185
apiKey: some-key
186186
dimension: 768`,
187-
want: `kind: source
187+
want: `
188+
kind: source
188189
name: my-pg-instance
189190
type: cloud-sql-postgres
190191
project: my-project
@@ -261,7 +262,8 @@ dimension: 768
261262
toolsets:
262263
example_toolset:
263264
- example_tool`,
264-
want: `kind: tool
265+
want: `
266+
kind: tool
265267
name: example_tool
266268
type: postgres-sql
267269
source: my-pg-instance
@@ -382,7 +384,8 @@ tools:
382384
kind: embeddingModel
383385
name: gemini-model2
384386
type: gemini`,
385-
want: `kind: source
387+
want: `
388+
kind: source
386389
name: my-pg-instance
387390
type: cloud-sql-postgres
388391
project: my-project
@@ -478,7 +481,8 @@ type: gemini
478481
},
479482
{
480483
desc: "no convertion needed",
481-
in: `kind: source
484+
in: `
485+
kind: source
482486
name: my-pg-instance
483487
type: cloud-sql-postgres
484488
project: my-project
@@ -503,7 +507,8 @@ kind: toolset
503507
name: example_toolset
504508
tools:
505509
- example_tool`,
506-
want: `kind: source
510+
want: `
511+
kind: source
507512
name: my-pg-instance
508513
type: cloud-sql-postgres
509514
project: my-project
@@ -534,13 +539,13 @@ tools:
534539
desc: "invalid source",
535540
in: `sources: invalid`,
536541
isErr: true,
537-
errStr: `doc 1: invalid config format at key "sources": expected map`,
542+
errStr: `doc 1: invalid config format at key "sources": expected nested format keys and type map`,
538543
},
539544
{
540545
desc: "invalid toolset",
541546
in: `toolsets: invalid`,
542547
isErr: true,
543-
errStr: `doc 1: invalid config format at key "toolsets": expected map`,
548+
errStr: `doc 1: invalid config format at key "toolsets": expected nested format keys and type map`,
544549
},
545550
}
546551
for _, tc := range tcs {

cmd/internal/migrate/command.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func runMigrate(cmd *migrateCmd, opts *internal.ToolboxOptions) error {
6565
return errMsg
6666
}
6767

68-
logger.InfoContext(ctx, "migration process will start; any comments present in the original configuration files will not be preserved in the migrated files")
68+
logger.InfoContext(ctx, "migration process will start; any comments (except for top-level comments) presented in the original configuration files will not be preserved in the migrated files")
6969
var errs []error
7070
// process each files independently.
7171
for _, filePath := range filePaths {
@@ -127,7 +127,7 @@ func runMigrate(cmd *migrateCmd, opts *internal.ToolboxOptions) error {
127127
}
128128
}
129129

130-
logger.InfoContext(ctx, "migration completed!")
130+
logger.InfoContext(ctx, "migration ended!")
131131
// If errs is empty, errors.Join returns nil
132132
return errors.Join(errs...)
133133
}

cmd/internal/migrate/command_test.go

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ sources:
5252
user: my_user
5353
password: my_pass`
5454

55-
toolsFileContentNew := `kind: source
55+
toolsFileContentNew := `
56+
kind: source
5657
name: my-pg-instance
5758
type: cloud-sql-postgres
5859
project: my-project
@@ -74,7 +75,8 @@ tools:
7475
type: string
7576
description: some description`
7677

77-
toolsFileContent2New := `kind: tool
78+
toolsFileContent2New := `
79+
kind: tool
7880
name: example_tool2
7981
type: postgres-sql
8082
source: my-pg-instance
@@ -268,7 +270,8 @@ sources:
268270
password: my_pass
269271
`
270272

271-
toolsFileContentNew := `kind: source
273+
toolsFileContentNew := `
274+
kind: source
272275
name: my-pg-instance
273276
type: cloud-sql-postgres
274277
project: my-project
@@ -283,18 +286,23 @@ tools:
283286
example_tool2:
284287
kind: postgres-sql
285288
source: my-pg-instance
286-
description: some description
289+
description: |
290+
some description
291+
- string
287292
statement: SELECT * FROM SQL_STATEMENT;
288293
parameters:
289294
- name: country
290295
type: string
291296
description: some description`
292297

293-
toolsFileContent2New := `kind: tool
298+
toolsFileContent2New := `
299+
kind: tool
294300
name: example_tool2
295301
type: postgres-sql
296302
source: my-pg-instance
297-
description: some description
303+
description: |
304+
some description
305+
- string
298306
statement: SELECT * FROM SQL_STATEMENT;
299307
parameters:
300308
- name: country

0 commit comments

Comments
 (0)