Skip to content

Commit 73efb3b

Browse files
whoabuddyJason Schraderclaude
authored
feat(substrate): opt-in substrate dispatch intake (Phase 5) (#5)
* feat(substrate): opt-in substrate intake — config, credential, claim, enqueue Adds @genesis-works/substrate-db as a dependency and wires a substrate dispatch intake path into the runOnce tick. Opt-in per slot: substrate is disabled by default (config.substrate.enabled=false). Flipping this flag live changes zero behavior on any slot that has not set it. Config keys (host config substrate block): substrate.enabled — bool, default false substrate.credential — string, credential id (NEVER plaintext password) substrate.kinds — string[], job kinds this slot handles substrate.slotId — string, this slot's claim identifier substrate.leaseSecs — number, default 300 (5 minutes) substrate.host/port/database/user — Postgres connection params On each tick (when enabled): 1. Credential resolved via existing encrypted-blob pattern. 2. claimNextJob(slotId, kinds) called against shared Postgres. Log: "[substrate] claim slot=<id> kinds=<list> result=<jobs.id|none>" 3. Claimed job converted to TaskInput (source="substrate:<jobId>"). 4. TaskInput enqueued as a local runtime task for next-tick execution. Failure conditions (all logged, none crash): - substrate unreachable → "[substrate] skip reason=db-unreachable error=..." - credential fail → "[substrate] skip reason=credential-fail error=..." - claimNextJob null → "[substrate] claim ... result=none" (silent no-op) Pinned substrate-db SHA: d458200b008efe8be3d37912a71b990d73ff2b17 (arc0btc/substrate-db — private, deployed alongside agent-runtime on slots) Co-Authored-By: Claude <noreply@anthropic.com> * feat(substrate): lease semantics — epoch-fenced write-back, max_attempts, lease recovery Adds the epoch-fenced write-back hook and lease recovery cadence to the substrate intake adapter. Write-back (after finalizeTaskAttempt): - If task.source starts with "substrate:", calls runSubstrateWriteBack. - Reads _substrate_job_id and _substrate_claim_epoch from task payload. - Calls completeJob(epoch) on success, failJob(epoch) on failure. - Epoch mismatch (stale executor woke after lease recovery) → logged no-op: "[substrate] complete-epoch-mismatch ... stale executor, ignored" - Pre-fencing row (epoch=0, expected>0) → distinct log line: "pre-fencing row, ignored" - completeJob/failJob return WriteBackResult — conflict is logged, not thrown. Lease recovery: - Runs releaseExpiredLeases on the substrate DB when isLeaseRecoveryOwner=true. - Cadence: leaseRecoveryCadenceSecs (default 60s) using a module-level timer. - ONE nominated owner runs this — not all slots — to avoid stampede. - Log: "[substrate] lease-recovery released=<n>" - Lease expiry measured against Postgres now() — NTP drift irrelevant. Log lines contract (stable, parseable): [substrate] claim slot=<id> kinds=<list> result=<jobs.id|none> [substrate] complete jobs.id=<id> epoch=<n> [substrate] fail jobs.id=<id> epoch=<n> reason=<...> [substrate] lease-recovery released=<n> [substrate] skip reason=credential-fail error=<...> [substrate] skip reason=db-unreachable error=<...> [substrate] complete-epoch-mismatch jobs.id=<id> expected=<n> actual=<m> — ... Tests: 125/125 pass (10 new substrate tests + 115 existing). Co-Authored-By: Claude <noreply@anthropic.com> * fix(substrate): review-feedback correctness pass — catch widening, init flag, host requirement, write-back guard, idempotency key Addresses 7 inline review findings (Copilot + secret-mars + arc0btc) on PR #5 — all substantive items resolved in-tree, no follow-ups left. **Catch widening (Copilot #4, secret-mars [blocking-risk], arc0btc #1)** `runSubstrateIntakeTick` previously only caught `claimNextJob`. Both `jobRowToTaskInput` and `enqueueTask` ran outside the try, so a malformed JobRow or a local SQLite lock contention escaped as a thrown error from `runOnce`. Each call site now has its own catch with a distinct skip reason — `job-parse-fail` and `local-enqueue-fail` — so the contract's "NEVER throws" guarantee is real. The substrate job stays under lease in both failure modes; `releaseExpiredLeases` reconciles on the next cycle. **`substrateDbInitialized` retry on credential fail (Copilot #1, all 3 reviewers seconded)** The flag was set to `true` BEFORE `createSubstrateConnection` succeeded. A transient credential read miss at first tick (e.g. encrypted blob not yet available) permanently disabled substrate intake for the process lifetime — the contract's documented `[substrate] skip reason=credential-fail` log line would never re-fire. Flag now sets only inside the success branch; next tick retries. **`runSubstrateWriteBack` swallows transient PG blips (arc0btc #3)** Was the only substrate fn whose Postgres calls were unguarded — a mid-write-back connection reset would propagate out of `runOnce` even though the local task already finished cleanly. Wrapped both `completeJob` and `failJob` in a single try; new `[substrate] write-back error=<msg> jobs.id=<id>` log line on transient fail. Lease recovery still reconciles eventually. **Default host removed (Copilot #3, arc0btc seconded)** `host` no longer defaults to a hard-coded private IP. When `substrate.enabled: true`, an explicit `substrate.host` is required — `createSubstrateConnection` throws a clear error if unset. Closes the "dev/test slot misconfigured at enabled:true accidentally connects to prod" footgun. **Self-contained log fallbacks (secret-mars #5)** The empty-else branches on `completeJob`/`failJob` `{ ok: false }` results relied on the substrate-db package emitting its own `[substrate] complete-epoch-mismatch ...` log. If that package's log format ever changes, those branches went silent. Added `[substrate] complete-failed jobs.id=<id> epoch=<n>` and `[substrate] fail-failed ...` fallbacks so this code is self-contained. **Idempotency-key threading (arc0btc #2 design, secret-mars idempotency follow-up)** Closes the "side-effecting jobs can execute twice when a lease expires mid-flight" hazard structurally — the fence on `jobs.claim_epoch` only protects the status write, not the action. `jobRowToTaskInput` now threads `payload.idempotency_key = "substrate-<job_id>-e<claim_epoch>"` so downstream side-effecting handlers (email send, PR open, tx broadcast) can dedup against their own per-handler key store. Bounds the *consequence* from "action runs twice" to "action runs once, second attempt no-ops." Substrate tasks also enqueue with `priority: 1` so they execute on the same or next tick — the lease window now tracks real execution latency instead of waiting behind lower-priority work, which shrinks the "lease expires while task waits in local queue" window further. **Silent null claim (Copilot #5)** Contract said null-claim is silent; impl logged `[substrate] claim ... result=none` every tick. Dropped the log — quiet-tick visibility lives in successful-claim and idle-dispatch event lines, not substrate tick-rate noise. **Other nits (Copilot #2, arc0btc #5)** - Removed unused `getTaskById` import in `substrate.ts`. - Documented `substrate` block's shallow-merge behavior in `types.ts` (unlike `profiles`/`adapters` which deep-merge). Slots that extend a base and want to flip only `isLeaseRecoveryOwner: true` must repeat the whole substrate block. Deferred (already declared out of scope by reviewers): - `pg_advisory_lock` guard on `isLeaseRecoveryOwner` race (secret-mars [suggestion]) — multi-slot mis-config protection is a follow-up. - `_substrate_*` payload-injection vs strict validators (secret-mars [suggestion]) — naming is defensive prefixing already. Contract block in src/substrate.ts updated with all new log lines and a new "Side-effect duplicate-execution guard" section. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(substrate): expand coverage via mock.module — claim/complete/fail/lease-recovery paths Closes the test-gap finding from arc0btc #4 and secret-mars: prior suite covered only `resolveSubstrateConfig`, `jobRowToTaskInput`, and two write-back *early-return* guards via a fakeDb that threw if touched. Claim, complete, fail, epoch-mismatch no-op, transient write-back throw, and lease recovery — the "highest correctness surface" per the PR's own Commit-2 description — were untested. Rewires the file to use `mock.module("@genesis-works/substrate-db", ...)` with mock fns per substrate-db export. Local DB is a real `openDb(:memory:)` so `enqueueTask` exercises the actual sqlite write path. console.{info,error,warn} are spied per-test for log-line assertions. New tests (18, alongside 10 existing for 28 total): - `runSubstrateIntakeTick`: - happy claim → enqueues local task, returns `claimed=true` with correct epoch + jobId, emits `[substrate] claim ...` log - null claim → silent (no `[substrate] claim` log line; just `claimed: false`) - db-unreachable → catches `claimNextJob` throw, logs `reason=db-unreachable` - local-enqueue-fail → forces `enqueueTask` to throw by closing the local db, asserts catch fires + log line includes `reason=local-enqueue-fail jobs.id=<id>` (proves Copilot #4 / arc0btc #1 fix is wired) - `runSubstrateWriteBack`: - non-substrate source no-op (existing — kept) - missing `_substrate_job_id` no-op (existing — kept) - complete happy → calls `completeJob` with `claim_epoch` arg, `[substrate] complete jobs.id=... epoch=...` log - complete `{ ok: false }` → emits self-contained fallback `[substrate] complete-failed` log (proves secret-mars #5 fix) - complete throws → catches transient PG blip, emits `[substrate] write-back error=... jobs.id=...` (proves arc0btc #3 fix is wired — write-back no longer propagates out of `runOnce`) - fail happy → calls `failJob`, logs reason snippet - fail `{ ok: false }` → emits `[substrate] fail-failed` fallback - `runSubstrateLeaseRecovery`: - released > 0 → logs `[substrate] lease-recovery released=<n>` - released = 0 → no log (quiet success) - throws → catches, logs `[substrate] lease-recovery error=...`, does NOT propagate - `createSubstrateConnection`: - throws when `substrate.host` is unset (proves Copilot #3 fix — no implicit default to a hard-coded private IP) - throws on whitespace-only host - `jobRowToTaskInput`: - asserts `payload.idempotency_key = "substrate-<job_id>-e<epoch>"` (proves idempotency-key threading is wired) - asserts `priority = 1` - asserts `_substrate_*` payload fields threaded - asserts `max_attempts = 1` (substrate handles retry) - existing source / subject-fallback tests kept Header comment rewritten to match what's actually covered (Copilot #7). Unused `mock`/`beforeEach`/`afterEach` imports replaced with the ones actually used (Copilot #6 — though now they ARE used, so the prior nit naturally resolves). Test count: 143 total (28 substrate + 115 existing) — up from 125. `bunx tsc --noEmit` clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Jason Schrader <jason@joinfreehold.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent ff1951b commit 73efb3b

7 files changed

Lines changed: 1169 additions & 1 deletion

File tree

bun.lock

Lines changed: 217 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616
"typescript": "^6.0.2"
1717
},
1818
"dependencies": {
19-
"canonicalize": "^3.0.0"
19+
"canonicalize": "^3.0.0",
20+
"@genesis-works/substrate-db": "file:../substrate-db",
21+
"drizzle-orm": "0.45.2",
22+
"postgres": "^3.4.9"
2023
}
2124
}

