fix(reborn): guide Google OAuth refresh setup - #5054
serrrfirat wants to merge 6 commits into
Conversation
|
🚅 Deployed to the ironclaw-pr-5054 environment in ironclaw-ci-preview
|
📝 WalkthroughWalkthroughAdds a ChangesGoogle OAuth access-only exchange and refresh guidance
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a new configuration field requires_refresh_token_on_exchange to HostOAuthProviderSpec to enforce the presence of a refresh token during the OAuth token exchange process. This is enabled for Google but disabled for Notion. Additionally, a check has been added to HostOAuthProviderClient::exchange_callback to return a TokenExchangeFailed error if a required refresh token is missing, and a corresponding unit test has been added to verify this behavior. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@crates/ironclaw_reborn_composition/src/auth_prompt.rs`:
- Around line 181-184: The Google OAuth refresh guidance is being appended
whenever any credential requirement is Google OAuth, but this is inconsistent
with the logic in auth_prompt_from_credential_requirement which only derives
provider-specific fields when there is exactly one requirement. This causes
Google-specific instructions to be added to prompts in multi-requirement
scenarios where multiple providers are involved. Modify the condition that calls
with_google_oauth_refresh_guidance to also verify that credential_requirements
has exactly one requirement, similar to the check performed around line 160, so
that Google-specific guidance is only injected when Google OAuth is the sole
requirement.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1379c17d-e57b-4f07-b8eb-7dcf7b6632cf
📒 Files selected for processing (3)
crates/ironclaw_reborn_composition/src/auth_prompt.rscrates/ironclaw_reborn_composition/src/oauth_provider_client/tests.rscrates/ironclaw_reborn_composition/src/projection/tests/turn_stream_auth.rs
Summary
Tests
Note: I did not rerun all-targets clippy after this update because this worktree filesystem had only ~116 MB free during verification; the broad all-targets test build failed on disk exhaustion before any code assertion ran. I cleaned generated Cargo incremental artifacts and ran the targeted library checks above.