diff --git a/skill/scripts/context-signals.mjs b/skill/scripts/context-signals.mjs index 0140dccbd..d37610343 100644 --- a/skill/scripts/context-signals.mjs +++ b/skill/scripts/context-signals.mjs @@ -104,31 +104,40 @@ function gitSignals(cwd) { const i = ref ? ref.indexOf('/') : -1; return i > 0 ? { name: ref.slice(i + 1), rev: ref } : null; }; - const upstream = splitRemoteRef(run(['rev-parse', '--abbrev-ref', '--symbolic-full-name', '@{u}'])); - const remoteHead = splitRemoteRef(run(['symbolic-ref', '--short', 'refs/remotes/origin/HEAD'])); + // A slashless @{u} is a LOCAL upstream (branch..remote = "."); it names + // a merge target just as validly as a remote-tracking ref does. + const asUpstream = (ref) => splitRemoteRef(ref) || (ref ? { name: ref, rev: ref } : null); const conventional = ['develop', 'main', 'master']; - const candidates = []; - const seen = new Set(); - const addCandidate = (name, revs) => { - if (!name || name === branch || seen.has(name)) return; - seen.add(name); - candidates.push({ name, revs }); - }; - // The upstream tracks the actual merge target, so its remote rev wins over - // a possibly stale local branch of the same name. - if (upstream) addCandidate(upstream.name, [upstream.rev]); - if (remoteHead) addCandidate(remoteHead.name, [remoteHead.name, remoteHead.rev]); - if (!conventional.includes(branch)) { - for (const name of conventional) addCandidate(name, [name, `origin/${name}`]); - } + // On an integration branch itself the scope hint is the working tree. No + // signal may override that: an origin/HEAD or upstream naming a DIFFERENT + // integration branch (sitting on develop while the remote default is + // main) would produce exactly the integration-vs-integration divergence + // this detection exists to prevent. + const onIntegrationBranch = conventional.includes(branch); let base = null; let baseRev = null; - for (const c of candidates) { - const rev = c.revs.find((r) => run(['rev-parse', '--verify', '--quiet', r]) !== null); - if (rev) { - base = c.name; - baseRev = rev; - break; + if (!onIntegrationBranch) { + const upstream = asUpstream(run(['rev-parse', '--abbrev-ref', '--symbolic-full-name', '@{u}'])); + const remoteHead = splitRemoteRef(run(['symbolic-ref', '--short', 'refs/remotes/origin/HEAD'])); + const candidates = []; + const seen = new Set(); + const addCandidate = (name, revs) => { + if (!name || name === branch || seen.has(name)) return; + seen.add(name); + candidates.push({ name, revs }); + }; + // The upstream tracks the actual merge target, so its own rev wins over + // a possibly stale local branch of the same name. + if (upstream) addCandidate(upstream.name, [upstream.rev]); + if (remoteHead) addCandidate(remoteHead.name, [remoteHead.name, remoteHead.rev]); + for (const name of conventional) addCandidate(name, [name, `origin/${name}`]); + for (const c of candidates) { + const rev = c.revs.find((r) => run(['rev-parse', '--verify', '--quiet', r]) !== null); + if (rev) { + base = c.name; + baseRev = rev; + break; + } } } const diffBase = base && branch && branch !== base ? base : null; diff --git a/tests/context-signals.test.mjs b/tests/context-signals.test.mjs index b75101518..3631f8d48 100644 --- a/tests/context-signals.test.mjs +++ b/tests/context-signals.test.mjs @@ -309,6 +309,48 @@ describe('gatherSignals', () => { assert.deepEqual(s.git.changedFiles, ['src/Hero.tsx']); }); + it('remote signals cannot bypass the integration-branch guard (#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'); + git('checkout', '-q', 'develop'); + // The remote default is main; sitting on develop must still not produce + // a develop-vs-main integration diff via the origin/HEAD signal. + git('update-ref', 'refs/remotes/origin/main', 'main'); + git('symbolic-ref', 'refs/remotes/origin/HEAD', 'refs/remotes/origin/main'); + write('src/App.tsx', 'export default 2;\n'); // dirty on develop + const s = await gatherSignals(scratch); + assert.equal(s.git.base, null); + assert.deepEqual(s.git.changedFiles, ['src/App.tsx']); + }); + + it('honors a local (slashless) upstream branch (#302)', async () => { + const { execFileSync } = await import('node:child_process'); + const git = (...args) => execFileSync('git', args, { cwd: scratch, stdio: 'ignore' }); + git('init', '-q', '-b', 'canary'); + 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/u'); + git('branch', '-q', '--set-upstream-to=canary'); // local upstream, no remote + write('src/Hero.tsx', 'export const Hero = () => null;\n'); + git('add', '.'); + git('commit', '-qm', 'feature work'); + const s = await gatherSignals(scratch); + // canary is neither conventional nor remote, but the configured + // upstream names it as the merge target. + assert.equal(s.git.base, 'canary'); + 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' });