diff --git a/docs/subsystems/agent-wiring.md b/docs/subsystems/agent-wiring.md index 2febccfa..6ef9b0a6 100644 --- a/docs/subsystems/agent-wiring.md +++ b/docs/subsystems/agent-wiring.md @@ -20,6 +20,7 @@ something else shows up as drift. | What came before | The stash (`stash.json`) remembers what the config said before magpie pointed it at the gateway, so Disconnect restores it rather than guessing. | [`stash.go`](../../internal/agent/stash.go) | | Disconnect and preview | `Disconnect` runs `Unwire`, then returns each field still on magpie to its default. `DisconnectPreview` runs the real Disconnect on a copy under a temporary home, in a subprocess (`DryRunArg`), and diffs the files. The preview is never a second account of what Disconnect does. | [`applied.go`](../../internal/agent/applied.go), [`preview.go`](../../internal/agent/preview.go) | | Keeping configs current | `Reseat` moves agents off a model magpie no longer serves (#200). `OnGateway` and `Rewire` write the new gateway address into every connected agent when the port changes. `Source` and `Stale` say what an unconnected agent runs on, and whether running copies still use the old list. | [`reseat.go`](../../internal/agent/reseat.go), [`rewire.go`](../../internal/agent/rewire.go), [`connectinfo.go`](../../internal/agent/connectinfo.go) | +| Named omp profiles | omp keeps a profile's settings under `~/.omp/profiles/` (or under `PI_CONFIG_DIR`), each with its own `config.yml` and `models.yml`. Every such profile is an agent of its own — `omp#` on Windows, `omp#@wsl:` in a distro — so each can be connected, drifted and disconnected on its own. They share omp's model lists (`ListsFor`), and the default `omp` row is the folder `ompDir`/`ompIn` already pick, so an `OMP_PROFILE` that row is already pointed at is not listed twice. A name omp refuses is not a row: `default`, a trailing dot, or a Windows reserved device name (`aux`, `nul`, `com1`, `lpt9.x`, `con.work`), on every platform (`ompProfileOK`). A distro's profile names come from its probe and are kept in `wsl.json`; a stopped distro is never started to list them, its profile rows showing what was last seen (`asleep`). | [`omp.go`](../../internal/agent/omp.go), [`wsl.go`](../../internal/agent/wsl.go) | | WSL twins | On Windows each running distro is probed once, without starting a stopped one. The `wslKinds` agents found there become agents of their own, `@wsl:`, edited through `\\wsl.localhost\`. A kind is found by its folder under the distro's home or by its command there (not one on a Windows drive). Where another tool shares one of them, the kind leaves it out: Gemini CLI is found by its command alone, because Antigravity keeps `~/.gemini` too. Goose is found by its folder alone, because a `goose` command can be pressly's migration tool. Reasonix also needs the version its command gives (`accept`), because the old npm client is named `reasonix` too. The gateway address and key follow the distro's networking: mirrored or NAT (`place`, `keyAt`). `ListsFor` keeps a twin's model picks under the Windows twin's id (#927). | [`wsl.go`](../../internal/agent/wsl.go), [`gatewaykey.go`](../../internal/agent/gatewaykey.go) | | Environment-only agents | Agents that read the gateway only from the environment get a `Launch` command. Cursor Private Inference instead gets user-level variables (`cursorLocalOn`, `KeepCursorLocalEnv`), and magpie writes none of Cursor's own files. Its effort field is kept in magpie (`provider.AgentEffort`) and asked by the gateway, its levels those of the models it lists (`cursorLocalEfforts`, #1003). | [`cursorlocal.go`](../../internal/agent/cursorlocal.go) | diff --git a/internal/agent/agents.go b/internal/agent/agents.go index 79cfeee2..a6dff62f 100644 --- a/internal/agent/agents.go +++ b/internal/agent/agents.go @@ -115,7 +115,7 @@ func All() []*Agent { atomcode(home), alma(), cindy(), - }, wslAgents()...) + }, append(ompProfiles(home), wslAgents()...)...) } // ---- accessors ------------------------------------------------------------- diff --git a/internal/agent/omp.go b/internal/agent/omp.go index 2d8d2142..b3b20a57 100644 --- a/internal/agent/omp.go +++ b/internal/agent/omp.go @@ -89,6 +89,30 @@ func ompYAMLList(v string) bool { return strings.HasPrefix(v, "[") || strings.Ha // normalizeProfileName); it refuses any other. var ompProfileName = regexp.MustCompile(`^[a-z0-9][a-z0-9._-]{0,63}$`) +// ompReserved is a Windows device name omp refuses on every platform +// (pi-utils dirs.ts, normalizeProfileName): CON, PRN, AUX, NUL, COM0–COM9, +// LPT0–LPT9, and the same with a dot and anything after (con.work). +var ompReserved = regexp.MustCompile(`(?i)^(?:CON|PRN|AUX|NUL|COM[0-9]|LPT[0-9])(?:\..*)?$`) + +// ompProfileOK says whether name is one omp takes as a profile (pi-utils +// dirs.ts): "default" is none, a trailing dot is refused, and so is a +// Windows reserved device name. What a profiles folder holds is checked +// against this, and so is what a distro's probe finds, and the profile +// OMP_PROFILE names for the default row. +func ompProfileOK(name string) bool { + return name != "" && name != "default" && ompProfileName.MatchString(name) && + !strings.HasSuffix(name, ".") && !ompReserved.MatchString(name) +} + +// ompRoot is where omp keeps its agent folder and its profiles: ~/.omp, or +// ~/$PI_CONFIG_DIR when that variable renames it. +func ompRoot(home string) string { + if d := appdir.Getenv("PI_CONFIG_DIR"); d != "" { + return filepath.Join(home, d) + } + return filepath.Join(home, ".omp") +} + // ompDir is omp's agent folder, found as omp's pi-utils (dirs.ts) finds it: // under ~/.omp, or ~/$PI_CONFIG_DIR; a profile's (OMP_PROFILE, else // PI_PROFILE; "default" is none) is profiles//agent there; with none, @@ -99,15 +123,12 @@ var ompProfileName = regexp.MustCompile(`^[a-z0-9][a-z0-9._-]{0,63}$`) // moved it, and Pi reads that variable too, so Pi's own folder may be what // it names. func ompDir(home string) (dir string, shared bool) { - root := filepath.Join(home, ".omp") - if d := appdir.Getenv("PI_CONFIG_DIR"); d != "" { - root = filepath.Join(home, d) - } + root := ompRoot(home) p, set := os.LookupEnv("OMP_PROFILE") if !set { p = os.Getenv("PI_PROFILE") } - if p = strings.TrimSpace(p); p != "" && p != "default" && ompProfileName.MatchString(p) && !strings.HasSuffix(p, ".") { + if p = strings.TrimSpace(p); ompProfileOK(p) { return filepath.Join(root, "profiles", p, "agent"), false } if d := appdir.Getenv("PI_CODING_AGENT_DIR"); filepath.IsAbs(d) { @@ -124,6 +145,45 @@ func omp(home string) *Agent { return a } +// ompProfiles are the named omp profiles under ~/.omp/profiles (or +// ~/$PI_CONFIG_DIR/profiles): each is an omp of its own, omp#, with +// its own config.yml and models.yml in profiles//agent. The one the +// default row already picks (OMP_PROFILE) is left out, so it is not listed +// twice, and the name rules are omp's own. A missing profiles folder is no +// profiles, not an error; a profile whose agent folder has no config.yml +// yet is still a row, and the first Set makes the file. +func ompProfiles(home string) []*Agent { + root := ompRoot(home) + entries, err := os.ReadDir(filepath.Join(root, "profiles")) + if err != nil { + return nil + } + cur, _ := ompDir(home) + var names []string + for _, e := range entries { + name := e.Name() + if !e.IsDir() || !ompProfileOK(name) { + continue + } + if filepath.Join(root, "profiles", name, "agent") == cur { + continue + } + names = append(names, name) + } + sort.Strings(names) + var out []*Agent + for _, name := range names { + a := ompAt(here(home), filepath.Join(root, "profiles", name, "agent"), func() ompProviderEntry { return ompProvider() }) + a.ID, a.Name = "omp#"+name, "omp · "+name + // the default row keeps omp's name, its command and its own lists + a.Aliases, a.UA, a.Bin = nil, nil, "" + profile := filepath.Join(root, "profiles", name) + a.detect = func() bool { return isDir(profile) } + out = append(out, a) + } + return out +} + // ompIn is omp in a WSL distro (see wsl.go): ~/.omp/agent, as the // distro's variables that move it aren't read. The omp there isn't the one // on Windows' PATH: its version is the one the distro's probe asked it diff --git a/internal/agent/omp_profile_test.go b/internal/agent/omp_profile_test.go new file mode 100644 index 00000000..444ba586 --- /dev/null +++ b/internal/agent/omp_profile_test.go @@ -0,0 +1,279 @@ +package agent + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// ompProfileAt writes a profile's config.yml with the model of its default +// role, and answers the folder it is in. +func ompProfileAt(t *testing.T, home, name, model string) string { + t.Helper() + dir := filepath.Join(home, ".omp", "profiles", name, "agent") + writeFile(t, filepath.Join(dir, "config.yml"), "modelRoles:\n default: "+model+"\n") + return dir +} + +// Named omp profiles under ~/.omp/profiles are agents of their own: each +// has its own config.yml and models.yml, shares omp's model lists, and +// leaves the default row's files alone. The default row is still the one +// ompDir picks, and still answers to omp's alias. +func TestOmpProfiles(t *testing.T) { + home := syncHome(t) + def := filepath.Join(home, ".omp", "agent") + writeFile(t, filepath.Join(def, "config.yml"), "modelRoles:\n default: a/model-a\n") + work := ompProfileAt(t, home, "work", "b/model-b") + lab := ompProfileAt(t, home, "lab", "c/model-c") + + for _, id := range []string{"omp", "omp#work", "omp#lab"} { + if _, err := Find(id); err != nil { + t.Fatalf("%s: %v", id, err) + } + } + w, err := Find("omp#work") + if err != nil { + t.Fatal(err) + } + if w.Name != "omp · work" || w.Icon != "omp" || w.ListsFor() != "omp" { + t.Fatalf("work: %+v", w) + } + if !w.Detected() || w.Dir != work { + t.Fatalf("work dir %q, detected %v", w.Dir, w.Detected()) + } + if f := w.Field("model"); f == nil || f.Get() != "b/model-b" { + t.Fatalf("work model %v", f) + } + + before, labBefore := readFile(filepath.Join(def, "config.yml")), readFile(filepath.Join(lab, "config.yml")) + if err := w.Field("model").Set("magpie/relay/glm-4.6"); err != nil { + t.Fatal(err) + } + if readFile(filepath.Join(work, "config.yml")) == "modelRoles:\n default: b/model-b\n" { + t.Fatal("work's config wasn't written") + } + if readFile(filepath.Join(def, "config.yml")) != before { + t.Fatalf("the default row's config moved:\n%s", readFile(filepath.Join(def, "config.yml"))) + } + if readFile(filepath.Join(lab, "config.yml")) != labBefore { + t.Fatalf("lab's config moved:\n%s", readFile(filepath.Join(lab, "config.yml"))) + } + + d, err := Find("omp") + if err != nil { + t.Fatal(err) + } + if d.Name != "omp" || d.Dir != def || d.ListsFor() != "omp" { + t.Fatalf("default: %+v", d) + } + if f := d.Field("model"); f.Get() != "a/model-a" { + t.Fatalf("default model %q", f.Get()) + } + if a, err := Find("oh-my-pi"); err != nil || a.ID != "omp" { + t.Fatalf("oh-my-pi: %v %v", a, err) + } +} + +// A profile name omp refuses ("Bad", "work.", a Windows reserved device +// name) is no profile, "default" is the default row, and a folder that +// isn't a directory is no profile either. One whose agent folder has no +// config.yml yet is still a row: the first Set makes the file, and only +// that profile's. "work." is not made as a folder: Windows drops the +// trailing dot, so MkdirAll("work.") would create "work" and list it. +func TestOmpProfilesNameRules(t *testing.T) { + home := syncHome(t) + root := filepath.Join(home, ".omp") + for _, name := range []string{"Bad", "default"} { + if err := os.MkdirAll(filepath.Join(root, "profiles", name, "agent"), 0o755); err != nil { + t.Fatal(err) + } + } + // a file where a profile folder would be + writeFile(t, filepath.Join(root, "profiles", "notes"), "not a profile\n") + fresh := filepath.Join(root, "profiles", "fresh") + if err := os.MkdirAll(fresh, 0o755); err != nil { + t.Fatal(err) + } + + got := map[string]bool{} + for _, a := range ompProfiles(home) { + got[a.ID] = true + } + if len(got) != 1 || !got["omp#fresh"] { + t.Fatalf("profiles %v", got) + } + f, err := Find("omp#fresh") + if err != nil { + t.Fatal(err) + } + if !f.Detected() { + t.Fatal("a profile folder is not detected") + } + if err := f.Field("model").Set("magpie/relay/glm-4.6"); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(filepath.Join(fresh, "agent", "config.yml")); err != nil { + t.Fatalf("fresh config: %v", err) + } + if _, err := os.Stat(filepath.Join(root, "agent", "config.yml")); !os.IsNotExist(err) { + t.Fatalf("the default row's config was made: %v", err) + } + for _, name := range []string{"work.", "aux", "nul", "com1", "lpt9.x", "con.work", "COM1"} { + if ompProfileOK(name) { + t.Fatalf("%q is a profile", name) + } + } + if d := parseProbe("U", "home:/home/me\nprofile:omp:aux\nprofile:omp:con.work\nprofile:omp:work.\n"); d == nil || len(d.OmpProfiles) != 0 { + t.Fatalf("probe kept a refused name: %+v", d) + } +} + +// OMP_PROFILE points the default row at profiles//agent, and that +// profile is not listed a second time; PI_CONFIG_DIR renames the root the +// profiles are read under. +func TestOmpProfilesDefaultRowAndConfigDir(t *testing.T) { + home := syncHome(t) + work := ompProfileAt(t, home, "work", "b/model-b") + ompProfileAt(t, home, "lab", "c/model-c") + + t.Setenv("OMP_PROFILE", "work") + d, err := Find("omp") + if err != nil { + t.Fatal(err) + } + if d.Dir != work { + t.Fatalf("default Dir %q", d.Dir) + } + got := map[string]bool{} + for _, a := range ompProfiles(home) { + got[a.ID] = true + } + if got["omp#work"] { + t.Fatal("the row's own profile is listed twice") + } + if !got["omp#lab"] { + t.Fatalf("profiles %v", got) + } + + // PI_CONFIG_DIR moves the root, profiles and all + t.Setenv("OMP_PROFILE", "") + os.Unsetenv("OMP_PROFILE") + t.Setenv("PI_CONFIG_DIR", ".omp-work") + renamed := filepath.Join(home, ".omp-work", "profiles", "home", "agent") + writeFile(t, filepath.Join(renamed, "config.yml"), "modelRoles:\n default: h/model-h\n") + if _, err := Find("omp#home"); err != nil { + t.Fatal(err) + } + if a, err := Find("omp"); err != nil || a.Dir != filepath.Join(home, ".omp-work", "agent") { + t.Fatalf("default under PI_CONFIG_DIR: %v %v", a, err) + } +} + +// The probe prints a named omp profile as profile:omp:, and only +// real directories under ~/.omp/profiles count. A profile row's place +// takes omp's version, which the probe keeps under the bare omp kind. +func TestParseProbeOmpProfiles(t *testing.T) { + d := parseProbe("U", "home:/home/me\ndir:.omp\nver:omp omp/16.5.1\n"+ + "profile:omp:work\nprofile:omp:Bad\nprofile:omp:work.\nprofile:pi:x\nprofile:omp:work\n") + if d == nil { + t.Fatal("no distro") + } + if len(d.OmpProfiles) != 1 || d.OmpProfiles[0] != "work" { + t.Fatalf("profiles %v", d.OmpProfiles) + } + if v := d.place("omp#work@wsl:U").version; v != "16.5.1" { + t.Fatalf("profile place version %q", v) + } + if !strings.Contains(wslProbeScript, `for d in "$HOME"/.omp/profiles/*`) || + !strings.Contains(wslProbeScript, `echo "profile:omp:$(basename "$d")"`) { + t.Fatalf("the probe doesn't list profiles: %s", wslProbeScript) + } +} + +// A distro's profile names come from its probe, not from a Windows +// ReadDir: a running distro lists omp#work@wsl: even though the +// fake root has no profile folder, and picking its model writes that +// profile's files alone. +func TestWSLOmpProfilesFromProbe(t *testing.T) { + syncHome(t) + root := t.TempDir() + dhome := filepath.Join(root, "home", "me") + defCfg := filepath.Join(dhome, ".omp", "agent", "config.yml") + writeFile(t, defCfg, "modelRoles:\n default: a/model-a\n") + t.Cleanup(FakeWSL( + map[string]string{"Ubuntu": "home:/home/me\ndir:.omp\nver:omp omp/18.4.8\nprofile:omp:work\nprofile:omp:Bad\n"}, + map[string]string{"Ubuntu": root})) + + if _, err := os.Stat(filepath.Join(dhome, ".omp", "profiles")); !os.IsNotExist(err) { + t.Fatalf("the fake root has a profiles folder: %v", err) + } + var a *Agent + for _, x := range wslAgents() { + if x.ID == "omp#work@wsl:Ubuntu" { + a = x + } + } + if a == nil { + t.Fatal("no omp#work@wsl:Ubuntu") + } + if a.Name != "omp · work · WSL Ubuntu" || !a.Detected() || a.ListsFor() != "omp" { + t.Fatalf("%+v", a) + } + if a.Dir != filepath.Join(dhome, ".omp", "profiles", "work", "agent") { + t.Fatalf("Dir %q", a.Dir) + } + before := readFile(defCfg) + if err := a.Field("model").Set("magpie/relay/glm-4.6"); err != nil { + t.Fatal(err) + } + workCfg := filepath.Join(dhome, ".omp", "profiles", "work", "agent", "config.yml") + if !strings.Contains(readFile(workCfg), "magpie/relay/glm-4.6") { + t.Fatalf("work config:\n%s", readFile(workCfg)) + } + if readFile(defCfg) != before { + t.Fatalf("the default distro omp moved:\n%s", readFile(defCfg)) + } + // the profile's own models.yml, not the default row's + if _, err := os.Stat(filepath.Join(dhome, ".omp", "agent", "models.yml")); !os.IsNotExist(err) { + t.Fatalf("the default row's models.yml was written: %v", err) + } +} + +// A stopped distro is never started to list its profiles: the names the +// probe remembered are still there, the row shows as asleep with no file +// read, and nothing of the distro is opened. +func TestWSLOmpProfilesStopped(t *testing.T) { + syncHome(t) + root := t.TempDir() + dhome := filepath.Join(root, "home", "me") + t.Cleanup(FakeWSL( + map[string]string{"Ubuntu": "home:/home/me\ndir:.omp\nver:omp omp/18.4.8\nprofile:omp:work\n"}, + map[string]string{"Ubuntu": root})) + if len(wslAgents()) == 0 { + t.Fatal("nothing listed while running") + } + StopFakeWSL("Ubuntu") + // the profile folder is gone: a stopped distro's row reads no file + if err := os.RemoveAll(filepath.Join(dhome, ".omp")); err != nil { + t.Fatal(err) + } + var a *Agent + for _, x := range wslAgents() { + if x.ID == "omp#work@wsl:Ubuntu" { + a = x + } + } + if a == nil { + t.Fatal("the profile row was dropped with the distro") + } + if a.Path != "" || a.Dir != "" { + t.Fatalf("asleep row reads files: Path %q Dir %q", a.Path, a.Dir) + } + if !a.Detected() { + t.Fatal("the remembered profile isn't detected") + } + if !strings.Contains(a.Notice(), "isn't running") { + t.Fatalf("notice %q", a.Notice()) + } +} diff --git a/internal/agent/wsl.go b/internal/agent/wsl.go index 2767756b..73b70803 100644 --- a/internal/agent/wsl.go +++ b/internal/agent/wsl.go @@ -140,6 +140,10 @@ type distro struct { // Versions is what the CLIs of wslKinds that ask (version) said their // versions are at the last probe, by the kind's id Versions map[string]string `json:"versions,omitempty"` + // OmpProfiles are the named omp profiles the probe found under + // ~/.omp/profiles there: each is an agent of its own (omp#). + // Kept so a stopped distro lists them without being started. + OmpProfiles []string `json:"ompProfiles,omitempty"` // Mirrored is whether the distro's 127.0.0.1 is Windows' (mirror): in // mirrored networking, or in consomme Mirrored bool `json:"-"` @@ -211,6 +215,12 @@ func (d distro) base() string { func (d distro) place(id string) place { kind, _, _ := strings.Cut(id, "@") + // a profile's row is omp#@wsl:, whose kind is + // omp#: the probe keeps the binary version under omp, so the + // lookup cuts the profile off + if k, _, ok := strings.Cut(kind, "#"); ok { + kind = k + } return place{home: d.local(d.Home), id: id, spell: d.native, sys: d.local, base: d.base, cold: !d.Running, version: d.Versions[kind]} } @@ -532,6 +542,9 @@ func wslFound(d distro) bool { // its row are the ones it is shown only when kept under that id too (#927). func (a *Agent) ListsFor() string { id, _, _ := strings.Cut(a.ID, "@wsl:") + // a named omp profile (omp#work) shares omp's lists: it is another + // omp, not another agent with a catalog of its own + id, _, _ = strings.Cut(id, "#") return id } @@ -722,6 +735,18 @@ func wslAgentsOf(ds []distro) []*Agent { out = append(out, a) } } + // the named omp profiles the probe remembered: the row is made + // because the probe found it, not because a default omp row is + // there, and its detection is that memory + for _, name := range d.OmpProfiles { + k := wslKindOf("omp") + k.id, k.name = "omp#"+name, "omp · "+name + k.in = func(at place) *Agent { + return ompAt(at, filepath.Join(at.home, ".omp", "profiles", name, "agent"), + func() ompProviderEntry { return ompProviderAt(at.gw(), at.version) }) + } + out = append(out, wslAgent(k, d)) + } } return out } @@ -1044,6 +1069,9 @@ var wslProbeScript = func() string { } // a drive's source in /proc/mounts is C:\ (written C:\134), under any automount root s += `awk '$1 ~ /^[A-Za-z]:/ {print "win:" $2}' /proc/mounts 2>/dev/null; ` + // named omp profiles: the probe runs only in a distro that is running, + // so this never starts a stopped one to find them + s += `for d in "$HOME"/.omp/profiles/*; do [ -d "$d" ] && echo "profile:omp:$(basename "$d")"; done; ` return s + `ip route show default 2>/dev/null | head -n1 | sed 's/^/route:/'; ` + `grep -m1 '^nameserver' /etc/resolv.conf 2>/dev/null | sed 's/^/ns:/'; ` + `command -v wslinfo >/dev/null 2>&1 && echo "net:$(wslinfo --networking-mode 2>/dev/null)"; true` @@ -1110,6 +1138,15 @@ func parseProbe(name, out string) *distro { if m := strings.ToLower(strings.TrimSpace(v)); m != "" && !strings.ContainsAny(m, " \t") { d.Net = m } + case "profile": + // profile:omp:: a named omp profile the distro has + kind, name, _ := strings.Cut(v, ":") + if kind != "omp" || !ompProfileOK(name) { + continue + } + if !slices.Contains(d.OmpProfiles, name) { + d.OmpProfiles = append(d.OmpProfiles, name) + } } } if !strings.HasPrefix(d.Home, "/") { diff --git a/internal/gui/agent_connect_test.go b/internal/gui/agent_connect_test.go index 9d069e3d..90aae2c2 100644 --- a/internal/gui/agent_connect_test.go +++ b/internal/gui/agent_connect_test.go @@ -71,6 +71,75 @@ func TestAgentConnectAPI(t *testing.T) { } } +// An omp profile's id is omp#, and the Agents page asks for it with +// the # encoded: left raw, a browser reads #work as the fragment and the +// request reaches the default omp row, whose config.yml then changes +// (#1187). Connecting and disconnecting the encoded id touches the +// profile's files alone. +func TestOmpProfileConnectURL(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) + t.Setenv("XDG_CACHE_HOME", filepath.Join(home, ".cache")) + def := filepath.Join(home, ".omp", "agent", "config.yml") + os.MkdirAll(filepath.Dir(def), 0o755) + os.WriteFile(def, []byte("modelRoles:\n default: own/model-a\n"), 0o600) + work := filepath.Join(home, ".omp", "profiles", "work", "agent") + os.MkdirAll(work, 0o755) + os.WriteFile(filepath.Join(work, "config.yml"), []byte("modelRoles:\n default: own/model-b\n"), 0o600) + if err := provider.Save(provider.Provider{ID: "relay", Name: "Relay", Key: "k", Chat: "http://127.0.0.1:1/v1", Models: []string{"m1"}}); err != nil { + t.Fatal(err) + } + h := Handler(nil, nil) + call := func(path string) stateJSON { + t.Helper() + rec := httptest.NewRecorder() + h.ServeHTTP(rec, httptest.NewRequest("POST", path, strings.NewReader("{}"))) + if rec.Code != 200 { + t.Fatalf("%s %d %s", path, rec.Code, rec.Body) + } + var s stateJSON + json.Unmarshal(rec.Body.Bytes(), &s) + return s + } + row := func(s stateJSON, id string) agentJSON { + for _, a := range s.Agents { + if a.ID == id { + return a + } + } + t.Fatalf("no %s", id) + return agentJSON{} + } + before, err := os.ReadFile(def) + if err != nil { + t.Fatal(err) + } + a := row(call("/api/agents/connect/omp%23work"), "omp#work") + if !a.Wired { + t.Fatal("omp#work isn't connected") + } + got, err := os.ReadFile(def) + if err != nil || string(got) != string(before) { + t.Fatalf("connecting the profile wrote the default omp config:\n%s", got) + } + if _, err := os.Stat(filepath.Join(home, ".omp", "agent", "models.yml")); !os.IsNotExist(err) { + t.Fatalf("connecting the profile wrote the default row's models.yml: %v", err) + } + profile, err := os.ReadFile(filepath.Join(work, "config.yml")) + if err != nil || !strings.Contains(string(profile), "magpie/relay/m1") { + t.Fatal("the profile's config wasn't connected") + } + s := call("/api/agents/disconnect/omp%23work") + if row(s, "omp#work").Wired || row(s, "omp").Wired { + t.Fatal("a row stayed connected") + } + if got, err = os.ReadFile(def); err != nil || string(got) != string(before) { + t.Fatalf("disconnecting the profile wrote the default omp config:\n%s", got) + } +} + // Claude Code's /model lists modelPicker's rows: connected, they are // magpie's models; switched off, they go, and a modelPicker the user wrote // is never touched. diff --git a/internal/gui/assets/app.js b/internal/gui/assets/app.js index f4f4ffb1..9bcfe6b5 100644 --- a/internal/gui/assets/app.js +++ b/internal/gui/assets/app.js @@ -70,6 +70,11 @@ const isFavorite = (o) => modelFavorites.has(favoriteKey(o)) || modelFavorites.h // heads, when given, is shown the answer's headers (loadQuotas' X-Magpie-Reading) async function api(path, body, heads) { if (web && path === "open") { window.open(body.url, "_blank", "noopener"); return null; } + // a segment may be an agent id, and omp's named profiles are omp#: + // left raw, the browser reads #work as the fragment and the request + // reaches the default omp row (#1187). Only # is encoded: ? = & stay a + // query string, which encodeURIComponent would turn into a 404. + path = path.replace(/#/g, "%23"); const res = await fetch("/api/" + path, { method: body === undefined ? "GET" : "POST", headers: { "Content-Type": "application/json" }, diff --git a/internal/gui/tests/README.md b/internal/gui/tests/README.md index adf3abef..0e33e955 100644 --- a/internal/gui/tests/README.md +++ b/internal/gui/tests/README.md @@ -2047,6 +2047,16 @@ magpie is added (green once it is), its button stands where the other rows' model picker does, as wide and lined up with it at 960 and 700px, and a click opens the app's link. English and Chinese, Chromium and WebKit. +`omp-profile-connect.test.cjs` checks an omp named profile's switch (#1187): +clicking **omp · work** posts `/api/agents/connect/omp%23work`, not a path the +browser cuts at `#`. A query string on the same page (`/api/usage?period=today`) +stays a query. English, Chromium and WebKit. The API is faked. Without the `#` +encoding in `api()` the posted path is `/api/agents/connect/omp`. + +```sh +node --test internal/gui/tests/omp-profile-connect.test.cjs +``` + `privacy-hides-accounts.test.cjs` checks that Privacy's Mask personal data hides the accounts on screen too (inaction on Discord): until Hide accounts is chosen on a computer it follows the setting, so Usage's email address is diff --git a/internal/gui/tests/omp-profile-connect.test.cjs b/internal/gui/tests/omp-profile-connect.test.cjs new file mode 100644 index 00000000..ba1b971b --- /dev/null +++ b/internal/gui/tests/omp-profile-connect.test.cjs @@ -0,0 +1,80 @@ +// Run with Node's test runner and Playwright on the module path; see README.md. +// An omp named profile is omp#. Left raw, the browser reads #work as +// the fragment and the connect reaches the default omp row (#1187). The +// switch must post /api/agents/connect/omp%23work. Encoding the whole path +// would also encode ? = &, so a query on the same page must stay a query. +// Without the # encoding this fails. English; no backend, the API is faked. +const assert = require("node:assert/strict"); +const fs = require("node:fs/promises"); +const path = require("node:path"); +const { test } = require("node:test"); +const { chromium, webkit } = require("playwright"); + +const assets = path.resolve(__dirname, "../assets"); +const options = [{ value: "gpt-5.4", label: "GPT-5.4" }, { value: "relay/m1", label: "m1", ref: "relay/m1", note: "Relay · via magpie" }]; +const fresh = () => ({ + agents: [ + { id: "omp", name: "omp", icon: "generic", path: "/fixture/omp", fields: [{ key: "model", label: "model", value: "gpt-5.4", options }] }, + { id: "omp#work", name: "omp · work", icon: "generic", path: "/fixture/omp/profiles/work", fields: [{ key: "model", label: "model", value: "gpt-5.4", options }] }, + ], + profiles: [], +}); + +function server(posts) { + let cur = fresh(); + const reply = (route) => route.fulfill({ json: { ...cur, settings: { lang: "en", theme: "light" } } }); + return async (route) => { + const url = new URL(route.request().url()); + const json = (data) => route.fulfill({ json: data }); + if (url.pathname === "/boot.js") return route.fulfill({ contentType: "text/javascript", body: `window.bootPrefs = {lang:"en",theme:"light",web:true};` }); + if (url.pathname === "/wails/runtime.js") return route.fulfill({ contentType: "text/javascript", body: "export const Window = {};" }); + if (url.pathname === "/api/state") return reply(route); + if (url.pathname === "/api/usage" && url.search === "?period=today") { + posts.push(url.pathname + url.search); + return json([]); + } + if (url.pathname === "/api/agents/connect/omp%23work" || url.pathname === "/api/agents/connect/omp") { + posts.push(url.pathname); + cur = JSON.parse(JSON.stringify(cur)); + const a = cur.agents.find((x) => x.id === decodeURIComponent(url.pathname.split("/").pop())); + if (!a) return route.fulfill({ status: 404, json: { error: "unknown agent" } }); + a.wired = true; + a.fields[0].value = "relay/m1"; + return reply(route); + } + if (url.pathname === "/api/usage/quotas") return json([]); + if (url.pathname === "/api/groups") return json({ groups: [] }); + if (url.pathname === "/api/plugins") return json({ plugins: [] }); + if (url.pathname === "/api/agents/cli") return json({ agents: {}, pending: false }); + if (url.pathname === "/api/providers") return json({ providers: [], presets: [], excluded: [], gateway: { running: true, window: true } }); + if (url.pathname.startsWith("/api/")) return json({}); + const file = path.join(assets, url.pathname === "/" ? "index.html" : url.pathname); + const contentType = { ".html": "text/html", ".js": "text/javascript", ".css": "text/css", ".svg": "image/svg+xml", ".png": "image/png" }[path.extname(file)]; + await route.fulfill({ body: await fs.readFile(file), contentType }); + }; +} + +for (const engine of (process.env.BROWSER ? [process.env.BROWSER] : ["chromium", "webkit"])) { + test(`${engine}: connecting omp#work posts the encoded id and leaves a query a query`, async (t) => { + const browser = await (engine === "webkit" ? webkit.launch() : chromium.launch({ channel: "chromium" })); + t.after(() => browser.close()); + const page = await (await browser.newContext({ viewport: { width: 980, height: 520 }, reducedMotion: "reduce" })).newPage(); + page.setDefaultTimeout(5000); + const errors = [], posts = [], dialogs = []; + page.on("pageerror", (e) => errors.push(e.message)); + page.on("dialog", (d) => { dialogs.push(d.message()); d.dismiss(); }); + await page.route("**/*", server(posts)); + await page.goto("http://magpie.test/?view=agents"); + const row = '.row.agent[data-id="omp#work"] .ag-conn'; + await page.locator(row).waitFor(); + await page.evaluate(() => api("usage?period=today")); + await page.locator(row).click(); + await page.waitForFunction((sel) => document.querySelector(sel)?.getAttribute("aria-checked") === "true", row); + assert.ok(posts.includes("/api/usage?period=today"), `query was encoded: ${posts}`); + assert.ok(!posts.some((p) => p.includes("%3F") || p.includes("%3D")), `a query character was encoded: ${posts}`); + assert.deepEqual(posts.filter((p) => p.startsWith("/api/agents/")), ["/api/agents/connect/omp%23work"]); + assert.equal(await page.locator('.row.agent[data-id="omp"] .ag-conn').getAttribute("aria-checked"), "false", "the default omp row stayed off"); + assert.deepEqual(dialogs, []); + assert.deepEqual(errors, []); + }); +}