psf-memo/docs/reviews/memo-multipush-encoding-summary.md

memo-multipush-encoding — Architect Review

Task: memo-multipush-encoding Component: psf-memo-client Base: c018a01 (last merged architect review); inbound refactorer commit 8310b6a

What was reviewed

Inbound refactorer batch (priority 10), merged onto swarmforge-architect by fast-forwarding to 8310b6a. The linear chain reviewed:

• 6eddd9c — specifier: Specify multi-push encoding for Memo multi-field actions. Adds psf-memo-client/specs/memo-multipush-encoding.feature (5 scenario outlines) requiring reply, topic message, add-poll-option, and poll-vote to broadcast the prefix plus fields as 3 separate OP_RETURN pushes, and create-poll as 4 pushes.
• 2e2db86 — coder: Implement multi-push encoding for Memo multi-field actions. Rewrites hex.js to return a pushes array instead of one combined payload, adds the memo-multipush wallet adapter, switches the five action services to pass field arrays, adds acceptance step handlers, and adds unit/property tests.
• 8310b6a — refactorer: Refactor multi-push helpers and add adapter property tests. Shares encodeUtf8 across hex/poll-create/topic-post, drops the redundant Uint8Array branch in the push normalizer, renames the txid-action builder parameter buildPayload → buildPushes, extracts the DB repairedFixture, and adds property tests for the adapter.

Architect review commit: fac01730c4 — the browser-Buffer fix and the regression test, the encodeScript test-helper extraction, the acceptance assertion de-duplication, the buffer dependency, and the mutate4javascript and soft gherkin-mutator manifests. The summary and verification record are committed on top, so git diff fac01730c4 HEAD touches only docs/. The record's git_sha is fac01730c4, the commit that contains the verified source state.

Architectural findings and fixes applied

The refactorer's structure is sound: hex.js and utf8.js are pure leaves, the five action services are core modules with injected wallet/store adapters, and memo-multipush.js confines the wallet wiring to a small adapter boundary. Two real issues were found and fixed.

