From d3163f22813418fbd92340cbb3a63cb404ad6813 Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Fri, 12 Jun 2026 20:50:52 -0400 Subject: [PATCH] fix(09): IN-02 extract shared resolveFinalRrule helper to deduplicate the update/create RRULE decision tree --- apps/api/src/broker/outboxWorker.ts | 118 +++++++++++++++------------- 1 file changed, 64 insertions(+), 54 deletions(-) diff --git a/apps/api/src/broker/outboxWorker.ts b/apps/api/src/broker/outboxWorker.ts index 6b2e7f5..09540aa 100644 --- a/apps/api/src/broker/outboxWorker.ts +++ b/apps/api/src/broker/outboxWorker.ts @@ -143,6 +143,56 @@ export function assembleRruleString( return s; } +/** + * IN-02: shared RRULE-resolution decision tree for the update and create branches. + * + * The two branches differ only in the SOURCE of `preservedRrule` (the update branch + * re-reads it from calendarEvents.rawVevent; the create branch reads the + * `_preservedRrule` payload field threaded by the edit-as-move route). The precedence + * logic is identical and was previously copy-pasted, risking drift between the two + * copies of the RFC-5545 bound-strip (`;(UNTIL|COUNT)=` removal) — see Pitfall 3. + * + * Precedence: + * 1. Explicit recurrence on the payload wins (recurrence:'none' clears the RRULE). + * 2. Else, if a preserved RRULE exists and the payload changes only the bound + * (UNTIL/COUNT), strip the preserved RRULE's existing bound and re-apply the new + * one — never naive-concatenate (would produce a double-UNTIL/COUNT). + * 3. Else fall back to the payload's preset (rruleFromPayload), or the preserved + * RRULE unchanged when no bound change was requested. + */ +function resolveFinalRrule( + fields: OutboxPayloadFields, + hasExplicitRecurrence: boolean, + rruleFromPayload: string | undefined, + preservedRrule: string | undefined, +): string | undefined { + if (hasExplicitRecurrence) { + // Explicit recurrence wins — recurrence:'none' yields undefined (no RRULE emitted) + return rruleFromPayload + ? assembleRruleString( + rruleFromPayload, + fields.recurrenceUntil, + fields.recurrenceCount, + fields.allDay, + ) + : undefined; + } + if (preservedRrule) { + if (fields.recurrenceUntil || fields.recurrenceCount !== undefined) { + // Bound change only: strip existing UNTIL/COUNT, then re-apply the new bound (Pitfall 3) + const strippedPreset = preservedRrule.replace(/;(UNTIL|COUNT)=[^;]*/g, ''); + return assembleRruleString( + strippedPreset, + fields.recurrenceUntil, + fields.recurrenceCount, + fields.allDay, + ); + } + return preservedRrule; + } + return rruleFromPayload; +} + // ── Drain concurrency guard (CR-05) ────────────────────────────────────────── /** @@ -422,33 +472,13 @@ async function dispatchRow(row: OutboxRow): Promise { // concatenate onto `FREQ=WEEKLY;BYDAY=...` which would produce double-UNTIL. // WR-01 note: preservedRrule is only set when !hasExplicitRecurrence (see above), // so the hasExplicitRecurrence branch always takes precedence over preserved RRULE. - let finalRruleString: string | undefined; - if (hasExplicitRecurrence) { - // Explicit recurrence wins — recurrence:'none' yields undefined (no RRULE emitted) - finalRruleString = rruleFromPayload - ? assembleRruleString( - rruleFromPayload, - fields.recurrenceUntil, - fields.recurrenceCount, - fields.allDay, - ) - : undefined; - } else if (preservedRrule) { - if (fields.recurrenceUntil || fields.recurrenceCount !== undefined) { - // Series edit with bound change only: strip existing UNTIL/COUNT, then re-apply - const strippedPreset = preservedRrule.replace(/;(UNTIL|COUNT)=[^;]*/g, ''); - finalRruleString = assembleRruleString( - strippedPreset, - fields.recurrenceUntil, - fields.recurrenceCount, - fields.allDay, - ); - } else { - finalRruleString = preservedRrule; - } - } else { - finalRruleString = rruleFromPayload; - } + // IN-02: shared decision tree extracted to resolveFinalRrule (mirrored in create branch). + const finalRruleString = resolveFinalRrule( + fields, + hasExplicitRecurrence, + rruleFromPayload, + preservedRrule, + ); const { icsString } = buildVeventString({ uid: row.uid, @@ -520,33 +550,13 @@ async function dispatchRow(row: OutboxRow): Promise { // CR-01: an explicit recurrence preset wins over _preservedRrule (deliberate user choice). // recurrence:'none' explicitly clears any RRULE — including when _preservedRrule is present. // If no explicit recurrence, fall back to _preservedRrule (edit-as-move RRULE carry-through). - let finalRruleString: string | undefined; - if (hasExplicitRecurrence) { - // Explicit recurrence wins — recurrence:'none' yields undefined (no RRULE emitted) - finalRruleString = rruleFromPayload - ? assembleRruleString( - rruleFromPayload, - fields.recurrenceUntil, - fields.recurrenceCount, - fields.allDay, - ) - : undefined; - } else if (preservedRrule) { - if (fields.recurrenceUntil || fields.recurrenceCount !== undefined) { - // Bound change on preserved RRULE: strip existing UNTIL/COUNT first (Pitfall 3) - const strippedPreset = preservedRrule.replace(/;(UNTIL|COUNT)=[^;]*/g, ''); - finalRruleString = assembleRruleString( - strippedPreset, - fields.recurrenceUntil, - fields.recurrenceCount, - fields.allDay, - ); - } else { - finalRruleString = preservedRrule; - } - } else { - finalRruleString = rruleFromPayload; - } + // IN-02: shared decision tree extracted to resolveFinalRrule (mirrored in update branch). + const finalRruleString = resolveFinalRrule( + fields, + hasExplicitRecurrence, + rruleFromPayload, + preservedRrule, + ); const { icsString } = buildVeventString({ uid: row.uid,