fix(auth): auto-refresh on PostHog 401/403 auth failures

[jonathanlab]

MCP tool discovery and renderer task creation were surfacing 401/403 authentication_failed to users because the existing refresh-and-retry paths didn't match the real error shapes (Invalid API key, Authentication failed, 403 with authentication_error body).

Extend the MCP proxy to retry-with-refresh on HTTP 401 plus the literal server strings, and the renderer fetcher to retry on 403 with authentication_failed in addition to 401.

---

Created with PostHog Code

[greptile-apps]

Comments Outside Diff (1)

1. apps/code/src/main/services/mcp-proxy/service.ts, line 176-178 (link)

   The comment is now stale and slightly misleading. authenticatedFetch already retries on HTTP 401/403 internally (in AuthService.authenticatedFetch), so the proxy's outer response.status === 401 check only fires when that first retry also returned a 401 — i.e., this is a second refresh attempt, not the first. The comment should reflect that and clarify the two distinct code paths being handled.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: apps/code/src/main/services/mcp-proxy/service.ts
   Line: 176-178
   
   Comment:
   The comment is now stale and slightly misleading. `authenticatedFetch` already retries on HTTP 401/403 internally (in `AuthService.authenticatedFetch`), so the proxy's outer `response.status === 401` check only fires when that first retry also returned a 401 — i.e., this is a second refresh attempt, not the first. The comment should reflect that and clarify the two distinct code paths being handled.
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

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

---

### Issue 1 of 4
apps/code/src/main/services/mcp-proxy/service.ts:176-178
The comment is now stale and slightly misleading. `authenticatedFetch` already retries on HTTP 401/403 internally (in `AuthService.authenticatedFetch`), so the proxy's outer `response.status === 401` check only fires when that first retry also returned a 401 — i.e., this is a second refresh attempt, not the first. The comment should reflect that and clarify the two distinct code paths being handled.

```suggestion
      // Two failure modes require a proxy-level refresh:
      // 1. HTTP 401 that survives authenticatedFetch's own 401/403 retry (i.e.
      //    the retried token was *also* rejected — a second refresh attempt).
      // 2. HTTP 200 with an auth-error JSON-RPC body, which authenticatedFetch
      //    never sees as a failure and therefore never retries.
```

### Issue 2 of 4
apps/code/src/main/services/mcp-proxy/service.ts:186-188
**Double-refresh risk with rotating OAuth clients**

`authenticatedFetch` already performs a refresh-and-retry when it sees a 401 (see `AuthService.authenticatedFetch` lines 182–190). The response reaching this outer check has therefore already been retried once; if it is still 401, this block triggers a second `refreshAccessToken()` call. With a rotating client, each call invalidates the previous token, so an edge-case concurrent-request scenario could see the second `refreshAccessToken()` invalidate the token that `authenticatedFetch` just minted for another in-flight call. This is consistent with the concurrent-refresh race the PR description already flags as a follow-up, but worth noting explicitly here.

### Issue 3 of 4
apps/code/src/main/services/mcp-proxy/service.ts:242-248
**Broad substring matches could trigger spurious refreshes on 200 responses**

`isAuthErrorBody` is checked on every non-SSE response regardless of HTTP status (lines 186–188 only add `response.status === 401` via `||`, so a 200 with body text containing `"Authentication failed"` still triggers a refresh). The two new unquoted strings — `"Invalid API key"` and `"Authentication failed"` — are short, common English phrases. A 200 JSON-RPC tool-result that happens to describe a downstream auth failure (e.g. a tool returning an error message explaining what went wrong for the user) would match and fire an unnecessary token refresh + retry. Narrowing the match — e.g., only applying the plain-text checks when `response.status >= 400`, or requiring a specific JSON shape — would prevent this.

### Issue 4 of 4
apps/code/src/renderer/api/fetcher.ts:55-67
**No tests cover the new retry paths**

The team's rule requires comprehensive tests for new retry logic. The PR adds two new retry trigger conditions — 403 with `authentication_failed`/`authentication_error` body in `fetcher.ts` and HTTP 401 in the MCP proxy — but includes no test coverage for either. In particular, the 403 branch (where `response.clone().json()` is used to inspect the body) is the more complex path and the most likely to regress silently.

_{Reviews (1): Last reviewed commit: "fix(auth): auto-refresh on PostHog 401/4..." | Re-trigger Greptile}