fix(broker): treat blank nested sessionRef as absent in bridge - #1478
Conversation
`attestation.session_ref.is_none()` did not cover the case where the
nested `sessionRef` inside `metadata.attestation` is a blank/whitespace
string (e.g. serialised as `" "`). A dispatcher that sends
`{"sessionRef":" "}` inside the attestation object would suppress a
valid top-level `session_ref`, leaving the Session-Id trailer unset.
Replace the bare `is_none()` guard with the same blank-aware check used
by `spawner.rs::is_valid_attestation_value`: treat any value whose
trimmed form is empty as absent, so the top-level alias still wins.
Adds test `blank_nested_session_ref_is_treated_as_absent` to cover
this branch directly. 895 broker unit tests pass.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughBroker attestation bridging now treats blank or whitespace-only nested session references as absent. A valid top-level ChangesSpawn attestation bridging
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Clippy (unnecessary_map_or) flagged the `.map_or(true, |v| v.is_empty())` pattern; `.is_none_or(|v| v.is_empty())` is the idiomatic equivalent and satisfies `-D warnings`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
relayfile-subs-lead-0811 E2E delivery test — 2026-08-11T11:14:18Z |
|
relayfile-subs-lead-0811 E2E delivery test — deploy confirmed green, firing live test 2026-08-11T11:45:02Z |
1 participant
Summary
Follow-up to #1477 addressing one post-merge coderabbit finding.
attestation.session_ref.is_none()inbroker_payload_from_actiondid not cover the case where the nestedsessionRefinsidemetadata.attestationis a blank/whitespace string (e.g." "). A dispatcher that sends{"sessionRef":" "}inside the attestation block would suppress a valid top-levelsession_ref, leaving theSession-Idcommit trailer unset.is_none()with the same blank-aware check used byspawner.rs::is_valid_attestation_value: trims the nested value and treats an empty result as absent, allowing the top-level alias to fill the field.blank_nested_session_ref_is_treated_as_absenttest to cover this branch directly.Test plan
cargo test -p agent-relay-broker --lib -- relaycast::bridge::tests::blank→ 3 tests pass (blank_nested, blank_session_ref_falls_through, blank_session_id_falls_through)cargo test -p agent-relay-broker --lib→ 895 tests pass, 0 failures🤖 Generated with Claude Code