diff --git a/skill/scripts/context-signals.mjs b/skill/scripts/context-signals.mjs index fd3613e48..5afc0246f 100644 --- a/skill/scripts/context-signals.mjs +++ b/skill/scripts/context-signals.mjs @@ -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 "/" 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..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..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) diff --git a/tests/context-signals.test.mjs b/tests/context-signals.test.mjs index 43daf8975..1f4599451 100644 --- a/tests/context-signals.test.mjs +++ b/tests/context-signals.test.mjs @@ -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' });