Merge dev/khelanmodi/index-management-ui into main and migrate to @microsoft/vscode-ext-webview#781
Merged
tnaum-ms merged 493 commits intoJul 6, 2026
Conversation
Bumps [launch-editor](https://github.com/vitejs/launch-editor) from 2.13.2 to 2.14.1. - [Commits](vitejs/launch-editor@v2.13.2...v2.14.1) --- updated-dependencies: - dependency-name: launch-editor dependency-version: 2.14.1 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
- Shorten option labels to noun-phrase style (e.g. 'Default kubeconfig',
'Kubeconfig file…', 'Paste kubeconfig YAML…')
- Fix resolution-order in default detail: KUBECONFIG env var listed first,
then the fallback path
- Detail for inline option now mentions what happens (saves as source)
rather than restating the obvious ('use clipboard content')
- Placeholder updated from 'Add Kubeconfig' to 'Select kubeconfig source'
Replace the long single-sentence warning message with a short message +
detail field pattern (matching VS Code modal conventions):
message: 'Your clipboard contents will be saved as a kubeconfig source.'
detail: 'Make sure the correct kubeconfig YAML is in your clipboard
before continuing.'
Removes the mention of VS Code Secret Storage from the prompt itself.
Update test matcher to objectContaining to accommodate the new detail field.
…default branch
Two gaps vs the equivalent default-branch errors:
- Load error: 'Failed to load kubeconfig: {msg}' gave no path context.
Now: 'The kubeconfig file "{path}" could not be loaded: {msg}. Fix the file
and try again.'
- No-contexts error: 'No Kubernetes contexts were found in "{path}".' had no
recovery guidance. Now appends: 'Fix the kubeconfig and try again.'
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…back - Implemented a confirmation modal for dropped kubeconfig files, allowing users to preview or import files explicitly. - Added detailed error messages for unsupported URI schemes, including network shares and remote hosts. - Introduced a utility function `shortenPathMiddle` to format long file paths for better display in modals. - Updated telemetry to track the number of confirmed, added, and invalid kubeconfig files. - Enhanced tests to cover new confirmation flow and path shortening functionality.
…nd remove unnecessary descriptions
…and support for default sources
…tion issues and silent confirmations
…and drag-and-drop support
…PC calls [R766-C01][R766-C02] R766-C01 (package): telemetryMiddlewareBody gated the 'Failed' result on !aborted but still stamped error/errorMessage for an aborted call that surfaced as a rejected result, so a cancellation looked like a failure on the error dimension. Gate the whole error-recording block on !aborted (option A): an aborted call is now recorded only as 'Canceled' (aborted='true') with no error* fields. Also export getInvocationSignal from ./host so consumers can share the abort check. Adds a regression test (aborted invocation that also throws -> no error* fields). R766-C02 (documentdb): documentDbTelemetryRunner ran its parseError enrichment (error/errorMessage/errorStack/errorCause) for any !result.ok, on the same telemetry bag, after the body -- so it re-stamped error fields and silently undid C01 for canceled ops. It now reads getInvocationSignal(invocation.ctx)?.aborted and skips enrichment when aborted (option B). Shipped with C01 so both telemetry layers agree. Also marks R766-C01/C02 implemented in the review doc.
…onse guard [R766-C03] The webview onReceive guard added in R766-N01 forwarded any object with an `id` property, including an inherited (prototype) `id` and a non-string `id` -- looser than the host-side isTransportRequestMessage guard (which already checks typeof id === 'string'). Add a shared isTransportResponseMessage type guard to shared/wireProtocol.ts (non-null object with its OWN string `id`, via Object.hasOwn + typeof) and use it in connectTrpc's onReceive, restoring host/webview symmetry and a single source of truth for the response shape (option C). Extend the R766-N01 test with a numeric-id case and an inherited-id case (whose id equals the pending request id, so the old guard would have resolved the query with undefined before the real response arrived). Also marks R766-C03 implemented in the review doc.
…n type [R766-C04] The DocumentDB useTrpcClient wrapper had an inferred return type. Declare it explicitly as TrpcClient<AppRouter> (imported from @microsoft/vscode-ext-webview/react), which locks the public surface, guards against accidental type widening if the framework hook signature changes, and matches the repo TypeScript guideline of always specifying return types. This was the Copilot reviewer's low-confidence (self-suppressed) comment. Also marks R766-C04 implemented in the review doc.
All four Copilot findings are implemented; add the Iteration 4 change-protocol table (C01/C02 -> 553bf4e, C03 -> 3301a70, C04 -> 19b99cb) and flip the chapter intro from "nothing implemented yet" to implemented. Records the green validation run (l10n no drift, prettier, eslint, jest 2662/159, tsc build).
The Iteration 4 chapter ended on the "Suggested batching" recommendations. Add a closing "Iteration 4 — done" section that lists the steps performed (triage, Batch 1 C01+C02 553bf4e, Batch 2 C03 3301a70, Batch 3 C04 19b99cb, doc finalize cb9cd3b, green validation) and a clear completion message, and reword the batching close to past tense.
…mics and onboarding
…ot extension mode The redesign chose the bundled-vs-dev source layout from extensionMode === Production, but the dev server (webpack serve) emits the bundled asset name (views.js). A webpack-bundled extension running in Development (the normal F5 dev flow, IS_BUNDLE=true, DEVSERVER=true) therefore requested the tsc dev file (index.js) from the dev server and got a 404, leaving the webview blank. Restore the pre-redesign contract: a required isBundled option drives the layout; extensionMode stays CSP-only. The DocumentDB consumer passes isBundled: git commit -q -F /tmp/done-msg.txt && git push -q origin HEAD && echo pushed && git --no-pager log -1 --format='%h %s'ext.isBundle. Adds regression tests.
…ubfolder Move rpcConcurrencyLogger (host) and reportObserverError (webview), plus their tests, into src/webviews/_integration/observability/ so the reference integration folder separates the router/transport wiring from the cross-cutting observability adapters. Update imports, the two external importers, the folder README, and add observability/README.md. No runtime behavior change.
…tion 7 (_integration observability regrouping)
…5 part 1) The host-side dispatch logger (rpcConcurrencyLogger) emitted a console.log on every completed RPC via the framework's consoleProcedureLogger. That line is useful when debugging the extension but is dead weight (string formatting + host-console I/O) on a shipped, installed build where no one watches that console. Gate it behind extensionMode !== Production so dev/F5 and Test builds still log while packaged builds never execute it. The concurrency telemetry gauge stays unconditional and records in every mode, production included. The mode is read per-call: ext.context is always populated by the time an RPC fires (the webview is open, so the extension has activated) and the comparison is O(1), so there is no module-load ordering risk and no measurable added cost. The framework's consoleProcedureLogger stays the neutral, opt-in sink; the gating decision lives in the DocumentDB adapter, preserving the package boundary. Tests stub ext.context.extensionMode per case and add Test-mode (still logs) and Production (no console line, gauge still recorded) coverage. Documented as Iteration 9 in the PR-766 review; Iteration 10 (accumulating-telemetry accumulate/flush redesign, part 2) staged as not-started.
… flush (R766-P05 part 2) callWithAccumulatingTelemetry batched the telemetry emit but still ran the full callWithTelemetryAndErrorHandling wrapper (IActionContext allocation, error- handling wiring, performance.now, Object.entries copy loops, an await) on EVERY call. On hot paths (per webview RPC, per keystroke) that per-call machinery is the cost that adds up, and it runs in production where the gauge is meant to. Redesign (Option 1 from the PR-766 review): the per-call path now runs the populator synchronously against a plain TelemetrySample bag and folds the values into in-memory batch totals - no IActionContext, no await, no callWithTelemetryAndErrorHandling. The Azure pipeline is entered exactly once, on flush, with the rolled-up event. - Callback contract: (sample: TelemetrySample) => void (was (ctx: IActionContext) => T | PromiseLike<T> returning Promise<T | undefined>). Retires the TelemetryWithDistributions cast; distributions is now a first-class bag field. Every call site already void-ed the result and used a synchronous callback. - Errors never accumulate: a populator throw discards the sample and is reported once through callWithTelemetryAndErrorHandling under the same callbackId (the only remaining heavy path, and rare). - Added a finite guard in accumulate so a stray NaN/Infinity cannot poison a sum or min/max reduction. - Unchanged: auto_duration_ms distribution, dist_* rollup keys, batch/interval throttle, flushAccumulatingTelemetry, flushed event name == callbackId. Migrated all direct callers (ClustersExtension, DocumentDBShellPty x6, collectionViewRouter, rpcConcurrencyLogger) and the meterSilentCatch shorthand; meterSilentCatch callers needed no change. Tests rewritten for the sync sample-bag API, adding finite-guard and 'per-call path never enters the telemetry pipeline, only flush does' coverage. Documented as Iteration 10 in the PR-766 review. Validation: jest 2668/2668, eslint (0 errors), tsc --noEmit clean, npm run build, prettier. No user-facing strings.
…teTelemetry (R766-P06) After the Iteration 10 accumulate/flush split, the callWith... name was misleading: the callWith... convention in @microsoft/vscode-azext-utils promises 'run my work inside a managed scope that hands me a full IActionContext (telemetry + errorHandling + ui + valuesToMask) and auto-records duration/result/ errors for it.' The redesigned helper does none of that on the per-call path - it fills a plain sample bag synchronously and returns void. Rename so the name stops promising a scoped call. Behavior unchanged; rename + file move only. - callWithAccumulatingTelemetry -> accumulateTelemetry (verb 'accumulate' signals 'record a data point into a batch', not 'run my work in a scope'). - flushAccumulatingTelemetry -> flushAccumulatedTelemetry. - AccumulatingTelemetryOptions -> AccumulateTelemetryOptions. - Unchanged: TelemetrySample, AUTO_DURATION_DISTRIBUTION_KEY, meterSilentCatch. - File: src/utils/callWithAccumulatingTelemetry.ts -> accumulatingTelemetry.ts (+ .test.ts) via git mv; all importers updated, including meterSilentCatch-only consumers and the two jest.mock paths. The verb now encodes whether your callback runs: accumulateTelemetry does NOT run your work; the deferred runWithAccumulatingTelemetry (issue #777) would, so a runWith... name is honest there. Reviewed the telemetry-instrumentation skill: it does not mention the accumulating helper by any name, so no skill edit was needed. Deliverable 2 (runWithAccumulatingTelemetry, wrap-an-action variant) filed as enhancement issue #777 for future pickup; not built here. Validation: jest 2668/2668, eslint 0 errors, tsc --noEmit clean, npm run build, prettier. No user-facing strings. Documented as Iteration 11 in the PR-766 review.
…s and new options bag requirements
…s and new options bag requirements
…rosoft/vscode-documentdb into dev/tnaum/webview-migrations # Conflicts: # docs/ai-and-plans/PRs/766-webview-ext-package-redesign/webview-ext-migration-manual.md
…ui' into copilot/index-management-ui-webview-migration # Conflicts: # l10n/bundle.l10n.json
…soft/vscode-ext-webview
Copilot
AI
changed the title
Merge dev/khelanmodi/index-management-ui into main + migrate to @microsoft/vscode-ext-webview
Merge dev/khelanmodi/index-management-ui into main and migrate to @microsoft/vscode-ext-webview
Jul 6, 2026
Copilot created this pull request from a session on behalf of
tnaum-ms
July 6, 2026 11:33
View session
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.
dev/khelanmodi/index-management-ui(Index Management tab feature) was 1 commit ahead but 491 commits behindmain. This PR brings it current withmainand ports the merged webview code onto the@microsoft/vscode-ext-webviewpackage that replaced@microsoft/vscode-ext-react-webviewin the meantime.Merge
origin/dev/khelanmodi/index-management-uionto currentmain.l10n/bundle.l10n.json— resolved by keepingmain's content and regenerating.openCollectionView.ts,IndexesItem.ts,appRouter.ts,CollectionView.tsx,collectionViewController.ts,ToolbarMainView.tsx) auto-merged cleanly and already reflected the new package's host-wiring pattern (openWebview/openAppWebview); no leftover references to the old package remained anywhere insrc/.Webview package migration
Two stragglers in the newly-merged Index Management files still targeted the old package's API shape:
IndexesTab.tsxused the retired tuple-return hook:indexViewRouter.tsimportedmeterSilentCatchfrom a path (utils/callWithAccumulatingTelemetry) that was renamed toutils/accumulatingTelemetryduring the 491-commit drift onmain.Localization
l10n/bundle.l10n.jsonto pick up the Index Management UI's new user-facing strings.Flagged, not addressed here
Two pre-existing items from the original feature branch, unrelated to this merge/migration:
IndexTable.tsx: the "Created" column label may be misleading —usageSincereflects when usage stats started accumulating, not index creation time.CollectionView.tsx: the "Results" tab was renamed to "Documents" — worth checking for consistency with other terminology in the codebase.Opened as a draft so CI automation doesn't trigger; will be retargeted manually.