fix(yuanbao): correct AuthBind plugin_version / bot_version mapping#2804
Conversation
- Swap DeviceInfo.app_version (plugin_version) and bot_version (framework) to match yuanbao-openclaw-plugin's sendAuthBind semantics. - Strip legacy `openhuman/` prefix from configured bot_version; default plugin version constant is now `0.1.0`. - Sync X-AppVersion/X-Bot-Version sign headers and auth_bind_smoke test. - Bump OPENHUMAN_INSTANCE_ID to 20 to match minted sign-token instance.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRefactors Yuanbao provider version handling: adds DEFAULT_PLUGIN_VERSION and a prefix-stripping helper, accepts ChangesYuanbao Version Alignment
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
lrt4836
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
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/providers/yuanbao/proto_constants.rs (1)
67-72:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix stale protocol doc value (
16vs20)The comment still says
OPENCLAW_ID = 16, but Line 72 now setsOPENHUMAN_INSTANCE_IDto20. Please keep the inline reference aligned to avoid future regressions.Suggested fix
-/// the `X-Instance-Id` HTTP header. Mirrors `OPENCLAW_ID = 16` used by +/// the `X-Instance-Id` HTTP header. Mirrors `OPENCLAW_ID = 20` used by🤖 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/providers/yuanbao/proto_constants.rs` around lines 67 - 72, The inline comment is outdated: it references `OPENCLAW_ID = 16` while the constant `OPENHUMAN_INSTANCE_ID` is set to "20"; update the comment to reflect the current value (20) and ensure the reference to `yuanbao-openclaw-plugin` (`OPENCLAW_ID`) matches this value so the documentation and `OPENHUMAN_INSTANCE_ID` stay aligned.
🧹 Nitpick comments (1)
src/openhuman/channels/providers/yuanbao/proto.rs (1)
603-619: ⚡ Quick winAssert version-field mapping in
auth_bind_smokeThe test updates inputs (Lines 610/612) but never verifies they land in the expected DeviceInfo fields. Add assertions for field
1(app_version) and field24(bot_version) to make this a real regression guard.Suggested enhancement
let frame = decode_conn_msg(&buf).unwrap(); assert_eq!(frame.cmd, cmd::AUTH_BIND); assert_eq!(frame.module, module::CONN_ACCESS); assert!(!frame.data.is_empty()); + + let req_fields = parse_fields(&frame.data).unwrap(); + let dev_buf = get_bytes(&req_fields, 3); + let dev_fields = parse_fields(&dev_buf).unwrap(); + assert_eq!(get_string(&dev_fields, 1), "0.1.0"); // app_version + assert_eq!(get_string(&dev_fields, 24), "1.0"); // bot_version🤖 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/providers/yuanbao/proto.rs` around lines 603 - 619, In auth_bind_smoke, add assertions that the DeviceInfo fields written by encode_auth_bind actually contain the expected app and bot versions: after calling decode_conn_msg(&buf).unwrap() and verifying cmd/module, extract the decoded DeviceInfo (from frame.data or the structure returned by decode_conn_msg) and assert that field 1 equals "0.1.0" (app_version) and field 24 equals "1.0" (bot_version); reference the existing helpers encode_auth_bind and decode_conn_msg when locating where to read frame.data and use the field keys 1 and 24 in your assertions.
🤖 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/channels/providers/yuanbao/config.rs`:
- Line 13: DEFAULT_PLUGIN_VERSION is set to "0.1.1" but must match the agreed
contract "0.1.0"; update the const DEFAULT_PLUGIN_VERSION to "0.1.0" in
config.rs and verify any code that relies on DEFAULT_PLUGIN_VERSION (e.g.,
AuthBind/sign flows) behaves the same with the corrected default to avoid
drifting from the intended openclaw compatibility.
---
Outside diff comments:
In `@src/openhuman/channels/providers/yuanbao/proto_constants.rs`:
- Around line 67-72: The inline comment is outdated: it references `OPENCLAW_ID
= 16` while the constant `OPENHUMAN_INSTANCE_ID` is set to "20"; update the
comment to reflect the current value (20) and ensure the reference to
`yuanbao-openclaw-plugin` (`OPENCLAW_ID`) matches this value so the
documentation and `OPENHUMAN_INSTANCE_ID` stay aligned.
---
Nitpick comments:
In `@src/openhuman/channels/providers/yuanbao/proto.rs`:
- Around line 603-619: In auth_bind_smoke, add assertions that the DeviceInfo
fields written by encode_auth_bind actually contain the expected app and bot
versions: after calling decode_conn_msg(&buf).unwrap() and verifying cmd/module,
extract the decoded DeviceInfo (from frame.data or the structure returned by
decode_conn_msg) and assert that field 1 equals "0.1.0" (app_version) and field
24 equals "1.0" (bot_version); reference the existing helpers encode_auth_bind
and decode_conn_msg when locating where to read frame.data and use the field
keys 1 and 24 in your assertions.
🪄 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: 07c0468a-b865-434c-be1f-f5be0ec81241
📒 Files selected for processing (5)
src/openhuman/channels/providers/yuanbao/config.rssrc/openhuman/channels/providers/yuanbao/connection.rssrc/openhuman/channels/providers/yuanbao/proto.rssrc/openhuman/channels/providers/yuanbao/proto_constants.rssrc/openhuman/channels/providers/yuanbao/sign.rs
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
@lrt4836 heads up — CI is failing on this PR (Rust Quality fmt+clippy, TypeScript type check, and PR Submission Checklist), so i'll hold off on a full review until those are sorted. i did spot a couple things while skimming:
DEFAULT_PLUGIN_VERSIONis"0.1.1"but the commit message and the test in proto.rs both target"0.1.0"— that inconsistency needs to be resolved.- The TOML config field
bot_versionnow routes toapp_version(plugin_version) in the protocol after this swap, but the key name still saysbot_version. Existing deployments that configured this field will silently see their value used as the plugin version rather than the framework version. Worth a note in the changelog or at minimum a deprecation comment on the field, so operators aren't surprised.
Fix the CI failures first and i'll do a proper review after!
…semantics - Fix typo where DEFAULT_PLUGIN_VERSION drifted to 0.1.1; align with the legacy "openhuman/0.1.0" value the prefix-strip is meant to preserve. - Add prominent NOTE on YuanbaoConfig.bot_version explaining the field controls plugin_version, not framework bot_version. - Accept `plugin_version` as a serde alias so new configs can use the clearer key without breaking existing `bot_version` deployments.
…pping - proto_constants.rs: update stale `OPENCLAW_ID = 16` comment to `20` so it matches the bumped `OPENHUMAN_INSTANCE_ID` constant. - proto.rs: extend `auth_bind_smoke` to decode the DeviceInfo payload and assert field 1 == app_version (plugin_version) and field 24 == bot_version, guarding against future regressions of the version-field swap.
OpenHuman has no separate plugin layer like yuanbao-openclaw-plugin's npm package, so the AuthBind plugin_version slot is repurposed for the yuanbao channel provider's own iteration version. Spell that out on both DEFAULT_PLUGIN_VERSION and the YuanbaoConfig.bot_version field so future operators know what to bump.
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
Previous changes are addressed. The four follow-up commits resolve both the CodeRabbit flag (DEFAULT_PLUGIN_VERSION 0.1.1 -> 0.1.0) and my bot_version semantic concern.
The doc comment on bot_version now clearly explains the wire-slot mapping, and the plugin_version serde alias gives new configs a clean path forward. The auth_bind_smoke test now validates field-level placement (DeviceInfo fields 1 and 24), which is a solid regression guard for the swap.
CI is fully green. Good to go.
oxoxDev
left a comment
oxoxDev
left a comment
There was a problem hiding this comment.
LGTM as 2nd maintainer pass. Clean field-mapping fix, comprehensive regression-guard test, serde alias preserves migration path. graycyrus's bot_version semantic concern + CodeRabbit's 0.1.1 → 0.1.0 flag both addressed by the 4 follow-up commits.
Verified
- Field swap consistency —
connection.rs:404-417extractsplugin_version = strip_version_prefix(&cfg.bot_version)+framework_version = env!("CARGO_PKG_VERSION")and passes them in the right positions (6th = plugin, 8th = bot) toencode_auth_bind. Matches the order the updatedauth_bind_smokeasserts. - Regression-guard test (
proto.rs:602-622) does FIELD-LEVEL placement verification viaparse_fields → get_bytes(req, 3) → parse_fields → get_string(dev, 1)/get_string(dev, 24). Catches any future re-swap regression — solid lock-in. - Serde alias (
config.rs:71) —#[serde(default = "default_bot_version", alias = "plugin_version")]lets new configs use the clearer key name while preserving backward-compat with the legacybot_versionkey. Good migration pattern. - Legacy prefix strip —
strip_version_prefixapplied at the connection boundary, not in config struct — keeps stored value verbatim, normalizes at use site. DEFAULT_PLUGIN_VERSIONconstant — single source of truth atconfig.rs:20, reused indefault_bot_version()ANDsign.rs:214X-AppVersionheader. Bump-once propagates everywhere.OPENHUMAN_INSTANCE_ID16→20 — comment explicitly pins toOPENCLAW_ID = 20inyuanbao-openclaw-plugin/src/access/ws/conn-codec.ts. Trusting the cross-repo pin (graycyrus + CodeRabbit confirmed).- Doc comments on
DEFAULT_PLUGIN_VERSION,YuanbaoConfig.bot_version,default_bot_version, andauth_bind_smokethoroughly explain wire-slot semantics + operator migration note for the legacybot_versionTOML key.
Ready to ship.
4 participants
Summary
Align Yuanbao provider's AuthBind / sign-header version semantics with
yuanbao-openclaw-plugin'ssendAuthBind:DeviceInfo.app_version(serverplugin_version) andbot_version(OpenHuman framework /CARGO_PKG_VERSION).openhuman/prefix from configuredbot_version; default plugin version is now0.1.0.X-AppVersion/X-Bot-Versionsign headers andauth_bind_smoketest.OPENHUMAN_INSTANCE_IDto20to match the instance id the sign endpoint mints.Test plan
cargo check -p openhumancargo test -p openhuman auth_bind_smokeextra_infoAuthBind shows swapped values.Summary by CodeRabbit
Bug Fixes
Documentation
Chores
Tests