Codex/v10.0.9 testnet canary - #1845
Merged
Merged
Conversation
…tion Fix/10.0.8 sync materialization
Move the root package and all 20 packages/* workspaces to 10.0.8 in lockstep, per the single-version release set rule in RELEASE_PROCESS.md §3, and add the 10.0.8 CHANGELOG entry. Version-only bump: pnpm-lock.yaml records third-party versions only and is untouched, so `pnpm install --frozen-lockfile` stays valid. `pnpm release:verify-versions --version 10.0.8` passes (21/21). No Solidity source, ABI, or deployment-registry changes since v10.0.7, so this release requires no contract deployment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
chore(release): bump version set to 10.0.8
Expose StorageACK peers for reliable remote queries
Keep test/publish synced with main
GH#1778 reported that a sealed rootless named KA shared member -> curator over SWM is "unsealed" on the curator and cannot be published to VM, and proposed adding the seal to the SWM wire protobuf. Investigation (incl. executing the real durable-sync + seal modules) showed the seal never rode SWM gossip in any version, so there is nothing to restore there. The seal already reaches the curator via the durable `_meta` sync lane. The real failure is two stacked off-chain defects, so no wire/protobuf change (and no metadata mirror) is needed: - Defect A: VM publish built the seal-lookup URI from the CALLER (curator) address, not the KA author. The seal sits at .../assertion/<member>/<name>; publish queried .../assertion/<curator>/<name>, found nothing, and reported "is not finalized" (surfaced as "unsealed"). Fix: resolveAssertionAuthor() reads the author from local `_meta` (prefer the caller's own KA; else the sole other author; else 409 AMBIGUOUS_ASSERTION_AUTHOR). The caller hint is the effective publish identity, so tokenless self-publish is unchanged. The route's existing "author cannot be supplied on vm/publish" 400-gate stays. - Defect B: the durable-sync integrity filter stripped dkg:assertionVersion from a not-yet-published seal subject (13/14 quads), making parseAssertionSealQuads throw "Partial graph-scoped assertion seal" once Defect A was fixed. Fix: admit assertionVersion for a self-consistent seal subject (carries assertionMerkleRoot, contentScopeVersion=2, single kaUal whose author == single authorAddress == the /assertion/<addr>/ segment). The seal's 13 sibling quads already sync unauthenticated; the real integrity boundary is the publish-time Merkle recheck, which stays authoritative. No receiver/wire/ACK changes; the receiver still writes no context-graph `_meta`. Adds unit + resolution + adversarial durable-sync regression tests. Also corrects two stale comments about `_meta` replication / seal transport. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review follow-ups on PR #1780: - BUG (otReviewAgent 🔴): `opts.agentAddress` was reused as the #1778 caller hint, but for direct programmatic callers it is an AUTHORITATIVE author selector. A caller requesting an author with no seal could be silently served a different same-named author. Split the roles: `agentAddress` stays authoritative (no substitution); the daemon publish routes now pass `callerAgentAddress` (token/caller identity) and only that hint drives resolution. Extracted the shared `resolveFinalizedAssertionPublishAuthor` helper so the sync and async publish paths cannot drift (🟡). - Centralised the assertion-coordinate shape in one core helper `parseContextGraphAssertionUri` (inverse of `contextGraphAssertionUri`), used by both the publish author resolver and the durable-sync seal identity check, replacing the bespoke prefix/suffix and regex parsing (🟡). - Tests: explicit-agentAddress-not-substituted; async intent auto-resolution; route-level 409 `AMBIGUOUS_ASSERTION_AUTHOR` { candidates } for sync and async publish; core round-trip for the new parser. Updated two existing route tests to the callerAgentAddress contract. core 1238 / agent 1120 green; CLI publish/route suites green (the remaining Windows-only CLI failures are pre-existing Hardhat-context / native-binary / unix-permission env issues, unrelated to this change). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…alisation Follow-ups on PR #1780 (otReviewAgent round 2): - BUG (🔴): `parseContextGraphAssertionUri` counted URI segments, but `validateContextGraphId` permits `/` (wallet-scoped CG ids like `0xabc…/project`), so such ids were mis-split into a subgraph and #1778 author resolution fell back to "is not finalized". Reworked the parser to anchor on the `/assertion/<addr>/<name>` suffix from the RIGHT and return the raw `scope` (cgId or cgId/subGraph, not split — a slash-containing cgId cannot be separated from an optional subgraph by the URI alone). Consumers compare `scope` to the known cg. Added core round-trip tests for slash cgIds (with/without subgraph) and agent resolver + durable-sync tests. - Moved the graph-seal self-consistency check into core as `graphScopedSealAuthor` (beside `ASSERTION_SEAL_PREDICATES` / `parseAssertionSealQuads`); durable-sync now calls it and no longer re-declares seal predicate constants (🟡). - Extracted the store/URI/EVM author lookup out of the large publish mixin into a focused `finalized-assertion-author.ts` module; the mixin method delegates (🟡). - Deduplicated the ambiguous-author 409 mapping behind `respondAmbiguousAssertionAuthor`, used by both publish routes; centralised the route caller-hint via `publishCallerHintLane` (🟡). - Enforced the selector-vs-hint contract: `resolveFinalizedAssertionPublishAuthor` now rejects supplying both `agentAddress` and `callerAgentAddress` (PUBLISH_AUTHOR_SELECTION_CONFLICT) (🟡). - Added subgraph author-resolution coverage (root/subgraph do not cross-match) (🟡). core 1238 / agent 1124 green (relay.test.ts flakes only under full-suite load; passes in isolation — unrelated to this change). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…l version Follow-ups on PR #1780 (otReviewAgent round 7): - BUG (🔴): the round-6 seal-version classifier admitted `dkg:assertionVersion` whenever the seal IDENTITY was self-consistent, but never checked the admitted field itself. A peer could append a second, conflicting `assertionVersion` (e.g. "1" and "999") and both rows were admitted, letting the full parser's last-writer-wins pick the tampered value. Now the seal predicate requires exactly ONE distinct `assertionVersion` (alongside kaUal/authorAddress), so a conflicting version drops ALL version rows and publish stays fail-closed. New durable-integrity test: two conflicting versions ⇒ none admitted, parse throws. - Renamed the core helper `graphScopedSealAuthor` → boolean `isSelfConsistentGraphScopedAssertionSeal` (🟡): it is a narrow durable-sync admission predicate, not a general author extractor — the boolean contract matches its only use and no longer reads like a reusable core author resolver. - System-override coverage (🟡): added a test that routes selection through `selectSystemOverrideMetadataIndexes` (acceptUnverified + orphan control) and proves the self-consistent seal keeps `assertionVersion` while the orphan control is dropped — the second admission site the classifier touches. core (isolation) / agent 1127 green; probe 14/14. (Full-suite core failures are flaky libp2p relay/networking tests, unrelated — they pass in isolation.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… default identity Round-8 follow-up on PR #1780 (otReviewAgent). Round 6 made async CG auto-registration use only `agent.getDefaultAgentAddress()` (collapse-proof, never the KA author). But a node with NO default identity then registered with no EVM actor at all, so a self-authored async publish (author == caller) to a fresh CG could fail at registration — a regression from the pre-PR `request.agentAddress` fallback. Fix: `getDefaultAgentAddress() ?? request.agentAddress`. The node identity still wins whenever present (preserving the round-6 collapse/curator fixes), and the degenerate no-default deployment falls back to the request author (correct on the self-publish path where author == caller). New test covers the no-default fallback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Round-9 follow-up on PR #1780 (otReviewAgent). - BUG (🔴): author resolution let ANY `_meta` subject carrying `dkg:assertionMerkleRoot` participate in selection, but durable `_meta` can hold stale/partial rows or unauthenticated fragments. A partial `OTHER/name` (merkleRoot only) alongside a valid `MEMBER/name` produced a false `AMBIGUOUS_ASSERTION_AUTHOR`, and a partial-only subject was selected and then failed at publish as a corrupt seal (500) instead of "not finalized". Fix: the resolver now CONSTRUCTs each candidate subject's full `_meta` rows and admits only those that `parseAssertionSealQuads` accepts as a complete graph-scoped v2 seal — the exact check publish performs — so a resolved author is always publishable and a partial subject is treated as not-finalized. New tests: valid + partial-only ⇒ resolves the valid author (no false ambiguity); partial-only ⇒ undefined (no unusable author). - Nit (🔵): reordered `respondAmbiguousAssertionAuthor` above the `respondAssertionError` JSDoc so that doc-comment reattaches to its function. core / agent 1129 / cli routes green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…andidate check Round-10 follow-up on PR #1780 (otReviewAgent). - BUG (🔴): the round-9 resolver validated that a candidate's rows parse as a graph-scoped seal, but then read the author from the SUBJECT URI without checking it against the seal's own `authorAddress`/`kaUal`. A malformed `_meta` with a complete seal at `.../assertion/0xMember/report` whose fields name `0xOther` would resolve `0xMember` while the attestation/scope belong to `0xOther` — a mismatched-identity publish. Fixed by requiring the subject coordinate author, `authorAddress`, and `kaUal` author to all agree. - Unified the two seal-candidate definitions (🟡): introduced ONE canonical core helper `parseGraphScopedAssertionSealCandidate(rows, subject)` — complete v2 seal + single-valued identity/version fields + coordinate/author/kaUal alignment — returning the parsed seal + coordinate. Both VM-publish author resolution (`finalized-assertion-author.ts`) and durable-sync admission (`durable-integrity.ts`) now consume it, replacing the previous `isSelfConsistentGraphScopedAssertionSeal`, so the definition of "publishable graph-scoped seal" lives in one place and cannot drift. New resolver tests: coordinate-mismatched seal is ignored (alone and alongside an aligned seal). Durable-sync + probe unchanged (14/14). core 104 / agent green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ane consistency) Human review (branarakic) Major concern: async VM-publish CG auto-registration stamped the curator with the NODE default identity (round-6/8 change), while the sync `vm/publish` lane stamps it with the token holder (`callerAgentAddress: requestAgentAddress`). `callerAgentAddress` becomes the CG curator via `stampAddressCurator`, so the same CG got a DIFFERENT curator depending on which lane registered it — a multi-tenant inconsistency. Fix: persist the enqueuing caller on the queued intent and register the async lane under it, matching sync. - `KnowledgeAssetVmPublishRequest.callerAgentAddress` re-added (type + persisted parser), distinct from the resolved-author `agentAddress`. - `resolveFinalizedAssertionVmPublishIntent` sets it to the enqueuing caller (`opts.callerAgentAddress ?? opts.agentAddress`), kept OUT of intentKey so it never forks dedup. - lifecycle.ts registers with `request.callerAgentAddress`; when absent (tokenless / pre-#1778 job) it passes none and `stampAddressCurator` falls back to the node default — exactly as the sync lane does when requestAgentAddress is undefined. Deduped jobs stamp under the first enqueuer, matching sync's first-writer-wins. Also: moved the dangling `parseAssertionSealQuads` doc block down to its export (review nit), and added an adversarial durable-sync test pinning the fail-closed behavior of a seal-plus-planted-`merkleRoot` subject. Rebased onto origin/main. agent 1132 / publisher 165 / cli routes green; probe 14/14. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…eads)
The round-10 CONSTRUCT { ?s ?p ?o } over the CG _meta graph tripped the SPARQL
scalability lint (R4: unbounded scan over a fleet-growing graph). Split into a
bound-predicate SELECT that finds the ~1 candidate subject by name, then an
EXACT-subject CONSTRUCT per candidate (a bounded per-subject read, lint-exempt).
Behaviour unchanged - same complete/self-consistent seal validation.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Answers one product question end to end, through the daemon HTTP API a real
agent uses — no fixtures, no library shortcuts:
"Can any agent subscribing to a public Context Graph reliably converge to
both the shared working memory and the finalized verifiable memory corpus?"
Covers both PUBLIC policy cells (open + curated), three receiver shapes
(author / live / late), curated authorization negatives, and an opt-in
RESILIENCE phase (scale, idempotence, author outage, hold-out).
WHY THIS EXISTS. It found three real defects that unit tests and code review
missed, two of which had been misdiagnosed twice by reading the code:
1. Markdown KAs replicating to peers with metadata but EMPTY entities/triples.
2. Member->curator SWM permanently dropped on public/curated CGs — the share
reported success while the write went nowhere.
3. Public SWM catch-up fetching and verifying every snapshot, then never
materializing it: a node that missed the live gossip stayed empty forever
("0 data + N meta triples"). Matches a live production symptom.
CONFOUND CONTROLS, each added after a green check turned out to measure nothing:
- Content presence is asserted SEPARATELY from sync-mechanism progress. A
receiver can hold content while the chain-reconcile watermark is stuck at 0.
- The decisive receiver is chosen AFTER the publish, from nodes provably outside
the storage-ACK set, and must differ from the live receiver's node.
- Corpus counts bind the published content subjects and require EXACT equality.
Counting every quad in the CG's graphs sweeps in metadata and passes while the
content is incomplete (observed 1450 against a 1000-quad target).
- Pre-subscribe baselines are recorded, and a receiver that already held the
corpus is reported NOT DECISIVE instead of passing.
- The hold-out proves reconstruction by ORDERING (the CG is created after the
node is stopped), because once catch-up works it converges faster than a
baseline read.
STRUCTURAL LIMITS THIS MAKES VISIBLE. A single-box devnet cannot produce a
non-host subscriber below ~5 nodes (ACK quorum is 3, so every peer must ACK),
and public gossip pre-populates every RUNNING node regardless of subscription.
Both hid real defects. The hold-out (stop a node before publication, publish,
restart via `devnet.sh restart-node`) is the only construction here that makes
corpus reconstruction decidable.
Also documents two operational traps: default devnet ports collide with a
locally-running production node, and `restart-node` re-derives every port from
env, so a caller that omits LIBP2P_PORT_BASE silently returns a node on the
default port where no peer can reach it while its API still answers 200.
Usage:
node devnet/public-cg-sync-proof/proof.mjs
RESILIENCE=1 NUM_NODES=6 node devnet/public-cg-sync-proof/proof.mjs
node devnet/public-cg-sync-proof/verify-fixes.mjs
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… graphs Sender and receiver decided the SWM encryption requirement from different authorities, and disagreed on exactly one set: accessPolicy=0 AND agent-gated — the public/curated cell. SENDER (packages/agent, resolveWorkspaceRecipientsGated) short-circuits on a LIVE on-chain accessPolicy of 0 and gossips PLAINTEXT, deliberately ignoring the agent gate. That is correct and deliberate: on a public CG the allowlist governs PUBLISH AUTHORITY, not READ ACCESS, so there is nothing to keep confidential. Encrypting instead bootstraps a sender-key handshake that non-gated recipients reject, which previously surfaced as HTTP 500 on promote — the bug packages/agent/test/swm-public-cg-plaintext.test.ts exists to prevent. RECEIVER (workspace-handler) required encryption whenever agentGateAddresses was non-null, derived from local allowedAgent/participantAgent triples — intent, not authority. It therefore dropped the sender's plaintext with retryable:false while the sender reported success, so every member->curator SWM share on a public/curated CG failed permanently and silently. Reproduced on a 6-node devnet: member share returns status=swm-shared, curator logs "Sender Key encrypted workspace payload required ... (permanent rejection)", content never converges, and the curator's subsequent vm/publish 409s as not finalized. FIX: the receiver now consults the SAME live on-chain predicate the sender uses, injected as publicAccessPolicyOnChainOracle alongside the existing chainAgentGateOracle and wired to isContextGraphPublicOnChain. An agent gate forces encryption UNLESS the CG is proven public on-chain. Fail-closed throughout — absent oracle, false, or a throw all mean "not proven public" and keep the requirement, so a stale mapping or RPC flake can never become a plaintext-acceptance hole. A sender-side fix was tried first and reverted: it broke 2 of 38 tests in swm-public-cg-plaintext.test.ts, whose assertion `store.query.calls == []` requires the public path not to touch the store at all. TEST COVERAGE. New packages/publisher/test/swm-public-gated-plaintext-accept.test.ts covers public+gated accept, gated-not-public require, probe-throws fail-closed, no-oracle fail-closed, and ungated unaffected. It was MUTATION-TESTED: restoring the pre-fix behaviour makes the public+gated case fail. CI GAP CLOSED. Both vitest.unit.config.ts files use explicit include lists, and neither the new test NOR swm-public-cg-plaintext.test.ts was listed — the latter guards a previously-fixed production bug and was not running in CI at all. Both added: publisher 37->38 files / 422->427 tests, agent 90->91 / 1132->1170. Mixed-fleet note: un-upgraded receivers keep rejecting plaintext on agent-gated public CGs until they take this build. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ch-up A node that missed the live gossip never obtained any SWM content. Reproduced deterministically: hold a node out of a publication cycle, restart it, subscribe — 0 of 100 quads after 180s, from five healthy holders. Now 100/100 in ~3s. "0 data + N meta triples" was a red herring: it is CORRECT responder behaviour. Graph-scoped (contentScopeVersion 2) KAs carry no dkg:rootEntity, so the aggregate data phase legitimately returns nothing for them — their content travels as immutable snapshots instead. The catch-up lane fetched and VERIFIED those snapshots and cached them, then never wrote them to the triple store. The held-out node was already holding swm-public-snapshots/81/98/8198388b...nq — the exact 20 quads for KA 27 — on disk, unmaterialized. The asymmetry: live gossip materializes (gossip-publish-handler), durable/VM sync materializes (materializeVerifiedGraphScopedAsset), and PRIVATE CG recovery materializes (swm-recovery.ts materializeReadySnapshot) — but the PUBLIC catch-up lane omitted the step. syncPublicSnapshotsForMeta already exposed an onSnapshotReady hook; the public caller simply never passed it. FIX mirrors the private lane: parse graph-scoped descriptors from verified meta, pass onSnapshotReady, and materialize each verified snapshot via replaceGraph. Deliberate properties: - replaceGraph, NOT insert. A KA graph is all-or-nothing and digest-verified; union-insert risks partial or duplicated graph state across retries, and would bypass per-KA digest verification. - Gated on wsMetaResult.completed. parseGraphScopedSwmRecoveryDescriptors throws on incomplete metadata and this lane pages meta, so an ungated parse would abort the whole context-graph fanout on a timed-out page. - Per-KA error isolation: one unmaterializable snapshot must not take down the rest of the corpus; the phase stays incomplete so the scheduler retries. - storeReplaceGraph is optional on the context so existing callers and test rigs compile unchanged; when absent, materialization is skipped rather than half-applied. TRAP AVOIDED: the tempting fix is to make the data lane work — resurrect dkg:rootEntity, or make readFreshSwmRoots match graph-scoped heads. That reintroduces the O(#KA) aggregate scan the graph-scoped design exists to eliminate and double-transports content. The defect is in materialization, not in the data lane. Because onSnapshotReady fires for 'cache' as well as 'network', nodes that already cached snapshots materialize them on the next pass without refetching. This matches the production symptom on the operator's Base node (fifa CG: query-remote returns 4606 quads while catch-up reports data=0), which may therefore be recoverable locally once this ships. Devnet, clean 6-node, hold-out decisive (baseline 0): before INCOMPLETE — 0/100 after 180788ms after 100/100 quads in 3019ms from remaining holders Full gate 33/35; the two remaining failures are the known redundant chain-reconcile watermark. Agent unit suite 1170 passing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ip graphs The materialization fix in d0373b704 replaced KA graphs unconditionally. Live gossip may already hold a RICHER version of the same graph, and replaceGraph is destructive, so catch-up silently DESTROYED content the node already had. Caught end-to-end: a peer that previously converged at 76 quads regressed to 27. Unit tests were green and the build was clean — only the devnet run showed it. The private recovery lane has a guard this port dropped: isGraphAssetMaterialized (an ASK for the head's dkg:assertionGraph marker) skips replacement when the graph is already present. Now wired on both sides, and materialization refuses to run at all when the guard is unavailable rather than proceeding blind. Also corrects the hold-out's decisiveness gate. It required a 0-quad baseline after restart, but that is a PROXY for being held out, and once catch-up works it loses the race: the node can materialize the corpus between restart and the measurement. The real proof is ORDERING — the context graph is created AFTER the node is stopped, so any content it holds must have arrived post-restart, because it did not exist before. Baseline is now INFO; the convergence assertion stands on ordering alone. Verified on a clean 6-node devnet, both directions of the trade-off: #1779 markdown peer population author=78 / peer=76 (was 78/27 while unguarded) hold-out reconstruction 100/100 in 3023ms, baseline 0 full gate 33/35 — the 2 remaining failures are the known redundant chain-reconcile watermark agent unit suite 1170 passing Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The SPARQL scalability lint blocked this PR with 5 findings of R2 graph-var-scan — "all-variable triple inside GRAPH ?var", the #1597 listGraphs-storm shape. The lint was right: the corpus counts were `GRAPH ?g { ?s ?p ?o }` filtered by a string CONTAINS, which enumerates every graph x every triple. BOUNDED (the real fix, 3 queries): the scale and hold-out counts now bind the exact subjects this run published with `VALUES ?s { ... }` instead of scanning and filtering. Cost becomes proportional to what the run published rather than to store size. These still trip the lint statically because the VALUES list is a template interpolation the scanner cannot resolve, so each carries a pragma stating precisely that. ACKNOWLEDGED (2 queries in verify-fixes.mjs): the markdown checks cannot be bound the same way — markdown import SKOLEMIZES its subjects (urn:dkg:ka-skolem:cN), so they are not knowable in advance. Scope is one purpose-built devnet store holding only this run's fixtures, and this file is never executed by node runtime code. Note for future harness work: pragmas must sit within 4 lines of the query literal (collectPragmas scans literalStartLine-4), so a longer justification comment silently fails to register. sparql-scale-lint: 0 unacknowledged findings across 2 files. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ed unit
Review found the first cut of this fix inverted the failure instead of
removing it: driving requiresEncryptedPayload to false for a public+agent-gated
CG also flipped the later validation from "encrypted required" to "encrypted
not supported", so any ENCRYPTED write for the same CG was rejected with
retryable:false. Sender and receiver chain probes legitimately disagree during
RPC flakes, stale mappings, rollout skew, or older clients — and a sender that
cannot prove public FAILS CLOSED and encrypts, which this receiver would then
permanently drop: the mirror image of the plaintext drop being fixed.
The two questions are now separate, in one exported decision the handler
itself consumes (resolveWorkspaceEncryptionRequirement):
requiresEncryptedPayload MUST it be encrypted? private, or gated and not
proven public on-chain (policy, chain-derived)
supportsEncryptedPayload MAY it be encrypted? private, or gated at all
(structure, local)
Both encodings are admitted during any skew window; an ungated public CG
still refuses Sender-Key payloads exactly as before. Applied to BOTH encoding
branches — the review flagged the senderKeyMessage arm, and the same flaw was
in the encryptedPayload arm.
Also from review: the regression test mirrored the fix instead of exercising
it — it reimplemented the boolean against a stubbed private helper, so
reverting the production decision would have kept it green. The decision is
now an exported function precisely so the test covers shipped logic; the suite
asserts the public/curated accept, the fail-closed unproven case, that a
public proof never downgrades a PRIVATE CG, the unchanged ungated behaviour,
and the skew invariant (never REQUIRE without SUPPORT) across all eight input
combinations.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review found the #1779 verification could pass without proving markdown content reached the peer: its filter matched any literal carrying the run stamp, which includes the context graph's own name ("FX md <stamp>"). A peer holding metadata and ZERO imported content — the exact #1779 signature the check exists to detect — could still count that literal and report PASS. The query now binds distinctive body strings that appear only inside the imported document and excludes the _meta graphs, so it can only be satisfied by the markdown content itself. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…content Closes the remaining review blockers on the catch-up materializer. The theme of all of them: the marker could exist without the graph, and the destructive replace could run against state it had not re-verified. LOCK (review P1). materializeReadySnapshot now runs inside the SAME per-KA write lock the live-gossip path takes: swmKaWriteLockKey() is a new shared helper in keyed-lock.ts consumed by BOTH call sites (a hand-rolled copy of the key format would fail silently — an unequal key does not error, it just stops serializing), and the agent passes withKeyedLocks over this.writeLocks — the exact map it already injects into SharedMemoryHandler. The key lowercases the UAL segment: address case varies by source, an under-merged key recreates the race, an over-merged one merely coarsens serialization. VERSION ORDERING (review P1). A lock prevents interleaving, not overwriting-with-older: gossip may advance the KA while catch-up waits. All decisions moved INSIDE the lock, starting with a stored-head assertionVersion read — stored newer than descriptor => skip; unparseable => skip, because state whose ordering we cannot establish must not be destroyed. CONTENT-PROVING GUARD (review P1). isGraphAssetMaterialized now counts the assertion graph and requires exact equality with the descriptor's publicQuadsCount. The prior marker ASK classified the PRE-FIX broken state (head metadata written, graph never written — the observed "0 data + N meta") as materialized, so the repair skipped exactly the nodes that need it, and a partially-fetched metadata round could strand an asset forever behind its own marker. Content-equality also makes multi-round metadata self-healing: a marker without its graph no longer blocks anything. COHESIVE DEPENDENCY (review). The loose optional trio becomes one snapshotMaterializer object — a caller can no longer half-configure materialization silently. TESTS, in CI's include list, driving the REAL runSharedMemorySync with the REAL lock functions: - held-out node materializes a cached snapshot (the production repair path) - the gossip race, deterministically: the test holds the actual lock as "gossip" (the hold IS the pause), commits version 2 while catch-up is provably blocked, releases, and asserts replace never fires — with a checksummed-case UAL on the gossip side so key normalization is exercised - pre-fix broken state heals (marker present, content absent => replaced) - already-materialized asset untouched - failed replace withholds the meta insert and fails the phase Both load-bearing behaviours mutation-tested: disabling the version re-check kills exactly the race test; swallowing failures kills exactly the meta-withholding test. Agent and publisher suites green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… binding Catch-up's meta tail is a union insert outside the lock, so a stale head row can coexist with gossip's newer one. An unordered SELECT taking bindings[0] could then return the older version, defeating the in-lock ordering guard and re-enabling overwrite-with-older on a later pass. Reading the maximum is always the conservative direction. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
# Conflicts: # packages/publisher/src/workspace-handler.ts
# Conflicts: # packages/agent/vitest.unit.config.ts
| // identity — again exactly as the sync lane does when `requestAgentAddress` | ||
| // is undefined. Deduped jobs stamp under the first enqueuer's caller, which | ||
| // is the same first-writer-wins the sync lane already has. | ||
| await agent.ensureRegisteredForPublish(request.contextGraphId, { |
There was a problem hiding this comment.
🟡 Issue: Old async publish jobs can auto-register a Context Graph under the wrong curator after upgrade
What's wrong
The async retry path no longer falls back to request.agentAddress when callerAgentAddress is missing. That is correct for new foreign-author jobs, but it changes the meaning of already-persisted jobs created before this field existed. If such a job is the one that triggers publish-time CG auto-registration, the chain registration can be stamped with the node default rather than the agent that queued the job.
Example
A multi-agent node running 10.0.7 enqueues an async VM publish for token agent 0xAAA against an unregistered CG, then upgrades before the worker retries. The persisted request has agentAddress=0xAAA and no callerAgentAddress. On CG_NOT_REGISTERED, the new worker calls ensureRegisteredForPublish(contextGraphId, {}) and stamps the node default curator instead of 0xAAA.
Suggested direction
Keep the new caller-vs-author split for new jobs, but add an explicit compatibility path for persisted requests that predate callerAgentAddress so their original request agent is not lost.
Confidence note
This depends on existing persisted async publish jobs that were enqueued before this PR added callerAgentAddress, but those jobs are a normal upgrade state for a release bump.
For Agents
In packages/cli/src/daemon/lifecycle.ts, preserve upgrade compatibility for persisted pre-#1778 KnowledgeAssetVmPublishRequest rows. Add a migration or fallback that distinguishes old requests from new curator-publishes-member requests, and test an old queued request with agentAddress set and callerAgentAddress absent through the CG_NOT_REGISTERED retry path.
| expiresAt: Date.now() + DURABLE_DATA_SYNC_SESSION_TTL_MS, | ||
| } | ||
| : responderSession; | ||
| persistUnfinishedSyncResponderSession( |
There was a problem hiding this comment.
🟡 Issue: Failed durable verification can retry against the same rejected responder snapshot
What's wrong
The requester now persists the responder session before the durable verification/store layer knows whether the fetched row list was usable. When that later layer rejects the batch and leaves the offset unchanged, the next retry can reuse the same stale immutable responder row list at offset zero, delaying recovery from a responder-side race or healed metadata until the session expires.
Example
A durable data fetch completes at offset 0 with a responder session, but processDurableBatchInWorker rejects one graph because the responder snapshot was missing matching metadata. The requester does not advance the checkpoint, but page-fetch has persisted the responderSessionId. The next retry sends offset=0 with the same syncSessionId, so the responder reuses the same immutable bad row list instead of rebuilding from its now-healed store.
Suggested direction
Avoid persisting, or explicitly clear, the responder session when the higher durable layer rejects the fetched batch or cannot materialize it. Persisting the token should be tied to a verified safe checkpoint, not just successful transport completion.
Confidence note
The stale-session loop lasts until the responder session TTL rather than forever, but it directly undermines retry behavior for failed durable verification/materialization attempts.
For Agents
Look at fetchSyncPages session persistence and runDurableSync's rejected/failed verification branches. Preserve session tokens only after the caller has either committed a safe prefix or can still verify against that same immutable list; clear the responder session when verification/materialization rejects the fetched snapshot. Add a test where a completed fetch is rejected, the responder data changes, and the next retry must mint a fresh session.
| // incomplete metadata, and this lane pages meta, so a timed-out page would | ||
| // otherwise abort the whole CG fanout. A parse failure here must degrade to | ||
| // "no materialization this round" — never take down the sync. | ||
| const snapshotDescriptorsByRef = new Map<string, GraphScopedSwmRecoveryDescriptor[]>(); |
There was a problem hiding this comment.
🟡 Issue: Public snapshot materialization is embedded too deeply in sync orchestration
What's wrong
This adds a policy-heavy subsystem to an already central sync loop. Transport completion, snapshot cache behavior, destructive store replacement, race avoidance, and phase-completion semantics are now tied together through closure state instead of a clear boundary.
Example
A maintainer trying to reason about whether metadata insertion is safe now has to trace meta paging, public snapshot sync, a nested materializer, in-lock stale-version checks, replacement side effects, and the later snapshotPhaseUsable gate inside one function.
Suggested direction
Extract the materialization policy into a dedicated helper with an explicit input/output contract.
For Agents
In packages/agent/src/sync/requester/shared-memory-sync.ts, preserve current behavior but move the graph-scoped public SWM snapshot materialization into a focused helper/module, e.g. materializeVerifiedPublicSwmSnapshots(...), returning { materializedGraphs, materializedQuads, failures }. Keep runSharedMemorySync responsible for phase orchestration and summary merge. Existing swm-public-snapshot-materialization tests should continue proving the helper behavior.
| * store/URI/EVM lookup so it lives beside the coordinate helpers rather than | ||
| * in this publish mixin. See that function for the full resolution rule. | ||
| */ | ||
| async resolveAssertionAuthor(this: DKGAgent, |
There was a problem hiding this comment.
🟡 Issue: Avoid adding a public thin delegate to the publish mixin
What's wrong
This expands an already very large agent mixin without reducing complexity. It adds API surface and another concept for maintainers to understand, while the focused module already owns the actual store/URI lookup.
Example
The publish lane now has three names for one resolution path: the standalone resolveFinalizedAssertionAuthor, the pass-through resolveAssertionAuthor, and resolveFinalizedAssertionPublishAuthor. Only the standalone resolver and publish-specific resolver appear to earn their keep.
Suggested direction
Delete the wrapper layer unless it is a deliberate API surface.
For Agents
In packages/agent/src/dkg-agent-publish.ts, preserve the selector-vs-hint behavior. Collapse the pass-through method by calling resolveFinalizedAssertionAuthor directly from resolveFinalizedAssertionPublishAuthor, or make the wrapper private/internal if the class boundary truly needs it. Update tests to exercise the module or publish resolver rather than a new public delegating method.
| // permanently dropped the plaintext writes the sender is supposed to | ||
| // send on a public CG — silently breaking member->curator SWM shares on | ||
| // every public/curated context graph. | ||
| publicAccessPolicyOnChainOracle: (cgId: string) => |
There was a problem hiding this comment.
🟡 Issue: Public/curated plaintext acceptance is only tested as a pure helper decision
What's wrong
The changed behavior depends on the agent wiring the public-access oracle into the receiver and the receiver applying it while handling a real SWM envelope. The current added test pins the helper’s truth table, but it does not prove plaintext member-to-curator SWM writes are accepted by the actual handler on a public agent-gated Context Graph.
Example
A regression that removes publicAccessPolicyOnChainOracle from getOrCreateSharedMemoryHandler, or leaves the handler ignoring the oracle result, would still keep the pure resolveWorkspaceEncryptionRequirement(publicGated) tests green while a real public/curated plaintext SWM share is dropped again.
Suggested direction
Cover the receiver path that actually failed, not just the extracted boolean policy helper.
For Agents
Add a handler-level regression around SharedMemoryHandler: seed/return agent-gate metadata, provide publicAccessPolicyOnChainOracle: async () => true, submit a plaintext graph-scoped workspace message, and assert applied: true; also assert the same plaintext is rejected when the oracle returns false/throws. If practical, add a small DKGAgent wiring test that getOrCreateSharedMemoryHandler passes the oracle through.
Contributor
|
Heads-up for testers: the canary carries a receive-path latency regression (~3ms → ~33ms per SWM gossip receive — an unconditional chain RPC from the #1843 review fix). Symptom: slow SWM/VM arrival on receiving nodes; not data loss. Fix PR against testnet-canary: see fix/testnet-canary-lazy-probe. Note #1848 (subscription catch-up) addresses the same symptom area — worth confirming whether it remains needed, is complementary, or overlaps once the lazy-probe fix lands. |
Contributor
|
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. |
Contributor
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Summary
Changes
Test Plan
pnpm test)pnpm build)dkg start(if applicable)Related Issues