refactor(core): relocate the web chat conduit out of channels/ (#5002)#5004
Conversation
📝 WalkthroughWalkthroughThe web chat conduit is relocated from ChangesWeb chat conduit relocation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/openhuman/channels/bus.rs (1)
92-101: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the stale web-chat diagnostic source label.
start_chatnow comes fromopenhuman::web_chat, butsrc/openhuman/web_chat/ops.rsstill reports the prompt-enforcement source aschannels.providers.web.start_chat. Update that label to the canonicalweb_chatpath so logs and security diagnostics do not retain the removed module reference.As per coding guidelines, new or changed flows must include verbose, grep-friendly diagnostics; the relocation also requires stale old-path references to be removed.
🤖 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/channels/bus.rs` around lines 92 - 101, Update the prompt-enforcement diagnostic source label used by start_chat in openhuman::web_chat to the canonical web_chat path, removing the stale channels.providers.web.start_chat reference. Keep the existing diagnostic behavior and ensure no remaining logs or security diagnostics retain the removed module path.Source: Coding guidelines
🧹 Nitpick comments (3)
src/openhuman/web_chat/web_tests.rs (2)
1935-1982: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftProve that both turn futures execute concurrently.
These tests only observe entries registered in
IN_FLIGHT/PARALLEL_IN_FLIGHT. They can pass even if execution is serialized after registration; the sharedstarted: AtomicBoolproves only that at least one task was polled. Use an active-task counter or two-party barrier and wait until both parked futures have entered concurrently.As per coding guidelines, changed behavior must be proved in Rust.
Also applies to: 2131-2181
🤖 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 1935 - 1982, Strengthen start_chat_runs_distinct_threads_concurrently and the analogous test around the shared started AtomicBool so the assertion observes execution, not only IN_FLIGHT registration. Add an active-task counter or two-party barrier to run_chat_task and wait until both distinct-thread futures are simultaneously parked inside execution, then assert that condition before completing cleanup.Source: Coding guidelines
1-12: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit the oversized relocated web-chat modules into focused files.
src/openhuman/web_chat/web_tests.rs#L1-L12: separate classifier, schemas, session cache, and concurrency tests.src/openhuman/web_chat/web_errors.rs#L1-L14: separate parsing, predicates, managed-code classification, and user-facing messages.As per coding guidelines, prefer Rust 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_tests.rs` around lines 1 - 12, Split the oversized web-chat modules into focused Rust files of approximately 500 lines or fewer: in src/openhuman/web_chat/web_tests.rs lines 1-12, separate classifier, schema, session-cache, and concurrency tests while preserving their existing behavior and imports; in src/openhuman/web_chat/web_errors.rs lines 1-14, separate parsing, predicate, managed-code classification, and user-facing message logic into focused modules, updating module declarations and imports accordingly.Source: Coding guidelines
src/openhuman/web_chat/progress_bridge.rs (1)
308-1457: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit the bridge orchestration into focused modules.
spawn_progress_bridgecombines every parent/subagent lifecycle branch, ledger persistence, wire presentation, heartbeats, and trace export in one 1,149-line function. Extract event handlers and move 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/progress_bridge.rs` around lines 308 - 1457, Refactor spawn_progress_bridge into focused sibling modules, extracting parent and subagent event handling, ledger persistence, wire-event publishing, heartbeat processing, and trace-export finalization while preserving behavior and shared state. Keep spawn_progress_bridge as the orchestration loop that delegates to these handlers, and move related tests into the corresponding modules; target files of approximately 500 lines or fewer.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/openhuman/approval/README.md`:
- Line 59: Update the architecture ownership statement for
ApprovalSurfaceSubscriber in the approval README to reference
src/openhuman/web_chat/, and remove the obsolete “No bus” wording while
preserving the surrounding documentation.
In `@src/openhuman/web_chat/presentation.rs`:
- Around line 220-225: Update segment_for_delivery and the additionally
referenced threshold and delay logic to use Unicode character counts via
chars().count() instead of str::len(), preserving the existing 80/40-character
behavior for actual characters. Add a boundary test covering multibyte CJK or
Arabic text at the relevant thresholds.
- Around line 64-73: The delivery flow around reaction_handle currently blocks
on optional reaction inference; wrap the reaction_handle await in a short
timeout, and if it expires, abort the task, log the timeout, and continue with
no reaction emoji. Preserve normal successful results and ensure segment and
chat_done delivery proceed without waiting indefinitely for try_reaction.
In `@src/openhuman/web_chat/session.rs`:
- Around line 182-203: Update build_session_fingerprint and
SessionCacheFingerprint to include the same normalized locale/directive used
when constructing the agent prompt, ensuring locale changes produce a distinct
fingerprint. Add or extend the session cache test to verify changing locale
causes a cache miss and agent rebuild.
- Around line 129-134: Update the short_thread calculation in the session
handling flow to truncate thread_id at a valid UTF-8 character boundary rather
than slicing at byte index 12. Preserve the 12-character maximum when possible,
avoid panics for multibyte thread IDs, and keep the existing
agent.set_agent_definition_name behavior.
In `@src/openhuman/web_chat/types.rs`:
- Around line 15-35: The session fingerprint must mirror all effective
agent-build inputs. In src/openhuman/web_chat/types.rs lines 15-35, add a
normalized locale field to SessionCacheFingerprint; in
src/openhuman/web_chat/run_task.rs lines 92-104, derive the provider role from
the model override or config.default_model and populate the locale fingerprint.
Add tests verifying locale changes and effective provider-binding changes force
a session rebuild.
In `@src/openhuman/web_chat/web_errors.rs`:
- Around line 739-750: Update the ClassifiedError construction to select message
and source together based on provider: retain the OpenHuman billing message for
provider "openhuman" or None, and use provider-specific upstream billing copy
for other providers while keeping source "provider". Pass the selected message
through with_provider_detail and preserve the existing retryability and metadata
fields.
---
Outside diff comments:
In `@src/openhuman/channels/bus.rs`:
- Around line 92-101: Update the prompt-enforcement diagnostic source label used
by start_chat in openhuman::web_chat to the canonical web_chat path, removing
the stale channels.providers.web.start_chat reference. Keep the existing
diagnostic behavior and ensure no remaining logs or security diagnostics retain
the removed module path.
---
Nitpick comments:
In `@src/openhuman/web_chat/progress_bridge.rs`:
- Around line 308-1457: Refactor spawn_progress_bridge into focused sibling
modules, extracting parent and subagent event handling, ledger persistence,
wire-event publishing, heartbeat processing, and trace-export finalization while
preserving behavior and shared state. Keep spawn_progress_bridge as the
orchestration loop that delegates to these handlers, and move related tests into
the corresponding modules; target files of approximately 500 lines or fewer.
In `@src/openhuman/web_chat/web_tests.rs`:
- Around line 1935-1982: Strengthen
start_chat_runs_distinct_threads_concurrently and the analogous test around the
shared started AtomicBool so the assertion observes execution, not only
IN_FLIGHT registration. Add an active-task counter or two-party barrier to
run_chat_task and wait until both distinct-thread futures are simultaneously
parked inside execution, then assert that condition before completing cleanup.
- Around line 1-12: Split the oversized web-chat modules into focused Rust files
of approximately 500 lines or fewer: in src/openhuman/web_chat/web_tests.rs
lines 1-12, separate classifier, schema, session-cache, and concurrency tests
while preserving their existing behavior and imports; in
src/openhuman/web_chat/web_errors.rs lines 1-14, separate parsing, predicate,
managed-code classification, and user-facing message logic into focused modules,
updating module declarations and imports accordingly.
🪄 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: ad14058d-aade-4b54-96b3-461e8bae7813
📒 Files selected for processing (67)
docs/TEST-COVERAGE-MATRIX.mdsrc/core/all.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/mod.rs
- 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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/openhuman/channels/bus.rs (1)
92-101: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the stale web-chat diagnostic source label.
start_chatnow comes fromopenhuman::web_chat, butsrc/openhuman/web_chat/ops.rsstill reports the prompt-enforcement source aschannels.providers.web.start_chat. Update that label to the canonicalweb_chatpath so logs and security diagnostics do not retain the removed module reference.As per coding guidelines, new or changed flows must include verbose, grep-friendly diagnostics; the relocation also requires stale old-path references to be removed.
🤖 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/channels/bus.rs` around lines 92 - 101, Update the prompt-enforcement diagnostic source label used by start_chat in openhuman::web_chat to the canonical web_chat path, removing the stale channels.providers.web.start_chat reference. Keep the existing diagnostic behavior and ensure no remaining logs or security diagnostics retain the removed module path.Source: Coding guidelines
🧹 Nitpick comments (3)
src/openhuman/web_chat/web_tests.rs (2)
1935-1982: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftProve that both turn futures execute concurrently.
These tests only observe entries registered in
IN_FLIGHT/PARALLEL_IN_FLIGHT. They can pass even if execution is serialized after registration; the sharedstarted: AtomicBoolproves only that at least one task was polled. Use an active-task counter or two-party barrier and wait until both parked futures have entered concurrently.As per coding guidelines, changed behavior must be proved in Rust.
Also applies to: 2131-2181
🤖 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 1935 - 1982, Strengthen start_chat_runs_distinct_threads_concurrently and the analogous test around the shared started AtomicBool so the assertion observes execution, not only IN_FLIGHT registration. Add an active-task counter or two-party barrier to run_chat_task and wait until both distinct-thread futures are simultaneously parked inside execution, then assert that condition before completing cleanup.Source: Coding guidelines
1-12: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit the oversized relocated web-chat modules into focused files.
src/openhuman/web_chat/web_tests.rs#L1-L12: separate classifier, schemas, session cache, and concurrency tests.src/openhuman/web_chat/web_errors.rs#L1-L14: separate parsing, predicates, managed-code classification, and user-facing messages.As per coding guidelines, prefer Rust 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_tests.rs` around lines 1 - 12, Split the oversized web-chat modules into focused Rust files of approximately 500 lines or fewer: in src/openhuman/web_chat/web_tests.rs lines 1-12, separate classifier, schema, session-cache, and concurrency tests while preserving their existing behavior and imports; in src/openhuman/web_chat/web_errors.rs lines 1-14, separate parsing, predicate, managed-code classification, and user-facing message logic into focused modules, updating module declarations and imports accordingly.Source: Coding guidelines
src/openhuman/web_chat/progress_bridge.rs (1)
308-1457: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit the bridge orchestration into focused modules.
spawn_progress_bridgecombines every parent/subagent lifecycle branch, ledger persistence, wire presentation, heartbeats, and trace export in one 1,149-line function. Extract event handlers and move 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/progress_bridge.rs` around lines 308 - 1457, Refactor spawn_progress_bridge into focused sibling modules, extracting parent and subagent event handling, ledger persistence, wire-event publishing, heartbeat processing, and trace-export finalization while preserving behavior and shared state. Keep spawn_progress_bridge as the orchestration loop that delegates to these handlers, and move related tests into the corresponding modules; target files of approximately 500 lines or fewer.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/openhuman/approval/README.md`:
- Line 59: Update the architecture ownership statement for
ApprovalSurfaceSubscriber in the approval README to reference
src/openhuman/web_chat/, and remove the obsolete “No bus” wording while
preserving the surrounding documentation.
In `@src/openhuman/web_chat/presentation.rs`:
- Around line 220-225: Update segment_for_delivery and the additionally
referenced threshold and delay logic to use Unicode character counts via
chars().count() instead of str::len(), preserving the existing 80/40-character
behavior for actual characters. Add a boundary test covering multibyte CJK or
Arabic text at the relevant thresholds.
- Around line 64-73: The delivery flow around reaction_handle currently blocks
on optional reaction inference; wrap the reaction_handle await in a short
timeout, and if it expires, abort the task, log the timeout, and continue with
no reaction emoji. Preserve normal successful results and ensure segment and
chat_done delivery proceed without waiting indefinitely for try_reaction.
In `@src/openhuman/web_chat/session.rs`:
- Around line 182-203: Update build_session_fingerprint and
SessionCacheFingerprint to include the same normalized locale/directive used
when constructing the agent prompt, ensuring locale changes produce a distinct
fingerprint. Add or extend the session cache test to verify changing locale
causes a cache miss and agent rebuild.
- Around line 129-134: Update the short_thread calculation in the session
handling flow to truncate thread_id at a valid UTF-8 character boundary rather
than slicing at byte index 12. Preserve the 12-character maximum when possible,
avoid panics for multibyte thread IDs, and keep the existing
agent.set_agent_definition_name behavior.
In `@src/openhuman/web_chat/types.rs`:
- Around line 15-35: The session fingerprint must mirror all effective
agent-build inputs. In src/openhuman/web_chat/types.rs lines 15-35, add a
normalized locale field to SessionCacheFingerprint; in
src/openhuman/web_chat/run_task.rs lines 92-104, derive the provider role from
the model override or config.default_model and populate the locale fingerprint.
Add tests verifying locale changes and effective provider-binding changes force
a session rebuild.
In `@src/openhuman/web_chat/web_errors.rs`:
- Around line 739-750: Update the ClassifiedError construction to select message
and source together based on provider: retain the OpenHuman billing message for
provider "openhuman" or None, and use provider-specific upstream billing copy
for other providers while keeping source "provider". Pass the selected message
through with_provider_detail and preserve the existing retryability and metadata
fields.
---
Outside diff comments:
In `@src/openhuman/channels/bus.rs`:
- Around line 92-101: Update the prompt-enforcement diagnostic source label used
by start_chat in openhuman::web_chat to the canonical web_chat path, removing
the stale channels.providers.web.start_chat reference. Keep the existing
diagnostic behavior and ensure no remaining logs or security diagnostics retain
the removed module path.
---
Nitpick comments:
In `@src/openhuman/web_chat/progress_bridge.rs`:
- Around line 308-1457: Refactor spawn_progress_bridge into focused sibling
modules, extracting parent and subagent event handling, ledger persistence,
wire-event publishing, heartbeat processing, and trace-export finalization while
preserving behavior and shared state. Keep spawn_progress_bridge as the
orchestration loop that delegates to these handlers, and move related tests into
the corresponding modules; target files of approximately 500 lines or fewer.
In `@src/openhuman/web_chat/web_tests.rs`:
- Around line 1935-1982: Strengthen
start_chat_runs_distinct_threads_concurrently and the analogous test around the
shared started AtomicBool so the assertion observes execution, not only
IN_FLIGHT registration. Add an active-task counter or two-party barrier to
run_chat_task and wait until both distinct-thread futures are simultaneously
parked inside execution, then assert that condition before completing cleanup.
- Around line 1-12: Split the oversized web-chat modules into focused Rust files
of approximately 500 lines or fewer: in src/openhuman/web_chat/web_tests.rs
lines 1-12, separate classifier, schema, session-cache, and concurrency tests
while preserving their existing behavior and imports; in
src/openhuman/web_chat/web_errors.rs lines 1-14, separate parsing, predicate,
managed-code classification, and user-facing message logic into focused modules,
updating module declarations and imports accordingly.
🪄 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: ad14058d-aade-4b54-96b3-461e8bae7813
📒 Files selected for processing (67)
docs/TEST-COVERAGE-MATRIX.mdsrc/core/all.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/mod.rs
- src/openhuman/channels/providers/mod.rs
🛑 Comments failed to post (7)
src/openhuman/approval/README.md (1)
59-59: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the contradictory legacy ownership statement.
This update correctly points
ApprovalSurfaceSubscribertosrc/openhuman/web_chat/, but line 62 still says the subscriber lives in the channels web provider. Update that statement tosrc/openhuman/web_chat/and remove the obsolete “No bus” wording.As per coding guidelines, architecture documentation must be updated when module ownership changes.
🤖 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 architecture ownership statement for ApprovalSurfaceSubscriber in the approval README to reference src/openhuman/web_chat/, and remove the obsolete “No bus” wording while preserving the surrounding documentation.Source: Coding guidelines
src/openhuman/web_chat/presentation.rs (2)
64-73: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not gate terminal delivery on optional reaction inference.
Spawning and immediately awaiting the task means no segment or
chat_doneevent is emitted until the local-model call finishes. A slow or wedged reaction decision stalls an otherwise completed chat. Use a short timeout, then abort and skip the optional reaction while logging the outcome.🤖 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, The delivery flow around reaction_handle currently blocks on optional reaction inference; wrap the reaction_handle await in a short timeout, and if it expires, abort the task, log the timeout, and continue with no reaction emoji. Preserve normal successful results and ensure segment and chat_done delivery proceed without waiting indefinitely for try_reaction.
220-225: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Count characters rather than UTF-8 bytes.
str::len()makes CJK and Arabic text cross the 80/40-character thresholds too early and inflates delays. Usechars().count()consistently and add a multibyte boundary test.Also applies to: 348-359, 412-426, 431-436
🤖 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 220 - 225, Update segment_for_delivery and the additionally referenced threshold and delay logic to use Unicode character counts via chars().count() instead of str::len(), preserving the existing 80/40-character behavior for actual characters. Add a boundary test covering multibyte CJK or Arabic text at the relevant thresholds.src/openhuman/web_chat/session.rs (2)
129-134: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Truncate
thread_idon character boundaries.
thread_id[..12]panics when byte 12 falls inside a multibyte UTF-8 character, aborting that chat turn.Proposed fix
- let short_thread = if thread_id.len() > 12 { - &thread_id[..12] - } else { - thread_id - }; + let short_thread: String = thread_id.chars().take(12).collect(); agent.set_agent_definition_name(format!("{target_agent_id}_{short_thread}"));📝 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.let short_thread: String = thread_id.chars().take(12).collect(); agent.set_agent_definition_name(format!("{target_agent_id}_{short_thread}"));🤖 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 calculation in the session handling flow to truncate thread_id at a valid UTF-8 character boundary rather than slicing at byte index 12. Preserve the 12-character maximum when possible, avoid panics for multibyte thread IDs, and keep the existing agent.set_agent_definition_name behavior.
182-203: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Include locale-derived prompt state in the session fingerprint.
The agent prompt incorporates
localeat Lines 87–91, but this fingerprint omits it. A locale change on the same thread therefore reuses the old localized agent. Add the normalized locale/directive toSessionCacheFingerprintand cover locale changes with a cache-miss test.🤖 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 182 - 203, Update build_session_fingerprint and SessionCacheFingerprint to include the same normalized locale/directive used when constructing the agent prompt, ensuring locale changes produce a distinct fingerprint. Add or extend the session cache test to verify changing locale causes a cache miss and agent rebuild.src/openhuman/web_chat/types.rs (1)
15-35: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make the session fingerprint exactly mirror effective agent-build inputs.
The cache currently misses both a build input and the effective provider-role calculation, allowing stale agents to survive configuration changes.
src/openhuman/web_chat/types.rs#L15-L35: add normalizedlocaletoSessionCacheFingerprint.src/openhuman/web_chat/run_task.rs#L92-L104: derive the provider role from the override orconfig.default_model, and populate the locale fingerprint.Add tests proving locale and effective provider-binding changes force a rebuild.
📍 Affects 2 files
src/openhuman/web_chat/types.rs#L15-L35(this comment)src/openhuman/web_chat/run_task.rs#L92-L104🤖 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 15 - 35, The session fingerprint must mirror all effective agent-build inputs. In src/openhuman/web_chat/types.rs lines 15-35, add a normalized locale field to SessionCacheFingerprint; in src/openhuman/web_chat/run_task.rs lines 92-104, derive the provider role from the model override or config.default_model and populate the locale fingerprint. Add tests verifying locale changes and effective provider-binding changes force a session rebuild.src/openhuman/web_chat/web_errors.rs (1)
739-750: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use provider-specific copy for upstream billing failures.
For
openrouter API error (402)and similar envelopes,sourcebecomes"provider"but the message still says the managed OpenHuman model is out of credits. Select the message together with the source so users are directed to the correct billing system.Proposed fix
- let source: &'static str = match provider.as_deref() { - Some("openhuman") | None => "openhuman_billing", - Some(_) => "provider", + let (source, summary): (&'static str, &'static str) = match provider.as_deref() { + Some("openhuman") | None => ( + "openhuman_billing", + inference_budget_exceeded_user_message(), + ), + Some(_) => ( + "provider", + "Your AI provider reports an exhausted balance or billing restriction. \ + Update that provider's billing or plan settings to continue.", + ), }; ClassifiedError { error_type: "budget_exhausted", - message: with_provider_detail(inference_budget_exceeded_user_message(), err), + message: with_provider_detail(summary, err),📝 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.let (source, summary): (&'static str, &'static str) = match provider.as_deref() { Some("openhuman") | None => ( "openhuman_billing", inference_budget_exceeded_user_message(), ), Some(_) => ( "provider", "Your AI provider reports an exhausted balance or billing restriction. \ Update that provider's billing or plan settings to continue.", ), }; ClassifiedError { error_type: "budget_exhausted", message: with_provider_detail(summary, err), source, retryable: false, retry_after_ms: None, provider, fallback_available: None,🤖 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 739 - 750, Update the ClassifiedError construction to select message and source together based on provider: retain the OpenHuman billing message for provider "openhuman" or None, and use provider-specific upstream billing copy for other providers while keeping source "provider". Pass the selected message through with_provider_detail and preserve the existing retryability and metadata fields.
…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.
d0dfce2 to
bfd1997
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/openhuman/web_chat/web_tests.rs (1)
1-12: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit this 2,314-line test module by concern.
Separate validation, error-classification, schema/session, and concurrency tests into focused submodules under
web_chat/web_tests/.As per coding guidelines, Rust files should be 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 - 12, Split the oversized web_tests module into focused submodules under web_chat/web_tests/, organizing validation, error-classification, schema/session, and concurrency tests separately. Update the module declarations and imports so all existing tests continue to compile and run, while keeping each Rust file approximately 500 lines or fewer.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/openhuman/web_chat/web_tests.rs`:
- Around line 18-31: Update the forced error/block override helpers and EnvGuard
used by start_chat/run_chat_task tests to return scoped, synchronous guards that
capture the exact prior state and restore it on Drop, including during panic or
timeout. Replace manual None resets and environment removal with guard
lifetimes, while retaining RUN_CHAT_TASK_TEST_LOCK for each test body so all
process-global mutations remain isolated.
---
Nitpick comments:
In `@src/openhuman/web_chat/web_tests.rs`:
- Around line 1-12: Split the oversized web_tests module into focused submodules
under web_chat/web_tests/, organizing validation, error-classification,
schema/session, and concurrency tests separately. Update the module declarations
and imports so all existing tests continue to compile and run, while keeping
each Rust file approximately 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: e28ff546-a3a9-4745-8d42-6bceecddde29
📒 Files selected for processing (74)
app/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/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
🚧 Files skipped from review as they are similar to previous changes (55)
- tests/raw_coverage/channels_web_yuanbao_round22_raw_coverage_e2e.rs
- src/openhuman/agent/triage/evaluator.rs
- src/openhuman/security/egress/mod.rs
- src/openhuman/mod.rs
- src/openhuman/web_chat/ops.rs
- tests/raw_coverage/channels_web_startup_raw_coverage_e2e.rs
- src/openhuman/channels/runtime/dispatch/processor.rs
- tests/web_cancel_request_scoping.rs
- src/openhuman/meet_agent/brain/llm.rs
- tests/agentbox_e2e.rs
- tests/raw_coverage/channels_web_telegram_raw_coverage_e2e.rs
- tests/raw_coverage/channels_bus_presentation_raw_coverage_e2e.rs
- src/openhuman/agent_orchestration/command_center/mod.rs
- src/openhuman/artifacts/store.rs
- src/core/event_bus/events.rs
- src/openhuman/approval/gate.rs
- src/openhuman/test_support/README.md
- docs/TEST-COVERAGE-MATRIX.md
- src/openhuman/agent/task_dispatcher/executor.rs
- tests/raw_coverage/tools_network_channels_raw_coverage_e2e.rs
- src/openhuman/inference/provider/error_code.rs
- src/openhuman/approval/README.md
- src/openhuman/channels/providers/telegram/remote_control.rs
- src/openhuman/web_chat/event_bus.rs
- tests/composio_list_tools_stack_overflow_regression.rs
- tests/raw_coverage/channels_provider_leftovers_raw_coverage_e2e.rs
- src/openhuman/threads/ops.rs
- src/core/socketio.rs
- src/openhuman/channels/bus.rs
- src/openhuman/threads/README.md
- src/main.rs
- src/openhuman/channels/runtime/startup.rs
- src/openhuman/cron/scheduler_tests.rs
- src/openhuman/channels/host/adapters.rs
- src/openhuman/test_support/introspect.rs
- src/openhuman/web_chat/mod.rs
- src/openhuman/wallet/README.md
- src/openhuman/prompt_injection/README.md
- src/openhuman/cron/scheduler.rs
- tests/raw_coverage/channels_large_round25_raw_coverage_e2e.rs
- src/core/jsonrpc.rs
- src/openhuman/agentbox/invoker.rs
- src/core/all.rs
- src/core/observability.rs
- src/openhuman/web_chat/session.rs
- tests/raw_coverage/channels_provider_deep_raw_coverage_e2e.rs
- src/openhuman/web_chat/run_task.rs
- src/openhuman/flows/ops.rs
- tests/raw_coverage/tools_approval_channels_raw_coverage_e2e.rs
- src/openhuman/web_chat/presentation.rs
- src/openhuman/web_chat/types.rs
- src/openhuman/web_chat/schemas.rs
- src/openhuman/web_chat/presentation_tests.rs
- src/openhuman/web_chat/web_errors.rs
- src/openhuman/web_chat/progress_bridge.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: 1
🧹 Nitpick comments (1)
src/openhuman/web_chat/web_tests.rs (1)
1-12: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit this 2,314-line test module by concern.
Separate validation, error-classification, schema/session, and concurrency tests into focused submodules under
web_chat/web_tests/.As per coding guidelines, Rust files should be 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 - 12, Split the oversized web_tests module into focused submodules under web_chat/web_tests/, organizing validation, error-classification, schema/session, and concurrency tests separately. Update the module declarations and imports so all existing tests continue to compile and run, while keeping each Rust file approximately 500 lines or fewer.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/openhuman/web_chat/web_tests.rs`:
- Around line 18-31: Update the forced error/block override helpers and EnvGuard
used by start_chat/run_chat_task tests to return scoped, synchronous guards that
capture the exact prior state and restore it on Drop, including during panic or
timeout. Replace manual None resets and environment removal with guard
lifetimes, while retaining RUN_CHAT_TASK_TEST_LOCK for each test body so all
process-global mutations remain isolated.
---
Nitpick comments:
In `@src/openhuman/web_chat/web_tests.rs`:
- Around line 1-12: Split the oversized web_tests module into focused submodules
under web_chat/web_tests/, organizing validation, error-classification,
schema/session, and concurrency tests separately. Update the module declarations
and imports so all existing tests continue to compile and run, while keeping
each Rust file approximately 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: e28ff546-a3a9-4745-8d42-6bceecddde29
📒 Files selected for processing (74)
app/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/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
🚧 Files skipped from review as they are similar to previous changes (55)
- tests/raw_coverage/channels_web_yuanbao_round22_raw_coverage_e2e.rs
- src/openhuman/agent/triage/evaluator.rs
- src/openhuman/security/egress/mod.rs
- src/openhuman/mod.rs
- src/openhuman/web_chat/ops.rs
- tests/raw_coverage/channels_web_startup_raw_coverage_e2e.rs
- src/openhuman/channels/runtime/dispatch/processor.rs
- tests/web_cancel_request_scoping.rs
- src/openhuman/meet_agent/brain/llm.rs
- tests/agentbox_e2e.rs
- tests/raw_coverage/channels_web_telegram_raw_coverage_e2e.rs
- tests/raw_coverage/channels_bus_presentation_raw_coverage_e2e.rs
- src/openhuman/agent_orchestration/command_center/mod.rs
- src/openhuman/artifacts/store.rs
- src/core/event_bus/events.rs
- src/openhuman/approval/gate.rs
- src/openhuman/test_support/README.md
- docs/TEST-COVERAGE-MATRIX.md
- src/openhuman/agent/task_dispatcher/executor.rs
- tests/raw_coverage/tools_network_channels_raw_coverage_e2e.rs
- src/openhuman/inference/provider/error_code.rs
- src/openhuman/approval/README.md
- src/openhuman/channels/providers/telegram/remote_control.rs
- src/openhuman/web_chat/event_bus.rs
- tests/composio_list_tools_stack_overflow_regression.rs
- tests/raw_coverage/channels_provider_leftovers_raw_coverage_e2e.rs
- src/openhuman/threads/ops.rs
- src/core/socketio.rs
- src/openhuman/channels/bus.rs
- src/openhuman/threads/README.md
- src/main.rs
- src/openhuman/channels/runtime/startup.rs
- src/openhuman/cron/scheduler_tests.rs
- src/openhuman/channels/host/adapters.rs
- src/openhuman/test_support/introspect.rs
- src/openhuman/web_chat/mod.rs
- src/openhuman/wallet/README.md
- src/openhuman/prompt_injection/README.md
- src/openhuman/cron/scheduler.rs
- tests/raw_coverage/channels_large_round25_raw_coverage_e2e.rs
- src/core/jsonrpc.rs
- src/openhuman/agentbox/invoker.rs
- src/core/all.rs
- src/core/observability.rs
- src/openhuman/web_chat/session.rs
- tests/raw_coverage/channels_provider_deep_raw_coverage_e2e.rs
- src/openhuman/web_chat/run_task.rs
- src/openhuman/flows/ops.rs
- tests/raw_coverage/tools_approval_channels_raw_coverage_e2e.rs
- src/openhuman/web_chat/presentation.rs
- src/openhuman/web_chat/types.rs
- src/openhuman/web_chat/schemas.rs
- src/openhuman/web_chat/presentation_tests.rs
- src/openhuman/web_chat/web_errors.rs
- src/openhuman/web_chat/progress_bridge.rs
🛑 Comments failed to post (1)
src/openhuman/web_chat/web_tests.rs (1)
18-31: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Make process-global test overrides scoped and panic-safe.
The mutex prevents concurrent writes, but a panic or timeout before the manual
Nonereset releases the lock while leaving the forced error/block installed. The timeoutEnvGuardalso removes the variable instead of restoring a pre-existing value. Subsequent tests can therefore inherit stale state and fail order-dependently.Have each override return a scoped guard that synchronously captures and restores the exact prior value on
Drop.Based on learnings, process-global test mutations should restore exact prior state with a scoped guard, even on panic.
Also applies to: 119-173, 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 18 - 31, Update the forced error/block override helpers and EnvGuard used by start_chat/run_chat_task tests to return scoped, synchronous guards that capture the exact prior state and restore it on Drop, including during panic or timeout. Replace manual None resets and environment removal with guard lifetimes, while retaining RUN_CHAT_TASK_TEST_LOCK for each test body so all process-global mutations remain isolated.Source: Learnings
🥞 Stacked PR — merge order (bottom → top)
This is position 2/5. Merge #5020 first.
#4801 depends on this relocation.
Summary
channels::providers::webto a top-levelopenhuman::web_chatmodule.#[cfg], no behaviour change, no wire-contract change, no symbol renames. Only file locations, import paths, and module declarations move.Problem
channels::providers::webwas filed as a chat provider, declared inproviders/mod.rsalongsidetelegram,discord,slack,whatsapp,wechat. It is not a pluggable medium — it is the in-app chat conduit:start_chat,cancel_chat_scoped,publish_web_channel_event,subscribe_web_channel_events,spawn_progress_bridge,ChatRequestMetadata,presentation::deliver_response. It sits there only because it implements the sameChanneltrait, and implementingChannelis not the same as being a medium.The dependency graph makes the mis-filing concrete — the arrows point the wrong way for a "provider":
providers/telegram/remote_control.rscallsweb::invalidate_thread_sessions;channels::buscallsweb::start_chat+web::subscribe_web_channel_events;channels::proactiveandchannels::host::adapterscallweb::publish_web_channel_event;channels::runtime::startupregisters the fourweb::register_*_surface_subscriberhandlers. 11 sites insidechannels/in total.super::import insideweb/resolved to web's own submodules — zero references tocontext,runtime,bus,proactive,host,relay_runtime,cli,controllers,commands, orroutes.threads::ops,agent::task_dispatcher::{executor,registry},agent::harness::session::runtime,core::socketio,core::jsonrpc,agentbox::invoker,cron::scheduler,flows::ops. Roughly two-thirds of all external references tochannelsreachproviders::webspecifically.register_*_surface_subscriberentry points under it are approval / automation-halt / egress / artifact surfaces — agent-harness concerns, not channel concerns.Why this blocks #4801.
DomainSet::harness()(#4796) is defined as agent + memory + threads + config + security, withchannels: false. Gating the conduit along with the mediums makesharness()a runtime configuration that cannot compile, becausethreadsandagentreach into it. The compile-time and runtime axes would contradict each other.This is the third instance of a pattern
AGENTS.mdalready names:Prior instances, both still outstanding:
mcp_client::{sanitize, client}(a docs tool dialsMcpHttpClient; the orchestrator sanitizes skill descriptions throughsanitize_for_llm) andmeet_agent::wav(a dependency-free RIFF writer called by the always-ondesktop_companionfor STT).Solution
Move
channels/providers/web/→openhuman/web_chat/and repoint every call site. After this,channels→web_chatis a one-way dependency: gated code depending on always-on code, which is the correct direction.Two
#[path]traps — both files lived inproviders/and were reached only via#[path]fromweb/mod.rs, so neither was declared inproviders/mod.rs:web_errors.rs(#[path = "../web_errors.rs"]) — moved intoweb_chat/,#[path]attribute dropped as it is now redundant.web_tests.rs(#[cfg(test)] #[path = "../web_tests.rs"]) — moved too. Missing this would have silently dropped 100+ tests from the build with CI still green. The#[path = "web_tests.rs"]form is retained so the module keeps the nametests(no symbol renames).No back-compat re-export.
channelsno longer exportswebat all. A shim would defeat the purpose —channelswould still export the conduit, and #4801 would break the re-export the moment it gates the module.src/core/all.rskeepsDomainGroup::Channelson the conduit's controller registration; only the path changed. The web RPC surface therefore stays hidden byDomainSet::harness()at runtime exactly as today. #4801 will#[cfg]that registration site — changing the group here would be an unreviewed behaviour change.Subscriber names (last commit, isolated deliberately): the four
EventHandler::name()returns readchannels::web::*, naming a module that no longer exists. Renamed toweb_chat::*, matching the<domain>::<purpose>convention its siblings follow (cron::delivery,memory::conversations::persistence). This is log-output only:name()is consumed atsrc/core/event_bus/bus.rs:145,169and both uses are tracing fields — dispatch filters ondomains(), notname(). Verified nothing keys on the old strings (no classifier, filter, or frontend reference).Two call sites did not fit the mechanical rewrite:
tests/raw_coverage/channels_large_round25_raw_coverage_e2e.rs—use ...::providers::web::{self, test_support as web_support}bound the local nameweb; the rewrite would have rebound it toweb_chatand broken 7 bareweb::uses. Written as{self as web, ...}to preserve the local binding rather than churn 7 call sites.src/openhuman/inference/provider/error_code.rs— carried a rustdoc intra-doc link tochannels::providers::web_errors::classify_inference_error. Repointed.cargo fmtreflowed 4 call sites (agent/task_dispatcher/registry.rs,agentbox/invoker.rs,channels/bus.rs,cron/scheduler.rs) purely because the shorter path now fits on fewer lines. Verified whitespace-only — identical fields, values, and..Default::default().Submission Checklist
diff-coverhas no new behaviour to measure.docs/TEST-COVERAGE-MATRIX.mdwere refreshed to match.)Closesin the## RelatedsectionImpact
/schemaoutput is identical.name()strings change in tracing output (channels::web::*→web_chat::*). Nothing consumes them programmatically.--all-targets(which compiles the test code) and a repo-wide grep proof returning zero hits for the old paths.Related
mcp_client::{sanitize, client},meet_agent::wavlearningemail-signature subscriber + rebuild trigger are silently conditional on channels being configured — pre-existing onmain, filed separately rather than bundled here)AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
refactor/5002-relocate-web-chat-conduitd0dfce27dValidation Run
pnpm --filter openhuman-app format:check— no frontend files changedpnpm typecheck— no TypeScript changedGGML_NATIVE=OFF cargo test --lib openhuman::web_chat→ 155 passed; 0 failed.cargo test --lib core::all::tests→ 55 passed; 0 failedcargo fmt --checkclean;GGML_NATIVE=OFF cargo check --manifest-path Cargo.toml→ 0 errors;--all-targets→ 0 errorsGGML_NATIVE=OFF cargo check --manifest-path app/src-tauri/Cargo.tomlpasses (2m38s) — the shell still buildsgrep -rn "channels::providers::web\|channels::web" src tests→ zero hitsPre-existing issue verified, not introduced:
cargo test --lib openhuman::web_chatstack-overflows on the default stack atcancel_chat_cooperatively_stops_in_flight_turn. Reproduced identically on base70c12ad2aunder the old path (openhuman::channels::providers::web::tests::cancel_chat_cooperatively_stops_in_flight_turn→fatal runtime error: stack overflow), so it predates this change. All 155 pass withRUST_MIN_STACK=33554432. Same deep-async-stack class as the knowntask_local::scopecase.Summary by CodeRabbit
New Features
Bug Fixes