Render maven's own reminders to a calendar she owns (#127) #56

2026-07-31T23:56:16+02:00

claude commented

2026-07-31 23:56:16 +02:00

What

Radicale becomes maven's own calendar, as a write-only render target rather than a second store. sqlite stays canonical.

internal/calendar — new pure package holding the one calendar data model everything else shares: Event, ParseICal/ParseICalDay, RenderICal, the FactKey/FactValue encoding, Busy/Overlapping, and the source constants (poll:caldav, poll:caldav:work, ambient:notif) with ReadOnlySource. The parse was inlined in cmd/mavcaldav and the fact key was a Sprintf in two files; #126 and #128 both have to agree with it.
cmd/mavcaldav/render.go — reads pending reminders from core each poll, PUTs one iCal resource per reminder (maven-reminder-<id>.ics), DELETEs the ones that fired or were cancelled. Unchanged reminders are not re-PUT. A failed PUT is not recorded as published, so it retries.
cmd/mavcaldav/main.go — new flags -render-url, -render-user, -render-pass, -render-duration. Rendering is off unless -render-url is set.

Why it cannot write to the work calendar

Three independent reasons, not one check:

The render URL and its credential are separate flags from the read ones.
checkRenderTarget refuses at startup when -render-url names the collection -url reads (slash- and case-insensitive).
Every path it addresses carries calendar.ReminderUIDPrefix, so even aimed at the wrong collection it can only touch resources it created.

A calendar maven cannot reach never breaks a reminder: errors are logged, the reminder lives in sqlite.

Fixed along the way

The inline parse took the day number off a local clock reading but built the window boundaries in UTC. East of Greenwich the window was shifted by the offset, so part of the evening fell outside "today" and the poller saw an empty calendar after 20:00 UTC. This is why TestPollOnce failed on a +04 box. "Today" is now the owner's day in the owner's location. Regression test: TestParseICalDayUsesOwnersDay.

Reminder payloads are owner-supplied text, so RenderICal applies RFC 5545 TEXT escaping — a payload cannot close the VEVENT and inject properties (TestRenderICalEscapesInjection).

Verified

make build and make test both exit 0. New tests: 9 in internal/calendar (parse window, all-day rejection, fact encoding, busy/overlap, source trust, render round-trip, determinism, injection), 6 in cmd/mavcaldav (publish only pending, skip unchanged, withdraw resolved, survive 5xx, recurring uses next occurrence, render-target guard).

Note mavcaldav is still not in docker-compose/deploy/ — it is flag-configured and has to be launched by hand. Wiring it and creating the Radicale collection is the deploy half.

Vikunja #127

