Skip to content

feat(agentex-ui): OIDC login with server-side access token - #351

Merged
erichwoo-scale merged 6 commits into
mainfrom
feat/agentex-ui-oidc-auth
Jul 10, 2026
Merged

feat(agentex-ui): OIDC login with server-side access token#351
erichwoo-scale merged 6 commits into
mainfrom
feat/agentex-ui-oidc-auth

Conversation

@erichwoo-scale

@erichwoo-scale erichwoo-scale commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Second of a 2-PR stack. Stacked on #350base is feat/agentex-ui-account-picker, so this diff is auth-only (GitHub auto-retargets to main once #350 merges). The chart + deploy wiring lives in scaleapi/sgp#3961.

Adds opt-in OIDC login on top of the BFF, keeping the access token out of the browser entirely.

  • NextAuth (generic OIDC provider) selected and enabled by a single env var, AGENTEX_UI_AUTH_PROVIDER_ID; disabled by default so non-auth deployments are unaffected (no SessionProvider, no /api/auth/session calls).
  • The BFF attaches the access token as a Bearer server-side via getToken and drops the UI session cookie — the token never reaches client JS or the NextAuth session (which exposes only error).
  • Token/end-session endpoints are resolved from the issuer's OIDC discovery document (cached; failures retried), so refresh and logout work for any compliant IdP — no hardcoded provider paths.
  • Middleware auto-redirects unauthenticated requests to sign-in; a client guard re-auths on refresh failure; POST-only RP-initiated logout ends the IdP session (a GET would be cross-site-triggerable — logout CSRF).
  • Supports client_secret_post (dev) and private_key_jwt (prod).

Why token-hidden (BFF) over exposing it on the session

Exposing the access token to client JS is contrary to the OAuth 2.0 for Browser-Based Apps BCP and would flag in security review. Keeping it server-side (the BFF attaches the Bearer) is the recommended pattern; the client only ever sees the same-origin proxy.

Stack

  1. feat(agentex-ui): account picker via same-origin BFF proxy #350 — account picker + BFF proxy
  2. This PR — OIDC login → stacked on feat(agentex-ui): account picker via same-origin BFF proxy #350
  3. scaleapi/sgp#3961 — Helm chart wires the OneAuth OIDC client + env (AGENTEX_UI_AUTH_PROVIDER_ID, OIDC_ISSUER_URL, the client Secret via envFrom)

Test plan

  • npm run typecheck / npm run lint — clean
  • Runtime login smoke test (sign-in → agents load via Bearer → logout ends the IdP session)
  • Chart wiring — done in scaleapi/sgp#3961

🤖 Generated with Claude Code

Greptile Summary

Adds opt-in OIDC login to agentex-ui via NextAuth v5, keeping the access token entirely server-side (BFF pattern): the middleware redirects unauthenticated requests to a server-side auto-signin handler, the BFF attaches the Bearer from the encrypted session JWT, and a POST-only RP-initiated logout endpoint ends the IdP session.

  • auth.ts — generic OIDC provider built from env vars; token/end-session endpoints resolved from OIDC discovery (cached per-process, failures not cached for retry); custom private_key_jwt signer for prod and client_secret_post for dev; refresh logic correctly treats 4xx (excluding 429) as terminal and 429/5xx/network as transient.
  • bff.ts — in auth mode, strips all cookies and attaches Bearer <accessToken> from the server-side session JWT; falls back to cookie forwarding in non-auth mode; the entire feature is gated on AGENTEX_UI_AUTH_PROVIDER_ID so non-auth deployments are unaffected.
  • agentex-provider.tsx / session-guard.tsx — client-side 401 handling retries once after a deduplicated session refresh; SessionGuard triggers re-auth on RefreshAccessTokenError for sessions that are purely making API calls without navigation.

Confidence Score: 5/5

Safe to merge — the BFF pattern is correctly implemented, the access token never reaches client JS, and all auth edge cases are handled correctly.

All the previously flagged issues have been addressed in the current diff. The discovery-based endpoint resolution caches correctly, the 4xx/429/5xx split on refresh errors is sound, and the middleware correctly gates only non-API routes. No new logic bugs were found.

