From 6dd4e5e3eb17a24d245a3aff12b332cd985b54a4 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Wed, 9 Sep 2026 08:28:17 -0500 Subject: [PATCH] fix(events): the public calendar, a stranded revert, and three dropped facts (Phase 16a) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects the acceptance walk found in shipped code. **The public calendar showed neither what is live nor what is recent.** §I says `GET /public/events` is "the calendar: upcoming, **live** and **recent**". Built, it was upcoming only: `listInWindow` filtered on `scheduled_for >= from` alone and the shipped page asks for no window at all, so it took the default of now → +31d. A run that began five minutes ago and has three hours to go was absent; so was one that ended an hour ago. The site contradicted itself — `live: true` on `/site/events/` while `/site/events` served `entries: []`. A run is an interval, not an instant. `listInWindow` now matches a run whose occupied interval OVERLAPS the window, which fixes the admin calendar's identical hole (a run that started last Sunday and is still going was missing from "this week"), and the public default reaches `DEFAULT_RECENT_DAYS` back so "recent" has somewhere to live. Forecasts are still computed from `now`, never from the tail: a projection into the past would advertise an occurrence that did not happen. **A resource left `reverting` by a crash was never reclaimed.** `claimRevert`'s comment said `reverting` is not claimable "exactly as a step with a live claim is" — but a step's claim carries `claim_expires_at` and is reclaimed when the lease lapses, and a resource in `reverting` had no expiry and nothing released it. A process killed mid-teardown stranded the row for good: the sweep skipped it every 15s for ever, `cleanup_status` never left `pending`, and `POST …/cleanup` — the recourse §I names — answered 200 and did nothing, because it claims through the same function. On the rig it stranded a lease, which then BLOCKED the next run of the same event from taking that value until the shard's own deadline lapsed. The stale test is `updated_at`, which for a `reverting` row is exactly when the claim was taken, so no column is added. `updated_at` is re-stamped explicitly and that is load-bearing rather than tidy: this connector sends `CLIENT_FOUND_ROWS`, so without the write a second claimer would still match the row. `revert_attempts` is untouched — a stale claim is a process that died, not an attempt that failed. **Three facts every event announcement computed and none could use.** `announce.js` `baseFor()` puts `summary`, `seriesName` and `timezone` on all seven `event.*` payloads, but four triggers declared none of them and a fifth declared one, so `validatePayload` dropped them, they were absent from the variable list an author picks from, and every emit logged `emit carried undeclared variables` at DEBUG. They are now one shared `EVENT_AMBIENT` declaration spread into all seven, with the per-trigger copies removed so the seven cannot drift. Verified against a real ServUO + sidecar + website rig: the public page now shows a live run as "Happening now" beside recent finished ones (it showed nothing at all before), and a lease stranded by a real mid-teardown crash was reclaimed within one sweep, taking `cleanup_status` from `pending` to `complete`. The three `claimRevert` tests live in `eventRunnerSql.test.js` against a real MariaDB, because every part of the answer is the server's — `NOW() - INTERVAL`, `ON UPDATE`, and above all what `affectedRows` counts. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016wDDVXWMDz82WqE1i969r4 --- server/engagement-triggers.json | 132 +++++++++++++++--- server/src/config/coreTriggers.js | 44 ++++-- server/src/model/events/eventPublic.model.js | 45 +++++- .../src/model/events/eventRunResources.db.js | 48 ++++++- server/src/model/events/eventRuns.db.js | 27 +++- server/test/eventPublic.test.js | 92 +++++++++++- server/test/eventRunnerSql.test.js | 71 ++++++++++ 7 files changed, 407 insertions(+), 52 deletions(-) diff --git a/server/engagement-triggers.json b/server/engagement-triggers.json index a72784a..1d87575 100644 --- a/server/engagement-triggers.json +++ b/server/engagement-triggers.json @@ -27,6 +27,27 @@ "example": "The Yew Invasion", "description": "The event title." }, + { + "name": "summary", + "type": "string", + "required": false, + "example": "Orcish warbands are massing north of Yew.", + "description": "The event’s one-line summary, when it has one." + }, + { + "name": "seriesName", + "type": "string", + "required": false, + "example": "The Yew Campaign", + "description": "The arc this event belongs to, when it belongs to one." + }, + { + "name": "timezone", + "type": "string", + "required": false, + "example": "America/New_York", + "description": "The zone the run was computed in — what a time in the body should be read as." + }, { "name": "phase", "type": "string", @@ -89,6 +110,27 @@ "example": "The Yew Invasion", "description": "The event title." }, + { + "name": "summary", + "type": "string", + "required": false, + "example": "Orcish warbands are massing north of Yew.", + "description": "The event’s one-line summary, when it has one." + }, + { + "name": "seriesName", + "type": "string", + "required": false, + "example": "The Yew Campaign", + "description": "The arc this event belongs to, when it belongs to one." + }, + { + "name": "timezone", + "type": "string", + "required": false, + "example": "America/New_York", + "description": "The zone the run was computed in — what a time in the body should be read as." + }, { "name": "reason", "type": "string", @@ -135,7 +177,21 @@ "type": "string", "required": false, "example": "Orcish warbands are massing north of Yew.", - "description": "The event summary, as authored." + "description": "The event’s one-line summary, when it has one." + }, + { + "name": "seriesName", + "type": "string", + "required": false, + "example": "The Yew Campaign", + "description": "The arc this event belongs to, when it belongs to one." + }, + { + "name": "timezone", + "type": "string", + "required": false, + "example": "America/New_York", + "description": "The zone the run was computed in — what a time in the body should be read as." }, { "name": "participantCount", @@ -185,6 +241,27 @@ "example": "The Yew Invasion", "description": "The event title." }, + { + "name": "summary", + "type": "string", + "required": false, + "example": "Orcish warbands are massing north of Yew.", + "description": "The event’s one-line summary, when it has one." + }, + { + "name": "seriesName", + "type": "string", + "required": false, + "example": "The Yew Campaign", + "description": "The arc this event belongs to, when it belongs to one." + }, + { + "name": "timezone", + "type": "string", + "required": false, + "example": "America/New_York", + "description": "The zone the run was computed in — what a time in the body should be read as." + }, { "name": "eventUrl", "type": "url", @@ -219,6 +296,27 @@ "example": "The Yew Invasion", "description": "The event title." }, + { + "name": "summary", + "type": "string", + "required": false, + "example": "Orcish warbands are massing north of Yew.", + "description": "The event’s one-line summary, when it has one." + }, + { + "name": "seriesName", + "type": "string", + "required": false, + "example": "The Yew Campaign", + "description": "The arc this event belongs to, when it belongs to one." + }, + { + "name": "timezone", + "type": "string", + "required": false, + "example": "America/New_York", + "description": "The zone the run was computed in — what a time in the body should be read as." + }, { "name": "phase", "type": "string", @@ -272,7 +370,7 @@ "type": "string", "required": false, "example": "Orcish warbands are massing north of Yew.", - "description": "The event summary, as authored." + "description": "The event’s one-line summary, when it has one." }, { "name": "seriesName", @@ -281,6 +379,13 @@ "example": "The Yew Campaign", "description": "The arc this event belongs to, when it belongs to one." }, + { + "name": "timezone", + "type": "string", + "required": false, + "example": "America/New_York", + "description": "The zone the run was computed in — what a time in the body should be read as." + }, { "name": "startsAt", "type": "datetime", @@ -288,13 +393,6 @@ "example": "2026-09-12T20:00:00.000Z", "description": "When the occurrence is due to start, UTC." }, - { - "name": "timezone", - "type": "string", - "required": false, - "example": "America/New_York", - "description": "The shard-local zone the schedule was authored in." - }, { "name": "startsAtLabel", "type": "string", @@ -341,7 +439,7 @@ "type": "string", "required": false, "example": "Orcish warbands are massing north of Yew.", - "description": "The event summary, as authored." + "description": "The event’s one-line summary, when it has one." }, { "name": "seriesName", @@ -350,6 +448,13 @@ "example": "The Yew Campaign", "description": "The arc this event belongs to, when it belongs to one." }, + { + "name": "timezone", + "type": "string", + "required": false, + "example": "America/New_York", + "description": "The zone the run was computed in — what a time in the body should be read as." + }, { "name": "startsAt", "type": "datetime", @@ -357,13 +462,6 @@ "example": "2026-09-12T20:00:00.000Z", "description": "When it actually started, UTC." }, - { - "name": "timezone", - "type": "string", - "required": false, - "example": "America/New_York", - "description": "The shard-local zone the schedule was authored in." - }, { "name": "startsAtLabel", "type": "string", diff --git a/server/src/config/coreTriggers.js b/server/src/config/coreTriggers.js index 414ccb6..711c058 100644 --- a/server/src/config/coreTriggers.js +++ b/server/src/config/coreTriggers.js @@ -29,6 +29,29 @@ // test-send without a live game event, which is the reason template systems go // untested. +/** + * The three facts `events/announce.js` puts on EVERY `event.*` payload, declared + * once because they are spread into all seven. + * + * `baseFor()` has always computed them and nothing declared them, so + * `engagementEmit.validatePayload` dropped all three before a template could see + * one — they were absent from the variable list an author picks from, and every + * single event emit logged `emit carried undeclared variables`. Found by the + * Phase 16 acceptance walk, in the DEBUG line it had been writing all along. + * + * All three are optional, and each for its own reason rather than by default: an + * event need not carry a summary, most events belong to no series, and a run + * whose definition has been deleted resolves no zone. + */ +const EVENT_AMBIENT = [ + { name: 'summary', type: 'string', required: false, example: 'Orcish warbands are massing north of Yew.', + description: 'The event’s one-line summary, when it has one.' }, + { name: 'seriesName', type: 'string', required: false, example: 'The Yew Campaign', + description: 'The arc this event belongs to, when it belongs to one.' }, + { name: 'timezone', type: 'string', required: false, example: 'America/New_York', + description: 'The zone the run was computed in — what a time in the body should be read as.' }, +] + const TRIGGERS = [ { id: 'news.post', @@ -215,14 +238,9 @@ const TRIGGERS = [ description: 'The run this is about. Also the cooldown subject.' }, { name: 'title', type: 'string', required: true, example: 'The Yew Invasion', description: 'The event title.' }, - { name: 'summary', type: 'string', required: false, example: 'Orcish warbands are massing north of Yew.', - description: 'The event summary, as authored.' }, - { name: 'seriesName', type: 'string', required: false, example: 'The Yew Campaign', - description: 'The arc this event belongs to, when it belongs to one.' }, + ...EVENT_AMBIENT, { name: 'startsAt', type: 'datetime', required: true, example: '2026-09-12T20:00:00.000Z', description: 'When the occurrence is due to start, UTC.' }, - { name: 'timezone', type: 'string', required: false, example: 'America/New_York', - description: 'The shard-local zone the schedule was authored in.' }, // **A presentational fragment, and §4.6.1 convention 1 is what sanctions // one.** `startsAt` is a `datetime`, which the seam normalises to an ISO // string — correct as data and unreadable in a mail, and a template has no @@ -254,14 +272,9 @@ const TRIGGERS = [ description: 'The run this is about. Also the cooldown subject.' }, { name: 'title', type: 'string', required: true, example: 'The Yew Invasion', description: 'The event title.' }, - { name: 'summary', type: 'string', required: false, example: 'Orcish warbands are massing north of Yew.', - description: 'The event summary, as authored.' }, - { name: 'seriesName', type: 'string', required: false, example: 'The Yew Campaign', - description: 'The arc this event belongs to, when it belongs to one.' }, + ...EVENT_AMBIENT, { name: 'startsAt', type: 'datetime', required: true, example: '2026-09-12T20:00:00.000Z', description: 'When it actually started, UTC.' }, - { name: 'timezone', type: 'string', required: false, example: 'America/New_York', - description: 'The shard-local zone the schedule was authored in.' }, // **A presentational fragment, and §4.6.1 convention 1 is what sanctions // one.** `startsAt` is a `datetime`, which the seam normalises to an ISO // string — correct as data and unreadable in a mail, and a template has no @@ -293,6 +306,7 @@ const TRIGGERS = [ description: 'The run this is about. Also the cooldown subject.' }, { name: 'title', type: 'string', required: true, example: 'The Yew Invasion', description: 'The event title.' }, + ...EVENT_AMBIENT, { name: 'phase', type: 'string', required: true, example: 'assault', description: 'The phase key just entered, as authored in the spec.' }, { name: 'phaseLabel', type: 'string', required: false, example: 'The assault', @@ -323,6 +337,7 @@ const TRIGGERS = [ description: 'The run this is about. Also the cooldown subject.' }, { name: 'title', type: 'string', required: true, example: 'The Yew Invasion', description: 'The event title.' }, + ...EVENT_AMBIENT, // The public page for THIS occurrence (Phase 14a). Relative, like // `postUrl` and `runUrl`: the seam resolves it against the site's own // base, and an absolute one baked in here would be wrong on every @@ -345,8 +360,7 @@ const TRIGGERS = [ description: 'The run this is about. Also the cooldown subject.' }, { name: 'title', type: 'string', required: true, example: 'The Yew Invasion', description: 'The event title.' }, - { name: 'summary', type: 'string', required: false, example: 'Orcish warbands are massing north of Yew.', - description: 'The event summary, as authored.' }, + ...EVENT_AMBIENT, // Counted from `event_run_participants` at emit. Zero on a run whose // module reported nobody, which is every run until a module collects — // a template that says "47 took part" needs a number that is never @@ -377,6 +391,7 @@ const TRIGGERS = [ description: 'The run this is about. Also the cooldown subject.' }, { name: 'title', type: 'string', required: true, example: 'The Yew Invasion', description: 'The event title.' }, + ...EVENT_AMBIENT, // **The operator's reason, and not the run's `last_error`.** `cancel` // takes a `{ reason }` a human typed for other humans; a diagnostic // string is for the run console and would read as gibberish in a mail. @@ -408,6 +423,7 @@ const TRIGGERS = [ description: 'The run this is about. Also the cooldown subject.' }, { name: 'title', type: 'string', required: true, example: 'The Yew Invasion', description: 'The event title.' }, + ...EVENT_AMBIENT, { name: 'phase', type: 'string', required: false, example: 'assault', description: 'The phase it failed in, when it had entered one.' }, { name: 'error', type: 'string', required: false, example: 'sidecar responded 503', diff --git a/server/src/model/events/eventPublic.model.js b/server/src/model/events/eventPublic.model.js index 19a1c71..73a77fd 100644 --- a/server/src/model/events/eventPublic.model.js +++ b/server/src/model/events/eventPublic.model.js @@ -37,11 +37,19 @@ const participantsDb = require('./eventRunParticipants.db') const calendarModel = require('./eventCalendar.model') const recurrence = require('../../events/recurrence') -// The public calendar's window when a caller names neither end: now through a -// month out. A visitor arriving at /site/events wants "what is on", and a client -// that had to compute a window before it could ask anything would make every -// deep link carry two ISO instants. +// The public calendar's window when a caller names neither end: a few days BACK +// through a month out. A visitor arriving at /site/events wants "what is on", and +// a client that had to compute a window before it could ask anything would make +// every deep link carry two ISO instants. +// +// **The backward tail is not padding — it is the "recent" in §I's "upcoming, live +// and recent".** The default used to start at `now`, so an event that finished an +// hour ago was already gone and a visitor had nowhere to find the results of the +// thing they had just attended. The LIVE half is answered by `listInWindow`'s +// overlap test rather than by this number, so the tail only has to be long enough +// to be a "recently" a reader would recognise. const DEFAULT_WINDOW_DAYS = 31 +const DEFAULT_RECENT_DAYS = 7 // How many past occurrences an event page carries. It shows what is next and // what happened recently; the whole history of a three-year-old weekly event is @@ -146,8 +154,15 @@ const publicProjectedEntry = (definition, occurrence) => ({ * surface that has no login in front of it. */ async function calendar({ from, to, seriesId = null, now = new Date() } = {}) { - const start = from ? new Date(from) : new Date(now) - const end = to ? new Date(to) : new Date(start.getTime() + DEFAULT_WINDOW_DAYS * recurrence.DAY_MS) + // The default `to` is measured from NOW, not from `start` — otherwise the + // backward tail would silently push the horizon a week further out and a caller + // naming only `from` would get a different span than one naming neither. + const start = from + ? new Date(from) + : new Date(new Date(now).getTime() - DEFAULT_RECENT_DAYS * recurrence.DAY_MS) + const end = to + ? new Date(to) + : new Date(new Date(now).getTime() + DEFAULT_WINDOW_DAYS * recurrence.DAY_MS) if (Number.isNaN(start.getTime()) || Number.isNaN(end.getTime())) { return { ok: false, status: 400, errors: ['from and to must be dates'] } @@ -183,7 +198,22 @@ async function calendar({ from, to, seriesId = null, now = new Date() } = {}) { if (!schedule || schedule.kind === 'manual') continue let occurrences = [] try { - occurrences = recurrence.occurrencesBetween(schedule, definition.timezone || 'UTC', start, end) + // **Forecast from `now`, never from `start`.** The default window now + // reaches a week backwards so that "recent" has somewhere to live, and a + // projection into that tail would advertise an occurrence that did not + // happen — a run that WAS created is a real row and arrives above, and one + // that was not is a slot the runner has already passed. A forecast is about + // the future; the tail is about the past. Only the materialised half fills + // it. + const forecastFrom = start > now ? start : new Date(now) + if (forecastFrom < end) { + occurrences = recurrence.occurrencesBetween( + schedule, + definition.timezone || 'UTC', + forecastFrom, + end, + ) + } } catch { // A version whose schedule the recurrence engine will not read is one the // runner will not expand either. The calendar then shows that definition's @@ -405,5 +435,6 @@ module.exports = { publicStatus, phaseLabel, DEFAULT_WINDOW_DAYS, + DEFAULT_RECENT_DAYS, PAST_RUNS, } diff --git a/server/src/model/events/eventRunResources.db.js b/server/src/model/events/eventRunResources.db.js index 9f11af4..badd823 100644 --- a/server/src/model/events/eventRunResources.db.js +++ b/server/src/model/events/eventRunResources.db.js @@ -194,18 +194,54 @@ async function unresolvedCounts(runIds) { } /** - * Claim one row for a revert: `pending | confirmed | orphaned | drifted → reverting`. + * Claim one row for a revert: `pending | confirmed | orphaned | drifted → reverting`, + * and `reverting` again once the claim on it has gone stale. * * The compare-and-set that keeps the cleanup leg and the manual cleanup route off - * each other's rows. `reverting` is deliberately not claimable — a row another - * pass is mid-revert on is left alone, exactly as a step with a live claim is. + * each other's rows. A row another pass is mid-revert on is left alone, exactly as + * a step with a live claim is. + * + * **"Exactly as a step" has to include the expiry, and it did not until the Phase + * 16 acceptance walk.** A step's claim carries `claim_expires_at`, so a step whose + * process died is reclaimed once the lease lapses — that reclaim is the whole + * reason §E's CAS survives §N4's single instance. A `reverting` row had no such + * bound and nothing released it, so a process killed mid-teardown stranded the row + * for good: the sweep skipped it every 15s forever, `cleanup_status` never left + * `pending`, and `POST …/cleanup` — the recourse §I names — answered 200 and did + * nothing, because it claims through this same function. Observed with a lease, + * which then blocked the NEXT run of the same event from taking the value. + * + * The stale test is `updated_at`, not a new column: the row is stamped exactly + * when it enters `reverting` and is not written again until the revert resolves, + * so for a `reverting` row `updated_at` IS "when this claim was taken". The bound + * is the run lease's, for the run lease's reason — it has to outlast a whole + * tick's work on one run, and every revert in a sweep is bounded by its action's + * own `budgetMs` long before this. + * + * `revert_attempts` is deliberately NOT incremented by reclaiming. A stale claim + * is a process that died, not an attempt that failed, and counting it would burn + * the retry budget on crashes — Engagement Phase 14's rule, one table over. + * + * **`updated_at` is re-stamped explicitly, and that is what keeps this a CAS.** + * This connector sends `CLIENT_FOUND_ROWS`, so `affectedRows` counts rows MATCHED + * rather than changed. For the four fresh statuses that is harmless — the winner + * moves the row to `reverting` and the loser's `status IN (…)` no longer matches. + * A stale `reverting` row has no such natural change: without re-stamping, the + * row would still satisfy `status = 'reverting' AND updated_at < …` and a second + * claimer would match it too. Writing the column is what makes the second one + * miss. */ +const REVERT_CLAIM_TTL_MS = Number(process.env.EVENT_REVERT_CLAIM_TTL_MS) || 15 * 60 * 1000 + async function claimRevert(id) { const result = await query( `UPDATE event_run_resources - SET status = 'reverting' - WHERE id = ? AND status IN ('pending', 'confirmed', 'orphaned', 'drifted')`, - [id], + SET status = 'reverting', updated_at = NOW() + WHERE id = ? + AND (status IN ('pending', 'confirmed', 'orphaned', 'drifted') + OR (status = 'reverting' + AND updated_at < (NOW() - INTERVAL ? MICROSECOND)))`, + [id, REVERT_CLAIM_TTL_MS * 1000], ) return (result.affectedRows || 0) > 0 } diff --git a/server/src/model/events/eventRuns.db.js b/server/src/model/events/eventRuns.db.js index 0825f4f..e333dfb 100644 --- a/server/src/model/events/eventRuns.db.js +++ b/server/src/model/events/eventRuns.db.js @@ -108,14 +108,32 @@ const materialise = async (run) => { } /** - * Every run whose instant falls inside a window — the calendar's real half. + * Every run whose OCCUPIED INTERVAL overlaps a window — the calendar's real half. * * Ascending, unlike the admin run list: a calendar is read forwards. The join * reaches the series so a month can be filtered to one arc without a second * round trip, and `d.timezone` is NOT what comes back — `r.timezone` is, because * a run records the zone it was COMPUTED in and a definition's zone can be * edited afterwards. + * + * **A run OVERLAPS the window; it does not merely START in it.** This asked + * `scheduled_for >= from` alone until the Phase 16 acceptance walk, and a run is + * not an instant — it is an interval, and a multi-phase event's whole point is + * that the interval is long. A run that began before `from` and has not ended is + * happening DURING the window and belongs in it. With the instant test, the + * public calendar answered `entries: []` while that same event's own page said + * `live: true`, so the site disagreed with itself about whether something was on + * — and `EVENTS.md` §I promises this route serves "upcoming, **live** and + * recent". The admin calendar had the same hole for the same reason: a run that + * started last Sunday and is still going was missing from "this week". + * + * A finished run needs no clause: it is `recent` only if its instant is in the + * window, which is what the window's own `from` decides (see + * `eventPublic.model.calendar`, which backdates its default `from` so that + * "recent" has somewhere to live). */ +const LIVE_STATUSES = ['starting', 'running', 'paused', 'ending'] + const listInWindow = async ({ from, to, @@ -125,8 +143,11 @@ const listInWindow = async ({ limit = 500, publicOnly = false, } = {}) => { - const where = ['r.scheduled_for >= ?', 'r.scheduled_for < ?'] - const args = [from, to] + const where = [ + `((r.scheduled_for >= ? AND r.scheduled_for < ?) + OR (r.scheduled_for < ? AND r.status IN (${LIVE_STATUSES.map(() => '?').join(',')})))`, + ] + const args = [from, to, to, ...LIVE_STATUSES] if (status) { where.push('r.status = ?') args.push(status) diff --git a/server/test/eventPublic.test.js b/server/test/eventPublic.test.js index 17119c5..b5758e2 100644 --- a/server/test/eventPublic.test.js +++ b/server/test/eventPublic.test.js @@ -115,10 +115,15 @@ function installStubs() { .filter((d) => d.state === 'ready' && (!listedOnly || d.listed)) .map((d) => ({ ...d, version_spec: d.spec })) + // Mirrors the OVERLAP predicate the real statement uses: a run is in the window + // if its instant falls inside it, OR if it began before the window and is still + // live. A run is an interval, not an instant — see `eventRuns.db.listInWindow`. runsDb.listInWindow = async ({ from, to, publicOnly = false }) => store.runs.filter((r) => { const at = new Date(r.scheduled_for) - if (at < from || at >= to) return false + const startsInside = at >= from && at < to + const liveAcross = at < to && ['starting', 'running', 'paused', 'ending'].includes(r.status) + if (!startsInside && !liveAcross) return false if (!publicOnly) return true const d = store.definitions.find((x) => x.id === r.definition_id) return !r.rehearsal && d && d.listed && d.state !== 'archived' @@ -163,12 +168,89 @@ test('a calendar entry carries no operational field at all', async () => { ]) }) -test('the calendar defaults to a month from now when no window is given', async () => { +test('the default window reaches back as well as forward', async () => { + // §I: this route is "upcoming, live and recent". The default used to start at + // `now`, which left no room for the third word — an event that finished an hour + // ago was already gone, so a visitor had nowhere to find the results of the + // thing they had just attended (Phase 16 walk). const result = await publicModel.calendar({ now: NOW }) assert.equal(result.ok, true) - assert.equal(new Date(result.window.from).getTime(), NOW.getTime()) - const days = (new Date(result.window.to) - new Date(result.window.from)) / 86_400_000 - assert.equal(days, publicModel.DEFAULT_WINDOW_DAYS) + const back = (NOW - new Date(result.window.from)) / 86_400_000 + const forward = (new Date(result.window.to) - NOW) / 86_400_000 + assert.equal(back, publicModel.DEFAULT_RECENT_DAYS) + assert.equal(forward, publicModel.DEFAULT_WINDOW_DAYS) +}) + +test('a run happening RIGHT NOW is on the calendar, whenever it started', async () => { + // The defect this pair was written for: the site said `live: true` on the + // event's own page and served `entries: []` from the calendar, because the + // window test read the START instant and a live run had already started. A run + // is an interval; the calendar asks which intervals overlap it. + store.runs = [ + { + ...run({ + status: 'running', + // Well before any default window would begin. + scheduled_for: new Date('2026-08-01T00:00:00Z'), + ended_at: null, + }), + definition_title: 'The Yew Invasion', + definition_slug: 'the-yew-invasion', + }, + ] + const result = await publicModel.calendar({ now: NOW }) + assert.equal(result.ok, true) + const entry = result.entries.find((e) => e.kind === 'run') + assert.ok(entry, 'a live run must appear however long ago it began') + assert.equal(entry.live, true) + assert.equal(entry.status, 'live') +}) + +test('a run that finished inside the recent tail is still on the calendar', async () => { + store.runs = [ + { + ...run({ + status: 'completed', + scheduled_for: new Date(NOW.getTime() - 2 * 86_400_000), + ended_at: new Date(NOW.getTime() - 2 * 86_400_000 + 3_600_000), + }), + definition_title: 'The Yew Invasion', + definition_slug: 'the-yew-invasion', + }, + ] + const result = await publicModel.calendar({ now: NOW }) + assert.equal(result.ok, true) + assert.equal(result.entries.filter((e) => e.kind === 'run').length, 1) +}) + +test('nothing is FORECAST into the recent tail', async () => { + // The tail is for what happened, and only the materialised half fills it. A + // projection into the past would advertise an occurrence that did not happen: + // one that WAS created is a real row and arrives as a run, and one that was not + // is a slot the runner has already gone past. + store.definitions = [ + definition({ + spec: { + ...SPEC, + schedule: { + kind: 'weekly', + days: ['monday', 'tuesday', 'wednesday', 'thursday', 'friday', 'saturday', 'sunday'], + time: '20:00', + }, + }, + }), + ] + store.runs = [] + const result = await publicModel.calendar({ now: NOW }) + assert.equal(result.ok, true) + const projected = result.entries.filter((e) => e.kind !== 'run') + assert.ok(projected.length > 0, 'a daily schedule must still forecast forwards') + for (const entry of projected) { + assert.ok( + new Date(entry.scheduledFor) >= NOW, + `forecast ${entry.scheduledFor} is before now — the tail must hold no projections`, + ) + } }) test('a window wider than the cap is refused rather than served slowly', async () => { diff --git a/server/test/eventRunnerSql.test.js b/server/test/eventRunnerSql.test.js index ce69605..9cb4cfa 100644 --- a/server/test/eventRunnerSql.test.js +++ b/server/test/eventRunnerSql.test.js @@ -1448,6 +1448,77 @@ test('many released rows on one target coexist, which is the whole encoding', as assert.equal(await dup(() => insertResource(a.runId, { kind: 'override', ref: 'demo.rate' })), null) }) +// ── claimRevert's stale-claim reclaim (Phase 16) ─────────────────────────── +// +// The statement's own comment explains WHY a `reverting` row must be reclaimable; +// this proves it against a real server, because every part of the answer is the +// server's: `NOW() - INTERVAL … MICROSECOND`, whether `ON UPDATE` re-stamps, and +// above all what `affectedRows` counts. This connector sends `CLIENT_FOUND_ROWS`, +// so it counts rows MATCHED — a stub counting CHANGED rows would call the reclaim +// a failure, and one counting matched rows would miss that the second claimer +// needs the re-stamp in order to lose. Only MariaDB settles it. + +const CLAIM_REVERT = ` + UPDATE event_run_resources + SET status = 'reverting', updated_at = NOW() + WHERE id = ? + AND (status IN ('pending', 'confirmed', 'orphaned', 'drifted') + OR (status = 'reverting' + AND updated_at < (NOW() - INTERVAL ? MICROSECOND)))` + +const claimRevert = async (id, ttlMs) => + Number((await pool.query(CLAIM_REVERT, [id, ttlMs * 1000]))?.affectedRows || 0) > 0 + +const ageResource = (id, seconds) => + pool.query('UPDATE event_run_resources SET updated_at = NOW() - INTERVAL ? SECOND WHERE id = ?', [ + seconds, + id, + ]) + +test('a reverting row whose claim has gone stale is claimable again', async (t) => { + if (needDb(t)) return + // The Phase 16 walk's finding: a process killed mid-teardown leaves the row in + // `reverting` and nothing releases it. The sweep ran every 15s for ever finding + // nothing it could claim, `cleanup_status` never left `pending`, and the manual + // retry answered 200 while doing nothing — it claims through this statement too. + const run = await seedRun() + const id = await insertResource(run.runId, { status: 'reverting' }) + + // Fresh: somebody else really is mid-revert on it. Left alone. + assert.equal(await claimRevert(id, 900_000), false) + + // Stale: the holder is not coming back. + await ageResource(id, 1800) + assert.equal(await claimRevert(id, 900_000), true) +}) + +test('reclaiming re-stamps, so the second claimer of one stale row loses', async (t) => { + if (needDb(t)) return + // Under CLIENT_FOUND_ROWS the four fresh statuses need no re-stamp — the winner + // moves the row out of `status IN (…)` and the loser stops matching. A stale + // `reverting` row has no such natural change, so without writing `updated_at` + // BOTH claimers would match it and two passes would revert the same resource. + const run = await seedRun() + const id = await insertResource(run.runId, { status: 'reverting' }) + await ageResource(id, 1800) + + assert.equal(await claimRevert(id, 900_000), true) + assert.equal(await claimRevert(id, 900_000), false, 'the re-stamp must make the second miss') +}) + +test('reclaiming a stale revert does not spend a retry attempt', async (t) => { + if (needDb(t)) return + // A stale claim is a process that died, not an attempt that failed. Counting it + // would burn MAX_REVERT_ATTEMPTS on crashes — Engagement Phase 14's rule, one + // table over. + const run = await seedRun() + const id = await insertResource(run.runId, { status: 'reverting' }) + await ageResource(id, 1800) + await claimRevert(id, 900_000) + const [row] = await pool.query('SELECT revert_attempts FROM event_run_resources WHERE id = ?', [id]) + assert.equal(Number(row.revert_attempts), 0) +}) + test('an UPDATE that releases a row frees the target at once', async (t) => { if (needDb(t)) return // The generated column is STORED, so this is really asking whether MariaDB