amadan
booking: spec revision 7 after the sixth review
Three blockers. The strict busy reader must also fail closed on a rule this app cannot parse, not only on a truncated expansion; a claim must build its payload from the row it actually claimed rather than the candidate it read first, or a reschedule landing between the two sends the old time with a valid claim in hand; and a held event's tentative status had no permitted host save.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 file changed,
+67
−31
docs/superpowers/specs/2026-09-07-booking-design.md+67 −31
diff --git a/docs/superpowers/specs/2026-09-07-booking-design.md b/docs/superpowers/specs/2026-09-07-booking-design.md| index 2d1dd0a..416cdb9 100644 |
| --- a/docs/superpowers/specs/2026-09-07-booking-design.md |
| +++ b/docs/superpowers/specs/2026-09-07-booking-design.md |
| @@ -4,10 +4,9 @@ |
| Fable 5.1. Approved in outline by Paul in the session that wrote it |
| (three questions, answered: confirm by email link; everything the host |
| can see counts as busy; pages are configurable per person). **Revision |
| -6.** Reviewed five times by GPT-6 (`gpt-6-astra` on the one run that |
| -exposed its identifier) through `codex-cli 0.153.0`; all five returned |
| -REWRITE, each round narrower than the last — round 5's four blockers |
| -were all inside the reminder queue's integration and one form. §12 records every finding |
| +7.** Reviewed six times by GPT-6 (`gpt-6-astra` on the one run that |
| +exposed its identifier) through `codex-cli 0.153.0`; all six returned |
| +REWRITE, each round narrower than the last — round 6 found three. §12 records every finding |
| and disposition, including the two scoped rather than solved. |
| Asked for: a Calendly / Cal.com style "book time with me" page to share |
| @@ -63,10 +62,10 @@ first — and is swept on the existing tick. |
| | B19 | **Mail says the CURRENT state**, worded for it, and a transition deletes every unsent row the new state makes false | Historical snapshots | Snapshots need a sealed body per row and per-booking serialised delivery (r3 f4); current-state wording needs neither and is what a reader wants to know. What has already reached the sender cannot be recalled, and the spec says so. | |
| | B20 | **The booker sees the page's title and description AS THEY WERE at hold time**, snapshotted sealed onto the booking | Live page text | Live text changes the ICS without changing its `SEQUENCE` (r3 f10). | |
| | B21 | **Reschedule writes `tz` = the host's current zone atomically with both local strings and both UTC columns** | Leaving `tz` as it was | A host who moved zones would get Tokyo strings on a Dublin record (r3 f2). | |
| -| B22 | **Both queues carry a random `claim_id` and a `retry_at` separate from the semantic `due_at`; every completion mutation — ack, fail, defer, give-up, advance — is conditional on the `claim_id` the worker holds; `ErrMailBusy` defers without counting an attempt** | Claim by ID or by `claimed_at`; retry time written into `due_at` | A crash after a claim advertised work nobody could take (r4 A6); four stalled sockets burned every other message's attempts (r4 B4); a reminder finishing during a reschedule deleted the replacement (r4 B3); a whole-second `claimed_at` is not an identity (r5 f1); a reminder's `due_at` IS the occurrence minus the offset, so writing a retry time into it moved the meeting (r5 f3). | |
| +| B22 | **Both queues carry a random `claim_id`; every completion mutation — ack, fail, defer, give-up, advance — is conditional on the `claim_id` the worker holds and is built from the ROW THE CLAIM RETURNED, never from a candidate read; `ErrMailBusy` defers without counting an attempt. Reminders keep their semantic `due_at` and retry through `retry_at`; booking mail's `due_at` is retry eligibility and nothing else** | Claim by ID or by `claimed_at`; retry time written into `due_at` | A crash after a claim advertised work nobody could take (r4 A6); four stalled sockets burned every other message's attempts (r4 B4); a reminder finishing during a reschedule deleted the replacement (r4 B3); a whole-second `claimed_at` is not an identity (r5 f1); a reminder's `due_at` IS the occurrence minus the offset, so writing a retry time into it moved the meeting (r5 f3). | |
| | B24 | **Reschedule repairs the event's reminder queue rows INSIDE its transaction** | `RescheduleReminder` after commit | A crash between the two leaves a queue row that sends a reminder for a time the meeting no longer has (r5 f2). | |
| | B25 | **A booked event's form carries its scheduling snapshot as hidden fields — including the event's own `tz` — with the visible controls disabled** | Disabled controls alone | Disabled controls submit nothing, so a title-only edit 422s; and the reader's `TZ = p.Zone` fails the comparison after a profile zone change (r5 f4). | |
| -| B23 | **Availability FAILS CLOSED**: if any series in the busy window could not be expanded to completion, the page is 503, never "free" | Trusting `Expand`'s slice | `Expand` stops at 100,000 steps and returns an ordinary slice; a forever-daily rule from 1700 contributes no busy time to 2026 (r4 B5). | |
| +| B23 | **Availability FAILS CLOSED**: if any busy series in the window could not be expanded to completion — truncated OR carrying a rule this app cannot parse — the page is 503, never "free" | Trusting `Expand`'s slice | `Expand` stops at 100,000 steps and returns an ordinary slice; a forever-daily rule from 1700 contributes no busy time to 2026 (r4 B5). | |
| --- |
| @@ -311,15 +310,19 @@ time must be what the calendar shows. |
| early. Fix: the day span is the difference of CIVIL dates |
| (`time.Date(y,m,d,0,0,0,0,time.UTC)` for each end, then `/24h`), |
| independent of the zone's offsets. |
| -- **Expansion that did not finish (r4 B5; B23).** `Expand` stops at |
| - 100,000 steps and returns an ordinary slice. It gains a companion |
| - that also reports completeness, `ExpandComplete(...) ([]Occurrence, |
| - bool)`; `Expand` calls it and drops the flag, so every existing |
| - caller is unchanged. `EventsInRange` gains a strict twin, |
| - `EventsInRangeStrict`, that returns `ErrIncompleteExpansion` when |
| - any master in the window was truncated. **Busy uses the strict twin** |
| - and a public handler answers 503 with `Retry-After: 60` when it |
| - errs; views keep the tolerant one. |
| +- **Expansion that did not finish, or could not start (r4 B5, r6 f1; |
| + B23).** `Expand` stops at 100,000 steps and returns an ordinary |
| + slice; and `EventsInRange` renders a master whose rule `ParseRule` |
| + rejects as its first occurrence only, never expanding it. Both are |
| + right for a calendar view and wrong for deciding free time. `Expand` |
| + gains a companion that also reports completeness, `ExpandComplete |
| + (...) ([]Occurrence, bool)`; `Expand` calls it and drops the flag. |
| + `EventsInRange` gains a strict twin, `EventsInRangeStrict`, that |
| + returns `ErrIncompleteExpansion` when any BUSY master in the window |
| + (`Busy(kind)`, not cancelled) was truncated OR has a rule `ParseRule` |
| + cannot read. **Busy uses the strict twin** and a public handler |
| + answers 503 with `Retry-After: 60` when it errs; views keep the |
| + tolerant one, and the stored rule stays verbatim (ruling 4). |
| Regression tests in `occurrence_test.go`: a daily series ending in |
| three days with its last occurrence moved ten days out shows on that |
| @@ -447,13 +450,20 @@ transactions, for any event a booking references: |
| still submits the old values passes because they match — and one |
| that submits values from before a public reschedule fails because |
| they do not (r4 A5). `kind` stays `meeting` forever (r4 B1). |
| -- `status` may only be `confirmed` or `cancelled`; `confirmed` is |
| - refused when the booking is `cancelled` or `expired` ("This booking |
| - was cancelled. Make a new event instead."); any update to a |
| - soft-deleted event is refused. |
| +- `status` depends on the booking's state (r6 f3). Booking `held`: |
| + the event is `tentative`; the host may leave it `tentative` or set |
| + `cancelled` (→ booking `expired`); `confirmed` is refused ("Only the |
| + person who booked this can confirm it.") — promotion is the confirm |
| + transaction's alone, and the lapsed-hold exclusion in §4 depends on |
| + a held event staying tentative. Booking `confirmed`: `confirmed` or |
| + `cancelled`. Booking `cancelled` or `expired`: `confirmed` is refused |
| + ("This booking was cancelled. Make a new event instead."); |
| + `tentative` is never accepted from the form for a booked event. Any |
| + update to a soft-deleted event is refused. |
| - `status = cancelled` on a `confirmed` booking → booking `cancelled`, |
| `cancelled_by = host`, reminder queue rows deleted, unsent mail |
| - deleted, `(cancelled, booker, seq)` queued. |
| + deleted, `(cancelled, booker, seq)` queued. On a `held` booking → |
| + `expired`, unsent mail deleted, nothing queued. |
| - `DeleteEvent` → the same, plus `sequence = sequence + 1` on the |
| soft-deleted row; a `held` booking → `expired`. |
| - `sequence` bumps are `gorm.Expr("sequence + 1")` everywhere. |
| @@ -584,7 +594,15 @@ row) and `retry_at`. `due_at` keeps its meaning — occurrence start |
| minus offset — and is never written by a retry (r5 f3). Claim |
| eligibility is `fired_at IS NULL AND due_at <= now AND (retry_at IS |
| NULL OR retry_at <= now) AND (claimed_at IS NULL OR claimed_at <= now - |
| -claimLease)` (`<=` at both boundaries — r5 f5). `MarkReminderSent`, |
| +claimLease)` (`<=` at both boundaries — r5 f5). **The claim is one |
| +transaction: the conditional UPDATE, then a SELECT of the same row by |
| +id, and the reminder's payload — the occurrence start it announces, |
| +the offset, the event — is built from THAT row, never from the |
| +candidate row the worker read before claiming** (r6 f2): a reschedule |
| +between the candidate read and the claim rewrites the row, the claim |
| +still succeeds, and a payload built from the stale candidate would |
| +announce the old time with a valid claim in hand. The same one |
| +transaction rule applies to `ClaimBookingMail`. `MarkReminderSent`, |
| `FailReminder` (attempt + 1, `claimed_at` refreshed), the give-up path, |
| the `ErrMailBusy` deferral (`retry_at = now + 30s`, `claimed_at = |
| NULL`, attempts untouched) and `RescheduleAfterFiring` are each |
| @@ -685,7 +703,9 @@ NULL`. |
| with `UNTIL` the night before (the 25-hour slack); the 47-hour |
| two-day weekly event keeps its second day on later occurrences; a |
| 1700 daily rule → `ErrIncompleteExpansion` from the strict reader |
| - and a 503 from the public page, while `/month` still renders. |
| + and a 503 from the public page, while `/month` still renders; an |
| + imported busy series with a `BYSETPOS` rule → the same, and the rule |
| + is still stored verbatim. |
| - **Concurrency**: 20 goroutines, one slot, one winner, `-race`. |
| - **Flow**: hold → one mail row, one send; GET link changes nothing; |
| confirm POST confirms, attendee, default reminder, one host mail; |
| @@ -737,7 +757,11 @@ NULL`. |
| re-claimed by another worker in the same second cannot mark, fail, |
| defer or advance it (`claim_id`, not `claimed_at`); a reschedule |
| followed by a simulated crash (no post-commit code) leaves the queue |
| - row already pointing at the new time; an `ErrMailBusy` deferral |
| + row already pointing at the new time; a reschedule that lands between |
| + a worker's candidate read and its claim makes the worker announce the |
| + NEW time (the payload comes from the claimed row); a title-only host |
| + save of a HELD event succeeds and leaves it tentative; a host attempt |
| + to set a held event `confirmed` is refused; an `ErrMailBusy` deferral |
| leaves `due_at` untouched and the reconstructed meeting time correct; |
| a claim exactly `claimLease` old is claimable; `NextDueAt` for a |
| claimed reminder is the lease end. |
| @@ -795,17 +819,18 @@ NULL`. |
| elapsed hours over 24."** |
| - Reminders → **Ticks**: **"`NextDue` is the earliest CLAIMABLE |
| instant across reminders, hold expiries, owed booking mail and |
| - IP-forgets. Retry times live in `due_at`. New scheduled work goes |
| - here, never in a goroutine. Cheap work runs before delivery; |
| - reminders get the first half of the delivery budget."** |
| + IP-forgets. New scheduled work goes here, never in a goroutine. |
| + Cheap work runs before delivery; reminders get the first half of the |
| + delivery budget."** |
| - Reminders: **"No sender error is definite. Never free a resource |
| because a send 'failed'. Every send goes through `boundedSender`. |
| `ErrMailBusy` is a deferral, not an attempt."** |
| - Reminders: **"Every queue row has a random `claim_id`; every |
| - completion mutation is conditional on it and checks RowsAffected. A |
| - reminder's `due_at` is the occurrence minus the offset and is never |
| - written by a retry — `retry_at` is. Booking mail's `due_at` is retry |
| - eligibility only."** |
| + completion mutation is conditional on it and checks RowsAffected, |
| + and a payload is built from the row the claim returned, never from |
| + the candidate read. A reminder's `due_at` is the occurrence minus the |
| + offset and is never written by a retry — `retry_at` is. Booking |
| + mail's `due_at` is retry eligibility only and has no `retry_at`."** |
| - Reminders: **"A reschedule repairs the event's queue rows in the |
| same transaction as the move."** |
| - Time: **"`Zone()` takes bounded input on public routes: `CommonZones` |
| @@ -883,4 +908,15 @@ B4 PARTIAL. Dispositions: |
| Rejected: none. |
| -**Round 6** — to be recorded below. |
| +**Round 6: REWRITE.** 3 blockers, 1 wording; round 5's six all HOLD. |
| + |
| +| Ref | Finding | Disposition | |
| +|---|---|---| |
| +| f1 | An unparseable rule is not "truncated", so the strict reader let it through | **Accepted — B23 widened.** Strict fails on unparseable busy rules too (§4.1). | |
| +| f2 | A valid claim can carry a stale payload from the candidate read | **Accepted — B22.** Claim = conditional UPDATE + re-read in one transaction; payload from the re-read (§6). | |
| +| f3 | A held event has no permitted unchanged status | **Accepted.** Status rules keyed on booking state; host cannot promote a hold (§5.5). | |
| +| f4 | B22/§11 said `retry_at` for both queues | **Accepted.** Wording aligned (B22, §11). | |
| + |
| +Rejected: none. |
| + |
| +**Round 7** — to be recorded below. |