From e2e5f4bb64b77bc4181d64c900979c8d910f7ffc Mon Sep 17 00:00:00 2001 From: yetone Date: Thu, 1 Oct 2026 23:21:35 +0800 Subject: [PATCH] =?UTF-8?q?agents:=20dsh's=20patch=20list=20written=20in?= =?UTF-8?q?=20flow=20style=20is=20edited=20like=20a=20block=20one,=20and?= =?UTF-8?q?=20a=20file=20magpie=20can't=20edit=20is=20named=20as=20itself?= =?UTF-8?q?=20(#445)=20=E2=80=94=20a=20profile's=20cordis.patch.yml=20whos?= =?UTF-8?q?e=20top-level=20array=20was=20written=20as=20[=20{=20id:=20some?= =?UTF-8?q?-plugin,=20disabled:=20false=20}=20]=20(valid=20YAML,=20and=20w?= =?UTF-8?q?hat=20dsh's=20own=20writers=20keep=20the=20template's=20[]=20as?= =?UTF-8?q?=20once=20they=20add=20a=20row)=20failed=20every=20model=20swit?= =?UTF-8?q?ch=20with=20"config.yaml=20is=20not=20a=20list=20of=20entries?= =?UTF-8?q?=20magpie=20can=20edit":=20the=20patch-list=20reader=20took=20o?= =?UTF-8?q?nly=20'-=20'=20lines,=20and=20the=20error=20named=20config.yaml?= =?UTF-8?q?=20whatever=20the=20file;=20since=20web's=20list=20goes=20first?= =?UTF-8?q?,=20one=20such=20profile=20stopped=20all=20of=20them.=20New=20e?= =?UTF-8?q?dit.BlockList=20reads=20a=20file=20whose=20one=20document=20is?= =?UTF-8?q?=20a=20flow=20sequence=20as=20the=20same=20list=20in=20block=20?= =?UTF-8?q?style=20(nested=20mappings=20and=20lists=20in=20block=20style,?= =?UTF-8?q?=20scalars=20quoted=20and=20tagged=20as=20they=20were,=20the=20?= =?UTF-8?q?lines=20before=20it=20and=20the=20comments=20after=20it=20kept)?= =?UTF-8?q?;=20dsh's=20model/effort=20wiring=20and=20its=20MCP=20servers?= =?UTF-8?q?=20(internal/library)=20read=20and=20write=20such=20a=20file=20?= =?UTF-8?q?through=20it,=20and=20its=20errors=20name=20the=20real=20path.?= =?UTF-8?q?=20dshProfiles=20puts=20web=20first=20by=20partition=20instead?= =?UTF-8?q?=20of=20a=20sort=20whose=20less=20wasn't=20a=20strict=20weak=20?= =?UTF-8?q?order.=20TestDshFlowPatchList=20(agent,=20library)=20fails=20wi?= =?UTF-8?q?thout=20the=20change;=20TestBlockList=20covers=20one-line,=20fo?= =?UTF-8?q?lded/nested/!!js,=20empty,=20CRLF=20and=20what=20isn't=20a=20fl?= =?UTF-8?q?ow=20list.=20Sandbox=20HOME:=20magpie=20dsh=20deepseek-v4-pro?= =?UTF-8?q?=20on=20main=20fails=20with=20the=20config.yaml=20error,=20with?= =?UTF-8?q?=20the=20change=20both=20profiles=20get=20the=20model=20and=20w?= =?UTF-8?q?eb=20keeps=20some-plugin.=20go=20vet,=20go=20build=20-tags=20no?= =?UTF-8?q?gui=20(darwin,=20GOOS=3Dlinux),=20GOOS=3Dwindows=20go=20build,?= =?UTF-8?q?=20go=20test=20-tags=20nogui=20./...=20pass.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- internal/agent/dsh.go | 31 +++++++---- internal/agent/dsh_flow_test.go | 92 +++++++++++++++++++++++++++++++ internal/edit/yaml.go | 57 +++++++++++++++++++ internal/edit/yaml_flow_test.go | 46 ++++++++++++++++ internal/library/dsh.go | 3 + internal/library/dsh_flow_test.go | 22 ++++++++ 6 files changed, 240 insertions(+), 11 deletions(-) create mode 100644 internal/agent/dsh_flow_test.go create mode 100644 internal/edit/yaml_flow_test.go create mode 100644 internal/library/dsh_flow_test.go diff --git a/internal/agent/dsh.go b/internal/agent/dsh.go index 8c1fe709..d7d3b8e1 100644 --- a/internal/agent/dsh.go +++ b/internal/agent/dsh.go @@ -30,7 +30,6 @@ import ( "os" "path/filepath" "regexp" - "sort" "strings" "gopkg.in/yaml.v3" @@ -129,10 +128,15 @@ func dsh(home string) *Agent { // dshProfiles are the profiles' patch lists (dsh 0.1.5 on), web's first. func dshProfiles(dir string) []string { files, _ := filepath.Glob(filepath.Join(dir, "profiles", "*", "cordis.patch.yml")) - sort.SliceStable(files, func(i, j int) bool { - return filepath.Base(filepath.Dir(files[i])) == "web" && filepath.Base(filepath.Dir(files[j])) != "web" - }) - return files + var web, rest []string + for _, f := range files { + if filepath.Base(filepath.Dir(f)) == "web" { + web = append(web, f) + } else { + rest = append(rest, f) + } + } + return append(web, rest...) } // dshItem is one entry of the patch list, as its lines. @@ -144,9 +148,14 @@ type dshItem struct { var dshIDLine = regexp.MustCompile(`^(?:- | )id:\s*['"]?([^'"#\s]+)['"]?\s*(#.*)?$`) -// dshParse splits the patch list into what comes before its first entry and -// the entries. A file that is not a plain block list is left alone. -func dshParse(raw string) (head []string, items []dshItem, err error) { +// dshParse splits the patch list at path into what comes before its first +// entry and the entries. A list written in flow style ([ {id: a} ], which +// dsh's own writers keep a profile's [] in) is read as the same list in block +// style, and written back so; a file that is no list is left alone. +func dshParse(path, raw string) (head []string, items []dshItem, err error) { + if b, ok := edit.BlockList(raw); ok { + raw = b + } for _, l := range splitLinesKeep(raw) { t := strings.TrimSpace(l) switch { @@ -154,14 +163,14 @@ func dshParse(raw string) (head []string, items []dshItem, err error) { items = append(items, dshItem{lines: []string{l}}) case len(items) == 0: if t != "" && !strings.HasPrefix(t, "#") && t != "[]" { - return nil, nil, fmt.Errorf("%s is not a list of entries magpie can edit", "config.yaml") + return nil, nil, fmt.Errorf("%s is not a list of entries magpie can edit", path) } if t != "[]" { head = append(head, l) } continue case t != "" && !strings.HasPrefix(t, "#") && !strings.HasPrefix(l, " "): - return nil, nil, fmt.Errorf("%s is not a list of entries magpie can edit", "config.yaml") + return nil, nil, fmt.Errorf("%s is not a list of entries magpie can edit", path) default: items[len(items)-1].lines = append(items[len(items)-1].lines, l) } @@ -188,7 +197,7 @@ func dshRead(path string) ([]string, []dshItem, error) { if err != nil { return nil, nil, err } - return dshParse(string(b)) + return dshParse(path, string(b)) } // dshWrite puts a patch list back together. diff --git a/internal/agent/dsh_flow_test.go b/internal/agent/dsh_flow_test.go new file mode 100644 index 00000000..cc1c6cd5 --- /dev/null +++ b/internal/agent/dsh_flow_test.go @@ -0,0 +1,92 @@ +package agent + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "gopkg.in/yaml.v3" + + "github.com/yetone/magpie/internal/provider" +) + +// A profile's patch list written in flow style, as dsh's own writers leave +// the template's [] once they add a row (#445), is edited like a block one: +// every profile gets the model, the user's row stays, and a file magpie +// can't edit is named in the error, not as config.yaml. +func TestDshFlowPatchList(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("DSH_HOME", "") + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) + t.Setenv("XDG_CACHE_HOME", filepath.Join(home, ".cache")) + if err := provider.Save(provider.Provider{ID: "deepseek", Name: "DeepSeek", Chat: "https://api.deepseek.com/v1", Key: "k", Models: []string{"pro", "flash"}}); err != nil { + t.Fatal(err) + } + dir := filepath.Join(home, ".dsh") + web, def := filepath.Join(dir, "profiles", "web", "cordis.patch.yml"), filepath.Join(dir, "profiles", "default", "cordis.patch.yml") + flow := "# Your patch layer for this dsh profile, applied after every bundle layer:\n# a top-level YAML array of loader patch entries ...\n[ { id: some-plugin, disabled: false } ]\n" + template := "# Your patch layer for this dsh profile.\n[]\n" + for p, s := range map[string]string{web: flow, def: template} { + os.MkdirAll(filepath.Dir(p), 0o755) + os.WriteFile(p, []byte(s), 0o644) + } + read := func(p string) string { b, _ := os.ReadFile(p); return string(b) } + + f := dsh(home).Field("model") + if err := f.Set("magpie/deepseek/pro"); err != nil { + t.Fatal(err) + } + for _, p := range []string{web, def} { + s := read(p) + for _, want := range []string{"# Your patch layer", "- id: llm-pi-ai # magpie", "- id: agent-default-model # magpie", `model: "deepseek/pro"`} { + if !strings.Contains(s, want) { + t.Fatalf("missing %q in %s:\n%s", want, p, s) + } + } + } + s := read(web) + if !strings.Contains(s, "- id: some-plugin\n disabled: false\n") || !strings.HasPrefix(s, "# Your patch layer for this dsh profile, applied after every bundle layer:\n# a top-level YAML array") { + t.Fatalf("the user's row or comments lost:\n%s", s) + } + var rows []map[string]any + if err := yaml.Unmarshal([]byte(s), &rows); err != nil || len(rows) != 3 || rows[0]["id"] != "some-plugin" || rows[0]["disabled"] != false { + t.Fatalf("%v %v\n%s", err, rows, s) + } + if f.Get() != "magpie/deepseek/pro" { + t.Fatalf("get: %q", f.Get()) + } + + // back to dsh as it ships: the user's row, now a block list + if err := f.Set(""); err != nil { + t.Fatal(err) + } + if got, want := read(web), "# Your patch layer for this dsh profile, applied after every bundle layer:\n# a top-level YAML array of loader patch entries ...\n- id: some-plugin\n disabled: false\n"; got != want { + t.Fatalf("reset:\n%q\nwant\n%q", got, want) + } + + // a file that is no list is named as itself + os.WriteFile(web, []byte("some-plugin:\n disabled: false\n"), 0o644) + err := f.Set("magpie/deepseek/pro") + if err == nil || !strings.Contains(err.Error(), web) || strings.Contains(err.Error(), "config.yaml") { + t.Fatalf("error: %v", err) + } +} + +// web's patch list is the first whatever the profiles are called. +func TestDshProfilesWebFirst(t *testing.T) { + dir := t.TempDir() + for _, n := range []string{"a", "web", "default", "z", "desktop"} { + p := filepath.Join(dir, "profiles", n, "cordis.patch.yml") + os.MkdirAll(filepath.Dir(p), 0o755) + os.WriteFile(p, []byte("[]\n"), 0o644) + } + var got []string + for _, f := range dshProfiles(dir) { + got = append(got, filepath.Base(filepath.Dir(f))) + } + if strings.Join(got, " ") != "web a default desktop z" { + t.Fatalf("%v", got) + } +} diff --git a/internal/edit/yaml.go b/internal/edit/yaml.go index 28e15c7b..112f5edc 100644 --- a/internal/edit/yaml.go +++ b/internal/edit/yaml.go @@ -320,3 +320,60 @@ func blockStyle(n *yaml.Node) { blockStyle(c) } } + +// BlockList is raw, a file whose one document is a top-level sequence +// written in flow style ([ {id: a}, {id: b} ], what dsh's own writers keep +// a file that starts as []), with that sequence written as a block list +// instead: one "- " item after another, each entry's mappings and lists in +// block style too, its scalars quoted as they were. The lines before the +// sequence and the comment lines after it stay as written. ok is false, and +// raw is left to the caller, for anything else: a block list, a mapping, +// several documents, or text that isn't YAML. +func BlockList(raw string) (string, bool) { + raw = strings.ReplaceAll(raw, "\r\n", "\n") + dec := yaml.NewDecoder(strings.NewReader(raw)) + var doc, more yaml.Node + if dec.Decode(&doc) != nil || dec.Decode(&more) == nil { + return "", false + } + if len(doc.Content) != 1 { + return "", false + } + seq := doc.Content[0] + if seq.Kind != yaml.SequenceNode || seq.Style&yaml.FlowStyle == 0 { + return "", false + } + lines := strings.Split(strings.TrimRight(raw, "\n"), "\n") + start := min(max(seq.Line-1, 0), len(lines)) + end := len(lines) + for end > start { + if t := strings.TrimSpace(lines[end-1]); t != "" && !strings.HasPrefix(t, "#") { + break + } + end-- + } + out := append([]string{}, lines[:start]...) + if len(seq.Content) == 0 { + out = append(out, "[]") + } else { + seq.HeadComment, seq.LineComment, seq.FootComment = "", "", "" + unflow(seq) + var buf bytes.Buffer + enc := yaml.NewEncoder(&buf) + enc.SetIndent(2) + if enc.Encode(seq) != nil || enc.Close() != nil { + return "", false + } + out = append(out, strings.Split(strings.TrimRight(buf.String(), "\n"), "\n")...) + } + out = append(out, lines[end:]...) + return strings.Join(out, "\n") + "\n", true +} + +// unflow writes n's mappings and sequences in block style. +func unflow(n *yaml.Node) { + n.Style &^= yaml.FlowStyle + for _, c := range n.Content { + unflow(c) + } +} diff --git a/internal/edit/yaml_flow_test.go b/internal/edit/yaml_flow_test.go new file mode 100644 index 00000000..e3bff90a --- /dev/null +++ b/internal/edit/yaml_flow_test.go @@ -0,0 +1,46 @@ +package edit + +import ( + "reflect" + "testing" + + "gopkg.in/yaml.v3" +) + +// A top-level list written in flow style, as dsh's own writers keep a +// profile's [] once they add to it (#445), comes back as the same list in +// block style, the lines around it as they were. +func TestBlockList(t *testing.T) { + cases := []struct{ name, in, want string }{ + {"one line", "# Your patch layer for this dsh profile\n# a top-level YAML array\n[ { id: some-plugin, disabled: false } ]\n", + "# Your patch layer for this dsh profile\n# a top-level YAML array\n- id: some-plugin\n disabled: false\n"}, + {"folded, nested, tagged", "[\n { id: tools, config: { disabled: [a, b] } },\n { id: \"@x/y\", config: { cwd: !!js process.cwd(), n: \"1\" } }\n]\n# after\n", + "- id: tools\n config:\n disabled:\n - a\n - b\n- id: \"@x/y\"\n config:\n cwd: !!js process.cwd()\n n: \"1\"\n# after\n"}, + {"empty", "# head\n[ ]\n", "# head\n[]\n"}, + {"bare", "[]\n", "[]\n"}, + {"crlf", "# h\r\n[ {id: a} ]\r\n", "# h\n- id: a\n"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got, ok := BlockList(c.in) + if !ok || got != c.want { + t.Fatalf("got %v %q, want %q", ok, got, c.want) + } + var before, after any + if err := yaml.Unmarshal([]byte(c.in), &before); err != nil { + t.Fatal(err) + } + if err := yaml.Unmarshal([]byte(got), &after); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(before, after) { + t.Fatalf("the list changed: %v -> %v", before, after) + } + }) + } + for _, in := range []string{"", "# only a comment\n", "- id: a\n", "a: [1]\n", "[a]\n---\n[b]\n", "[ {id: a\n", "- [a, b]\n"} { + if got, ok := BlockList(in); ok { + t.Fatalf("%q taken for a flow list: %q", in, got) + } + } +} diff --git a/internal/library/dsh.go b/internal/library/dsh.go index d38f7dc8..41548bcb 100644 --- a/internal/library/dsh.go +++ b/internal/library/dsh.go @@ -82,6 +82,9 @@ func dshRead(path string) (*dshPatches, error) { return nil, err } p := &dshPatches{} + if b, ok := edit.BlockList(string(raw)); ok { // [ {...} ]: read, and written back, as a block list + raw = []byte(b) + } s := strings.TrimRight(strings.ReplaceAll(string(raw), "\r\n", "\n"), "\n") if s == "" { return p, nil diff --git a/internal/library/dsh_flow_test.go b/internal/library/dsh_flow_test.go new file mode 100644 index 00000000..78124c65 --- /dev/null +++ b/internal/library/dsh_flow_test.go @@ -0,0 +1,22 @@ +package library + +import ( + "path/filepath" + "strings" + "testing" +) + +// A patch list in flow style (#445) is read and written like a block one: +// the user's rows stay, magpie's server goes in as an insert of its own. +func TestDshFlowPatchList(t *testing.T) { + h := sandbox(t) + p := filepath.Join(h, ".dsh/profiles/web/cordis.patch.yml") + write(t, p, "# Your patch layer for this dsh profile\n[ { id: some-plugin, disabled: false } ]\n") + ok(t)(SaveServer("", Server{Name: "fs", Transport: "stdio", Command: "npx", Args: []string{"-y", "@mcp/fs"}, Agents: []string{"dsh"}})) + s := read(t, p) + for _, want := range []string{"# Your patch layer for this dsh profile\n- id: some-plugin\n disabled: false\n", "- insert: # magpie\n - id: magpie-mcp-fs\n", `serverName: "fs"`} { + if !strings.Contains(s, want) { + t.Fatalf("missing %q in\n%s", want, s) + } + } +}