feat(core): shed embedded deps behind inference/documents/crash-reporting/http-server gates (#5048)#5068
Merged
senamakel merged 13 commits intoJul 22, 2026
Conversation
…main
Remove dependencies and a domain that have zero references in the tree
(compile time, binary size, and supply-chain surface for no benefit).
Dependencies (root Cargo.toml), each verified with `cargo tree -i`:
- Real sheds (nothing else pulls them): postgres, prometheus, dialoguer,
shellexpand, clap_complete, opentelemetry{,_sdk,-otlp}.
- Redundant direct declarations (the crate stays in-tree transitively via
ethers-signers / tinychannels / tinyjuice / pdf-extract; only the direct
declaration is dropped): ripemd, prost, mail-parser, async-imap,
unicode-{segmentation,width,normalization}.
- 4 tree-sitter optional decls that no feature activates (tinyjuice owns
its own grammars; tokenjuice-treesitter behaviour is unchanged).
Domain:
- redirect_links (RPC-registered but never wired into the message
pipeline): drop the module plus its controller registration, namespace
description, security-policy internal-dir entry, and test-inventory
allowlist entry.
npm:
- @rive-app/react-canvas (root; the app uses @rive-app/react-webgl2),
remotion, @remotion/player, @remotion/zod-types (app). Lockfile pruned.
webview_accounts is intentionally NOT removed: contrary to the issue's
claim, the core module is a live cross-crate API (WechatScanPayload,
list_ingest_payload, ...) consumed by app/src-tauri/src/wechat_scanner,
so deleting it breaks the desktop build. memory_archivist is likewise
kept: its 44 LOC hold real Config->MemoryConfig adapter logic, so
inlining would duplicate that across call sites.
Verified: core default, core --no-default-features, and Tauri shell
cargo check; frontend tsc; and the test-inventory guard all pass.
Closes tinyhumansai#5051
…ff Linux/macOS Two RAM-footprint fixes toward the embedded/headless budget (tinyhumansai#5046), both independent of any feature gate: - reqwest: the base declaration enabled rustls-tls AND native-tls together, so every build linked two TLS backends and two root-cert sets. native-tls is only ever exercised on Windows (tls::tls_client_builder() calls .use_native_tls() under cfg(windows) for the SChannel/OS cert store); macOS and Linux use rustls. Move native-tls into [target.'cfg(windows)'.dependencies] so it coexists with rustls only on Windows — Linux/macOS, including the headless embedding target, now link a single TLS stack + cert set. - tokio blocking pool: the long-lived multi-thread runtimes that host agent turns set thread_stack_size(AGENT_WORKER_STACK_BYTES = 16 MiB), which sizes blocking threads too, yet left max_blocking_threads at tokio's 512 default — up to 512 x 16 MiB of potential idle stack. Add MAX_BLOCKING_THREADS = 64 next to the stack constant and apply it at the six tuned runtimes (cli.rs, agent_cli.rs, and the desktop Tauri host). 64 leaves ample headroom for concurrent spawn_blocking (SQLite, grep/glob, document parsing, url_guard) while capping the idle footprint; threads still retire after the 10 s idle timeout. Part of tinyhumansai#5048.
…e` feature The `voice` gate deferred the whisper/cpal dependency shed to "a separate inference gate" (Cargo.toml comment, tinyhumansai#4803). This adds it: a default-ON `inference` feature that drops `whisper-rs` (including the macOS `metal` variant and the `whisper-rs-sys` patch, which go inert once the crate leaves the graph) and `cpal` from `--no-default-features` builds — the largest single dependency blocker to the 20-30 MiB embedded RSS budget (tinyhumansai#5046). whisper_engine becomes a facade following the type-carve-out pattern (AGENTS.md): - `whisper_engine/types.rs` keeps the inert `TranscriptionResult` compiled in both build states (it is named by the always-compiled local-AI service and by inference::voice::streaming), so its fields can never drift between builds. - `whisper_engine/real.rs` holds the `whisper-rs` / `WhisperContext` engine, compiled only with `--features inference`. - `whisper_engine/stub.rs` mirrors the function + handle surface exactly with disabled-error / no-op bodies, so the always-compiled callers need no per-call `#[cfg]`. In-process STT returns a disabled error; the local-AI service still reaches Ollama / LM Studio over HTTP. The `cpal` microphone probe in the always-compiled `accessibility::permissions` is gated the same way (feature x OS matrix) with an `inference`-off fallback that reports `Unknown` rather than a misleading Granted/Denied. `voice` now requires `inference`, so the shipped desktop build is byte-identical. Verified: the full default build and the disabled build (`--no-default-features --features tokenjuice-treesitter`) both compile; `cargo tree -i whisper-rs` and `cargo tree -i cpal` return nothing in the disabled graph; the gate is forwarded to the desktop shell (app/src-tauri/Cargo.toml); both-ways tests live in core::all_tests and whisper_engine::stub. Part of tinyhumansai#5048.
…-ON `documents` feature Makes the three office-document crates optional and drops them from `--no-default-features` builds — part of the embedded RSS budget (tinyhumansai#5046; the "additional gates" acceptance line of tinyhumansai#5048). Leaf gate (no stub facade). With `documents` off: - the `generate_presentation` (ppt-rs) and `generate_document` (docx-rs) agent tools are absent from the tool list — the `impl::{presentation,document}` modules and their `tools/ops.rs` registrations are `#[cfg]`-gated; - PDF text extraction during multimodal file ingest degrades the file to a reference instead of extracted text: `extract_pdf_text` returns the disabled error, which the existing caller already routes to `FilePayload::Reference`; - the `artifacts.ai_regenerate` RPC — the one non-tool consumer of `PresentationTool` — keeps its always-compiled handler but delegates the tool path to a `#[cfg]`-gated helper that reports the build limitation when off. The `presentation_agent` builtin still lists `generate_presentation` in its TOML tool scope (data, not code); that unresolved-tool id is tolerated exactly like the `mcp_agent` precedent, so no agent-registry change is needed. Verified: the full default build and the disabled build (`--no-default-features --features tokenjuice-treesitter`) both compile; `cargo tree -i` shows pdf-extract / ppt-rs / docx-rs absent from the disabled graph and present in the full graph; the gate is forwarded to the desktop shell; both-ways tool-presence tests live in tools/ops_tests.rs. Part of tinyhumansai#5048.
…` feature Makes `sentry` optional and drops it from `--no-default-features` builds — part of the embedded RSS budget (tinyhumansai#5046; the "additional gates" line of tinyhumansai#5048). Type-carve-out facade (AGENTS.md), no stub file needed — every always-compiled caller reaches sentry through a fn whose body branches on `#[cfg]`: - observability.rs: the 23 `&sentry::protocol::Event` before-send classifiers (plus the private `all_provider_attempts_are_transient` they call) are gated; the interspersed sentry-free `is_*_message(&str)` string classifiers and `expected_error_kind` stay compiled in both builds (they are consumed by always-on error handling). `report_error_message` / `report_warning_message` keep their signatures — only the `sentry::with_scope`/`capture_message` block is gated, the `tracing::error!/warn!` diagnostic stays in both builds. - main.rs: the `sentry::init` statement + its exclusive helpers (`build_release_tag`, `resolve_environment`, `scrub_secrets`, `SECRET_PATTERNS` and their `once_cell`/`regex` imports) are gated. - logging.rs: `sentry_tracing_layer` becomes a cfg-pair — the real sentry-tracing layer vs a no-op `Identity` layer — so the two `.with(...)` call sites need no `#[cfg]`. - cli.rs: `run_sentry_test_command` is a cfg-pair (real vs disabled-build error); the dispatch arm + help stay compiled (mcp CLI precedent). - credentials/sentry_scope.rs: `bind`/`clear` keep their signatures; only the `configure_scope` body is gated. The desktop build stays byte-identical: `voice`-style forwarding adds `crash-reporting` to the shell's `openhuman_core` features. `sentry` stays a plain `[dev-dependencies]` entry so the ~test transport is available to gated tests; the non-test `--no-default-features` build sheds it entirely. Verified: full + disabled builds compile; `cargo tree -i sentry -e normal,build` shows sentry absent from the disabled non-test graph and present in the full graph; gated both-ways tests across observability / sentry_scope / composio / jsonrpc; forwarded to the desktop shell. Part of tinyhumansai#5048.
…fault-ON `http-server` feature Makes the HTTP/JSON-RPC + Socket.IO server transport optional so a headless / embedded core (tinyhumansai#5046) can drop it — the last of the tinyhumansai#5048 "additional gates". Dep-shed (honest, not overclaimed): - `socketioxide` — shed entirely from the disabled non-test graph. - `tower` — moved from `[dependencies]` to `[dev-dependencies]` (its only non-test use was `ServiceExt::oneshot` in router tests); it remains transitively under axum, so no binary shed, but the direct prod edge is gone. - `axum` — its openhuman surface (~13 files: the core router, the nested inference/agentbox sub-routers, the dictation WebSocket, the auth middleware) is compiled out, but the crate stays linked via `tinychannels` (relay-websocket). Fully removing axum would require gating tinychannels' websocket transport — out of scope here. Facade + type carve-out (AGENTS.md): - `CoreRuntime::serve()` keeps its signature; the axum bind-half is extracted to a gated `serve_http()`, and the `#[cfg(not)]` arm reuses the existing `!rpc_http` background-services return — with http-server off the core runs services and binds no listener. - `core/socketio.rs` is deliberately NOT whole-gated: `WebChannelEvent` + the usage/progress payload structs are consumed by ~10 always-compiled modules (web_chat, cron, channels, agent, agentbox, flows), so only the socketioxide/axum items are gated in place. - `inference/http`: the `EXTERNAL_OPENAI_COMPAT_PROVIDER` const + `types` stay compiled (consumed by always-on `core/auth.rs`); only the router is gated. - Three cross-gate couplings resolved and build-tested in every combo: `mcp` x `http-server` (HTTP split from stdio), `voice` x `http-server` (the dictation-WS signature spans the real impl, the voice facade re-export, and the voice-off stub), and the inference const carve-out. - `text_input`/CLI use disabled-build error arms (mcp CLI precedent); the `openhuman-fleet` bin gains `required-features = ["http-server"]`. New `core::http_server_status::HTTP_SERVER_COMPILED_IN` marker (mirrors `VOICE_COMPILED_IN`/`INFERENCE_COMPILED_IN`); the desktop shell const-asserts it so a mis-forward becomes a build error rather than a listener-less app. Gate forwarded to the shell. Verified: full + disabled builds, plus the `voice`x`http-server` and `mcp`x`http-server` combo builds, all compile; `cargo tree -i socketioxide` shows it absent from the disabled graph; both-ways tests (`http_server_compiled_out_when_feature_off`, `http_host_controllers_absent_when_http_server_off`) pass; gate forwarded + shell compile-assert. Part of tinyhumansai#5048.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis pull request adds feature gates for HTTP transport, inference, documents, and crash reporting; forwards those features to the Tauri shell; introduces disabled-build facades; centralizes secret redaction; and applies shared Tokio blocking-thread limits. ChangesFeature-gated core subsystems
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
Keep dep-gating (inference/documents/crash-reporting/http-server) intact; adopt main's tauri-cef pin bump; Cargo.lock + app/src-tauri/Cargo.lock resynced (cargo metadata --locked passes), pnpm-lock unchanged.
Reconcile dep-gating with main (now incl. tinyhumansai#5052/tinyhumansai#5060/tinyhumansai#5061/tinyhumansai#5062): - Cargo.toml: union gating (optional axum/sentry/ppt-rs + http-server/ inference/documents/crash-reporting features) with main's tower + tui deps and desktop-automation/tui features; default = union of both feature sets. - app/src-tauri: shell forwards crash-reporting/http-server/desktop-automation (feature-forwarding gate passes; tui allowlisted). - all_tests.rs: union of http-server + desktop-automation gate tests. - types.rs: keep main's redirect_links security denylist entry (tinyhumansai#5052). Cargo.lock + app/src-tauri/Cargo.lock resynced (--locked passes); cargo fmt, feature-forwarding gate, and clippy -D warnings all clean locally.
YellowSnnowmann
marked this pull request as ready for review
July 21, 2026 16:28
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3ef93e469
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
coderabbitai
Bot
added
feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/openhuman/tools/ops.rs (1)
786-800: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the stale registration comments.
The preceding comments say both tools are “Always registered,” but these new
#[cfg(feature = "documents")]gates remove them from slim builds. State that they register only whendocumentsis enabled.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/tools/ops.rs` around lines 786 - 800, Update the registration comments for PresentationTool and DocumentTool to state that each tool is registered only when the documents feature is enabled, matching their #[cfg(feature = "documents")] gates; remove the stale “Always registered” wording.src/openhuman/credentials/sentry_scope.rs (1)
33-43: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not log the full account identifier.
Line [43] emits
idverbatim. This is an account-linked user identifier, and the log remains unconditional even when crash reporting is disabled. Log onlyuser_bound = true, or use a truncated/hash representation.As per coding guidelines, changed Rust flows must never log secrets or full PII.
Suggested adjustment
- tracing::debug!(user_id = %id, "[sentry] scope user bound"); + tracing::debug!(user_bound = true, "[sentry] scope user bound");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/credentials/sentry_scope.rs` around lines 33 - 43, Update the tracing::debug! call in the sentry scope flow to stop emitting the full id account identifier; log only a non-sensitive user_bound = true field. Keep the existing sentry::configure_scope behavior and unconditional diagnostic logging intact.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Cargo.toml`:
- Around line 226-242: Move the `tower` dependency from the production
`[dependencies]` section to `[dev-dependencies]`, retaining its existing version
and feature configuration so test-only call sites continue to resolve without
including it in slim builds.
In `@src/core/all_tests.rs`:
- Around line 1130-1144: Update the comments around
http_server_compiled_in_when_feature_on and the disabled-feature test to remove
claims that axum is shed or absent from the dependency graph. Mention only
dependencies actually removed by the feature gate, such as socketioxide, while
preserving the existing compile-time and listener behavior descriptions.
In `@src/core/http_server_status.rs`:
- Around line 21-23: Update the documentation comment near the HTTP/Socket.IO
feature gating to remove the claim that disabling the feature drops axum from
the dependency graph. State only that the gated HTTP/Socket.IO transport and
direct socketioxide dependency are omitted, while acknowledging axum may remain
transitively available through tinychannels.
In `@src/core/observability.rs`:
- Around line 2427-2440: Redact observability messages before logging in the
non-crash-reporting error and warning branches around the existing tracing
calls. Apply the shared message-redaction helper before using message in either
log’s error field or formatted text, including the flow near the warning branch
around lines 2491-2501; do not rely on Sentry’s before_send processing.
In `@src/core/runtime/builder.rs`:
- Around line 404-426: Update the public serve documentation to explicitly
describe the http-server-disabled contract: rpc_http=true returns Ok(()), starts
selected background services, drops ready_tx, and never binds a listener. Extend
the feature matrix to cover both http-server-enabled and disabled behavior, and
add cfg-appropriate tests covering both branches, including service startup,
return status, ready_tx handling, and absence of binding, while maintaining at
least 80% diff coverage.
In `@src/core/shutdown.rs`:
- Around line 59-63: Update the documentation comment above the shutdown handler
to avoid stating that axum is absent from slim builds; describe axum as
unavailable as a direct dependency or API surface there, while preserving the
explanation that an intra-doc link would fail and keeping the axum::serve
graceful-shutdown context.
In `@src/core/socketio.rs`:
- Around line 3-15: The changed cfg gating in src/core/socketio.rs incorrectly
hides SubagentProgressDetail from non-http-server builds. Keep
SubagentProgressDetail and its supporting definitions ungated, and apply the
http-server gate only to the transport-only payloads SocketRpcRequest,
ChatStartPayload, and ChatCancelPayload while preserving their existing
transport gating.
In `@src/openhuman/inference/local/service/whisper_engine/stub.rs`:
- Around line 54-93:
src/openhuman/inference/local/service/whisper_engine/stub.rs lines 54-93: add
grep-friendly debug or warn diagnostics before each disabled return, identifying
the transcription operation and disabled compile-time gate without audio or PII.
In src/openhuman/agent/multimodal.rs lines 1571-1577, log that PDF extraction
was skipped because the documents feature is disabled. In
src/openhuman/artifacts/ops.rs lines 247-260, log that presentation regeneration
was rejected because documents is disabled; preserve the existing error
behavior.
In `@src/openhuman/mcp_server/local.rs`:
- Around line 97-110: Add a disabled-build test alongside the existing tests,
gated with #[cfg(all(test, not(feature = "http-server")))], that calls
ensure_local_http and asserts it returns the expected unavailable-server error
message. Keep the current feature-enabled test module unchanged and verify the
fallback behavior without starting an HTTP server.
---
Outside diff comments:
In `@src/openhuman/credentials/sentry_scope.rs`:
- Around line 33-43: Update the tracing::debug! call in the sentry scope flow to
stop emitting the full id account identifier; log only a non-sensitive
user_bound = true field. Keep the existing sentry::configure_scope behavior and
unconditional diagnostic logging intact.
In `@src/openhuman/tools/ops.rs`:
- Around line 786-800: Update the registration comments for PresentationTool and
DocumentTool to state that each tool is registered only when the documents
feature is enabled, matching their #[cfg(feature = "documents")] gates; remove
the stale “Always registered” wording.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 369e152c-ed7f-4275-bed6-de7d11530219
📒 Files selected for processing (42)
Cargo.tomlapp/src-tauri/Cargo.tomlapp/src-tauri/src/lib.rssrc/core/agent_cli.rssrc/core/all.rssrc/core/all_tests.rssrc/core/auth.rssrc/core/cli.rssrc/core/http_server_status.rssrc/core/jsonrpc.rssrc/core/jsonrpc_tests.rssrc/core/logging.rssrc/core/mod.rssrc/core/observability.rssrc/core/runtime/builder.rssrc/core/runtime/mod.rssrc/core/shutdown.rssrc/core/socketio.rssrc/main.rssrc/openhuman/accessibility/permissions.rssrc/openhuman/agent/multimodal.rssrc/openhuman/agentbox/mod.rssrc/openhuman/artifacts/ops.rssrc/openhuman/composio/ops_tests.rssrc/openhuman/credentials/sentry_scope.rssrc/openhuman/inference/http/mod.rssrc/openhuman/inference/local/service/whisper_engine/mod.rssrc/openhuman/inference/local/service/whisper_engine/real.rssrc/openhuman/inference/local/service/whisper_engine/stub.rssrc/openhuman/inference/local/service/whisper_engine/types.rssrc/openhuman/inference/mod.rssrc/openhuman/inference/voice/mod.rssrc/openhuman/mcp_server/local.rssrc/openhuman/mcp_server/mod.rssrc/openhuman/mcp_server/stdio.rssrc/openhuman/mod.rssrc/openhuman/text_input/mod.rssrc/openhuman/tools/impl/mod.rssrc/openhuman/tools/ops.rssrc/openhuman/tools/ops_tests.rssrc/openhuman/voice/mod.rssrc/openhuman/voice/stub.rs
… tower dev-dep, axum-shed comment - runtime/builder: when `rpc_http` is requested but `http-server` is compiled out, bail with a build-feature error instead of returning Ok with no listener bound (codex P2). No-transport embedders still hit the `!rpc_http` early return. - agent_registry/loader: gate `presentation_agent` in lockstep with its `generate_presentation` tool — filtered out of the registry under `--no-default-features` without `documents` so it is not advertised as `make_presentation` with no deck tool available (codex P2). - Cargo.toml: drop the duplicate production `tower` (merge artifact from reconciling with main); it is test-only and already a dev-dependency. - all_tests: correct the http-server gate comments — `axum` is retained transitively via `tinychannels`, so only `socketioxide` is shed. cargo fmt, clippy -D warnings, feature-forwarding gate, --locked all clean.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/agent_registry/agents/loader.rs`:
- Around line 343-367: The changed built-in registry behavior in builtin_enabled
must be covered by tests around load_builtins. Add feature-aware tests asserting
presentation_agent is included when the documents feature is enabled and
excluded when it is disabled, while preserving existing loader validation and
other built-in expectations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 191eb0da-2ccd-497e-bb0f-6dd82dfd6e22
📒 Files selected for processing (4)
Cargo.tomlsrc/core/all_tests.rssrc/core/runtime/builder.rssrc/openhuman/agent_registry/agents/loader.rs
💤 Files with no reviewable changes (1)
- Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/all_tests.rs
…e diagnostics, docs - observability: scrub secrets from report_error/warning_message before they reach any log sink (the tracing line logs in every build, incl. slim builds with no crash-reporting before_send). New always-compiled core::log_redaction owns the one shared pattern set; main.rs Sentry before_send delegates to it. - agent_registry/loader: fix the gates-off test breakage from the presentation_agent gate — all_builtins_parse now compares against the filtered count, and the specialist-tools assertion is #[cfg(feature = documents)]. - mcp_server/local: add a #[cfg(all(test, not(feature = http-server)))] test pinning the disabled ensure_local_http fallback error. - whisper stub / multimodal / artifacts: grep-friendly debug logs on the disabled-feature branches (no audio/PII). - http_server_status / shutdown / builder docs: correct the axum-shed claims (axum stays transitively via tinychannels; only socketioxide is shed) and document serve()'s slim-build rpc_http build-feature error. cargo fmt, clippy -D warnings, feature-forwarding gate, gates-off gate-contract tests (cargo test --no-default-features … core:: loader::), and slim cargo check all pass locally.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/log_redaction.rs`:
- Around line 45-46: Broaden the generic `sk-` regex in the redaction rules to
match the complete token, including separator characters such as underscores,
while preserving the existing minimum length and catch-all ordering. Add a
regression test covering a separator-containing token like
`sk-abcdefghijklmnopqrst_uv` and verify the entire value is replaced with
`[REDACTED]`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 164cf0ed-3a79-4a5b-86a0-322167f15356
📒 Files selected for processing (12)
src/core/http_server_status.rssrc/core/log_redaction.rssrc/core/mod.rssrc/core/observability.rssrc/core/runtime/builder.rssrc/core/shutdown.rssrc/main.rssrc/openhuman/agent/multimodal.rssrc/openhuman/agent_registry/agents/loader.rssrc/openhuman/artifacts/ops.rssrc/openhuman/inference/local/service/whisper_engine/stub.rssrc/openhuman/mcp_server/local.rs
🚧 Files skipped from review as they are similar to previous changes (9)
- src/core/shutdown.rs
- src/core/http_server_status.rs
- src/openhuman/agent_registry/agents/loader.rs
- src/openhuman/agent/multimodal.rs
- src/core/runtime/builder.rs
- src/openhuman/inference/local/service/whisper_engine/stub.rs
- src/core/mod.rs
- src/openhuman/artifacts/ops.rs
- src/openhuman/mcp_server/local.rs
Add presentation_agent_registered_when_documents_on (#[cfg(documents)]) and presentation_agent_absent_when_documents_off (#[cfg(not(documents))]) so the load_builtins gate contract is covered in both feature states, not just via the document-tool registration test. Both pass locally (default + gates-off).
The generic sk- catch-all only matched [a-zA-Z0-9], so a key with a mid-suffix
separator (sk-…_uv) leaked the trailing fragment as [REDACTED]_uv. Broaden to
[A-Za-z0-9_-]{20,} and add a separator-containing regression test. Fixes both the
observability log path and main.rs Sentry before_send (both share this module).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
openhuman_core—inference(whisper-rs + cpal),documents(pdf-extract / ppt-rs / docx-rs),crash-reporting(sentry), andhttp-server(axum + socketioxide) — so a headless / embedded core (Keep embedded OpenHuman within a 20–30 MiB RAM budget for OpenCompany #5046) can shed server- and desktop-only dependencies via--no-default-features.reqwestdrops the second TLS stack off Linux/macOS (native-tls → Windows-only), and the long-lived tokio runtimes boundmax_blocking_threads(512 → 64).Cargo.tomlcomment that claimedllamais a dependency.Problem
voicegate deferred to "a separate inference gate", plus the "additional gates" (sentry, the office-document engines, and the axum/socketioxide HTTP transport). These are the largest dependency + compile-surface blockers to the 20–30 MiB embedded RSS budget tracked in Keep embedded OpenHuman within a 20–30 MiB RAM budget for OpenCompany #5046.Solution
Facade + type-carve-out pattern (AGENTS.md): inert types stay compiled in both build states, only behaviour is gated, and stubs mirror caller signatures so callers need no per-call
#[cfg].whisper_enginebecomes a facade (real/stub/types);TranscriptionResultis carved out ungated; thecpalmicrophone probe in the always-compiledaccessibility::permissionsis gated by an exhaustive feature × OS matrix.voicenow requiresinference.generate_presentation/generate_documenttools; PDF ingest degrades the file to a reference when off; the one non-toolPresentationToolconsumer (artifacts.ai_regenerate) keeps its always-compiled RPC handler and delegates the tool path to a gated helper.sentry::protocol::Eventclassifiers +sentry::init+ the tracing layer + scope binding are gated; the sentry-freeis_*_messagestring classifiers and thereport_error*signatures (with theirtracingdiagnostics) stay compiled in both builds.CoreRuntime::serve()keeps its signature; the axum bind-half is extracted to a gatedserve_http()and the off-arm reuses the existing!rpc_httpbackground-services return;core/socketio.rsuses an in-place type carve-out (theWebChannelEvent/usage payload structs stay compiled); three cross-gate couplings are resolved and build-tested (mcp×http-server,voice×http-server, theinferenceconst carve-out);towermoves to a dev-dependency.Honest dependency shed: whisper-rs, cpal, pdf-extract, ppt-rs, docx-rs, sentry, and socketioxide all drop from the disabled non-test graph (proven with
cargo tree -i).axumstays linked viatinychannels(relay-websocket) — its openhuman surface (~13 transport files) compiles out, but fully removing the crate needs a tinychannels change (follow-up).Submission Checklist
*_when_feature_on/*_when_feature_offtests per gate (core::all_tests,whisper_engine::stub,tools::ops_tests,core::observability/credentials::sentry_scope/composio::ops_tests,core::http_server_status), covering the enabled path and the disabled/stub failure path.#[cfg(not(feature = …))]disabled-only stub arms are not compiled under the default-feature coverage run, so changed-lines coverage may dip. This is inherent to every compile-time gate in this repo; it is not resolvable locally. Flagging for the coverage job.N/A: build-configuration / behaviour-identical change. Gates are compile-time and the desktop build is byte-identical; no product feature leaf is added, removed, or changed.N/A: dependency removal / gating only.N/A: no release-cut surface behaviour change (desktop byte-identical).Closes #5048in## Related.Impact
HTTP_SERVER_COMPILED_INconst-assert in the shell turns a mis-forward into a build error rather than a listener-less app.--no-default-featuresbuild: sheds whisper-rs, cpal, pdf-extract, ppt-rs, docx-rs, sentry, socketioxide; single TLS stack; bounded blocking pool; axum surface compiled out (crate remains via tinychannels).report_error*signatures +tracingdiagnostics in both builds (no error swallowing); all disabled paths return explicit errors, none silently succeed.Related
694290747) belongs to chore(core): remove zero-reference deps and redirect_links domain #5052 — review the five gate commits2e10a0574..323798280. Will rebase ontomainonce chore(core): remove zero-reference deps and redirect_links domain #5052 merges.axumremoval (gate tinychannels' relay-websocket transport); Keep embedded OpenHuman within a 20–30 MiB RAM budget for OpenCompany #5046 (the embedded RSS budget this unblocks); Gate the desktop-automation cluster, then extract it out of the core #5049 / Stop building the full tool surface and cloning Config on every agent construction #5050 (remaining Keep embedded OpenHuman within a 20–30 MiB RAM budget for OpenCompany #5046 children).AI Authored PR Metadata
N/A— human-authored.Linear Issue
Commit & Branch
Validation Run
Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
HTTP_SERVER_COMPILED_INshell assert; both-ways gate tests; disabled-build compile as the drift catcher.Duplicate / Superseded PR Handling
Summary by CodeRabbit