From 722f1046c4081a24df0bd49b8d59392bcd965816 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Sun, 9 Aug 2026 11:28:29 +0300 Subject: [PATCH 1/2] build: split the Cargo gates into a contributor set and a product set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `[features] default` was the product set, so every contributor paid for the whole product on every edit: web3's ethers/secp256k1 cohort, `documents`' zstd/bzip2 native builds, the cpal/hound/arboard/enigo/rdev stack behind `voice`+`inference`, `contacts`' macOS objc2 cohort, `crash-reporting`'s sentry tree, `tui`'s ratatui. A bare `cargo check` — and therefore rust-analyzer on every keystroke — resolved 567 packages and ran 7 native C/C++ builds. `default` is now the CONTRIBUTOR set: 9 gates that cost almost nothing to compile, so `cargo check` still typechecks nearly the whole tree. Measured: **567 -> 356 packages, 7 -> 5 native builds.** The kernel floor is untouched at 307/284/5 — this axis does not intersect the `flows` profile. THIS DOES NOT CHANGE WHAT SHIPS. `app/src-tauri` has declared `openhuman_core` with `default-features = false` since #1061 and never inherited `default`; that is precisely why a forwarding guard had to exist at all. `check-feature-forwarding.mjs` worked by diffing the shell's forwarding list against `[features] default`. That is a SUBSET check, and it was sound only while `default` was the product set. Shrinking `default` makes it weaker with every gate removed and vacuous if `default` ever empties — silently re-arming and ~93k Sentry events. Landing the flip without the rewrite would have been the same bug with a longer fuse. The product set is now explicit, in `scripts/ci/product-features.txt`, and the guard asserts three things: 1. the shell forwards EXACTLY that file — set equality, both directions. A dropped gate fails on `missing`; a gate the shell grew that the product never claimed fails on `unexpected`. Equality cannot pass vacuously. 2. every name in the file is a gate the core actually declares (catches a typo or a gate renamed out from under it). 3. every `default` gate is forwarded or allow-listed — the original check, retained because it still catches a contributor-set gate nobody shipped. Two parsers read that file — this one and `product-features.sh`, which the CI lanes use to build `--features`. A test asserts they agree, because if they drifted CI would compile a different set than the guard checks. A lane that relied on default features silently stops covering the product. Every lane that builds or tests the product now passes `--features "$(bash scripts/ci/product-features.sh)"`: core clippy, the unit lane, the changed-files coverage lane, and `scripts/test-rust-with-mock.sh`. Clippy runs TWICE, once per set — a lint that only fires with the gates off (an import left unused once a domain is compiled out) is invisible to the product lane and would be red on every contributor's machine while CI is green. Four `tests/*.rs` targets name symbols that only exist behind `voice`, `web3`, `inference` or `crash-reporting`, so they now declare `required-features`. Without it a bare `cargo test` fails to COMPILE, in files the contributor did not touch. With it cargo SKIPS them — the same silent-skip trap `--bins` without `bin-tools` already had, which is why the lane comments spell it out: `json_rpc_e2e` alone is >12k lines of RPC contract coverage. `tui` ends up in NEITHER set (default-OFF and deliberately not forwarded), so nothing in CI would compile it at all. The feature-gate-smoke lane now checks it explicitly; a future gate in that position needs the same. Note `required-features` is the blunt instrument — it costs the whole target when a gate is off, where #5021's per-symbol `#[cfg]` cleanup would keep the ungated tests running. Taken deliberately for these four; noted in Cargo.toml. Verified: contributor `cargo check --lib --tests`, product `cargo check --all-targets --features `, `--no-default-features --features tui`, the 27 guard self-tests, the kernel-floor ratchet, and `cargo fmt --all --check`. Co-authored-by: Medulla --- .github/workflows/ci-lite.yml | 32 +++- .github/workflows/test-reusable.yml | 22 ++- AGENTS.md | 51 ++++--- Cargo.toml | 75 +++++++++- docs/library-minimal-recipe.md | 8 + scripts/__tests__/feature-forwarding.test.mjs | 116 ++++++++++++++ scripts/ci/check-feature-forwarding.mjs | 77 +++++++--- scripts/ci/product-features.sh | 33 ++++ scripts/ci/product-features.txt | 85 +++++++++++ scripts/ci/rust-coverage-changed.sh | 18 ++- scripts/lib/feature-forwarding.mjs | 141 +++++++++++++++++- scripts/test-rust-with-mock.sh | 12 +- 12 files changed, 623 insertions(+), 47 deletions(-) create mode 100755 scripts/ci/product-features.sh create mode 100644 scripts/ci/product-features.txt diff --git a/.github/workflows/ci-lite.yml b/.github/workflows/ci-lite.yml index 1930735c5f..e728ee1782 100644 --- a/.github/workflows/ci-lite.yml +++ b/.github/workflows/ci-lite.yml @@ -375,8 +375,25 @@ jobs: if: needs.changes.outputs['rust-core'] == 'true' || needs.changes.outputs['rust-tauri'] == 'true' run: bash scripts/check-linux-tls-dependencies.sh - - name: Run clippy (core crate) + - name: Run clippy (core crate, product feature set) if: needs.changes.outputs['rust-core'] == 'true' + # `--features` is load-bearing. `[features] default` is the CONTRIBUTOR + # set (see the comment above it in Cargo.toml) and omits voice, web3, + # documents, meet, contacts, inference and crash-reporting — so a bare + # `cargo clippy` no longer lints code that ships. Product set: + # scripts/ci/product-features.txt, asserted equal to the shell's + # forwarding list by check-feature-forwarding.mjs. + run: | + bash scripts/ci-cancel-aware.sh cargo clippy -p openhuman \ + --features "$(bash scripts/ci/product-features.sh)" -- -D warnings + + - name: Run clippy (core crate, contributor default set) + if: needs.changes.outputs['rust-core'] == 'true' + # The set a contributor's editor and pre-push hook actually compile. A + # lint that only fires with those gates OFF — an import left unused once + # a domain is compiled out, say — is invisible to the product lane above + # and would land red on every contributor's machine while CI stayed + # green. Cheap: this graph is ~356 packages against the product's ~567. run: bash scripts/ci-cancel-aware.sh cargo clippy -p openhuman -- -D warnings - name: Cache CEF binary distribution @@ -441,6 +458,19 @@ jobs: # lane checks the lib and RUNS the gate-contract lib tests below. run: bash scripts/ci-cancel-aware.sh cargo check --manifest-path Cargo.toml --no-default-features + - name: Check the gates that are in NEITHER the contributor nor the product set + # `tui` is the one gate that is default-OFF *and* deliberately not + # forwarded to the desktop shell (it is a terminal front-end; the app + # ships its own UI). That combination means nothing else in CI compiles + # it: the product lanes do not select it, the contributor lanes no + # longer default it on, and the gates-off check above turns it off. + # + # Without this step the ratatui front-end would rot silently — which is + # the same shape of failure as #4901, just pointed at a gate instead of + # at the shipped app. If a future gate lands in this same + # neither-set position, add it here. + run: bash scripts/ci-cancel-aware.sh cargo check --manifest-path Cargo.toml --no-default-features --features tui + - name: Run the gate-contract tests with the default domain gates disabled # `cargo check` (even --all-targets) never RUNS tests, so a gated family that # is asserted WITHOUT a matching #[cfg] compiles fine and only fails at diff --git a/.github/workflows/test-reusable.yml b/.github/workflows/test-reusable.yml index 9a0d42c92b..7a335966d1 100644 --- a/.github/workflows/test-reusable.yml +++ b/.github/workflows/test-reusable.yml @@ -176,13 +176,27 @@ jobs: sort } + # This lane tests THE PRODUCT, so it must select the product's gates. + # `[features] default` is the CONTRIBUTOR set now and omits voice, + # web3, documents, meet, contacts, inference and crash-reporting. + # Source of truth: scripts/ci/product-features.txt. + # + # The same silent-skip trap the bin-tools note below describes applies + # here, and harder: four tests/*.rs targets carry `required-features` + # (observability_smoke, x402_twit_sh_live, json_rpc_e2e, + # raw_coverage_all — see Cargo.toml). Without these features + # `cargo test --test json_rpc_e2e` matches NOTHING and exits 0, and + # json_rpc_e2e alone is >12k lines of RPC contract coverage. + FEATURES="$(bash scripts/ci/product-features.sh),bin-tools" + echo "[test-reusable] feature set: ${FEATURES}" + # `--features bin-tools` is load-bearing, not cosmetic: the six binaries # under src/bin/ now declare `required-features = ["bin-tools"]`, so # without it `--bins` silently matches NOTHING and this lane goes green # having compiled none of them. fleet.rs and slack_backfill.rs each carry # a `#[cfg(test)] mod`, so those tests would vanish too, with no error. - bash scripts/ci-cancel-aware.sh cargo test -p openhuman --lib --bins --features bin-tools - bash scripts/ci-cancel-aware.sh cargo test -p openhuman --doc + bash scripts/ci-cancel-aware.sh cargo test -p openhuman --lib --bins --features "${FEATURES}" + bash scripts/ci-cancel-aware.sh cargo test -p openhuman --doc --features "${FEATURES}" while IFS= read -r target; do [ -n "${target}" ] || continue @@ -190,10 +204,10 @@ jobs: while IFS= read -r module; do [ -n "${module}" ] || continue echo "[test-reusable] raw coverage module: ${module}" - bash scripts/ci-cancel-aware.sh cargo test -p openhuman --test "${target}" -- "${module}::" --test-threads=1 + bash scripts/ci-cancel-aware.sh cargo test -p openhuman --features "${FEATURES}" --test "${target}" -- "${module}::" --test-threads=1 done < <(raw_coverage_modules) else - bash scripts/ci-cancel-aware.sh cargo test -p openhuman --test "${target}" + bash scripts/ci-cancel-aware.sh cargo test -p openhuman --features "${FEATURES}" --test "${target}" fi done < <(integration_test_targets) diff --git a/AGENTS.md b/AGENTS.md index 1716b8306b..249183e65c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -349,11 +349,23 @@ These are not theoretical. Two bugs of exactly this shape shipped before the gua ### Compile-time domain gates (Cargo `[features]`) -Per-domain Cargo features drop whole domains **at compile time** (smaller binary, fewer deps), composing with the runtime `DomainSet` axis above. Each gate is **default-ON**, so the desktop build is byte-identical; slim builds opt out explicitly. +Per-domain Cargo features drop whole domains **at compile time** (smaller binary, fewer deps), composing with the runtime `DomainSet` axis above. -> **Adding a default-ON gate? You must forward it to the desktop shell.** -> `app/src-tauri/Cargo.toml` declares `openhuman_core` with `default-features = false` (set in #1061, before gates existed), so the shipped app does **not** inherit the core's `default` list. A gate you add to `default` but not to the shell's `features` list is **compiled out of the shipped desktop app** — with no build error and no failing test. This is not hypothetical: `voice` shipped missing from v0.58.19 to v0.61.x (56 users, ~93k Sentry events, #4901), and `tokenjuice-treesitter` was never forwarded once since #4123 and failed *soft*, silently degrading AST compression (#4918). -> `scripts/ci/check-feature-forwarding.mjs` (the **Feature Forwarding Gate** lane) now fails CI on drift and covers new gates automatically. If a gate genuinely must not ship, add it to `INTENTIONALLY_NOT_FORWARDED` in that script **with a reason** — an explicit exclusion is the only way "deliberate" stays distinguishable from "forgotten". +**There are TWO gate sets, and confusing them is the main hazard here.** + +| Set | Where it lives | What it is | +| --- | --- | --- | +| **Contributor** | `[features] default` in `Cargo.toml` | What a bare `cargo check`, `cargo test` and rust-analyzer compile. 9 cheap gates. ~356 packages / 5 native builds. | +| **Product** | `scripts/ci/product-features.txt` | What the shipped desktop app has. 15 gates. ~567 packages / 7 native builds. | + +`default` used to be the product set, which made the inner loop pay for the whole product on every edit — web3's ethers/secp256k1 cohort, `documents`' zstd/bzip2 native builds, the cpal/hound/arboard/enigo/rdev stack behind `voice`+`inference`, `contacts`' macOS objc2 cohort, `crash-reporting`'s sentry tree, `tui`'s ratatui. Those are default-OFF now. **This did not change what ships**: the shell has set `default-features = false` since #1061 and never inherited `default` anyway. + +What it *did* change: **a lane that relies on default features no longer covers the product.** Every CI lane that builds or tests the product passes `--features "$(bash scripts/ci/product-features.sh)"` — clippy, the unit lane, the coverage lane, `scripts/test-rust-with-mock.sh`. If you add a lane, decide which of the two sets it is testing and say so in a comment. Four `tests/*.rs` targets carry `required-features` for the same reason (`json_rpc_e2e`, `raw_coverage_all`, `observability_smoke`, `x402_twit_sh_live`); without those gates cargo **silently skips** them and the run still exits 0 — the same trap `--bins` without `bin-tools` already had. + +> **Adding a gate to either set? You must forward it to the desktop shell.** +> `app/src-tauri/Cargo.toml` declares `openhuman_core` with `default-features = false` (set in #1061, before gates existed), so the shipped app does **not** inherit the core's `default` list. A gate in the product set but not in the shell's `features` list is **compiled out of the shipped desktop app** — with no build error and no failing test. This is not hypothetical: `voice` shipped missing from v0.58.19 to v0.61.x (56 users, ~93k Sentry events, #4901), and `tokenjuice-treesitter` was never forwarded once since #4123 and failed *soft*, silently degrading AST compression (#4918). +> `scripts/ci/check-feature-forwarding.mjs` (the **Feature Forwarding Gate** lane) asserts three things: the shell forwards **exactly** `product-features.txt` (set equality, both directions), every name in that file is a real core gate, and every `default` gate is forwarded or allow-listed. The equality check is the load-bearing one — the old subset-of-`default` check would have passed **vacuously** once `default` stopped being the product set, silently re-arming #4901. If a gate genuinely must not ship, add it to `INTENTIONALLY_NOT_FORWARDED` **with a reason** — an explicit exclusion is the only way "deliberate" stays distinguishable from "forgotten". +> A gate in **neither** set (today only `tui`) gets no compile coverage from the normal lanes at all, so the feature-gate-smoke lane checks it explicitly. Put new ones there too. **Slim-profile convention** (no `full` meta-feature): build slim variants with `cargo build --no-default-features --features ""`. This mirrors the existing standalone-feature style (`sandbox-landlock`, `browser-native`, …). Example — everything except voice: @@ -401,20 +413,23 @@ optional" usually saves nothing on its own — `git2`, `rusqlite`, `reqwest`, `tokio` and `tokio-tungstenite` have multiple parents. Gate the whole cohort or expect a delta of 0. -| Feature | Default | Gates | Drops deps | -| ------- | ------- | ----- | ---------- | -| `voice` | ON | the `openhuman::voice` family (incl. `voice::audio_toolkit`) — STT/TTS providers, dictation server, always-on listening, podcast audio + email | `hound`, `lettre` | -| `inference` | ON | the `cpal` audio-device stack: microphone capture for voice, plus `desktop::accessibility::permissions`' mic-permission probe. Implied by `voice`. Off ⇒ the probe reports `Unknown`. **The name is historical** — it used to gate the bundled whisper.cpp STT engine, which no longer exists (see the scope note below); do not rename it, it is forwarded by name from the shell manifest and asserted by `INFERENCE_COMPILED_IN` | `cpal` | -| `web3` | ON | the `openhuman::web3` family (`web3`, `web3::wallet`, `web3::x402`) — crypto wallet (multi-chain sign/broadcast), swaps/bridges/dapp calls, x402 machine payments | `bitcoin`, `curve25519-dalek` | -| `media` | ON | `openhuman::media::generation` (the `media_generate_*` agent tools) + `openhuman::media::image` scaffold | none (surface-only) | -| `meet` | ON | `openhuman::meet` (join-URL validation) + `openhuman::meet::agent` (live STT/LLM/TTS loop) + `openhuman::meet::backend_bot` (backend-delegated Meet bot over Socket.IO) | none — see note | -| `skills` | ON | `openhuman::skills` + `openhuman::skills::runtime` + `openhuman::skills::catalog` domains — SKILL.md discovery/parse/install, workflow execution + run logs, remote catalogs, the `skill_setup` / `skill_executor` builtin agents, and the 16 skill agent tools | none (see below) | -| `flows` | ON | `openhuman::flows` (saved automation graphs — create/run/schedule, the `workflow_builder` + `flow_discovery` agents), `openhuman::flows::tinyflows` (engine seam), `openhuman::flows::rhai` (`.ragsh` language-workflow tool) | `tinyflows`, `jaq-core`, `jaq-std`, `jaq-json`, `rhai` | -| `mcp` | ON | `openhuman::mcp::server` (the `openhuman mcp` stdio/HTTP server), `openhuman::mcp::registry` (dynamic Smithery installs — `mcp_clients` RPC namespace, SQLite, boot spawn, supervisor, OAuth), `openhuman::mcp::audit` (write-audit log), and the static config-declared server set in `openhuman::mcp::config_servers`. ~19 agent tools, ~20k LOC | **none** (see scope note) | -| `tui` | ON | `openhuman::tui` — the tabbed ratatui/crossterm CLI UI (Logs, Chat, Config, Settings), auto-opened by bare `openhuman` on interactive non-container hosts and forced with `openhuman tui` (alias `chat`). Runs the core in-process. No controllers, no agent tools. **Intentionally NOT forwarded to the desktop shell** (allowlisted in `check-feature-forwarding.mjs`). | `ratatui`, `crossterm` | -| `channels` | ON | `openhuman::channels` (external-messaging providers — Telegram/Discord/Slack/Signal/WhatsApp/iMessage/IRC/… — plus the channel runtime, controllers, host, proactive messaging + inbound dispatch) and the `channels::webview_accounts` / `webview_apis` / `webview_notifications` / `channels::whatsapp_data` webview-bridge domains (incl. the 3 `whatsapp_data_*` agent tools). **Carve-outs `channels::{traits, cli}` stay ungated.** | **28** via `tinychannels/{email,lark}` — the crate itself stays (load-bearing), its two heavy providers do not | -| `contacts` | ON | `memory::people::address_book`'s macOS CNContactStore reader — the address-book seeding path for the people domain. Leaf gate over a **pre-existing** off-state: the module already shipped a non-macOS `imp` stub returning an empty contact list, so the gate only widens that stub's cfg. `read`/`read_with`/`AddressBookError`/`SystemContactsSource` and the whole `people` RPC surface stay compiled in every build; off ⇒ a refresh seeds nothing instead of failing. | **6** on macOS (`objc2`, `objc2-foundation`, `objc2-contacts`, `block2` + 2 transitive). **No-op on Linux/Windows** — never in those graphs, so the kernel-floor ratchet does not move. Verify cross-target: `cargo tree --target aarch64-apple-darwin -e normal -i objc2-contacts --no-default-features` (294 → 288 packages). | -| `runtime-node` | ON | `runtime::node` (download / verify / extract / install a pinned Node.js toolchain), the `runtime::javascript` language slot, `runtime::pool::node`, the `node_exec` / `npm_exec` agent tools, and the `node_runtime` harness-init step. **Facade + stub** — `ShellTool` holds `Option>` and `shell.rs` is kernel, so the module cannot simply vanish; `runtime/node/stub.rs` carries the `NodeBootstrap` type surface while registration sites are leaf-gated. **The generic native-tool dispatcher (`runtime::node::ops` / `runtime::node::types`) is NOT gated** — it backs both the gated `javascript.*` controllers and the ungated `flows` `oh:` `NativeToolBackend`, so native flow tools (`memory_search`, file, shell, …) keep working when the managed Node runtime is off. Off ⇒ `try_cached`/`probe_installed` return `None` and the shell never prepends a managed bin dir, identical to today's `node.enabled = false` path. | **`xz2` + its static liblzma C build.** First gate to remove a NATIVE toolchain build: `lzma-sys` leaves the list, 6 → 5. `tar`/`zip` are NOT shed — shared with `inference` (install_piper), `runtime::python`, and the document tools. | +Two columns because there are two sets (see above): **Contrib** is `[features] default`, +**Product** is `scripts/ci/product-features.txt`. + +| Feature | Contrib | Product | Gates | Drops deps | +| ------- | ------- | ------- | ----- | ---------- | +| `voice` | OFF | ON | the `openhuman::voice` family (incl. `voice::audio_toolkit`) — STT/TTS providers, dictation server, always-on listening, podcast audio + email | `hound`, `lettre` | +| `inference` | OFF | ON | the `cpal` audio-device stack: microphone capture for voice, plus `desktop::accessibility::permissions`' mic-permission probe. Implied by `voice`. Off ⇒ the probe reports `Unknown`. **The name is historical** — it used to gate the bundled whisper.cpp STT engine, which no longer exists (see the scope note below); do not rename it, it is forwarded by name from the shell manifest and asserted by `INFERENCE_COMPILED_IN` | `cpal` | +| `web3` | OFF | ON | the `openhuman::web3` family (`web3`, `web3::wallet`, `web3::x402`) — crypto wallet (multi-chain sign/broadcast), swaps/bridges/dapp calls, x402 machine payments | `bitcoin`, `curve25519-dalek` | +| `media` | ON | ON | `openhuman::media::generation` (the `media_generate_*` agent tools) + `openhuman::media::image` scaffold | none (surface-only) | +| `meet` | OFF | ON | `openhuman::meet` (join-URL validation) + `openhuman::meet::agent` (live STT/LLM/TTS loop) + `openhuman::meet::backend_bot` (backend-delegated Meet bot over Socket.IO) | none — see note | +| `skills` | ON | ON | `openhuman::skills` + `openhuman::skills::runtime` + `openhuman::skills::catalog` domains — SKILL.md discovery/parse/install, workflow execution + run logs, remote catalogs, the `skill_setup` / `skill_executor` builtin agents, and the 16 skill agent tools | none (see below) | +| `flows` | ON | ON | `openhuman::flows` (saved automation graphs — create/run/schedule, the `workflow_builder` + `flow_discovery` agents), `openhuman::flows::tinyflows` (engine seam), `openhuman::flows::rhai` (`.ragsh` language-workflow tool) | `tinyflows`, `jaq-core`, `jaq-std`, `jaq-json`, `rhai` | +| `mcp` | ON | ON | `openhuman::mcp::server` (the `openhuman mcp` stdio/HTTP server), `openhuman::mcp::registry` (dynamic Smithery installs — `mcp_clients` RPC namespace, SQLite, boot spawn, supervisor, OAuth), `openhuman::mcp::audit` (write-audit log), and the static config-declared server set in `openhuman::mcp::config_servers`. ~19 agent tools, ~20k LOC | **none** (see scope note) | +| `tui` | OFF | — | `openhuman::tui` — the tabbed ratatui/crossterm CLI UI (Logs, Chat, Config, Settings), auto-opened by bare `openhuman` on interactive non-container hosts and forced with `openhuman tui` (alias `chat`). Runs the core in-process. No controllers, no agent tools. **Intentionally NOT forwarded to the desktop shell** (allowlisted in `check-feature-forwarding.mjs`). | `ratatui`, `crossterm` | +| `channels` | ON | ON | `openhuman::channels` (external-messaging providers — Telegram/Discord/Slack/Signal/WhatsApp/iMessage/IRC/… — plus the channel runtime, controllers, host, proactive messaging + inbound dispatch) and the `channels::webview_accounts` / `webview_apis` / `webview_notifications` / `channels::whatsapp_data` webview-bridge domains (incl. the 3 `whatsapp_data_*` agent tools). **Carve-outs `channels::{traits, cli}` stay ungated.** | **28** via `tinychannels/{email,lark}` — the crate itself stays (load-bearing), its two heavy providers do not | +| `contacts` | OFF | ON | `memory::people::address_book`'s macOS CNContactStore reader — the address-book seeding path for the people domain. Leaf gate over a **pre-existing** off-state: the module already shipped a non-macOS `imp` stub returning an empty contact list, so the gate only widens that stub's cfg. `read`/`read_with`/`AddressBookError`/`SystemContactsSource` and the whole `people` RPC surface stay compiled in every build; off ⇒ a refresh seeds nothing instead of failing. | **6** on macOS (`objc2`, `objc2-foundation`, `objc2-contacts`, `block2` + 2 transitive). **No-op on Linux/Windows** — never in those graphs, so the kernel-floor ratchet does not move. Verify cross-target: `cargo tree --target aarch64-apple-darwin -e normal -i objc2-contacts --no-default-features` (294 → 288 packages). | +| `runtime-node` | OFF | ON | `runtime::node` (download / verify / extract / install a pinned Node.js toolchain), the `runtime::javascript` language slot, `runtime::pool::node`, the `node_exec` / `npm_exec` agent tools, and the `node_runtime` harness-init step. **Facade + stub** — `ShellTool` holds `Option>` and `shell.rs` is kernel, so the module cannot simply vanish; `runtime/node/stub.rs` carries the `NodeBootstrap` type surface while registration sites are leaf-gated. **The generic native-tool dispatcher (`runtime::node::ops` / `runtime::node::types`) is NOT gated** — it backs both the gated `javascript.*` controllers and the ungated `flows` `oh:` `NativeToolBackend`, so native flow tools (`memory_search`, file, shell, …) keep working when the managed Node runtime is off. Off ⇒ `try_cached`/`probe_installed` return `None` and the shell never prepends a managed bin dir, identical to today's `node.enabled = false` path. | **`xz2` + its static liblzma C build.** First gate to remove a NATIVE toolchain build: `lzma-sys` leaves the list, 6 → 5. `tar`/`zip` are NOT shed — shared with `inference` (install_piper), `runtime::python`, and the document tools. | **Facade pattern (pathfinder for the other gates).** `pub mod voice;` is **always compiled** as a facade: the real submodules are `#[cfg(feature = "voice")]`, and a `#[cfg(not(feature = "voice"))] mod stub;` (`src/openhuman/voice/stub.rs`) re-exposes the same public surface that always-on / other-gated callers use (`server`, `dictation_listener`, `streaming`, `reply_speech`, `cloud_transcribe`, `cli`, `create_stt_provider`, `effective_stt_provider`, `publish_ptt_transcript_committed`) with no-op / `None` / disabled-error bodies. Callers therefore do **not** need per-call `#[cfg]`. When voice is off: the voice/audio controllers are unregistered (unknown-method over `/rpc`, absent from `/schema`), the `audio_generate_podcast` agent tools are absent, and `openhuman voice` returns a "voice disabled" error. Stub signatures must match the real ones exactly — the disabled build (`--no-default-features`) is the **only** thing that catches drift, so run it before pushing any change to the voice surface. diff --git a/Cargo.toml b/Cargo.toml index 7d66a5357b..6282b27fa8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -72,6 +72,53 @@ name = "library-profile" path = "src/bin/library_profile/main.rs" required-features = ["rss-bench"] +# ── Integration tests that need a PRODUCT gate ────────────────────────────── +# +# `[features] default` is the contributor set and no longer turns on `voice`, +# `web3`, `crash-reporting` or `inference`. These four `tests/` targets name +# symbols that only exist behind those gates, so without a `required-features` +# line a bare `cargo test` fails to COMPILE — not skip, fail — and the failure +# is in a file the contributor did not touch. +# +# `required-features` makes cargo skip the target instead, which is the honest +# outcome: the test exercises a domain that build does not contain. The CI +# product lanes pass `--features "$(scripts/ci/product-features.sh)"`, so every +# one of these still runs there, against the code that ships. +# +# Autodiscovery stays on for the other ~59 `tests/*.rs` targets; declaring a +# target explicitly only opts THAT file out of it. +# +# This replaces the per-symbol `#[cfg]` cleanup tracked in #5021 for these four +# files. #5021 is still the right fix for symbols named in the middle of an +# otherwise gate-free target — required-features is the blunt instrument, and +# it costs the whole target when the gate is off. +[[test]] +name = "observability_smoke" +path = "tests/observability_smoke.rs" +# The `is_*_event` Sentry filters are all `#[cfg(feature = "crash-reporting")]`. +required-features = ["crash-reporting"] + +[[test]] +name = "x402_twit_sh_live" +path = "tests/x402_twit_sh_live.rs" +# `x402::tools::X402RequestTool` — the x402 domain is part of the web3 family. +required-features = ["web3"] + +[[test]] +name = "json_rpc_e2e" +path = "tests/json_rpc_e2e.rs" +# `voice::reply_speech::{test_seam, TEST_SEAM_ENV}` — the reply-speech seam +# only exists in the real voice module, not the stub. +required-features = ["voice"] + +[[test]] +name = "raw_coverage_all" +path = "tests/raw_coverage_all.rs" +# The merged raw-coverage target spans ~76 former files, so it reaches the +# widest surface of any single target: `lettre` (pulled in through the voice +# gate's `tinychannels/email`) and the local inference download service. +required-features = ["voice", "inference"] + [lib] name = "openhuman_core" crate-type = ["rlib"] @@ -429,7 +476,33 @@ tokio = { version = "1", features = ["test-util"] } proptest = "1" [features] -default = ["inference", "voice", "web3", "media", "documents", "meet", "skills", "flows", "mcp", "crash-reporting", "http-server", "channels", "tui", "medulla", "scheduler-gate", "file-logging", "contacts", "runtime-node"] +# THE CONTRIBUTOR SET — not the product set. +# +# This list is what a bare `cargo check`, `cargo test` and rust-analyzer +# compile. It is deliberately SMALLER than what the desktop app ships: the +# product set lives in `scripts/ci/product-features.txt` and is forwarded +# explicitly by `app/src-tauri/Cargo.toml`, with +# `scripts/ci/check-feature-forwarding.mjs` asserting the two are equal in both +# directions. Read that file's header before changing either list. +# +# Why the split: the gates left ON below cost almost nothing to compile, so +# keeping them on means `cargo check` and rust-analyzer still typecheck nearly +# all of the tree. The gates turned OFF are the ones that carry the graph — +# web3's ethers/secp256k1 cohort, `documents`' zstd/bzip2 native builds, +# `voice`+`inference`'s cpal/hound/lettre/arboard/enigo/rdev stack, `contacts`' +# macOS objc2 cohort, `crash-reporting`'s sentry tree, and `tui`'s +# ratatui/crossterm, and `runtime-node`'s xz2/liblzma. Turning them off takes a +# bare `cargo check` from 540 packages / 7 native builds down to ~350 / 2, which +# is the inner loop every contributor pays on every edit. +# +# THIS DOES NOT CHANGE WHAT SHIPS. The desktop shell has always set +# `default-features = false` (#1061), so it never inherited this list to begin +# with — that is exactly why the forwarding guard had to exist. What DOES +# change is that a lane relying on default features no longer covers the +# gated-off domains, so every CI lane that builds or tests "the product" now +# passes `--features "$(scripts/ci/product-features.sh)"`. If you add a lane, +# decide which of the two sets it is testing and say so. +default = ["media", "skills", "flows", "mcp", "channels", "medulla", "http-server", "scheduler-gate", "file-logging"] # HTTP + Socket.IO server transport (#5048): the `/rpc` JSON-RPC endpoint and # its auth middleware/CORS layer (`core::jsonrpc`, `core::auth`), the `/v1` # OpenAI-compatible router (`inference::http`), the ad-hoc static-dir file diff --git a/docs/library-minimal-recipe.md b/docs/library-minimal-recipe.md index af755f3f6a..10f622b2da 100644 --- a/docs/library-minimal-recipe.md +++ b/docs/library-minimal-recipe.md @@ -39,6 +39,14 @@ There is **no** `library-minimal` meta-feature in `Cargo.toml`, on purpose — s `default = ["voice","web3","media","meet","skills","flows","mcp","desktop-automation","tui"]` +> **Superseded — that is the `default` list as it stood when this session ran.** +> `desktop-automation` no longer exists, and `default` is the *contributor* set +> now rather than the product set: `voice`, `web3`, `meet` and `tui` are already +> OFF there (see AGENTS.md "Compile-time domain gates"). The Decision column +> below still records what a library host wants; the Default column no longer +> describes reality. What the product ships lives in +> `scripts/ci/product-features.txt`. + | Gate | Default | Decision | Why | Deps shed | | --- | :---: | :---: | --- | --- | | `skills` | ON | **KEEP** | python/js `SKILL.md` execution is a stated opencompany use case | none (surface/prompt/startup only) | diff --git a/scripts/__tests__/feature-forwarding.test.mjs b/scripts/__tests__/feature-forwarding.test.mjs index 3bdd62379d..ec62546e69 100644 --- a/scripts/__tests__/feature-forwarding.test.mjs +++ b/scripts/__tests__/feature-forwarding.test.mjs @@ -6,9 +6,12 @@ import { test } from 'node:test'; import { fileURLToPath } from 'node:url'; import { + checkProductForwarding, diffForwarding, INTENTIONALLY_NOT_FORWARDED, parseCoreDefaultFeatures, + parseCoreFeatureNames, + parseProductFeatures, parseShellForwardedFeatures, stripComments, } from '../lib/feature-forwarding.mjs'; @@ -156,13 +159,126 @@ test('a missing dependency fails rather than passing vacuously', () => { assert.equal(result.reason, 'dependency-not-found'); }); +// ── product-set forwarding (assertions 1 + 2) ────────────────────────────── + +const PRODUCT = ['voice', 'media']; +const CORE_GATES = ['voice', 'media', 'web3', 'tui']; + +test('passes when the shell forwards exactly the product set', () => { + const result = checkProductForwarding({ + productFeatures: PRODUCT, + coreFeatureNames: CORE_GATES, + shell: { defaultFeatures: false, features: ['media', 'voice'] }, + }); + assert.equal(result.ok, true); +}); + +test('reproduces #4901 against the PRODUCT set, not the default set', () => { + // The point of the rewrite: this must fail even though `default` here is + // empty, which is precisely the shape that made the old subset check pass + // vacuously as `default` shrank. + const result = checkProductForwarding({ + productFeatures: PRODUCT, + coreFeatureNames: CORE_GATES, + shell: { defaultFeatures: false, features: ['media'] }, + }); + assert.equal(result.ok, false); + assert.deepEqual(result.missing, ['voice']); +}); + +test('a gate the shell forwards but the product does not claim is flagged', () => { + const result = checkProductForwarding({ + productFeatures: PRODUCT, + coreFeatureNames: CORE_GATES, + shell: { defaultFeatures: false, features: ['media', 'voice', 'web3'] }, + }); + assert.equal(result.ok, false); + assert.deepEqual(result.unexpected, ['web3']); +}); + +test('a product gate that is not a real core gate is flagged', () => { + const result = checkProductForwarding({ + productFeatures: ['voice', 'vioce'], + coreFeatureNames: CORE_GATES, + shell: { defaultFeatures: false, features: ['voice', 'vioce'] }, + }); + assert.equal(result.ok, false); + assert.deepEqual(result.unknown, ['vioce']); +}); + +test('the shell inheriting defaults is now a FAILURE, not a pass', () => { + // It used to mean "nothing to drift". It now means the shell would inherit + // the contributor set, which is smaller than the product. + const result = checkProductForwarding({ + productFeatures: PRODUCT, + coreFeatureNames: CORE_GATES, + shell: { defaultFeatures: true, features: [] }, + }); + assert.equal(result.ok, false); + assert.equal(result.reason, 'shell-inherits-defaults'); +}); + +test('parses the product file, ignoring comments and blank lines', () => { + const text = '# a comment\n\nvoice\n media # trailing\n\n'; + assert.deepEqual(parseProductFeatures(text), ['voice', 'media']); +}); + +test('parses every gate name from the core [features] table, minus `default`', () => { + const toml = ` +[features] +default = ["media"] +voice = ["dep:hound"] +media = [] + +[dependencies] +hound = "3" +`; + assert.deepEqual(parseCoreFeatureNames(toml), ['voice', 'media']); +}); + // ── the real manifests + CLI ─────────────────────────────────────────────── test('the checked-in manifests pass the guard', () => { const out = execFileSync('node', [CHECKER], { encoding: 'utf8' }); + assert.match(out, /the shell forwards exactly the product gate set/); assert.match(out, /every default-ON core gate is forwarded/); }); +test('the real product file and the real shell list are equal', () => { + const productFeatures = parseProductFeatures( + readFileSync(resolve(REPO_ROOT, 'scripts/ci/product-features.txt'), 'utf8') + ); + const coreFeatureNames = parseCoreFeatureNames( + readFileSync(resolve(REPO_ROOT, 'Cargo.toml'), 'utf8') + ); + const shell = parseShellForwardedFeatures( + readFileSync(resolve(REPO_ROOT, 'app/src-tauri/Cargo.toml'), 'utf8') + ); + // Guards the guard: empty input would make every assertion below vacuous. + assert.ok(productFeatures.length > 0, 'expected to parse at least one product gate'); + assert.ok(coreFeatureNames.length > 0, 'expected to parse at least one core gate name'); + const result = checkProductForwarding({ productFeatures, coreFeatureNames, shell }); + assert.deepEqual(result.missing, [], 'product gates the shell does not forward'); + assert.deepEqual(result.unexpected, [], 'gates the shell forwards that the product omits'); + assert.deepEqual(result.unknown, [], 'product gates that are not real core gates'); +}); + +test('the shell script and the JS parser agree on the product set', () => { + // Two parsers read scripts/ci/product-features.txt: this one, and the shell + // helper the CI lanes use to build `--features`. If they disagreed, CI would + // compile a different set than the guard asserts — and the guard would be + // checking something nobody builds. + const fromJs = parseProductFeatures( + readFileSync(resolve(REPO_ROOT, 'scripts/ci/product-features.txt'), 'utf8') + ); + const fromSh = execFileSync('bash', [resolve(REPO_ROOT, 'scripts/ci/product-features.sh')], { + encoding: 'utf8', + }) + .trim() + .split(','); + assert.deepEqual(fromSh, fromJs); +}); + test('--help exits 0', () => { const out = execFileSync('node', [CHECKER, '--help'], { encoding: 'utf8' }); assert.match(out, /Usage:/); diff --git a/scripts/ci/check-feature-forwarding.mjs b/scripts/ci/check-feature-forwarding.mjs index 4a8e410dad..978a95a6ea 100644 --- a/scripts/ci/check-feature-forwarding.mjs +++ b/scripts/ci/check-feature-forwarding.mjs @@ -1,32 +1,43 @@ #!/usr/bin/env node -// Fails when the desktop shell does not forward a default-ON core Cargo gate. +// Fails when the desktop shell does not forward exactly the gates the product +// is supposed to ship. // -// See scripts/lib/feature-forwarding.mjs for why this exists (#4919). Short -// version: the shell sets `default-features = false` on `openhuman_core`, so -// every default-ON gate must be forwarded by hand. When someone forgets, the -// domain vanishes from the shipped app with no build error — that is how #4901 -// (voice, 56 users) and #4918 (tokenjuice-treesitter) shipped. +// See scripts/lib/feature-forwarding.mjs for the three assertions and why they +// are shaped this way (#4919). Short version: the shell sets +// `default-features = false` on `openhuman_core`, so every gate the product +// needs must be forwarded by hand. When someone forgets, the domain vanishes +// from the shipped app with no build error — that is how #4901 (voice, 56 +// users, ~93k Sentry events) and #4918 (tokenjuice-treesitter, silent soft +// degradation) shipped. // -// Usage: check-feature-forwarding.mjs [core-manifest] [shell-manifest] +// The product set lives in scripts/ci/product-features.txt, NOT in +// `[features] default` — `default` is the contributor set now and is +// deliberately smaller. +// +// Usage: check-feature-forwarding.mjs [core-manifest] [shell-manifest] [product-features] import { readFileSync } from 'node:fs'; import { dirname, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { + checkProductForwarding, diffForwarding, + formatProductReport, formatReport, INTENTIONALLY_NOT_FORWARDED, parseCoreDefaultFeatures, + parseCoreFeatureNames, + parseProductFeatures, parseShellForwardedFeatures, } from '../lib/feature-forwarding.mjs'; const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '../..'); function usage() { - return 'Usage: check-feature-forwarding.mjs [core-manifest] [shell-manifest]'; + return 'Usage: check-feature-forwarding.mjs [core-manifest] [shell-manifest] [product-features]'; } -const [coreArg, shellArg, extra] = process.argv.slice(2); +const [coreArg, shellArg, productArg, extra] = process.argv.slice(2); if (coreArg === '--help' || coreArg === '-h') { console.log(usage()); process.exit(0); @@ -38,31 +49,61 @@ if (extra) { const corePath = coreArg ? resolve(coreArg) : resolve(REPO_ROOT, 'Cargo.toml'); const shellPath = shellArg ? resolve(shellArg) : resolve(REPO_ROOT, 'app/src-tauri/Cargo.toml'); +const productPath = productArg + ? resolve(productArg) + : resolve(REPO_ROOT, 'scripts/ci/product-features.txt'); let coreToml; let shellToml; +let productText; try { coreToml = readFileSync(corePath, 'utf8'); shellToml = readFileSync(shellPath, 'utf8'); + productText = readFileSync(productPath, 'utf8'); } catch (err) { - console.error(`Could not read manifests: ${err.message}`); + console.error(`Could not read inputs: ${err.message}`); process.exit(2); } const coreDefaults = parseCoreDefaultFeatures(coreToml); +const coreFeatureNames = parseCoreFeatureNames(coreToml); +const productFeatures = parseProductFeatures(productText); const shell = parseShellForwardedFeatures(shellToml); -// A parser that silently finds nothing would turn this guard into a rubber -// stamp, which is worse than not having it. Treat "no defaults found" as a -// failure of the check itself rather than a pass. -if (coreDefaults.length === 0) { +// Guard the guard. A parser that silently found nothing would turn this into a +// rubber stamp, which is worse than having no check at all — so treat empty +// input as a failure OF THE CHECK (exit 2), distinct from a real drift (exit 1). +// +// `coreDefaults` is deliberately NOT in this list: an empty `default` is a +// legitimate configuration (a core where every gate is opt-in), and assertion 1 +// does not depend on it. That is the whole point of the product-set rewrite. +if (productFeatures.length === 0) { + console.error( + `FAIL: parsed zero product gates from ${productPath}.\n` + + 'Either the file changed shape or the parser is broken — refusing to pass vacuously.' + ); + process.exit(2); +} +if (coreFeatureNames.length === 0) { console.error( - `FAIL: parsed zero default features from ${corePath}.\n` + + `FAIL: parsed zero feature names from ${corePath}.\n` + 'Either the manifest changed shape or the parser is broken — refusing to pass vacuously.' ); process.exit(2); } -const result = diffForwarding({ coreDefaults, shell, allowlist: INTENTIONALLY_NOT_FORWARDED }); -console.log(formatReport(result, { coreDefaults, shell, allowlist: INTENTIONALLY_NOT_FORWARDED })); -process.exit(result.ok ? 0 : 1); +// Assertions 1 + 2. +const product = checkProductForwarding({ productFeatures, coreFeatureNames, shell }); +console.log(formatProductReport(product, { productFeatures, shell })); + +// Assertion 3. Still worth running: it is what catches a gate added to +// `default` (so contributors get it) that nobody remembered to also ship. +const defaults = diffForwarding({ + coreDefaults, + shell, + allowlist: INTENTIONALLY_NOT_FORWARDED, +}); +console.log(''); +console.log(formatReport(defaults, { coreDefaults, shell, allowlist: INTENTIONALLY_NOT_FORWARDED })); + +process.exit(product.ok && defaults.ok ? 0 : 1); diff --git a/scripts/ci/product-features.sh b/scripts/ci/product-features.sh new file mode 100755 index 0000000000..c9b241cad0 --- /dev/null +++ b/scripts/ci/product-features.sh @@ -0,0 +1,33 @@ +#!/usr/bin/env bash +# Print the desktop product's core Cargo gates as a comma-separated list, +# ready to paste into `cargo --features "$(scripts/ci/product-features.sh)"`. +# +# Source of truth: scripts/ci/product-features.txt (one gate per line). +# scripts/ci/check-feature-forwarding.mjs asserts that same file equals the +# list app/src-tauri/Cargo.toml forwards, so the product lanes and the shipped +# app can never diverge. +# +# Why the lanes need this at all: `[features] default` is the CONTRIBUTOR set +# now, not the product set. A lane that relies on default features would stop +# compiling and testing voice, web3, documents, meet, contacts and +# crash-reporting — a silent loss of coverage over code that still ships. +set -euo pipefail + +FILE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/product-features.txt" + +if [[ ! -f "$FILE" ]]; then + echo "product-features.txt not found at $FILE" >&2 + exit 2 +fi + +# Strip comments and blank lines, then join with commas. Refuse to emit an +# empty list: a lane silently running with NO features would look green while +# covering nothing, which is the failure mode this whole guard exists to stop. +LIST="$(sed -e 's/#.*//' -e 's/[[:space:]]//g' "$FILE" | grep -v '^$' | paste -sd, -)" + +if [[ -z "$LIST" ]]; then + echo "product-features.txt parsed to an empty gate list — refusing to emit it" >&2 + exit 2 +fi + +printf '%s\n' "$LIST" diff --git a/scripts/ci/product-features.txt b/scripts/ci/product-features.txt new file mode 100644 index 0000000000..caafe215d8 --- /dev/null +++ b/scripts/ci/product-features.txt @@ -0,0 +1,85 @@ +# DESKTOP_PRODUCT_FEATURES — the core Cargo gates the shipped desktop app has. +# +# This file is the single source of truth for "what OpenHuman the product is", +# as distinct from `[features] default` in Cargo.toml, which is now only the +# CONTRIBUTOR set (what a bare `cargo check` / rust-analyzer compiles). +# +# Read by: +# * scripts/ci/check-feature-forwarding.mjs — asserts app/src-tauri/Cargo.toml +# forwards exactly this set, and that every name here is a real core gate. +# * the CI product lanes (clippy, unit tests, coverage), via +# scripts/ci/product-features.sh, so they keep compiling and testing the +# code the product actually ships even though `default` no longer does. +# +# Why the split (#4901, #4919): the shell declares `openhuman_core` with +# `default-features = false`, so it never inherited `default` anyway. Before +# this file, the guard worked by diffing the shell's list against `default` — +# which meant SHRINKING `default` made the guard pass vacuously and silently +# re-armed the exact failure that shipped `voice` missing to 56 users. An +# explicit product list cannot pass vacuously: it is asserted equal, in both +# directions, to the list the shell forwards. +# +# Adding a gate here means the product ships it. Removing one means the product +# loses it — a user-visible change, not a build tweak. +# +# Format: one gate per line. `#` comments and blank lines are ignored. + +# Messaging providers (Telegram/Discord/Slack/WhatsApp/iMessage/...), the +# channel runtime, and the webview account bridges. +channels + +# The media_generate_* agent tools. +media + +# Local audio-device access (cpal): voice recording plus the accessibility +# microphone-permission probe. `voice` requires it. +inference + +# STT/TTS, dictation server, reply speech. Shipped MISSING from v0.58.19 to +# v0.61.x (#4901) — the reason this whole guard exists. +voice + +# Wallet / web3 / x402 domains and their agent tools. +web3 + +# Document ingestion and conversion. +documents + +# Saved automation graphs: create/run/schedule + the workflow_builder and +# flow_discovery agents. +flows + +# Meeting join-URL validation, the live STT/LLM/TTS meeting loop, and the +# backend-delegated Meet bot. +meet + +# SKILL.md discovery/parse/install, workflow execution, remote catalogs. +skills + +# The MCP stdio/HTTP server, dynamic Smithery installs, and the write-audit log. +mcp + +# Sentry. Without it a crash in the packaged app is invisible to us. +crash-reporting + +# The /rpc JSON-RPC endpoint and the Socket.IO bridge. The shell reaches the +# in-process core ONLY over http://127.0.0.1:/rpc, so this is mandatory — +# enforced independently by the HTTP_SERVER_COMPILED_IN assert in the shell. +http-server + +# Battery/AC probe. Without it a user who sets `require_ac_power` gets no +# enforcement and battery_floor throttling never fires — silently, because the +# off-state is a valid "on AC" reading. +scheduler-gate + +# The packaged app's only durable log. Without it a support request comes back +# with nothing to attach, and the absence is silent. +file-logging + +# macOS CNContactStore address-book seeding for the people domain. +contacts + +# The managed Node.js toolchain (download/verify/extract/install a pinned +# release) and the JavaScript language surface on top of it. Carries `xz2` and +# its static liblzma C build. +runtime-node diff --git a/scripts/ci/rust-coverage-changed.sh b/scripts/ci/rust-coverage-changed.sh index cdc05736e1..668bb422f3 100755 --- a/scripts/ci/rust-coverage-changed.sh +++ b/scripts/ci/rust-coverage-changed.sh @@ -31,8 +31,24 @@ MAX_CHANGED_FILES="${MAX_CHANGED_FILES:-200}" log() { echo "[ci][rust-cov-changed] $*"; } +# The desktop product's gates. `[features] default` is the CONTRIBUTOR set now +# and deliberately omits voice, web3, documents, meet, contacts, inference and +# crash-reporting — so a coverage run on default features would silently stop +# measuring code that ships, and the diff-coverage gate would pass a PR whose +# changed lines were never compiled. Source of truth: +# scripts/ci/product-features.txt. +PRODUCT_FEATURES="$(bash scripts/ci/product-features.sh)" + llvm_cov() { - bash scripts/ci-cancel-aware.sh cargo llvm-cov "$@" + # `clean` and `report` are cargo-llvm-cov subcommands that take no feature + # selection; passing --features to them is an error. + case "${1:-}" in + clean | report | show-env) + bash scripts/ci-cancel-aware.sh cargo llvm-cov "$@" + return + ;; + esac + bash scripts/ci-cancel-aware.sh cargo llvm-cov --features "${PRODUCT_FEATURES}" "$@" } integration_test_targets() { diff --git a/scripts/lib/feature-forwarding.mjs b/scripts/lib/feature-forwarding.mjs index a29e5dda50..7f13db2b2e 100644 --- a/scripts/lib/feature-forwarding.mjs +++ b/scripts/lib/feature-forwarding.mjs @@ -1,5 +1,21 @@ -// Detects drift between the core crate's default-ON Cargo gates and the gates -// the Tauri shell forwards to its embedded copy of that crate. +// Detects drift between what the desktop product is supposed to ship and the +// gates the Tauri shell forwards to its embedded copy of the core crate. +// +// THREE ASSERTIONS, in order of strength: +// +// 1. The shell forwards EXACTLY `scripts/ci/product-features.txt` — set +// equality, both directions. This is the load-bearing one. +// 2. Every name in that file is a gate the core actually declares. +// 3. Every core `[features] default` gate is forwarded or explicitly +// allow-listed below. Retained from the original guard. +// +// Assertion 3 used to be the whole guard, and it was sound only while `default` +// meant "everything the product ships". It does not any more: `default` is the +// CONTRIBUTOR set (what a bare `cargo check` and rust-analyzer compile) and the +// product set is larger. A subset check against a shrinking list gets weaker +// every time the list shrinks, and would pass vacuously if `default` ever +// reached zero gates — silently re-arming the exact failure described below. +// That is why assertion 1 exists and why it compares an explicit list. // // Why this exists (#4919): the shell declares `openhuman_core` with // `default-features = false`, so it does NOT inherit the core's `default` list. @@ -34,7 +50,7 @@ */ export const INTENTIONALLY_NOT_FORWARDED = { // 'some-gate': 'Reason it must not ship in the desktop build.', - tui: 'Terminal UI subcommand (openhuman tui/chat); the desktop app ships its own Tauri UI and never runs the ratatui terminal front-end.', + tui: 'Terminal UI subcommand (openhuman tui/chat); the desktop app ships its own Tauri UI and never runs the ratatui terminal front-end. NOTE: `tui` is also default-OFF, so it is in NEITHER the contributor nor the product set and no ordinary lane compiles it — the feature-gate-smoke lane checks it explicitly. Any future entry here in the same position needs the same treatment.', medulla: 'Medulla orchestration-backend client; the desktop app is OpenHuman\'s own product and never dials a Medulla backend. Consumed by the Medulla TUI, which embeds this crate directly.', }; @@ -141,6 +157,125 @@ export function parseShellForwardedFeatures(shellToml, depName = 'openhuman_core return { defaultFeatures, features }; } +/** + * Every gate name declared in the core's `[features]` table. + * + * Used to catch a product-feature entry that is a typo, or that names a gate + * someone renamed or deleted. Without this, `DESKTOP_PRODUCT_FEATURES` could + * quietly list a gate that no longer exists: cargo would reject it when the + * shell is built, but this guard runs first and would already have said OK. + */ +export function parseCoreFeatureNames(coreToml) { + const text = stripComments(coreToml); + const header = text.match(/^[ \t]*\[features\][ \t]*$/m); + if (!header) return []; + const rest = text.slice(header.index + header[0].length); + const nextTable = rest.search(/^[ \t]*\[[^[\]]+\][ \t]*$/m); + const section = nextTable === -1 ? rest : rest.slice(0, nextTable); + const names = [...section.matchAll(/^[ \t]*([A-Za-z0-9_-]+)[ \t]*=/gm)].map(m => m[1]); + return names.filter(name => name !== 'default'); +} + +/** + * Parse `scripts/ci/product-features.txt`: one gate per line, `#` comments and + * blank lines ignored. Mirrors `scripts/ci/product-features.sh` exactly — the + * CI lanes build their `--features` list with that shell script while this + * guard asserts against this function, so the two parsers must agree or a lane + * could compile a different set than the one being checked. + */ +export function parseProductFeatures(text) { + return text + .split(/\r?\n/) + .map(line => line.replace(/#.*/, '').trim()) + .filter(line => line.length > 0); +} + +/** + * Assertions 1 and 2: the shell forwards EXACTLY the product set, and every + * product gate is a real core gate. + * + * This is the half of the guard that cannot pass vacuously. `diffForwarding` + * below compares the shell against `[features] default`, which was sound while + * `default` meant "everything the product ships" — but `default` is the + * CONTRIBUTOR set now, and a subset check gets weaker every time that set + * shrinks. Set EQUALITY against an explicit product list has no such property: + * dropping a gate from the shell fails on `missing`, and adding one the + * product never agreed to fails on `unexpected`. + */ +export function checkProductForwarding({ productFeatures, coreFeatureNames, shell }) { + const empty = { missing: [], unexpected: [], unknown: [] }; + if (shell === null) return { ok: false, reason: 'dependency-not-found', ...empty }; + if (shell.defaultFeatures) { + // The shell would inherit `default`, which is now deliberately SMALLER + // than the product set. That is a defect, not the benign "nothing to + // drift" case it used to be. + return { ok: false, reason: 'shell-inherits-defaults', ...empty }; + } + const forwarded = new Set(shell.features); + const product = new Set(productFeatures); + const known = new Set(coreFeatureNames); + // In the product set, absent from the shell → compiled out of the shipped app. + const missing = productFeatures.filter(gate => !forwarded.has(gate)); + // Forwarded by the shell but not in the product set → the product grew a + // gate without anyone editing the file that says what the product is. + const unexpected = shell.features.filter(gate => !product.has(gate)); + // Named in the product set but not declared by the core → typo, or a gate + // renamed/deleted without updating this list. + const unknown = productFeatures.filter(gate => !known.has(gate)); + return { + ok: missing.length === 0 && unexpected.length === 0 && unknown.length === 0, + reason: null, + missing, + unexpected, + unknown, + }; +} + +export function formatProductReport(result, { productFeatures, shell }) { + if (result.reason === 'dependency-not-found') { + return 'FAIL: could not find the `openhuman_core` dependency in the shell manifest.\nThe guard cannot verify forwarding — fix the parser or the manifest.'; + } + if (result.reason === 'shell-inherits-defaults') { + return [ + 'FAIL: the shell no longer sets `default-features = false` on `openhuman_core`.', + 'It would inherit `[features] default`, which is the CONTRIBUTOR set and is', + 'deliberately smaller than the product — voice, web3, documents, meet, contacts', + 'and crash-reporting would vanish from the shipped app.', + ].join('\n'); + } + const lines = [ + `Product gates (${productFeatures.length}, scripts/ci/product-features.txt): ${productFeatures.join(', ') || '(none)'}`, + `Shell forwards (${shell.features.length}): ${shell.features.join(', ') || '(none)'}`, + ]; + if (result.unknown.length > 0) { + lines.push('', 'Product gates that are not declared in the core `[features]` table:'); + for (const gate of result.unknown) lines.push(` - ${gate}`); + lines.push('Either the name is a typo or the gate was renamed/deleted.'); + } + if (result.missing.length > 0) { + lines.push('', 'Product gates NOT forwarded by the desktop shell:'); + for (const gate of result.missing) lines.push(` - ${gate}`); + lines.push( + '', + 'Each of these is compiled OUT of the shipped desktop app, silently.', + 'Add it to the `openhuman_core` features list in app/src-tauri/Cargo.toml.', + 'See #4901 (voice, 56 users) and #4918 (tokenjuice-treesitter).' + ); + } + if (result.unexpected.length > 0) { + lines.push('', 'Gates the shell forwards that the product set does not list:'); + for (const gate of result.unexpected) lines.push(` - ${gate}`); + lines.push( + '', + 'The shipped app would grow a domain that scripts/ci/product-features.txt', + 'does not claim. Either add it there (a product decision — the CI product', + 'lanes will then cover it too) or drop it from the shell.' + ); + } + if (result.ok) lines.push('', 'OK: the shell forwards exactly the product gate set.'); + return lines.join('\n'); +} + /** * Compare the two lists. * diff --git a/scripts/test-rust-with-mock.sh b/scripts/test-rust-with-mock.sh index ebbb0176c7..90c388b4a3 100755 --- a/scripts/test-rust-with-mock.sh +++ b/scripts/test-rust-with-mock.sh @@ -66,8 +66,18 @@ if [ -f "$HOME/.cargo/env" ]; then source "$HOME/.cargo/env" fi +# `pnpm test:rust` is the "does the product still work" runner, so it selects +# the product's gates rather than `[features] default`, which is the smaller +# contributor set. Without them the four `required-features` integration +# targets (json_rpc_e2e, raw_coverage_all, observability_smoke, +# x402_twit_sh_live) are silently SKIPPED and the run still exits 0 — the same +# trap `--features bin-tools` already guards for the `src/bin/` targets. +# Source of truth: scripts/ci/product-features.txt. +PRODUCT_FEATURES="$(bash "$REPO_ROOT/scripts/ci/product-features.sh")" + cargo_test() { - cargo test --manifest-path Cargo.toml --workspace --features bin-tools "$@" + cargo test --manifest-path Cargo.toml --workspace \ + --features "${PRODUCT_FEATURES},bin-tools" "$@" } integration_test_targets() { From a499125676335c358c63f48d8ae151345723158c Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 10 Aug 2026 16:12:21 +0300 Subject: [PATCH 2/2] build(memory): gate the git-backed diff ledger behind `memory-git` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Takes the kernel profile to **2 native builds — the spec target** (G6 in MIGRATION-PLAN, and the goal named in `kernel-floor.limits`' own baseline entry). `git2` with vendored libgit2 leaves and takes `libgit2-sys` **and** `libz-sys` with it; together with the `runtime-node` gate already on main, only `libsqlite3-sys` and `ring` remain, and both are load-bearing. Kernel floor 305/282/4 -> 302/279/2, from 418 names / 6 native when the program started. `memory-git` is default-OFF, product-ON, carrying `dep:git2` plus tinycortex's `git-diff` and `wiki-git`. ## The type carve-out is what makes this gateable `memory::diff::types` compiles in BOTH builds. The always-on subconscious memory profile renders `CrossSourceDiff` and `ChangeKind` into agent prompts, so stubbing them would put two definitions of one serde shape in the tree, free to drift. tinycortex needed the matching split first — its `git-diff` gated the whole `memory::diff` module, so a libgit2-less host could not name the types at all. Its `memory::diff::{types,source}` are ungated now (they reach no `git2` symbol; only `ledger.rs` and `ledger_helpers.rs` do) and the `Ledger`/`DiffEngine` half stays behind the feature. Gitlink bumped to it. ## Stub vs `#[cfg]`, decided per call site Stubbed: the three `ops` entry points always-on code reaches — `auto_snapshot_after_sync` (`memory::sources::sync`), `diff_since_checkpoint` and `create_checkpoint` (the subconscious profile). Those domains stay feature-unaware. They return an `Err` naming the gate, not an empty diff. An empty `CrossSourceDiff` asserts "your world did not change", which the subconscious would act on; the error routes into a path the caller already handles by logging and skipping. `#[cfg]`'d: the registration sites, which want absence. The schema aggregators return empty vecs (`memory_diff` becomes unknown-method and drops off `/schema`) and `MemoryDiffTool` disappears from its single registration site — a registered tool that always errors is worse than an absent one, because the model keeps choosing it and reporting the failure to the user. ## Capability honesty The embedded driver drops `Capability::Diff` from `advertised_capabilities()` and `as_diff()` returns `None`. Both halves move together deliberately: `audit_provider` fails on either alone, which is the check that keeps them from drifting, and callers may trust the advertised set rather than probing every accessor. ## The wiki mirror degrades, it does not fail `content::wiki_git::commit_summaries` is gated at its two call sites rather than stubbed. The git wiki is a derived view — the summary's own content file is written either way — so skipping it loses the mirror, not the summary. `seal.rs`'s `summary_committed` gains a `#[cfg(not(...))]` sibling returning `Ok(())`: accurate rather than lenient, since nothing the caller depended on failed to happen. `tests/memory_artifacts_e2e.rs` opens the ledger with `git2::Repository::open`, so it declares `required-features = ["memory-git"]`. ## Review follow-ups from #5477 also folded in - `scripts/ci/product-features.sh` died with a bare exit 1 and no message on a comments-only file: `grep -v` exits 1 when it selects nothing, and `set -e` aborted inside the command substitution, making the explicit diagnostic unreachable. Fixed, and the helper now takes an optional path so the self-test can drive that path with a fixture rather than mutating the checked-in file. Two regression tests. - Stale figures in AGENTS.md corrected against measurement (contributor 353/3-native, product 540/7-native, kernel baseline 302/279/2). - `docs/library-minimal-recipe.md`'s obsolete single `Default` column replaced with the real Contrib/Product pair rather than annotated as wrong. Co-authored-by: Medulla --- AGENTS.md | 24 ++++-- Cargo.lock | 4 +- Cargo.toml | 37 ++++++++- app/src-tauri/Cargo.toml | 4 + docs/library-minimal-recipe.md | 44 ++++++----- scripts/__tests__/feature-forwarding.test.mjs | 45 ++++++++++- scripts/ci/product-features.sh | 14 +++- scripts/ci/product-features.txt | 4 + scripts/kernel-floor.limits | 34 +++++++- src/core/all_tests.rs | 44 +++++++++++ src/openhuman/memory/diff/mod.rs | 35 +++++++++ src/openhuman/memory/diff/stub.rs | 77 +++++++++++++++++++ src/openhuman/memory/driver/embedded/mod.rs | 29 ++++++- src/openhuman/memory/store/content/mod.rs | 9 ++- src/openhuman/memory/tinycortex/seal.rs | 19 +++++ src/openhuman/memory/tree/ingest.rs | 10 ++- src/openhuman/tools/mod.rs | 1 + src/openhuman/tools/ops.rs | 4 + vendor/tinycortex | 2 +- 19 files changed, 395 insertions(+), 45 deletions(-) create mode 100644 src/openhuman/memory/diff/stub.rs diff --git a/AGENTS.md b/AGENTS.md index 249183e65c..157005ec55 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -355,8 +355,8 @@ Per-domain Cargo features drop whole domains **at compile time** (smaller binary | Set | Where it lives | What it is | | --- | --- | --- | -| **Contributor** | `[features] default` in `Cargo.toml` | What a bare `cargo check`, `cargo test` and rust-analyzer compile. 9 cheap gates. ~356 packages / 5 native builds. | -| **Product** | `scripts/ci/product-features.txt` | What the shipped desktop app has. 15 gates. ~567 packages / 7 native builds. | +| **Contributor** | `[features] default` in `Cargo.toml` | What a bare `cargo check`, `cargo test` and rust-analyzer compile. 9 cheap gates. **353 packages / 3 native builds** (`libsqlite3-sys`, `lzma-sys`, `ring`). | +| **Product** | `scripts/ci/product-features.txt` | What the shipped desktop app has. 16 gates. **540 packages / 7 native builds** (adds `bzip2-sys`, `libgit2-sys`, `libz-sys`, `zstd-sys`). | `default` used to be the product set, which made the inner loop pay for the whole product on every edit — web3's ethers/secp256k1 cohort, `documents`' zstd/bzip2 native builds, the cpal/hound/arboard/enigo/rdev stack behind `voice`+`inference`, `contacts`' macOS objc2 cohort, `crash-reporting`'s sentry tree, `tui`'s ratatui. Those are default-OFF now. **This did not change what ships**: the shell has set `default-features = false` since #1061 and never inherited `default` anyway. @@ -385,17 +385,26 @@ unconditional today (`git2`/vendored-libgit2, `rusqlite`/bundled, and landed that way had a number moved in CI when they did. ```bash -scripts/kernel-floor.sh flows # CI Linux: 312 packages / 285 names / 6 native +scripts/kernel-floor.sh flows # CI Linux: 304 packages / 281 names / 3 native scripts/kernel-floor.sh flows --json scripts/check-kernel-floor.sh # the CI ratchet (Rust Feature-Gate Smoke lane) scripts/dep-sim.py --cut-nothing # calibration: must equal kernel-floor.sh scripts/dep-sim.py --cut arboard,enigo,rdev # project a cohort before doing it ``` -**CI Linux baseline 2026-08-02: 312 packages / 285 unique names / 6 native builds** -(`aws-lc-sys`, `libgit2-sys`, `libsqlite3-sys`, `libz-sys`, `lzma-sys`, `ring`). -On macOS the same target-specific graph currently resolves to 319 packages / 292 -names / 6 native builds; the CI ratchet is intentionally calibrated on Linux. +**CI Linux baseline 2026-08-09: 302 packages / 279 unique names / 2 native +builds** (`libsqlite3-sys`, `ring`). **This is the target** — MIGRATION-PLAN G6 +set 2 native builds as the goal, and the profile is there, down from 418 names +/ 6 native when the program started. The four that left: `aws-lc-sys` (the +tinychannels rustls pin), `lzma-sys` (the `runtime-node` gate), and +`libgit2-sys` + `libz-sys` together (the `memory-git` gate). The macOS graph +resolves a few packages higher because of target-specific edges; the CI ratchet +is intentionally calibrated on Linux. + +Reaching the target does not retire the ratchet — it is what stops the floor +growing back, and an unmeasured floor grows. `libsqlite3-sys` and `ring` are +both load-bearing (the memory store and TLS), so this is the floor, not a +waypoint. Limits live in `scripts/kernel-floor.limits`; the ratchet fails on growth **and** on a shed that was not written back, since an unratcheted improvement grows back unnoticed. @@ -428,6 +437,7 @@ Two columns because there are two sets (see above): **Contrib** is `[features] d | `mcp` | ON | ON | `openhuman::mcp::server` (the `openhuman mcp` stdio/HTTP server), `openhuman::mcp::registry` (dynamic Smithery installs — `mcp_clients` RPC namespace, SQLite, boot spawn, supervisor, OAuth), `openhuman::mcp::audit` (write-audit log), and the static config-declared server set in `openhuman::mcp::config_servers`. ~19 agent tools, ~20k LOC | **none** (see scope note) | | `tui` | OFF | — | `openhuman::tui` — the tabbed ratatui/crossterm CLI UI (Logs, Chat, Config, Settings), auto-opened by bare `openhuman` on interactive non-container hosts and forced with `openhuman tui` (alias `chat`). Runs the core in-process. No controllers, no agent tools. **Intentionally NOT forwarded to the desktop shell** (allowlisted in `check-feature-forwarding.mjs`). | `ratatui`, `crossterm` | | `channels` | ON | ON | `openhuman::channels` (external-messaging providers — Telegram/Discord/Slack/Signal/WhatsApp/iMessage/IRC/… — plus the channel runtime, controllers, host, proactive messaging + inbound dispatch) and the `channels::webview_accounts` / `webview_apis` / `webview_notifications` / `channels::whatsapp_data` webview-bridge domains (incl. the 3 `whatsapp_data_*` agent tools). **Carve-outs `channels::{traits, cli}` stay ungated.** | **28** via `tinychannels/{email,lark}` — the crate itself stays (load-bearing), its two heavy providers do not | +| `memory-git` | OFF | ON | `openhuman::memory::diff` (git-backed snapshots/checkpoints/read markers, the `memory_diff` RPC namespace + agent tool) and the git wiki mirror in `memory::store::content::wiki_git`. **Type carve-out**: `memory::diff::types` compiles in BOTH builds — the always-on subconscious memory profile renders `CrossSourceDiff`/`ChangeKind` into prompts, and tinycortex makes the matching split (its `memory::diff::{types,source}` are ungated, only the `Ledger`/`DiffEngine` half sits behind `git-diff`). Off ⇒ `memory_diff` is unknown-method, the tool is absent, the embedded driver drops `Capability::Diff` **and** `as_diff()` returns `None` in lockstep (`audit_provider` fails on either half alone), and summary nodes are still written to disk but not mirrored into git. | **3**: `git2`, `libgit2-sys`, `libz-sys` — two of the five native C builds in the kernel profile, the largest native shed in the program | | `contacts` | OFF | ON | `memory::people::address_book`'s macOS CNContactStore reader — the address-book seeding path for the people domain. Leaf gate over a **pre-existing** off-state: the module already shipped a non-macOS `imp` stub returning an empty contact list, so the gate only widens that stub's cfg. `read`/`read_with`/`AddressBookError`/`SystemContactsSource` and the whole `people` RPC surface stay compiled in every build; off ⇒ a refresh seeds nothing instead of failing. | **6** on macOS (`objc2`, `objc2-foundation`, `objc2-contacts`, `block2` + 2 transitive). **No-op on Linux/Windows** — never in those graphs, so the kernel-floor ratchet does not move. Verify cross-target: `cargo tree --target aarch64-apple-darwin -e normal -i objc2-contacts --no-default-features` (294 → 288 packages). | | `runtime-node` | OFF | ON | `runtime::node` (download / verify / extract / install a pinned Node.js toolchain), the `runtime::javascript` language slot, `runtime::pool::node`, the `node_exec` / `npm_exec` agent tools, and the `node_runtime` harness-init step. **Facade + stub** — `ShellTool` holds `Option>` and `shell.rs` is kernel, so the module cannot simply vanish; `runtime/node/stub.rs` carries the `NodeBootstrap` type surface while registration sites are leaf-gated. **The generic native-tool dispatcher (`runtime::node::ops` / `runtime::node::types`) is NOT gated** — it backs both the gated `javascript.*` controllers and the ungated `flows` `oh:` `NativeToolBackend`, so native flow tools (`memory_search`, file, shell, …) keep working when the managed Node runtime is off. Off ⇒ `try_cached`/`probe_installed` return `None` and the shell never prepends a managed bin dir, identical to today's `node.enabled = false` path. | **`xz2` + its static liblzma C build.** First gate to remove a NATIVE toolchain build: `lzma-sys` leaves the list, 6 → 5. `tar`/`zip` are NOT shed — shared with `inference` (install_piper), `runtime::python`, and the document tools. | diff --git a/Cargo.lock b/Cargo.lock index c25e84fa65..f8075a69cb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3690,9 +3690,9 @@ checksum = "68ab91017fe16c622486840e4c83c9a37afeff978bd239b5293d61ece587de66" [[package]] name = "libgit2-sys" -version = "0.18.5+1.9.4" +version = "0.18.7+1.9.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "005d6ae6eac1912906073e069f7db60b1fa98e052a68227824afe3e3a1c59ca2" +checksum = "23c7391e4b9f4ffab1a624223cc1d7385ff9a678f490768add717de7ea2f4d89" dependencies = [ "cc", "libc", diff --git a/Cargo.toml b/Cargo.toml index 6282b27fa8..4756da4eb3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -98,6 +98,13 @@ path = "tests/observability_smoke.rs" # The `is_*_event` Sentry filters are all `#[cfg(feature = "crash-reporting")]`. required-features = ["crash-reporting"] +[[test]] +name = "memory_artifacts_e2e" +path = "tests/memory_artifacts_e2e.rs" +# Opens the wiki ledger with `git2::Repository::open` and asserts on +# `content::wiki_git` artifacts — both of which exist only behind this gate. +required-features = ["memory-git"] + [[test]] name = "x402_twit_sh_live" path = "tests/x402_twit_sh_live.rs" @@ -183,12 +190,13 @@ tinyagents = { version = "2.1", features = ["sqlite"] } # aligned to the host pins (=0.40 / 0.21) so one bundled SQLite + one libgit2 # link. The submodule intentionally tracks reviewed upstream main commits; # keep this semver requirement compatible with the vendored crate version. +# `git-diff` and `wiki-git` are NOT here: they are pulled in by the +# `memory-git` gate below, which is where the git2/libgit2-sys/libz-sys cohort +# enters the graph. Everything else tinycortex needs is unconditional. tinycortex = { version = "0.1", features = [ - "git-diff", "obsidian", "persona", "sync", - "wiki-git", ] } # The memory *contract* — value types, the thirteen capability families, the # `MemoryProvider` driver trait, and the null reference driver. A direct path @@ -262,7 +270,7 @@ sha2 = "0.10" # Git-backed change ledger for the memory_diff module: snapshots are commits, # checkpoints are tags, read markers are refs, diffs are git tree diffs. # Vendored libgit2 (no system git dependency on end-user machines). -git2 = { version = "0.21", default-features = false, features = ["vendored-libgit2"] } +git2 = { version = "0.21", default-features = false, features = ["vendored-libgit2"], optional = true } hmac = "0.12" # Archive extraction for the Node.js runtime bootstrap. Unix Node # distributions ship as .tar.xz, Windows as .zip. `xz2` with `static` @@ -524,6 +532,29 @@ default = ["media", "skills", "flows", "mcp", "channels", "medulla", "http-serve # domains construct them — so `pub mod socketio;` is UNGATED and only the # socketioxide/axum-touching bodies are gated. Likewise `inference::http::types` # and `EXTERNAL_OPENAI_COMPAT_PROVIDER` stay compiled for `core::auth`. +# Git-backed memory diff: snapshots as commits, checkpoints as tags, read +# markers as refs, diffs as git tree diffs (`openhuman::memory::diff`), plus the +# git-backed wiki content format in `memory::store::content::wiki_git`. +# Default-OFF, product-ON. +# +# The most expensive gate in the tree by native-build cost: it carries `git2` +# with vendored libgit2, so turning it off drops `git2` + `libgit2-sys` + +# `libz-sys` and takes the kernel profile from 5 native C builds to 3. +# +# TYPE CARVE-OUT (see AGENTS.md): `memory::diff::types` stays compiled in BOTH +# builds. It re-exports tinycortex's `serde`-only diff wire types, which the +# always-on subconscious memory profile renders into prompts; a stub copy would +# be a second definition of one serde shape, free to drift. tinycortex makes the +# same split — its `memory::diff::{types,source}` are ungated, and only the +# git-touching `ledger`/`DiffEngine` half sits behind `git-diff`. +# +# Off-state: the `memory_diff` RPC namespace is unknown-method and absent from +# `/schema`; the `memory_diff` agent tool is absent from the tool list; the +# embedded driver stops advertising `Capability::Diff` and `as_diff()` returns +# `None`; and the three `ops` entry points always-on code calls return a +# build-fact error, so a post-sync snapshot or a subconscious diff is logged and +# skipped rather than silently reported as "nothing changed". +memory-git = ["dep:git2", "tinycortex/git-diff", "tinycortex/wiki-git"] http-server = ["dep:axum", "dep:socketioxide"] # Local audio-device access: the `cpal` capture stack behind voice recording # and the accessibility microphone-permission probe. Default-ON. Slim / diff --git a/app/src-tauri/Cargo.toml b/app/src-tauri/Cargo.toml index 5991004da5..d072f869aa 100644 --- a/app/src-tauri/Cargo.toml +++ b/app/src-tauri/Cargo.toml @@ -174,6 +174,10 @@ openhuman_core = { path = "../..", package = "openhuman", default-features = fal "file-logging", "contacts", "runtime-node", + # Without this the memory_diff RPC namespace is unknown-method in the + # shipped app, the memory_diff agent tool is absent, and the embedded + # driver stops advertising Capability::Diff — the product keeps all three. + "memory-git", ] } tinyjuice = { version = "0.2.1", default-features = false } diff --git a/docs/library-minimal-recipe.md b/docs/library-minimal-recipe.md index 10f622b2da..df1db1094c 100644 --- a/docs/library-minimal-recipe.md +++ b/docs/library-minimal-recipe.md @@ -37,27 +37,29 @@ There is **no** `library-minimal` meta-feature in `Cargo.toml`, on purpose — s ## Keep / drop table -`default = ["voice","web3","media","meet","skills","flows","mcp","desktop-automation","tui"]` - -> **Superseded — that is the `default` list as it stood when this session ran.** -> `desktop-automation` no longer exists, and `default` is the *contributor* set -> now rather than the product set: `voice`, `web3`, `meet` and `tui` are already -> OFF there (see AGENTS.md "Compile-time domain gates"). The Decision column -> below still records what a library host wants; the Default column no longer -> describes reality. What the product ships lives in -> `scripts/ci/product-features.txt`. - -| Gate | Default | Decision | Why | Deps shed | -| --- | :---: | :---: | --- | --- | -| `skills` | ON | **KEEP** | python/js `SKILL.md` execution is a stated opencompany use case | none (surface/prompt/startup only) | -| `flows` | ON | **KEEP** | saved-workflow (`flows_create`+`flows_run`) runs are a stated use case | — (adds `tinyflows`, `jaq-*`, `rhai`; see cost note) | -| `voice` | ON | **DROP** | STT/TTS/dictation/podcast — a headless host does no audio I/O | `hound`, `lettre` | -| `web3` | ON | **DROP** | crypto wallet / swap / x402 machine payments — not an opencompany path | `bitcoin`, `curve25519-dalek` | -| `media` | ON | **DROP** | `media_generate_*` image/video tools — surface-only | none (backend-proxied) | -| `meet` | ON | **DROP** | Google-Meet join/live-STT/TTS bot — no headless use | none | -| `mcp` | ON | **DROP** | MCP stdio/HTTP server + Smithery registry (~20k LOC, ~19 tools) — a library host is not an MCP host | none (hand-rolled over tokio/reqwest/axum) | -| `desktop-automation` | ON | **DROP** | AX / `computer` tool family drives a **local desktop UI** — meaningless headless | `uiautomation` | -| `tui` | ON | **DROP** | `openhuman tui`/`chat` terminal UI — no terminal in a library host | `ratatui`, `crossterm`, `unicode-width` | +The single `default` list this session was written against no longer exists. +There are two sets now (AGENTS.md, "Compile-time domain gates"): **Contrib** is +`[features] default`, what a bare `cargo check` compiles; **Product** is +`scripts/ci/product-features.txt`, what the desktop app ships. Both columns +below are current. `desktop-automation` has since been removed from the tree +altogether, hence the dashes; `tui` is in neither set. + +Note how much of this recipe the contributor set already gives you for free — +`voice`, `web3`, `meet` and `tui` are default-OFF today. The Decision column +still records what a **library host** wants, which is the thing this document +is actually for. + +| Gate | Contrib | Product | Decision | Why | Deps shed | +| --- | :---: | :---: | :---: | --- | --- | +| `skills` | ON | ON | **KEEP** | python/js `SKILL.md` execution is a stated opencompany use case | none (surface/prompt/startup only) | +| `flows` | ON | ON | **KEEP** | saved-workflow (`flows_create`+`flows_run`) runs are a stated use case | — (adds `tinyflows`, `jaq-*`, `rhai`; see cost note) | +| `voice` | OFF | ON | **DROP** | STT/TTS/dictation/podcast — a headless host does no audio I/O | `hound`, `lettre` | +| `web3` | OFF | ON | **DROP** | crypto wallet / swap / x402 machine payments — not an opencompany path | `bitcoin`, `curve25519-dalek` | +| `media` | ON | ON | **DROP** | `media_generate_*` image/video tools — surface-only | none (backend-proxied) | +| `meet` | OFF | ON | **DROP** | Google-Meet join/live-STT/TTS bot — no headless use | none | +| `mcp` | ON | ON | **DROP** | MCP stdio/HTTP server + Smithery registry (~20k LOC, ~19 tools) — a library host is not an MCP host | none (hand-rolled over tokio/reqwest/axum) | +| `desktop-automation` | — | — | **DROP** | AX / `computer` tool family drives a **local desktop UI** — meaningless headless | `uiautomation` | +| `tui` | OFF | — | **DROP** | `openhuman tui`/`chat` terminal UI — no terminal in a library host | `ratatui`, `crossterm`, `unicode-width` | **Non-default optional features** (`sandbox-landlock`, `sandbox-bubblewrap`, `peripheral-rpi`, `browser-native`/`fantoccini`, `landlock`, `whatsapp-web`, diff --git a/scripts/__tests__/feature-forwarding.test.mjs b/scripts/__tests__/feature-forwarding.test.mjs index ec62546e69..ef67de6bde 100644 --- a/scripts/__tests__/feature-forwarding.test.mjs +++ b/scripts/__tests__/feature-forwarding.test.mjs @@ -1,7 +1,8 @@ import assert from 'node:assert/strict'; -import { execFileSync } from 'node:child_process'; -import { readFileSync } from 'node:fs'; -import { dirname, resolve } from 'node:path'; +import { execFileSync, spawnSync } from 'node:child_process'; +import { readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join, resolve } from 'node:path'; import { test } from 'node:test'; import { fileURLToPath } from 'node:url'; @@ -263,6 +264,44 @@ test('the real product file and the real shell list are equal', () => { assert.deepEqual(result.unknown, [], 'product gates that are not real core gates'); }); +test('the shell helper reports an empty gate list instead of dying silently', () => { + // Regression. The helper filters comments with `grep -v`, which exits 1 when + // it selects nothing; under `set -e` that aborted the script INSIDE the + // command substitution, so a comments-only file exited 1 with no output at + // all and the explicit diagnostic below it was unreachable. A CI lane would + // have seen a bare failure with nothing naming the cause. + const tmp = join(tmpdir(), `product-features-empty-${process.pid}.txt`); + writeFileSync(tmp, '# only a comment\n\n \n'); + try { + const result = spawnSync( + 'bash', + [resolve(REPO_ROOT, 'scripts/ci/product-features.sh'), tmp], + { encoding: 'utf8' } + ); + assert.equal(result.status, 2, 'an empty gate list must exit 2, not 1'); + assert.match(result.stderr, /empty gate list/); + assert.equal(result.stdout.trim(), '', 'nothing may be emitted for an empty list'); + } finally { + rmSync(tmp, { force: true }); + } +}); + +test('the shell helper parses a fixture the same way the JS parser does', () => { + const tmp = join(tmpdir(), `product-features-fixture-${process.pid}.txt`); + writeFileSync(tmp, '# heading\n\nvoice\n media # trailing comment\n\nweb3\n'); + try { + const out = execFileSync( + 'bash', + [resolve(REPO_ROOT, 'scripts/ci/product-features.sh'), tmp], + { encoding: 'utf8' } + ).trim(); + assert.equal(out, 'voice,media,web3'); + assert.deepEqual(out.split(','), parseProductFeatures(readFileSync(tmp, 'utf8'))); + } finally { + rmSync(tmp, { force: true }); + } +}); + test('the shell script and the JS parser agree on the product set', () => { // Two parsers read scripts/ci/product-features.txt: this one, and the shell // helper the CI lanes use to build `--features`. If they disagreed, CI would diff --git a/scripts/ci/product-features.sh b/scripts/ci/product-features.sh index c9b241cad0..31fb77f3b1 100755 --- a/scripts/ci/product-features.sh +++ b/scripts/ci/product-features.sh @@ -13,7 +13,10 @@ # crash-reporting — a silent loss of coverage over code that still ships. set -euo pipefail -FILE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/product-features.txt" +# Optional argument: an alternative gate file. CI always uses the default; the +# override exists so the self-test can drive the empty-list path with a fixture +# instead of mutating the checked-in file. +FILE="${1:-$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/product-features.txt}" if [[ ! -f "$FILE" ]]; then echo "product-features.txt not found at $FILE" >&2 @@ -23,7 +26,14 @@ fi # Strip comments and blank lines, then join with commas. Refuse to emit an # empty list: a lane silently running with NO features would look green while # covering nothing, which is the failure mode this whole guard exists to stop. -LIST="$(sed -e 's/#.*//' -e 's/[[:space:]]//g' "$FILE" | grep -v '^$' | paste -sd, -)" +# +# `|| true` on the grep is load-bearing, not defensive noise. grep exits 1 when +# it selects no lines, and under `set -e` that aborts the script *inside* the +# command substitution — so on a comments-only file this died with a bare +# exit 1 and no message, and the diagnostic below was unreachable. The whole +# point of that diagnostic is to name the failure; swallowing grep's status is +# what lets it run. +LIST="$(sed -e 's/#.*//' -e 's/[[:space:]]//g' "$FILE" | { grep -v '^$' || true; } | paste -sd, -)" if [[ -z "$LIST" ]]; then echo "product-features.txt parsed to an empty gate list — refusing to emit it" >&2 diff --git a/scripts/ci/product-features.txt b/scripts/ci/product-features.txt index caafe215d8..03fc5dad3e 100644 --- a/scripts/ci/product-features.txt +++ b/scripts/ci/product-features.txt @@ -83,3 +83,7 @@ contacts # release) and the JavaScript language surface on top of it. Carries `xz2` and # its static liblzma C build. runtime-node + +# Git-backed memory diff (snapshots/checkpoints/read markers) and the git wiki +# mirror of summary nodes. Carries git2 + vendored libgit2. +memory-git diff --git a/scripts/kernel-floor.limits b/scripts/kernel-floor.limits index 0010a4cd1f..025c8b7cf4 100644 --- a/scripts/kernel-floor.limits +++ b/scripts/kernel-floor.limits @@ -13,6 +13,38 @@ # Simulate with: scripts/dep-sim.py --cut # # History +# 302/279/2 2026-08-09 `memory-git` gate (-3 packages / -3 names / -2 NATIVE), +# measured on top of the runtime-node entry below. +# +# *** THE SPEC TARGET IS REACHED. *** Two native builds +# remain — `libsqlite3-sys` and `ring` — which is exactly +# the goal set in docs/plans MIGRATION-PLAN G6 and named +# at the bottom of this file's 2026-08-01 baseline entry. +# From 418 names / 6 native at the start of the program. +# +# The largest native-build shed in the program: `git2` +# with vendored libgit2 leaves the kernel profile, taking +# `libgit2-sys` AND `libz-sys` with it. Two of the five +# remaining native C builds, gone in one gate. +# Required a tinycortex change first: `git-diff` gated the +# WHOLE `memory::diff` module, so a libgit2-less host +# could not even name a `CrossSourceDiff`. Its `types` +# and `source` submodules are serde/std-only and are now +# ungated there; only `ledger`/`ledger_helpers` (the two +# that touch git2), the impls written against `Ledger`, +# and `DiffEngine` sit behind the feature. OpenHuman +# mirrors the split: `memory::diff::types` compiles in +# both directions because the always-on subconscious +# profile renders those types into prompts. +# Off-state: `memory_diff` is unknown-method, the +# `memory_diff` agent tool is absent, the embedded driver +# drops `Capability::Diff` (and `as_diff()` returns +# `None`, in lockstep — `audit_provider` fails on either +# half alone), and the git wiki mirror of summary nodes is +# skipped while the summaries themselves are still +# written. Default (full-feature) profile moved 356 -> 353. +# Verified with `scripts/assert-shed.sh flows git2 +# libgit2-sys libz-sys`. # 305/282/4 2026-08-09 runtime-node gate, measured on top of the # upstream/main merge below (307/284/5 -> 305/282/4). # `xz2` + its static liblzma C build are exclusive to @@ -158,4 +190,4 @@ # Native: aws-lc-sys libgit2-sys libsqlite3-sys libz-sys # lzma-sys ring. Target after gating is 222 names / 2 native # (libsqlite3-sys, ring) — see docs/plans MIGRATION-PLAN G6. -flows:305:282:4 +flows:302:279:2 diff --git a/src/core/all_tests.rs b/src/core/all_tests.rs index 66aa22d978..e6c00e0fc5 100644 --- a/src/core/all_tests.rs +++ b/src/core/all_tests.rs @@ -2443,3 +2443,47 @@ fn javascript_controllers_absent_when_feature_off() { "runtime-node OFF must not register the `javascript` namespace" ); } + +// ---- memory-git gate ------------------------------------------------------- + +/// `memory-git` ON: the git-backed diff surface is registered. +#[cfg(feature = "memory-git")] +#[test] +fn memory_diff_controllers_registered_when_feature_on() { + let namespaces: Vec<&str> = all_controller_schemas() + .iter() + .map(|s| s.namespace) + .collect(); + assert!( + namespaces.contains(&"memory_diff"), + "with the `memory-git` feature ON the `memory_diff` controllers must be registered" + ); +} + +/// `memory-git` OFF: `memory_diff` leaves no trace in the registry, while the +/// rest of the memory surface stays. +/// +/// This is the half that proves the gate does something. The stub's schema +/// aggregators return empty vecs rather than always-erroring handlers, so the +/// namespace must be genuinely unknown-method — not present-but-broken, which +/// would still advertise itself on `/schema`. +/// +/// `memory` is asserted present in the same test on purpose: the gate is +/// supposed to remove the git ledger, not the memory domain. Splitting that +/// into a separate test would let one pass while the other silently regressed. +#[cfg(not(feature = "memory-git"))] +#[test] +fn memory_diff_controllers_absent_when_feature_off() { + let namespaces: Vec<&str> = all_controller_schemas() + .iter() + .map(|s| s.namespace) + .collect(); + assert!( + !namespaces.contains(&"memory_diff"), + "with `memory-git` OFF the `memory_diff` controllers must not be registered, got: {namespaces:?}" + ); + assert!( + namespaces.contains(&"memory"), + "the `memory-git` gate must remove the git ledger, not the memory domain" + ); +} diff --git a/src/openhuman/memory/diff/mod.rs b/src/openhuman/memory/diff/mod.rs index b2ba908d3b..a99565ad97 100644 --- a/src/openhuman/memory/diff/mod.rs +++ b/src/openhuman/memory/diff/mod.rs @@ -26,17 +26,52 @@ //! - Named checkpoints for cross-source "what changed since X" queries //! - Agent tool for in-conversation diff queries +//! ## The `memory-git` gate +//! +//! All of the above needs a git ledger, and libgit2 is one of the two most +//! expensive native builds left in the graph — so the behaviour sits behind +//! `memory-git` (default-OFF, product-ON), which also carries `git2` and +//! tinycortex's `git-diff`/`wiki-git`. Off, it sheds `git2` + `libgit2-sys` + +//! `libz-sys`, taking the kernel profile from 5 native builds to 3. +//! +//! **`types` stays ungated**, mirroring the carve-out on the tinycortex side: +//! it re-exports `serde`-only wire types that always-on callers name. The +//! subconscious memory profile renders `CrossSourceDiff` and `ChangeKind` into +//! prompts, and duplicating those in a stub would be two definitions of one +//! serde shape, free to drift apart. +//! +//! The three `ops` entry points always-on code calls are stubbed rather than +//! `#[cfg]`'d at each call site, so `memory::sources::sync` and the +//! subconscious profile need no feature awareness — a diff simply never +//! materialises. Registration sites get the opposite treatment: the schema +//! aggregators return empty vecs (the controllers become unknown-method) and +//! `MemoryDiffTool` is `#[cfg]`'d out at its one registration site in +//! `tools/ops.rs`, because a registered tool that always errors is worse than +//! an absent one — the model would keep choosing it and reporting the failure. + +#[cfg(feature = "memory-git")] pub mod ops; +#[cfg(feature = "memory-git")] pub mod rpc; +#[cfg(feature = "memory-git")] pub mod schemas; +#[cfg(feature = "memory-git")] pub mod source; +#[cfg(feature = "memory-git")] pub mod tools; pub mod types; +#[cfg(not(feature = "memory-git"))] +mod stub; +#[cfg(not(feature = "memory-git"))] +pub use stub::{all_memory_diff_controller_schemas, all_memory_diff_registered_controllers, ops}; + +#[cfg(feature = "memory-git")] pub use schemas::{ all_controller_schemas as all_memory_diff_controller_schemas, all_registered_controllers as all_memory_diff_registered_controllers, }; +#[cfg(feature = "memory-git")] pub use tools::MemoryDiffTool; pub use types::{ ChangeKind, Checkpoint, CrossSourceDiff, DiffResult, DiffSummary, ItemChange, Snapshot, diff --git a/src/openhuman/memory/diff/stub.rs b/src/openhuman/memory/diff/stub.rs new file mode 100644 index 0000000000..ed0e1428e2 --- /dev/null +++ b/src/openhuman/memory/diff/stub.rs @@ -0,0 +1,77 @@ +//! The `memory-git`-disabled surface of `memory::diff`. +//! +//! Mirrors **functions only**. The wire types stay in [`super::types`] and are +//! compiled in both directions, so — unlike the `voice` stub, which had to +//! re-declare types living inside its gated tree — there is zero type +//! duplication here and nothing that can drift. +//! +//! Only the three entry points that always-on code reaches are mirrored: +//! +//! | Caller | Function | +//! | --- | --- | +//! | `memory::sources::sync` | `auto_snapshot_after_sync` | +//! | `subconscious::profiles::memory` | `diff_since_checkpoint`, `create_checkpoint` | +//! +//! Everything else in the real `ops` is reached only from inside this module's +//! own gated files, so it needs no mirror. If you add a cross-domain caller, +//! add its function here rather than `#[cfg]`-ing the call site — keeping +//! feature awareness out of always-on domains is the whole point of the stub. +//! +//! **These return `Err`, not `Ok`-with-empty.** An empty `CrossSourceDiff` +//! would say "your world did not change", which the subconscious profile would +//! faithfully act on; an error says "this build cannot tell you", which it +//! already knows how to log and skip. Failing closed matters more than being +//! quiet: the caller in `profiles/memory.rs` logs and moves on. + +use crate::openhuman::config::Config; +use crate::openhuman::memory::sources::types::MemorySourceEntry; + +use super::types::{Checkpoint, CrossSourceDiff, Snapshot}; + +/// The message every disabled entry point returns. +/// +/// Names the feature, because the reader is a developer looking at a log line +/// from a slim build and the actionable fact is which gate to turn on. +const DISABLED: &str = "memory diff is disabled at compile time (built without the `memory-git` \ + feature); rebuild with `--features memory-git` for git-backed snapshots, \ + checkpoints and diffs"; + +/// Function mirrors of the real [`super::ops`]. +pub mod ops { + use super::*; + + /// See [`super::super::ops::auto_snapshot_after_sync`]. + pub async fn auto_snapshot_after_sync( + _source: &MemorySourceEntry, + _config: &Config, + ) -> Result { + Err(DISABLED.to_string()) + } + + /// See [`super::super::ops::create_checkpoint`]. + pub async fn create_checkpoint(_label: &str, _config: &Config) -> Result { + Err(DISABLED.to_string()) + } + + /// See [`super::super::ops::diff_since_checkpoint`]. + pub async fn diff_since_checkpoint( + _checkpoint_id: &str, + _config: &Config, + _include_text_diff: bool, + ) -> Result { + Err(DISABLED.to_string()) + } +} + +/// No controllers: the `memory_diff` namespace answers unknown-method. +/// +/// Empty rather than a set of always-erroring handlers, so `/schema` does not +/// advertise a surface this build cannot serve. +pub fn all_memory_diff_controller_schemas() -> Vec { + Vec::new() +} + +/// No controllers to register. See [`all_memory_diff_controller_schemas`]. +pub fn all_memory_diff_registered_controllers() -> Vec { + Vec::new() +} diff --git a/src/openhuman/memory/driver/embedded/mod.rs b/src/openhuman/memory/driver/embedded/mod.rs index 068aab661d..7b8476acfe 100644 --- a/src/openhuman/memory/driver/embedded/mod.rs +++ b/src/openhuman/memory/driver/embedded/mod.rs @@ -41,6 +41,7 @@ //! becomes safe to gate on (M4). mod core_family; +#[cfg(feature = "memory-git")] mod diff; mod documents; mod entities; @@ -59,6 +60,8 @@ use std::sync::Arc; use async_trait::async_trait; use tinycortex_api::capabilities::Capabilities; +#[cfg(not(feature = "memory-git"))] +use tinycortex_api::capabilities::Capability; use tinycortex_api::error::MemoryError; use tinycortex_api::health::MemoryHealth; use tinycortex_api::provider::MemoryProvider; @@ -86,7 +89,19 @@ pub const EMBEDDED_DRIVER_ID: &str = "tinycortex"; /// family added to the contract widens `all()` here and fails /// `audit_provider` until its accessor lands, which is the intended pressure. fn advertised_capabilities() -> Capabilities { - Capabilities::all() + // Without `memory-git` there is no git ledger, so the diff family has no + // implementation to reach. Dropping it here is not cosmetic: a provider + // that advertises a capability whose accessor returns `None` fails + // `audit_provider`, and callers are entitled to trust the advertised set + // rather than probing every accessor. + #[cfg(not(feature = "memory-git"))] + { + Capabilities::all().without(Capability::Diff) + } + #[cfg(feature = "memory-git")] + { + Capabilities::all() + } } /// The in-process tinycortex driver for one workspace. @@ -289,8 +304,18 @@ impl MemoryProvider for EmbeddedMemoryProvider { Some(self) } + /// `None` without `memory-git`, in lockstep with + /// [`advertised_capabilities`] — `audit_provider` fails on either half + /// alone, which is exactly the check that keeps these two from drifting. fn as_diff(&self) -> Option<&dyn tinycortex_api::provider::MemoryDiff> { - Some(self) + #[cfg(not(feature = "memory-git"))] + { + None + } + #[cfg(feature = "memory-git")] + { + Some(self) + } } fn as_goals(&self) -> Option<&dyn tinycortex_api::provider::MemoryGoals> { diff --git a/src/openhuman/memory/store/content/mod.rs b/src/openhuman/memory/store/content/mod.rs index eb17623422..6a62c3dfb0 100644 --- a/src/openhuman/memory/store/content/mod.rs +++ b/src/openhuman/memory/store/content/mod.rs @@ -18,9 +18,14 @@ pub mod read; pub mod tags; pub use tinycortex::memory::chunks::StagedChunk; +/// The git-backed wiki content format. Re-exported only when `memory-git` is +/// on: it lives behind tinycortex's `wiki-git` feature, which the gate carries +/// along with `git-diff` and the libgit2 cohort. +#[cfg(feature = "memory-git")] +pub use tinycortex::memory::store::content::wiki_git; pub use tinycortex::memory::store::content::{ - atomic, compose, obsidian, obsidian_registry, paths, raw, stage_chunks, wiki_git, - StagedSummary, SummaryComposeInput, SummaryTreeKind, + atomic, compose, obsidian, obsidian_registry, paths, raw, stage_chunks, StagedSummary, + SummaryComposeInput, SummaryTreeKind, }; /// Update the `tags:` block in a summary's on-disk `.md` file after an diff --git a/src/openhuman/memory/tinycortex/seal.rs b/src/openhuman/memory/tinycortex/seal.rs index 91dba83a7f..b6125c24ae 100644 --- a/src/openhuman/memory/tinycortex/seal.rs +++ b/src/openhuman/memory/tinycortex/seal.rs @@ -7,6 +7,7 @@ use chrono::Duration; use crate::core::bus::BUS; use crate::core::events::DomainEvent; use crate::openhuman::config::Config; +#[cfg(feature = "memory-git")] use crate::openhuman::memory::store::content::wiki_git::{SummaryCommitBatch, SummaryCommitEntry}; use crate::openhuman::memory::store::trees::types::{Buffer, SummaryNode, Tree}; use crate::openhuman::memory::tree::score::embed::{ @@ -65,6 +66,24 @@ impl tinycortex::memory::tree::SealObserver for Observer<'_> { }); } + /// Record a sealed summary in the git wiki mirror. + /// + /// A no-op without `memory-git`: the summary's own content file is written + /// by the caller either way, and this only mirrors it into the git ledger. + /// Returning `Ok(())` is therefore accurate rather than lenient — nothing + /// the caller depends on failed to happen. + #[cfg(not(feature = "memory-git"))] + fn summary_committed( + &self, + _tree: &Tree, + _node: &SummaryNode, + _content_path: &str, + _reason: &str, + ) -> Result<()> { + Ok(()) + } + + #[cfg(feature = "memory-git")] fn summary_committed( &self, tree: &Tree, diff --git a/src/openhuman/memory/tree/ingest.rs b/src/openhuman/memory/tree/ingest.rs index ede41f7f6d..60a117663f 100644 --- a/src/openhuman/memory/tree/ingest.rs +++ b/src/openhuman/memory/tree/ingest.rs @@ -1,8 +1,11 @@ //! Product artifact hooks around tinycortex-owned direct summary ingestion. -use anyhow::{Context, Result}; +#[cfg(feature = "memory-git")] +use anyhow::Context; +use anyhow::Result; use crate::openhuman::config::Config; +#[cfg(feature = "memory-git")] use crate::openhuman::memory::store::content::wiki_git::{SummaryCommitBatch, SummaryCommitEntry}; use crate::openhuman::memory::store::trees::types::Tree; use crate::openhuman::memory::tinycortex::{memory_config_from, HostSummariser}; @@ -34,6 +37,11 @@ pub async fn ingest_summary( ) .await?; + // The git wiki mirror is a DERIVED view: `ingest_summary` above has already + // written the summary to disk, and this only records it in the git-backed + // mirror. Skipping it when `memory-git` is off loses the mirror, not the + // summary — so the call site is gated rather than stubbed. + #[cfg(feature = "memory-git")] crate::openhuman::memory::store::content::wiki_git::commit_summaries( &content_root, &SummaryCommitBatch { diff --git a/src/openhuman/tools/mod.rs b/src/openhuman/tools/mod.rs index 7f92fd1e50..9e438c8979 100644 --- a/src/openhuman/tools/mod.rs +++ b/src/openhuman/tools/mod.rs @@ -44,6 +44,7 @@ pub use crate::openhuman::integrations::tools::*; #[cfg(feature = "mcp")] pub use crate::openhuman::mcp::registry::tools::*; pub use crate::openhuman::memory::agent::tools::*; +#[cfg(feature = "memory-git")] pub use crate::openhuman::memory::diff::tools::*; pub use crate::openhuman::memory::goals::tools::*; pub use crate::openhuman::memory::people::tools::*; diff --git a/src/openhuman/tools/ops.rs b/src/openhuman/tools/ops.rs index 087f77452c..542b8708be 100644 --- a/src/openhuman/tools/ops.rs +++ b/src/openhuman/tools/ops.rs @@ -782,6 +782,10 @@ pub fn all_tools_with_runtime( // Memory diff — structured "what changed in the agent's world since a // checkpoint/last sync". Drives the subconscious tick's first stage and is // available to any agent that lists it. Unit struct, no runtime deps. + // Absent rather than erroring when `memory-git` is off: a registered tool + // that always fails is worse than no tool, because the model keeps + // choosing it and reporting the failure back to the user. + #[cfg(feature = "memory-git")] tools.push(Box::new(crate::openhuman::memory::diff::MemoryDiffTool)); // Subconscious user-facing handoff — notify_user proactive delivery. diff --git a/vendor/tinycortex b/vendor/tinycortex index ce98837b50..be7b395354 160000 --- a/vendor/tinycortex +++ b/vendor/tinycortex @@ -1 +1 @@ -Subproject commit ce98837b50178ec7db23571064360f0258a2d429 +Subproject commit be7b395354271082953d2594765aded73975b54c