Skip to content

Address PR344 follow-up review comments - #2

Open
capy-ai[bot] wants to merge 4 commits into
mainfrom
fix-capy-pr344-followup
Open

Address PR344 follow-up review comments#2
capy-ai[bot] wants to merge 4 commits into
mainfrom
fix-capy-pr344-followup

Conversation

@capy-ai

@capy-ai capy-ai Bot commented Apr 22, 2026

Copy link
Copy Markdown
Contributor

Addresses the remaining follow-up review comments from upstream PR #344.

Changes:

  • avoid stale auth state reads when refreshing navbar active session after account switch
  • fully resync recovery state after failed account switch
  • simplify dropdown ARIA semantics away from incomplete menu semantics
  • reuse shared SavedAccount type in auth service

Scoped to the review follow-up only, based on latest main.

PriyanshAg-1 and others added 4 commits April 22, 2026 17:48
* Fix account switch session validation during refresh

* Fail switch-account refresh when user identity cannot be verified

* Update opennow-stable/src/main/gfn/auth.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: PriyanshAg-1 <204138848+PriyanshAg-1@users.noreply.github.com>
Co-authored-by: priyanshrv1-oss <priyanshrv1@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

@capy-ai capy-ai Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added 2 comments

className="navbar-user navbar-user--clickable"
onClick={() => setAccountDropdownOpen((previous) => !previous)}
aria-expanded={accountDropdownOpen}
aria-label="Account options"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[🟡 Medium] [🔵 Bug]

In @opennow-stable/src/renderer/src/components/Navbar.tsx the new trigger button hard-codes a generic aria-label, which overrides the visible username/tier text when the button’s accessible name is computed: ```tsx
// opennow-stable/src/renderer/src/components/Navbar.tsx
<button
type="button"
className="navbar-user navbar-user--clickable"
onClick={() => setAccountDropdownOpen((previous) => !previous)}
aria-expanded={accountDropdownOpen}
aria-label="Account options"


```suggestion
                aria-label={`${user.displayName} account options`}

const refreshFailed =
result.refresh.outcome === "failed" || result.refresh.outcome === "missing_refresh_token";
const switchedUserMismatch = result.session?.user.userId !== userId;
if (!result.session || refreshFailed || switchedUserMismatch) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[🟡 Medium] [🔵 Bug]

// opennow-stable/src/main/gfn/auth.ts
const refreshFailed =
  result.refresh.outcome === "failed" || result.refresh.outcome === "missing_refresh_token";
const switchedUserMismatch = result.session?.user.userId !== userId;
if (!result.session || refreshFailed || switchedUserMismatch) {
  await this.removeAccount(userId);
}

ensureValidSessionWithStatus() explicitly returns outcome: "failed" with "Token refresh failed. Using saved session token." and outcome: "missing_refresh_token" with "No refresh token available. Using saved session token." when the access token is still valid. Treating both outcomes as fatal here means a transient refresh/userinfo failure during account switch will delete the saved account and force a relogin even though the session can still be used, which breaks the PR’s reliability goal under failures. Only evict the account when no session remains, the token is actually expired/invalid, or the refreshed identity mismatches the requested user; otherwise keep the saved account and continue with the returned session or a non-destructive warning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants