From 39fc78d45dfa6350e15c8d9f190b7dbd7d84cc6e Mon Sep 17 00:00:00 2001 From: Tim Erwin Date: Thu, 3 Sep 2026 16:59:08 +0700 Subject: [PATCH 1/7] deps: clear the two dependency advisories grpc v1.82.0 (GO-2026-6061) and x/text v0.38.0 (GO-2026-5970), both indirect, both with fixes available. These ship regardless of which Go toolchain builds the release, unlike the stdlib advisories govulncheck also reports, which the CI toolchain already covers. --- go.mod | 14 +++++++------- go.sum | 36 ++++++++++++++++++------------------ 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/go.mod b/go.mod index c10ce8a..5ab8d5e 100644 --- a/go.mod +++ b/go.mod @@ -26,10 +26,10 @@ require ( github.com/spf13/cobra v1.10.2 go.yaml.in/yaml/v4 v4.0.0-rc.2 golang.org/x/image v0.43.0 - golang.org/x/net v0.56.0 + golang.org/x/net v0.58.0 golang.org/x/oauth2 v0.36.0 - golang.org/x/sys v0.46.0 - golang.org/x/term v0.44.0 + golang.org/x/sys v0.47.0 + golang.org/x/term v0.45.0 google.golang.org/genai v1.63.0 modernc.org/sqlite v1.52.0 mvdan.cc/sh/v3 v3.13.1 @@ -88,12 +88,12 @@ require ( go.opentelemetry.io/otel v1.44.0 // indirect go.opentelemetry.io/otel/metric v1.44.0 // indirect go.opentelemetry.io/otel/trace v1.44.0 // indirect - golang.org/x/crypto v0.53.0 // indirect - golang.org/x/sync v0.21.0 // indirect - golang.org/x/text v0.38.0 // indirect + golang.org/x/crypto v0.55.0 // indirect + golang.org/x/sync v0.22.0 // indirect + golang.org/x/text v0.41.0 // indirect google.golang.org/api v0.287.1 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20260630182238-925bb5da69e7 // indirect - google.golang.org/grpc v1.82.0 // indirect + google.golang.org/grpc v1.83.2 // indirect google.golang.org/protobuf v1.36.11 // indirect modernc.org/libc v1.72.3 // indirect modernc.org/mathutil v1.7.1 // indirect diff --git a/go.sum b/go.sum index 444346e..68cceb9 100644 --- a/go.sum +++ b/go.sum @@ -195,29 +195,29 @@ go.yaml.in/yaml/v4 v4.0.0-rc.2/go.mod h1:aZqd9kCMsGL7AuUv/m/PvWLdg5sjJsZ4oHDEnfP golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20210421170649-83a5a9bb288b/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= -golang.org/x/crypto v0.53.0 h1:QZ4Muo8THX6CizN2vPPd5fBGHyogrdK9fG4wLPFUsto= -golang.org/x/crypto v0.53.0/go.mod h1:DNLU434OwVakk9PzuwV8w62mAJpRJL3vsgcfp4Qnsio= +golang.org/x/crypto v0.55.0 h1:+KWHjbgOaAQ66dh/YlkZKHlz9ZUlq61AFirAR9ntP8M= +golang.org/x/crypto v0.55.0/go.mod h1:uq0V9dE/fzQuJtbnL+2EhWOE63vo164FY8xqEnV9xis= golang.org/x/exp v0.0.0-20231006140011-7918f672742d h1:jtJma62tbqLibJ5sFQz8bKtEM8rJBtfilJ2qTU199MI= golang.org/x/exp v0.0.0-20231006140011-7918f672742d/go.mod h1:ldy0pHrwJyGW56pPQzzkH36rKxoZW1tw7ZJpeKx+hdo= golang.org/x/image v0.43.0 h1:FLxcP4ec2350nTfOC8ysKtqYSIFbk/QGjw1ZHNP4tsY= golang.org/x/image v0.43.0/go.mod h1:rrpelvGFt+kLPAjPM4HeWPgrl0FtafueU//e5N0qk/Q= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= -golang.org/x/mod v0.36.0 h1:JJjpVx6myfUsUdAzZuOSTTmRE0PfZeNWzzvKrP7amb4= -golang.org/x/mod v0.36.0/go.mod h1:moc6ELqsWcOw5Ef3xVprK5ul/MvtVvkIXLziUOICjUQ= +golang.org/x/mod v0.38.0 h1:MECBjubtXD7yj4HrhIUcywNaGeNVUdfVnxmPajOk4yk= +golang.org/x/mod v0.38.0/go.mod h1:V6Xz0pq8TQ3dGqVQ1FVHuelZpAL0uNhSkk9ogYP3c40= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= -golang.org/x/net v0.56.0 h1:Rw8j/hFzGvJUZwNBXnAtf5sVDVt+65SK2C7IxCxZt5o= -golang.org/x/net v0.56.0/go.mod h1:D3Ku6r+V6JROoZK144D2XfMHFcMq/0zSfLelVTCFKec= +golang.org/x/net v0.58.0 h1:ynWG7rqYi4ccpTEuPZ2QGWHktVEM9DMCj9yzDE0Q7To= +golang.org/x/net v0.58.0/go.mod h1:YwCddHnFlT7eLQqVprV19OnhLGtc5xOKgE0RyqgfWAU= golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.21.0 h1:HLII4xRRTtCRkxYp4HNFF0Js/Og6q2i++KXbg0gHCwM= -golang.org/x/sync v0.21.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= +golang.org/x/sync v0.22.0 h1:SZjpbeLmrCk4xhRSZFNZW5gFUeCeFgjekvI/+gfScek= +golang.org/x/sync v0.22.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= @@ -225,26 +225,26 @@ golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.46.0 h1:noSf2Fq6F8DBgS+LysIkx7rIExoNHJsxOAtPp4rthXw= -golang.org/x/sys v0.46.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= +golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= -golang.org/x/term v0.44.0 h1:0rLvDRCtNj0gZkyIXhCyOb2OAzEhLVqc4B+hrsBhrmc= -golang.org/x/term v0.44.0/go.mod h1:7ze4MdzUzLXpSAoFP1H0bOI9aXDqveSvatT5vKcFh2Y= +golang.org/x/term v0.45.0 h1:NwWyBmoJCbfTHpxrWoZ9C6/VxOf7ic219I8xZZFdrf0= +golang.org/x/term v0.45.0/go.mod h1:9aqxs0blBcrm/n0L9QW0aRVD+ktan8ssZromtqJC43w= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= -golang.org/x/text v0.38.0 h1:sXmwo9DwP3OK9EZ7PqAdaooSGozfl/3a6/xJcbzPRhE= -golang.org/x/text v0.38.0/go.mod h1:YXZt3QhHUKYT53r2lLKFIVi6Ao1jdzrTR/KQ09qyxF4= +golang.org/x/text v0.41.0 h1:vz/seA0lnX87Othu2f/0L24RcgrXD9/YFTSuGjj3rH8= +golang.org/x/text v0.41.0/go.mod h1:jvf1O8ajNzZqhSrQBPbutR/EB83Cc0CFrezNQIwbb5M= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= -golang.org/x/tools v0.45.0 h1:18qN3FAooORvApf5XjCXgsuayZOEtXf6JK18I3+ONa8= -golang.org/x/tools v0.45.0/go.mod h1:LuUGqqaXcXMEFEruIVJVm5mgDD8vww/z/SR1gQ4uE/0= +golang.org/x/tools v0.48.0 h1:3+hClM1aLL5mjMKm5ovokw9epgRXPuu2tILgismM6RE= +golang.org/x/tools v0.48.0/go.mod h1:08xX0orndb/F7jJxGDicx061tyd5pcMto75YMAXr6lk= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gonum.org/v1/gonum v0.17.0 h1:VbpOemQlsSMrYmn7T2OUvQ4dqxQXU+ouZFQsZOx50z4= gonum.org/v1/gonum v0.17.0/go.mod h1:El3tOrEuMpv2UdMrbNlKEh9vd86bmQ6vqIcDwxEOc1E= @@ -258,8 +258,8 @@ google.golang.org/genproto/googleapis/api v0.0.0-20260630182238-925bb5da69e7 h1: google.golang.org/genproto/googleapis/api v0.0.0-20260630182238-925bb5da69e7/go.mod h1:KqHwBx2upmfa1XSi1WuRvC+2VGCLtooKkfmyvRbUmqA= google.golang.org/genproto/googleapis/rpc v0.0.0-20260630182238-925bb5da69e7 h1:eM/YSd5bBFagF51o1E745Ta7RwzpW0h+z+QDNZOgmQ8= google.golang.org/genproto/googleapis/rpc v0.0.0-20260630182238-925bb5da69e7/go.mod h1:4Hqkh8ycfw05ld/3BWL7rJOSfebL2Q+DVDeRgYgxUU8= -google.golang.org/grpc v1.82.0 h1:vguDnZUPjE26w09A63VoxZPnvPjB5Riyc0mkXPFmAIU= -google.golang.org/grpc v1.82.0/go.mod h1:yzTZ1TB1Z3SG+LIYaI+WiE8D5+PZ3ArnrSp8zF3+/ZA= +google.golang.org/grpc v1.83.2 h1:EManeRomTObA0BU7I8vXgg/78uE5MJ9M8B39EX2WscU= +google.golang.org/grpc v1.83.2/go.mod h1:YPI1hK3kDked6iHvgX3tR0y+nX/qpMFKhPgFsokw1S8= google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE= google.golang.org/protobuf v1.36.11/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= From 3849bd0e7f307da6e0b2694ef5e601a2a499bc17 Mon Sep 17 00:00:00 2001 From: Tim Erwin Date: Thu, 3 Sep 2026 16:59:08 +0700 Subject: [PATCH 2/7] vxui: the permission mode actually persists now MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit config.Mode has always documented itself as "persisted when the user cycles it", and cmd/interactive.go has always READ it at startup. Nothing ever wrote it back, so Shift+Tab and /mode silently reset on every restart — a setting that quietly forgets, which is the hardest kind of bug to notice. Both entry points now route through one setMode that changes the session and saves the config together, with a guard test that fails if a third call site skips the save. --- internal/vxui/app.go | 10 ++++- internal/vxui/commands.go | 2 +- internal/vxui/modepersist_test.go | 69 +++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 2 deletions(-) create mode 100644 internal/vxui/modepersist_test.go diff --git a/internal/vxui/app.go b/internal/vxui/app.go index a168af0..338c3e3 100644 --- a/internal/vxui/app.go +++ b/internal/vxui/app.go @@ -1000,10 +1000,18 @@ func (s *appState) cycleMode() { case permissions.ModeAllowAll: next = permissions.ModeAsk } - s.w.sess.SetMode(next) + s.setMode(next) s.SetState(func() {}) } +// setMode changes the permission mode AND persists it, which is what +// config.Mode has always documented ("persisted when cycled or /mode"). Both +// entry points — this cycle and /mode — go through here so they cannot drift. +func (s *appState) setMode(m permissions.Mode) { + s.w.sess.SetMode(m) + s.updateConfig(func(cfg *config.Config) { cfg.Mode = string(m) }) +} + // answerApproval replies to the blocked engine goroutine and clears the prompt. // HandleEvent is the sole input handler. The composer is rendered (not a focusable TextField) diff --git a/internal/vxui/commands.go b/internal/vxui/commands.go index b21cc98..b0c4529 100644 --- a/internal/vxui/commands.go +++ b/internal/vxui/commands.go @@ -49,7 +49,7 @@ func (s *appState) runSlash(line string) (quit bool) { case "": s.cycleMode() case "ask", "auto", "allow-all": - s.w.sess.SetMode(permissions.Mode(args)) + s.setMode(permissions.Mode(args)) default: s.sysln("usage: /mode ask|auto|allow-all") return false diff --git a/internal/vxui/modepersist_test.go b/internal/vxui/modepersist_test.go new file mode 100644 index 0000000..60ce5c8 --- /dev/null +++ b/internal/vxui/modepersist_test.go @@ -0,0 +1,69 @@ +package vxui + +import ( + "go/ast" + "go/parser" + "go/token" + "os" + "path/filepath" + "strings" + "testing" +) + +// TestModeChangesGoThroughSetMode is the mechanical form of "the permission mode +// persists". config.Mode has always documented itself as "persisted when the user +// cycles it", and cmd/interactive.go has always READ it at startup — but nothing +// wrote it back, so Shift+Tab and /mode silently reset on every restart. +// +// The write now lives in exactly one place, setMode, which changes the session and +// saves the config together. This guard keeps it that way: a second call site that +// skips the save would reintroduce the bug invisibly, since the only symptom is a +// setting that quietly forgets. +func TestModeChangesGoThroughSetMode(t *testing.T) { + files, err := filepath.Glob("*.go") + if err != nil { + t.Fatal(err) + } + fset := token.NewFileSet() + var offenders []string + + for _, path := range files { + if strings.HasSuffix(path, "_test.go") { + continue + } + src, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + f, err := parser.ParseFile(fset, path, src, 0) + if err != nil { + t.Fatalf("%s: %v", path, err) + } + // Walk each function; record any SetMode call outside setMode itself. + for _, decl := range f.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Name.Name == "setMode" { + continue + } + ast.Inspect(fn, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok { + return true + } + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok || sel.Sel.Name != "SetMode" { + return true + } + offenders = append(offenders, + fset.Position(call.Pos()).String()+" (in "+fn.Name.Name+")") + return true + }) + } + } + + if len(offenders) > 0 { + t.Errorf("SetMode called outside setMode — these changes will not persist:\n %s\n\n"+ + "Route the change through setMode so the session and the config move together.", + strings.Join(offenders, "\n ")) + } +} From 3a8573d311d3fb52780e3f20554effaf99ff2a3e Mon Sep 17 00:00:00 2001 From: Tim Erwin Date: Thu, 3 Sep 2026 16:59:09 +0700 Subject: [PATCH 3/7] config: a pin that cannot be recorded says so ResolvePin promised the seed was "persisted to BOTH stores before returning, so this is the only run that ever consults default_model". Both writes discarded their errors, and the user-store writer dropped four without returning anything. Best-effort persistence is right and stays: failing to remember a preference must never fail the operation. But silence was wrong. If neither store is writable, every run re-seeds from default_model, and because that value legitimately changes as models are added and retired, a user's model can drift between releases while everything here believes the pin is stable. ResolvePinSeeded reports that case so the caller can say it once; ResolvePin keeps the old signature for callers that cannot act on it. --- cmd/interactive.go | 5 ++- cmd/run.go | 5 ++- internal/config/pin.go | 66 +++++++++++++++++++++++++++---------- internal/config/pin_test.go | 47 ++++++++++++++++++++++++++ 4 files changed, 104 insertions(+), 19 deletions(-) diff --git a/cmd/interactive.go b/cmd/interactive.go index 06abfc6..685c235 100644 --- a/cmd/interactive.go +++ b/cmd/interactive.go @@ -78,7 +78,10 @@ func runInteractive(ctx context.Context, mode permissions.Mode, modeExplicit boo // The session's model: session override -> workspace -> user -> the // default_model seed (persisted on first use). ResolvePin is the ONLY // place that chain lives. - pin, win := config.ResolvePin(cfg, "") + pin, win, warn := config.ResolvePinSeeded(cfg, "") + if warn != nil { + fmt.Fprintf(os.Stderr, "warning: %v\n", warn) + } sess.SetPin(pin, win) sess.SetPolicy(sessionPolicy(cfg.Root, pin)) } diff --git a/cmd/run.go b/cmd/run.go index 77c2cf5..2885857 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -144,7 +144,10 @@ for local gateway development. Never store keys in .memcode.`, } if !onEndpoint { modelFlag, _ := cmd.Flags().GetString("model") - pin, win := config.ResolvePin(cfg, modelFlag) + pin, win, warn := config.ResolvePinSeeded(cfg, modelFlag) + if warn != nil { + fmt.Fprintf(os.Stderr, "warning: %v\n", warn) + } sess.SetPin(pin, win) sess.SetPolicy(sessionPolicy(cfg.Root, pin)) // The header must name the model that will actually serve. It used diff --git a/internal/config/pin.go b/internal/config/pin.go index d6b4467..7ead796 100644 --- a/internal/config/pin.go +++ b/internal/config/pin.go @@ -2,6 +2,8 @@ package config import ( "encoding/json" + "errors" + "fmt" "os" "path/filepath" @@ -75,31 +77,31 @@ func loadUserPrefs() prefsFile { // writeUserPrefs persists the whole prefs file. Callers mutate a loaded copy so // one pin never clobbers the other — changing the primary must not silently // reset an explicitly configured delegated model. -func writeUserPrefs(p prefsFile) { +func writeUserPrefs(p prefsFile) error { path := UserPrefsPath() if path == "" { - return + return errors.New("no user config directory") } if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { - return + return err } b, err := json.MarshalIndent(p, "", " ") if err != nil { - return + return err } - _ = atomicfile.WriteFile(path, append(b, '\n'), 0o600) + return atomicfile.WriteFile(path, append(b, '\n'), 0o600) } // SaveUserPin records the PRIMARY model at the USER level, so a different repo // starts on the same one. Best-effort by design: failing to remember a // preference must never fail the operation the user actually asked for. -func SaveUserPin(label string, window int) { +func SaveUserPin(label string, window int) error { if label == "" { - return + return nil } p := loadUserPrefs() p.PinnedModel, p.PinnedWindow = label, window - writeUserPrefs(p) + return writeUserPrefs(p) } // ResolvePin returns the model this session runs on, plus its context window. @@ -108,37 +110,67 @@ func SaveUserPin(label string, window int) { // this invocation's model, not a new preference. // // When resolution reaches the seed, the pin is persisted to BOTH stores before -// returning, so this is the only run that ever consults default_model. +// returning, so this is normally the only run that ever consults default_model. +// +// Persistence is BEST-EFFORT and deliberately so: failing to remember a +// preference must never fail the operation the user actually asked for. But a +// failure is not nothing — if neither store is writable, every run re-seeds from +// default_model, and because that value legitimately changes as models are added +// and retired, the user's model can drift between releases while everything here +// believes the pin is stable. ResolvePinSeeded reports that case so a caller can +// say so once; ResolvePin keeps the plain signature for callers that cannot act +// on it anyway. func ResolvePin(cfg *Config, override string) (label string, window int) { + label, window, _ = ResolvePinSeeded(cfg, override) + return label, window +} + +// ResolvePinSeeded is ResolvePin plus the seed-persistence outcome: warn is +// non-nil only when this run had to seed from default_model AND could not record +// it, which means the next run will seed again. +func ResolvePinSeeded(cfg *Config, override string) (label string, window int, warn error) { if override != "" { - return override, catalog.ContextWindow(override) + return override, catalog.ContextWindow(override), nil } if cfg != nil && cfg.PinnedModel != "" { - return cfg.PinnedModel, cfg.PinnedWindow + return cfg.PinnedModel, cfg.PinnedWindow, nil } if p := loadUserPrefs(); p.PinnedModel != "" { // Remembered at the user level but not in this workspace — adopt it here - // too, so the workspace answers for itself next time. + // too, so the workspace answers for itself next time. A failure here is + // not reported: the pin IS remembered, just not yet in this workspace, + // so the next run still resolves to the same model. if cfg != nil { cfg.PinnedModel, cfg.PinnedWindow = p.PinnedModel, p.PinnedWindow _ = cfg.Save() } - return p.PinnedModel, p.PinnedWindow + return p.PinnedModel, p.PinnedWindow, nil } seed := catalog.DefaultModel() if seed == "" { // No seed declared: return empty and let selection refuse with its own // message. Inventing a model here is the one thing this must not do. - return "", 0 + return "", 0, nil } w := catalog.ContextWindow(seed) + + // Seeding is the one branch whose whole purpose is to not happen again, so + // this is where a write failure actually costs something. Report it only if + // BOTH stores failed — either one alone still pins the model for next time. + var wsErr, usrErr error if cfg != nil { cfg.PinnedModel, cfg.PinnedWindow = seed, w - _ = cfg.Save() + wsErr = cfg.Save() + } else { + wsErr = errors.New("no workspace config") + } + usrErr = SaveUserPin(seed, w) + if wsErr != nil && usrErr != nil { + warn = fmt.Errorf("could not record the model pin (workspace: %v; user: %v) — "+ + "this session runs on %s, but the next one will seed again", wsErr, usrErr, seed) } - SaveUserPin(seed, w) - return seed, w + return seed, w, warn } // The DELEGATED pin lived here briefly and moved to internal/policy as the diff --git a/internal/config/pin_test.go b/internal/config/pin_test.go index b7b8049..2882a1b 100644 --- a/internal/config/pin_test.go +++ b/internal/config/pin_test.go @@ -3,6 +3,7 @@ package config import ( "os" "path/filepath" + "strings" "testing" "github.com/memcode-ai/memcode/catalog" @@ -116,3 +117,49 @@ func TestCorruptUserPrefsDegradeToTheSeed(t *testing.T) { t.Fatalf("corrupt prefs = %q, want the seed", label) } } + +// TestSeedPersistFailureIsReported: seeding is the one branch whose purpose is to +// not happen again, so when it cannot be recorded the caller must be able to say +// so. Silence here means the model can drift between releases (default_model +// legitimately changes) while everything believes the pin is stable. +func TestSeedPersistFailureIsReported(t *testing.T) { + // Point the user store at a regular FILE, so creating a directory under it + // fails the way a read-only home or a full disk would. No workspace config + // either, so both stores are unwritable. + dir := t.TempDir() + blocker := filepath.Join(dir, "blocked") + if err := os.WriteFile(blocker, []byte("not a directory"), 0o600); err != nil { + t.Fatal(err) + } + t.Setenv("XDG_CONFIG_HOME", blocker) + t.Setenv("HOME", blocker) + + label, _, warn := ResolvePinSeeded(nil, "") + if label != catalog.DefaultModel() { + t.Fatalf("must still resolve to the seed, got %q", label) + } + if warn == nil { + t.Fatal("both stores failed — that must be reported, not swallowed") + } + if !strings.Contains(warn.Error(), "seed again") { + t.Errorf("the warning must say what happens next, got %q", warn) + } +} + +// TestSeedPersistSuccessIsQuiet: the normal path reports nothing, and one store +// succeeding is enough — the pin is remembered either way. +func TestSeedPersistSuccessIsQuiet(t *testing.T) { + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) + cfg := &Config{Root: t.TempDir()} + if _, _, warn := ResolvePinSeeded(cfg, ""); warn != nil { + t.Errorf("a successful seed must be silent, got %v", warn) + } +} + +// TestOverrideNeverWarns: --model is this invocation's model, never a preference, +// so it writes nothing and cannot fail. +func TestOverrideNeverWarns(t *testing.T) { + if _, _, warn := ResolvePinSeeded(nil, "haiku"); warn != nil { + t.Errorf("an override persists nothing, so it cannot warn: %v", warn) + } +} From 71c8df1c8790d9ba208c6548b965e27b5dcf7773 Mon Sep 17 00:00:00 2001 From: Tim Erwin Date: Thu, 3 Sep 2026 16:59:24 +0700 Subject: [PATCH 4/7] test: cover the webhook token verifier and the explore helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit webjwt is the single verifier for platform-signed inbound webhook tokens (Bot Framework, Google Chat) and had no tests at all. "Fails closed" is a claim worth proving on an authentication boundary, so this exercises each way a token can be wrong: expired, unexpiring, wrong issuer, wrong audience, unknown signing key, unknown kid, alg-none, HMAC against an RSA key set, unconfigured verifier, and an unreachable JWKS. It also pins the stated flood defense — 25 forged kids must not become 25 outbound fetches — and the Bot Framework metadata path. 87% coverage on a package that had none. explore covers the concurrency-cap resolution (0 means default, not zero readers) and the finding indenter. --- internal/explore/explore_test.go | 51 +++++++ internal/webjwt/webjwt_test.go | 245 +++++++++++++++++++++++++++++++ 2 files changed, 296 insertions(+) create mode 100644 internal/explore/explore_test.go create mode 100644 internal/webjwt/webjwt_test.go diff --git a/internal/explore/explore_test.go b/internal/explore/explore_test.go new file mode 100644 index 0000000..9e034f1 --- /dev/null +++ b/internal/explore/explore_test.go @@ -0,0 +1,51 @@ +package explore + +import ( + "strings" + "testing" +) + +// TestReadersResolvesTheConcurrencyCap: 0 means "nothing configured", which must +// land on the default rather than on zero readers — a caller with no policy layer +// wired up would otherwise silently fan out to nobody. +func TestReadersResolvesTheConcurrencyCap(t *testing.T) { + for _, tc := range []struct { + name string + in int + want int + }{ + {"unconfigured", 0, defaultReaders}, + {"negative is treated as unset", -3, defaultReaders}, + {"explicit single reader", 1, 1}, + {"explicit override", 12, 12}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := readers(tc.in); got != tc.want { + t.Errorf("readers(%d) = %d, want %d", tc.in, got, tc.want) + } + }) + } +} + +// TestIndentPreservesStructure: findings are indented into the synthesis prompt, +// so a blank answer must not become a line of stray spaces and a multi-line +// answer must keep every line aligned. +func TestIndentPreservesStructure(t *testing.T) { + if got := indent(""); got != "" { + t.Errorf("empty stays empty, got %q", got) + } + if got := indent("one"); got != " one" { + t.Errorf("single line = %q", got) + } + + got := indent("first\nsecond\n\nfourth") + want := " first\n second\n \n fourth" + if got != want { + t.Errorf("multi-line indent =\n%q\nwant\n%q", got, want) + } + for i, line := range strings.Split(got, "\n") { + if !strings.HasPrefix(line, " ") { + t.Errorf("line %d lost its indent: %q", i, line) + } + } +} diff --git a/internal/webjwt/webjwt_test.go b/internal/webjwt/webjwt_test.go new file mode 100644 index 0000000..6818c7d --- /dev/null +++ b/internal/webjwt/webjwt_test.go @@ -0,0 +1,245 @@ +package webjwt + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "encoding/base64" + "encoding/json" + "math/big" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/golang-jwt/jwt/v5" +) + +// This package is the authentication boundary for inbound platform webhooks +// (Bot Framework, Google Chat). "Fails closed" is a claim worth proving rather +// than asserting, so these tests exercise each way a token can be wrong. + +const ( + testIssuer = "https://issuer.example/v1" + testAud = "aud-under-test" + testKID = "key-1" +) + +// jwksServer serves a JWKS containing key, counting how many times it is hit so +// the refresh throttle can be observed. +type jwksServer struct { + *httptest.Server + hits atomic.Int64 +} + +func newJWKS(t *testing.T, kid string, pub *rsa.PublicKey) *jwksServer { + t.Helper() + js := &jwksServer{} + js.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + js.hits.Add(1) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{"keys": []map[string]string{{ + "kty": "RSA", + "kid": kid, + "n": base64.RawURLEncoding.EncodeToString(pub.N.Bytes()), + "e": base64.RawURLEncoding.EncodeToString(big.NewInt(int64(pub.E)).Bytes()), + }}}) + })) + t.Cleanup(js.Close) + return js +} + +// signed mints a compact RS256 token with the given claims and kid. +func signed(t *testing.T, key *rsa.PrivateKey, kid string, claims jwt.MapClaims) string { + t.Helper() + tok := jwt.NewWithClaims(jwt.SigningMethodRS256, claims) + tok.Header["kid"] = kid + raw, err := tok.SignedString(key) + if err != nil { + t.Fatal(err) + } + return raw +} + +func goodClaims() jwt.MapClaims { + return jwt.MapClaims{ + "iss": testIssuer, + "aud": testAud, + "exp": time.Now().Add(10 * time.Minute).Unix(), + "iat": time.Now().Add(-time.Minute).Unix(), + } +} + +func newKey(t *testing.T) *rsa.PrivateKey { + t.Helper() + k, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) + } + return k +} + +// TestVerifyAcceptsAWellFormedToken is the baseline: without it, every negative +// test below could pass for the wrong reason. +func TestVerifyAcceptsAWellFormedToken(t *testing.T) { + key := newKey(t) + js := newJWKS(t, testKID, &key.PublicKey) + v := &Verifier{JWKSURL: js.URL, Issuer: testIssuer, Audience: testAud} + + if err := v.Verify(context.Background(), signed(t, key, testKID, goodClaims())); err != nil { + t.Fatalf("a valid token must verify: %v", err) + } +} + +// TestVerifyFailsClosed walks every way a token can be wrong. Each case must be +// an error — a webhook verifier that accepts any of these is an open door. +func TestVerifyFailsClosed(t *testing.T) { + key := newKey(t) + other := newKey(t) + js := newJWKS(t, testKID, &key.PublicKey) + + expired := goodClaims() + expired["exp"] = time.Now().Add(-time.Minute).Unix() + + unexpiring := goodClaims() + delete(unexpiring, "exp") + + wrongIss := goodClaims() + wrongIss["iss"] = "https://attacker.example" + + wrongAud := goodClaims() + wrongAud["aud"] = "someone-elses-app" + + for _, tc := range []struct { + name string + tok string + why string + }{ + {"expired", signed(t, key, testKID, expired), "an expired token is replayable forever"}, + {"no expiry at all", signed(t, key, testKID, unexpiring), "a token that never expires is a permanent credential"}, + {"wrong issuer", signed(t, key, testKID, wrongIss), "another platform could mint tokens for this endpoint"}, + {"wrong audience", signed(t, key, testKID, wrongAud), "a token for a different app would be accepted"}, + {"signed by an unknown key", signed(t, other, testKID, goodClaims()), "anyone could sign their own tokens"}, + {"unknown kid", signed(t, key, "not-a-real-kid", goodClaims()), "the key must actually be published"}, + {"garbage", "not.a.token", "malformed input must not panic or pass"}, + {"empty", "", "an absent token must not verify"}, + } { + t.Run(tc.name, func(t *testing.T) { + v := &Verifier{JWKSURL: js.URL, Issuer: testIssuer, Audience: testAud} + if err := v.Verify(context.Background(), tc.tok); err == nil { + t.Errorf("must be rejected — %s", tc.why) + } + }) + } +} + +// TestUnsignedTokenIsRejected covers the alg-confusion classic: a token with +// "alg":"none", or one signed with a symmetric algorithm, must never verify +// against an RSA key set. +func TestUnsignedTokenIsRejected(t *testing.T) { + key := newKey(t) + js := newJWKS(t, testKID, &key.PublicKey) + v := &Verifier{JWKSURL: js.URL, Issuer: testIssuer, Audience: testAud} + + none := jwt.NewWithClaims(jwt.SigningMethodNone, goodClaims()) + none.Header["kid"] = testKID + raw, err := none.SignedString(jwt.UnsafeAllowNoneSignatureType) + if err != nil { + t.Fatal(err) + } + if err := v.Verify(context.Background(), raw); err == nil { + t.Error(`an "alg":"none" token must never verify`) + } + + hs := jwt.NewWithClaims(jwt.SigningMethodHS256, goodClaims()) + hs.Header["kid"] = testKID + if rawHS, err := hs.SignedString([]byte("guessable")); err == nil { + if err := v.Verify(context.Background(), rawHS); err == nil { + t.Error("an HMAC-signed token must not verify against an RSA key set") + } + } +} + +// TestMissingConfigurationNeverVerifies: an unconfigured verifier must reject +// everything rather than defaulting to permissive. +func TestMissingConfigurationNeverVerifies(t *testing.T) { + key := newKey(t) + js := newJWKS(t, testKID, &key.PublicKey) + tok := signed(t, key, testKID, goodClaims()) + + for _, tc := range []struct { + name string + v *Verifier + }{ + {"no issuer", &Verifier{JWKSURL: js.URL, Audience: testAud}}, + {"no audience", &Verifier{JWKSURL: js.URL, Issuer: testIssuer}}, + {"no key source", &Verifier{Issuer: testIssuer, Audience: testAud}}, + } { + t.Run(tc.name, func(t *testing.T) { + if err := tc.v.Verify(context.Background(), tok); err == nil { + t.Error("an unconfigured verifier must fail closed") + } + }) + } +} + +// TestUnknownKidFloodIsThrottled pins the stated defense: an unknown kid may +// trigger at most one refetch per interval, so forged random kids cannot be +// amplified into unbounded traffic against the platform's JWKS host. +func TestUnknownKidFloodIsThrottled(t *testing.T) { + key := newKey(t) + js := newJWKS(t, testKID, &key.PublicKey) + v := &Verifier{JWKSURL: js.URL, Issuer: testIssuer, Audience: testAud} + + for i := 0; i < 25; i++ { + _ = v.Verify(context.Background(), signed(t, key, "forged-kid", goodClaims())) + } + if hits := js.hits.Load(); hits > 2 { + t.Errorf("25 forged kids caused %d JWKS fetches; the throttle should bound this to ~1", hits) + } + + // The throttle must not lock out a legitimate token already in the key set. + if err := v.Verify(context.Background(), signed(t, key, testKID, goodClaims())); err != nil { + t.Errorf("a known kid must still verify during a flood: %v", err) + } +} + +// TestMetadataURLIsFollowed covers the Bot Framework path: the verifier reads +// jwks_uri out of an OpenID configuration document rather than being handed the +// JWKS endpoint directly. +func TestMetadataURLIsFollowed(t *testing.T) { + key := newKey(t) + js := newJWKS(t, testKID, &key.PublicKey) + + meta := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]string{"jwks_uri": js.URL}) + })) + defer meta.Close() + + v := &Verifier{MetadataURL: meta.URL, Issuer: testIssuer, Audience: testAud} + if err := v.Verify(context.Background(), signed(t, key, testKID, goodClaims())); err != nil { + t.Fatalf("metadata discovery must reach the JWKS: %v", err) + } +} + +// TestUnreachableJWKSFailsClosed: if the key source is down, tokens are rejected +// rather than admitted. +func TestUnreachableJWKSFailsClosed(t *testing.T) { + key := newKey(t) + dead := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "down", http.StatusInternalServerError) + })) + defer dead.Close() + + v := &Verifier{JWKSURL: dead.URL, Issuer: testIssuer, Audience: testAud} + err := v.Verify(context.Background(), signed(t, key, testKID, goodClaims())) + if err == nil { + t.Fatal("an unreachable key source must reject, never admit") + } + if strings.Contains(strings.ToLower(err.Error()), "panic") { + t.Errorf("unexpected failure shape: %v", err) + } +} From c2ee6a8f27e283ce93904d2bbc9c4dc7ca8695df Mon Sep 17 00:00:00 2001 From: Tim Erwin Date: Thu, 3 Sep 2026 16:59:24 +0700 Subject: [PATCH 5/7] setsim: one Jaccard instead of three MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The same function was written three times, in two map shapes, across prefs clustering, mood's repetition detector, and lesson dedup — and had already begun to diverge: the lessons copy had dropped a guard the other two kept. (Unreachable, as it happens, but divergence is the point.) A metric that decides whether two things are "the same" should have one definition and one set of tests. Generic over the value type so callers keep whichever set spelling they already use. --- internal/agent/mood/mood.go | 21 ++----------- internal/lessons/lessons.go | 18 ++--------- internal/prefs/clustering.go | 22 ++----------- internal/setsim/setsim.go | 34 ++++++++++++++++++++ internal/setsim/setsim_test.go | 57 ++++++++++++++++++++++++++++++++++ 5 files changed, 100 insertions(+), 52 deletions(-) create mode 100644 internal/setsim/setsim.go create mode 100644 internal/setsim/setsim_test.go diff --git a/internal/agent/mood/mood.go b/internal/agent/mood/mood.go index d1b192d..0b17a91 100644 --- a/internal/agent/mood/mood.go +++ b/internal/agent/mood/mood.go @@ -18,6 +18,8 @@ import ( "sort" "strings" "sync" + + "github.com/memcode-ai/memcode/internal/setsim" ) // State is the interaction state inferred for a turn (or the running aggregate). @@ -499,7 +501,7 @@ func (t *Tracker) Current() Reading { func (t *Tracker) repeatedNegative(toks map[string]struct{}) bool { for _, prev := range t.recent { - if jaccard(toks, prev) >= 0.5 { + if setsim.Jaccard(toks, prev) >= 0.5 { return true } } @@ -607,23 +609,6 @@ func tokenSet(text string) map[string]struct{} { return m } -func jaccard(a, b map[string]struct{}) float64 { - if len(a) == 0 || len(b) == 0 { - return 0 - } - inter := 0 - for k := range a { - if _, ok := b[k]; ok { - inter++ - } - } - union := len(a) + len(b) - inter - if union == 0 { - return 0 - } - return float64(inter) / float64(union) -} - func appendUniq(s []string, v string) []string { for _, x := range s { if x == v { diff --git a/internal/lessons/lessons.go b/internal/lessons/lessons.go index fbe3f22..5953756 100644 --- a/internal/lessons/lessons.go +++ b/internal/lessons/lessons.go @@ -32,6 +32,8 @@ import ( "github.com/memcode-ai/memcode/internal/atomicfile" "github.com/memcode-ai/memcode/internal/events" "github.com/memcode-ai/memcode/internal/store" + + "github.com/memcode-ai/memcode/internal/setsim" ) const ( @@ -172,7 +174,7 @@ func clusterSignals(signals []Signal) [][]Signal { tk := tokenSet(s.Trigger + " " + s.Strategy) placed := false for i := range clusters { - if jaccard(tokens[i], tk) >= jaccardThreshold { + if setsim.Jaccard(tokens[i], tk) >= jaccardThreshold { clusters[i] = append(clusters[i], s) for w := range tk { // grow the cluster's vocabulary tokens[i][w] = true @@ -448,20 +450,6 @@ func tokenSet(text string) map[string]bool { return out } -func jaccard(a, b map[string]bool) float64 { - if len(a) == 0 || len(b) == 0 { - return 0 - } - inter := 0 - for w := range a { - if b[w] { - inter++ - } - } - union := len(a) + len(b) - inter - return float64(inter) / float64(union) -} - func slug(text string, n int) string { words := strings.FieldsFunc(strings.ToLower(text), func(r rune) bool { return !(r >= 'a' && r <= 'z') && !(r >= '0' && r <= '9') diff --git a/internal/prefs/clustering.go b/internal/prefs/clustering.go index 917ff21..432cfbb 100644 --- a/internal/prefs/clustering.go +++ b/internal/prefs/clustering.go @@ -3,6 +3,8 @@ package prefs import ( "sort" "strings" + + "github.com/memcode-ai/memcode/internal/setsim" ) // cluster groups signals by axis, then within each axis groups by lexical @@ -67,7 +69,7 @@ func jaccardGroup(signals []signalEvent) [][]signalEvent { toks := tokens(sig.Text) merged := false for i, ct := range clusterTokens { - if jaccard(toks, ct) >= jaccardThreshold { + if setsim.Jaccard(toks, ct) >= jaccardThreshold { clusters[i] = append(clusters[i], sig) // Refresh the representative tokens so later signals can match the // growing cluster. @@ -99,24 +101,6 @@ func tokens(s string) map[string]bool { return set } -// jaccard is the token-set Jaccard similarity |A∩B| / |A∪B|. -func jaccard(a, b map[string]bool) float64 { - if len(a) == 0 || len(b) == 0 { - return 0 - } - inter := 0 - for t := range a { - if b[t] { - inter++ - } - } - union := len(a) + len(b) - inter - if union == 0 { - return 0 - } - return float64(inter) / float64(union) -} - // polarity returns +1 for affirmative directives ("always", "use", "prefer") and // -1 for negated ones ("never", "stop", "don't", "no more", "avoid"). Same-axis // signals with opposite polarity are contradictions, not merges. diff --git a/internal/setsim/setsim.go b/internal/setsim/setsim.go new file mode 100644 index 0000000..8b03632 --- /dev/null +++ b/internal/setsim/setsim.go @@ -0,0 +1,34 @@ +// Package setsim holds set-similarity metrics shared by the layers that cluster +// text: preference clustering, mood's repetition detector, and lesson dedup. +// +// It exists because the same function had been written three times, in two map +// shapes, and had already started to diverge — one copy had dropped a guard the +// others kept. A similarity metric that decides whether two things are "the +// same" is exactly the kind of code that should have one definition and one set +// of tests. +package setsim + +// Jaccard is the intersection-over-union of two sets, in [0,1]. +// +// The value type is free so callers can keep whichever map shape they already +// use — map[string]bool and map[string]struct{} are both idiomatic set spellings +// and neither should have to convert to share this. +// +// Two empty sets score 0, not 1: callers use this to ask "is this new thing a +// repeat of that one", and an empty token set carries no evidence either way. +// Scoring it as a perfect match would make every content-free input look like a +// duplicate of every other. +func Jaccard[V any](a, b map[string]V) float64 { + if len(a) == 0 || len(b) == 0 { + return 0 + } + inter := 0 + for k := range a { + if _, ok := b[k]; ok { + inter++ + } + } + // len(a) and len(b) are both > 0 here, so union >= 1 and the division is safe. + union := len(a) + len(b) - inter + return float64(inter) / float64(union) +} diff --git a/internal/setsim/setsim_test.go b/internal/setsim/setsim_test.go new file mode 100644 index 0000000..12a9834 --- /dev/null +++ b/internal/setsim/setsim_test.go @@ -0,0 +1,57 @@ +package setsim + +import "testing" + +func set(keys ...string) map[string]bool { + m := map[string]bool{} + for _, k := range keys { + m[k] = true + } + return m +} + +func structSet(keys ...string) map[string]struct{} { + m := map[string]struct{}{} + for _, k := range keys { + m[k] = struct{}{} + } + return m +} + +func TestJaccard(t *testing.T) { + for _, tc := range []struct { + name string + a, b map[string]bool + want float64 + }{ + {"identical", set("a", "b"), set("a", "b"), 1}, + {"disjoint", set("a"), set("b"), 0}, + {"half overlap", set("a", "b"), set("b", "c"), 1.0 / 3.0}, + {"subset", set("a"), set("a", "b"), 0.5}, + {"both empty", set(), set(), 0}, + {"one empty", set("a"), set(), 0}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := Jaccard(tc.a, tc.b); got != tc.want { + t.Errorf("Jaccard = %v, want %v", got, tc.want) + } + }) + } +} + +// TestJaccardIsSymmetric: a similarity metric that depended on argument order +// would make clustering results depend on iteration order. +func TestJaccardIsSymmetric(t *testing.T) { + a, b := set("x", "y", "z"), set("y", "q") + if Jaccard(a, b) != Jaccard(b, a) { + t.Error("Jaccard must be symmetric") + } +} + +// TestJaccardAcceptsEitherSetSpelling is the point of the generic: the three +// call sites this replaced used two different map shapes. +func TestJaccardAcceptsEitherSetSpelling(t *testing.T) { + if got := Jaccard(structSet("a", "b"), structSet("b", "c")); got != 1.0/3.0 { + t.Errorf("map[string]struct{} = %v, want 1/3", got) + } +} From 07e3e03a9d2d49b8e153a50af1157b8765cd5af2 Mon Sep 17 00:00:00 2001 From: Tim Erwin Date: Thu, 3 Sep 2026 16:59:24 +0700 Subject: [PATCH 6/7] clean: remove ten unreachable functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reachability analysis across the module, tests included, found ten functions nothing can call. All are in internal/ packages, so there are no external consumers to consider. The interesting group was three package-level wrappers in autonomy whose comment said "used by cmd" — cmd never used them. The methods they wrapped are live and stay. deadcode now reports nothing. --- internal/agent/autonomy/interactions.go | 11 ----------- internal/buildinfo/buildinfo.go | 15 --------------- internal/cloudclient/client.go | 3 --- internal/explore/explore.go | 3 --- internal/providers/gemini/gemini.go | 12 ------------ internal/repofiles/repofiles.go | 7 ------- internal/sessionlog/sessionlog.go | 22 ---------------------- internal/structure/load.go | 3 --- 8 files changed, 76 deletions(-) diff --git a/internal/agent/autonomy/interactions.go b/internal/agent/autonomy/interactions.go index 8d57679..ed6b182 100644 --- a/internal/agent/autonomy/interactions.go +++ b/internal/agent/autonomy/interactions.go @@ -98,14 +98,3 @@ func (s *Store) CancelInteraction(ctx context.Context, id string) error { _, err := s.db.ExecContext(ctx, `UPDATE interactions SET status='cancelled' WHERE id=? AND status='pending'`, id) return err } - -// Package-level wrappers used by cmd (store passed explicitly). -func PendingInteractions(s *Store, agentID string) ([]Interaction, error) { - return s.PendingInteractions(context.Background(), agentID) -} -func GetInteraction(s *Store, id string) (Interaction, bool, error) { - return s.GetInteraction(context.Background(), id) -} -func ResolveInteraction(s *Store, id, answer string) error { - return s.ResolveInteraction(context.Background(), id, answer) -} diff --git a/internal/buildinfo/buildinfo.go b/internal/buildinfo/buildinfo.go index 0abd5dc..1094bce 100644 --- a/internal/buildinfo/buildinfo.go +++ b/internal/buildinfo/buildinfo.go @@ -10,7 +10,6 @@ import ( "fmt" "os" "runtime/debug" - "time" ) // baseVersion is the current development semver — the version the NEXT release will carry. @@ -80,20 +79,6 @@ func String() string { return fmt.Sprintf("%s (commit %s, built %s)", v, c, d) } -// Short is a compact identifier — "dev · a1b2c3d-dirty · built 11:02:05" — enough -// to tell two builds apart at a glance. -func Short() string { - v, c, d := resolve() - if c == "none" { - return v - } - t := d - if parsed, err := time.Parse("2006-01-02 15:04:05", d); err == nil { - t = parsed.Format("15:04:05") // just the time - } - return fmt.Sprintf("%s · %s · built %s", v, c, t) -} - // Compact is the build identifier for the always-on footer — always a REAL version. A clean // release shows just its semver (e.g. "1.2.3"); a dev build shows the synthesized // baseVersion-dev+commit (e.g. "0.1.0-dev+a1b2c3d-dirty"), where the commit is the per-build diff --git a/internal/cloudclient/client.go b/internal/cloudclient/client.go index 341df9f..65072b4 100644 --- a/internal/cloudclient/client.go +++ b/internal/cloudclient/client.go @@ -39,9 +39,6 @@ type Client struct { // Option configures a Client. type Option func(*Client) -// WithHTTPClient overrides the default HTTP client (for tests or a custom timeout). -func WithHTTPClient(h *http.Client) Option { return func(c *Client) { c.http = h } } - // WithRetryNotify registers a callback invoked before each retry backoff, so a // caller (the runtime loop) can surface "⊙ retrying…" in the TUI. The attempt is // 1-based (1 = first retry); err is the failure that triggered the retry; delay diff --git a/internal/explore/explore.go b/internal/explore/explore.go index b9e02cf..15a3e3d 100644 --- a/internal/explore/explore.go +++ b/internal/explore/explore.go @@ -82,9 +82,6 @@ func fanOut(ctx context.Context, st store.Store, runner *llm.Runner, root, model // scope), so the UI can show compact per-agent progress instead of every call. type Progress func(scope string, done bool, err error) -// Scopes is the public picker for the subsystem keys to fan out over. -func Scopes(ctx context.Context, st store.Store) []string { return pickScopes(ctx, st) } - // FanOut runs one read-only explorer per scope concurrently (capped by // concurrency, 0 = the default), reporting lifecycle via progress (may be nil), and returns the // findings. Each explorer is its own read-only session writing to io.Discard — diff --git a/internal/providers/gemini/gemini.go b/internal/providers/gemini/gemini.go index 27f4482..d01b93e 100644 --- a/internal/providers/gemini/gemini.go +++ b/internal/providers/gemini/gemini.go @@ -78,18 +78,6 @@ func NewGemini(apiKey string) *Gemini { } } -// NewGeminiVertex returns a client that runs on Vertex AI using a GCP service -// account JSON key. project is the GCP project -// ID; location is the Vertex AI region (e.g. "global" or "us-central1"). -func NewGeminiVertex(serviceAccountJSON []byte, project, location string) *Gemini { - return &Gemini{ - serviceAccountJSON: serviceAccountJSON, - project: project, - location: location, - http: provcore.NewTurnHTTPClient(), - } -} - // SetBaseURL points the adapter at a different Gemini host (tests, proxies). // "" restores the SDK default. func (g *Gemini) SetBaseURL(u string) { g.baseURL = u } diff --git a/internal/repofiles/repofiles.go b/internal/repofiles/repofiles.go index 952dea8..f093520 100644 --- a/internal/repofiles/repofiles.go +++ b/internal/repofiles/repofiles.go @@ -8,7 +8,6 @@ package repofiles import ( "context" "io/fs" - "os" "os/exec" "path" "path/filepath" @@ -123,9 +122,3 @@ func walkList(ctx context.Context, root string) []string { }) return files } - -// Exists reports whether path (relative to root) is a real, non-ignored file. -func Exists(root, rel string) bool { - info, err := os.Stat(filepath.Join(root, rel)) - return err == nil && !info.IsDir() -} diff --git a/internal/sessionlog/sessionlog.go b/internal/sessionlog/sessionlog.go index 258ad3d..921ff73 100644 --- a/internal/sessionlog/sessionlog.go +++ b/internal/sessionlog/sessionlog.go @@ -776,28 +776,6 @@ func LessonSignals(root string) ([]Record, error) { return out, nil } -// AdherenceRecords returns every adherence record across all sessions, oldest -// first — the reducers' backfill path for adherence weighting (files canonical, -// SQLite derived; same contract as LessonSignals). -func AdherenceRecords(root string) ([]Record, error) { - refs, err := sessionRefs(root) - if err != nil { - return nil, err - } - var out []Record - for _, ref := range refs { - recs, _ := readRecords(ref.path) - for _, r := range recs { - if r.Kind == KindAdherence { - r.SessionID = ref.id - out = append(out, r) - } - } - } - sort.Slice(out, func(i, j int) bool { return out[i].TS.Before(out[j].TS) }) - return out, nil -} - // SessionRecords returns the full record list of ONE session by id, oldest first // — the post-session learning loop reads a finished session's trail to build the // adherence digest. (nil, nil) when the session has no log. diff --git a/internal/structure/load.go b/internal/structure/load.go index ed3db74..9452602 100644 --- a/internal/structure/load.go +++ b/internal/structure/load.go @@ -61,8 +61,5 @@ func Load(ctx context.Context, s store.Store) (Result, error) { return res, nil } -// EntityID returns the entity id for a subsystem key. -func EntityID(key string) string { return subsystemPrefix + key } - // SubsystemKey strips the "subsystem:" prefix from an entity id. func SubsystemKey(id string) string { return strings.TrimPrefix(id, subsystemPrefix) } From 1b36ccce8d70da1939f046fd419d86027bd6d988 Mon Sep 17 00:00:00 2001 From: Tim Erwin Date: Thu, 3 Sep 2026 16:59:39 +0700 Subject: [PATCH 7/7] docs: describe the system that exists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v0.29.0 removed automatic model routing. The code recorded that carefully, with a tombstone comment at every site explaining what went and why. The documents around the code were never given the same treatment, so they kept advertising the deleted system in the present tense. README - "Out of the box it uses cheap models for routine work and strong models when the task is hard or risky" described the deleted ladder. It was also the only false claim a prospective user reads before installing anything. Replaced with what is true and arguably the better pitch: one model, chosen by you, that does not change underneath you, with delegated work able to ride its own pin. - /plan no longer "gets a second model's review before you approve it" on its own; that review is a one-shot the user triggers from the approval card. ROUTING.md — rewritten. Every symbol it documented as current (decideLane, ResolveModel, SteerResolvedModel) has zero references in non-test code, and it carried its own warning that finding those names means something is stale. It now describes the pin chain, the two narrow exceptions (utility plumbing and infrastructure failure), why capability gaps refuse rather than substitute, and which error classes may walk a fallback chain. COMPACTION.md — the budget is window-relative (85% of learned capacity), not the ~45K constant this claimed. Worth correcting beyond accuracy: an absolute default is precisely what the current design rejects, so the stale number misrepresented a deliberate position as a tunable. The summarizer is the catalog's utility_model, not a forced Anthropic call. Two file headers contradicted correct comments lower in the same file: llm/lanes.go said Automatic selection prefers keyed vendors, and llm/resolve.go described steering as live policy while resolveHosted below it explained the deletion. --- COMPACTION.md | 23 +++- README.md | 6 +- ROUTING.md | 296 ++++++++++++++++++---------------------- internal/llm/lanes.go | 14 +- internal/llm/resolve.go | 20 +-- 5 files changed, 175 insertions(+), 184 deletions(-) diff --git a/COMPACTION.md b/COMPACTION.md index 8b86152..7da19df 100644 --- a/COMPACTION.md +++ b/COMPACTION.md @@ -2,9 +2,19 @@ Status: **SHIPPED (CLI), 2026-06-10.** Auto-compaction + manual `/compact` are live. memcode used to send the FULL append-only `ChatState.messages` every turn; -now, at a safe turn boundary, when the estimated prompt exceeds the budget -(`MEMCODE_COMPACT_BUDGET`, default ~45K; `off` disables), the older turns are -summarized by Anthropic into a warm block and only the last ~8 turns stay raw. +now, at a safe turn boundary, when the estimated prompt exceeds the budget, the +older turns are summarized into a warm block and only the last ~8 turns stay raw. + +**The budget is window-RELATIVE, not a constant.** It is 85% of the serving +model's learned input capacity, falling back to 80% of the catalog window before +any turn has revealed it. The whole window minus headroom is the budget: context +pressure (evictions, the cache busts and re-reads they cause) is the expensive +failure, and resident tokens ride cheaply as cache reads. +`MEMCODE_COMPACT_BUDGET` is an explicit override (`off` disables compaction) and +`MEMCODE_CONTEXT_SOFT_CAP` lowers the ceiling for cost-capped setups; there is +deliberately no built-in absolute clip. An earlier revision of this document +named a ~45K default, which is exactly the kind of absolute constant the current +design rejects. ## Where it lives (built) @@ -14,7 +24,8 @@ summarized by Anthropic into a warm block and only the last ~8 turns stay raw. (`compaction_test.go`): facts-survive, adjacency-never-broken, boundary-only. - `internal/agent/runtime/compact.go` — orchestration: `compactBudget`, `compactIfNeeded` (auto, fired from `Submit` before the turn is assembled), - `Compact` (manual /compact), the Anthropic-forced summarizer call, the + `Compact` (manual /compact), the summarizer call (`compact` is a utility + purpose, so it rides the catalog's `utility_model` rather than the pin), the synthetic summary turn, telemetry + episodic-log write. - `compact` mode + `compactDoctrine` in `internal/doctrine/prompts.go` (the compactor prompt is composed client-side by the doctrine composer, like every @@ -96,7 +107,9 @@ session still compacts to stay cheap and fast. Coarse is fine. ## Hard rules -1. **Compactor model = Anthropic** (v1). A bad summary becomes the session's truth. +1. **The compactor model is the catalog's `utility_model`.** A bad summary + becomes the session's truth, so this is one of the few calls that does not + ride the user's pin. (Later: the cheap lane may summarize low-risk tool output.) 2. **Tool-use adjacency is sacred.** Never split an assistant tool_use from its tool_result. Only compact at a completed boundary (no pending tool call). diff --git a/README.md b/README.md index 8b5dcf9..590d3b4 100644 --- a/README.md +++ b/README.md @@ -32,11 +32,11 @@ Run `memcode` in a repo and you get a full terminal coding agent. **It remembers.** Ask it to pick up where you left off last week and it can. It knows your repo's layout, what has been tried before, and the preferences you have corrected it on. Memory lives in `.memcode`, so it travels with the repo and your whole team benefits. -**Pick a model or let it decide.** Out of the box it uses cheap models for routine work and strong models when the task is hard or risky. Pin any model with `/model` when you want control. +**One model, and it stays put.** You pick the model your session runs on and nothing swaps it out mid-task. Change it any time with `/model`. Sub-agents and scouts can run on a cheaper model of your choosing, so the expensive one is spent where you decided it should be. **Reads the room.** When you are correcting it, it slows down, asks before acting, and stops cutting corners. When things are calm it stays out of your way. -**Plan first when it matters.** `/plan` researches your codebase, drafts an approach, and gets a second model's review before you approve it. Execution then sticks to what you approved. +**Plan first when it matters.** `/plan` researches your codebase and drafts an approach for you to approve, and execution then sticks to what you approved. Send the draft to a second model for review first when the stakes are worth it. **Work in parallel.** Hand off side quests to sub-agents and background jobs, keep working, and check on them with `/jobs` and `/tail`. @@ -121,7 +121,7 @@ The user manual lives at [memcode.ai/docs](https://memcode.ai/docs): Internals and reference docs live in this repo: -- [ROUTING.md](ROUTING.md): how Automatic mode picks models. +- [ROUTING.md](ROUTING.md): how the session model is chosen, and what happens when a provider fails. - [HOOKS.md](HOOKS.md): the hook surface. - [COMPACTION.md](COMPACTION.md): context compaction. - [docs/gateway/README.md](docs/gateway/README.md): gateway operations and channel secrets. diff --git a/ROUTING.md b/ROUTING.md index 15b1168..21f07b5 100644 --- a/ROUTING.md +++ b/ROUTING.md @@ -1,167 +1,137 @@ -# memcode — Routing (the CLI is the agent; every backend is dumb serving) - -> **North star (2026-08-08, the all-policy-client-side migration):** the **CLI owns model -> policy** — the semantic ladder, physical model selection, BYOK steering, escalation, -> fallback, and mid-call recovery all run client-side (`internal/llm`). Every backend — -> the memcode gateway, Ollama, LM Studio, any OpenAI-compatible endpoint — is a **dumb -> serving surface**: it serves exactly the concrete model the agent asked for, or returns a -> **typed error**. This is the standard client-owns-policy shape (client-side model config + fallback -> chains), and it is what makes memcode behave **identically** across hosted, BYOK, local, -> and arbitrary-endpoint setups: one agent, one wire, one policy. +# memcode — Model selection (one pin per session) + +> **North star (v0.29.0, the routing removal):** there is **exactly one model per +> session**, and the **user chose it**. Nothing inspects a task's difficulty, risk, or +> cost to pick a model on the user's behalf. The pin is authoritative: what it names is +> what serves, or the turn fails with a reason. > -> The doctrine itself is unchanged: *spend intelligence only where uncertainty or risk -> demands it, and default toward CAPABLE when unsure — misrouting DOWN is the expensive -> failure.* What changed is WHERE it runs. +> Two mechanisms are allowed to run a call on something other than the pin, and both are +> deliberately narrow: **utility plumbing** (classify, compact, shrinkwrap) rides a +> declared `utility_model`, and **infrastructure failure** may fall back down a declared +> chain for the current call only. Neither may ever be reached from a judgment about the +> work. Anything that influences *which model runs* based on *what the work looks like* +> is routing, and routing is gone. > > Design/decision record; the **code is the source of truth** -> (`internal/llm/{lane.go,resolve.go,recover.go}` + the shared catalog -> `models.json` (repo root, synced) + the serving gateway, which is deployed -> separately from this repo). - -> **History:** the ladder lived server-side (the pre-fold monorepo's -> `api/internal/provider/{resolve,steer}.go`) from 2026-06 to 2026-08-08 — the port was -> proven against parity goldens -> generated from that code before its deletion (`internal/llm/testdata/*.json`, 6,608 -> rows reproduced exactly). Before that, an even earlier era self-hosted vLLM on GPUs; -> deleted at the 2026-06-12 Fireworks cutover. If you find "lanes", "decideLane", -> "ResolveModel", "SteerResolvedModel", or "MEMCODE_WIRE" anywhere, it is stale — `git log` -> has the old designs. - -## The division of responsibility - -**CLI (`internal/llm` — the agent, the single routing authority):** - -- **Semantic ladder** (`lane.go`): intent → lane. Inputs are all CLI-produced — purpose, - mode, the turn_intent judge's difficulty verdict, the room/risk signals, thinking effort. -- **Physical resolution** (`resolve.go`): lane → concrete catalog **label**, decided over - the hosted **routing control plane** (`GET /v1/models`: role config, per-model vendor, - capabilities, byok coverage, credits state) — or, in endpoint mode, the endpoint's - session model. -- **Steering** — BYOK-first: Automatic prefers vendors the user brought keys for; at $0 an - Automatic turn never targets an unfunded lane. Selection policy, not a server overlay. -- **Capability absorbs** — an image on a no-vision lane, a PDF on a lane without document - input, a prompt past the window: remapped client-side BEFORE the call, visibly - (`FallbackReason` feeds the ⇄ line). The gateway's typed errors are the backstop. -- **Recovery** (`recover.go`): on a model-class failure (after the transport's own - transient retries), walk the catalog **fallback chain** — current call only, next turn - re-selects the primary — and only while **nothing was emitted** to the user. - Billing/entitlement/key/auth/overflow errors are terminal for the chain: they carry - their own policy (compaction, the billing dialog, /apikeys, /login). -- **delegateDoctrine** — the cheap-coding-lane "hand non-code work to a strong agent" - fragment is appended post-selection (routing-owned prose lives with the routing - decision). - -**Gateway (deployed separately from this repo — a metered serving endpoint):** - -- Auth door, entitlement enforcement (subscription/lock/credits as **typed 402s** that - decline, never redirect), BYOK vault + key injection, metering/debit, rate limits. -- **Strict label gate**: `model` must be a servable catalog label — `auto`, vendor ids, - and typos are `400 unknown_model`. There is no server-side Automatic. -- **Typed errors, no absorbs**: `413 context_overflow`, `400 model_capability`, +> (`internal/config/pin.go`, `internal/llm/{resolve.go,recover.go}`, `internal/policy`, +> and the shared catalog `models.json`). + +> **History.** A semantic ladder chose a model per turn, first server-side (2026-06 to +> 2026-08-08), then ported into the CLI. v0.29.0 deleted it: lanes, roles, tier triples, +> BYOK steering, the $0 fundability remap, and capability substitution all went at once. +> The removal is recorded in tombstone comments at each site rather than here. If you +> find `decideLane`, `ResolveModel`, `SteerResolvedModel`, `laneFor`, or a *live* claim +> that selection prefers a vendor, it is stale — `git log` has the old designs. + +## Where the model comes from + +The pin resolver settles this once, at session start (`config.ResolvePin`): + +| Source | Persisted? | Notes | +|---|---|---| +| `--model` / the session override | **No** | This invocation's model, not a new preference | +| Workspace (`.memcode` config) | Yes | This repo's answer | +| User store | Yes | Adopted into the workspace on first use here | +| `default_model` (catalog) | **Seeds once**, then persisted | So only one run ever consults it | + +Each step is consulted only when the one above it is empty, so a session that has a pin +never re-derives one. If the seed cannot be recorded to either store, the run still works +and says so: the next run would otherwise seed again, and `default_model` legitimately +changes as models are added and retired. + +The pin is a catalog **label** (`sonnet`, `glm-5p2`), never a vendor id or an alias +resolved at call time. + +## What the pin governs + +Everything the user's work runs on: the main loop, plan drafting, delegated workers, and +scouts. `/model` changes it mid-session. + +**Delegated work** may run on a *different* pin, chosen by the user, through the policy +layer (`internal/policy`, target `agent.delegated`, which sub-agent and scout targets +inherit from). Unset means inherit the primary. This is the user spending the expensive +model where they decided it belongs, not the system deciding for them. + +## The two exceptions + +**1. Utility plumbing.** `classify`, `compact`, and `shrinkwrap` ride the catalog's +`utility_model`. These summarize and route internal state; none of them produces user +work, and none may select, substitute, escalate, downgrade, or steer the pinned model. If +no utility model is declared, the work falls through to the pin rather than inventing a +model: losing a classifier is better than a silent pick. + +**2. Infrastructure failure.** See below. + +Nothing else. A capability gap is *not* an exception. + +## Capability gaps refuse, they do not substitute + +Pasting a screenshot at a model without vision, a PDF at a model without document input, +or a prompt past the window is a **visible refusal that names the fix** +(`capabilityCheck`). This used to substitute a capable model silently, which moved the +turn onto a model the user never chose and never saw. That was routing wearing a +different hat. + +## Fallback is infrastructure resilience only + +Each catalog model declares a `fallback` chain. On a provider or transport failure the +Runner walks it and retries the same call, naming the substitute on the ⇄ line so a +rescue is never silent. + +The rules that keep this from becoming routing again: + +- **Current call only.** The next turn resolves the primary pin as normal. +- **Never after output was emitted.** +- **Never from a judgment.** No code path leads here from "the result looked weak" or + "this task looks hard". Only from an error. +- **The first hop always changes vendor.** `gemini-flash` falling back to `gemini-pro` is + not a fallback: same adapter, same API, same failure. Enforced by + `TestFallbackFirstHopLeavesTheVendor`. + +### Which errors walk the chain + +The distinction is whether a *different model* could plausibly succeed: + +| Class | Behavior | Why | +|---|---|---| +| 400 malformed, 401/403 auth, 404 unknown model | **Terminal** | The request is what is wrong; every model rejects it identically | +| 408 timeout, 429 rate limit | Walk | Timing, not shape | +| 5xx, transport failures, stream cuts | Walk | Infrastructure | +| Context overflow | Terminal for the chain | Compaction handles it, then retries | +| Billing, entitlement, BYOK key, sign-out | Terminal for the chain | Each carries its own policy: the billing dialog, `/apikeys`, `/login` | + +Getting this wrong is expensive in a way that hides: a single malformed request walked +across a chain becomes one bug reported as many failures, and a terminal failure treated +as retryable becomes a run that never ends. + +## The gateway serves; it does not choose + +The gateway is a **metered serving endpoint**, deployed separately from this repo. + +- **Strict label gate.** `model` must be a servable catalog label. Vendor ids, aliases, + `auto`, and typos are `400 unknown_model`. +- **Typed errors, no absorbs.** `413 context_overflow`, `400 model_capability`, `422 byok_key_failed`, `402 insufficient_credits` / `subscription_required` / - `account_locked`, `502` for upstream failures. The gateway never reroutes a request to a - different model — in either direction. -- **The control plane**: `GET /v1/models` serves every fact selection reads. Anything new - the policy needs gets added there explicitly — never smuggled back into gateway routing. - -## The semantic ladder (`lane.go`) - -A lane is a chain of deployment **roles** with a vendor-**tier** fallback. Roles come from -the gateway's `config.json` via the control plane; tier triples are catalog data -(`models.json` `tiers`: frontier/balanced/cheap per vendor). - -| Turn | Lane | -|---|---| -| `review` (plan critic) | role `reviewer`, else frontier | -| plan mode: classify / scouts | roles `classify`→`standard` / `standard`, else balanced | -| plan mode: executive draft | role `planner`, else frontier; **frontier directly** on `plan_review_escalate`, `plan_synth_incomplete`, `self_heal`, or a `high_risk_surface` plan (the plan is the binding contract) | -| `classify` (judges) | roles `classify`→`standard`, else cheap | -| `explore` / `route` | role `standard`, else cheap | -| `reflect` / `synth` | role `planner`, else frontier | -| `predict` / `learn` / `compact` / `shrinkwrap` / `overview` | role `standard`, else balanced | -| main loop: `self_heal` / `agent_frontier` | **frontier tier** (the error valve / background agents) | -| main loop: `agent_strong` | balanced tier (a dispatched strong agent) | -| main loop: judged `deep`, or effort-high unjudged | role `planner`, else frontier | -| main loop: judged `lookup`, no risk | role `standard`, else cheap | -| main loop: everything else | role `standard`, else balanced | - -Roles today: `planner`/`standard` = glm-5p2 (Fireworks, 1M), `reviewer` = luna (OpenAI), -`classify` = gpt-oss-120b (Fireworks). The swap knob is the gateway's `config.json` + -redeploy — the CLI reads the roles off `/v1/models`, so a role swap needs no CLI release. - -## Steering + the $0 invariant (`resolve.go`) - -BYOK-first, decided at selection time: an Automatic turn that resolved to a strong vendor -the user has **no key** for remaps to the **keyed preference** (deployment default vendor -first, then catalog order) at the **same tier altitude**. Explicit choices are never -overridden: a pin, or a non-default `/model ` flavor, rides through untouched. -With no BYOK keys the whole pass is a byte-identical no-op. - -At **$0 credits** with keys present, an Automatic selection must land on a keyed (fundable) -lane — the old `credits_byok` absorb, now decided up-front and visibly. Pins are exempt: an -unkeyed pin at $0 gets the gateway's clean 402 naming the vendor, never a coercion. The -gateway still **enforces** all money invariants server-side; client selection just stops -doomed requests before they burn a round trip. - -**The billing lane is explicit on the wire** (`memcode_billing`: `byok_preferred` default | -`byok_only` | `credits`): the gateway enforces the requested lane and never chooses one — -silent rerouting between the user's keys and credits is impossible in either direction. A -`byok_key_failed` turn is NOT hard-terminal anymore: the CLI's default policy surfaces the -■ notice and never silently retries on credits, but an **explicit, consented** "retry this -turn on credits" is legitimate client policy (consent is not silence). - -## Fallback + recovery (`recover.go`, catalog `fallback` chains) - -Each catalog model declares its mid-turn failure chain (labels), e.g. `glm-5p2 → -[kimi-k2p7-code, terra]`, `sol → [terra]`, `gemini-pro → [gemini-flash, terra]`. On a -model-class error the Runner walks the chain — filtered by availability on this backend, -capability fit for this payload, and fundability under the org's credit state — and -retries the same call (≤2 hops). Standard client-fallback semantics: **current call only** (the -next turn retries the primary), **never after output was emitted**, and terminal classes -(402s, key/auth failures, overflow, stream cuts) never enter the chain. The ⇄ line shows -`model_error: …` so rescue is visible, never silent. - -## Pinned models — `/model` - -Unchanged in spirit: the picker pins one catalog label for every real request; invisible -plumbing (`classify`/`compact`/`shrinkwrap`) stays on the utility lanes; a stale/unknown -pin falls through to Automatic. The pin is now simply the selection policy's first branch -(`resolve.go`), and "serve exactly what was asked" is the entire gateway contract rather -than a special `servePinned` path. - -## Endpoint mode (Ollama, LM Studio, vLLM, provider clouds — no gateway) - -The same agent, no gateway: the endpoint's session model (picked via `/model`, -remembered per-endpoint) serves **every** lane — there are no roles or tiers to resolve, -which IS the uniformity story: the ladder degrades to the available model set. Fallback -chains apply only to cataloged labels, so an uncataloged local model fails honestly -instead of being silently swapped. No memcode extensions leave the machine. - -**Wire selection** (ONE implementation per provider protocol, shared by gateway and -direct mode — `internal/providers/{provcore,openai,anthropic,gemini,compat, -memcode}`): a provider's OWN API gets its full-fidelity native dialect — -`api.openai.com`/`api.x.ai` speak the **Responses API**, `api.anthropic.com` the -**Messages API**, `generativelanguage.googleapis.com` the **Gemini API** — via the exact -adapters the hosted gateway runs. Everything else — local runtimes, compat clouds — -speaks the generic chat/completions engine (`providers/compat`, which is ALSO the -gateway's Fireworks lane client: salvage net + lane error contract as configuration), -with a probe-and-degrade retry (`reasoning_effort:"none"`) for compat endpoints that -refuse tools while reasoning is active. The memcode dialect itself is a provider -(`providers/memcode`): the compat engine + the memcode extensions + the /v1/models -control-plane client. The gateway keeps NO wire code: its provider layer is -construction + serving policy (key injection, capability gates, money) only. **Keys**: explicit `MEMCODE_ENDPOINT_KEY` > config `key_env` > the -ecosystem-standard env var for well-known hosts (`OPENAI_API_KEY`, `XAI_API_KEY`, -`GROQ_API_KEY`, …); local hosts stay keyless. - -## Verification - -- `internal/llm/parity_test.go` — 5,992 ladder rows + 616 steering rows reproduced - from the pre-deletion gateway goldens. -- `internal/llm/policy_test.go` — capability absorbs, $0 rule, chain walk, emitted - guard, delegate append, endpoint bypass, control-plane outage degradation. -- `internal/provider/uniformity_test.go` — one scripted session against a - gateway-shaped server AND a bare endpoint: identical wire shape (zero memcode headers), - identical policy, different model sets only. -- `internal/providers/compat` (the shared compat engine + its tests) plus the gateway - deployment's own serving-contract suite — strict label gate, typed errors, money - canary, self-conformance. + `account_locked`, `400 upstream_request_invalid` for a provider 4xx we caused, `502` + for genuine upstream trouble. It never reroutes a request to a different model in + either direction. +- **Enforcement stays server-side.** Auth, entitlement, BYOK key injection, metering, + rate limits. The client stops doomed requests early as a courtesy; the gateway is what + actually holds the line. +- **The control plane.** `GET /v1/models` serves every fact selection reads: labels, + vendor identity, capabilities, windows, BYOK coverage, credits state. Anything new the + client needs gets added there explicitly, never smuggled back into gateway routing. + +## Endpoint and local backends + +With `MEMCODE_ENDPOINT_URL` (or Ollama, LM Studio, any OpenAI-compatible server), the +endpoint's configured model is the session model and the pin chain is not consulted. The +same one-model-per-session rule holds; it is simply the endpoint that names it. + +## The rule, restated + +> The pinned model is authoritative. Utility plumbing and infrastructure failure are the +> only paths to another model, and neither may be reached from a judgment about the work. + +If a change would let memcode run a turn on a model the user did not choose, because of +something about the *task*, it is reintroducing routing. diff --git a/internal/llm/lanes.go b/internal/llm/lanes.go index b3df9c2..6db25fe 100644 --- a/internal/llm/lanes.go +++ b/internal/llm/lanes.go @@ -8,11 +8,15 @@ import ( "github.com/memcode-ai/memcode/internal/wire" ) -// Lane-aware policy: attached subscriptions are $0 serving paths, so -// AUTOMATIC selection prefers their vendors (never overriding explicit pins -// or vendor choices), and signed-out sessions resolve only over attached -// families. Every branch here guards on lane presence, so with no lanes the -// selection pipeline is byte-identical to the gateway parity goldens. +// Lane FACTS, not lane policy. Attached subscriptions and own-key vendors are +// $0 serving paths, so this file folds them into the control-plane snapshot: +// which vendors are servable, which are keyed, and what the default is when +// there is no gateway. +// +// It does NOT choose a model. The steering that once lived alongside this +// (Automatic preferring keyed vendors, the $0 fundability remap) went with the +// routing removal in v0.29.0 — the pin is the only selection authority now, and +// a lane can make a model reachable or unreachable but never preferred. // applyLaneFacts stamps the control-plane snapshot with the local lane // reality: sub vendors join SubVendors; own-key vendors merge into the BYOK diff --git a/internal/llm/resolve.go b/internal/llm/resolve.go index 5378956..749d9cf 100644 --- a/internal/llm/resolve.go +++ b/internal/llm/resolve.go @@ -13,14 +13,18 @@ import ( "github.com/memcode-ai/memcode/internal/wire" ) -// resolve.go — PHYSICAL RESOLUTION: lane → concrete model label, decided over -// the hosted routing control plane (GET /v1/models: roles, byok coverage, -// credits state, capabilities) plus the shared catalog (vendor tier triples, -// windows). Steering — prefer vendors the user brought keys for, never select -// an unfundable lane at $0 — is SELECTION policy here, moved from the gateway -// (steer.go, deleted) and proven against parity goldens -// (testdata/steer_goldens.json). The gateway can no longer reroute anything: -// what this file picks is what serves, or a typed error comes back. +// resolve.go — the ONE non-pin decision and the capability gate. +// +// There is exactly one model per session and the pin resolver already settled +// it (session -> workspace -> user -> default_model). What remains here is +// routing internal plumbing to the catalog's utility_model, and refusing a turn +// the pinned model physically cannot serve. +// +// The ladder this file used to hold — role/tier verdicts, BYOK steering, the $0 +// fundability remap, capability SUBSTITUTION — is deleted (v0.29.0). See +// resolveHosted and capabilityCheck for why each one had to go. The gateway +// cannot reroute either: what the pin names is what serves, or a typed error +// comes back. // modelsTTL bounds how stale the control-plane snapshot may get before a // refresh; invalidation (login, /apikeys, 402s) cuts it short.