refactor(core): run tiny domains as TinyBus modules - #5525
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis change introduces local memory, document-format, and wallet primitive contracts. It replaces legacy embedded memory and bundled wallet/document integrations with TinyMemory module bindings and crate-local APIs. ChangesMemory API and provider contracts
Document format contracts
Wallet primitive contracts
Estimated code review effort: 5 (Critical) | ~120 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
be83f39 to
f62f3e1
Compare
senamakel
changed the title
f62f3e1 to
fb6da79
Compare
Co-authored-by: Medulla <medulla@tinyhumans.ai>
fb6da79 to
3ee5a3c
Compare
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (18)
src/openhuman/memory/api/goals_tests.rs (1)
9-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for
next_idand the parse → render round trip.The current tests cover the header and line filtering only.
GoalsDoc::next_idcontains the sole non-trivial branch in the module: it starts atlen + 1and skips ids already taken. A gap in the id sequence, for exampleg1andg3, exercises that skip path. The parse → render round trip is the documented contract of the file and is also untested.🧪 Proposed additional tests
#[test] fn next_id_skips_ids_already_taken() { let doc = GoalsDoc::parse("- [g1] one\n- [g3] three\n"); // len == 2, so the first candidate `g3` is taken and `g4` is returned. assert_eq!(doc.next_id(), "g4"); assert!(doc.contains_id("g3")); assert!(!doc.contains_id("g4")); } #[test] fn parse_render_round_trips_recognised_items() { let doc = GoalsDoc::parse("- [g1] first goal\n- [g2] second goal\n"); assert_eq!(GoalsDoc::parse(&doc.render()), doc); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/memory/api/goals_tests.rs` around lines 9 - 22, Add tests covering GoalsDoc::next_id skipping an occupied candidate when IDs have a gap, verifying g4 is returned for g1 and g3, and add a parse-to-render round-trip test asserting recognized items remain equal after rendering and reparsing.src/openhuman/memory/api/mod.rs (1)
1-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate the stale crate framing in the module docs.
These docs describe the contract as a separate crate and state that "depending on the contract never drags in SQLite, git2, reqwest, regex, or an async runtime". The contract now lives inside this binary crate as
openhuman::memory::api, so nothing enforces that dependency boundary. A reader who trusts the claim may add a heavy dependency here and assume the isolation still holds.Lines 30-36 have the same problem: they describe
tinycortex-apiand engine alias paths as current facts.Restate both passages as intent rather than as an enforced property. State that the module is kept dependency-light by convention, and mark the TinyCortex paragraph as history.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/memory/api/mod.rs` around lines 1 - 12, Update the module-level documentation in the memory API module to describe dependency-light design as a convention rather than an enforced crate boundary, removing claims that this module is a separate crate or prevents specific dependencies from being included. Revise the passages referencing tinycortex-api and engine alias paths to present them as historical context, and identify TinyCortex as the default embedded engine without implying those paths remain current.src/openhuman/modules/memory.rs (1)
200-219: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate the
verifydoc:assumedis now every family, so "advertises less" is the likely case.Line 214 changed
assumedfrom the mandatory set toCapabilities::all(). The doc above still reasons about "the mandatory three" and treats "advertises less" as the rare problem. With the new baseline, any module that does not implement all thirteen families produces the warning, and the response is still onlylog::warn!.State the consequence in the doc.
binding.rscachescapabilities()at bind time (lines 77-81 there) and the host filters its RPC and tool surface from that cached set. If the artifact serves fewer families, the host still registers the missing surface, and each call fails at the bus instead of being filtered out.♻️ Proposed doc update
/// Cross-check the module's advertised capabilities against what this build /// assumes, once per process. /// - /// Logged rather than fatal. A module that advertises *more* than the - /// mandatory three is not dangerous — the kernel simply will not use the - /// extra families, because it filtered its surface from the static set — but - /// it does mean the registry pin and the artifact have diverged, which is - /// worth seeing. A module advertising *less* is the real problem, and says so. + /// Logged rather than fatal, because `capabilities()` must answer + /// synchronously at bind time and cannot await the module. + /// + /// This build assumes every family. A module that advertises *less* means + /// the host has already registered RPC and tool surface it cannot serve, so + /// those calls fail per-call at the bus rather than being filtered out. A + /// module that advertises *more* is inert: the host never calls the extra + /// families. Either way the registry pin and the artifact have diverged. async fn verify(&self, proxy: &tinybus::Proxy) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/modules/memory.rs` around lines 200 - 219, Update the documentation above verify to describe Capabilities::all() as the assumed baseline rather than the mandatory three, and state that artifacts advertising fewer families still leave the host’s cached RPC/tool surface registered, causing calls for missing families to fail at the bus instead of being filtered out. Keep the existing warning-only behavior and note that advertising additional families remains non-fatal.src/openhuman/modules/memory_host.rs (2)
101-107: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReplace the JSON round-trip with a typed conversion.
Lines 105-106 serialize the host response to a
serde_json::Valueand immediately deserialize it intoSpacyResponse. Two consequences follow. The conversion allocates a fullValuetree on every extraction call, which runs per NLP extraction during ingestion. The conversion also depends on the two types agreeing on field names at runtime, so a rename in the host type surfaces as a failed extraction rather than a compile error.Implement
From<PythonSpacyResponse> for SpacyResponseinmemory/api/host/nlp.rsand call it here.♻️ Proposed change
async fn extract_spacy(&self, text: String) -> tinybus::Result<SpacyResponse> { let response = crate::openhuman::runtime::python_server::extract_spacy(&self.0, &text) .await .map_err(method_error)?; - serde_json::from_value(serde_json::to_value(response).map_err(method_error)?) - .map_err(method_error) + Ok(SpacyResponse::from(response)) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/modules/memory_host.rs` around lines 101 - 107, Implement a typed From<PythonSpacyResponse> for SpacyResponse conversion in the host NLP API, then update extract_spacy to convert the awaited Python response directly instead of serializing and deserializing through serde_json. Preserve the existing method_error handling for the runtime call.
281-286: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test that pins the
LocalModelUnavailablesuppression.This arm is the only one that diverges from the mapping: it publishes a user-facing error and returns
Noneso nothing reachesBUS. That behavior is deliberate and matches the one-error-per-operation latch inmemory/tree/health/. It is also invisible to the type system, so a later refactor could restore the publish without any signal.The rest of the mapping is safe by construction, because struct-field shorthand binds by name on both sides and a transposition would not compile. This arm has no such protection.
Add a small test module asserting
into_domain_event(MemoryEvent::LocalModelUnavailable { .. })returnsNone, and that one representative mapped variant returnsSome.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/modules/memory_host.rs` around lines 281 - 286, Add a focused test module for into_domain_event covering the LocalModelUnavailable arm: assert that this variant returns None and does not produce a domain event, and assert that one representative ordinary MemoryEvent mapping returns Some. Keep the test scoped to the existing conversion behavior and use the relevant constructors/imports already available in the module.src/openhuman/memory/api/host/storage_memory.rs (1)
56-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCorrect the documented defaults for the
agentmemory_*fields.The doc comments state that
agentmemory_urldefaults tohttp://localhost:3111and thatagentmemory_timeout_msdefaults to 5000 ms.MemoryConfig::default()sets both toNone, and neither field has a#[serde(default = "...")]provider. The fallback therefore lives in the consumer, not in this contract.This file is now the owned contract for these fields. State that
Nonemeans the consumer substitutes the documented value, so a future consumer does not treatNoneas "not configured".♻️ Proposed doc clarification
/// Base URL for the `agentmemory` REST server. Honored only when - /// `backend = "agentmemory"`. Defaults to `http://localhost:3111` - /// (the agentmemory loopback default). + /// `backend = "agentmemory"`. `None` here; the backend client + /// substitutes `http://localhost:3111` (the agentmemory loopback + /// default) when this is unset. #[serde(default)] pub agentmemory_url: Option<String>,/// Per-request timeout for the agentmemory REST client, in - /// milliseconds. Defaults to 5000 ms. + /// milliseconds. `None` here; the backend client substitutes + /// 5000 ms when this is unset. #[serde(default)] pub agentmemory_timeout_ms: Option<u64>,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/memory/api/host/storage_memory.rs` around lines 56 - 74, Update the doc comments for MemoryConfig fields agentmemory_url and agentmemory_timeout_ms to state that None is preserved in the configuration and the consumer substitutes the effective defaults of http://localhost:3111 and 5000 ms respectively. Do not imply that serde or MemoryConfig::default() populates these values; clarify that None means the consumer applies the documented fallback.src/openhuman/memory/ops/tool_memory.rs (1)
118-126: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffConsider adding provider reads for a single rule and for a batch of tools.
Two handlers now fetch more than they need:
tool_rule_getloads every rule for the tool, then filters by id in memory. The removed store exposed a directget_rule.tool_rules_for_promptissues one sequentialtool_rulescall per requested tool, where the store previously served one grouped read.The cost is bounded for a local module provider. For an external HTTP driver, the prompt path becomes N sequential round trips on the turn-construction hot path. Add
get_tool_rule(tool, id)and a multi-tool read to the tool-memory family, or fetch the per-tool lists concurrently.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/memory/ops/tool_memory.rs` around lines 118 - 126, Update the tool-memory provider interface and handlers around tool_rule_get and tool_rules_for_prompt to avoid over-fetching and sequential remote reads: add a direct get_tool_rule(tool, id) operation and a grouped multi-tool rule read, then use them in the respective handlers; if the provider API cannot support grouped reads, fetch per-tool rule lists concurrently instead.src/openhuman/web3/wallet/primitives/address/btc/test.rs (2)
206-211: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the duplicated foreign-bech32 test.
Both tests use the same
ltc1qw508d6qejxtdg4y5r3zarvary0c5xw7kgmn4n9fixture.rejects_a_bech32_address_for_another_coinonly assertsis_err().reports_a_foreign_bech32_chain_as_the_wrong_network_not_as_bad_base58asserts the exact variant and reason, so it fully subsumes the first.Also applies to: 276-288
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/web3/wallet/primitives/address/btc/test.rs` around lines 206 - 211, Remove the redundant rejects_a_bech32_address_for_another_coin test, keeping reports_a_foreign_bech32_chain_as_the_wrong_network_not_as_bad_base58 as the single test for the shared Litecoin bech32 fixture and its detailed validation outcome.
196-204: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the weaker of the two duplicated version-byte tests.
rejects_a_base58_address_with_an_unknown_version_byte(Line 196) andrejects_a_base58_address_with_an_unrecognised_version_byte(Line 236) assert the same rule. The first accepts eitherInvalidAddressorWrongNetwork, so it cannot detect a regression that swaps the two. The second constructs its fixture and asserts the exact variant and reason.Keep the constructed test and delete the tolerant one.
Also applies to: 236-245
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/web3/wallet/primitives/address/btc/test.rs` around lines 196 - 204, Remove the weaker duplicate test function rejects_a_base58_address_with_an_unknown_version_byte, including its fixture and tolerant match assertion. Keep rejects_a_base58_address_with_an_unrecognised_version_byte and its constructed fixture with the exact error-variant assertion.src/openhuman/web3/wallet/primitives/address/mod.rs (1)
74-77: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCollapse the repeated
feature = "web3"operands in thecfgpredicates. The vendored code used one feature per chain. The rewrite replaced every distinct feature name withweb3and left the repeated operands in place.any(web3, web3, web3, web3)andall(web3, web3, web3, web3)both reduce toweb3, so the behavior is correct today, but the predicates suggest a per-chain gate that does not exist.
src/openhuman/web3/wallet/primitives/address/mod.rs#L74-L77: replacenot(any(feature = "web3", feature = "web3", feature = "web3", feature = "web3"))withnot(feature = "web3"), and replace thenot(all(...))predicate on Line 88 withnot(feature = "web3").src/openhuman/web3/wallet/primitives/abi/mod.rs#L68: replaceall(feature = "web3", feature = "web3", feature = "web3")in the doc example withfeature = "web3".♻️ Proposed predicate simplification for `address/mod.rs`
-#[cfg_attr( - not(any(feature = "web3", feature = "web3", feature = "web3", feature = "web3")), - allow(unused_variables) -)] +#[cfg_attr(not(feature = "web3"), allow(unused_variables))] pub fn validate(chain: Chain, address: &str) -> Result<String> { match chain {- #[cfg(not(all(feature = "web3", feature = "web3", feature = "web3", feature = "web3")))] + #[cfg(not(feature = "web3"))] other => Err(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/web3/wallet/primitives/address/mod.rs` around lines 74 - 77, Collapse the redundant web3 feature operands in the cfg predicates: in address/mod.rs lines 74-77 and the not(all(...)) predicate around line 88, replace each repeated-feature expression with not(feature = "web3"); in abi/mod.rs line 68, simplify the doc example’s all(...) expression to feature = "web3".src/openhuman/web3/wallet/execution.rs (1)
365-365: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate the stale
dispatch=tinywalletlog tag.This PR removes the
tinywalletdependency. Dispatch now targets the in-tree wallet primitives. The log value now names a component that no longer participates.♻️ Proposed log tag update
- debug!("{LOG_PREFIX} validate_address chain={chain:?} role=recipient dispatch=tinywallet"); + debug!("{LOG_PREFIX} validate_address chain={chain:?} role=recipient dispatch=primitives");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/web3/wallet/execution.rs` at line 365, Update the debug log in validate_address so the dispatch tag no longer references tinywallet and instead identifies the in-tree wallet primitives used by the current implementation.src/openhuman/web3/wallet/primitives/address/solana.rs (1)
61-84: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider a length bound before decoding.
decoderunsbs58::decodeover the whole trimmed input and checks the byte count afterwards. A valid Solana address is 32 bytes, which is 43 or 44 base58 characters. An early bound ontrimmed.len()rejects an oversized string without decoding it, and it produces the sameInvalidAddresserror.♻️ Proposed early bound
+/// Maximum base58 characters that can encode [`ADDRESS_BYTES`] bytes. +const MAX_ADDRESS_CHARS: usize = 44; + pub fn decode(address: &str) -> Result<[u8; ADDRESS_BYTES]> { let trimmed = address.trim(); if trimmed.is_empty() { return Err(Error::EmptyAddress { chain: Chain::Solana, }); } + if trimmed.len() > MAX_ADDRESS_CHARS { + return Err(Error::InvalidAddress { + chain: Chain::Solana, + address: trimmed.to_string(), + reason: format!( + "expected at most {MAX_ADDRESS_CHARS} base58 characters, got {}", + trimmed.len() + ), + }); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/web3/wallet/primitives/address/solana.rs` around lines 61 - 84, Update decode to reject trimmed inputs longer than the maximum 44 base58 characters before calling bs58::decode, returning the existing Solana InvalidAddress error with the input and expected-length context. Preserve empty-input handling and normal decoding for inputs within the bound.src/openhuman/web3/wallet/primitives/address/btc.rs (1)
190-199: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConfirm the intended error class for a base58 address whose pre-separator text is all lowercase letters.
parseroutes any string whose text before the last1is non-empty and all ASCII lowercase toparse_bech32. Every mainnet and testnet base58 form in the tests contains a digit or starts with1or3, so each one falls through toparse_base58. A base58 string with no digit before its last1would instead return a bech32InvalidAddresserror.This changes only the reported error class, not acceptance, because both decoders verify a checksum. State the intent in the doc comment if the fallthrough is accepted.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/web3/wallet/primitives/address/btc.rs` around lines 190 - 199, Clarify the intended error classification in the documentation for parse: when the text before the final separator is non-empty lowercase ASCII, parsing routes through parse_bech32 and may report its InvalidAddress error even for a base58 candidate. If this behavior is intentional, document it explicitly; otherwise adjust the routing while preserving checksum validation and existing acceptance behavior.src/openhuman/web3/wallet/primitives/chain/mod.rs (1)
18-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign the doc comment with the unconditional serde derives.
The doc states that serde support is conditional. The derives on Lines 24-26 are unconditional, so this enum always requires
serde. Correct the wording, or gate the derives withcfg_attrif the conditional behavior is still intended.📝 Proposed doc correction
-/// Serde support is conditional so the enum stays dependency-free in builds -/// that do not need it. The representation is the lowercase variant name +/// The serde representation is the lowercase variant name🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/web3/wallet/primitives/chain/mod.rs` around lines 18 - 27, Update the documentation for the enum with the unconditional serde::Serialize and serde::Deserialize derives to remove the claim that serde support is conditional, while preserving the existing lowercase representation and boundary behavior.src/openhuman/web3/wallet/primitives/key/evm.rs (1)
34-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
hex::encodeinstead of the localhex_lowerhelper.
hex0.4 is already available in this file's dependency set, andhex::encodeproduces the same lowercase output.key/btc.rsandaddress/tron.rsalready use the crate directly.♻️ Proposed refactor
- let body = hex_lower(&hash[12..]); + let body = hex::encode(&hash[12..]); @@ -fn hex_lower(bytes: &[u8]) -> String { - use std::fmt::Write as _; - bytes.iter().fold(String::new(), |mut out, b| { - let _ = write!(out, "{b:02x}"); - out - }) -}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/web3/wallet/primitives/key/evm.rs` around lines 34 - 40, Remove the local hex_lower helper and replace its call sites with hex::encode, preserving the existing lowercase hexadecimal output. Remove the now-unused std::fmt::Write import if applicable, following the direct crate usage already established in the surrounding key/address modules.src/openhuman/web3/wallet/primitives/address/test.rs (1)
21-34: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCollapse the repeated
web3feature checks.
all(feature = "web3", feature = "web3", feature = "web3", feature = "web3")is equivalent tofeature = "web3". The four separate arms are also equivalent to one wildcard arm. The repetition is a leftover from per-chain gates and it hides the actual contract.♻️ Proposed simplification
const fn chain_enabled(chain: Chain) -> bool { - match chain { - #[cfg(feature = "web3")] - Chain::Btc => true, - #[cfg(feature = "web3")] - Chain::Evm => true, - #[cfg(feature = "web3")] - Chain::Solana => true, - #[cfg(feature = "web3")] - Chain::Tron => true, - #[cfg(not(all(feature = "web3", feature = "web3", feature = "web3", feature = "web3")))] - _ => false, - } + let _ = chain; + cfg!(feature = "web3") }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/web3/wallet/primitives/address/test.rs` around lines 21 - 34, Update chain_enabled to use a single feature = "web3" conditional for the enabled case and a complementary disabled-feature wildcard returning false; remove the repeated per-chain match arms and duplicated all(feature = "web3") condition while preserving true for supported chains only when web3 is enabled.src/openhuman/web3/wallet/primitives/x402/types.rs (1)
164-225: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the duplicated doc blocks on the proof types.
Five items carry two separate doc comments split by their attributes. Rust merges both blocks into one doc string, so rustdoc renders the description twice. The affected items are
PaymentProof(lines 164-172),SolanaPaymentProof(lines 179-186),EvmPaymentProof(lines 191-197),EvmAuthorization(lines 204-212), andPaymentChain(lines 299-302).Keep one block per item and place it before the attributes.
♻️ Proposed fix for `PaymentProof` and `SolanaPaymentProof`
-/// Chain-specific payment proof. Serializes flat (untagged) so the facilitator -/// sees either `{ "transaction": "..." }` (Solana) or -/// `{ "signature": "0x...", "authorization": {...} }` (EVM). -#[derive(Debug, Clone, Serialize, Deserialize)] -#[serde(untagged)] /// A chain-specific payment proof. /// -/// Serialises untagged, so a facilitator sees the chain's object directly. +/// Serialises untagged, so a facilitator sees the chain's object directly: +/// `{ "transaction": "..." }` for Solana, or +/// `{ "signature": "0x...", "authorization": {...} }` for EVM. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(untagged)] pub enum PaymentProof { /// A Solana partially-signed transaction. Solana(SolanaPaymentProof), /// An EVM EIP-3009 authorisation. Evm(EvmPaymentProof), } -/// Solana `exact` scheme payload — a partially-signed `VersionedTransaction` -/// serialized as standard base64. The facilitator adds its fee-payer signature -/// and broadcasts. -#[derive(Debug, Clone, Serialize, Deserialize)] -/// Solana `exact` proof: a partially-signed transaction, base64. +/// Solana `exact` proof: a partially-signed `VersionedTransaction`, standard +/// base64. /// /// The facilitator adds its fee-payer signature and broadcasts. +#[derive(Debug, Clone, Serialize, Deserialize)] pub struct SolanaPaymentProof {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/web3/wallet/primitives/x402/types.rs` around lines 164 - 225, Remove the duplicated documentation blocks for PaymentProof, SolanaPaymentProof, EvmPaymentProof, EvmAuthorization, and PaymentChain. Keep the preferred combined description once for each item, placing its single doc block before the derive and serde attributes so rustdoc renders it only once.src/openhuman/web3/wallet/transport.rs (1)
59-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe wildcard arm is now dead and hides a future compile error.
Chainnow lives in this crate.#[non_exhaustive]binds only downstream crates, so the four arms at lines 62-87 are exhaustive here. That is why line 59 needs#[allow(unreachable_patterns)].The
otherarm at lines 91-94 therefore never runs. Worse, it changes the failure mode: if somebody adds aChainvariant, this match silently returns a runtimeTransportError::Rpcinstead of failing the build at the one place that must map a chain to an endpoint. The comment at lines 88-90 states the pre-move reasoning, which no longer holds.Consider removing the wildcard arm and the
allow, and adding auseforChainto shorten the seven fully-qualified paths.♻️ Proposed refactor
-#[allow(unreachable_patterns)] fn resolve(network: NetworkId) -> Result<String, TransportError> { match network.chain { - crate::openhuman::web3::wallet::primitives::Chain::Evm => { + Chain::Evm => { // An EVM request names its EIP-155 chain id; resolving it here is // what keeps the wallet primitives free of OpenHuman's network enum. let chain_id = network.evm_chain_id.ok_or_else(|| TransportError::Rpc { network, message: "EVM requests require an EIP-155 chain id".to_string(), })?; let evm = EvmNetwork::ALL .iter() .copied() .find(|n| n.chain_id() == chain_id) .ok_or_else(|| TransportError::Rpc { network, message: format!("no configured EVM endpoint for chain id {chain_id}"), })?; Ok(rpc_url_for_evm_network(evm)) } - crate::openhuman::web3::wallet::primitives::Chain::Btc => { - Ok(rpc_url_for_chain(WalletChain::Btc)) - } - crate::openhuman::web3::wallet::primitives::Chain::Solana => { - Ok(rpc_url_for_chain(WalletChain::Solana)) - } - crate::openhuman::web3::wallet::primitives::Chain::Tron => { - Ok(rpc_url_for_chain(WalletChain::Tron)) - } - // `crate::openhuman::web3::wallet::primitives::Chain` is `#[non_exhaustive]`, so a future variant must - // be handled. Reporting it as authoritative is correct: no endpoint is - // configured for it, and retrying elsewhere cannot change that. - other => Err(TransportError::Rpc { - network, - message: format!("no endpoint configured for chain {other}"), - }), + Chain::Btc => Ok(rpc_url_for_chain(WalletChain::Btc)), + Chain::Solana => Ok(rpc_url_for_chain(WalletChain::Solana)), + Chain::Tron => Ok(rpc_url_for_chain(WalletChain::Tron)), + // `Chain` is `#[non_exhaustive]`, but it is now defined in this crate, + // so a new variant is a compile error here. That is deliberate: every + // chain must be given an endpoint at this one place. } }Add the import alongside the existing primitives imports:
use crate::openhuman::web3::wallet::primitives::Chain;Then update the three test sites at lines 249-252, 267-269, and 279 to use the shortened
Chain::…paths.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/web3/wallet/transport.rs` around lines 59 - 95, Update resolve to remove the #[allow(unreachable_patterns)] attribute and wildcard other arm, relying on exhaustive handling of the local Chain enum so newly added variants fail compilation until mapped to an endpoint; remove the obsolete non-exhaustive comment. Import Chain from the primitives module and replace the fully qualified Chain references in resolve and the identified test sites with Chain::… paths.
🔇 Additional comments (25)
Cargo.toml (1)
457-457: 🗄️ Data Integrity & Integration
⚠️ Unverified finding
Sandbox verification was unavailable.Verify isolated feature builds after replacing the legacy crates.
documentsnow enables onlymodules, whileweb3manually forwards the wallet-support dependencies. An--all-featurescheck can hide missing feature edges because it enables every optional dependency. Search for staletinydocsandtinywalletreferences, then verifydocumentsandweb3independently. Confirm that the lockfile matches the new dependency graph.Also applies to: 477-484, 681-681, 744-750
src/openhuman/tools/ops.rs (1)
1571-1572: 🗄️ Data Integrity & Integration
⚠️ Unverified finding
Sandbox verification was unavailable.Verify the local
Capabilityenum defines every variant this mapper uses.
tool_capabilitynow returnscrate::openhuman::memory::api::capabilities::Capability. The match arms below useCore,Recall,Tree,Entities,Diff,Maintenance,ToolMemory, andGoals. The local enum definition is not in this review context, so I cannot confirm the variant names match the formertinycortex_apitype. Confirm that no variant was renamed or dropped during the copy.src/openhuman/memory/api/goals.rs (1)
91-98: 📐 Maintainability & Code Quality
⚠️ Unverified finding
Sandbox verification was unavailable.Verify that goal text cannot contain a newline before it reaches
render.
renderwrites each item as a single- [{id}] {text}line.GoalItem::newtrims surrounding whitespace but keeps interior newlines. Iftextholds a newline,renderemits extra lines, and the nextparsedrops everything after the first line. The result is silent goal truncation across a parse → mutate → render round trip.The module doc places validation in the engine
GoalsDocMutationstrait, which is not part of this cohort. Confirm that layer rejects or normalizes embedded newlines.src/openhuman/memory/api/types.rs (2)
79-86: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
⚠️ Unverified finding
Sandbox verification was unavailable.Fix the doctest import paths, or mark the blocks as non-compiling.
Both examples use plain
```fences, so rustdoc compiles them as doctests. A doctest compiles as an external consumer of the crate. Inside it,crate::refers to the doctest's own anonymous crate, not to this library, souse crate::openhuman::memory::api::types::MemoryTaint;cannot resolve.Use the crate name in the path, or change the fence to
textif the block is illustrative only.🐛 Proposed fix for the first example
/// # Examples /// - /// ``` - /// use crate::openhuman::memory::api::types::MemoryTaint; + /// ```text + /// MemoryTaint::Internal.as_db_str() == "internal" + /// MemoryTaint::ExternalSync.as_db_str() == "external_sync" + /// ```Confirm whether the crate runs doctests and whether the path is publicly reachable:
Also applies to: 103-112
188-212: 🗄️ Data Integrity & Integration | ⚡ Quick win
⚠️ Unverified finding
Sandbox verification was unavailable.Verify the
serdedefaults on the portedOptionand collection fields.The module doc states that a field without
#[serde(default)]is a breaking change for a host reading previously written data, and it recommends#[serde(default)]for new fields. The added structs apply that rule unevenly:
MemoryEntry:namespaceandtaintcarry#[serde(default)];session_idandscoredo not.StoredMemoryDocument: onlytaintcarries it;session_id,tags, andmetadatado not, while the siblingNamespaceDocumentInputgives all three a default.
serdedoes not treatOption<T>as optional without#[serde(default)]. Any persisted JSON that omitssession_idtherefore fails to deserialize.The file states these types are a byte-compatible port. If the upstream shapes match exactly, keep them and no change is needed. Confirm the upstream field attributes before merging.
Also applies to: 292-325
src/openhuman/memory/api/host/cloud_providers.rs (1)
323-346: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
⚠️ Unverified finding
Sandbox verification was unavailable.
AuthStyleserde output andas_strdisagree forOpenhumanJwt.
#[serde(rename_all = "lowercase")]only lowercases the variant name. It does not insert an underscore. SoOpenhumanJwtserializes as"openhumanjwt", whileas_str()returns"openhuman_jwt". The other three variants agree.
CloudProviderCredsderivesSerializeandDeserialize, soauth_stylepersists as"openhumanjwt". Any consumer that compares a persisted or wire value againstas_str(), or any frontend that writes"openhuman_jwt", fails to match. The tests in this file assert the enum value but never assert either string, so the drift is unguarded.Pick one spelling and pin it. If configs already hold
"openhumanjwt", keep the serde form and correctas_str. If the intended wire form is"openhuman_jwt", add an explicit#[serde(rename)]and an alias for the old value.🐛 Proposed fix that keeps already-persisted values readable
pub enum AuthStyle { /// OpenAI-compatible: `Authorization: Bearer <key>` #[default] Bearer, /// Anthropic: `x-api-key: <key>` + `anthropic-version: 2023-06-01` Anthropic, /// OpenHuman session JWT (injected by the backend provider, not stored here). + #[serde(rename = "openhuman_jwt", alias = "openhumanjwt")] OpenhumanJwt, /// No auth header — e.g. local Ollama. None, }Then add a pinning test so the two forms cannot drift again:
#[test] fn auth_style_serde_form_matches_as_str() { for style in [ AuthStyle::Bearer, AuthStyle::Anthropic, AuthStyle::OpenhumanJwt, AuthStyle::None, ] { assert_eq!( serde_json::to_value(style).unwrap(), serde_json::json!(style.as_str()), "serde form drifted from as_str for {style:?}" ); } }Confirm the emitted string and find every consumer of both forms:
src/openhuman/memory/api/version.rs (1)
72-84: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
⚠️ Unverified finding
Sandbox verification was unavailable.Fix the doctest import path.
Rustdoc compiles this example as a separate crate that links the library. Inside that generated crate,
crate::resolves to the doctest crate, not to this library. Theuse crate::openhuman::memory::api::{...}line therefore fails to resolve, andcargo test --docfails.Replace
crate::with the library crate name, and confirm the whole module path is publicly reachable from the crate root. Ifmemoryorapiis a private module, the example cannot name these items at all; in that case convert the block to a plain text block or move the assertions intoversion_tests.rs, where they already exist.🐛 Proposed fix using the crate name
/// ``` -/// use crate::openhuman::memory::api::{is_compatible, CONTRACT_VERSION}; +/// use openhuman::openhuman::memory::api::{is_compatible, CONTRACT_VERSION}; ///src/openhuman/memory/api/provider/content.rs (1)
43-56: 🗄️ Data Integrity & Integration
⚠️ Unverified finding
Sandbox verification was unavailable.Confirm that
IngestItemcarries aMemoryTaint.Rule 4 in
src/openhuman/memory/api/provider/mod.rsstates thatMemoryTaintis an argument on every write path.ingest_documentandingest_chatare write paths, but neither takes aMemoryTaintparameter, so provenance must travel insideIngestItem.
MemoryCore::storetakestaintexplicitly, andNamespaceDocumentInputcarries ataintfield. IfIngestItemhas no equivalent field, these two ingest paths let a driver assign provenance itself, which is the laundering failure mode the taint rule prevents.
src/openhuman/memory/api/provider/types.rswas not supplied with this review.src/openhuman/memory/api/tool_memory.rs (1)
128-136: 📐 Maintainability & Code Quality
⚠️ Unverified finding
Sandbox verification was unavailable.Verify the UUID v4 feature.
Uuid::new_v4()requires theuuidcratev4feature. Confirm that the resolved dependency enables it, or this module will not compile. (docs.rs)src/openhuman/memory/api/host/composio.rs (1)
9-11: 📐 Maintainability & Code Quality
⚠️ Unverified finding
Sandbox verification was unavailable.Verify the referenced module path exists.
The doc comment points at
crate::openhuman::memory::core_impl::composio_host. The contracts moved intomemory/api/host/in this PR. Ifcore_impldoes not exist, this reference sends readers to a dead path.src/openhuman/memory/binding.rs (1)
344-353: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
⚠️ Unverified finding
Sandbox verification was unavailable.
bind_providerdiscards the operator's memory hooks and the driver trust state.
GuardPolicy::newnow receivesMemoryHooksConfig::default()and the literal"trusted". The caller hascfginbuild, andcfg.hookscarries operator settings that are overridable at runtime (OPENHUMAN_MEMORY_HOOKS_AUTO_RECALL,OPENHUMAN_MEMORY_HOOKS_MAX_CONTEXT_TOKENS, and the rest — seesrc/openhuman/config/schema/load_tests.rslines 817-821). IfGuardPolicyreads any hooks field, every binding now behaves as if the defaults were configured and those overrides have no effect.The hardcoded
"trusted"is not exploitable in this revision, becauseadmitrefuses everyDriverClass::Externalat lines 256-270. It does remove the seam the guard would need once the M4 transport lands. Thread the real values through instead of pinning them.♻️ Proposed change: pass the configured hooks and trust state
fn bind_provider( provider: Arc<dyn MemoryProvider>, driver_id: String, class: DriverClass, + hooks: crate::openhuman::config::schema::MemoryHooksConfig, + trust_state: &str, fallback: Option<FallbackReason>, ) -> MemoryBinding { let capabilities = provider.capabilities(); let guard = Arc::new(MemoryGuard::new( Arc::clone(&provider), Arc::new(GuardPolicy::new( driver_id.clone(), class, - crate::openhuman::config::schema::MemoryHooksConfig::default(), - "trusted", + hooks, + trust_state, )), ));
buildthen passescfg.hooks.clone()and the admitted driver'strust_state;bind_provider_for_testkeeps the defaults.src/openhuman/memory/driver/mod.rs (1)
1-5: 📐 Maintainability & Code Quality
⚠️ Unverified finding
Sandbox verification was unavailable.Confirm no orphaned sources remain under
driver/.This file no longer declares the
embeddedandmodule_adaptermodules, and it now declares nothing at all. The PR file list still namessrc/openhuman/memory/driver/embedded/core_family_tests.rs. If that directory survives without amod embedded;declaration, the files stay in the tree but leave the crate graph, so they no longer compile or run.If the directory is gone, consider whether a doc-only
mod.rsstill earns its place or whether the note belongs inmemory/mod.rs.src/openhuman/memory/ops/guard_tests.rs (1)
42-53: 🗄️ Data Integrity & Integration
⚠️ Unverified finding
Sandbox verification was unavailable.Verify that
modules::memory::MODULE_IDmatches the driver id the default binding reports.The assertion now compares
guard.driver_id()againstcrate::openhuman::modules::memory::MODULE_ID. The default[subsystems.memory] driveris"tinymemory"(default_memory_driverinsrc/openhuman/memory/api/host/subsystems.rs), andsrc/openhuman/memory/ops/provider.rsassertsstatus.driver == cfg.driverfor the same default binding. Both assertions hold only ifMODULE_IDequals the configured driver id string.Confirm the constant value and the id that
binding::for_workspacestamps on the guard.src/openhuman/memory/ops/provider.rs (1)
115-117: 🗄️ Data Integrity & Integration
⚠️ Unverified finding
Sandbox verification was unavailable.Confirm the local
CONTRACT_VERSIONpreserves the value previously emitted on the wire.
SubsystemStatus::contract_versionis a wire field the frontend reads, as the capability assertion below notes. The source constant moved from the TinyCortex API tocrate::openhuman::memory::api::CONTRACT_VERSION. The tests at lines 145-147 and 167-169 read the same constant as production, so they cannot detect a changed value.Verify that the local constant carries the same major/minor pair the previous constant carried.
src/openhuman/memory/ops/tool_memory.rs (1)
202-207: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
⚠️ Unverified finding
Sandbox verification was unavailable.The eager-rule filter is lost;
tool_rules_for_promptnow flattens every rule.The removed store call
rules_for_promptreturned only the rules that are eagerly pinned into the system prompt. The replacement callstool_rules(tool)for each requested tool and extendsflatwith everything it returns. No priority or eagerness filter remains in this handler.The existing test at line 367 asserts
prompt.rules.len() == 1with the message "only eager rules should be included", after storing oneHighrule forprimary_tooland oneNormalrule forsecondary_tooland requesting both tools. With this implementation,flatholds both rules, so the assertion fails unlesstool_rulesitself applies the eager filter.Either restore the filter in this handler, or confirm that the provider's
tool_rulesis the eager-scoped read and update the code comment to state that contract.🐛 Proposed fix if `tool_rules` returns every rule
let provider = tool_memory().await?; let family = provider.as_tool_memory().expect("checked"); let mut flat = Vec::new(); for tool in ¶ms.tools { - flat.extend(family.tool_rules(tool).await.map_err(|e| e.to_string())?); + flat.extend( + family + .tool_rules(tool) + .await + .map_err(|e| e.to_string())? + .into_iter() + .filter(|rule| rule.priority >= ToolMemoryPriority::High), + ); }Run the following script to determine whether
tool_rulesfilters by priority:src/openhuman/memory/tool_memory/capture.rs (1)
36-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
⚠️ Unverified finding
Sandbox verification was unavailable.This file still imports the tool-memory types from the external
tinycortexcrate.Line 39 resolves
ToolMemoryPriorityandToolMemorySourcefromtinycortex::memory::tool_memory. Every other migrated site in this change resolves the same two types fromcrate::openhuman::memory::api::tool_memory, includingsrc/openhuman/memory/ops/tool_memory.rsandsrc/openhuman/memory/tool_memory/prompt.rs.Two identically named types from different crates are distinct types. The test at line 464 passes rules produced through this file's store into the local
ToolMemoryRulesSection::new, which bridges by serde round-trip and drops values that fail to convert. A shape divergence between the two definitions therefore degrades silently at that boundary.If the external import is deliberate residue until the remaining host call sites migrate, add a short comment that says so and names the follow-up. Otherwise repoint it to the local API path.
src/openhuman/web3/wallet/mod.rs (1)
36-36: 🩺 Stability & Availability
⚠️ Unverified finding
Sandbox verification was unavailable.Verify that
primitivescompiles when theweb3feature is off.Line 36 declares
primitiveswithout a feature gate, but the sibling modulesrpc,schemas, andtoolsare gated onweb3. Submodules insideprimitivesgate their own contents onweb3in some places and not in others. If any un-gated item in theprimitivestree depends on aweb3-only dependency such asbech32,bs58, orsha3, a build withoutweb3will fail.Run the following script to inspect the gating of the
primitivestree and the optionality of its dependencies:src/openhuman/web3/wallet/primitives/address/mod.rs (1)
62-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
⚠️ Unverified finding
Sandbox verification was unavailable.Doc examples use
crate::paths that cannot compile as doctests. These modules were moved in from a standalone crate, wherecrate::resolved inside that crate. Rustdoc compiles each doc example as a separate crate, socrate::openhuman::…resolves to the doctest crate and the import fails. The items also sit underpub(crate) mod primitivesinsrc/openhuman/web3/wallet/mod.rs, so an external doctest crate cannot reach them by any path.Convert each example to a non-compiling fence such as
```textor```ignore, or move the assertions into the siblingtestmodule where they run against real code. Apply the same change at every site:
src/openhuman/web3/wallet/primitives/address/mod.rs#L62-L70: change thevalidateexample that importscrate::openhuman::web3::wallet::primitives::{address, chain::Chain}.src/openhuman/web3/wallet/primitives/abi/mod.rs#L67-L80: change theencode_erc20_transferexample, including theOk::<(), crate::openhuman::…::abi::Error>(())line.src/openhuman/web3/wallet/primitives/address/btc.rs#L94-L103: change thevalidateexample; apply the same change to thevalidate_senderexample at Lines 124-133.src/openhuman/web3/wallet/primitives/address/evm.rs#L45-L57: change thevalidateexample; apply the same change to theis_checksum_validexample at Lines 112-120 and theto_checksummedexample at Lines 135-141.src/openhuman/web3/wallet/primitives/address/solana.rs#L28-L36: change thevalidateexample; apply the same change to thedecodeexample at Lines 54-60 and theencodeexample at Lines 93-97.Run the following script to confirm whether any of these examples are collected and compiled:
src/openhuman/web3/wallet/primitives/address/tron.rs (1)
40-47: 📐 Maintainability & Code Quality | ⚡ Quick win
⚠️ Unverified finding
Sandbox verification was unavailable.Fix the
crate::paths in the doc examples, or mark the examples as non-compiled.Rustdoc compiles each
```example as its own crate that links this library. Inside that example,crate::resolves to the example crate, not to this library. Theuse crate::openhuman::…;line and theOk::<(), crate::openhuman::…::Error>(())line therefore fail to resolve when doctests run for the library target. The same pattern appears in theto_hexexample (Lines 97-104) and theencodeexample (Lines 124-130).Use the crate name in the example paths, or fence the examples as
text/ignoreif these items are not reachable from the crate root.src/openhuman/web3/wallet/primitives/error/mod.rs (1)
84-91: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
⚠️ Unverified finding
Sandbox verification was unavailable.Update the
ChainNotCompiledmessage to name the real crate and feature.The message tells the reader to enable a feature named after the chain, for example
'btc', in a crate namedtinywallet. This code now lives in the openhuman crate, andaddress/test.rsgates all four chains behind the singleweb3feature. A reader who follows this message enables a feature that does not exist.📝 Proposed message correction
#[error( - "tinywallet was built without support for {chain}; \ - enable the '{chain}' feature to validate its addresses" + "this build has no support for {chain}; \ + enable the 'web3' feature to validate its addresses" )]src/openhuman/web3/wallet/primitives/key/evm.rs (1)
25-32: 🎯 Functional Correctness | ⚡ Quick win
⚠️ Unverified finding
Sandbox verification was unavailable.Confirm that
to_checksummedaccepts an unprefixed hex body.
bodyholds 40 hex characters with no0xprefix. Ifto_checksummedrequires the prefix, this call always fails, the error is discarded, and every derived EVM address falls back to the lowercase form. The address stays valid, but the EIP-55 contract documented on Lines 23-24 no longer holds, and the fallback hides that.Confirm the accepted input form. If the prefix is required, pass
&format!("0x{body}").src/openhuman/web3/wallet/primitives/rpc/mod.rs (1)
162-200: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
⚠️ Unverified finding
Sandbox verification was unavailable.Fix the
usepath in the doc example.The example is a running doc test. A doc test compiles as a separate crate, so
crate::resolves to the doc test crate and not to this library. The pathcrate::openhuman::web3::wallet::primitives::rpc::{...}therefore fails to resolve whencargo test --docruns.Use the crate name, or mark the block as non-compiling.
♻️ Proposed fix using the crate name
/// ``` /// use async_trait::async_trait; /// use serde_json::Value; -/// use crate::openhuman::web3::wallet::primitives::rpc::{NetworkId, Transport, TransportError, TransportResult}; +/// use openhuman::openhuman::web3::wallet::primitives::rpc::{ +/// NetworkId, Transport, TransportError, TransportResult, +/// };src/openhuman/web3/wallet/transport.rs (1)
1-5: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
⚠️ Unverified finding
Sandbox verification was unavailable.Update the stale module documentation.
Two references no longer describe this codebase. Line 3 still names
tinywallet, which this PR replaced with the local primitives. Line 5 namescrate::openhuman::web3::wallet::primitives::client, and the primitives tree in this PR contains noclientmodule.Both are backticked prose rather than intra-doc links, so rustdoc will not fail. The text still misleads a reader.
src/openhuman/web3/x402/ops.rs (1)
598-598: 🗄️ Data Integrity & Integration | ⚡ Quick win
⚠️ Unverified finding
Sandbox verification was unavailable.Verify the swapped primitives API surface.
These three sites move from
tinywalletto the local primitives. The signatures must match exactly, because a mismatch changes the signed EIP-712 digest rather than failing loudly.The sibling tests in
src/openhuman/web3/x402/x402_tests.rscall the samekey::deriveandeip712helpers and pin the derived address, which corroborates the swap. Confirm the full surface anyway.Also applies to: 720-721, 735-735
vendor/tinymemory (1)
1-1: 🗄️ Data Integrity & Integration
⚠️ Unverified finding
Sandbox verification was unavailable.Verify the updated TinyMemory commit before merge.
Only the submodule pointer is visible here. Confirm that commit
bbffe68fd7327184c911741f3993bad70e3ecc1cis the intended TinyMemory release, inspect its diff from the previous pointer, and run the TinyMemory unit, integration, module E2E, and release-matrix tests. This is required to prove compatibility with the new OpenHuman memory contracts.
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
# Conflicts: # src/openhuman/flows/tinyflows/caps/ops.rs # src/openhuman/flows/tools.rs
|
@coderabbitai review |
✅ Action performedReview finished.
|
Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/memory/ops/tool_memory.rs`:
- Around line 8-11: Update tool_rule_put, tool_rule_get, tool_rules_for_prompt,
and tool_rules_json to invoke the provider through guard.as_tool_memory() rather
than guard.inner(), ensuring every handler passes through MemoryGuard admission
while preserving the existing NO_TOOL_MEMORY behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8d343b6c-8397-480c-81d5-7f5abd6c37d2
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockapp/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (36)
.github/workflows/ci-lite.ymldocs/specs/memory-guard-allowlist.mdsrc/core/all_tests.rssrc/core/cli_capability_tests.rssrc/core/runtime/context.rssrc/openhuman/config/schema/load_tests.rssrc/openhuman/flows/agents/workflow_builder/prompt.mdsrc/openhuman/flows/ops.rssrc/openhuman/flows/ops_tests.rssrc/openhuman/memory/api/null.rssrc/openhuman/memory/api/provider/content.rssrc/openhuman/memory/api/provider/knowledge.rssrc/openhuman/memory/api/version.rssrc/openhuman/memory/api/version_tests.rssrc/openhuman/memory/binding.rssrc/openhuman/memory/binding_tests.rssrc/openhuman/memory/bypass_allowlist_tests.rssrc/openhuman/memory/guard/families.rssrc/openhuman/memory/guard/test_support.rssrc/openhuman/memory/ops/documents.rssrc/openhuman/memory/ops/guard_tests.rssrc/openhuman/memory/ops/kv_graph.rssrc/openhuman/memory/ops/provider.rssrc/openhuman/memory/ops/tool_memory.rssrc/openhuman/modules/boot.rssrc/openhuman/modules/host.rssrc/openhuman/modules/memory.rssrc/openhuman/modules/memory_host.rssrc/openhuman/modules/memory_tests.rssrc/openhuman/modules/ops.rssrc/openhuman/modules/registry.rssrc/openhuman/tools/impl/document/format/mod.rssrc/openhuman/tools/impl/presentation/engine.rssrc/openhuman/tools/impl/presentation/mod.rssrc/openhuman/tools/ops_tests.rsvendor/tinymemory
💤 Files with no reviewable changes (2)
- src/openhuman/memory/binding_tests.rs
- src/openhuman/memory/bypass_allowlist_tests.rs
🚧 Files skipped from review as they are similar to previous changes (14)
- vendor/tinymemory
- src/openhuman/memory/ops/guard_tests.rs
- src/openhuman/modules/boot.rs
- src/core/all_tests.rs
- src/openhuman/memory/api/provider/content.rs
- src/openhuman/tools/impl/presentation/mod.rs
- src/openhuman/memory/guard/test_support.rs
- src/openhuman/modules/memory_tests.rs
- src/openhuman/tools/impl/presentation/engine.rs
- src/core/runtime/context.rs
- src/openhuman/tools/impl/document/format/mod.rs
- src/openhuman/memory/api/null.rs
- src/openhuman/memory/binding.rs
- src/openhuman/modules/memory_host.rs
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
1 participant
Summary
Upstream releases
Verification
cargo fmt --allcargo check --manifest-path Cargo.toml --lib --all-featuresBefore ready
TinyCortex/TinyMemory legacy crate dependencies still need to be removed after the remaining low-level host call sites are migrated to the module provider. This PR is intentionally draft until that last boundary is gone.
Summary by CodeRabbit