Closes #1451.
The bug
findExistingDashboard returned null for two completely different outcomes:
this.logger.info('No existing dashboard issue found')
return null
}
catch (error) {
this.logger.warn(`Failed to search for existing dashboard: ${error}`)
return null // <- indistinguishable from "searched, found nothing"
}The caller branches on truthiness alone, so it cannot tell them apart. The only network call inside that try is getIssues(), which rethrows on any failure — so a 5xx, timeout, socket error or secondary rate limit became null → "no dashboard exists" → createIssue(). The run then exited 0, leaving a permanent duplicate dashboard behind a momentary failure.
The pre-create race check calls the same helper, so it was no protection at all here: whatever broke the first lookup is usually still broken a moment later.
The fix
Log at error and rethrow, so absence and failure stay distinguishable. Both call sites go through this one catch, so the initial lookup and the race check are both covered by the single change.
The principle from the issue: a failed run is strictly better than a duplicate dashboard. A failed run is recoverable — the next scheduled run retries. A duplicate has to be noticed and closed by hand.
On the issue's second path
The issue also describes a failed update falling through to create, and suggests the race-check guard raceCheckIssue.number !== existingIssue?.number is defeated. That path is already closed on main, and I did not change it.
updateDashboardIssue only returns null when the error isNotFound:
if (error instanceof GitHubApiError && error.isNotFound) { ...; return null }
throw errorSo a transient update failure already fails the run, which is what the issue asks for. null is reserved for a dashboard that genuinely no longer exists — where creating a replacement is correct, and where skipping a stale race-check hit on that same number is also correct. test/dashboard-missing-issue.test.ts pins exactly this: 404 and 410 return null; 403 and non-API errors rethrow. The guard's comment ("the issue we just found to be missing") is accurate as written.
Worth saying plainly since the issue is mine: that half of the report was based on a misreading of the current code.
I've left the third suggestion — reconciling by closing older marker-bearing dashboards after a create — out of scope. It's a self-healing mechanism for duplicates that slip through by other means, and it deletes user-visible issues, which deserves its own change.
Tests
test/buddy-orchestration.test.ts had a case pinning the old behaviour by name:
it('failure case - a getIssues failure degrades to null instead of throwing', ...)That was asserting the bug, so I replaced it rather than adding alongside it, and extended the block to cover the failure modes that matter:
- a
getIssuesfailure rejects instead of reporting absence - the pre-create race check rejects on the same failure
- a configured
issueNumberlookup rejects rather than reporting the dashboard missing
The existing found/absent cases in that block already cover the other side of the contract (marker match, labels-plus-title, never adopting a foreign title-only dashboard, configured number genuinely absent), so those are unchanged.
I first wrote these in a separate test/dashboard-lookup-failure.test.ts, then folded them into the existing findExistingDashboard block instead — that file already owned this function's coverage and a second file would have duplicated most of it.
Checks
bun test— 2399 pass, 0 fail across 146 filesbun run lint— exit 0bun run typecheck— exit 0
🤖 Generated with Claude Code