From d92781bd1b3aa3b68d5bffe38db4008f330f2f9e Mon Sep 17 00:00:00 2001 From: kevin9327 Date: Thu, 3 Sep 2026 07:10:49 +0900 Subject: [PATCH] Cap a reported message's time at the server's clock, so a browser that runs ahead cannot hide what a routine said MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `POST /:channelId/activity` took `at` from the browser and `recordActivity` applied it with a forward-only guard, returning silently when nothing moved. The parser's comment said a wrong clock could lose a report, not corrupt the row. That is the clock that is behind. One that is ahead lands its report, stamps the row into the future, and then every correct report loses until real time catches up: the routine runner's, which is the one report a headless firing makes; a relayed handoff answer's; every other member's browser. The reply was in the thread and the roster never said so, and the run row read `succeeded`. A reported time is now capped at the server's own clock — a browser may say when, but not later than now. Clamped rather than refused, because clocks are a little ahead all the time; a stamp in the past is kept, so a message and its reply reported by the same clock still land in that order. `now` is injectable, and the test that pins the clamp fails before this change. --- CHANGELOG.md | 8 +++ server/src/channels/routes.ts | 23 +++++++-- server/tests/channel-activity-input.test.ts | 55 +++++++++++++++++++++ 3 files changed, 81 insertions(+), 5 deletions(-) create mode 100644 server/tests/channel-activity-input.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index d446e0438..d620e1cc4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,14 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### A browser clock that runs ahead no longer hides what a routine said + +The roster line and unread dot for a channel are moved by the last report that arrived, and only +ever forwards. A browser whose clock was ahead stamped its report into the future, and then every +report from a correct clock — a routine's reply, a relayed handoff answer, another member's browser +— was dropped without a word until the real time caught up: the reply was in the thread, and the +roster never said so. A reported time is now capped at the server's own clock. + ### Coworkers are made in a wizard and managed in a dialog Creating a coworker is now a three-step wizard — who it is, who may see it, then where it runs, diff --git a/server/src/channels/routes.ts b/server/src/channels/routes.ts index 58c3864e9..c481e17a2 100644 --- a/server/src/channels/routes.ts +++ b/server/src/channels/routes.ts @@ -908,10 +908,22 @@ type ActivityInputParseResult = /** * Parse a reported message. * - * `at` comes from the client that saw the message, because only it knows when the message arrived, * but it is never trusted as a clock: the store compares it against what is stored and only ever - * moves forwards, so a wrong one can lose a report, not corrupt the row. + * `at` comes from the client that saw the message, because only it knows when the message arrived, + * and it may say when, but not later than now. The store compares it against what is stored and + * only ever moves forwards, and that guard is shared with every other clock in the deployment: + * the routine runner's, a relayed handoff answer's, every other member's browser. A browser whose + * clock ran seven minutes ahead used to stamp the row seven minutes into the future, and it was + * not that report that got lost — every correct one for the next seven minutes was, silently: a + * routine's reply landed in the thread and never on the roster. Clamped rather than refused, + * because clocks are a little ahead all the time and a report a second early is still the report. + * A stamp in the past is kept as it is, so a person's message and the reply, reported separately + * by the same clock, still land in the order that clock saw them. */ -export function parseActivityInput(input: unknown): ActivityInputParseResult { +export function parseActivityInput( + input: unknown, + /** The server's own clock, injectable so a test can be about a specific gap. */ + now: Date = new Date(), +): ActivityInputParseResult { if (!isChannelInputObject(input)) { return { ok: false, error: "Activity must be a JSON object." }; } @@ -926,10 +938,11 @@ export function parseActivityInput(input: unknown): ActivityInputParseResult { if (typeof object.at !== "string") { return { ok: false, error: "Timestamp is required." }; } - const at = new Date(object.at); - if (Number.isNaN(at.getTime())) { + const reported = new Date(object.at); + if (Number.isNaN(reported.getTime())) { return { ok: false, error: "Timestamp must be an ISO-8601 date." }; } + const at = reported.getTime() > now.getTime() ? now : reported; return { ok: true, diff --git a/server/tests/channel-activity-input.test.ts b/server/tests/channel-activity-input.test.ts new file mode 100644 index 000000000..88f249178 --- /dev/null +++ b/server/tests/channel-activity-input.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, test } from "bun:test"; +import { parseActivityInput } from "../src/channels/routes"; + +/** + * The moment a browser says a message arrived is the browser's clock, and the row it lands on is + * compared against by every other clock in the deployment: the routine runner's, a relayed + * handoff answer's, and every other member's browser. `recordActivity` only moves forwards, so a + * report stamped in the future is not the report that gets lost — every correct one after it is. + */ +describe("parsing a reported message", () => { + const now = new Date("2026-09-03T10:00:00.000Z"); + + test("keeps a timestamp that is not ahead of the server", () => { + const at = "2026-09-03T09:59:30.000Z"; + const parsed = parseActivityInput( + { text: "hello", agentId: null, at }, + now, + ); + expect(parsed.ok).toBe(true); + if (!parsed.ok) return; + expect(parsed.value.at.toISOString()).toBe(at); + }); + + test("clamps a timestamp from a clock that runs ahead to now", () => { + const parsed = parseActivityInput( + { text: "hello", agentId: null, at: "2026-09-03T10:07:00.000Z" }, + now, + ); + expect(parsed.ok).toBe(true); + if (!parsed.ok) return; + expect(parsed.value.at.toISOString()).toBe(now.toISOString()); + }); + + test("still refuses a timestamp that is not a date", () => { + const parsed = parseActivityInput( + { text: "hello", agentId: null, at: "yesterday" }, + now, + ); + expect(parsed).toEqual({ + ok: false, + error: "Timestamp must be an ISO-8601 date.", + }); + }); + + test("keeps the rest of the report as it was", () => { + const parsed = parseActivityInput( + { text: "hello", agentId: " agent-1 ", at: "2026-09-03T09:00:00.000Z" }, + now, + ); + expect(parsed.ok).toBe(true); + if (!parsed.ok) return; + expect(parsed.value.text).toBe("hello"); + expect(parsed.value.agentId).toBe("agent-1"); + }); +});