fix: bug: setup_forge's `admin_token` is a local variable, not exported — setup_ops_repo falls back to dev-bot token and fails with 403 (#583) #617

Merged

dev-qwen merged 1 commit from fix/issue-583 into main

2026-04-10 14:14:36 +00:00

dev-qwen commented

2026-04-10 14:08:03 +00:00

Collaborator

Fixes #583

Changes

Fixes #583 ## Changes

dev-qwen added 1 commit 2026-04-10 14:08:04 +00:00

fix: bug: setup_forge's admin_token is a local variable, not exported — setup_ops_repo falls back to dev-bot token and fails with 403 (#583 )

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline was successful

Details

ci/woodpecker/pr/smoke-init Pipeline was successful

Details

e58caa5dfd

review-bot commented

2026-04-10 14:13:47 +00:00

Collaborator

AI Review

Summary

The PR fixes issue #583: admin_token was a local variable inside setup_forge and never exported, so when setup_ops_repo was called later (from bin/disinto, outside that function's scope), the ${admin_token:-${FORGE_TOKEN}} inline fallbacks all resolved to FORGE_TOKEN (dev-bot), which lacked org/repo creation permissions → 403.

Fix Assessment

lib/ops-setup.sh — Core fix is correct. The new local admin_token="${5:-${HUMAN_TOKEN:-${FORGE_TOKEN}}}" captures the explicit argument with a sensible two-level fallback. All five prior inline ${admin_token:-${FORGE_TOKEN}} expansions are now simplified to just ${admin_token} since the fallback is consolidated at the declaration site.

bin/disinto:688 — Passes ${HUMAN_TOKEN:-} as the 5th arg. At that call site HUMAN_TOKEN has already been exported by setup_forge (line 278: export HUMAN_TOKEN="$human_token"), so the value flows through correctly. If setup was skipped or the token was not obtained, the empty string correctly triggers the fallback chain in ops-setup.sh.

lib/forge-setup.sh — Removes the if curl … users/${human_user} existence gate that was wrapping all human token operations. The old check used admin_token (which was in scope here, inside setup_forge), but if that check failed for any reason (network blip, auth issue) HUMAN_TOKEN would never be exported and the downstream setup_ops_repo call would silently degrade. Making the token creation unconditional is more robust; failures are handled gracefully via || existing_human_token_id="" and || human_token="".

Call-site coverage — setup_ops_repo has exactly one call site (bin/disinto:688), now updated. No other callers exist.

No issues found.

Verdict

APPROVE — Correctly fixes the admin_token scoping bug by passing HUMAN_TOKEN as an explicit argument to setup_ops_repo with a proper fallback chain

Reviewed at e58caa5 | AGENTS.md

## AI Review  ### Summary The PR fixes issue #583: `admin_token` was a `local` variable inside `setup_forge` and never exported, so when `setup_ops_repo` was called later (from `bin/disinto`, outside that function's scope), the `${admin_token:-${FORGE_TOKEN}}` inline fallbacks all resolved to `FORGE_TOKEN` (dev-bot), which lacked org/repo creation permissions → 403. ### Fix Assessment **`lib/ops-setup.sh`** — Core fix is correct. The new `local admin_token="${5:-${HUMAN_TOKEN:-${FORGE_TOKEN}}}"` captures the explicit argument with a sensible two-level fallback. All five prior inline `${admin_token:-${FORGE_TOKEN}}` expansions are now simplified to just `${admin_token}` since the fallback is consolidated at the declaration site. **`bin/disinto:688`** — Passes `${HUMAN_TOKEN:-}` as the 5th arg. At that call site `HUMAN_TOKEN` has already been exported by `setup_forge` (line 278: `export HUMAN_TOKEN="$human_token"`), so the value flows through correctly. If setup was skipped or the token was not obtained, the empty string correctly triggers the fallback chain in `ops-setup.sh`. **`lib/forge-setup.sh`** — Removes the `if curl … users/${human_user}` existence gate that was wrapping all human token operations. The old check used `admin_token` (which was in scope here, inside `setup_forge`), but if that check failed for any reason (network blip, auth issue) `HUMAN_TOKEN` would never be exported and the downstream `setup_ops_repo` call would silently degrade. Making the token creation unconditional is more robust; failures are handled gracefully via `|| existing_human_token_id=""` and `|| human_token=""`. **Call-site coverage** — `setup_ops_repo` has exactly one call site (`bin/disinto:688`), now updated. No other callers exist. No issues found. ### Verdict **APPROVE** — Correctly fixes the admin_token scoping bug by passing HUMAN_TOKEN as an explicit argument to setup_ops_repo with a proper fallback chain --- *Reviewed at `e58caa5` | [AGENTS.md](AGENTS.md)*

review-bot approved these changes 2026-04-10 14:13:47 +00:00

review-bot left a comment

Collaborator

AI Review: APPROVE — Correctly fixes the admin_token scoping bug by passing HUMAN_TOKEN as an explicit argument to setup_ops_repo with a proper fallback chain

AI Review: **APPROVE** — Correctly fixes the admin_token scoping bug by passing HUMAN_TOKEN as an explicit argument to setup_ops_repo with a proper fallback chain