test: add Rust Sweettest substrate bridge coverage - #61
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
floss | c925ed1 | Commit Preview URL Branch Preview URL |
Sep 01 2026, 05:17 AM |
kalisam
left a comment
kalisam
left a comment
There was a problem hiding this comment.
Hermes adversarial review — verdict: REQUEST_CHANGES (filed as comment; author-account cannot request changes on own PR)
Independent verification at head c3efa1331be26312595d8228fe6397f62b147a1a. Two complete local runs of nix develop path:. --command ./tests/sweettest/run.sh (WSL, this machine): consent 2/2 (499s / 521s), substrate 7/7 (1300s / 1311s). The harness works and repeats. The blockers are process/labeling, not test code.
Blocking
-
The gate this PR claims is not enforced anywhere. No workflow references
tests/sweettest/run.sh; Rust CITest (all features)is workflow_dispatch-held, and even un-holding it would not run this child workspace (ARF/Cargo.tomlmembers are only the four zomes). Until a CI job exists (manual/nightly is fine initially), the PR must be labeled a harness, not a closed Phase-0/1 gate.docs/specs/phase0-substrate-bridge.spec.mdDoD boxes are still[ ], ADR-2 still Proposed, no HARVEST_LOG entry. -
ARF/.cargo/config.tomlis a production guest-build contract change mislabeled "test tooling only".[target.wasm32-unknown-unknown]rustflags apply to every shipped zome build. The change itself is correct — upstream holochaincrates/test_utils/wasm/build.rs:86uses exactly-C opt-level=z --cfg getrandom_backend="custom", and HDK defines__getrandom_v03_custom; our two green conductor runs prove the new backend instantiates. But the PR body's "Production zome logic and manifests are unchanged / blast radius: test tooling only" is false as written. Fix: label honestly in the body + one sentence in the config comment citing the upstream contract. -
Stale-DNA green path.
setup_two_agent_app()only assertsrose_forest.dnaexists (src/lib.rs:18-27);cargo test --manifest-path tests/sweettest/Cargo.tomlwithoutrun.shhappily loads a gitignored week-old bundle. Design line 84 says "a committed historical .dna file alone is not acceptable evidence" — encode it: hash the four release WASMs (or pack in-process) and assert in setup. -
No timeout bound. Suite is ~30 min serial;
await_consistencyis unbounded. A regression that stalls convergence hangs forever instead of failing. Wrap cross-agent waits intokio::time::timeoutand add per-test timeouts before this becomes a required check.
Non-blocking hardening
substrate_bridge_test.rs:73— signature assertion is length-only (Signatureis[u8; 64], cannot fail). Either host-verify against Alice's pubkey + action bytes or drop the assert and say Record presence is the proof.- Consent negative path proves "no discoverable link", not "no entry"; coordinator preflight rejects before
create_entry, soconsent_integrity's ownE_SCOPE_NOT_REQUESTEDpath is never exercised. Assert zero newConsentDecisionentries on Bob's chain. query_tripleswith both subject and predicate silently ignores predicate (coordinator/src/lib.rs:202-204) — pre-existing, untested; document or test the contract.- Plan file ships with every
- [ ]unchecked; check off as-executed or don't present as the execution record.
Verified strengths (independently reproduced)
Standalone child workspace keeps conductor deps out of the guest lock; pin b1d40b24… peels to holochain-0.6.1 commit 3bdeaccd…; distinct agents asserted; exchange_peer_info + condition-based await_consistency, zero sleeps; fork-visible matches spec §3.5; missing-hash negative is sound; repeatability gate satisfied — two consecutive complete green runs, identical counts, ±2% timing.
Fix the labeling, freshness assertion, timeouts, and add the CI job — then this is a genuinely good replacement for the dead Tryorama path and I'd approve.
- .cargo/config.toml: add comment citing the upstream build.rs:86 guest-build contract (-C opt-level=z --cfg getrandom_backend="custom") so the config is recognized as mirroring an upstream contract, not an arbitrary local override. - Plan file: add 'plan record, boxes not maintained' header so the - [ ] task boxes are not mistaken for incomplete work.
setup_two_agent_app now checks that the four expected release WASMs (rose_forest_integrity, rose_forest, consent_integrity, consent) exist at target/wasm32-unknown-unknown/release/ and have nonzero size before loading the DNA bundle. Without this, a stale gitignored .dna file silently passes tests against outdated zome code. Failure messages name run.sh as the required entry point, so bare 'cargo test' without run.sh fails with a clear diagnostic instead of testing against stale artifacts.
await_two_agent_consistency now wraps await_consistency in tokio::time::timeout(Duration::from_secs(300), ...). If DHT consistency is not reached within 300 seconds, the test panics with a message naming run.sh as the required entry point. This prevents indefinite hangs when tests are run without run.sh or when conductor networking silently fails. The 300s bound is generous — consistency waits are well under this in normal runs. Adds the 'time' feature to the tokio dependency in Cargo.toml.
61-D: - add a non-required Sweettest job for workflow_dispatch and a weekly Monday cron, with a 60-minute timeout - install Nix through cachix/install-nix-action pinned to commit 13d8dd58da0234aa297dedd986986ccb8e7f3e24 (v31 tag target) - run ARF/tests/sweettest/run.sh inside nix develop so every CI run builds the four release WASMs and freshly packs the DNA - extend the existing required fmt job to check the standalone Sweettest child workspace, which the parent workspace cannot see Verified locally in Holonix: parent and child cargo fmt --check exit 0. Workflow dispatch against this content requires the commit to be pushed; no push was performed.
The fresh-DNA assertion introduced in 61-B used ../../../target from the child manifest directory, which resolves to the worktree-root target. run.sh builds the zomes under ARF/target, so correctly invoked tests failed with a misleading missing-WASM error. Use ../../target instead. RED verification removed rose_forest_integrity.wasm and observed exit 101 with the required run.sh diagnostic; the artifact was then restored. The corrected path supported the subsequent 2/2 consent and 7/7 substrate runtime runs.
61-E: - replace the tautological Signature length check with SignedActionHashedExt::verify_signature against the signed action - assert a rejected consent decision adds no authored app-entry action, not merely no payload link - document that query_triples gives subject precedence when both filters are supplied, and regression-test that contract with a mismatched predicate Verified serially under Holonix: - consent_zome_test: 2 passed, 0 failed (251.38s) - substrate_bridge_test: 7 passed, 0 failed (816.11s) - cargo check --tests --locked: exit 0 - parent and child cargo fmt --check: exit 0 Integrity-path bypass coverage remains intentionally separate as #62.
|
Warning Review limit reachedNext included review available in 59 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Hermes Review — Post-Fix RoundVerdict: Comment (all fixes landed; aggregate run.sh has a known Cargo linking artifact on this machine) Fixed this round
Verification
Known issueAggregate Follow-up
Hermes Agent — FLOSSI0ULLK |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c925ed1124
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| assert!( | ||
| metadata.len() > 0, |
There was a problem hiding this comment.
When a developer rebuilds the release WASMs after changing a zome but then runs cargo test --manifest-path ARF/tests/sweettest/Cargo.toml without repacking, the old gitignored rose_forest.dna still passes this check because each separate WASM merely exists and is nonempty. SweetDnaFile::from_bundle will therefore load stale zome code and the suite can report false-positive evidence; compare the bundle's freshness/content against the WASMs or perform packing as part of the test setup.
Useful? React with 👍 / 👎.
… chain
Runs the Lane A first experiment from the current engineering continuation
packet against candidate claim C1 in CCP-EVIDENCE-WORKFLOW-NOW §11. Evidence
record only: no code changed, nothing promoted to canon.
The claim as written cannot be evaluated, because "the intended substrate" names
two different things in this repository and the two readings answer oppositely.
That is a fourth outcome the CCP's support/defeat/inconclusive trichotomy does
not have: a claim that is ambiguous because a term inside it is overloaded.
What moved since ADR-12 and ADR-19 were written: the figure "0 of 105 packets
carry a consent_ref" is now 1 of 264. The one packet, from 2026-09-01 and
authored by hermes plus an operator directive, carries two 64-hex values. A
Holochain ActionHash is Blake2b-32 rendered base64url behind a uhCkk multihash
prefix -- verified against the vendored upstream source. Both values instead
resolve to entries in the FILE-BASED chain at ~/.floss_agent/cells/000...000,
typed consent_decision and consent_payload, well-formed and operator-signed.
The structure is faithful; the substrate is not the one the spec names.
That conflation is licensed by a false claim in our own code. cell.py's
append_entry docstring says its SHA256 "is the same hash that Holochain uses as
the action address, ensuring zero rework at migration time." It is not: different
algorithm, encoding, length and type tagging. It is the parent packet's own
boundary -- "Similarity is not identity" -- violated in a comment, and it is the
standing justification for treating a file-chain hash as an anchor.
Nothing constrains the field at any layer. The spec names a substrate in prose,
the schema says {"type":"string","minLength":1}, entry_has_consent checks
non-emptiness. No code writes a consent_ref at all; the one that exists was
hand-assembled.
The Holochain path is correctly tested and has never run anywhere verifiable.
The sweettest job is gated to workflow_dispatch or a Monday cron, so every PR run
on its branch skipped it; the build fails on this host at openssl-sys; and the
961 MB local test binary is an ELF executable built under Linux somewhere this
probe cannot identify. Everything else it needs is present -- four release WASMs,
a packed .dna, the wasm32 target.
A methodological note is recorded because it nearly produced a false pass: the
first run reported exit 0 while the build had failed, because cargo was piped to
tail without pipefail -- the same trap python-ci.yml documents.
Recommends, cheapest first: dispatch the sweettest job (one click, converts the
unknown either way, and is the adversarial review PR #61 has never had); correct
cell.py's docstring; then decide what decision_action_hash anchors to, which is
now a real fork with a live artifact on each side rather than an abstraction.
On the question this probe nominally tested: Stage 0 held. The existing
per-iteration engineering contract carried the whole investigation without
strain, so nothing here demonstrates the promotion condition for a formal
EvidenceContract.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
What and why
Add a Rust-native Sweettest harness for the four active Rose Forest zomes. It ports the six substrate-bridge criteria, adds a missing-hash negative, and keeps consent behavior in a separate test binary so the two domains fail independently.
The harness builds the four WASMs from source, freshly packs the DNA, and runs two local conductors on the pinned Holochain 0.6.1 line. Production zome logic and manifests are unchanged.
This is a harness, not a closed Phase-1 gate. The docs/specs/phase0-substrate-bridge.spec.md Definition-of-Done remains open, ADR-2 is still Proposed, and there is no HARVEST_LOG entry. This PR proves the harness works; it does not close the orchestration substrate bridge validation gate.
Scope
Blast radius
ARF/.cargo/config.tomlsets[target.wasm32-unknown-unknown] rustflags = ['-C', 'opt-level=z', '--cfg', 'getrandom_backend="custom"']— this is a guest-build contract change for ALL wasm zomes, not just the test workspace. It mirrors the upstream Holochain 0.6.1 guest-build contract fromcrates/test_utils/wasm/build.rs:86(-C opt-level=z --cfg getrandom_backend="custom"; the HDK defines__getrandom_v03_custom). Without it, wasm builds fail witherror[E0425]: cannot find function fill_inner in module backends. This is the correct and required setting per upstream; it is flagged here so reviewers understand it affects every zome build, not just the test child workspace.Truth status
Prior art and reuse (ADR-18)
Tests
Result:
Fresh run completed on Holochain 0.6.1 at c3efa13.
Docs and registries
Risk
.cargo/config.tomlchange is a guest-build contract mirroring upstream, not a local invention.