Repository navigation
fix(channels): unify hot-activation owner_id type and capabilities fallback - #2471
Merged
Merged
Conversation
The bundled Telegram capabilities.json ships `"owner_id": null`. The previous code only called `Value::as_i64()`, which returns `None` for `Null`, so the fallback silently produced no owner — the fix never actually worked for Telegram. Changes: - Handle `Null`, `String`, and `Number` variants in `owner_actor_id_for_channel()` so the real production payload works. - Propagate the *resolved* owner_id into the WASM runtime config map regardless of whether it came from runtime config or capabilities fallback (previously only the runtime-config path injected it). - Add `tracing::debug!` for non-scalar owner_id values to aid debugging. - Add tests: null config, missing capabilities file, empty string, non-scalar value, and caller-level register_channel tests that verify config injection and null-owner-id handling. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ot-activation Address two follow-up items from PR #2349 review: 1. Type consistency: boot path injected owner_id as Value::String, but hot-activation path (build_wasm_channel_runtime_config_updates) used Value::Number. Changed the function to accept Option<&str> and inject as Value::String, matching the boot path. 2. Capabilities fallback: hot-activation paths (complete_loaded_wasm_channel_activation and refresh_active_channel) only checked runtime HashMap and settings store. Now they also consult capabilities.json via the extracted owner_id_from_capabilities() helper, matching the boot path's behavior. Also updates the telegram WASM module to accept both string and number JSON for owner_id via a custom deserializer, since all other channels already use Option<String>. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
size: L
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the handling of owner_id for Telegram and WASM channels, transitioning the field from a numeric type to a string to support both JSON numbers and strings. It introduces a robust fallback mechanism for resolving the owner ID by checking runtime configuration, the settings store, and the channel's capabilities file. Additionally, the PR includes new deserialization utilities, updated dependency versions in Cargo.toml, and comprehensive unit tests for the new resolution logic. I have no feedback to provide as there are no review comments.
serrrfirat
marked this pull request as draft
April 14, 2026 20:20
serrrfirat
marked this pull request as draft
April 14, 2026 20:20
serrrfirat
marked this pull request as ready for review
April 17, 2026 07:17
henrypark133
previously approved these changes
Apr 17, 2026
henrypark133
left a comment
henrypark133
left a comment
Collaborator
There was a problem hiding this comment.
Review: hot-activation owner_id parity
No verified findings in the current diff.
The current head makes the boot path and hot-activation path agree on both owner-id typing and capabilities fallback, and the added tests cover numeric/string/null capability values plus the actual hot-activation call site. This is still stacked on #2349, so merge order matters, but I did not find a new blocker in this follow-up diff.
Conflicts resolved: - Cargo.toml / deny.toml: trivial key reordering; kept the superset of advisory ignores from staging. - src/channels/wasm/setup.rs: took staging's async resolve_owner_actor_id_for_channel (adds pairing_store fallback) and staging's crate::pairing::approval::build_runtime_config_updates helper. Kept this PR's extraction of owner_id_from_capabilities() so the hot-activation path can still reuse it. Tests updated to expect the numeric owner_id representation that the shared helper produces. - src/channels/wasm/wrapper.rs: took staging's async owner_actor_id_for_test (field is now Arc<RwLock<Option<String>>>). - src/extensions/manager.rs: dropped this PR's local build_wasm_channel_runtime_config_updates in favor of the shared helper (imported under the same name via a use-alias in staging). The capabilities fallback for complete_loaded_wasm_channel_activation and refresh_active_channel now chains with staging's pairing_store lookup (current_channel_owner_actor_id -> owner_id_from_capabilities). - The test_hot_activation_uses_capabilities_owner_id_fallback and test_telegram_hot_activation_runtime_config_includes_owner_id assertions were updated from Value::String to the numeric representation produced by build_runtime_config_updates.
github-actions
Bot
added
the
risk: medium
ilblackdragon
approved these changes
Apr 18, 2026
3 tasks
5 of 7 tasks
ilblackdragon
added a commit
that referenced
this pull request
Apr 18, 2026
PR #2471 changed `TelegramConfig::owner_id` from `Option<i64>` to `Option<String>` with a string-or-number deserializer, but missed the test case in channels-src/telegram/src/lib.rs:2622 which still asserts `Some(42)`. That breaks the Telegram Channel Tests CI job and has been blocking the staging→main promotion PR #2612. Also bump registry/channels/telegram.json from 0.2.9 to 0.2.10 so the Version Bump Check passes (changing channels-src/telegram/ requires a registry version bump). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 of 3 tasks
Merged
theredspoon
pushed a commit
to theredspoon/ironclaw
that referenced
this pull request
Jun 21, 2026
…llback (nearai#2471) * Fix WASM channel owner_id fallback * ci: ignore rand advisory * ci: satisfy cargo-deny path dependency versions * fix(telegram): handle null/string owner_id and propagate to WASM config The bundled Telegram capabilities.json ships `"owner_id": null`. The previous code only called `Value::as_i64()`, which returns `None` for `Null`, so the fallback silently produced no owner — the fix never actually worked for Telegram. Changes: - Handle `Null`, `String`, and `Number` variants in `owner_actor_id_for_channel()` so the real production payload works. - Propagate the *resolved* owner_id into the WASM runtime config map regardless of whether it came from runtime config or capabilities fallback (previously only the runtime-config path injected it). - Add `tracing::debug!` for non-scalar owner_id values to aid debugging. - Add tests: null config, missing capabilities file, empty string, non-scalar value, and caller-level register_channel tests that verify config injection and null-owner-id handling. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(channels): unify owner_id type and add capabilities fallback to hot-activation Address two follow-up items from PR nearai#2349 review: 1. Type consistency: boot path injected owner_id as Value::String, but hot-activation path (build_wasm_channel_runtime_config_updates) used Value::Number. Changed the function to accept Option<&str> and inject as Value::String, matching the boot path. 2. Capabilities fallback: hot-activation paths (complete_loaded_wasm_channel_activation and refresh_active_channel) only checked runtime HashMap and settings store. Now they also consult capabilities.json via the extracted owner_id_from_capabilities() helper, matching the boot path's behavior. Also updates the telegram WASM module to accept both string and number JSON for owner_id via a custom deserializer, since all other channels already use Option<String>. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Illia Polosukhin <ilblackdragon@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #2349 addressing two "Concerning" items from @henrypark133's re-review:
build_wasm_channel_runtime_config_updatesnow takesOption<&str>and injectsowner_idasValue::String, matching the boot path. Previously the hot-activation path injectedValue::Numberwhile the boot path usedValue::String— WASM modules received different JSON types depending on which path ran.complete_loaded_wasm_channel_activationandrefresh_active_channelnow consultcapabilities.jsonvia the extractedowner_id_from_capabilities()helper when neither runtime HashMap nor settings store have an owner_id. Previously only the boot path had this fallback.owner_id: Option<i64>toOption<String>with a custom deserializer that accepts both string and number JSON (all other channels already useOption<String>).Depends on #2349 (branched from its HEAD).
Test plan
owner_actor_id_*tests insetup.rsstill pass (pure refactor of extraction)test_telegram_hot_activation_runtime_config_includes_owner_idassertsValue::Stringtest_hot_activation_uses_capabilities_owner_id_fallbackdrivescomplete_loaded_wasm_channel_activationwith capabilities-only owner_id and verifies bothowner_actor_idand config injectioncargo clippy --all --benches --tests --examples --all-features— zero warningscargo test --lib— 4712 passed (1 pre-existing failure unrelated to this PR)🤖 Generated with Claude Code