Repository navigation
feat(invites): send calendar invites from your workspace instead of the host's calendar - #110
Conversation
A per-event-type setting, invite_delivery. 'calendar' (default) keeps today's behaviour: the host's connected calendar invites the booker, from the host's own address. 'calnode' writes the hosts' events without guests (so Google and Microsoft email nobody) and attaches Calnode's .ics to the booker's emails, organized by the instance sender from Settings -> Email. For hosts who don't want their personal address on every invite, and for teams booking under one name. The mode is stored on each booking so its reschedule, cancel, reassign and reconcile follow the channel the invite actually went out on.
|
All contributors have signed the CLA. ✅ |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — minor suggestions inline.
Reviewed changes
- New per-event-type
invite_delivery(migration 00068).calendar(default) orcalnode, stored on bothevent_typesand eachbookingsrow so reschedule/cancel/reassign/reconciler follow the channel the invite actually went out on. - Calnode-sent invites. Host calendar events are created guestless via
calendarInvitee, hosts still get the meeting on their own calendar, and Calnode attaches its own.icsto the booker's emails withORGANIZER= the instance sender (HideHostInInvite), omitted rather than falling back to a host when no sender is configured. - Validation on change. Switching an event type to
calnoderequires a configured sender (inviteSenderReady); an event type already in that mode stays saveable if email is later removed, matching the CLAUDE.md on-change rule. - Surface plumbing. Create/patch/duplicate,
etColumns/scan,loadBookableEventType, the admin editor, and the mailer +BuildICSare extended consistently; the duplicate drift test, ARCHITECTURE, and CHANGELOG are updated.
ℹ️ Host-side copy still attaches an .ics naming the booker as ATTENDEE in Calnode mode
hostBookingData resets HideHostInInvite but leaves OrganizerEmail (the booker) in place, so when a host has no auto-inviting destination the host's own confirmation carries a REQUEST .ics with the booker as attendee. Whether anyone acts on it depends on the host's mail client, but it is the one remaining host-side path that can name the booker as a guest — worth a decision rather than leaving implicit.
Technical details
# Host copy can still carry the booker as ATTENDEE in Calnode mode
## Affected sites
- `internal/handler/booking_handler.go:1048` — `hd.AttachICS = h.noConnectedDestination(ctx, host.UserID)`.
- `internal/handler/booking_handler.go:1051` — `hd.HideHostInInvite = false` (added here); `OrganizerEmail` remains the booker.
- Send sites: `createHostEventsAndNotify` (`:1249`), `cancelSideEffects` (`:1780`).
## Required outcome
- Decide whether `calnode` mode should drop the booker as attendee on host copies, so no host-side calendar client can re-invite the booker from the host's account.
## Open questions for the human
- Is the host's copy meant to name the attendee at all, or only the meeting details/link?ℹ️ Calnode reschedule / reassign / reconciler paths are mode-aware but untested
The PR description says reschedule, cancel, reassign and the reconciler all read the mode from the booking, but the new tests only cover creation, cancellation, validation, and BuildICS. Those three extra sites are exactly the kind of helper that a later edit can quietly drop.
Technical details
# Missing regression coverage for the mode-aware lifecycle paths
## Affected sites
- `internal/handler/manage_handler.go:317` — `rescheduleSideEffects` → `applyInviteDelivery`.
- `internal/handler/reassign.go:212` — Calnode re-issue branch.
- `internal/handler/calendar_reconcile.go:251` — `calendarInvitee(m.inviteMode, m.orgEmail)`.
## Required outcome
- A test per path (at minimum reschedule + reassign) asserting the booking's stored mode drives the `.ics` organizer, so a future refactor cannot silently regress the promise "no host address reaches the booker".
## Open questions for the human
- Are these paths covered by an existing test I missed? `invite_delivery_internal_test.go` exercises create/cancel/validation only.ℹ️ Nitpicks
calendarInvitee's doc comment (internal/handler/invite_delivery.go:44) says an empty address is what "every provider (Google, Microsoft, CalDAV) reads as 'no attendee'". CalDAV'sbuildICSmapsOrganizerEmailtoORGANIZER, notATTENDEE(it never sets an attendee), so for CalDAV""means no organizer. Behaviour is fine — CalDAV never emails invites — but the comment overstates the shared semantics.validInviteDelivery("")isfalse, so a client sending"invite_delivery": ""gets a 400 while the booking service treats empty ascalendar(service.go:155). Only the admin editor is affected today, so this is a consistency nit.
opencode-go/deepseek-v4.1-flash ｜ 𝕏
| func (h *Handler) inviteSenderReady(ctx context.Context) bool { | ||
| var from, host, resendKey string | ||
| if err := h.db.QueryRowContext(ctx, | ||
| `SELECT email_from, smtp_host, resend_api_key_enc FROM server_settings WHERE id = 1`). | ||
| Scan(&from, &host, &resendKey); err != nil { | ||
| h.logger.ErrorContext(ctx, "check invite sender", "error", err) | ||
| return false | ||
| } | ||
| return from != "" && (host != "" || resendKey != "") | ||
| } |
There was a problem hiding this comment.
This re-derives the transport choice from raw settings (host != "" || resend_api_key_enc != ""), duplicating the decision BuildMailer owns and CLAUDE.md calls out as the single place it is made. If a third transport or a selection change lands, this gate can drift from what the mailer actually does. h.isEmailEnabled() already reflects the live mailer built by BuildMailer; combine it with the sender-address check.
| func (h *Handler) inviteSenderReady(ctx context.Context) bool { | |
| var from, host, resendKey string | |
| if err := h.db.QueryRowContext(ctx, | |
| `SELECT email_from, smtp_host, resend_api_key_enc FROM server_settings WHERE id = 1`). | |
| Scan(&from, &host, &resendKey); err != nil { | |
| h.logger.ErrorContext(ctx, "check invite sender", "error", err) | |
| return false | |
| } | |
| return from != "" && (host != "" || resendKey != "") | |
| } | |
| func (h *Handler) inviteSenderReady(ctx context.Context) bool { | |
| var from string | |
| if err := h.db.QueryRowContext(ctx, | |
| `SELECT email_from FROM server_settings WHERE id = 1`).Scan(&from); err != nil { | |
| h.logger.ErrorContext(ctx, "check invite sender", "error", err) | |
| return false | |
| } | |
| return from != "" && h.isEmailEnabled() | |
| } |
|
I have read the CLA Document and I hereby sign the CLA |
shockalotti
left a comment
shockalotti
left a comment
There was a problem hiding this comment.
Reviewed the full diff — this is in good shape. Verified every invite-touching path (create via both REST/MCP/assistant funnels, reschedule, cancel incl. manage-token, reassign, reconciler heal) honors the stored per-booking mode, validation is change-only per the repo rule, BuildMailer untouched, no-sender degrade is deliberate and tested, migration 00068 sequences correctly.
No blockers. Nits for your consideration:
- Host copy can still name the booker as ATTENDEE in Calnode mode (hostBookingData keeps OrganizerEmail as booker) — worth an explicit decision on dropping ATTENDEE there.
- The mode-aware reschedule/reassign/reconciler paths have no regression tests (create/cancel/validation do) — one test per path would pin the routing.
- bookingInviteDelivery fails open to calendar on DB error; threading the mode from the loaded booking would avoid re-querying.
- calendarInvitee comment overstates the CalDAV case (no ATTENDEE there, only no ORGANIZER).
- validInviteDelivery("") is false while Service.Create treats "" as calendar — pick one meaning.
- No operator signal when email is removed after enabling Calnode mode (silent organizer-less ICS) — possible Settings hint follow-up.
Since this is still a draft, holding merge until you mark it ready. The stacked #111 depends on this landing first.
- Host copies of a Calnode-invited booking no longer list the booker and go out as METHOD:PUBLISH, so no host calendar holds them as a re-invitable guest. - The invite mode travels on booking.Booking instead of a per-call lookup that failed open to 'calendar'. - inviteSenderReady defers the transport decision to the live mailer. - The API treats an empty invite_delivery as 'calendar', like the service. - The event type editor warns when Calnode mode is on but email is not. - Regression tests for reschedule, reassign and the reconciler.
|
Thanks both. All addressed in 4cc5c06:
Also took the bot's inline suggestion: |
There was a problem hiding this comment.
ℹ️ No critical issues. All six points from the prior review are addressed; two questions remain —
calnodestill shows the host's name, and the provider contract it relies on is untested.
Reviewed changes (delta since prior pullfrog review 4256586)
- Host copies no longer name the booker.
BookingData.ICSWithoutAttendee+icsMethoddrop theATTENDEEline and sendMETHOD:PUBLISH(mailer/ics.go);applyHostInviteapplies it to every host send. - Mode travels on the booking. The per-call
bookingInviteDeliveryDB lookup is gone;dispatchBookingConfirmation, cancel, reschedule and reassign readbooking.Booking.InviteDelivery, so it can no longer fail open mid-lifecycle. - Sender check defers to the mailer.
inviteSenderReadynow usesisEmailEnabled()(theLivemailer) instead of re-derivingsmtp_host/resend_api_key. - Empty accepted consistently.
validInviteDelivery→normalizeInviteDeliverymaps""tocalendarin both create and patch. - Operator signal.
InviteSenderReadyon the single GET plus the amber editor warning. - Regression tests for reschedule, reassign and the reconciler heal.
ℹ️ The provider contract the feature rests on is untested
The calnode guarantee — no provider (and so no host account) emails the booker — depends entirely on gcal, Microsoft Graph and CalDAV treating an empty OrganizerEmail as "no guest" (gcal/events.go:86-92, calendar/microsoft/events.go:83-88, caldav/events.go:350-356). The new tests assert only at the fake-provider boundary (invite_delivery_internal_test.go), so a regression that drops one of those guards would compile, pass this PR's whole suite, and silently resume host-account invites.
Technical details
# Provider-side "empty organizer ⇒ no invite" is only tested through the fake provider
## Affected sites
- `internal/gcal/events.go:86-92` — attendee appended only when `OrganizerEmail != ""`.
- `internal/calendar/microsoft/events.go:83-88` — same guard; `attendees,omitempty`.
- `internal/caldav/events.go:350-356` — emits `ORGANIZER` only when non-empty; never emits `ATTENDEE`.
## Required outcome
- One payload-level assertion per provider (empty organizer → no attendee in the Google/Microsoft request body, no `ORGANIZER`/`ATTENDEE` in the CalDAV `.ics`). Optional, but it is the only place the new guarantee can actually break.
## Open questions for the human
- Is this in scope while the three providers are otherwise unchanged?ℹ️ Nitpicks
eventTypeJSON.InviteSenderReady(internal/handler/event_type.go:79) has noomitempty, andListEventTypesserializeseventTypeJSON(:397), so list responses carry"invite_sender_ready": falseon every row — contradicting theapi.tscomment "Only returned on the single event type GET". Harmless today, but a consumer reading the list could misread it as "email not set up".docs/ARCHITECTURE.md(new lines ~552-553) says an emptycalendarInviteeis read by "every provider" as "no attendee"; for CalDAV the field maps toORGANIZER, not an attendee (thecalendarInviteecode comment was corrected this commit; the doc was not).
opencode-go/deepseek-v4.1-flash ｜ 𝕏
| func (h *Handler) applyInviteDelivery(ctx context.Context, d *mailer.BookingData, mode, primaryHostID string) { | ||
| if mode == booking.InviteByCalnode { | ||
| d.AttachICS = true | ||
| d.HideHostInInvite = true |
There was a problem hiding this comment.
In calnode mode the address is hidden but d.HostName/d.HostEmail are left at the host values, so the booker still sees the host's name: the With: row in the confirmation/reschedule/cancellation emails (text and HTML), the .ics DESCRIPTION (BuildICS reads d.HostName directly, not the organizer), and the Google/Outlook deep-link details/body. If hiding only the address is the intent this is fine — but the PR body's "book as a team under one name" reads like the identity should be the workspace's. The new tests assert only on the host's address, so they can't catch this either.
Technical details
# Host display name still reaches the booker in `calnode` mode
## Affected sites
- `internal/handler/invite_delivery.go:80` — calnode branch swaps ORGANIZER but never clears `d.HostName`/`d.HostEmail`.
- `internal/mailer/ics.go:40` — `DESCRIPTION` built from `d.HostName` regardless of `HideHostInInvite`.
- `internal/mailer/booking.go:182` (`calDetails`, used by `GoogleCalURL`/`OutlookCalURL`) and the `email_label_with` rows at `booking.go:440,485,521` and `html.go:74,101,126`.
- Callers seeding the host name before `applyInviteDelivery`: `booking_handler.go:1333` (confirm), `:1770` (cancel), `manage_handler.go:317` (reschedule), `reassign.go:206` (reassign). Out of scope but same theme: worker reminders and the `/manage/<token>` page render `HostName` with no mode awareness.
## Required outcome
- Decide whether `calnode` hides the host's address only or their whole identity. If the latter, attendee-facing renders need the workspace sender/brand name; host copies (`applyHostInvite`, `hostBookingData`) must keep the host's own name.
## Open questions for the human
- Is the host's name on the booker's invite intentional (the public booking page already shows the host), or should it be the workspace name?
2 participants
Why
Today the booker's calendar invite is sent by the host's own calendar. Google and Outlook send it from the host's personal account, so every booker sees that person's email address.
That doesn't fit two common setups:
What
A new per-event-type setting, Calendar invite → Sent by:
With the second option, hosts still get the meeting on their own calendar with its Meet/Teams link. The only difference is that the booker isn't added as a guest there, so Google/Microsoft email nobody.
Notes for review
Tests
invite_delivery_internal_test.gocovers four cases. Calnode mode: the host's calendar gets no guest, and nothing the booker receives contains the host's address. Default mode: unchanged, and the booker gets no duplicate.ics. Cancel: follows the booking's mode, not the event type's. Validation: Calnode mode is refused without email.ics_test.go: with no sender configured, the invite has no organizer rather than the host's address."Palamond" <bookings@…>, hasORGANIZER;CN="Palamond":mailto:bookings@…, and contains no host address anywhere.