[aj-rosado]

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-42224

📔 Objective

While a login-with-device request waits for approval, AuthRequestManagerImpl.createAuthRequestWithUpdates polls the server every 4 seconds. Any single failed poll emitted CreateAuthRequestResult.Error, so a brief network interruption (for example while the app is backgrounded) showed an error even though the request could still be approved a moment later.

This change treats poll failures as transient: they are retried, and an error is emitted once after 3 consecutive failures (MAX_CONSECUTIVE_POLL_FAILURES). A successful poll resets the counter, and polling continues after the error so a later poll can still recover, as before.

Tests: three new cases in AuthRequestManagerTest — a single transient failure is retried without an Error, repeated failures emit exactly one Error and then recover, and the failure count resets after a successful poll.

🤖 AI-assisted review

• The initial fix was drafted by an automated bug-smasher run, then amended by the author (retry threshold of 3 instead of 5) and formatted by detekt.
• Local code review (Standard, code-review-local) of the branch: APPROVE, no findings. It covered the branch except a final one-line trailing-comma formatting amend.
• Preflight: AuthRequestManagerTest (36 tests) and detekt pass.
• Open question from the reviewer, not raised as a finding: polls run every 4 seconds, so 3 failures take about 12 seconds. A longer outage while the app is backgrounded still shows the error (it clears on the next successful poll, as before). Whether this resolves the original repro on a device is not covered by these tests.

[github-actions]

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR changes AuthRequestManagerImpl.createAuthRequestWithUpdates so a failed login-with-device poll no longer emits CreateAuthRequestResult.Error right away. At the current head, MAX_CONSECUTIVE_POLL_FAILURES is 3, so the error is emitted once, on the third consecutive failure. Any successful poll resets the counter, polling continues after the error, and the approved, declined, and expired terminal states work as before. The three new AuthRequestManagerTest cases match the threshold of 3: a single transient failure, five failures that produce exactly one Error and then recover, and two failure streaks of two separated by a pending poll, which produce no Error.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.85%. Comparing base (5a5399f) to head (95bfbae).
⚠️ Report is 21 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7602      +/-   ##
==========================================
+ Coverage   86.82%   86.85%   +0.03%     
==========================================
  Files         896      898       +2     
  Lines       65805    65204     -601     
  Branches     9411     9377      -34     
==========================================
- Hits        57135    56636     -499     
+ Misses       5188     5097      -91     
+ Partials     3482     3471      -11     

Flag	Coverage Δ
app-data	18.99% <100.00%> (+0.28%)	⬆️
app-ui-auth-tools	18.75% <0.00%> (+0.14%)	⬆️
app-ui-platform	16.02% <0.00%> (-0.45%)	⬇️
app-ui-vault	27.02% <0.00%> (+0.05%)	⬆️
authenticator	6.06% <0.00%> (+0.04%)	⬆️
lib-core-network-bridge	4.16% <0.00%> (+0.03%)	⬆️
lib-data-ui	1.33% <0.00%> (+0.07%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[david-livefront]

The pattern we usually follow it to make these private helper classes and functions be top-level at the bottom of the file.

Looks like the setUpPollFailureScenario function needs to be in the test class, but can you move it to the bottom.

[david-livefront]

Should this be consecutiveFailures >= MAX_CONSECUTIVE_POLL_FAILURES