fix(events): the public calendar, a stranded revert, and three dropped facts (Phase 16a)
Some checks failed
PR Checks / client-build (pull_request) Successful in 34s
PR Checks / bot-tests (pull_request) Successful in 33s
PR Checks / server-tests (pull_request) Failing after 5m47s

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/<slug>` 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wDDVXWMDz82WqE1i969r4
This commit is contained in:
2026-09-09 08:28:17 -05:00
parent af9f4e191c
commit 6dd4e5e3eb
7 changed files with 407 additions and 52 deletions

View File

@@ -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
}