fix(core): unbreak the gates-off build and make the smoke lane run tests (#5022)#5023
Merged
Merged
Conversation
…5002) Move channels/providers/web -> web_chat/, taking web_errors.rs and web_tests.rs with it (both were reached via #[path] from web/mod.rs and never declared in providers/mod.rs). Drop the now-redundant ../ path attribute on web_errors. No back-compat re-export: channels no longer exports web.
…humansai#5002) The 5 channels/ modules that consume the conduit (bus, host::adapters, proactive, telegram::remote_control, runtime::startup) now import it from its new home. Dependency is one-way: channels -> web_chat.
) Harness-family consumers (core::{socketio,jsonrpc,all,observability, event_bus}, agent::task_dispatcher, threads::ops, agentbox::invoker, cron::scheduler, flows::ops, test_support::introspect) now import the conduit from openhuman::web_chat. src/core/all.rs keeps its DomainGroup::Channels tag: the web RPC surface must stay hidden by DomainSet::harness() at runtime exactly as today. Re-tagging it would be an unreviewed behaviour change; tinyhumansai#4801 will cfg this registration site instead. rustfmt reflows 4 call sites purely because the shorter path now fits.
channels_large_round25 imported the module with {self}, which bound the
local name 'web'; rebound as {self as web} so its 7 bare web:: uses keep
resolving rather than renaming them.
Comment/README-only. Several already named a stale 'channels/providers/web.rs' (the module has been a directory for a while); the move made them wrong twice over. Historical plan snapshots under docs/plans, docs/superpowers and docs/tinyagents-full-migration-plan are left as-is -- they record a past state.
…le (tinyhumansai#5002) The four EventHandler::name() returns still read `channels::web::*`, naming a module that no longer exists after the relocation. AGENTS.md's convention is `<domain>::<purpose>`, and sibling subscribers follow it (`cron::delivery`, `memory::conversations::persistence`). Log-output only: `name()` is consumed at src/core/event_bus/bus.rs:145,169 and both uses are tracing fields. Dispatch filters on `domains()`, not `name()`. Verified nothing keys on the old strings — no classifier, filter, or frontend reference.
…chat (tinyhumansai#5002) The PromptEnforcementContext source label still read `channels.providers.web.start_chat`, naming a module that no longer exists. Siblings use the dotted module path (`agent.bus.run_turn`, `agent.runtime.run_single`), so the canonical value is `web_chat.start_chat`. Diagnostic-only: `source` is consumed at src/openhuman/prompt_injection/detector.rs:504 as a `tracing::info!` field. Verified nothing matches on the literal. Caught by CodeRabbit on tinyhumansai#5004 — the original grep proof searched the Rust path form (`channels::providers::web`) and was blind to the dot-separated string.
…nyhumansai#5002) Comment referenced `channels/providers/web::ArtifactSurfaceSubscriber`; the subscriber now lives in `web_chat`. Comment-only, no executable change.
Contributor
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (76)
💤 Files with no reviewable changes (2)
📝 WalkthroughWalkthroughThis PR moves web-chat functionality to ChangesWeb chat relocation and runtime behavior
Feature-gate smoke coverage
Native shell adjustments
Estimated code review effort: 5 (Critical) | ~120 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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Comment |
![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: b657f74245
ℹ️ 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".
Comment on lines
+464
to
+465
| ACTUAL=$(grep -rlE '#\[cfg\((not\()?feature = "(voice|media|web3|meet|mcp|skills|flows)"' src --include='*.rs' \ | ||
| | xargs grep -lE '#\[test\]|#\[tokio::test\]|fn .*_test' 2>/dev/null | sed 's|^src/||' | sort -u) |
There was a problem hiding this comment.
This matcher only recognizes #[cfg(feature = ...)] and #[cfg(not(feature = ...))] when that predicate appears first, so it already omits feature-gated test modules using the conventional compound form, such as #[cfg(all(test, feature = "mcp"))] in src/openhuman/mcp_server/tools/mod.rs:38 and #[cfg(all(test, feature = "skills"))] in src/openhuman/skill_registry/mod.rs:34. Adding or removing gated tests in those modules will not change ACTUAL, allowing the supposedly self-maintaining guard to silently under-cover the exact cases it is meant to detect; make the scan recognize feature predicates nested in all(...)/any(...) as well.
Useful? React with 👍 / 👎.
coderabbitai
Bot
added
bug
rust-core
![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: 10
🧹 Nitpick comments (2)
src/openhuman/web_chat/progress_bridge.rs (1)
308-1457: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit the progress bridge into focused modules.
This 1,100-line function mixes event dispatch, ledger persistence, tracing export, heartbeat state, and wire projection. Move these into an export-focused
progress_bridge/mod.rswith dedicated dispatcher, ledger, tracing, and projection modules.As per coding guidelines, Rust files should preferably remain approximately 500 lines or fewer, and
mod.rsfiles should remain export-focused.🤖 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/web_chat/progress_bridge.rs` around lines 308 - 1457, The monolithic spawn_progress_bridge function combines event dispatch, ledger persistence, tracing, heartbeat management, and wire-event projection. Split it into an export-focused progress_bridge/mod.rs plus dedicated dispatcher, ledger, tracing, and projection modules, moving each responsibility behind focused functions while preserving existing behavior and public exports. Keep mod.rs limited to module declarations and re-exports, and keep Rust source files preferably under approximately 500 lines.Source: Coding guidelines
src/openhuman/web_chat/web_errors.rs (1)
507-936: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit the oversized classifier and test suite into focused modules.
src/openhuman/web_chat/web_errors.rs#L507-L936: extract managed-code, transport, provider, and copy-building classifiers while preserving top-level precedence.src/openhuman/web_chat/web_tests.rs#L1-L31: split classifier, schema, cache, and concurrency tests into sibling modules.As per coding guidelines: “Prefer files of approximately 500 lines or fewer.”
🤖 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/web_chat/web_errors.rs` around lines 507 - 936, Split the oversized web-chat error classifier into focused sibling modules while preserving classify_inference_error’s existing precedence and behavior: move managed-code, transport, provider, and user-facing copy-building logic into appropriate modules with clear interfaces. Also split web_tests.rs into sibling modules for classifier, schema, cache, and concurrency tests, updating module declarations and imports so all tests remain discoverable and passing. Apply changes to src/openhuman/web_chat/web_errors.rs lines 507-936 and src/openhuman/web_chat/web_tests.rs lines 1-31; no behavior changes are needed.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 @.github/workflows/ci-lite.yml:
- Around line 435-470: Update the ACTUAL computation in the “Guard — new
feature-gated test modules must be acknowledged” workflow step to detect feature
predicates nested in composite cfg expressions such as any(...) and all(...),
including multiline predicates. Replace the current prefix-only grep with a
predicate-aware scanner while preserving the existing test-file filtering and
feature set.
- Around line 383-385: Update the smoke-lane Cargo command near the workflow
step around the all-targets comment to include --all-targets, ensuring
integration tests and other targets are compiled after gated target references
are fixed. Keep the timeout and surrounding workflow behavior unchanged, and
ensure the comments accurately describe the command.
In `@src/openhuman/approval/README.md`:
- Line 59: Update the event-bus ownership documentation in the approval README
so the subscriber location referenced by the sentence around the channels web
provider uses web_chat consistently, matching ApprovalSurfaceSubscriber and the
preceding DomainEvent description.
In `@src/openhuman/web_chat/presentation.rs`:
- Around line 64-74: The reaction task is awaited before response delivery
without a time limit. Update the reaction handling around try_reaction and
reaction_handle to apply an explicit timeout, abort the task on expiry, and add
grep-friendly diagnostics for timeout and join-error outcomes while preserving
successful reactions; ensure slow local inference cannot delay emitting segments
or chat_done.
In `@src/openhuman/web_chat/progress_bridge.rs`:
- Around line 196-209: Update session_profile_user_attribution to return only
the opaque state.user_id and never read or return the user's email; preserve
None for signed-out or unreadable profiles so callers can use anonymous
transport attribution. Update the related stored-session test to expect "u-1"
instead of the email, and ensure the trace builder paths at the referenced
usages do not reintroduce email as userId.
In `@src/openhuman/web_chat/run_task.rs`:
- Around line 115-124: Update the agent setup flow around the cache reuse match
and fresh construction to call set_event_context with the current client_id and
request-scoped context after either path produces the agent. Ensure reused
agents receive the new socket’s context while preserving the existing
construction and cache behavior.
- Around line 92-104: The session fingerprint must include every input used to
construct an agent. In src/openhuman/web_chat/run_task.rs lines 92-104, update
provider_role_for_model_override usage to derive the effective role from the
request/profile override or config.default_model, matching build_session_agent.
In src/openhuman/web_chat/session.rs lines 182-203, extend
SessionCacheFingerprint with normalized locale or the composed prompt-suffix
signature and populate it wherever the fingerprint is constructed.
In `@src/openhuman/web_chat/session.rs`:
- Around line 129-134: Update the short_thread construction in the session setup
to truncate thread_id by up to 12 Unicode characters rather than byte-slicing,
using chars().take(12) and collecting into a String so multibyte UTF-8 input
cannot panic; preserve the existing agent.set_agent_definition_name format.
In `@src/openhuman/web_chat/types.rs`:
- Around line 81-83: Constrain WebChatParams.source at the RPC boundary before
constructing ChatRequestMetadata: parse it against an allowlisted source enum
and reject or normalize unknown values, ensuring ChatRequestMetadata.source
cannot contain arbitrary user-authored text. Update the affected metadata
construction paths and add a regression test covering an unknown or sensitive
source value.
In `@src/openhuman/web_chat/web_errors.rs`:
- Around line 668-696: Update the rate-limit condition in the error
classification branch so a bare “429” substring is not sufficient; require
explicit rate-limit wording or recognized provider/HTTP status envelope patterns
such as “api error (429” or “too many requests”. Keep context-overflow detection
and existing rate-limit handling unchanged for genuinely matching errors.
---
Nitpick comments:
In `@src/openhuman/web_chat/progress_bridge.rs`:
- Around line 308-1457: The monolithic spawn_progress_bridge function combines
event dispatch, ledger persistence, tracing, heartbeat management, and
wire-event projection. Split it into an export-focused progress_bridge/mod.rs
plus dedicated dispatcher, ledger, tracing, and projection modules, moving each
responsibility behind focused functions while preserving existing behavior and
public exports. Keep mod.rs limited to module declarations and re-exports, and
keep Rust source files preferably under approximately 500 lines.
In `@src/openhuman/web_chat/web_errors.rs`:
- Around line 507-936: Split the oversized web-chat error classifier into
focused sibling modules while preserving classify_inference_error’s existing
precedence and behavior: move managed-code, transport, provider, and user-facing
copy-building logic into appropriate modules with clear interfaces. Also split
web_tests.rs into sibling modules for classifier, schema, cache, and concurrency
tests, updating module declarations and imports so all tests remain discoverable
and passing. Apply changes to src/openhuman/web_chat/web_errors.rs lines 507-936
and src/openhuman/web_chat/web_tests.rs lines 1-31; no behavior changes are
needed.
🪄 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: fb7f49ae-1462-4a8b-a64b-e3ccfa88afef
📒 Files selected for processing (76)
.github/workflows/ci-lite.ymlapp/src-tauri/src/claude_code.rsapp/src-tauri/src/imessage_scanner/mod.rsapp/src-tauri/src/lib.rsapp/src-tauri/src/mascot_native_window.rsapp/src-tauri/src/native_notifications/mod.rsapp/src-tauri/src/notch_window.rsapp/src/services/chatService.tsdocs/TEST-COVERAGE-MATRIX.mdsrc/core/all.rssrc/core/all_tests.rssrc/core/event_bus/events.rssrc/core/jsonrpc.rssrc/core/observability.rssrc/core/socketio.rssrc/main.rssrc/openhuman/agent/README.mdsrc/openhuman/agent/progress_tracing/journal_projection.rssrc/openhuman/agent/task_dispatcher/executor.rssrc/openhuman/agent/task_dispatcher/registry.rssrc/openhuman/agent/triage/evaluator.rssrc/openhuman/agent/turn_origin.rssrc/openhuman/agent_orchestration/command_center/mod.rssrc/openhuman/agent_orchestration/run_ledger_finalize.rssrc/openhuman/agentbox/invoker.rssrc/openhuman/approval/README.mdsrc/openhuman/approval/gate.rssrc/openhuman/artifacts/store.rssrc/openhuman/channels/bus.rssrc/openhuman/channels/host/adapters.rssrc/openhuman/channels/mod.rssrc/openhuman/channels/proactive.rssrc/openhuman/channels/providers/mod.rssrc/openhuman/channels/providers/telegram/remote_control.rssrc/openhuman/channels/runtime/dispatch/processor.rssrc/openhuman/channels/runtime/startup.rssrc/openhuman/cron/scheduler.rssrc/openhuman/cron/scheduler_tests.rssrc/openhuman/flows/ops.rssrc/openhuman/inference/provider/error_code.rssrc/openhuman/meet_agent/brain/llm.rssrc/openhuman/mod.rssrc/openhuman/prompt_injection/README.mdsrc/openhuman/security/egress/mod.rssrc/openhuman/test_support/README.mdsrc/openhuman/test_support/introspect.rssrc/openhuman/threads/README.mdsrc/openhuman/threads/ops.rssrc/openhuman/wallet/README.mdsrc/openhuman/wallet/execution.rssrc/openhuman/web_chat/event_bus.rssrc/openhuman/web_chat/mod.rssrc/openhuman/web_chat/ops.rssrc/openhuman/web_chat/presentation.rssrc/openhuman/web_chat/presentation_tests.rssrc/openhuman/web_chat/progress_bridge.rssrc/openhuman/web_chat/run_task.rssrc/openhuman/web_chat/schemas.rssrc/openhuman/web_chat/session.rssrc/openhuman/web_chat/types.rssrc/openhuman/web_chat/web_errors.rssrc/openhuman/web_chat/web_tests.rstests/agent_harness_e2e.rstests/agentbox_e2e.rstests/composio_list_tools_stack_overflow_regression.rstests/raw_coverage/channels_bus_presentation_raw_coverage_e2e.rstests/raw_coverage/channels_large_round25_raw_coverage_e2e.rstests/raw_coverage/channels_provider_deep_raw_coverage_e2e.rstests/raw_coverage/channels_provider_leftovers_raw_coverage_e2e.rstests/raw_coverage/channels_runtime_raw_coverage_e2e.rstests/raw_coverage/channels_web_startup_raw_coverage_e2e.rstests/raw_coverage/channels_web_telegram_raw_coverage_e2e.rstests/raw_coverage/channels_web_yuanbao_round22_raw_coverage_e2e.rstests/raw_coverage/tools_approval_channels_raw_coverage_e2e.rstests/raw_coverage/tools_network_channels_raw_coverage_e2e.rstests/web_cancel_request_scoping.rs
💤 Files with no reviewable changes (2)
- src/openhuman/channels/providers/mod.rs
- src/openhuman/channels/mod.rs
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 10
🧹 Nitpick comments (2)
src/openhuman/web_chat/progress_bridge.rs (1)
308-1457: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit the progress bridge into focused modules.
This 1,100-line function mixes event dispatch, ledger persistence, tracing export, heartbeat state, and wire projection. Move these into an export-focused
progress_bridge/mod.rswith dedicated dispatcher, ledger, tracing, and projection modules.As per coding guidelines, Rust files should preferably remain approximately 500 lines or fewer, and
mod.rsfiles should remain export-focused.🤖 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/web_chat/progress_bridge.rs` around lines 308 - 1457, The monolithic spawn_progress_bridge function combines event dispatch, ledger persistence, tracing, heartbeat management, and wire-event projection. Split it into an export-focused progress_bridge/mod.rs plus dedicated dispatcher, ledger, tracing, and projection modules, moving each responsibility behind focused functions while preserving existing behavior and public exports. Keep mod.rs limited to module declarations and re-exports, and keep Rust source files preferably under approximately 500 lines.Source: Coding guidelines
src/openhuman/web_chat/web_errors.rs (1)
507-936: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit the oversized classifier and test suite into focused modules.
src/openhuman/web_chat/web_errors.rs#L507-L936: extract managed-code, transport, provider, and copy-building classifiers while preserving top-level precedence.src/openhuman/web_chat/web_tests.rs#L1-L31: split classifier, schema, cache, and concurrency tests into sibling modules.As per coding guidelines: “Prefer files of approximately 500 lines or fewer.”
🤖 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/web_chat/web_errors.rs` around lines 507 - 936, Split the oversized web-chat error classifier into focused sibling modules while preserving classify_inference_error’s existing precedence and behavior: move managed-code, transport, provider, and user-facing copy-building logic into appropriate modules with clear interfaces. Also split web_tests.rs into sibling modules for classifier, schema, cache, and concurrency tests, updating module declarations and imports so all tests remain discoverable and passing. Apply changes to src/openhuman/web_chat/web_errors.rs lines 507-936 and src/openhuman/web_chat/web_tests.rs lines 1-31; no behavior changes are needed.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 @.github/workflows/ci-lite.yml:
- Around line 435-470: Update the ACTUAL computation in the “Guard — new
feature-gated test modules must be acknowledged” workflow step to detect feature
predicates nested in composite cfg expressions such as any(...) and all(...),
including multiline predicates. Replace the current prefix-only grep with a
predicate-aware scanner while preserving the existing test-file filtering and
feature set.
- Around line 383-385: Update the smoke-lane Cargo command near the workflow
step around the all-targets comment to include --all-targets, ensuring
integration tests and other targets are compiled after gated target references
are fixed. Keep the timeout and surrounding workflow behavior unchanged, and
ensure the comments accurately describe the command.
In `@src/openhuman/approval/README.md`:
- Line 59: Update the event-bus ownership documentation in the approval README
so the subscriber location referenced by the sentence around the channels web
provider uses web_chat consistently, matching ApprovalSurfaceSubscriber and the
preceding DomainEvent description.
In `@src/openhuman/web_chat/presentation.rs`:
- Around line 64-74: The reaction task is awaited before response delivery
without a time limit. Update the reaction handling around try_reaction and
reaction_handle to apply an explicit timeout, abort the task on expiry, and add
grep-friendly diagnostics for timeout and join-error outcomes while preserving
successful reactions; ensure slow local inference cannot delay emitting segments
or chat_done.
In `@src/openhuman/web_chat/progress_bridge.rs`:
- Around line 196-209: Update session_profile_user_attribution to return only
the opaque state.user_id and never read or return the user's email; preserve
None for signed-out or unreadable profiles so callers can use anonymous
transport attribution. Update the related stored-session test to expect "u-1"
instead of the email, and ensure the trace builder paths at the referenced
usages do not reintroduce email as userId.
In `@src/openhuman/web_chat/run_task.rs`:
- Around line 115-124: Update the agent setup flow around the cache reuse match
and fresh construction to call set_event_context with the current client_id and
request-scoped context after either path produces the agent. Ensure reused
agents receive the new socket’s context while preserving the existing
construction and cache behavior.
- Around line 92-104: The session fingerprint must include every input used to
construct an agent. In src/openhuman/web_chat/run_task.rs lines 92-104, update
provider_role_for_model_override usage to derive the effective role from the
request/profile override or config.default_model, matching build_session_agent.
In src/openhuman/web_chat/session.rs lines 182-203, extend
SessionCacheFingerprint with normalized locale or the composed prompt-suffix
signature and populate it wherever the fingerprint is constructed.
In `@src/openhuman/web_chat/session.rs`:
- Around line 129-134: Update the short_thread construction in the session setup
to truncate thread_id by up to 12 Unicode characters rather than byte-slicing,
using chars().take(12) and collecting into a String so multibyte UTF-8 input
cannot panic; preserve the existing agent.set_agent_definition_name format.
In `@src/openhuman/web_chat/types.rs`:
- Around line 81-83: Constrain WebChatParams.source at the RPC boundary before
constructing ChatRequestMetadata: parse it against an allowlisted source enum
and reject or normalize unknown values, ensuring ChatRequestMetadata.source
cannot contain arbitrary user-authored text. Update the affected metadata
construction paths and add a regression test covering an unknown or sensitive
source value.
In `@src/openhuman/web_chat/web_errors.rs`:
- Around line 668-696: Update the rate-limit condition in the error
classification branch so a bare “429” substring is not sufficient; require
explicit rate-limit wording or recognized provider/HTTP status envelope patterns
such as “api error (429” or “too many requests”. Keep context-overflow detection
and existing rate-limit handling unchanged for genuinely matching errors.
---
Nitpick comments:
In `@src/openhuman/web_chat/progress_bridge.rs`:
- Around line 308-1457: The monolithic spawn_progress_bridge function combines
event dispatch, ledger persistence, tracing, heartbeat management, and
wire-event projection. Split it into an export-focused progress_bridge/mod.rs
plus dedicated dispatcher, ledger, tracing, and projection modules, moving each
responsibility behind focused functions while preserving existing behavior and
public exports. Keep mod.rs limited to module declarations and re-exports, and
keep Rust source files preferably under approximately 500 lines.
In `@src/openhuman/web_chat/web_errors.rs`:
- Around line 507-936: Split the oversized web-chat error classifier into
focused sibling modules while preserving classify_inference_error’s existing
precedence and behavior: move managed-code, transport, provider, and user-facing
copy-building logic into appropriate modules with clear interfaces. Also split
web_tests.rs into sibling modules for classifier, schema, cache, and concurrency
tests, updating module declarations and imports so all tests remain discoverable
and passing. Apply changes to src/openhuman/web_chat/web_errors.rs lines 507-936
and src/openhuman/web_chat/web_tests.rs lines 1-31; no behavior changes are
needed.
🪄 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: fb7f49ae-1462-4a8b-a64b-e3ccfa88afef
📒 Files selected for processing (76)
.github/workflows/ci-lite.ymlapp/src-tauri/src/claude_code.rsapp/src-tauri/src/imessage_scanner/mod.rsapp/src-tauri/src/lib.rsapp/src-tauri/src/mascot_native_window.rsapp/src-tauri/src/native_notifications/mod.rsapp/src-tauri/src/notch_window.rsapp/src/services/chatService.tsdocs/TEST-COVERAGE-MATRIX.mdsrc/core/all.rssrc/core/all_tests.rssrc/core/event_bus/events.rssrc/core/jsonrpc.rssrc/core/observability.rssrc/core/socketio.rssrc/main.rssrc/openhuman/agent/README.mdsrc/openhuman/agent/progress_tracing/journal_projection.rssrc/openhuman/agent/task_dispatcher/executor.rssrc/openhuman/agent/task_dispatcher/registry.rssrc/openhuman/agent/triage/evaluator.rssrc/openhuman/agent/turn_origin.rssrc/openhuman/agent_orchestration/command_center/mod.rssrc/openhuman/agent_orchestration/run_ledger_finalize.rssrc/openhuman/agentbox/invoker.rssrc/openhuman/approval/README.mdsrc/openhuman/approval/gate.rssrc/openhuman/artifacts/store.rssrc/openhuman/channels/bus.rssrc/openhuman/channels/host/adapters.rssrc/openhuman/channels/mod.rssrc/openhuman/channels/proactive.rssrc/openhuman/channels/providers/mod.rssrc/openhuman/channels/providers/telegram/remote_control.rssrc/openhuman/channels/runtime/dispatch/processor.rssrc/openhuman/channels/runtime/startup.rssrc/openhuman/cron/scheduler.rssrc/openhuman/cron/scheduler_tests.rssrc/openhuman/flows/ops.rssrc/openhuman/inference/provider/error_code.rssrc/openhuman/meet_agent/brain/llm.rssrc/openhuman/mod.rssrc/openhuman/prompt_injection/README.mdsrc/openhuman/security/egress/mod.rssrc/openhuman/test_support/README.mdsrc/openhuman/test_support/introspect.rssrc/openhuman/threads/README.mdsrc/openhuman/threads/ops.rssrc/openhuman/wallet/README.mdsrc/openhuman/wallet/execution.rssrc/openhuman/web_chat/event_bus.rssrc/openhuman/web_chat/mod.rssrc/openhuman/web_chat/ops.rssrc/openhuman/web_chat/presentation.rssrc/openhuman/web_chat/presentation_tests.rssrc/openhuman/web_chat/progress_bridge.rssrc/openhuman/web_chat/run_task.rssrc/openhuman/web_chat/schemas.rssrc/openhuman/web_chat/session.rssrc/openhuman/web_chat/types.rssrc/openhuman/web_chat/web_errors.rssrc/openhuman/web_chat/web_tests.rstests/agent_harness_e2e.rstests/agentbox_e2e.rstests/composio_list_tools_stack_overflow_regression.rstests/raw_coverage/channels_bus_presentation_raw_coverage_e2e.rstests/raw_coverage/channels_large_round25_raw_coverage_e2e.rstests/raw_coverage/channels_provider_deep_raw_coverage_e2e.rstests/raw_coverage/channels_provider_leftovers_raw_coverage_e2e.rstests/raw_coverage/channels_runtime_raw_coverage_e2e.rstests/raw_coverage/channels_web_startup_raw_coverage_e2e.rstests/raw_coverage/channels_web_telegram_raw_coverage_e2e.rstests/raw_coverage/channels_web_yuanbao_round22_raw_coverage_e2e.rstests/raw_coverage/tools_approval_channels_raw_coverage_e2e.rstests/raw_coverage/tools_network_channels_raw_coverage_e2e.rstests/web_cancel_request_scoping.rs
💤 Files with no reviewable changes (2)
- src/openhuman/channels/providers/mod.rs
- src/openhuman/channels/mod.rs
🛑 Comments failed to post (10)
.github/workflows/ci-lite.yml (2)
383-385: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
The smoke lane still does not compile all targets.
Line 416 omits
--all-targets, while Lines 383-384 claim that all targets are now linked. Integration tests and other targets therefore remain unchecked, contrary to the stated PR objective. Restorecargo check --all-targetsafter fixing the gated target references, or revise the objective and comments.Also applies to: 410-416
🤖 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 @.github/workflows/ci-lite.yml around lines 383 - 385, Update the smoke-lane Cargo command near the workflow step around the all-targets comment to include --all-targets, ensuring integration tests and other targets are compiled after gated target references are fixed. Keep the timeout and surrounding workflow behavior unchanged, and ensure the comments accurately describe the command.
435-470: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Composite
cfgpredicates bypass the allowlist guard.Line 464 only recognizes predicates beginning directly with
featureornot(feature). Tests using common forms such ascfg(any(feature = ...))orcfg(all(..., feature = ...))are omitted fromACTUAL, allowing silent undercoverage. Use a predicate-aware or multiline scanner.🤖 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 @.github/workflows/ci-lite.yml around lines 435 - 470, Update the ACTUAL computation in the “Guard — new feature-gated test modules must be acknowledged” workflow step to detect feature predicates nested in composite cfg expressions such as any(...) and all(...), including multiline predicates. Replace the current prefix-only grep with a predicate-aware scanner while preserving the existing test-file filtering and feature set.src/openhuman/approval/README.md (1)
59-59: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep the event-bus ownership documentation consistent.
Line 59 now points to
src/openhuman/web_chat/, but Line 62 still says the subscriber lives in thechannelsweb provider. Update that sentence toweb_chatas well.🤖 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/approval/README.md` at line 59, Update the event-bus ownership documentation in the approval README so the subscriber location referenced by the sentence around the channels web provider uses web_chat consistently, matching ApprovalSurfaceSubscriber and the preceding DomainEvent description.src/openhuman/web_chat/presentation.rs (1)
64-74: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound the optional reaction call so it cannot block response delivery.
The spawned task is immediately awaited before the first event, so slow local inference delays
chat_doneand all segments. Apply an explicit timeout, abort on expiry, and log timeout/join-error branches—or emit the reaction independently after delivering the response.As per coding guidelines, new flows require grep-friendly diagnostics covering external calls, retries/timeouts, and errors.
🤖 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/web_chat/presentation.rs` around lines 64 - 74, The reaction task is awaited before response delivery without a time limit. Update the reaction handling around try_reaction and reaction_handle to apply an explicit timeout, abort the task on expiry, and add grep-friendly diagnostics for timeout and join-error outcomes while preserving successful reactions; ensure slow local inference cannot delay emitting segments or chat_done.Source: Coding guidelines
src/openhuman/web_chat/progress_bridge.rs (1)
196-209: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not export email addresses as tracing user IDs.
The cold-cache path prefers
user_id, while the trace builder also falls back to email orclient_id. When tracing or usage sharing is enabled, this exports full PII asuserId. Use only the opaque backend ID and leave anonymous attribution unset.Proposed fix
fn session_profile_user_attribution(config: &crate::openhuman::config::Config) -> Option<String> { let state = crate::openhuman::credentials::session_support::build_session_state(config).ok()?; - state - .user - .as_ref() - .and_then(|u| u.get("email")) - .and_then(serde_json::Value::as_str) - .map(str::to_string) - .or(state.user_id) + state.user_id } - let user_attributed = identity.is_some(); let user_id = identity - .and_then(|i| i.id.or(i.email)) - .or_else(|| session_profile_user_attribution(&config)) - .unwrap_or_else(|| client_id.clone()); + .and_then(|i| i.id) + .or_else(|| session_profile_user_attribution(&config)); + let user_attributed = user_id.is_some(); - let mut trace_ctx = TraceContext::new(trace_id, Some(user_id)) + let mut trace_ctx = TraceContext::new(trace_id, user_id)Update the stored-session test to expect
"u-1"rather than the email.As per coding guidelines, diagnostics must never log secrets or full PII.
Also applies to: 365-406, 1494-1523
🤖 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/web_chat/progress_bridge.rs` around lines 196 - 209, Update session_profile_user_attribution to return only the opaque state.user_id and never read or return the user's email; preserve None for signed-out or unreadable profiles so callers can use anonymous transport attribution. Update the related stored-session test to expect "u-1" instead of the email, and ensure the trace builder paths at the referenced usages do not reintroduce email as userId.Source: Coding guidelines
src/openhuman/web_chat/run_task.rs (2)
92-104: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep the session fingerprint aligned with every agent-construction input.
The cache currently tracks neither the actual effective provider role in all cases nor locale-conditioned prompt state, allowing stale agents to survive relevant configuration changes.
src/openhuman/web_chat/run_task.rs#L92-L104: derive the role from the request/profile override orconfig.default_model, matchingbuild_session_agent.src/openhuman/web_chat/session.rs#L182-L203: add normalized locale or the composed prompt-suffix signature toSessionCacheFingerprint.📍 Affects 2 files
src/openhuman/web_chat/run_task.rs#L92-L104(this comment)src/openhuman/web_chat/session.rs#L182-L203🤖 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/web_chat/run_task.rs` around lines 92 - 104, The session fingerprint must include every input used to construct an agent. In src/openhuman/web_chat/run_task.rs lines 92-104, update provider_role_for_model_override usage to derive the effective role from the request/profile override or config.default_model, matching build_session_agent. In src/openhuman/web_chat/session.rs lines 182-203, extend SessionCacheFingerprint with normalized locale or the composed prompt-suffix signature and populate it wherever the fingerprint is constructed.
115-124: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Refresh request-scoped event context when reusing an agent.
The cache is thread-scoped across socket reconnects, but
set_event_contextis only called during construction. A reused agent retains the previous socket’sclient_id, misattributing subsequent agent-originated events. Reapply the current context after both cache reuse and fresh construction.🤖 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/web_chat/run_task.rs` around lines 115 - 124, Update the agent setup flow around the cache reuse match and fresh construction to call set_event_context with the current client_id and request-scoped context after either path produces the agent. Ensure reused agents receive the new socket’s context while preserving the existing construction and cache behavior.src/openhuman/web_chat/session.rs (1)
129-134: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Truncate
thread_idon a character boundary.
thread_id[..12]panics when byte 12 falls inside a multibyte UTF-8 character. Since chat ingress accepts arbitrary strings, usethread_id.chars().take(12).collect::<String>().🤖 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/web_chat/session.rs` around lines 129 - 134, Update the short_thread construction in the session setup to truncate thread_id by up to 12 Unicode characters rather than byte-slicing, using chars().take(12) and collecting into a String so multibyte UTF-8 input cannot panic; preserve the existing agent.set_agent_definition_name format.src/openhuman/web_chat/types.rs (1)
81-83: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Constrain
sourcebefore it reaches telemetry.
WebChatParams.sourceaccepts arbitrary caller-supplied text and is copied intoChatRequestMetadata.source, which is used for analytics/log filtering. This permits PII, credentials, or unbounded high-cardinality values to enter downstream telemetry. Parse an allowlisted source enum or reject/normalize unknown values at the RPC boundary, and add a regression test.As per coding guidelines, analytics dimensions must be privacy-safe and must never carry user-authored text.
Also applies to: 129-132
🤖 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/web_chat/types.rs` around lines 81 - 83, Constrain WebChatParams.source at the RPC boundary before constructing ChatRequestMetadata: parse it against an allowlisted source enum and reject or normalize unknown values, ensuring ChatRequestMetadata.source cannot contain arbitrary user-authored text. Update the affected metadata construction paths and add a regression test covering an unknown or sensitive source value.Source: Coding guidelines
src/openhuman/web_chat/web_errors.rs (1)
668-696: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not classify every bare
429as rate limiting.An unrelated error such as “context length is 429 tokens” is claimed here before the context-overflow branch. Anchor this to rate-limit wording or provider/HTTP status envelopes such as
api error (429andtoo many requests.🤖 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/web_chat/web_errors.rs` around lines 668 - 696, Update the rate-limit condition in the error classification branch so a bare “429” substring is not sufficient; require explicit rate-limit wording or recognized provider/HTTP status envelope patterns such as “api error (429” or “too many requests”. Keep context-overflow detection and existing rate-limit handling unchanged for genuinely matching errors.
This was referenced Jul 17, 2026
senamakel
pushed a commit
to M3gA-Mind/openhuman
that referenced
this pull request
Jul 18, 2026
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.
🥞 Stacked PR — merge order (bottom → top)
This is position 3/5. Branched on top of #5004, which is on top of #5020, so the diff currently shows their commits too; they self-clean as each merges below it. Review the
#5022-scoped commits (test(core): …,ci(feature-gate-smoke): …).Summary
Two things, both about the compile-time feature gates from epic #4795:
mainin the gates-off (--no-default-features --features tokenjuice-treesitter) build.Rust Feature-Gate SmokeCI lane so that class of failure can never reachmainsilently again.Problem
voicewas the pathfinder gate (#4803) and predates the per-assert#[cfg]convention its five successors (web3/meet/mcp/skills/flows) all follow. Two asserts insrc/core/all_tests.rsreference thevoicenamespace without a#[cfg(feature = "voice")], while every sibling gate's equivalent assert is gated:harness_excludes_gated_namespaces—assert!(full_ns.contains("voice"), …)group_mapping_smoke—assert_eq!(group_for_namespace("voice"), Some(DomainGroup::Voice));group_for_namespace/ the full namespace set are registry-derived, so withvoicecompiled out there is no entry to match and both asserts fail at runtime.Why this sat red on
main: therust-feature-gate-smokelane runs a singlecargo check.cargo checknever compiles test code and never runs tests, so a runtime assert failure in the gates-off build is structurally invisible to it. Four separate agents building the other gates each hit these two failures locally and each proved them not-their-diff — the lane never caught them.Solution
The asserts (commit 1): add
#[cfg(feature = "voice")]to both, matching their siblings. Left untouched:all_tests.rs:128(namespace_description("voice")— a static match table, answers for every namespace regardless of features) and thevoiceentry in the harness absence list (absence holds in both directions — voice is runtime-filtered when the feature is on, compiled out when off).The lane (commit 2), three changes:
cargo check→cargo check --all-targets— type-checks the gates-off test/bench/integration targets too, catching compile drift intests/that a bare check never sees.core::all:: core::cli:: core::jsonrpc:: core::legacy_aliases:: agent_registry::agents::loader::.This is what actually catches the ungated-assert class. Scope is deliberate: the full gates-off
--librun aborts on a pre-existingtask_localstack overflow (agent::harness::session::tests::turn_dispatches_spawn_subagent_through_full_path) that reproduces in the gates-ON build too and OOMs unscoped — these filters route around it by scope, not by skip. Full-suite execution is deferred to CI: run the full gates-off test suite (blocked on task_local stack overflow) #5021.#[cfg]-gate a test on a domain feature equals a checked-in allowlist. When a future gate adds a gated test file, CI fails and the author must acknowledge it (and extend the scoped filter if the new module can carry a regression of this class). Stops the lane from silently under-covering as the gate surface grows.Plus
RUST_MIN_STACK(deep-async-stack belt, matching the coverage lanes) and a timeout bump (25 → 40; the lane now links and runs a test binary).--all-targetsdoes not replace the test step — it compiles test code but never runs it, so it cannot catch a runtime assert failure. The test step is the load-bearing part.Submission Checklist
52 passed; 0 failed, was50 passed; 2 failed) and still execute in the gates-on build.Closesin## Related.Impact
--all-targets) and runs a scoped test binary — a few minutes more runtime for a real gates-off regression net where there was none.Related
task_localstack overflow is fixed (blocks dropping the scoped filter)AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/5022-gates-off-smokeValidation Run
pnpm typecheck/format:check— no TypeScript or frontend files changedGGML_NATIVE=OFF cargo test --no-default-features --features tokenjuice-treesitter --lib core::all::tests→ 52 passed; 0 failed (was 50/2)--all-targetscheck (new lane step):GGML_NATIVE=OFF cargo check --no-default-features --features tokenjuice-treesitter --all-targetscargo fmt --checkclean; workflow YAML validatedSummary by CodeRabbit