diff --git a/docs/subsystems/agent-wiring.md b/docs/subsystems/agent-wiring.md index ee59f3a2..d3113713 100644 --- a/docs/subsystems/agent-wiring.md +++ b/docs/subsystems/agent-wiring.md @@ -35,6 +35,7 @@ something else shows up as drift. - magpie only removes what it wrote. Anything else in an agent's config stays as it was. - JetBrains Air's `magpie-opencode.json` keeps user-added fields in `provider.magpie`, including nested options, on sync and model picks (`air` in [`air.go`](../../internal/agent/air.go), using `theirsKept` in [`sync.go`](../../internal/agent/sync.go)). The gateway fields and complete model list are still replaced, so removed models leave the list. +- JSON/JSONC config edits and their previews reject malformed input rather than editing a partially parsed document. Object edits and VS Code's model-group array edits leave the original file untouched on a parse error; valid comments and trailing commas remain supported (`internal/edit`). - Codex signed in to ChatGPT stays beside its sign-in (`openai_base_url`) unless the user picks magpie API. magpie becomes its provider only while the Codex app holds the active account, which it sends nothing for: `/wham/usage` says it isn't allowed, and it has no credits and isn't a workspace still within its overage (`codexHeld`, read by `provider.CodexUsedUp`). Where the app goes by what usage can't show (an experiment's gate, a reserve, a Team plan's overage under the reserve experiment), it counts as held: a wrong move costs ChatGPT extras, a missed one every turn. A saved reading kept through a failed read stops saying held once its used-up window has started again; a hold with no window used up stays till a fresh read. The app then loses its ChatGPT state (durable threads, remote control), so a window at 100% alone, which credits get past, doesn't move it (`codex.out` marks the move, and Sync undoes it once the account has room). - Moving a field from one of magpie's models to another (the agent's own picker) is not drift. Moving it off magpie is. - A field that follows another while empty (`Follows`: Claude Code's tiers and subagents on its main model) is read by `Drift` as on that field's model. The main model moved, in Claude Code's own `/model`, onto the model magpie had set a tier to is not drift (#1050). diff --git a/internal/agent/vscode_invalid_test.go b/internal/agent/vscode_invalid_test.go new file mode 100644 index 00000000..2bcf5070 --- /dev/null +++ b/internal/agent/vscode_invalid_test.go @@ -0,0 +1,28 @@ +package agent + +import ( + "path/filepath" + "testing" +) + +func TestVSCodeConnectPreservesInvalidConfig(t *testing.T) { + for _, file := range []string{"settings.json", "chatLanguageModels.json"} { + t.Run(file, func(t *testing.T) { + a := vscodeAt(filepath.Join(syncHome(t), "vscode", "User")) + writeFile(t, a.Path, `{"chat.defaultModel":"auto"}`) + models := filepath.Join(a.Dir, "chatLanguageModels.json") + writeFile(t, models, `[{"name":"mine","vendor":"ollama"}]`) + path, bad := a.Path, `{"chat.defaultModel":"auto" "editor.fontSize":14}` + if file == "chatLanguageModels.json" { + path, bad = models, `[{"name":"mine","vendor":"ollama"} {"name":"other"}]` + } + writeFile(t, path, bad) + if err := a.Connect(); err == nil { + t.Error("connecting VS Code with malformed config succeeded") + } + if got := readFile(path); got != bad { + t.Errorf("connecting VS Code changed malformed %s: %q", file, got) + } + }) + } +} diff --git a/internal/edit/json.go b/internal/edit/json.go index 400b74d4..883d511d 100644 --- a/internal/edit/json.go +++ b/internal/edit/json.go @@ -87,6 +87,9 @@ func setJSONBytes(raw []byte, keyPath string, value any) ([]byte, error) { // Comments are blanked out in a copy of identical length so gjson offsets // map 1:1 back onto the original bytes. stripped := jsonc.ToJSONInPlace(append([]byte(nil), raw...)) + if !json.Valid(stripped) { + return nil, fmt.Errorf("invalid JSON/JSONC") + } root := gjson.ParseBytes(stripped) if !root.IsObject() { return nil, fmt.Errorf("top level is not a JSON object") @@ -266,6 +269,9 @@ func DelJSON(path string, keyPaths ...string) error { func delJSONBytes(raw []byte, keyPath string) ([]byte, bool, error) { stripped := jsonc.ToJSONInPlace(append([]byte(nil), raw...)) + if !json.Valid(stripped) { + return nil, false, fmt.Errorf("invalid JSON/JSONC") + } r := gjson.GetBytes(stripped, keyPath) if !r.Exists() || r.Index == 0 { return raw, false, nil diff --git a/internal/edit/json_invalid_test.go b/internal/edit/json_invalid_test.go new file mode 100644 index 00000000..8011573b --- /dev/null +++ b/internal/edit/json_invalid_test.go @@ -0,0 +1,74 @@ +package edit + +import ( + "bytes" + "os" + "testing" +) + +func TestJSONEditsPreserveInvalidFiles(t *testing.T) { + for _, input := range []string{ + `{"model":"old", "keep":`, + `{"model":"old" "keep":true}`, + `{"model":"old"} {"keep":true}`, + } { + for name, edit := range map[string]func(string) error{ + "set": func(p string) error { return SetJSON(p, KV{"model", "new"}) }, + "delete": func(p string) error { return DelJSON(p, "model") }, + "delete absent": func(p string) error { return DelJSON(p, "missing") }, + } { + t.Run(name+"/"+input, func(t *testing.T) { + p := tmpFile(t, "settings.json", input) + if err := edit(p); err == nil { + t.Error("editing invalid JSON succeeded") + } + if got, err := os.ReadFile(p); err != nil || string(got) != input { + t.Errorf("invalid config was changed: %q, %v", got, err) + } + }) + } + for name, changes := range map[string]struct { + set []KV + del []string + }{ + "set": {set: []KV{{"model", "new"}}}, + "delete": {del: []string{"model"}}, + } { + t.Run("preview/"+name+"/"+input, func(t *testing.T) { + raw := []byte(input) + if _, err := PatchJSON(raw, changes.set, changes.del); err == nil { + t.Error("previewing an edit of invalid JSON succeeded") + } + if !bytes.Equal(raw, []byte(input)) { + t.Fatal("preview changed input") + } + }) + } + } +} + +func TestJSONItemEditsPreserveInvalidFiles(t *testing.T) { + where := map[string]string{"name": "magpie"} + for _, input := range []string{ + `[{"name":"magpie"}, {"name":]`, + `[{"name":"magpie"} {"name":"keep"}]`, + `[{"name":"magpie"}] [{"name":"keep"}]`, + } { + for name, edit := range map[string]func(string) error{ + "set": func(p string) error { + return SetJSONItem(p, where, map[string]string{"name": "magpie", "model": "new"}) + }, + "delete": func(p string) error { return DelJSONItem(p, where) }, + } { + t.Run(name+"/"+input, func(t *testing.T) { + p := tmpFile(t, "chatLanguageModels.json", input) + if err := edit(p); err == nil { + t.Error("editing invalid JSON array succeeded") + } + if got, err := os.ReadFile(p); err != nil || string(got) != input { + t.Errorf("invalid config was changed: %q, %v", got, err) + } + }) + } + } +} diff --git a/internal/edit/jsonarray.go b/internal/edit/jsonarray.go index 3a101d15..ca76124b 100644 --- a/internal/edit/jsonarray.go +++ b/internal/edit/jsonarray.go @@ -42,6 +42,9 @@ func SetJSONItem(path string, where map[string]string, value any) error { return WriteAtomic(path, []byte("[\n\t"+string(v)+"\n]\n")) } stripped := jsonc.ToJSONInPlace(append([]byte(nil), raw...)) + if !json.Valid(stripped) { + return fmt.Errorf("%s: invalid JSON/JSONC", path) + } root, open, close := topArray(stripped) if close < 0 { return fmt.Errorf("%s: top level is not a JSON array", path) @@ -85,6 +88,9 @@ func DelJSONItem(path string, where map[string]string) error { changed := false for { stripped := jsonc.ToJSONInPlace(append([]byte(nil), raw...)) + if !json.Valid(stripped) { + return fmt.Errorf("%s: invalid JSON/JSONC", path) + } r, ok := findItem(stripped, where) if !ok { break