ReviewOS

stacks/buddy

rebase subcommand can force-push an empty commit when file-update generator returns []

#1361
Closed glennmichael123 opened this 9 days ago ·
9 days ago

Summary

Same bug shape as #1360, in a different code path. The rebase subcommand (bin/cli.ts:696-851) — invoked when a user ticks the "rebase this PR" checkbox — calls generateAllFileUpdates and then unconditionally proceeds to commitChanges without checking whether any files actually need updating. If the generator returns [] (typical for pantry/pkgx deps that don't write a JS lock/yaml file), the rebase force-pushes an empty commit onto the PR branch.

Repro

  1. A buddy-bot PR exists for a pantry/pkgx-only dep group (e.g. curl.se, gnu.org/gmp, gnu.org/mpc).
  2. User ticks the - [ ] <!-- rebase-check --> checkbox in the PR body.
  3. update-check workflow fires, spawns bunx buddy-bot rebase <pr-number>.
  4. rebase runs:
    • buddy.generateAllFileUpdates(group.updates) → returns [] (no JS lock/yaml writer for these deps).
    • gitProvider.commitChanges(pr.head, group.title, [], baseBranch) runs anyway.
    • Branch is reset to base and force-pushed with what amounts to an empty commit.
    • gitProvider.updatePullRequest(prNum, { title, body }) rewrites the body and unchecks the box.
  5. PR now has a fresh "rebased" timestamp but no actual file changes since the prior state.

Root cause

bin/cli.ts:820-828:

// Generate new file changes (package.json, dependency files, GitHub Actions)
const packageJsonUpdates = await buddy.generateAllFileUpdates(group.updates)

// Recreate branch from base with latest dependency changes (Renovate-style)
const baseBranch = config.repository?.baseBranch || 'main'
await gitProvider.commitChanges(pr.head, group.title, packageJsonUpdates, baseBranch)
logger.info(`✅ Recreated branch ${pr.head} from ${baseBranch} with latest changes`)

No if (packageJsonUpdates.length === 0) guard. The equivalent createPullRequests path in src/buddy.ts had the same omission — that was fixed in #1360 (8408d29) by adding an early bail-out before any branch ops.

Fix

Mirror the #1360 fix. Before commitChanges:

const packageJsonUpdates = await buddy.generateAllFileUpdates(group.updates)
if (packageJsonUpdates.length === 0) {
  logger.info(`ℹ️ No file changes for PR #${prNum} — pantry/pkgx-style deps with no JS writer. Skipping rebase.`)
  // Optionally still uncheck the box so the workflow doesn't loop on it.
  const newBody = (pr.body || '').replace(/- \[x\] <!-- rebase-check -->/g, '- [ ] <!-- rebase-check -->')
  if (newBody !== pr.body) {
    await gitProvider.updatePullRequest(prNum, { body: newBody })
  }
  return
}

The checkbox-untick on bail-out matters — otherwise the next update-check run will see the still-ticked box and re-trigger the same no-op rebase forever.

  • #1359 — workflow-file PRs failing silently without BUDDY_BOT_TOKEN
  • #1360 — same empty-commit pattern in createPullRequests (already fixed)

Surfaced during a code-review pass of the rebase path. Hasn't been observed in the wild yet because pantry/pkgx-dep PRs rarely have users ticking the rebase box — but the bug is real and behaves identically to the #1360 path once it does fire.

Sign in to comment on this issue.