v3.3.0 - #694
Draft
tobixen wants to merge 41 commits into
Draft
Conversation
This is part of a series of commits done to fix code review issues listed in docs/design/FULL_CODE_REVIEW_2026-06.md. The fixes were AI-generated; prompts were on the style "fix the code review issues". §1.3 create_event() has guarded against this since the original implementation: if 'new-0' not in created: raise JMAPMethodError(...). create_task() was missing the same check in both sync and async clients, so a JMAP server returning an empty created dict (possible when the server silently ignores the create) raised a bare KeyError instead of the documented JMAPMethodError. §4.1 jscal_to_ical: override child VEVENT with no "start" patch key got DTSTART from the master start_str instead of the occurrence's own time (the override key). Common title-only changes relocated every override to the master's first occurrence, breaking override display entirely. Default is now the override_key itself. §4.2 jscal_to_ical: EXDATE and RECURRENCE-ID values were always emitted as naive floating DATE-TIMEs. Per RFC 5545 the value type must match DTSTART. A floating EXDATE on a TZID-anchored event does not match any instance, so excluded occurrences reappeared. Override keys are now parsed with the event timezone applied (TZID events) or converted to date objects (all-day events). §4.3 _utils.py _format_local_dt(): UTC datetimes produced a Z-suffixed string. RFC 8984 §1.4 defines LocalDateTime (required for recurrenceOverrides keys and recurrenceRules.until) as YYYY-MM-DDThh:mm:ss without any suffix. Z-suffixed override keys cannot match LocalDateTime occurrence keys on strict servers. Function now always returns a timezone-stripped representation. §4.4 ical_to_jscal and jscal_to_ical: STATUS was silently dropped in both conversion directions. STATUS:CANCELLED round-tripped as status:confirmed (JSCalendar default), making cancelled meetings appear active. Added mappings CONFIRMED↔confirmed, TENTATIVE↔tentative, CANCELLED↔cancelled in both directions. §4.5 RFC 8620 §3.3: absent keys in a PatchObject preserve the server value; only explicit null entries delete a property. update_event sent the full converted JSCalendar object as the patch, so properties the caller removed (LOCATION, VALARM, DESCRIPTION, etc.) were simply absent and silently persisted on the server after the update. After converting ical_str to a JSCalendar dict, set all optional top-level properties to null when they are absent from the result. The list is maintained in caldav/jmap/convert/_patch.py and applied identically in both the sync (client.py) and async (async_client.py) update_event methods. §4.6: JMAPCalendar.search() passed datetime args through isoformat(), producing +HH:MM or bare datetimes instead of the UTCDate format (...Z) JMAP requires. Added _to_utcdate() helper in calendar.py that converts to UTC and strips microseconds. §4.7: get_objects_by_sync_token() discarded newState from CalendarEvent/changes into _, forcing callers to do a separate get_sync_token() call (race window). Now returns a 4-tuple (added, modified, deleted, new_sync_token). Updated all callers in unit and integration tests. §5.1: Moves all response-parsing logic from JMAPClient and AsyncJMAPClient into static methods on _JMAPClientBase. Each sync/async public method is now a ~3-line wrapper: get session, dispatch _request(), delegate to the shared parser. async_client.py drops from 550 → 415 lines; the parsers live in one place so future fixes (like the §1.13 create_task KeyError and §4.5 update_event nulling that this branch already carries) no longer need to be duplicated. §5.5: Hold one persistent HTTP session per client instead of per request prompt: (abridged) fix the code review issues [the commit body records the prompts as having been on that style, against docs/design/FULL_CODE_REVIEW_2026-06.md]
Recalibration of server hints, some changes of the hints and lots of additions. As of the 3.x-series, the compatibility_hints is defined to be unstable, so changes in the feature and server compliance database neither classifies as "features" nor "fixes". This commit is in tandem with lots of work done in the caldav-server-tester companion tool. The upcoming commits also adjust a little bit of logic based on the compatibility hints, and also deals with test breakages. There are different reasons for all the changes - but I've decided to squash it into a lump commit. The reasons include: * OX was "difficult" and largely ignored by both the caldav server tester and by the integration tests, we've gotten OX under the fold now but it required lots of modifications and lots of new "features" that are unsupported for OX. * There was this big legacy `old_flags` - I think I had hopes to replace it with "features" verified by the checker already before releasing caldav v2.0, but it's mostly tedious work, sometimes difficult work, and it took far too much time. Now that I have Claude assisting it seemed about time to get rid of this technical debt. * Some bugs was also found in the checker, both while working with this and while doing a code review of the checker. Some of the server hints has been realigned due to bugs that was found. * New Calendar-As-A-Service provider added - Swiss infomaniak / kSuite. The caldav interface is based on SabreDAV, but they do have quite some quirks in their setup making caldav integration difficult. New feature definitions: - save-load.stable-url - OX changes the URL for calendars (cal://0/NNN aliasing). - save-load.mutable.attendee-partstat - ref #399 - save-load.mutable.if-match-optional - OX gives 409 if an event is edited without If-Match - save-load.event.recurrences.exception.reschedule - another OX quirk, it does not allow rescheduling a recurrence series having variant recurrences (exceptions). - calendar-color, calendar-color.hex, calendar-order - this is not part of the caldav standard. The default has been set to "fragile" for the purely technical reason that testCheckCompatibility will pass if the features are not defined, no matter if the features are supported or not. - propfind.allprop.resourcetype - replaces the old propfind_allprop_failure flag. While being at it, propfind and propfind.allprop has also been defined and are verified by the check script. - propfind.displayname - we have a check to check if displayname can be set (and then read) on a calendar, now we also have a test that the displayname can be read. Apparently all servers tested supports this, so maybe it's not needed. - save.duplicate-event - replacement for the old `duplicates_not_allowed` flag. - non-existing-raises-not-found - replacement for the old `non_existing_raises_other`. Robur yields 403 instead of 404 when trying to access a calendar. TODO: this one needs a rename! - search.time-range.todo.no-dtstart - replaces the old vtodo_datesearch_nodtstart_task_is_skipped flag. Reported to be unsupported for synlogy and davical, "fragile" for stalwart. - well-known (default unknown) - I decided to add a check on weather the server supports .well-known forwarding from a domain to an URL. This feature typically lives outside the server software itself, it has to be tested on real production setups (like "my xandikos server" or "the calendar-as-a-service provider Infomaniak"), so "unknown" is a sane default. - write-delay (type server-peculiarity) - the server processes writes asynchronously (PUT/DELETE/MKCALENDAR/...), so a client must wait after every write before it's possible to search for content, GET content or even add items to a calendar. This problem may go unnoticed for ordinary clients, but for tests and the server checker that save data and query the same data right afterwards it's certainly needed to sleep between the operations. (There is also the related search-cache - but this is broader) - added some missing explicit defaults The derivation-logic has been kind of a head-ache for a long time. If foo is not supported, then foo.bar.zoo should implicitly be considered not supported. I thought it would make sense the other way around as well, if foo.bar.zoo and foo.bar.moo is unsupported, then foo.bar should be considered to be unsupported. This fell apart a bit - the current rule is that the reverse only applies if foo.bar is a "virtual" grouping node. If foo.bar is an independent feature which is checked by the checker, then the children should not matter. I've decided to tag features as independent by giving them an explicit default value (TODO: does that make sense?) There has been bugs and issues all since I started on the derivation logic, hopefully this commits fix the derivation in stone once and for all. Per-server recalibration - some of it due to new features appearing, some due to changed behaviour in the most recent versions, and some due to bugs found and fixed in the caldav-server-tester: - search.comp-type.optional -> full on nextcloud, cyrus, zimbra, bedework, baikal, davical, davis, ccs, ox (the old ungraceful/fragile readings were a checker bug where the probe carried a time-range SabreDAV rejects); sogo unsupported - time-range comp-type-optional: full on xandikos, radicale, davical, zimbra; fragile on bedework; full on stalwart (+ text.comp-type-optional) - create-calendar.set-displayname: full on both ox and zimbra; the new create-calendar.stable-url is unsupported on both (the display name sticks, but the server assigns a different canonical URL - Zimbra a display-name-derived path, OX an opaque cal://0/NNN - which the library adopts after create) - ox: save-load.stable-url / mutable.if-match-optional / mutable.attendee-partstat unsupported; search.text full; search.comp-type and search.is-not-defined unsupported; time-range.open.start.duration full; granular recurrence-search hints; exception.reschedule unsupported - ox/ccs: granular search.recurrences.* hints + search.time-range.todo.strict broken - stalwart: search.recurrences.expanded.exception fragile (SEQUENCE-dependent) - ccs/sogo: dropped stale negatives now passing with near-future fixtures (time-range old-dates/open, freebusy-query) - new infomaniak server profile (SabreDAV 4.3.1 / kSuite) Some related issues: #681 #684 #399 Prompts used (may be incomplete, particularly as some of the work was done from the caldav-server-tester project - sorry that it has become an incomprehensible mess): * `pytest -k 'compat and xandikos'` fails, radicale fails similarly. I find it a bit weird if search.comp-type.optional is ungraceful while search.time-range.comp-type.optional is supported. Please investigate. (pasted error logs) * Some servers does not support comp-type.optional, but do support time-range.comp-type.optional - that sounds odd, please verify that it's actually true. * (pasted test error logs) * create-calendar.set-displayname is now unsupported for NextCloud, but expected to work. Please investigate * (pointing out broken tests that ought to have been fixed already) * git bisect run pytest -k 'testLookupEvent and zimbra' shows things broke on commit (...) please investigate * We need a better check under ~/caldav-server-tester and possibly new feature flag(s) to fully describe the zimbra-behaviour, that would be better than just marking it fragile * save-load.put-overwrite should perhaps be moved under the save-load.mutable umbrella? * (...) this is weird. As long as save-load has an explicit default of full, it should by default be considered supported even if all the children are unsupported. I just added a unit test for this, and it passes. * the information you just saved as a the memory note should be taken care of by inline comments and strings in the compatibility matrix and tests * We still have some test-failures on Stalwart. Hypothesis is that the "rolling window"-behaviour of Stalwart (ignore all events that are far in the past or far in the future doing searches) causes this, and that the tests are built with hard-coded DTSTART, DTEND, DUE etc. Please investigate. Here is the test output: (...) * pytest --last-failure [shows OX testCheckCompatibility search.comp-type broken] - is the comp-filter broken or just unsupported? * I'd like to get rid of the "old_flags" in compatibility_hints.py. Everything that is not used in tests can be just removed. The rest needs proper tests in ~/caldav-server-tester and some rewriting in the tests. * ungraceful should be used when the server raises an error. It should be broken if it returns something unexpected. * "ungraceful" is not a breach of the RFC, but we cannot have ungraceful as default * blue -> #CEE7FFFF should yield "supported" with a behaviour note; make an explicit hex probe too; fragile default; read-only stays broken. * I'm not sure if I agree with the latest work. The feature added is propfind.allprop.resourcetype and it's tested. This leaves propfind and propfind.allprop as "grouping nodes" which will be automatically deemed non-supported if propfind.allprop.resourcetype is not supported. I think the namespace is correct, but we may need separate tests on propfind.allprop and propfind, otherwise it will appear like propfind is not supported at all for cases where propfind.allprop.resourcetype is unsupported. Following the convention, tested features should have an explicit default in the compatibility_hints.py definitions. * continue [with removing the old flags] * robur is down so we cannot test it now * We do have a create-calendar.set-displayname feature in ~/caldav/caldav/compatibility_hints.py, but nothing to check if we can get the displayname from a calendar. Please fix this check. * I don't like the name. Other suggestions? Maybe propfind.get-calendar-displayname? (...) Ok, let it be propfind.displayname * Check test issues and github code review comments in the PR * See docs/design/FULL_CODE_REVIEW_2026_06.md - work on 5.8. Remember to mark task as finished in the code review and to commit. * Is 6.1 (from the code review) still relevant, or was it already removed during one of the deduplication sessions? * I'd like a new section for infomaniak in the ~/caldav/caldav/compatibility_hints.py file as soon as the tests passes * Rerun the server compatibility checks, it may come up with different results now (due to the infomaniac write-delay). apply all updates now Co-Authored-By: Claude Sonnet 4.6 and Opus 4.8 <noreply@anthropic.com>
As noted in the previous commit, I fell into a rabbit hole when doing some modifications to caldav-server-tester and test framework, partly initiated by #681 (where it was found that my search logic was not in accordance with the RFC). While working on it I decided to also allow servers like OX and CCS to be tested properly (they had problems with the static data set used in the testing) and to clean up legacy "compatibility flags" once and for all. There was also problems with a service provider reported in #684. Tons of small commits were made tweaking and tuning the compatibility matrix, with related changes in test code and code logics. (Claude Opus terms it the "compatibility-matrix campaign"). To get a better overview, I decided to rebase and squash them all together, and then split it into three parts - the previous commit is with the compatibility matrix changes, this commit is with the changes done to the tests and test framework, and the next will be with the actual changes in server logics. (Arguably, this split is silly as it produces commits where the tests won't pass, and possibly the previous commit also breaks workarounds in the search). Here is the list of changes in this commit: * tests/test_search.py — new mocked unit tests for the comp-type-less search (ref #681). * tests/test_compatibility_hints.py — I failed utterly on getting the derivation logic right (if search is unsupported, assume all search.*.* is unsupported. If all children of search is unsupported, assume search is unsupported. But then again, if search.comp-type.optional is unsupported it doesn't mean that search.comp-type is unsupported - in a brief moment of insanity I thought the special case here was that the parent only had one child - but the special case is that search.comp-type is an independent testable feature, while "search" is just a group of sub-features) had become a lot more complex and buggy than what I ever anticipated it to be. The unit test has been hardened up and refactored. * Integration tests: * Static dates in the test data has been replaced with dynamically generated "near" dates, making the test data visible for OX and CCS. * Investigation of the Calendar-as-a-Service-offering in the Infomaniak kSuite caused the need to implement support for a "write-delay peculiarity" for servers saving data asynchronously. All test code will sleep 15s after every write to ensure data can be fetched in the next read. (found while investigating #684) * comp-type-less search integration tests (ref issue #681) * probe/gate the new features: save-load.stable-url, save-load.mutable.attendee-partstat, put-overwrite via save-load.mutable + if-match-optional, create-calendar.set-displayname, create-calendar.stable-url, calendar-color/-order, non-existing-raises-not-found, save.duplicate-event, save-load.event.recurrences.exception.reschedule, search.time-range.todo.no-dtstart, propfind.allprop.resourcetype, search.text.category, search.time-range.event, migrate the old_flags assertions to is_supported() checks * make display-name fixtures idempotent for wipe-calendar + unique-name servers (SOGo) The work was mostly done by AI-generation, which is deemed OK for test code. prompt: (summary, not verbatim) Asked to do replace hard-coded old dates with dynamic date in the near future on all objects, making it possible to test this with CCS and OX. Asked to remove the old flags, had various discussions and opinions on how it's best to organize the features, and asked Claude to look into test failures, Co-Authored-By: Claude Opus 4.8 and Sonnet 4.6 <noreply@anthropic.com>
The earlier commit squashing together all the recent compatibility matrix changes introduced two new features "search.time-range.comp-type-optional" and "search.text.comp-type-optional". The old "search.comp-type.optional" is still kept, but only for searches that does not involve property filtering. The RFC comes with examples, all of them including the component type filter, however it was always my understanding that when sending a search query to the server without a component type filter, the server should return both VEVENT, VTODO and VJOURNAL. Now I've learned that at least for searches that includes filtering by text property or by date ranges the component type filter is actually mandatory according to the RFC. This means that the two new features should be set only for servers that (deliberately or not) misunderstand the searching logic in the RFC. Hence the default for the two new features are set to false. The purpose of this commit is to resolve #681 and ensure search works no matter if the caller has explicitly been setting the component type filter or not. This means that unless the server is explicitly configured with those features, the client will now send three requests instead of one to make sure everything relevant is found. **If you only need events, specifying `events=True` in the search parameters is best practice - both before and after this commit** Since the caldav-server-tester needs to do more "raw" queries, a parameter `compatibility_workarounds` can be set to False to deactivate this and other compatibility workarounds. This commit was predominantly AI-generated, but thoroughly reviewed by the maintainer. prompt: (summary of a long discussion, not a verbatim prompt) discussions with Claude on how to solve #681, what to do with the comp-type thing, how to name the new features, and why tests are failing Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>
Found and fixed some hallucinations in test documentation. prompt: There has been a hallucination that environmental variables like TEST_STALWART=true are needed for running tests towards Stalwart, and same for the other test servers. ... Please look through and clean up. followup-prompt: Please delete the stale file and fix all references, then commit. (the prompt causing the nextcloud username/password to be fixed is lost - but probably it was something like "fix it" after Claude unsuccessfully tried to connect to the server using the wrong credentials) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A few servers assign a freshly-created calendar a canonical URL that differs from the one derived from the requested cal_id: Zimbra relocates the collection to a display-name-derived path when a display name is supplied at creation, and OX always exposes an opaque cal://0/NNN URL. Both servers do have some alias redirection so the expected calendar URL will still work - however, for Zimbra the objects in the collection will give 404 unless the new calendar URL is given. For servers where create-calendar.stable-url is unsupported, the library now discovers and adjusts the server's canonical URL after creation (_adopt_canonical_url, sync + async) by looking the calendar up by the display name it was created with. This retires the old "omit the display name" workaround: users keep their calendar name AND every later URL-based operation resolves - identical handling for Zimbra (display-name-derived path) and OX (opaque cal://0/NNN), with no per-server branching. This is part of a longer working session on compatibility problems, and there has been quite much forth and back on the compatibility definitions and matrix. To get an overview of the *net* changes in the compatibility matrix, all changes in compatibility_hints.py has been split out from this and other commits and squashed into one commit. Similarly, there is one commit for all the changes done to the tests while working on compatibility. prompt: (lots of forth and back on this one - asking for more research to be done and finally for canonical calendar URL discovery) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Config-file sections without a `caldav_url` should be accepted for well-known service providers. Claude Fable labelled this as a feature - but this feature has been intended for a long while and even exercised by the test code, all that was needed was to edit a code line returning None on missing URL, so I'm redefining it as a `fix`. prompt: I have this configuration in ~/.config/calendar.conf: [ecloud section with redacted caldav_pass, caldav_user, features but no URL]. The URL should not be needed - the caldav library should find it based on the ecloud configuration in compatibility_hints.py. This seems to work when running tests in the caldav library. However, without the URL I get this error: "No server specified" [from caldav-server-tester]. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This is part of a series of commits done to fix code review issues. This addresses 2.1, 2.2, 2.3 and 2.4 in the code review document. The fixes were AI-generated. Prompts were on the style "fix the code review issues". All changes have been carefully reviewed. The code covered in this commit is all about handling incorrect icalendar data. We've been discussing moving this to the icalendar library, but so far we haven't nailed the details - it's easy to do some quick regexp'ing to fix up a handful of observed real-world problems, it's hard to do it in a correct and general way. Changes: * COMPLETED date fixup corrupted the following iCal property line * ical_fragment injected into VALARM instead of VEVENT/VTODO/VJOURNAL * trailing-whitespace fix in vcal.fix() was dead code — add re.MULTILINE * backslash-unescape regex in vcal.fix() was a no-op for lone quotes One could argue that all this code could probably just be deleted - those bugs are serious, still they've been undiscovered for quite some time. It may be because all testing I've done have been towards server versions where those issues are resolved. More likely, the problems may also come from various clients, my integration tests do not involve foreign clients. Also, earlier we used vobject internally, it tends to crash when it's observing invalid data, without vobject in the mix those errors are less visible. Anyway, all issues have been observed in the real world, so there is a use-case for it. prompt: (abridged) fix the code review issues [the commit body records the prompts as having been on that style; §2.1-2.4 of docs/design/FULL_CODE_REVIEW_2026-06.md] Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>
This is part of a series of commits done to fix code review issues. This addresses 2.1, 2.2, 2.3 and 2.4 in the code review document. The fixes were AI-generated. Prompts were on the style "fix the code review issues". All changes have been carefully reviewed. The rest of this commit message is mostly AI-generated.
§1.1 Iterating a CaseInsensitiveDict (niquests) or httpx.Headers yields key
strings, not (name, value) tuples. So `x[0]` was the first character of
each header name, never "location" — the list comprehension was always
empty and any 302 from a server raised IndexError.
§1.6 add_attendee UnboundLocalError on uppercase MAILTO: scheme
RFC 3986 §3.1 says URI schemes are case-insensitive; the startswith("mailto:")
check was case-sensitive, so "MAILTO:user@example.com" fell through all string
branches leaving attendee_obj unassigned.
§1.7 change_attendee_status bare KeyError + literal %s in error message
ical_obj["attendee"] raises KeyError when no ATTENDEE property exists; the
NotFoundError-catching dispatch loop could not catch it. Also fixed the
not-found message which contained an unsubstituted %s placeholder.
§1.8 lib/auth.py IndexError on WWW-Authenticate with trailing comma
A header ending in "," (seen in the wild) made h.split()[0] raise IndexError
on the empty segment. Added if h.strip() guard.
§1.9 config.py expand_config_section KeyError on absent section name
config[section] raised KeyError when section was not in the config dict,
crashing caldav.get_calendars() with KeyError: 'default' on configs that
have no default section.
§2.13 base_client.py: calendar with empty displayname dropped from results
The truthiness check on get_display_name()'s return value treated "" as
falsy. Async already used is not None; sync is now consistent.
§2.16 async_davclient.py: HTML-on-401 hint checked self.headers instead of r.headers
The diagnostic "HTML was returned, consider setting auth_type" hint read
self.headers (client request headers) instead of r.headers (server response).
§2.17 config.py: disable:true ignored for sections fetched by explicit name
expand_config_section used the literal string "section" as config key
instead of the section variable, so disable was only effective under "*".
prompt: (continue with fix-soon items from docs/design/FULL_CODE_REVIEW_2026-06.md)
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>
This is part of a series of commits done to fix code review issues. This addresses 2.5 in the code review document. The fixes were AI-generated. Prompts were on the style "fix the code review issues". All changes have been carefully reviewed. The rest of the commit message is AI-generated.
Two related bugs in canonical():
(a) arr was built from self.url_parsed whose netloc still contains
user:pass@, so the returned URL retained credentials. This caused
__eq__/__hash__ comparisons between an authenticated client URL and a
credential-free server href to return False, breaking URL matching.
(b) unauth() returns self when there are no credentials; canonical() then
overwrote url_raw/url_parsed in place. A bare == or hash() call
silently mutated the URL object, potentially re-encoding '+' to '%2B'
and directing subsequent requests to the wrong resource.
Fix: use url.url_parsed (the auth-stripped form) for arr, and always
return URL(urlunparse(arr)) — a fresh object — instead of mutating url.
prompt: "continue with §2.5" (from docs/design/FULL_CODE_REVIEW_2026-06.md)
Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>
This is part of a series of commits done to fix code review issues
listed in docs/design/FULL_CODE_REVIEW_2026-06.md. The fixes were AI-generated; prompts were on the
style "fix the code review issues". All changes have been carefully
reviewed. This commit handles three small fixes to caldav/search.py.
§2.6 combined-is-logical-and workaround silently dropped property filters
The workaround for servers where time-range and property filters cannot be
combined (search.combined-is-logical-and: unsupported, e.g. Nextcloud)
strips all property filters from the server query and is supposed to apply
them client-side. It passed the ambient post_filter (None) to filter()
instead of True. _filter_search_results short-circuits when post_filter is
falsy, so a search with a time range + SUMMARY/LOCATION/etc filter returned
every object in the time range unfiltered. The sibling workarounds already
forced post_filter=True; this branch now does the same.
§2.7 undef operator missed the category→CATEGORIES alias
The undef branch emitted PropFilter("CATEGORY"), a nonexistent property, so
is-not-defined matched every object. Applied the same alias the non-undef
branch already uses.
§2.8 operator='==' exact-match guarantee was never enforced
post_filter was only set True for 'in' operators, so the server's substring
semantics leaked through ('==' 'rain' matched "Training").
icalendar_searcher.check_component() already handles '==' as exact-match, so
the fix is adding '==' to the post_filter trigger condition.
prompt: Please reorganize so that the fixes for 2.6, 2.7 and 2.8 in the code
review (three small fixes to search.py, with a bit bigger test code) are in
one commit
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>
This is part of a series of commits done to fix code review issues. The fixes were AI-generated. Prompts were on the style "fix the code review issues", referencing the file docs/design/FULL_CODE_REVIEW_2026-06.md. All changes have been carefully reviewed. §2.9 — calendarobjectresource._set_data() raw-string branch: Cleared _data/_vobject_instance/_icalendar_instance but never reset self._state. Once _state was populated (e.g. by event.id or is_loaded()), _ensure_state() returned the stale state on all subsequent reads. After load() fetched new data, get_data(), get_icalendar_instance(), and .id all served the pre-reload content. Fix: add self._state = RawDataState(self._data) in that branch. §2.10 — datastate.RawDataState.get_component_type(): Tested for 'BEGIN:FREEBUSY' but real iCalendar data uses 'BEGIN:VFREEBUSY', so FreeBusy objects always got component_type=None: is_loaded()/has_component() returned False, save() silently no-oped, and load(only_if_unloaded=True) reloaded spuriously every call. Same typo in the base-class fallback parsers (comp.name 'FREEBUSY' vs the library's actual 'VFREEBUSY'). Fix: s/FREEBUSY/VFREEBUSY/ in all three places in datastate.py. §2.11 _get_duration: isinstance check on vDDDTypes wrapper, not .dt isinstance(i["DTSTART"], datetime) tested the wrapper object (never a datetime), so a timed DTSTART with no DUE/DURATION returned 1 day instead of 0, shifting the next-occurrence due date by one day after completing a recurring task. §2.12 _complete_recurring_safe: completion_timestamp not forwarded to complete() The sync path called completed.complete() with no timestamp (defaults to now). The async twin already passed the timestamp through via _complete_ical(). §2.18 get_connection_params: explicit kwargs dropped when url/features absent Explicit params (e.g. password='secret') were only respected when url or features was also given. When an env or config-file source won, explicit params were silently discarded. Now merged on top of the winning source. §2.19 resolve_features and testing.py: module-level hint dicts mutated resolve_features(str) returned the module-level dict directly (no copy); XandikosServer/RadicaleServer used shallow .copy() then mutated a nested key. Both contaminated the module-level dict for the process lifetime. Fixed: use copy.deepcopy() in all three sites. prompt: (abridged) fix the code review issues [the commit body records the prompts as having been on that style, against docs/design/FULL_CODE_REVIEW_2026-06.md] Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>
**Note**: This is not the *correct* fix - see #687 for details This is part of a series of commits done to fix code review issues. This addresses 3.1 in the code review document. The fixes were AI-generated. Prompts were on the style "fix the code review issues", referencing the file docs/design/FULL_CODE_REVIEW_2026-06.md. All changes have been carefully reviewed. _well_known_lookup never received require_tls. A same-domain redirect to http:// passed the _is_subdomain_or_same domain check and was returned as ServiceInfo(tls=False). discover_service returned it unchecked, so a misconfigured or MITM server could silently downgrade the connection to plaintext despite the documented guarantee that require_tls=True (the default) "ONLY accepts TLS connections". Fix: after _well_known_lookup returns, check well_known_info.tls against require_tls and emit a warning + return None on mismatch. prompt: (abridged) fix the code review issues [the commit body records the prompts as having been on that style, against docs/design/FULL_CODE_REVIEW_2026-06.md] Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>
This is part of a series of commits done to fix code review issues listed in docs/design/FULL_CODE_REVIEW_2026-06.md. The fixes were AI-generated; prompts were on the style "fix the code review issues". All changes have been carefully reviewed. etree.XMLParser was called without resolve_entities=False or no_network=True, leaving entity expansion and DTD network fetches enabled by default. A malicious or MITM server could inject arbitrary text into parsed property values via inline DOCTYPE entity definitions. Add resolve_entities=False and no_network=True. dtd_validation=False is lxml's default and not needed explicitly. prompt: (abridged) fix the code review issues [the commit body records the prompts as having been on that style, against docs/design/FULL_CODE_REVIEW_2026-06.md] Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>
This is part of a series of commits done to fix code review issues listed in docs/design/FULL_CODE_REVIEW_2026-06.md. The fixes were AI-generated; prompts were on the style "fix the code review issues". All changes have been carefully reviewed. §1.10 compatibility_hints.py copyFeatureSet AssertionError on string override The 'support' not in server_node guard prevented a string-valued feature from being overridden if the key already existed, falling through to else: raise AssertionError. Removed the guard — string values always overwrite. §1.11 compatibility_hints.py copyFeatureSet stores unknown feature after warning A typo'd feature name emitted UserWarning but was still stored; a later collapse()/is_supported() hit a message-less AssertionError far from the config. Added continue after the warning so unknown keys are never stored. §1.12 lib/vcal.py bare assert on truncated server-supplied data Truncated iCalendar without an END: line triggered a bare assert, giving no useful message and silently passing under python -O. Now logs a warning and returns the data unchanged instead of crashing. prompt: (abridged) fix the code review issues [the commit body records the prompts as having been on that style; §1.10-1.12 of docs/design/FULL_CODE_REVIEW_2026-06.md] Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>
This is part of a series of commits done to fix code review issues listed in docs/design/FULL_CODE_REVIEW_2026-06.md. The fixes were AI-generated; prompts were on the style "fix the code review issues". All changes have been carefully reviewed. §3.3 of the June 2026 code review (docs/design/FULL_CODE_REVIEW_2026-06.md): when PYTHON_CALDAV_COMMDUMP is set, request/response bodies and headers — potentially including personal data and custom auth headers — are written to uniquely-named files under /tmp that accumulate indefinitely. Emit a logging.warning() at import time so the operator is reminded of the exposure. prompt: (abridged) fix the code review issues [the commit body records the prompts as having been on that style, against docs/design/FULL_CODE_REVIEW_2026-06.md] Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>
This is part of a series of commits done to fix code review issues. The bullet points refer to docs/design/FULL_CODE_REVIEW_2026-06.md. The fixes were AI-generated; prompts were on the style "fix the code review issues". All changes have been carefully reviewed. §1.2 DAVClient: URL with user but no password crashed + wrong credential precedence unquote(self.url.password) raised TypeError when URL has user@ but no :password. Also: URL credentials silently overrode explicit kwargs (async was the opposite). Fix: guard the password unquote; only use URL creds when kwargs are absent. §1.3 rate-limit retry: None + float TypeError on second 429 with no Retry-After sleep_seconds += rate_limit_time_slept / 2 ran before the sleep_seconds is None check, so None += 2.5 raised TypeError instead of RateLimitError. Same bug copy-pasted in both sync and async clients. §1.4 async aio.get_calendars(calendar_name=...) never found any calendars The name-based lookup called await principal.calendar(name=cal_name), but Principal.calendar(name=...) is not async-aware: it calls get_calendars() synchronously, which for an async client returns a coroutine. Iterating the coroutine (not awaiting it) silently produced no matches, so every calendar_name lookup returned nothing. Fix: fetch all calendars with await principal.get_calendars() and filter by display-name directly, bypassing the non-async principal.calendar() path. §1.5 collection.py freebusy_request: async path called add_attendee() before dispatching to _async_freebusy_request(), so Principal.get_vcal_address() returned a coroutine instead of a vCalAddress — add_attendee then crashed on attendee_obj.params[...]. Moved attendee loop into _async_freebusy_request and added `await attendee.get_vcal_address()` for Principal objects. The integration tests missed this because both the sync and async freebusy tests only ever passed a pre-resolved vCalAddress as the attendee, never a Principal object — so the Principal branch (the one that returned an un-awaited coroutine) was never exercised. The async test_freebusy now also calls freebusy_request() with a Principal attendee directly to cover it. §2.14 async get_calendars lacks GMX principal-URL fallback Sync client falls back to principal URL when calendar-home-set is absent; async returned [] immediately. Parity restored. §2.15 async_davclient.py: issue-#158 connection-abort workaround sent a probe GET to detect the auth challenge. If the probe returned anything other than 401+WWW-Authenticate the code fell through to `response = DAVResponse(probe_r, self)`, silently returning the probe's response instead of the original request's result or error. Now the original exception is re-raised when the probe does not yield a proper auth challenge. (§2.6/§2.7/§2.8 search.py fixes moved to the consolidated search.py code-review fix commit.) prompt: (abridged) fix the code review issues [the commit body records the prompts as having been on that style, against docs/design/FULL_CODE_REVIEW_2026-06.md] Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>
This is part of a series of commits done to fix code review issues. This commit is a mix of hand-edits and AI-generated edits. The PYTHON_CALDAV_COMMDUMP feature carried a security analysis when it was introduced (v1.4.0), but **only in the CHANGELOG**, and old CHANGELOG entries are pruned. Added to SECURITY.md alongside the new v3.4 import-time warning that was added in the §3.3 fix. prompt: please also find the original SECURITY-notes from an old version of the CHANGELOG and add it to ~/caldav/SECURITY.md prompt: Consider the file SECURITY.md. Please look through it and see if the text is good or not. Anything else that is relevant to mention? Anything clearly irrelevant that should be removed? followup-prompt: I've done some edits to the file. Please correct both old and new typo mistakes and add the missing details. prompt: Point 3.3 in the review document has not been marked as FIXED? Co-Authored-By: Claude Sonnet 4.6 and Claude Opus 4.8 <noreply@anthropic.com>
CoPilot review flagged bare `except: pass` blocks as bad practice. Rather than just adding a comment, the fix checks the server's compatibility feature matrix to decide whether the exception is expected: - Add `_warn_unreadable_display_name()` helper in `base_client.py` (shared by sync and async paths) that silently skips only when the server is known not to support `propfind.displayname`; otherwise emits a log.warning so unexpected failures are visible. - Wire the helper into the name-matching loop in `CalendarSet.calendar()` (`collection.py`) and `get_calendars()` (`async_davclient.py`), replacing the bare `except Exception: pass` blocks. - Add comment to the `PropsetError` swallow in the integration test (`test_caldav.py`) explaining why that one is intentionally silent (best-effort cleanup after an assertion has already passed). - `config.py`: simplify `return explicit_conn or None` → `return None` with a comment explaining the reasoning (no connectable client possible at that point). - Add `TestWarnUnreadableDisplayName` unit tests covering all four branches of the helper (feature supported, feature explicitly unsupported, parent propfind unsupported, no feature matrix). prompt: See docs/design/FULL_CODE_REVIEW_2026_06.md - work on 5.1 prompt: "There is now uncommitted work in the repository, dealing with code review comments from CoPilot, it didn't like empty except-blocks - so the except-block is now checking the compatibility hint on weather the exception is expected or not, adds comments on why it's expected, and logs warnings if it's unexpected." Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This is part of a series of commits done to fix code review issues. Paragraphs refers to docs/design/FULL_CODE_REVIEW_2026-06.md. The fixes were AI-generated; prompts were on the style "fix the code review issues". All changes have been carefully reviewed. Replace the two per-object LOAD_OBJECT loops in _search_impl with a single LOAD_OBJECTS_BATCH action that delegates to Calendar._batch_load_objects(). Before this fix, a search returning N unloaded objects triggered N individual GET requests; after, one batched calendar-multiget REPORT is issued instead. Add Calendar._batch_load_objects() and _async_batch_load_objects() to collection.py. Both try _multiget/_async_multiget first; on failure they fall back to per-object load() calls so error handling is preserved. The shared post-processing (URL-indexing the multiget results and assigning obj.data) is extracted into _assign_multiget_data(), so the only remaining sync/async difference is the irreducible await on the multiget REPORT and the fallback load(). Add SearchAction.LOAD_OBJECTS_BATCH enum member and handle it in both the sync search() and async async_search() drivers. Closes code-review item §5.4 from docs/design/FULL_CODE_REVIEW_2026-06.md. prompt: (abridged) fix the code review issues [the commit body records the prompts as having been on that style, against docs/design/FULL_CODE_REVIEW_2026-06.md] Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>
This is part of a series of commits done to fix code review issues. Paragraphs refers to docs/design/FULL_CODE_REVIEW_2026-06.md. The fixes were AI-generated; prompts were on the style "fix the code review issues". All changes have been carefully reviewed. §5.8 of FULL_CODE_REVIEW_2026-06: search() and async_search() each carried a ~40-line driver loop that interleaved Phase 1 (execute the yielded SearchAction) with Phase 2 (the gen.throw/gen.send/StopIteration exception-rethrow protocol). The two copies were byte-identical except for `await`, making the subtle Phase-2 protocol a drift hazard. Extract the Phase-2 protocol into a single module-level _advance_search_gen() shared by both drivers, and move Phase-1 dispatch into paired _dispatch_search_action / _async_dispatch_search_action methods. Each driver loop is now a thin prime-then-while; only the irreducible `await` and the per-action one-liners remain duplicated. No behaviour change. prompt: (abridged) fix the code review issues [the commit body records the prompts as having been on that style, against docs/design/FULL_CODE_REVIEW_2026-06.md] Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>
…lients Addresses §5.3 of docs/design/FULL_CODE_REVIEW_2026-06.md. The sync (DAVClient) and async (AsyncDAVClient) clients duplicated three blocks that were byte-identical apart from time.sleep vs asyncio.sleep and await — the same duplication that produced the §1.3 retry-loop bug and the §2.14 GMX get_calendars gap. The pure logic now lives once in BaseDAVClient: - _init_rate_limit_config() — the rate-limit __init__ tail - _rate_limit_sleep_seconds() — the retry sleep-decision (where §1.3 lived) - _calendar_home_url() / _build_calendars_from_propfind() — the get_calendars post-processing (where §2.14 lived) Only the irreducible per-twin parts remain duplicated: the actual time.sleep vs await asyncio.sleep, the awaited PROPFIND/principal I/O, and the library-specific session/header setup in __init__. Keeping the sleep call in each module preserves the existing unit-test monkeypatch points (caldav.davclient.time.sleep / caldav.async_davclient.asyncio.sleep). Behaviour unchanged; existing sync + async rate-limit and client unit tests pass. prompt: See docs/design/FULL_CODE_REVIEW_2026_06.md - work on 5.3. Remember to mark task as finished in the code review and to commit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This is part of a series of commits done to fix code review issues
listed in docs/design/FULL_CODE_REVIEW_2026-06.md. The fixes were
AI-generated; prompts were on the style "fix the code review issues".
All changes have been carefully reviewed.
Addresses §5.2 and §5.6 — two sync/async duplication cleanups in calendarobjectresource.py.
§5.2 _post_put + _update_tag_props:
The _post_put block was pasted twice in sequence; the second copy
(302/status/Etag handling) was dead code, since the first copy either
raises, returns a retry coroutine, or falls through with status in
{302, 201, 204} — making the second `elif r.status not in (204, 201)`
branch unreachable. Removed the dead second copy, and extracted the
Etag/Schedule-Tag header->props capture (repeated in _post_put, load and
_async_load) into a shared _update_tag_props() helper, removing the
"consider refactoring - this is repeated many places now" comment.
Added characterization unit tests (Etag capture from PUT, 302->URL update)
in tests/test_schedule_tag.py.
§5.6 recurring-task completion twins:
Todo._async_complete_recurring_thisandfuture copied ~60 lines of its sync
twin verbatim, and the async "safe" variant had drifted into PUTting the
standalone completed copy twice (saved as still-pending, then again as
completed). Extracted the pure (no-I/O) icalendar mutation into
_prepare_recurring_thisandfuture() and _build_recurring_safe_completed(),
shared by both sync and async wrappers; each wrapper now only does the
await-able save(s). The completed copy is finished in memory and PUT once
per object. Added offline unit tests (TestRecurringCompleteHelpers)
covering the mutation result and the single-PUT invariant.
prompt: (abridged) fix the code review issues [the commit body records
the prompts as having been on that style, against
docs/design/FULL_CODE_REVIEW_2026-06.md]
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>
This is part of a series of commits done to fix code review issues listed in docs/design/FULL_CODE_REVIEW_2026-06.md. The fixes were AI-generated; prompts were on the style "fix the code review issues". All changes have been carefully reviewed. Two sync/async duplication cleanups around multistatus REPORT handling. §5.7 share propstat-collection between the two multistatus stacks (response.py): response.py had two parallel multistatus-parsing stacks (legacy _find_objects_and_props/expand_simple_props vs the dataclass parsers). The named quirks were already shared — Confluence %2540 / absolute-URL normalization in _normalize_href, purelymail and stalwart 404 shapes in _parse_response. The one genuinely-duplicated piece left was the propstat iteration plus the "404 propstat means the property is absent" skip, implemented twice. Extract that into a single _collect_prop_elements() helper used by both _extract_properties (dataclass stack) and _find_objects_and_props (legacy stack). The legacy path drops its over-strict per-propstat asserts that the dataclass path never had, so both stacks now treat odd-shaped propstats identically. The two value-conversion APIs (_element_to_value vs _expand_simple_prop) are intentionally left as-is. TestParserStackEquivalence guards that both stacks agree on the 404-skip. collapse the _multiget sync/async twins (collection.py): The _multiget (sync) and _async_multiget (async) twins were byte-identical apart from `await` and sync-being-a-generator vs async-returning-a-list (both even carried "mirror any changes there" warnings). Extract the two pure (no-I/O) halves into _build_multiget_root() (builds the calendar-multiget REPORT body) and _extract_multiget_results() (applies the raise_notfound 404 check and returns (href, data) tuples). Each twin is now just "(await) self._query(...)" around the shared helpers. _multiget now returns a list instead of a generator so both twins return the same type; all three call sites only iterate the result and the empty-result case still raises NotFoundError, so behaviour is preserved — verified against Xandikos (sync + async lookup/load/multiget/search) and the offline unit suites. prompt: (abridged) fix the code review issues [the commit body records the prompts as having been on that style, against docs/design/FULL_CODE_REVIEW_2026-06.md] Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>
The get_objects_by_sync_token sync body and its _async twin were ~80
near-identical lines each (both carried a "lots of code duplication here"
TODO), differing only in the awaited round-trips.
Extract the three pure (no-I/O) halves into shared helpers:
* _should_use_sync_token() — the "real sync-collection vs fallback"
decision (incl. the disable_fallback ReportError);
* _apply_fallback_etags() — mapping ETags from the depth-1 PROPFIND
response onto the object list;
* _build_fallback_sync_result() — the fake-token emulation tail.
Each twin now keeps only the interleaved awaitable I/O
(_request_report_build_resultlist, the obj.load() loops, search(),
_query_properties); everything else is shared. Behaviour preserved —
verified against Xandikos (sync + async sync-token round-trips) and the
offline test_sync_token_fallback / unit suites.
prompt: continue. The sync and async code should largely be the same and
all common logic should be collapsed.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
… fingerprints
_parse_response hardcoded Stalwart's "No resources found" responsedescription
text and purelymail's {https://purelymail.com}does-not-exist error child to
recognise 404s in multistatus replies. Both <error> and <responsedescription>
are optional, server-defined children of <response> per RFC 4918, so the
content assertions were never warranted and adding the next server's 404 shape
meant editing the generic parser.
Accept either element generically. A genuinely novel tag still hits
error.weirdness(). The check_404 debug guard is now None-safe since a server
may legally send these elements without a response-level status.
Addresses §6.1 of docs/design/FULL_CODE_REVIEW_2026-06.md.
prompt: (abridged, typos corrected) for point 6.1 in the code review document, perhaps the solution is to
fix so that any <error> or <responsedescription> is accepted, without any
hardcoded server fingerprints (though this will fail once a third server throws
in a <errortext> or something like that)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds the [Unreleased] CHANGELOG section covering the v3.3.0 work. Also reworks the narrative part of docs/source/http-libraries.rst: how niquests arrived and why it was accepted, that requests 2.x is still under maintenance even though 3.0 never materialised, and that niquests and httpx both cover sync as well as async. Moves the "if you have strong personal opinions against niquests" bullet to the top of the recommendations, and pins the "async is still a bit experimental" caveat to v3.2.1. That prose is hand-written by the maintainer. Plus a CONTRIBUTING.md wording fix and a pyproject.toml tweak. prompt: (not recorded) Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
… a coroutine For async clients, principal.calendar(cal_id=<bare id>) / (name=...) raised "TypeError: argument of type 'coroutine' is not a container or iterable": the synchronous calendar_home_set property evaluated `"@" in <coroutine>` without awaiting the async get_property. It now returns a coroutine that resolves the calendar home set (PROPFIND) before constructing the Calendar; a full-URL cal_id/cal_url needs no home set and still returns synchronously. The async integration tests' pre-test cleanup called this and swallowed the error in a bare `except`, so leftover calendars from earlier runs were never removed and the subsequent MKCALENDAR 405'd with "resource already exists" (reproduced against Infomaniak / SabreDAV). Cleanup is centralised in a new adelete_calendar_if_present() helper with a narrow except, and the created calendar is removed in a finally block. Adds a regression unit test (TestAsyncPrincipalCalendar) since this async-only breakage went uncaught. Also folds in two async-integration-test hygiene fixes uncovered by the same investigation: - test_principal_make_calendar leaked its calendar (no finally) and, after the coroutine fix above, hit "AttributeError: 'coroutine' object has no attribute 'url'" on the make_calendar reuse path; it now clears leftovers with adelete_calendar_if_present(), awaits the reuse path, and deletes in a finally. - the async DisplayName round-trip test reused the sync fixture's "Yep" display name, so a stray async calendar could hijack the sync suite's principal.calendar(name="Yep") lookup; renamed to "AsyncYep" / "hooray-async" per the documented no-reuse-of-Yep convention. prompt: pytest -k infomaniak --last-failed gives lots of 405-errors; this is most certainly due to incompatibilities on the server side that wasn't caught by the caldav-server-tester script. Please investigate. followup-prompt: Could it be because of missing cleanup of old calendar(s)? followup-prompt: Should be fixed in the library to return a coroutine [...] the cleanup should always sit in a finally-block [...] narrow the except. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ruff-pre-commit v0.9.4->v0.15.20, pre-commit-hooks v5.0.0->v6.0.0, ai-prompt-auto-commit v0.0.5->v0.0.8, conventional-pre-commit v3.4.0->v4.4.0, lychee v0.24.1->v0.24.2. pre-commit-hooks v6 removed check-byte-order-marker, so migrated to fix-byte-order-marker. ruff check passes clean; ruff format would reformat one test file (handled separately). prompt: here are some more files to look into (only deal with ~/caldav) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The bare `ruff` hook id is now a deprecated legacy alias (pre-commit prints "ruff (legacy alias)"); ruff-check is the canonical id. prompt: migrate ruff to ruff-check Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The nightly lychee run reported https://www.open-xchange.com/ as a 404 (#693). It is in fact a 301 to https://ox.io/, so follow the redirect to the resolved URL in both the OX docker test server README and the ox compatibility hints comment. Also add sync.infomaniak.com to .lycheeignore - it is an auth-required CalDAV endpoint returning 401, same category as the other entries in that section. prompt: Check if an issue was created recently by the lychee run action followup-prompt: The workflow for ~/calendar.-cli seems to close issues. Please fix. Check the link, if it's a 301 to https://ox.io then the link should be updated. Co-authored-by: Claude Opus 5 via Claude Code <noreply@anthropic.com>
Every failing nightly run opened a brand new "Link Checker Report" issue, so five near-identical ones were open at once (#685, #689, #691, #692, #693 - see #693). Replace peter-evans/create-issue-from-file with the gh-based logic already in use in calendar-cli: keep the oldest open report issue as canonical, auto-close the duplicates, update the canonical body in place, and close all open report issues once the links are healthy again. Issue handling is gated on refs/heads/main so branch and PR runs only report, never file. Also persist the lychee cache via actions/cache (the --cache flag was a no-op across runs without it), add a concurrency group, and gitignore the resulting local .lycheecache. prompt: The workflow for ~/calendar.-cli seems to close issues. Please fix. Check the link, if it's a 301 to https://ox.io then the link should be updated. Co-authored-by: Claude Opus 5 via Claude Code <noreply@anthropic.com>
New `audit` tox env running pip-audit against the resolved runtime dependency tree, plus a workflow running it weekly, on manual dispatch, and on pull requests touching pyproject.toml or tox.ini. The audit is time-triggered rather than change-triggered: a new advisory can be published against a dependency tree that has not changed. Since we declare open-ended version ranges a vulnerable release is normally resolved away by itself, so the real value is catching the case where an upper bound of ours (currently only icalendar-searcher<2) would hold users back on a release with a known vulnerability. Deliberately not added to the pre-push hooks: pip-audit makes one network request per dependency and a single slow response is enough to fail the run (happened once while testing this), which is a bad trade for a check whose findings are rare and rarely actionable at push time. Prompt: please tell me about pip-audit/bandit/semgrep/trivy Followup-prompt: install pip-audit Followup-prompt: tell me more. should it be included in project dependencies? can it be integrated in the pre-commit hooks or scan-project or ... how to use this? Followup-prompt: Let's start with the ~/caldav project Co-authored-by: Claude Opus 5 via Claude Code <noreply@anthropic.com>
The default branch of this repository is master, so `github.ref == 'refs/heads/main'` was never true: since the steps were introduced the nightly link check has neither created, updated nor closed its report issue. Broken links were only visible as a step exit code in the workflow log. Prompt: please fix all those Context: reported as one of four unrelated findings while adding the pip-audit workflow. Co-authored-by: Claude Opus 5 via Claude Code <noreply@anthropic.com>
The project has been pyproject.toml-only since the move to hatchling, but the four pip cache keys still hashed setup.py. hashFiles() returns an empty string when nothing matches, so that component of the key was constant and a change of dependencies no longer invalidated the cache. Prompt: please fix all those Context: reported as one of four unrelated findings while adding the pip-audit workflow. Co-authored-by: Claude Opus 5 via Claude Code <noreply@anthropic.com>
Presumably a typo for py39, which is out of scope anyway since requires-python is >=3.10. Note that the entry was harmless: the section is named [tox:tox], which is the form used when the configuration lives in setup.cfg. In a tox.ini the section has to be [tox], so tox never read this envlist at all - `tox list -d` reports a single default environment, py. Renaming the section would make a bare `tox` suddenly try to run py310 through py314 plus docs, style, deptry and audit, so that is left as a deliberate decision rather than folded into a typo fix. Prompt: please fix all those Context: reported as one of four unrelated findings while adding the pip-audit workflow. Co-authored-by: Claude Opus 5 via Claude Code <noreply@anthropic.com>
The build backend has been hatchling since the packaging rewrite, and hatchling never looks at [tool.setuptools] or [tool.setuptools_scm]. The version file is generated by [tool.hatch.build.hooks.vcs] and the sdist/wheel contents are controlled by the [tool.hatch.build.targets.*] excludes, so the setuptools tables were leftovers that only invited confusion about which one is in effect. Verified as a no-op by building sdist and wheel before and after the change: identical file lists (55 wheel entries, 710 sdist entries) and identical METADATA. Prompt: please fix all those Context: reported as one of four unrelated findings while adding the pip-audit workflow. Co-authored-by: Claude Opus 5 via Claude Code <noreply@anthropic.com>
[tox:tox] is the section name for tox configuration embedded in setup.cfg; in a tox.ini the section has to be [tox]. The envlist has therefore been ignored for as long as it has existed, and `tox list -d` reported a single default environment, py. It now reports py310 through py314, docs, style, deptry and audit, as the list has always claimed. Consequence worth knowing: a bare `tox` is now an expensive command - it runs the full test suite, including the integration tests, on every interpreter it can find. Missing interpreters are skipped rather than failing the run (skip_missing_interpreters defaults to true in tox 4), so a machine with only one Python still gets a sensible run. Use `tox -e py` for the old behaviour. Prompt: rename the section to [tox] and fix the envlist. Delete or rename the backup tag. Co-authored-by: Claude Opus 5 via Claude Code <noreply@anthropic.com>
Commit f8935846 bumped ruff-pre-commit from v0.9.4 to v0.15.20 and noted that ruff format would reformat one test file, to be handled separately. This is that reformatting: a single lambda assignment where the newer formatter moves the parentheses from around the assignment to around the lambda body. No behavioural change. Prompt: please investigate and commit the uncommitted work [the working tree held three unrelated uncommitted changes; this is one of them] Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
AI sessions leave draft commit messages, review notes and failure reports under docs/design/ with a tmp- prefix. None of them have ever been tracked, but they showed up in every `git status` and in every commit-helper report. Ignore the prefix instead. Prompt: please investigate and commit the uncommitted work [the working tree also held a pile of untracked scratch files that made the status report unreadable] Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The require_tls docstring added in "fix: require_tls=True not enforced on well-known URI redirect" used `http://host/dav/` as its example URL. lychee tries to resolve it, so the pre-push hook and the CI link checker both fail on it. Switched to `http://your.server.example.com/dav/`, which .lycheeignore already excludes and which reads better as an example anyway. Deliberately a separate commit rather than a fixup: the commit that introduced the docstring carries a Reviewed-by trailer, and amending it would silently extend that review to text the reviewer never saw. Prompt: We should follow the /review-and-push framework here for making a pull request towards master on github [the pre-push lychee hook rejected the push; this is the fix] Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+27
to
+43
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v5 | ||
| with: | ||
| # hatch-vcs derives the version from git tags; without them the | ||
| # project metadata pip-audit reads cannot be built. | ||
| fetch-depth: 0 | ||
| - uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: "3.13" | ||
| - uses: actions/cache@v4 | ||
| with: | ||
| path: ~/.cache/pip | ||
| key: pip|${{ hashFiles('pyproject.toml') }}|${{ hashFiles('tox.ini') }} | ||
| - run: pip install tox | ||
| - name: Audit dependencies for known vulnerabilities | ||
| run: tox -e audit |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| assert not len(c1.get_events()) | ||
| assert not len(c2.get_events()) | ||
| e1_ = c1.add_event(ev1) | ||
| e1_ = c1.add_event(near_now_ics(ev1)) |
|
|
||
| # add event | ||
| e1 = c.add_event(ev1.replace("Bastille Day Party", "Bringebærsyltetøyfestival")) | ||
| e1 = c.add_event( |
|
|
||
| # add event | ||
| e1 = c.add_event(to_str(ev1.replace("Bastille Day Party", "Bringebærsyltetøyfestival"))) | ||
| e1 = c.add_event( |
| # To actually delete a property the patch must set it to null. | ||
| # An ical → jscal conversion that omits LOCATION/DESCRIPTION must send | ||
| # {"locations": null, "description": null, ...} so the server removes them. | ||
| _ICAL_WITH_LOCATION = ( |
| # a CalendarEvent/set update when they are absent from the converted result. | ||
| # This ensures properties removed client-side (e.g. LOCATION deleted from | ||
| # the iCalendar) are actually removed on the server, not silently preserved. | ||
| _NULL_FOR_UPDATE: frozenset[str] = frozenset( |
| self.set_duration(duration, movable_attr="DUE") | ||
| return completed | ||
|
|
||
| def _complete_recurring_safe(self, completion_timestamp): |
| if action == SearchAction.LOAD_OBJECT: | ||
| load_result = data.load(only_if_unloaded=True) | ||
| if inspect.isawaitable(load_result): | ||
| await load_result |
| ## coroutine and crashed on attendee_obj.params[...]. | ||
| coro = principals[0].freebusy_request(dtstart, dtend, [principals[0]]) | ||
| assert asyncio.iscoroutine(coro) | ||
| await coro |
| for obj in unloaded: | ||
| try: | ||
| obj.load(only_if_unloaded=True) | ||
| except Exception: |
| load_result = obj.load(only_if_unloaded=True) | ||
| if inspect.isawaitable(load_result): | ||
| await load_result | ||
| except Exception: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft. Opened to get CI running —
tests.yamlonly triggers on pushes tomasterand PRs targetingmaster, so this branch has never had a test run.The description below is factual scaffolding; it will be rewritten before this
comes out of draft.
41 commits, 63 files, ~5500 insertions. Highlights:
old_flagslegacy is gone, replaced bythe feature matrix in
compatibility_hints.py; OX App Suite and CCS are nowtestable; a new server-compatibility test suite.
search.comp-type-less searches now issue one request percomponent type, so results no longer depend on the caller having passed
events=True(Perfectly good script breaks after update: "You cannot add time-range filters on the VCALENDAR component" #681).operator='=='finally enforces exact match.calendar-multigetis used for unloaded search results.calendarobjectresourcetwins, rate-limit andget_calendarslogic, and thesearch.pydriver protocol are no longer duplicated per client.several silent-wrong-result bugs and a number of async-path crashes.
response.py,require_tlsenforcement on well-known URI redirects, and credential-handling fixes in
DAVClient.__init__/URL.canonical(). SeeSECURITY.md.pip-auditdependency-audit workflow, link-checker rework, toxenvlist repairs.
Pre-rewrite history is preserved on
backup/v3.3.0-dev-pre-autosquash.Full details in
CHANGELOG.mdunder[Unreleased].