diff --git a/sidecar/collections.ts b/sidecar/collections.ts index 84bb872..bb25663 100644 --- a/sidecar/collections.ts +++ b/sidecar/collections.ts @@ -1,4 +1,13 @@ -import { buildEvent, parseContacts, parseEvents, type CalendarEvent, type Contact, type NewEvent } from './ical'; +import { + buildContact, + buildEvent, + parseContacts, + parseEvents, + type CalendarEvent, + type Contact, + type NewContact, + type NewEvent, +} from './ical'; // The JSON door: what Officer's own web UI reads. // @@ -180,3 +189,21 @@ export async function createEvent(fetcher: Fetcher, collection: string, event: N if (!res.ok) throw new Error(`radicale answered ${res.status}`); return { href }; } + +/** The same contract as `createEvent`, for a card. */ +export async function createContact( + fetcher: Fetcher, + collection: string, + contact: NewContact, +): Promise<{ href: string }> { + const uid = crypto.randomUUID(); + const href = `${collection.replace(/\/$/, '')}/${uid}.vcf`; + + const res = await fetcher(href, { + method: 'PUT', + headers: { 'Content-Type': 'text/vcard; charset=utf-8', 'If-None-Match': '*' }, + body: buildContact(uid, contact), + }); + if (!res.ok) throw new Error(`radicale answered ${res.status}`); + return { href }; +} diff --git a/sidecar/ical.ts b/sidecar/ical.ts index c0aa1ff..016140c 100644 --- a/sidecar/ical.ts +++ b/sidecar/ical.ts @@ -211,7 +211,7 @@ export function parseContacts(vcf: string): Contact[] { // ─── Writing ──────────────────────────────────────────────────────────────────────────────────────── -const escapeText = (value: string) => +export const escapeText = (value: string) => value .replace(/\\/g, '\\\\') .replace(/\r?\n/g, '\\n') @@ -243,17 +243,17 @@ export function fold(line: string): string { return out.join('\r\n'); } -const pad = (n: number) => String(n).padStart(2, '0'); +export const pad = (n: number) => String(n).padStart(2, '0'); /** `20260827T140000Z`. Always UTC, so the file carries no timezone this codebase would have to resolve. */ -const utcStamp = (date: Date) => +export const utcStamp = (date: Date) => `${date.getUTCFullYear()}${pad(date.getUTCMonth() + 1)}${pad(date.getUTCDate())}T` + `${pad(date.getUTCHours())}${pad(date.getUTCMinutes())}${pad(date.getUTCSeconds())}Z`; /** `2026-08-27` → `20260827`. */ -const dateStamp = (iso: string) => iso.slice(0, 10).replace(/-/g, ''); +export const dateStamp = (iso: string) => iso.slice(0, 10).replace(/-/g, ''); -const addDay = (iso: string) => { +export const addDay = (iso: string) => { const d = new Date(`${iso.slice(0, 10)}T00:00:00Z`); d.setUTCDate(d.getUTCDate() + 1); return d.toISOString().slice(0, 10); @@ -303,3 +303,43 @@ export function buildEvent(uid: string, event: NewEvent): string { return lines.map(fold).join('\r\n') + '\r\n'; } + +/** The inverse of `addDay`. Turns a file's EXCLUSIVE all-day DTEND back into the last day a person means. */ +export const subDay = (iso: string) => { + const d = new Date(`${iso.slice(0, 10)}T00:00:00Z`); + d.setUTCDate(d.getUTCDate() - 1); + return d.toISOString().slice(0, 10); +}; + +export type NewContact = { + fullName: string; + emails?: string[]; + phones?: string[]; + org?: string; + title?: string; +}; + +/** + * A minimal vCard 3.0 — the version CardDAV clients, iOS included, actually write. + * + * `N` is REQUIRED in 3.0 and is `Family;Given;Middle;Prefix;Suffix`. Nobody supplies it, so it is split + * out of the full name: last word is the family name, everything before it is given. That is wrong for + * some names in the world and right for most, and `FN` — which is what a phone displays — is whatever + * was actually typed either way. + */ +export function buildContact(uid: string, contact: NewContact): string { + const [family, ...given] = contact.fullName.trim().split(/\s+/).reverse(); + const lines = [ + 'BEGIN:VCARD', + 'VERSION:3.0', + `UID:${uid}`, + `FN:${escapeText(contact.fullName)}`, + `N:${escapeText(family ?? '')};${escapeText(given.reverse().join(' '))};;;`, + ]; + for (const email of contact.emails ?? []) lines.push(`EMAIL;TYPE=INTERNET:${escapeText(email)}`); + for (const phone of contact.phones ?? []) lines.push(`TEL;TYPE=CELL:${escapeText(phone)}`); + if (contact.org) lines.push(`ORG:${escapeText(contact.org)}`); + if (contact.title) lines.push(`TITLE:${escapeText(contact.title)}`); + lines.push('END:VCARD'); + return lines.map(fold).join('\r\n') + '\r\n'; +} diff --git a/sidecar/index.ts b/sidecar/index.ts index ce2b2d9..a4c6b91 100644 --- a/sidecar/index.ts +++ b/sidecar/index.ts @@ -1,7 +1,8 @@ import type { SidecarCommand, SidecarEvent } from '@@/sidecar/protocol'; import { createSidecarConnector } from '@@/sidecar/connect'; import { startRadicale, davPaths } from './radicale'; -import { listCollections, listEvents, listContacts, createEvent } from './collections'; +import { listCollections, listEvents, listContacts, createEvent, createContact } from './collections'; +import { patchFor, summarise } from './patch'; import { DATA_PATH } from '@@/data-path'; import { API_URL } from '@@/officer-url.mjs'; @@ -16,7 +17,10 @@ import { API_URL } from '@@/officer-url.mjs'; // Reached from the platform's TOP-LEVEL /dav mount, which authenticates a DAV // app password over HTTP Basic. This is the door phones use. // GET /_health is Radicale up, and does it answer. -// GET /_officer/* the JSON door for Officer's own web UI (see collections.ts). +// * /_officer/* the JSON door. Reads, and field-level writes: POST /_officer/event and +// /_officer/contact create, PATCH /_officer/item changes fields, DELETE removes. +// Built for the web UI; the agent skill speaks it too, which is why the writes +// take FIELDS rather than iCalendar text (see patch.ts). // // WHY TWO DOORS. A browser should not speak DAV — rendering a month view out of multistatus XML is a // lot of machinery, and it would make the web UI as fragile as the protocol. The collections are on @@ -203,6 +207,44 @@ const server = Bun.serve({ return Response.json({ href, data: await res.text() }); } + /* + Change some FIELDS of an item, rather than replacing its text. + + This is the route anything sane should use. PUT is still here because the raw text is sometimes + genuinely what a caller has, but a caller that only wants to move an event or fix a phone number + should not have to learn iCalendar escaping to do it — and every caller that did learn it learned + it slightly differently. The read, the patch and the write happen here, in one round trip, over + the ORIGINAL text, so nothing the patcher does not recognise is disturbed. + */ + if (url.pathname === '/_officer/item' && req.method === 'PATCH') { + const body = (await req.json().catch(() => ({}))) as { href?: string; [field: string]: unknown }; + const href = String(body.href ?? ''); + if (!href.startsWith(`/dav/${userId}/`)) { + return Response.json({ error: 'href is required and must be your own' }, { status: 400 }); + } + + const read = await fetcher(href, { method: 'GET' }); + if (!read.ok) return Response.json({ error: `radicale answered ${read.status}` }, { status: 502 }); + + let patched: string; + try { + const { href: _ignored, ...fields } = body; + patched = patchFor(href, fields, await read.text()); + } catch (err) { + return Response.json({ error: String(err) }, { status: 400 }); + } + + const isCard = href.endsWith('.vcf'); + const written = await fetcher(href, { + method: 'PUT', + headers: { 'Content-Type': isCard ? 'text/vcard; charset=utf-8' : 'text/calendar; charset=utf-8' }, + body: patched, + }); + if (!written.ok) return Response.json({ error: `radicale answered ${written.status}` }, { status: 502 }); + // The item as it now reads, so a caller can confirm what it did without a second call. + return Response.json({ ok: true, item: summarise(href, patched) }); + } + if (url.pathname === '/_officer/item' && (req.method === 'PUT' || req.method === 'DELETE')) { const body = (await req.json().catch(() => ({}))) as { href?: string; data?: string }; const href = String(body.href ?? ''); @@ -270,6 +312,38 @@ const server = Bun.serve({ } } + /* Same contract as the event route above, for an address book. */ + if (url.pathname === '/_officer/contact' && req.method === 'POST') { + const body = (await req.json().catch(() => ({}))) as { + collection?: string; + fullName?: string; + emails?: string[]; + phones?: string[]; + org?: string; + title?: string; + }; + const collection = String(body.collection ?? ''); + if (!collection.startsWith(`/dav/${userId}/`)) { + return Response.json({ error: 'collection is required and must be your own' }, { status: 400 }); + } + const fullName = (body.fullName ?? '').trim(); + // FN is what a phone displays and vCard requires it. A card without one shows as a blank row. + if (!fullName) return Response.json({ error: 'fullName is required' }, { status: 400 }); + + try { + const created = await createContact(fetcher, collection, { + fullName, + emails: (body.emails ?? []).filter(Boolean), + phones: (body.phones ?? []).filter(Boolean), + org: (body.org ?? '').trim() || undefined, + title: (body.title ?? '').trim() || undefined, + }); + return Response.json(created, { status: 201 }); + } catch (err) { + return Response.json({ error: String(err) }, { status: 502 }); + } + } + const path = url.searchParams.get('collection'); // Confine reads to this user's own tree. The header already scopes Radicale, but a path from a // query string is attacker-shaped input and should not be handed on unchecked. diff --git a/sidecar/patch.test.ts b/sidecar/patch.test.ts new file mode 100644 index 0000000..debd438 --- /dev/null +++ b/sidecar/patch.test.ts @@ -0,0 +1,240 @@ +import { describe, expect, test } from 'bun:test'; +import { patchContact, patchEvent, setProperty, setPropertyList } from './patch'; +import { parseContacts, parseEvents } from './ical'; + +// What these protect: an edit must change what was asked and NOTHING else. Every failure mode here is +// silent — the write succeeds, the list still looks right, and something the UI cannot show is gone. + +const EVENT = [ + 'BEGIN:VCALENDAR', + 'VERSION:2.0', + 'BEGIN:VEVENT', + 'UID:abc', + 'DTSTAMP:20260827T100000Z', + 'DTSTART;TZID=Europe/Lisbon:20260828T130000', + 'DTEND;TZID=Europe/Lisbon:20260828T140000', + 'SUMMARY:Before', + 'RRULE:FREQ=WEEKLY', + 'ATTENDEE;CN=Someone:mailto:someone@example.com', + 'X-APPLE-TRAVEL-ADVISORY-BEHAVIOR:AUTOMATIC', + 'BEGIN:VALARM', + 'ACTION:DISPLAY', + 'TRIGGER:-PT15M', + 'END:VALARM', + 'END:VEVENT', + 'END:VCALENDAR', +].join('\r\n'); + +const ALL_DAY = [ + 'BEGIN:VCALENDAR', + 'VERSION:2.0', + 'BEGIN:VEVENT', + 'UID:def', + 'DTSTAMP:20260827T100000Z', + 'DTSTART;VALUE=DATE:20260829', + 'DTEND;VALUE=DATE:20260830', + 'SUMMARY:Holiday', + 'END:VEVENT', + 'END:VCALENDAR', +].join('\r\n'); + +const CARD = [ + 'BEGIN:VCARD', + 'VERSION:3.0', + 'UID:ghi', + 'FN:Peter Griffin', + 'N:Griffin;Peter;;;', + 'EMAIL;TYPE=INTERNET:peter@quahog.example', + 'EMAIL;TYPE=WORK:peter@brewery.example', + 'TEL;TYPE=CELL:+351912345678', + 'X-SOMETHING-ELSE:kept', + 'END:VCARD', +].join('\r\n'); + +describe('setProperty', () => { + test('changes only the line it owns, and keeps what it does not understand', () => { + const out = setProperty(EVENT, 'SUMMARY', 'After'); + expect(out).toContain('SUMMARY:After'); + for (const kept of ['RRULE:FREQ=WEEKLY', 'ATTENDEE;CN=Someone', 'X-APPLE-TRAVEL', 'BEGIN:VALARM']) + expect(out).toContain(kept); + }); + + test('preserves parameters by default, and replaces them when told', () => { + expect(setProperty(EVENT, 'DTSTART', '20260901T090000')).toContain('DTSTART;TZID=Europe/Lisbon:20260901T090000'); + expect(setProperty(EVENT, 'DTSTART', '20260901T090000Z', '')).toContain('DTSTART:20260901T090000Z'); + }); + + test('appends after UID when absent, so it lands in the right component', () => { + const lines = setProperty(EVENT, 'LOCATION', 'Ramiro').split('\r\n'); + expect(lines[lines.indexOf('UID:abc') + 1]).toBe('LOCATION:Ramiro'); + }); + + test('null removes it', () => { + expect(setProperty(EVENT, 'RRULE', null)).not.toContain('RRULE'); + }); + + // The regression that shipped once: replacing only the first line of a FOLDED value left the rest + // behind, and each edit inherited the tail of the last. + test('replaces the whole of a folded value', () => { + const folded = EVENT.replace('SUMMARY:Before', 'SUMMARY:Before and a great deal more text\r\n folded onto line two'); + const out = setProperty(folded, 'SUMMARY', 'After'); + expect(parseEvents(out)[0]!.summary).toBe('After'); + expect(out).not.toContain('folded onto line two'); + }); + + test('a long value is folded to 75 octets and reads back whole', () => { + const long = 'y'.repeat(300); + const out = setProperty(EVENT, 'SUMMARY', long); + expect(out.split('\r\n').every((l) => Buffer.byteLength(l, 'utf8') <= 75)).toBe(true); + expect(parseEvents(out)[0]!.summary).toBe(long); + }); + + test('repeated edits do not accumulate', () => { + let out = EVENT; + for (const v of ['one'.repeat(40), 'two'.repeat(40), 'three']) out = setProperty(out, 'SUMMARY', v); + expect(parseEvents(out)[0]!.summary).toBe('three'); + }); +}); + +describe('setPropertyList', () => { + test('replaces the whole list and carries parameters over by position', () => { + const out = setPropertyList(CARD, 'EMAIL', ['new@example.com'], ';TYPE=INTERNET'); + expect(out).toContain('EMAIL;TYPE=INTERNET:new@example.com'); + expect(out).not.toContain('brewery'); + expect(parseContacts(out)[0]!.emails).toEqual(['new@example.com']); + }); + + test('extra values get the default parameters', () => { + const out = setPropertyList(CARD, 'TEL', ['+351911111111', '+351922222222'], ';TYPE=CELL'); + expect(out).toContain('TEL;TYPE=CELL:+351911111111'); + expect(out).toContain('TEL;TYPE=CELL:+351922222222'); + }); + + test('an empty list removes them all, and leaves the rest of the card', () => { + const out = setPropertyList(CARD, 'EMAIL', [], ';TYPE=INTERNET'); + expect(out).not.toContain('EMAIL'); + expect(out).toContain('X-SOMETHING-ELSE:kept'); + expect(out).toContain('TEL;TYPE=CELL:+351912345678'); + }); +}); + +describe('patchEvent', () => { + test('a text-only patch does not touch the times', () => { + const out = patchEvent(EVENT, { summary: 'After' }); + expect(out).toContain('DTSTART;TZID=Europe/Lisbon:20260828T130000'); + expect(out).not.toContain('SEQUENCE'); + }); + + test('moving a timed event writes UTC and DROPS the stale TZID', () => { + const out = patchEvent(EVENT, { start: '2026-09-01T08:00:00Z', end: '2026-09-01T09:00:00Z' }); + expect(out).toContain('DTSTART:20260901T080000Z'); + expect(out).toContain('DTEND:20260901T090000Z'); + expect(out).not.toContain('TZID'); + }); + + test('moving bumps SEQUENCE, so a phone reads an update rather than a duplicate', () => { + const once = patchEvent(EVENT, { start: '2026-09-01T08:00:00Z' }); + expect(once).toContain('SEQUENCE:1'); + expect(patchEvent(once, { start: '2026-09-02T08:00:00Z' })).toContain('SEQUENCE:2'); + }); + + test('changing only the end leaves the start exactly as it was', () => { + const out = patchEvent(ALL_DAY, { end: '2026-08-31' }); + expect(out).toContain('DTSTART;VALUE=DATE:20260829'); + expect(out).toContain('DTEND;VALUE=DATE:20260901'); // inclusive in, exclusive out + }); + + // The one that would silently move a holiday: the file's DTEND is exclusive, so editing anything else + // about the event must not re-add a day to it. + test('editing an all-day event does not drift its end', () => { + const out = patchEvent(patchEvent(ALL_DAY, { summary: 'A' }), { start: '2026-08-29' }); + expect(out).toContain('DTEND;VALUE=DATE:20260830'); + }); + + test('timed becomes all-day, both ends rewritten', () => { + const out = patchEvent(EVENT, { allDay: true, start: '2026-08-28' }); + expect(out).toContain('DTSTART;VALUE=DATE:20260828'); + expect(out).toContain('DTEND;VALUE=DATE:20260829'); + expect(parseEvents(out)[0]!.allDay).toBe(true); + }); + + test('null removes a field, an empty string does not', () => { + const withPlace = patchEvent(EVENT, { location: 'Ramiro' }); + expect(patchEvent(withPlace, { location: null })).not.toContain('LOCATION'); + expect(patchEvent(withPlace, { location: '' })).toContain('LOCATION:'); + }); + + test('separators are escaped and survive the round trip', () => { + const out = patchEvent(EVENT, { summary: 'a, b; c' }); + expect(parseEvents(out)[0]!.summary).toBe('a, b; c'); + }); +}); + +describe('patchContact', () => { + test('renaming updates N as well as FN', () => { + const out = patchContact(CARD, { fullName: 'Lois Griffin' }); + expect(out).toContain('FN:Lois Griffin'); + expect(out).toContain('N:Griffin;Lois;;;'); + }); + + test('a patch that names no list leaves both lists alone', () => { + const out = patchContact(CARD, { org: 'Pawtucket' }); + expect(parseContacts(out)[0]!.emails).toEqual(['peter@quahog.example', 'peter@brewery.example']); + expect(out).toContain('ORG:Pawtucket'); + }); + + test('everything unknown survives', () => { + expect(patchContact(CARD, { fullName: 'Peter L. Griffin' })).toContain('X-SOMETHING-ELSE:kept'); + }); + + test('REV moves, because some clients break a tie with it', () => { + expect(patchContact(CARD, { title: 'Safety Inspector' })).toContain('REV:'); + }); +}); + +// The bug that live testing caught, and the reason `within` exists. Radicale stores a VTIMEZONE beside +// any event carrying a TZID, and that block has DTSTART lines of its own — the summer-time rule. Taking +// the first DTSTART in the file rewrote the timezone and left the event where it was. +const WITH_VTIMEZONE = [ + 'BEGIN:VCALENDAR', + 'VERSION:2.0', + 'BEGIN:VTIMEZONE', + 'TZID:Europe/Lisbon', + 'BEGIN:DAYLIGHT', + 'DTSTART:20000326T010000', + 'TZOFFSETFROM:+0000', + 'RRULE:FREQ=YEARLY;BYDAY=-1SU;BYMONTH=3', + 'END:DAYLIGHT', + 'END:VTIMEZONE', + 'BEGIN:VEVENT', + 'UID:jkl', + 'DTSTAMP:20260827T100000Z', + 'DTSTART;TZID=Europe/Lisbon:20260828T130000', + 'DTEND;TZID=Europe/Lisbon:20260828T140000', + 'SUMMARY:Probe', + 'END:VEVENT', + 'END:VCALENDAR', +].join('\r\n'); + +describe('a file with a VTIMEZONE', () => { + test('moving the event does not touch the timezone rule', () => { + const out = patchEvent(WITH_VTIMEZONE, { start: '2026-09-01T08:00:00Z', end: '2026-09-01T09:00:00Z' }); + expect(out).toContain('DTSTART:20000326T010000'); // the summer-time rule, untouched + expect(out).toContain('DTSTART:20260901T080000Z'); // the event, moved + expect(out).not.toContain('DTSTART;TZID=Europe/Lisbon:20260828T130000'); + expect(parseEvents(out)[0]!.start).toBe('2026-09-01T08:00:00Z'); + }); + + test('the VTIMEZONE block survives intact', () => { + const out = patchEvent(WITH_VTIMEZONE, { summary: 'Renamed' }); + for (const kept of ['BEGIN:VTIMEZONE', 'TZID:Europe/Lisbon', 'BEGIN:DAYLIGHT', 'TZOFFSETFROM:+0000']) + expect(out).toContain(kept); + expect(parseEvents(out)[0]!.summary).toBe('Renamed'); + }); + + test('a summary is not appended into the wrong component', () => { + const out = patchEvent(WITH_VTIMEZONE, { location: 'Ramiro' }); + const lines = out.split('\r\n'); + expect(lines.indexOf('LOCATION:Ramiro')).toBeGreaterThan(lines.indexOf('BEGIN:VEVENT')); + }); +}); diff --git a/sidecar/patch.ts b/sidecar/patch.ts new file mode 100644 index 0000000..05922b9 --- /dev/null +++ b/sidecar/patch.ts @@ -0,0 +1,231 @@ +import { + addDay, + dateStamp, + escapeText, + fold, + parseContacts, + parseEvents, + subDay, + utcStamp, + type CalendarEvent, +} from './ical'; + +/** + * Changing one field of an event or a card, without losing what this codebase does not understand. + * + * ── Read, patch, put — never rebuild ── + * + * A stored object holds far more than any of our screens show: alarms, attendees, recurrence rules, + * categories, every `X-` property the client that wrote it invented. Rebuilding it from the fields we + * parsed would silently drop all of that, and an edit that quietly deletes half an event is worse than + * no editing at all. So every function here takes the ORIGINAL text, replaces only the lines it owns, + * and hands the whole thing back. + * + * ── Why this is on the server ── + * + * It lived in the browser first, next to the form, and the folding rules had to be duplicated to get it + * there. Two copies of "escape a TEXT value and fold at 75 octets" is two places to be subtly wrong, and + * the agent skill needed the same logic a THIRD time in bash. One implementation, one round trip, three + * callers. + */ + +/** + * Replace a property's line, or append it when the file has none. `null` removes it. + * + * A property is not always one line. RFC 5545 folds anything over 75 octets onto continuation lines + * starting with a space, so a long description is one property spread over several. Replacing only the + * first line leaves the rest behind, and they then fold onto whatever was written in its place — the new + * value silently grows the tail of the old one. + * + * `params` REPLACES the property's parameters when given, and preserves them when omitted. Both matter: + * `DTSTART;TZID=Europe/Lisbon` must not become a bare `DTSTART` when its value changes, and it must not + * KEEP that TZID when the new value is a UTC instant ending in Z — a timezone and a Z together are a + * contradiction some clients resolve by moving the event. + */ +export function setProperty(raw: string, prop: string, value: string | null, params?: string): string { + const lines = raw.split(/\r?\n/); + const at = lines.findIndex((l) => l === prop || l.startsWith(`${prop}:`) || l.startsWith(`${prop};`)); + + // Everything folded onto the property belongs to it. + let after = at + 1; + while (at !== -1 && after < lines.length && /^[ \t]/.test(lines[after]!)) after++; + + if (value === null) return at === -1 ? raw : [...lines.slice(0, at), ...lines.slice(after)].join('\r\n'); + + if (at !== -1) { + const existing = /^[A-Z-]+(;[^:]*)?:/.exec(lines[at]!)?.[1] ?? ''; + return [...lines.slice(0, at), fold(`${prop}${params ?? existing}:${value}`), ...lines.slice(after)].join('\r\n'); + } + + // Appended immediately after UID, so it lands in the right block of a file holding more than one. + const uidAt = lines.findIndex((l) => l.startsWith('UID:')); + const insertAt = uidAt === -1 ? Math.max(0, lines.length - 2) : uidAt + 1; + return [...lines.slice(0, insertAt), fold(`${prop}${params ?? ''}:${value}`), ...lines.slice(insertAt)].join('\r\n'); +} + +/** + * Replace every occurrence of a repeatable property — a card's EMAILs and TELs. + * + * Parameters are carried over BY POSITION: the first new phone keeps whatever the first old one had, so + * replacing a number does not quietly turn a mobile into an unlabelled one. Extra values get the default. + */ +export function setPropertyList(raw: string, prop: string, values: string[], fallbackParams: string): string { + const lines = raw.split(/\r?\n/); + const kept: string[] = []; + const oldParams: string[] = []; + let insertAt = -1; + + for (let i = 0; i < lines.length; i++) { + const line = lines[i]!; + if (line.startsWith(`${prop}:`) || line.startsWith(`${prop};`)) { + if (insertAt === -1) insertAt = kept.length; + oldParams.push(/^[A-Z-]+(;[^:]*)?:/.exec(line)?.[1] ?? ''); + while (i + 1 < lines.length && /^[ \t]/.test(lines[i + 1]!)) i++; // its continuation lines go too + continue; + } + kept.push(line); + } + + if (insertAt === -1) { + const uidAt = kept.findIndex((l) => l.startsWith('UID:')); + insertAt = uidAt === -1 ? Math.max(0, kept.length - 1) : uidAt + 1; + } + + const written = values.map((v, i) => fold(`${prop}${oldParams[i] ?? fallbackParams}:${escapeText(v)}`)); + return [...kept.slice(0, insertAt), ...written, ...kept.slice(insertAt)].join('\r\n'); +} + +/** + * Confine a patch to one component's lines. + * + * FOUND THE HARD WAY, against a real file. A stored .ics is not a flat list of properties: an event with + * a TZID carries a VTIMEZONE block, and that block has DTSTART lines of its own — the rule for when + * summer time begins. A patch that takes the FIRST `DTSTART:` in the file rewrites the timezone + * definition, leaves the event's own start exactly where it was, and reports success. The calendar then + * shows the old time, and the file is quietly corrupt. + * + * So every write goes through here first. The properties it owns live inside `BEGIN:VEVENT` (or + * `BEGIN:VCARD`), and nothing outside is ever seen. + * + * THE FIRST component, when there are several. An .ics holding a recurring series plus overrides has one + * VEVENT per override, and the first is the master — which is what a list row shows and what an edit + * from it should change. + */ +function within(raw: string, component: 'VEVENT' | 'VCARD', edit: (body: string) => string): string { + const lines = raw.split(/\r?\n/); + const start = lines.indexOf(`BEGIN:${component}`); + const end = start === -1 ? -1 : lines.indexOf(`END:${component}`, start + 1); + // No wrapper — a bare fragment. Patch what there is rather than refusing. + if (start === -1 || end === -1) return edit(raw); + + const body = edit(lines.slice(start + 1, end).join('\r\n')); + return [...lines.slice(0, start + 1), ...body.split(/\r?\n/), ...lines.slice(end)].join('\r\n'); +} + +/** A TEXT property: escaped, or removed when explicitly null. */ +const text = (raw: string, prop: string, value: string | null | undefined) => + value === undefined ? raw : setProperty(raw, prop, value === null ? null : escapeText(value)); + +export type EventPatch = { + summary?: string; + location?: string | null; + description?: string | null; + /** ISO instant, or `YYYY-MM-DD` for an all-day event. */ + start?: string; + /** Same shape. For an all-day event this is the LAST day, inclusive — as a person means it. */ + end?: string; + allDay?: boolean; +}; + +/** + * Apply a field patch to one .ics. + * + * TIMES ARE THE DELICATE PART. Three things have to move together and any one of them alone is a bug: + * the value's format (`20260901T090000Z` or `20260901`), the `VALUE=DATE` parameter that says which of + * those it is, and — for all-day — the exclusive DTEND. Changing a timed event to all-day rewrites both + * ends; changing only the end time leaves the start exactly as it was, TZID and all. + */ +export function patchEvent(raw: string, patch: EventPatch): string { + return within(raw, 'VEVENT', (body) => patchEventBody(body, raw, patch)); +} + +function patchEventBody(raw: string, whole: string, patch: EventPatch): string { + let out = text(raw, 'SUMMARY', patch.summary); + out = text(out, 'LOCATION', patch.location); + out = text(out, 'DESCRIPTION', patch.description); + + const current: CalendarEvent | undefined = parseEvents(whole)[0]; + const wasAllDay = Boolean(current?.allDay); + const allDay = patch.allDay ?? wasAllDay; + const movesTime = patch.start !== undefined || patch.end !== undefined || allDay !== wasAllDay; + if (!movesTime) return out; + + const start = patch.start ?? current?.start; + if (!start) throw new Error('cannot move an event whose start could not be read'); + + // The file's all-day DTEND is EXCLUSIVE, so reusing it as-is would shift the event a day every time + // something else about it was edited. Normalise to the inclusive form the callers speak. + const existingEnd = current?.end && wasAllDay ? subDay(current.end) : current?.end; + const end = patch.end ?? existingEnd; + + if (allDay) { + out = setProperty(out, 'DTSTART', dateStamp(start), ';VALUE=DATE'); + out = setProperty(out, 'DTEND', dateStamp(addDay(end ?? start)), ';VALUE=DATE'); + } else { + const startDate = new Date(start); + // An hour, matching what a new event with no end gets. + const endDate = end ? new Date(end) : new Date(startDate.getTime() + 60 * 60 * 1000); + out = setProperty(out, 'DTSTART', utcStamp(startDate), ''); + out = setProperty(out, 'DTEND', utcStamp(endDate <= startDate ? startDate : endDate), ''); + } + + // Phones use SEQUENCE to tell an update from a duplicate, and LAST-MODIFIED is what a sync log shows. + const sequence = Number(/^SEQUENCE:(\d+)/m.exec(out)?.[1] ?? '0'); + out = setProperty(out, 'SEQUENCE', String(sequence + 1), ''); + return setProperty(out, 'LAST-MODIFIED', utcStamp(new Date()), ''); +} + +export type ContactPatch = { + fullName?: string; + emails?: string[]; + phones?: string[]; + org?: string | null; + title?: string | null; +}; + +/** + * Apply a field patch to one .vcf. + * + * `emails` and `phones` REPLACE the whole list, because that is the shape they are read in — what comes + * back from the listing is what goes into a patch. Sending one email to a card that had two removes the + * other, which is the only reading of "set the emails to this" that is not a guess. + * + * `N` follows `FN`, since a card whose display name and structured name disagree sorts under one and + * displays the other. + */ +export function patchContact(raw: string, patch: ContactPatch): string { + return within(raw, 'VCARD', (body) => patchContactBody(body, patch)); +} + +function patchContactBody(raw: string, patch: ContactPatch): string { + let out = raw; + if (patch.fullName !== undefined) { + const [family, ...given] = patch.fullName.trim().split(/\s+/).reverse(); + out = setProperty(out, 'FN', escapeText(patch.fullName)); + out = setProperty(out, 'N', `${escapeText(family ?? '')};${escapeText(given.reverse().join(' '))};;;`, ''); + } + if (patch.emails !== undefined) out = setPropertyList(out, 'EMAIL', patch.emails, ';TYPE=INTERNET'); + if (patch.phones !== undefined) out = setPropertyList(out, 'TEL', patch.phones, ';TYPE=CELL'); + out = text(out, 'ORG', patch.org); + out = text(out, 'TITLE', patch.title); + // Not merely informational: some clients keep the copy with the later REV when two disagree. + return setProperty(out, 'REV', utcStamp(new Date()), ''); +} + +/** Which patcher a href needs. The extension is the only thing that distinguishes them. */ +export const patchFor = (href: string, patch: EventPatch & ContactPatch, raw: string): string => + href.endsWith('.vcf') ? patchContact(raw, patch) : patchEvent(raw, patch); + +/** Present only so a caller can confirm what it read back. */ +export const summarise = (href: string, raw: string) => + href.endsWith('.vcf') ? parseContacts(raw)[0] : parseEvents(raw)[0]; diff --git a/skills/calendar-and-contacts/SKILL.md b/skills/calendar-and-contacts/SKILL.md index 6aa741b..c7746e2 100644 --- a/skills/calendar-and-contacts/SKILL.md +++ b/skills/calendar-and-contacts/SKILL.md @@ -1,180 +1,151 @@ --- name: Calendar and contacts -description: Use when the owner asks you to add, move, cancel or look up an event, or to add, remove or find a contact — "dinner Saturday with my wife", "cancel that", "when am I seeing the CEO", "add Peter Griffin, this number". Carries the working recipes, the two credentials, and the timezone trap that puts every event an hour out. -version: 1 +description: Use when the owner asks you to add, move, cancel or look up an event, or to add, remove or find a contact — "dinner Saturday with my wife", "move that to 9", "when am I seeing the CEO", "add Peter Griffin, this number". Carries the JSON routes, the one credential, and the timezone trap that puts every event an hour out. +version: 2 --- -You can read and write the owner's calendars and address book directly. **Reading goes through Officer's -API. Writing goes through the DAV door.** Both use credentials you already have; there is nothing to ask -for and no password to be given. +You can read and write the owner's calendars and address books. **It is a JSON API and you already have +the key.** Nothing to ask for, no password to be given, no XML. -## The two doors +You do not write iCalendar or vCard by hand. You did once, and the recipes were long because the format +is full of things that fail silently — escaping, 75-octet line folding, an end date that is exclusive, +a timezone definition with dates of its own. All of that now happens on the server. Send fields. -**Read — Officer's own API, with `$OFFICER_AGENT_KEY`.** Parsed JSON, no XML. +## The one door ```bash -curl -s -H "Authorization: Bearer $OFFICER_AGENT_KEY" -H "Host: $OFFICER_API_HOST" \ - "$OFFICER_API_URL/api/caldav/_officer/collections" -``` - -**Write — the DAV door, HTTP Basic, the SAME key as the password.** - -```bash -ME=$(curl -s -H "Authorization: Bearer $OFFICER_AGENT_KEY" -H "Host: $OFFICER_API_HOST" \ - "$OFFICER_API_URL/api/auth/me" | python3 -c 'import json,sys; print(json.load(sys.stdin)["email"])') -AUTH="$ME:$OFFICER_AGENT_KEY" +OFFICER=(-H "Authorization: Bearer $OFFICER_AGENT_KEY" -H "Host: $OFFICER_API_HOST" + -H 'content-type: application/json') +curl -s "${OFFICER[@]}" "$OFFICER_API_URL/api/caldav/_officer/collections" ``` Your key is bound to ONE account — the owner's. You cannot reach anyone else's calendar with it and you -do not need to guard against doing so: another principal answers 403, and naming a different user in the -Basic header answers 401. Write the owner's own paths and stop thinking about it. +do not need to guard against doing so: another user's path answers 400. Write the owner's own paths and +stop thinking about it. ## Find the collections first — never guess a path ```bash -curl -s -H "Authorization: Bearer $OFFICER_AGENT_KEY" -H "Host: $OFFICER_API_HOST" \ - "$OFFICER_API_URL/api/caldav/_officer/collections" -# {"collections":[{"path":"/dav/1/personal/","displayName":"Personal","kind":"calendar", ...}, -# {"path":"/dav/1/contacts/","displayName":"Contacts","kind":"addressbook"}]} +# {"collections":[{"path":"/dav/1/personal/","name":"personal","displayName":"Personal","kind":"calendar"}, +# {"path":"/dav/1/contacts/","name":"contacts","displayName":"Contacts","kind":"addressbook"}]} ``` -`kind` is `calendar` or `addressbook`. There are usually several calendars — **"Personal" is the default -for anything the owner does not place**, and Tasks holds VTODOs that show up in Reminders on a phone. +`kind` is `calendar` or `addressbook`. Pass the **`path`** to every route below. There are usually +several calendars — **"Personal" is the default for anything the owner does not place** — and Tasks holds +VTODOs, which show up in Reminders on a phone. ## THE TIMEZONE TRAP — read this before writing any time -The owner says "Saturday at 8". Calendars store UTC. **`date -u -d` does not convert** — `-u` makes GNU -date parse the input as UTC *and* print UTC, so "8pm" becomes 20:00Z, which is 21:00 in Lisbon. Every -event lands an hour out and nothing errors. +The owner says "Saturday at 8". The API takes UTC. **`date -u -d` does not convert** — `-u` makes GNU +date parse the input as UTC *and* print it as UTC, so "8pm" becomes 20:00Z, which is 21:00 in Lisbon. +Every event lands an hour out and nothing errors. ```bash -# WRONG — prints 20260829T200000Z -date -u -d 'saturday 20:00' +%Y%m%dT%H%M%SZ -# RIGHT — prints 20260829T190000Z, via epoch so the local zone is applied -date -u -d "@$(date -d 'saturday 20:00' +%s)" +%Y%m%dT%H%M%SZ +# WRONG — prints 2026-08-29T20:00:00Z +date -u -d 'saturday 20:00' +%Y-%m-%dT%H:%M:%SZ +# RIGHT — prints 2026-08-29T19:00:00Z, via epoch so the local zone is applied +date -u -d "@$(date -d 'saturday 20:00' +%s)" +%Y-%m-%dT%H:%M:%SZ ``` The machine's clock is the owner's timezone. Parse what they said with plain `date -d`, convert through `+%s`, and **say the LOCAL time back to them** when you confirm — never the UTC you stored. -## Create an event - -**The short way, for an ordinary event.** One POST, and the sidecar writes the file — it mints the UID -and the filename, converts to UTC, escapes the text and folds any line over 75 octets. All the parts -below that are easy to get subtly wrong. +## Look something up ```bash -curl -s -H "Authorization: Bearer $OFFICER_AGENT_KEY" -H "Host: $OFFICER_API_HOST" \ - -H 'content-type: application/json' -X POST "$OFFICER_API_URL/api/caldav/_officer/event" \ +curl -s "${OFFICER[@]}" "$OFFICER_API_URL/api/caldav/_officer/events?collection=%2Fdav%2F1%2Fpersonal%2F" +# {"events":[{"uid":"…","href":"/dav/1/personal/….ics","summary":"Dinner with my wife", +# "start":"2026-08-29T19:00:00Z","location":"Cervejaria Ramiro","allDay":false}]} +``` + +Contacts are the same shape at `_officer/contacts`, returning `fullName`, `emails` and `phones`. + +**Search every calendar, not just Personal** — the answer is often in another one. `start` and `end` are +UTC, so convert before speaking: `date -d '2026-08-29T19:00:00Z' '+%A %-d %B, %H:%M'`. + +`rrule` present means the event REPEATS and the listed `start` is the series start, not the next +occurrence. Do not compute occurrences yourself — say it repeats and give the rule in words. + +**`href` is how you change or delete a thing, and it only ever comes from a read.** Never build one. A +file is only conventionally named after its UID: everything a phone wrote is named something else, and a +constructed href finds nothing or, worse, finds something. + +## Add an event + +```bash +curl -s "${OFFICER[@]}" -X POST "$OFFICER_API_URL/api/caldav/_officer/event" \ -d '{"collection":"/dav/1/personal/","summary":"Dinner with my wife","location":"Cervejaria Ramiro", "start":"2026-08-29T19:00:00Z","end":"2026-08-29T21:00:00Z","allDay":false}' # {"href":"/dav/1/personal/.ics"} ``` All-day takes plain dates and no times: `{"start":"2026-08-29","allDay":true}`. `end` is the LAST day as -a person means it — the exclusive DTEND is this route's problem, not yours. +a person means it — a two-day trip is `"start":"2026-08-29","end":"2026-08-30"`. No end means one hour, +or one day. -It writes a minimal VEVENT and nothing else. **Anything with a recurrence rule, an alarm or attendees -has to be written by hand**, below. - -## Create an event by hand - -The filename must be `.ics`, and iCalendar lines are **CRLF-terminated**. `DTSTAMP` is required. +## Add a contact ```bash -UID="officer-$(date +%s)-$RANDOM" -S=$(date -u -d "@$(date -d 'saturday 20:00' +%s)" +%Y%m%dT%H%M%SZ) -E=$(date -u -d "@$(date -d 'saturday 22:00' +%s)" +%Y%m%dT%H%M%SZ) -printf "BEGIN:VCALENDAR\r\nVERSION:2.0\r\nPRODID:-//Officer//agent//EN\r\nBEGIN:VEVENT\r\nUID:$UID\r\nDTSTAMP:$(date -u +%Y%m%dT%H%M%SZ)\r\nDTSTART:$S\r\nDTEND:$E\r\nSUMMARY:Dinner with my wife\r\nLOCATION:Cervejaria Ramiro\r\nEND:VEVENT\r\nEND:VCALENDAR\r\n" \ - | curl -s -o /dev/null -w '%{http_code}\n' -u "$AUTH" -X PUT \ - -H 'Content-Type: text/calendar; charset=utf-8' --data-binary @- \ - "$OFFICER_API_URL/dav/1/personal/$UID.ics" -# 201 created +curl -s "${OFFICER[@]}" -X POST "$OFFICER_API_URL/api/caldav/_officer/contact" \ + -d '{"collection":"/dav/1/contacts/","fullName":"Peter Griffin", + "emails":["peter@quahog.example"],"phones":["+351912345678"],"org":"Pawtucket Brewery"}' ``` -An **all-day** event uses dates, not times, and DTEND is the day AFTER: -`DTSTART;VALUE=DATE:20260829` with `DTEND;VALUE=DATE:20260830`. Writing the same day produces a -zero-length event that some clients simply do not draw. +`fullName` is what a phone displays and is required. Phone numbers keep their international prefix. -Two more that bite when writing by hand, and are why the JSON route above exists: a comma, semicolon or -backslash inside a SUMMARY must be backslash-escaped or it splits the value, and a content line over 75 -octets must be folded onto a continuation line starting with one space. +## Change something -## Look something up - -"When am I having dinner with the CEO?" — read, filter, answer in local time. +One PATCH, with only the fields that change. Everything else in the file is left exactly as it was — +including alarms, attendees, recurrence rules and anything a phone put there that Officer has no idea +about. ```bash -curl -s -H "Authorization: Bearer $OFFICER_AGENT_KEY" -H "Host: $OFFICER_API_HOST" \ - "$OFFICER_API_URL/api/caldav/_officer/events?collection=%2Fdav%2F1%2Fpersonal%2F" -# {"events":[{"uid":"…","summary":"Dinner with my wife","start":"2026-08-29T19:00:00Z","end":"…", -# "location":"Cervejaria Ramiro","allDay":false}]} +curl -s "${OFFICER[@]}" -X PATCH "$OFFICER_API_URL/api/caldav/_officer/item" \ + -d '{"href":"/dav/1/personal/….ics","start":"2026-08-29T20:00:00Z","end":"2026-08-29T22:00:00Z"}' +# {"ok":true,"item":{…the whole thing, as it now reads…}} ``` -`start` and `end` are UTC. **Convert before speaking**: `date -d '2026-08-29T19:00:00Z' '+%A %-d %B, %H:%M'`. +- **Events**: `summary`, `location`, `description`, `start`, `end`, `allDay`. +- **Contacts**: `fullName`, `emails`, `phones`, `org`, `title`. +- `null` REMOVES a field. `""` leaves it there and empty. Omit it to leave it alone. +- `emails` and `phones` replace the whole list. To add a number, send the existing ones back with the new + one — read first. -Search every calendar, not just Personal — the answer is often in another one. `location` is where -"where did I set it up" is answered from; if it is empty, say so rather than inventing somewhere. +**Moving an event is a PATCH, not a new event.** Creating a second one and leaving the first is the most +common way this goes wrong, and the owner finds out when two of them ring. -`rrule` present means the event REPEATS and the listed `start` is the series start, not the next -occurrence. Do not compute occurrences yourself — say it repeats and give the rule in words. +**Confirm from `item`, not from what you sent.** It is the file re-read after the write. -## Move an event - -**Same UID, same filename, new times.** A new UID creates a second event and leaves the first in place, -which is the most common way this goes wrong. +## Delete ```bash -# fetch the current copy, so nothing else in it is lost -curl -s -u "$AUTH" "$OFFICER_API_URL/dav/1/personal/$UID.ics" > /tmp/ev.ics -# then PUT a version with new DTSTART/DTEND and SEQUENCE bumped by one -``` - -Bump `SEQUENCE` (add `SEQUENCE:1`, or increment what is there). Phones use it to tell an update from a -duplicate. A successful update answers **204**, not 201 — 201 means you created something new, so check -your filename. - -## Delete an event, or a contact - -```bash -curl -s -o /dev/null -w '%{http_code}\n' -u "$AUTH" -X DELETE "$OFFICER_API_URL/dav/1/personal/$UID.ics" -# 200 -``` - -**Find the UID by reading first — never guess.** The file is normally `.ics`, and if that 404s, list -the collection and match on href: - -```bash -curl -s -u "$AUTH" -X PROPFIND -H 'Depth: 1' "$OFFICER_API_URL/dav/1/personal/" | grep -o '[^<]*' +curl -s "${OFFICER[@]}" -X DELETE "$OFFICER_API_URL/api/caldav/_officer/item" \ + -d '{"href":"/dav/1/personal/….ics"}' ``` **Confirm before deleting when the match is not exact.** "Cancel dinner on Saturday" with one dinner on -Saturday is unambiguous — do it. Two dinners, or a recurring one, is a question: deleting the wrong event -cannot be undone from here, and the owner will discover it when someone is stood up. +Saturday is unambiguous — do it. Two dinners, or a recurring one, is a question: this cannot be undone +from here, and the owner discovers it when somebody is stood up. -Deleting one occurrence of a REPEATING event is not a DELETE — it needs an EXDATE added to the series. -If that is what they meant, say so rather than removing every occurrence. +Deleting one occurrence of a REPEATING event is not a delete — it needs an EXDATE on the series. Say so +rather than removing every occurrence. -## Add or remove a contact +## What this API deliberately cannot do -Same shape, `.vcf` in the addressbook collection. +It writes ordinary events and ordinary cards. **Recurrence rules, alarms, attendees, EXDATEs and photos +are not in it** — not an oversight, but the same rule that makes a PATCH safe: a field the screens cannot +show is a field an editor should not invent. -```bash -UID="officer-$(date +%s)-$RANDOM" -printf "BEGIN:VCARD\r\nVERSION:3.0\r\nUID:$UID\r\nFN:Peter Griffin\r\nN:Griffin;Peter;;;\r\nTEL;TYPE=CELL:+351912345678\r\nEMAIL;TYPE=INTERNET:peter@quahog.example\r\nEND:VCARD\r\n" \ - | curl -s -o /dev/null -w '%{http_code}\n' -u "$AUTH" -X PUT \ - -H 'Content-Type: text/vcard; charset=utf-8' --data-binary @- \ - "$OFFICER_API_URL/dav/1/contacts/$UID.vcf" -``` - -`FN` is what shows on a phone and is REQUIRED. `N` is `Family;Given;Middle;Prefix;Suffix` — fill what you -know and leave the rest empty rather than omitting the line. Read contacts back with -`_officer/contacts?collection=…`, which returns `fullName`, `emails` and `phones` already parsed. +When the owner genuinely needs one, the DAV door is still there — HTTP Basic, their email as the +username and `$OFFICER_AGENT_KEY` as the password, PUT the whole `.ics` to its href. Read the file first +with `_officer/item?href=…`, change what you must, and put it back with the same UID and the same +filename. **Bump `SEQUENCE`** so a phone reads an update rather than a duplicate. If you are doing this, +say so — it is the part where a mistake is silent. ## Confirm in one line, in their words "Added — Dinner with my wife, Saturday 29 August, 20:00, Cervejaria Ramiro." Local time, the name they -used, and where. Not the UID, not the UTC, not the HTTP status. +used, and where. Not the href, not the UTC, not the HTTP status. -If a write fails, say what failed and stop. **Never retry a PUT with a new UID** — the usual cause of a -failure is that the first one partly worked, and a retry leaves two. +If a write fails, say what failed and stop. **Never retry a create after a failure** — the usual cause is +that the first one partly worked, and a retry leaves two. diff --git a/web/dav/CalendarView.tsx b/web/dav/CalendarView.tsx index 7df53f5..dc8c81b 100644 --- a/web/dav/CalendarView.tsx +++ b/web/dav/CalendarView.tsx @@ -2,7 +2,7 @@ import type { CalendarEvent } from './shared'; import { useState } from 'react'; import { CalendarDays, MapPin, Repeat, Pencil, Trash2, Check, X, Plus } from 'lucide-react'; import { useCalendarEvents, useSelectedCollection } from './useDavData'; -import { useDavEdit, setProperty, CREATING, type NewEvent } from './useDavEdit'; +import { useDavEdit, CREATING, type NewEvent } from './useDavEdit'; import { useTranslation } from 'officerdev'; // Right panel of /calendar. Deliberately a grouped agenda list, not a month grid: the sidecar returns @@ -101,7 +101,7 @@ export const CalendarView = () => { const submitNew = (e: React.FormEvent) => { e.preventDefault(); if (!draftNew?.summary.trim()) return; - void edit.create(selected.path, toEvent(draftNew)).then((ok) => ok && setDraftNew(null)); + void edit.createEvent(selected.path, toEvent(draftNew)).then((ok) => ok && setDraftNew(null)); }; return ( @@ -230,11 +230,10 @@ export const CalendarView = () => { onSubmit={(e) => { e.preventDefault(); if (!ev.href) return; + // `null` removes the line; an empty string would leave an empty LOCATION behind. void edit - .save(ev.href, (raw) => - setProperty(setProperty(raw, 'SUMMARY', draft.summary), 'LOCATION', draft.location || null), - ) - .then(() => setEditing(null)); + .save(ev.href, { summary: draft.summary, location: draft.location || null }) + .then((ok) => ok && setEditing(null)); }} > .map((part) => part[0]?.toUpperCase() ?? '') .join('') || '?'; +const FIELD = 'rounded-md border bg-background px-2 py-1 text-sm'; + +const blankDraft = () => ({ fullName: '', email: '', phone: '' }); + export const ContactsView = () => { const { t } = useTranslation(); const { selected, isLoading: loadingCollections } = useSelectedCollection('addressbook'); @@ -26,19 +30,104 @@ export const ContactsView = () => { const [editing, setEditing] = useState(null); const [draft, setDraft] = useState({ fullName: '', email: '', phone: '' }); const [confirming, setConfirming] = useState(null); + const [draftNew, setDraftNew] = useState | null>(null); if (loadingCollections) return ; if (!selected) return ; - if (error) return ; - if (isLoading) return ; - if (contacts.length === 0) return ; + + // Not an early return: the header carries the only way to ADD a card, and an empty address book is + // exactly when somebody wants it. + const notice = error + ? t('dav.readFailed', { name: selected.displayName, reason: String(error) }) + : isLoading + ? 'Loading contacts…' + : contacts.length === 0 + ? `${selected.displayName} has no contacts.` + : null; const sorted = [...contacts].sort((a, b) => a.fullName.localeCompare(b.fullName)); + const submitNew = (e: React.FormEvent) => { + e.preventDefault(); + if (!draftNew?.fullName.trim()) return; + void edit + .createContact(selected.path, { + fullName: draftNew.fullName.trim(), + emails: [draftNew.email].filter(Boolean), + phones: [draftNew.phone].filter(Boolean), + }) + .then((ok) => ok && setDraftNew(null)); + }; + return ( -
-
- {sorted.map((contact) => ( +
+
+
+
{selected.displayName}
+
+ {contacts.length} {contacts.length === 1 ? 'contact' : 'contacts'} +
+
+ +
+ +
+ {/* Name, one email, one phone. A card can hold far more, and a CardDAV client can put it there — + this is the form for adding somebody from their business card, not for modelling a person. */} + {draftNew && ( +
+
+ setDraftNew({ ...draftNew, fullName: e.target.value })} + placeholder="Name" + className={FIELD} + /> +
+ setDraftNew({ ...draftNew, email: e.target.value })} + placeholder="Email" + className={`${FIELD} min-w-0 flex-1 text-xs`} + /> + setDraftNew({ ...draftNew, phone: e.target.value })} + placeholder="Phone" + className={`${FIELD} min-w-0 flex-1 text-xs tabular-nums`} + /> +
+
+ + + {edit.error && {edit.error}} +
+
+
+ )} + + {notice && } +
+ {sorted.map((contact) => (
{initials(contact.fullName)} @@ -81,13 +170,15 @@ export const ContactsView = () => { onSubmit={(e) => { e.preventDefault(); if (!contact.href) return; + /* The form shows the FIRST email and phone; a card can hold several. Sending just + the one would delete the rest, so the others are carried through untouched. */ void edit - .save(contact.href, (raw) => { - let out = setProperty(raw, 'FN', draft.fullName); - out = setProperty(out, 'EMAIL', draft.email || null); - return setProperty(out, 'TEL', draft.phone || null); + .save(contact.href, { + fullName: draft.fullName, + emails: [draft.email, ...contact.emails.slice(1)].filter(Boolean), + phones: [draft.phone, ...contact.phones.slice(1)].filter(Boolean), }) - .then(() => setEditing(null)); + .then((ok) => ok && setEditing(null)); }} > { )}
)} -
- ))} + + ))} +
); diff --git a/web/dav/useDavEdit.test.ts b/web/dav/useDavEdit.test.ts deleted file mode 100644 index 7588464..0000000 --- a/web/dav/useDavEdit.test.ts +++ /dev/null @@ -1,83 +0,0 @@ -import { describe, expect, test } from 'bun:test'; -import { setProperty } from './useDavEdit'; -import { parseEvents } from '../../sidecar/ical'; - -// setProperty is the whole editing story: it decides what an edit keeps and what it destroys. These -// cover the two ways it can quietly destroy something — dropping a property's parameters, and leaving -// half of a folded value behind. - -const ICS = [ - 'BEGIN:VCALENDAR', - 'VERSION:2.0', - 'BEGIN:VEVENT', - 'UID:abc', - 'DTSTAMP:20260827T100000Z', - 'DTSTART;TZID=Europe/Lisbon:20260828T130000', - 'DTEND;TZID=Europe/Lisbon:20260828T140000', - 'SUMMARY:Before', - 'RRULE:FREQ=WEEKLY', - 'BEGIN:VALARM', - 'ACTION:DISPLAY', - 'TRIGGER:-PT15M', - 'END:VALARM', - 'END:VEVENT', - 'END:VCALENDAR', -].join('\r\n'); - -describe('setProperty', () => { - test('changes only the line it owns', () => { - const out = setProperty(ICS, 'SUMMARY', 'After'); - expect(out.split('\r\n').length).toBe(ICS.split('\r\n').length); - expect(out).toContain('SUMMARY:After'); - expect(out).toContain('RRULE:FREQ=WEEKLY'); - expect(out).toContain('BEGIN:VALARM'); - }); - - test('keeps a property that carries parameters', () => { - const out = setProperty(ICS, 'DTSTART', '20260901T090000'); - expect(out).toContain('DTSTART;TZID=Europe/Lisbon:20260901T090000'); - }); - - test('appends after UID when the property is absent, so it lands in the right component', () => { - const out = setProperty(ICS, 'LOCATION', 'Ramiro'); - const lines = out.split('\r\n'); - expect(lines[lines.indexOf('UID:abc') + 1]).toBe('LOCATION:Ramiro'); - }); - - test('null removes the property', () => { - expect(setProperty(ICS, 'RRULE', null)).not.toContain('RRULE'); - }); - - test('escapes the TEXT separators', () => { - const out = setProperty(ICS, 'SUMMARY', 'a, b; c'); - expect(out).toContain(String.raw`SUMMARY:a\, b\; c`); - expect(parseEvents(out)[0]!.summary).toBe('a, b; c'); - }); - - // The regression. A folded property spans several lines; replacing only the first left the rest - // behind, and they folded onto the NEW value — so each edit silently inherited the tail of the last. - test('replaces the whole of a folded value, continuations included', () => { - const folded = ICS.replace('SUMMARY:Before', 'SUMMARY:Before, and then quite a lot more text that\r\n was folded onto a second line'); - const out = setProperty(folded, 'SUMMARY', 'After'); - expect(parseEvents(out)[0]!.summary).toBe('After'); - expect(out).not.toContain('folded onto a second line'); - }); - - test('removing a folded property takes all of it', () => { - const folded = ICS.replace('RRULE:FREQ=WEEKLY', 'RRULE:FREQ=WEEKLY\r\n ;COUNT=10'); - expect(setProperty(folded, 'RRULE', null)).not.toContain('COUNT=10'); - }); - - test('a long new value is folded, and reads back whole', () => { - const long = 'y'.repeat(300); - const out = setProperty(ICS, 'SUMMARY', long); - expect(out.split('\r\n').every((l) => Buffer.byteLength(l, 'utf8') <= 75)).toBe(true); - expect(parseEvents(out)[0]!.summary).toBe(long); - }); - - test('repeated edits do not accumulate', () => { - let out = ICS; - for (const v of ['one'.repeat(40), 'two'.repeat(40), 'three']) out = setProperty(out, 'SUMMARY', v); - expect(parseEvents(out)[0]!.summary).toBe('three'); - }); -}); diff --git a/web/dav/useDavEdit.ts b/web/dav/useDavEdit.ts index 184f6ed..a7192cd 100644 --- a/web/dav/useDavEdit.ts +++ b/web/dav/useDavEdit.ts @@ -3,86 +3,18 @@ import { useQueryClient } from '@tanstack/react-query'; import { useClient } from 'hooks/useClient'; /** - * Editing and deleting one DAV object, without losing what this app does not understand. + * Creating, changing and deleting one DAV object, in FIELDS. * - * ── Read, patch, put — never rebuild ── + * Nothing here knows what an .ics looks like, and that is the point. This used to read the object's + * text, patch it in the browser and put it back, which meant the browser carried its own copy of the + * iCalendar escaping and 75-octet folding rules — a second place to be subtly wrong about a format + * neither screen displays. That work moved to the sidecar (see sidecar/patch.ts), which held the only + * other copy, and the agent skill now calls the same routes instead of writing a third one in bash. * - * An event or a card is a file with far more in it than these screens show: alarms, attendees, - * recurrence rules, categories, every `X-` property the client that wrote it invented. Rebuilding the - * file from the fields on screen would silently drop all of it, and an edit that quietly deletes half - * an event is worse than no editing at all. - * - * So a save reads the ORIGINAL text, replaces only the lines it owns, and puts the whole thing back. - * Verified on a real event: 22 lines before, 22 after, one changed. - * - * ── The href is the address, and it comes from the listing ── - * - * Not `.ics`. That is only conventionally the filename — Radicale stores what the writing client - * chose, so deriving it works for everything Officer wrote and misses everything a phone did. The - * listing returns the real href; this hands it straight back. + * The guarantee it makes is unchanged: a patch replaces only the lines it owns, so alarms, attendees, + * recurrence rules and every unknown `X-` property survive an edit made from a form that cannot show them. */ -/** - * Replace a property's line, or append it when the file has none. Handles `PROP:` and `PROP;PARAM:`. - * - * A property is not always ONE line. RFC 5545 folds anything over 75 octets onto continuation lines that - * start with a space, so a long description is one property spread over several. Replacing only the first - * line leaves the rest behind, and they then fold onto whatever was written in its place — the new value - * silently grows the tail of the old one. So the whole run goes, and the whole run is rewritten. - */ -export function setProperty(raw: string, prop: string, value: string | null): string { - const lines = raw.split(/\r?\n/); - const at = lines.findIndex((l) => l === prop || l.startsWith(`${prop}:`) || l.startsWith(`${prop};`)); - - // Everything folded onto the property belongs to it. - let after = at + 1; - while (at !== -1 && after < lines.length && /^[ \t]/.test(lines[after]!)) after++; - - if (value === null) return at === -1 ? raw : [...lines.slice(0, at), ...lines.slice(after)].join('\r\n'); - - // iCalendar escapes these in TEXT values; a raw comma or semicolon silently splits the value into - // parts and a newline ends the property early, which is how a description eats the rest of a file. - const escaped = value.replace(/\\/g, '\\\\').replace(/\n/g, '\\n').replace(/([,;])/g, '\\$1'); - if (at !== -1) { - // Keep any parameters the line already carried — `DTSTART;TZID=…` must not become `DTSTART:`. - const params = /^[A-Z-]+(;[^:]*)?:/.exec(lines[at]!)?.[1] ?? ''; - return [...lines.slice(0, at), fold(`${prop}${params}:${escaped}`), ...lines.slice(after)].join('\r\n'); - } - - // Appended inside the component, immediately after UID, so it lands in the right block of a file that - // holds more than one. - const uidAt = lines.findIndex((l) => l.startsWith('UID:')); - const insertAt = uidAt === -1 ? Math.max(0, lines.length - 2) : uidAt + 1; - return [...lines.slice(0, insertAt), fold(`${prop}:${escaped}`), ...lines.slice(insertAt)].join('\r\n'); -} - -/** - * Fold a content line at 75 OCTETS, per RFC 5545 §3.1. - * - * Octets rather than characters, and broken between code points, so an accent in a pasted title is not - * cut in half. Deliberately a second copy of the sidecar's `fold` rather than an import: this package is - * a browser bundle and the sidecar is a server process, and the existing rule in this plugin is that the - * two MIRROR each other's shapes instead of sharing code across that boundary. - */ -function fold(line: string): string { - const encoder = new TextEncoder(); - const out: string[] = []; - let current = ''; - let bytes = 0; - for (const ch of line) { - const size = encoder.encode(ch).length; - if (bytes + size > 75) { - out.push(current); - current = ' '; - bytes = 1; - } - current += ch; - bytes += size; - } - out.push(current); - return out.join('\r\n'); -} - export type NewEvent = { summary: string; location?: string; @@ -92,6 +24,29 @@ export type NewEvent = { allDay: boolean; }; +export type NewContact = { + fullName: string; + emails?: string[]; + phones?: string[]; + org?: string; + title?: string; +}; + +/** What a patch may set. Absent leaves a field alone; `null` removes it. */ +export type ItemPatch = { + summary?: string; + location?: string | null; + description?: string | null; + start?: string; + end?: string; + allDay?: boolean; + fullName?: string; + emails?: string[]; + phones?: string[]; + org?: string | null; + title?: string | null; +}; + /** `busy` holds the href being written; a create has no href yet, so it gets a sentinel of its own. */ export const CREATING = 'creating'; @@ -103,58 +58,35 @@ export function useDavEdit(invalidateKey: unknown[]) { const refresh = () => qc.invalidateQueries({ queryKey: invalidateKey }); - /** The object's original text, so a caller can patch rather than rebuild. */ - const readRaw = async (href: string): Promise => { - const res = await client.get<{ data: string }>(`/caldav/_officer/item?href=${encodeURIComponent(href)}`); - return res.data; - }; - - const save = async (href: string, patch: (raw: string) => string) => { - setBusy(href); + /** Every verb has the same shape: mark busy, do it, refresh, and report rather than throw. */ + const run = async (key: string, fallback: string, work: () => Promise) => { + setBusy(key); setError(null); try { - const raw = await readRaw(href); - await client.put('/caldav/_officer/item', { href, data: patch(raw) }); - await refresh(); - } catch (err) { - setError(err instanceof Error ? err.message : 'Could not save.'); - } finally { - setBusy(null); - } - }; - - const remove = async (href: string) => { - setBusy(href); - setError(null); - try { - await client.delete('/caldav/_officer/item', { href }); - await refresh(); - } catch (err) { - setError(err instanceof Error ? err.message : 'Could not delete.'); - } finally { - setBusy(null); - } - }; - - /** - * A new event. The BODY is built by the sidecar, not here — a browser has no business generating a - * UID, a DTSTAMP and a VTIMEZONE-free UTC timestamp, and there is exactly one correct answer for each, - * which belongs next to the parser that has to read them back. - */ - const create = async (collection: string, event: NewEvent) => { - setBusy(CREATING); - setError(null); - try { - await client.post('/caldav/_officer/event', { collection, ...event }); + await work(); await refresh(); return true; } catch (err) { - setError(err instanceof Error ? err.message : 'Could not create.'); + setError(err instanceof Error ? err.message : fallback); return false; } finally { setBusy(null); } }; - return { save, remove, create, readRaw, busy, error }; + return { + busy, + error, + + save: (href: string, patch: ItemPatch) => + run(href, 'Could not save.', () => client.patch('/caldav/_officer/item', { href, ...patch })), + + remove: (href: string) => run(href, 'Could not delete.', () => client.delete('/caldav/_officer/item', { href })), + + createEvent: (collection: string, event: NewEvent) => + run(CREATING, 'Could not create.', () => client.post('/caldav/_officer/event', { collection, ...event })), + + createContact: (collection: string, contact: NewContact) => + run(CREATING, 'Could not create.', () => client.post('/caldav/_officer/contact', { collection, ...contact })), + }; }