Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions server/services/branchReconcile.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand Down
47 changes: 47 additions & 0 deletions server/services/branchReconcile.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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/<branch>` ref can say "merged" about commits origin has
// since moved past; the live SHA is the only safe thing to judge.
Expand Down