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>
This commit is contained in:
garrytanandcapy-ai[bot] committed 2026-11-01 16:54:11 +00:00
1 parent 87b7c922e0
commit 2a1f123d9e
4 files changed
+80 -20

No files matched your search

+14 -7
View File
@@ -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/<base>, 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/<base>, 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
+21 -13
View File
@@ -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;
}
+18
View File
@@ -362,6 +362,24 @@ describe('a bare base name reads origin/<base> 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)', () => {
+27
View File
@@ -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-'));