[k11kirky]

Problem

The GitHub user connect logic was duplicated across GitHubConnectionBanner and GitIntegrationStep, making it harder to maintain and reason about. Each component independently managed connection state, polling timers, timeout handling, query invalidation, and the window-focus refetch fallback.

Changes

Extracts all GitHub user connect logic into a new useGithubUserConnect hook that encapsulates:

• Connection state (idle, connecting, timed-out)
• Opening the GitHub install URL in the browser
• Dev-mode polling fallback for query invalidation
• Safety timeout after 5 minutes of inactivity
• Window-focus refetch while a connection is in flight
• useGitHubIntegrationCallback integration for deep-link success/error handling

GitHubConnectionBanner and GitIntegrationStep are updated to use the new hook, removing their inline implementations. The isConnectingGithub field and setConnectingGithub action are removed from the onboarding store since connection state is now owned by the hook.

[k11kirky]

• feat(code): list all GitHub installations and allow adding another org #2001
• feat(code): detect stale GitHub integration and offer Reconnect #2000
• feat(code): add inline GitHub disconnect to onboarding step #1999
• feat(code): show persistent error state for GitHub connect failures #1998
• feat(code): show fast-path GitHub sign-in when current project has team install #1997
• feat(code): unify user-scoped GitHub connect into a shared hook #1996 👈 (View in Graphite)
• feat(code): cleaner GitHub integration onboarding step #1990
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[greptile-apps]

Prompt To Fix All With AI

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

---

### Issue 1 of 2
apps/code/src/renderer/features/integrations/hooks/useGithubUserConnect.ts:89-93
**`toast.error` fires unconditionally regardless of connection state**

`onError` calls `toast.error(message)` even when `state === "idle"`. For `GitHubConnectionBanner` — which is mounted in the persistent inbox UI — this means that if a stale pending error callback is consumed on mount via `consumePendingCallback` (e.g. left over from a previous connect attempt in another session), the user will see an unexpected error toast with no obvious cause. The original banner didn't use `useGitHubIntegrationCallback` at all, so this is a new behaviour for that consumer.

Guard the toast (and optionally the full `onError` body) behind a state check:

```typescript
onError: (message) => {
  stopPolling();
  if (stateRef.current === "connecting") {
    toast.error(message);
  }
  setState("idle");
},
```

### Issue 2 of 2
apps/code/src/renderer/features/integrations/hooks/useGithubUserConnect.ts:114-119
**Redundant `projectId !== null` guard**

`projectId !== null` is already guaranteed at this point — the `connect` callback returns early on line 103 if `projectId === null`. The extra check is superfluous and contradicts the simplicity rule of "no superfluous parts".

```typescript
      if (IS_DEV) {
        pollTimerRef.current = setInterval(
          () => invalidate(projectId),
          POLL_INTERVAL_MS,
        );
      }
```

_{Reviews (1): Last reviewed commit: "feat(code): unify user-scoped GitHub con..." | Re-trigger Greptile}

[greptile-apps]

toast.error fires unconditionally regardless of connection state

onError calls toast.error(message) even when state === "idle". For GitHubConnectionBanner — which is mounted in the persistent inbox UI — this means that if a stale pending error callback is consumed on mount via consumePendingCallback (e.g. left over from a previous connect attempt in another session), the user will see an unexpected error toast with no obvious cause. The original banner didn't use useGitHubIntegrationCallback at all, so this is a new behaviour for that consumer.

Guard the toast (and optionally the full onError body) behind a state check:

onError: (message) => {
  stopPolling();
  if (stateRef.current === "connecting") {
    toast.error(message);
  }
  setState("idle");
},

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/code/src/renderer/features/integrations/hooks/useGithubUserConnect.ts
Line: 89-93

Comment:
**`toast.error` fires unconditionally regardless of connection state**

`onError` calls `toast.error(message)` even when `state === "idle"`. For `GitHubConnectionBanner` — which is mounted in the persistent inbox UI — this means that if a stale pending error callback is consumed on mount via `consumePendingCallback` (e.g. left over from a previous connect attempt in another session), the user will see an unexpected error toast with no obvious cause. The original banner didn't use `useGitHubIntegrationCallback` at all, so this is a new behaviour for that consumer.

Guard the toast (and optionally the full `onError` body) behind a state check:

```typescript
onError: (message) => {
  stopPolling();
  if (stateRef.current === "connecting") {
    toast.error(message);
  }
  setState("idle");
},
```

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

[greptile-apps]

Redundant projectId !== null guard

projectId !== null is already guaranteed at this point — the connect callback returns early on line 103 if projectId === null. The extra check is superfluous and contradicts the simplicity rule of "no superfluous parts".

      if (IS_DEV) {
        pollTimerRef.current = setInterval(
          () => invalidate(projectId),
          POLL_INTERVAL_MS,
        );
      }

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/code/src/renderer/features/integrations/hooks/useGithubUserConnect.ts
Line: 114-119

Comment:
**Redundant `projectId !== null` guard**

`projectId !== null` is already guaranteed at this point — the `connect` callback returns early on line 103 if `projectId === null`. The extra check is superfluous and contradicts the simplicity rule of "no superfluous parts".

```typescript
      if (IS_DEV) {
        pollTimerRef.current = setInterval(
          () => invalidate(projectId),
          POLL_INTERVAL_MS,
        );
      }
```

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!

[jonathanlab]

nice

[k11kirky]

Merge activity

• May 4, 2:36 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• May 4, 2:38 PM UTC: Graphite rebased this pull request as part of a merge.
• May 4, 2:46 PM UTC: @k11kirky merged this pull request with Graphite.