Skip to content

fix(extensions): re-run the restart-required query when the Show link is clicked again - #339889

Open
Irish Joseph (Irish-Joseph) wants to merge 1 commit into
microsoft:mainfrom
Irish-Joseph:fix/extensions-restartrequired-show-link
Open

Irish Joseph (Irish-Joseph) wants to merge 1 commit into
microsoft:mainfrom
Irish-Joseph:fix/extensions-restartrequired-show-link

Conversation

@Irish-Joseph

Copy link
Copy Markdown

Description

Fixes #321178

When extensions are updated in the background, the Extensions view shows a
banner like "2 extensions require a restart to take effect. Show". Clicking
Show filters the list with the @restartrequired query.

After more extensions are updated, the banner increments (e.g. to 4), but
the list still shows only the original 2. Clicking Show again does nothing,
and only clearing the filter and clicking Show once more reveals all 4
extensions (as reported in the issue, still reproducible on 1.136.1).

Root cause — ExtensionsViewPaneContainer.search() only sets the search
box value when it differs from the current one:

search(value: string): void {
    if (this.searchBox && this.searchBox.getValue() !== value) {
        this.searchBox.setValue(value);
    }
}

The first Show click puts @restartrequired into the search box, which
triggers the search. A second click requests the same value, so nothing is
set and no search is re-run — the list keeps the stale result.

Fix

  • search() accepts an optional refresh flag: when the search box already
    holds the requested value and refresh is set, the query is re-run
    (doSearch(true)) instead of being a no-op.
  • The Show link (mouse click and Enter/Space key handling) passes
    refresh: true.
  • The refresh flag is threaded through doSearch() →
    showExtensionsViews() → ExtensionsListView.show(query, refresh), which
    discards the previous query result (including its change watcher) and
    re-queries. (Previously the flag was dropped, so the list view's existing
    refresh support was unreachable from the viewlet.)

Testing

Added extensionsRestartRequiredView.test.ts which drives the real
ExtensionsWorkbenchService + ExtensionsListView and the
ExtensionsViewPaneContainer.search() entry point, simulating extension
updates that are installed while the previous version is still running (the
exact condition that puts extensions into the restart-required state):

  • search() with refresh re-runs an already active query (#321178) — the
    regression test. Fails without the fix (the re-click never re-runs the
    search) and passes with it.

  • re-running the same query with refresh picks up newly restart-required extensions — the list view re-query contract the fix
    relies on (1 extension → update → re-run with refresh → 2 extensions).

  • the @restartrequired list updates when more extensions require a restart
    — the existing change-watcher live-update path still works.

  • The full extensionsViews.test.ts (141 tests) and
    extensionsWorkbenchService.test.ts (214 tests) suites still pass.

  • The touched files type-check cleanly under the repository's strict
    TypeScript settings.

… is clicked again

Clicking the 'Show' link on the 'extensions require a restart' banner sets the
@restartrequired query in the search box. When more extensions subsequently
require a restart, clicking 'Show' again was a no-op because the query string
had not changed, so the list kept showing the stale set of extensions.

search() now accepts an optional refresh flag that re-runs the query even when
the search box already holds the same value, and the 'Show' link (mouse and
keyboard) passes it. The refresh flag is also threaded through doSearch() and
showExtensionsViews() to the list view's show(), so the re-query actually
discards the previous result and its change watcher.

Fixes microsoft#321178
Copilot AI balanced review requested due to automatic review settings October 5, 2026 19:47
@Irish-Joseph

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The test fixture uses the wrong install operation and does not verify refresh propagation through the container.

Review effort: Balanced
Findings: 2 Medium severity

Open (2)
What changed in this PR

Fixes stale @restartrequired extension results when users click Show repeatedly.

Changes:

  • Adds refresh-aware extension searches.
  • Propagates refresh requests to extension list views.
  • Adds restart-required regression tests.
File Description
extensionsRestartRequiredView.test.ts Tests restart-required refresh behavior.
extensions.ts Extends the search API with a refresh flag.
extensionsViewlet.ts Re-runs and propagates same-query refreshes.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

function installResult(local: ILocalExtension): InstallExtensionResult {
return {
identifier: local.identifier,
operation: 1, // InstallOperation.Update
Comment on lines +268 to +272
const doSearchStub = sinon.stub(viewlet as unknown as { doSearch: (refresh?: boolean) => Promise<void> }, 'doSearch');
try {
// Clicking "Show" while the same query is active must re-run the search...
viewlet.search('@restartrequired', true);
assert.ok(doSearchStub.calledOnceWith(true));

This branch has not been deployed

No deployments
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.

@restartrequired tag "Show" link doesn't refresh when new extensions are updated

4 participants