Skip to content

test(e2e): port view-state suite with reload and rehydration coverage (vps2 F6) - #1565

Draft
easonLiangWorldedtech wants to merge 13 commits into
Zoo-Code-Org:mainfrom
easonLiangWorldedtech:vps2/f6-e2e-viewstate
Draft

test(e2e): port view-state suite with reload and rehydration coverage (vps2 F6)#1565
easonLiangWorldedtech wants to merge 13 commits into
Zoo-Code-Org:mainfrom
easonLiangWorldedtech:vps2/f6-e2e-viewstate

Conversation

@easonLiangWorldedtech

Copy link
Copy Markdown
Contributor

Tracking: easonLiangWorldedtech#41 (vps2 series ledger). Upstream issue: #1564 (this series' gap record; the review lineage is #929, A5 re-review minor 2). Port source: the CS worktree apps/vscode-e2e (e9a44b2, the A5 re-review state) — 3 files byte-identical to CS; the reload test is F6-authored.

Scope

4 files, 543 insertions, 0 deletions (measured vs stack base 95a18b2); all under apps/vscode-e2e/ — no production code:

  • apps/vscode-e2e/fixtures/modes.json (14+/0-) — the debug-mode switch_mode fixture entry (userMessage match + tool call call_modes_switch_002) used by the suite's ten-rounds test.
  • apps/vscode-e2e/src/runTest.ts (37+/0-) — the addViewStateFixtures wiring + the two model-agnostic post-switch predicate fixtures (toolResultContains on call_modes_post_switch_001/002 → attempt_completion "Switched to ... mode as requested.").
  • apps/vscode-e2e/src/fixtures/view-state.ts (95+/0-) — the per-task marker fixtures (addViewStateFixtures) + getFollowupModeIsolationPlan (the three-panel, ten-staggered-rounds plan).
  • apps/vscode-e2e/src/suite/view-state.test.ts (397+/0-) — the ported suite "Roo Code View State" (2 CS tests, byte-identical) + the F6-authored sidebar webview reload/rehydration test (the only non-verbatim content in this PR).

Gate evidence

  • Full solo e2e gate (two runs; USE_MOCK=true; from apps/vscode-e2e after pnpm -w bundle + the @roo-code/vscode-webview build): run 1: 70 passing / 6 failing — the sixth failure was the authored reload test, which deterministically hit command-not-found on workbench.action.toggleLeftSidebarVisibility (an ID absent from the pinned VS Code 1.100.0 bundle); the other five were the load-class tool-suite timeouts detailed at the end of this line. After the 2-line command-ID fix, run 2: 71 passing / 5 failing — all 3 view-state tests pass by name: 'sidebar and tab panel keep mode isolated through the real ContextProxy singleton' (10025ms), 'three panels keep follow-up option mode switches isolated across ten staggered rounds' (31735ms), 'sidebar webview reload rehydrates the durable per-view mode' (13004ms). The 5 failures are byte-identical across both runs (same suites, same tests, same durations, all from the out/suite/utils.js wait helper): the 'Should execute simple echo command' (60s), 'Should execute command with custom working directory' (60s), 'Should execute multiple commands sequentially' (90s), and 'Should handle long-running commands' (60s) tests of the Roo Code execute_command Tool suite, plus the 'before all' hook (30s) of the Roo Code use_mcp_tool Tool — suites this diff does not touch; this class is documented in the [BUG] GPT-5.5 Codex uses incorrect context window #41 ledger (run A: 7 concurrent-load timeouts on the same machine). Both full run logs are kept as evidence of record.
  • check-types (turbo, includes @roo-code/vscode-e2e tsc -p tsconfig.esm.json --noEmit): exit 0.
  • e2e app lint (eslint src --ext=ts --max-warnings=0): exit 0.
  • prettier --check --end-of-line=auto on the 4 files: clean.
  • stryker-diff: vacuous — all 4 files under apps/vscode-e2e are outside the mutation-tested package set; the gate reports "No changed executable lines in mutation-tested packages; mutation testing is not applicable." and exits 0.
  • a+d 543 vs the 400-soft / 1000-hard budget: soft-over (the ported suite is 309 lines verbatim plus the authored test), hard-under.

