diff --git a/server/services/branchReconcile.js b/server/services/branchReconcile.js index b17a984768..e64c85398d 100644 --- a/server/services/branchReconcile.js +++ b/server/services/branchReconcile.js @@ -542,9 +542,14 @@ export function parseRemoteHeads(stdout) { */ export async function listRemoteHeads(repoPath) { const res = await execGit(['ls-remote', '--heads', RECONCILED_REMOTE], repoPath, { ignoreExitCode: true }) - .catch(() => null); + .catch((err) => ({ error: err.message })); if (!res || res.exitCode !== 0) { - console.error(`❌ branch-reconcile: git ls-remote ${RECONCILED_REMOTE} failed — remote branch state unknown this cycle`); + // One log line serves every caller across every managed app, so it has to + // name the repo that went unread and git's own reason — without both, a + // network blip, an unauthenticated remote and a repo with no `origin` at + // all are one indistinguishable line and the operator has nothing to act on. + const why = (res?.error || res?.stderr || '').trim().split('\n')[0] || `exit ${res?.exitCode}`; + console.error(`❌ branch-reconcile: git ls-remote ${RECONCILED_REMOTE} failed in ${repoPath} (${why}) — remote branch state unknown this cycle`); return null; } return parseRemoteHeads(res.stdout); @@ -679,8 +684,9 @@ async function worktreeAgeMs(worktreePath) { * `activeAgentIds` distinguishes a live agent's worktree from an abandoned one (see * `isAbandonedAgentWorktree`); omitting it leaves every agent worktree protected. * `remoteHeads` is `listRemoteHeads`' answer when the caller already has it; - * omitted, this reads it itself, and `null` (unreadable remote) is carried - * through as "we could not ask" rather than "the remote is empty". + * omitted, this reads it itself when the repo has an origin, and `null` + * (unreadable remote, or no origin to read) is carried through as "we could + * not ask" rather than "the remote is empty". * `hasOrigin` is `getOriginInfo`'s verdict when the caller already has it (same * rationale); omitted, this reads it itself, so a standalone caller still gets a * truthful answer rather than the fail-closed default. @@ -705,11 +711,19 @@ export async function gatherBranchState(repoPath, { defaultBranch, activeAgentId ? Boolean(origin?.hasOrigin) : Boolean(providedHasOrigin); + // The remote read is gated on `hasOrigin` for the same reason reconcile's own + // two are: a repo with no `origin` has no remote to ask, and probing one + // anyway logs a failure every cycle. This gather runs once per managed app in + // the read-only leftover-branch detector, and an app's repoPath can be a + // directory that was never a clone — `null` there means "could not ask", + // which is exactly what an origin-less repo can answer. const [branches, worktrees, prsByHeadOrNull, remoteHeads] = await Promise.all([ getBranches(repoPath), listWorktrees(repoPath).catch(() => []), getOpenPrsByHead(repoPath, origin), - providedRemoteHeads === undefined ? listRemoteHeads(repoPath) : providedRemoteHeads + providedRemoteHeads !== undefined ? providedRemoteHeads + : hasOrigin ? listRemoteHeads(repoPath) + : null ]); // null = the forge could not be read (see getOpenPrsByHead). Carried onto every // input so the classifier can refuse to conclude "no PR" from an unread forge. diff --git a/server/services/branchReconcile.test.js b/server/services/branchReconcile.test.js index b3272dde08..8be9d8ea3c 100644 --- a/server/services/branchReconcile.test.js +++ b/server/services/branchReconcile.test.js @@ -470,6 +470,35 @@ describe('gatherBranchState', () => { expect(inputs.find((i) => i.branch === 'next/issue-88').openPr).toBeNull(); expect(execGh).not.toHaveBeenCalled(); }); + + // The read-only leftover-branch detector calls this once per managed app, and + // an app's repoPath can be a directory with no origin (a static-site folder, + // a checkout whose remote was never added). `origin` is already resolved by + // the time the reads fan out, so probing the remote anyway bought nothing and + // logged `❌ … git ls-remote origin failed` for that app on every cycle — the + // exact per-cycle failure reconcile's own two remote reads are gated to avoid. + it('skips the remote probe on a repo with no origin', async () => { + getOriginInfo.mockResolvedValue({ + hasOrigin: false, isGithub: false, host: null, fullName: null + }); + git.getBranches.mockResolvedValue([ + { name: 'local/only', isDefault: false, current: false, tracking: null, merged: false } + ]); + wt.listWorktrees.mockResolvedValue([]); + git.isBranchMergedInto.mockResolvedValue(false); + execGit.mockImplementation(async (args) => (args[0] === 'ls-remote' + ? { stdout: '', stderr: "fatal: 'origin' does not appear to be a git repository", exitCode: 128 } + : { stdout: '', exitCode: 0 })); + const errors = vi.spyOn(console, 'error').mockImplementation(() => {}); + + const inputs = await gatherBranchState('/repo', { defaultBranch: 'main' }); + expect(inputs.find((i) => i.branch === 'local/only').hasOrigin).toBe(false); + expect(execGit).not.toHaveBeenCalledWith( + expect.arrayContaining(['ls-remote']), expect.anything(), expect.anything() + ); + expect(errors).not.toHaveBeenCalled(); + errors.mockRestore(); + }); }); describe('isAbandonedAgentWorktree', () => { @@ -1543,6 +1572,24 @@ describe('orphaned remote branches', () => { expect(git.deleteBranch).toHaveBeenCalledWith('/repo', 'stale/merged', { remote: true }); }); + it('names the repo and git\'s own reason when the remote cannot be read', async () => { + // One shared log line serves every caller across every managed app, so a + // bare "ls-remote failed" tells the operator neither which repo went + // unread nor whether it was a network blip or a broken remote. + execGit.mockResolvedValue({ + stdout: '', stderr: 'ssh: Could not resolve hostname github.com', exitCode: 128 + }); + git.getBranches.mockResolvedValue([]); + const errors = vi.spyOn(console, 'error').mockImplementation(() => {}); + + const res = await reapOrphanedRemotes('/repo', 'main'); + expect(res.remoteUnavailable).toBe(true); + const line = errors.mock.calls.map(([msg]) => String(msg)).find((m) => m.includes('ls-remote')); + expect(line).toContain('/repo'); + expect(line).toContain('Could not resolve hostname'); + errors.mockRestore(); + }); + it('merge-checks the SHA ls-remote reported, not the branch name', async () => { // A stale `origin/` ref can say "merged" about commits origin has // since moved past; the live SHA is the only safe thing to judge.