oficina / calendar Public

booking: the second review's twelve, all fixed

GPT-6's second pass found four blockers and eight partials in the
first round of fixes. Every one is closed here, each with a regression.

The leak that survived the first round: the bounded sender returned
the SMTP sender's own error, which names the recipient, and rastrillo's
sign-in handler logs what it is handed. Now the wrapper discards the
text at its boundary — busy, timeout, refused are the only three
errors above it.

A browser's textarea submits CRLF and the note refused its own line
breaks; normalised before anything is measured. The hold sealed its
snapshot before taking the write lock, so a page renamed in between
gave the booking one title and the event another; sealed from the
re-read page now. The reschedule re-read the host but not the page and
its windows; withdrawn hours and a closed page refuse the move. A
confirmed booking forgets its owed hold mail in its own transaction. A
deferral was reported as a send, on the page and in the drain's count;
deferred and retired are errors to the caller now. Bookkeeping after a
send runs on its own short context, never the phase's, and a reminder
phase that runs out hands over to booking mail rather than returning.
Reminder deferral reports its rows. The booked form's locked controls
include zone, kind, all-day and repeat. The UNTIL slack test pins UNTIL
at the final occurrence's start, so it now exercises the slack.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Paul Campbell pushed by paul@keymail.dev 1 month ago 5e3f047abc3890105137ecccf0b1394be0d3ebc0 parent 1e2ba83

12 files changed, +326 −54

internal/calendar/booking_mail.go +21 −6
internal/calendar/handlers_booking_public.go +5 −20
internal/calendar/handlers_booking_util.go +18 −0
internal/calendar/handlers_jobs.go +19 −6
internal/calendar/mail_bounded.go +11 −1
internal/calendar/store_booking_confirm.go +14 −4
internal/calendar/store_booking_flow.go +8 −5
internal/calendar/store_drain.go +6 −6
internal/calendar/templates/event_form.html +7 −1
internal/calendartest/booking_flow_test.go +3 −1
internal/calendartest/booking_review2_test.go +202 −0
internal/calendartest/occurrence_strict_test.go +12 −4

diff --git a/internal/calendar/booking_mail.go b/internal/calendar/booking_mail.go

index 9827f83..7bc5691 100644

--- a/internal/calendar/booking_mail.go

+++ b/internal/calendar/booking_mail.go

