fix(coordinator): reset the auth state only on a positive proof by jleinenbach · Pull Request #1267 · jleinenbach/GoogleFindMy-HA

jleinenbach · 2026-09-03T05:48:00Z

What was wrong

coordinator/polling.py and coordinator/locate.py read every non-throwing return of api.async_get_device_location as proof that the credentials work, and cleared the auth error state — in the poll loop also the transient auth counter and the stored cause — before asking whether the result had any content.

An empty dict is weak evidence that the request was accepted, not proof of it. Since #1263 the 5xx, the 429, the network error and the failed FCM registration raise LocationRequestNotAcceptedError instead of flattening into {}, but four pre-accept failures still arrive as an empty dict, and so does every healthy idle BLE tag.

The idle tag is what made this expensive. _consecutive_transient_auth_failures exists to escalate a genuinely expired login after three consecutive cycles. It is incremented in one place and was reset in exactly one everyday place: this success path. On an account with a single idle BLE tag it was cleared on every pass, so the threshold was never reached and the re-auth prompt this mechanism was built for never appeared.

What changed

Poll cycle — the empty guard runs first, the reset sits behind it. A location with content still clears both.
Manual locate — same move, with one deliberate limit: the reset sits behind the empty guard but in front of the coordinate check. A record carrying only last_seen is an authenticated server answer; the method returns {} for it, but the account was accepted. A test holds it in that position.
The counter's reset now sits on the clock that increments it. This took three attempts, and the two rejected ones are worth stating because each looked right in isolation:
- Resetting from the device-list refresh starves the escalation. The refresh runs inside the same _async_update_data and before the poll cycle, so at the default cadence the count was zeroed immediately before every attempt.
- Guarding that with a pending marker removed the cadence coupling but left the count on the foreign clock. With a 60 s poll interval and the fixed 300 s list interval, up to five cycles run without a refresh, so three rejections at minutes 0, 2 and 4 — each separated by a clean cycle — reached the threshold although they were never consecutive.
A poll cycle now zeroes the count and the stored cause when it booked no rejection and at least one of its requests was not refused. Both halves matter: cycle-wide (not per device) keeps an idle tag from wiping the rejection a sibling booked in the same pass, and requiring a non-refused request keeps a cycle of pure outage from clearing a budget on the strength of an outage. A pass that finds no pollable device clears the count too — but only while the account has devices, none is enabled and no cycle is still in flight, so an empty device list (an outage) clears nothing, and a cycle still spending its streak keeps it. That last condition defers the reset rather than suppressing it: the next refresh after the cycle retires clears the final count.

_set_auth_state(failed=False) stays on the device-list refresh. That branch proves the account token, which is what the auth state reports; it says nothing about the action RPC accepting that token, which is what the counter counts.

All five _set_auth_state(failed=False) sites now carry the same rule. The two sound handlers already stated it ("only an ACCEPTED submission proves the credentials worked"); this extends it to the two that did not.

What a user notices

In a healthy state, nothing: _set_auth_state(failed=False) is a no-op while no Repairs issue is open and _auth_error_active is unset.

In a failure state, one latency changes — how long an auth error message stays after the problem is gone. It used to be cleared by the next poll of any kind; now it takes an actual location or the next device-list refresh, so the added delay is bounded by roughly one device-list tick.

Measured limits, stated rather than discovered later

A persistent rejection escalates: with a broken tracker next to an idle one the threshold is reached on cycle three.
An intermittent rejection does not escalate when the cycles in between carried information: 403, empty, 403, empty, 403 measures [1, 0, 1, 0, 1] and raises no reauth, at any cadence.
It does still escalate when the cycles in between carried none: on a single-device account 403, timeout, 403, timeout, 403 measures [1, 1, 2, 2, 3] and does reauth. "Consecutive" is counted over cycles that carried information. This is a deliberate trade — the alternative lets an outage clear a budget, which is the fault this whole change removes.
If no poll cycle can run at all for a long time, the count is held in memory and a restart clears it.

Evidence

Every test was written first and run red against the unfixed tree, with the failure text recorded — assert not [{'failed': False}], assert 2 == 0, assert not [call(failed=False)], assert [1, 1, 2, 2, 3] == [1, 0, 1, 0, 1], got []. Thirty-eight single-line mutations were run and reverted individually; all but three die, and all three survivors are behaviour-neutral invariants marked as such in the code.

Prose that still described the old behaviour was swept in a separate commit, and AGENTS.md carries the rule with its measured numbers. Three review rounds by a reading agent and eight Codex rounds are folded in; each finding was re-measured before it was accepted, and those that turned out to be prose overreach rather than defects are corrected as prose.

The second verdict of a cycle

The counter is one of two outputs of a poll cycle. The other is last_exception, which feeds async_set_update_error and therefore every tracker's availability, and the last commits are about it.

A cycle that books a credential rejection and then gets location content from a sibling accepts that content as proof for the counter. It has to accept it for availability too, or one cycle yields two verdicts and every tracker goes offline right next to the device that just answered. The report is therefore withdrawn — bound to the identity of the rejection this cycle booked, so a timeout or a stale owner key that got there first stays.

