Skip to content

fix: sweep last context.octokit.repos.getBranch in app.js to .request() form#44

Open
avrabe wants to merge 1 commit intomainfrom
fix/dashboard-octokit-sweep
Open

fix: sweep last context.octokit.repos.getBranch in app.js to .request() form#44
avrabe wants to merge 1 commit intomainfrom
fix/dashboard-octokit-sweep

Conversation

@avrabe
Copy link
Copy Markdown
Contributor

@avrabe avrabe commented May 1, 2026

Fixes Bug #16 from docs/agent-fleet/bugs.md (wave-1 Probot/Octokit expert).

src/app.js:268 was the one remaining context.octokit.repos.getBranch call — same Probot-v14 namespace failure mode as PRs #22/#29/#30. Swept to .request() form. Test mock retrofitted so existing assertions on repos.getBranch.mockX keep working via route dispatch.

(dashboard.js is intentionally not touched — it uses its own @octokit/rest client where namespaces are reliable.)

  • 834 tests pass
  • eslint clean

🤖 Generated with Claude Code

…() form

## Why
Wave-1 Probot/Octokit expert flagged this as Bug #16 in
`docs/agent-fleet/bugs.md`: `src/app.js:268` was the one remaining
`context.octokit.repos.getBranch(...)` call after PR #22 / #29 / #30 swept
`.issues.X` to `.request()`. Same root cause — Probot v14 sometimes ships
`context.octokit` without the namespaced REST plugin, so the `.repos`
namespace is undefined at runtime and the call throws synchronously inside
the `repository.created` handler retry loop.

Note: `dashboard.js` also uses namespaced calls (`octokit.repos.X`,
`octokit.pulls.list`, etc.) but it builds its OWN `@octokit/rest@22` client
(`dashboard.js:31-36`) where the namespaces are reliably loaded; those calls
are intentionally left as-is.

## What
Replace the lone `context.octokit.repos.getBranch({owner, repo, branch})`
with `octokit.request('GET /repos/{owner}/{repo}/branches/{branch}', {...})`.
Identical behaviour, idiomatic v14, immune to the namespace issue.

Test mock retrofit: `createMockOctokit` in `__tests__/integration/app.test.js`
now also dispatches `GET /repos/{owner}/{repo}/branches/{branch}` into the
existing `repos.getBranch` jest.fn() so tests asserting on
`context.octokit.repos.getBranch.mockRejectedValue(...)` still work.

## Test plan
- [x] 834 tests pass — including the existing `repository.created` tests
      that assert the 404-retry loop and the `skipBranchScopedWork` path
- [x] eslint clean

## Risk & rollout
- Risk: low. Pure call-shape change. Same endpoint, same params.
- Rollout: self-update on merge.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant