Skip to content

Fix issue reporter wizard dropping preset extension data#320103

Merged
ulugbekna merged 1 commit into
microsoft:mainfrom
ulugbekna:fix/issue-reporter-preset-data-320101
Jun 5, 2026
Merged

Fix issue reporter wizard dropping preset extension data#320103
ulugbekna merged 1 commit into
microsoft:mainfrom
ulugbekna:fix/issue-reporter-preset-data-320101

Conversation

@ulugbekna
Copy link
Copy Markdown
Contributor

Fixes #320101

The new IssueReporterOverlay wizard dropped the data payload provided via the workbench.action.openIssueReporter command when an extensionId was also preset. This broke callers like Copilot's NES feedback flow, which attaches an STest and recording context via the data that context was silently lost from the resulting issue body.field

Root cause

The constructor calls updateSelectedExtension(extensionId, /* loadExtensionData */ false). With loadExtensionData=false, the existing applyExtensionIssueData path that propagates data onto the selected extension and into the model (extensionData + includeExtensionData) was never invoked. The old reporter (baseIssueReporterService.updateExtensionStatus) did this propagation; the new wizard didn't.

Fix

In updateSelectedExtension, when loadExtensionData=false but the caller-supplied this.data carries data / uri / privateUri, apply it via the same helper. Guarded by !extension.data so it doesn't re-apply when updateExtensionOptions re-invokes updateSelectedExtension after updateModel repopulates allExtensions (covers the case where the extension list arrives asynchronously after the wizard is already built).

User-driven selections from the dropdown / source buttons are they call updateSelectedExtension with the default loadExtensionData=true, taking the existing if branch.unaffected

cc @Giuspepe

Copilot AI review requested due to automatic review settings June 5, 2026 13:22
@ulugbekna ulugbekna self-assigned this Jun 5, 2026
@ulugbekna ulugbekna requested a review from Giuspepe June 5, 2026 13:22
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a regression in the new wizard-based issue reporter (IssueReporterOverlay) where programmatic invocations of workbench.action.openIssueReporter with a preset extensionId would drop the caller-provided extension data / uri / privateUri, causing that context to be missing from the generated issue body (e.g. Copilot NES feedback flow).

Changes:

  • When updateSelectedExtension(..., /* loadExtensionData */ false) is used, propagate caller-supplied data / uri / privateUri onto the selected extension and into the model via the existing applyExtensionIssueData helper.
  • Add a guard intended to avoid re-applying that payload when the extension list is later repopulated asynchronously.
Show a summary per file
File Description
src/vs/workbench/contrib/issue/browser/issueReporterOverlay.ts Ensures preset extension payload (data/uri/privateUri) is not dropped when the wizard is opened programmatically with loadExtensionData=false.

Copilot's findings

  • Files reviewed: 1/1 changed files
  • Comments generated: 1

