feat(core): channels compile-time feature gate (#4801) — completes epic #4795#5029
feat(core): channels compile-time feature gate (#4801) — completes epic #4795#5029oxoxDev wants to merge 44 commits into
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.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07bb1df35c
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/socketio.rs (1)
559-564: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle and diagnose
chat:cancelfailures.The returned
Resultis discarded, so cancellation errors produce no client feedback or exit/error diagnostic. Match the result, log a privacy-safe outcome, and emit an error response when cancellation fails.As per coding guidelines, changed flows require grep-friendly entry/exit, branch, external-call, state-transition, and error diagnostics without secrets or full PII.
🤖 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/core/socketio.rs` around lines 559 - 564, Update the chat:cancel handling around cancel_chat_scoped to match its Result instead of discarding it. Log grep-friendly, privacy-safe entry, branch, external-call, success/state-transition, and failure diagnostics without secrets or full PII, and emit the established client error response when cancellation fails.Source: Coding guidelines
🧹 Nitpick comments (3)
src/openhuman/web_chat/web_tests.rs (1)
1-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit this 2,314-line test module by responsibility.
Separate validation, error classification, schema/cache, and concurrency lifecycle tests into focused sibling modules. This will reduce the shared import surface and make global-state ownership easier to audit.
As per coding guidelines, prefer small Unix-style modules with clear single responsibilities and a target size 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_tests.rs` around lines 1 - 31, Split the large web_tests module into focused sibling test modules organized by validation, error classification, schema/cache behavior, and concurrency lifecycle coverage. Move each related test and its helper imports together, keeping shared setup and symbols such as RUN_CHAT_TASK_TEST_LOCK accessible through a deliberate common test-support module, and preserve all existing test behavior while targeting modules of roughly 500 lines or fewer.Source: Coding guidelines
src/openhuman/web_chat/schemas.rs (1)
112-164: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd controller-boundary diagnostics.
Log grep-friendly entry, outcome, and parsing/external-call failures for each handler using only operation names and privacy-safe identifiers.
As per coding guidelines, “New or changed flows must include verbose, grep-friendly diagnostics for entry/exit, branches, external calls, retries, state transitions, and errors, without logging secrets or full PII.”
🤖 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/schemas.rs` around lines 112 - 164, Add privacy-safe, grep-friendly diagnostics to handle_chat, handle_queue_status, handle_queue_clear, and handle_cancel: log operation entry and successful outcome, and log parsing or downstream channel-call failures with operation names and non-sensitive identifiers such as client_id, thread_id, and request_id where appropriate. Keep secrets and full message contents out of logs, and ensure every error path from deserialize_params or the channel_* call is recorded before propagation.Source: Coding guidelines
src/openhuman/web_chat/progress_bridge.rs (1)
304-1457: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit the progress bridge into focused event sinks.
This module combines Socket.IO mapping, ledger persistence, tracing, heartbeat control, attribution, and lifecycle orchestration. Extract ledger, tracing, and event-mapping components while retaining a small orchestration loop.
As per coding guidelines, “Prefer small Unix-style modules with a target size of approximately 500 lines or fewer and clear single responsibilities.”
🤖 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 304 - 1457, Refactor spawn_progress_bridge so its loop remains a small orchestration layer and delegates responsibilities to focused components: extract ledger persistence, tracing/attribution setup and finalization, and AgentProgress-to-WebChannelEvent mapping into dedicated helpers or modules. Preserve heartbeat control, event ordering, lifecycle state updates, and existing behavior while keeping each resulting module near the 500-line target and using clear single-responsibility symbols.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 `@src/core/all.rs`:
- Around line 261-262: Add a single grep-friendly “[controllers]” debug
diagnostic after the controller registry is built, reporting
channels_feature_enabled and the number of registered channel controllers. Place
it in the shared post-registration path so it covers all channels-gated
branches, and ensure the feature state is represented accurately when channels
is enabled or disabled.
In `@src/openhuman/approval/README.md`:
- Line 59: Update the adjacent subscriber-location statement in the approval
README to reference the `src/openhuman/web_chat/` location, keeping it
consistent with the `ApprovalSurfaceSubscriber` reference above and removing the
outdated `channels` web provider wording.
In `@src/openhuman/learning/startup.rs`:
- Around line 49-72: Replace the OnceLock caches around EMAIL_SIG_HANDLE and
CLIENT_HANDLES with synchronized retryable state that records completion only
after subscriber registration returns valid handles; leave failed or unavailable
registrations retryable on later startup calls. Update the registration flow and
register_with_client to emit grep-friendly logs for failures, retries,
already-registered states, and successful state transitions, while preserving
the ready-path behavior. Add a regression test covering an initially absent
bus/client followed by availability and verifying Phase 2/3/4 subscriptions are
eventually registered.
In `@src/openhuman/tools/ops.rs`:
- Around line 461-468: Add debug logging around the channels-gated registrations
in the tool collection, covering both cfg(feature = "channels") and
cfg(not(feature = "channels")) branches. When enabled, record the three
WhatsApp-data tool names; when disabled, record that these tools were compiled
out, using the [tools::ops][channels] context.
In `@src/openhuman/web_chat/presentation.rs`:
- Around line 64-73: Keep optional presentation work from blocking completed
text delivery: in src/openhuman/web_chat/presentation.rs lines 64-73, add a
deadline to the reaction_handle await and continue with no emoji on timeout or
failure; in src/openhuman/web_chat/run_task.rs lines 311-337, time-bound TTS or
dispatch it independently so text delivery and session recaching proceed without
waiting for speech synthesis.
In `@src/openhuman/web_chat/progress_bridge.rs`:
- Around line 546-574: Update the run-ledger upserts in the TurnStarted,
completed, and interrupted handling paths to derive agent_id from
metadata.agent_id, falling back to "orchestrator" when absent. Replace the
hardcoded AgentRunUpsert agent IDs consistently while preserving the existing
metadata and state transitions.
- Around line 196-210: Update session_profile_user_attribution to stop returning
the stored email; use the opaque backend user ID when available, or a stable
pseudonymous hash of the email with the established hashing approach. In
SubagentFailed handling and the referenced logging sites, replace raw error
output with sanitized error classifications or safe summaries that exclude
provider/tool payloads, secrets, and full PII.
In `@src/openhuman/web_chat/run_task.rs`:
- Around line 92-104: Update build_session_fingerprint and its call from the
run-task flow to include every input used by build_session_agent: incorporate
config.default_model when no model override is present, and include the profile
locale. Ensure changes to either value produce a different fingerprint while
preserving existing override and routing inputs.
In `@src/openhuman/web_chat/session.rs`:
- Around line 129-134: Update the short_thread construction in the session flow
to truncate thread_id by Unicode characters using thread_id.chars().take(12),
rather than byte-slicing at index 12. Preserve the existing behavior for IDs of
12 characters or fewer and continue passing the result to
set_agent_definition_name.
- Around line 149-168: Update locale_reply_directive to normalize and parse the
trimmed locale as a BCP-47 tag before matching, including case-insensitive
language and region subtags. Map supported language codes and regional variants
such as es-MX, pt-BR, and zh-cn to the existing directives, while returning None
for unsupported languages.
In `@src/openhuman/web_chat/web_errors.rs`:
- Around line 668-768: Update the error classification branches around the
rate-limit, authentication, budget, and provider-error checks to recognize HTTP
statuses only when they appear in an actual provider/API error envelope or
parsed HTTP status field, rather than matching bare substrings such as “429”,
“401”, “402”, “500”, or “503”. Preserve the existing textual phrases and ensure
unrelated messages like “500 tokens” continue to reach the appropriate
classification such as context_overflow.
In `@src/openhuman/web_chat/web_tests.rs`:
- Around line 118-173: Update the affected tests, including
start_chat_emits_sanitized_chat_error_on_inference_failure and the additional
forced-hook tests, to use scoped Drop-based guards that snapshot each hook and
environment variable’s exact prior state. Restore the captured state
automatically on every exit path, including assertion panics and timeouts, while
holding FORCED_ERROR_TEST_LOCK where required. Replace cleanup that
unconditionally clears state or removes environment variables with restoration
of the original value or absence.
In `@tests/agent_harness_e2e.rs`:
- Around line 1476-1478: Update the approval-event contract comment near
ApprovalSurfaceSubscriber to reference the producer symbol in event_bus.rs
rather than the stale source range, and document the current wire shape using
the message and args fields it emits. Keep the example aligned with the actual
approval_request payload and parsing guidance.
---
Outside diff comments:
In `@src/core/socketio.rs`:
- Around line 559-564: Update the chat:cancel handling around cancel_chat_scoped
to match its Result instead of discarding it. Log grep-friendly, privacy-safe
entry, branch, external-call, success/state-transition, and failure diagnostics
without secrets or full PII, and emit the established client error response when
cancellation fails.
---
Nitpick comments:
In `@src/openhuman/web_chat/progress_bridge.rs`:
- Around line 304-1457: Refactor spawn_progress_bridge so its loop remains a
small orchestration layer and delegates responsibilities to focused components:
extract ledger persistence, tracing/attribution setup and finalization, and
AgentProgress-to-WebChannelEvent mapping into dedicated helpers or modules.
Preserve heartbeat control, event ordering, lifecycle state updates, and
existing behavior while keeping each resulting module near the 500-line target
and using clear single-responsibility symbols.
In `@src/openhuman/web_chat/schemas.rs`:
- Around line 112-164: Add privacy-safe, grep-friendly diagnostics to
handle_chat, handle_queue_status, handle_queue_clear, and handle_cancel: log
operation entry and successful outcome, and log parsing or downstream
channel-call failures with operation names and non-sensitive identifiers such as
client_id, thread_id, and request_id where appropriate. Keep secrets and full
message contents out of logs, and ensure every error path from
deserialize_params or the channel_* call is recorded before propagation.
In `@src/openhuman/web_chat/web_tests.rs`:
- Around line 1-31: Split the large web_tests module into focused sibling test
modules organized by validation, error classification, schema/cache behavior,
and concurrency lifecycle coverage. Move each related test and its helper
imports together, keeping shared setup and symbols such as
RUN_CHAT_TASK_TEST_LOCK accessible through a deliberate common test-support
module, and preserve all existing test behavior while targeting modules of
roughly 500 lines or fewer.
🪄 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: 8f0b9d39-b181-4832-88db-525e052f184d
📒 Files selected for processing (90)
.github/workflows/ci-lite.ymlAGENTS.mdCargo.tomlapp/src-tauri/Cargo.tomlapp/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/legacy_aliases.rssrc/core/observability.rssrc/core/runtime/context.rssrc/core/runtime/services.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/audio_toolkit/ops.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/bus.rssrc/openhuman/cron/scheduler.rssrc/openhuman/cron/scheduler_tests.rssrc/openhuman/flows/ops.rssrc/openhuman/inference/provider/error_code.rssrc/openhuman/learning/mod.rssrc/openhuman/learning/startup.rssrc/openhuman/meet_agent/brain/llm.rssrc/openhuman/memory_conversations/bus.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/tools/mod.rssrc/openhuman/tools/ops.rssrc/openhuman/tools/ops_tests.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 (1)
- src/openhuman/channels/providers/mod.rs
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: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/socketio.rs (1)
559-564: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle and diagnose
chat:cancelfailures.The returned
Resultis discarded, so cancellation errors produce no client feedback or exit/error diagnostic. Match the result, log a privacy-safe outcome, and emit an error response when cancellation fails.As per coding guidelines, changed flows require grep-friendly entry/exit, branch, external-call, state-transition, and error diagnostics without secrets or full PII.
🤖 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/core/socketio.rs` around lines 559 - 564, Update the chat:cancel handling around cancel_chat_scoped to match its Result instead of discarding it. Log grep-friendly, privacy-safe entry, branch, external-call, success/state-transition, and failure diagnostics without secrets or full PII, and emit the established client error response when cancellation fails.Source: Coding guidelines
🧹 Nitpick comments (3)
src/openhuman/web_chat/web_tests.rs (1)
1-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit this 2,314-line test module by responsibility.
Separate validation, error classification, schema/cache, and concurrency lifecycle tests into focused sibling modules. This will reduce the shared import surface and make global-state ownership easier to audit.
As per coding guidelines, prefer small Unix-style modules with clear single responsibilities and a target size 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_tests.rs` around lines 1 - 31, Split the large web_tests module into focused sibling test modules organized by validation, error classification, schema/cache behavior, and concurrency lifecycle coverage. Move each related test and its helper imports together, keeping shared setup and symbols such as RUN_CHAT_TASK_TEST_LOCK accessible through a deliberate common test-support module, and preserve all existing test behavior while targeting modules of roughly 500 lines or fewer.Source: Coding guidelines
src/openhuman/web_chat/schemas.rs (1)
112-164: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd controller-boundary diagnostics.
Log grep-friendly entry, outcome, and parsing/external-call failures for each handler using only operation names and privacy-safe identifiers.
As per coding guidelines, “New or changed flows must include verbose, grep-friendly diagnostics for entry/exit, branches, external calls, retries, state transitions, and errors, without logging secrets or full PII.”
🤖 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/schemas.rs` around lines 112 - 164, Add privacy-safe, grep-friendly diagnostics to handle_chat, handle_queue_status, handle_queue_clear, and handle_cancel: log operation entry and successful outcome, and log parsing or downstream channel-call failures with operation names and non-sensitive identifiers such as client_id, thread_id, and request_id where appropriate. Keep secrets and full message contents out of logs, and ensure every error path from deserialize_params or the channel_* call is recorded before propagation.Source: Coding guidelines
src/openhuman/web_chat/progress_bridge.rs (1)
304-1457: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit the progress bridge into focused event sinks.
This module combines Socket.IO mapping, ledger persistence, tracing, heartbeat control, attribution, and lifecycle orchestration. Extract ledger, tracing, and event-mapping components while retaining a small orchestration loop.
As per coding guidelines, “Prefer small Unix-style modules with a target size of approximately 500 lines or fewer and clear single responsibilities.”
🤖 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 304 - 1457, Refactor spawn_progress_bridge so its loop remains a small orchestration layer and delegates responsibilities to focused components: extract ledger persistence, tracing/attribution setup and finalization, and AgentProgress-to-WebChannelEvent mapping into dedicated helpers or modules. Preserve heartbeat control, event ordering, lifecycle state updates, and existing behavior while keeping each resulting module near the 500-line target and using clear single-responsibility symbols.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 `@src/core/all.rs`:
- Around line 261-262: Add a single grep-friendly “[controllers]” debug
diagnostic after the controller registry is built, reporting
channels_feature_enabled and the number of registered channel controllers. Place
it in the shared post-registration path so it covers all channels-gated
branches, and ensure the feature state is represented accurately when channels
is enabled or disabled.
In `@src/openhuman/approval/README.md`:
- Line 59: Update the adjacent subscriber-location statement in the approval
README to reference the `src/openhuman/web_chat/` location, keeping it
consistent with the `ApprovalSurfaceSubscriber` reference above and removing the
outdated `channels` web provider wording.
In `@src/openhuman/learning/startup.rs`:
- Around line 49-72: Replace the OnceLock caches around EMAIL_SIG_HANDLE and
CLIENT_HANDLES with synchronized retryable state that records completion only
after subscriber registration returns valid handles; leave failed or unavailable
registrations retryable on later startup calls. Update the registration flow and
register_with_client to emit grep-friendly logs for failures, retries,
already-registered states, and successful state transitions, while preserving
the ready-path behavior. Add a regression test covering an initially absent
bus/client followed by availability and verifying Phase 2/3/4 subscriptions are
eventually registered.
In `@src/openhuman/tools/ops.rs`:
- Around line 461-468: Add debug logging around the channels-gated registrations
in the tool collection, covering both cfg(feature = "channels") and
cfg(not(feature = "channels")) branches. When enabled, record the three
WhatsApp-data tool names; when disabled, record that these tools were compiled
out, using the [tools::ops][channels] context.
In `@src/openhuman/web_chat/presentation.rs`:
- Around line 64-73: Keep optional presentation work from blocking completed
text delivery: in src/openhuman/web_chat/presentation.rs lines 64-73, add a
deadline to the reaction_handle await and continue with no emoji on timeout or
failure; in src/openhuman/web_chat/run_task.rs lines 311-337, time-bound TTS or
dispatch it independently so text delivery and session recaching proceed without
waiting for speech synthesis.
In `@src/openhuman/web_chat/progress_bridge.rs`:
- Around line 546-574: Update the run-ledger upserts in the TurnStarted,
completed, and interrupted handling paths to derive agent_id from
metadata.agent_id, falling back to "orchestrator" when absent. Replace the
hardcoded AgentRunUpsert agent IDs consistently while preserving the existing
metadata and state transitions.
- Around line 196-210: Update session_profile_user_attribution to stop returning
the stored email; use the opaque backend user ID when available, or a stable
pseudonymous hash of the email with the established hashing approach. In
SubagentFailed handling and the referenced logging sites, replace raw error
output with sanitized error classifications or safe summaries that exclude
provider/tool payloads, secrets, and full PII.
In `@src/openhuman/web_chat/run_task.rs`:
- Around line 92-104: Update build_session_fingerprint and its call from the
run-task flow to include every input used by build_session_agent: incorporate
config.default_model when no model override is present, and include the profile
locale. Ensure changes to either value produce a different fingerprint while
preserving existing override and routing inputs.
In `@src/openhuman/web_chat/session.rs`:
- Around line 129-134: Update the short_thread construction in the session flow
to truncate thread_id by Unicode characters using thread_id.chars().take(12),
rather than byte-slicing at index 12. Preserve the existing behavior for IDs of
12 characters or fewer and continue passing the result to
set_agent_definition_name.
- Around line 149-168: Update locale_reply_directive to normalize and parse the
trimmed locale as a BCP-47 tag before matching, including case-insensitive
language and region subtags. Map supported language codes and regional variants
such as es-MX, pt-BR, and zh-cn to the existing directives, while returning None
for unsupported languages.
In `@src/openhuman/web_chat/web_errors.rs`:
- Around line 668-768: Update the error classification branches around the
rate-limit, authentication, budget, and provider-error checks to recognize HTTP
statuses only when they appear in an actual provider/API error envelope or
parsed HTTP status field, rather than matching bare substrings such as “429”,
“401”, “402”, “500”, or “503”. Preserve the existing textual phrases and ensure
unrelated messages like “500 tokens” continue to reach the appropriate
classification such as context_overflow.
In `@src/openhuman/web_chat/web_tests.rs`:
- Around line 118-173: Update the affected tests, including
start_chat_emits_sanitized_chat_error_on_inference_failure and the additional
forced-hook tests, to use scoped Drop-based guards that snapshot each hook and
environment variable’s exact prior state. Restore the captured state
automatically on every exit path, including assertion panics and timeouts, while
holding FORCED_ERROR_TEST_LOCK where required. Replace cleanup that
unconditionally clears state or removes environment variables with restoration
of the original value or absence.
In `@tests/agent_harness_e2e.rs`:
- Around line 1476-1478: Update the approval-event contract comment near
ApprovalSurfaceSubscriber to reference the producer symbol in event_bus.rs
rather than the stale source range, and document the current wire shape using
the message and args fields it emits. Keep the example aligned with the actual
approval_request payload and parsing guidance.
---
Outside diff comments:
In `@src/core/socketio.rs`:
- Around line 559-564: Update the chat:cancel handling around cancel_chat_scoped
to match its Result instead of discarding it. Log grep-friendly, privacy-safe
entry, branch, external-call, success/state-transition, and failure diagnostics
without secrets or full PII, and emit the established client error response when
cancellation fails.
---
Nitpick comments:
In `@src/openhuman/web_chat/progress_bridge.rs`:
- Around line 304-1457: Refactor spawn_progress_bridge so its loop remains a
small orchestration layer and delegates responsibilities to focused components:
extract ledger persistence, tracing/attribution setup and finalization, and
AgentProgress-to-WebChannelEvent mapping into dedicated helpers or modules.
Preserve heartbeat control, event ordering, lifecycle state updates, and
existing behavior while keeping each resulting module near the 500-line target
and using clear single-responsibility symbols.
In `@src/openhuman/web_chat/schemas.rs`:
- Around line 112-164: Add privacy-safe, grep-friendly diagnostics to
handle_chat, handle_queue_status, handle_queue_clear, and handle_cancel: log
operation entry and successful outcome, and log parsing or downstream
channel-call failures with operation names and non-sensitive identifiers such as
client_id, thread_id, and request_id where appropriate. Keep secrets and full
message contents out of logs, and ensure every error path from
deserialize_params or the channel_* call is recorded before propagation.
In `@src/openhuman/web_chat/web_tests.rs`:
- Around line 1-31: Split the large web_tests module into focused sibling test
modules organized by validation, error classification, schema/cache behavior,
and concurrency lifecycle coverage. Move each related test and its helper
imports together, keeping shared setup and symbols such as
RUN_CHAT_TASK_TEST_LOCK accessible through a deliberate common test-support
module, and preserve all existing test behavior while targeting modules of
roughly 500 lines or fewer.
🪄 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: 8f0b9d39-b181-4832-88db-525e052f184d
📒 Files selected for processing (90)
.github/workflows/ci-lite.ymlAGENTS.mdCargo.tomlapp/src-tauri/Cargo.tomlapp/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/legacy_aliases.rssrc/core/observability.rssrc/core/runtime/context.rssrc/core/runtime/services.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/audio_toolkit/ops.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/bus.rssrc/openhuman/cron/scheduler.rssrc/openhuman/cron/scheduler_tests.rssrc/openhuman/flows/ops.rssrc/openhuman/inference/provider/error_code.rssrc/openhuman/learning/mod.rssrc/openhuman/learning/startup.rssrc/openhuman/meet_agent/brain/llm.rssrc/openhuman/memory_conversations/bus.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/tools/mod.rssrc/openhuman/tools/ops.rssrc/openhuman/tools/ops_tests.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 (1)
- src/openhuman/channels/providers/mod.rs
🛑 Comments failed to post (13)
src/core/all.rs (1)
261-262: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add a controller-registration diagnostic for the channels gate.
These changed registration branches have no observable enabled/disabled state. Emit one grep-friendly
[controllers]debug record withchannels_feature_enabledand the registered channel-controller count after building the registry.Also applies to: 393-399, 718-719, 765-766, 831-832
🤖 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/core/all.rs` around lines 261 - 262, Add a single grep-friendly “[controllers]” debug diagnostic after the controller registry is built, reporting channels_feature_enabled and the number of registered channel controllers. Place it in the shared post-registration path so it covers all channels-gated branches, and ensure the feature state is represented accurately when channels is enabled or disabled.Source: Coding guidelines
src/openhuman/approval/README.md (1)
59-59: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the adjacent subscriber-location statement.
Line 62 still says the subscriber lives in the
channelsweb provider, contradicting this relocation tosrc/openhuman/web_chat/.Proposed documentation fix
-No `bus.rs` in this module — it only publishes; the subscriber lives in the `channels` web provider. +No `bus.rs` in this module — it only publishes; the subscriber lives in `src/openhuman/web_chat/event_bus.rs`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.No `bus.rs` in this module — it only publishes; the subscriber lives in `src/openhuman/web_chat/event_bus.rs`.🤖 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 adjacent subscriber-location statement in the approval README to reference the `src/openhuman/web_chat/` location, keeping it consistent with the `ApprovalSurfaceSubscriber` reference above and removing the outdated `channels` web provider wording.src/openhuman/learning/startup.rs (1)
49-72: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not permanently cache failed subscriber registration.
get_or_initstoresNonewhen the bus or memory client is unavailable, so subsequent calls cannot retry after initialization. This can permanently disable Phase 2/3/4 learning, despite Line 95 stating it will resume once the client initializes.Use retryable synchronized state, only mark registration complete after successful handles are obtained, and add an absent-then-ready regression test. Also log failed Phase 3/4 subscriptions and retry/already-registered branches.
As per coding guidelines, new Rust flows require verbose, grep-friendly diagnostics for branches, retries, state transitions, and errors.
Also applies to: 92-103, 125-151, 261-270
🤖 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/learning/startup.rs` around lines 49 - 72, Replace the OnceLock caches around EMAIL_SIG_HANDLE and CLIENT_HANDLES with synchronized retryable state that records completion only after subscriber registration returns valid handles; leave failed or unavailable registrations retryable on later startup calls. Update the registration flow and register_with_client to emit grep-friendly logs for failures, retries, already-registered states, and successful state transitions, while preserving the ready-path behavior. Add a regression test covering an initially absent bus/client followed by availability and verifying Phase 2/3/4 subscriptions are eventually registered.Source: Coding guidelines
src/openhuman/tools/ops.rs (1)
461-468: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Log the channels-dependent tool registration branch.
Add a
[tools::ops][channels]debug record for both feature states, including the three registered WhatsApp-data tool names or that they were compiled out. This makes slim-build tool-surface differences diagnosable.🤖 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 461 - 468, Add debug logging around the channels-gated registrations in the tool collection, covering both cfg(feature = "channels") and cfg(not(feature = "channels")) branches. When enabled, record the three WhatsApp-data tool names; when disabled, record that these tools were compiled out, using the [tools::ops][channels] context.Source: Coding guidelines
src/openhuman/web_chat/presentation.rs (1)
64-73: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Keep optional presentation work off the terminal response path.
Both emoji inference and speech synthesis are awaited without deadlines after the agent has already produced its answer. A stalled auxiliary backend therefore prevents the user from receiving completed text.
src/openhuman/web_chat/presentation.rs#L64-L73: time-bound reaction inference and continue without an emoji when unavailable.src/openhuman/web_chat/run_task.rs#L311-L337: time-bound or independently dispatch TTS so text delivery and session recaching proceed.📍 Affects 2 files
src/openhuman/web_chat/presentation.rs#L64-L73(this comment)src/openhuman/web_chat/run_task.rs#L311-L337🤖 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 - 73, Keep optional presentation work from blocking completed text delivery: in src/openhuman/web_chat/presentation.rs lines 64-73, add a deadline to the reaction_handle await and continue with no emoji on timeout or failure; in src/openhuman/web_chat/run_task.rs lines 311-337, time-bound TTS or dispatch it independently so text delivery and session recaching proceed without waiting for speech synthesis.src/openhuman/web_chat/progress_bridge.rs (2)
196-210: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Keep full PII and raw errors out of observability data.
The tracing fallback prefers a stored email address and exports it as
user_id;SubagentFailedalso logs the raw error, which may contain provider/tool payloads. Use an opaque backend ID or pseudonymous hash and log only sanitized classifications.As per coding guidelines, diagnostics must be emitted “without logging secrets or full PII.”
Also applies to: 365-376, 525-535
🤖 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 - 210, Update session_profile_user_attribution to stop returning the stored email; use the opaque backend user ID when available, or a stable pseudonymous hash of the email with the established hashing approach. In SubagentFailed handling and the referenced logging sites, replace raw error output with sanitized error classifications or safe summaries that exclude provider/tool payloads, secrets, and full PII.Source: Coding guidelines
546-574: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Persist the resolved agent ID in every run-ledger state.
run_task.rsstampsmetadata.agent_id, but running, completed, and interrupted rows all hardcode"orchestrator". Profile and task-agent turns are therefore permanently misattributed. Usemetadata.agent_idwith an orchestrator fallback consistently.Also applies to: 1287-1315, 1382-1404
🤖 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 546 - 574, Update the run-ledger upserts in the TurnStarted, completed, and interrupted handling paths to derive agent_id from metadata.agent_id, falling back to "orchestrator" when absent. Replace the hardcoded AgentRunUpsert agent IDs consistently while preserving the existing metadata and state transitions.src/openhuman/web_chat/run_task.rs (1)
92-104: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Include every agent-construction input in the session fingerprint.
The fingerprint derives routing only from the explicit override, while
build_session_agentfalls back toconfig.default_model; it also applieslocale, which is not fingerprinted. Changing either value can therefore reuse an agent with stale routing or language instructions.🤖 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, Update build_session_fingerprint and its call from the run-task flow to include every input used by build_session_agent: incorporate config.default_model when no model override is present, and include the profile locale. Ensure changes to either value produce a different fingerprint while preserving existing override and routing inputs.src/openhuman/web_chat/session.rs (2)
129-134: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Avoid byte-slicing the caller-provided thread ID.
&thread_id[..12]panics when byte 12 is not a UTF-8 boundary. Build the suffix withthread_id.chars().take(12)instead.🤖 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 flow to truncate thread_id by Unicode characters using thread_id.chars().take(12), rather than byte-slicing at index 12. Preserve the existing behavior for IDs of 12 characters or fewer and continue passing the result to set_agent_definition_name.
149-168: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Normalize BCP-47 locales before matching.
Exact, case-sensitive matching silently ignores valid tags such as
es-MX,pt-BR, andzh-cn. Normalize casing and parse the language/region subtags before selecting the directive.🤖 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 149 - 168, Update locale_reply_directive to normalize and parse the trimmed locale as a BCP-47 tag before matching, including case-insensitive language and region subtags. Map supported language codes and regional variants such as es-MX, pt-BR, and zh-cn to the existing directives, while returning None for unsupported languages.src/openhuman/web_chat/web_errors.rs (1)
668-768: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Anchor HTTP status checks to actual error envelopes.
Bare checks such as
contains("429"),"401","402","500", and"503"match unrelated values. For example, a context error mentioning “500 tokens” is classified asprovider_errorbefore reachingcontext_overflow. Parse provider/HTTP status framing instead.🤖 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 - 768, Update the error classification branches around the rate-limit, authentication, budget, and provider-error checks to recognize HTTP statuses only when they appear in an actual provider/API error envelope or parsed HTTP status field, rather than matching bare substrings such as “429”, “401”, “402”, “500”, or “503”. Preserve the existing textual phrases and ensure unrelated messages like “500 tokens” continue to reach the appropriate classification such as context_overflow.src/openhuman/web_chat/web_tests.rs (1)
118-173: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Restore process-global test state on every exit path.
These tests clear forced-error/forced-block hooks only after all assertions succeed. A panic releases the shared lock but leaves the hook active for the next test. The timeout guard also removes the variable instead of restoring a pre-existing value.
Use scoped guards that snapshot and restore the exact previous hook/environment state during
Drop, including unwind paths.Also applies to: 647-822, 1935-2191
🤖 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_tests.rs` around lines 118 - 173, Update the affected tests, including start_chat_emits_sanitized_chat_error_on_inference_failure and the additional forced-hook tests, to use scoped Drop-based guards that snapshot each hook and environment variable’s exact prior state. Restore the captured state automatically on every exit path, including assertion panics and timeouts, while holding FORCED_ERROR_TEST_LOCK where required. Replace cleanup that unconditionally clears state or removes environment variables with restoration of the original value or absence.tests/agent_harness_e2e.rs (1)
1476-1478: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the approval-event contract comment.
The cited range no longer contains the producer, and
ApprovalSurfaceSubscriberbuildsmessage/argsfields rather thanaction_summary/args_redacted. Reference the producer symbol and document the current wire shape so this parsing guidance remains accurate.🤖 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 `@tests/agent_harness_e2e.rs` around lines 1476 - 1478, Update the approval-event contract comment near ApprovalSurfaceSubscriber to reference the producer symbol in event_bus.rs rather than the stale source range, and document the current wire shape using the message and args fields it emits. Keep the example aligned with the actual approval_request payload and parsing guidance.
# Conflicts: # .github/workflows/ci-lite.yml # src/openhuman/channels/mod.rs
| // (CodeRabbit review on #4746). | ||
| use super::RUN_CHAT_TASK_TEST_LOCK as FORCED_ERROR_TEST_LOCK; | ||
|
|
||
| struct ForcedErrorGuard { |
There was a problem hiding this comment.
Deferred from the CodeRabbit nitpick: splitting this 2.3k-line test module is a broad mechanical refactor with substantial conflict risk and no bearing on the channels feature gate or the correctness fixes in this PR. The touched test helpers now restore global hooks exactly and have focused regression coverage; the file split should be tracked as a separate cleanup PR.
| use std::collections::HashMap; | ||
|
|
||
| tokio::spawn(async move { | ||
| let resolved_agent_id = metadata |
There was a problem hiding this comment.
Deferred from the CodeRabbit nitpick: splitting this progress bridge into submodules is a broad architectural refactor beyond the channels-gate scope. This PR keeps the functional changes narrow (privacy-safe attribution/error payloads and resolved-agent ledger attribution) and validates them independently; module decomposition should be handled in a dedicated follow-up.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddc4887341
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/openhuman/web_chat/session.rs (1)
146-158: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve Traditional Chinese locale variants.
Matching only the base language makes
zh-TW,zh-HK, andzh-Hantinstruct the agent to respond in Simplified Chinese. Handle Traditional Chinese script/region subtags before the base-language fallback and add assertions for those variants.🤖 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 146 - 158, Update the locale matching logic around normalized and language_code so Traditional Chinese script and region variants (zh-TW, zh-HK, and zh-Hant) resolve to Traditional Chinese before the generic zh fallback. Add assertions covering each specified variant, while preserving Simplified Chinese for other zh locales.
🤖 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/all.rs`:
- Around line 820-833: Update the controller filter used for channel diagnostics
to include the ungated "channel" namespace alongside the existing
channel-related namespaces, or use DomainGroup::Channels if that is the
established grouping. Keep the registered_channel_controllers log accurate when
the channels feature is disabled.
In `@src/openhuman/web_chat/presentation.rs`:
- Around line 73-87: Add stable, content-free diagnostics around the reaction
handling match: log when reaction inference starts, and log successful
completion for both an emoji result and a no-reaction result. Preserve the
existing timeout and task-failure warnings, and keep all diagnostics
grep-friendly without including response content.
- Around line 73-87: Update the reaction handling around reaction_handle and the
REACTION_DEADLINE timeout so the JoinHandle remains available when the deadline
expires, then call abort() in the Err(_) timeout branch before returning None.
Preserve the existing behavior and warning for task failures and timeouts.
---
Outside diff comments:
In `@src/openhuman/web_chat/session.rs`:
- Around line 146-158: Update the locale matching logic around normalized and
language_code so Traditional Chinese script and region variants (zh-TW, zh-HK,
and zh-Hant) resolve to Traditional Chinese before the generic zh fallback. Add
assertions covering each specified variant, while preserving Simplified Chinese
for other zh locales.
🪄 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: 493b2a1d-b78b-483d-9e70-dae4b67f9c97
📒 Files selected for processing (18)
.github/workflows/ci-lite.ymlsrc/core/all.rssrc/core/event_bus/subscriber.rssrc/core/socketio.rssrc/openhuman/approval/README.mdsrc/openhuman/learning/startup.rssrc/openhuman/tools/ops.rssrc/openhuman/web_chat/ops.rssrc/openhuman/web_chat/presentation.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/raw_coverage/composio_credentials_state_raw_coverage_e2e.rs
🚧 Files skipped from review as they are similar to previous changes (9)
- src/openhuman/tools/ops.rs
- src/openhuman/approval/README.md
- tests/agent_harness_e2e.rs
- src/openhuman/web_chat/run_task.rs
- src/openhuman/web_chat/schemas.rs
- src/openhuman/web_chat/types.rs
- src/openhuman/web_chat/ops.rs
- src/openhuman/learning/startup.rs
- src/openhuman/web_chat/web_tests.rs
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/web_chat/event_bus.rs`:
- Around line 523-538: Update both event-receive loops around the halted-event
wait to wrap each receive in a timeout and handle broadcast results explicitly:
continue on RecvError::Lagged, terminate cleanly on RecvError::Closed, and fail
with a clear timeout when the expected event does not arrive. Preserve the
existing source-filtering behavior and AutomationHalted event matching.
🪄 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: 5ac3287b-85e8-40b4-8cc3-c852dbabbec2
📒 Files selected for processing (1)
src/openhuman/web_chat/event_bus.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 317a415485
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
🥞 Stacked PR — merge order (bottom → top)
This is position 5/5 — the last child of epic #4795. Stacked on all four below, so the diff shows their commits until each merges; they self-clean bottom-up. Review the
#4801-scoped commits (the 9feat/refactor/test/ci(…#4801)ones). #4801 is possible in one PR only because #5002 moved the web-chat conduit out ofchannels/and #5025 moved the learning subscribers out.Summary
Adds the default-ON
channelsCargo feature that compiles out the channels domain (all chat providers + the webview provider modules +whatsapp_data) in slim/harness builds. Desktop is byte-identical. This completes epic #4795 (compile-time gates for every core subsystem).Problem
channelswas the last un-gated subsystem. It couldn't be gated cleanly until two entanglements were removed (both now merged/stacked below): the in-app web-chat conduit lived insidechannels/providers/web/(#5002 relocated it to always-onopenhuman::web_chat), and three always-onlearningsubscribers were wired insidestart_channels(#5025 moved them to the Platform boot path).The remaining apparent blocker —
start_channelswiring ~18 non-channel things (event-bus init, agent handlers, task pollers, native handlers, …) — is verified redundant: its sole caller isspawn_channels_service, and every one of those registrations is also performed idempotently inbootstrap_core_runtime/register_domain_subscribers/start_bootstrap_jobs. So the module compiles out without dissection.Solution
Follows the established sibling-gate patterns (AGENTS.md "Compile-time domain gates"):
channels/mod.rs):traits(1-linetinychannelsre-export) andcli(CliChannel— dep-free local stdin/stdout REPL used by the always-on interactive harness) stay ungated; everything else is#[cfg(feature = "channels")]. Precedent:meet_agent::wav.tinychannels(cron/bus.rs,memory_conversations/bus.rs,audio_toolkit/ops.rs) — these named channel types/impls that actually live in the dep crate. This also removes the voice→channels cross-gate edge.webview_accounts,webview_apis,webview_notifications,whatsapp_data) whose only external references are registration sites.all.rs— but notweb_chat's (the in-app chat is not compile-gated). Dual-cfg the runtime wiring injsonrpc.rs,runtime/services.rs,runtime/context.rs, and the whatsapp_data agent tools.channelsto the desktop shell (app/src-tauri/Cargo.toml) — the Voice transcription unavailable — 'Voice transcription is unavailable in this build' error on latest release #4901 rule; the Feature Forwarding Gate lane confirms it.all_tests.rs(registered-when-on / absent-when-off, the OFF test also asserting thechannelweb-chat namespace survives — pinning refactor(core): relocate the web chat conduit out of channels/ #5002's decoupling), the whatsapp_data tool absence pinned intools/ops_tests.rs, and thelegacy_aliases.rsdrift guards extended to ignore gated namespaces when channels is off (the mcp gate'sis_compiled_out_methodpattern). Extends the fix(core): unbreak the gates-off build and make the smoke lane run tests (#5022) #5023 smoke-lane guard regex to coverchannels.Scope correction (DoD)
The issue's "exclusive dependencies drop out of the build" line is superseded, same as the mcp/meet gates. Gating
channelssheds zero deps —tinychannelsstays load-bearing for always-on code (config schema, theDomainEventinbound envelope, security pairing), so its provider crates compile unconditionally. This gate's value is compile-time surface, RPC/tool surface, and startup cost. Real dep-shedding needs atinychannelsprovider-feature split (filed as follow-up).whatsapp-webbecomes a refinement of the gate (whatsapp-web = ["channels", "tinychannels/whatsapp-web"]).Submission Checklist
web_chat'schannelnamespace survives channels-off.channelsindefaultand in the desktop shell's feature list;node scripts/ci/check-feature-forwarding.mjs→ OK (Core 9 / Shell 9).channelsON.Closesin## Related.Impact
check/clippy -D warnings/fmtclean; gates-off compiles; channels-on-only slim compiles; voice-on/channels-off cross-gate compiles (audio_toolkit retarget); app shell compiles + forwards; both-ways + scoped gates-off tests pass (231/0). Theweb_chatpush inall.rsis deliberately left ungated (NOTE in code).Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
feat/4801-channels-feature-gateValidation Run
pnpm typecheck/format:check— no TypeScript or frontend files changedcargo fmt --checkclean; defaultcargo check+cargo clippy -- -D warningscleanGGML_NATIVE=OFF cargo check --no-default-features --features tokenjuice-treesitter(0 errors)node scripts/ci/check-feature-forwarding.mjs→ OK, channels forwardedcore::all::tests56 passedchannelsaddedSummary by CodeRabbit