mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-14 23:26:39 +03:00
Detect the diff base instead of assuming main/master
context-signals hardcoded ['main', 'master'] as diff-base candidates, so repos integrating through develop (or any other branch) diffed against the wrong base: git.changedFiles carried the entire divergence and downstream commands scanned the wrong set (issue #302). The base is now detected, most specific signal first: the branch's configured upstream (@{u}; a branch pushed with -u tracks itself and is skipped by the self-check), then the remote's default-branch symref (origin/HEAD), then the conventional integration names including develop. The conventional fallbacks are withheld when the current branch is itself one of them, so sitting on main in a repo that also has develop keeps the working-tree scope instead of diffing two integration branches against each other. Five tests (three failing-first): develop-based feature branch, origin/HEAD detection with a non-standard default name, upstream tracking, on-the-integration-branch fallback, and the integration-vs-integration guard. 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
d272b9bd5d
commit
a50702f2b6
@@ -86,8 +86,27 @@ function gitSignals(cwd) {
|
||||
return { isRepo: false, branch: null, base: null, changedFiles: [], changedCount: 0 };
|
||||
}
|
||||
const branch = run(['rev-parse', '--abbrev-ref', 'HEAD']);
|
||||
// The merge target is detected, not assumed. A hardcoded main/master list
|
||||
// diffed develop-based repos against the wrong base, so git.changedFiles
|
||||
// carried the whole develop/main divergence into scan.targets (issue
|
||||
// #302). Signals, most specific first: the branch's configured upstream
|
||||
// (@{u}; a branch pushed with -u tracks itself and is skipped by the
|
||||
// self-check), then the remote's default-branch symref (origin/HEAD),
|
||||
// then the conventional integration names. The conventional fallbacks
|
||||
// are withheld when the current branch IS one of them: sitting on main
|
||||
// in a repo that also has develop must not diff the two integration
|
||||
// branches against each other.
|
||||
const stripOrigin = (ref) => (ref && ref.startsWith('origin/') ? ref.slice('origin/'.length) : null);
|
||||
const upstreamBase = stripOrigin(run(['rev-parse', '--abbrev-ref', '--symbolic-full-name', '@{u}']));
|
||||
const remoteHeadBase = stripOrigin(run(['symbolic-ref', '--short', 'refs/remotes/origin/HEAD']));
|
||||
const conventional = ['develop', 'main', 'master'];
|
||||
const candidates = new Set([
|
||||
...[upstreamBase, remoteHeadBase].filter(Boolean),
|
||||
...(conventional.includes(branch) ? [] : conventional),
|
||||
]);
|
||||
let base = null;
|
||||
for (const b of ['main', 'master']) {
|
||||
for (const b of candidates) {
|
||||
if (b === branch) continue;
|
||||
if (run(['rev-parse', '--verify', '--quiet', b]) !== null) {
|
||||
base = b;
|
||||
break;
|
||||
|
||||
@@ -185,6 +185,105 @@ describe('gatherSignals', () => {
|
||||
assert.deepEqual(s.scan.targets, ['src']);
|
||||
});
|
||||
|
||||
it('diffs a feature branch against a develop integration branch (#302)', async () => {
|
||||
const { execFileSync } = await import('node:child_process');
|
||||
const git = (...args) => execFileSync('git', args, { cwd: scratch, stdio: 'ignore' });
|
||||
git('init', '-q', '-b', 'develop');
|
||||
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', 'feature/x');
|
||||
write('src/Hero.tsx', 'export const Hero = () => null;\n');
|
||||
git('add', '.');
|
||||
git('commit', '-qm', 'feature work');
|
||||
const s = await gatherSignals(scratch);
|
||||
// The hardcoded main/master candidate list found no base here, so the
|
||||
// committed feature work was invisible to the scan targets.
|
||||
assert.equal(s.git.base, 'develop');
|
||||
assert.deepEqual(s.git.changedFiles, ['src/Hero.tsx']);
|
||||
assert.deepEqual(s.scan.targets, ['src/Hero.tsx']);
|
||||
});
|
||||
|
||||
it('prefers the remote default branch (origin/HEAD) as the diff base (#302)', async () => {
|
||||
const { execFileSync } = await import('node:child_process');
|
||||
const git = (...args) => execFileSync('git', args, { cwd: scratch, stdio: 'ignore' });
|
||||
git('init', '-q', '-b', 'trunk');
|
||||
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');
|
||||
// Fabricate the remote's default-branch symref without a network remote.
|
||||
git('update-ref', 'refs/remotes/origin/trunk', 'HEAD');
|
||||
git('symbolic-ref', 'refs/remotes/origin/HEAD', 'refs/remotes/origin/trunk');
|
||||
git('checkout', '-q', '-b', 'feature/y');
|
||||
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, 'trunk');
|
||||
assert.deepEqual(s.git.changedFiles, ['src/Hero.tsx']);
|
||||
});
|
||||
|
||||
it('a branch tracking the integration branch diffs against its upstream (#302)', async () => {
|
||||
const { execFileSync } = await import('node:child_process');
|
||||
const git = (...args) => execFileSync('git', args, { cwd: scratch, stdio: 'ignore' });
|
||||
git('init', '-q', '-b', 'release');
|
||||
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');
|
||||
// A self-pointing remote gives git the fetch refspec it needs to map
|
||||
// refs/heads/release -> refs/remotes/origin/release; no network involved.
|
||||
git('remote', 'add', 'origin', '.');
|
||||
git('update-ref', 'refs/remotes/origin/release', 'HEAD');
|
||||
git('checkout', '-q', '-b', 'feature/z');
|
||||
git('branch', '-q', '--set-upstream-to=origin/release');
|
||||
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, 'release');
|
||||
assert.deepEqual(s.git.changedFiles, ['src/Hero.tsx']);
|
||||
});
|
||||
|
||||
it('sitting on the integration branch itself falls back to the working tree', async () => {
|
||||
const { execFileSync } = await import('node:child_process');
|
||||
const git = (...args) => execFileSync('git', args, { cwd: scratch, stdio: 'ignore' });
|
||||
git('init', '-q', '-b', 'develop');
|
||||
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');
|
||||
write('src/App.tsx', 'export default 2;\n'); // dirty, uncommitted
|
||||
const s = await gatherSignals(scratch);
|
||||
// No self-diff: base must be null and the dirty working tree is the scope.
|
||||
assert.equal(s.git.base, null);
|
||||
assert.deepEqual(s.git.changedFiles, ['src/App.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' });
|
||||
git('init', '-q', '-b', 'main');
|
||||
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('branch', '-q', 'develop'); // both integration branches exist
|
||||
write('src/App.tsx', 'export default 2;\n'); // dirty on main
|
||||
const s = await gatherSignals(scratch);
|
||||
// Sitting on main must not pick develop as a base; the dirty working
|
||||
// tree is the scope, exactly as before this change.
|
||||
assert.equal(s.git.base, null);
|
||||
assert.deepEqual(s.git.changedFiles, ['src/App.tsx']);
|
||||
});
|
||||
|
||||
it('has empty scan.targets only when there is no code at all', async () => {
|
||||
const s = await gatherSignals(scratch);
|
||||
assert.deepEqual(s.scan.targets, []);
|
||||
|
||||
Reference in New Issue
Block a user