That withdrawal must not take an unrelated report with it. last_exception is first-come, so a rejection booked first made a later timeout drop its own report, and the withdrawal then emptied the slot: the coordinator update reported success with a device that never answered. An overwrite would be wrong the other way round, because whether the withdrawal fires is only known after the loop, and without content the rejection is the more specific diagnosis. The later failure is therefore parked in a second slot and the withdrawal hands the place over instead of emptying it. Both directions are pinned, in both device orders.

The poll loop cleared the auth error state and the transient auth counter before it looked at whether the result had any content, so every empty return counted as proof that the credentials work. An empty dict is weak evidence that the request was accepted, not proof of it. The 5xx, the 429, the network error and the failed FCM registration raise LocationRequestNotAcceptedError today rather than flattening into {}, but four pre-accept failures still arrive as an empty dict, and so does every healthy idle BLE tag. That last part is what made this expensive. The counter exists to escalate a genuinely expired login after three consecutive cycles; a fleet with one idle tag reset it on every single pass, so the threshold was never reached and the re-auth prompt this mechanism was built for never appeared. The guard now runs first and the reset sits behind it. The positive half is unchanged: a location WITH content still clears both, which is the rule the sound handlers already state at their own sites. test_an_empty_return_still_clears_the_counter characterised the old behaviour and is renamed rather than deleted, with its assertions inverted and its history kept in the docstring.

…efresh The previous commit took away the counter's only everyday reset source. Left alone, a counter that once reached two would sit there forever on an account of idle BLE tags, and a single later hiccup would ask a user with a perfectly good login to sign in again. The two commits are one change and must not be shipped apart. The device list is the strongest proof source in the integration: async_get_basic_device_list has no non-throwing error exit, every except branch ends in raise ConfigEntryAuthFailed or raise UpdateFailed. A return that reaches this line therefore means Nova accepted the account token, which is exactly what the counter counts. An expired login raises before it can get there, so unlike the poll path it replaces, this source cannot mask the very failure it is meant to escalate. Both resets stay in the fetch branch. The cached branch skips the call entirely and so proves nothing, which is the same mistake one layer up.

async_locate_device carried the same false-success reasoning as the poll cycle: it cleared the auth error state before it had looked at whether the result had content. Pressing "locate" on an idle tag therefore wiped a pending credential finding raised by another device, which is the evidence the Repairs issue rests on. The reset moves behind the empty guard and stays in front of the coordinate check. A record carrying only last_seen is an authenticated server answer: the method returns {} for it, but the account was accepted, and refusing to count that would be the same error in the opposite direction. A test holds it in that position. test_manual_locate_client_error_leaves_the_auth_state_alone keeps its assertions; only its docstring changes, because the second half no longer depends on api.py passing the error through.

…eset CI caught what the plan's blast-radius grep did not. It searched the test tree for the literal `failed=False`, which finds an assertion written as `kw.get("failed") is False` but not one written as `_set_auth_state.assert_called_once_with(failed=False)` on a mock. One test was in the second shape and pinned the old behaviour from the positive side, on purpose, at a time when the reset in front of the empty guard was a known finding tracked separately. It is inverted rather than deleted, and its docstring carries the history and the reason: an empty dict is weak evidence that the request was accepted, not proof of it, and four pre-accept failures still arrive in exactly that shape. The positive half it was written to protect has not been given up, it moved -- a result with content, and a record carrying only last_seen, both still clear the auth state, and both are pinned in test_coordinator_locate_basics.py.

jleinenbach · 2026-09-03T07:32:38Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c72ae45e82

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-09-03T07:35:22Z

-    credentials work. That is tracked as a finding of its own with its own
-    approval gate (`PLAN_GFMY_AUTH_RESET_POSITIVE_PROOF`). This test is here so
-    the day it changes, it changes on purpose.
+async def test_an_empty_return_proves_nothing_about_the_credentials() -> None:


Update the contract alongside the renamed test

Renaming this test leaves custom_components/googlefindmy/AGENTS.md:254 citing test_an_empty_return_still_clears_the_counter, which no longer exists. The repository-wide TestTheDocumentedExtentStaysTrue::test_every_test_name_the_component_contracts_cite_exists resolves every component contract citation against collected test names, so this commit deterministically fails that check until the contract is updated to the new behavior and name.

AGENTS.md reference: AGENTS.md:L305-L305

Useful? React with 👍 / 👎.

…ding The reset now sits behind the empty guard on both the poll cycle and the manual locate, and the transient-auth counter is reset from the device list. Eleven places in the tree still described that as an open defect in the present tense, which is how a later reader is sent back to a fix that already landed. Every claim is rewritten as history rather than deleted, so a grep for the old wording still lands on the paragraph that explains what replaced it: - AGENTS.md: the "why the location handler raises" argument no longer rests on the reset it used to protect; raising is now justified by the rejection being a failure that an empty dict cannot express. The "what is still NOT fixed there" paragraph names the per-path proof instead, plus the bounded price (a pending auth error clears on the next device-list refresh, same 300s cadence) and the three tests that pin it. - api.py, location_request.py, polling.py, locate.py: same correction at each comment that argued from the defect as a current fact. - test docstrings: the characterisation history stays readable, including the old test names, per the plan's rule that a pin is rewritten and not removed. No executable code is touched: an AST comparison with docstrings stripped is identical on all six Python files. Fixes the deterministic CI failure in TestTheDocumentedExtentStaysTrue::test_every_test_name_the_component_contracts_cite_exists, which flagged AGENTS.md still citing the pre-rename test_an_empty_return_still_clears_the_counter.

