Skip to content
6 changes: 5 additions & 1 deletion packages/agent/src/dkg-agent-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1664,6 +1664,7 @@ export class DKGAgentBase {

/** Drain and checkpoint the inbox before releasing the broader persistence lifetime. */
protected async closeFinalizationRecoveryStore(): Promise<void> {
await this.finalizationHandler?.stopRecoveryWorker();
const store = this.finalizationRuntime.detachRecoveryStore();
await store?.close();
}
Expand All @@ -1679,6 +1680,7 @@ export class DKGAgentBase {
degradedReason: 'not-configured',
stateCounts: {},
livePayloadBytes: 0,
dueEntries: 0,
};
}
const health = await store.health();
Expand All @@ -1692,7 +1694,9 @@ export class DKGAgentBase {
&& canonicalReceiptCapability === 'supported',
canonicalReceiptCapability,
...(
canonicalReceiptCapability === 'unsupported' && health.available
canonicalReceiptCapability === 'unsupported'
&& health.available
&& health.degradedReason === undefined
? {
degradedReason: 'canonical-finalization-receipt-unsupported',
}
Expand Down
8 changes: 8 additions & 0 deletions packages/agent/src/dkg-agent-lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3273,6 +3273,14 @@ export class LifecycleSyncMethods extends DKGAgentBase {
}, MESSAGE_OUTBOX_TICK_MS);
if (this.messengerOutboxTimer.unref) this.messengerOutboxTimer.unref();

// The durable finalization inbox is an executable retry queue, not only a
// write-ahead journal. Its lifecycle is independent of chain-cursor
// progress so entries received after a watermark advance are still
// reconsidered. The worker batches SQLite reads but serializes graph work.
if (this.finalizationRuntime.getRecoveryStore()) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Issue: Recovery worker lifecycle is scattered across agent layers

What's wrong
This couples a handler-internal retry loop to broad agent startup, stop, and persistence teardown code. It works, but it makes future lifecycle changes brittle because every new stop/reconfigure path has to remember the same resource and ordering rule.

Example
The lifecycle of one timer-backed resource is now split across three classes: FinalizationHandler owns it, LifecycleSyncMethods starts it, and both DKGAgent plus DKGAgentBase know to stop it.

Suggested direction
Give the owner a single lifecycle method, for example FinalizationHandler.start()/close() or a runtime-owned finalization lifecycle hook, and call that from the existing agent lifecycle boundary. Avoid making generic persistence cleanup know about a handler-internal worker.

For Agents
Consolidate worker lifecycle ownership around FinalizationHandler or the finalization runtime. Preserve the shutdown ordering that stops the worker before chain/graph-store dependencies close, but expose one lifecycle boundary rather than requiring multiple agent layers to know about the worker.

this.getOrCreateFinalizationHandler().startRecoveryWorker();
}