src/runtime.ts

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,14 @@ import { evaluateActiveWorkflows } from "./workflow-runtime";
2323
import { writeArtifactIfNeeded } from "./artifacts";
2424
import { extractRetryHint } from "./retry-hints";
2525
import { selectAdapterWithFallback } from "./adapter-probe";
26+
import {
27+
resolveSubstrateConfig,
28+
createSubstrateConnection,
29+
runSubstrateIntakeTick,
30+
runSubstrateWriteBack,
31+
runSubstrateLeaseRecovery,
32+
type SubstrateDb,
33+
} from "./substrate";
2634

2735
type RuntimeSession = {
2836
runnerId: string;
@@ -36,8 +44,20 @@ const runtimeSession: RuntimeSession = {
3644

3745
let bootPrepared = false;
3846

47+
// ---------------------------------------------------------------------------
48+
// Substrate connection cache — created lazily on first tick where substrate
49+
// is enabled and configured. Re-used across ticks (postgres.js connection pool).
50+
// ---------------------------------------------------------------------------
51+
let substrateDb: SubstrateDb | null = null;
52+
let substrateDbInitialized = false;
53+
// Lease recovery: track last run time for cadence enforcement.
54+
let lastLeaseRecoveryAt = 0;
55+
3956
export function resetRuntimeForTests(): void {
4057
bootPrepared = false;
58+
substrateDb = null;
59+
substrateDbInitialized = false;
60+
lastLeaseRecoveryAt = 0;
4161
}
4262

4363
export function getRunnerId(): string {
@@ -181,6 +201,54 @@ export async function runOnce(db: Database, config: RuntimeConfig): Promise<Reco
181201
});
182202
}
183203