1. Browser runtime defect — the adapter read the Node global Buffer. This is the important finding. memo-multipush.js called Buffer.from(...) directly, but the CRA 5 production bundle does not polyfill Node globals and the externally loaded wallet script does not define window.Buffer (verified: the built bundle had no Buffer assignment and the only other Buffer.from came from axios behind a typeof Blob guard). Every real browser reply, topic message, poll option, poll vote, and create-poll would have thrown Buffer is not defined at runtime, while the Node test suite passed. @psf/bitcoincashjs-lib's compile2 requires genuine Buffer instances (Buffer.isBuffer + .copy), so a Uint8Array substitution is not possible. Fix: import the browser-safe implementation (const { Buffer } = require('buffer')) — the same module the bundled bitcoin library uses — and declare buffer as a direct dependency so the adapter does not depend on a dev-transitive package. Confirmed in the built bundle: the adapter now compiles to const{Buffer:r}=n(6382) instead of a free global. A regression test re-requires the module with global.Buffer removed and asserts pushes still build.
2. Mutation survivors in attachMultiPushOpReturn. The first run killed 2 of 4 sites. The !wallet || wallet.__multiPushAttached logical mutation survived because nothing tested a falsy wallet, and __multiPushAttached = true survived because the idempotence test only counted wallet calls (which a harmless double-wrap does not change). Focused tests now assert a falsy wallet is a no-op and that a second attach leaves the wrapper function identity unchanged; the file re-runs 4/4.
3. DRY — duplicated test/acceptance helpers. The encodeScript Bitcoin script double was copied verbatim between the new unit and property tests; extracted to test/support/script-encoding.js (a test helper, outside test/unit/** and test/property/*.test.js). The three new acceptance handlers that assert a UTF-8 push (topic/text/question) were structurally identical; extracted assertUtf8Push. Scoped DRY dropped the duplicate blocks in the touched set from 132 to 129 and removed the new-handler duplicates.
4. UI/Core separation, dependency rule, information hiding. memo-multipush depends on nothing; hex/utf8 are pure; the action services depend inward on those leaves; no React/DOM/router/LevelDB/wallet-global leaks. The wire order still has one definition (txidToWireBytes) and the txid+text push shape one definition (buildTxidTextPushes). The adapter hides the bchjs.Script.encode2 swap behind attachMultiPushOpReturn, restoring it synchronously inside the same tick (the property tests pin the expansion and delegation contract).

No further boundary change was required.

Verification results

Language mutation (mutate4javascript, --mutate-all when the wrapper

detected differential under-selection, --max-workers 8)

File	Sites	Killed	Survived	Uncovered
src/services/memo-multipush.js	4	4	0	0
src/services/hex.js	5	5	0	0
src/services/utf8.js	0	0	0	0
src/services/memo-reply.js	2	2	0	0
src/services/memo-topic-post.js	7	7	0	0
src/services/memo-poll-create.js	7	7	0	0
src/services/memo-poll-option.js	0	0	0	0
src/services/memo-poll-vote.js	0	0	0	0
src/services/memo-txid-action.js	4	4	0	0

utf8.js, memo-poll-option.js, and memo-poll-vote.js were confirmed with --scan as structural zeros (Total mutation sites: 0): they contain no arithmetic/comparison/boolean/logical/0<->1 sites, only string/array work and base-class delegation. async-load.js is the browser/wallet adapter shell and is excluded from tools that run the test suite, consistent with the standing precedent for adapter shells.

DRY (dry4javascript, scoped to the touched production files, tests, and

adapters)

129 duplicate blocks remain in the scoped set, all pre-existing pattern boilerplate: acceptance/lib/handlers.js step-handler repetition (192 of the line references), the established per-suite makeWallet test double (also present across ~28 client test files), and parallel poll/topic/reply unit-test bodies that exercise the parallel MemoTxidAction subclasses on purpose. The only two task-local duplications (encodeScript, the three UTF-8 push assertions) were extracted. The production-service duplicates (memo-poll-option/memo-poll-vote config blocks, the two 2-field constructors) are intentional parallel-config declarations and were left as-is to preserve their per-action clarity.

CRAP / cyclomatic complexity (crap4javascript)

All changed functions at or below CRAP 5.0 with ~100% coverage: hexToBytes (CC 5, 100%, 5.0), MemoPollCreate.create (CC 4, 4.0), attachMultiPushOpReturn/broadcastMultiPush (CC 3, 3.0), and the rest CC ≤ 3. MemoReply.reply is 90% covered but CRAP 2.0. Well below the 8.0 threshold.

Soft Gherkin acceptance mutation (gherkin-mutator --level soft)

memo-multipush-encoding.feature: 20 executed, 8 killed, 12 survived, 0 errors. Every survivor is a single-character case mutation of a text, topic, or question example value. Those values are used consistently on the setup and assertion sides of their scenario (...with the text "<text>" / ...is the UTF-8 text "<text>"), so the mutant is an intrinsic equivalent: the push bytes match whatever the example says. The point of the feature — the separate-push shape (count), the little-endian wire txid, the poll type, and the option count — is killed (all 8 non-text mutations, including the option_count and txid mutations). No implementation change is warranted. The tool left its manifest (empty scenarios, because every scenario has equivalents) in the feature file; it is committed as tool-written.

Suite status

Canonical record against review commit fac01730c4beaf5b412302f919e47871778f4d08:

• swarmforge/scripts/verify.sh client --record docs/reviews/memo-multipush-encoding-verification.json --task memo-multipush-encoding -> pass (5/5): unit 425 pass / 0 fail, property 85 pass / 0 fail, acceptance all 31 suites passed, lint ok, build ok.

Handoffs sent

• End-of-chain git_handoff to the specifier (task memo-multipush-encoding) with the review commit fac01730c4 so it can merge swarmforge-architect into master.
• No coder/refactorer handoff: the review is a runtime fix, mutation hardening, and local DRY with no follow-up work for those roles.

By architect.