// Wire V10 Random Sampling prover. Edge nodes no-op. Core nodes with
// transient identity/RPC startup failures retry in the background so
// one flaky `getIdentityId()` call does not disable proving until the
Expand Down
4 changes: 4 additions & 0 deletions packages/agent/src/dkg-agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1754,6 +1754,10 @@ export class DKGAgent extends DKGAgentBase {
);
}
}
// Stop admission and await the active finalization recovery batch while
// chain and graph-store dependencies are still alive. No new retry may
// begin after this boundary.
await this.finalizationHandler?.stopRecoveryWorker();
// OT-RFC-64 Gate 1: unregister the public catalog protocols and drain the
// receiver scheduler (awaiting in-flight durable stage writes) while the
// router, node, and control-object store are all still live — before
Expand Down
19 changes: 19 additions & 0 deletions packages/agent/src/finalization-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ import {
type FinalizationRecoveryPreparedMaterialization,
type FinalizationRecoveryReplayOutcome,
} from './finalization-recovery.js';
import {
FinalizationRecoveryWorker,
} from './finalization-recovery-worker.js';
import type {
FinalizationRecoveryEntry,
FinalizationRecoveryStore,
Expand Down Expand Up @@ -381,6 +384,7 @@ export class FinalizationHandler {
private readonly negativeSnapshotMemo = new Map<string, NegativeSnapshotMemoEntry>();
/** Equivalent finalization/reconcile reads share one promise until it settles. */
private readonly scanSingleFlights = new Map<string, Promise<unknown>>();
private readonly recoveryWorker: FinalizationRecoveryWorker;

constructor(
store: TripleStore,
Expand Down Expand Up @@ -448,6 +452,21 @@ export class FinalizationHandler {
},
materializer,
);
this.recoveryWorker = new FinalizationRecoveryWorker(
(limit) => this.recovery.processDueBatch(limit),
{
info: (message) => this.log.info(createOperationContext('system'), message),
warn: (message) => this.log.warn(createOperationContext('system'), message),
},
);
}

startRecoveryWorker(): void {
this.recoveryWorker.start();
}

stopRecoveryWorker(): Promise<void> {
return this.recoveryWorker.stop();
}

async handleFinalizationMessage(
Expand Down
73 changes: 69 additions & 4 deletions packages/agent/src/finalization-recovery-sqlite-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,17 @@ export type {
SqliteFinalizationRecoveryStoreOptions,
} from './finalization-recovery-sqlite-policy.js';

const DUE_FINALIZATION_SQL_PREDICATE = `
(
state IN ('RECEIVED','VERIFIED','REORGED')
OR (
state = 'SETTLED'
AND (publisher_upgrade_pending = 1 OR next_attempt_at IS NOT NULL)
)
)
AND (next_attempt_at IS NULL OR next_attempt_at <= ?)
`;

function sameFinalizationRecoveryIdentity(
existing: FinalizationRecoveryEntry,
input: FinalizationRecoveryReceiveInput,
Expand Down Expand Up @@ -80,6 +91,16 @@ export class SqliteFinalizationRecoveryStore implements FinalizationRecoveryStor
return this.#closed;
}

async get(key: string): Promise<FinalizationRecoveryEntry | undefined> {
if (this.#closed || this.#closing) return undefined;
await this.#mutationTail;
if (this.#closed) return undefined;
const row = this.database.prepare(
'SELECT * FROM finalization_inbox_v1 WHERE key = ?',
).get(key);
return row ? finalizationRecoveryRowToEntry(row) : undefined;
}

receive(input: FinalizationRecoveryReceiveInput): Promise<FinalizationRecoveryReceiveResult> {
if (this.#closed || this.#closing) return Promise.resolve({ status: 'closed' });
return this.mutate(() => {
Expand Down Expand Up @@ -422,6 +443,25 @@ export class SqliteFinalizationRecoveryStore implements FinalizationRecoveryStor
return entry.nextAttemptAt === undefined || entry.nextAttemptAt <= this.#policy.now();
}

async listDue(limit: number): Promise<FinalizationRecoveryEntry[]> {
if (this.#closed || this.#closing) return [];
if (!Number.isFinite(limit)) return [];
const boundedLimit = Math.min(
this.#policy.maxEntries,
Math.max(0, Math.trunc(limit)),
);
if (boundedLimit === 0) return [];
await this.#mutationTail;
if (this.#closed) return [];
const now = this.#policy.now();
return this.database.prepare(`
SELECT * FROM finalization_inbox_v1
WHERE ${DUE_FINALIZATION_SQL_PREDICATE}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Issue: Autonomous SETTLED recovery lacks direct regression coverage

What's wrong
The PR expands the due queue beyond live entries, but the tests do not exercise the new autonomous replay path for SETTLED entries. That leaves a meaningful gap: regressions in the worker-only setup of replay input, matching, or pending publisher-upgrade handling could still pass because the current SETTLED tests enter through chain reconciliation instead.

Example
A persisted SETTLED row with publisherUpgradePending: true after restart should be picked up by processDueBatch(16), validate the trusted publisher, apply the upgraded access semantics, and clear publisherUpgradePending. Likewise, a SETTLED receipt retry whose nextAttemptAt is due should be replayed by the worker, not only by replayMatching.

Suggested direction
Add a worker-driven test for at least one due SETTLED row, preferably covering the publisher-upgrade case because it is newly admitted by publisher_upgrade_pending even without a retry timestamp.

For Agents
Look in packages/agent/test/finalization-recovery.test.ts or the handler integration tests. Seed or create a SETTLED row that is due, then call processDueBatch or startRecoveryWorker; assert the row is recovered/updated and that retry state is cleared or advanced as appropriate. Preserve the existing chain-reconciliation tests; this should specifically prove the autonomous path.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Issue: Bounded due-batch behavior is not directly tested

What's wrong
This PR makes the finalization inbox an autonomous retry queue, and the safety story depends on bounded SQLite reads plus serialized graph work. The new tests cover due filtering and ordering, but they do not prove that a positive limit is honored, so a future regression could process an unbounded backlog while this suite stays green.

Example
Seed three due RECEIVED rows, call await store.listDue(1), and expect exactly the oldest single key. A companion processDueBatch(1) assertion could prove only one due row is processed per batch.

Suggested direction
Add a regression test with more due entries than the requested positive limit, not just the zero-limit edge case.

For Agents
Look at packages/agent/test/finalization-recovery-sqlite-store.test.ts near the new listDue coverage. Add a case where due rows exceed the requested positive limit and assert oldest-first truncation; optionally verify FinalizationRecovery.processDueBatch(1) only processes one row to preserve worker backpressure.

ORDER BY COALESCE(next_attempt_at, updated_at), updated_at, key
LIMIT ?
`).all(now, boundedLimit).map(finalizationRecoveryRowToEntry);
}

async listForKnowledgeAsset(input: {
chainId: string;
contextGraphId: string;
Expand Down Expand Up @@ -497,17 +537,24 @@ export class SqliteFinalizationRecoveryStore implements FinalizationRecoveryStor
return this.mutate(() => {
if (this.#closed) return;
const now = this.#policy.now();
const nextAttemptAt = retryDelayMs === undefined ? null : now + retryDelayMs;
this.database.prepare(`
UPDATE finalization_inbox_v1
SET attempt_count = attempt_count + 1,
last_error = ?,
next_attempt_at = ?,
next_attempt_at = CASE
WHEN ? IS NULL THEN next_attempt_at
WHEN next_attempt_at IS NULL THEN ?
ELSE MAX(next_attempt_at, ?)
END,
updated_at = ?
WHERE key = ? AND generation = ?
AND state IN ('RECEIVED','VERIFIED','REORGED','SETTLED')
`).run(
lastError ?? null,
retryDelayMs === undefined ? null : now + retryDelayMs,
nextAttemptAt,
nextAttemptAt,
nextAttemptAt,
now,
key,
generation,
Expand All @@ -523,17 +570,31 @@ export class SqliteFinalizationRecoveryStore implements FinalizationRecoveryStor
degradedReason: 'closing',
stateCounts: {},
livePayloadBytes: 0,
dueEntries: 0,
};
}
await this.#mutationTail;
if (this.#closed) {
return { available: false, closed: true, degradedReason: 'closed', stateCounts: {}, livePayloadBytes: 0 };
return {
available: false,
closed: true,
degradedReason: 'closed',
stateCounts: {},
livePayloadBytes: 0,
dueEntries: 0,
};
}
const counts = this.database.prepare(
'SELECT state, COUNT(*) AS count FROM finalization_inbox_v1 GROUP BY state',
).all();
const stateCounts: Partial<Record<FinalizationRecoveryState, number>> = {};
for (const row of counts) stateCounts[String(row.state) as FinalizationRecoveryState] = Number(row.count);
const now = this.#policy.now();
const due = this.database.prepare(`
SELECT COUNT(*) AS count, MIN(created_at) AS oldest
FROM finalization_inbox_v1
WHERE ${DUE_FINALIZATION_SQL_PREDICATE}
`).get(now) as { count: number | bigint; oldest: number | bigint | null };
const capacity = readFinalizationRecoveryCapacity(this.database, this.#policy);
return {
available: true,
Expand All @@ -542,9 +603,13 @@ export class SqliteFinalizationRecoveryStore implements FinalizationRecoveryStor
...(capacity.capacityExhausted ? { degradedReason: 'capacity-exhausted' } : {}),
stateCounts,
livePayloadBytes: capacity.livePayloadBytes,
dueEntries: Number(due.count),
...(due.oldest === null
? {}
: { oldestDueAgeMs: Math.max(0, now - Number(due.oldest)) }),
...(capacity.oldest === undefined
? {}
: { oldestPendingAgeMs: Math.max(0, this.#policy.now() - capacity.oldest) }),
: { oldestPendingAgeMs: Math.max(0, now - capacity.oldest) }),
};
}

Expand Down
10 changes: 10 additions & 0 deletions packages/agent/src/finalization-recovery-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,15 @@ export interface FinalizationRecoveryHealth {
degradedReason?: string;
stateCounts: Partial<Record<FinalizationRecoveryState, number>>;
livePayloadBytes: number;
dueEntries: number;
oldestDueAgeMs?: number;
oldestPendingAgeMs?: number;
}

export interface FinalizationRecoveryStore {
readonly closed: boolean;
/** Reloads one entry after waiting on an in-process serialization boundary. */
get(key: string): Promise<FinalizationRecoveryEntry | undefined>;
receive(input: FinalizationRecoveryReceiveInput): Promise<FinalizationRecoveryReceiveResult>;
recordTrustedPublisher(
key: string,
Expand Down Expand Up @@ -113,6 +117,12 @@ export interface FinalizationRecoveryStore {
clearSettledRetry(key: string, generation: number): Promise<void>;
rejectSettled(key: string, generation: number, lastError: string): Promise<boolean>;
isAttemptDue(entry: FinalizationRecoveryEntry): boolean;
/**

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Issue: Due-queue semantics leak across the store and recovery layers

What's wrong
The durable inbox is now both a persistence store and a retry queue, but the queue contract is spread across low-level methods and comments. That makes the implementation depend on callers remembering to re-read snapshots, re-check deadlines, and rely on generation transitions in the right order. This is maintainability debt in the new worker path rather than a clean ownership boundary.

Example
Adding a new retryable state or changing due eligibility now requires keeping DUE_FINALIZATION_SQL_PREDICATE, isLiveEntry, listDue, isAttemptDue, and replayDueEntryLocked aligned by convention.

Suggested direction
Replace the loose trio of listDue/get/isAttemptDue with a higher-level due-work API, such as loadDueBatch(limit) returning fresh due entries under the store’s serialization boundary, or a small queue repository that owns the SQL predicate, freshness check, and retry clock. That would let recovery process entries without reassembling queue invariants manually.

Confidence note
This is a structural concern from the diff shape; the generation-checked transitions may make behavior safe, but the queue ownership boundary is still unnecessarily leaky.

For Agents
Look at packages/agent/src/finalization-recovery-store.ts, packages/agent/src/finalization-recovery-sqlite-store.ts, and processDueBatch/replayDueEntryLocked in finalization-recovery.ts. Preserve bounded oldest-first processing and generation-checked transitions, but move due-entry selection/freshness into a single store/repository-level operation or a dedicated due-queue abstraction.

* Returns a bounded, oldest-first snapshot of entries whose persisted retry
* gate is open. Callers must still rely on generation-checked transitions:
* live gossip or reconciliation may update an entry after this read.
*/
listDue(limit: number): Promise<FinalizationRecoveryEntry[]>;
listForKnowledgeAsset(input: {
chainId: string;
contextGraphId: string;
Expand Down
100 changes: 100 additions & 0 deletions packages/agent/src/finalization-recovery-worker.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
export const FINALIZATION_RECOVERY_WORKER_BATCH_SIZE = 16;
export const FINALIZATION_RECOVERY_WORKER_POLL_INTERVAL_MS = 5_000;

interface FinalizationRecoveryWorkerLog {
info(message: string): void;
warn(message: string): void;
}

interface FinalizationRecoveryWorkerOptions {
batchSize?: number;
pollIntervalMs?: number;
}

/**
* Lifecycle-owned asynchronous retry loop for the durable finalization inbox.
*
* SQLite reads are batched, while graph materialization remains serial inside
* `processDueBatch`. This is an I/O scheduler on the Node.js event loop, not a
* worker thread: it never blocks startup and never introduces concurrent
* Blazegraph finalization writes.
*/
export class FinalizationRecoveryWorker {
readonly #batchSize: number;
readonly #pollIntervalMs: number;
#running = false;
#timer: ReturnType<typeof setTimeout> | undefined;
#inFlight: Promise<void> | undefined;

constructor(
private readonly processDueBatch: (limit: number) => Promise<number>,
private readonly log: FinalizationRecoveryWorkerLog,
options: FinalizationRecoveryWorkerOptions = {},
) {
this.#batchSize = Math.max(
1,
Math.trunc(options.batchSize ?? FINALIZATION_RECOVERY_WORKER_BATCH_SIZE),
);
this.#pollIntervalMs = Math.max(
1,
Math.trunc(
options.pollIntervalMs ?? FINALIZATION_RECOVERY_WORKER_POLL_INTERVAL_MS,
),
);
}

get running(): boolean {
return this.#running;
}

start(): void {
if (this.#running) return;
this.#running = true;
this.schedule(0);
}

async stop(): Promise<void> {
this.#running = false;
if (this.#timer) {
clearTimeout(this.#timer);
this.#timer = undefined;
}
await this.#inFlight?.catch(() => undefined);
}

private schedule(delayMs: number): void {
if (!this.#running || this.#timer) return;
this.#timer = setTimeout(() => {
this.#timer = undefined;
if (!this.#running) return;
const run = this.runBatch();
this.#inFlight = run;
void run.finally(() => {
if (this.#inFlight === run) this.#inFlight = undefined;
});
}, delayMs);
this.#timer.unref?.();
}

private async runBatch(): Promise<void> {
let selected = 0;
try {
selected = await this.processDueBatch(this.#batchSize);
if (selected > 0) {
this.log.info(
`Finalization recovery worker inspected ${selected} due inbox entr`
+ `${selected === 1 ? 'y' : 'ies'}`,
);
}
} catch (error) {
this.log.warn(
`Finalization recovery worker batch failed: `
+ `${error instanceof Error ? error.message : String(error)}`,
);
} finally {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Issue: Worker failure recovery is not verified

What's wrong
The autonomous inbox worker is meant to be the durable retry mechanism, but the added tests only cover successful batches and shutdown. A transient exception from the batch callback could stop future retries if the catch/finally scheduling regressed, leaving due finalization entries stuck with all current tests still green.

Example
A focused test could use vi.fn().mockRejectedValueOnce(new Error('busy')).mockResolvedValueOnce(0), start a worker with a tiny pollIntervalMs, and assert the batch function is called a second time and warn is emitted. Without the catch/finally reschedule behavior, that test would fail while the current worker tests still pass.

Suggested direction
Add a worker test that exercises the catch/finally path after a transient processDueBatch rejection and proves the retry loop continues.

For Agents
Add coverage in packages/agent/test/finalization-recovery-worker.test.ts for a rejected processDueBatch: preserve that the worker logs the failure, remains running, and schedules another poll without overlapping work. Use a short poll interval or fake timers so the test is deterministic.

if (this.#running) {
this.schedule(selected >= this.#batchSize ? 0 : this.#pollIntervalMs);
}
}
}
}
Loading
Loading