Contacts sync had a real, wiremock-tested CardDAV client that production
could never reach: `AccountConfig` had no way to express a CardDAV
address book, so `Contacts/Sync` always fell to its honest-error branch
unless a test double was injected.
Add CardDAV as a first-class account protocol, mirroring how CalDAV is
already resolved and driven:
- `AccountProtocol::CardDav` and `Transport::CardDav(DavTransport)`, plus
`is_caldav()`/`is_carddav()` so callers pick a specific DAV flavour
rather than the ambiguous `is_dav()`. Calendar sync now filters on
`is_caldav()` so a CardDAV account can never be mistaken for a calendar.
- Persistence reuses the existing `collection_url` column, distinguished
by the `protocol` discriminator; no schema change.
- Proto gains `ACCOUNT_PROTOCOL_CARDDAV` and a `carddav` transport case
(both additive), with the committed descriptor golden regenerated.
- `contacts_sync::sync_carddav_account` builds a real `CardDavClient` from
the persisted collection URL and a credential read just-in-time from the
OS keyring, zeroized on drop.
- `Contacts/Sync` resolves configured CardDAV accounts and returns a typed
`NotConfigured` (FAILED_PRECONDITION) when none matches -- including
when the named account exists but speaks another protocol -- instead of
an untyped internal error or a fabricated zero count.
- `account add --protocol carddav --collection-url ...` in the CLI.
Offline E2E proves the production path: a CardDAV account added over the
real Accounts API, synced through a real client against wiremock, with
credentials resolved from a mocked vault.
Refs #255
The CalDAV and CardDAV clients found their payloads by splitting the
response body on the literal strings `<c:calendar-data>` and
`<card:address-data>`. That is only correct for servers that happen to
pick the same prefixes Nuncio does. A compliant server that binds the
namespaces to `D:`/`CAL:`, or declares `DAV:` as the default namespace,
produced zero events or zero contacts and reported success -- the user
saw an empty calendar with nothing in the logs. An element whose name
merely contains the searched one (`<c:calendar-data-summary>`) was read
as if it were the payload, and a property returned under a `404`
propstat was treated as a value.
Add `nuncio_core::dav`, a namespace-aware `multistatus` walker built on
quick-xml (already a workspace dependency, previously declared but
unused) and shared by both clients, since RFC 4791 and RFC 6352 return
the same document shape. Elements are matched on their resolved
`{namespace}local-name`; `<propstat>` status is honoured; and a body
that cannot be traversed is now a `MalformedResponse` error rather than
an empty result indistinguishable from an empty collection. Responses
that carry no readable value are counted and logged at `warn` with the
href and reason, so an omission is visible instead of silent.
Refs #257
A single unauthenticatable body ciphertext took down whole operations:
the FTS backfill runs inside migrate(), so one bit-rotted row stopped
the daemon booting at all, and listing, triage and export each failed
outright over one bad row out of fifty.
Single-item get_message stays fail-closed and is unchanged. It names one
message, so there is no other row left to hand back, and a message with
an empty body reads exactly like a genuinely empty one.
Bulk paths now return each row as MessageRead (Ok(email) or
Err(UnreadableBody)) instead of failing the call. The row keeps its slot,
which is what lets keyset cursors and chunk boundaries stay exact, and no
row ever comes back carrying a body the store could not authenticate.
Each failure is logged at warn! naming only the message key.
The daemon reports the omissions on the wire rather than swallowing them:
unreadable_message_ids on ListMessagesResponse and ExportResponse (an
archive outlives the store, so its gap is named), unreadable_count on
TriageProgress, and the CLI prints all three. Triage never evaluates
rules against a body that failed authentication. The FTS backfill skips
the row entirely rather than indexing an empty body, so every subsequent
open re-reports it and a repaired row heals itself.
Refs #265
NaturalLanguageScheduler ignored the requested weekday and time
entirely, always returning the reference instant plus 24 hours, and
never errored on unrecognised phrasing. Every input produced a
plausible-looking event at the wrong time, which a user only discovers
by missing the meeting.
Replace it with a small, exactly-specified grammar:
<summary> today|tomorrow|YYYY-MM-DD at <time> for <duration>
Everything outside that grammar returns an error. Weekday phrasings
("next Tuesday") are refused as genuinely ambiguous rather than
resolved, since the two common readings differ by a week. A time of
day and a duration are both mandatory, so neither is defaulted.
Resolution now requires an explicit IANA time zone; a bare Unix
timestamp cannot anchor "3pm". Wall-clock times go through the zone's
DST rules and both anomalies are reported: times inside a
spring-forward gap and times inside a fall-back overlap are refused
rather than silently shifted or disambiguated. Durations are added as
elapsed time, so events spanning a transition keep their real length.
parse now returns ParsedSchedulingIntent instead of a CalendarEvent
with invented "default" account and calendar ids. There are no
in-tree callers, so no behaviour visible to a client changes.
Refs #258
The headless runner still built an ephemeral DatabaseEngine, an EventBus,
and a TempDir that no command read -- a remnant of the CLI-holds-its-own-store
design that predates the gRPC API. Nothing but two dead_code accessors touched
them, so remove the fields, the accessors, and the now-unreachable RunnerError.
With no store to open, construction is neither fallible nor async, so the
constructors lose their Result and async, and 'ephemeral' no longer describes
them: they are renamed connect/connect_with/connect_with_token_override.
Doc comments that contrasted the daemon's persistent store against 'this
runner's own ephemeral local db' now described a field that does not exist;
they are rewritten to say the CLI keeps no store at all.
The NSQL size field was computed from body_plain alone, so an HTML-only
message measured zero bytes and a short note carrying a large attachment
measured only the note. Every size comparison then answered confidently
and wrongly on exactly the mail such rules are written to catch.
size is now the total byte size of the content the store holds: both body
parts plus every attachment payload. Both parts are summed because a
multipart/alternative message genuinely carries both on the wire. Headers
and MIME framing are excluded and documented as excluded -- the raw RFC822
octets are not retained and no column carries their length, so the figure
is a consistent lower bound rather than a guess.
Refs #256
tls_mode_from_db coerced any unrecognized imap_tls_mode/smtp_tls_mode
token to ImplicitTls on the path every ordinary account read takes, so a
corrupted or tampered row silently handed callers a transport the operator
never configured -- a STARTTLS account read back as implicit TLS, with no
error and no log.
Parse now fails closed with DatabaseError::Corrupted naming the account,
the column, and the offending value. list_accounts already returned a
Result, so no caller signature changes and no caller panics; get_account
inherits the behaviour through it.
Refs #271
nunciod mostly mints the key material it reads, so the OS keyring
normally answers without ceremony. That is a habit, not a guarantee: a
keychain locked by screen-lock, a restored or migrated keychain whose
ACL no longer trusts the binary, a re-signed or rebuilt nunciod, or a
Linux Secret Service with no unlocked collection all turn the read into
a wait on something nobody can answer. Unbounded, that left the daemon
hung at startup with no error and no log line.
Bound both of the daemon's key-material reads -- the gRPC bearer token
minted at startup and the webhook signing key provisioned on first CALL
WEBHOOK dispatch -- so each terminates with an honest, logged error that
names the key material and the likely cause. Startup still fails closed
through the same path it already used; only the hang case now reaches
it.
Lift the bounded read the CLI already uses into nuncio-store so both
binaries share one implementation. It still runs on a detached OS thread
behind a timeout rather than spawn_blocking: a keyring read cannot be
cancelled, and Tokio joins its blocking pool at shutdown, which would
merely relocate the hang to process exit.
The daemon deliberately gains no environment override. It mints and owns
the authoritative token and must never accept one from its environment.
Refs #421
test_stage_2_backup_creation_and_stage_3_table_salvage built its fixture
engine inside a block and let it drop rather than closing it. A dropped
sqlx pool tears its connections down asynchronously, so SQLite's
close-time WAL handling -- which operates on -wal/-shm by path -- could
land after salvage had unlinked those paths and created a fresh database
at the same location. That pairs the new, empty main file with a -wal
describing the old one, and replaying it reads past end-of-file:
SQLITE_IOERR_SHORT_READ (522), surfaced as the observed
Query(Database(SqliteError { code: 522, message: "disk I/O error" })).
close() awaits the teardown, so the window ceases to exist rather than
merely narrowing. This mirrors what the production caller
(DatabaseEngine::close_and_salvage) already does and what the sibling
stage-1 detection test already did. Record the precondition on
SqliteRecoveryEngine::salvage so the next caller does not repeat it.
nunciod mints the gRPC bearer token into the OS keyring, so the keychain
item's ACL trusts nunciod. nuncio-cli is a different binary, so reading
that item raises an interactive consent prompt. Nothing can answer that
prompt from CI, ssh, or a background shell, and the blocking keyring read
never returns -- the CLI hung with no output and no error.
Add an opt-in NUNCIO_GRPC_TOKEN override: when set and non-empty the CLI
uses it verbatim and never touches the keyring. The keyring remains the
default and is otherwise unchanged.
The keyring read itself can no longer stall forever either. It now runs on
a detached OS thread behind a bounded timeout that reports what is blocking
and how to bypass it. A detached thread rather than spawn_blocking, because
Tokio joins its blocking pool during runtime shutdown, which would just move
the hang to process exit.
Collapse the eight copies of the token-resolution block onto that one
resolver so no call site can drift.
Refs #419
`parse_email_get_response` labelled every placement it produced with a
fixed `"inbox"`, so a caller outside the sync loop got occupancies keyed
by a mailbox the account may never sync -- and a placement keyed by a
folder that is never enumerated can never be matched again.
Take the folder id as a parameter and drop the private in-folder variant
the sync loop had been calling to work around the fixed label. Callers
now say which mailbox they are parsing for.
Populating `Message.placements` cost one query per message, so a page of
fifty messages cost fifty-one round trips against the store. Add a
batched store read that fetches a whole set of message keys in one
statement per chunk and groups the result by key, and use it from the
listing path.
The batch chunks at the existing bind-parameter ceiling and orders by
folder id within each key, so the first placement remains the message's
canonical mailbox exactly as the per-message read had it.
Prove the new wire surface end to end without a network: a multi-mailbox
message refuses an unaddressed move and accepts an addressed one; the
queued outbox row really carries the occupancy the caller named; naming
an occupancy the message does not hold is refused rather than re-aimed;
reads report every occupancy while folder_id still names the one they
arrived through; conflicts survive from record through resolution; and
filters_enabled round-trips over UpdateAccount.
Also pin the executor half: a mutation whose payload names a placement
dispatches against exactly that occupancy instead of hitting the
ambiguity refusal.
Refs #395
IMAP hardcoded no object id into the message key, so it could only ever
reach the Message-ID+content tier or the folder-scoped surrogate, while
JMAP entered at the top tier from the server's Email id. One account
synced through both protocols therefore minted disjoint key spaces.
RFC 8474 EMAILID is the same identifier JMAP exposes, so requesting it
makes the two keys equal. Negotiate per capability: EMAILID only where
the server advertises OBJECTID, X-GM-MSGID only where it advertises
X-GM-EXT-1. A server offering neither behaves exactly as before.
EMAILID cannot go through the typed client: imap-proto's msg_att parser
is a closed alt with no catch-all, and async-imap turns the resulting
nom error into a fatal io::Error that closes the session. The raw layer
reads it off the transport beneath ImapStream instead, so an attribute
the crate cannot parse can never reach its decoder.
Lifting stored rows to a stronger tier changes every message_key, so
IDENTITY_SCHEMA_VERSION moves to 3 and the existing reconciliation
rebuilds the cached store rather than repointing rows one at a time.
The engine already executed verified remote mutations, but the Mail
service had no way to ask for one, so a human could not move, delete or
flag a message at all. Add MoveMessage, DeleteMessage and FlagMessage,
each addressing a specific mailbox occupancy rather than a bare message
key: identity is content-addressed and folder-independent, so one message
can sit in several mailboxes, and aiming a MOVE or DELETE at the wrong
occupancy is destructive and has no undo. The outbox already refused an
ambiguously-addressed mutation permanently; the queued row now carries
the occupancy its intent was formed against, which turns that refusal
into something a caller can avoid instead of a dead end. A named
occupancy is honoured only after confirming the message still holds it.
Conflicts become a typed error and durable state rather than a hopeful
field. Append ERROR_REASON_CONFLICT (13) and surface it through the
existing ErrorInfo status-details mechanism, so a client predating it
takes the error path instead of misreading success; a conflict is raised
only on the server's positive evidence, never on an unverifiable outcome.
Because the event stream is a lossy broadcast with no resumption, the
conflict is also written to a queryable table, with ListConflicts and
ResolveConflict over it.
Message now carries its complete placement set. folder_id keeps its
meaning -- the occupancy the message was reached through -- so this is
purely additive; its doc comment now points at the new field instead of
saying no such field exists.
filters_enabled reaches the wire. It was per-account and settable only by
hand-editing the accounts row, which made filters untestable by hand; it
is now on AccountConfig and flippable through UpdateAccount, still
defaulting to off because filters take irreversible remote actions.
CLI verbs land alongside: mail move/delete/flag/unflag, mail
conflicts/resolve, and account filters.
Refs #395
The engine is ahead of its contract: it executes verified mutations but the
Mail service has no RPC to ask for one, so a human cannot move, delete or
flag anything. By this repo's definition of done the placement work has an
engine and nothing else.
Records the three steps that buy manual usability, what works and what does
not if you boot it today, the 81-issue backlog behind them, and the two test
traps (tracing callsite Interest, tokio::time::pause over real I/O) that are
invisible at default parallelism.
JMAP's folder sync could only ever add. It returned `removals: []` and
`present: None` unconditionally, so mail deleted or moved by any other
client -- another daemon, a phone, webmail -- stayed in the local store
forever and every JMAP account accumulated ghosts indefinitely.
`sync_changes` now resumes through `Email/changes` (RFC 8621 s4.2 over
the RFC 8620 s5.2 core method):
- `created`/`updated` become upserts, with bodies fetched by `Email/get`
- `destroyed` becomes placement removals
- `hasMoreChanges` is followed to convergence, bounded so a server that
never advances its `newState` cannot pin the sync task
- `cannotCalculateChanges` falls back to a full enumeration, the one
pass entitled to report `present: Some(...)` because it is the only
one that genuinely looked at the whole mailbox
An ordinary incremental pass still reports `present: None`: it never
enumerated the folder, and claiming otherwise would delete every message
that simply did not change.
`Email/changes` is account-wide, so a folder pass sees ids belonging to
other mailboxes. `Email/get` now requests `mailboxIds`, which both keeps
those ids from being placed here and reconciles away a message another
client moved out of this folder.
Checkpoints are scheme-tagged (`v1:jmapstate:{state}`) like the IMAP
ones, since a JMAP state is an opaque token with no self-describing
shape. An untagged or unknown token re-enumerates rather than being
handed back to a server that never issued it.
Placements now carry the mailbox that was actually synced. Removals are
addressed by that same key, so a placement stored under any other folder
id could never be reconciled away again.
Refs #408
dev split message identity from placement, made filter evaluation per
placement, and replaced the schema-migration helpers with create-only
schemas. This branch's single-owner filter execution had to be re-seated
on top of all three.
The account gate stays where it was in intent -- a daemon that does not
own an account runs none of that account's rule actions -- but now sits
outside the per-placement fire-once claim rather than beside a per-message
one. On the sync path the gate is checked with the new-arrival test before
`apply_filter_actions` is reached, so a non-owner never takes a claim it
has already decided not to act on; leaving the claim unconsumed keeps it
available to the daemon that does own the account. Triage does the same
one level up: a message whose account this daemon does not own is counted
as scanned but its placements are never even read, and a daemon owning no
account still refuses the RPC outright instead of streaming an empty scan.
`filters_enabled` is now only a column in the created `accounts` schema;
the additive `ALTER TABLE` helper and its two migration tests are dropped,
since dev creates schemas and never upgrades them. The flag's persistence
is still covered -- the identity-rebuild test now asserts filter ownership
survives a rebuild, and the store round-trip assertions are unchanged.
The non-owner sync test keeps every assertion it had, translated to the
placement API: the mail is still stored, the occupancy's read flag is
still unflipped, and no execution log or outbox mutation is written.
The two hung-mutation tests paused the Tokio clock so auto-advance would
jump the 30s per-item timeout. That cannot be made reliable: the executor
does real SQLite I/O on a blocking pool inside the same window, and
auto-advance fires whenever the runtime looks idle. It also fires sqlx's
own pool-acquire timer, which defaults to the same 30 seconds -- turning a
healthy list_pending_mutations into PoolTimedOut, whose error arm returns
an all-zero summary. That is exactly the shape CI reported: completed 0
and failed 0 both as expected, retried 0 instead of 1.
Make the per-item timeout an overridable method on RemoteExecutionEnv and
give the hanging test env 250ms, so the tests wait it out in real time and
no test pauses the clock. The hang is infinite, so a loaded runner makes
these slower, never wrong.
50 runs at --test-threads=2 under 16-way CPU load: 0 failures.
james.maes
James Maes