codecov · 2026-09-03T08:08:35Z

Codecov Report

❌ Patch coverage is 96.07843% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...tom_components/googlefindmy/coordinator/polling.py	96.00%	0 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

jleinenbach · 2026-09-03T08:13:14Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f4c01296dd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-09-03T08:18:35Z

+                        "Device list refresh succeeded; clearing %d transient auth failure(s).",
+                        self._consecutive_transient_auth_failures,
+                    )
+                    self._consecutive_transient_auth_failures = 0


Preserve failures across the default refresh cadence

With the default configuration, both DEVICE_LIST_POLL_INTERVAL and DEFAULT_LOCATION_POLL_INTERVAL are 300 seconds, and _async_update_data() fetches the list before scheduling the location cycle. Consequently, if nbe_execute_action persistently returns a non-permanent credential rejection for a single tracker while nbe_list_devices succeeds, this line resets the counter to zero immediately before every location poll; that poll can only raise it back to 1, so the threshold of 3 and the reauthentication flow are never reached. The new tests call _async_start_poll_cycle() directly and therefore miss this production ordering. Keep the action failure budget independent of the list RPC, or otherwise ensure a list refresh cannot erase every due location failure.

AGENTS.md reference: custom_components/googlefindmy/AGENTS.md:L117-L119

Useful? React with 👍 / 👎.

@codex review Is this fixed?

Review findings on f4c0129, each measured against the source before it was written down. api.py: the new comment claimed the 5xx branch stays "for the sync wrapper, which has no such layer in front of it". The sync wrapper calls async_get_device_location itself, so it has exactly the same layer. Measured instead: get_location_data_for_device converts the 5xx and the 429 into LocationRequestNotAcceptedError, and its broad handler re-raises only before the accept line and returns [] after it, so no NovaHTTPError leaves that layer on any path. The branch is a guard for the day that conversion moves. AGENTS.md: the paragraph claimed the change buys an escalation that used to be starved. It does not, and the opposite reading is the dangerous one. Every device-list refresh still resets the transient-auth counter, and at default settings a refresh falls into every poll cycle, so one device failing transiently forever still cannot pass _MAX_TRANSIENT_AUTH_FAILURES -- the threshold is reached within a cycle or not at all. That is the trade the plan records as R-3, and an expired sign-in does not depend on the counter anyway because async_get_basic_device_list raises ConfigEntryAuthFailed. The paragraph now says what is actually bought: a pending auth error survives long enough to be seen. Same paragraph: "both run on the same 300-second cadence" held only for the default. DEVICE_LIST_POLL_INTERVAL is fixed at 300s, the poll interval is an option between 60 and 3600, so the bounded wait is up to five cycles at the shortest setting. And the per-path list of proofs mixed paths with counters: on the poll path a location with content clears the counter too. tests/test_api_basics.py was named in the plan's sweep list and missed by all eleven grep patterns: "positive proof" is split across a line break and the file writes "BEFORE it looks at" with a capital. It carried both defects -- the present-tense claim about the ordering, and a live pointer to test_an_empty_return_still_clears_the_counter, which no longer exists. Still no executable code: AST comparison with docstrings stripped is identical on both Python files.

jleinenbach · 2026-09-03T08:25:11Z

@codex review

chatgpt-codex-connector · 2026-09-03T08:30:43Z

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: 769c8f9c7d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 769c8f9c7d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-09-03T08:48:44Z

+                        "Device list refresh succeeded; clearing %d transient auth failure(s).",
+                        self._consecutive_transient_auth_failures,
+                    )
+                    self._consecutive_transient_auth_failures = 0


Preserve failures across device-list refreshes

At the default 300-second device-list and location cadence, _async_update_data() refreshes the list before scheduling each due location cycle, so when one tracker repeatedly raises a non-permanent credential NovaAuthError while list requests succeed, this reset runs before every attempt and the counter can only return to 1; _MAX_TRANSIENT_AUTH_FAILURES is therefore never reached and reauthentication never starts. Fresh evidence in this revision is the newly added reset here, which moves the starvation from empty location results to successful list refreshes rather than fixing the production ordering; the new tests either invoke _async_start_poll_cycle() directly or explicitly assert this reset, so they do not cover that ordering.

AGENTS.md reference: custom_components/googlefindmy/AGENTS.md:L263-L269

Useful? React with 👍 / 👎.

