feat(calendar): support conditional CalDAV delete for tasks and events #121

Merged

yuris merged 3 commits from feat/117-caldav-delete into main 2026-09-24 07:42:30 +00:00

Conversation 5 Commits 3 Files changed 12 +2208 -39

yuris commented 2026-09-24 07:12:59 +00:00

Owner

Copy link

Closes #117.

What changed

• Implement CalDavClient::delete with conditional If-Match header and origin/redirect safety.
• Add authenticated DELETE routes /v1/tasks/{id} and /v1/events/{id} requiring write scope.
• Enforce durable two-phase deletion: prepare intent and item snapshot in SQLite journal before dispatch, transition to applied, append task_deleted or event_deleted fact, and remove projections atomically upon acknowledged CalDAV deletion.
• Enforce recurring series scope: require explicit series scope for recurring items, rejecting occurrence-only deletion.
• Add CLI commands life task delete and life event delete with --etag, --operation-id, --input, and --scope flags.
• Add extensive unit and integration tests in tests/delete_tests.rs covering success, stale/missing/weak ETags, recurrence safety, replay, and unauthorized caller.

Verified

• nix develop --command cargo test --test delete_tests (14 passed)
• nix develop --command cargo test --test skill_package_tests (7 passed)
• nix develop --command cargo fmt -- --check
• nix develop --command cargo clippy --all-targets -- -D warnings

Deviations

none

Closes #117. ## What changed - Implement CalDavClient::delete with conditional If-Match header and origin/redirect safety. - Add authenticated DELETE routes /v1/tasks/{id} and /v1/events/{id} requiring write scope. - Enforce durable two-phase deletion: prepare intent and item snapshot in SQLite journal before dispatch, transition to applied, append task_deleted or event_deleted fact, and remove projections atomically upon acknowledged CalDAV deletion. - Enforce recurring series scope: require explicit series scope for recurring items, rejecting occurrence-only deletion. - Add CLI commands life task delete and life event delete with --etag, --operation-id, --input, and --scope flags. - Add extensive unit and integration tests in tests/delete_tests.rs covering success, stale/missing/weak ETags, recurrence safety, replay, and unauthorized caller. ## Verified - nix develop --command cargo test --test delete_tests (14 passed) - nix develop --command cargo test --test skill_package_tests (7 passed) - nix develop --command cargo fmt -- --check - nix develop --command cargo clippy --all-targets -- -D warnings ## Deviations none

yuris added 1 commit 2026-09-24 07:12:59 +00:00

feat(calendar): support conditional CalDAV delete for tasks and events 6516d70dc2

yuris reviewed 2026-09-24 07:18:05 +00:00

yuris left a comment

Copy link

VERDICT: request-changes
src/api/delete.rs:L303: P1: Crash recovery requires a representation field, but delete intents omit it; every prepared or uncertain deletion remains unreconcilable after restart. Teach recovery to classify delete intents from the recorded target and live absence.
src/history/journal.rs:L735: P1: A concurrent sync can read the resource before deletion and upsert its stale projection after this transaction, resurrecting the deleted item. Fence sync commits against the deletion fact or serialize the operations.
src/calendar/client.rs:L299: P1: The shared CalDAV client follows same-origin redirects while carrying Basic credentials; DELETE can follow a redirect with credentials. Refuse redirects for conditional DELETE rather than sending a second credentialed request.
src/api/delete.rs:L379: P2: Any 2xx is treated as completed deletion, but 202 only accepts work for later and 207 can contain a failed member; these responses wrongly record applied and remove projections. Accept completed DELETE statuses only, and retain an unconfirmed state otherwise.

VERDICT: request-changes src/api/delete.rs:L303: P1: Crash recovery requires a representation field, but delete intents omit it; every prepared or uncertain deletion remains unreconcilable after restart. Teach recovery to classify delete intents from the recorded target and live absence. src/history/journal.rs:L735: P1: A concurrent sync can read the resource before deletion and upsert its stale projection after this transaction, resurrecting the deleted item. Fence sync commits against the deletion fact or serialize the operations. src/calendar/client.rs:L299: P1: The shared CalDAV client follows same-origin redirects while carrying Basic credentials; DELETE can follow a redirect with credentials. Refuse redirects for conditional DELETE rather than sending a second credentialed request. src/api/delete.rs:L379: P2: Any 2xx is treated as completed deletion, but 202 only accepts work for later and 207 can contain a failed member; these responses wrongly record applied and remove projections. Accept completed DELETE statuses only, and retain an unconfirmed state otherwise.

yuris added 1 commit 2026-09-24 07:29:34 +00:00

fix(calendar): address review findings for conditional CalDAV delete ... fa79d23ccd

Address review findings from round 1:
- Make pending deletion intents readable by crash recovery without requiring a representation field, classifying live absence as matching intent.
- Prevent concurrent sync from restoring deleted projections by fencing upsert_projection against deletion facts recorded after observation time.
- Disable credentialed redirects for conditional CalDAV DELETE using a terminal redirect policy client.
- Treat accepted-but-unfinished HTTP 202 DELETE responses as unconfirmed rather than completed.

yuris commented 2026-09-24 07:29:40 +00:00

Author

Owner

Copy link

Addressed review round 1 findings:

1. Updated recovery to classify delete intents without requiring a representation field, treating live absence as matching intent.
2. Fenced upsert_projection against concurrent sync resurrecting projections when a deletion fact was recorded at or after sync observation time.
3. Disabled redirects for conditional CalDAV DELETE using a terminal redirect policy client in CalDavClient.
4. Treated accepted-but-unfinished HTTP 202 DELETE responses as unconfirmed rather than completed.
   Added regression tests covering all four fixes in tests/delete_tests.rs; all 18 integration tests pass.

