fix(worker): start with a repository-only token #12

2026-10-25T21:18:28Z

greg commented

2026-10-25 21:18:28 +00:00

#11's start-up check called GET /user, which needs the read:user scope. The bot's token has only repository scope (all the worker needs), so the worker got 403 and restarted in a loop.

On 403 from /user, the commit author comes from BOT_NAME (default gbr-bot). Push access on the games repo is still checked.
A wrong token (401) still stops the worker with a clear error.
New test for a repository-only token.

🤖 Generated with Claude Code

#11's start-up check called `GET /user`, which needs the `read:user` scope. The bot's token has only repository scope (all the worker needs), so the worker got 403 and restarted in a loop. - On 403 from `/user`, the commit author comes from `BOT_NAME` (default `gbr-bot`). Push access on the games repo is still checked. - A wrong token (401) still stops the worker with a clear error. - New test for a repository-only token. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

👀 1

greg added 1 commit 2026-10-25 21:18:28 +00:00

fix(worker): start with a repository-only token

Validate game templates / validate (push) Successful in 20s

Details

Validate game templates / boot-test (push) Skipped

Details

Validate game templates / publish (push) Skipped

Details

0c868f7bac

The start-up check read the token's account with GET /user, which needs the
read:user scope. A token with only repository scope, which is all the worker
needs, got 403 and the worker exited in a restart loop. On 403 it now
names the commit author from BOT_NAME (default gbr-bot) and still checks
push access on the games repo.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

greg merged commit dd92b6be4c into main

2026-10-25 21:18:40 +00:00

greg deleted branch fix/worker-repo-scope-token

2026-10-25 21:18:40 +00:00

git_ai_bot reviewed

2026-10-25 21:19:00 +00:00

git_ai_bot left a comment

🤖 AI Code Review

1. Blocking issues

None.

2. Non-blocking suggestions

String-based error matching is fragile.

if !strings.HasPrefix(err.Error(), "HTTP 403") {

This couples checkBot to the exact error-string format produced by giteaGet. If giteaGet is ever refactored (e.g. to add a prefix like "gitea: HTTP 403 …" or to use a structured error), the 403 fallback silently breaks and the worker starts failing with a confusing wrapped error.

A more robust approach: have giteaGet return the status code alongside the error (or define a small *giteaError struct with a Status int field), then branch on the code:

type giteaError struct {
    Status int
    Body   string
}
func (e *giteaError) Error() string { return fmt.Sprintf("HTTP %d: %s", e.Status, e.Body) }

This isn't blocking because both functions live in the same file and the format is simple, but it would prevent a future regression.

Test server has no default 404.

In TestCheckBotWithRepositoryOnlyToken, the switch on r.URL.Path has no default case, so any unexpected request path returns 200 with an empty body. Adding default: w.WriteHeader(http.StatusNotFound) (as the first test does) would make the test fail loudly if checkBot ever hits an unexpected endpoint.

3. Tests

Coverage is good for the new behaviour. A few additions that would strengthen the suite:

403 on /user + push false on the repo. Verifies the error message still names the BOT_NAME fallback (not an empty string) when push is denied. Currently this path is untested.
401 still fails even when BOT_NAME is set. Guards against a future change accidentally treating all non-200 as "scope-missing." The existing test covers 401, but only in the /user-succeeds variant; a quick sub-test with the repo-only handler returning 401 would close the gap.
BOT_NAME unset → default gbr-bot. The test explicitly sets the env var; a variant with it unset (or t.Setenv("BOT_NAME", "")) would lock in the documented default.

4. Overall assessment

Solid, well-scoped fix. The 403 → BOT_NAME fallback correctly preserves the push-permission gate and the 401 hard-fail, and the new test exercises the happy path for a repository-only token. The only structural concern is the string-prefix error check, which is acceptable for now but worth tightening if the file grows. Ready to merge with the minor test additions noted above, or as-is if you prefer to follow up separately.

Automated review by AI Git Bot

## 🤖 AI Code Review ## 1. Blocking issues None. ## 2. Non-blocking suggestions **String-based error matching is fragile.** ```go if !strings.HasPrefix(err.Error(), "HTTP 403") { ``` This couples `checkBot` to the exact error-string format produced by `giteaGet`. If `giteaGet` is ever refactored (e.g. to add a prefix like `"gitea: HTTP 403 …"` or to use a structured error), the 403 fallback silently breaks and the worker starts failing with a confusing wrapped error. A more robust approach: have `giteaGet` return the status code alongside the error (or define a small `*giteaError` struct with a `Status int` field), then branch on the code: ```go type giteaError struct { Status int Body string } func (e *giteaError) Error() string { return fmt.Sprintf("HTTP %d: %s", e.Status, e.Body) } ``` This isn't blocking because both functions live in the same file and the format is simple, but it would prevent a future regression. **Test server has no default 404.** In `TestCheckBotWithRepositoryOnlyToken`, the `switch` on `r.URL.Path` has no `default` case, so any unexpected request path returns 200 with an empty body. Adding `default: w.WriteHeader(http.StatusNotFound)` (as the first test does) would make the test fail loudly if `checkBot` ever hits an unexpected endpoint. ## 3. Tests Coverage is good for the new behaviour. A few additions that would strengthen the suite: - **403 on `/user` + push `false` on the repo.** Verifies the error message still names the `BOT_NAME` fallback (not an empty string) when push is denied. Currently this path is untested. - **401 still fails even when `BOT_NAME` is set.** Guards against a future change accidentally treating all non-200 as "scope-missing." The existing test covers 401, but only in the `/user`-succeeds variant; a quick sub-test with the repo-only handler returning 401 would close the gap. - **`BOT_NAME` unset → default `gbr-bot`.** The test explicitly sets the env var; a variant with it unset (or `t.Setenv("BOT_NAME", "")`) would lock in the documented default. ## 4. Overall assessment Solid, well-scoped fix. The 403 → `BOT_NAME` fallback correctly preserves the push-permission gate and the 401 hard-fail, and the new test exercises the happy path for a repository-only token. The only structural concern is the string-prefix error check, which is acceptable for now but worth tightening if the file grows. Ready to merge with the minor test additions noted above, or as-is if you prefer to follow up separately. --- *Automated review by AI Git Bot*

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: gbreed-public/gbr-games#12