Skip to content

Decompose App.tsx phase 2: extract the coupled clusters on a useSessionRef seam #2129

Description

@cliffhall

Phase 2 of #2126 — decomposing clients/web/src/App.tsx (5,270 lines).

The hard phase. Roughly 1,700 lines across four clusters that participate in the connection ↔ OAuth ↔ commands cycle, so they cannot be extracted independently without a seam.

This phase should be several PRs, not one. Land useSessionRef first and alone; then one cluster per PR, in the order below.

Step 1 (prerequisite): useSessionRef

App.tsx currently mirrors six values into refs so callbacks can read current values without re-creating:

Line Ref
1803 activeServerNameRef
1804 activeServerIdRef
1805 serversRef
1811–1822 inspectorClientRef (+ its sync effect)
1056–1060 pendingStepUpRef (+ its sync effect)
1082–1085 pendingReauthRef (+ its sync effect)

Collapse them into one useSessionRef({ … }) returning a single stable ref. Six refs and their sync effects become one.

What this is not: not a store, not reactive, not context, nothing renders off it. It is exactly the pattern already in the file, written once instead of six times. It works as a seam because a hook taking sessionRef takes a stable dependency, so extracting one cluster does not force the others into its argument list.

These are the latest-ref pattern and are not what AGENTS.md's "never sync state from a prop in an effect" rule forbids. Any genuine prop→state derivation introduced here still uses useValueChange.

Step 2: useOAuthRecovery(session) — ~560 lines

The largest single cluster and the one with the most subtle behavior.

  • 1049–1120 — step-up state, trySetPendingStepUp, the in-progress guards
  • 1850–1884 — showReAuthBanner
  • 1885–1965 — the OAuth callback effect and OAuthDetails
  • 1973–2011 — webOAuthStorage, sessionStorageAdapter
  • 2047–2145 — onBeforeOAuthRedirect, prepareOAuthRedirect
  • 2146–2430 — tryApplyStoredAuthRecovery, runVisibleInteractiveAuth, deferAmbientReauth, handleCommandScopedAuthRecovery, runWithCommandAuthRecovery, runCommandInBackground, resumePendingReauth
  • 2431–2570 — the four OAuth effects
  • 4620–4700 — clearServerOAuthAndDisconnect, handleClearConnectionOAuth, handleClearStoredOAuthFromSettings

Returns the banner state, the step-up state and handlers, and — critically — runWithCommandAuthRecovery, which Step 4 wraps every command in.

The oauthCallbackHandledRef / staleOAuthCheckedRef / oauthResumeUiAppliedRef / reauthResumeInProgressRef / stepUpAuthorizeInProgressRef guards (1086–1135) are once-only latches guarding real re-entrancy, several of them there to survive StrictMode's effect replay. Move them with the cluster and do not "simplify" them.

resumePendingReauth needs to drive a reconnect, which Step 3 owns. Inject it as a callback (onReconnect) rather than importing across — that keeps the direction one-way.

Step 3: useConnectionLifecycle(session, oauth, stores) — ~700 lines

  • 1121–1126, 1823–1836 — latencyMs, connectStartRef, connectErrorMessage, recordConnectError
  • 1433–1503 — clearOAuthResumeOnExplicitDisconnect, finalizeExplicitDisconnect, resetSessionScopedUiState
  • 1504–1745 — the connection effects
  • 2571–2785 — setupClientForServer (215 lines, the heaviest single function in the file)
  • 2786–3037 — the 250-line effect that drives it
  • 3038–3233 — onToggleConnection
  • 3234–3415 — onDisconnect, its effect, onReauthenticateFromBanner

resetSessionScopedUiState (1446) reaches into most of Phase 1's UI state to clear it on disconnect. Give it a narrow reset() surface from each Phase 1 hook rather than handing it every setter — otherwise this hook depends on everything and nothing was decoupled.

Step 4: useServerCommands(session, oauth, stores) — ~530 lines

  • 3416–3595 — onCallTool, onClearToolResult, onToolsUiChange
  • 3682–3818 — onGetPrompt, onReadResource, onReadResourceContents, onSubscribeResource, onUnsubscribeResource, onCompleteArgument
  • 3819–3894 — onCancelTask, onCancelToolCall, onClearCompletedTasks, onSetLogLevel, onSetModernLogLevel
  • 3895–3949, 4085–4127 — the refresh and load-more callbacks
  • 3950–4084 — onTogglePaginatedLists (135 lines on its own)
  • 1015–1032 — toolCallState, getPromptState, readResourceState