@@ -83,8 +83,10 @@ func (h *handlers) sendBookingMail(ctx context.Context, row BookingMail, now tim

// expired, a rescheduled mail after a cancellation. Mail says the

// current state or says nothing.

if obsolete(row, v, now) {

- _, err := h.store.MarkBookingMailSent(ctx, row.ID, row.ClaimID, now.Unix())

- return err

+ if _, err := h.store.MarkBookingMailSent(keep(ctx), row.ID, row.ClaimID, now.Unix()); err != nil {

+ return err

+ }

+ return errMailRetired

}

to, subject, body, err := h.renderBookingMail(row, v, now)

if err != nil {

@@ -95,13 +97,18 @@ func (h *handlers) sendBookingMail(ctx context.Context, row BookingMail, now tim

err = h.mailer.Send(sendCtx, to, subject, body)

switch {

case err == nil:

- _, err := h.store.MarkBookingMailSent(ctx, row.ID, row.ClaimID, now.Unix())

+ ok, err := h.store.MarkBookingMailSent(keep(ctx), row.ID, row.ClaimID, now.Unix())

+ if err == nil && !ok {

+ return errClaimLost

+ }

return err

case errors.Is(err, ErrMailBusy):

- _, err := h.store.DeferBookingMail(ctx, row.ID, row.ClaimID, now.Unix())

- return err

+ if _, err := h.store.DeferBookingMail(keep(ctx), row.ID, row.ClaimID, now.Unix()); err != nil {

+ return err

+ }

+ return errMailDeferred

default:

- gaveUp, merr := h.store.MarkBookingMailFailed(ctx, row.ID, row.ClaimID)

+ gaveUp, merr := h.store.MarkBookingMailFailed(keep(ctx), row.ID, row.ClaimID)

if merr != nil {

return merr

}

@@ -117,6 +124,14 @@ func (h *handlers) sendBookingMail(ctx context.Context, row BookingMail, now tim

}

+// The two outcomes that are neither a send nor a failure. Both are

+// errors to the caller so a deferral is never counted as delivery and a

+// hold page never says "sent" about a row that went back in the queue.

+var (

+ errMailDeferred = errors.New("calendar: mail deferred")

+ errMailRetired = errors.New("calendar: mail retired unsent")

// obsolete says whether a queued row has been overtaken by the state

// it would describe.

func obsolete(row BookingMail, v BookingView, now time.Time) bool {

diff --git a/internal/calendar/handlers_booking_public.go b/internal/calendar/handlers_booking_public.go

index fded7f1..d7a8d40 100644

--- a/internal/calendar/handlers_booking_public.go

+++ b/internal/calendar/handlers_booking_public.go

@@ -135,13 +135,6 @@ func (h *handlers) picker(w http.ResponseWriter, r *http.Request, page BookingPa

return d, true

}

-func hostNameOf(host Person) string {

- if host.Handle != nil {

- return *host.Handle

- }

- return ""

// bookPage is GET /book/{handle}/{code}.

func (h *handlers) bookPage(w http.ResponseWriter, r *http.Request) {

page, host, ok := h.publicPage(w, r)

@@ -212,22 +205,14 @@ func (h *handlers) bookPost(w http.ResponseWriter, r *http.Request) {

render(w, "book_page", h.page(r, page.Title, d))

}

-// hasControl reports a control character (a line break, a NUL) in a

-// single-line field.

-func hasControl(s string) bool {

- for _, r := range s {

- if r < 0x20 || r == 0x7f {

- return true

- }

- return false

// hold validates the details and makes the hold (§5.2).

func (h *handlers) hold(w http.ResponseWriter, r *http.Request, page BookingPageView, host Person, d bookPageData, zone string) {

- d.Name = strings.TrimSpace(r.FormValue("name"))

+ // A browser submits a textarea's line breaks as CRLF. Normalised

+ // to LF before anything is measured or refused, so two paragraphs

+ // are two paragraphs and not a control character.

+ d.Name = strings.TrimSpace(strings.ReplaceAll(r.FormValue("name"), "\r", ""))

d.Address = normalizeAddress(r.FormValue("address"))

- d.Note = strings.TrimSpace(r.FormValue("note"))

+ d.Note = strings.TrimSpace(strings.ReplaceAll(r.FormValue("note"), "\r\n", "\n"))

reject := func(field, msg string) {

d.Error, d.ErrorField = msg, field

w.WriteHeader(http.StatusUnprocessableEntity)

diff --git a/internal/calendar/handlers_booking_util.go b/internal/calendar/handlers_booking_util.go

index 720e64d..11ac92d 100644

--- a/internal/calendar/handlers_booking_util.go

+++ b/internal/calendar/handlers_booking_util.go

@@ -190,3 +190,21 @@ func parseUnix(s string) (int64, bool) {

n, err := strconv.ParseInt(strings.TrimSpace(s), 10, 64)

return n, err == nil && n > 0

}

+func hostNameOf(host Person) string {

+ if host.Handle != nil {

+ return *host.Handle

+ }

+ return ""

+// hasControl reports a control character (a line break, a NUL) in a

+// single-line field.

+func hasControl(s string) bool {

+ for _, r := range s {

+ if r < 0x20 || r == 0x7f {

+ return true

+ }

+ return false

diff --git a/internal/calendar/handlers_jobs.go b/internal/calendar/handlers_jobs.go

index 8304aac..e1a0e54 100644

--- a/internal/calendar/handlers_jobs.go

+++ b/internal/calendar/handlers_jobs.go

@@ -67,9 +67,11 @@ func (h *handlers) drain(ctx context.Context, now time.Time) (int, error) {

rctx, rcancel := context.WithDeadline(ctx, now.Add(drainDeadline/2))

sent, err := h.drainReminderQueue(rctx, now, now.Add(drainDeadline/2))

rcancel()

- if err != nil {

+ if err != nil && !errors.Is(err, context.DeadlineExceeded) {

return sent, err

}

+ // The reminder phase running out is a handover, not a failure:

+ // booking mail still gets the second half.

mctx, mcancel := context.WithDeadline(ctx, deadline)

defer mcancel()

mailed, err := h.drainBookingMail(mctx, now, deadline)

@@ -110,7 +112,7 @@ func (h *handlers) drainReminderQueue(ctx context.Context, now, deadline time.Ti

if errors.Is(err, ErrMailBusy) {

// Nothing was tried: the sender is full. Back in the

// queue in thirty seconds, attempts untouched.

- if err := h.store.DeferReminder(ctx, due.Queue.ID, due.ClaimID, now.Unix()); err != nil {

+ if _, err := h.store.DeferReminder(keep(ctx), due.Queue.ID, due.ClaimID, now.Unix()); err != nil {

return sent, err

}

continue

@@ -120,7 +122,7 @@ func (h *handlers) drainReminderQueue(ctx context.Context, now, deadline time.Ti

// permanently undeliverable address being retried forever.

h.logger.Error("calendar: send reminder", "stage", "reminders.send",

"why", mailCode(err), "attempts", due.Queue.Attempts)

- if err := h.store.FailReminder(ctx, due.Queue.ID, due.ClaimID, due.StartUTC); err != nil {

+ if err := h.store.FailReminder(keep(ctx), due.Queue.ID, due.ClaimID, due.StartUTC); err != nil {

return sent, err

}

continue

@@ -128,7 +130,7 @@ func (h *handlers) drainReminderQueue(ctx context.Context, now, deadline time.Ti

// Mark and advance in one transaction, conditional on the

// claim this worker holds: a reschedule that overtook it finds

// this a no-op, which is the whole point of the claim id.

- if err := h.store.CompleteReminder(ctx, due, now.Unix()); err != nil {

+ if err := h.store.CompleteReminder(keep(ctx), due, now.Unix()); err != nil {

return sent, err

}

sent++

@@ -217,7 +219,7 @@ func (h *handlers) DrainOnRequest(ctx context.Context) {

}

if err := h.sendReminder(ctx, due, now); err != nil {

if errors.Is(err, ErrMailBusy) {

- _ = h.store.DeferReminder(ctx, due.Queue.ID, due.ClaimID, now.Unix())

+ _, _ = h.store.DeferReminder(keep(ctx), due.Queue.ID, due.ClaimID, now.Unix())

return

}

h.logger.Error("calendar: opportunistic reminder", "stage", "reminders.send",

@@ -244,8 +246,19 @@ func (h *handlers) drainBookingMail(ctx context.Context, now, deadline time.Time

return sent, nil

}

if err := h.sendBookingMail(ctx, row, time.Now()); err == nil {

- sent++

+ sent++ // deferred and retired rows report themselves as errors, and are not sends

}

return sent, nil

}

+// keep is the bookkeeping context: what a send's outcome is recorded

+// under. Detached from the phase deadline — a send that ran out its

+// budget still has to be marked failed or deferred, or the lease does

+// the same job five minutes later at the cost of a duplicate — and

+// bounded on its own so bookkeeping cannot hang either.

+func keep(ctx context.Context) context.Context {

+ k, cancel := context.WithTimeout(context.WithoutCancel(ctx), 5*time.Second)

+ _ = cancel // released by the timeout; the store calls are short

+ return k

diff --git a/internal/calendar/mail_bounded.go b/internal/calendar/mail_bounded.go

index a59ac76..1f5f413 100644

--- a/internal/calendar/mail_bounded.go

+++ b/internal/calendar/mail_bounded.go

@@ -19,6 +19,13 @@ var ErrMailBusy = errors.New("calendar: mail sender busy")

// progress and may still arrive; at-least-once holds.

var ErrMailTimeout = errors.New("calendar: mail send timed out")

+// ErrMailRefused is EVERY other failure of the underlying sender, with

+// the sender's own text discarded at this boundary. rastrillo's SMTP

+// sender writes the recipient into its error and quotes a refused

+// subject; anything above this wrapper — rastrillo's sign-in handler

+// included, which logs what it is handed — must never see that.

+var ErrMailRefused = errors.New("calendar: mail refused")

// Sender limits, as constants because they are policy.

const (

maxSendsInFlight = 4

@@ -75,7 +82,10 @@ func (b *boundedSender) Send(ctx context.Context, to, subject, body string) erro

select {

case err := <-done:

- return err

+ if err != nil {

+ return ErrMailRefused

+ }

+ return nil

case <-time.After(wait):

return ErrMailTimeout

}

diff --git a/internal/calendar/store_booking_confirm.go b/internal/calendar/store_booking_confirm.go

index f3150af..264e99d 100644

--- a/internal/calendar/store_booking_confirm.go

+++ b/internal/calendar/store_booking_confirm.go

@@ -55,6 +55,11 @@ func (s *Store) ConfirmBooking(ctx context.Context, bookingID int64, now time.Ti

if err := s.setAttendeesOn(tx, e.ID, []string{addr}); err != nil {

return err

}

+ // An owed hold mail is now false: forgotten here, not merely

+ // skipped later.

+ if err := deleteUnsentMail(tx, bookingID, MailHold); err != nil {

+ return err

+ }

return queueBookingMail(tx, bookingID, MailConfirmed, []string{ToHost}, e.Sequence, now)

})

if err != nil || !changed {

@@ -118,11 +123,16 @@ func (s *Store) RescheduleBooking(ctx context.Context, bookingID, startUTC, endU

if err := tx.Exec(`UPDATE booking_pages SET updated_at = updated_at WHERE id = ?`, v.PageID).Error; err != nil {

return fmt.Errorf("calendar: take write lock: %w", err)

}

- // The host as they are NOW, behind the lock: tz below is the

- // zone the slot was computed in (B21), so compute it there.

- if err := tx.First(&v.Host, v.Host.ID).Error; err != nil {

- return fmt.Errorf("calendar: re-read host: %w", err)

+ // The page, its windows and the host as they are NOW, behind

+ // the lock: windows the host has since withdrawn cannot be

+ // moved into, and tz below is the zone the slot was computed

+ // in (B21), so compute it there. currentPageOn refuses an

+ // unpublished page, which is the host's way of closing it.

+ page, host, err := s.currentPageOn(tx, v.PageID)

+ if err != nil {

+ return err

}

+ v.Page, v.Host = page, host

if loc, err = Zone(v.Host.Zone); err != nil {

return err

}

diff --git a/internal/calendar/store_booking_flow.go b/internal/calendar/store_booking_flow.go

index f0fa932..64307a9 100644

--- a/internal/calendar/store_booking_flow.go

+++ b/internal/calendar/store_booking_flow.go

@@ -155,14 +155,10 @@ var errTokenCollision = errors.New("calendar: token collision")

func (s *Store) holdOnce(ctx context.Context, in HoldInput, addrFP string, day, dayEnd, now time.Time) (Booking, error) {

token := newToken()

- sealed, err := s.sealBooker(in, token)

- if err != nil {

- return Booking{}, err

- }

loc, _ := Zone(in.Host.Zone)

var b Booking

- err = s.g.WithContext(ctx).Transaction(func(tx *gorm.DB) error {

+ err := s.g.WithContext(ctx).Transaction(func(tx *gorm.DB) error {

// The write lock, before any read this decision depends on.

if err := tx.Exec(`UPDATE booking_pages SET updated_at = updated_at WHERE id = ?`, in.Page.ID).Error; err != nil {

return fmt.Errorf("calendar: take write lock: %w", err)

@@ -179,6 +175,13 @@ func (s *Store) holdOnce(ctx context.Context, in HoldInput, addrFP string, day,

if err != nil {

return err

}

+ // The snapshot is sealed from the page AS RE-READ, so the

+ // booking's title and the event's title come from one version

+ // of it.

+ sealed, err := s.sealBooker(in, token)

+ if err != nil {

+ return err

+ }

again, err := s.freeSlotsOn(tx, page, host, now, day, dayEnd, 0, 0)

if err != nil {

return err

diff --git a/internal/calendar/store_drain.go b/internal/calendar/store_drain.go

index c48aa57..4e09d47 100644

--- a/internal/calendar/store_drain.go

+++ b/internal/calendar/store_drain.go

@@ -213,14 +213,14 @@ func (s *Store) CompleteReminder(ctx context.Context, due DueReminder, now int64

// DeferReminder puts a row back without counting an attempt: the sender

// was busy and nothing was tried. retry_at, never due_at — due_at IS

// the meeting time minus the offset.

-func (s *Store) DeferReminder(ctx context.Context, queueID int64, claimID string, now int64) error {

- err := s.g.WithContext(ctx).Model(&ReminderQueue{}).

+func (s *Store) DeferReminder(ctx context.Context, queueID int64, claimID string, now int64) (ok bool, err error) {

+ res := s.g.WithContext(ctx).Model(&ReminderQueue{}).

Where("id = ? AND claim_id = ?", queueID, claimID).

- Updates(map[string]any{"retry_at": now + int64(deferFor.Seconds()), "claimed_at": nil, "claim_id": nil}).Error

- if err != nil {

- return fmt.Errorf("calendar: defer reminder: %w", err)

+ Updates(map[string]any{"retry_at": now + int64(deferFor.Seconds()), "claimed_at": nil, "claim_id": nil})

+ if res.Error != nil {

+ return false, fmt.Errorf("calendar: defer reminder: %w", res.Error)

}

- return nil

+ return res.RowsAffected == 1, nil

}

// MaxReminderAttempts is when a reminder stops being retried.

diff --git a/internal/calendar/templates/event_form.html b/internal/calendar/templates/event_form.html

index 3844def..6996775 100644

--- a/internal/calendar/templates/event_form.html

+++ b/internal/calendar/templates/event_form.html

@@ -53,7 +53,13 @@

- <p cal-booked-when><small>{{.Data.TZ}} · {{.Data.Kind}}</small></p>

+ <label rst-field-label for="tz-locked">Zone</label>

+ <input rst-input="short" id="tz-locked" type="text" value="{{.Data.TZ}}" disabled>

+ <label rst-field-label for="kind-locked">Kind</label>

+ <input rst-input="short" id="kind-locked" type="text" value="{{.Data.Kind}}" disabled>

+ <label rst-switch><input type="checkbox" disabled{{if .Data.AllDay}} checked{{end}}><span rst-switch-track aria-hidden="true"></span> All day</label>

+ <label rst-field-label for="repeat-locked">Repeats</label>

+ <input rst-input="short" id="repeat-locked" type="text" value="Never" disabled>

<small rst-field-hint>This is a booked meeting{{with .Data.BookedBy}} with {{.}}{{end}}. Its time is changed by the person who booked it, or by cancelling and letting them rebook.</small>

{{with (if_eq .Data.ErrorField "start" .Data.Error)}}<small rst-field-error>{{.}}</small>{{end}}

</div>

diff --git a/internal/calendartest/booking_flow_test.go b/internal/calendartest/booking_flow_test.go

index 3f1ed0e..de75fbf 100644

--- a/internal/calendartest/booking_flow_test.go

+++ b/internal/calendartest/booking_flow_test.go

@@ -191,8 +191,10 @@ func TestConfirmIsAConditionalUpdateWithGuards(t *testing.T) {

if att, _ := st.AttendeesFor(ctx, b.EventID); len(att) != 1 || att[0] != "doc@tito.io" {

t.Errorf("attendees = %v", att)

}

+ // The hold mail still owed is FORGOTTEN by the confirmation, in its

+ // own transaction: what remains is the one true row.

owed := st.UnsentBookingMailForTest(b.ID)

- if len(owed) != 2 || owed[1].Kind != calendar.MailConfirmed || owed[1].Recipient != calendar.ToHost || owed[1].Revision != 1 {

+ if len(owed) != 1 || owed[0].Kind != calendar.MailConfirmed || owed[0].Recipient != calendar.ToHost || owed[0].Revision != 1 {

t.Errorf("owed = %+v", owed)

}

if rems, _ := st.RemindersFor(ctx, host.ID, b.EventID); len(rems) != 1 {

diff --git a/internal/calendartest/booking_review2_test.go b/internal/calendartest/booking_review2_test.go

new file mode 100644

index 0000000..567041b

--- /dev/null

+++ b/internal/calendartest/booking_review2_test.go

@@ -0,0 +1,202 @@

+package calendartest

+import (

+ "context"

+ "errors"

+ "log/slog"

+ "net/http"

+ "net/url"

+ "path/filepath"

+ "strings"

+ "sync"

+ "sync/atomic"

+ "testing"

+ "time"

+ calendar "amadan.net/oficina/calendar/internal/calendar"

+// Regressions from the SECOND code review of the booking branch.

+// A browser submits a textarea's line breaks as CRLF. Two paragraphs

+// are two paragraphs, not a control character (review 2, B3).

+func TestAMultilineNoteFromABrowserIsAccepted(t *testing.T) {

+ app, _, path, _ := publishedPage(t)

+ h := app.Handler()

+ _, starts := firstDaySlots(t, h, path)

+ form := holdForm(starts[0], "Doc", "doc@tito.io")

+ form.Set("note", "First paragraph.\r\n\r\nSecond paragraph.")

+ if rec := strangerPost(t, h, path, form); rec.Code != http.StatusOK {

+ t.Fatalf("a two-paragraph note: %d\n%s", rec.Code, rec.Body.String())

+ }

+ var id int64

+ app.DBForTest().QueryRow(`SELECT id FROM bookings`).Scan(&id)

+ v, err := app.StoreForTest().BookingByID(context.Background(), id, false)

+ if err != nil || v.Note != "First paragraph.\n\nSecond paragraph." {

+ t.Errorf("stored note = %q (err %v), want LF-normalised", v.Note, err)

+ }

+// The snapshot is sealed from the page AS RE-READ behind the write

+// lock, so the booking and its event describe one version of the page

+// (review 2, B4).

+func TestTheSnapshotIsSealedBehindTheLock(t *testing.T) {

+ app, host, page := hostWithPage(t)

+ st := app.StoreForTest()

+ ctx := context.Background()

+ now := time.Now()

+ slot := slotsFor(t, app, page, host, now, 3)[0]

+ ws, _ := calendar.ParseAvailability("every day 9-5")

+ app.SetAfterSlotCheckForTest(func() {

+ app.SetAfterSlotCheckForTest(nil)

+ if err := st.UpdateBookingPage(ctx, host.ID, page.ID, calendar.BookingPageInput{

+ Title: "Renamed", Description: "Half an hour.", Availability: "every day 9-5",

+ DurationMin: 30, HorizonDays: 30}, ws); err != nil {

+ t.Error(err)

+ }

+ })

+ b, err := st.HoldBooking(ctx, holdInput(page, host, slot, "doc@tito.io", "ip"), now)

+ if err != nil {

+ t.Fatal(err)

+ }

+ v, _ := st.BookingByID(ctx, b.ID, false)

+ ev, _ := st.Event(ctx, host.ID, b.EventID)

+ if v.Title != "Renamed" || ev.Title != "Renamed — Doc" {

+ t.Errorf("booking says %q, event says %q: two versions of one page", v.Title, ev.Title)

+ }

+// A reschedule re-reads the page and its windows behind the lock: hours

+// the host withdrew, or a page they closed, cannot be moved into

+// (review 2, A6).

+func TestARescheduleIntoWithdrawnHoursIsRefused(t *testing.T) {

+ app, host, page := hostWithPage(t)

+ st := app.StoreForTest()

+ ctx := context.Background()

+ now := time.Now()

+ slots := slotsFor(t, app, page, host, now, 3)

+ b, _ := st.HoldBooking(ctx, holdInput(page, host, slots[0], "doc@tito.io", "ip"), now)

+ st.ConfirmBooking(ctx, b.ID, now)

+ late := slots[len(slots)-1] // 16:30 on the third day: outside 9-10

+ ws, _ := calendar.ParseAvailability("every day 9-10")

+ app.SetAfterSlotCheckForTest(func() {

+ app.SetAfterSlotCheckForTest(nil)

+ if err := st.UpdateBookingPage(ctx, host.ID, page.ID, calendar.BookingPageInput{

+ Title: "Chat", Description: "Half an hour.", Availability: "every day 9-10",

+ DurationMin: 30, HorizonDays: 30}, ws); err != nil {

+ t.Error(err)

+ }

+ })

+ if _, err := st.RescheduleBooking(ctx, b.ID, late.StartUTC, late.EndUTC, 1, now); !errors.Is(err, calendar.ErrSlotTaken) {

+ t.Errorf("moving into withdrawn hours: %v, want ErrSlotTaken", err)

+ }

+ if err := st.PublishBookingPage(ctx, host.ID, page.ID, false); err != nil {

+ t.Fatal(err)

+ }

+ if _, err := st.RescheduleBooking(ctx, b.ID, slots[1].StartUTC, slots[1].EndUTC, 1, now); !errors.Is(err, calendar.ErrSlotTaken) {

+ t.Errorf("moving on a closed page: %v, want ErrSlotTaken", err)

+ }

+ v, _ := st.BookingByID(ctx, b.ID, false)

+ if v.Event.StartUTC != slots[0].StartUTC || v.Reschedules != 0 {

+ t.Error("the meeting moved anyway")

+ }

+// A sender's error text stops at the bounded sender. rastrillo's SMTP

+// sender writes the recipient into its error and its sign-in handler

+// logs what it is handed (review 2, B1).

+func TestASenderErrorReachesNoLogWithItsText(t *testing.T) {

+ var log strings.Builder

+ mailer := &pickyMailer{refuse: "allowed@example.com"}

+ app, err := calendar.New(calendar.Config{

+ DBPath: filepath.Join(t.TempDir(), "calendar.db"),

+ Env: "development",

+ Origin: testOrigin,

+ AllowedAddresses: "allowed@example.com",

+ Mailer: mailer,

+ Logger: slog.New(slog.NewTextHandler(&log, nil)),

+ })

+ if err != nil {

+ t.Fatalf("calendar.New: %v", err)

+ }

+ t.Cleanup(func() { app.Close() })

+ post(t, app.Handler(), "/signin", url.Values{"address": {"allowed@example.com"}})

+ if out := log.String(); strings.Contains(out, "nobody@example.com") || strings.Contains(out, "connection refused") {

+ t.Errorf("the sender's own text reached the log:\n%s", out)

+ }

+// stuckMailer, once armed, holds every send until released, so the

+// bounded sender's slots can be filled on purpose. Before that (the

+// host's sign-in) it delivers at once.

+type stuckMailer struct {

+ armed atomic.Bool

+ entered chan struct{}

+ release chan struct{}

+func (m *stuckMailer) Send(_ context.Context, _, _, _ string) error {

+ if !m.armed.Load() {

+ return nil

+ }

+ m.entered <- struct{}{}

+ <-m.release

+ return nil

+// With every sender slot taken, a hold's immediate attempt is DEFERRED

+// to the drain: the hold stands, and the page says so rather than

+// "sent" (review 2, B5).

+func TestADeferredHoldMailIsNotReportedSent(t *testing.T) {

+ mailer := &stuckMailer{entered: make(chan struct{}, 8), release: make(chan struct{})}

+ app, err := calendar.New(calendar.Config{

+ DBPath: filepath.Join(t.TempDir(), "calendar.db"), Env: "development", Origin: testOrigin,

+ Mailer: mailer, Logger: slog.New(slog.NewTextHandler(&strings.Builder{}, nil)),

+ })

+ if err != nil {

+ t.Fatal(err)

+ }

+ t.Cleanup(func() { app.Close() })

+ session := app.SignInForTest(t, "host@example.com")

+ h := app.Handler()

+ postAs(t, h, session, "/settings", url.Values{"zone": {"UTC"}, "week_start": {"1"},

+ "default_reminder_min": {"10"}, "default_all_day_min": {"0"}, "handle": {"paul"}})

+ postAs(t, h, session, "/settings/booking", url.Values{"title": {"Chat"}, "description": {"Half an hour."},

+ "availability": {"every day 0-24"}, "duration_min": {"30"}, "buffer_min": {"0"}, "notice_min": {"0"}, "horizon_days": {"30"}})

+ postAs(t, h, session, "/settings/booking/1/publish", url.Values{"publish": {"1"}})

+ path := strings.TrimSpace(rePublicURL.FindString(getAs(t, h, session, "/settings/booking").Body.String()))

+ _, starts := firstDaySlots(t, h, path)

+ if len(starts) < 5 {

+ t.Fatalf("only %d slots on the first day", len(starts))

+ }

+ // Four holds whose mail is stuck in the sender: every slot taken.

+ mailer.armed.Store(true)

+ var wg sync.WaitGroup

+ for i := 0; i < 4; i++ {

+ wg.Add(1)

+ go func(i int) {

+ defer wg.Done()

+ strangerPost(t, h, path, holdForm(starts[i], "Doc", "doc"+string(rune('a'+i))+"@tito.io"))

+ }(i)

+ }

+ for i := 0; i < 4; i++ {

+ select {

+ case <-mailer.entered:

+ case <-time.After(10 * time.Second):

+ t.Fatal("the sender slots never filled")

+ }

+ rec := strangerPost(t, h, path, holdForm(starts[4], "Doc", "doce@tito.io"))

+ close(mailer.release)

+ wg.Wait()

+ if rec.Code != http.StatusOK || !strings.Contains(rec.Body.String(), "If nothing arrives") {

+ t.Errorf("the fifth hold, deferred, was reported sent: %d\n%s", rec.Code, rec.Body.String())

+ }

+ var n int64

+ app.DBForTest().QueryRow(`SELECT COUNT(*) FROM booking_mails WHERE sent_at IS NULL AND attempts = 0`).Scan(&n)

+ if n != 1 {

+ t.Errorf("owed rows with no attempt = %d, want the deferred one", n)

+ }

diff --git a/internal/calendartest/occurrence_strict_test.go b/internal/calendartest/occurrence_strict_test.go

index ada0eb2..d125077 100644

--- a/internal/calendartest/occurrence_strict_test.go

+++ b/internal/calendartest/occurrence_strict_test.go

@@ -92,10 +92,18 @@ func TestTheUntilBoundHasADayOfSlack(t *testing.T) {

"title": {"Night shift"}, "start": {"2026-10-23T00:00"}, "end": {"2026-10-23T04:00"},

"kind": {"commitment"}, "status": {"confirmed"}, "repeat": {"daily"}, "repeat_end": {"2026-10-25"},

})

- // The final occurrence starts 00:00 IST on the 25th and, across the

- // fold, runs FIVE elapsed hours to 04:00 GMT. Query only its fifth

- // hour: a master-duration bound (four hours) would have excluded the

- // master here; the day of slack keeps it.

+ // The final occurrence starts 00:00 IST on the 25th — 23:00Z on the

+ // 24th — and, across the fold, runs FIVE elapsed hours to 04:00 GMT.

+ // A bare-date repeat_end indexes UNTIL at the END of that day, which

+ // would pass without any slack, so pin UNTIL to the final START as

+ // an importer would write it, then query only the fifth hour: a

+ // master-duration bound (four hours from 23:00Z is 03:00Z) excludes

+ // the master; the day of slack keeps it.

+ until := time.Date(2026, 10, 24, 23, 0, 0, 0, time.UTC)

+ if _, err := app.DBForTest().Exec(`UPDATE events SET rrule = ?, until_utc = ? WHERE rrule <> ''`,

+ "FREQ=DAILY;UNTIL=20261024T230000Z", until.Unix()); err != nil {

+ t.Fatal(err)

+ }

host := app.PersonByAddressForTest(t, "someone@example.com")

dublin, _ := time.LoadLocation("Europe/Dublin")

from := time.Date(2026, 10, 25, 3, 30, 0, 0, dublin).Unix()