You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
github: attribute callers and prioritize shared requests - #348379
Supply stable caller attribution for shared GitHub requests and preserve caller-provided request options.
Use contextual priorities for passive metadata, automatic plugin updates, and mutation reconciliation. Preserve interactive and mutation defaults, including user-blocking marketplace refreshes, and keep native Git behavior unchanged.
Update the request guidance and existing test expectations.
Preserve request attribution and scheduling intent across session, connector, and plugin consumers. Keep interactive and mutation defaults while assigning passive reads and post-write reconciliation their appropriate priorities.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Align the existing commit, file, and branch lookup assertions with the shared service's caller metadata.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Priority-handling and backpressure defects can delay or reject user-blocking operations and fail concurrent marketplace clones, while overlapping forced refreshes can repeat pagination.
No open issues 🎉
8 previously missed issues
In code that hasn't changed since last review
Preserve subscription priority and promotion during hydration
Hydration cancels each resource's scheduled fetch, but hard-coding visible replaces even an interactive subscription's request with lower-priority work. Because the batch is not tracked as entry.operation, interactive interest added while it is queued cannot promote it either. Derive the batch priority from its subscriptions and track the shared request so later interest can raise it.
Promote serialized clone and update work for waiting foreground loads
A foreground marketplace load for the same repository must wait for an automatic background clone or refresh to finish because _cloneSequencer serializes them. Its interactive options never reach the transport in time to promote that work, so the user-blocking load remains behind passive requests. Make the running operation honor the highest waiting priority while retaining serialized cache writes; the same issue affects _updateSequencer in src/vs/workbench/contrib/chat/browser/pluginInstallService.ts:439.
Bound concurrent blob requests to avoid clone overload
With six uncached GitHub URL marketplaces downloading concurrently, the per-repository limits can retain 60 pluginGit.blob requests before any completes. Background requests have only 56 retained slots per caller, so this newly forwarded priority can reject a clone with overloaded and omit its marketplace. Bound blob requests across repositories or handle admission backpressure without treating it as a clone failure.
Keep initial session discovery at mutation priority
The initial session discovery awaits feed.start() before listing sessions, including during a manual refresh. Making its negotiation background work can delay or reject that user-blocking refresh despite reserved capacity for mutations. Keep the initial negotiation's mutation default; use background priority only for independent reconnects.
Avoid duplicate reconciliation scans during overlapping refreshes
When a reply or thread resolution overlaps a lower-priority refresh, refresh(true, …) can start an upgraded scan and then another forced scan, repeating pagination before the action completes. Make refresh() wait for the existing promise and then run one fresh reconciliation scan; simply dropping the final scan is unsafe because transport coalescing can reuse a pre-mutation response. Same issue in src/vs/workbench/contrib/sessions/browser/github/models/githubPullRequestCIModel.ts:294; src/vs/sessions/contrib/codeReview/browser/codeReviewService.ts:355 uses the affected thread model.
Keep repository selection checks at interactive priority
These eligibility checks also block repository selection and the mobile automation picker, rather than serving only passive refreshes. At background priority they can be delayed or rejected despite reserved interactive capacity, and a failure is cached as a disabled target. Keep the interactive default here and update the associated test expectation.
The existing browser Git tests only exercise omitted priority, while the new automatic-update tests stop at repository-service mocks. Add a background clone case in src/vs/workbench/contrib/chat/test/browser/pluginGitCommandService.test.ts that checks the priority on ref, tree, and every blob request, retaining the default-priority case.
The updated fetcher tests only supply a priority, so they would still pass if helpers overwrote an explicit caller or dropped deadline and cache metadata. Add a case supplying caller, priority, deadline, and noCache, and verify they survive both the pull-request lookup and thread-summary request while retaining fallback coverage.
Restore user-blocking reads and feed negotiation to their prior priority defaults. Avoid redundant forced reconciliation scans and defer plugin priority propagation until shared promotion and backpressure are supported.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Queue-priority handling can delay post-write reconciliation and reject branch-protection reads without retry, leaving protection data stale and potentially skipping protected-branch commit warnings.
These branch-protection reads can now be rejected at the queue's reserved-capacity threshold, even while interactive reads would still be admitted. GitHubBranchProtectionProvider only logs that failure and does not retry, so opening a repository under load can leave protection data empty or stale and skip the protected-branch commit warning. Keep both reads at their default interactive priority, or retry transient queue overloads; the ruleset read at line 186 has the same issue.
Addressed the branch-protection finding in the "previously missed" section of this review in 1d3c0785511. Both repository viewer-info and ruleset reads now use the default interactive priority, while caller attribution and avatar enrichment are preserved. Existing bridge tests pass in Electron (15) and Chromium (14), and targeted lint passes.
The separate forced-refresh/reconciliation thread is unchanged.
This branch has not been deployed
No deployments
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
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.
Related https://github.com/microsoft/vscode-internalbacklog/issues/9678