## What Radicale becomes maven's own calendar, as a **write-only render target** rather than a second store. sqlite stays canonical. - `internal/calendar` — new pure package holding the one calendar data model everything else shares: `Event`, `ParseICal`/`ParseICalDay`, `RenderICal`, the `FactKey`/`FactValue` encoding, `Busy`/`Overlapping`, and the source constants (`poll:caldav`, `poll:caldav:work`, `ambient:notif`) with `ReadOnlySource`. The parse was inlined in `cmd/mavcaldav` and the fact key was a `Sprintf` in two files; #126 and #128 both have to agree with it. - `cmd/mavcaldav/render.go` — reads pending reminders from core each poll, PUTs one iCal resource per reminder (`maven-reminder-<id>.ics`), DELETEs the ones that fired or were cancelled. Unchanged reminders are not re-PUT. A failed PUT is not recorded as published, so it retries. - `cmd/mavcaldav/main.go` — new flags `-render-url`, `-render-user`, `-render-pass`, `-render-duration`. Rendering is **off unless `-render-url` is set**. ## Why it cannot write to the work calendar Three independent reasons, not one check: 1. The render URL and its credential are separate flags from the read ones. 2. `checkRenderTarget` refuses at startup when `-render-url` names the collection `-url` reads (slash- and case-insensitive). 3. Every path it addresses carries `calendar.ReminderUIDPrefix`, so even aimed at the wrong collection it can only touch resources it created. A calendar maven cannot reach never breaks a reminder: errors are logged, the reminder lives in sqlite. ## Fixed along the way The inline parse took the day number off a **local** clock reading but built the window boundaries in **UTC**. East of Greenwich the window was shifted by the offset, so part of the evening fell outside "today" and the poller saw an empty calendar after 20:00 UTC. This is why `TestPollOnce` failed on a +04 box. "Today" is now the owner's day in the owner's location. Regression test: `TestParseICalDayUsesOwnersDay`. Reminder payloads are owner-supplied text, so `RenderICal` applies RFC 5545 TEXT escaping — a payload cannot close the VEVENT and inject properties (`TestRenderICalEscapesInjection`). ## Verified `make build` and `make test` both exit 0. New tests: 9 in `internal/calendar` (parse window, all-day rejection, fact encoding, busy/overlap, source trust, render round-trip, determinism, injection), 6 in `cmd/mavcaldav` (publish only pending, skip unchanged, withdraw resolved, survive 5xx, recurring uses next occurrence, render-target guard). Note `mavcaldav` is still not in `docker-compose`/`deploy/` — it is flag-configured and has to be launched by hand. Wiring it and creating the Radicale collection is the deploy half. Vikunja #127

claude added 1 commit 2026-07-31 23:56:16 +02:00

Render maven's own reminders to a calendar she owns (#127 ) 3af290152c

Radicale becomes a write-only render target, not a store. sqlite stays
canonical: every poll mavcaldav reads the pending reminders out of core and
publishes each one as a single-event iCal resource, withdrawing the ones that
have fired or been cancelled. Losing the collection costs nothing — the next
tick rebuilds it, and nothing is ever read back from it.

It structurally cannot write to a calendar maven only reads. The render URL and
credential are their own flags, and -render-url is refused at startup when it
names the collection -url reads; the only paths it addresses carry the
maven-reminder- prefix, so even aimed at the wrong collection it can only touch
resources it created. Rendering is off unless -render-url is given.

The calendar data model now lives in one place, internal/calendar: the Event,
the iCal parse it comes from and the render it goes to, the fact key/value
encoding, and the source constants that say which calendars may be written to.
It was a parse inlined in cmd/mavcaldav and a Sprintf in two files; #126 and
#128 both need to agree with it.

Fixes a latent day-boundary bug moved out of that inline parse: it took the day
number off a local clock reading but built the window boundaries in UTC, so on
a box east of Greenwich part of the evening fell outside "today" and the poller
saw an empty calendar after 20:00 UTC. Today is now the owner's day in the
owner's location, which is what the busy gate and the day plan mean.

kami commented

2026-08-01 10:26:07 +02:00

tbh, I'm pissed that this diff is 1000 LOC.
but besides this - everything else look great, but I once again noticed that we could benefit from (well, at least basic and small) DI pattern here. just saying.

tbh, I'm pissed that this diff is 1000 LOC. but besides this - everything else look great, but I once again noticed that we could benefit from (well, at least basic and small) DI pattern here. just saying.

claude reviewed 2026-08-01 11:24:51 +02:00

claude left a comment

The render side is built the right way round. sqlite stays canonical, the collection is a view, nothing is ever read back, and checkRenderTarget refuses at startup rather than trusting at runtime. ReminderUIDPrefix bounding every path it addresses is the second lock, and it is the one that still holds if the first is misconfigured. Pulling the iCal parser into internal/calendar also kills the duplicate that was about to appear in the ambient reader.

Two real problems.

1. ParseICalDay swaps one timezone bug for its mirror image. The window is now built in now.Location(), but parseDT still stamps a local DTSTART;TZID=... value as UTC. So the boundaries are local and the events are UTC, and the comparison mixes frames. Work it through on a +03 box. Local midnight is 21:00Z the previous day, so the window is 21:00Z to 21:00Z. An event at 22:00 local parses to 22:00Z, which is past the end, and gets dropped. The old code built both sides in UTC and included it. The doc comment says the old version lost part of the evening. As written, this version loses the evening the old one kept, from 21:00 local onward. Either parse TZID for real, or keep both sides in the same frame and label the day off the local date.

