diff --git a/electron-builder.yaml b/electron-builder.yaml index a1407205f2..177ea63d40 100644 --- a/electron-builder.yaml +++ b/electron-builder.yaml @@ -61,6 +61,13 @@ nsis: # allow to install to custom location oneClick: false allowToChangeInstallationDirectory: true + # We ship no in-app auto-updater: electron-updater is not a dependency and the + # autoUpdater block in electron/start-app.ts is commented out. elevate.exe is + # only ever invoked by electron-updater to apply privileged (per-machine) + # updates, so electron-builder bundles it here as dead weight that AV engines + # frequently false-positive on. Drop it. Safe because perMachine is not true + # (with perMachine: true, packElevateHelper: false is ignored). See #8135. + packElevateHelper: false portable: artifactName: ${name}-${arch}.${ext} appx: diff --git a/packages/plugin-dev/caldav-calendar-provider/src/plugin.spec.ts b/packages/plugin-dev/caldav-calendar-provider/src/plugin.spec.ts index 9926d1753a..87726bf595 100644 --- a/packages/plugin-dev/caldav-calendar-provider/src/plugin.spec.ts +++ b/packages/plugin-dev/caldav-calendar-provider/src/plugin.spec.ts @@ -812,6 +812,201 @@ END:VCALENDAR }); }); + // Issue #7492 follow-up: an expanded RRULE occurrence (`#occ=` id) maps + // back to the shared master `.ics`. Write paths must NOT silently mutate the + // whole series, and getById must report the occurrence's own time. + describe('per-occurrence write safety (issue #7492)', () => { + const cfg = { + serverUrl: 'https://cloud.example.com', + username: 'user', + password: 'pass', + writeCalendarId: '/dav/calendars/user/default/', + }; + const calHref = '/dav/calendars/user/default/'; + const eventHref = '/dav/calendars/user/default/weekly.ics'; + const occId = (ms: number): string => `${calHref}::${eventHref}#occ=${ms}`; + const masterId = `${calHref}::${eventHref}`; + + const timedMaster = [ + 'BEGIN:VCALENDAR', + 'VERSION:2.0', + 'BEGIN:VEVENT', + 'UID:weekly-uid', + 'DTSTART:20260105T100000Z', + 'DTEND:20260105T110000Z', + 'RRULE:FREQ=WEEKLY;COUNT=4', + 'SUMMARY:Weekly Standup', + 'LAST-MODIFIED:20260101T090000Z', + 'SEQUENCE:0', + 'END:VEVENT', + 'END:VCALENDAR', + ].join('\r\n'); + + const allDayMaster = [ + 'BEGIN:VCALENDAR', + 'VERSION:2.0', + 'BEGIN:VEVENT', + 'UID:weekly-allday-uid', + 'DTSTART;VALUE=DATE:20260105', + 'DTEND;VALUE=DATE:20260106', + 'RRULE:FREQ=WEEKLY;COUNT=4', + 'SUMMARY:Weekly Holiday', + 'END:VEVENT', + 'END:VCALENDAR', + ].join('\r\n'); + + let mockHttp: any; + beforeEach(() => { + mockHttp = { + get: vi.fn().mockResolvedValue(timedMaster), + post: vi.fn(), + put: vi.fn(), + patch: vi.fn(), + delete: vi.fn(), + request: vi.fn(), + }; + }); + + it('updateIssue refuses a single occurrence and writes nothing', async () => { + const err: any = await definition.updateIssue!( + occId(Date.parse('2026-01-12T10:00:00Z')), + { summary: 'Changed' }, + cfg as any, + mockHttp as any, + ) + .then(() => null) + .catch((e) => e); + expect(err).toBeInstanceOf(Error); + expect(String(err.message)).toMatch(/single occurrence/i); + // Marker tells the host's two-way sync this is an expected limitation, not + // a failure, so editing the task stays silent (issue #7492). + expect(err.isExpectedSyncSkip).toBe(true); + // Guard runs before any network I/O — the master is never touched. + expect(mockHttp.get).not.toHaveBeenCalled(); + expect(mockHttp.put).not.toHaveBeenCalled(); + }); + + it('deleteIssue refuses a single occurrence and deletes nothing', async () => { + const err: any = await definition.deleteIssue!( + occId(Date.parse('2026-01-12T10:00:00Z')), + cfg as any, + mockHttp as any, + ) + .then(() => null) + .catch((e) => e); + expect(err).toBeInstanceOf(Error); + expect(String(err.message)).toMatch(/single occurrence/i); + expect(err.isExpectedSyncSkip).toBe(true); + expect(mockHttp.delete).not.toHaveBeenCalled(); + }); + + it('updateIssue still writes a non-occurrence (no #occ=) event', async () => { + await definition.updateIssue!( + masterId, + { summary: 'Changed' }, + cfg as any, + mockHttp as any, + ); + expect(mockHttp.put).toHaveBeenCalledTimes(1); + }); + + it('getById re-anchors a timed occurrence to its own start/end', async () => { + const occMs = Date.parse('2026-01-19T10:00:00Z'); + const issue: any = await definition.getById!( + occId(occMs), + cfg as any, + mockHttp as any, + ); + expect(issue.start).toBe('20260119T100000Z'); + expect(issue.end).toBe('20260119T110000Z'); + // Must NOT collapse onto the master's first (Jan 5) instance. + expect(issue.start).not.toBe('20260105T100000Z'); + // And it round-trips through the two-way-sync extractor to the right instant. + const vals = definition.extractSyncValues!(issue); + expect(vals.start_dateTime).toBe('2026-01-19T10:00:00.000Z'); + expect(vals.duration_ms).toBe(60 * 60 * 1000); + }); + + it('getById re-anchors an all-day occurrence to its own date', async () => { + mockHttp.get = vi.fn().mockResolvedValue(allDayMaster); + // ical.js represents an all-day occurrence as local midnight, matching + // the value the expander stamps into the id. + const occMs = new Date(2026, 0, 19).getTime(); + const issue: any = await definition.getById!( + occId(occMs), + cfg as any, + mockHttp as any, + ); + expect(issue.start).toBe('20260119'); + expect(issue.startParams).toBe('VALUE=DATE'); + expect(issue.end).toBe('20260120'); + const vals = definition.extractSyncValues!(issue); + expect(vals.start_date).toBe('2026-01-19'); + }); + + it('getById returns the master start for a non-occurrence id', async () => { + const issue: any = await definition.getById!(masterId, cfg as any, mockHttp as any); + expect(issue.start).toBe('20260105T100000Z'); + }); + + it('getById re-anchors a TZID-master occurrence to the right instant', async () => { + const tzidMaster = [ + 'BEGIN:VCALENDAR', + 'VERSION:2.0', + 'BEGIN:VEVENT', + 'UID:weekly-tzid-uid', + 'DTSTART;TZID=America/New_York:20260105T090000', + 'DTEND;TZID=America/New_York:20260105T100000', + 'RRULE:FREQ=WEEKLY;COUNT=4', + 'SUMMARY:Weekly NY Standup', + 'END:VEVENT', + 'END:VCALENDAR', + ].join('\r\n'); + mockHttp.get = vi.fn().mockResolvedValue(tzidMaster); + // Jan 19 2026 09:00 in America/New_York (EST, UTC-5) = 14:00Z. + const occMs = Date.parse('2026-01-19T14:00:00Z'); + const issue: any = await definition.getById!( + occId(occMs), + cfg as any, + mockHttp as any, + ); + // The occurrence instant is preserved as UTC, regardless of the runner's TZ. + expect(issue.start).toBe('20260119T140000Z'); + expect(issue.end).toBe('20260119T150000Z'); + const vals = definition.extractSyncValues!(issue); + expect(vals.start_dateTime).toBe('2026-01-19T14:00:00.000Z'); + // Duration comes from the TZID-resolved master (09:00->10:00 = 1h), not a + // timezone-skewed value. + expect(vals.duration_ms).toBe(60 * 60 * 1000); + }); + + it('getById re-anchors a floating-time master occurrence and preserves duration', async () => { + const floatingMaster = [ + 'BEGIN:VCALENDAR', + 'VERSION:2.0', + 'BEGIN:VEVENT', + 'UID:weekly-floating-uid', + 'DTSTART:20260105T090000', + 'DTEND:20260105T103000', + 'RRULE:FREQ=WEEKLY;COUNT=4', + 'SUMMARY:Weekly Floating Standup', + 'END:VEVENT', + 'END:VCALENDAR', + ].join('\r\n'); + mockHttp.get = vi.fn().mockResolvedValue(floatingMaster); + const occMs = Date.parse('2026-01-19T09:00:00Z'); + const issue: any = await definition.getById!( + occId(occMs), + cfg as any, + mockHttp as any, + ); + expect(issue.start).toBe('20260119T090000Z'); + expect(issue.end).toBe('20260119T103000Z'); + const vals = definition.extractSyncValues!(issue); + expect(vals.duration_ms).toBe(90 * 60 * 1000); + }); + }); + describe('timeBlock.upsertEvent', () => { const cfg = { serverUrl: 'https://dav.example.com/', diff --git a/packages/plugin-dev/caldav-calendar-provider/src/plugin.ts b/packages/plugin-dev/caldav-calendar-provider/src/plugin.ts index dcd8f8cd93..cde9b20704 100644 --- a/packages/plugin-dev/caldav-calendar-provider/src/plugin.ts +++ b/packages/plugin-dev/caldav-calendar-provider/src/plugin.ts @@ -62,21 +62,49 @@ const toCompoundId = ( const parseCompoundId = ( id: string, fallbackCalendarHref: string, -): { calendarHref: string; eventHref: string } => { +): { calendarHref: string; eventHref: string; occurrenceMs?: number } => { // Strip the occurrence suffix first so the remaining base id is unambiguous. + // A present `#occ=` marks an expanded RRULE instance whose eventHref + // still resolves to the shared master resource — surface it so write/read + // paths can stay occurrence-aware instead of silently hitting the master. const occIdx = id.lastIndexOf(OCCURRENCE_SEP); - const baseId = - occIdx !== -1 && /^\d+$/.test(id.slice(occIdx + OCCURRENCE_SEP.length)) - ? id.slice(0, occIdx) - : id; + const hasOccurrence = + occIdx !== -1 && /^\d+$/.test(id.slice(occIdx + OCCURRENCE_SEP.length)); + const occurrenceMs = hasOccurrence + ? parseInt(id.slice(occIdx + OCCURRENCE_SEP.length), 10) + : undefined; + const baseId = hasOccurrence ? id.slice(0, occIdx) : id; const sep = baseId.indexOf(COMPOUND_SEP); - if (sep === -1) return { calendarHref: fallbackCalendarHref, eventHref: baseId }; + if (sep === -1) { + return { calendarHref: fallbackCalendarHref, eventHref: baseId, occurrenceMs }; + } return { calendarHref: baseId.slice(0, sep), eventHref: baseId.slice(sep + COMPOUND_SEP.length), + occurrenceMs, }; }; +/** + * A write that targets a single expanded RRULE occurrence (an `#occ=` id) can't + * be applied: the occurrence maps back to the shared master resource, so writing + * it would mutate the whole series. We refuse with this marked error. The + * `isExpectedSyncSkip` flag tells the host's two-way sync that this is an + * expected limitation — the user edited/deleted their *task*, not the calendar — + * so it stays silent instead of showing a sync-failure snack. Explicit calendar + * actions (agenda reschedule/delete) don't check the flag and still surface the + * message, which is the honest feedback there. See issue #7492. + */ +const unsupportedOccurrenceWriteError = (action: 'edit' | 'delete'): Error => + Object.assign( + new Error( + `${action === 'edit' ? 'Editing' : 'Deleting'} a single occurrence of a ` + + `recurring event is not supported yet. ` + + `Please ${action} the event in your calendar app instead.`, + ), + { isExpectedSyncSkip: true }, + ); + // --- iCal Helpers --- /** Unfold iCal line continuations (RFC 5545 Section 3.1) */ @@ -1107,7 +1135,7 @@ PluginAPI.registerIssueProvider({ http: PluginHttp, ): Promise { const cfg = config as unknown as CaldavCalendarConfig; - const { eventHref } = parseCompoundId(issueId, getWriteCalendarId(cfg)); + const { eventHref, occurrenceMs } = parseCompoundId(issueId, getWriteCalendarId(cfg)); const eventUrl = resolveHref(cfg, eventHref); const icalData = await http.get(eventUrl, { responseType: 'text' }); const events = parseVEvents(icalData); @@ -1116,8 +1144,48 @@ PluginAPI.registerIssueProvider({ throw new Error('Event not found: ' + eventHref); } - const startDate = parseIcalDateTime(event.dtstart, event.dtstartParams); - const endDate = parseIcalDateTime(event.dtend, event.dtendParams); + // The master VEVENT only carries the series' first DTSTART. For an expanded + // occurrence, re-anchor start/end onto THIS instance so the detail panel and + // the two-way-sync pull use the occurrence's time instead of collapsing every + // instance onto the master's. occurrenceMs is the same `toJSDate().getTime()` + // value the expander stamped into the id, so it round-trips exactly. See #7492. + let { dtstart: start, dtend: end } = event; + let startParams = event.dtstartParams; + let endParams = event.dtendParams; + if (occurrenceMs !== undefined) { + const allDay = isDateOnly(event.dtstart, event.dtstartParams); + const masterStart = parseIcalDateTime(event.dtstart, event.dtstartParams); + let durationMs = parseDuration(event.duration); + if (!durationMs && masterStart && event.dtend) { + const masterEnd = parseIcalDateTime(event.dtend, event.dtendParams); + if (masterEnd) durationMs = masterEnd.getTime() - masterStart.getTime(); + } + const occStart = new Date(occurrenceMs); + if (allDay) { + // ical.js represents all-day occurrences as local midnight, so the + // local-getter formatter (toIcalDate) yields the correct calendar date. + start = toIcalDate(occStart); + startParams = 'VALUE=DATE'; + if (durationMs > 0) { + const occEnd = new Date(occStart); + occEnd.setDate(occEnd.getDate() + Math.round(durationMs / 86400000)); + end = toIcalDate(occEnd); + endParams = 'VALUE=DATE'; + } else { + end = ''; + endParams = ''; + } + } else { + start = toIcalUtcDateTime(occStart); + startParams = ''; + end = + durationMs > 0 ? toIcalUtcDateTime(new Date(occurrenceMs + durationMs)) : ''; + endParams = ''; + } + } + + const startDate = parseIcalDateTime(start, startParams); + const endDate = parseIcalDateTime(end, endParams); return { id: issueId, @@ -1128,10 +1196,10 @@ PluginAPI.registerIssueProvider({ ? parseIcalDateTime(event.lastModified, '')?.getTime() : undefined, summary: event.summary || '(No title)', - start: event.dtstart, - end: event.dtend, - startParams: event.dtstartParams, - endParams: event.dtendParams, + start, + end, + startParams, + endParams, startFormatted: startDate?.toLocaleString() || '', endFormatted: endDate?.toLocaleString() || '', status: event.status, @@ -1237,7 +1305,9 @@ PluginAPI.registerIssueProvider({ http: PluginHttp, ): Promise { const cfg = config as unknown as CaldavCalendarConfig; - const { eventHref } = parseCompoundId(id, getWriteCalendarId(cfg)); + const { eventHref, occurrenceMs } = parseCompoundId(id, getWriteCalendarId(cfg)); + // Editing one occurrence would rewrite the shared master (whole series). + if (occurrenceMs !== undefined) throw unsupportedOccurrenceWriteError('edit'); const eventUrl = resolveHref(cfg, eventHref); // Fetch current iCal data @@ -1478,7 +1548,9 @@ PluginAPI.registerIssueProvider({ http: PluginHttp, ): Promise { const cfg = config as unknown as CaldavCalendarConfig; - const { eventHref } = parseCompoundId(id, getWriteCalendarId(cfg)); + const { eventHref, occurrenceMs } = parseCompoundId(id, getWriteCalendarId(cfg)); + // Deleting one occurrence would DELETE the shared master (whole series). + if (occurrenceMs !== undefined) throw unsupportedOccurrenceWriteError('delete'); const eventUrl = resolveHref(cfg, eventHref); await http.delete(eventUrl, { responseType: 'text' }); }, diff --git a/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.spec.ts b/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.spec.ts index 3d28f7e38e..6e4a4dbd01 100644 --- a/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.spec.ts +++ b/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.spec.ts @@ -710,6 +710,48 @@ describe('IssueTwoWaySyncEffects', () => { adapterRegistry.unregister('TEST_PROVIDER'); })); + + it('should NOT show a snack when push rejects with an expected sync-skip marker (#7492)', fakeAsync(() => { + // The marker normally originates from a provider's updateIssue (e.g. a + // single recurring CalDAV occurrence); injected here via fetchIssue, which + // surfaces through the same push-pipe catch. + const expectedSkip = Object.assign(new Error('single occurrence'), { + isExpectedSyncSkip: true, + }); + const adapter = createMockAdapter({ + getFieldMappings: jasmine + .createSpy('getFieldMappings') + .and.returnValue([isDoneFieldMapping]), + getSyncConfig: jasmine.createSpy('getSyncConfig').and.returnValue({}), + fetchIssue: jasmine.createSpy('fetchIssue').and.rejectWith(expectedSkip), + }); + adapterRegistry.register('TEST_PROVIDER', adapter); + + const task = createMockTask({ + id: 'task-1', + issueType: 'TEST_PROVIDER' as any, + issueId: 'issue-1', + issueProviderId: 'provider-1', + issueLastSyncedValues: { status: 'NEEDS-ACTION' }, + }); + + taskServiceSpy.getByIdOnce$.and.returnValue(of(task)); + issueProviderServiceSpy.getCfgOnce$.and.returnValue(of(createMockIssueProvider())); + + effects.pushFieldsOnTaskUpdate$.subscribe(); + + actions$.next( + TaskSharedActions.updateTask({ + task: { id: 'task-1', changes: { isDone: true } }, + }), + ); + + tick(); + + expect(snackServiceSpy.open).not.toHaveBeenCalled(); + + adapterRegistry.unregister('TEST_PROVIDER'); + })); }); describe('pushTagChangesAfterTagDelete$', () => { @@ -895,6 +937,38 @@ describe('IssueTwoWaySyncEffects', () => { adapterRegistry.unregister('TEST_PROVIDER'); })); + + it('should NOT show a snack when delete rejects with an expected sync-skip marker (#7492)', fakeAsync(() => { + const expectedSkip = Object.assign(new Error('single occurrence'), { + isExpectedSyncSkip: true, + }); + const deleteIssueSpy = jasmine + .createSpy('deleteIssue') + .and.rejectWith(expectedSkip); + const adapter = createMockAdapter({ deleteIssue: deleteIssueSpy }); + adapterRegistry.register('TEST_PROVIDER', adapter); + + const cfg = createMockIssueProvider(); + issueProviderServiceSpy.getCfgOnce$.and.returnValue(of(cfg)); + + const task = createMockTask({ + id: 'task-1', + issueType: 'TEST_PROVIDER' as any, + issueId: 'issue-1', + issueProviderId: 'provider-1', + }) as TaskWithSubTasks; + (task as any).subTasks = []; + + effects.deleteIssueOnTaskDelete$.subscribe(); + + actions$.next(TaskSharedActions.deleteTask({ task })); + + tick(); + + expect(snackServiceSpy.open).not.toHaveBeenCalled(); + + adapterRegistry.unregister('TEST_PROVIDER'); + })); }); describe('deleteIssueOnBulkTaskDelete$', () => { diff --git a/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.ts b/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.ts index 3c868bb4e7..fd2a08a732 100644 --- a/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.ts +++ b/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.ts @@ -43,6 +43,19 @@ const SYNCABLE_TASK_FIELDS: ReadonlySet = new Set([ 'tagIds', ]); +/** + * A provider's write may reject with this marker to signal an *expected* + * limitation rather than a real failure — e.g. the CalDAV plugin can't yet edit + * or delete a single occurrence of a recurring event (#7492). When it does, + * two-way sync stays silent: the user changed/removed their task, not the + * calendar. Explicit calendar actions (agenda reschedule/delete) don't consult + * this and still surface the message, which is the honest feedback there. + */ +const isExpectedSyncSkipError = (err: unknown): boolean => + typeof err === 'object' && + err !== null && + (err as { isExpectedSyncSkip?: boolean }).isExpectedSyncSkip === true; + const toSortedStringArray = (value: unknown): string[] => Array.isArray(value) ? value @@ -168,6 +181,11 @@ export class IssueTwoWaySyncEffects { concatMap(({ fullTask, changes }) => this._pushChanges$(fullTask, changes).pipe( catchError((err) => { + // Expected provider limitation (e.g. a single recurring occurrence, + // #7492) — the local task edit stands; don't alarm the user. + if (isExpectedSyncSkipError(err)) { + return EMPTY; + } IssueLog.err('Two-way sync push failed', err); this._snackService.open({ type: 'ERROR', @@ -214,6 +232,9 @@ export class IssueTwoWaySyncEffects { concatMap((task) => this._pushChanges$(task, { tagIds: task.tagIds }).pipe( catchError((err) => { + if (isExpectedSyncSkipError(err)) { + return EMPTY; + } IssueLog.err('Two-way sync tag delete push failed', err); this._snackService.open({ type: 'ERROR', @@ -488,6 +509,11 @@ export class IssueTwoWaySyncEffects { concatMap((cfg) => from(adapter.deleteIssue!(issueId, cfg)).pipe( catchError((err) => { + // Expected provider limitation (e.g. a single recurring occurrence, + // #7492) — the local task is already removed; stay silent. + if (isExpectedSyncSkipError(err)) { + return EMPTY; + } // 404/410 means the remote issue is already gone — treat as success // to avoid false "delete failed" toasts (e.g. when polling detects // a remote deletion and then deleteIssue is called on the same issue)