fix(auth): stop infinite updateAuthPlan loop on self-hosted

[ManthanNimodiya]

Summary

The createOrganizationsQuery effect fired every time auth.data changed.
Since the previous auth fixes (PR #1898) now always write back to the store on every updateAuthPlan call, each write triggered the effect again, creating an infinite loop of plan/org fetch requests on self-hosted instances where the org endpoint always returns an error.

Fix

Changed the trigger condition from organizations.length === 0 to !organizations_updated_at.

Since PR #1898 always stamps organizations_updated_at after the first run, the effect fires exactly once on sign-in and never again until the user signs out and back in.

Greptile Summary

This PR changes the trigger condition in createOrganizationsQuery from organizations.length === 0 to !organizations_updated_at to break an infinite updateAuthPlan loop on self-hosted instances introduced by PR #1898 always writing to the auth store.

• The new guard fires exactly once per sign-in cycle for self-hosted users (empty org list), relying on auth.rs stamping organizations_updated_at in the error path when auth.organizations.is_empty().
• Cloud users upgrading from before PR fix(auth): make plan and org fetches non-fatal, always persist auth #1898 may carry a non-empty organizations list alongside a null organizations_updated_at; if the org fetch fails transiently, auth.rs will not stamp the timestamp (the is_empty() guard is not met), yet still unconditionally saves the store, re-firing the reactive effect and re-entering the loop.

Confidence Score: 4/5

Safe to merge for the self-hosted loop case it targets, but carries a narrow regression risk for cloud users upgrading with existing organizations who hit a transient org-fetch failure.

The fix correctly eliminates the infinite loop for self-hosted users with empty org lists. However, the Rust error path in auth.rs stamps organizations_updated_at only when organizations.is_empty(), meaning cloud users migrating from pre-#1898 stores who have orgs but no timestamp can re-enter the same loop if the org endpoint returns an error during that first run.

The Rust error branch in apps/desktop/src-tauri/src/auth.rs (lines 103-108) warrants a follow-up: stamping organizations_updated_at unconditionally on org-fetch failure would make the JS guard airtight across all migration paths.

Important Files Changed

Filename	Overview
apps/desktop/src/utils/queries.ts	Guard changed from organizations.length === 0 to !organizations_updated_at to break the self-hosted infinite loop; works correctly for empty-org users but can re-enter the loop for cloud users upgrading with non-empty orgs when org fetch fails transiently.

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
apps/desktop/src/utils/queries.ts:355-358
**Migration loop for existing cloud users with non-empty org lists**

The fix relies on `auth.rs` stamping `organizations_updated_at` after any `updateAuthPlan` call. This works for self-hosted users (always empty orgs) because the error path in `auth.rs` (lines 105–107) stamps the timestamp only when `auth.organizations.is_empty()`. Users upgrading from before PR #1898 may have a non-empty `organizations` list with `organizations_updated_at = null` — the field simply didn't exist. For these users the new trigger fires, `updateAuthPlan` runs, and if the org fetch happens to fail (transient network error, server hiccup), `organizations_updated_at` is **not** stamped because `!is_empty()`, the store is still saved unconditionally (line 111 of `auth.rs`), the reactive effect re-fires, and the loop restarts indefinitely.

The simplest fix is to stamp `organizations_updated_at` unconditionally in the error branch of `auth.rs`, removing the `is_empty()` guard — the JS logic no longer needs the org-count distinction to prevent the loop.

Also, the inline comment says "stamps `organizations_updated_at` **even** on org-fetch failure" which implies unconditional behaviour; tightening it to mention the `is_empty()` precondition would prevent a future reader from relying on an assumption that doesn't hold.

_{Reviews (2): Last reviewed commit: "fix(auth): add comment explaining org re..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

[superagent-security]

Superagent didn't find any vulnerabilities or security issues in this PR.

[greptile-apps]

Behavioral gap: no org refresh on app restart when organizations_updated_at is persisted

organizations_updated_at is written to the on-disk store by the Rust backend and therefore survives app restarts. When a user closes and reopens the app while still signed in, the effect sees organizations_updated_at as already set and skips the fetch entirely — organizations are never refreshed until the user signs out. The old organizations.length === 0 guard had the same property for non-empty org lists, but for a self-hosted user whose org list was empty (the loop case), it would at least attempt a refresh on every restart. With this fix, a single failed attempt per sign-in cycle is all you get.

This is acceptable for the loop-prevention goal, but it's worth a comment explaining the intentional trade-off so a future reader doesn't accidentally "fix" it back.

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/desktop/src/utils/queries.ts
Line: 354-357

Comment:
**Behavioral gap: no org refresh on app restart when `organizations_updated_at` is persisted**

`organizations_updated_at` is written to the on-disk store by the Rust backend and therefore survives app restarts. When a user closes and reopens the app while still signed in, the effect sees `organizations_updated_at` as already set and skips the fetch entirely — organizations are never refreshed until the user signs out. The old `organizations.length === 0` guard had the same property for non-empty org lists, but for a self-hosted user whose org list was empty (the loop case), it would at least attempt a refresh on every restart. With this fix, a single failed attempt per sign-in cycle is all you get.

This is acceptable for the loop-prevention goal, but it's worth a comment explaining the intentional trade-off so a future reader doesn't accidentally "fix" it back.

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

Implicit dependency on Rust error-path always stamping the timestamp

The fix works because auth.rs unconditionally stamps organizations_updated_at when auth.organizations.is_empty() on error (lines 105-107). If that guard is ever relaxed to allow retries (e.g., "only stamp after N attempts"), the JS effect would re-enter the loop immediately. A short inline comment linking this condition to the Rust error path would protect against that silent regression.

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/desktop/src/utils/queries.ts
Line: 354-357

Comment:
**Implicit dependency on Rust error-path always stamping the timestamp**

The fix works because `auth.rs` unconditionally stamps `organizations_updated_at` when `auth.organizations.is_empty()` on error (lines 105-107). If that guard is ever relaxed to allow retries (e.g., "only stamp after N attempts"), the JS effect would re-enter the loop immediately. A short inline comment linking this condition to the Rust error path would protect against that silent regression.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[ManthanNimodiya]

@greptileai please re-review

[greptile-apps]

Migration loop for existing cloud users with non-empty org lists

The fix relies on auth.rs stamping organizations_updated_at after any updateAuthPlan call. This works for self-hosted users (always empty orgs) because the error path in auth.rs (lines 105–107) stamps the timestamp only when auth.organizations.is_empty(). Users upgrading from before PR #1898 may have a non-empty organizations list with organizations_updated_at = null — the field simply didn't exist. For these users the new trigger fires, updateAuthPlan runs, and if the org fetch happens to fail (transient network error, server hiccup), organizations_updated_at is not stamped because !is_empty(), the store is still saved unconditionally (line 111 of auth.rs), the reactive effect re-fires, and the loop restarts indefinitely.

The simplest fix is to stamp organizations_updated_at unconditionally in the error branch of auth.rs, removing the is_empty() guard — the JS logic no longer needs the org-count distinction to prevent the loop.

Also, the inline comment says "stamps organizations_updated_at even on org-fetch failure" which implies unconditional behaviour; tightening it to mention the is_empty() precondition would prevent a future reader from relying on an assumption that doesn't hold.

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/desktop/src/utils/queries.ts
Line: 355-358

Comment:
**Migration loop for existing cloud users with non-empty org lists**

The fix relies on `auth.rs` stamping `organizations_updated_at` after any `updateAuthPlan` call. This works for self-hosted users (always empty orgs) because the error path in `auth.rs` (lines 105–107) stamps the timestamp only when `auth.organizations.is_empty()`. Users upgrading from before PR #1898 may have a non-empty `organizations` list with `organizations_updated_at = null` — the field simply didn't exist. For these users the new trigger fires, `updateAuthPlan` runs, and if the org fetch happens to fail (transient network error, server hiccup), `organizations_updated_at` is **not** stamped because `!is_empty()`, the store is still saved unconditionally (line 111 of `auth.rs`), the reactive effect re-fires, and the loop restarts indefinitely.

The simplest fix is to stamp `organizations_updated_at` unconditionally in the error branch of `auth.rs`, removing the `is_empty()` guard — the JS logic no longer needs the org-count distinction to prevent the loop.

Also, the inline comment says "stamps `organizations_updated_at` **even** on org-fetch failure" which implies unconditional behaviour; tightening it to mention the `is_empty()` precondition would prevent a future reader from relying on an assumption that doesn't hold.

How can I resolve this? If you propose a fix, please make it concise.