The device-list reset from the previous commit runs inside the same `_async_update_data` and BEFORE the poll cycle is scheduled. At the default settings both cadences are 300 seconds, so the counter was zeroed immediately before every attempt: a tracker whose action RPC kept returning a non-permanent credential rejection could only ever climb back to 1 and never reached `_MAX_TRANSIENT_AUTH_FAILURES`. The reset that was meant to replace one starvation had created another. A booked location failure now sets `_transient_auth_failure_since_list_refresh`, and the next refresh consumes that marker instead of clearing the count. The list proves the ACCOUNT token and nothing more; whether the action RPC accepts that same token again is what the counter counts. A one-off hiccup still heals, by the refresh after next rather than the next one; a persistent rejection accumulates until the threshold. `test_the_production_order_still_reaches_the_reauth_threshold` drives the real `_async_update_data` -> `_async_start_poll_cycle` sequence for three cycles with the real booking site, which is the ordering the previous tests missed by calling `_async_start_poll_cycle()` directly. Against the unfixed code it fails with `assert 1 >= 3` and logs `(1/3)` three times over. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

jleinenbach · 2026-09-03T09:37:31Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f5fb8d0bb8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-09-03T09:43:30Z

+                    # Consume the marker instead of making it sticky: a one-off
+                    # hiccup is healed by the refresh after next, a persistent
+                    # rejection accumulates across cycles until the threshold.
+                    self._transient_auth_failure_since_list_refresh = False


Do not consume the marker while its poll is still running

When a background poll lasts beyond the 300-second device-list interval—for example, five or more trackers approaching the 65-second per-device timeout—the 60-second coordinator ticks can perform a successful list refresh while _is_polling is still true. This line then consumes the marker set by an early credential rejection, but _is_polling prevents that refresh from starting another location cycle; after the old cycle finishes, the following list refresh clears the counter before the rejected tracker is attempted again, so persistent action-RPC rejection can still remain below the reauth threshold. Fresh evidence beyond the earlier cadence finding is that _is_polling gates only scheduling at line 1522, not these list refreshes during a long-running cycle; retain the marker until a subsequent poll cycle can actually test the action RPC.

AGENTS.md reference: custom_components/googlefindmy/AGENTS.md:L270-L272

Useful? React with 👍 / 👎.

… the refresh Review of the full diff found the previous commit's marker still starved the escalation, one layer along. `_transient_auth_failure_since_list_refresh` was consumed by the device-list refresh, which silently assumes exactly one refresh per poll cycle. Nothing couples the two: `DEVICE_LIST_POLL_INTERVAL` is a fixed 300 s while `location_poll_interval` is an option up to 3600 s, and a deferred empty list re-fetches on the next 60 s tick without moving `_last_list_poll_mono`. At any poll setting above 600 s, and already at the default whenever one empty list is deferred, the first refresh ate the marker and the second cleared the count -- so the threshold stayed unreachable. The marker (now `_transient_auth_failure_pending`) is released by a POLL CYCLE that books no rejection instead. That holds at every cadence: while a booked rejection is the newest word on the credentials, no number of refreshes clears the count; one clean cycle releases it and the next refresh heals. `test_two_refreshes_between_two_polls_do_not_clear_the_counter` pins the independence and fails against the consuming design. Also from the same review, each verified against the source: - the tests no longer backdate `_last_list_poll_mono` to 0.0. That reads as "long ago" only while `time.monotonic()` already exceeds 300, which is true on a long-running workstation and false on a freshly booted CI runner -- the reason `test_the_refresh_after_next_still_heals_a_single_hiccup` passed here and failed there. Measured both ways with a patched clock before the change. - the ordering test raises 403, not 401: `is_credential_rejection` records that a 401 surviving the refresh sequence arrives as NovaAuthPermanentError, which leaves 403 as the only plain credential rejection a handler sees. - `main_coordinator_stub` mirrors the new attribute like the others. - three comment line references into `api.py` had rotted by 6 and 18 lines and are now anchors instead of numbers. - the "no non-throwing error exit" claim now states what was counted: nine except branches, eight raising UpdateFailed/ConfigEntryAuthFailed, one bare raise for CancelledError, and exactly one return, on the success path. `_set_auth_state(failed=False)` on that path stays unconditional on purpose; AGENTS.md names the consequence rather than leaving it to be discovered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

jleinenbach · 2026-09-03T10:17:42Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6f8b2cc151

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-09-03T10:21:03Z

+                if not cycle_booked_transient_auth:
+                    # No device rejected the credentials this cycle, so the
+                    # earlier rejection is no longer the newest word on them and
+                    # the device-list refresh may clear the counter again. This
+                    # is the ONLY release path besides the two sites that zero
+                    # the counter outright; binding it to the cycle instead of to
+                    # the refresh is what makes the guard independent of how the
+                    # two cadences happen to line up.
+                    self._transient_auth_failure_pending = False


Release the marker when no poll cycle can run

If all tracker entities are disabled or ignored after a transient auth failure is booked, _async_update_data() produces an empty devices_to_poll list and never calls _async_start_poll_cycle, so this sole normal release path is unreachable. Every successful device-list refresh then continues preserving the stale counter indefinitely; if a tracker is later enabled, its first transient rejection inherits the old failures and can trigger reauthentication prematurely. Handle the no-pollable-device case so the new suppression does not strand a one-off failure budget.

