[canonical-hours-25ff14] fix(worker): guard the tick with a DO-backed lease - #31
Merged
Merged
Conversation
… lease
runTick's overlap guard is a module-scope `running` flag, so it protects
only against concurrency within one isolate. Cloudflare dispatches
concurrent requests across many isolates, each with its own unset flag, so
two `POST /tick` calls (or the MCP `trigger_tick` tool) can both proceed.
Two corrections to the bead's framing:
1. It assumed Vercel Cron. There is no Vercel deployment — the project is
eve-compliant but runs on Cloudflare, developed locally via workerd. Its
close-condition (a), "confirm Vercel Cron reuses a warm process", can
never be satisfied.
2. It filed this as deferred until deployment. It is not: worker/index.ts
already calls runTick in the worker isolate from two live trigger paths.
Earlier research on the bead concluded a fix "needs new external infra
(Redis/Vercel KV/a DB) that this project doesn't have". That was sound for
a Vercel target but does not hold here: CanonicalHoursBoardObject is a
Durable Object that already owns the board, and a DO is single-threaded per
object id, so its check-and-set is atomic across isolates. Nothing to
provision. It also works in the local workerd loop, since cloister supports
DO namespaces with localDisk storage.
The lease is taken on the SAME object id the board store uses
(idFromName("default")), so it lives on the DO it guards rather than a
second instance, and is released in a `finally` so a throw from
configFromEnv/buildSources cannot strand it.
It expires. An isolate evicted mid-tick can never release, and a
permanently-held lock would convert an overlap bug into a total stall —
strictly the worse failure. Expiry is evaluated on read, not by alarm, so a
dead holder cannot keep its own lease alive.
The decision is a pure function in agent/lib/tick-lease.ts rather than
inline in the DO method: it holds the edge cases worth testing, and
worker/index.ts imports `cloudflare:workers`, which the node test runner
cannot load. 6 tests cover unset/held/expired, the exact-expiry boundary, a
backwards clock jump, and that the TTL leaves a slow tick room to finish.
The module-scope flag stays: it is still the cheapest correct guard for
same-isolate concurrency, and vespers is consumed outside this worker.
272/272 tests pass (266 on main plus these 6); tsc clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the overlap gap in
runTickfor the deployment this project actually has.What the bead got wrong
It was filed against Vercel Cron, and deferred until "whoever picks up the deployment tasks". Two corrections:
worker/index.tsalready callsrunTickin the worker isolate from two trigger paths:POST /tickand the MCPtrigger_ticktool.runTick's guard is a module-scoperunningflag, which lives per isolate — Cloudflare can dispatch two concurrent calls to two isolates, each seeing its own unset flag.Prior research on the bead concluded a fix "needs new external infra (Redis/Vercel KV/a DB) that this project doesn't have". Sound for a Vercel target; not true here.
The fix
CanonicalHoursBoardObjectis a Durable Object that already owns the board. A DO is single-threaded per object id, so its check-and-set is atomic across every isolate — a distributed lock already deployed, nothing to provision. It also works in the local workerd loop, since cloister supports DO namespaces withdurableObjectStorage = (localDisk = ...).Details that matter:
idFromName("default")), so it lives on the DO it guards rather than a second instance.finally—runTickabsorbs its own errors, but a throw fromconfigFromEnv/buildSourceswould otherwise strand it."skipped_overlap", the same valuerunTickuses for the in-process case, so callers can't tell which guard fired.Testing
The decision lives in
agent/lib/tick-lease.tsas a pure function rather than inline in the DO method: it holds the edge cases, andworker/index.tsimportscloudflare:workers, which the node runner can't load. 6 tests cover unset/held/expired, the exact-expiry boundary, a backwards clock jump, and that the TTL leaves a slow tick room to finish.272/272 tests pass (main's 266 plus these 6),
tscclean.Note
Does not touch
canonical-hours-07c93c. PR #30 is open and doing that ledger work; this shares only the observation that the DO is the right primitive for both.🤖 Generated with Claude Code