2. Withdrawal does not survive a restart. published is in-memory, so the second loop only withdraws reminders this process published. Fire a reminder, restart mavcaldav, and its event stays in the collection forever. Nothing ever revisits it. The comment "losing it costs nothing, the next tick rebuilds it" holds for events that should be there. It does not hold for the ones that should not. A PROPFIND over the collection, filtered to ReminderUIDPrefix, would let a fresh process reconcile what it finds against what is pending. Simpler alternative: on startup, delete every ReminderUIDPrefix resource and republish from scratch.

Smaller:

checkRenderTarget compares against one read URL. PR 57 adds a second calendar to read. Make the check take a list before that lands. Otherwise the guarantee in the package comment stops covering every read target.
writeIfChanged gained a confidence parameter, and every caller in this diff passes 1.0. Presumably PR 57 needs it. Dead parameters that arrive one PR early are hard to review, since there is nothing to check the intent against.

The render side is built the right way round. sqlite stays canonical, the collection is a view, nothing is ever read back, and `checkRenderTarget` refuses at startup rather than trusting at runtime. `ReminderUIDPrefix` bounding every path it addresses is the second lock, and it is the one that still holds if the first is misconfigured. Pulling the iCal parser into `internal/calendar` also kills the duplicate that was about to appear in the ambient reader. Two real problems. **1. `ParseICalDay` swaps one timezone bug for its mirror image.** The window is now built in `now.Location()`, but `parseDT` still stamps a local `DTSTART;TZID=...` value as UTC. So the boundaries are local and the events are UTC, and the comparison mixes frames. Work it through on a +03 box. Local midnight is `21:00Z` the previous day, so the window is `21:00Z` to `21:00Z`. An event at 22:00 local parses to `22:00Z`, which is past the end, and gets dropped. The old code built both sides in UTC and included it. The doc comment says the old version lost part of the evening. As written, this version loses the evening the old one kept, from 21:00 local onward. Either parse `TZID` for real, or keep both sides in the same frame and label the day off the local date. **2. Withdrawal does not survive a restart.** `published` is in-memory, so the second loop only withdraws reminders this process published. Fire a reminder, restart mavcaldav, and its event stays in the collection forever. Nothing ever revisits it. The comment "losing it costs nothing, the next tick rebuilds it" holds for events that should be there. It does not hold for the ones that should not. A PROPFIND over the collection, filtered to `ReminderUIDPrefix`, would let a fresh process reconcile what it finds against what is pending. Simpler alternative: on startup, delete every `ReminderUIDPrefix` resource and republish from scratch. Smaller: - `checkRenderTarget` compares against one read URL. PR 57 adds a second calendar to read. Make the check take a list before that lands. Otherwise the guarantee in the package comment stops covering every read target. - `writeIfChanged` gained a `confidence` parameter, and every caller in this diff passes `1.0`. Presumably PR 57 needs it. Dead parameters that arrive one PR early are hard to review, since there is nothing to check the intent against.

kami referenced this issue from a commit

2026-08-01 14:43:48 +02:00

calendar: resolve DTSTART against its own TZID

kami referenced this issue from a commit

2026-08-01 14:43:52 +02:00

mavcaldav: reconcile the render collection on startup

kami closed this pull request

2026-08-01 14:51:41 +02:00

kami commented

2026-08-01 14:52:34 +02:00

Landed on master. The stack was one linear chain, so #84 carried every commit from #50 up, and master now contains this branch in full. Merging this PR on its own is an empty diff, so it is closed rather than merged. The review findings for it were fixed in the 2026-08-01 pass and are on master as commits on the stack tip, not on this branch.

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: kami/Maven#56