deadstyle/lazyforge
Watch
1
0
Fork
You've already forked lazyforge
 0
Code Issues 1 Pull requests 2 Projects Releases 3 Packages Wiki Activity Actions

GitHub adapter: client and core Forge methods #84

New issue
Closed
opened 2026-10-09 09:35:56 -05:00 by deadstyle · 2 comments
deadstyle commented 2026-10-09 09:35:56 -05:00
Owner
Copy link

Problem

lazyforge can't talk to GitHub. This ticket builds the adapter's foundation: the API client and every core Forge method, so a GitHub session can list repos, show PRs and issues with CI status, merge, approve, comment and close. Connecting from onboarding is #86, and runs, logs and labels are #85. Parent: #83.

Acceptance criteria

  • internal/forge/github implements forge.Forge and forge.Approver, and passes forgetest.RunContract (ADR 0002: every adapter passes the contract suite).
  • It works against github.com and GitHub Enterprise Server. A web root of https://github.com talks to https://api.github.com, and any other root talks to <root>/api/v3. Info().URL stays the web root, so o opens github.com pages (ticket #83; design.md → Keymap o).
  • PRs and the PR list show one CI state folded from both check runs and commit statuses on the head SHA (ADR 0006: "On GitHub that means check-runs plus commit statuses").
  • Merge sends the confirmed head SHA, and a stale head returns ErrHeadChanged (ADR 0006; ADR 0011).
  • The merge strategy is the repo's default from the forge (design.md → Decided: "always the repo's default from the forge"). GitHub has no single default, so it's the one GitHub's merge button offers first among the methods the repo allows: merge commit, then squash, then rebase. The confirm dialog shows it, as it does on Forgejo.
  • Issues never include PRs (ADR 0006 paper check).
  • Hitting GitHub's rate limit, including its 403 form, surfaces as ErrRateLimited and never as a bad-token error (architecture.md → How the forges differ: rate limits).
  • An unchanged list costs no rate-limit budget on refresh (conditional requests, see Plan) (architecture.md: "the core cache is required there"; design.md → r and the five-minute refresh).
  • ADR 0017 records the adapter decisions below, and docs/architecture.md's GitHub column is updated to the verified endpoints.

Plan