Addressed review round 1 findings: 1. Updated recovery to classify delete intents without requiring a representation field, treating live absence as matching intent. 2. Fenced upsert_projection against concurrent sync resurrecting projections when a deletion fact was recorded at or after sync observation time. 3. Disabled redirects for conditional CalDAV DELETE using a terminal redirect policy client in CalDavClient. 4. Treated accepted-but-unfinished HTTP 202 DELETE responses as unconfirmed rather than completed. Added regression tests covering all four fixes in tests/delete_tests.rs; all 18 integration tests pass.

yuris reviewed 2026-09-24 07:32:53 +00:00

yuris left a comment

Copy link

VERDICT: request-changes
P2 src/history/journal.rs:345 — Preserve subsecond ordering in the deletion fence. SQLite datetime() truncates fractional seconds, so a resource deleted and then recreated or synced with the same UID in the same second compares as though its later observation preceded the deletion. upsert_projection silently drops the live projection, and later reads of the projection stay empty. Compare timestamps at their actual precision so a deletion only fences earlier observations.
P2 src/history/journal.rs:351 — Fence stale sync observations along with their projections. The sync transactions append external baseline or change facts before calling upsert_projection. If a deletion commits between the live fetch and that sync transaction, this guard skips the stale projection but leaves a stale presence fact appended after task_deleted or event_deleted. The items absence lookup scans facts by insertion order and then reports the latest fact as not a known removal, so a deleted item loses its confirmed-removal history. Skip both the stale observation fact and projection together.
The other round-one fixes are present: recovery reads deletion intents without a representation, DELETE redirects are blocked, and HTTP 202 is unconfirmed. Focused delete tests passed (18/18), skill package tests passed (7/7), formatter and Clippy passed per the integration run. Both branch commit messages and this comment contain zero backticks.

VERDICT: request-changes P2 src/history/journal.rs:345 — Preserve subsecond ordering in the deletion fence. SQLite datetime() truncates fractional seconds, so a resource deleted and then recreated or synced with the same UID in the same second compares as though its later observation preceded the deletion. upsert_projection silently drops the live projection, and later reads of the projection stay empty. Compare timestamps at their actual precision so a deletion only fences earlier observations. P2 src/history/journal.rs:351 — Fence stale sync observations along with their projections. The sync transactions append external baseline or change facts before calling upsert_projection. If a deletion commits between the live fetch and that sync transaction, this guard skips the stale projection but leaves a stale presence fact appended after task_deleted or event_deleted. The items absence lookup scans facts by insertion order and then reports the latest fact as not a known removal, so a deleted item loses its confirmed-removal history. Skip both the stale observation fact and projection together. The other round-one fixes are present: recovery reads deletion intents without a representation, DELETE redirects are blocked, and HTTP 202 is unconfirmed. Focused delete tests passed (18/18), skill package tests passed (7/7), formatter and Clippy passed per the integration run. Both branch commit messages and this comment contain zero backticks.

yuris added 1 commit 2026-09-24 07:38:47 +00:00

fix(history): refine deletion fence subsecond precision and sync observation filtering ... 6da9c79219

Address review findings from round 2:
- Preserve subsecond timestamp precision in is_deleted_after using DateTime parse_from_rfc3339 comparison rather than SQLite datetime() truncation.
- Skip appending stale sync observation events in append_event when a deletion occurred after the observation, keeping event history consistent with projection suppression.
- Add regression tests covering subsecond ordering and stale observation filtering.

yuris commented 2026-09-24 07:38:58 +00:00

Author

Owner

Copy link

Addressed review round 2 findings:

1. Replaced SQLite datetime() with chrono DateTime subsecond comparison in is_deleted_after, preserving fractional second precision so legitimate same-second recreations are never blocked.
2. In append_event, skip appending stale sync observation facts (external_baseline_observed, external_change_observed) if a deletion occurred after the observation, keeping event history consistent with projection suppression.
3. Added regression tests in tests/delete_tests.rs covering subsecond ordering and stale observation filtering; all 19 integration tests pass.

Addressed review round 2 findings: 1. Replaced SQLite datetime() with chrono DateTime subsecond comparison in is_deleted_after, preserving fractional second precision so legitimate same-second recreations are never blocked. 2. In append_event, skip appending stale sync observation facts (external_baseline_observed, external_change_observed) if a deletion occurred after the observation, keeping event history consistent with projection suppression. 3. Added regression tests in tests/delete_tests.rs covering subsecond ordering and stale observation filtering; all 19 integration tests pass.

yuris reviewed 2026-09-24 07:41:04 +00:00

yuris left a comment

Copy link

VERDICT: approve
Round 3: both round-2 findings are resolved. The deletion fence compares parsed RFC3339 timestamps at subsecond precision, and the sync observation append skips stale presence facts in the same transaction that suppresses stale projections. Focused regression cases cover both paths. Commit messages and this review contain zero backticks. No blocking findings.

VERDICT: approve Round 3: both round-2 findings are resolved. The deletion fence compares parsed RFC3339 timestamps at subsecond precision, and the sync observation append skips stale presence facts in the same transaction that suppresses stale projections. Focused regression cases cover both paths. Commit messages and this review contain zero backticks. No blocking findings.

yuris merged commit 31629c6f8e into main 2026-09-24 07:42:30 +00:00

yuris deleted branch feat/117-caldav-delete 2026-09-24 07:42:31 +00:00

yuris referenced this pull request from a commit 2026-09-24 07:42:31 +00:00

feat(calendar): support conditional CalDAV delete for tasks and events (#121)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

yuris/life-service!121

No description provided.