Every one of these routes through runWithCommandAuthRecovery from Step 2. That wrapper is the reason this cluster cannot precede the OAuth one.

⚠️ #2095 (the pagination toggle showing a stale value after switching servers and back) lives in onTogglePaginatedLists / paginatedListsOverride. Do not fix it here. Land the move inert, then fix it on top — a behavior change buried in a 135-line relocation is unreviewable.

Step 5 (optional, can slip): useMcpApps(session) — ~200 lines

Lines 773–970: appRendererRef, listedResourcesRef, getListedResourceMeta, publishDocument, sandboxBridgeFactory, elicitationBridgeFactory, the app-elicitation controller/session refs, newAppElicitationSession, handleAppElicitationSettle, handleAppElicitationFail.

Nearly a leaf — it touches the client but not the OAuth/command cycle. It could arguably go in Phase 1; it is parked here because the sandbox and elicitation bridges are load-bearing for smoke:web:app and smoke:web:elicit and deserve unhurried attention rather than being tacked onto the easy phase.

⚠️ Coverage — this is where the effort actually is

Phases 0 and 1 are cheap to cover. This one is not. ~1,700 lines of async control flow, effects, re-entrancy guards, and error paths all cross into the ≥90% gate on all four dimensions, including branches.

Scope each PR with that in mind: the test-writing will exceed the moving, and a cluster that lands under-tested cannot merge — the gate is CI-enforced per file.

Reach for /* v8 ignore … -- <reason> */ only for genuinely unreachable branches per the AGENTS.md policy (StrictMode replay blocks are an accepted reason and will legitimately come up here). Do not lower the gate.

The three web smokes (smoke:web:browser, smoke:web:app, smoke:web:elicit) are the backstop that the moves stayed inert end to end — they are in npm run ci and must stay green through every step.

Done when

Activity

  1. added this to the v2.5.0 milestone on Aug 25, 2026
  2. added
    v2Issues and PRs for v2
    choreMaintenance: deps, build tooling, CI, cleanup — no user-facing behavior change
    on Aug 25, 2026
  3. kbsoso9999-maker commented on Aug 26, 2026

    @kbsoso9999-maker
  4. self-assigned this
    on Aug 27, 2026
  5. cliffhall commented on Aug 27, 2026

    @cliffhall
    MemberAuthor

    Phase 2 is a stack

    Split into five sub-issues, one per step, each landing as its own PR. They are stacked: every branch is cut from the previous step's branch and every PR targets that branch rather than v2/main, so each diff shows only its own step and the order dependence is visible on the PR itself. GitHub retargets a child to v2/main as its parent merges, so the stack drains in order — provided nothing jumps the queue.

    Step Sub-issue Hook Base branch State
    1 #2157 useSessionRef v2/main PR #2152, in review
    2 #2153 useOAuthRecovery (~560 ln) step 1's not started
    3 #2154 useConnectionLifecycle (~700 ln) step 2's not started
    4 #2155 useServerCommands (~530 ln) step 3's not started
    5 #2156 useMcpApps (~200 ln, optional) step 4's not started

    The order is forced rather than chosen. Step 4 wraps every command in runWithCommandAuthRecovery, which step 2 owns. Step 2's resumePendingReauth needs a reconnect that step 3 owns, which is why it takes it as an injected onReconnect rather than importing across — that keeps the direction one-way and is what lets 2 precede 3. Step 5 is nearly a leaf and is last only because it can be dropped without disturbing the others.

    This issue stays open as the phase tracker and reaches Done when its last sub-issue closes; each PR closes its own step, not this one.

    ⚠️ Restating the part that governs how big each PR gets: for steps 2–4 the test-writing will exceed the moving. If a step turns out to be unreviewable at that size, it gets split within the step rather than landing under-tested — the ≥90% gate is per-file and CI-enforced, and lowering it is not on the table.

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

Metadata

Metadata

Assignees

Labels

choreMaintenance: deps, build tooling, CI, cleanup — no user-facing behavior changerefactorCode refactoringv2Issues and PRs for v2

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions