From 46f29ca8b314e42813c5fd9702c94f27d0e80918 Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Sun, 26 Jul 2026 18:14:34 -0700 Subject: [PATCH] Guard detached HEADs and non-origin remote defaults Two more real gaps from the post-rebase review round: a detached checkout reads its branch as the literal HEAD, so the integration guard never fired and candidate selection could diff a detached tip on main against develop; and the remote-default check only consulted origin, so a fork-parent layout whose only remote is upstream lost the guard on its default branch entirely. The guard now treats a detached HEAD as no-diff-base, and default-branch symrefs are collected from every remote (origin first), feeding both the guard and the candidate list. Two failing-first tests cover a detached tip beside a diverged develop and an upstream-only trunk default. Prepared with AI assistance (Claude Code), directed by @pbakaus. Co-Authored-By: Claude Code --- skill/scripts/context-signals.mjs | 20 ++++++++++++---- tests/context-signals.test.mjs | 40 +++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/skill/scripts/context-signals.mjs b/skill/scripts/context-signals.mjs index 15e78213d..743130a3c 100644 --- a/skill/scripts/context-signals.mjs +++ b/skill/scripts/context-signals.mjs @@ -113,10 +113,20 @@ function gitSignals(cwd) { // integration branch (sitting on develop while the remote default is // main) would produce exactly the integration-vs-integration divergence // this detection exists to prevent. "Integration branch" means a - // conventional name OR the remote's default branch, so a non-standard - // default like trunk is guarded the same way. - const remoteHead = splitRemoteRef(run(['symbolic-ref', '--short', 'refs/remotes/origin/HEAD'])); - const onIntegrationBranch = conventional.includes(branch) || branch === remoteHead?.name; + // conventional name OR any remote's default branch (origin first, but a + // fork-parent layout may only have an `upstream` remote), so a + // non-standard default like trunk is guarded the same way. A detached + // 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); + } + const onIntegrationBranch = branch === 'HEAD' + || conventional.includes(branch) + || remoteHeads.some((head) => head.name === branch); let base = null; let baseRev = null; if (!onIntegrationBranch) { @@ -136,7 +146,7 @@ function gitSignals(cwd) { // main; an existing develop therefore outranks the remote default. This // is #302's own repro shape, and repos without develop are unaffected. addCandidate('develop', ['develop', 'origin/develop']); - if (remoteHead) addCandidate(remoteHead.name, [remoteHead.name, remoteHead.rev]); + for (const head of remoteHeads) addCandidate(head.name, [head.name, head.rev]); for (const name of ['main', 'master']) addCandidate(name, [name, `origin/${name}`]); for (const c of candidates) { const rev = c.revs.find((r) => run(['rev-parse', '--verify', '--quiet', r]) !== null); diff --git a/tests/context-signals.test.mjs b/tests/context-signals.test.mjs index e2172d5da..a0ce8e959 100644 --- a/tests/context-signals.test.mjs +++ b/tests/context-signals.test.mjs @@ -396,6 +396,46 @@ describe('gatherSignals', () => { assert.deepEqual(s.git.changedFiles, ['src/App.tsx']); }); + it('a detached HEAD keeps the working-tree scope (#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', '--detach'); + write('src/App.tsx', 'export default 2;\n'); // dirty on a detached tip + const s = await gatherSignals(scratch); + // A detached checkout has no branch identity to diff for; picking + // develop here would refill changedFiles with integration divergence. + assert.equal(s.git.base, null); + assert.deepEqual(s.git.changedFiles, ['src/App.tsx']); + }); + + it('the integration guard sees non-origin remote defaults (#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'); + git('branch', '-q', 'develop'); + // The only remote is upstream (fork-parent layout, no origin at all); + // its default branch is trunk, which is exactly where we're sitting. + git('remote', 'add', 'upstream', '.'); + git('update-ref', 'refs/remotes/upstream/trunk', 'HEAD'); + git('symbolic-ref', 'refs/remotes/upstream/HEAD', 'refs/remotes/upstream/trunk'); + write('src/App.tsx', 'export default 2;\n'); // dirty on trunk + const s = await gatherSignals(scratch); + 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' });