From 2a1f123d9eec9719a5aab475cc2fe7b896bbedba Mon Sep 17 00:00:00 2001 From: garrytan Date: Sun, 1 Nov 2026 16:54:11 +0000 Subject: [PATCH] fix: Step 11 review fixes (a fork branch, a colliding tag or another remote never replaces the base) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-diff-scope | 21 ++++++++++++++------- lib/gate-diff/git.ts | 34 +++++++++++++++++++++------------- test/diff-scope.test.ts | 18 ++++++++++++++++++ test/gate-diff-cli.test.ts | 27 +++++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 20 deletions(-) diff --git a/bin/gstack-diff-scope b/bin/gstack-diff-scope index d481cbb8a..6bfef3252 100755 --- a/bin/gstack-diff-scope +++ b/bin/gstack-diff-scope @@ -34,15 +34,22 @@ _default_base() { echo "main" } BASE="${1:-$(_default_base)}" -# A bare branch name reads its remote-tracking twin when there is one: /ship -# merges against origin/, and a linked worktree or a Capy clone never -# moves the local branch, so a stale one widens the scope (#3650). -_NOT_BRANCH='^[0-9a-f]{7,64}$|[~^]|@\{|^[A-Z_]*HEAD$|^refs/' -if [[ ! "$BASE" =~ $_NOT_BRANCH ]] && git rev-parse --verify -q "refs/remotes/origin/${BASE}^{commit}" >/dev/null 2>&1; then - BASE="origin/${BASE}" -fi HEAD_REF="" if [ "${2:-}" = "--head" ]; then HEAD_REF="${3:-}"; fi +# A bare branch name reads origin's tracking branch, by its full ref, when that +# shares history with HEAD: /ship merges against origin/, and a linked +# worktree or a Capy clone never moves the local branch, so a stale one widens +# the scope (#3650). A remote-qualified name is taken as given. +_NOT_BRANCH='^[0-9a-f]{7,64}$|[~^]|@\{|^[A-Z_]*HEAD$|^refs/' +_QUALIFIED=false +for _remote in $(git remote 2>/dev/null); do + case "$BASE" in "$_remote"/*) _QUALIFIED=true ;; esac +done +if [[ ! "$BASE" =~ $_NOT_BRANCH ]] && [ "$_QUALIFIED" = false ] \ + && git rev-parse --verify -q "refs/remotes/origin/${BASE}^{commit}" >/dev/null 2>&1 \ + && git merge-base "refs/remotes/origin/${BASE}" "${HEAD_REF:-HEAD}" >/dev/null 2>&1; then + BASE="refs/remotes/origin/${BASE}" +fi FRONTEND=false BACKEND=false diff --git a/lib/gate-diff/git.ts b/lib/gate-diff/git.ts index ee60115b9..cac2f298d 100644 --- a/lib/gate-diff/git.ts +++ b/lib/gate-diff/git.ts @@ -85,23 +85,31 @@ export function isShallow(cwd: string): boolean { /** * The base as a ref that resolves here and, when `head` is given, shares - * history with it: for a branch name its remote-tracking branch first - * (`origin/main` for `main`), the base /ship merges against, else the ref - * itself. A local `main` that is missing, behind (a linked worktree or a Capy - * clone never moves it, #3650) or shares no history with HEAD (a fetched-over - * clone) yields to the twin, so the fix line's fetch into the tracking ref is - * a fix and not a loop. + * history with it. For a bare branch name that is origin's tracking branch + * (`refs/remotes/origin/main` for `main`), the base /ship merges against, + * else the related local branch, else another remote's tracking branch. A + * local `main` that is behind (a linked worktree or a Capy clone never moves + * it, #3650), missing or unrelated to HEAD (a fetched-over clone) yields to the + * twin, so the fix line's fetch into the tracking ref is a fix and not a loop. + * A remote-qualified name (`upstream/main`) is taken as given, and the twin is + * read by its full ref, so a fork's branch or a tag cannot stand in for it. */ export function resolveBase(ref: string, cwd: string, head?: string): { ref: string; sha: string; local?: 'missing' | 'unrelated' | 'differs' } | null { const related = (sha: string) => !head || mergeBase(sha, head, cwd) !== null; const direct = resolvesToCommit(ref, cwd); - if (isBranchName(ref)) { - for (const remote of remoteNames(cwd)) { - const tracking = `${remote}/${ref}`; - const sha = resolvesToCommit(tracking, cwd); - if (!sha || !related(sha)) continue; - return sha === direct ? { ref: tracking, sha } : { ref: tracking, sha, local: !direct ? 'missing' : related(direct) ? 'differs' : 'unrelated' }; - } + const remotes = remoteNames(cwd); + if (!isBranchName(ref) || remotes.some(remote => ref.startsWith(`${remote}/`))) return direct ? { ref, sha: direct } : null; + const twin = (remote: string) => { + const sha = resolvesToCommit(`refs/remotes/${remote}/${ref}`, cwd); + if (!sha || !related(sha)) return null; + return sha === direct ? { ref: `${remote}/${ref}`, sha } : { ref: `${remote}/${ref}`, sha, local: !direct ? 'missing' as const : related(direct) ? 'differs' as const : 'unrelated' as const }; + }; + const origin = remotes.includes('origin') ? twin('origin') : null; + if (origin) return origin; + if (direct && related(direct)) return { ref, sha: direct }; + for (const remote of remotes) { + const other = twin(remote); + if (other) return other; } return direct ? { ref, sha: direct } : null; } diff --git a/test/diff-scope.test.ts b/test/diff-scope.test.ts index f9d31df84..05192b07e 100644 --- a/test/diff-scope.test.ts +++ b/test/diff-scope.test.ts @@ -362,6 +362,24 @@ describe('a bare base name reads origin/ when it exists (#3650)', () => { git('update-ref', `refs/remotes/origin/${git('rev-parse', 'release/x')}`, 'HEAD'); expect(scopeAt(dir, git('rev-parse', 'release/x')).SCOPE_BACKEND).toBe('true'); }); + + test('a colliding tag, a fork branch over an explicit remote base, or an unrelated twin never replaces the base (#3650 review)', () => { + const { dir, git } = behindBase('main'); + const upstreamCommit = git('rev-parse', 'origin/main'); + git('tag', 'origin/main', 'HEAD'); + expect([scopeAt(dir, 'main').SCOPE_BACKEND, scopeAt(dir, 'main').SCOPE_DOCS]).toEqual(['false', 'true']); + git('remote', 'add', 'upstream', 'file:///nonexistent-upstream'); + git('update-ref', 'refs/remotes/upstream/main', git('rev-parse', 'main')); + git('update-ref', 'refs/remotes/origin/upstream/main', 'HEAD'); + expect(scopeAt(dir, 'upstream/main').SCOPE_BACKEND).toBe('true'); + git('tag', '-d', 'origin/main'); + git('checkout', '-q', '--orphan', 'island'); + git('commit', '-q', '--allow-empty', '-m', 'unrelated'); + git('update-ref', 'refs/remotes/origin/main', 'HEAD'); + git('checkout', '-q', 'feature/test'); + expect(scopeAt(dir, 'main').SCOPE_BACKEND).toBe('true'); + expect(upstreamCommit).not.toBe(''); + }); }); describe('uncommitted work is visible (#2299)', () => { diff --git a/test/gate-diff-cli.test.ts b/test/gate-diff-cli.test.ts index ce8aace18..315bfccda 100644 --- a/test/gate-diff-cli.test.ts +++ b/test/gate-diff-cli.test.ts @@ -179,6 +179,33 @@ describe('gstack-gate-diff arguments and exit contract', () => { expect(bare.stderr).toContain('gstack-gate-diff: local main is not origin/main; base is origin/main'); }); + // Value: protects=a ref a contributor controls (a fork's origin/upstream/main branch, a tag named origin/main, another + // remote's main) never replaces the base the caller named or a related local base, so it cannot hide a gate edit; + // fails_when=resolveBase rewrites remote-qualified names, resolves the twin by its short name, or lets any remote beat a + // related local base; why_new=the cases above have one remote and no colliding refs; seam=none; no_claim=nothing about + // a remote's HEAD symref. + test('a fork branch, a colliding tag or another remote never replaces the base (#3650 review)', () => { + const src = repo({ fixture: 'rh1-skip', commit: true }); + const seed = gitIn(src, ['rev-parse', 'main']).trim(); + const clone = (remote: string) => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-gate-diff-refs-')); + repos.push(dir); + gitIn(dir, ['clone', '-q', '-o', remote, '--branch', 'feature', `file://${src}`, '.']); + return dir; + }; + const named = clone('upstream'); + gitIn(named, ['remote', 'add', 'origin', 'file:///nonexistent-fork']); + gitIn(named, ['update-ref', 'refs/remotes/origin/upstream/main', 'HEAD']); + expect(summary(run(named, ['upstream/main']).stdout).tagged).toBe('RH-1:1'); + const tagged = clone('origin'); + gitIn(tagged, ['tag', 'origin/main', 'HEAD']); + expect(summary(run(tagged, ['main']).stdout).tagged).toBe('RH-1:1'); + const fork = clone('zfork'); + gitIn(fork, ['branch', '-f', 'main', seed]); + gitIn(fork, ['update-ref', 'refs/remotes/zfork/main', 'HEAD']); + expect(summary(run(fork, ['main']).stdout).tagged).toBe('RH-1:1'); + }); + test('a clone whose only remote is upstream resolves the base to upstream/main, and the fix line fetches from upstream', () => { const src = repo({ fixture: 'rh1-skip', commit: true }); const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-gate-diff-upstream-'));