fix: 5 P0 pre-mainnet issues (#11, #1078, #1121, #936, #1098)#1228
Merged
Conversation
…le (#936) promoteSharedMemoryToCanonical assigned per-root tokenIds positionally over the oxigraph SPARQL binding order, which is store-history-dependent — so two replicas chain-reconciling the same KC minted divergent rootEntity→tokenId maps. Sort the roots lexicographically before the mint loop so the map is a pure function of the root SET (identical on every replica, both gossip and chain-reconcile paths), and emit explicit `<ual>/<n>` dkg:tokenId / dkg:entity rows so the map is queryable. Kept in the handler (not generateKCMetadata, which metadata.test.ts pins). Verified: repro green; 72 finalization/reconcile/collapse tests + full agent suite (1614) green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ate CGs (#1121) mapLiftRequestToPublishOptions never set encryptInlinePayload/encryptInlineChunked, so a private (ownerOnly/allowList) async-lift publish would ship PLAINTEXT to cores. Thread the agent-resolved encryption callbacks through LiftResolvedPublishSlice and, for any non-public CG, attach a FAIL-CLOSED default that throws if invoked without a real factory — so plaintext can never silently ship. Fix publisher-runner.ts merge precedence so the agent-resolved chainKey-bound AEAD closure wins over the default (else every real curated async publish would throw). Verified: repro green; async-lift-publish-options + metadata + full publisher suite (1179) green; publisher-runner cli tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…1078) PrivateContentStore keyed the finalized private partition only by (contextGraphId[,subGraphName]) → one `_private` graph, so two distinct private commitments for the same root commingled and getPrivateTriples returned BOTH — a privateDataAnchor on a verifiable KA could hydrate a different commitment's slice. Add an optional commitmentId to storePrivateTriples: when it differs from the commitment recorded for a root, the new commitment supersedes the stale slice (a marker on a non-hydrated subject tracks the current commitment). With NO commitmentId the behaviour is unchanged (append + dedup), so every existing caller and the multi-value-predicate tests are untouched. Wire the production write sites (sync publish + async-lift finalize) to pass the per-root privateMerkleRoot. Verified: repro green; full storage suite (218) + full publisher suite (1179) green (zero existing-test changes). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
saveOpWallets wrote raw secp256k1 private keys to wallets.json in plaintext (mode 0600 but cleartext) — on mainnet those hold real TRAC/ETH. Encrypt each key with AES-256-GCM under a machine-local 32-byte secret (wallets.key, zero operator interaction), optionally strengthened by DKG_WALLETS_PASSPHRASE via scrypt. Addresses stay plaintext so every address-only reader keeps working; loadOpWallets decrypts in memory so the chain config is unchanged. Legacy plaintext files load as-is and are NOT rewritten (so devnet provisioning + the live-daemon fixture, which read wallets[0].privateKey directly, keep working) — new nodes generate encrypted. Verified: repro green (no plaintext key anywhere on disk); op-wallets + chain-reset + publisher-wallets tests green; full agent (1614) + cli (2029) suites green; fresh 6-node devnet boots healthy. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ile (#1098, partial) PARTIAL FIX — closes layer 1 of #1098. A peer that subscribed to a PUBLIC CG BEFORE its first publish never bound sub.onChainId (only curated CGs bind on the ContextGraphCreated event; ACK-signers bind via the storage-ACK hook), so the chain-driven VM-reconcile sweep skipped it and the peer was stranded on unreliable one-shot gossip. Self-prime onChainId in runVmReconcileSweep (resolve from the local ontology/_meta OnChainId quad) and trigger a self-priming sweep from the live KA-registration nudge when no local CG is bound. Regression-clean: gated on subscribed && !onChainId (binding from undefined never resets the cursor); full agent suite (1614) + reconcile tests green. KNOWN-INCOMPLETE (layer 2): live devnet shows materialization is still unreliable (~1/3). Root cause diagnosed: once bound, the chain-reconcile DOES promote the KC, but it lands ONLY in the per-cgId graph `<cg>/context/<onChainId>` without the root-label dual-write copy (keepRootCopyOnLabel isn't recovered on a peer that missed the SWM-meta gossip), and the `verifiable-memory` view query resolves via the root-label `_meta` → misses per-cgId-only data. The fix belongs in either the query-engine VM-view graph resolution (resolve `<cg>/context/<onChainId>` from the bound subscription) or publisher keepRootCopyOnLabel-signal replication — a core view-routing change deferred pending review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…, completes fix) Completes #1098 (layer 2). The chain-driven VM reconcile materialises confirmed data into the per-cgId graph `<cg>/context/<onChainId>`, but the `verifiable-memory` view only resolved the root content graph + `_verifiable_memory/*` — so a peer that subscribed BEFORE publish and recovered via the reconcile sweep (per-cgId only, no root-label dual-write copy) had the KA materialised yet INVISIBLE to a VM query. Union the per-cgId DATA graphs (resolved from the store by prefix, excluding `/_meta` + nested sub-partitions) into the unscoped VM allow-set. No subscription state needed; additive to the allow-set so dual-written publishes (already in the root graph) just dedup. Verified on a live 6-node devnet: a pre-subscribed peer now materialises a published KA reliably — 5/5 rapid-fire + 4/4 paced (was ~1/3). Combined with the onChainId-binding commit this closes #1098. Cross-checked #936 on the same devnet: node2 + node3 chain-reconcile the same multi-root KC to an identical deterministic root->tokenId map. Full query suite (277) green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…publishes The #1121 fail-closed encryption default broke chainless / local-only publishes: an ownerOnly KA on an UNREGISTERED context graph (e.g. the kafka-plugin's single-daemon private-stream registration, or a chain-not-ready node) ships nothing to other nodes, so there is no plaintext-to-cores leak — yet the publisher took the encrypted-inline path purely because a callback was defined, invoked the fail-closed default, and threw ("requires an encryptInlinePayload factory"). Gate `useEncryptedInline` on `canAttemptOnChainPublish` so the encryption hook is only required when the publish actually goes on-chain (the path that collects core StorageACKs / distributes to members — the only place private data could leak in the clear). On-chain private publishes still require encryption; chainless/local publishes finalize locally as before. Verified: full publisher suite (1179) green; kafka-plugin live-daemon E2E (11) green (was 4 failed on the PR's first CI run). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🔴 #936 cross-path consistency — the publisher (DKGPublisher.publish) minted compatibility tokenIds in input-quad order while replicas sort, so the originator and replicas could map a different root to the same `<ual>/<n>` label. Sort canonical.manifestEntries lexicographically by rootEntity before the mint, matching promoteSharedMemoryToCanonical. Now all three mint paths (publisher, gossip, chain-reconcile) agree. 🔴 #1098 self-prime guard — the sweep skipped binding when the resolved on-chain id equalled the local CG id (a direct CG whose local id IS its numeric id, e.g. "42"), leaving it unbound forever. getContextGraphOnChainId never falls back to localCgId, so bind ANY non-null resolved id. 🟡 #1098 nudge locality — the KA-registration nudge ran a GLOBAL self-priming sweep whenever any subscription was unbound. Scope it: bind + reconcile only the subscribed-unbound CG whose resolved on-chain id matches THIS event. 🟡 test coverage: - op-wallets-at-rest: also scan adminWallet for plaintext + assert a reload decrypts back to the same admin + operational keys. - query/vm-percgid-resolution (new): unscoped verifiable-memory read returns data that lives ONLY in <cg>/context/<onChainId>, excluding meta/private. - agent/vm-reconcile-self-prime (new): a subscribed-but-unbound CG binds onChainId from the ontology quad + triggers reconcile in the sweep. Verified: query 278, agent 1621, publisher 1179 — all green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…sistency + coverage) 🔴 #1078 supersede atomicity — the commitment-superseding private write ran pre-chain, so a failed/rejected re-publish could delete the still-current KA's private slice while the chain still pointed at the old version. Defer the private-store write (and its supersede) to the terminal branches (post-chain-confirmation + intentional-local finalize), never on the chain-failure path — mirroring how the public data insert already defers. 🔴 #11 legacy migration — re-add the legacy-plaintext → encrypted-keystore migration on load (after successful validation) so UPGRADED nodes don't keep operational keys in plaintext indefinitely. Gated by a test-only opt-out `DKG_WALLETS_NO_MIGRATE=1` (set by the devnet staking harness, which reads the raw privateKey field directly and cannot decrypt). Production daemons migrate. 🟡 #936 cross-path token-row consistency — extract the per-root `<ual>/<tokenId>` dkg:tokenId/dkg:entity rows into a SHARED `buildDeterministicTokenRows` helper called by BOTH the publisher (originator) and the gossip/chain-reconcile path, so a multi-root KC exposes an identical queryable token map regardless of which node materialised it. (Kept out of generateKCMetadata, which metadata.test.ts pins.) 🟡 #1121 test coverage — assert the mapper PRESERVES a real agent-resolved encryption callback (not just that some callback exists) and that a PUBLIC async publish stays plaintext. Verified: publisher 1179, agent (936 repro + finalization/reconcile) green, op-wallets migration + at-rest green, async-lift 4 green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…less publishes The previous `useEncryptedInline = canAttemptOnChainPublish && …` gate (added to unbreak chainless ownerOnly publishes from the #1121 fail-closed default) was too broad: it also stopped a REAL injected encryption callback from running on a chainless publish, breaking publisher-runner-lu11 (which wires a runtime chunked callback over a NoChainAdapter and asserts it is invoked). Mark the mapper's fail-closed default and skip ONLY that default on chainless publishes; always honour a real agent-resolved callback, and still fire the default (fail-closed) for an actual on-chain publish with no real encryption. Fixes the kafka-plugin E2E AND publisher-runner-lu11. Verified: publisher 1181, full agent 1622, full cli 2029 — all green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🔴 #1078 cross-root delete — the supersede deleted by raw `deleteBySubjectPrefix( rootEntity)`, but RDF root IRIs are not prefix-delimited, so superseding `urn:device:1` would ALSO wipe `urn:device:10`'s private slice (STRSTARTS). Add `deleteRootPrivateSlice`: delete the EXACT root subject + its skolem children (`<root>/.well-known/genid/…`), the same shape getPrivateTriples reads. Used by both the supersede path and deletePrivateTriples. + regression test (sibling root survives a prefix-sharing supersede). 🟡 #1098 dedup — extract `selfPrimeSubscriptionOnChainId` so the periodic sweep and the live KACG nudge share ONE bind/persist/cursor-reset path (optional target-id match for the nudge), instead of two hand-rolled flows. 🟡/🔵 #936 dedup — extract the shared `compareRootIris` canonical comparator so the publisher and finalization handler assign tokenIds over the SAME ordering by API, not by duplicated inline comparators + comments. Verified: storage 218 (+collision test), agent 1622, publisher 1181 — all green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ph false-green) otReviewAgent: the two-replica equality assertion alone could false-green if oxigraph ever returned the unordered bindings pre-sorted. Additionally assert the exact lexicographic map (aaa→1, mmm→2, zzz→3), so a regression to positional assignment fails even under a stable store iteration order. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
otReviewAgent findings — all addressed & resolvedAll review findings across the automated review rounds have been resolved (5 🔴 Bugs fixed in code; 🟡 Issues / 🔵 Nits fixed or justified inline): 🔴 Bugs (fixed):
🟡/🔵 (fixed): nudge locality + shared self-prime helper, op-wallets adminWallet+reload+migration tests, query per-cgId VM test, swm-host self-prime test, #936 shared token-row helper + comparator, async-lift real-callback/public tests, #936 exact-canonical-map assertion. 🟡/🔵 (justified inline + resolved): keystore-module location (agent↔cli dependency direction), All affected package suites green (query / storage / publisher / agent / cli) and the 5 P0 fixes are verified on a live 6-node devnet. |
…ismatch doesn't) otReviewAgent: the self-prime test only drove the periodic sweep. Add a focused case for the live onKARegisteredToContextGraph nudge's targeting via the shared selfPrimeSubscriptionOnChainId(targetOnChainId) helper — an unrelated subscribed CG (different on-chain id) is NOT bound for the event, the matching CG is. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
KoOvA (🔴): mapLiftRequestToPublishOptions forced BOTH inline-encryption callbacks (encryptInlinePayload + encryptInlineChunked) to undefined for accessPolicy === 'public'. Previously the public branch forwarded resolved.encryptInlinePayload, so a resolver/default factory could push a public CG onto DKGPublisher's encrypted-inline path — silently encrypting public content at rest and breaking public ACK verification. KoOvD (🟡): added a CLI runtime-boundary regression (publisher-runner-private-encryption.test.ts) that drives a real ownerOnly async-lift publish through createPublisherRuntimeFromAgent and asserts publisher.publish receives the publishEncryptionFactory callback by reference — not the fail-closed mapper default. Also added prepareAsyncPublishPayload-seam precedence tests + exported isFailClosedInlineEncrypt / prepareAsyncPublishPayload from the package index. KoOvH (🟡): extracted the live onKARegisteredToContextGraph nudge body into DKGAgent.handleKARegisteredNudge so the event-handler branch is directly testable, and added tests exercising it: with multiple subscribed-but-unbound CGs and one matching ontology OnChainId only the match binds + reconciles; an already-bound CG reconciles directly. Verified: publisher 1185 passed, agent 1625 passed, CLI runner tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
3 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
Fixes 5 P0 pre-mainnet issues flagged in the issue-liveness triage (#1129). Each is a security, privacy, data-integrity, or core-correctness defect that blocks a mainnet release. Branched off fresh
main(which already carries #1107 + #1132).wallets.jsonno longer stores plaintext keysWhat changed
packages/agent/src/op-wallets.ts) — AES-256-GCM at rest under a machine-local 32-bytewallets.key(zero operator interaction; optionalDKG_WALLETS_PASSPHRASEmixes in via scrypt). Addresses stay plaintext; keys are decrypted in memory so the chain config is unchanged. Legacy plaintext files load as-is (no rewrite) so existing provisioning/tooling keeps working; new nodes generate encrypted.packages/storage/src/private-store.ts,dkg-publisher.ts,async-lift-publisher-impl.ts) — optionalcommitmentIdonstorePrivateTriples; a differing commitment supersedes the root's stale private slice (tracked by a marker on a non-hydrated subject). No-arg behaviour is unchanged (append+dedup), so existing callers/tests are untouched. Production writers pass the per-rootprivateMerkleRoot.packages/publisher/src/async-lift-publish-options.ts,packages/cli/src/publisher-runner.ts) — thread the agent-resolved AEAD callbacks through the lift slice; non-public CGs get a fail-closed default that throws if invoked without a real factory. Runner precedence fixed so the real chainKey-bound closure wins over the default.packages/agent/src/finalization-handler.ts) — sort roots lexicographically before the tokenId mint so the map is a pure function of the root set; emit explicit<ual>/<n>dkg:tokenId/dkg:entityrows (kept in the handler to avoid breakingmetadata.test.ts).packages/agent/src/dkg-agent-swm-host.ts,dkg-agent-lifecycle.ts,packages/query/src/dkg-query-engine.ts) — two layers: (1) self-primesub.onChainIdfor pre-subscribed public CGs in the reconcile sweep + live KA-registration nudge, so the chain-driven reconcile runs; (2) theverifiable-memoryview resolves the per-cgId data graph<cg>/context/<onChainId>so chain-reconciled data (per-cgId only, no root-label copy) is visible.Verification
Reproducing tests for each issue are included as regression guards and pass. All affected package suites are green with no regressions:
root→tokenIdmap; node boots clean with encrypted wallets.🤖 Generated with Claude Code