From 43cb531dc2943f76e9da7b39faf6862e10788692 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Thu, 27 Aug 2026 10:51:22 +0000 Subject: [PATCH] =?UTF-8?q?one=20CRUD,=20three=20callers=20=E2=80=94=20the?= =?UTF-8?q?=20browser=20and=20the=20agent=20stop=20writing=20iCalendar?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the skill told an agent how to hand-write a .ics: escape the commas, fold at 75 octets, CRLF, DTSTAMP is required, and DTEND is the day after. all true, all silent when wrong, and the browser had learned the same lesson separately in its own copy of the code. this makes the sidecar the only thing that knows the format, and gives everyone else fields. WHAT WAS MISSING BEFORE THE SKILL COULD CHANGE. reading was already JSON, and create-event landed this morning. this adds PATCH /_officer/item — change some fields of an event or a card — and POST /_officer/contact. delete already worked. with those the skill has no reason left to mention iCalendar, so it does not. PATCH IS READ-PATCH-PUT, SERVER SIDE. it reads the stored text, replaces only the lines it owns, and puts the whole thing back, so alarms, attendees, RRULEs and every X- property a phone invented survive an edit made from a form that cannot show them. that logic came out of the browser, where it had needed a duplicate of the escaping and folding rules to live; the agent would have been a third copy in bash. the web bundle now contains no iCalendar literals at all. AND LIVE TESTING FOUND THE BUG THAT MATTERS. an event with a TZID is stored with a VTIMEZONE block beside it, and that block has DTSTART lines of its own — the rule for when summer time starts. taking the first DTSTART in the file rewrote the TIMEZONE DEFINITION, left the event where it was, and answered ok. the calendar showed the old time and the file was quietly corrupt. every write is now confined to the first VEVENT or VCARD, and the file that caught it is a test. three more traps handled rather than documented: a patch that reuses an all-day event's stored DTEND must subtract the exclusive day first or the holiday drifts every time the title is edited; moving a timed event must DROP a stale TZID rather than preserve it, because a TZID and a Z together are a contradiction clients resolve by moving the event; and a move bumps SEQUENCE, which is how a phone tells an update from a duplicate. contacts got the create form the calendar had, so both screens now do all four. editing a card sends the other emails and phones back untouched — the form shows the first of each, and sending only that would delete the rest. 47 tests across the writer and the patcher. verified live against a real event carrying an alarm, an attendee, an RRULE and an X- property: all four intact after a rename, a relocation and a move to a different day. Co-Authored-By: Claude Opus 5 Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043 --- sidecar/collections.ts | 29 +++- sidecar/ical.ts | 50 +++++- sidecar/index.ts | 78 ++++++++- sidecar/patch.test.ts | 240 ++++++++++++++++++++++++++ sidecar/patch.ts | 231 +++++++++++++++++++++++++ skills/calendar-and-contacts/SKILL.md | 205 ++++++++++------------ web/dav/CalendarView.tsx | 11 +- web/dav/ContactsView.tsx | 122 +++++++++++-- web/dav/useDavEdit.test.ts | 83 --------- web/dav/useDavEdit.ts | 170 ++++++------------ 10 files changed, 871 insertions(+), 348 deletions(-) create mode 100644 sidecar/patch.test.ts create mode 100644 sidecar/patch.ts delete mode 100644 web/dav/useDavEdit.test.ts 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 })), + }; }