AGENTS.md reference: custom_components/googlefindmy/AGENTS.md:L277-L278

Useful? React with 👍 / 👎.

…ng it Second review round over the full diff. No behaviour change; every item is a statement that promised more than the code delivers. The release of `_transient_auth_failure_pending` hangs on the ABSENCE of a rejection in a cycle, not on a positive proof. Measured over ten cycles: a persistent rejection escalates (a broken tracker beside an idle one reaches the threshold on cycle three, because every cycle books and nothing is released), an intermittent one on a single device does not (the cycles in between release the marker and the next refresh clears the count). That is what a consecutive counter means, and it is now written down in AGENTS.md together with the reason the alternative was not taken: binding the release to a location WITH content would make the device-list refresh a dead reset source, since such a location already clears the counter outright at its own site. Also corrected, each verified against the source: - the block comment above the tests still described the consuming design the previous commit replaced. - the "no non-throwing error exit" count now says which `try` it counted: nine branches on the outer one, plus four inner branches that do swallow but never leave the method. - `test_two_refreshes_between_two_polls_do_not_clear_the_counter` states its scope: it drives no poll cycle and sets the marker by hand. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

jleinenbach · 2026-09-03T10:53:36Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a371b64794

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-09-03T11:00:02Z

+                    # the counter outright; binding it to the cycle instead of to
+                    # the refresh is what makes the guard independent of how the
+                    # two cadences happen to line up.
+                    self._transient_auth_failure_pending = False


Reset the count when a clean cycle breaks the streak

With a 60-second location interval and the fixed 300-second device-list interval, this only clears the marker while leaving _consecutive_transient_auth_failures intact. Consequently, three 403 responses at minutes 0, 2, and 4—each separated by a clean empty-result cycle—accumulate to the threshold and trigger reauthentication before any list refresh can reset the count, even though the failures were not consecutive. Clear the stored count when a clean cycle breaks the streak, or otherwise track consecutive cycles independently of the slower list-refresh cadence. This also contradicts the documented invariant that intermittent rejection must not escalate.

AGENTS.md reference: custom_components/googlefindmy/AGENTS.md:L284-L288

Useful? React with 👍 / 👎.

The transient auth counter was incremented per poll cycle and zeroed from the device-list refresh, and the two cadences are independently configurable. Every ratio produced one of two errors, and both were found in review: at a long poll interval the refresh cleared the count before every attempt and the escalation starved; at a 60 s poll interval up to five cycles run without a refresh, so three rejections at minutes 0, 2 and 4 -- each separated by a clean cycle -- reached the threshold although they were never consecutive. Move the reset onto the clock that increments: a poll cycle that books no rejection AND saw at least one accepted request zeroes the count and the stored cause. Cycle-wide rather than per device, so an idle BLE tag still cannot wipe the rejection a sibling booked in the same pass. The second half of the condition keeps a cycle of pure outage (5xx, timeout, request not accepted) from counting as evidence. A pass that finds no pollable device at all clears the count too, so a budget is not stranded while every tracker is disabled. `_transient_auth_failure_pending` is removed: it guarded a reset that no longer exists. `_set_auth_state(failed=False)` stays on the list refresh, which is what proves the account token.

Terminal diff review of the previous commit found three things, all measured before changing anything. An empty device list satisfied "nothing to poll" just as well as a fleet of disabled trackers, and the two-pass empty-list quorum lets a backend hiccup through as an accepted empty list. The condition now also requires that the account HAS devices, so an outage no longer clears a count. Pinned by `test_an_empty_device_list_does_not_clear_the_count`, which fails against the previous condition. Two prose claims went further than the code. The tally of accepted requests counts an empty result, and four pre-accept failures still arrive as an empty dict, so it is weak evidence rather than proof that Nova took the request; that is now stated where the tally is declared. And the streak rule is measured in both directions rather than asserted in one: 403, empty, 403, empty, 403 does not escalate, while 403, timeout, 403, timeout, 403 does, because a cycle of pure outage neither books nor breaks. Counting "consecutive" over cycles that carried information is a deliberate trade, and it is now written down as one in AGENTS.md next to the number it produces. The streak test additionally asserts that all five cycles really asked, so its zeros cannot come from the no-pollable-device rule instead.

jleinenbach · 2026-09-03T11:56:44Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a6a3e9736d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-09-03T12:00:50Z

+                        # A location WITH content, and only that, proves on this
+                        # path that the credentials worked -- the same rule the
+                        # sound handlers state at their own sites ("only an
+                        # ACCEPTED submission proves the credentials worked").
+                        # An empty return proves only that nothing raised.


Update the API contract for empty location returns

After this change, an empty normal return exits before clearing authentication state in both the polling path and LocateOperations.async_locate_device, but the public GoogleFindMyAPI.async_get_device_location docstring at api.py:1310-1312 and api.py:1322-1326 still says that callers treat every normal return as accepted credentials and clear the account state. This now gives future callers and maintainers the opposite contract from the implementation, so update those sections to distinguish an empty result from a content-bearing proof.

