Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,8 @@ committed booking) — which is why the reconciler (§11) exists.

## 10. Calendar integration (provider abstraction)

Availability checks return an error if any selected conflict calendar cannot be checked. Partial provider responses and unreadable busy periods are not treated as free time. An unavailable calendar can therefore temporarily prevent booking; reconnect it or deselect it from conflict checks to restore availability.

Calnode talks to calendars through a **provider abstraction**, not a single vendor:

- **`internal/calendar`** defines the `Provider` interface (Name, InvitesGuests,
Expand Down
6 changes: 1 addition & 5 deletions internal/caldav/caldav.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,6 @@ func (c *Client) loadConn(ctx context.Context, userID string, checkConflicts, is
return cn, true, nil
}

// conflictConns returns every CalDAV connection for the user with check_conflicts = 1, so a
// user can connect several CalDAV accounts and have them all checked. Decrypt failures on one
// row are logged and skipped (fail-open).
func (c *Client) conflictConns(ctx context.Context, userID string) ([]conn, error) {
rows, err := c.db.QueryContext(ctx, `
SELECT id, COALESCE(account_email,''), access_token_enc, calendar_id
Expand Down Expand Up @@ -222,8 +219,7 @@ func (c *Client) conflictConns(ctx context.Context, userID string) ([]conn, erro
}
pw, err := c.decrypt(d.pwEnc)
if err != nil {
c.logger.Warn("caldav: skipping connection with bad credentials", "user_id", userID, "error", err)
continue
return nil, err
}
for _, calURL := range calIDs {
conns = append(conns, conn{id: d.id, username: d.username, password: string(pw), calURL: calURL})
Expand Down
6 changes: 2 additions & 4 deletions internal/caldav/freebusy.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ const calendarQueryTmpl = `<?xml version="1.0" encoding="utf-8"?>

// FreeBusy returns the union of busy intervals across every CalDAV account the user has
// connected with check_conflicts = 1. Each account is queried with a CalDAV calendar-query
// REPORT; transparent and cancelled events don't block. Fail-open per account: a connection
// that errors is logged and skipped (a flaky calendar never blocks availability).
// REPORT; transparent and cancelled events don't block.
func (c *Client) FreeBusy(ctx context.Context, userID string, from, to time.Time) ([]slots.Interval, error) {
conns, err := c.conflictConns(ctx, userID)
if err != nil {
Expand All @@ -47,8 +46,7 @@ func (c *Client) FreeBusy(ctx context.Context, userID string, from, to time.Time
for _, cn := range conns {
iv, err := c.freeBusyForConn(ctx, cn, body)
if err != nil {
c.logger.Warn("caldav: freebusy connection failed", "user_id", userID, "calendar", cn.calURL, "error", err)
continue
return nil, err
}
out = append(out, iv...)
}
Expand Down
16 changes: 1 addition & 15 deletions internal/calendar/calendar.go
Original file line number Diff line number Diff line change
Expand Up @@ -348,29 +348,15 @@ func (s *Service) InvitesGuests(ctx context.Context, userID string) bool {
return false
}

// FreeBusy returns the UNION of busy intervals for the user across EVERY connected provider
// (each provider internally unions its own connected accounts with check_conflicts = 1). This
// is what lets a user connect multiple calendars and have them all checked. Fail-open: a
// provider that errors is skipped; an error is returned only if every provider failed (so a
// flaky calendar never blocks availability or a booking).
func (s *Service) FreeBusy(ctx context.Context, userID string, from, to time.Time) ([]slots.Interval, error) {
var out []slots.Interval
var firstErr error
anyOK := false
for _, p := range s.providers {
iv, err := p.FreeBusy(ctx, userID, from, to)
if err != nil {
if firstErr == nil {
firstErr = err
}
continue
return nil, err

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only production caller of Service.FreeBusy — hostAvailability at internal/handler/slots_handler.go:497 — still treats the result as non-fatal: it logs the error and returns the host with an empty Busy slice, so an unreadable calendar leaves every slot bookable. Because this method now returns on the first error instead of unioning the providers that succeeded, it also drops the busy intervals of accounts/providers that did respond, which makes multi-account and multi-provider users more available than under the old fail-open code.

Technical details
# Fail-closed policy is not wired into the availability path

## Affected sites
- `internal/handler/slots_handler.go:497-503` — `hostAvailability` logs `gc.FreeBusy` errors and returns `slots.HostAvailability` with no calendar busy intervals (this file is not part of the PR).
- `internal/calendar/calendar.go:356` — new `return nil, err` aborts the provider union on the first error, discarding intervals from providers/accounts that succeeded.
- `docs/ARCHITECTURE.md:452` — documents the fail-closed policy this path does not yet implement.

## Required outcome
- A free/busy failure for a host must not silently yield an empty conflict set; it must either prevent that host's slots from being offered or fail the slot computation.
- Keep the union semantics consistent with the chosen policy: if a host has several accounts/providers, either fail closed on any error, or keep the successful accounts' intervals and surface the failure separately — do not return `nil` intervals together with an error to a caller that ignores it.
- Add caller-level coverage. The new gcal tests stop at the provider boundary and pass while availability remains fail-open end to end.

## Suggested approach
- Return the error from `hostAvailability` so `computeSlots` (`slots_handler.go:223-227`) fails the request; that produces the fail-closed semantics the PR describes. Consider the booker-facing surface before making the whole event type error out (see the informational note in the review body).

}
anyOK = true
out = append(out, iv...)
}
if !anyOK && firstErr != nil {
return nil, firstErr
}
return out, nil
}

Expand Down
7 changes: 1 addition & 6 deletions internal/calendar/microsoft/freebusy.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,6 @@ type calendarViewResp struct {
NextLink string `json:"@odata.nextLink"`
}

// FreeBusy returns the UNION of busy intervals for userID in [from, to) across every
// connected Microsoft account with check_conflicts = 1 (every event in the window counts as
// busy). Returns (nil, nil) if the user has no such connection. Fail-open: an account that
// errors is logged and skipped.
func (c *Client) FreeBusy(ctx context.Context, userID string, from, to time.Time) ([]slots.Interval, error) {
conns, err := c.freeBusyConnections(ctx, userID)
if err != nil {
Expand All @@ -45,8 +41,7 @@ func (c *Client) FreeBusy(ctx context.Context, userID string, from, to time.Time
for _, fc := range conns {
intervals, err := c.freeBusyForConn(ctx, fc, from, to)
if err != nil {
c.logger.Warn("microsoft: freebusy for connection failed, skipping", "user_id", userID, "error", err)
continue
return nil, err
}
out = append(out, intervals...)
}
Expand Down
4 changes: 1 addition & 3 deletions internal/calendar/microsoft/microsoft.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,6 @@ func (c *Client) httpClient(ctx context.Context, userID string, checkConflicts,

// freeBusyConnections returns an authorized client for EVERY Microsoft connection the user
// has with check_conflicts = 1 (so several connected Microsoft accounts are all checked).
// Bad-credential rows are logged and skipped (fail-open).
// msFBConn is one conflict-check account's authorized client plus which calendars to read.
// useDefault means query /me/calendarView (the account's default calendar) — the pre-picker
// behaviour, used when the user hasn't selected sub-calendars; otherwise calIDs holds the
Expand Down Expand Up @@ -336,8 +335,7 @@ func (c *Client) freeBusyConnections(ctx context.Context, userID string) ([]msFB
}
hc, err := c.buildClient(ctx, userID, d.accessEnc, d.refreshEnc, d.calID, d.expiryStr, d.accountEmail)
if err != nil {
c.logger.Warn("microsoft: skipping connection with bad credentials", "user_id", userID, "error", err)
continue
return nil, err
}
// Unconfigured accounts fall back to the stored "primary" placeholder id, which is not a
// real Graph calendar id — read the default calendar view for those.
Expand Down
20 changes: 9 additions & 11 deletions internal/gcal/freebusy.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,14 @@ type freeBusyReq struct {

type freeBusyResp struct {
Calendars map[string]struct {
Busy []struct {
Errors []json.RawMessage `json:"errors"`
Busy []struct {
Start string `json:"start"`
End string `json:"end"`
} `json:"busy"`
} `json:"calendars"`
}

// FreeBusy returns the UNION of Google Calendar busy intervals for userID in [from, to)
// across every connected Google account with check_conflicts = 1 (so all of a user's
// connected calendars are honoured, not just one). Returns (nil, nil) if the user has no such
// connection. Fail-open: a single account that errors is logged and skipped, so a flaky
// connection never blocks availability or a booking.
func (c *Client) FreeBusy(ctx context.Context, userID string, from, to time.Time) ([]slots.Interval, error) {
conns, err := c.freeBusyConnections(ctx, userID)
if err != nil {
Expand All @@ -40,8 +36,7 @@ func (c *Client) FreeBusy(ctx context.Context, userID string, from, to time.Time
for _, conn := range conns {
intervals, err := c.freeBusyForConn(ctx, conn, from, to)
if err != nil {
c.logger.Warn("gcal: freebusy for connection failed, skipping", "user_id", userID, "error", err)
continue
return nil, err
}
out = append(out, intervals...)
}
Expand Down Expand Up @@ -84,13 +79,16 @@ func (c *Client) freeBusyForConn(ctx context.Context, conn fbConn, from, to time
return nil, fmt.Errorf("gcal: freeBusy decode: %w", err)
}
var out []slots.Interval
for _, cal := range fbr.Calendars {
for _, id := range conn.calIDs {
cal, ok := fbr.Calendars[id]
if !ok || len(cal.Errors) > 0 {
return nil, fmt.Errorf("gcal: calendar busy status unavailable")
}
for _, b := range cal.Busy {
s, err1 := time.Parse(time.RFC3339, b.Start)
e, err2 := time.Parse(time.RFC3339, b.End)
if err1 != nil || err2 != nil {
c.logger.Warn("gcal: skipping unparseable busy interval", "start", b.Start, "end", b.End)
continue
return nil, fmt.Errorf("gcal: invalid busy interval")
}
out = append(out, slots.Interval{Start: s, End: e})
}
Expand Down
27 changes: 22 additions & 5 deletions internal/gcal/freebusy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,7 @@ func TestFreeBusy_notConnected_returnsNil(t *testing.T) {
}
}

func TestFreeBusy_nonOK_failsOpen(t *testing.T) {
// A connection returning non-200 is skipped (fail-open), not surfaced as an error —
// a flaky calendar must never block availability or a booking.
func TestFreeBusy_nonOK_rejectsBookingCheck(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.Error(w, "unauthorized", http.StatusUnauthorized)
}))
Expand All @@ -151,8 +149,8 @@ func TestFreeBusy_nonOK_failsOpen(t *testing.T) {
saveAndConnectClient(t, c, "user-1", "primary", "bad-token")

got, err := c.FreeBusy(context.Background(), "user-1", time.Now(), time.Now().Add(time.Hour))
if err != nil {
t.Errorf("fail-open: expected nil error, got %v", err)
if err == nil {
t.Error("expected an error from failed calendar check")
}
if len(got) != 0 {
t.Errorf("expected no busy intervals from a failed connection, got %d", len(got))
Expand All @@ -179,3 +177,22 @@ func TestFreeBusy_onlyCheckConflictsConnections(t *testing.T) {
t.Errorf("got intervals from check_conflicts=0 connection; want nil")
}
}

func TestFreeBusy_rejectsIncompleteCalendarResponse(t *testing.T) {
for _, body := range []string{
`{"calendars":{}}`,
`{"calendars":{"primary":{"errors":[{"reason":"notFound"}],"busy":[]}}}`,
`{"calendars":{"primary":{"busy":[{"start":"invalid","end":"2026-06-15T10:00:00Z"}]}}}`,
} {
t.Run(body, func(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Write([]byte(body)) }))
defer srv.Close()
c := newTestClient(t)
c.apiBase = srv.URL
saveAndConnectClient(t, c, "user-1", "primary", "token")
if _, err := c.FreeBusy(context.Background(), "user-1", time.Now(), time.Now().Add(time.Hour)); err == nil {
t.Fatal("expected calendar check failure")
}
})
}
}
8 changes: 1 addition & 7 deletions internal/gcal/gcal.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,11 +225,6 @@ type fbConn struct {
calIDs []string
}

// freeBusyConnections returns an authorized client for EVERY Google connection the user has
// with check_conflicts = 1 (so a user can connect several Google accounts and have them all
// checked for conflicts), each paired with the set of that account's calendars to check for
// conflicts (per-account sub-calendar selection). Decrypt failures on one row are logged and
// skipped (fail-open); an account whose calendars are all deselected is dropped.
func (c *Client) freeBusyConnections(ctx context.Context, userID string) ([]fbConn, error) {
rows, err := c.db.QueryContext(ctx, `
SELECT access_token_enc, COALESCE(refresh_token_enc,''), calendar_id, COALESCE(expiry_at,''), COALESCE(account_email,'')
Expand Down Expand Up @@ -265,8 +260,7 @@ func (c *Client) freeBusyConnections(ctx context.Context, userID string) ([]fbCo
}
hc, err := c.buildClient(ctx, userID, d.accessEnc, d.refreshEnc, d.calID, d.expiryStr, d.accountEmail)
if err != nil {
c.logger.Warn("gcal: skipping connection with bad credentials", "user_id", userID, "error", err)
continue
return nil, fmt.Errorf("gcal: load conflict calendar credentials: %w", err)
}
conns = append(conns, fbConn{hc: hc, calIDs: calIDs})
}
Expand Down
Loading