[easonLiangWorldedtech]

Draft PR — vps2 unit F7 (modes skeleton until view state loads).

Tracking: fork issue 41 (vps2 series ledger). Upstream issue: #1566 (this series' gap record; the original upstream bug is #915). Port source: upstream PR #928 (fix(webview): add loading skeleton for view state initialization) — closed/superseded; the fork issue 41 ledger names it as the F7 port source (webview-ui sections only).

What this unit does at head 109f821

1. webview-ui/src/context/ExtensionStateContext.tsx — the viewStateLoaded plumbing: interface field + setViewStateLoaded, the useState (initial false), setViewStateLoaded(true) right after setDidHydrateState(true) in the state-message handler, and both on the context value.
2. webview-ui/src/components/modes/ModesView.tsx — the loading skeleton (data-testid modes-view-loading-skeleton, aria-busy) returned while viewStateLoaded is false, before the main return, so the modes surface never flashes stale mode/profile text during view-state initialization.
3. webview-ui/src/components/modes/tests/ModesView.spec.tsx — the mock fields + the "shows a loading skeleton while view-local state is initializing" test (skeleton testid present, mode-select-trigger absent, stale mode/profile text absent).
4. webview-ui/src/context/tests/ExtensionStateContext.spec.tsx — the TestComponent wiring + 3 tests: initializes false; true after the first state message; setViewStateLoaded callable.
5. webview-ui/src/App.tsx — the bare webviewDidLaunch useEffect deleted; the payload-carrying webviewDidLaunch post already lives in ExtensionStateContext.tsx (F1c) and is untouched. This hunk was planned for F1c but shipped without it; it is functionally part of the upstream bug fix.
6. webview-ui/src/tests/App.spec.tsx — the getViewStateId mock field in the vscode mock factory.
7. src-side coverage carried from the stack lineage (this unit's earlier rounds): the focusInput instance-lookup guard + its two registerCommands.spec tests, the delete-profile prune tests in ClineProvider.spec, the sticky-profile suite, and the profile-deletion comment fix in ClineProvider.ts.
8. Upstream main 496bd11 merged (this round): 8 conflicts reconciled with both-sides review. App.tsx keeps main's new webviewDidFocus interaction listener while webviewDidLaunch stays removed (ExtensionStateContext posts it once the view state is hydrated; re-posting in App would double the launch message). The refreshed F5/F6 lineage content (fresh-tab createClineTabPanel surface, view-identity fixes, api-spec factory args, tracker constructor args) is carried in; the three new ClineProvider constructions in the delete-profile tests pass the shared tracker.

Budget

Unit delta vs the stack base: 6 webview-ui files, +96/-5 (plus the src-side coverage carried from earlier rounds of this unit). The PR diff vs upstream/main is larger because the branch is stacked on the unmerged F1-F6 series; the series merges bottom-up, at which point each PR's CI gate measures its own unit. The CI mutation-diff changed-line advisory is a by-construction artifact of the stacked diff.

How to test

• pnpm --dir webview-ui test src/components/modes/__tests__/ModesView.spec.tsx — the skeleton test.
• pnpm --dir webview-ui test src/context/__tests__/ExtensionStateContext.spec.tsx — the viewStateLoaded lifecycle tests.
• pnpm --dir webview-ui test src/__tests__/App.spec.tsx — launch/focus message wiring.
• pnpm --dir src test src/activate/__tests__/registerCommands.spec.ts — the focusInput tab-priority guard tests.

Verification numbers (head 109f821)

• vitest src batch (ClineProvider.spec + sticky-mode + parallelMode + sticky-profile + registerCommands + Task + 3 api specs + webviewMessageHandler + flicker-free-cancel + ProviderSettingsManager): 824/824 across 12 files; api-start-new-task 6/6.
• webview-ui: ExtensionStateContext + App.spec + ModesView 61/61; packages/types global-settings 9/9.
• eslint --max-warnings=0 clean on all touched files; eslint-suppressions unchanged (ClineProvider.spec.ts stays at 194; no count increased).
• tsc: only 2 local-only errors remain (donor @roo-code/telemetry dist predates the 4-arg captureConsecutiveMistakeError signature; CI builds from source).
• Negative control: disabling the skeleton gate reddens ModesView "shows a loading skeleton while view-local state is initializing"; restored byte-exact (sha256 match).
• Prettier: full-repo check passes on the PR merge tree (re-verified at the current head).
• The apps/vscode-e2e suites (incl. the view-state suite touched by this series) run in CI; they need the real extension host and are not runnable in this local environment.

Updated this round (9859261 -> 1e87d0b)

• Merged upstream/main 496bd11; resolved 8 conflicts with both-sides review (registerCommands + spec, api.ts, ClineProvider + spec, Task.spec, ExtensionStateContext.spec, App.tsx).
• App.tsx: kept main's webviewDidFocus interaction listener; webviewDidLaunch stays removed (single launch post from ExtensionStateContext).
• Carried in the refreshed F5/F6 lineage: fresh-tab createClineTabPanel surface reconciled with main's typed options + shared WebviewFocusTracker, the view-identity review fixes with this series' setValue mode validation / corrupt-entry pruning / mutation-marker load-merge semantics, the restored viewStateReadiness/isDisposed members, api-spec ClineProviderFactory args, and the tracker constructor args across the specs.
• The three new ClineProvider constructions in this unit's delete-profile tests now pass the shared tracker.
• eslint-suppressions unchanged.
• (Second commit 7f5ed4e) Ported the F6 adaptation of main's api-start-new-task.spec.ts to this unit's fresh-tab design (mocks the registerCommands createClineTabPanel function, asserts the exact tracker-carrying payload, pins the factory out of the tab path).
• (Third commit 94e7742) Committed the WebviewFocusTracker argument for this unit's three delete-profile test constructions (CI check-types caught that the fix had not been staged into the merge commit).
• (Fourth commit 109f821) Prettier-formatted those constructions (CI format:check).

Series mechanics

• PR base is main; the branch is stacked on the F6 head; merge order F1a -> F1b -> F1c -> F2 -> F3 -> F4 -> F5 -> F6 -> F7.
• CS not-ported register (for consistency): kimi-code OAuth try/catch + routerModels.spec +29; ApiConfigManager min-w-0->grow; ApiConfigManager.visual.tsx deletion + 2 PNG baselines; mojibake comment; providers/, .coderabbit.yaml, .github/, CONTRIBUTING.md, .gitignore churn — none of these touch the files in this PR.

Temp-id test hygiene (head 7537805)

• AT-HEAD thread 4251997133 (Trivial, maintainability): the three temporary-id tests now dispose their providers in finally (constructor registers into the static activeInstances), restore the rejecting globalState.update stub in finally, and await the constructor's loadViewState promise instead of a fixed 10 ms sleep; the CloudService double gained on/off for dispose. Proofs: sticky-mode 30/30, eslint clean, prettier gate ALL-MATCHED; test-only change, no mutation control applies.
• AT-HEAD thread 4251997137 (Minor, functional correctness) argued: the modes skeleton is unreachable by design because webviewDidLaunch awaits setViewStateId -> loadViewState before the first state post, so viewStateLoaded flipping with didHydrateState is accurate and no wrong-mode frame exists; pre-gating the skeleton would flash it on every boot regardless of the active tab. Details in the thread reply.

CI re-trigger: head fa68982 is an empty commit over 7537805 (no content change) to re-run the flaky e2e-mock lane (known 30s-timeout flake); all verification numbers above apply unchanged.

Pre-merge ERROR-row pre-check (head 3fd9e42)

• Security Boundaries / view-state id rows: addressed at 3fd9e42. setViewStateId now rejects non-string ids (crafted postMessage), bounds the key length (MAX_VIEW_STATE_ID_LENGTH), and refuses ids already owned by another live instance; a refused view stays on its temporary id against shared state. Regression tests: non-string, oversized, claimed-by-another-view. Negative controls: disabling the ownership check reddens exactly the claim test; widening the length bound reddens exactly the oversized test; restores byte-exact (hash match).
• All three refusal paths resolve viewStateReadiness, so API-driven task creation never falls through to its timeout when a registration is refused.
• The spec gained an afterEach clearing the static instance registry (suites construct providers directly and never dispose most of them; leaked instances would claim ids ahead of later tests).
• Persistence Integrity setValues/setValue ordering rows: verified already addressed by the pushed code: _saveViewLocalStateFromMutation persists the non-secret subset before updating the buffer; the re-key is a single queued write (no delete-then-write window; authorship clears only after the write lands); the mutation hook keeps the buffer synced across setValue/setValues and profile upsert/activation/deletion.
• Vertex credential exposure via loadViewState: addressed by design - persisted viewStates entries carry only mode + currentApiConfigName (non-secret subset); loadViewState resolves the profile into the in-memory buffer, which reaches the webview through the existing ExtensionState apiConfiguration channel, and logs name the profile only. No secret is written to workspace storage.
• Proofs at 3fd9e42: sticky-mode 33/33; eslint --max-warnings=0 clean; tsc clean except the two known local telemetry-dist noise lines; prettier merge-ref gate ALL-MATCHED.
• Non-activating saves (upsertProviderProfile with activate=false, and the zoo-gateway callback's non-active path) rewrote the profile on disk but left pinned views serving the pre-save snapshot; the non-activating branch now refreshes pinned sibling views (and this view when it holds the pin), and the callback routes through upsertProviderProfile so the refresh cannot be forgotten.
• getState() no longer spreads the pinned overlay over the shared provider settings (a key the profile never defined leaked in from the shared profile: model id, region, tuning). The overlay is a complete snapshot: used in place, with only apiProvider falling back to shared state.
• Regression tests: "refreshes views pinned to a profile saved without activation"; "does not leak shared provider keys into a pinned view's api configuration; the zoo-gateway callback test asserts the upsert path.
• Negative controls: dropping the else-branch refresh reddens exactly the refresh test, reverting the merge semantics reddens exactly the leak test; restores byte-exact (hash match).
• Proofs at 5bcc785: ClineProvider.spec green, sticky-mode and webviewMessageHandler green, eslint --max-warnings=0 clean, tsc clean except the two known local telemetry-dist noise lines, prettier merge-ref gate ALL-MATCHED.

Follow-up: overlay read from the view buffer (required-lane fix)

The previous round's overlay-completeness change read the overlay from the merged state values, which also carry the raw apiConfiguration state key. That key is a partial patch written by shared-store paths, not a view snapshot: with no overlay present and the raw key partial, getState() served only the partial object and dropped the shared provider settings the previous merge produced (the parallel-mode activation tests caught exactly this - the activated profile's model id vanished; required lane red on the units carrying that spec). The overlay is now read from the view buffer itself, where a present value is by contract a complete profile and an absent one means the shared store serves the settings.

Proofs: ClineProvider.parallelMode.spec green where present (the six synchronization tests that regressed), ClineProvider.spec green including the overlay-completeness tests, sticky-mode and webviewMessageHandler green; negative control: reverting the overlay to a shallow merge over the shared settings still reddens exactly the leak test; restores byte-exact (hash match); eslint --max-warnings=0 clean; tsc clean except the two known local telemetry-dist noise lines; prettier merge-ref gate ALL-MATCHED.

Follow-up: active task handlers rebuilt on non-activating saves (head 268bb11)

A review round on the view-identity unit found that the non-activating-save refresh updated pinned views' buffers but not their active tasks' API handlers, and the posted state prefers the task's own apiConfiguration: a conversation running in a view pinned to the saved profile kept sending the old token (e.g. after the Zoo Gateway callback rotated it) even though the view's buffer had caught up. The refresh now rebuilds the affected tasks' handlers in the pinned views, and the non-activating branch rebuilds this view's own handler when it holds the pin.

Proofs: ClineProvider.spec green (the pinned-sibling refresh test now asserts the active task's handler is rebuilt with the saved settings, not just the private buffer); negative control: dropping the rebuild from the refresh reddens exactly that assertion, restores byte-exact (hash match); parallelMode/sticky-mode/webviewMessageHandler green; eslint --max-warnings=0 clean; tsc clean except the two known local telemetry-dist noise lines; prettier merge-ref gate ALL-MATCHED.

Follow-up: test-quality round from the review (head 726e230)

The review round on the api-wiring unit named four test-quality issues; the ones present in this unit are fixed here: the viewStates describe block moved out of the alwaysDenyUnapprovedCommands block (test names carried the wrong subject), the prune test's toBeDefined() survival checks replaced with exact-shape toEqual assertions (they pass even if pruning strips fields), the stale-temporary-entry test's fixed 10 ms wait replaced with awaiting the load itself (the negative assertion could pass vacuously on a slow load), and the readiness-cancel test's fake timers restored in a finally block (a failed assertion left fake timers active and hung a later real-timer test).

Proofs: global-settings 9/9 where present, ClineProvider.spec green, sticky-mode green where present, api-task-control green where present; negative controls: pruning that strips updatedAt reddens exactly the prune test, a loadViewState that adopts the unowned temporary-id entry reddens exactly the stale-entry test, restores byte-exact (hash match); eslint --max-warnings=0 clean (src and types); prettier merge-ref gate ALL-MATCHED.

Follow-up: review-round fixes from the series (head f528dcc)

A review round on the mode-profile-writes and e2e-view-state units found fixes this unit carries. When the view's durable mode pin write rejected inside the mode-switch handler, the task-history write and the in-memory task mode had already landed and the caller delivered the submitted message with the new task mode against the view's unchanged mode and mode-linked API configuration - the failure now restores the pre-switch task mode and history entry before rethrowing (new sticky-mode test; negative control: dropping the rollback reddens exactly that test).
A review round on the mode-profile-writes unit found that the profile-deletion re-pin updated each sibling view's buffer but not its running task: the posted state prefers the task's own apiConfiguration, so a conversation in a view pinned to the deleted profile kept sending the deleted profile's settings. The re-pin now rebuilds the sibling's API handler with the replacement's settings and re-points the task's sticky profile name, mirroring the activation path.

Proofs: ClineProvider.spec and sticky-mode green; eslint --max-warnings=0 clean; tsc clean except the two known local telemetry-dist noise lines; prettier merge-ref gate ALL-MATCHED. The rebuild's negative control (dropping it reddens exactly the sibling re-pin test, byte-exact restore) runs in the units that own that test.

In addition: The view-state readiness bound no longer rejects headless callers whose webview never launched (it only rejects when the webview has launched and readiness failed), and the IPC start-task listener catches rejections since the IPC server does not await async listeners (new headless-fallback and launched-stuck tests; negative control: an unconditional timeout reddens exactly the fallback test). The readiness-cancel test now pins the bound's own timer via fake timers and a zero timer count instead of a global clearTimeout spy, and the mode-ordering test asserts the switch has not been called before the pending initialization is released (negative controls: dropping the timer clear / the initialization await reddens exactly the respective assertion).

Proofs: ClineProvider.spec 299/299, sticky-mode 34/34, parallel-mode 40/40, Task.spec 190/190, api-task-control 26/26; every negative control reddens exactly its target test and restores byte-exact (hash match); eslint --max-warnings=0 clean; tsc clean except the two known local telemetry-dist noise lines; prettier merge-ref gate ALL-MATCHED.

---

Updated this round (CodeRabbit follow-up, head 78c21d1)

• Non-activating profile saves no longer force-rebuild the API handler of a view whose active task follows a different sticky profile: both rebuild sites (the acting view's pinned branch and the pinned-sibling refresh) are guarded by taskApiConfigName === saved profile. Pinned-view overlays still refresh; regression tests cover both sites (negative control: removing either guard reddens exactly its test).
• Sticky-mode afterEach now disposes the per-test provider before resetting the instance registry, so providers leave McpServerManager's static provider set between tests.
• Verified: ClineProvider.spec 301 green, sticky-mode 34 green, parallelMode 40 green, eslint --max-warnings=0 clean, tsc real=0, prettier clean.
• Test hygiene (head bb3a988, same series-wide round): prune assertions compare full kept entries; the two re-key storage tests seed through the durable write path so their assertions can fail; the webview state-payload test renamed to describe what it asserts.

---

Updated this round (CodeRabbit follow-up 2, head 41bbc27)

• Deleted-profile re-pin: the forced task-handler rebuild and the sticky-name rewrite now require taskApiConfigName === deleted profile (the view-buffer re-pin stays unconditional), so a pinned view whose task follows another sticky profile keeps its conversation's settings and persisted profile name. Regression test included (negative control: removing the guard reddens exactly that test); the existing sibling re-pin test's task double now names the deleted profile as its sticky profile, matching what the test asserts.
• Verified: ClineProvider.spec 302 green, sticky-mode 34 green, parallelMode 40 green, eslint --max-warnings=0 clean, tsc real=0, prettier clean.

Stryker suppression comments removed

Per the AGENTS.md policy (never add Stryker directives; remove them when a file is touched), all // Stryker disable directives were dropped from this PR. Equivalence rationale for the removed suppressions:

• Stryker disable next-line ConditionalExpression: defensive clobber guard, unreachable per the ordering argument above.
• Stryker disable next-line StringLiteral: setPanel branches only on type === "sidebar", so any other literal routes to the identical tab-ref assignment
• Stryker disable next-line StringLiteral: setPanel only distinguishes "sidebar"; any other value routes to the tab-ref assignment
• Stryker disable next-line ConditionalExpression: at the catch, this call's own state-change handler only ever re-points the tracked ref at newPanel itself, so tabPanel === newPanel is the only reachable state here; a concurrent re-pointing race is not representable in this spec.
• Stryker disable next-line ConditionalExpression: only tool_use and mcp_tool_use entries ever carry an id — the text block pushed in the text case has no id field and the start events reaching this guard always carry the string id locked in by processRawChunk — so the type check cannot change the outcome of entry.id === event.id for any entry this pipeline produces; the false replacement is additionally killed by the rejection tests.
• Stryker disable next-line ConditionalExpression: event.name is always a string on this path — the only delta events reaching this handler are emitted by processRawChunk with the tracked start name (never undefined), so the legacy nameless branch below is unreachable defensive code and the conditional always takes the named side.
• Stryker disable next-line ConditionalExpression: streamingKey can never be undefined here — makeStreamingKey always returns a string and the legacy lookup branch above is unreachable (see the directive on the event.name check) — so the null side is unreachable defensive code.
• Stryker disable next-line ConditionalExpression, EqualityOperator: streamingKey is always a defined string on this path (makeStreamingKey never returns undefined; the legacy branch is unreachable per the directives above), and processStreamingChunk returns a partial ToolUse for a known key, so both defensive conditions reduce to the partialToolUse check and cannot change the observed behavior.
• Stryker disable next-line LogicalOperator: event.name is always a string on this path — finalizeRawChunks emits end events with the tracked start name (a string, set before the tracker can start), so the ?? event.id fallback guards a nameless end event this pipeline cannot produce.
• Stryker disable next-line ConditionalExpression, LogicalOperator, EqualityOperator: the both-miss case is unreachable via the raw-chunk pipeline — end events carry the locked start name so the compound key resolves the registered entry, and a same-ID rejected-start end is always processed after the accepted start's end has already removed that entry; this defensive path protects against a cross-name end event the pipeline cannot produce.
• Stryker disable next-line ConditionalExpression: unreachable defensive region — getStreamingToolCallById can only resolve when the both-miss case above is reachable, which the raw-chunk pipeline cannot produce (see the directive there).
• Stryker disable next-line CallExpression,ObjectLiteral: _updateViewLocalStateFromMutation applies only mode, currentApiConfigName, apiConfiguration and provider-settings keys, so a listApiConfigMeta-only payload (deleted call or empty object) is a behavior-preserving no-op.
• Stryker disable next-line ConditionalExpression: in this fallback the view does not pin the deleted profile, so when the deleted profile was not the shared selection profileToActivate equals the shared selection itself; forcing the branch only rewrites that same value back into the store.
• Stryker disable next-line ConditionalExpression,BlockStatement: hostile non-stabilizing encodings are unreachable from the webview (its posts are plain link targets); the bound is defensive
• Stryker disable next-line CallExpression: hostile non-stabilizing encodings are pinned by the direct decodeUntrustedPathToStable bound test; the webview never posts such values
• Stryker disable next-line ConditionalExpression,OptionalChaining: equivalent mutant - when localStorage is unavailable the guard-false path and the throwing body both return this.fallbackState from this catch
• Stryker disable next-line ConditionalExpression,OptionalChaining: equivalent mutant - when localStorage is unavailable the guard-false path and the throwing body both mark the write failed and return newState

CR round 2026-10-15 (b)

• Ported the payload-customModes fix for setValues mode validation (identical flagged block; regression test added) so the units stay byte-identical at merge.

CR round 2026-10-15 (c)

• Ported: shared apiProvider fallback in getConfiguration and the dispose() finally-path deregistration (identical flagged blocks; regression tests added).

CR round 2026-10-15 (d)

• getConfiguration re-applies the shared-provider fallback after flattening, so an explicit apiProvider: undefined in the nested view-local profile can no longer clobber it (matching getState()'s ?? semantics). Regression test added; the identical block in fix(api): wire task controls and sidebar-targeted configuration (vps2 F5) #1557/test(e2e): port view-state suite with reload and rehydration coverage (vps2 F6) #1565 received the same fix.

CR round 2026-10-15 (e)

• Ported: whole-operation deregistration finally in dispose() and createTask mode restore on custom-mode registration failure (identical flagged blocks; regression tests added).

CR round 2026-10-15 (f)

• The createTask registration-failure rollback now restores the shared mode and the view's own mode pin independently (unset pin stays unset); divergent-pin regression test added. Same fix in all six units carrying the flagged block.

Row-9 update (head 92ae27e)

• Activation rollback completeness: the pre-activation snapshot captures the exact durable viewStates map and the rollback restores it through the serialized queue, recovering sibling entries the activation's pruning evicted (CR Persistence Integrity).
• setGlobalState persists parsed.data instead of the raw payload (CR Security Boundaries).
• Activation writes from the stored record (read-back feeds every post-save consumer; CR Security Boundaries).
• Clear/re-point cache divergence is guarded at the ContextProxy#updateGlobalState layer on this unit; the new tests pin that guard.

Verification: new tests are focused negative controls (reverting each guard reddens exactly the matching test); ClineProvider.spec, webviewMessageHandler.spec spot-checks, api specs, spec-inclusive tsc --noEmit, eslint --max-warnings=0 (suppression counts unchanged) and prettier all green on this head.

Upstream refresh (head 92ae27e)

Merged the current upstream/main (merge commit; the row-9 fix commit is unchanged and is the first parent) to clear the review gate's mergeability hold. Upstream refactored upsertProviderProfile into a new persistProviderProfile method and added a runToCompletion queue option; this unit's transactional body was ported into the new method unchanged and upstream's Promise.allSettled activation fix was adopted in it. ClineProvider.spec, webviewMessageHandler.spec, tsc --noEmit, eslint (suppression counts unchanged) and prettier re-verified green at this head. The first refreshed head's windows unit-test lane caught that upstream's new updateProfileModel tests in ClineProvider.apiHandlerRebuild.spec.ts mock the settings manager with a static getProfile, which starves the post-save read-back; the mock now repoints getProfile at what saveConfig wrote (the real manager's behavior), and the file's 38 tests pass locally alongside the rest of the verification set.