one CRUD, three callers — the browser and the agent stop writing iCalendar
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 <noreply@anthropic.com> Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043
This commit is contained in:
Claude Opus 5
@@ -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];
|
||||
Reference in New Issue
Block a user