Repository navigation
fix(management): retry transient Devin OAuth callback read errors - #13
Conversation
waitDevinOAuthCallback ended the login with "Failed to read OAuth callback" on any read error other than not-exist. On Windows a read that lands while the callback handler renames the file into place fails with a sharing violation, so a login could fail although the callback arrived. Retry on the next tick, as the other OAuth flows already do. The context deadline and session state still bound the wait. This was the cause of the intermittent "token exchange did not start" in TestDevinRemoteOAuthFlow. The test now reports the session status on that failure, and its 5s failure bounds are 15s. Backport of upstream router-for-me#7482 (a50f54e), with the Devin test bound change from 97b8a03 that the regression test depends on.
|
No actionable comments were generated in the recent review. Walkthrough
Merge Risk:
|
[Medium impact] This PR appears safe to merge. SummaryDevin OAuth now retries callback-file read errors instead of ending the login immediately.
Reviews (1) · Last reviewed commit: "fix(management): retry transient Devin O..." · Reviewed by Greptile |
Devin logins on Windows could fail with "Failed to read OAuth callback" even after the callback arrived. This backports the fix from upstream PR router-for-me#7482 so the fork has it before upstream merges.
The callback handler writes a temp file and renames it into place. On Windows, a read of that path during the rename fails with a sharing violation.
waitDevinOAuthCallbacktreated every read error except not-exist as fatal and ended the login. It now retries on the next 500ms tick, as the Claude, Codex and Antigravity flows already do. The context deadline and the session state still end the wait.TestWaitDevinOAuthCallbackRetriesTransientReadErrorcovers the fix and fails on the old code.TestDevinRemoteOAuthFlownow prints the session status when the token exchange does not start, and its 5s failure bounds are 15s.The two files match the upstream PR head exactly, so the next upstream sync merges cleanly. #12 had its own 5s to 15s bump in the same test. I dropped it there, and the two PRs merge cleanly in either order.
go build,go vetand themanagementpackage tests pass on Windows. The Devin tests passed 20/20 in a loop.Summary by CodeRabbit