Skip to content
This repository was archived by the owner on Jun 26, 2026. It is now read-only.

fix: handle GitHub user switching via PAT change - #24

Merged
matejvasek merged 1 commit into
masterfrom
chng-gh-user
May 13, 2026
Merged

matejvasek merged 1 commit into
masterfrom
chng-gh-user

Conversation

@matejvasek

Copy link
Copy Markdown
Collaborator

Summary

  • Refresh function list when the user switches GitHub PAT via the Avatar button. The useEffect dependency array had no signal for PAT changes, and GithubService kept a stale repo cache from the previous user.
  • Add a connectionId counter to ForgeConnectionContext that increments on every connect, triggering a re-fetch in the list page and clearing caches in GithubService.
  • Clear PAT input field when reopening the modal (PatModal stays mounted, so React preserved stale state across open/close cycles).

Test plan

  • Open the Function List Page, connect with PAT for user A (has functions), verify functions appear
  • Click Avatar, enter PAT for user B (no functions), verify list clears and empty state shows
  • Click Avatar, enter PAT for user A again, verify functions reappear
  • Click Avatar, verify PAT input field is empty (not pre-populated with old value)
  • Refresh page with PAT in session, verify list loads correctly
  • First visit (no PAT in session), verify modal auto-opens and list shows empty state

🤖 Generated with Claude Code

@matejvasek
matejvasek requested a review from twoGiants May 5, 2026 18:45

@twoGiants twoGiants left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Alright, thanks for the fix! We need this. See my comments below.

Comment thread src/context/ForgeConnectionProvider.tsx Resolved
Comment thread src/components/UserAvatar.tsx Resolved
Comment thread src/components/UserAvatar.tsx Resolved
Comment thread src/components/UserAvatar.tsx Resolved
Comment thread src/components/UserAvatar.tsx Resolved
Comment thread src/components/UserAvatar.tsx Resolved
matejvasek added a commit that referenced this pull request May 13, 2026
Replace the wasOpen render-time tracking with explicit cleanup
at the two exit points: setPat('') after successful connect,
and a handleClose callback that clears pat and error on cancel
or dismiss.

Addresses review feedback from PR #24.

Signed-off-by: Matej Vašek <matejvasek@gmail.com>
Co-Authored-By: Claude <noreply@anthropic.com>
@matejvasek
matejvasek requested a review from twoGiants May 13, 2026 11:16

@twoGiants twoGiants left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good! Just a few unit tests missing.

I'll approve but please add them before merging. Thank you 👍

Comment thread src/components/UserAvatar.tsx Resolved
Switching GitHub users via the Avatar button left the function
list stale. Two root causes:

1. useFunctionListPage had no dependency that changed on PAT
   switch (isConnectedToForge stayed true), so the useEffect
   never re-ran.
2. GithubService kept a cached repo list from the previous
   user, which bled into the new user's results through the
   "unfetched" union logic.

Add a connectionId counter to ForgeConnectionContext that
increments on every connectToForge call. The list page uses
it to trigger a re-fetch and clear stale function items.
Clear the repo and commit SHA caches in GithubService when
the token changes.

Also fix the PAT modal retaining the previous token on
reopen: clear input on successful connect, and reset both
input and error on cancel/dismiss via a handleClose callback.

Signed-off-by: Matej Vašek <matejvasek@gmail.com>
Co-Authored-By: Claude <noreply@anthropic.com>
@matejvasek
matejvasek merged commit 213d4fc into master May 13, 2026
1 check passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants