Skip to content

fix(tests): production fixes + new tests to make CI green#325

Closed
Bojan131 wants to merge 1 commit intomainfrom
fix_tests
Closed

fix(tests): production fixes + new tests to make CI green#325
Bojan131 wants to merge 1 commit intomainfrom
fix_tests

Conversation

@Bojan131
Copy link
Copy Markdown
Contributor

Summary

Squashed, focused replacement for PR #229 (fix/caught_bugs). The original branch had 110 commits of bot-review iteration noise; this PR collapses the entire net diff into a single commit so reviewers can see what was actually fixed instead of the tit-for-tat history.

End state is byte-for-byte identical to fix/caught_bugs — every production fix and new test that made CI green is preserved.

Scope: 168 files, +37,643 / −2,434 lines, 26 new test files, 16 packages touched (adapter-elizaos, adapter-openclaw, agent, attested-assets, chain, cli, core, epcis, evm-module, mcp-dkg, mcp-server, network-sim, node-ui, publisher, query, storage).

What's done

Production fixes (real bugs that the new tests catch)

Agent — A-5, A-7, A-13, A-15

  • finalization race fixes; mock-fixture drift aligned with spec
  • per-CG quorum: account for self-sign in signature count
  • RPC error propagation through chain adapters
  • requiredSignatures resolution: structural parsedId discriminator

Publisher — P-1, P-2, P-13

  • PreBroadcastJournalEntry.publicByteSize serialised as string (bigint safety)
  • WAL recovery uses a valid OperationName
  • async-lift subtraction respects key bound
  • envelope ↔ publisher attribution extended to KAUpdate + Finalization paths

Storage / Blazegraph — ST-2, ST-7, K-1, K-2, K-4/5, K-9, K-11

  • deleteByPattern split by graph filter; single-graph path materialised via SELECT then DELETE DATA
  • no-graph path enumerates graphs explicitly
  • default-graph triple re-check before commit
  • deterministic AES-GCM-SIV at-rest privacy
  • private-literal sealing
  • graph-manager + private-store coverage lifts

EVM module — E-7, E-9, E-11, E-17, E-20

  • dual-emit KnowledgeBatchCreated on V10 publish (V8/V9 indexer back-compat)
  • restore Hub UnauthorizedAccess ABI
  • MigratorV10Staking audit hardening (cascade-rejection, ownable, access-control)

CLI — CLI-1, CLI-7, CLI-9, CLI-10, CLI-11, CLI-16

  • scrypt KDF parameter floor (N≥2¹⁵, r≥8, p≥1, salt ≥16 bytes, dklen=32)
  • signed-request auth: nonce store, freshness window, HMAC bound to path+search
  • token rotation/revocation with hot-reload from disk
  • SPARQL endpoint 4xx mapping (no more 500s on bad upstream)
  • /api/verify and /api/ccl error-code mapping
  • context-graph IDs reject path traversal (../, ./../, etc.)
  • new Blazegraph compose service + quads-mode namespace (NoAxioms) for storage conformance

Chain / chain-event-poller

  • classify upstream RPC blips (5xx, ECONNRESET, hardhat head-race) as transient WARN with escalation policy instead of fatal ERROR
  • 81 new behavioural tests for MockChainAdapter

