Merge #977 (durable per-view state base) into the file-write-safety trial branch - #34
Open
easonLiangWorldedtech wants to merge 33 commits into
Open
Merge #977 (durable per-view state base) into the file-write-safety trial branch#34easonLiangWorldedtech wants to merge 33 commits into
easonLiangWorldedtech wants to merge 33 commits into
Conversation
A just-resolved globalState write can momentarily lag a synchronous globalState.get in the extension host. The per-view writes are already awaited through the serialized view-state write queue before the tasks complete, so poll until both the sidebar and tab persisted selections are visible before asserting, instead of reading globalState once.
Address the CodeRabbit docstring coverage warning on the durable per-view state PR by documenting the new view-state persistence/merge helpers in ClineProvider, the task-scoped API controls in the extension API, and the viewStateId generation/restoration helpers in the webview wrapper.
# Conflicts: # src/core/webview/ClineProvider.ts # src/core/webview/__tests__/webviewMessageHandler.routerModels.spec.ts # src/core/webview/__tests__/webviewMessageHandler.spec.ts # src/core/webview/webviewMessageHandler.ts
…sted view-local secrets - selectTaskFollowupSuggestion() now passes the registered task explicitly to handleModeSwitch(), so answering a follow-up on task B no longer switches the mode of the provider's currently focused task A (resolves review comment on src/extension/api.ts). - getConfiguration() flattens the nested view-local apiConfiguration onto the top level before the isSecretStateKey() filter, so nested provider secrets (apiKey, openRouterApiKey, ...) cannot leak through the API — a regression introduced by the per-view state base's nested apiConfiguration shape. - Consolidate the duplicate kimi-code oauth vi.mock in the routerModels spec and replace raw provider identifier literals flagged by the merged zoo/no-raw-provider-identifiers rule with providerIdentifiers.* constants. Validated: 112 targeted vitest tests pass, pnpm --dir src run check-types clean, eslint --max-warnings=0 clean on all touched files.
The .husky/_ directory contains husky-generated internal shims that were committed by accident. Ignore the directory and untrack the existing shims so they no longer show up as modified files on every install.
- Scope handleModeSwitch to the target task: switches for non-focused tasks no longer rewrite the view's durable mode pin, emit ModeChanged, or activate provider profiles. - History restore persists the mode through the view's own pin instead of the shared global mode setting. - loadViewState discards stale results when a newer view id is registered during the load, and pre-launch temporary view ids never write durable entries (orphan prevention). - deleteProviderProfile re-points persisted view pins that referenced the deleted profile, and only overwrites this view's in-memory profile pin when it actually pinned the deleted profile. - resetState also clears this view's persisted entry. - saveViewState is now public and fully typed (no explicit any left in the provider). - Mock ContextProxy mirrors the real state cache so tests can exercise stale-cache and fresh-read behavior; add regression tests for all of the above.
…iable - removeRegisteredTask only drops the registration when the stored controller is the same instance, so a replaced task reusing a taskId is not torn down by the old instance's abort/unfocus events. - selectTaskFollowupSuggestion delivers the answer even when the follow-up mode switch fails, logging the failure instead of losing the user's response.
…selection - webviewDidLaunch rescue: when the view's own pin is invalid, re-pin the view to the shared global selection if it is still valid instead of overwriting the global setting and activating a global profile from one view's launch path. - updateSettings persists provider settings through the provider-level setValue so the durable write path is the one the provider serializes. - Lower the recorded no-explicit-any suppression counts for the touched files (fixes, not new suppressions).
…red fixtures - view-state.test.ts derives the round count from the follow-up isolation fixture instead of a hardcoded 10, and drops an unused map. - Document the new mode-switch predicate fixtures alongside the legacy model-scoped fixtures in runTest.ts.
- The webviewDidLaunch describe now assigns its runtime members through a structural LaunchProviderFixture cast instead of per-line as-any, and the getState mock return is typed against the provider signature. - Drops the webviewMessageHandler.spec.ts suppression count back to the PR base level (35).
…ion spec Use typed structural casts (ClineProvider / OutputChannel) for the API double and remove the now-empty suppression entry for the file.
CI e2e-mock regression: the mode switch inside a task lands before the tab webview's launch message registers its stable viewStateId, so the ephemeral-skip silently dropped the view's durable mode write and the "sidebar and tab panel keep mode isolated" e2e timed out waiting for the persisted entries. Pre-launch writes now persist under the temporary view id and are re-keyed to the stable id when the webview registers it (setViewStateId runs the re-key through the serialized write queue before loadViewState). A pre-existing stable entry wins and the temporary entry is dropped, since temporary ids are session counters that can collide across window reloads. The stale-load guard is unchanged. Unit tests: the ephemeral-skip assertion is replaced with re-key tests (write under temporary id, re-key on registration, stable entry wins) and the stale-load test is restructured so it genuinely exercises the guard through the cached read path.
…elMode spec Address the CodeRabbit maintainability finding: drop the blanket no-explicit-any suppression (137) for ClineProvider.parallelMode.spec.ts and type the spec properly instead. - Private member access moves from (provider as any).x to bracket notation (provider["x"]); public members (saveViewState, setValue, setValues, handleModeSwitch, resolveWebviewView, log) drop the cast entirely and keep their native generics. - MockContextProxy now takes vscode.ExtensionContext; memento and mock callbacks use unknown instead of any; the webview structural double is cast once as unknown as vscode.WebviewView. - Key/value casts are removed where the key is a valid RooCodeSettings key; the one genuine exception (apiConfiguration is a GlobalState key outside the proxy's generic) keeps a documented double assertion. - api-configuration.spec.ts: document the as-unknown-as-ClineProvider structural double in the new test (API.getConfiguration only reads sidebarProvider.getValues). check-types clean; parallelMode 49/49 and api-configuration 3/3 green; eslint --prune-suppressions clean with the parallelMode entry removed from eslint-suppressions.json and every other count unchanged.
… per-view state base) for combined trial
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR merges the head of upstream PR #977 ("fix(webview): add durable per-view state base",
easonLiangWorldedtech:feat/view-local-state-base@f91e19c03) into the file-write-safety (FWS) trial branchfeat/fws-trial-all.What is Zoo-Code-Org#977
PR Zoo-Code-Org#977 introduces a durable per-view state base for the webview: each webview instance gets a stable view id, and non-secret selections (mode, current API config) are persisted per view in registered global state, so parallel/stacked panels keep isolated selections. It adds a
viewStateSchemaplus aviewStatesrecord to the global settings schema, view-state load/save plumbing inClineProvider(setViewStateId/saveViewStateand theviewLocalStatemerge ingetState),viewStateIdrouting on thewebviewDidLaunchmessage, a repair path for invalid view pins that does not clobber the global config selection, and test-only API surfaces (approveTaskAsk,selectTaskFollowupSuggestion,getGlobalState,preserveOpenTabs) with matching unit and e2e coverage.Why it is included here
The FWS trial branch is a stacked trial of the file-write-safety series (change cards, rollback service, per-write checkpoints, checkpoint settings, 18-locale i18n). Including Zoo-Code-Org#977 here produces a combined trial of both series in one branch so reviewers can test the FWS features together with the per-view state base and flag any interaction early, instead of waiting for sequential merges.
Conflict resolution
Three files conflicted; all were resolved by keeping both sides:
packages/types/src/global-settings.ts— both sides added new declarations at the same location: kept the FWS defaults (DEFAULT_PER_WRITE_CHECKPOINTS,DEFAULT_CHANGE_CARD_DETAIL) and the fix(webview): add durable per-view state base Zoo-Code-Org/Zoo-Code#977viewStateSchema(referenced by the auto-mergedviewStatesfield ofglobalSettingsSchema).src/core/webview/ClineProvider.ts— thegetStatereturn object: fix(webview): add durable per-view state base Zoo-Code-Org/Zoo-Code#977 switched these keys fromstateValuestomergedStateValues(global state with the per-view overlay on top); kept the fix(webview): add durable per-view state base Zoo-Code-Org/Zoo-Code#977mergedStateValuessource for all shared keys and added the two FWS fields (perWriteCheckpoints,changeCardDetail) read from the same merged source.webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx— import block: kept the FWS four-symbol import (includingcreateInitialExtensionState) plus the fix(webview): add durable per-view state base Zoo-Code-Org/Zoo-Code#977@src/utils/vscodemock; the merged spec body uses symbols from both sides.The remaining 24 changed files auto-merged cleanly (webview state context, message handler, API types, e2e fixtures, new spec files).
Local gate results
tsc --noEmit: src exit 0; webview-ui exit 0eslint --max-warnings=0on every merge-touched file: src (11 files) exit 0; webview-ui (6 files) exit 0; packages/types (4 files) exit 0. The net change tosrc/eslint-suppressions.jsonis a single count reduction (37 -> 36, carried by fix(webview): add durable per-view state base Zoo-Code-Org/Zoo-Code#977 itself); no suppressions were added.vitest run(webview-ui): Test Files 163 passed (163); Tests 1794 passed (1794)vitest run(src): Test Files 475 passed | 1 failed | 3 skipped (481); Tests 8016 passed | 5 failed | 38 skipped (8059). The single failing file isservices/rules/__tests__/rules.spec.ts(5 tests) — allfs.symlinkEPERMerrors specific to this Windows host; the spec and the rules service are byte-identical to the trial base and untouched by this merge.node scripts/find-missing-translations.js: exit 0 — all 17 non-English locales complete across backend, frontend, andpackage.nls(fix(webview): add durable per-view state base Zoo-Code-Org/Zoo-Code#977 added no new i18n keys).Note
Merging this PR updates
feat/fws-trial-all(the head branch of upstream trial PR Zoo-Code-Org#1413), so that upstream trial PR picks up the combined contents automatically.