fix(channels): use live owner binding during wasm hot activation#1171
fix(channels): use live owner binding during wasm hot activation#1171zmanian merged 2 commits intonearai:stagingfrom
Conversation
Summary of ChangesHello, 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 addresses an issue where hot-activated WASM channels could lose their owner_id if the owner binding changed after the application's initial startup. The solution ensures that the system dynamically fetches the most current owner_id from live settings, improving the robustness of channel activation and preventing channels from regressing to a pairing state due to outdated owner information. Highlights
Changelog
Activity
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
|
There was a problem hiding this comment.
Code Review
This pull request correctly addresses the issue of resolving WASM channel owner IDs during hot activation by fetching them from live settings instead of relying on a startup snapshot. The introduction of current_channel_owner_id with a fast-path cache and a fallback to the database is a solid approach. The changes are well-contained and the logic is sound. I've added one suggestion to enhance the test coverage to ensure both the cache and database fallback paths are verified.
|
Addressed in commit
|
zmanian
left a comment
There was a problem hiding this comment.
Review: Read owner binding live during WASM hot activation
Good fix. Owner binding was read from a stale snapshot taken when the runtime lock was acquired. If binding happened after (e.g., Telegram verification), the activation path missed it.
Positives:
current_channel_owner_id()checks runtime state first, then falls back to database- Removes
wasm_channel_owner_idsfrom the destructured tuple, so it's never read from the snapshot - Test coverage for both runtime fast-path and DB fallback (libsql feature-gated)
- Defensive string-to-i64 parsing for stored values
LGTM.
…rai#1171) * fix(channels): use live owner binding during wasm hot activation * test(channels): cover owner-id store fallback without panic macros
…rai#1171) * fix(channels): use live owner binding during wasm hot activation * test(channels): cover owner-id store fallback without panic macros
Summary
channels.wasm_channel_owner_ids.<name>) instead of only using the startup snapshotWhy
activate_wasm_channel()previously read owner bindings only fromset_channel_runtime()state captured at startup. If owner binding changed after startup, hot-activated channels could miss owner_id and regress to pairing state.Fixes #1052
Validation
cargo fmt --allcargo test test_current_channel_owner_id_uses_runtime_state --quiet