No files require special attention; auth.ts is the most complex file but its token refresh and OIDC discovery logic are well-guarded.

Important Files Changed

Filename Overview
agentex-ui/auth.ts Core OIDC auth module — discovery caching, token refresh with correct 4xx/5xx split, private_key_jwt signing, and end-session endpoint resolution all look correct.
agentex-ui/app/api/_lib/bff.ts Auth mode branches correctly — strips cookies, reads session JWT server-side, and attaches Bearer; falls back to cookie forwarding when auth is disabled.
agentex-ui/app/api/auth/logout/route.ts POST-only RP-initiated logout reads end_session_endpoint from OIDC discovery (not hardcoded), clears the session cookie, and returns the IdP logout URL for client-side navigation.
agentex-ui/middleware.ts Auth middleware is opt-in (gated on authEnabled), uses Node runtime, and correctly excludes API routes and Next.js internals from the matcher.
agentex-ui/app/api/auth/auto-signin/route.ts Server-side sign-in handler validates redirect_url for open redirect, short-circuits if already authenticated, and falls back to 404 when auth is disabled.
agentex-ui/components/providers/agentex-provider.tsx 401 retry with deduplicated session refresh is correct; module-level sessionRefresh singleton is safe in browser context; ref-based account ID avoids stale-closure issues.
agentex-ui/components/session-guard.tsx Client-side guard for RefreshAccessTokenError triggers re-auth via auto-signin; correctly mounted inside SessionProvider in layout.tsx.
agentex-ui/app/layout.tsx SessionProvider with refetchInterval=240 correctly drives jwt-callback refresh; SessionGuard mounted inside the provider boundary.
agentex-ui/app/api/auth/[...nextauth]/route.ts NextAuth route guard returns 404 in non-auth mode, preventing the provider-less handler from returning 500 on accidental hits.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant B as Browser
    participant MW as Middleware
    participant AS as auto-signin
    participant NA as NextAuth
    participant BFF as BFF
    participant UP as Upstream API
    participant IDP as IdP (OIDC)

    B->>MW: GET /
    MW->>MW: auth() no session
    MW-->>B: redirect /api/auth/auto-signin
    B->>AS: GET /api/auth/auto-signin
    AS->>IDP: signIn (PKCE + state)
    IDP-->>B: redirect /api/auth/callback
    B->>NA: GET /api/auth/callback
    NA->>IDP: exchange code for tokens
    NA->>NA: jwt callback stores tokens in encrypted cookie
    NA-->>B: redirect / (session cookie set)
    B->>BFF: fetch /api/agentex
    BFF->>BFF: getSessionToken extracts accessToken
    BFF->>UP: GET with Authorization Bearer token
    UP-->>B: 200
    Note over B,NA: Token refresh (refetchInterval=240s)
    B->>NA: GET /api/auth/session
    NA->>IDP: POST token_endpoint refresh_token
    IDP-->>NA: new access_token
    NA-->>B: Set-Cookie updated session
    Note over B,BFF: Logout
    B->>BFF: POST /api/auth/logout
    BFF->>BFF: signOut + discover end_session_endpoint
    BFF-->>B: url to IdP end_session
    B->>IDP: navigate to end_session
    IDP-->>B: redirect to origin
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant B as Browser
    participant MW as Middleware
    participant AS as auto-signin
    participant NA as NextAuth
    participant BFF as BFF
    participant UP as Upstream API
    participant IDP as IdP (OIDC)

    B->>MW: GET /
    MW->>MW: auth() no session
    MW-->>B: redirect /api/auth/auto-signin
    B->>AS: GET /api/auth/auto-signin
    AS->>IDP: signIn (PKCE + state)
    IDP-->>B: redirect /api/auth/callback
    B->>NA: GET /api/auth/callback
    NA->>IDP: exchange code for tokens
    NA->>NA: jwt callback stores tokens in encrypted cookie
    NA-->>B: redirect / (session cookie set)
    B->>BFF: fetch /api/agentex
    BFF->>BFF: getSessionToken extracts accessToken
    BFF->>UP: GET with Authorization Bearer token
    UP-->>B: 200
    Note over B,NA: Token refresh (refetchInterval=240s)
    B->>NA: GET /api/auth/session
    NA->>IDP: POST token_endpoint refresh_token
    IDP-->>NA: new access_token
    NA-->>B: Set-Cookie updated session
    Note over B,BFF: Logout
    B->>BFF: POST /api/auth/logout
    BFF->>BFF: signOut + discover end_session_endpoint
    BFF-->>B: url to IdP end_session
    B->>IDP: navigate to end_session
    IDP-->>B: redirect to origin