New package internal/forge/github, shaped like internal/forge/gitea:

  1. Client (github.go): one *http.Client, per-call ctx. Headers Authorization: Bearer <token>, Accept: application/vnd.github+json, X-GitHub-Api-Version: 2022-11-28. New(ctx, webURL, token, hc) reads GET /user (and GET /meta on GHES for installed_version) so a bad token fails at connect. Probe(ctx, webURL, hc) does an unauthenticated GET /meta: 2xx returns forge.KindGitHub, and a non-JSON reply or 404 returns ErrNotFound. #86 wires it in.
  2. Errors: 401 → ErrUnauthorized; 403 with X-RateLimit-Remaining: 0 or a "secondary rate limit" message → ErrRateLimited; any other 403 → ErrUnauthorized; 404 → ErrNotFound; 429 → ErrRateLimited; merge 409 → ErrHeadChanged; 405 and 422 → ErrRefused carrying GitHub's message. Verify the merge 409 and 405 bodies against the docs and write them in as synthesized fixtures, as the gitea adapter does.
  3. Pagination: per_page=100, follow Link: rel="next" to the end. Never stop on a short page.
  4. Conditional requests: the client keeps a bounded, mutex-guarded cache (URL → ETag, body) for GETs and sends If-None-Match. A 304 replays the cached body. GitHub doesn't count 304s against the limit. Cap the cache at about 1,000 entries and evict the oldest. It lives in the adapter: core's cache can't do this, because it never sees HTTP.
  5. Methods:
    • ListRepos: GET /user/repos?affiliation=owner,collaborator,organization_member&sort=pushed. Access: admin → admin, push or maintain → write, pull or triage → read, and archived → read. LastActivity is pushed_at, sorted client-side as the contract requires.
    • ListChangeRequests / GetChangeRequest: GET /repos/{o}/{r}/pulls?state=…. For closed, fold merged_at != null into StateMerged. CI comes from GET …/commits/{sha}/check-runs (paginated, {total_count, check_runs}) plus GET …/commits/{sha}/status.
    • CI fold: a combined status with total_count: 0 contributes nothing (GitHub reports pending there). Precedence fail > cancelled > running > pending > pass > skipped > none. Check-run failure, timed_out, action_required and startup_failure are fail. cancelled is cancelled. in_progress is running. queued, waiting, requested and pending are pending. success and neutral are pass. skipped and stale are skipped. Status error and failure are fail.
    • Merge: PUT …/pulls/{n}/merge with {sha, merge_method}. An empty Method reads GET /repos/{o}/{r} for allow_merge_commit, allow_squash_merge and allow_rebase_merge, which /user/repos doesn't return (verified 2026-10-09: null there, set on the single-repo GET). Set Repo.MergeStyle the same way where the dialog reads it. Check how the dialog gets it today and keep that path working with one extra GET per repo at most.
    • Approve: POST …/pulls/{n}/reviews {event: "APPROVE"}. Approving your own PR is a 422, which maps to ErrRefused.
    • ListIssues: GET …/issues?state=…, dropping items with a pull_request key. Merged returns nil with no request, as gitea does.
    • EditIssueBody: PATCH …/issues/{n}. ListComments / Comment: …/issues/{n}/comments for both kinds (paginated). Close: PATCH …/pulls/{n} or …/issues/{n} {state: closed}.
    • ListReleases: GET …/releases. Gate: always nil. Attachments: nil, since GitHub has no attachment list (#87 covers images).
  6. Tests: fixtures recorded with gh api into testdata/, with a README in the gitea format (source, date, hand edits). A stateful httptest server like gitea/server_test.go backs RunContract. Unit tests cover the CI fold table, the rate-limit 403, Link paging across three pages, the 304 replay, and merge-method selection.
  7. ADR 0017 (in this MR): a thin net/http client and no google/go-github, for consistency with ADR 0005, control over the ETag cache, and no new module. It also records the CI fold, the merge-method rule and the conditional-request cache.

Context

  • Interface and sentinels: internal/forge/forge.go. Availability: internal/forge/can.go. Contract: internal/forge/forgetest/contract.go.
  • The pattern to mirror: internal/forge/gitea/gitea.go (client, statusError, walk), forge.go (methods) and server_test.go (stateful fake server).
  • The ADR 0006 paper check already maps every method to GitHub, and the architecture.md API table has the GitHub column marked unverified. Verify each endpoint against current docs while building.
  • Verified 2026-10-09 against github.com: list endpoints send a weak ETag, X-RateLimit-Limit: 5000, and /user/repos returns permissions with maintain and triage but null allow_* merge flags.
  • The rate limit matters because ★ Renovate scans every repo, and each PR costs 2 CI calls on GitHub. ETags make steady-state refreshes nearly free. If the first load proves too slow on a large account, GraphQL is the follow-up, not part of this ticket.

Out of scope

Onboarding and connect wiring (#86). RunLister, LogReader and Labeler (#85). Images (#87). Re-run, review comments, and GitHub App auth.

## Problem lazyforge can't talk to GitHub. This ticket builds the adapter's foundation: the API client and every core `Forge` method, so a GitHub session can list repos, show PRs and issues with CI status, merge, approve, comment and close. Connecting from onboarding is #86, and runs, logs and labels are #85. Parent: #83. ## Acceptance criteria - `internal/forge/github` implements `forge.Forge` and `forge.Approver`, and passes `forgetest.RunContract` *(ADR 0002: every adapter passes the contract suite)*. - It works against github.com and GitHub Enterprise Server. A web root of `https://github.com` talks to `https://api.github.com`, and any other root talks to `<root>/api/v3`. `Info().URL` stays the web root, so `o` opens github.com pages *(ticket #83; design.md → Keymap `o`)*. - PRs and the PR list show one CI state folded from both check runs and commit statuses on the head SHA *(ADR 0006: "On GitHub that means check-runs plus commit statuses")*. - Merge sends the confirmed head SHA, and a stale head returns `ErrHeadChanged` *(ADR 0006; ADR 0011)*. - The merge strategy is the repo's default from the forge *(design.md → Decided: "always the repo's default from the forge")*. GitHub has no single default, so it's the one GitHub's merge button offers first among the methods the repo allows: merge commit, then squash, then rebase. The confirm dialog shows it, as it does on Forgejo. - Issues never include PRs *(ADR 0006 paper check)*. - Hitting GitHub's rate limit, including its 403 form, surfaces as `ErrRateLimited` and never as a bad-token error *(architecture.md → How the forges differ: rate limits)*. - An unchanged list costs no rate-limit budget on refresh (conditional requests, see Plan) *(architecture.md: "the core cache is required there"; design.md → `r` and the five-minute refresh)*. - ADR 0017 records the adapter decisions below, and `docs/architecture.md`'s GitHub column is updated to the verified endpoints. ## Plan New package `internal/forge/github`, shaped like `internal/forge/gitea`: 1. **Client** (`github.go`): one `*http.Client`, per-call `ctx`. Headers `Authorization: Bearer <token>`, `Accept: application/vnd.github+json`, `X-GitHub-Api-Version: 2022-11-28`. `New(ctx, webURL, token, hc)` reads `GET /user` (and `GET /meta` on GHES for `installed_version`) so a bad token fails at connect. `Probe(ctx, webURL, hc)` does an unauthenticated `GET /meta`: 2xx returns `forge.KindGitHub`, and a non-JSON reply or 404 returns `ErrNotFound`. #86 wires it in. 2. **Errors:** 401 → `ErrUnauthorized`; 403 with `X-RateLimit-Remaining: 0` or a "secondary rate limit" message → `ErrRateLimited`; any other 403 → `ErrUnauthorized`; 404 → `ErrNotFound`; 429 → `ErrRateLimited`; merge 409 → `ErrHeadChanged`; 405 and 422 → `ErrRefused` carrying GitHub's `message`. Verify the merge 409 and 405 bodies against the docs and write them in as synthesized fixtures, as the gitea adapter does. 3. **Pagination:** `per_page=100`, follow `Link: rel="next"` to the end. Never stop on a short page. 4. **Conditional requests:** the client keeps a bounded, mutex-guarded cache (URL → ETag, body) for GETs and sends `If-None-Match`. A 304 replays the cached body. GitHub doesn't count 304s against the limit. Cap the cache at about 1,000 entries and evict the oldest. It lives in the adapter: core's cache can't do this, because it never sees HTTP. 5. **Methods:** - `ListRepos`: `GET /user/repos?affiliation=owner,collaborator,organization_member&sort=pushed`. Access: admin → admin, push or maintain → write, pull or triage → read, and archived → read. `LastActivity` is `pushed_at`, sorted client-side as the contract requires. - `ListChangeRequests` / `GetChangeRequest`: `GET /repos/{o}/{r}/pulls?state=…`. For closed, fold `merged_at != null` into `StateMerged`. CI comes from `GET …/commits/{sha}/check-runs` (paginated, `{total_count, check_runs}`) plus `GET …/commits/{sha}/status`. - **CI fold:** a combined status with `total_count: 0` contributes nothing (GitHub reports `pending` there). Precedence fail > cancelled > running > pending > pass > skipped > none. Check-run `failure`, `timed_out`, `action_required` and `startup_failure` are fail. `cancelled` is cancelled. `in_progress` is running. `queued`, `waiting`, `requested` and `pending` are pending. `success` and `neutral` are pass. `skipped` and `stale` are skipped. Status `error` and `failure` are fail. - `Merge`: `PUT …/pulls/{n}/merge` with `{sha, merge_method}`. An empty `Method` reads `GET /repos/{o}/{r}` for `allow_merge_commit`, `allow_squash_merge` and `allow_rebase_merge`, which `/user/repos` doesn't return (verified 2026-10-09: null there, set on the single-repo GET). Set `Repo.MergeStyle` the same way where the dialog reads it. Check how the dialog gets it today and keep that path working with one extra GET per repo at most. - `Approve`: `POST …/pulls/{n}/reviews` `{event: "APPROVE"}`. Approving your own PR is a 422, which maps to `ErrRefused`. - `ListIssues`: `GET …/issues?state=…`, dropping items with a `pull_request` key. Merged returns nil with no request, as gitea does. - `EditIssueBody`: `PATCH …/issues/{n}`. `ListComments` / `Comment`: `…/issues/{n}/comments` for both kinds (paginated). `Close`: `PATCH …/pulls/{n}` or `…/issues/{n}` `{state: closed}`. - `ListReleases`: `GET …/releases`. `Gate`: always nil. `Attachments`: nil, since GitHub has no attachment list (#87 covers images). 6. **Tests:** fixtures recorded with `gh api` into `testdata/`, with a README in the gitea format (source, date, hand edits). A stateful `httptest` server like `gitea/server_test.go` backs `RunContract`. Unit tests cover the CI fold table, the rate-limit 403, `Link` paging across three pages, the 304 replay, and merge-method selection. 7. **ADR 0017** (in this MR): a thin `net/http` client and no `google/go-github`, for consistency with ADR 0005, control over the ETag cache, and no new module. It also records the CI fold, the merge-method rule and the conditional-request cache. ## Context - Interface and sentinels: `internal/forge/forge.go`. Availability: `internal/forge/can.go`. Contract: `internal/forge/forgetest/contract.go`. - The pattern to mirror: `internal/forge/gitea/gitea.go` (client, `statusError`, `walk`), `forge.go` (methods) and `server_test.go` (stateful fake server). - The ADR 0006 paper check already maps every method to GitHub, and the `architecture.md` API table has the GitHub column marked unverified. Verify each endpoint against current docs while building. - Verified 2026-10-09 against github.com: list endpoints send a weak `ETag`, `X-RateLimit-Limit: 5000`, and `/user/repos` returns `permissions` with `maintain` and `triage` but null `allow_*` merge flags. - The rate limit matters because ★ Renovate scans every repo, and each PR costs 2 CI calls on GitHub. ETags make steady-state refreshes nearly free. If the first load proves too slow on a large account, GraphQL is the follow-up, not part of this ticket. ## Out of scope Onboarding and connect wiring (#86). `RunLister`, `LogReader` and `Labeler` (#85). Images (#87). Re-run, review comments, and GitHub App auth.
deadstyle commented 2026-10-09 09:37:11 -05:00
Author
Owner
Copy link

🤖 Variance: GitHub's /user/repos omits the allow_* merge flags, so the adapter leaves Repo.MergeStyle empty and the confirm dialog shows "repo default" (its existing text for forges that don't report one). Merge still resolves the method with one GET /repos/{o}/{r} at merge time. Fetching it per repo in the list would cost one request per repo on every load.

🤖 Variance: GitHub's /user/repos omits the allow_* merge flags, so the adapter leaves Repo.MergeStyle empty and the confirm dialog shows "repo default" (its existing text for forges that don't report one). Merge still resolves the method with one GET /repos/{o}/{r} at merge time. Fetching it per repo in the list would cost one request per repo on every load.
deadstyle commented 2026-10-09 09:54:06 -05:00
Author
Owner
Copy link

🤖 merged: #88

🤖 merged: https://git.bobparsons.dev/deadstyle/lazyforge/pulls/88
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
develop
renovate/go-non-major
renovate/ci-tools-non-major
main
chore/trim-agent-guides
feature/label-editing
feature/workshop-splash
claude/laughing-ritchie-hbzxsf
chore/renovate-patterns
v0.3.0
v0.2.0
v0.1.0
Labels
Clear labels   
bug
Something behaves wrong

  
docs
Documentation only

  
feature
New user-visible capability

  
maintenance
Dependency updates and upkeep (Renovate)

  
needs-pm
Parked: waiting on a product decision from the PM

  
opus
Driver: Opus drives it (complex, risky, or design-heavy)

  
sonnet
Driver: Sonnet can drive it (well-bounded, low risk)

  
spike
Time-boxed investigation that answers a question

  
tech-debt
Engineering cleanup with no user-visible change

No labels
bug
docs
feature
maintenance
needs-pm
opus
sonnet
spike
tech-debt
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
deadstyle/lazyforge#84
No description provided.