agentHost: only offer Sync Changes for a remote-tracking upstream - #336309
Open
Lucas Farias (Lucasfarg) wants to merge 5 commits into
Open
Lucas Farias (Lucasfarg) wants to merge 5 commits into
Lucas Farias (Lucasfarg) wants to merge 5 commits into
Conversation
The sync operation resolves the upstream through refs/remotes/<remote>/ and rejects anything else, but the provider advertised "Sync Changes" for every branch that reported an upstream with ahead/behind counts. A branch whose upstream is a local branch (branch.<name>.remote = .) therefore showed the button and failed on every click with "Could not resolve the remote". Use the existing parseUpstreamBranchName check so the provider and the handler agree on what can be synced.
Copilot started reviewing on behalf of
Lucas Farias (Lucasfarg)
September 15, 2026 18:09
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Local upstream names containing slashes still pass validation and produce the original failure.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Aligns Sync Changes availability with supported remote-tracking upstreams.
Changes:
- Filters unsupported upstream names.
- Adds regression coverage for a simple local upstream.
File summaries
| File | Description |
|---|---|
agentHostSyncOperationProvider.ts |
Adds upstream validation. |
agentHostSyncOperationProvider.test.ts |
Tests a local upstream. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Inferring "remote-tracking" from the shape of the upstream name misses a
local upstream whose name contains a slash (feature/base). Ask git instead:
for-each-ref %(upstream:remotename) reports the remote of the upstream and
"." for a local branch. Persist it as upstreamRemote in the session git
state and let the sync provider require it. The same value replaces the
split('/')[0] guess that fed the GitHub head-owner lookup.
…tream remote in parallel The persisted upstreamRemote is only consumed by the sync provider. Feeding it into the GitHub head-owner lookup changed which sessions get a head owner and opened a new path into the pull request handlers, which still parse the short upstream name. Leave that lookup as it was and run the new probe alongside the other per-branch probes. Tests: cover a remote whose name contains a slash with real git, pin that a local upstream keeps no head owner, and add the "remote-looking name without a remote" case to the provider test.
Copilot started reviewing on behalf of
Lucas Farias (Lucasfarg)
September 15, 2026 18:35
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Slash-containing remotes still fail during Sync, and legacy persisted state can incorrectly hide the operation.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Balanced
…fresh legacy state
Two gaps in the previous commits:
- A remote whose name contains a slash (my/fork) was offered for Sync but the
branch parser derived the remote from the first path segment, so the handler
pulled from "my". getRefs now asks for %(upstream:remotename) and the parser
carries that value into branch.upstream.remote.
- Git state persisted before upstreamRemote existed was indistinguishable from
a known local upstream, so Sync stayed hidden until an unrelated refresh.
upstreamRemote now stores git's value as is ("." for a local upstream), the
provider treats "." as not syncable, and needsSessionGitStateRefresh asks for
one recompute when a state names an upstream without its remote.
The name-derived guess only feeds the GitHub head-owner lookup and is named as
such. A failed remote probe is logged.
The provider and the handler now share one predicate. The persisted upstreamRemote keeps the remote only when parseUpstreamRef accepts the upstream ref, which is what the handler resolves; a local upstream or a fetch refspec that keeps the tracking ref outside refs/remotes/<remote>/ is stored as ".", so Sync Changes is not offered for an upstream the handler would reject. Covered by a resolveUpstreamRemote unit test and a real-git integration test with a custom fetch refspec.
Copilot started reviewing on behalf of
Lucas Farias (Lucasfarg)
September 15, 2026 19:08
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new Git format atom breaks branch discovery on supported Git versions older than 2.16.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 1
- Review effort level: Balanced
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 #336314
Sync Changes was offered for any branch with an upstream, but the sync handler only works with a remote-tracking upstream. With a local upstream (
git branch --set-upstream-to=main) the button showed up and every click failed with "Could not resolve the remote for the branch".The session git state now keeps the upstream's remote as git reports it (
%(upstream:remotename),.for a local branch), and Sync Changes is only offered when there's an actual remote. Ref parsing uses the same value, so a remote with a slash in its name (my/fork) now syncs too. Git state saved before this change is recomputed once. Upstreams git can't resolve torefs/remotes/<remote>/(for example a custom fetch refspec) are stored as local too, so the button isn't offered for them either.To test: in a session repo,
git checkout -b x && git branch --set-upstream-to=main, makexandmaindiverge; the button no longer appears.Tests cover local upstreams (including
feature/base), amy/forkremote against real git, and the refresh of old state.