amadan
booking: spec revision 6 after the fifth review
All four blockers were inside the reminder queue's integration and one form. Reminders gain a random claim_id and a retry_at separate from due_at, because a whole-second claimed_at is not an identity and a reminder's due_at IS the occurrence minus the offset — writing a retry time into it moved the meeting. Reschedule rebuilds the queue rows inside its own transaction. And disabled form controls submit nothing, so a booked event's form carries its scheduling snapshot as hidden inputs, including the event's own tz.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 file changed,
+81
−35
docs/superpowers/specs/2026-09-07-booking-design.md+81 −35
diff --git a/docs/superpowers/specs/2026-09-07-booking-design.md b/docs/superpowers/specs/2026-09-07-booking-design.md| index eaafe13..534834c 100644 |
| --- a/docs/superpowers/specs/2026-09-07-booking-design.md |
| +++ b/docs/superpowers/specs/2026-09-07-booking-design.md |
| @@ -4,9 +4,10 @@ |
| 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 |
| -5.** Reviewed four times by GPT-6 (`gpt-6-astra` on the one run that |
| -exposed its identifier) through `codex-cli 0.153.0`; all four returned |
| -REWRITE, each round narrower than the last. §12 records every finding |
| +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 |
| and disposition, including the two scoped rather than solved. |
| Asked for: a Calendly / Cal.com style "book time with me" page to share |
| @@ -62,7 +63,9 @@ 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 | **A claim advances `due_at` to the lease end, for booking mail AND reminders' `NextDueAt`; `ErrMailBusy` defers without counting an attempt; a reminder's completion is conditional on the claim it holds** | Claim by ID; every error an attempt | A crash after a claim advertised work nobody could take (r4 A6); four stalled sockets would burn every other message's attempts without ever reaching SMTP (r4 B4); a reminder finishing during a reschedule deleted the replacement (r4 B3). | |
| +| 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). | |
| +| 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). | |
| --- |
| @@ -145,8 +148,10 @@ UNIQUE (booking_id, kind, recipient, revision) |
| `recipient` ∈ {`booker`, `host`}; `revision` = the event's |
| `sequence` when the row was made. |
| - Rows are created inside the transition's transaction with `due_at = |
| - now`. The handler that made them tries once, immediately (§6); the |
| - drain owns retries. |
| + now`. For booking mail `due_at` means ONLY "eligible to claim from"; |
| + it carries no other meaning, which is why a claim may advance it |
| + (contrast reminders, §6). The handler that made them tries once, |
| + immediately (§6); the drain owns retries. |
| - **Claim (B13, B22):** `UPDATE … SET claimed_at = now, claim_id = |
| <random>, due_at = now + claimLease WHERE id = ? AND sent_at IS NULL |
| AND due_at <= now`; the claimer keeps `claim_id`. Advancing `due_at` |
| @@ -430,8 +435,13 @@ transactions, for any event a booking references: |
| `ValidationError{"start", "This is a booked meeting. Its time is |
| changed by the person who booked it, or by cancelling and letting |
| them rebook."}` (one message; the field named is whichever differed |
| - first). The event form renders those fields disabled with that |
| - sentence beside them when the event is booked; a stale form that |
| + first). When the event is booked, the form renders those controls |
| + DISABLED with that sentence beside them AND carries the stored |
| + values as hidden inputs — `start`, `end`, `tz` (the EVENT's own zone, |
| + not the host's profile zone), `all_day`, `kind`, `repeat` — so a |
| + scriptless title-only save submits exactly what is stored and |
| + passes; `readEventForm` takes `tz` from that hidden field when the |
| + event is booked and from `p.Zone` otherwise (B25). A stale form that |
| 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). |
| @@ -463,7 +473,10 @@ sequence = sequence + 1 WHERE id = ? AND sequence = ?` (rev) — **all |
| five time columns together, `tz` = the zone the slot was computed in** |
| (B21) — zero rows → re-render; `reschedules = reschedules + 1`; unsent |
| `rescheduled` rows deleted; `(rescheduled, booker + host, seq+1)` |
| -queued. After commit: `RescheduleReminder` per reminder. Try both. |
| +queued; **and every reminder queue row for the event is rebuilt in the |
| +same transaction** (a `tx` form of `rescheduleAfter` that recomputes |
| +`due_at` from the new start, clears `claim_id`, `claimed_at` and |
| +`retry_at`) (B24). Try both mails after commit. |
| `POST …/cancel`: `UPDATE bookings SET state='cancelled', cancelled_at, |
| cancelled_by='booker' WHERE id=? AND state='confirmed'`; event |
| @@ -562,15 +575,21 @@ Any other error → fail with the claim id (attempts + 1; `due_at` is |
| already the lease end). Nothing is concluded from an error except "try |
| again later" (B16). |
| -**Reminders get the same three outcomes** (B22): `FailReminder` gains |
| -a defer path for `ErrMailBusy`, and `MarkReminderSent` and |
| -`RescheduleAfterFiring` become conditional on the claim the worker |
| -holds — `WHERE id = ? AND claimed_at = ?` with the value the claim |
| -wrote — so a reminder that finishes sending while a reschedule has |
| -already reset the row does NOT mark the new row fired and does NOT |
| -advance it past the new time (r4 B3). Zero rows → the worker stops; |
| -its message was sent and described the old time, which is stated in |
| -§10. |
| +**Reminders get the same three outcomes and the same identity** (B22). |
| +`reminder_queues` gains two nullable columns by an additive migration: |
| +`claim_id` (random, set on claim, cleared by any reschedule of the |
| +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`, |
| +`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 |
| +`WHERE id = ? AND claim_id = ?` and each checks `RowsAffected`; the |
| +mark and the advance happen in ONE transaction so a worker that loses |
| +its claim between them does neither. Zero rows → the worker stops; its |
| +message was sent and described the old time (§10). |
| --- |
| @@ -634,9 +653,9 @@ shared by everything, **cheap work first, delivery last** (r3 f3): |
| any is due, else a due reminder, else one booking mail. Never more, |
| and never with more than a 5-second budget. |
| -`NextDueAt` = the earliest CLAIMABLE instant of: reminders — `due_at` |
| -when unclaimed, else `max(due_at, claimed_at + claimLease)` (the |
| -existing mismatch, fixed here — r4 A6); next `held` `hold_expires_at`; |
| +`NextDueAt` = the earliest CLAIMABLE instant of: reminders — |
| +`max(due_at, coalesce(retry_at, 0), coalesce(claimed_at + claimLease, |
| +0))` per unfired row (the existing mismatch, fixed here — r4 A6); next `held` `hold_expires_at`; |
| next `booking_mails.due_at` with `sent_at IS NULL` (claims advance it, |
| so it is always claimable); next `ip_forget_at` with `ip_fp IS NOT |
| NULL`. |
| @@ -679,9 +698,11 @@ NULL`. |
| mailed, page says cancelled by host; host deletes → same, CANCEL ICS |
| with higher sequence; host edits refused: any change to start, end, tz, all_day, |
| kind or rrule (each tested), reviving a cancelled booking, editing a |
| - deleted event; a host edit that changes only the title succeeds; a |
| - stale host form submitted after a public reschedule is refused; |
| - the event form shows the scheduling fields disabled for a booked |
| + deleted event; a scriptless host edit that changes only the title succeeds, |
| + and still succeeds after the host changes their profile zone (the |
| + hidden `tz` is the event's); a stale host form submitted after a |
| + public reschedule is refused; the event form shows the scheduling |
| + controls disabled AND carries hidden inputs for each, for a booked |
| event; page title changed after hold → |
| manage and ICS still show the snapshot. |
| - **Zone change**: host moves Dublin → Tokyo; reschedule writes |
| @@ -710,9 +731,14 @@ NULL`. |
| - **`Zone()`**: 100 goroutines × garbage under `-race`; no failures |
| cached. |
| - **Reminders**: a cancelled event's queue row is dropped by the claim |
| - path; a worker that claimed a row which was then rescheduled |
| - (claimed_at reset) cannot mark it fired or advance it; `NextDueAt` |
| - for a claimed reminder is the lease end. |
| + path; a worker that claimed a row which was then rescheduled AND |
| + 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 |
| + 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. |
| - **ICS**: a booker named `Doe, Jane "JJ": Support` produces a file |
| the app's parser AND a line-by-line check accept, with the address |
| unchanged and no `CN=`. |
| @@ -729,13 +755,15 @@ NULL`. |
| - Group / round-robin pages; booker custom fields; payments; video |
| links; reminders to the booker. |
| -- **Host-initiated reschedule that tells the booker.** A host edit |
| - that moves the time does not mail the booker; the event page says so |
| - beside the booking badge. Cancel and delete DO tell the booker. |
| +- **Host-initiated reschedule.** A booked event's time belongs to the |
| + booking flow (B18); the host's form cannot move it, and says so |
| + beside the disabled controls. To move a booked meeting the host |
| + cancels and lets the booker rebook. Cancel and delete DO tell the |
| + booker. |
| - **Automatic calendar sync.** The ICS is a download; the change mail |
| says to download it again. |
| - **A revision field on the general event form.** Booked events are |
| - protected by invariants instead (§5.5). |
| + protected by the field lock instead (§5.5). |
| - **A context-honouring SMTP sender.** Upstream (§6). |
| - **Recalling a reminder mid-send.** A reminder that was already |
| handed to the sender when the booker rescheduled describes the old |
| @@ -771,9 +799,13 @@ NULL`. |
| - 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: **"A claim advances `due_at` to the lease end (booking |
| - mail) or `NextDue` treats a claimed row as due at the lease end |
| - (reminders). Completion is conditional on the claim held."** |
| +- 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."** |
| +- 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` |
| plus the host's own zone."** |
| - Time: **"A booking slot is the same length on the wall clock and the |
| @@ -835,4 +867,18 @@ A5, A6, A7 PARTIAL; 3 and 8 HOLD as scoped. Dispositions: |
| Rejected: none. |
| -**Round 5** — to be recorded below. |
| +**Round 5: REWRITE.** 4 blockers, 2 hardening; of round 4, A6, B3, |
| +B4 PARTIAL. Dispositions: |
| + |
| +| Ref | Finding | Disposition | |
| +|---|---|---| |
| +| A6/B3/f1 | `claimed_at` (whole seconds) is not a claim identity; failure and give-up paths unconditioned | **Accepted — B22 rewritten.** `claim_id` on `reminder_queues`; every mutation conditional; mark + advance atomic (§6). | |
| +| f2 | Reschedule commits before the reminder repair | **Accepted — B24.** Queue rows rebuilt inside the reschedule transaction (§5.6). | |
| +| B4/f3 | Writing a retry time into a reminder's `due_at` moves the meeting | **Accepted — B22.** `retry_at` column; `due_at` never written by a retry; `NextDueAt` over all three (§6, §8). | |
| +| f4 | Disabled controls submit nothing; reader's `TZ = p.Zone` | **Accepted — B25.** Hidden snapshot incl. the event's `tz`; reader takes it for booked events (§5.5). | |
| +| f5 | `<` vs `<=` at the lease boundary | **Accepted.** `<=` both sides (§6). | |
| +| f6 | §10 still described host rescheduling | **Accepted.** Rewritten (§10). | |
| + |
| +Rejected: none. |
| + |
| +**Round 6** — to be recorded below. |