Adapter ElizaOS

  • DKGService chat-turn persistence overload narrowing (compile-time enforcement of typed contract)
  • DKG_NODE_URL fail-fast on missing/invalid origin
  • headless assistant-reply emits full envelope
  • assistant-text supersede flag + per-runtime cache
  • HMAC framing on signed gossip; signing failures surface as ERROR (don't throw)

Adapter OpenClaw

  • mcp_auth bearer validation; channel/node plugin error surfaces

OriginTrail game / coordinator — G-2, G-3, G-4

  • leader's CCL install state broadcast in expedition:launched
  • force-resolve no-op when no votes/proposal pending
  • tombstone-aware join + auth on locations endpoint
  • success-log substring contract ('published to context graph')

Autoupdater (the CI-green fix)

  • always run pnpm --filter dkg-evm-module clean && build when contract sources change, regardless of whether the runtime build went through pnpm build:runtime or the legacy pnpm build fallback
  • prevents stale ABI/typechain output for deleted/renamed contracts from surviving into the inactive slot and being swapped live
  • contract-diff retries via git fetch --depth=1 <fetchUrl> <currentCommit> on missing parent commit; honours autoUpdate.buildTimeoutMs.contracts

New tests + coverage lifts

Package Before After Highlights
chain 50 % 75 % +81 MockChainAdapter behavioural tests
storage 79 % 86 % graph-manager 61→95 %, private-store 75→100 %
agent 74.6 % 76.1 % op-wallets + workspace-config → 100 %
adapter-elizaos, cli rotateToken invalidation regression test
publisher per-CG quorum state, WAL recovery, async-lift bound, chain-event-poller transient classifier, update-handler
query SPARQL form-detection
cli path-traversal, scrypt-floor, signed-auth, autoupdater hardening

26 brand-new test files in total; 92 *.test.ts files touched or added.

CI infrastructure

  • new Bura: cli + Bura: query + attested-assets jobs
  • Blazegraph quads-mode namespace creation in ci.yml
  • Codex Review timeout bumped 15 → 30 minutes (was being cancelled mid-stream on long runs)

Test plan

  • All 14 packages build cleanly (tsc exit 0).
  • Local vitest test suites pass for every changed package (modulo three pre-existing local-only failures: storage ST-1 needs BLAZEGRAPH_URL, cli catchup-runner.test.ts is a Node ESM-loader quirk that passes on CI, chain is a pnpm shim issue that passes on CI).
  • CI on the source branch (fix/caught_bugs, PR Fix the caught bugs by the tests #229) is 34 pass / 0 fail / 1 skipping at HEAD 7e41cda9 — the diff in this PR is identical, so CI here will produce the same result.
  • Reviewer to verify CI on this branch matches.

Why this PR exists alongside #229

PR #229 has the full audit trail (130 individual commits across 31 review rounds plus 30 follow-up commits). This PR is intended for the actual code review — same code, no narrative noise. Either can be merged; whichever you prefer, the other can be closed.

Made with Cursor

Squashes the full PR #229 net diff into a single commit. Net effect:
168 files, +37,643 / -2,434 lines.

Production fixes (real bugs caught by the new tests):

  • Agent: A-5, A-7, A-13, A-15 — finalization races, mock-fixture
    drift, per-CG quorum self-sign accounting, RPC error propagation,
    requiredSignatures parsedId discriminator.
  • Publisher: P-1, P-2, P-13 — PreBroadcastJournalEntry bigint
    serialization, WAL recovery OperationName, async-lift subtraction
    key bound, envelope↔publisher attribution on KAUpdate +
    Finalization paths.
  • Storage / Blazegraph: ST-2, ST-7, K-1, K-2, K-4/5, K-9, K-11 —
    deleteByPattern split by graph filter, single-graph materialise
    via SELECT + DELETE DATA, no-graph enumeration, default-graph
    triple re-check, deterministic AES-GCM-SIV at-rest privacy,
    private-literal sealing, graph-manager + private-store coverage.
  • EVM module: E-7, E-9, E-11, E-17, E-20 — dual-emit
    KnowledgeBatchCreated on V10 publish, restore Hub
    UnauthorizedAccess ABI, MigratorV10 audit hardening, Hub
    Ownable/AccessControl correctness.
  • CLI: CLI-1, CLI-7, CLI-9, CLI-10, CLI-11, CLI-16 — scrypt KDF
    parameter floor, signed-request auth + nonce store + freshness
    window, token rotation/revocation, SPARQL endpoint 4xx mapping,
    /api/verify error code mapping, context-graph path-traversal
    rejection, Blazegraph service for storage conformance.
  • Chain / chain-event-poller: classify upstream RPC blips
    (5xx, ECONNRESET, hardhat head-race) as transient WARN with
    escalation policy.
  • Adapter ElizaOS: chat-turn persistence overload narrowing,
    DKG_NODE_URL fail-fast, headless assistant-reply envelope
    emission, assistant-text supersede flag, per-runtime cache,
    HMAC framing on signed gossip.
  • OriginTrail game / coordinator: G-2, G-3, G-4 — leader CCL
    install state in expedition:launched, force-resolve no-op when
    no proposal pending, tombstone-aware join + auth on locations
    endpoint, success-log substring contract.
  • Autoupdater (CI green fix): always run hardhat-clean +
    contract rebuild when sources change, regardless of which
    runtime build path was used. Prevents stale ABI/typechain from
    deleted contracts surviving into the inactive slot.

Tests:

  • +81 chain MockChainAdapter behavioural tests (lines 50→75 %).
  • Storage coverage 79→86 % (graph-manager 61→95 %, private-store
    75→100 %).
  • Agent coverage 74.6→76.1 % (op-wallets + workspace-config →
    100 %).
  • Adapter-elizaos / cli rotateToken invalidation regression test.
  • Publisher: per-CG quorum state, WAL recovery, async-lift bound,
    chain-event-poller transient classifier, update-handler.
  • Query: SPARQL form-detection.
  • New CI infra: Blazegraph quads-mode service for the storage
    conformance suite (NoAxioms namespace).

Made-with: Cursor
* frame format).
* 2. Replace any absolute filesystem path containing a line:col suffix
* with `<redacted-path>` — covers the common `(/Users/.../foo.ts:12:34)`
* and `at /Users/.../foo.ts:12:34` patterns produced by Error.stack.
// `Record<string, unknown>` options bag (the catch-all overload
// was the smuggling path the bot called out). The runtime guards
// inside `persistChatTurnImpl` still validate this payload shape.
return _dkgServiceLoose.persistChatTurn(runtime, message, state, opts);
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: onAssistantReplyHandler returns here without ever calling markAssistantPersisted(...). For turns where onChatTurn did not already embed the assistant leg, a replayed/retried onAssistantReply still sees an empty cache and re-appends schema:text / schema:dateCreated / schema:author to the same msg:agent:${turnKey} subject. Capture the resolved reply text and update the assistant cache after _dkgServiceLoose.persistChatTurn(...) succeeds.

const assistantReplyOpt = optsForAssistant.assistantReply as { text?: unknown } | undefined;
const stateForAssistant = (state ?? {}) as { lastAssistantReply?: unknown };
const assistantText =
(typeof optsForAssistant.assistantText === 'string' && optsForAssistant.assistantText)
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: this cache uses a plain || chain, but persistChatTurnImpl now resolves assistant text with a trim-aware “first non-empty string” selector. With assistantText = ' ' and assistantReply.text = 'final', the canonical write stores final while the cache records whitespace, so the next onAssistantReply falsely takes the supersede/headless path. Reuse the same selector here (and for incomingReplyText above) so the cache matches what was actually persisted.

);
}
const characterName = runtime.character?.name ?? runtime.getSetting('DKG_AGENT_NAME') ?? 'elizaos-agent';
const contextGraphId = optsAny.contextGraphId
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: this resolver treats '' as an explicit context/assertion name, while resolveDestinationFromOptions() in src/index.ts falls back when the value is empty. If getSetting() returns '' for an unset env var, the write goes to an empty destination but the persisted-turn caches key under agent-context / chat-turns, so reply correlation can desync from the actual store. Normalize to non-empty strings here or reuse the same destination resolver.

@Bojan131 Bojan131 closed this Apr 29, 2026
@Bojan131 Bojan131 deleted the fix_tests branch April 29, 2026 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants