Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 2df5b67

Browse filesBrowse files
glennmichael123claude
glennmichael123
and
claude
committed
fix(github-provider): use API for open-PR detection instead of HTML scraping
Branch cleanup was silently auto-closing freshly-opened PRs. `isPROpen` determined PR state by scraping the GitHub PR page for `State--open` / `State--closed` CSS classes. GitHub removed those classes from the rendered HTML, so the scraper returned `false` for every PR — open or not. As a result, every buddy-bot branch with an open PR was misclassified as "orphaned", `cleanupStaleBranches` deleted the branch, and GitHub auto-closed the PR (no comment, since neither `checkAndCloseSatisfiedPRs` nor `checkAndCloseObsoletePRs` ran the close path). The next scheduled update job created a fresh PR; the cycle repeated every ~20 minutes. See e.g. the closed PR list on stacksjs/ts-css for the symptom. Replace `getOpenPRBranchesViaGit` (HTML scraping + git-SHA matching) with `getOpenPRBranches`, which uses the existing authoritative API (`getPullRequests('open')`) and filters to `buddy-bot/*` head refs. Rename `httpDetectionSuccessful` → `prDetectionSuccessful`. Drop `isPROpen` (unused). Keep the conservative age-based fallback so an API outage degrades gracefully (30-day protection in `getOpenPRBranches`, 7-day cleanup threshold in `cleanupStaleBranches`) instead of nuking everything. Add `test/orphan-branch-detection.test.ts` covering: open PR's branch is protected, branches without open PRs are orphaned, mixed matching, "API success ⇒ delete every orphan", "API failure + young branch ⇒ spare", "API failure + old branch ⇒ delete". Reverted-fix run fails 4/6; with-fix run passes 6/6. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent b9b49c3 commit 2df5b67
Copy full SHA for 2df5b67

2 files changed

+213-177Lines changed: 213 additions & 177 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎src/git/github-provider.ts‎