AGENTS.md reference: AGENTS.md:L628-L628

Useful? React with 👍 / 👎.

Codex found the last two places where the public contract still promised the old behaviour: `async_get_device_location` told callers that every normal return means the credentials were accepted and that they clear the account auth state on it. Both callers have checked for emptiness first since this branch, so the docstring handed maintainers the opposite rule from the code. Both sections now separate a return WITH CONTENT (proof, clears the auth state) from an EMPTY return (nothing raised, four pre-accept failures and every idle BLE tag arrive this way, breaks a rejection streak and nothing more). The four other places in this file that describe the same seam were already rewritten as history in an earlier commit and are unchanged; a sweep over the file finds no further claim of the old contract.

jleinenbach · 2026-09-03T12:04:35Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 31279d8775

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-09-03T12:09:51Z

+                        # Marks the cycle, not the device: the post-loop reset
+                        # only fires when NO device rejected, so an idle sibling
+                        # cannot clear what this device just booked.
+                        cycle_booked_transient_auth = True


Count at most one auth failure per poll cycle

When two trackers return non-permanent credential 403s and a later idle tracker returns {}, this marker prevents the cycle-wide reset, but _consecutive_transient_auth_failures += 1 has already run once per rejecting device. The first cycle therefore ends at 2 and the first rejection in cycle two reaches the threshold of 3, starting reauthentication after only two consecutive failing cycles. Record the rejection for the cycle and increment the streak once after the device loop rather than once per tracker.

AGENTS.md reference: custom_components/googlefindmy/AGENTS.md:L290-L296

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-09-03T12:09:51Z

+WITH content, in the manual locate a record that survives the empty guard, and for the transient-auth counter a
+successful `async_get_basic_device_list` -- the strongest source in the tree, because it has no non-throwing error exit
+and an expired login raises before it can reach the reset. On the poll path the same location WITH content clears the
+counter as well, so the device list is the everyday source, not the only one. An empty result clears nothing at all any


Remove the false device-list counter reset claim

This paragraph says a successful async_get_basic_device_list is a proof source for the transient-auth counter and calls the device list its everyday reset source, but this commit deliberately removes that reset from polling.py; the very next paragraph also says the counter is not reset by device-list refreshes. Leaving both claims in the operating contract gives future changes mutually exclusive invariants, so describe the list refresh as clearing only the account auth state.

AGENTS.md reference: AGENTS.md:L628-L629

Useful? React with 👍 / 👎.

Codex found that the increment sat inside the device loop and therefore ran once per rejecting tracker. Measured over two cycles with two broken trackers: the count read [2, 3] and reauthentication was raised inside the second cycle, so the threshold of three was crossed after two failing cycles rather than three. The counter measures cycles -- its name says so and the reauth message reports "persisted across %d poll cycles" -- so the per-device tally escalated early and reported a number that was not a cycle count. It now reads [1, 2] and raises nothing. The cause is still recorded for every rejecting device, because the newest one is the useful one to show. That half was unpinned: a mutation dropping the assignment survived every test in the tree, so the new test asserts it. Also removes a contradiction Codex found in the same round: the AGENTS.md paragraph that introduces the proof sources still called a successful device list the everyday reset source for the counter, while the paragraph two below explains why it deliberately is not. The list now appears there as what it is, proof of the account token and the source that clears the auth state.

jleinenbach · 2026-09-03T12:22:05Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 063dac8ada

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-09-03T12:28:04Z

+                        if not cycle_booked_transient_auth:
+                            self._consecutive_transient_auth_failures += 1


Defer reauthentication until the poll cycle finishes

When the counter enters a cycle at 2 and the first device raises a transient credential 403, this increment reaches the threshold and the branch below starts reauthentication and returns before polling later devices. If a later sibling would return location content, that is positive proof which the documented contract says must clear the counter; reversing the device order does observe that proof and avoids reauthentication. The same responses therefore produce a needless reauth solely based on device order, so defer the threshold verdict until the cycle has examined every sibling.

AGENTS.md reference: custom_components/googlefindmy/AGENTS.md:L251-L260

Useful? React with 👍 / 👎.

…e reauth Codex found that the threshold check sat inside the device loop and returned as soon as it fired. A cycle entering at 2 therefore reauthed on the first rejecting tracker and never asked the siblings — including one that would have returned a location WITH content, the one strong proof on this path, which clears the counter outright. Measured: with the rejecting device first, one reauth and a counter left at 2; with the same responses in the opposite order, no reauth and a counter at 0. The verdict now belongs to the cycle. The device records that it was rejected and carries its name forward; after the loop the streak is raised by one and the threshold judged once, gated on the cycle-wide content tally, so a proof held anywhere in the cycle outranks a non-permanent rejection elsewhere in it. Both orders now end at 0. The warning inside the loop no longer prints a count, because at that point the count still belongs to the previous cycle. Two prose passages that the change outdated are corrected in the same commit: the block comment still named the order dependency as an accepted price, and AGENTS.md did not carry the cycle verdict at all.