Loading

Reviews (11): Last reviewed commit: "fix(agentex-ui): escalate refresh failur..." | Re-trigger Greptile

@erichwoo-scale
erichwoo-scale requested a review from a team as a code owner July 8, 2026 18:36
@socket-security

socket-security Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addednpm/​next-auth@​5.0.0-beta.31991007886100

View full report

Comment thread agentex-ui/auth.ts Outdated
Comment thread agentex-ui/app/api/auth/logout/route.ts Outdated
@erichwoo-scale
erichwoo-scale force-pushed the feat/agentex-ui-oidc-auth branch from bb896c7 to c53787d Compare July 8, 2026 19:21
@erichwoo-scale

Copy link
Copy Markdown
Contributor Author

Addressed the Greptile P1s in c53787d:

  • Hardcoded token endpoint (auth.ts) and hardcoded end_session_endpoint (logout/route.ts): both are now resolved from the issuer's .well-known/openid-configuration (cached per-process), with the Ory paths kept only as a last-resort fallback. This fixes the silent refresh→re-auth loop and the transparent re-login on non-Ory IdPs, and makes the "generic OIDC" claim actually hold.

Rebased onto the updated base branch so the stack stays clean; the footer now renders Give Feedback → Account Picker → Log out (account picker placement lands in #350).

@erichwoo-scale
erichwoo-scale force-pushed the feat/agentex-ui-oidc-auth branch 2 times, most recently from 6e749ef to db8bec0 Compare July 8, 2026 19:31
Comment thread agentex-ui/app/api/auth/logout/route.ts Outdated
@erichwoo-scale
erichwoo-scale force-pushed the feat/agentex-ui-oidc-auth branch from db8bec0 to 00d2195 Compare July 8, 2026 19:40
@erichwoo-scale
erichwoo-scale force-pushed the feat/agentex-ui-account-picker branch from 8eb5b47 to 5f243a0 Compare July 8, 2026 20:00
@erichwoo-scale
erichwoo-scale force-pushed the feat/agentex-ui-oidc-auth branch 5 times, most recently from 054fad9 to 5cd4b21 Compare July 8, 2026 21:41
@declan-scale

Copy link
Copy Markdown
Collaborator

I went through this myself and it looks good. I also had Claude do a review pass focused on regressions and open-source (auth-disabled) safety, and it surfaced 6 items worth a look.

Default path is clean — verified with a real next build + next start (no AGENTEX_UI_AUTH_PROVIDER_ID): pages serve 200, BFF cookie-forwarding is preserved, no SessionProvider//api/auth/session calls, and Node.js middleware needs no experimental flag on next@15.5.18. One cosmetic note: /api/auth/session now returns 500 instead of 404 in default mode (unreachable in normal use; guard on authEnabled if you want it to stay 404).

A few things to address on the auth-enabled path, roughly by severity:

  1. getToken() reads the wrong cookie name over HTTPS (blocking). app/api/_lib/bff.ts and app/api/auth/logout/route.ts call getToken({ req, secret }) with no secureCookie/cookieName. Over TLS, NextAuth writes __Secure-authjs.session-token, but getToken defaults to the non-secure name and returns null → no Bearer attached (all /api/agentex/* → 401) and logout skips the RP-initiated end-session (IdP SSO survives, middleware signs the user back in). Works in local HTTP dev, which is likely why the smoke test passed. Suggest a shared helper that derives secureCookie from the forwarded proto.

  2. Transient refresh failures permanently kill the session. auth.ts (jwt callback): the !res.ok and catch branches wipe refreshToken and set terminal RefreshAccessTokenError for any failure. A single 5xx/network/discovery blip on a 240s refetch logs out every active user with no retry. Only 400 invalid_grant should be terminal; return the token unchanged otherwise.

  3. Discovery caches an incomplete 200 doc forever. discoverOidc() only invalidates on fetch failure — a 200 missing token_endpoint is cached for the process lifetime. Combined with Bump cross-spawn from 7.0.3 to 7.0.6 in /agentex-web #2, a brief malformed .well-known mass-logs-out sessions until restart. Invalidate on semantic validation too.

  4. BFF can forward a stale/expired token. Refresh only runs in cookie-writable contexts; the middleware matcher excludes api/, so the 240s SessionProvider poll is the only refresher. Short access-token lifetimes or a backgrounded/throttled tab can let an expired Bearer go upstream. Consider short-circuiting in the BFF when expiresAt is past, or re-auth on upstream 401.

  5. private_key_jwt omits kid on the login exchange but sets it on refresh (buildProvider vs signClientAssertion). If the IdP requires kid to select the key, initial login fails invalid_client while refresh works.

  6. /login → auto-signin does redundant OIDC round-trips with no already-authenticated check; benign today via SSO short-circuit, but an IdP with prompt=login would make it a visible re-prompt.

@declan-scale declan-scale 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.

Forgot to hit approve, just double check that first claude point and I think we should be good

@erichwoo-scale

Copy link
Copy Markdown
Contributor Author

Thanks Declan — all 6 addressed in 676365a, plus the /api/auth/session 404 cosmetic.

1. getToken cookie name (blocking). Added a shared getSessionToken(req) in auth.ts that passes secureCookie derived from AUTH_URL (https:// → the __Secure- cookie NextAuth actually writes). Both bff.ts and logout/route.ts use it now, so the Bearer attaches and RP-initiated logout fires over TLS. Keyed off AUTH_URL rather than the forwarded proto — it's the canonical external origin, already required for NextAuth's callbacks, and not attacker-controllable.

2. Transient refresh no longer terminal. The jwt callback now wipes the token + sets RefreshAccessTokenError only on 400 invalid_grant. Every other !res.ok and the catch return the token unchanged, so a 5xx/network/discovery blip retries next cycle instead of mass-logging-out.

3. Discovery validates before caching. discoverOidc() throws and clears the cached promise if a 200 is missing token_endpoint, so a malformed .well-known can't stick for the process lifetime (this was the multiplier on #2).

4. Stale Bearer on upstream 401. Went with re-auth-on-401 over a BFF expiresAt short-circuit — the BFF can't rotate the cookie, and decrypting the JWT on every proxy hop is wasteful. The SDK fetch wrapper, on a 401, hits /api/auth/session (which runs the jwt-callback refresh + rotates the cookie) and retries once. Deduped so a burst of 401s (e.g. a refocused tab) shares one refresh.

5. kid on the login exchange. buildProvider passes clientPrivateKey as { key, kid } so oauth4webapi stamps kid on the login client_assertion, matching signClientAssertion's refresh path.

6. auto-signin already-authenticated check. GET /api/auth/auto-signin returns a redirect when auth() already holds a valid (error-free) session, skipping the OIDC round-trip; also added a same-origin guard on redirect_url.

Cosmetic — /api/auth/session 404. The [...nextauth] route 404s in default (no-provider) mode instead of the provider-less handler's 500.

Comment thread agentex-ui/auth.ts
erichwoo-scale added a commit that referenced this pull request Jul 10, 2026
## Summary

First of a **2-PR stack** bringing account scoping + OIDC login to
agentex-ui. This base PR routes the SDK through a same-origin BFF and
adds an account switcher. Chart + deploy side lives in
scaleapi/sgp#3961.

- Routes the agentex SDK through a same-origin **BFF** (`/api/agentex`)
instead of calling the API directly from the browser — the upstream URL
and credentials never reach client JS.
- Shared `applyBffCredentials` forwards `x-selected-account-id`, drops
any client-sent `Authorization`, and strips the account-scoped `_jwt`
cookie so SGP honors the **selected** account rather than the one you
linked in with (identity stays via `_identityJwt`). The proxy also
strips `Location` on upstream 3xx so internal redirect targets don't
leak.
- Account selection is driven by the `account_id` query param (no
cookie), injected on every SDK request via a synchronous ref. Switching
accounts resets the open task + selected agent to the account's home
grid, and `resetQueries` drops the previous account's cached data so it
never briefly renders the old agents.
- `/api/user-info` fetches the caller's accounts; the picker bootstraps
to the first when the param is missing/stale, shows a disabled/empty
loading state (not a skeleton), renders single-account as static
context, and lives in the sidebar footer (collapsed → icon-only).
- Env-gated on the platform API being configured (`SGP_API_URL` /
`NEXT_PUBLIC_SGP_APP_URL`); no-op otherwise.

**Env:** `NEXT_PUBLIC_AGENTEX_API_BASE_URL` → server-only
`AGENTEX_API_URL`.

In this PR the BFF runs in default (cookie) mode — no auth dependency.
The token-hiding Bearer path arrives in the stacked PR.

## Stack

1. **This PR** — account picker + same-origin BFF proxy → `main`
2. #351 — OIDC login with server-side access token → stacked on this
branch
3. scaleapi/sgp#3961 — Helm chart + system-manager pack (deploy side)

## Test plan

- [x] `npm run typecheck` / `npm run lint` — clean
- [x] Runtime: switching accounts re-scopes agents/tasks (and SGP calls
via the `_jwt` strip); single / multi / no-account render; loading
state; no stale-account flash

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Base automatically changed from feat/agentex-ui-account-picker to main July 10, 2026 03:08
erichwoo-scale and others added 6 commits July 9, 2026 23:12
Add opt-in OIDC login on top of the BFF, keeping the access token out of the
browser entirely.

- NextAuth (generic OIDC provider) selected + enabled by a single env var,
  `AGENTEX_UI_AUTH_PROVIDER_ID`; disabled by default so non-auth deployments are
  unaffected (no SessionProvider mounts, no /api/auth/session calls).
- The BFF attaches the access token as a Bearer server-side via `getToken` and
  drops the UI session cookie — the token never reaches client JS or the NextAuth
  session (which exposes only `error`).
- Middleware auto-redirects unauthenticated requests to sign-in; a client guard
  re-auths on refresh failure; RP-initiated logout clears the IdP session.
- Supports `client_secret_post` (dev) and `private_key_jwt` (prod).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address Greptile P1s. The refresh path and RP-initiated logout hardcoded Ory's
`/oauth2/token` and `/oauth2/sessions/logout` paths, which 404 on non-Ory IdPs —
silently forcing a re-auth loop (refresh) or a transparent re-login (logout).

Resolve `token_endpoint` and `end_session_endpoint` from the issuer's
`.well-known/openid-configuration` (cached per-process; failures not cached). No
Ory-specific fallback: a missing token_endpoint fails the refresh cleanly (→ re-auth),
and a missing end_session_endpoint means local-only logout rather than a guessed path.
OneAuth (Ory Hydra) advertises both in discovery, and NextAuth already relies on the
same document for sign-in.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A GET /api/auth/logout is reachable by a crafted cross-site navigation (SameSite=Lax
sends the session cookie on top-level GET), letting an attacker end the user's IdP SSO
session. Switch to POST (Lax cookies aren't sent on cross-site POST); the client POSTs
and follows the returned RP-initiated logout URL.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…t mode

Trim verbose/redundant comments across auth.ts, middleware, session-guard, layout and the
auth routes; drop provider-specific ("Ory") wording (endpoints come from discovery); rename
the non-auth "legacy mode" to "default mode". No behavior change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- getToken passes secureCookie (from AUTH_URL) so the BFF finds the `__Secure-` session
  cookie over HTTPS; without it the Bearer was dropped (401s) and logout skipped end-session.
- Refresh: only invalid_grant is terminal; transient failures keep the token and retry.
- OIDC discovery: don't cache a 200 that's missing token_endpoint.
- private_key_jwt: add `kid` to the login client_assertion (parity with refresh).
- auto-signin: short-circuit when already authenticated; sanitize the redirect target.
- /api/auth/*: 404 in default (no-auth) mode instead of the provider-less handler's 500.
- SDK fetch: on a 401, refresh the session (deduped) and retry once.
- Expose the non-secret `sub` claim on the session (access/id/refresh tokens stay server-side).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…invalid_grant

Any 4xx (except 429) means the grant/client/request is bad and won't recover by
retrying → terminal (force re-auth); 429/5xx and network errors stay transient
(keep the token, retry next cycle). Broadens the terminal set beyond invalid_grant
so a persistent invalid_client (e.g. rotated secret) surfaces a real error instead
of silently forwarding an expired token, while still not logging users out on a
transient IdP blip. Drops the now-redundant response-body parse.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@erichwoo-scale
erichwoo-scale force-pushed the feat/agentex-ui-oidc-auth branch from 56998b0 to 4f6c53a Compare July 10, 2026 03:13
@erichwoo-scale
erichwoo-scale merged commit 824286c into main Jul 10, 2026
13 of 14 checks passed
@erichwoo-scale
erichwoo-scale deleted the feat/agentex-ui-oidc-auth branch July 10, 2026 03:15
erichwoo-scale added a commit that referenced this pull request Jul 10, 2026
…t-switch races (#355)

## Problem

Switching accounts intermittently corrupted the URL state: `task_id`
would survive the switch (leaving a stuck "phantom task" under the new
account), `agent_name` would clear, and sometimes `account_id` even
reverted to the old value **while the sidebar already showed the new
account's tasks**.

## Root cause

`updateParams` rebuilt the entire query string from the
`useSearchParams()` **snapshot**, which lags a render behind
`router.push`. An account switch fires **two** navigations from two
components:

- the provider's `setSelectedAccountId` clears `task_id`
- the agent-validation effect in `agentex-ui-root` clears `agent_name`

Whichever call read the stale snapshot and landed **last** clobbered the
other's change. The data layer switches synchronously
(`selectedAccountIdRef`), so tasks refetched for the new account even
when the URL reverted — the smoking-gun symptom. It was intermittent
because it depended on whether `useSearchParams` had propagated between
the two writes.

## Fix

- **`use-safe-search-params.ts`** — `updateParams` now merges each
update against the **live URL** (`window.location.search`), which is
always current, so concurrent writes compose instead of overwrite. This
is the systemic fix and addresses all three symptoms.
- **`agentex-provider.tsx`** — an explicit account switch now clears
`task_id` **and** `agent_name` in one atomic navigation (no dependence
on a downstream effect, no flash of stale params).
- **`use-safe-search-params.test.tsx`** — regression test proving a
write built from a stale snapshot no longer resurrects
`task_id`/`account_id`.

## Test plan

- `npm run typecheck` ✓
- `npx vitest run` — 50/50 ✓
- `npm run lint` ✓

Independent of #351 (OIDC); can merge in any order.

<!-- greptile_comment -->

<h3>Greptile Summary</h3>

This PR fixes an intermittent URL-state corruption that occurred during
account switches. The root cause was `updateParams` building its query
string from the `useSearchParams()` snapshot, which lags behind
`router.push()` calls — so two rapid navigations from different
components would race and the last writer would clobber the first's
changes.

- **`use-safe-search-params.ts`**: `updateParams` now reads
`window.location.search` (always current, updated synchronously by the
History API) instead of the React snapshot, so concurrent writes compose
rather than overwrite. Also fixes a minor trailing-`?` edge case for
empty param sets.
- **`agentex-provider.tsx`**: The explicit account-switch now clears
both `task_id` and `agent_name` atomically in one navigation,
eliminating the need for a downstream effect to clean up `agent_name`.
- **`use-safe-search-params.test.tsx`**: Regression tests proving the
stale-snapshot clobber is gone, covering set/delete/preserve semantics
and `replace` vs `push` routing.

<details><summary><h3>Confidence Score: 5/5</h3></summary>

Safe to merge — the fix is narrowly scoped to URL merge behavior and is
directly validated by new regression tests.

The change replaces a React snapshot read with a synchronous
window.location.search read inside updateParams, which is always safe in
browser environments and has a correct SSR fallback. The provider change
and the new tests are consistent with the fix, and no existing behavior
outside the race window is altered.

No files require special attention.
</details>

<details><summary><h3>Important Files Changed</h3></summary>

| Filename | Overview |
|----------|----------|
| agentex-ui/hooks/use-safe-search-params.ts | Core fix: reads from
`window.location.search` instead of the stale `useSearchParams()`
snapshot so concurrent writes compose correctly. Also adds a clean
fallback for SSR and fixes the trailing-`?` edge case for empty param
sets. |
| agentex-ui/components/providers/agentex-provider.tsx | Adds
`agent_name: null` alongside `task_id: null` in the explicit-switch
branch, so both account-scoped params are cleared atomically in one
navigation rather than relying on a downstream effect. |
| agentex-ui/hooks/use-safe-search-params.test.tsx | New regression test
suite with three cases: stale-snapshot clobber prevention, combined
set/delete/preserve in one update, and `replace` vs `push` routing.
Correctly uses `vi.hoisted` to share mock state with the factory
closure. |

</details>

<details><summary><h3>Sequence Diagram</h3></summary>

<a href="#gh-light-mode-only">

```mermaid
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant P as AgentexProvider
    participant E as agentex-ui-root effect
    participant H as useSafeSearchParams
    participant W as window.location
    participant R as Next.js Router

    Note over P,R: Before fix - stale snapshot clobber
    P->>H: "updateParams({account_id:new, task_id:null})"
    H->>W: reads stale snapshot (account_id:old, task_id:T)
    H->>R: "push(?account_id=new&task_id=null)"
    R->>W: window.location.search updated
    Note over E: snapshot not yet updated
    E->>H: "updateParams({agent_name:null})"
    H->>W: reads stale snapshot (account_id:old, task_id:T)
    H->>R: "push(?account_id=old&task_id=T) clobbers first write"

    Note over P,R: After fix - live URL merge
    P->>H: "updateParams({account_id:new, task_id:null, agent_name:null})"
    H->>W: reads window.location.search (live)
    H->>R: "push(?account_id=new) atomic, one navigation"
    R->>W: window.location.search updated
    Note over E: effect fires but agent_name already cleared
```

</a>
<a href="#gh-dark-mode-only">

```mermaid
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant P as AgentexProvider
    participant E as agentex-ui-root effect
    participant H as useSafeSearchParams
    participant W as window.location
    participant R as Next.js Router

    Note over P,R: Before fix - stale snapshot clobber
    P->>H: "updateParams({account_id:new, task_id:null})"
    H->>W: reads stale snapshot (account_id:old, task_id:T)
    H->>R: "push(?account_id=new&task_id=null)"
    R->>W: window.location.search updated
    Note over E: snapshot not yet updated
    E->>H: "updateParams({agent_name:null})"
    H->>W: reads stale snapshot (account_id:old, task_id:T)
    H->>R: "push(?account_id=old&task_id=T) clobbers first write"

    Note over P,R: After fix - live URL merge
    P->>H: "updateParams({account_id:new, task_id:null, agent_name:null})"
    H->>W: reads window.location.search (live)
    H->>R: "push(?account_id=new) atomic, one navigation"
    R->>W: window.location.search updated
    Note over E: effect fires but agent_name already cleared
```

</a>
</details>

<sub>Reviews (2): Last reviewed commit: ["fix(agentex-ui): merge URL
updates
again..."](dc3529b)
| [Re-trigger
Greptile](https://app.greptile.com/api/retrigger?id=43214802)</sub>

<!-- /greptile_comment -->

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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.

2 participants