Fix --scope this/future to operate on the actual occurrence (silent no-op bug)

[jasonkuhrt]

Bug

events delete --scope this --occurrence-start <iso> and events update --scope this --occurrence-start <iso> silently no-op against EKEventStore. Both return {"status":"deleted","ok":true} (or the equivalent update payload) without actually mutating anything.

Reproduction (against real EventKit, 0.3.0 from brew)

# 1. Create a weekly recurring event
acal events create --calendar "Personal" --title "REPRO" \
  --start 2027-01-04T12:00:00-05:00 --end 2027-01-04T13:00:00-05:00 \
  --timezone America/Toronto --repeat weekly --interval 1 --count 5

# Note the returned id; expect 5 occurrences: Jan 4, 11, 18, 25, Feb 1.

# 2. Delete just the third occurrence
acal events delete --id <id> --scope this --occurrence-start 2027-01-18T12:00:00-05:00
# returns {"data": {"id": "<id>", "scope": "this", "status": "deleted"}, "ok": true}

# 3. List Jan 18 — the occurrence is STILL THERE
acal events list --calendar "Personal" --from 2027-01-18T00:00:00-05:00 --to 2027-01-18T23:59:59-05:00
# REPRO event still listed; no exception was created.

Root cause

In Sources/EventKitAdapter/EventKitAdapter.swift, both deleteEvent and updateEvent did:

guard let event = store.calendarItem(withIdentifier: id) as? EKEvent else { ... }
// ...
try store.remove(event, span: eventSpan(for: input.scope), commit: true)

store.calendarItem(withIdentifier: id) returns the series master, not the specific occurrence at occurrenceStart. EventKit's EKSpan.thisEvent (and .futureEvents) require the caller to pass a specific OCCURRENCE — passing the master is undefined behaviour and silently no-ops in practice. occurrenceStart was parsed and validated as required for non-.all scopes but never actually used to locate the occurrence (in updateEvent it was only stamped onto the response record for display).

Fix

Add a private helper resolveTargetEvent(masterEvent:scope:occurrenceStart:) that:

• returns the master directly for .all
• short-circuits to the master for non-recurring events (the master is the only occurrence)
• for .this / .future: expands the recurrence over a 1-day window around occurrenceStart via store.predicateForEvents(withStart:end:calendars:) → store.events(matching:) (the same pattern already used in listEvents at line ~191), filters to calendarItemIdentifier == masterEvent.calendarItemIdentifier, and picks the matching occurrence (1-second tolerance on startDate equality to absorb Date round-trip drift)

Both deleteEvent and updateEvent now route through the helper. The duplicate inline --occurrence-start required validation in deleteEvent is removed (helper now owns it; updateEvent previously lacked the validation entirely, which the helper also fixes).

Why the test suite passes

Tests use InMemoryCalendarStore (Sources/AppCore/InMemoryCalendarStore.swift), whose deleteEvent does:

eventsByID.removeValue(forKey: id)

regardless of scope. The fake doesn't model EventKit's recurrence/exception semantics at all, so it passes whether the production code targets the master or an occurrence. The existing tests verify the envelope/IO contract but never exercise the actual EventKit codepath; the bug is invisible to CI.

A proper regression test would either need real-EventKit integration (slow, requires sandbox calendar permissions) or a substantial enhancement to InMemoryCalendarStore to track per-occurrence exclusion dates. Happy to follow up with whichever direction you prefer; flagging here so it's visible rather than shipping a half-baked fake.

Validation

Manually validated against real EKEventStore on macOS 26 (acal full_access):

Step	Pre-fix	Post-fix
Create 5-occurrence weekly series	✓	✓
delete --scope this --occurrence-start <Jan-18> returns ok=true	✓	✓
Jan 18 occurrence actually removed	✗ still present	✓ removed
Other 4 occurrences intact	✓	✓
Series master intact (further occurrences computed correctly)	✓	✓
delete --scope all cleans up	✓	✓

Also validated on a real production scenario: 15 distinct occurrences of recurring co-parenting routine events for a specific 23-day window, all --scope this deletions correctly applied without affecting the series elsewhere.

Severity

High — affects every scope-targeted op on recurring events:

• events delete --scope this — silently no-op (this PR fixes)
• events delete --scope future — same code path, same bug (this PR fixes)
• events update --scope this — same code path, same bug (this PR fixes)
• events update --scope future — same code path, same bug (this PR fixes)

The [ok=true, status=deleted] envelope makes the failure mode invisible to scripted callers — they assume success and move on.

[gemini-code-assist]

Code Review

This pull request introduces a resolveTargetEvent helper method to correctly identify specific occurrences of recurring events when applying mutations with 'this' or 'future' scopes. This ensures that EventKit's EKSpan semantics are respected. Feedback was provided regarding the event search predicate: if the event's calendar is missing, passing an empty array to the predicate results in no matches, so it is recommended to pass nil to search all calendars instead.

[gemini-code-assist]

In EventKit, passing an empty array to predicateForEvents(withStart:end:calendars:) results in a search that matches no events. While masterEvent.calendar should not be nil for a stored event, the use of compactMap here suggests a defensive approach. If the calendar is missing, it is safer to pass nil to the predicate to search all calendars rather than searching none, which would lead to a confusing notFound error.

        let calendars = [masterEvent.calendar].compactMap { $0 }
        guard
            let dayBefore = Calendar.current.date(byAdding: .day, value: -1, to: occStart),
            let dayAfter = Calendar.current.date(byAdding: .day, value: 1, to: occStart)
        else {
            throw ACalError.validation("Could not compute occurrence search window for \(DateCodec.iso8601String(from: occStart)).")
        }
        let predicate = store.predicateForEvents(
            withStart: dayBefore,
            end: dayAfter,
            calendars: calendars.isEmpty ? nil : calendars
        )

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9dc123536e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Allow non-recurring updates without occurrence-start

This new validation now rejects all .this/.future updates when occurrenceStart is omitted, even for non-recurring events, because it throws before the non-recurring short-circuit is reached. That regresses prior updateEvent behavior and contradicts the CLI/MCP contract text that occurrenceStart is for recurring-instance edits; a single event update like --scope this now fails with a validation error despite having only one occurrence.

Useful? React with 👍 / 👎.