jleinenbach · 2026-09-03T12:44:14Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1256222acf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-09-03T12:48:58Z

                        await asyncio.sleep(self.device_poll_delay)

                _LOGGER.debug("Completed polling cycle for %d devices", len(devices))
+                if cycle_booked_transient_auth and not cycle_content_proofs:


Clear the stored rejection when content wins the cycle

When a content-bearing device is processed before a sibling that raises a transient credential rejection, the content path first zeroes both fields, but the later rejection repopulates _last_transient_auth_error; this condition then skips both the increment branch and the cleanup branch because both cycle_booked_transient_auth and cycle_content_proofs are true. Reversing the device order clears the cause at the content site, so identical cycle outcomes leave either (count=0, cause=<stale error>) or (count=0, cause=None). Clear the stored cause in the cycle-level content-proof verdict as well so the paired state and the documented order-independent verdict remain consistent.

AGENTS.md reference: custom_components/googlefindmy/AGENTS.md:L278-L287

Useful? React with 👍 / 👎.

Codex found the diagnostic half of the order dependency the previous commit closed for the counter. A content-bearing device zeroes both fields at its own site; a rejection processed after it writes the cause back, and the end-of-cycle branches skipped that combination entirely. Identical answers therefore ended as (count 0, cause set) in one device order and (count 0, cause None) in the other. The diagnostic snapshot exports both, and a cause standing next to a zero count names a failure that is over. The reset branch now runs on `cycle_accepted_requests` alone, so it also covers the mixed case, and it clears the cause outside the "count > 0" guard, because in exactly that case the count is already zero while a cause still stands.

jleinenbach · 2026-09-03T13:01:12Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e0fae4ad83

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-09-03T13:06:55Z

+                # This is the strongest proof source in the integration, and
+                # after the poll loop stopped resetting on an empty result it is
+                # also the only everyday one left for the transient counter.


Describe the actual cycle-level counter reset

This newly added comment still calls the device-list success path the transient counter's “only everyday” source, although this branch explicitly does not reset that counter at lines 1290-1293; the everyday reset now occurs after a clean poll cycle at lines 2594-2655. Fresh evidence since the earlier review is that the component contract was corrected to describe the cycle-level reset, while this contradictory in-code explanation remains and could encourage reintroducing the cadence bug.

AGENTS.md reference: custom_components/googlefindmy/AGENTS.md:L286-L296

Useful? React with 👍 / 👎.

…y source Codex found the last in-code passage that still described the previous design. The comment above the device-list success path called it the only everyday reset source left for the transient counter, while the paragraph twenty lines below it explains at length why it deliberately no longer resets that counter at all. A reader following the first sentence would have reintroduced the cadence bug. The passage now says what the branch does: it is the strongest proof source for the ACCOUNT token, which is what the auth state reports, and the counter is a different claim that is not settled here. A sweep for the same wording across the coordinator and both contracts finds two further mentions of an "everyday source"; both already name the poll cycle and are correct.

jleinenbach · 2026-09-03T13:14:46Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a25112effd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-09-03T13:19:59Z

