Skip to content

fix(publish): bounded grace for the post-confirm tail — publish response no longer gated on store-queue congestion (#1572)#1590

Open
Bojan131 wants to merge 1 commit into
mainfrom
fix/1572-publish-response-tail-grace
Open

fix(publish): bounded grace for the post-confirm tail — publish response no longer gated on store-queue congestion (#1572)#1590
Bojan131 wants to merge 1 commit into
mainfrom
fix/1572-publish-response-tail-grace

Conversation

@Bojan131

Copy link
Copy Markdown
Contributor

What

Fixes #1572: the sync publish HTTP response (and the finalization broadcast) no longer wait, unboundedly, for the post-confirm store-mutation tail.

After a V10 publish confirms on-chain, the caller's result (ual/kaId/txHash/status) is complete — but the response still awaited a serial tail of store mutations with no deadline:

  1. the SWM cleanup sweep (clearPublishedSwmRoots / clearRemainingSharedMemory) in publisher.publishFromSharedMemory,
  2. the durable keep-root stamps after the finalization broadcast in agent.publishFromSharedMemory,
  3. the _meta lifecycle tail (VM-pointer stamps, memoryLayer/state flips, publishedUal, clearSwmShareComplete) in publishFromFinalizedAssertion.

On a node with a congested store queue that tail takes minutes: on mainnet cores we measured confirm at +4 s and the response at +9m11s / +14m42s / +17m48s (growing per KA), with the TCP connection dying at ~12 min — every client times out on a publish that succeeded, and peers don't learn about the KA until the equally-delayed broadcast.

How

New awaitTailWithGrace helper (dkg-core) + DKG_PUBLISH_TAIL_GRACE_MS (default 5000 ms, 0 = fully detached). Each tail segment above is awaited through it:

  • settles within the grace → byte-identical behavior to a plain await, including error propagation (a healthy node never notices this change);
  • grace elapses → the response proceeds; the segment keeps running to completion (same code, same order, exactly once) and its eventual settlement is logged (WARN on failure) instead of becoming an unhandled rejection.

The finalization broadcast now also goes out seconds after the confirm instead of after the cleanup sweep, fixing the delayed cross-node visibility of fresh KAs.

Evidence & verification

store-queue load stock main this branch
idle 14.4 s unchanged (grace never elapses on a healthy node)
moderate (12 concurrent writers) 57.5 s 20.3 s (response = confirm + one grace)
heavy (24 writers) never (>300 s) delivered, tails completed detached (log-verified)
  • Tests: new unit tests for the helper (in-grace completion, in-grace error rethrow, detach timing, post-grace failure routed to the callback — no unhandled rejection, graceMs=0); suites green: core 1185/1185, publisher 268/268, agent 432/432.

Notes for review

Fixes #1572

🤖 Generated with Claude Code

…he publish response on store-queue congestion (#1572)

After a V10 publish confirms on-chain, the caller's result (ual/kaId/
txHash/status) is complete, but the sync response still awaited a serial
tail of store mutations with no deadline: the SWM cleanup sweep in
publisher.publishFromSharedMemory, the durable keep-root stamps after the
finalization broadcast, and the _meta lifecycle tail (VM-pointer stamps,
memoryLayer/state flips, publishedUal, clearSwmShareComplete) in
publishFromFinalizedAssertion. On a node whose store queue is congested
(large accumulated graphs + sustained inbound ACK/SWM traffic) that tail
takes minutes: measured on mainnet cores as confirm at +4s but the
response at +9m11s / +14m42s / +17m48s (growing per KA), with the TCP
connection dying at ~12min — every client times out on a publish that
actually succeeded, and peers don't learn about the KA until the equally
delayed finalization broadcast. Reproduced causally on an idle edge:
saturating the store queue turned a 14s response into no-response->
transport-death while the publish still confirmed on-chain; releasing the
load restored seconds. See #1572 for the full evidence.

The fix keeps every tail step intact — same order, same code, exactly
once, error propagation preserved — but awaits each segment through
awaitTailWithGrace (new, dkg-core): if the segment settles within
DKG_PUBLISH_TAIL_GRACE_MS (default 5000, 0 = fully detached) behavior is
byte-identical to before, including rethrow of in-grace failures; if the
grace elapses the response proceeds and the segment finishes detached,
its eventual settlement logged (failure at WARN) instead of becoming an
unhandled rejection. On a healthy node the grace never elapses. The
finalization broadcast now also goes out within seconds of the confirm
instead of after the cleanup sweep, so cross-node visibility of fresh
KAs is no longer delayed by minutes.

Verified: new unit tests for the grace helper (completion, in-grace
rethrow, detach timing, post-grace failure routing, grace=0); core
1185/1185, publisher 268/268, agent 432/432 green; live A/B on a
Base-Sepolia edge under artificial store-queue saturation — stock build:
confirmed on-chain, response never delivered (>300s); this fix: response
delivered with all three grace detaches logged and the tail completing
in the background.

Fixes #1572

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Bojan131

Copy link
Copy Markdown
Contributor Author

Post-push verification on the exact PR commit (bd8ef16), live Base-Sepolia edge

Heavy store-queue load (24 concurrent 900-quad writers — the scenario where stock main never responds): response delivered, HTTP 201 vm-confirmed, all three grace detaches logged, detached tails completed in the background. Stock baseline on the same load: no response within 300 s, transport death.

The actual Jenkins publish harness (strict response check) against this build under realistic conditions:

✅ Published KA #1 | UAL: did:dkg:base:84532/…907786 | tx: 0x14d231c2…
✅ Published KA #2 | UAL: did:dkg:base:84532/…907787 | tx: 0x1de7062c…
✔ publish responses arrived in time (node bug OriginTrail/dkg#1572 inactive)
2 passing (58s) — no read-back rescue triggered

One honest boundary: under sustained artificial saturation the pre-chain steps (create/seal/share) and the VM promotion — intentionally kept synchronous so read-your-writes holds — still scale with load, so a response can exceed an aggressive client timeout on a torture-loaded node. On the production cores those segments measured ~10 s and ~3 s respectively (their only pathology was the post-confirm tail this PR bounds), so expected core response ≈ well under a minute.

Bojan131 added a commit that referenced this pull request Jul 10, 2026
Reverts the 10 July changes (node-version preflight logging 709ffc4, the
response-loss VM read-back rescue fb56929, and the response-loss assertion/
flag/entity-triples peer read e921c30) so the mainnet publish tests run
exactly as they did on 9 July while the team evaluates the #1572 node fix
(PR #1590) separately.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
{
const tailCtx = opts?.operationCtx ?? createOperationContext('publishFromSWM');
const graceMs = resolvePublishTailGraceMs();
const outcome = await awaitTailWithGrace(graceMs, metaTail, (err) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Bug: Detached publish metadata can overwrite newer lifecycle state

What's wrong
The detached tail is not just logging or ancillary bookkeeping. It mutates the authoritative lifecycle rows that later API calls use for status and publish routing. Returning before those mutations are ordered means a later user operation can be applied first and then be clobbered by the stale publish tail.

Example
A publish confirms, the store queue is congested, and awaitTailWithGrace returns detached. The caller immediately starts an edit flow for the same KA through pull-from/create, which writes the fresh WM lifecycle rows. When the old detached publish tail resumes, it deletes those state/layer rows and writes published/VM, so the new draft is reported and routed as already VM-confirmed.

Suggested direction
Keep the slow work bounded, but make the lifecycle state transition atomic or version-checked before returning, or have the detached tail skip its writes when the lifecycle has advanced since the publish it belongs to.

For Agents
In PublishMethods.publishFromFinalizedAssertion, do not let stale lifecycle mutations run after the confirmed publish response unless they are ordered against later lifecycle operations. Preserve the ability to respond quickly, but add an awaited materialization/version guard or lifecycle lock so a detached publish tail cannot overwrite a newer create/pull-from/discard. Add a test with a delayed metadata tail followed by pull-from/create proving the newer WM state survives.

}
})();
const graceMs = resolvePublishTailGraceMs();
const outcome = await awaitTailWithGrace(graceMs, swmCleanup, (err) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Bug: Confirmed publish can return while the same SWM roots remain publishable

What's wrong
Before this change, a confirmed publish did not resolve until the selected SWM roots had been consumed. With the new grace path, callers can observe success while those roots are still present and accepted by the next publish call, which can create duplicate transactions/reverts and inconsistent local lifecycle behavior.

Example
On a congested store queue, publishFromSharedMemory(CG, { rootEntities: [alice] }) returns confirmed after the grace window while alice is still in SWM. A sequential retry or second publish with the same selection can load the same quads and submit another publish instead of failing with no SWM data/already-consumed share.

Suggested direction
Separate the expensive cleanup sweep from the small correctness gate: await an idempotent consumed marker or marker clear that all publish entry points honor, then optionally detach the bulk deletion.

For Agents
In DKGPublisher.publishFromSharedMemory and the named publish path, preserve the post-confirm consumption contract before resolving a successful publish. If the full graph delete is too expensive to await, write and check an awaited per-root/per-share consumed marker before returning, then let only the bulk deletion run detached. Add a delayed-cleanup test where a second sequential publish of the same roots is rejected.

No caller-level regression test proves publish returns before a blocked tail

What's wrong
The added unit tests verify the generic helper, but they do not verify the user-facing publish methods that were changed to use it. This PR is specifically meant to stop confirmed publishes from timing out behind SWM cleanup and metadata writes, and that behavior can regress at the integration point while all helper tests remain green.

Example
A regression could accidentally restore await this.clearPublishedSwmRoots(...) in publishFromSharedMemory, or omit the metadata tail wrapper in publishQueuedKnowledgeAssetVmPublish. The new core helper tests would still pass, but a confirmed publish with a blocked store cleanup would again wait for cleanup before returning instead of responding after DKG_PUBLISH_TAIL_GRACE_MS=0 or a tiny grace.

Suggested direction
Add at least one focused publish-path test with a delayed or manually controlled store cleanup/meta mutation and a tiny grace budget. The assertion should be on the public publish method's timing/result, not only on awaitTailWithGrace in isolation.

For Agents
Add caller-level regression coverage around DKGPublisher.publishFromSharedMemory and the agent publish path that forces the cleanup/meta write promise to stay pending past a tiny grace, then asserts the publish method returns a confirmed result before the tail resolves and that the tail still runs/logs settlement afterward. Keep existing healthy-path cleanup assertions for the within-grace behavior.

* eventually settles (with the error when it failed, `undefined` on
* success), guaranteeing the rejection is consumed.
*/
export async function awaitTailWithGrace(

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: The tail-grace abstraction is too low-level, so the policy is still scattered across call sites

What's wrong
The new core helper only races a pre-started Promise<void>, so every caller still has to know the protocol for starting an IIFE, resolving the env grace, wiring detached settlement, deciding whether in-grace errors propagate, and formatting logs. That leaves the new behavior as repeated incidental control flow rather than a single canonical publish-tail policy. It is especially visible in the keep-root stamp, which has an extra catch solely because the helper's default error semantics do not match that tail's best-effort semantics.

Example
The keep-root path has to create keepRootStamp, define keepRootOnSettled, call awaitTailWithGrace, and add a catch that feeds back into the same callback. The SWM cleanup and metadata-tail paths duplicate the same shape with slightly different log text.

Suggested direction
Raise this to a publish-tail policy helper, e.g. runPublishTailWithGrace({ ctx, logger, label, graceMs, errorPolicy }, work), or provide explicit propagateWithinGrace vs bestEffort modes. Let that helper own grace resolution, attaching the late rejection handler, and the standard detached/completed logging so the call sites describe only the work being delayed.

For Agents
Look in packages/core/src/publish-tail-grace.ts and the three new call sites. Preserve the current grace behavior: in-grace failures should behave according to each tail's existing policy, post-grace failures must be consumed and logged, and pending tails must continue. Prove the helper with the existing publish-tail-grace tests plus at least one call-site-level test or focused unit that shows the call site no longer needs bespoke try/catch/log glue.

// store queue cannot hold the publish response hostage; on a healthy node
// it finishes inside the grace and behavior is unchanged, including error
// propagation.
const metaTail = (async () => {

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: The detachable metadata tail should be a named operation, not a 140-line inline async closure

What's wrong
The PR identifies a real conceptual boundary: everything after this point is post-response metadata tail work. But instead of making that boundary part of the structure, it wraps a large, already dense block in an immediately invoked async function inside a 5k-line file. That makes the publish flow harder to scan and makes future edits riskier because the detached work's scope is defined by distant braces rather than a named operation.

Example
A reader has to scan from const metaTail = (async () => { through all VM pointer stamps, lifecycle flips, publishedUal writes, assertion graph updates, and swmShareComplete cleanup before reaching the line that actually applies the grace. The new async boundary is not visible from a named function or indentation structure.

Suggested direction
Use the new tail boundary as the decomposition point. Move the metadata bookkeeping into a focused writePublishMetadataTail/runPostPublishMetadataTail helper that receives the few values it needs, and keep the main publish flow as: compute result, run metadata tail with grace, return result. That deletes the hidden closure and makes the detachable work explicit without changing behavior.

For Agents
Extract the block currently wrapped by metaTail in packages/agent/src/dkg-agent-publish.ts into a named private method or local pure orchestration function such as writePublishMetadataTail(...). Preserve all existing internal best-effort logging and the final return shape. Then call the publish-tail grace runner around that named work. Existing publish/finalize tests should still cover the metadata rows; add a narrow test if extracting changes the observable sequencing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants