From da1da149b80aaf003508ab528af7e3a8586a4435 Mon Sep 17 00:00:00 2001 From: Kyle Isom Date: Fri, 25 Sep 2026 03:35:31 -0700 Subject: [PATCH 1/3] v1 plan: note the task 01 stop/resume Co-Authored-By: Claude Fable 5.1 --- docs/plans/v1/README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/plans/v1/README.md b/docs/plans/v1/README.md index aa00e99..5da2bcc 100644 --- a/docs/plans/v1/README.md +++ b/docs/plans/v1/README.md @@ -67,3 +67,6 @@ tools/run-plan.sh docs/plans/v1 # from a clean checkout on master not edit. Ornith diagnosed it (gofmt on its own code was clean) and did not touch them. Owner's fault (the compile check ran `go vet`, not `gofmt`, on the given files). Fixed by formatting the given files; the plan checklist now includes `gofmt -l docs/` before handover. + Ornith stopped correctly (a `stopped` row, code left in the tree, only the log committed); + a second session was told where the first had stopped and finished steps 6–7 without + starting over — the boxmaker precedent (`M3a/19`). The driver was then resumed from task 02. From 0874e00bddbe5de2657b0c3bf8a13ac6688f82dc Mon Sep 17 00:00:00 2001 From: Kyle Isom Date: Fri, 25 Sep 2026 03:47:15 -0700 Subject: [PATCH 2/3] v1 plan: task 02 replacement fixtures for the lease_idle/unknown-key conflict; AGENTS.md: earlier plans' given files stay protected Co-Authored-By: Claude Fable 5.1 --- AGENTS.md | 7 +- docs/plans/v1/02-fingerprint-config.md | 2 + docs/plans/v1/README.md | 7 + .../v1/_files/internal/config/config_test.go | 191 ++++++++++++++++++ .../config/testdata/bad-unknown-key.toml | 9 + 5 files changed, 214 insertions(+), 2 deletions(-) create mode 100644 docs/plans/v1/_files/internal/config/config_test.go create mode 100644 docs/plans/v1/_files/internal/config/testdata/bad-unknown-key.toml diff --git a/AGENTS.md b/AGENTS.md index 692be77..607d897 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -21,8 +21,11 @@ implementing it one task at a time. ## Files you must never edit - `PLAN.md`, `docs/plans/`, `AGENTS.md` -- Anything a task told you to copy from `docs/plans/**/_files/`: tests, testdata, `Makefile`, - scripts, `example.toml`, `cmd/fakeupstream`. If a copied test fails, your code is wrong. +- Anything a task told you to copy from `docs/plans/**/_files/` — in this plan **or any earlier + one** — stays protected: tests, testdata, `Makefile`, + scripts, `example.toml`, `cmd/fakeupstream`. If a copied test fails, your code is wrong. If a + copied test can no longer be right because the new task changes what it tested, that is the + owner's error: stop and report it; the owner hands over the replacement. ## Code rules diff --git a/docs/plans/v1/02-fingerprint-config.md b/docs/plans/v1/02-fingerprint-config.md index 926460d..17b977a 100644 --- a/docs/plans/v1/02-fingerprint-config.md +++ b/docs/plans/v1/02-fingerprint-config.md @@ -20,6 +20,8 @@ requests fall back to the route-level lease (task 04). ## Files - Copy: `internal/fingerprint/fingerprint_test.go`, `internal/config/config_v1_test.go` +- Copy (**replaces** v0's): `internal/config/config_test.go`, `internal/config/testdata/bad-unknown-key.toml` + (v0's unknown-key example was `lease_idle`, which this task makes valid; the replacements use `bogus_key`) - Create: `internal/fingerprint/fingerprint.go` - Modify: `internal/config/config.go`, `docs/implementer-log.md` diff --git a/docs/plans/v1/README.md b/docs/plans/v1/README.md index 5da2bcc..f99b373 100644 --- a/docs/plans/v1/README.md +++ b/docs/plans/v1/README.md @@ -70,3 +70,10 @@ tools/run-plan.sh docs/plans/v1 # from a clean checkout on master Ornith stopped correctly (a `stopped` row, code left in the tree, only the log committed); a second session was told where the first had stopped and finished steps 6–7 without starting over — the boxmaker precedent (`M3a/19`). The driver was then resumed from task 02. +- 2026-09-25, task 02: v0's given `testdata/bad-unknown-key.toml` used `lease_idle` as its + unknown-key example, and v1 makes `lease_idle` a real key, so v0's `TestBadFiles` broke — the + task did not hand over replacements for the two v0 given files (tip T19). Ornith changed the + example to `bogus_key` in both files and logged it in Deviations; the content is right, but the + files were protected and the rule was to stop. Owner's fault for the conflict; the model's + deviation is noted. The corrected files now sit in `_files/internal/config/` as the reference + copies for the reviewer's byte-exact check. diff --git a/docs/plans/v1/_files/internal/config/config_test.go b/docs/plans/v1/_files/internal/config/config_test.go new file mode 100644 index 0000000..610025f --- /dev/null +++ b/docs/plans/v1/_files/internal/config/config_test.go @@ -0,0 +1,191 @@ +package config_test + +import ( + "fmt" + "path/filepath" + "strings" + "testing" + "time" + + "git.wntrmute.dev/kyle/crossbar/internal/config" +) + +func TestGoodFile(t *testing.T) { + c, err := config.Load(filepath.Join("testdata", "good.toml")) + if err != nil { + t.Fatalf("Load: %v", err) + } + if c.Listen != "100.64.0.9:7777" { + t.Errorf("Listen = %q", c.Listen) + } + if c.PollInterval.Duration != 5*time.Second { + t.Errorf("PollInterval = %v", c.PollInterval.Duration) + } + if c.QueueMax != 4 { + t.Errorf("QueueMax = %d", c.QueueMax) + } + alpha := c.Hosts["alpha"] + if alpha.BaseURL != "http://alpha.example:11434" { + t.Errorf("trailing slash not stripped: %q", alpha.BaseURL) + } + if alpha.Weight != 2 { + t.Errorf("alpha.Weight = %v", alpha.Weight) + } + if alpha.Models["ornith-1.5-35b-a3b"].Parallel != 4 || alpha.Models["small-9b"].Parallel != 6 { + t.Errorf("alpha.Models = %+v", alpha.Models) + } + beta := c.Hosts["beta"] + if beta.Weight != 1 { + t.Errorf("beta.Weight default = %v, want 1", beta.Weight) + } + if beta.Models["ornith-1.5-35b-a3b"].Parallel != 1 { + t.Errorf("beta parallel default = %d, want 1", beta.Models["ornith-1.5-35b-a3b"].Parallel) + } + r := c.Routes["opencode-a"] + if len(r.Hosts) != 2 || r.Hosts[0] != "alpha" || r.Hosts[1] != "beta" { + t.Errorf("route hosts = %v", r.Hosts) + } + if r.DefaultModel != "ornith-1.5-35b-a3b" { + t.Errorf("DefaultModel = %q", r.DefaultModel) + } + if c.Routes["hermes-x"].DefaultModel != "" { + t.Errorf("hermes-x DefaultModel should be empty") + } + if !c.Serves("alpha", "small-9b") || c.Serves("beta", "small-9b") || c.Serves("nope", "m") { + t.Errorf("Serves is wrong") + } +} + +func TestDefaults(t *testing.T) { + c, err := config.Parse(strings.NewReader(` +listen = "127.0.0.1:1" +[hosts.a] +base_url = "http://a:1" +models = { "m" = { } } +[routes.r] +hosts = ["a"] +`)) + if err != nil { + t.Fatalf("Parse: %v", err) + } + if c.PollInterval.Duration != config.DefaultPollInterval { + t.Errorf("PollInterval default = %v", c.PollInterval.Duration) + } + if c.QueueMax != config.DefaultQueueMax { + t.Errorf("QueueMax default = %d", c.QueueMax) + } +} + +func TestBadFiles(t *testing.T) { + cases := []struct{ file, field string }{ + {"bad-listen.toml", "listen"}, + {"bad-unknown-host.toml", "routes.r.hosts"}, + {"bad-default-model.toml", "routes.r.default_model"}, + {"bad-unknown-key.toml", "bogus_key"}, + } + for _, tc := range cases { + t.Run(tc.file, func(t *testing.T) { + _, err := config.Load(filepath.Join("testdata", tc.file)) + if err == nil { + t.Fatalf("want error") + } + e, ok := config.IsError(err) + if !ok { + t.Fatalf("want *config.Error, got %T: %v", err, err) + } + if e.Field != tc.field { + t.Errorf("Field = %q, want %q (%v)", e.Field, tc.field, err) + } + if !strings.HasPrefix(err.Error(), "config: "+tc.field+": ") { + t.Errorf("Error() = %q", err.Error()) + } + }) + } +} + +func TestBadValues(t *testing.T) { + base := ` +listen = %q +poll_interval = %q +[hosts.a] +base_url = %q +weight = %v +models = { "m" = { parallel = %d } } +[routes.%s] +hosts = ["a"] +` + cases := []struct { + name string + listen, poll, url, route string + weight float64 + parallel int + field string + }{ + {"empty listen", "", "5s", "http://a:1", "r", 1, 1, "listen"}, + {"no port", "127.0.0.1", "5s", "http://a:1", "r", 1, 1, "listen"}, + {"v6 any", "[::]:7", "5s", "http://a:1", "r", 1, 1, "listen"}, + {"poll too short", "127.0.0.1:7", "500ms", "http://a:1", "r", 1, 1, "poll_interval"}, + {"ftp url", "127.0.0.1:7", "5s", "ftp://a:1", "r", 1, 1, "hosts.a.base_url"}, + {"no host", "127.0.0.1:7", "5s", "http://", "r", 1, 1, "hosts.a.base_url"}, + {"query", "127.0.0.1:7", "5s", "http://a:1/v1?x=1", "r", 1, 1, "hosts.a.base_url"}, + {"negative weight", "127.0.0.1:7", "5s", "http://a:1", "r", -1, 1, "hosts.a.weight"}, + {"negative parallel", "127.0.0.1:7", "5s", "http://a:1", "r", 1, -2, "hosts.a.models.m.parallel"}, + {"route name", "127.0.0.1:7", "5s", "http://a:1", "Bad_Name", 1, 1, "routes.Bad_Name"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + text := fmt.Sprintf(base, tc.listen, tc.poll, tc.url, tc.weight, tc.parallel, tc.route) + _, err := config.Parse(strings.NewReader(text)) + if err == nil { + t.Fatalf("want error for %s", tc.name) + } + e, ok := config.IsError(err) + if !ok { + t.Fatalf("want *config.Error, got %T: %v", err, err) + } + if e.Field != tc.field { + t.Errorf("Field = %q, want %q (%v)", e.Field, tc.field, err) + } + }) + } +} + +func TestMissingSections(t *testing.T) { + for _, tc := range []struct{ name, text, field string }{ + {"no hosts", "listen = \"127.0.0.1:7\"\n[routes.r]\nhosts = [\"a\"]\n", "hosts"}, + {"no routes", "listen = \"127.0.0.1:7\"\n[hosts.a]\nbase_url = \"http://a:1\"\nmodels = { \"m\" = { } }\n", "routes"}, + {"host without models", "listen = \"127.0.0.1:7\"\n[hosts.a]\nbase_url = \"http://a:1\"\n[routes.r]\nhosts = [\"a\"]\n", "hosts.a.models"}, + {"route without hosts", "listen = \"127.0.0.1:7\"\n[hosts.a]\nbase_url = \"http://a:1\"\nmodels = { \"m\" = { } }\n[routes.r]\n", "routes.r.hosts"}, + {"host twice", "listen = \"127.0.0.1:7\"\n[hosts.a]\nbase_url = \"http://a:1\"\nmodels = { \"m\" = { } }\n[routes.r]\nhosts = [\"a\", \"a\"]\n", "routes.r.hosts"}, + } { + t.Run(tc.name, func(t *testing.T) { + _, err := config.Parse(strings.NewReader(tc.text)) + e, ok := config.IsError(err) + if !ok { + t.Fatalf("want *config.Error, got %v", err) + } + if e.Field != tc.field { + t.Errorf("Field = %q, want %q", e.Field, tc.field) + } + }) + } +} + +func TestNotTOML(t *testing.T) { + _, err := config.Parse(strings.NewReader("listen = [unterminated")) + if err == nil { + t.Fatal("want error") + } + if _, ok := config.IsError(err); ok { + t.Errorf("a syntax error is not a validation Error") + } + if !strings.HasPrefix(err.Error(), "config: ") { + t.Errorf("Error() = %q", err.Error()) + } +} + +func TestMissingFile(t *testing.T) { + if _, err := config.Load(filepath.Join("testdata", "does-not-exist.toml")); err == nil { + t.Fatal("want error") + } +} diff --git a/docs/plans/v1/_files/internal/config/testdata/bad-unknown-key.toml b/docs/plans/v1/_files/internal/config/testdata/bad-unknown-key.toml new file mode 100644 index 0000000..67a773b --- /dev/null +++ b/docs/plans/v1/_files/internal/config/testdata/bad-unknown-key.toml @@ -0,0 +1,9 @@ +listen = "127.0.0.1:7777" +bogus_key = 1 + +[hosts.alpha] +base_url = "http://alpha.example:11434" +models = { "m" = { } } + +[routes.r] +hosts = ["alpha"] From 29b3a5c38e6f1f59249f0e2ca3f9352a763b7ac5 Mon Sep 17 00:00:00 2001 From: Kyle Isom Date: Fri, 25 Sep 2026 04:05:02 -0700 Subject: [PATCH 3/3] v1 plan: fix TestPinAndUnpin event assertion (positions -> order and content); note the finding Co-Authored-By: Claude Fable 5.1 --- docs/plans/v1/README.md | 6 ++++++ .../v1/_files/internal/lease/lease_test.go | 21 +++++++++++++++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/docs/plans/v1/README.md b/docs/plans/v1/README.md index f99b373..bfb58bb 100644 --- a/docs/plans/v1/README.md +++ b/docs/plans/v1/README.md @@ -77,3 +77,9 @@ tools/run-plan.sh docs/plans/v1 # from a clean checkout on master files were protected and the rule was to stop. Owner's fault for the conflict; the model's deviation is noted. The corrected files now sit in `_files/internal/config/` as the reference copies for the reviewer's byte-exact check. +- 2026-09-25, task 04: my `TestPinAndUnpin` asserted the pin event at exactly `len-3` and the + release event at `len-1`, but the task's own rules make acquires under a pin record events too, + so a faithful implementation produces `[new pin pin new new release]` and the assertion cannot + hold. Ornith spent its first ten minutes puzzling over exactly that. Test fault (mine): the + assertion now checks order and content (a pin event naming beta, followed later by a release), + not positions. diff --git a/docs/plans/v1/_files/internal/lease/lease_test.go b/docs/plans/v1/_files/internal/lease/lease_test.go index a1db60e..5fdf3d2 100644 --- a/docs/plans/v1/_files/internal/lease/lease_test.go +++ b/docs/plans/v1/_files/internal/lease/lease_test.go @@ -217,9 +217,26 @@ func TestPinAndUnpin(t *testing.T) { if host, _, _ := tbl.Acquire(k, []string{"alpha", "beta"}, t0.Add(4*time.Minute)); host != "beta" { t.Errorf("after unpin the existing lease (on beta) simply continues: %q", host) } - if got := p.reasons(); got[len(got)-3] != store.ReasonPin || got[len(got)-1] != store.ReasonRelease { - t.Errorf("events = %v, want a pin event and a release event", got) + // Events: a pin event naming beta must exist, and the unpin's release event must come after it. + // Acquires under the pin may record their own events in between; their number is not fixed here. + got := p.reasons() + pinAt, releaseAt := -1, -1 + for i, r := range got { + if r == store.ReasonPin && pinAt < 0 { + pinAt = i + } + if r == store.ReasonRelease { + releaseAt = i + } } + if pinAt < 0 || releaseAt < pinAt { + t.Errorf("events = %v, want a pin event followed later by a release event", got) + } + p.mu.Lock() + if pinAt >= 0 && p.events[pinAt].ToHost != "beta" { + t.Errorf("pin event = %+v, want ToHost beta", p.events[pinAt]) + } + p.mu.Unlock() } func TestDrainKeepsExistingRefusesNew(t *testing.T) {