amadan
schedule: spec §5 carries due_at; concurrency test polls recorded rows; ValidPath comment
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3 files changed,
+23
−12
docs/superpowers/specs/2026-08-23-scheduled-work-design.md+5 −3internal/schedule/declaration.go+7 −5internal/schedule/runner_test.go+11 −4
CI passed — run details
diff --git a/docs/superpowers/specs/2026-08-23-scheduled-work-design.md b/docs/superpowers/specs/2026-08-23-scheduled-work-design.md| index c35738c7c..6ad9ec92c 100644 |
| --- a/docs/superpowers/specs/2026-08-23-scheduled-work-design.md |
| +++ b/docs/superpowers/specs/2026-08-23-scheduled-work-design.md |
| @@ -196,6 +196,7 @@ CREATE TABLE IF NOT EXISTS timers ( |
| last_ms INTEGER NOT NULL DEFAULT 0, |
| attempts INTEGER NOT NULL DEFAULT 0, -- consecutive failed fires of the current occurrence |
| created_at INTEGER NOT NULL, |
| + due_at INTEGER NOT NULL DEFAULT 0, -- the natural occurrence being delivered; set on insert/advance/run, never on a retry (it is the X-Carlos-Schedule-At value) |
| PRIMARY KEY (host, name) |
| ); |
| CREATE INDEX IF NOT EXISTS timers_due ON timers (next_at) WHERE next_at > 0; |
| @@ -204,9 +205,10 @@ CREATE INDEX IF NOT EXISTS timers_due ON timers (next_at) WHERE next_at > 0; |
| Registry methods: `UpsertTimer`, `DeleteTimer`, `TimersFor(host)`, |
| `DueTimers(now, limit)`, `PruneTimers(hostsAlive, doneBefore)`, |
| `FireTimerNow(host, name)`, `MarkTimerFired(host, name, at, status, ms, |
| -attempts, next)`. `Fingerprint()` is **not** extended — timers change nothing |
| -about routing, and the edge has no reason to reload for one (same reasoning |
| -as `Caps`). |
| +attempts, next, due)`, `SetTimerStatus(host, name, status)` — writes |
| +last_status alone, for a row converge parked with "never fires". `Fingerprint()` is |
| +**not** extended — timers change nothing about routing, and the edge has no |
| +reason to reload for one (same reasoning as `Caps`). |
| **Why the registry and not the bucket (decision):** the runner polls it every |
| 15s and updates it on every fire; that is a local SQLite's job, not an object |
diff --git a/internal/schedule/declaration.go b/internal/schedule/declaration.go| index 74355ff4d..fd87938ef 100644 |
| --- a/internal/schedule/declaration.go |
| +++ b/internal/schedule/declaration.go |
| @@ -24,11 +24,13 @@ var nameRE = regexp.MustCompile(`^[a-z0-9][a-z0-9-]{0,31}$`) |
| func ValidName(name string) bool { return nameRE.MatchString(name) } |
| -// ValidPath: absolute, no scheme/host, no "..", short, and parseable by |
| -// the same parser the runner builds the tick with (url.ParseRequestURI) — |
| -// so a path accepted here is a path BuildRequest can send, and a bad one |
| -// fails at the terminal, not on the box. A path is what the tick POSTs to |
| -// on the instance's own socket; nothing here can reach another host. |
| +// ValidPath: absolute, no scheme/host, no "..", short, and rejects whatever |
| +// url.ParseRequestURI rejects, plus a scheme/host/userinfo of its own. It is |
| +// NOT the same parser BuildRequest sends through (http.NewRequest / |
| +// url.Parse): the two differ on "#" — a fragment passes validation here and |
| +// is silently dropped from the tick, which is harmless since nothing reads |
| +// it. A path is what the tick POSTs to on the instance's own socket; |
| +// nothing here can reach another host. |
| func ValidPath(p string) error { |
| switch { |
| case p == "" || p[0] != '/': |
diff --git a/internal/schedule/runner_test.go b/internal/schedule/runner_test.go| index ce92cea36..3363ffccd 100644 |
| --- a/internal/schedule/runner_test.go |
| +++ b/internal/schedule/runner_test.go |
| @@ -345,15 +345,22 @@ func TestOneSlowHostDoesNotBlockTheOthersOrRefire(t *testing.T) { |
| case <-time.After(10 * time.Second): |
| t.Fatal("Tick did not return while one host was parked") |
| } |
| - // Every other host fires while slow is parked. |
| + // Every other host fires AND is recorded while slow is parked — record |
| + // writes the row after Deliver returns, so poll the recorded state |
| + // itself rather than the delivery count (which races ahead of it). |
| deadline := time.Now().Add(10 * time.Second) |
| for { |
| - fired := d.firedHosts() |
| - if len(fired) == len(hosts)-1 && fired[d.slow] == 0 { |
| + recorded := 0 |
| + for _, h := range hosts[1:] { |
| + if tm, _ := reg.GetTimer(h, "j"); tm != nil && tm.LastStatus != "" { |
| + recorded++ |
| + } |
| + } |
| + if recorded == len(hosts)-1 && d.firedHosts()[d.slow] == 0 { |
| break |
| } |
| if time.Now().After(deadline) { |
| - t.Fatalf("fired = %v, want every host but %s", fired, d.slow) |
| + t.Fatalf("fired = %v, want every host but %s recorded", d.firedHosts(), d.slow) |
| } |
| time.Sleep(5 * time.Millisecond) |
| } |