Port fidelity

  • The 3 ported files are byte-identical to the CS blobs (git hash-object verified at cut and re-apply): modes.json f38634d0bf13ef3e5ffbcb392701779890e40617; runTest.ts ab4c0cbe29a465b57989a4d9dd1e582c1a768d59; view-state.ts 4225e341aa235885af4ee579106f5840d6c67f08.
  • The authored test (A5 minor 2): establishes the sidebar view's durable mode "ask" through a real switch_mode task, polls the persisted viewStates for the ask entry (30s budget), then hides the primary sidebar with workbench.action.toggleSidebarVisibility (2s settle), shows it again with the same command (2s settle) plus a 5s rehydration settle, and asserts (a) the persisted viewStates still contain the per-view entry with mode "ask" and (b) api.getConfiguration().mode === "ask". Fresh-task mode inheritance is deliberately skipped at the test site with a recorded rationale (no deterministic extension-side getter; inheritance is exercised by the two isolation tests through per-task TaskModeSwitched events and the persisted per-view entries). Command-ID note: the first e2e run used workbench.action.toggleLeftSidebarVisibility and the test failed deterministically with a command-not-found error — that ID has 0 occurrences in the pinned VS Code 1.100.0 workbench bundle, while workbench.action.toggleSidebarVisibility (and the Toggle Primary Side Bar Visibility command title) is present; the fix swaps the 2 command strings only, and the assertions are byte-identical before and after.
  • Root-cause note: at the pre-stack F5 head this suite's test 1 failed deterministically ('code' !== 'ask' — the tool-side mode switch wrote the global-only mode while the sidebar's viewLocalState masked it). The F3 per-view writes and the F4 cross-instance reset (stacked below) close the gap; the ported suite is byte-identical here (no timing or assertion changes), and the authored reload test carries only the 2-line command-ID fix above.
  • FLAG (not fixed in this unit): the apps/vscode-e2e test:ci / test:ci:mock scripts use the POSIX env prefix "USE_MOCK=true pnpm test:run", which is broken on Windows; the working invocation is the $env:USE_MOCK='true' + pnpm test:run form used for the gate run.

Series mechanics

  • Base of record: upstream/main @ 0d937c0; PR base is main; the branch is stacked on the re-stacked F5 head 95a18b2 (the F1-series, F2, F3, F4, F5 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-F6 stack: tree ee7ca39f267b3ee4fa416e65852921bf24251b43; 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 / F6 73323b9f3c43 — identical profile to the F5 merge check; F6 touches no src/ file; 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: 2dbc99ac-ac3e-4fb3-ae0a-656ab8bc63bb

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 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: Wait for 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)
Ports the vps2 CS API wiring onto the F4 head 80c147f:

- startNewTask(newTab, preserveOpenTabs): skips editor revert/close-all
  when preserveOpenTabs is set
- task ask registry: approveTaskAsk + selectTaskFollowupSuggestion
  (per-provider mode validation; a failed mode switch does not swallow
  the follow-up answer; a stale instance's teardown cannot evict its
  replacement)
- setConfiguration routes through ClineProvider.setValues so the
  view-local subset stays in sync with the sidebar view's state
- getConfiguration flattens the nested view-local apiConfiguration and
  strips secrets before returning
- getGlobalState read surface (test-only)
- docs: setConfiguration JSDoc now states writes target the
  extension-host (sidebar) view (parked A4 major, documented
  limitation); @PARAM note added for preserveOpenTabs
- specs: api-task-control (12 tests), api-set-configuration (1),
  api-configuration getConfiguration flatten/strip-secrets (1)

Upstream: Zoo-Code-Org#982 (vps2 F5)
Port source: CS e9a44b2 apps/vscode-e2e (A5 Zoo-Code-Org#929 re-review series) — 3 files byte-identical, 2 suite tests
F6-authored: sidebar webview reload/rehydration test (A5 minor 2) — the durable per-view mode survives a sidebar hide/show cycle
@github-actions github-actions Bot removed the has-conflicts PR has merge conflicts with the base branch label Sep 7, 2026
@codecov

codecov Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.56757% with 9 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 ⚠️
src/extension/api.ts 94.28% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

…ing the tracked one

The F0 single-tab reuse in openClineInNewTab made the 2nd/3rd
startNewTask({newTab:true}) return the first tab's provider, so its
evictCurrentTask() aborted the live task that tab was serving (TaskUnfocused
-> removeRegisteredTask, breaking follow-up suggestion delivery) and
re-registering listeners on the reused provider stacked duplicate task
handlers (Nth task events re-emitted N times).

Extract the creation body as createClineTabPanel (always creates a fresh
panel + provider), point API.startNewTask at it, keep the UI button path on
openClineInNewTab's reuse behavior, guard registerListeners with a
per-provider set, and only clear the tracked tab ref on dispose when it
still points at the disposed panel.

Restores the 3-panel follow-up coordination contract; the e2e 3-panel
coordinated-rounds test that failed 6/6 now passes locally.
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