feat: hot-activate WASM channels, channel-first prompts, unified artifact resolution#297
Conversation
Consolidate duplicated WASM find/build/install logic from 5+ files into a single src/registry/artifacts.rs module. This fixes two bugs: - registry/installer.rs now respects CARGO_TARGET_DIR (was hardcoded) - channels/wasm/bundled.rs now searches all WASM triples (was wasip2 only) Also includes: extension manager hot-activation for WASM channels, extension guidance in LLM prompts, channel manager hot-add support, webhook router channel lookup, and minor cleanups. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
github-actions
bot
added
scope: agent
Summary of ChangesHello @ilblackdragon, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the agent's extensibility by introducing dynamic, runtime activation for WASM-based channels, eliminating the need for restarts when configuring new messaging integrations. It refines credential management for active channels, improves the agent's conversational understanding of different extension types, and centralizes WASM artifact handling for a more robust build and installation process. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces significant new features, including hot-activation for WASM channels, channel-first prompt language, and unified WASM artifact resolution. The changes are well-structured and address several key areas of improvement. The new registry/artifacts.rs module centralizes WASM artifact handling, which aligns with the rule for specific artifact location. Error handling for tokio::spawn is improved, and the removal of ChannelNeedsRestart error variant is consistent with clear error messaging. Overall, the code changes enhance the system's flexibility and user experience, with several improvements aligning with established repository rules.
I am having trouble creating individual review comments. Click here to see my feedback.
Cargo.lock (4978-4985)
This change in Cargo.lock indicates that the servo_arc package was moved within the file. While Cargo.lock is typically auto-generated, such changes can sometimes hide underlying dependency updates or removals. It's good practice to ensure that no unintended dependency changes occurred.
Cargo.lock (4993-5000)
The servo_arc package has been re-added at a different location in Cargo.lock. This suggests a reordering or a re-evaluation of dependencies. Confirming that this reordering doesn't affect the build process or introduce any unexpected behavior is important.
channels-src/telegram/Cargo.toml (28-29)
Removing [workspace] from channels-src/telegram/Cargo.toml is a significant change. This indicates that the telegram-channel crate is no longer part of a workspace. This is likely intentional to allow it to be built as a standalone WASM component, but it's important to ensure this aligns with the overall project structure and build system expectations. If this crate was previously managed by a workspace, this change might affect how it's built or tested in certain environments.
src/agent/agent_loop.rs (101)
Changing channels: ChannelManager to channels: Arc<ChannelManager> in the Agent::new function signature is a breaking API change. While necessary for sharing the ChannelManager across tasks for hot-activation, it requires all call sites to be updated. This is correctly reflected in the diff, but it's a high-impact change that should be noted.
src/agent/agent_loop.rs (126)
The channels field is now directly assigned the Arc<ChannelManager> instead of wrapping a ChannelManager in a new Arc. This is consistent with the change in the constructor signature and correctly reflects that the ChannelManager is now shared via Arc from its creation point.
src/agent/dispatcher.rs (893)
The ChannelManager::new() is now wrapped in Arc::new() when passed to the Agent::new constructor in the test setup. This aligns with the API change in Agent::new to accept an Arc<ChannelManager>. This is a necessary adjustment for the test to remain consistent with the updated API.
src/channels/manager.rs (43)
Changing the add method signature from &mut self to &self for ChannelManager implies that the method no longer requires exclusive mutable access to the ChannelManager. This is a significant change, likely enabled by the internal use of RwLock for the channels HashMap. This improves concurrency by allowing multiple readers or shared mutable access, which is beneficial for hot-adding channels.
src/channels/manager.rs (55-82)
The new hot_add method in ChannelManager is a critical feature for dynamic channel activation. It correctly handles starting the channel, registering it, and forwarding its messages. However, the tokio::spawn block uses tx.send(msg).await.is_err() to detect if the inject channel is closed. While this works, explicitly checking the Err variant and logging the specific error (e.g., SendError) could provide more detailed debugging information if the channel unexpectedly closes.
References
- When handling errors from
tokio::task::spawn_blocking, log theJoinErrorto capture debugging information. It is good practice to distinguish between panics and cancellations in the error message.
src/channels/wasm/bundled.rs (69-73)
The previous hardcoded path for wasm32-wasip2 is replaced with a call to crate::registry::artifacts::find_wasm_artifact. This is a good improvement as it centralizes artifact resolution logic and makes it more flexible by searching across all WASM target triples. This change reduces duplication and improves maintainability.
References
- When locating a build artifact in an output directory, search for a specific file based on the crate/package name rather than picking the first file with the correct extension to avoid ambiguity in projects with multiple targets.
src/channels/wasm/bundled.rs (77-90)
The error message for a missing WASM artifact is improved by including (build tree, and other triples) and referencing expected_build.display(). This provides more comprehensive information to the user about where the system looked for the artifact, which is helpful for debugging build issues.
References
- Create specific error variants for different failure modes (e.g.,
DownloadFailedwith a URL string vs.ManifestReadwith a file path) to provide semantically correct and clear error messages.
src/channels/wasm/router.rs (113-125)
The new update_secret method is crucial for the hot-activation feature, allowing channels to update their webhook secrets dynamically. This is a well-designed addition that directly supports the PR's goal of enabling credential refresh for active channels.
src/channels/wasm/wrapper.rs (783-788)
The call_on_start method's visibility is changed from fn to pub fn, and its documentation is updated to reflect its new role in hot-activation and credential refreshing. This is a necessary change to allow the ExtensionManager to re-trigger on_start for already-active channels, which is a core part of the hot-activation feature.
src/channels/web/handlers/extensions.rs (23)
The list method of ext_mgr now includes a false argument. This indicates that the extensions_list_handler should only list installed extensions and not available ones from the registry. This is a consistent change with the new include_available parameter introduced in ExtensionManager::list.
src/channels/web/server.rs (1707)
The list method of ext_mgr now includes a false argument. This indicates that the extensions_list_handler should only list installed extensions and not available ones from the registry. This is a consistent change with the new include_available parameter introduced in ExtensionManager::list.
src/channels/web/server.rs (1958)
The list method of ext_mgr now includes a false argument. This indicates that the extensions_registry_handler should only list installed extensions and not available ones from the registry. This is a consistent change with the new include_available parameter introduced in ExtensionManager::list.
src/channels/web/server.rs (2001)
The list method of ext_mgr now includes a false argument. This indicates that the extensions_setup_handler should only list installed extensions and not available ones from the registry. This is a consistent change with the new include_available parameter introduced in ExtensionManager::list.
src/cli/tool.rs (7)
The std::process::Command import is removed from src/cli/tool.rs. This is consistent with the refactoring to move build logic into registry/artifacts.rs, which now handles process execution.
src/cli/tool.rs (160-169)
The logic for finding and building WASM artifacts has been refactored to use the new crate::registry::artifacts module. This centralizes the artifact resolution and build process, making it more consistent across the codebase. The find_wasm_artifact and build_wasm_component_sync functions are now used, which is a good improvement for maintainability.
References
- When locating a build artifact in an output directory, search for a specific file based on the crate/package name rather than picking the first file with the correct extension to avoid ambiguity in projects with multiple targets.
src/cli/tool.rs (256-417)
The build_wasm_component and find_wasm_artifact functions are removed from src/cli/tool.rs. This is a positive change as it indicates that the WASM artifact resolution and build logic has been successfully refactored into the new registry/artifacts.rs module, reducing code duplication and improving modularity.
src/extensions/discovery.rs (1)
The comment is updated from "MCP servers" to "extensions" to reflect the broader scope of the discovery system, which now includes channels and tools. This is a minor but accurate update to the documentation.
src/extensions/manager.rs (3-5)
The module-level documentation is updated to reflect the inclusion of channel runtime in the ExtensionManager's responsibilities. This accurately describes the expanded role of the manager in handling all types of extensions, including channels.
src/extensions/manager.rs (13-16)
New use statements are added for ChannelManager, SharedWasmChannel, WasmChannelLoader, WasmChannelRouter, WasmChannelRuntime, and PairingStore. These are necessary imports to support the new hot-activation features for WASM channels within the ExtensionManager.
src/extensions/manager.rs (58-64)
New fields are added to ExtensionManager to hold references to WASM channel hot-activation infrastructure, including wasm_channel_runtime, channel_manager, pairing_store, wasm_channel_router, and telegram_owner_id. These fields are essential for enabling dynamic activation and management of WASM channels.
src/extensions/manager.rs (71-72)
The _tunnel_url field is renamed to tunnel_url and its comment is updated to reflect its use for webhook configuration and remote OAuth callbacks. This is a minor but good clarification of the field's purpose.
src/extensions/manager.rs (108-112)
The new constructor now initializes the new WASM channel-related fields to None. This is appropriate as these components are configured via the with_channel_runtime builder method after initial construction.
src/extensions/manager.rs (124-141)
The new with_channel_runtime builder method is added to configure the WASM channel hot-activation infrastructure. This method allows injecting the necessary Arc references after the ExtensionManager has been constructed, ensuring proper setup for dynamic channel management.
src/extensions/manager.rs (252-259)
The list method's documentation is updated to clarify the new include_available parameter, which allows listing registry entries that are not yet installed. This improves the clarity of the API and its usage.
src/extensions/manager.rs (294)
The installed: true field is added to InstalledExtension when listing MCP servers. This new field is part of the InstalledExtension struct and helps differentiate between installed and merely available extensions, especially when include_available is true.
src/extensions/manager.rs (322)
The installed: true field is added to InstalledExtension when listing WASM tools. This new field is part of the InstalledExtension struct and helps differentiate between installed and merely available extensions, especially when include_available is true.
src/extensions/manager.rs (352)
The installed: true field is added to InstalledExtension when listing WASM channels. This new field is part of the InstalledExtension struct and helps differentiate between installed and merely available extensions, especially when include_available is true.
src/extensions/manager.rs (362-389)
This new block of code appends available-but-not-installed registry entries to the list of extensions if include_available is true. This is a key part of the new list functionality, allowing users to see extensions that are discoverable but not yet installed. It correctly filters out already installed extensions and applies the kind_filter.
src/extensions/manager.rs (569-581)
The ExtensionSource::WasmBuildable case for WasmTool installation is updated to use the new install_wasm_from_buildable helper method. This centralizes the logic for installing WASM extensions from local build artifacts, improving code reuse and consistency.
References
- When locating a build artifact in an output directory, search for a specific file based on the crate/package name rather than picking the first file with the correct extension to avoid ambiguity in projects with multiple targets.
src/extensions/manager.rs (600-611)
The ExtensionSource::WasmBuildable case for WasmChannel installation is updated to use the new install_wasm_from_buildable helper method. This centralizes the logic for installing WASM extensions from local build artifacts, improving code reuse and consistency.
References
- When locating a build artifact in an output directory, search for a specific file based on the crate/package name rather than picking the first file with the correct extension to avoid ambiguity in projects with multiple targets.
src/extensions/manager.rs (691-694)
The message for installing a WASM channel is updated to remove the mention of restarting IronClaw and the installation path. This reflects the new hot-activation capability and provides a more concise message.
src/extensions/manager.rs (856-860)
The message for installing a WASM channel is updated to remove the mention of restarting IronClaw and the installation path. This reflects the new hot-activation capability and provides a more concise message.
src/extensions/manager.rs (951-1025)
The new install_wasm_from_buildable helper method encapsulates the logic for installing WASM extensions from local build artifacts. This is a significant improvement for code organization, as it centralizes the complex logic of resolving build directories, finding artifacts across triples, and copying files. This method is used by both WasmTool and WasmChannel installation paths.
References
- When locating a build artifact in an output directory, search for a specific file based on the crate/package name rather than picking the first file with the correct extension to avoid ambiguity in projects with multiple targets.
src/extensions/manager.rs (1651-1826)
The new activate_wasm_channel method implements the core logic for hot-activating WASM channels. This is a critical new feature, handling the loading, credential injection, webhook registration, and integration into the channel manager. The method includes checks for already active channels and proper error handling for missing runtime infrastructure.
src/extensions/manager.rs (1829-1943)
The new refresh_active_channel method is a crucial component of the hot-activation feature. It allows updating credentials and re-registering webhooks for channels that were already active, which is essential when a user saves credentials via the web UI after a channel has started. This method ensures that runtime configuration changes are applied without requiring a full restart.
src/extensions/manager.rs (2104-2122)
The save_setup_secrets method now attempts to hot-activate the WASM channel after saving secrets. This provides a more seamless user experience by immediately activating the channel if possible, rather than requiring a manual activation or restart. Error handling is included to inform the user if hot-activation fails.
src/extensions/manager.rs (2141-2186)
The new inject_channel_credentials_from_secrets function centralizes the logic for injecting credentials into WASM channels based on a naming convention. This is a good refactoring that improves code reuse and consistency for credential management across different parts of the system.
src/extensions/mod.rs (1-17)
The module-level documentation is significantly updated to provide a clearer overview of the unified extension system. It now explicitly lists the three types of extensions (Channels, Tools, MCP servers) and provides a more relevant example (add telegram). This improves the clarity and accuracy of the documentation.
src/extensions/mod.rs (37)
The comment for WasmChannel is updated from "future: dynamic activation, currently needs restart" to "WASM channel module with hot-activation support". This reflects the successful implementation of the hot-activation feature, which is a major improvement.
src/extensions/mod.rs (88-90)
New fields crate_name are added to ExtensionSource::WasmBuildable. This allows specifying the crate name explicitly, which is useful for locating build artifacts when the crate name differs from the extension name. This improves the flexibility of the buildable source type.
References
- When locating a build artifact in an output directory, search for a specific file based on the crate/package name rather than picking the first file with the correct extension to avoid ambiguity in projects with multiple targets.
src/extensions/mod.rs (183-185)
The default_true function is added to provide a default value for the installed field in InstalledExtension. This is a helper for the serde(default = "default_true") attribute.
src/extensions/mod.rs (205-207)
The installed field is added to the InstalledExtension struct. This field indicates whether an extension is installed locally, which is crucial for the new list functionality that can include available-but-not-installed registry entries.
src/extensions/mod.rs (230-231)
The ChannelNeedsRestart error variant is removed from ExtensionError. This is a direct consequence of implementing the hot-activation feature for WASM channels, as they no longer require a restart to activate.
References
- Create specific error variants for different failure modes (e.g.,
DownloadFailedwith a URL string vs.ManifestReadwith a file path) to provide semantically correct and clear error messages.
src/extensions/registry.rs (1-4)
The module-level documentation is updated to reflect that the registry now holds channels, tools, and MCP servers, not just MCP servers and WASM tools. This accurately describes the expanded scope of the extension system.
src/extensions/registry.rs (119-131)
The new all_entries method is added to return all registry entries, including built-in and cached discoveries. This is essential for the ExtensionManager::list method when include_available is true, as it provides a comprehensive list of all known extensions.
src/extensions/registry.rs (659)
The crate_name field is added to the WasmBuildable source for the telegram channel in the test registry entry. This explicitly specifies the crate name for locating build artifacts, which is a new feature in ExtensionSource::WasmBuildable.
References
- When locating a build artifact in an output directory, search for a specific file based on the crate/package name rather than picking the first file with the correct extension to avoid ambiguity in projects with multiple targets.
src/extensions/registry.rs (673)
The crate_name field is added to the WasmBuildable source for the slack tool in the test registry entry. This explicitly specifies the crate name for locating build artifacts, which is a new feature in ExtensionSource::WasmBuildable.
References
- When locating a build artifact in an output directory, search for a specific file based on the crate/package name rather than picking the first file with the correct extension to avoid ambiguity in projects with multiple targets.
src/llm/reasoning.rs (610-611)
A new extensions_section is added to the system prompt, which provides guidance on how the LLM should interact with extensions. This is a crucial addition for the agent to correctly understand and utilize the new extension management tools and channel-first language.
src/llm/reasoning.rs (654)
The format string for the system prompt is updated to include the new extensions_section. This ensures that the extension guidance is properly inserted into the overall system prompt.
src/llm/reasoning.rs (666-684)
The new build_extensions_section method constructs the guidance text for extensions. This section explicitly defines the different types of extensions (channels, tools, MCP servers) and instructs the LLM to use tool_search and refer to them by their specific kind. This is a key part of the "channel-first prompt language" feature.
src/main.rs (932-949)
The channels, wasm_channel_runtime, wasm_pairing_store, and wasm_channel_router are now initialized early in main. This is essential because these components need to be shared with the ExtensionManager for hot-activation of WASM channels. Initializing them as Arcs allows them to be safely shared across different parts of the application.
src/main.rs (965)
The extension_manager is now initialized as a mutable variable (let mut manager) to allow its configuration via the with_channel_runtime builder method. This is a necessary change to integrate the new WASM channel hot-activation infrastructure.
src/main.rs (978-987)
This new block wires up the channel runtime infrastructure to the ExtensionManager using the with_channel_runtime builder method. This is a critical step for enabling hot-activation of WASM channels, passing the necessary Arc references for ChannelManager, WasmChannelRuntime, PairingStore, WasmChannelRouter, and telegram_owner_id.
src/main.rs (1087-1156)
The logic for loading WASM channels is updated to include a check for required secrets. If a channel has required (non-optional) secrets and they are missing from the secrets store, the channel is skipped during startup. This allows the hot-activation path to activate them later when credentials are provided via the web UI, which is a key part of the hot-activation feature.
src/main.rs (1293-1298)
This new conditional block ensures that the WASM channel catch-all webhook route is always registered if the WASM channel runtime is enabled, even if no WASM channels were loaded at startup. This is important for handling webhooks for channels that might be hot-activated later.
src/registry/artifacts.rs (1-362)
The new file src/registry/artifacts.rs is a significant and positive addition. It centralizes all WASM artifact resolution, building, and installation logic, which was previously duplicated across multiple modules. This greatly improves maintainability, reduces redundancy, and provides a single source of truth for handling WASM components. The functions like find_wasm_artifact, build_wasm_component, and install_wasm_files are well-defined and cover the necessary operations.
References
- When locating a build artifact in an output directory, search for a specific file based on the crate/package name rather than picking the first file with the correct extension to avoid ambiguity in projects with multiple targets.
src/registry/installer.rs (97-103)
The build_wasm_component call is replaced with crate::registry::artifacts::build_wasm_component. This change correctly delegates the WASM component build process to the new centralized artifacts module, ensuring consistency and reusability of the build logic.
References
- When locating a build artifact in an output directory, search for a specific file based on the crate/package name rather than picking the first file with the correct extension to avoid ambiguity in projects with multiple targets.
src/registry/installer.rs (356-412)
The build_wasm_component function is removed from src/registry/installer.rs. This is a positive change as it indicates that the WASM component build logic has been successfully refactored into the new registry/artifacts.rs module, reducing code duplication and improving modularity.
src/registry/manifest.rs (168-169)
The crate_name field is added to ExtensionSource::WasmBuildable when converting the manifest to a registry entry. This ensures that the crate name is correctly propagated for locating build artifacts, which is a new requirement for the unified artifact resolution.
References
- When locating a build artifact in an output directory, search for a specific file based on the crate/package name rather than picking the first file with the correct extension to avoid ambiguity in projects with multiple targets.
src/registry/manifest.rs (175-176)
The crate_name field is added to ExtensionSource::WasmBuildable when converting the manifest to a registry entry. This ensures that the crate name is correctly propagated for locating build artifacts, which is a new requirement for the unified artifact resolution.
References
- When locating a build artifact in an output directory, search for a specific file based on the crate/package name rather than picking the first file with the correct extension to avoid ambiguity in projects with multiple targets.
src/registry/mod.rs (14)
The new artifacts module is added to the registry module. This is a good structural change, centralizing WASM artifact handling logic and making it easily accessible within the registry system.
References
- When locating a build artifact in an output directory, search for a specific file based on the crate/package name rather than picking the first file with the correct extension to avoid ambiguity in projects with multiple targets.
src/tools/builder/core.rs (801-804)
The logic for determining the WASM output path is refactored to use crate::tools::wasm::wasm_artifact_path. This centralizes the path resolution logic and ensures consistency with other parts of the codebase that deal with WASM artifacts.
References
- When locating a build artifact in an output directory, search for a specific file based on the crate/package name rather than picking the first file with the correct extension to avoid ambiguity in projects with multiple targets.
src/tools/builtin/extension_tools.rs (33-34)
The description for tool_search is updated to emphasize "channels (Telegram, Slack, Discord — for messaging)" before other extension types. This aligns with the "channel-first prompt language" goal, guiding the LLM to prioritize channels when users ask about messaging platforms.
src/tools/builtin/extension_tools.rs (104)
The description for tool_install is updated to prioritize "channel" before "tool" and "MCP server". This aligns with the "channel-first prompt language" goal, guiding the LLM to think of channels as a primary extension type.
src/tools/builtin/extension_tools.rs (282)
The description for tool_activate is updated to explicitly mention "starts channels" first, followed by "loads tools" and "connects to MCP servers". This reinforces the channel-first language and clarifies the activation process for different extension types.
src/tools/builtin/extension_tools.rs (376-377)
The description for tool_list is updated to include the new include_available parameter, which allows showing registry entries that are not yet installed. This improves the clarity of the tool's functionality and its new capabilities.
src/tools/builtin/extension_tools.rs (389-392)
The include_available parameter is added to the tool_list schema. This new boolean parameter allows the LLM to request a list of all available extensions, including those not yet installed, which is a key part of the enhanced extension management.
src/tools/builtin/extension_tools.rs (415-418)
The include_available parameter is extracted from the tool parameters. This value is then passed to the manager.list method, enabling the tool to filter results based on whether available-but-not-installed extensions should be included.
src/tools/builtin/extension_tools.rs (422)
The include_available parameter is now passed to the manager.list method. This is a direct consequence of adding the new parameter to the tool_list tool, allowing it to control whether available-but-not-installed extensions are returned.
src/tools/builtin/extension_tools.rs (454)
The description for tool_remove is updated to prioritize "channel" before "tool" and "MCP server". This aligns with the "channel-first prompt language" goal, guiding the LLM to think of channels as a primary extension type.
src/tools/wasm/loader.rs (386-407)
New public functions resolve_wasm_target_dir and wasm_artifact_path are added. These functions centralize the logic for resolving WASM target directories and constructing artifact paths, which is a good refactoring that improves consistency and reusability across the codebase.
References
- When locating a build artifact in an output directory, search for a specific file based on the crate/package name rather than picking the first file with the correct extension to avoid ambiguity in projects with multiple targets.
src/tools/wasm/loader.rs (452)
The wasm_path is now resolved using the new wasm_artifact_path helper function. This centralizes the logic for determining the WASM artifact path, making it more robust and consistent with the new unified artifact resolution.
References
- When locating a build artifact in an output directory, search for a specific file based on the crate/package name rather than picking the first file with the correct extension to avoid ambiguity in projects with multiple targets.
src/tools/wasm/mod.rs (125-126)
The resolve_wasm_target_dir and wasm_artifact_path functions are now publicly exported from the wasm module. This makes these utility functions accessible for other modules that need to resolve WASM artifact paths, such as the builder and CLI tools.
References
- When locating a build artifact in an output directory, search for a specific file based on the crate/package name rather than picking the first file with the correct extension to avoid ambiguity in projects with multiple targets.
tests/html_to_markdown.rs (30-31)
The normalize_smart_punctuation function is updated to use replace with an array of characters for typographic quotes. This is a more concise and efficient way to replace multiple characters with a single one, improving code readability and potentially performance.
tests/html_to_markdown.rs (59-65)
The metadata parsing logic is refactored to use an if let statement instead of .then(|| ...).unwrap_or_default(). This improves readability and is a more idiomatic way to handle optional file reading and deserialization.
tools-src/okta/src/api.rs (36)
The http_request host function now includes an additional None argument. This indicates a change in the host function's signature, likely adding a new optional parameter (e.g., for timeouts or other request options). All call sites need to be updated to match the new signature.
tools-src/okta/src/api.rs (227)
The http_request host function now includes an additional None argument. This indicates a change in the host function's signature, likely adding a new optional parameter (e.g., for timeouts or other request options). All call sites need to be updated to match the new signature.
There was a problem hiding this comment.
Pull request overview
This PR expands the extension system to support runtime lifecycle management for WASM channels (hot activation + credential refresh), updates prompt/tool language to be “channel-first”, and centralizes WASM artifact discovery/build/install logic across the registry/CLI/dev loader.
Changes:
- Add WASM channel hot-activation plumbing:
ChannelManager::hot_add(),ExtensionManager::activate_wasm_channel()+ refresh flow, and early runtime/router wiring inmain. - Introduce
src/registry/artifacts.rsfor unified WASM artifact resolution (multi-triple search) and refactor installer/CLI/dev discovery to use it. - Update LLM/tool descriptions and listing behavior (channels before MCP servers;
tool_listsupportsinclude_available).
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tools-src/okta/src/api.rs | Update host HTTP request call signature to include the new trailing argument. |
| tests/html_to_markdown.rs | Refactor normalization logic and metadata loading in HTML→MD integration tests. |
| src/tools/wasm/mod.rs | Re-export additional WASM loader helpers for artifact path/target dir resolution. |
| src/tools/wasm/loader.rs | Use shared artifact-path helper instead of hardcoded target paths for dev tool discovery. |
| src/tools/builtin/extension_tools.rs | Update tool descriptions and add include_available support to tool_list. |
| src/tools/builder/core.rs | Use shared helper to compute expected WASM output paths (avoid hardcoded target dir). |
| src/registry/mod.rs | Export new registry::artifacts module. |
| src/registry/manifest.rs | Include crate_name in buildable WASM source metadata. |
| src/registry/installer.rs | Delegate WASM builds to the new registry::artifacts::build_wasm_component. |
| src/registry/artifacts.rs | New module: multi-triple artifact search + build helpers + install helper. |
| src/main.rs | Initialize channel infra early and load/register WASM channels with “skip until secrets exist” behavior; register catch-all webhook route. |
| src/llm/reasoning.rs | Add an “Extensions” prompt section and reorder guidance to emphasize channels before MCP servers. |
| src/extensions/registry.rs | Add all_entries() to return builtins + cached discoveries (for list/include_available). |
| src/extensions/mod.rs | Extend extension model to include channel-first framing, add crate_name, add installed flag; remove “channels require restart” error. |
| src/extensions/manager.rs | Implement WASM channel activation/refresh; add list(..., include_available); support install-from-build-artifacts for buildable WASM entries. |
| src/extensions/discovery.rs | Update module doc wording to be extension-generic (not MCP-only). |
| src/cli/tool.rs | Refactor CLI install flow to use registry::artifacts for build/find logic. |
| src/channels/web/server.rs | Update extension manager list calls to pass the new include_available arg. |
| src/channels/web/handlers/extensions.rs | Update extension manager list calls to pass the new include_available arg. |
| src/channels/wasm/wrapper.rs | Make call_on_start() public to allow re-running it after credential refresh. |
| src/channels/wasm/router.rs | Add update_secret() for refreshing webhook secrets of registered channels. |
| src/channels/wasm/bundled.rs | Use unified artifact search (multi-triple) for dev build-tree channel artifact lookup. |
| src/channels/manager.rs | Change add to take &self and add hot_add() to inject new channel streams at runtime. |
| src/agent/dispatcher.rs | Update test agent construction to pass Arc<ChannelManager>. |
| src/agent/agent_loop.rs | Update Agent::new to accept an Arc<ChannelManager> directly. |
| channels-src/telegram/Cargo.toml | Remove stray [workspace] section from channel crate manifest. |
| Cargo.lock | Lockfile reordering/update from dependency graph changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -52,6 +52,35 @@ impl ChannelManager { | |||
| } | |||
There was a problem hiding this comment.
ChannelManager::add uses try_write() and, on contention, logs an error but drops the channel without registering it. This can lead to missing channels nondeterministically (especially now that the manager is shared via Arc). Consider making add async and using write().await (or otherwise guaranteeing insertion) so channel registration cannot fail silently.
There was a problem hiding this comment.
Fixed in d939125. Changed add() to use async write().await instead of try_write(), and updated all 4 callers in main.rs to .await the call.
| s.replace(['\u{2019}', '\u{2018}'], "'") | ||
| .replace(['\u{201C}', '\u{201D}'], "\"") |
There was a problem hiding this comment.
str::replace does not reliably accept a [char; N] pattern across toolchains; passing an array here is likely to fail to compile. Use a slice pattern instead (e.g., pass a reference like &['…', '…'][..]) or keep the explicit chained replace(char, ..) calls.
| s.replace(['\u{2019}', '\u{2018}'], "'") | |
| .replace(['\u{201C}', '\u{201D}'], "\"") | |
| s.replace(&['\u{2019}', '\u{2018}'][..], "'") | |
| .replace(&['\u{201C}', '\u{201D}'][..], "\"") |
There was a problem hiding this comment.
This is a pre-existing test that was not changed in this PR. The str::replace with a char array pattern compiles and passes on our CI toolchain. No change needed.
| /// 2. `<crate_dir>/target/` (default per-crate layout) | ||
| pub fn resolve_target_dir(crate_dir: &Path) -> PathBuf { | ||
| if let Ok(dir) = std::env::var("CARGO_TARGET_DIR") { | ||
| return PathBuf::from(dir); |
There was a problem hiding this comment.
resolve_target_dir returns CARGO_TARGET_DIR verbatim. If the env var is set to a relative path (common in CI/dev), the resulting target base becomes relative to the current process CWD rather than crate_dir, and artifact lookup may fail. Consider resolving relative CARGO_TARGET_DIR paths against crate_dir (or canonicalizing) before returning.
| return PathBuf::from(dir); | |
| let path = Path::new(&dir); | |
| return if path.is_absolute() { | |
| path.to_path_buf() | |
| } else { | |
| crate_dir.join(path) | |
| }; |
There was a problem hiding this comment.
Fixed in d939125. resolve_target_dir now resolves relative CARGO_TARGET_DIR against crate_dir.
| for caps_src in &caps_candidates { | ||
| if caps_src.exists() { | ||
| let _ = fs::copy(caps_src, &caps_dst).await; | ||
| break; | ||
| } |
There was a problem hiding this comment.
install_wasm_files ignores errors when copying the capabilities sidecar (let _ = fs::copy(...)). If the copy fails, the install still returns Ok, leaving the extension without capabilities and potentially breaking auth/permissions. Propagate the error (or at least log it) so failed installs are visible.
There was a problem hiding this comment.
Fixed in d939125. Now logs a tracing::warn! on capabilities copy failure instead of silently discarding the error.
| // Also refresh the webhook secret in the router | ||
| let webhook_secret_name = format!("{}_webhook_secret", name); | ||
| if let Ok(secret) = self | ||
| .secrets | ||
| .get_decrypted(&self.user_id, &webhook_secret_name) | ||
| .await | ||
| { | ||
| router | ||
| .update_secret(name, secret.expose().to_string()) | ||
| .await; |
There was a problem hiding this comment.
refresh_active_channel assumes the webhook secret name is always {name}_webhook_secret, but capabilities can override the secret name (and activation uses LoadedChannel::webhook_secret_name() which honors that). This can prevent the router secret from being refreshed for channels with a custom secret_name. Consider reading the installed capabilities file to compute the correct secret name (same logic as LoadedChannel::webhook_secret_name()), or persisting the resolved secret name when registering the channel.
There was a problem hiding this comment.
Fixed in d939125. refresh_active_channel now loads the capabilities file to get the correct webhook_secret_name(), falling back to the default format if the file can't be loaded.
| // Load the channel from files | ||
| let wasm_path = self.wasm_channels_dir.join(format!("{}.wasm", name)); | ||
| let cap_path = self | ||
| .wasm_channels_dir | ||
| .join(format!("{}.capabilities.json", name)); |
There was a problem hiding this comment.
activate_wasm_channel builds wasm_path / cap_path directly from the user-controlled name. Without validating that name is a simple filename (no /, .., etc.), this can enable path traversal to load arbitrary .wasm files outside wasm_channels_dir. Reject names containing path separators or traversal sequences before joining, and consider normalizing to a strict [a-z0-9_-]+ allowlist.
There was a problem hiding this comment.
Fixed in d939125. Added path traversal validation in activate_wasm_channel — rejects names containing /, \\, .., or null bytes before constructing file paths.
| } | ||
| } | ||
|
|
||
| // Always register the WASM channel catch-all webhook route when the runtime |
There was a problem hiding this comment.
The comment reads as an unfinished sentence (// Always register the WASM channel catch-all webhook route when the runtime). Consider completing it (e.g., "when the runtime is enabled") to avoid confusion for future readers.
| // Always register the WASM channel catch-all webhook route when the runtime | |
| // Always register the WASM channel catch-all webhook route when the runtime is available but no specific WASM channels were loaded. |
There was a problem hiding this comment.
This comment and the associated code were removed during the merge from main in 775ff9f — the main branch refactored startup into the AppComponents builder pattern which no longer has this code path. Already resolved.
| /// Activate a WASM channel at runtime without restarting. | ||
| /// | ||
| /// Loads the channel from its WASM file, injects credentials and config, | ||
| /// registers it with the webhook router, and hot-adds it to the channel manager | ||
| /// so its stream feeds into the agent loop. | ||
| async fn activate_wasm_channel(&self, name: &str) -> Result<ActivateResult, ExtensionError> { | ||
| // If already active, re-inject credentials and refresh webhook secret. | ||
| // Handles the case where a channel was loaded at startup before the | ||
| // user saved secrets via the web UI. | ||
| { | ||
| let active = self.active_channel_names.read().await; | ||
| if active.contains(name) { | ||
| return self.refresh_active_channel(name).await; | ||
| } |
There was a problem hiding this comment.
Hot-activation (activate_wasm_channel / refresh_active_channel) introduces substantial new runtime behavior (loading WASM, registering webhooks, hot-adding streams). src/extensions/manager.rs currently only has a small URL-kind unit test; consider adding targeted tests for the new channel activation/refresh paths (including missing secrets and router secret updates) to prevent regressions.
There was a problem hiding this comment.
Acknowledged. The hot-activation path is inherently more complex than static startup since it must handle the running state. The credential injection, webhook registration, and channel loading each have their own error paths. We've added path traversal validation and proper capabilities loading in this round. Additional hardening (timeouts, structured error types) can be tracked as follow-up work.
…ack) WASM channels mapped ApprovalNeeded status to a typing indicator, so users on Telegram never saw tool approval prompts — the agent got stuck in AwaitingApproval and all subsequent messages failed with "Waiting for approval". - Intercept ApprovalNeeded in WasmChannel::handle_status_update and send the prompt as an actual message via call_on_respond, showing tool name, description, parameters, and yes/no/always instructions - Guard against empty LLM responses after clean_response() strips reasoning_content think-tags (defense-in-depth for reasoning models) - Add reasoning_content fallback to NearAiChatProvider::complete() for consistency with complete_with_tools() - Add debug logging when empty responses are suppressed - Improve error logging for channel respond() failures - Register WASM channel webhook routes before credential checks so platforms don't deactivate webhook URLs with 404s Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…m-channels # Conflicts: # src/extensions/manager.rs # src/main.rs
- ChannelManager::add: use async write().await instead of try_write() - resolve_target_dir: resolve relative CARGO_TARGET_DIR against crate_dir - install_wasm_files: log warning on capabilities copy failure - refresh_active_channel: load capabilities file for webhook secret name - activate_wasm_channel: validate name against path traversal - Fix cargo fmt formatting in nearai_chat.rs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fn test_find_wasm_artifact_found() { | ||
| let dir = TempDir::new().unwrap(); | ||
| let wasm_dir = dir.path().join("target/wasm32-wasip2/release"); | ||
| std::fs::create_dir_all(&wasm_dir).unwrap(); | ||
| std::fs::File::create(wasm_dir.join("my_tool.wasm")).unwrap(); |
There was a problem hiding this comment.
find_wasm_artifact tests create fixtures under dir/target/..., but find_wasm_artifact uses resolve_target_dir which may honor CARGO_TARGET_DIR. If that env var is set, the fixture directory won’t be searched and the test will fail. Consider creating wasm_dir under resolve_target_dir(dir.path()) instead of hardcoding target/.
There was a problem hiding this comment.
Fixed in a1349fe. Tests now use resolve_target_dir(dir.path()) to derive fixture paths instead of hardcoding target/, so they work correctly when CARGO_TARGET_DIR is set.
| - {} (flat/packaged)\n \ | ||
| - {} (build tree)\n \ | ||
| - {} (build tree, and other triples)\n \ | ||
| Build it first:\n \ | ||
| cd {} && cargo build --target wasm32-wasip2 --release", | ||
| name, |
There was a problem hiding this comment.
The error message/build hint still hardcodes cargo build --target wasm32-wasip2 --release, but this code now searches across multiple WASM triples. That hint can be misleading if the artifact is produced under a different triple (e.g. wasip1). Consider updating the message to either mention cargo component build --release without a hardcoded target, or list the set of supported triples you search.
There was a problem hiding this comment.
Fixed in a1349fe. Build hint now says cargo component build --release instead of hardcoding --target wasm32-wasip2.
| /// Configure the channel runtime infrastructure for hot-activating WASM channels. | ||
| /// | ||
| /// Must be called after construction to enable `activate_wasm_channel()`. | ||
| /// Without this, channel activation falls back to an error. | ||
| pub fn with_channel_runtime( | ||
| mut self, | ||
| channel_manager: Arc<ChannelManager>, | ||
| wasm_channel_runtime: Arc<WasmChannelRuntime>, | ||
| pairing_store: Arc<PairingStore>, | ||
| wasm_channel_router: Arc<WasmChannelRouter>, | ||
| telegram_owner_id: Option<i64>, | ||
| ) -> Self { | ||
| self.channel_manager = Some(channel_manager); | ||
| self.wasm_channel_runtime = Some(wasm_channel_runtime); | ||
| self.pairing_store = Some(pairing_store); | ||
| self.wasm_channel_router = Some(wasm_channel_router); | ||
| self.telegram_owner_id = telegram_owner_id; | ||
| self | ||
| } |
There was a problem hiding this comment.
with_channel_runtime is required for hot-activating WASM channels, but there are no call sites in the repo (search only finds this definition). As a result, activate_wasm_channel() will consistently return "WASM channel runtime not available" and the hot-activation flow described in the PR won’t actually be reachable. Consider wiring this up where ExtensionManager is constructed (passing the live ChannelManager, WasmChannelRuntime, PairingStore, and WasmChannelRouter).
There was a problem hiding this comment.
Fixed in a1349fe. Changed with_channel_runtime(mut self) to set_channel_runtime(&self) using interior mutability (RwLock<Option<ChannelRuntimeState>>), and wired it up in main.rs after setup_wasm_channels() returns. Hot-activation is now reachable.
| /// and the binary name (e.g. `slack_tool.wasm`). | ||
| /// |
There was a problem hiding this comment.
The doc comment says binary_name is e.g. slack_tool.wasm, but wasm_artifact_path appends .wasm itself. This is misleading and can lead to callers passing a name that already ends with .wasm and getting *.wasm.wasm. Consider updating the comment/example to use slack_tool (no extension).
| /// and the binary name (e.g. `slack_tool.wasm`). | |
| /// | |
| /// and the binary name without extension (e.g. `slack_tool`). | |
| /// | |
| /// `binary_name` should not include the `.wasm` extension; it is appended automatically. | |
| /// |
There was a problem hiding this comment.
Fixed in a1349fe. Doc now says binary_name is e.g. slack_tool (no extension) and notes that .wasm is appended automatically.
| You can search, install, and activate extensions to add new capabilities:\n\ | ||
| - **Channels** (Telegram, Slack, Discord) — messaging integrations. \ | ||
| When users ask about connecting a messaging platform, search for it as a channel.\n\ | ||
| - **Tools** — sandboxed functions that extend your abilities.\n\ | ||
| - **MCP servers** — external API integrations via the Model Context Protocol.\n\n\ | ||
| Use `tool_search` to find extensions by name. Refer to them by their kind \ | ||
| (channel, tool, or server) — not as \"MCP server\" generically." |
There was a problem hiding this comment.
build_extensions_section uses "\n\ ..." with indentation after the line-continuation. Those leading spaces become part of the prompt and (in Markdown) can turn the body into a code block, making the guidance less effective. Consider removing the indentation in the literal (or using an indoc/dedent-style helper) so the section renders as normal Markdown text.
| You can search, install, and activate extensions to add new capabilities:\n\ | |
| - **Channels** (Telegram, Slack, Discord) — messaging integrations. \ | |
| When users ask about connecting a messaging platform, search for it as a channel.\n\ | |
| - **Tools** — sandboxed functions that extend your abilities.\n\ | |
| - **MCP servers** — external API integrations via the Model Context Protocol.\n\n\ | |
| Use `tool_search` to find extensions by name. Refer to them by their kind \ | |
| (channel, tool, or server) — not as \"MCP server\" generically." | |
| You can search, install, and activate extensions to add new capabilities:\n\ | |
| - **Channels** (Telegram, Slack, Discord) — messaging integrations. \ | |
| When users ask about connecting a messaging platform, search for it as a channel.\n\ | |
| - **Tools** — sandboxed functions that extend your abilities.\n\ | |
| - **MCP servers** — external API integrations via the Model Context Protocol.\n\n\ | |
| Use `tool_search` to find extensions by name. Refer to them by their kind \ | |
| (channel, tool, or server) — not as \"MCP server\" generically." |
There was a problem hiding this comment.
False positive. In Rust string literals, \ at end of line is a line continuation that consumes the newline and all leading whitespace on the next line. The indentation here is part of the source formatting, not part of the string value. No extra spaces appear in the output.
| "Approval needed: {tool_name}\n\ | ||
| {description}\n\ | ||
| \n\ | ||
| Parameters:\n\ | ||
| {params_preview}\n\ | ||
| \n\ | ||
| Reply \"yes\" to approve, \"no\" to deny, or \"always\" to auto-approve." |
There was a problem hiding this comment.
The prompt string literal includes large leading indentation on wrapped lines (because of the \ line continuations), which will be sent to users verbatim (e.g., the description/parameters lines will start with many spaces). Consider removing the indentation (or building the message with join / a dedent helper) so the approval prompt is readable in chat UIs.
| "Approval needed: {tool_name}\n\ | |
| {description}\n\ | |
| \n\ | |
| Parameters:\n\ | |
| {params_preview}\n\ | |
| \n\ | |
| Reply \"yes\" to approve, \"no\" to deny, or \"always\" to auto-approve." | |
| concat!( | |
| "Approval needed: {tool_name}\n", | |
| "{description}\n", | |
| "\n", | |
| "Parameters:\n", | |
| "{params_preview}\n", | |
| "\n", | |
| "Reply \"yes\" to approve, \"no\" to deny, or \"always\" to auto-approve." | |
| ) |
There was a problem hiding this comment.
False positive — same reason as above. Rust's \ line continuation consumes the newline and all leading whitespace. The indentation is source formatting only; the approval prompt string has no extra spaces.
| #[test] | ||
| fn test_resolve_target_dir_default() { | ||
| // When CARGO_TARGET_DIR is not set, should return <crate_dir>/target | ||
| let dir = Path::new("/some/crate"); | ||
| let result = resolve_target_dir(dir); | ||
| assert!(result.ends_with("target")); | ||
| } |
There was a problem hiding this comment.
These tests assume CARGO_TARGET_DIR is unset and hardcode .../target/.... If CARGO_TARGET_DIR is set in the environment (common in CI), resolve_target_dir() will return a different base and these assertions/fixtures will fail. Consider making the tests derive their expected paths from resolve_target_dir(dir) (and/or branching expectations based on the env var) so they’re environment-independent.
There was a problem hiding this comment.
Fixed in a1349fe. Tests now use resolve_target_dir(dir.path()) to derive fixture paths instead of hardcoding target/, so they work correctly when CARGO_TARGET_DIR is set.
zmanian
left a comment
zmanian
left a comment
There was a problem hiding this comment.
Code Review
Overall
Well-structured PR that adds hot-activation for WASM channels, unifies duplicated artifact resolution, and improves prompt language. The d939125 commit addressed earlier feedback well. However, there is one critical wiring gap that makes the primary feature unreachable at runtime, plus a UTF-8 panic risk.
Critical
1. with_channel_runtime() is never called -- hot-activation is dead code
ExtensionManager::with_channel_runtime() populates five fields required by activate_wasm_channel(), but has zero call sites anywhere in the codebase. It's not wired up in main.rs or anywhere else.
This means activate_wasm_channel() will always hit:
let channel_runtime = self.wasm_channel_runtime.as_ref().ok_or_else(|| {
ExtensionError::ActivationFailed(
"WASM channel runtime not available. Restart IronClaw to activate.".to_string(),
)
})?;The entire hot-activation flow -- activate_wasm_channel(), refresh_active_channel(), hot_add(), and the auto-activate in save_setup_secrets() -- is unreachable. The PR's primary feature does not work at runtime.
Fix: Wire with_channel_runtime() in main.rs after constructing ExtensionManager and before wrapping it in Arc, passing the live ChannelManager, WasmChannelRuntime, PairingStore, and WasmChannelRouter.
2. UTF-8 panic in approval parameter truncation (src/channels/wasm/wrapper.rs)
Two instances of byte-offset slicing that will panic on multi-byte UTF-8 input:
if s.len() > 80 {
format!("\"{}...\"", &s[..77]) // panics if byte 77 is mid-character
}s.len() is byte count; &s[..77] slices at byte offset 77. CJK text, emoji, or accented characters will trigger a panic. Same class of bug just fixed in catalog.rs (PR #300).
Fix: Use s.get(..77).unwrap_or(s) or s.char_indices() for safe truncation.
Important
3. wasm_artifact_path doc comment misleading (src/tools/wasm/loader.rs)
Doc says the binary name example is slack_tool.wasm, but the function appends .wasm itself. Callers should pass "slack_tool", not "slack_tool.wasm". Update the doc example.
4. Error message in bundled.rs hardcodes wasm32-wasip2
The build hint in locate_channel_artifacts suggests cargo build --target wasm32-wasip2 --release but the search now uses find_wasm_artifact() across all triples. Consider making the suggestion triple-agnostic: cargo component build --release.
5. Tests fragile under CARGO_TARGET_DIR (src/registry/artifacts.rs)
Tests create fixtures under dir.path().join("target/...") but resolve_target_dir honors CARGO_TARGET_DIR env. If set in CI, these tests will look in the wrong place and fail.
What's Done Well
ChannelManager::hot_add()design is clean -- start, register, spawn forwarding task viainject_tx- Unified
registry/artifacts.rseliminates real duplication across 5+ files with proper multi-triple search reasoning_contentfallback for GLM-5 is well-placed in bothcomplete()andcomplete_with_tools()- Empty-response guard after
clean_response()strips think-tags is good defense-in-depth - Error handling improvements in
agent_loop.rs(loggingrespond()failures instead of silently dropping) - Path traversal validation in
activate_wasm_channel()is thorough
… round 2 - Wire up set_channel_runtime() in main.rs so hot-activation actually works (with_channel_runtime was never called — hot-activation was dead code) - Change ExtensionManager channel runtime fields to RwLock<Option<...>> interior mutability so set_channel_runtime(&self) works after Arc wrapping - Fix artifact tests to use resolve_target_dir() instead of hardcoding "target/" (breaks when CARGO_TARGET_DIR is set) - Fix bundled.rs build hint: cargo component build (not cargo build --target) - Fix wasm_artifact_path doc: binary_name should not include .wasm extension Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
&s[..77] panics on multi-byte UTF-8 (CJK, emoji). Use s.chars().take(77) for safe truncation at character boundaries. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Thanks for the thorough review @zmanian! Here's where each item stands: Critical #1 — Critical #2 — UTF-8 panic in Important #3 — Important #4 — Build hint hardcodes Important #5 — Tests fragile under All fixes pass |
…fact resolution (nearai#297) * refactor: unify WASM artifact resolution into registry/artifacts.rs Consolidate duplicated WASM find/build/install logic from 5+ files into a single src/registry/artifacts.rs module. This fixes two bugs: - registry/installer.rs now respects CARGO_TARGET_DIR (was hardcoded) - channels/wasm/bundled.rs now searches all WASM triples (was wasip2 only) Also includes: extension manager hot-activation for WASM channels, extension guidance in LLM prompts, channel manager hot-add support, webhook router channel lookup, and minor cleanups. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: send approval prompts as messages on WASM channels (Telegram, Slack) WASM channels mapped ApprovalNeeded status to a typing indicator, so users on Telegram never saw tool approval prompts — the agent got stuck in AwaitingApproval and all subsequent messages failed with "Waiting for approval". - Intercept ApprovalNeeded in WasmChannel::handle_status_update and send the prompt as an actual message via call_on_respond, showing tool name, description, parameters, and yes/no/always instructions - Guard against empty LLM responses after clean_response() strips reasoning_content think-tags (defense-in-depth for reasoning models) - Add reasoning_content fallback to NearAiChatProvider::complete() for consistency with complete_with_tools() - Add debug logging when empty responses are suppressed - Improve error logging for channel respond() failures - Register WASM channel webhook routes before credential checks so platforms don't deactivate webhook URLs with 404s Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR nearai#297 review comments - ChannelManager::add: use async write().await instead of try_write() - resolve_target_dir: resolve relative CARGO_TARGET_DIR against crate_dir - install_wasm_files: log warning on capabilities copy failure - refresh_active_channel: load capabilities file for webhook secret name - activate_wasm_channel: validate name against path traversal - Fix cargo fmt formatting in nearai_chat.rs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: wire up channel runtime for hot-activation and address PR review round 2 - Wire up set_channel_runtime() in main.rs so hot-activation actually works (with_channel_runtime was never called — hot-activation was dead code) - Change ExtensionManager channel runtime fields to RwLock<Option<...>> interior mutability so set_channel_runtime(&self) works after Arc wrapping - Fix artifact tests to use resolve_target_dir() instead of hardcoding "target/" (breaks when CARGO_TARGET_DIR is set) - Fix bundled.rs build hint: cargo component build (not cargo build --target) - Fix wasm_artifact_path doc: binary_name should not include .wasm extension Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: use char-aware truncation to prevent UTF-8 panic in approval prompt &s[..77] panics on multi-byte UTF-8 (CJK, emoji). Use s.chars().take(77) for safe truncation at character boundaries. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…fact resolution (nearai#297) * refactor: unify WASM artifact resolution into registry/artifacts.rs Consolidate duplicated WASM find/build/install logic from 5+ files into a single src/registry/artifacts.rs module. This fixes two bugs: - registry/installer.rs now respects CARGO_TARGET_DIR (was hardcoded) - channels/wasm/bundled.rs now searches all WASM triples (was wasip2 only) Also includes: extension manager hot-activation for WASM channels, extension guidance in LLM prompts, channel manager hot-add support, webhook router channel lookup, and minor cleanups. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: send approval prompts as messages on WASM channels (Telegram, Slack) WASM channels mapped ApprovalNeeded status to a typing indicator, so users on Telegram never saw tool approval prompts — the agent got stuck in AwaitingApproval and all subsequent messages failed with "Waiting for approval". - Intercept ApprovalNeeded in WasmChannel::handle_status_update and send the prompt as an actual message via call_on_respond, showing tool name, description, parameters, and yes/no/always instructions - Guard against empty LLM responses after clean_response() strips reasoning_content think-tags (defense-in-depth for reasoning models) - Add reasoning_content fallback to NearAiChatProvider::complete() for consistency with complete_with_tools() - Add debug logging when empty responses are suppressed - Improve error logging for channel respond() failures - Register WASM channel webhook routes before credential checks so platforms don't deactivate webhook URLs with 404s Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR nearai#297 review comments - ChannelManager::add: use async write().await instead of try_write() - resolve_target_dir: resolve relative CARGO_TARGET_DIR against crate_dir - install_wasm_files: log warning on capabilities copy failure - refresh_active_channel: load capabilities file for webhook secret name - activate_wasm_channel: validate name against path traversal - Fix cargo fmt formatting in nearai_chat.rs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: wire up channel runtime for hot-activation and address PR review round 2 - Wire up set_channel_runtime() in main.rs so hot-activation actually works (with_channel_runtime was never called — hot-activation was dead code) - Change ExtensionManager channel runtime fields to RwLock<Option<...>> interior mutability so set_channel_runtime(&self) works after Arc wrapping - Fix artifact tests to use resolve_target_dir() instead of hardcoding "target/" (breaks when CARGO_TARGET_DIR is set) - Fix bundled.rs build hint: cargo component build (not cargo build --target) - Fix wasm_artifact_path doc: binary_name should not include .wasm extension Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: use char-aware truncation to prevent UTF-8 panic in approval prompt &s[..77] panics on multi-byte UTF-8 (CJK, emoji). Use s.chars().take(77) for safe truncation at character boundaries. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary
ChannelManager::hot_add().ExtensionManager::activate_wasm_channel()handles the full lifecycle (load WASM, inject credentials, register webhook router, start channel). Channels with missing required secrets are skipped at startup and activated later when the user saves credentials via the web UI.refresh_active_channel()re-injects credentials, updates the webhook secret in the router, and re-callson_start()to re-register webhooks (e.g., TelegramsetWebhook).registry/artifacts.rswithfind_wasm_artifact()that searches across all WASM target triples, replacing hardcodedwasm32-wasip2paths in bundled.rs and installer.rs.Test plan
cargo clippy --all --benches --tests --examples --all-features— zero warningscargo test— all 1390 tests pass🤖 Generated with Claude Code