fix(telegram): String owner_id test + registry bump - #2620
Merged
Merged
Conversation
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>
github-actions
Bot
added
size: XS
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes the Telegram channel’s test_config_full to match the post-#2471 owner_id: Option<String> behavior, and bumps the Telegram registry version so the CI version-bump gate passes for this change.
Changes:
- Update
test_config_fullto assertowner_id == Some("42".to_string())(string) instead ofSome(42)(integer). - Bump
registry/channels/telegram.jsonversion from0.2.9to0.2.10.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| registry/channels/telegram.json | Patch version bump to satisfy the version-bump check when channels-src/telegram/ changes. |
| channels-src/telegram/src/lib.rs | Fixes a failing test by asserting the string-typed owner_id produced by the new deserializer. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
serrrfirat
approved these changes
Apr 18, 2026
ilblackdragon
added a commit
that referenced
this pull request
Apr 19, 2026
…variants
- Expand UserRole to {Owner, Admin, Regular}
- UserId carries role; methods is_owner()/is_admin()/is_regular()
- Remove From<String>/From<&str> impls (enforces types.md rule)
- Validated construction via new(); from_trusted() for DB-sourced values
Addresses bug pattern from #2561, #2620, #2349 where owner_id silently
round-tripped as String.
5 of 6 tasks
ilblackdragon
added a commit
that referenced
this pull request
Apr 20, 2026
…variants (#2677) * refactor(ownership): collapse OwnerId+Identity into UserId with role variants - Expand UserRole to {Owner, Admin, Regular} - UserId carries role; methods is_owner()/is_admin()/is_regular() - Remove From<String>/From<&str> impls (enforces types.md rule) - Validated construction via new(); from_trusted() for DB-sourced values Addresses bug pattern from #2561, #2620, #2349 where owner_id silently round-tripped as String. * refactor(ownership): address review feedback — id-only equality, persist owner role, doc fixes - UserId PartialEq/Eq/Hash now compare only `id`, not `role`. Role is metadata that travels with the identity; two UserIds with the same id but different roles must be interchangeable as HashMap/HashSet keys and cache lookup targets. Added a regression test that builds a HashSet keyed on UserId and asserts cross-role `.contains()` membership, plus a hash-equality check. - CLI pairing path now persists the "owner" role string (via UserRole::Owner.as_db_role()) instead of the hardcoded "admin", so a reload through UserRole::from_db_role stays Owner rather than being silently downgraded to Admin. - Update the feature/pairing approve handler to mirror the refactor: build UserId via from_trusted + UserRole::from_db_role(&user.role) instead of the removed OwnerId::from. - AdminScope doc comment now reflects that Owner also passes is_admin(). - AdminUser extractor error message now reads "Admin privileges required (admin or owner)" so the forbidden response matches the actual gate. --------- Co-authored-by: Henry Park <henrypark133@gmail.com>
theredspoon
pushed a commit
to theredspoon/ironclaw
that referenced
this pull request
Jun 21, 2026
theredspoon
pushed a commit
to theredspoon/ironclaw
that referenced
this pull request
Jun 21, 2026
…variants (nearai#2677) * refactor(ownership): collapse OwnerId+Identity into UserId with role variants - Expand UserRole to {Owner, Admin, Regular} - UserId carries role; methods is_owner()/is_admin()/is_regular() - Remove From<String>/From<&str> impls (enforces types.md rule) - Validated construction via new(); from_trusted() for DB-sourced values Addresses bug pattern from nearai#2561, nearai#2620, nearai#2349 where owner_id silently round-tripped as String. * refactor(ownership): address review feedback — id-only equality, persist owner role, doc fixes - UserId PartialEq/Eq/Hash now compare only `id`, not `role`. Role is metadata that travels with the identity; two UserIds with the same id but different roles must be interchangeable as HashMap/HashSet keys and cache lookup targets. Added a regression test that builds a HashSet keyed on UserId and asserts cross-role `.contains()` membership, plus a hash-equality check. - CLI pairing path now persists the "owner" role string (via UserRole::Owner.as_db_role()) instead of the hardcoded "admin", so a reload through UserRole::from_db_role stays Owner rather than being silently downgraded to Admin. - Update the feature/pairing approve handler to mirror the refactor: build UserId via from_trusted + UserRole::from_db_role(&user.role) instead of the removed OwnerId::from. - AdminScope doc comment now reflects that Owner also passes is_admin(). - AdminUser extractor error message now reads "Admin privileges required (admin or owner)" so the forbidden response matches the actual gate. --------- Co-authored-by: Henry Park <henrypark133@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
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
PR #2471 (hot-activation
owner_idunification) changedTelegramConfig::owner_idfromOption<i64>toOption<String>with adeserialize_string_or_numberdeserializer, but missed thetest_config_fullcase inchannels-src/telegram/src/lib.rs:2622which still assertsSome(42)(integer). That breaks the Telegram Channel Tests CI job and is one of four blockers on the staging→main promotion PR #2612.Bumped
registry/channels/telegram.json0.2.9 → 0.2.10 so the Version Bump Check passes as well.Change Type
Linked Issue
Unblocks #2612.
Validation
cargo test --lib test_config_full(inchannels-src/telegram/) passes locallySecurity Impact
None.
Database Impact
None.
Blast Radius
channels-src/telegramtest-only change + registry patch version bump.Rollback Plan
Revert this commit on staging.
Review track: A (test + version bump)
🤖 Generated with Claude Code