fix(swm): evaluate on-chain public probe lazily — restores receive-path latency - #1852
Conversation
…ated CGs
The must-vs-may refactor turned the probe unconditional: every SWM gossip
receive awaited isContextGraphProvenPublicOnChain (a live chain read through
resolveOnChainAccessPolicyState with its timeout machinery) even when the CG
has no agent gate — the case where resolveWorkspaceEncryptionRequirement never
reads the answer, because both requires and supports collapse to
hasPrivateAccessPolicy.
Measured on a 6-node devnet, that took SWM receive-apply from ~3ms to ~33ms
and turned the public-CG sync gate red in BOTH public cells: the author's next
convergence poll slipped one 3s cycle, the VM publish therefore landed AFTER
the live receiver's subscribe-triggered catch-up had already run, and — since
chain-promote defers without transaction provenance and the snapshot
materializer lane is separately parser-blocked ("ambiguous publishedAt") — the
live receiver starved until the ~5-minute periodic sweep. Symptom in the gate:
"LIVE receiver has VM content — TIMEOUT ... reconcile pending watermark=0/1"
at 150s, while the LATE receiver (fresh catch-up, runs post-publish by
construction) converged in ~3s.
Restore the pre-refactor evaluation order — probe only when
agentGateAddresses !== null — while keeping the must-vs-may split intact.
For a gated CG behaviour is unchanged.
Tests drive the REAL handler with a counting oracle, in the unit include list:
- ungated CG: write applies AND the probe is never invoked. Mutation-tested:
re-introducing the unconditional await fails exactly this test.
- gated CG: the probe IS invoked and a public proof admits plaintext, so
laziness cannot regress into "never".
Note: the underlying exposure is pre-existing and NOT closed here — a LIVE
subscriber whose catch-up ran before the publish has no event-driven VM
delivery lane (finalization broadcast schedules no durable sync; chain-promote
requires provenance it never gets; the SWM materializer heal lane dies on
"ambiguous publishedAt" fleet-wide). The baseline passes the gate only because
its receive path is fast enough that the publish beats the queued catch-up.
Deserves its own issue.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| hasPrivateAccessPolicy, | ||
| agentGateAddresses, | ||
| provenPublicOnChain: await this.isContextGraphProvenPublicOnChain(contextGraphId, ctx), | ||
| provenPublicOnChain: agentGateAddresses !== null |
There was a problem hiding this comment.
🟡 Issue: Encode the lazy probe invariant in the boundary, not in an inline call-site ternary
What's wrong
This fixes the immediate eager await, but leaves the original design trap intact: resolveWorkspaceEncryptionRequirement asks for a boolean that callers naturally compute before calling it, even though computing it is only valid for one branch. The production code now depends on a local special case and a very long comment to preserve evaluation order, which makes the handler more brittle and harder to scan.
Example
A future caller can still write resolveWorkspaceEncryptionRequirement({ hasPrivateAccessPolicy, agentGateAddresses, provenPublicOnChain: await probe() }) and reintroduce the exact hot-path RPC this patch is guarding against, because the helper contract does not encode the laziness invariant.
Suggested direction
Make the API shape carry the invariant instead of relying on a long comment plus a one-off conditional. The cleaner structure is: first derive whether the graph is gated, then only the gated branch has access to the on-chain probe result. That deletes the need for most of the explanatory incident narrative and makes the expensive call structurally hard to invoke on ungated graphs.
For Agents
Look at resolveWorkspaceEncryptionRequirement and its handler call site. Preserve the current must-vs-may encryption behavior, but move the on-chain probe decision behind a small boundary that owns laziness, such as an async handler-level resolveWorkspaceEncryptionRequirementForContextGraph(...), or change the policy input shape so a public proof cannot be supplied for ungated graphs. Keep the existing pure matrix test for the policy calculation and add/keep a handler-boundary test proving the ungated path does not call the oracle.
…the agent-to-handler oracle wiring Two review follow-ups on #1843: - resolveWorkspaceEncryptionRequirement (and its must-vs-may rationale) moves from the 2.3k-line workspace-handler.ts into workspace-encryption-policy.ts; the handler keeps only the local lazy-probe sequencing note, and the policy tests import the focused module instead of the orchestration file. - New agent-level regression builds SharedMemoryHandler through the REAL getOrCreateSharedMemoryHandler and delivers a signed plaintext write on an agent-gated CG with a counting isContextGraphPublicOnChain: the write must apply and the probe must be consulted. Removing the publicAccessPolicyOnChainOracle wiring from dkg-agent-swm-substrate.ts fails this test with the exact production drop it prevents (mutation-verified). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Cherry-picked |
|
|
||
| export interface WorkspaceEncryptionPolicyInput { | ||
| readonly hasPrivateAccessPolicy: boolean; | ||
| readonly agentGateAddresses: readonly string[] | null; |
There was a problem hiding this comment.
🟡 Issue: Keep the policy boundary about policy, not address-list representation
What's wrong
The new canonical policy module leaks a handler-specific representation into a pure decision helper. That makes the helper look like it depends on the allowlist contents when it does not, and it keeps the important invariant hidden behind a nullable collection convention.
Example
agentGateAddresses: [] and agentGateAddresses: ['0x...'] produce the same policy result. The addresses are not part of this abstraction; only gated-vs-ungated is.
Suggested direction
Make the extracted policy helper accept the concept it actually uses, for example { hasPrivateAccessPolicy, isAgentGated, provenPublicOnChain }. That removes the nullable-address sentinel from the canonical policy module and makes future callers less likely to cargo-cult the handler’s local storage shape.
For Agents
In packages/publisher/src/workspace-encryption-policy.ts, replace agentGateAddresses with isAgentGated or a small access-shape enum. Derive it once in workspace-handler.ts from agentGateAddresses !== null. Update the existing truth-table tests to prove behavior is unchanged.
|
|
||
| // Counting oracle on the AGENT method the substrate closure delegates to. | ||
| let probeCalls = 0; | ||
| (agent as unknown as { |
There was a problem hiding this comment.
🟡 Issue: Avoid protecting wiring through broad private-agent casts
What's wrong
The test is meant to guard an important production wiring boundary, but it does so by monkey-patching and poking private agent internals through broad casts. That creates a brittle, cast-heavy test shape and signals that the codebase lacks a clean seam for verifying SWM handler construction.
Example
A refactor that renames localAgents, changes its value shape, or changes getOrCreateSharedMemoryHandler internals can break this test at runtime while TypeScript remains silent because the test supplied its own structural type.
Suggested direction
Move the setup behind a focused test helper in packages/agent/test/_helpers, or add a narrow test seam that exposes exactly the dependencies this wiring test needs. The test should still build through the real accessor, but the private state mutation and method replacement should not be hand-typed at each call site.
Confidence note
This is test-only code, and wiring tests sometimes need white-box access; the concern is that this PR adds several casts at the exact boundary it is trying to protect.
For Agents
In packages/agent/test/swm-plaintext-oracle-wiring.test.ts, introduce a narrow package-local test fixture or constructor seam for building a DKGAgent with a counting public-on-chain oracle and seeded local agent membership. Preserve the behavior assertion that the real handler accessor passes the oracle through.
|
Step-1 canary composition is up as #1876 (this change included): current testnet-canary + #1871 + #1852, gated green on the 6-node devnet sync suite (verify-fixes 8/10 excepted-only; proof 33/35 and 32/34, watermark-only failures — full numbers in #1876). Merging #1876 supersedes #1852. Big-CG pair #1868+#1842 follows as step 2. |
2 participants
Symptom (on this canary)
Devnet sync gate regression in BOTH public cells: LIVE receivers timing out on VM content (previously ~3s), and markdown KAs reported EMPTY on peers within the 90s window. Testers may have seen slow SWM/VM arrival on receiving nodes — not data loss; content still converges via the durable lane.
Root cause
The #1843 review fix routed the receiver's SWM encryption decision through
resolveWorkspaceEncryptionRequirement, but the handler integration awaitedisContextGraphProvenPublicOnChainunconditionally — putting a live chain RPC on the hot path of every SWM gossip receive, including plain ungated public CGs where its answer is irrelevant (the helper only reads it behindisAgentGated).Measured on a 6-node devnet: receive-apply went ~3ms → ~33ms, slipping receiver polling by a full cycle so LIVE receivers missed the VM/markdown windows.
Fix
Evaluate the probe lazily — only when the CG is actually agent-gated. The ungated fast path never touches the chain.
Handler-boundary tests with a counting oracle (in the vitest include list):
Validation status
Unit + mutation coverage attached and green. The full devnet gate re-run is in progress in parallel (environment flakiness, not red results, has delayed it) — pushed now at operator request because the team is actively testing on the regressed canary. Gate numbers will be posted here when the run completes; hold merge for them if you prefer.
Cherry-picked cleanly onto current
testnet-canaryhead (includes #1848).🤖 Generated with Claude Code