-
Notifications
You must be signed in to change notification settings - Fork 10
fix(agent): relieve repeated sync pressure #2053
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
7d2bfe5
fix(agent): relieve repeated sync pressure
3151cd5
fix(agent): harden sync pressure recovery
ce4a919
fix(agent): preserve exact recovery proof invariants
55fc6dd
fix(agent): bound incomplete exact recovery
916d82a
fix(agent): keep incomplete recovery fail-open
b798e45
fix(agent): guard exact recovery lifecycle
e53a0f4
fix(agent): bound exact sync accumulation
82ad1cf
fix(agent): preserve sync fairness across restart
da6a42d
fix(agent): retire stale reconcile workers across restart
334f9f6
test(agent): isolate terminal rotation cleanup
6813b25
fix(agent): harden reconcile retry lifecycle
c6bd113
fix(agent): close sync lifecycle review gaps
91b29d1
test: stabilize sync lifecycle CI fixtures
ffa9931
fix: quarantine stale graph-scoped commits
5b96e71
fix(agent): preserve valid public sync commits
4bd402d
fix(agent): canonicalize curator wallet lookup
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| export function bumpContextGraphBindingGeneration( | ||
| generations: Map<string, number>, | ||
| localCgId: string, | ||
| ): number { | ||
| const generation = (generations.get(localCgId) ?? 0) + 1; | ||
| generations.set(localCgId, generation); | ||
| return generation; | ||
| } | ||
|
|
||
| export function captureContextGraphBindingGeneration( | ||
| generations: Map<string, number>, | ||
| localCgId: string, | ||
| ): number { | ||
| return generations.get(localCgId) ?? 0; | ||
| } | ||
|
|
||
| export function isContextGraphBindingGenerationCurrent( | ||
| generations: Map<string, number>, | ||
| localCgId: string, | ||
| generation: number, | ||
| ): boolean { | ||
| return captureContextGraphBindingGeneration(generations, localCgId) === generation; | ||
| } | ||
|
|
||
| export function clearContextGraphBindingGeneration( | ||
| generations: Map<string, number>, | ||
| localCgId: string, | ||
| ): void { | ||
| generations.delete(localCgId); | ||
| } |
159 changes: 159 additions & 0 deletions
159
packages/agent/src/context-graph-membership-persist-scheduler.ts
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,159 @@ | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| export class ContextGraphMembershipPersistQueueFullError extends Error { | ||
| readonly code = 'CG_MEMBERSHIP_PERSIST_QUEUE_FULL'; | ||
|
|
||
| constructor(message: string) { | ||
| super(message); | ||
| this.name = 'ContextGraphMembershipPersistQueueFullError'; | ||
| } | ||
| } | ||
|
|
||
| export class ContextGraphMembershipPersistQueueClosedError extends Error { | ||
| readonly code = 'CG_MEMBERSHIP_PERSIST_QUEUE_CLOSED'; | ||
|
|
||
| constructor() { | ||
| super('Context-graph membership persistence is closed'); | ||
| this.name = 'ContextGraphMembershipPersistQueueClosedError'; | ||
| } | ||
| } | ||
|
|
||
| export const CONTEXT_GRAPH_MEMBERSHIP_PERSIST_SHUTDOWN_TIMEOUT_ERROR_CODE = | ||
| 'CG_MEMBERSHIP_PERSIST_SHUTDOWN_TIMEOUT'; | ||
|
|
||
| export class ContextGraphMembershipPersistShutdownTimeoutError extends Error { | ||
| readonly code = CONTEXT_GRAPH_MEMBERSHIP_PERSIST_SHUTDOWN_TIMEOUT_ERROR_CODE; | ||
|
|
||
| constructor(timeoutMs: number) { | ||
| super(`Context-graph membership persistence did not drain within ${timeoutMs}ms`); | ||
| this.name = 'ContextGraphMembershipPersistShutdownTimeoutError'; | ||
| } | ||
| } | ||
|
|
||
| interface PendingWrite { | ||
| strict: boolean; | ||
| write: () => Promise<void>; | ||
| resolve: () => void; | ||
| reject: (error: unknown) => void; | ||
| } | ||
|
|
||
| interface PersistLane { | ||
| active: boolean; | ||
| pending: PendingWrite[]; | ||
| drained: Promise<void>; | ||
| resolveDrained: () => void; | ||
| } | ||
|
|
||
| export interface ContextGraphMembershipPersistSchedulerStatus { | ||
| closed: boolean; | ||
| lanes: number; | ||
| active: number; | ||
| pending: number; | ||
| } | ||
|
|
||
| /** | ||
| * Bounded keyed serialization for membership-store mutations. | ||
| * | ||
| * Strict operations preserve FIFO order and receive explicit backpressure. | ||
| * Adjacent background mutations coalesce to their latest write while the | ||
| * displaced caller settles successfully: those callers are deliberately | ||
| * best-effort, and only the final persisted state is meaningful. | ||
| */ | ||
| export class ContextGraphMembershipPersistScheduler { | ||
| private readonly lanes = new Map<string, PersistLane>(); | ||
| private closed = false; | ||
|
|
||
| constructor( | ||
| private readonly maxLanes = 1_000, | ||
| private readonly maxPendingPerLane = 16, | ||
| ) { | ||
| if (!Number.isSafeInteger(maxLanes) || maxLanes < 1) { | ||
| throw new Error('Membership persistence maxLanes must be a positive safe integer'); | ||
| } | ||
| if (!Number.isSafeInteger(maxPendingPerLane) || maxPendingPerLane < 1) { | ||
| throw new Error('Membership persistence maxPendingPerLane must be a positive safe integer'); | ||
| } | ||
| } | ||
|
|
||
| enqueue( | ||
| key: string, | ||
| write: () => Promise<void>, | ||
| options: { strict?: boolean } = {}, | ||
| ): Promise<void> { | ||
| if (this.closed) { | ||
| return Promise.reject(new ContextGraphMembershipPersistQueueClosedError()); | ||
| } | ||
|
|
||
| let lane = this.lanes.get(key); | ||
| if (!lane) { | ||
| if (this.lanes.size >= this.maxLanes) { | ||
| return Promise.reject(new ContextGraphMembershipPersistQueueFullError( | ||
| `Context-graph membership persistence reached its ${this.maxLanes}-lane limit`, | ||
| )); | ||
| } | ||
| let resolveDrained!: () => void; | ||
| const drained = new Promise<void>((resolve) => { resolveDrained = resolve; }); | ||
| lane = { active: false, pending: [], drained, resolveDrained }; | ||
| this.lanes.set(key, lane); | ||
| } | ||
|
|
||
| const strict = options.strict === true; | ||
| return new Promise<void>((resolve, reject) => { | ||
| const tail = lane!.pending.at(-1); | ||
| if (!strict && tail && !tail.strict) { | ||
|
Jurij89 marked this conversation as resolved.
|
||
| tail.resolve(); | ||
| lane!.pending[lane!.pending.length - 1] = { strict, write, resolve, reject }; | ||
| } else { | ||
| if (lane!.pending.length >= this.maxPendingPerLane) { | ||
| reject(new ContextGraphMembershipPersistQueueFullError( | ||
| `Context-graph membership persistence key "${key}" reached its ` | ||
| + `${this.maxPendingPerLane}-write pending limit`, | ||
| )); | ||
| return; | ||
| } | ||
| lane!.pending.push({ strict, write, resolve, reject }); | ||
| } | ||
| if (!lane!.active) { | ||
| lane!.active = true; | ||
| void this.runLane(key, lane!); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| closeAndDrain(): Promise<void> { | ||
| this.closed = true; | ||
| return Promise.all([...this.lanes.values()].map((lane) => lane.drained)).then(() => undefined); | ||
| } | ||
|
|
||
| reopen(): void { | ||
| if (this.lanes.size > 0) { | ||
| throw new Error('Cannot reopen context-graph membership persistence before it drains'); | ||
| } | ||
| this.closed = false; | ||
| } | ||
|
|
||
| status(): ContextGraphMembershipPersistSchedulerStatus { | ||
| let active = 0; | ||
| let pending = 0; | ||
| for (const lane of this.lanes.values()) { | ||
| if (lane.active) active += 1; | ||
| pending += lane.pending.length; | ||
| } | ||
| return { closed: this.closed, lanes: this.lanes.size, active, pending }; | ||
| } | ||
|
|
||
| private async runLane(key: string, lane: PersistLane): Promise<void> { | ||
| while (lane.pending.length > 0) { | ||
| const operation = lane.pending.shift()!; | ||
| try { | ||
| await operation.write(); | ||
| operation.resolve(); | ||
| } catch (error) { | ||
| operation.reject(error); | ||
| } | ||
| } | ||
| lane.active = false; | ||
| if (this.lanes.get(key) === lane) this.lanes.delete(key); | ||
| lane.resolveDrained(); | ||
| } | ||
| } | ||
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Issue: Do not leak exact-recovery rotation identity into the pure chain reconciler target
What's wrong
The PR widens the generic reconciler contract with agent-local identity data. That couples the pure watermark engine to one recovery implementation and makes future callers/tests pay for fields they do not conceptually own.
Example
The tests now need a
vmRecoveryTargethelper that fabricateslocalCgId,onChainCgId, andmerkleRootfor every target even when the scenario only cares about ordinal scheduling.Suggested direction
Keep
OrdinalRecoveryTargetminimal or make the recovery payload owned by the VM recovery layer. The pure reconciler should not carry fields whose only purpose is process-local rotation/backoff bookkeeping.For Agents
Keep
chain-reconciler.tsfocused on ordinal reconciliation. Move exact-recovery identity into an agent-local target type or construct aVmExactRecoveryTargetat therecoverVmReconcileBatchboundary from the batch context plus the pending outcome. Preserve the existing recovery callback behavior, and verify the chain reconciler tests no longer need VM rotation-specific fields.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Issue: Avoid duplicating batch identity on every recovery target
What's wrong
The new target shape creates two sources of truth for the context graph identity: the method parameters and the per-target fields. That weakens the type boundary and pushes stale-target defense into incidental control flow rather than making the invariant clear at construction time.
Example
recoverVmReconcileBatch('current', 1n, [{ localCgId: 'other', onChainCgId: '1', ... }], ...)is now a representable state and is silently filtered out. The type permits contradictory batch identity, so the implementation has to defend against that contradiction in multiple places.Suggested direction
Replace the duplicated fields with a single explicit batch/slot model so contradictory target identity is impossible or isolated at one boundary. That should remove the ad-hoc filtering and reduce repeated equality checks in the recovery loop.
For Agents
Start with
OrdinalRecoveryTargetinpackages/agent/src/chain-reconciler.tsand its construction inreconcileChainOrdinal. Preserve stale in-flight response protection, but model graph identity once: either pass a typedRecoveryBatch/RecoverySlotobject through the flow, or keep targets context-free and derive the slot key from the batch boundary. Add focused tests for mismatched/stale target handling after the boundary is explicit.