Skip to content

Commit c979f6a

Browse files
chitcommitchitcommit
andauthored
feat(meta): DO-based role arbiter replacing Neon cc_node_leases (#131)
* feat(meta): DO-based role arbiter replacing Neon cc_node_leases ADR-001 elected a meta-orchestrator leader across chittymini-01..06 using Neon `cc_node_leases`. Two problems with that in practice: - The daemon runs nowhere. No systemd unit, no daemon/loop process on chittyserv-vm. The Worker plane being up masks it: the interface is live while the always-on coordinator is not. - The fleet it floats across is 6/7 offline (chittymini-01..06 last seen ~7d). chittymini-00 is the operator seat and must not hold persistent infra, so a leader designed to float has nowhere to float. A Durable Object is already a strongly-consistent, single-threaded singleton. Using one does not reimplement leader election — it removes the need for it, along with the fleet dependency and the Neon lease table. Neon cost pressure makes this favourable; it would be the right shape regardless. CommandCoordinator (meta/coordinator.ts) arbitrates role leases in DO storage. daemon/coordinator-lease.ts is a drop-in client exporting the same four functions with identical signatures, so daemon/loop.ts switches by changing one import. It fails closed with POLICY_BLOCKED_COORDINATOR_UNAVAILABLE rather than making an unarbitrated local decision, which would permit split-brain. Wire semantics are a deliberate 1:1 port of the SQL, preserving both prior review findings: session ownership required on heartbeat (codex-p2 PR#101 finding-5) and on release (finding-2). Nodes keep a role as executors that pull work — justified by needing local filesystem and repo access. Leader election never was that reason. Tests: 13 cases in real workerd against real DO storage, no mocks. Covers exclusion, idempotent re-claim, expiry takeover, both session-ownership rejections, heartbeat extension, release/reclaim, role isolation, lease clamping, and validation. Scope: daemon/leader.ts and the Tier-5 Neon usage are untouched. Migrating those is a separate decision that should be made on spend data. * fix(meta): address adversarial review of the DO coordinator Separated review (silent-failure-hunter, fresh context) found three real defects plus a vacuous test. Split-brain, SQL semantic parity, and auth came back clean and are unchanged. P1 — daemon/coordinator-lease.ts could not load on Node. It imported META_LEADER_ROLE (a value) from meta/coordinator.ts, which evaluates `cloudflare:workers`. The documented one-line switch in daemon/loop.ts would have crashed the daemon at module load, before main() and before any log line: no leader claimed, intent queue silently stopped. tsc passed because the failure is runtime/bundle-only. Extracted the runtime-free meta/lease-types.ts; both sides import from it. Verified: `esbuild --platform=node` fails on the old code with `Could not resolve "cloudflare:workers"` and succeeds on the new. P2 — fail-open on partial config. COORDINATOR_URL set with no COORDINATOR_TOKEN sent unauthenticated requests, drawing a 401 that loop.ts logs as a claim error and retries forever — a permanently dead daemon whose logs read like a transient auth blip. Both values are now required, failing closed with POLICY_BLOCKED_COORDINATOR_UNAVAILABLE. P3 — greedy path regex. /^.*\/coordinator/ matched the LAST occurrence, so `/api/meta/coordinator/a/coordinator/release` dispatched `release` from a path that does not name it. Anchored on the first segment. Also: a corrupted leaseExpiresAt parsed to NaN, and every NaN comparison is false, leaving the role permanently unclaimable — a fail-closed deadlock with no SQL analogue, since Postgres typed the column. Unparseable now reads as expired. Tests 13 → 24. The review's sharpest point was that the suite covered the class and skipped the seam where P1 lived, so this adds a full HTTP surface suite: path parsing, method switch, 404s, malformed-body 400, the bare-null describe signal, and the {released} envelope the client unwraps. Plus MIN-clamp, release-by-other-node, omitted-vs-null sessionId, and corrupt- expiry recovery. `extends the expiry on a valid heartbeat` was vacuous — it claimed at 1s and heartbeat at 60s, so the assertion held by construction and would have passed against an implementation computing expiry from claimedAt. Rewritten to use identical leaseSeconds on both calls. Both new regression tests were mutation-checked: reverting each fix makes exactly that test fail. Kept on review advice: claimedAt preserved across takeover. Not a defect — it reads as "when this role was first continuously held" — and it is load-bearing, since the client returns null on falsy claimedAt. Comment corrected to say so. Not fixed here (pre-existing, outside this diff): daemon/loop.ts:277-295 logs exec_heartbeat_lost on takeover without aborting the in-flight dispatch, so a demoted leader finishes its current intent alongside the new one. That is the one real split-brain path and it is unreachable from the DO. --------- Co-authored-by: chitcommit <nb@chitty.cc>
1 parent d1ac49a commit c979f6a

10 files changed

Lines changed: 1606 additions & 12 deletions

File tree

daemon/coordinator-lease.ts

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
/**
2+
* Coordinator-backed lease client — drop-in replacement for daemon/leader.ts.
3+
*
4+
* Exports the same four functions with the same signatures and return shapes,
5+
* so daemon/loop.ts switches by changing one import path. The arbiter moves
6+
* from Neon `cc_node_leases` to the CommandCoordinator Durable Object
7+
* (meta/coordinator.ts).
8+
*
9+
* Why: ADR-001 used Neon leases to elect a leader across chittymini-01..06.
10+
* A DO is already a strongly-consistent singleton, so election is unnecessary —
11+
* and the Neon dependency, which is cost-driven pressure, leaves this layer.
12+
*
13+
* Fails closed. With no coordinator configured this throws rather than
14+
* degrading to an unarbitrated local decision, which would permit split-brain.
15+
*
16+
* @canonical-uri chittycanon://docs/architecture/chittycommand/ADR-001
17+
*/
18+
19+
// Import from lease-types, NOT meta/coordinator: the latter evaluates
20+
// `cloudflare:workers`, which does not resolve on Node and would crash the
21+
// daemon at module load, before main() and before any log line.
22+
import { META_LEADER_ROLE, type StoredLease } from '../meta/lease-types';
23+
24+
export { META_LEADER_ROLE };
25+
26+
export const POLICY_BLOCKED_COORDINATOR_UNAVAILABLE =
27+
'POLICY_BLOCKED_COORDINATOR_UNAVAILABLE';
28+
29+
export interface CoordinatorEnv {
30+
/** Base URL of the ChittyCommand worker, e.g. https://command.chitty.cc */
31+
COORDINATOR_URL?: string;
32+
/** Bearer token for the coordinator routes. Broker-provided; never inlined. */
33+
COORDINATOR_TOKEN?: string;
34+
}
35+
36+
/** Identical to NodeLease in daemon/leader.ts. */
37+
export interface NodeLease {
38+
role: string;
39+
nodeId: string;
40+
nodeDescriptor: string | null;
41+
sessionId: string | null;
42+
claimedAt: Date;
43+
heartbeatAt: Date;
44+
leaseExpiresAt: Date;
45+
metadata: Record<string, unknown>;
46+
}
47+
48+
export interface ClaimOptions {
49+
nodeId: string;
50+
nodeDescriptor?: string;
51+
sessionId?: string;
52+
leaseSeconds?: number;
53+
role?: string;
54+
metadata?: Record<string, unknown>;
55+
}
56+
57+
/**
58+
* Both the URL and the token are required. A configured URL with no token
59+
* yields a 401 on every call, which loop.ts logs as a claim error and retries
60+
* forever — a permanently dead daemon whose logs read like a transient auth
61+
* blip. Fail closed on the config error instead, with a distinguishable code.
62+
*/
63+
function requireConfig(env: CoordinatorEnv): { url: string; token: string } {
64+
const url = env.COORDINATOR_URL?.replace(/\/+$/, '');
65+
if (!url) throw new Error(POLICY_BLOCKED_COORDINATOR_UNAVAILABLE);
66+
if (!env.COORDINATOR_TOKEN) throw new Error(POLICY_BLOCKED_COORDINATOR_UNAVAILABLE);
67+
return { url, token: env.COORDINATOR_TOKEN };
68+
}
69+
70+
async function call<T>(
71+
env: CoordinatorEnv,
72+
method: 'GET' | 'POST',
73+
path: string,
74+
body?: unknown,
75+
): Promise<T> {
76+
const { url, token } = requireConfig(env);
77+
const headers: Record<string, string> = {
78+
'content-type': 'application/json',
79+
authorization: `Bearer ${token}`,
80+
};
81+
82+
const res = await fetch(`${url}/api/meta/coordinator${path}`, {
83+
method,
84+
headers,
85+
...(body === undefined ? {} : { body: JSON.stringify(body) }),
86+
});
87+
88+
if (!res.ok) {
89+
throw new Error(
90+
`[daemon/coordinator-lease] ${method} ${path} failed: ${res.status} ${await res.text()}`,
91+
);
92+
}
93+
return (await res.json()) as T;
94+
}
95+
96+
/** Rehydrate ISO strings into Dates. Returns null for an unheld lease. */
97+
function toLease(stored: StoredLease | null): NodeLease | null {
98+
if (!stored?.nodeId || !stored.claimedAt || !stored.heartbeatAt || !stored.leaseExpiresAt) {
99+
return null;
100+
}
101+
return {
102+
role: stored.role,
103+
nodeId: stored.nodeId,
104+
nodeDescriptor: stored.nodeDescriptor,
105+
sessionId: stored.sessionId,
106+
claimedAt: new Date(stored.claimedAt),
107+
heartbeatAt: new Date(stored.heartbeatAt),
108+
leaseExpiresAt: new Date(stored.leaseExpiresAt),
109+
metadata: stored.metadata ?? {},
110+
};
111+
}
112+
113+
export async function claimLeadership(
114+
env: CoordinatorEnv,
115+
options: ClaimOptions,
116+
): Promise<NodeLease | null> {
117+
if (!options?.nodeId) throw new Error('[daemon/coordinator-lease] nodeId is required');
118+
return toLease(
119+
await call<StoredLease | null>(env, 'POST', '/claim', {
120+
nodeId: options.nodeId,
121+
nodeDescriptor: options.nodeDescriptor ?? null,
122+
sessionId: options.sessionId ?? null,
123+
leaseSeconds: options.leaseSeconds,
124+
role: options.role,
125+
metadata: options.metadata ?? {},
126+
}),
127+
);
128+
}
129+
130+
export async function heartbeat(
131+
env: CoordinatorEnv,
132+
nodeId: string,
133+
options: { role?: string; leaseSeconds?: number; sessionId?: string | null } = {},
134+
): Promise<NodeLease | null> {
135+
if (!nodeId) throw new Error('[daemon/coordinator-lease] nodeId is required for heartbeat');
136+
return toLease(
137+
await call<StoredLease | null>(env, 'POST', '/heartbeat', {
138+
nodeId,
139+
role: options.role,
140+
leaseSeconds: options.leaseSeconds,
141+
sessionId: options.sessionId ?? null,
142+
}),
143+
);
144+
}
145+
146+
export async function releaseLeadership(
147+
env: CoordinatorEnv,
148+
nodeId: string,
149+
options: { role?: string; sessionId?: string | null } = {},
150+
): Promise<boolean> {
151+
if (!nodeId) throw new Error('[daemon/coordinator-lease] nodeId is required for release');
152+
const res = await call<{ released: boolean }>(env, 'POST', '/release', {
153+
nodeId,
154+
role: options.role,
155+
sessionId: options.sessionId ?? null,
156+
});
157+
return res.released === true;
158+
}
159+
160+
export async function describeLease(
161+
env: CoordinatorEnv,
162+
options: { role?: string } = {},
163+
): Promise<NodeLease | null> {
164+
const role = options.role ?? META_LEADER_ROLE;
165+
return toLease(
166+
await call<StoredLease | null>(
167+
env,
168+
'GET',
169+
`/describe?role=${encodeURIComponent(role)}`,
170+
),
171+
);
172+
}

meta/coordinator.ts

Lines changed: 221 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,221 @@
1+
/**
2+
* CommandCoordinator — Durable Object arbiter for meta-orchestrator role leases.
3+
*
4+
* Replaces the Neon `cc_node_leases` table (daemon/leader.ts) as the *arbiter*
5+
* of who holds a role. A Durable Object is already a strongly-consistent,
6+
* single-threaded singleton, so the properties the Neon lease bought us come
7+
* free:
8+
*
9+
* - mutual exclusion: the DO serializes requests; no two claimers race
10+
* - durability: DO storage survives eviction and restart
11+
* - availability: runs on the Cloudflare edge, not on operator hardware
12+
*
13+
* ADR-001 elected a leader across `chittymini-01..06` via Neon leases so the
14+
* coordinator would never be down. That mechanism is unnecessary here — there
15+
* is no fleet to elect across, because the DO *is* the leader. Nodes remain
16+
* meaningful as executors that pull work (they have local filesystem and repo
17+
* access, which is the real reason to run on hardware); they are no longer
18+
* candidates for leadership.
19+
*
20+
* Wire semantics are a deliberate 1:1 port of the SQL in daemon/leader.ts,
21+
* including two behaviours established by prior review:
22+
* - heartbeat requires (role, nodeId, sessionId) to match the holder
23+
* (codex-p2 PR#101 finding-5)
24+
* - release requires the same triple (codex-p2 PR#101 finding-2)
25+
*
26+
* @canonical-uri chittycanon://docs/architecture/chittycommand/ADR-001
27+
* @canon chittycanon://gov/governance#core-types — a node is a Location (L);
28+
* a lease claim is an Event (E).
29+
*/
30+
31+
import { DurableObject } from 'cloudflare:workers';
32+
33+
import {
34+
DEFAULT_LEASE_SECONDS,
35+
MAX_LEASE_SECONDS,
36+
META_LEADER_ROLE,
37+
MIN_LEASE_SECONDS,
38+
normalizeLeaseSeconds,
39+
type ClaimBody,
40+
type StoredLease,
41+
} from './lease-types';
42+
43+
// Re-exported so existing importers of this module keep working. The
44+
// definitions live in lease-types.ts because the daemon runs on Node and
45+
// cannot evaluate `cloudflare:workers`.
46+
export {
47+
DEFAULT_LEASE_SECONDS,
48+
MAX_LEASE_SECONDS,
49+
META_LEADER_ROLE,
50+
MIN_LEASE_SECONDS,
51+
normalizeLeaseSeconds,
52+
};
53+
export type { ClaimBody, StoredLease };
54+
55+
/** Storage key for a role's lease. */
56+
const keyFor = (role: string) => `lease:${role}`;
57+
58+
export class CommandCoordinator extends DurableObject {
59+
async fetch(request: Request): Promise<Response> {
60+
const url = new URL(request.url);
61+
// Anchor on the FIRST '/coordinator' segment. A greedy match here let
62+
// `/api/meta/coordinator/a/coordinator/release` dispatch `release`.
63+
const marker = '/coordinator';
64+
const at = url.pathname.indexOf(marker);
65+
const path = at === -1 ? '/' : url.pathname.slice(at + marker.length) || '/';
66+
67+
try {
68+
switch (`${request.method} ${path}`) {
69+
case 'POST /claim':
70+
return json(await this.claim(await readJson<ClaimBody>(request)));
71+
case 'POST /heartbeat':
72+
return json(await this.heartbeat(await readJson(request)));
73+
case 'POST /release':
74+
return json({ released: await this.release(await readJson(request)) });
75+
case 'GET /describe':
76+
return json(await this.describe(url.searchParams.get('role') ?? META_LEADER_ROLE));
77+
default:
78+
return json({ error: 'not_found', path }, 404);
79+
}
80+
} catch (err) {
81+
const message = err instanceof Error ? err.message : String(err);
82+
return json({ error: 'coordinator_error', message }, 400);
83+
}
84+
}
85+
86+
private async read(role: string): Promise<StoredLease | undefined> {
87+
return this.ctx.storage.get<StoredLease>(keyFor(role));
88+
}
89+
90+
/**
91+
* Claim `role`. Succeeds when the role is unheld, already held by this same
92+
* node, or the incumbent's lease has expired.
93+
*
94+
* Parity note: `claimedAt` is preserved across takeover, mirroring
95+
* `COALESCE(claimed_at, NOW())` in the SQL. That means a takeover from an
96+
* expired holder reports the *previous* holder's claim time. This is
97+
* deliberate and is not a defect: `claimedAt` reads as "when this role was
98+
* first continuously held", which is a coherent semantic.
99+
*
100+
* It is also load-bearing, not diagnostic — daemon/coordinator-lease.ts
101+
* returns null when `claimedAt` is falsy, so it participates in the
102+
* lease/no-lease decision. Any future change has a second consumer.
103+
*/
104+
async claim(body: ClaimBody): Promise<StoredLease | null> {
105+
if (!body?.nodeId) throw new Error('[meta/coordinator] nodeId is required');
106+
107+
const role = body.role ?? META_LEADER_ROLE;
108+
const leaseSeconds = normalizeLeaseSeconds(body.leaseSeconds);
109+
const now = Date.now();
110+
const current = await this.read(role);
111+
112+
// A corrupted timestamp parses to NaN, and every NaN comparison is false —
113+
// which would leave the role permanently unclaimable. Postgres typed this
114+
// column so the SQL had no such failure mode; treat unparseable as expired.
115+
const expiresAt = current?.leaseExpiresAt ? Date.parse(current.leaseExpiresAt) : NaN;
116+
const expired = !Number.isFinite(expiresAt) || expiresAt < now;
117+
const claimable = !current?.nodeId || current.nodeId === body.nodeId || expired;
118+
if (!claimable) return null;
119+
120+
const nowIso = new Date(now).toISOString();
121+
const lease: StoredLease = {
122+
role,
123+
nodeId: body.nodeId,
124+
nodeDescriptor: body.nodeDescriptor ?? null,
125+
sessionId: body.sessionId ?? null,
126+
claimedAt: current?.claimedAt ?? nowIso,
127+
heartbeatAt: nowIso,
128+
leaseExpiresAt: new Date(now + leaseSeconds * 1000).toISOString(),
129+
metadata: body.metadata ?? {},
130+
};
131+
132+
await this.ctx.storage.put(keyFor(role), lease);
133+
return lease;
134+
}
135+
136+
/**
137+
* Extend the lease. Returns null when this node is no longer the holder, or
138+
* when `sessionId` does not match the session recorded on the lease — a
139+
* restarted process reusing a nodeId cannot heartbeat over a fresh leader.
140+
*/
141+
async heartbeat(body: {
142+
nodeId: string;
143+
role?: string;
144+
leaseSeconds?: number;
145+
sessionId?: string | null;
146+
}): Promise<StoredLease | null> {
147+
if (!body?.nodeId) throw new Error('[meta/coordinator] nodeId is required for heartbeat');
148+
149+
const role = body.role ?? META_LEADER_ROLE;
150+
const current = await this.read(role);
151+
if (!current || current.nodeId !== body.nodeId) return null;
152+
if (current.sessionId !== (body.sessionId ?? null)) return null;
153+
154+
const now = Date.now();
155+
const leaseSeconds = normalizeLeaseSeconds(body.leaseSeconds);
156+
const lease: StoredLease = {
157+
...current,
158+
heartbeatAt: new Date(now).toISOString(),
159+
leaseExpiresAt: new Date(now + leaseSeconds * 1000).toISOString(),
160+
};
161+
162+
await this.ctx.storage.put(keyFor(role), lease);
163+
return lease;
164+
}
165+
166+
/**
167+
* Release the role. Only the holding (nodeId, sessionId) pair may release;
168+
* a different node or a newer session of the same node is a no-op.
169+
*/
170+
async release(body: {
171+
nodeId: string;
172+
role?: string;
173+
sessionId?: string | null;
174+
}): Promise<boolean> {
175+
if (!body?.nodeId) throw new Error('[meta/coordinator] nodeId is required for release');
176+
177+
const role = body.role ?? META_LEADER_ROLE;
178+
const current = await this.read(role);
179+
if (!current || current.nodeId !== body.nodeId) return false;
180+
if (current.sessionId !== (body.sessionId ?? null)) return false;
181+
182+
await this.ctx.storage.put(keyFor(role), {
183+
role,
184+
nodeId: null,
185+
nodeDescriptor: null,
186+
sessionId: null,
187+
claimedAt: null,
188+
heartbeatAt: null,
189+
leaseExpiresAt: null,
190+
metadata: current.metadata,
191+
} satisfies StoredLease);
192+
return true;
193+
}
194+
195+
/**
196+
* Inspect the lease without mutating it. Returns null when unheld.
197+
*
198+
* Parity note: an *expired but unreleased* lease is still returned, matching
199+
* `describeLease()` in daemon/leader.ts, which filters only on `node_id`.
200+
* Callers must not treat a non-null result as proof of live leadership.
201+
*/
202+
async describe(role: string = META_LEADER_ROLE): Promise<StoredLease | null> {
203+
const current = await this.read(role);
204+
return current?.nodeId ? current : null;
205+
}
206+
}
207+
208+
async function readJson<T>(request: Request): Promise<T> {
209+
try {
210+
return (await request.json()) as T;
211+
} catch {
212+
throw new Error('invalid JSON body');
213+
}
214+
}
215+
216+
function json(data: unknown, status = 200): Response {
217+
return new Response(JSON.stringify(data), {
218+
status,
219+
headers: { 'content-type': 'application/json' },
220+
});
221+
}

0 commit comments

Comments
 (0)