204+
// ---------------------------------------------------------------------------
205+
// Substrate intake: opt-in dispatch from shared Postgres job queue.
206+
// Disabled by default — only active when config.substrate.enabled === true.
207+
// If substrate is configured, claim one job and enqueue it as a local task.
208+
// The local task's source will be "substrate:<jobId>" so the write-back hook
209+
// can complete/fail the substrate job after execution.
210+
// ---------------------------------------------------------------------------
211+
const subConfig = resolveSubstrateConfig(config);
212+
if (subConfig) {
213+
// Lazy-init the substrate DB connection (credential resolved on success;
214+
// re-attempted next tick on failure so a transient credential / host
215+
// read miss does not permanently disable substrate intake).
216+
if (!substrateDbInitialized) {
217+
try {
218+
substrateDb = await createSubstrateConnection(subConfig);
219+
substrateDbInitialized = true; // only mark initialized on success
220+
} catch (error) {
221+
const msg = error instanceof Error ? error.message : String(error);
222+
console.error(`[substrate] skip reason=credential-fail error=${msg}`);
223+
substrateDb = null;
224+
// substrateDbInitialized stays false — next tick retries.
225+
}
226+
}
227+
228+
if (substrateDb) {
229+
const tickResult = await runSubstrateIntakeTick(substrateDb, subConfig, db, config);
230+
if (tickResult.claimed) {
231+
await appendLog(config, {
232+
event: "substrate_intake",
233+
job_id: tickResult.jobId,
234+
task_id: tickResult.taskId,
235+
epoch: tickResult.epochUsed,
236+
});
237+
}
238+
239+
// Lease recovery: run on the nominated owner at configured cadence (default 60s).
240+
// Only one slot should be the owner — set isLeaseRecoveryOwner=true in config.
241+
if (subConfig.isLeaseRecoveryOwner) {
242+
const cadenceSecs = subConfig.leaseRecoveryCadenceSecs ?? 60;
243+
const nowSecs = Math.floor(Date.now() / 1000);
244+
if (nowSecs - lastLeaseRecoveryAt >= cadenceSecs) {
245+
lastLeaseRecoveryAt = nowSecs;
246+
await runSubstrateLeaseRecovery(substrateDb);
247+
}
248+
}
249+
}
250+
}
251+
184252
const claim = claimNextTask(db, runtimeSession.runnerId);
185253
if (!claim) {
186254
recordEvent(db, "dispatch_idle", null, { runner_id: runtimeSession.runnerId });
@@ -519,6 +587,17 @@ export async function runOnce(db: Database, config: RuntimeConfig): Promise<Reco
519587
diagnostics: executionResult.diagnostics,
520588
...attemptPaths
521589
});
590+
591+
// ---------------------------------------------------------------------------
592+
// Substrate write-back: if this task originated from a substrate job claim,
593+
// complete or fail the substrate job with epoch-fenced semantics.
594+
// Stale-lease executors that wake up after recovery hold an outdated epoch
595+
// and their write-back becomes a logged no-op (not a stomp).
596+
// ---------------------------------------------------------------------------
597+
if (substrateDb && task.source.startsWith("substrate:")) {
598+
await runSubstrateWriteBack(substrateDb, task, outcome);
599+
}
600+
522601
return { ok: true, status: outcome.status, task_id: task.task_id, attempt_id: attempt.attempt_id };
523602
} finally {
524603
if (lockFd !== null) {

0 commit comments

Comments
 (0)