if (issueData) {
this.applyExtensionIssueData(extension, issueData);
}
} else if (!extension.data && (this.data.data !== undefined || this.data.uri !== undefined || this.data.privateUri !== undefined)) {
@ulugbekna ulugbekna force-pushed the fix/issue-reporter-preset-data-320101 branch from 8d87b16 to 3c97415 Compare June 5, 2026 13:32
@ulugbekna
Copy link
Copy Markdown
Contributor Author

Force-pushed a correction.

The previous version guarded with !extension.data, but issueService.ts pre-populates extension.data on every entry in enabledExtensions (data: options.data), so my guard was always false and the new branch never the bug remained in practice.fired

Switched the guard to !this.includeExtensionData, which is the actual "extension data has been applied to the model" flag (initialized to false, flipped to true only by applyExtensionIssueData). This correctly fires on first resolution of the preset extension and still short-circuits on re-invocation after the extension list arrives asynchronously.

@ulugbekna ulugbekna force-pushed the fix/issue-reporter-preset-data-320101 branch from 3c97415 to cc9e7e1 Compare June 5, 2026 13:36
@ulugbekna
Copy link
Copy Markdown
Contributor Author

Found a deeper bug. Force-pushed.

When the wizard opens, the constructor calls updateSelectedExtension(extensionId, false) before populateReporterDataAsync has finished filling allExtensions. With an empty list, find() returns undefined, and two lines then clobber the requested extensionId to undefined:

  • updateSelectedExtension line 796: this.data.extensionId = extension?.id;
  • updateIssueSourceFlags line 733: this.data.extensionId = fileOnExtension ? this.selectedExtension?.id : undefined;

Once cleared, the catch-up retry in updateExtensionOptions (line 776: if (!this.selectedExtension && this.data.extensionId)) never fires when extensions finally load, so my new propagation branch never gets a chance to run. The preset data is lost regardless of any guard logic.

Fix: preserve the originally requested extensionId in both places when the extension list hasn't resolved yet. With this, the catch-up retry runs as designed and the new branch propagates data into the model.

The new IssueReporterOverlay wizard dropped the `data` payload provided via
`workbench.action.openIssueReporter` when an `extensionId` was also preset.
This broke callers like Copilot's NES feedback flow, which attaches an STest
and recording context via the `data`  that context was silently lostfield
from the resulting issue body.

The constructor calls `updateSelectedExtension(extensionId, /* loadExtensionData */ false)`.
With `loadExtensionData=false`, the existing `applyExtensionIssueData` path
that propagates `data` onto the selected extension and into the model
(`extensionData` + `includeExtensionData`) was never invoked.

Fix: in `updateSelectedExtension`, when `loadExtensionData=false` but the
caller-supplied `this.data` carries `data`/`uri`/`privateUri`, apply it
when `updateExtensionOptions` re-invokes `updateSelectedExtension` after
`updateModel` repopulates `allExtensions`.

Fixes microsoft#320101

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@ulugbekna ulugbekna force-pushed the fix/issue-reporter-preset-data-320101 branch from cc9e7e1 to 89df1e1 Compare June 5, 2026 13:55
@ulugbekna
Copy link
Copy Markdown
Contributor Author

Pushed follow-ups for the two issues spotted after the payload started arriving:

fileOnExtension=false, so the section vanished from the UI even though the body still included it. Dropped the fileOnExtension gate so the review UI matches what gets submitted.

  1. Preset data was silently dropped for built-in updateSelectedExtension has an early-return that switches built-in extensions to the VSCode source. My else if branch lived after that return, so for built-in callers the preset data never made it onto the model. Hoisted the propagation call to run before the built-in shortcut.extensions

  2. Redundant nested header on the GH Copilot's NES feedback flow passed issueBody: '# Description\n...', and the wizard wraps the description in its own ### Description section, producing a nested ### Description / # Description header. Dropped the redundant # Description\n prefix from inlineEditDebugComponent.ts.preview

cc @Giuspepe

@ulugbekna
Copy link
Copy Markdown
Contributor Author

Update: how the new wizard handles oversize extension data

For posterity, the new wizard's overflow handling lives in IssueFormService.openIssueOnGithub (src/vs/workbench/contrib/issue/browser/issueFormService.ts) and is functionally equivalent to the old reporter — just on a different code path:

  1. The body builder wraps Extension Data (and System Info, Process Info, Workspace Info, Experiments, etc.) in <details><summary>…</summary>…</details> blocks via createDetails (issueReporterOverlay.ts:2186).
  2. When the generated GH URL exceeds MAX_URL_LENGTH (7500), tryCreateBodyWithIssueDataAttachment calls extractIssueData, which strips every <details> block out of the body and bundles them into a single issue-data.md file. That file is uploaded through GitHubUploadService.uploadViaMobileApi (cached by content hash), and createBodyWithIssueDataLink re-inserts a ### Additional Issue Data\n\n[issue-data.md](<assetUrl>) link where the blocks used to be.
  3. If the upload path is unavailable (no GitHub token) or the shortened body is still too long, it falls back to showClipboardDialog → writes the full body to the clipboard and opens GH with a "We have written the needed data into your clipboard…" placeholder.

So the upload-as-attachment behavior the old reporter had is preserved; the bug fixed by #320103 is strictly about the preset data never reaching model.extensionData in the first place — once it lands there, the existing oversize plumbing takes over.

@ulugbekna ulugbekna merged commit ac855f1 into microsoft:main Jun 5, 2026
25 checks passed
@vs-code-engineering vs-code-engineering Bot added this to the 1.124.0 milestone Jun 5, 2026
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.

Issue reporter wizard drops preset extension "data" when opened with extensionId

3 participants