diff --git a/groups_cli.go b/groups_cli.go index fd0b54cf..ca0454a7 100644 --- a/groups_cli.go +++ b/groups_cli.go @@ -760,7 +760,10 @@ func groupUses() map[string][]string { continue } v := strings.TrimPrefix(a.Fields[0].Get(), "magpie/") - if id, ok := strings.CutPrefix(v, provider.GroupPrefix); ok { + // a group an agent is on carries Claude Code's [1m] mark, as a + // model's does, and these are the agents' own settings: the mark + // comes off before the id is looked up by it (GroupFinder) + if id, ok := strings.CutPrefix(strings.TrimSuffix(strings.TrimSpace(v), "[1m]"), provider.GroupPrefix); ok { out[id] = append(out[id], a.Name) } } diff --git a/groups_cli_test.go b/groups_cli_test.go index f9ae50b6..ac11006c 100644 --- a/groups_cli_test.go +++ b/groups_cli_test.go @@ -3,6 +3,7 @@ package main import ( "encoding/json" "os" + "path/filepath" "strings" "testing" @@ -29,6 +30,36 @@ func groupsHome(t *testing.T) { } // storedGroups: the groups as the file keeps them. +// A group an agent is on carries Claude Code's [1m] mark in the agent's +// own settings, as a model's does: `magpie group` lists "← agent" beside a +// group by its id, so the mark has to come off the id before it is looked +// up. Claude Code on group/auto-gpt-5[1m] was on no row at all. +func TestGroupUsesTakesTheOneMMarkOff(t *testing.T) { + groupsHome(t) + if err := provider.SaveGroup(provider.Group{ID: "auto-gpt-5", Name: "GPT 5", Members: []string{"a/gpt-5"}}); err != nil { + t.Fatal(err) + } + // Claude Code's settings.json, as the reporter's reads + if err := os.MkdirAll(filepath.Join(os.Getenv("HOME"), ".claude"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(os.Getenv("HOME"), ".claude/settings.json"), + []byte(`{"model":"group/auto-gpt-5[1m]"}`), 0o644); err != nil { + t.Fatal(err) + } + uses := groupUses() + if got := uses["auto-gpt-5"]; len(got) != 1 || !strings.Contains(got[0], "Claude") { + t.Errorf("the group's agents: %q, all uses %q", got, uses) + } + if _, marked := uses["auto-gpt-5[1m]"]; marked { + t.Errorf("the marked id got its own row: %q", uses) + } +} + +// sameGroup compares two values of a field, one of them read back from the +// agent's own settings where the [1m] mark rides on the group's id: the +// marked one is the group magpie set, not another group. (internal/agent's +// own, tested there.) func storedGroups(t *testing.T) []map[string]any { t.Helper() b, err := os.ReadFile(provider.Path()) diff --git a/internal/agent/applied.go b/internal/agent/applied.go index 8159f240..3936a43a 100644 --- a/internal/agent/applied.go +++ b/internal/agent/applied.go @@ -256,7 +256,12 @@ func magpieValue(a *Agent, f Field, v string, vals map[string]string) bool { // that the gateway takes as a group's (provider.GroupFor: gpt-6.1-sol is // group/auto-gpt-6-1-sol); "" for any other. func groupNamed(v string) string { - v = strings.TrimPrefix(strings.TrimSpace(v), magpieID+"/") + // Claude Code's [1m] mark rides on the group's id as it does on a + // model's, and one of the two values here comes from the agent's own + // settings: without it off, a group read back marked is not the one + // magpie set (GroupFor takes it off; so does GroupFinder) + v = strings.TrimSuffix(strings.TrimSpace(v), "[1m]") + v = strings.TrimPrefix(v, magpieID+"/") if strings.HasPrefix(v, provider.GroupPrefix) { return v } diff --git a/internal/agent/applied_test.go b/internal/agent/applied_test.go index e59cdd4c..a96958fb 100644 --- a/internal/agent/applied_test.go +++ b/internal/agent/applied_test.go @@ -13,6 +13,28 @@ import ( "github.com/yetone/magpie/internal/usage" ) +// sameGroup compares two values of a field, one of them read back from the +// agent's own settings where Claude Code's [1m] mark rides on the group's +// id: the marked one is the group magpie set, not another group — the +// gateway serves either (#750's comparison, with the mark on one side). +func TestSameGroupTakesTheOneMMarkOff(t *testing.T) { + for _, pair := range [][2]string{ + {"group/auto-gpt-5", "group/auto-gpt-5[1m]"}, + {"group/auto-gpt-5[1m]", "group/auto-gpt-5"}, + {"group/auto-gpt-5[1m]", "magpie/group/auto-gpt-5"}, + } { + if !sameGroup(pair[0], pair[1]) { + t.Errorf("sameGroup(%q, %q) = false", pair[0], pair[1]) + } + } + if sameGroup("group/auto-gpt-5", "group/auto-other") { + t.Error("two different groups are the same") + } + if sameGroup("group/auto-gpt-5", "a/gpt-5") { + t.Error("a group and a model are the same") + } +} + // Something else rewrote Codex's config: the base URL gone while the model // is still magpie's is unwired, and setting it again wires it back. func TestCodexDriftUnwired(t *testing.T) { diff --git a/internal/agent/reseat.go b/internal/agent/reseat.go index 43c9b8c9..e2cc28f5 100644 --- a/internal/agent/reseat.go +++ b/internal/agent/reseat.go @@ -134,7 +134,9 @@ func served(ref string) bool { // down to one provider, it is that model from the first provider still // serving it; none for any other routing group, which is nobody else's. func sameModel(opts []Option, ref string) Option { - if gid, ok := strings.CutPrefix(ref, provider.GroupPrefix); ok { + // a group's id carries Claude Code's [1m] mark as a model's does, and + // this ref comes from the agent's own settings + if gid, ok := strings.CutPrefix(strings.TrimSuffix(strings.TrimSpace(ref), "[1m]"), provider.GroupPrefix); ok { if !strings.HasPrefix(gid, "auto-") { return Option{} } diff --git a/internal/agent/reseat_test.go b/internal/agent/reseat_test.go index 12e20511..6c2d1a22 100644 --- a/internal/agent/reseat_test.go +++ b/internal/agent/reseat_test.go @@ -28,6 +28,35 @@ func reseatHome(t *testing.T) string { return home } +// A group Claude Code is on carries its [1m] mark in the agent's own +// settings, as a model's does. sameModel reads that ref back when the +// group's model is down to one provider and the agent has to be moved to +// the same model elsewhere, so the mark has to come off there too: leaving +// it on compares "auto-gpt-5[1m]" with "auto-gpt-5" and finds nothing, +// and an agent whose group is gone is left on it. +func TestSameModelTakesTheOneMMarkOffAGroupRef(t *testing.T) { + reseatHome(t) + for _, p := range []provider.Provider{ + {ID: "other", Name: "Other", Chat: "https://other.example/v1", Key: "k", Models: []string{"gpt-5"}}, + } { + if err := provider.Save(p); err != nil { + t.Fatal(err) + } + } + opts := []Option{{Value: "magpie/group/auto-gpt-5", Ref: "group/auto-gpt-5"}, {Value: "other/gpt-5", Ref: "other/gpt-5"}} + if got := sameModel(opts, "group/auto-gpt-5[1m]"); got.Ref != "other/gpt-5" { + t.Errorf("[1m] on the group: %+v", got) + } + if got := sameModel(opts, "group/auto-gpt-5"); got.Ref != "other/gpt-5" { + t.Errorf("the bare group: %+v", got) + } + // a group that is nobody else's still gives nothing (nothing to move to) + none := []Option{{Value: "magpie/group/mine", Ref: "group/mine"}, {Value: "other/gpt-5", Ref: "other/gpt-5"}} + if got := sameModel(none, "group/mine[1m]"); got.Ref != "" { + t.Errorf("a group of the user's own: %+v", got) + } +} + func mustFind(t *testing.T, id string) *Agent { t.Helper() a, err := Find(id) diff --git a/internal/gui/providers_group_uses_test.go b/internal/gui/providers_group_uses_test.go new file mode 100644 index 00000000..0d270ce1 --- /dev/null +++ b/internal/gui/providers_group_uses_test.go @@ -0,0 +1,91 @@ +package gui + +import ( + "net/http" + "net/http/httptest" + "os" + "testing" + + "github.com/yetone/magpie/internal/agent" + "github.com/yetone/magpie/internal/provider" +) + +// An agent on a routing group whose ref carries Claude Code's [1m] mark is +// on that group: the gateway asks GroupFor, which takes the mark off, and +// the Providers page asks GroupFinder, which did not — so the page listed +// the agent on none of the group's providers while every request it made +// went through them. +func TestGroupFinderTakesTheOneMMark(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + t.Setenv("USERPROFILE", os.Getenv("HOME")) + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) + t.Setenv("XDG_CACHE_HOME", t.TempDir()) + p := provider.Provider{ID: "relay", Name: "Relay", Chat: "http://127.0.0.1:1/v1", Key: "key", + Models: []string{"m1"}} + if err := provider.Save(p); err != nil { + t.Fatal(err) + } + if err := provider.SaveGroup(provider.Group{ID: "m1", Name: "M1", Members: []string{"relay/m1"}}); err != nil { + t.Fatal(err) + } + + find := provider.GroupFinder() + for _, ref := range []string{"group/m1", "group/m1[1m]"} { + g, members, ok := find(ref) + if !ok { + t.Errorf("%s: the group is not found, though the gateway routes it", ref) + continue + } + if g.ID != "m1" { + t.Errorf("%s: found %q", ref, g.ID) + } + if len(members) != 1 || members[0].Provider.ID != "relay" || members[0].Model != "m1" { + t.Errorf("%s: members %+v", ref, members) + } + } + // the gateway's own way of asking, for the same ref: it takes the mark + // off already, which is the difference this page was on the wrong side of + if id, ok := provider.GroupFor("m1[1m]"); !ok || id != "group/m1" { + t.Errorf("GroupFor: %q %v", id, ok) + } +} + +// The Providers page row: an agent on a group is shown on each of that +// group's providers, however its ref is marked. +func TestProviderInfoShowsAnAgentOnItsGroupsProviders(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + t.Setenv("USERPROFILE", os.Getenv("HOME")) + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) + t.Setenv("XDG_CACHE_HOME", t.TempDir()) + up := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Write([]byte(`{"data":[{"id":"m1"}]}`)) + })) + defer up.Close() + p := provider.Provider{ID: "relay", Name: "Relay", Chat: up.URL + "/v1", Key: "key"} + if err := provider.Save(p); err != nil { + t.Fatal(err) + } + if _, err := p.Fetch(t.Context()); err != nil { + t.Fatal(err) + } + if err := provider.SaveGroup(provider.Group{ID: "g", Name: "G", Members: []string{"relay/m1"}}); err != nil { + t.Fatal(err) + } + + a := &agent.Agent{ID: "claude", Name: "Claude Code", Icon: "claude-color", Bin: "claude"} + a.Fields = []agent.Field{{Key: "model", Label: "model", Get: func() string { return "magpie/group/g[1m]" }}} + uses := agentUses([]*agent.Agent{a}, provider.GroupFinder()) + if !uses[0].inGroup { + t.Fatalf("the agent is on a group: %+v", uses[0]) + } + if len(uses[0].members) != 1 { + t.Fatalf("members %+v", uses[0].members) + } + j := providerInfo(p, uses) + if len(j.Agents) != 1 || !j.Agents[0].Current || j.Agents[0].Group != "G" { + t.Fatalf("the row tells of no agent on it: %+v", j.Agents) + } + if j.Agents[0].Model != "m1" { + t.Errorf("model %q", j.Agents[0].Model) + } +} diff --git a/internal/provider/group.go b/internal/provider/group.go index 25eaf015..6a464e5e 100644 --- a/internal/provider/group.go +++ b/internal/provider/group.go @@ -491,7 +491,10 @@ func GroupFinder() func(id string) (Group, []Member, bool) { read bool ) return func(id string) (Group, []Member, bool) { - gid, ok := strings.CutPrefix(strings.TrimSpace(id), GroupPrefix) + // the mark Claude Code puts on a 1M window rides on the group's + // id, as it does on a model's: GroupFor takes it off for the + // gateway, and a ref the gateway routes is a group here too + gid, ok := strings.CutPrefix(strings.TrimSuffix(strings.TrimSpace(id), "[1m]"), GroupPrefix) if !ok { return Group{}, nil, false }