Skip to content

fix(webview): invalidate live sibling view state on reset and settings import (vps2 F4) - #1562

Draft
easonLiangWorldedtech wants to merge 9 commits into
Zoo-Code-Org:mainfrom
easonLiangWorldedtech:vps2/f4-cross-instance-reset
Draft

fix(webview): invalidate live sibling view state on reset and settings import (vps2 F4)#1562
easonLiangWorldedtech wants to merge 9 commits into
Zoo-Code-Org:mainfrom
easonLiangWorldedtech:vps2/f4-cross-instance-reset

Conversation

@easonLiangWorldedtech

Copy link
Copy Markdown
Contributor

Draft PR — vps2 unit F4 (cross-instance reset + import invalidation).

Tracking: easonLiangWorldedtech#41 (vps2 series ledger). Upstream issue: #1561 (this series' gap record; the original upstream bug is #980). Port source: upstream PR #981 (fix(webview): invalidate per-view state after reset and import) — closed draft, superseded by the vps2 series; the #41 ledger names #981 as the F4 port source.

Scope

5 files, 342 insertions, 1 deletion (measured vs stack base 8da5c6e):

  • src/core/webview/ClineProvider.ts (21+/0−) — the broadcastResetToAllInstances() method: every live instance clears its in-memory view-local state (_clearViewLocalState), the global contextProxy.setValue("viewStates", undefined) write clears the durable per-view entries in a single write-queue clear, and each non-calling instance re-posts its state to its webview. Wired into resetState (comment + await this.broadcastResetToAllInstances()) before its final postStateToWebview().
  • src/core/config/importExport.ts (13+/0−) — ImportWithProviderOptions gains the optional broadcastResetToAllInstances(): Promise; importSettingsWithFeedback gains the guarded broadcast block after the settingsImportedAt write (try/catch: a broadcast failure is console.warn'd and never fails the import).
  • src/core/config/tests/importExport.spec.ts (158+/0−) — the 3 fix(webview): invalidate per-view state after reset and import #981 tests (broadcast on successful import; skip when the callback is missing — with a gate-required console.warn negative assertion pinning the guarded call; import result kept successful when the broadcast throws — console.warn spied and restored); the raw provider-identifier casts of the fix(webview): invalidate per-view state after reset and import #981 text are adapted to the providerIdentifiers.* constants (lint-required, no semantic change).
  • src/core/webview/tests/ClineProvider.parallelMode.spec.ts (116+/0−) — the F4 multi-instance + _clearViewLocalState describes appended from the CS copy (116 lines: CS L1674-L1789, byte-identical; 5 new tests; CS is the spec source of record — fix(webview): invalidate per-view state after reset and import #981's parallelMode file is 1357 lines and lacks the F3 describes).
  • src/core/webview/tests/ClineProvider.spec.ts (34+/1−) — (a) the forward fix of the F3 resetState sentinel: F4's single global viewStates clear removes the key (real VS Code Memento semantics: update(key, undefined) deletes the key), so the F3-era toEqual({}) expectation becomes toBeUndefined(); the test intent (no persisted per-view entry after reset) is preserved and satisfied more strongly; (b) one new multi-instance test (gate-required): two live instances, one saves a view-local mode, the other calls resetState() — the sibling's view-local state is cleared, the sibling receives exactly one state post, and the caller receives exactly one (its final post).

Budget

  • a+d 343 vs the 400-soft / 1000-hard budget: under the 400-soft target (headroom 57); the 1000 hard cap is not approached.
  • Stryker-diff gate (vs 8da5c6e, final head 80c147f): 11 raw mutants across the 20 executable changed lines (importExport.ts 6, ClineProvider.ts 5): 11 Killed, 0 Survived, 0 NoCoverage, 0 Ignored; thresholds 100/100; exit 0 at the final head.
  • vitest: 309 passed (4 suites, 4 spec files) — per file: importExport.spec 54, ClineProvider.parallelMode 29, ClineProvider.sticky-mode 19, ClineProvider.spec 207 (sticky-mode + ClineProvider.spec included as regression sentinels for the resetState change).
  • check-types, eslint (--prune-suppressions, max-warnings 0; suppression counts flat-or-down), prettier (--end-of-line=auto): all pass — check-types exit 0 (the standing proof of the webviewMessageHandler no-change decision: the importSettings case passing the full ClineProvider type-checks against the extended ImportWithProviderOptions.provider type); eslint exit 0 on the touched files (suppression counts flat — eslint-suppressions.json is 0/0 in this commit); prettier --end-of-line=auto clean.

Port fidelity (coordinator-verified)

  • ClineProvider.ts: the broadcastResetToAllInstances method is ported from the exact fix(webview): invalidate per-view state after reset and import #981 diff text (declaration, JSDoc, body) hunk-by-hunk; the resetState wiring inserts the exact fix(webview): invalidate per-view state after reset and import #981 comment + call before the final postStateToWebview() (the base resetState already carries _clearViewLocalState() + clearPersistedViewState()). The global viewStates clear removes entries only — viewStateSchema holds no secrets (mode/currentApiConfigName/updatedAt), and the 50-entry prune cap (F2 tests) is unaffected by clearing.
  • src/core/webview/webviewMessageHandler.ts is 0/0 vs base. fix(webview): invalidate per-view state after reset and import #981's structural provider-wrapper hunk (the importSettings case carrying the broadcast callback) is REDUNDANT in this stack: the base importSettings case already passes the full provider object (provider: provider), which structurally satisfies the extended ImportWithProviderOptions.provider type (settingsImportedAt + postStateToWebview present; broadcastResetToAllInstances optional and present on the full ClineProvider after this port). The guarded call inside importSettingsWithFeedback therefore reaches the real method with zero WMH changes; the check-types gate is the standing proof.
  • importExport.ts: the two fix(webview): invalidate per-view state after reset and import #981 hunks verbatim (the optional method on ImportWithProviderOptions; the guarded broadcast block in importSettingsWithFeedback after the settingsImportedAt write).
  • importExport.spec.ts: the 3 fix(webview): invalidate per-view state after reset and import #981 tests ported with two adaptations: (1) lint-required — the raw provider-identifier string casts of the fix(webview): invalidate per-view state after reset and import #981 text are replaced with the providerIdentifiers.* constants (zoo/no-raw-provider-identifiers), no semantic change; (2) gate-required — the skip test gains a console.warn negative assertion (the fix(webview): invalidate per-view state after reset and import #981 text had none).
  • ClineProvider.spec.ts: exactly one assertion change — the F3 resetState sentinel ("should clear viewLocalState and the persisted entry when resetting state"): toEqual({} (the F3-era artifact: F3's last viewStates write was clearPersistedViewState() writing {}) becomes toBeUndefined() with a 2-line comment, because F4's mandated single global clear (contextProxy.setValue("viewStates", undefined)) forwards to updateGlobalState(key, undefined), which removes the key under real VS Code Memento semantics. The test's intent (no persisted per-view entry after reset) is preserved and satisfied more strongly; the fix(webview): invalidate per-view state after reset and import #981-mandated broadcast body is unchanged. Plus one new multi-instance test (gate-required, not from fix(webview): invalidate per-view state after reset and import #981 — see below).
  • ClineProvider.parallelMode.spec.ts: the F4 describes (CS L1675-L1789, with the L1674 blank separator — 116 lines total) appended byte-identical from the CS copy; nothing earlier in the file touched.
  • Stryker gate evidence (why the two test additions above exist): at the F3-head gate, the fix(webview): invalidate per-view state after reset and import #981/CS port alone left 5 Survived mutants — the importExport truthiness-guard mutant (ConditionalExpression -> "true", hidden because the guarded try/catch swallows the TypeError a missing method would throw) and 4 broadcastResetToAllInstances mutants (the _clearViewLocalState call site and the instance !== this condition, covered only by single-instance tests). fix(webview): invalidate per-view state after reset and import #981 was a closed draft that never ran this gate. The two additions (one CP.spec multi-instance test, one console.warn negative assertion) bring the gate to 0 Survived / 0 NoCoverage with no Stryker disable directives and no source changes.
  • DO-NOT-PORT verifications (per the fix(webview): invalidate per-view state after reset and import #981 file map): packages/types 3 files (index.test.ts / global-settings.ts / vscode-extension-host.ts) — already shipped by F1a/F1c (presence verified by content, 0/0 here); ClineProvider.spec.ts 2 hunks + ClineProvider.sticky-mode.spec.ts 4 hunks — F3-owned (shipped); webviewMessageHandler.ts + webviewMessageHandler.spec.ts — F1c territory / redundant (above); webview-ui 4 files (App.tsx, App.spec.tsx, ExtensionStateContext.tsx, utils/vscode.ts) — F7 / F1c territory (0/0 here).

Structural note on the parallelMode spec (coordinator-verified)

The CS parallelMode.spec.ts is 1790 lines: a shared preamble (L1-672), an F1-series test section (L673-1363: viewId uniqueness, local state isolation, saveViewState, stale temporary-id load), the F3 describes (L1364-1673), a blank separator (L1674), the F4 multi-instance describes (L1675-L1789), and the file's final top-level close (L1790). In this series the F1-series section lives in ClineProvider.spec.ts (shipped by F1a/F1b/F1c — the deliberate F1-series describe restructure), and the F2 unit shipped the persisted-pruning and #1065 retention tests inside the parallelMode file (absent from the CS parallelMode file). Both placements are behaviorally covered; the divergence is structural, not a coverage gap.

Series mechanics

  • Base of record: upstream/main @ 0d937c0; PR base is main; the branch is stacked on the F3 head 8da5c6e (the F1-series and F2/F3 heads merge below it in the series merge order).
  • Draft PR per unit; merge order F1a to F1b to F1c to F2 to F3 to F4 to F5 to F6 to F7.
  • CS not-ported register (for consistency): (1) kimi-code OAuth try/catch + routerModels.spec.ts +29; (2) ApiConfigManager.tsx min-w-0 shrink to grow; (3) ApiConfigManager.visual.tsx deletion + 2 PNG baselines; (4) mojibake comment; (5) unused defaultModeSlug import — resolved by F3; (6) providers/, .coderabbit.yaml, .github/, CONTRIBUTING.md, .gitignore churn.
  • Merge check against upstream main 4c7474d (merge-base = base of record 0d937c0), via git merge-tree on the full F0-to-F4 stack: tree e3af144dead05acf4fc681a33786d03dd4e1b681; auto-merges Task.ts, Task.spec.ts, ClineProvider.ts, ClineProvider.spec.ts; the sole conflict is src/eslint-suppressions.json (stage blobs base 0706dbe6fb5c / upstream 381cf0c1e03f / F4 73323b9f3c43 — F3's flat-suppression state vs upstream drift; resolved by mechanical prune at merge time).

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (1)
  • coderabbit-review-active

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: 538d2d83-bc9c-43b7-9423-406b0510e246

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the has-conflicts PR has merge conflicts with the base branch label Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Review status

Thanks for contributing. This comment tracks the review sequence and the next action.

Current step: Fix the failing required CI checks; awaiting-maintainer requires CI and automated review completion.

Review-state labels are managed by this workflow; do not edit them manually.

…States

Each ClineProvider instance now owns a unique viewId (renderContext plus a
monotonic counter) and registers a stable viewStateId for durable persistence.

- Per-view state buffer (viewLocalState) holds mode / currentApiConfigName /
  apiConfiguration overrides in memory; saveViewState persists the non-secret
  subset durably under the active view id, rekeyed to the stable id on
  registration.
- viewStates is stored as a map pruned to the newest 50 entries; writes go
  through a serialized queue so concurrent provider instances merge without
  lost updates.
- setViewStateId sanitizes ids and rejects "__proto__" so a per-view entry can
  never be keyed through the Object.prototype setter.
- postMessageToWebview no longer awaits the webview ack: a remounted or
  disposed page never acknowledges, and awaiting would wedge task-critical
  callers.
- History restore falls back to the default mode view-locally instead of
  writing the shared global mode.
- GlobalState gains the "viewStates" key and GLOBAL_STATE_KEYS tracks it.

Adds F1a coverage in ClineProvider.spec.ts (viewId uniqueness, saveViewState
persistence semantics, loadViewState fallback and failure, pruning, the
__proto__ guard) and adapts the two history-restore tests in
ClineProvider.sticky-mode.spec.ts to the view-local restore. getState()
merging of hydrated per-view values and the remaining view-state suites land
in the follow-up (F1b).
…overrides

Fold ClineProvider viewLocalState on top of ContextProxy values in getState() (mode, apiConfiguration, and all per-view fields) so each webview reports its own selections while falling back to shared global state for everything else. Ports the getState-merging and local-state-isolation spec coverage from the superseded vps2 source.

Also pins the full default surface of the merged read path, including the apiConfiguration provider fill-in when provider settings sanitize the raw value away (mutation-diff gate).
… state

WMH webviewDidLaunch persists the webview view-state id via provider.setViewStateId and re-pins the view-local currentApiConfigName through provider.saveViewState when the view-local profile is missing but the shared global selection is still valid. updateSettings is routed through provider.setValue so view-local buffer and pin sync stay consistent with the other mutation paths. The webview VSCodeAPIWrapper gains a stable getViewStateId persisted via setState (with an in-memory fallback) and the launch effect posts the id with the webviewDidLaunch message.
Port vps2 F3 (mode/profile writes) from upstream 978, hunk-by-hunk against
the F1c..CS residual:

- ClineProvider: add repointPersistedViewStates() to re-point
  currentApiConfigName across per-view entries when a profile is renamed
  or replaced, and prune orphaned entries; validate handleModeSwitch slugs
  against the custom-modes manager and no-op with a log on unknown modes;
  drop the as-any cast in delegateParentAndOpenChild.
- Task: route mode switches through provider.handleModeSwitch(task) and
  keep the submitted message on failure instead of setMode().
- SwitchModeTool and specs: durable per-view mode writes.
- webviewMessageHandler: no change vs base - the kimi-code OAuth hunk in
  the residual is CS-only divergence (not-ported register item 1), not
  part of Zoo-Code-Org#978/Zoo-Code-Org#979.
- webviewMessageHandler.spec: ported only Zoo-Code-Org#979's 4 mock fields +
  defaultModeSlug import; the stack-side legacy-repair test, Key-aware
  getValue mock and em-dash comment are retained (register item 4).
  Zoo-Code-Org#979's mode-routing WMH.spec describe ("routes mode messages through
  handleModeSwitch instead of writing ContextProxy directly") exists in
  neither CS nor the stack and is ported by no unit (open question,
  logged).
- Tests: H3/H4 durable handleModeSwitch writes in ClineProvider.spec.ts;
  profile-mutation, profile-activation and handleModeSwitch-integration
  describes (incl. A4 non-focused-target regression and new
  mutation-killing tests) in ClineProvider.parallelMode.spec.ts;
  sticky-mode and webviewMessageHandler spec updates; retain the
  setViewStateId __proto__ guard + spec test - shipped F1a hardening;
  the residual's guard removal is lineage divergence, not F3 content.
- eslint-suppressions.json: no-explicit-any counts decrease for
  core/webview/ClineProvider.ts (12 -> 11) and
  core/webview/__tests__/ClineProvider.sticky-mode.spec.ts (36 -> 33).

Upstream: Zoo-Code-Org#978 (vps2 F3) - issue Zoo-Code-Org#978; content ported
hunk-by-hunk from the F1c..CS residual, cross-checked against upstream PR
…s import

- ClineProvider: new broadcastResetToAllInstances() clears each live instance's
  view-local cache and issues the single global contextProxy setValue("viewStates", undefined)
  write (single write-queue clear; no secrets involved, no prune-cap regression).
- resetState: awaits broadcastResetToAllInstances() before the final postStateToWebview
  so parallel tabs do not keep stale durable/in-memory per-view state.
- importExport: ImportWithProviderOptions.provider gains optional
  broadcastResetToAllInstances?(); importSettingsWithFeedback calls it in a guarded
  try/catch (log-only) after a successful import, so a failing broadcast never fails the import.
- importExport spec: 3 new tests (broadcast called when available / skipped when missing /
  import result preserved when broadcast throws, console.warn asserted; the skip test also
  asserts the broadcast-failure warn is NOT reached). Provider identifiers use
  providerIdentifiers.* per the zoo/no-raw-provider-identifiers rule (lint-required
  adaptation from Zoo-Code-Org#981's raw-string casts; no semantic change).
- parallelMode spec: appends the CS source-of-record describes (multi-instance isolation,
  _clearViewLocalState) — 5 new tests.
- ClineProvider spec: forward fix of the F3 resetState sentinel (F4's global viewStates
  clear removes the key; the F3-era toEqual({}) expectation is replaced by toBeUndefined())
  plus a new cross-instance resetState test pinning the multi-instance broadcast path
  (sibling view-local cache cleared; sibling and caller each post state exactly once).
- webviewMessageHandler.ts was NOT edited: the importSettings case already passes the full
  ClineProvider, which structurally satisfies the extended provider type and reaches the real
  broadcast method — Zoo-Code-Org#981's structural wrapper hunk is redundant in this stack.

Upstream: Zoo-Code-Org#980 / PR Zoo-Code-Org#981 (vps2 F4)
@easonLiangWorldedtech
easonLiangWorldedtech force-pushed the vps2/f4-cross-instance-reset branch from 80c147f to da97f98 Compare September 7, 2026 06:11
@codecov

codecov Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.91045% with 7 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/core/webview/ClineProvider.ts 98.38% 0 Missing and 4 partials ⚠️
webview-ui/src/utils/vscode.ts 86.36% 1 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

…/180s under runner load, 13 failing / 72 passing)
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