oficina / calendar Public

booking: spec revision 5 after the fourth review

Six blockers and one polish item, all accepted. Two more calendar bugs
join the list this spec owns: daysBetween divides elapsed hours by 24
and so loses a day across spring DST, and Expand stops at 100,000
steps and returns an ordinary slice, so a rule from 1700 contributes
no busy time to 2026 — availability now fails closed on an incomplete
expansion. The stale-host-form hole I called harmless was not: an old
form re-saves an old start past every invariant, so a booked event's
scheduling fields are now the booking flow's and the host's form shows
them disabled. Reminders get a claim-conditional completion, because a
reminder finishing mid-reschedule deleted the replacement. CN= is
dropped from the ICS rather than given a parameter encoder it does not
need.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Paul Campbell pushed by paul@keymail.dev ab6a3a76d56bf7ed968a884e190d045536056204 parent ab18356
1 file changed, +161 −62
  • docs/superpowers/specs/2026-09-07-booking-design.md +161 −62
diff --git a/docs/superpowers/specs/2026-09-07-booking-design.md b/docs/superpowers/specs/2026-09-07-booking-design.md
index a874e8d..eaafe13 100644
--- a/docs/superpowers/specs/2026-09-07-booking-design.md
+++ b/docs/superpowers/specs/2026-09-07-booking-design.md
@@ -4,10 +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
-4.** Reviewed three times by GPT-6 (`gpt-6-astra` on the one run that
-exposed its identifier) through `codex-cli 0.153.0`; all three returned
-REWRITE. §12 records every finding and disposition, including the two
-this revision scopes rather than solves.
+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
+and disposition, including the two scoped rather than solved.
Asked for: a Calendly / Cal.com style "book time with me" page to share
with people outside the team. The host sets availability in plain text
@@ -58,10 +58,12 @@ first — and is swept on the existing tick.
| B15 | **A hold expires at the EARLIER of thirty minutes and the meeting's start; confirmation requires `now < start`** | Thirty minutes flat | A five-minute meeting confirmed after it ended (r2 B5). |
| B16 | **No sender error is "definite."** Nothing is freed because a send failed | Marking holds failed | `smtp.SendMail` can fail after acceptance (r2 A12). |
| B17 | **A slot is the same length on the wall clock and the elapsed clock, or it is not offered** | Round-tripping the start only | The fold makes a two-hour hour (r2 A13). |
-| B18 | **A booked event carries INVARIANTS that every host edit must keep**: timed, 5–480 minutes, wall = elapsed, status ∈ {confirmed, cancelled}, no rule, and never revived from a terminal booking state | Letting the host edit it like any event | A zero-length edit makes slot generation loop forever inside the mutex (r3 f1); a stale form revives a cancelled booking (r3 f5). |
+| B18 | **A booked event's SCHEDULING is owned by the booking flow.** The host's form may change title, notes, location, and status to cancelled; it may NOT change start, end, tz, all_day, kind or rrule, and may not revive a terminal booking. `UpdateEvent` refuses, naming the booking | Invariant checks on host-supplied times | Invariants were not enough: a stale form re-saved with an old 09:00 passes every invariant and moves a rescheduled meeting back (r4 A5); a kind change to `note` makes a confirmed booking non-busy (r4 B1). Locking the fields closes both. |
| 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). |
+| 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,15 +147,19 @@ UNIQUE (booking_id, kind, recipient, revision)
- 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.
-- **Claim (B13):** `UPDATE … SET claimed_at = now, claim_id = <random>
- WHERE id = ? AND sent_at IS NULL AND due_at <= now AND (claimed_at IS
- NULL OR claimed_at < now - claimLease)`; the claimer keeps
- `claim_id`. **Acknowledge:** `SET sent_at = now WHERE id = ? AND
- claim_id = ?`. **Fail:** `SET attempts = attempts + 1, due_at = now +
- claimLease, claimed_at = NULL WHERE id = ? AND claim_id = ?` — the
- retry time is EXPLICIT in `due_at`, so `NextDueAt` never advertises
- work nobody may claim (r3 f6). At `MaxReminderAttempts`, `sent_at =
- -1` (gave up) and a log line says `stage=booking.mail code=gave_up`.
+- **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`
+ on claim is what makes `due_at` the ONE claimable instant: a crash
+ after a claim leaves a row that becomes due again when the lease
+ ends, and `NextDueAt` reads nothing but `due_at` (r4 A6).
+ **Acknowledge:** `SET sent_at = now WHERE id = ? AND claim_id = ?`.
+ **Fail (a real send error):** `SET attempts = attempts + 1 WHERE id =
+ ? AND claim_id = ?` — `due_at` already points at the lease end.
+ **Defer (`ErrMailBusy`, no SMTP call was made):** `SET due_at = now +
+ 30s, claimed_at = NULL WHERE id = ? AND claim_id = ?`, attempts
+ untouched (r4 B4). At `MaxReminderAttempts`, `sent_at = -1` (gave up)
+ and a log line says `stage=booking.mail code=gave_up`.
- **Supersession (B19):** when a transition makes a booking
`cancelled` or `expired`, every unsent row for that booking is
deleted in the same transaction and the `cancelled` rows are queued
@@ -284,15 +290,38 @@ time must be what the calendar shows.
drops an override whose master the query did not select. Fix:
override rows returned by `rangeRows` contribute their `SeriesID` to
the set `overridesFor` loads, so pass 2 places them.
-- **Long final occurrence (r3 f7).** `rangeRows` excludes a master
- when `until_utc < from`, but `UNTIL` bounds the last occurrence's
- START, not its end. Fix: the exclusion becomes `until_utc +
- (end_utc - start_utc) < from`, using the master's own duration.
-
-Regression tests for both in `occurrence_test.go`: a daily series
-ending in three days with its last occurrence moved ten days out shows
-on that day; a series whose final occurrence spans four days shows on
-its third.
+- **Long final occurrence (r3 f7, r4 A7).** `rangeRows` excludes a
+ master when `until_utc < from`, but `UNTIL` bounds the last
+ occurrence's START, not its end, and an occurrence's elapsed length
+ is not the master's across a DST day. Fix: the exclusion becomes
+ `until_utc + (end_utc - start_utc) + 90000 < from` — the master's
+ duration plus a 25-hour day of slack, which is conservative for any
+ single transition. A day's extra expansion is cheap; a lost
+ occurrence is not.
+- **Multi-day span across spring DST (r4 B2).** `daysBetween` divides
+ elapsed hours by 24, so a 28 March 09:00 → 30 March 09:00 event in
+ Dublin (47 h) spans "one day" and every later occurrence ends a day
+ 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.
+
+Regression tests in `occurrence_test.go`: a daily series ending in
+three days with its last occurrence moved ten days out shows on that
+day; a series whose final occurrence spans four days shows on its
+third; a weekly two-day Dublin event created 28 March 09:00 – 30 March
+09:00 still occupies 30 March on its first AND a later occurrence; a
+daily rule with `DTSTART` in 1700 makes `EventsInRangeStrict` return
+`ErrIncompleteExpansion` for a 2026 window while `EventsInRange` still
+answers.
### 4.2 Cost
@@ -395,14 +424,21 @@ host after confirmation and asserts none reaches manage, ICS or mail.
ordinary form, and `UpdateEvent`/`DeleteEvent` enforce, inside their
transactions, for any event a booking references:
-- Refused with a `ValidationError` naming the booking: an `RRule`
- ("This is a booked meeting; it cannot repeat."); `all_day`; a
- duration outside 5–480 minutes or with `end <= start`; a start/end
- pair whose wall-clock length differs from its elapsed length in the
- event's `tz` (the B17 check, applied to the edit); `status =
- confirmed` when the booking is `cancelled` or `expired` ("This
- booking was cancelled. Make a new event instead."); `status` other
- than confirmed/cancelled; any update to a soft-deleted event.
+- **Scheduling fields are read-only to the host.** `UpdateEvent`
+ compares the submitted `start_local`, `end_local`, `tz`, `all_day`,
+ `kind` and `rrule` with the stored row and refuses if ANY differs:
+ `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
+ 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 = cancelled` on a `confirmed` booking → booking `cancelled`,
`cancelled_by = host`, reminder queue rows deleted, unsent mail
deleted, `(cancelled, booker, seq)` queued.
@@ -411,11 +447,9 @@ transactions, for any event a booking references:
- `sequence` bumps are `gorm.Expr("sequence + 1")` everywhere.
- The reminder claim path drops a queue row whose event is `cancelled`.
-A stale host form is covered without a revision field: the only edits
-a stale form could apply are ones the invariants above refuse or ones
-that are harmless to the booking (title, notes, location, which the
-booker never sees). Adding a revision to the general event form is
-explicitly not done here.
+No revision field on the general event form is needed: the only
+fields a stale form could move are refused by the comparison above,
+and the rest (title, notes, location) the booker never sees.
### 5.6 Reschedule and cancel
@@ -470,8 +504,8 @@ DTSTAMP:{event updated_at}
SEQUENCE:{event sequence}
DTSTART;TZID={event tz}:{start_local} DTEND;TZID=…:{end_local}
SUMMARY:{snapshot title} DESCRIPTION:{snapshot description, if any}
-ORGANIZER;CN={host name or handle}:mailto:{host address}
-ATTENDEE;CN={booker name};PARTSTAT=ACCEPTED;RSVP=FALSE:mailto:{booker}
+ORGANIZER:mailto:{host address}
+ATTENDEE;PARTSTAT=ACCEPTED;RSVP=FALSE:mailto:{booker}
STATUS:CONFIRMED | CANCELLED -- effective state, not events.status alone
END:VEVENT / END:VCALENDAR
```
@@ -480,9 +514,13 @@ Served at `GET /book/m/{token}/calendar.ics` for `confirmed` and
`cancelled` bookings (`text/calendar; charset=utf-8`,
`Content-Disposition: attachment; filename="booking.ics"`); `held` and
`expired` → 404. The event row is read including soft-deleted ones, so
-a host-deleted booking still exports its CANCEL. Folding, escaping and
-date formatting reuse `ics_export.go`'s helpers. No `URL:`, no
-`LOCATION`, no other attendee.
+a host-deleted booking still exports its CANCEL. Folding, TEXT escaping and
+date formatting reuse `ics_export.go`'s helpers. **No `CN=`
+parameters**: a parameter value has its own quoting rules (RFC 5545
+§3.2, RFC 6868) that `escapeText` does not implement, and a name is
+not needed for the invitation to work — `SUMMARY` names the meeting
+and the addresses name the people (r4 B6). No `URL:`, no `LOCATION`,
+no other attendee.
---
@@ -517,9 +555,22 @@ around the configured sender, `boundedSender`:
that honours `ctx` — and is recorded in the calendar spec's debt list
in the same commit. This spec does not claim to bound what it cannot.
-Success → acknowledge with the claim id. Error → fail with the claim
-id (`due_at = now + claimLease`). Nothing is concluded from an error
-except "try again later" (B16).
+Success → acknowledge with the claim id. `ErrMailBusy` → DEFER with
+the claim id: due again in 30 seconds, attempts untouched, because no
+SMTP call was made and a full wrapper is not a failed address (r4 B4).
+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.
---
@@ -542,8 +593,9 @@ Over → 429 "Too many bookings in progress. Try again later."
An in-process token bucket per `ip_fp` on every public route: 60
requests/minute, 429 beyond. **Bounded:** at most 10,000 keys; a key
-idle for five minutes is evicted on the next insert; when full, the
-oldest idle key goes. Memory only, resets on restart. The gate also
+idle for five minutes is evicted on the next insert; when full with no
+idle key, a NEW key is refused with 429 and `Retry-After: 60` (r4
+B7). Memory only, resets on restart. The gate also
sets the two public headers. It is the cheap wall in front of the
semaphore (§4.2) and the durable caps (§7.1).
@@ -582,10 +634,12 @@ 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 of: next reminder `due_at`; next `held`
-`hold_expires_at`; next `booking_mails.due_at` with `sent_at IS NULL`
-(retry times are in `due_at`, so this is claimable work); next
-`ip_forget_at` with `ip_fp IS NOT NULL`.
+`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`;
+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`.
---
@@ -605,8 +659,12 @@ and never with more than a 5-second budget.
transition, both ends round-trip, every slot is exactly its duration
in elapsed seconds, pinned unix numbers; `DurationMin: 0` and an
inverted window return errors, not lists.
-- **Range query**: the moved-override-of-finished-series case and the
- long-final-occurrence case both show.
+- **Range query**: the moved-override-of-finished-series case; the
+ long-final-occurrence case; the final occurrence across the fold
+ 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.
- **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;
@@ -619,9 +677,12 @@ and never with more than a 5-second budget.
`METHOD:CANCEL`, unsent rows gone; host adds note/location/attendee →
none reaches manage, ICS or mail; host cancels via edit → booker
mailed, page says cancelled by host; host deletes → same, CANCEL ICS
- with higher sequence; host edits refused: RRule, all-day, zero
- length, 481 minutes, a DST-straddling pair, reviving a cancelled
- booking, editing a deleted event; page title changed after hold →
+ 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
+ event; page title changed after hold →
manage and ICS still show the snapshot.
- **Zone change**: host moves Dublin → Tokyo; reschedule writes
`tz=Asia/Tokyo` and all four time columns agree (`ToUTC(start_local,
@@ -633,7 +694,10 @@ and never with more than a 5-second budget.
send one row twice; cancel deletes every unsent row; `NextDueAt`
reflects `due_at` not the lease; `boundedSender` returns
`ErrMailTimeout` within budget against a stalling sender and
- `ErrMailBusy` when four are in flight.
+ `ErrMailBusy` when four are in flight; a row deferred by
+ `ErrMailBusy` keeps `attempts = 0` and sends once capacity returns;
+ a crash after a claim (simulated: claim, do not ack) leaves
+ `NextDueAt` at the lease end, not now.
- **Caps, gate, semaphore**: sixth open hold per IP → 429; 21st attempt
in 24h → 429 with none open; `MaxReschedules`; 61st public GET in a
minute → 429; the gate holds ≤ 10,000 keys after 20,000 distinct IPs;
@@ -646,7 +710,12 @@ and never with more than a 5-second budget.
- **`Zone()`**: 100 goroutines × garbage under `-race`; no failures
cached.
- **Reminders**: a cancelled event's queue row is dropped by the claim
- path.
+ 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.
+- **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=`.
- **The one rule**: booker name, address, note, the token, the
snapshotted title and description, the page's availability — none in
the raw bytes.
@@ -668,6 +737,9 @@ and never with more than a 5-second budget.
- **A revision field on the general event form.** Booked events are
protected by invariants 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
+ time; the reschedule mail that follows describes the new one.
- Locale; retention (rows stay, sealed, minus `ip_fp`).
---
@@ -683,16 +755,25 @@ and never with more than a 5-second budget.
- Data: **"Every booking transition is a conditional UPDATE naming the
state and revision it expects, checking RowsAffected. Zero rows is
'someone moved it first', never a retry."**
-- Data: **"A booked event keeps its invariants under every host edit:
- timed, 5–480 minutes, wall = elapsed, confirmed or cancelled, no
- rule, never revived."**
+- Data: **"A booked event's start, end, tz, all_day, kind and rrule
+ belong to the booking flow. A host edit that changes any of them is
+ refused; the form shows them disabled."**
+- Recurrence: **"`Expand` may stop early. Anything that decides FREE
+ time reads `EventsInRangeStrict` and fails closed on
+ `ErrIncompleteExpansion`."**
+- Recurrence: **"A day span is a difference of CIVIL dates, never
+ 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."**
- Reminders: **"No sender error is definite. Never free a resource
- because a send 'failed'. Every send goes through `boundedSender`."**
+ 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."**
- 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
@@ -736,4 +817,22 @@ Rejected: none. Scoped rather than solved: the socket-level send
bound (finding 3, upstream) and the per-request cost of recurrence
expansion (finding 8, the calendar's own).
-**Round 4** — to be recorded below.
+**Round 4: REWRITE.** 7 new (6 blockers, 1 hardening); of round 3,
+A5, A6, A7 PARTIAL; 3 and 8 HOLD as scoped. Dispositions:
+
+| Ref | Finding | Disposition |
+|---|---|---|
+| A5 | A stale host form re-saves old scheduling fields past every invariant | **Accepted — B18 rewritten.** Scheduling fields are read-only to the host; `UpdateEvent` refuses any difference (§5.5). |
+| A6 | A claim does not advance `due_at`; reminders have the same mismatch | **Accepted — B22.** Claim sets `due_at = lease end`; reminders' `NextDueAt` uses the claimable instant (§2.5, §8). |
+| A7 | Master duration is not a conservative bound across DST | **Accepted.** 25-hour slack on the `UNTIL` exclusion (§4.1). |
+| B1 | Kind → `note` makes a confirmed booking non-busy | **Accepted — B18.** `kind` is a locked field. |
+| B2 | `daysBetween` loses a day across spring DST | **Accepted, a third calendar bug.** Civil-date difference (§4.1). |
+| B3 | A reminder finishing mid-reschedule deletes the replacement | **Accepted — B22.** Mark and advance conditional on the claim (§6). |
+| B4 | `ErrMailBusy` burns attempts without reaching SMTP | **Accepted — B22.** Defer path, both queues (§2.5, §6). |
+| B5 | `Expand` truncation read as free time | **Accepted — B23.** `ExpandComplete`, `EventsInRangeStrict`, 503 fail-closed (§4.1). |
+| B6 | `CN=` cannot reuse TEXT escaping | **Accepted by omission.** No `CN=` (§5.8). |
+| B7 | Full bucket with no idle key undefined | **Accepted.** New key refused, 429 + `Retry-After` (§7.2). |
+
+Rejected: none.
+
+**Round 5** — to be recorded below.