From 4e1dd03d07d2b5159ec832e2a68569724158221b Mon Sep 17 00:00:00 2001 From: Kyle Isom Date: Fri, 25 Sep 2026 13:38:24 -0700 Subject: [PATCH] Route templates: a route named x-* serves any request route x- Implemented-By: OpenCode session (model recorded in docs/implementer-log.md) --- cmd/crossbar/main.go | 2 +- docs/implementer-log.md | 1 + internal/admin/admin.go | 4 +- internal/admin/admin_ops.go | 2 +- internal/admin/admin_template_test.go | 92 +++++++++++++++++++++ internal/config/config.go | 2 +- internal/config/config_v22_test.go | 115 ++++++++++++++++++++++++++ internal/config/route.go | 40 +++++++++ internal/proxy/proxy.go | 10 +-- internal/proxy/template_test.go | 85 +++++++++++++++++++ 10 files changed, 344 insertions(+), 9 deletions(-) create mode 100644 internal/admin/admin_template_test.go create mode 100644 internal/config/config_v22_test.go create mode 100644 internal/config/route.go create mode 100644 internal/proxy/template_test.go diff --git a/cmd/crossbar/main.go b/cmd/crossbar/main.go index 3360531..a2b592c 100644 --- a/cmd/crossbar/main.go +++ b/cmd/crossbar/main.go @@ -104,7 +104,7 @@ func run() error { checker = identity.NewHeaderChecker() } handler = identity.Middleware(checker, func(route string) ([]string, bool) { - rt, ok := cfg.Routes[route] + rt, _, ok := cfg.Route(route) return rt.Peers, ok }, p) } diff --git a/docs/implementer-log.md b/docs/implementer-log.md index 54fb634..7aab652 100644 --- a/docs/implementer-log.md +++ b/docs/implementer-log.md @@ -5,6 +5,7 @@ owner fills in the Model column. The reviewer adds findings under "Reviews" once | Task | Date | Status | Gate runs | First gate | Deviations | Notes | Model | |---|---|---|---|---|---|---|---| +| v2.2/01-route-templates | 2026-09-25 | done | 1 | fail | `internal/config` red only on `Wake.Addresses()` (task 02), the one allowed red; `go build ./...` clean, proxy/admin/health/wake/lease/store/identity/fingerprint all pass under `-race`. New `internal/config/route.go`: `templateName` pattern `^[a-z0-9][a-z0-9-]*-\*$` and `Route()` (valid-name guard excludes `*`; exact wins; else longest `"-*"`, prefix keeps the dash, non-empty remainder required, longest-prefix wins deterministically). `config.go`: the route-name check accepts a template too (one line). `proxy.go`: `route()` and `ServeHTTP` resolve both path and `X-Crossbar-Route` header forms through `cfg.Route`, and the conflicting-route check compares concrete names via `cfg.Route` (identical to before for non-template configs). `admin.go` `routeView` lists a lease under the exact key it matches or the longest template key; `admin_ops.go` `routePin` resolves through `cfg.Route` so a concrete route under a template can be pinned before its first request and the template name 404s. `main.go` identity lookup uses `cfg.Route`. All three given tests byte-identical (`config_v22_test.go` keeps `TestWakeBroadcasts`, which is why config is red). | ? | | v2.1/02-props-loaded-only | 2026-09-25 | done | 1 | pass | `movedHeader` separator `><`→`>` and the `CtxHeader` doc comment in `proxy.go`, both forced by the given router test (`moved:small>big`) which the task text did not mention; no production code parses the separator (`forward.go` passes it straight through) so it is safe. | Implemented per-model context. `health`: added `ModelCtx` and a `Models map[string]ModelCtx` field on `Status`, plus `PerSlotCtxFor(model)` (per-model figure when present, else host-level `PerSlotCtx` for a loaded model, else 0); moved `props` into a new `props.go` and added `propsModel`/`propsModels`. Poller rules 1-4: `/v1/models` treats an entry as loaded only with no `status` or `status.value=="loaded"` (other values dropped from `Loaded`); plain `/props` with `role:router` leaves host NCtx/Slots 0; each loaded model is asked `GET /props?model=` and a failed/malformed answer leaves that id absent without failing the host; `Models` is a fresh non-nil map every successful poll, `MarkDown` leaves it. `ctxguard.go`: every `PerSlotCtx()` became `PerSlotCtxFor(model)` (leased host, candidates, wake "cannot serve" check) and `largestSlotCtx(hosts,h,model)` counts only hosts that have it loaded. `admin.go`: `HostView` gains `models` (empty object, never null). A plain single server keeps working as v2. All three given tests byte-identical; `make gate` → `gate: ok` on the first run. | ? | | v2.1/01-cancel-record | 2026-09-25 | done | 1 | pass | none | Implemented the rule: added a `cancelled` field to `forwardState`; the `ErrorHandler` sets it when it observes `context.Canceled` (client gone before any response byte) so the delivered row is no longer turned into a 499 by a pooled close after the body; removed the post-hoc `r.Context().Err()` check in the normal path, leaving the recover path's `http.ErrAbortHandler` (mid-body) check as the other 499 source. Given test failed the first run (`Errors:7`, status counts held 25×200/7×499), passes 3× under `-race`; `TestClientCancelMidStreamIsRecorded`, `TestClientCancelWhileQueuedIsRecorded` and `TestQueueFullIs503` still pass; `forward.go` 230 lines; `make gate` printed `gate: ok` on the first run. | ? | | v2/05-wiring-smoke | 2026-09-25 | done | 1 | pass | none | The wiring in `cmd/crossbar/main.go` and `internal/proxy/{proxy,forward,ctxguard}.go` plus the README section were already in the working tree from a prior session; this session only ran the tests, the gate, the log row, and the commit. `go test -race -count=1 ./...` failed once on `TestQueueFullIs503` (`Errors:2`, the 503 not recorded) — the known v1 recording defect the owner scheduled as a v2.1 task 01; reran once and it passed. `make gate` printed `gate: ok` on the first run. Committed the two owner-corrected given v1 tests (`internal/limiter/limiter_test.go`, `internal/proxy/proxy_test.go`) alongside the prior session's changes. | ? | diff --git a/internal/admin/admin.go b/internal/admin/admin.go index 6915d70..9f3ff01 100644 --- a/internal/admin/admin.go +++ b/internal/admin/admin.go @@ -173,7 +173,9 @@ func (hx *handler) routesGet(w http.ResponseWriter, r *http.Request) { func (hx *handler) routeView(route string, hosts []string, defaultModel string, snap []lease.Lease) RouteView { leases := make([]LeaseView, 0) for _, l := range snap { - if l.Route == route { + // A concrete route lists under the exact key it matches, or the longest + // template that matches it; a template's row is every such lease. + if _, key, ok := hx.cfg.Route(l.Route); ok && key == route { leases = append(leases, leaseView(l)) } } diff --git a/internal/admin/admin_ops.go b/internal/admin/admin_ops.go index 0aa98da..2719c86 100644 --- a/internal/admin/admin_ops.go +++ b/internal/admin/admin_ops.go @@ -24,7 +24,7 @@ func (hx *handler) routePin(w http.ResponseWriter, r *http.Request) { return } route := r.PathValue("route") - routeCfg, ok := hx.cfg.Routes[route] + routeCfg, _, ok := hx.cfg.Route(route) if !ok { writeError(w, http.StatusNotFound, "unknown route") return diff --git a/internal/admin/admin_template_test.go b/internal/admin/admin_template_test.go new file mode 100644 index 0000000..8fb5561 --- /dev/null +++ b/internal/admin/admin_template_test.go @@ -0,0 +1,92 @@ +package admin_test + +import ( + "encoding/json" + "path/filepath" + "strings" + "testing" + "time" + + "git.wntrmute.dev/kyle/crossbar/internal/admin" + "git.wntrmute.dev/kyle/crossbar/internal/config" + "git.wntrmute.dev/kyle/crossbar/internal/health" + "git.wntrmute.dev/kyle/crossbar/internal/lease" + "git.wntrmute.dev/kyle/crossbar/internal/limiter" + "git.wntrmute.dev/kyle/crossbar/internal/store" +) + +// The routes view lists a template once, under its own name, with the leases of every concrete +// route it matched. A concrete route can be pinned; the template itself cannot. +func TestRoutesViewAndPinWithTemplates(t *testing.T) { + cfg, err := config.Parse(strings.NewReader(` +listen = "127.0.0.1:1" +[hosts.alpha] +base_url = "http://alpha:1" +models = { "m" = { parallel = 2 } } +[routes."opencode-*"] +hosts = ["alpha"] +default_model = "m" +`)) + if err != nil { + t.Fatal(err) + } + st, err := store.Open(filepath.Join(t.TempDir(), "x.db")) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = st.Close() }) + hosts := &fakeHosts{ + st: map[string]health.Status{"alpha": {Healthy: true, Loaded: []string{"m"}}}, + draining: map[string]bool{}, + } + lt, err := lease.New(st, hosts, hosts, 30*time.Minute) + if err != nil { + t.Fatal(err) + } + if _, _, err := lt.Acquire(lease.Key{Route: "opencode-projecta-4242", FP: "fp1", Model: "m"}, []string{"alpha"}, time.Now()); err != nil { + t.Fatal(err) + } + if _, _, err := lt.Acquire(lease.Key{Route: "opencode-projectb-7", FP: "fp2", Model: "m"}, []string{"alpha"}, time.Now()); err != nil { + t.Fatal(err) + } + lim := limiter.New() + lim.Configure("alpha", "m", 2, 8) + r := &rig{h: admin.Handler(cfg, hosts, lt, lim, st, hosts), store: st, leases: lt, hosts: hosts} + + rec := r.do(t, "GET", "/_crossbar/routes", "") + if rec.Code != 200 { + t.Fatalf("%d %s", rec.Code, rec.Body.String()) + } + var out map[string]admin.RouteView + if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil { + t.Fatal(err) + } + v, ok := out["opencode-*"] + if !ok || len(out) != 1 { + t.Fatalf("routes view keys = %v, want exactly the template", keysOf(out)) + } + if len(v.Hosts) != 1 || v.Hosts[0] != "alpha" || v.DefaultModel != "m" || len(v.Leases) != 2 { + t.Errorf("template view = %+v, want hosts [alpha], model m and the two concrete routes' leases", v) + } + + rec = r.do(t, "POST", "/_crossbar/routes/opencode-projecta-4242", `{"host":"alpha","pin":true}`) + if rec.Code != 200 { + t.Errorf("pin of a concrete templated route: %d %s, want 200", rec.Code, rec.Body.String()) + } + rec = r.do(t, "POST", "/_crossbar/routes/opencode-*", `{"host":"alpha","pin":true}`) + if rec.Code != 404 { + t.Errorf("pin of the template itself: %d, want 404 unknown route", rec.Code) + } + rec = r.do(t, "POST", "/_crossbar/routes/opencode-nothing-yet", `{"host":"alpha","pin":true}`) + if rec.Code != 200 { + t.Errorf("pin of a not-yet-seen concrete route under a template: %d %s, want 200 (it is a valid route)", rec.Code, rec.Body.String()) + } +} + +func keysOf(m map[string]admin.RouteView) []string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + return out +} diff --git a/internal/config/config.go b/internal/config/config.go index 51988c4..6559877 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -358,7 +358,7 @@ func (c *Config) checkRoutes(peersDefined map[string]bool, identityDefined bool) for _, name := range names { r := c.Routes[name] - if !routeName.MatchString(name) { + if !routeName.MatchString(name) && !templateName.MatchString(name) { return &Error{Field: fmt.Sprintf("routes.%s", name), Msg: "must match [a-z0-9][a-z0-9-]*"} } diff --git a/internal/config/config_v22_test.go b/internal/config/config_v22_test.go new file mode 100644 index 0000000..9becd6a --- /dev/null +++ b/internal/config/config_v22_test.go @@ -0,0 +1,115 @@ +package config_test + +import ( + "strings" + "testing" + + "git.wntrmute.dev/kyle/crossbar/internal/config" +) + +const templateBase = ` +listen = "127.0.0.1:1" +[hosts.a] +base_url = "http://a:1" +models = { "m" = { }, "n" = { } } +[routes."opencode-*"] +hosts = ["a"] +default_model = "m" +[routes."opencode-rust-*"] +hosts = ["a"] +default_model = "n" +[routes.opencode-fixed] +hosts = ["a"] +[routes.paper] +hosts = ["a"] +` + +// A route whose name ends in "-*" is a template: any request route that starts with the part +// before the star, with something after it, uses that route's config. An exact name wins over a +// template; the longest matching template wins over shorter ones. +func TestRouteTemplatesResolve(t *testing.T) { + c, err := config.Parse(strings.NewReader(templateBase)) + if err != nil { + t.Fatal(err) + } + for _, tc := range []struct { + name, wantKey, wantModel string + ok bool + }{ + {"paper", "paper", "", true}, + {"opencode-fixed", "opencode-fixed", "", true}, // exact beats template + {"opencode-projecta-4242", "opencode-*", "m", true}, // template + {"opencode-rust-a-7", "opencode-rust-*", "n", true}, // longest template wins + {"opencode-", "", "", false}, // nothing after the prefix + {"opencode", "", "", false}, // the dash is part of the prefix + {"opencodex", "", "", false}, // not a prefix match + {"opencode-*", "", "", false}, // a literal star is never a request route + {"Opencode-A", "", "", false}, // not a valid route name + {"nope", "", "", false}, + } { + r, key, ok := c.Route(tc.name) + if ok != tc.ok || key != tc.wantKey || (ok && r.DefaultModel != tc.wantModel) { + t.Errorf("Route(%q) = (%+v, %q, %v), want key %q model %q ok %v", tc.name, r, key, ok, tc.wantKey, tc.wantModel, tc.ok) + } + } +} + +func TestRouteTemplateNamesAreValidated(t *testing.T) { + for name, tc := range map[string]struct { + route string + wantErr string + }{ + "star in the middle": {`"open*code"`, "routes.open*code"}, + "star without dash": {`"opencode*"`, "routes.opencode*"}, + "bare star": {`"*"`, "routes.*"}, + "double star": {`"opencode-**"`, "routes.opencode-**"}, + } { + t.Run(name, func(t *testing.T) { + text := "listen = \"127.0.0.1:1\"\n[hosts.a]\nbase_url = \"http://a:1\"\nmodels = { \"m\" = { } }\n[routes." + tc.route + "]\nhosts = [\"a\"]\n" + _, err := config.Parse(strings.NewReader(text)) + ce, ok := err.(*config.Error) + if !ok || ce.Field != tc.wantErr { + t.Fatalf("err = %v, want *config.Error on %q", err, tc.wantErr) + } + }) + } + // A template alone satisfies "at least one route". + if _, err := config.Parse(strings.NewReader("listen = \"127.0.0.1:1\"\n[hosts.a]\nbase_url = \"http://a:1\"\nmodels = { \"m\" = { } }\n[routes.\"x-*\"]\nhosts = [\"a\"]\n")); err != nil { + t.Errorf("a template-only config must parse: %v", err) + } +} + +// broadcasts: a wake target may name several broadcast addresses (a host that roams between two +// Wi-Fi networks). `broadcast` (one) and `broadcasts` (a list) are alternatives: exactly one. +func TestWakeBroadcasts(t *testing.T) { + head := "listen = \"127.0.0.1:1\"\n[hosts.a]\nbase_url = \"http://a:1\"\nmodels = { \"m\" = { } }\n[hosts.a.wake]\nmac = \"aa:bb:cc:dd:ee:ff\"\n" + tail := "\n[routes.r]\nhosts = [\"a\"]\n" + c, err := config.Parse(strings.NewReader(head + `broadcasts = ["192.168.88.255:9", "192.168.1.255:9"]` + tail)) + if err != nil { + t.Fatal(err) + } + if got := c.Hosts["a"].Wake.Addresses(); len(got) != 2 || got[0] != "192.168.88.255:9" || got[1] != "192.168.1.255:9" { + t.Errorf("Addresses() = %v, want both, in order", got) + } + c, err = config.Parse(strings.NewReader(head + `broadcast = "192.168.88.255:9"` + tail)) + if err != nil { + t.Fatal(err) + } + if got := c.Hosts["a"].Wake.Addresses(); len(got) != 1 || got[0] != "192.168.88.255:9" { + t.Errorf("Addresses() = %v, want the single broadcast", got) + } + for name, body := range map[string]string{ + "both": "broadcast = \"192.168.88.255:9\"\nbroadcasts = [\"192.168.1.255:9\"]", + "neither": "wait = \"30s\"", + "empty list": "broadcasts = []", + "bad entry": "broadcasts = [\"192.168.1.255\"]", // no port + } { + t.Run(name, func(t *testing.T) { + _, err := config.Parse(strings.NewReader(head + body + tail)) + ce, ok := err.(*config.Error) + if !ok || !strings.HasPrefix(ce.Field, "hosts.a.wake") { + t.Fatalf("err = %v, want *config.Error under hosts.a.wake", err) + } + }) + } +} diff --git a/internal/config/route.go b/internal/config/route.go new file mode 100644 index 0000000..43f3ce4 --- /dev/null +++ b/internal/config/route.go @@ -0,0 +1,40 @@ +package config + +import ( + "regexp" + "strings" +) + +// templateName matches a route template: a valid route name ending in "-*". +var templateName = regexp.MustCompile(`^[a-z0-9][a-z0-9-]*-\*$`) + +// Route resolves a request route name: an exact entry wins; else the longest template +// "-*" whose prefix (including the dash) starts name with a non-empty remainder; +// else ok is false. key is the config key that matched (the template's name for a template). +// A name that is not a valid route name (the pattern below) or contains '*' never matches. +func (c *Config) Route(name string) (r Route, key string, ok bool) { + if !routeName.MatchString(name) { + return Route{}, "", false + } + if rt, found := c.Routes[name]; found { + return rt, name, true + } + var ( + best Route + bestKey string + bestLen int + ) + for tmpl, rt := range c.Routes { + if !templateName.MatchString(tmpl) { + continue + } + prefix := tmpl[:len(tmpl)-1] // drop the trailing '*', keeping the dash + if len(prefix) > bestLen && strings.HasPrefix(name, prefix) && len(name) > len(prefix) { + best, bestKey, bestLen = rt, tmpl, len(prefix) + } + } + if bestKey == "" { + return Route{}, "", false + } + return best, bestKey, true +} diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go index 654cca1..7614a13 100644 --- a/internal/proxy/proxy.go +++ b/internal/proxy/proxy.go @@ -144,13 +144,13 @@ func (p *Handler) route(r *http.Request) (route, rest string, code int, msg stri if hdr != "" { rest := r.URL.Path // A path that also carries a (different) route name is a client mistake: the header is the - // operator's intent, but the path disagrees. + // operator's intent, but the path disagrees. Compare concrete names. if rname, _, ok := SplitRoute(rest); ok { - if _, known := p.cfg.Routes[rname]; known && rname != hdr { + if _, _, rok := p.cfg.Route(rname); rok && rname != hdr { return "", "", http.StatusBadRequest, "conflicting route" } } - if _, known := p.cfg.Routes[hdr]; !known { + if _, _, ok := p.cfg.Route(hdr); !ok { return "", "", http.StatusNotFound, "unknown route" } if !allowedPath(rest) { @@ -162,7 +162,7 @@ func (p *Handler) route(r *http.Request) (route, rest string, code int, msg stri if !ok { return "", "", http.StatusBadRequest, "missing route" } - if _, known := p.cfg.Routes[route]; !known { + if _, _, ok := p.cfg.Route(route); !ok { return "", "", http.StatusNotFound, "unknown route" } if !allowedPath(rest) { @@ -207,7 +207,7 @@ func (p *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { p.writeError(w, code, msg) return } - routeCfg := p.cfg.Routes[route] + routeCfg, _, _ := p.cfg.Route(route) model, body, err := peekModel(r) if err != nil { diff --git a/internal/proxy/template_test.go b/internal/proxy/template_test.go new file mode 100644 index 0000000..2374f24 --- /dev/null +++ b/internal/proxy/template_test.go @@ -0,0 +1,85 @@ +package proxy_test + +import ( + "net/http" + "testing" + "time" + + "git.wntrmute.dev/kyle/crossbar/internal/proxy" + "git.wntrmute.dev/kyle/crossbar/internal/store" +) + +const templateHosts = ` +listen = "127.0.0.1:1" +[hosts.alpha] +base_url = %q +models = { "shared" = { parallel = 4 } } +[routes."opencode-*"] +hosts = ["alpha"] +default_model = "shared" +[routes.opencode-fixed] +hosts = ["alpha"] +default_model = "shared" +` + +// One OpenCode instance per route, without listing every instance in the config: a route named +// "opencode-*" serves any request route "opencode-". Leases and accounting are keyed +// by the concrete route name, so two instances never share a lease and each gets its own usage +// row. The literal template name is never a request route. +func TestRouteTemplateServesConcreteRoutes(t *testing.T) { + alpha := newUpstream(t, "alpha") + r := newRig(t, templateHosts, alpha) + + resp := r.post("/opencode-projecta-4242/v1/chat/completions", conversation(1, 1)) + drain(resp) + if resp.StatusCode != 200 || resp.Header.Get(proxy.HostHeader) != "alpha" || resp.Header.Get(proxy.LeaseHeader) != "new" { + t.Fatalf("first turn on a templated route: %d %q %q, want 200 alpha new", resp.StatusCode, resp.Header.Get(proxy.HostHeader), resp.Header.Get(proxy.LeaseHeader)) + } + resp = r.post("/opencode-projecta-4242/v1/chat/completions", conversation(1, 2)) + drain(resp) + if resp.Header.Get(proxy.LeaseHeader) != "reused" { + t.Errorf("second turn should reuse the lease, got %q", resp.Header.Get(proxy.LeaseHeader)) + } + // A second instance with the same conversation shape is a different route: its own lease. + resp = r.post("/opencode-projectb-7/v1/chat/completions", conversation(1, 1)) + drain(resp) + if resp.StatusCode != 200 || resp.Header.Get(proxy.LeaseHeader) != "new" { + t.Errorf("another instance must get its own lease: %d %q", resp.StatusCode, resp.Header.Get(proxy.LeaseHeader)) + } + // The header form resolves templates too. + resp = r.post("/v1/chat/completions", conversation(2, 1), proxy.RouteHeader, "opencode-projectc-1") + drain(resp) + if resp.StatusCode != 200 { + t.Errorf("X-Crossbar-Route with a templated name: %d, want 200", resp.StatusCode) + } + // An exact route still works and is not shadowed by the template. + resp = r.post("/opencode-fixed/v1/chat/completions", conversation(3, 1)) + drain(resp) + if resp.StatusCode != 200 { + t.Errorf("exact route: %d, want 200", resp.StatusCode) + } + + for _, path := range []string{"/opencode-*/v1/models", "/opencode-/v1/models", "/opencode/v1/models", "/opencodex/v1/models"} { + req, _ := http.NewRequest(http.MethodGet, r.front.URL+path, nil) + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + drain(resp) + if resp.StatusCode != 404 { + t.Errorf("%s: %d, want 404 unknown route", path, resp.StatusCode) + } + } + + rows, _ := r.store.Usage(time.Time{}, store.ByRoute) + keys := map[string]int64{} + for _, row := range rows { + keys[row.Key] = row.Requests + } + if keys["opencode-projecta-4242"] != 2 || keys["opencode-projectb-7"] != 1 || keys["opencode-projectc-1"] != 1 || keys["opencode-fixed"] != 1 { + t.Errorf("usage by route = %v, want rows per concrete route", keys) + } + if _, present := keys["opencode-*"]; present { + t.Errorf("the template name must never be an accounting key: %v", keys) + } +}