mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-12 14:16:28 +03:00
Only strip a remote prefix that names a configured remote
Round-five bot findings, one real root cause: splitRemoteRef treated the first slash in any ref as a remote separator. A local upstream named release/2.0 was truncated to "2.0", and feature/foo tracking from branch foo collapsed to the current branch's own name and was self-skipped, discarding a valid base both times. The split now happens only when the prefix names a configured remote; otherwise the whole ref is one local branch name. The per-remote HEAD symref loop strips its own queried prefix directly (that remote may be fabricated in tests or partial clones without appearing in git remote). The reported pruned-upstream shape already resolves via the multi-remote rev lists from the previous round; its test now guards that. Prepared with AI assistance (Claude Code), directed by @pbakaus. Co-Authored-By: Claude Code <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Code
parent
386d3e7051
commit
f89b6c10b1
@@ -100,12 +100,19 @@ function gitSignals(cwd) {
|
||||
// try, in order. A remote ref like `upstream/release` (fork workflows) or
|
||||
// an origin/HEAD target with no local checkout is a perfectly good diff
|
||||
// base, so revs are not limited to local branch names.
|
||||
const remotes = (run(['remote']) || '').split('\n').filter(Boolean);
|
||||
// Strip a leading "<remote>/" only when that remote is actually
|
||||
// configured: a slash does not make a ref remote. A local upstream named
|
||||
// release/2.0 is one branch name, and truncating it to "2.0" (or
|
||||
// feature/foo to "foo", which then matches the current branch and gets
|
||||
// self-skipped) loses a valid base.
|
||||
const splitRemoteRef = (ref) => {
|
||||
const i = ref ? ref.indexOf('/') : -1;
|
||||
return i > 0 ? { name: ref.slice(i + 1), rev: ref } : null;
|
||||
if (i < 1) return null;
|
||||
return remotes.includes(ref.slice(0, i)) ? { name: ref.slice(i + 1), rev: ref } : null;
|
||||
};
|
||||
// A slashless @{u} is a LOCAL upstream (branch.<x>.remote = "."); it names
|
||||
// a merge target just as validly as a remote-tracking ref does.
|
||||
// An @{u} that carries no configured remote prefix is a LOCAL upstream
|
||||
// (branch.<x>.remote = "."); it names a merge target just as validly.
|
||||
const asUpstream = (ref) => splitRemoteRef(ref) || (ref ? { name: ref, rev: ref } : null);
|
||||
const conventional = ['develop', 'main', 'master'];
|
||||
// On an integration branch itself the scope hint is the working tree. No
|
||||
@@ -119,10 +126,12 @@ function gitSignals(cwd) {
|
||||
// checkout (branch reads as the literal `HEAD`) has no branch identity to
|
||||
// diff for and keeps the working-tree scope too.
|
||||
const remoteHeads = [];
|
||||
const remotes = (run(['remote']) || '').split('\n').filter(Boolean);
|
||||
for (const r of ['origin', ...remotes.filter((name) => name !== 'origin')]) {
|
||||
const head = splitRemoteRef(run(['symbolic-ref', '--short', `refs/remotes/${r}/HEAD`]));
|
||||
if (head) remoteHeads.push(head);
|
||||
for (const r of [...new Set(['origin', ...remotes])]) {
|
||||
// The symref's own prefix is the remote just queried, so it is stripped
|
||||
// directly; the remote need not be in `git remote` output (tests and
|
||||
// partial clones fabricate refs/remotes/origin/* without a remote).
|
||||
const ref = run(['symbolic-ref', '--short', `refs/remotes/${r}/HEAD`]);
|
||||
if (ref && ref.startsWith(`${r}/`)) remoteHeads.push({ name: ref.slice(r.length + 1), rev: ref });
|
||||
}
|
||||
const onIntegrationBranch = branch === 'HEAD'
|
||||
|| conventional.includes(branch)
|
||||
|
||||
@@ -482,6 +482,71 @@ describe('gatherSignals', () => {
|
||||
assert.deepEqual(s.git.changedFiles, ['src/Hero.tsx']);
|
||||
});
|
||||
|
||||
it('a local upstream with a slash in its name is not misparsed (#302)', async () => {
|
||||
const { execFileSync } = await import('node:child_process');
|
||||
const git = (...args) => execFileSync('git', args, { cwd: scratch, stdio: 'ignore' });
|
||||
git('init', '-q', '-b', 'release/2.0');
|
||||
git('config', 'user.email', 't@example.com');
|
||||
git('config', 'user.name', 'Test');
|
||||
write('src/App.tsx', 'export default 1;\n');
|
||||
git('add', '.');
|
||||
git('commit', '-qm', 'init');
|
||||
git('checkout', '-q', '-b', 'hotfix/x');
|
||||
git('branch', '-q', '--set-upstream-to=release/2.0');
|
||||
write('src/Hero.tsx', 'export const Hero = () => null;\n');
|
||||
git('add', '.');
|
||||
git('commit', '-qm', 'hotfix work');
|
||||
const s = await gatherSignals(scratch);
|
||||
// "release" is not a remote here; the whole ref is the local base name.
|
||||
assert.equal(s.git.base, 'release/2.0');
|
||||
assert.deepEqual(s.git.changedFiles, ['src/Hero.tsx']);
|
||||
});
|
||||
|
||||
it('a local upstream sharing the branch leaf name is not self-skipped (#302)', async () => {
|
||||
const { execFileSync } = await import('node:child_process');
|
||||
const git = (...args) => execFileSync('git', args, { cwd: scratch, stdio: 'ignore' });
|
||||
git('init', '-q', '-b', 'feature/foo');
|
||||
git('config', 'user.email', 't@example.com');
|
||||
git('config', 'user.name', 'Test');
|
||||
write('src/App.tsx', 'export default 1;\n');
|
||||
git('add', '.');
|
||||
git('commit', '-qm', 'init');
|
||||
git('checkout', '-q', '-b', 'foo');
|
||||
git('branch', '-q', '--set-upstream-to=feature/foo');
|
||||
write('src/Hero.tsx', 'export const Hero = () => null;\n');
|
||||
git('add', '.');
|
||||
git('commit', '-qm', 'work');
|
||||
const s = await gatherSignals(scratch);
|
||||
// Truncating feature/foo to "foo" made it look like the current branch
|
||||
// and the valid upstream was discarded.
|
||||
assert.equal(s.git.base, 'feature/foo');
|
||||
assert.deepEqual(s.git.changedFiles, ['src/Hero.tsx']);
|
||||
});
|
||||
|
||||
it('a pruned upstream tracking ref falls back to other remotes (#302)', async () => {
|
||||
const { execFileSync } = await import('node:child_process');
|
||||
const git = (...args) => execFileSync('git', args, { cwd: scratch, stdio: 'ignore' });
|
||||
git('init', '-q', '-b', 'feature/p');
|
||||
git('config', 'user.email', 't@example.com');
|
||||
git('config', 'user.name', 'Test');
|
||||
write('src/App.tsx', 'export default 1;\n');
|
||||
git('add', '.');
|
||||
git('commit', '-qm', 'init');
|
||||
git('remote', 'add', 'origin', '.');
|
||||
git('remote', 'add', 'upstream', '.');
|
||||
// The branch tracks origin/main, but that tracking ref was pruned; the
|
||||
// live main exists only on the upstream remote.
|
||||
git('config', 'branch.feature/p.remote', 'origin');
|
||||
git('config', 'branch.feature/p.merge', 'refs/heads/main');
|
||||
git('update-ref', 'refs/remotes/upstream/main', 'HEAD');
|
||||
write('src/Hero.tsx', 'export const Hero = () => null;\n');
|
||||
git('add', '.');
|
||||
git('commit', '-qm', 'feature work');
|
||||
const s = await gatherSignals(scratch);
|
||||
assert.equal(s.git.base, 'main');
|
||||
assert.deepEqual(s.git.changedFiles, ['src/Hero.tsx']);
|
||||
});
|
||||
|
||||
it('never diffs one integration branch against another (#302)', async () => {
|
||||
const { execFileSync } = await import('node:child_process');
|
||||
const git = (...args) => execFileSync('git', args, { cwd: scratch, stdio: 'ignore' });
|
||||
|
||||
Reference in New Issue
Block a user