fix(webview): isolate parallel mode and provider profile writes (vps2 F3) - #1560
fix(webview): isolate parallel mode and provider profile writes (vps2 F3)#1560easonLiangWorldedtech wants to merge 30 commits into
Conversation
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
Review statusThanks for contributing. This comment tracks the review sequence and the next action. Current step: Resolve the merge conflicts. The review sequence resumes after the branch is mergeable. Review-state labels are managed by this workflow; do not edit them manually. |
8da5c6e to
7d91ff4
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
39423f2 to
510689f
Compare
510689f to
7f69ecf
Compare
…en view-identity tests Track the in-flight tab panel creation with a module-level promise so concurrent openClineInNewTab calls reuse one panel and provider (adds a Promise.all regression test). ClineProvider.spec sets the private view via the public resolveWebviewView() instead of a ts-ignore assignment. registerCommands.spec types evictCurrentTask/refreshWorkspace on the fixture and drops the as any attachment. eslint-suppressions: prune the registerCommands.spec.ts entry (two as any suppressions removed).
…n the concurrency assertion
…-bar posts - openClineInNewTab: extract the unserialized creation body into createTabPanelUnlocked and guard the in-flight slot clear so a settled creation cannot clobber a replacement already stored in the slot. - onDidDispose: clear the tracked tab ref only when the disposing panel is still the tracked one, so a late disposal of a replaced panel cannot clobber the replacement's ref. - MDM lookup failure: log the fallback to the output channel instead of swallowing it silently. - Route the six title-bar button handlers through a shared postActions helper that posts each action in order and logs failures with the handler-specific prefix. - package.json: add the four InTab commands to the command palette, scoped to the active tab panel. - Tests: handler-level regression for openInNewTab + popoutButtonClicked started before the first creation resolves; fresh-creation test for a settled in-flight promise; stale-panel disposal regression; retained panel assertion for disposed tab instances; rightmost-editor column placement assertion; MDM fallback output assertion; %s placeholders for primitive it.each titles. - Stryker directives for the two equivalent setPanel type-literal mutants (setPanel branches only on type === sidebar).
7f69ecf to
f9233a4
Compare
Replace the weak toBeDefined() assertion in the dispose spec with an identity check against the panel returned during creation, per the CodeRabbit actionable comment on this PR (review run 7c4cfeb3-6dd9-4615- 9a58-70cfc705eca2). The tracked tab is now pinned with toBe(panel) before the dispose assertions, so a wrong or duplicated tracked panel fails the suite instead of passing a defined-only check. Upstream: Zoo-Code-Org#1528 (vps2 F0)
cac941f to
210899f
Compare
Retain the tracked tab panel in the InTab handler cases and assert that getInstanceForView was called with that exact panel, per the CodeRabbit actionable comment on this PR (review run 4afe1273-8739-4235-90d3-311db5f6ccb9, inline comment 3952466254 on the tabHandlerCases spec). A handler resolving any other view now fails instead of passing on the stubbed provider result alone; the same identity pin is applied to plusButtonClickedInTab. Upstream: Zoo-Code-Org#1528 (vps2 F0)
…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).
…lude viewStates from settings transfer
210899f to
8576b18
Compare
… describe by subject Tie-case pruning and editor-teardown retention now assert the full survivor objects instead of mere presence, and the durable viewStates persistence tests move out of the getState merging block into a persisted view state describe so each block name matches its subject.
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
Route the CS API setConfiguration write through ClineProvider.setValues instead of the raw context proxy so the view-local subset stays in sync with the sidebar view's state. A direct context-proxy write leaves the view-local apiConfiguration stale; the mode-switch / profile-activation paths re-apply the current provider settings to the focused task on every user message, so a task started after an API setConfiguration runs with the stale configuration (wrong model / reasoning flags / endpoint). That is the deterministic 13-test e2e provider-suite failure (72/13) on this head: the provider suites configure the provider via setConfiguration and the mock serves fixtures by the model the request actually carries. Ported from the F5 head (vps2 F5) where the e2e provider suites pass; includes the unit spec asserting the routing. Upstream: Zoo-Code-Org#982 (vps2 F3)
… mocks API.setConfiguration now routes through ClineProvider.setValues (8576b18 "fix(api): route setConfiguration through ClineProvider.setValues"), but the pre-existing spec still shaped the provider mock around the old contextProxy.setValues call. Both tests in this file failed with "this.sidebarProvider.setValues is not a function" on the platform-unit-test runs (F3 Zoo-Code-Org#1560 windows unit, job 101863508169, run 34161293841). Add the provider-level setValues mock to both tests to match the new routing. Upstream: Zoo-Code-Org#1560 (vps2 F3)
The F1a buffer routing and the new delete-time activation branch interacted in two ways: the unrelated-pin branch re-wrote currentApiConfigName through setValue although the view-local buffer already holds the surviving pin (drop the write: it only triggered a viewStates prune pass and could clobber the pin with the shared slot value), and deleting the current profile now activates the fallback, which reads its settings via providerSettingsManager, so the spec test uses a test double like its siblings in the describe.
…tion mutations - E1: move the provider-release switch test into switchModeTool.spec where the switchModeTool layer is the narrowest proof - E2: assert the state postMessage shape in the mode-lock switch test - E3: prune invalid persisted view-state entries before applying the cap - wrap the upsert and activate buffer-refresh calls so the view-local apiConfiguration buffer tracks the activated profile again; flat provider-settings keys no longer merge into the buffer
b80d59a to
065116c
Compare
Draft PR — vps2 unit F3 (mode/profile writes).
Tracking: easonLiangWorldedtech#41 (vps2 series ledger). Upstream issue: #1559 (this series' gap record; the original upstream bug is #978). Port source: the F1c-head-to-CS residual, cross-checked against upstream PR #979 (fix(webview): isolate parallel mode and provider profile writes) — closed/superseded by the vps2 series; the #41 ledger names #979 as the F3 port source.
Scope
10 files, 919 insertions, 65 deletions (measured vs stack base aa0f3b1):
Budget
Mutation gate trust note
Port fidelity (coordinator-verified against the vps2 CS e9a44b2)
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), and the F4 multi-instance describes (L1675-1790). 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 (they are absent from the CS parallelMode file). Both placements are behaviorally covered; the divergence is structural, not a coverage gap.
Series mechanics