Copy file name to clipboardExpand all lines: src/git/github-provider.ts
+34-177Lines changed: 34 additions & 177 deletions
Original file line numberDiff line numberDiff line change
@@ -1088,16 +1088,7 @@ export class GitHubProvider implements GitProvider {
10881088
try {
10891089
const buddyBranches = await this.getBuddyBotBranches()
10901090

1091-
// Try to get PR branches using local git first
1092-
let prBranches: Set<string>
1093-
try {
1094-
prBranches = await this.getOpenPRBranchesViaGit()
1095-
}
1096-
catch (error) {
1097-
console.warn('⚠️ Failed to get PR branches via git, falling back to API:', error)
1098-
const openPRs = await this.getPullRequests('open')
1099-
prBranches = new Set(openPRs.map(pr => pr.head))
1100-
}
1091+
const prBranches = await this.getOpenPRBranches()
11011092

11021093
// Filter out branches that have active PRs
11031094
const orphanedBranches = buddyBranches.filter(branch => !prBranches.has(branch.name))
@@ -1111,173 +1102,39 @@ export class GitHubProvider implements GitProvider {
11111102
}
11121103

11131104
/**
1114-
* Check if a PR is open by making HTTP request to GitHub PR page (no API auth needed)
1105+
* Tracks whether the most recent open-PR detection succeeded authoritatively
1106+
* (via the GitHub API). Consumed by cleanupStaleBranches to decide between
1107+
* "delete every orphan" and "fall back to age-based protection".
11151108
*/
1116-
private async isPROpen(prNumber: number): Promise<boolean> {
1117-
try {
1118-
const url = `https://github.com/${this.owner}/${this.repo}/pull/${prNumber}`
1119-
const response = await fetch(url, {
1120-
method: 'GET',
1121-
headers: {
1122-
'User-Agent': 'buddy-bot/1.0',
1123-
},
1124-
})
1125-
1126-
if (!response.ok) {
1127-
// If we can't fetch the PR page, assume it might be open (be conservative)
1128-
return true
1129-
}
1130-
1131-
const html = await response.text()
1132-
1133-
// Look for PR status indicators in the HTML
1134-
// Open PRs have: class="State State--open"
1135-
// Closed PRs have: class="State State--closed" or class="State State--merged"
1136-
const isOpen = html.includes('State--open') && !html.includes('State--closed') && !html.includes('State--merged')
1137-
1138-
return isOpen
1139-
}
1140-
catch (error) {
1141-
// If HTTP request fails, be conservative and assume PR might be open
1142-
console.warn(`⚠️ Could not check PR #${prNumber} status via HTTP:`, error)
1143-
return true
1144-
}
1145-
}
1109+
private prDetectionSuccessful = false
11461110

11471111
/**
1148-
* Get branches that have open PRs using HTTP requests to GitHub (no API auth needed)
1112+
* Get the set of buddy-bot branches that currently have open PRs.
1113+
*
1114+
* Uses the GitHub REST API as the authoritative source. Earlier versions
1115+
* scraped the PR HTML page for `State--open` CSS classes, which silently
1116+
* broke when GitHub removed those classes — every open PR was misreported
1117+
* as closed, causing cleanupStaleBranches to delete the live PR's branch
1118+
* (which in turn auto-closed the PR).
11491119
*/
1150-
private httpDetectionSuccessful = false
1120+
private async getOpenPRBranches(): Promise<Set<string>> {
1121+
this.prDetectionSuccessful = false
11511122

1152-
private async getOpenPRBranchesViaGit(): Promise<Set<string>> {
11531123
try {
1154-
const protectedBranches = new Set<string>()
1155-
this.httpDetectionSuccessful = false
1156-
1157-
console.log('🔍 Using HTTP requests to check actual PR status (no API auth needed)...')
1158-
1159-
// Method 1: Get PR numbers from GitHub PR refs and check their status via HTTP
1160-
try {
1161-
const prRefsOutput = await this.runCommand('git', ['ls-remote', 'origin', 'refs/pull/*/head'])
1162-
const prNumbers: number[] = []
1163-
const prBranchMap = new Map<number, string[]>() // PR number -> branch names
1164-
1165-
for (const line of prRefsOutput.split('\n')) {
1166-
if (line.trim()) {
1167-
// Extract PR number from ref: "sha refs/pull/123/head"
1168-
const parts = line.trim().split('\t')
1169-
if (parts.length === 2) {
1170-
const ref = parts[1] // refs/pull/123/head
1171-
const sha = parts[0]
1172-
const prMatch = ref.match(/refs\/pull\/(\d+)\/head/)
1173-
1174-
if (prMatch) {
1175-
const prNumber = Number.parseInt(prMatch[1])
1176-
prNumbers.push(prNumber)
1177-
1178-
// Find which buddy-bot branch has this SHA
1179-
try {
1180-
const branchOutput = await this.runCommand('git', ['branch', '-r', '--contains', sha])
1181-
const branches: string[] = []
1182-
1183-
for (const branchLine of branchOutput.split('\n')) {
1184-
const branchName = branchLine.trim().replace(/^origin\//, '')
1185-
if (branchName.startsWith('buddy-bot/')) {
1186-
branches.push(branchName)
1187-
}
1188-
}
1189-
1190-
if (branches.length > 0) {
1191-
prBranchMap.set(prNumber, branches)
1192-
}
1193-
}
1194-
catch {
1195-
// Ignore errors finding branches for specific SHAs
1196-
}
1197-
}
1198-
}
1199-
}
1200-
}
1201-
1202-
// Only check PRs that have associated buddy-bot branches
1203-
const prNumbersToCheck = Array.from(prBranchMap.keys())
1204-
console.log(`📋 Found ${prNumbers.length} PR refs, ${prNumbersToCheck.length} have buddy-bot branches`)
1205-
console.log(`🔍 Checking ${prNumbersToCheck.length} PRs via HTTP (skipping ${prNumbers.length - prNumbersToCheck.length} non-buddy-bot PRs)...`)
1206-
1207-
// Check each PR's status via HTTP (in batches to be nice to GitHub)
1208-
const batchSize = 5
1209-
let checkedCount = 0
1210-
let openCount = 0
1211-
1212-
for (let i = 0; i < prNumbersToCheck.length; i += batchSize) {
1213-
const batch = prNumbersToCheck.slice(i, i + batchSize)
1214-
1215-
// Process batch with small delay between requests
1216-
const batchPromises = batch.map(async (prNumber, index) => {
1217-
// Small delay to avoid overwhelming GitHub
1218-
await new Promise(resolve => setTimeout(resolve, index * 100))
1219-
1220-
const isOpen = await this.isPROpen(prNumber)
1221-
checkedCount++
1222-
1223-
if (isOpen) {
1224-
openCount++
1225-
// Protect all branches associated with this open PR
1226-
const branches = prBranchMap.get(prNumber) || []
1227-
for (const branch of branches) {
1228-
protectedBranches.add(branch)
1229-
}
1230-
}
1231-
1232-
return { prNumber, isOpen }
1233-
})
1234-
1235-
await Promise.all(batchPromises)
1236-
1237-
// Small delay between batches
1238-
if (i + batchSize < prNumbers.length) {
1239-
await new Promise(resolve => setTimeout(resolve, 500))
1240-
}
1241-
}
1242-
1243-
console.log(`✅ Checked ${checkedCount} buddy-bot PRs via HTTP: ${openCount} open, ${checkedCount - openCount} closed`)
1244-
console.log(` (Skipped ${prNumbers.length - prNumbersToCheck.length} non-buddy-bot PRs)`)
1245-
console.log(`🛡️ Protected ${protectedBranches.size} branches with confirmed open PRs`)
1246-
console.log(`🎯 HTTP detection successful - no age-based protection needed`)
1247-
this.httpDetectionSuccessful = true
1248-
}
1249-
catch (error) {
1250-
console.warn('⚠️ Could not check PR status via HTTP, applying conservative fallback:', error)
1251-
1252-
// Only apply fallback protection if HTTP detection completely failed
1253-
try {
1254-
const allBuddyBranches = await this.getBuddyBotBranches()
1255-
const oneDayAgo = new Date()
1256-
oneDayAgo.setDate(oneDayAgo.getDate() - 1)
1257-
1258-
let fallbackCount = 0
1259-
for (const branch of allBuddyBranches) {
1260-
if (branch.lastCommitDate > oneDayAgo && !protectedBranches.has(branch.name)) {
1261-
protectedBranches.add(branch.name)
1262-
fallbackCount++
1263-
}
1264-
}
1265-
1266-
if (fallbackCount > 0) {
1267-
console.log(`🛡️ Emergency fallback: ${fallbackCount} very recent branches (< 1 day) protected due to HTTP failure`)
1268-
}
1269-
}
1270-
catch {
1271-
console.log('⚠️ Could not apply emergency fallback protection')
1272-
}
1273-
}
1274-
1275-
console.log(`🎯 HTTP-based analysis complete: protecting ${protectedBranches.size} branches total`)
1124+
const openPRs = await this.getPullRequests('open')
1125+
const protectedBranches = new Set<string>(
1126+
openPRs
1127+
.map(pr => pr.head)
1128+
.filter(head => head.startsWith('buddy-bot/')),
1129+
)
12761130

1131+
console.log(`🔍 GitHub API reports ${openPRs.length} open PR(s); ${protectedBranches.size} are buddy-bot branches`)
1132+
console.log(`🛡️ Protecting ${protectedBranches.size} branches with confirmed open PRs`)
1133+
this.prDetectionSuccessful = true
12771134
return protectedBranches
12781135
}
12791136
catch (error) {
1280-
console.warn('⚠️ HTTP-based analysis failed, using conservative fallback:', error)
1137+
console.warn('⚠️ Failed to fetch open PRs via API, falling back to age-based protection:', error)
12811138

12821139
// Conservative fallback: protect branches less than 30 days old
12831140
try {
@@ -1296,8 +1153,9 @@ export class GitHubProvider implements GitProvider {
12961153
return conservativeBranches
12971154
}
12981155
catch {
1299-
// Ultimate fallback: protect everything
1300-
console.log('🛡️ Ultimate fallback: protecting all branches')
1156+
// Ultimate fallback: leave prDetectionSuccessful=false so cleanupStaleBranches
1157+
// applies its age-based filter against whatever branches it can enumerate.
1158+
console.log('⚠️ Could not enumerate branches for age-based fallback')
13011159
return new Set<string>()
13021160
}
13031161
}
@@ -1312,21 +1170,20 @@ export class GitHubProvider implements GitProvider {
13121170
const orphanedBranches = await this.getOrphanedBuddyBotBranches()
13131171
console.log(`🔍 Found ${orphanedBranches.length} orphaned buddy-bot branches (no associated open PRs)`)
13141172

1315-
// Since we have 100% accurate HTTP-based PR detection, we can clean up ALL orphaned branches
1316-
// Only apply age filter if HTTP detection failed (indicated by very conservative protection)
1173+
// When PR detection succeeded authoritatively, every orphan is a true orphan.
1174+
// If detection fell back to age-based protection, only delete branches older
1175+
// than the configured threshold to avoid wiping out branches whose live PRs
1176+
// we couldn't verify.
13171177
let branchesToDelete = orphanedBranches
13181178

1319-
// Use the HTTP detection success flag to determine cleanup strategy
1320-
if (this.httpDetectionSuccessful) {
1321-
// HTTP detection worked perfectly - clean up ALL orphaned branches regardless of age
1322-
console.log(`🎯 HTTP detection successful - cleaning up ALL ${branchesToDelete.length} orphaned branches (any age)`)
1179+
if (this.prDetectionSuccessful) {
1180+
console.log(`🎯 PR detection successful - cleaning up ALL ${branchesToDelete.length} orphaned branches (any age)`)
13231181
}
13241182
else {
1325-
// HTTP detection failed - apply conservative age filter
13261183
const cutoffDate = new Date()
13271184
cutoffDate.setDate(cutoffDate.getDate() - olderThanDays)
13281185
branchesToDelete = orphanedBranches.filter(branch => branch.lastCommitDate < cutoffDate)
1329-
console.log(`⚠️ HTTP detection failed - only deleting branches older than ${olderThanDays} days`)
1186+
console.log(`⚠️ PR detection failed - only deleting branches older than ${olderThanDays} days`)
13301187
console.log(`🔍 Found ${branchesToDelete.length} stale buddy-bot branches (older than ${olderThanDays} days)`)
13311188
}
13321189

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.