Repository navigation
canvases: refactor: share definition refresh logic across windows - #341826
Merged
Ulugbek Abdullaev (ulugbekna) merged 1 commit intoOct 15, 2026
Merged
Ulugbek Abdullaev (ulugbekna) merged 1 commit into
Ulugbek Abdullaev (ulugbekna) merged 1 commit into
Conversation
The Editor window Canvas menu and the Agents window Add Tab > Canvas menu each kept their own canvas-definition refresh loop, and the two had drifted apart in owner identity, refresh coalescing, owner-switch latency, stale-request handling, and failure behavior. Move the shared lifecycle into a Workbench-owned CanvasDefinitionStore that both windows consume, while each window keeps its own chat routing, menus, and open actions. Refreshes keep the current list so open menus keep working commands, results apply in order per chat, a new chat is listed immediately, stale results are dropped, and a failed refresh keeps the last list. Fixes #341696 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Refresh races can restore obsolete definitions or delay the latest refresh behind an outdated source.
2 open findings
What changed in this PR
Shares canvas-definition refresh logic between the Editor and Agents windows while keeping menu placement and canvas-opening behavior separate.
Changes:
- Adds a shared observable store for refresh scheduling, filtering, ordering, and failure handling.
- Replaces both window-specific refresh implementations with adapters.
- Adds focused lifecycle and refresh regression tests.
| File | Description |
|---|---|
| src/vs/workbench/contrib/canvases/test/common/canvasDefinitionStore.test.ts | Tests shared refresh behavior. |
| src/vs/workbench/contrib/canvases/common/canvasDefinitionStore.ts | Implements the shared definition store. |
| src/vs/workbench/contrib/canvases/browser/editorCanvasActions.ts | Adapts Editor menus to the store. |
| src/vs/sessions/contrib/canvases/electron-browser/sessionCanvasService.ts | Adapts Agents registry to the store. |
🧠 Review effort: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| this.loaded.set(undefined, undefined); | ||
| } | ||
| if (next) { | ||
| this.refreshDelayer.trigger(() => this.refreshes.queue(next.ownerKey, () => this.refresh(next))).catch(error => { |
| } | ||
|
|
||
| private isCurrent(request: ICanvasDefinitionRequest): boolean { | ||
| return !this._store.isDisposed && request.ownerKey === this.ownerKey; |
Henning Dieterichs (hediet)
approved these changes
Oct 15, 2026
Ulugbek Abdullaev (ulugbekna)
deleted the
ulugbekna/agents/investigate-vscode-issue-341696
branch
October 15, 2026 17:31
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
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.
Fixes #341696
The Editor window New Chat › Canvas menu (#341597) and the Agents window Add Tab › Canvas menu (#340575) each had their own loop for refreshing the canvases registered in the active chat. The two had drifted apart:
ICanvasContextobject identity; a new object for the same chat removed the menu's commandsChanges
CanvasDefinitionStoreinvs/workbench/contrib/canvases/common. Each window passes a callback that returns its active chat (ICanvasOwner) and a way to list that chat's canvases; anything the callback reads is a refresh trigger. The store exposes the openable definitions in menu order and:SessionCanvasRegistryService(Agents window) andregisterEditorCanvasActions(Editor window) now only supply their chat routing to the store. Menu contributions, command registration, ordering and opening canvases are unchanged.User-visible behavior is unchanged, except that a failed refresh in the Agents window now keeps the Canvas submenu instead of emptying it, matching the Editor window.
Follow-up, not in this PR: the Agents window still refreshes on every
chat.canvasesemission. Switching it to the dedicated canvas-registry signal the Editor window uses would cut list calls to the agent host, but it needs "chat is ready" gating or a retry, as in #341701.Validation
node build/next/index.ts transpile— passed. (npm run build-fast -- --client-onlystops before compiling in this worktree because its sharednode_moduleshas an older@github/copilot-sdkthanpackage.jsondeclares.)./scripts/test.sh --reporter dot --runGlob "**/canvases/test/**/*.test.js"— passed, 113 tests: every canvas test invs/workbenchandvs/sessions, including the existing Editor and Agents window menu tests, unchanged.npm run eslint -- src/vs/workbench/contrib/canvases/common/canvasDefinitionStore.ts src/vs/workbench/contrib/canvases/test/common/canvasDefinitionStore.test.ts src/vs/workbench/contrib/canvases/browser/editorCanvasActions.ts src/vs/sessions/contrib/canvases/electron-browser/sessionCanvasService.ts— passed.npm run typecheck-client— failed (exit 1) with 20 errors, all insrc/vs/platform/agentHost/**from the older@github/copilot-sdkin this worktree'snode_modules; none in the changed files.npm run valid-layers-check— failed (exit 1) on the same 20src/vs/platform/agentHost/**type errors after the layer checker itself passed.Not run: manual validation in a Dev build, and the full unit and integration suites.