+            if (
+                filtered_devices
+                and not devices_to_poll
+                and self._consecutive_transient_auth_failures > 0


Guard the no-pollable reset against an in-flight cycle

When every tracker is disabled or ignored while a slow background poll is still running, the next coordinator refresh reaches this new branch with a nonempty filtered_devices, an empty devices_to_poll, and _is_polling == True, so it clears the existing streak even though the old cycle can subsequently book another rejection. For example, a streak of 2 becomes 0 here and then 1 when that cycle finishes; because no further cycle runs while the entities remain disabled, the stale count survives and can cause premature reauthentication after they are re-enabled. Fresh evidence beyond the earlier in-flight-marker review is this newly added reset source, whose “no cycle will run” premise ignores the cycle already running; defer the reset until no cycle is in flight or ensure the retiring cycle cannot repopulate it.

AGENTS.md reference: custom_components/googlefindmy/AGENTS.md:L286-L296

Useful? React with 👍 / 👎.

The no-pollable-device reset justified itself with "no cycle will run", but the poll cycle is a fire-and-forget task: a refresh can reach that branch while an earlier cycle is still asking its devices. If every tracker was disabled in the meantime, the branch saw nothing to poll and cleared a streak that cycle was still spending -- a standing 2 became 0, then 1 when the cycle booked its rejection, and with no further cycle able to run that stale 1 outlived the outage it came from. Guard the branch with `not self._is_polling`. It is a deferral, not a suppression: once the cycle retires, the next refresh clears the final count, and the test asserts both halves. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

jleinenbach · 2026-09-03T13:55:20Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fffb685b6e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-09-03T14:00:18Z

                        await asyncio.sleep(self.device_poll_delay)

                _LOGGER.debug("Completed polling cycle for %d devices", len(devices))
+                if cycle_booked_transient_auth and not cycle_content_proofs:


Clear the superseded transient error after content proof

When a cycle enters with a streak of 2, one tracker raises a non-permanent credential 403, and a sibling returns location content, this new condition correctly suppresses the increment and reauthentication, but the earlier branch has already stored the 403 in last_exception. Nothing in either cycle-level branch clears it, so the finally block still calls async_set_update_error(last_exception) and marks every tracker unavailable despite the sibling's positive proof; the new order-independence test mocks this method without asserting it was not called. Reconcile the transient exception when content wins the cycle while preserving any independent account-wide error.

AGENTS.md reference: custom_components/googlefindmy/AGENTS.md:L136-L145

Useful? React with 👍 / 👎.

A cycle that books a credential rejection and then gets location content from a sibling accepts that content as proof for the transient-auth counter, but still left the rejection in `last_exception`. That slot is the sole driver of `async_set_update_error`, and entity availability follows the coordinator's `last_update_success`, so every tracker went unavailable right next to the device that had just answered -- two verdicts out of one cycle. The mixed-cycle branch now withdraws that report, bound to the identity of the rejection this cycle booked: a timeout, a stale owner key or an account-wide decrypt failure that reached the slot first stays, because one device answering refutes none of them. `cycle_failed` is kept, so the diagnostic attribute still says the cycle did not poll every device. The order-independence test mocked `async_set_update_error` without ever asserting against it, which is why this went unseen; it asserts now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

jleinenbach · 2026-09-03T14:22:32Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 17a8db37e8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-09-03T14:26:30Z

+                    if (
+                        first_rejection_error is not None
+                        and last_exception is first_rejection_error
+                    ):
+                        last_exception = None


Preserve later failures when withdrawing a rejection

When a transient credential rejection is the first failure, it occupies last_exception; if a later device times out while FCM is disconnected, the timeout branch at lines 2226-2230 assigns only when that slot is empty, so the timeout is discarded. If any sibling also returns location content, this identity check then clears the original rejection, leaving nothing for the async_set_update_error call at line 2855 and marking the coordinator update successful despite the independent timeout. Preserve a later independent failure rather than erasing the slot; test_content_withdraws_only_the_rejection_not_a_timeout currently covers only the opposite ordering, where the timeout reaches the slot first.

AGENTS.md reference: custom_components/googlefindmy/AGENTS.md:L150-L159

Useful? React with 👍 / 👎.

`last_exception` is first-come and drives `async_set_update_error`, which drives `last_update_success` and with it every tracker's availability. A credential rejection holds that slot only provisionally: the mixed-cycle branch withdraws its report once a sibling answers with content. Booked FIRST, that rejection made a later timeout, stale owner key or generic failure drop its own report, and the withdrawal then emptied the slot. The coordinator update reported SUCCESS and every tracker stayed available, although `last_poll_result` said failed and a device had never answered at all. Measured as a red probe in the mirror ordering of the existing pin, which covers only timeout-first and stays green either way: `got []`, no report at all. An overwrite would be wrong the other way round, because whether the withdrawal fires is only known after the loop. Without content the rejection is the more specific diagnosis and the cause of the running escalation, and a first attempt at this swapped it for "nobody answered". The failure therefore goes into a second slot and the withdrawal hands the place over instead of emptying it. The one-shot guard for `first_rejection_error` moves from the device NAME to the error itself: `dev.get("name", dev_id)` is None for a device carrying an explicit `"name": None`, which would let a second rejection overwrite the identity both checks compare against. Contract updated in both places it is stated, including the second copy further down that still read as "first failure wins", plus four measured corrections: `last_poll_result` says failed while the coordinator update reports success, two slots below not three, an account-wide decrypt failure cannot reach the slot before the withdrawal, and the device list is refreshed in nearly every cycle at the default 300s cadence rather than mostly served from cache. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

jleinenbach · 2026-09-03T16:14:47Z

@codex review

chatgpt-codex-connector · 2026-09-03T16:20:40Z

Codex Review: Didn't find any major issues. What shall we delve into next?

Reviewed commit: b3e459d6cd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

jleinenbach added 4 commits September 3, 2026 05:46

chatgpt-codex-connector Bot reviewed Sep 3, 2026

View reviewed changes

jleinenbach added 2 commits September 3, 2026 11:35

chatgpt-codex-connector Bot reviewed Sep 3, 2026

View reviewed changes

jleinenbach marked this pull request as ready for review September 3, 2026 17:17

jleinenbach merged commit 87e4abd into 1.7 Sep 3, 2026
25 checks passed

		if not cycle_booked_transient_auth:
		self._consecutive_transient_auth_failures += 1

Conversation

jleinenbach commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What was wrong

What changed

What a user notices

Measured limits, stated rather than discovered later

Evidence

The second verdict of a cycle

Uh oh!

jleinenbach commented Sep 3, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jleinenbach commented Sep 3, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

jleinenbach Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

jleinenbach commented Sep 3, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 3, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

jleinenbach commented Sep 3, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

jleinenbach commented Sep 3, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

jleinenbach commented Sep 3, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

jleinenbach commented Sep 3, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Sep 3, 2026

jleinenbach commented Sep 3, 2026 •

edited

Loading

codecov Bot commented Sep 3, 2026 •

edited

Loading