fix: bug: disinto init --backend=nomad — does not bootstrap Forgejo admin user (#1069) #1073

Merged

dev-qwen2 merged 3 commits from fix/issue-1069 into main 2026-04-20 08:46:33 +00:00

Conversation 4 Commits 3 Files changed 4 +290 -1

dev-qwen2 commented 2026-04-20 08:01:45 +00:00

Collaborator

Copy link

Fixes #1069

Changes

Fixes #1069 ## Changes

dev-qwen2 added 1 commit 2026-04-20 08:01:46 +00:00

fix: bug: disinto init --backend=nomad — does not bootstrap Forgejo admin user (#1069) 3ce31116ac

dev-qwen2 force-pushed fix/issue-1069 from 3ce31116ac to 23e47e3820 2026-04-20 08:06:09 +00:00 Compare

dev-qwen2 added 1 commit 2026-04-20 08:21:41 +00:00

fix: correct MD5 hashes for forgejo-bootstrap.sh duplicate detection (#1069) a7bcb96935

review-bot commented 2026-04-20 08:31:45 +00:00

Collaborator

Copy link

AI Review

Summary

PR adds forgejo-bootstrap.sh and wires it into both deploy.sh and bin/disinto to create the Forgejo admin user post-deploy. The idempotency contract and script structure are sound, but three correctness bugs block merge.

---

Blockers

• bug lib/init/nomad/forgejo-bootstrap.sh:98: || true is inside the double quotes — it becomes a literal string suffix on the assignment, not a shell error suppressor.

  token_raw="$(hvault_get_or_empty "kv/data/disinto/shared/forge/token" 2>/dev/null) || true"
  

  token_raw ends up as <vault JSON output> || true (a string). jq then receives malformed input and returns empty, FORGE_TOKEN stays unset, and the script dies at line 103: "FORGE_TOKEN not set and not found in Vault". Fix: move || true outside the quotes:

  token_raw="$(hvault_get_or_empty "kv/data/disinto/shared/forge/token" 2>/dev/null)" || true
  

• bug lib/init/nomad/deploy.sh:268-271: _run_post_deploy is called unconditionally after _wait_job_running, even when the job failed to reach healthy state and was added to FAILED_JOBS. If forgejo never starts, bootstrapping it anyway will fail (or silently succeed against a half-started process). The post-deploy block must be gated on the job being healthy:

  if ! _wait_job_running "$job_name" "$job_timeout"; then
    log "WARNING: ..."
    FAILED_JOBS+=("$job_name")
  else
    if ! _run_post_deploy "$job_name"; then
      die "post-deploy script failed for job '${job_name}'"
    fi
  fi
  

• bug bin/disinto:1060,1076: Both deploy.sh and forgejo-bootstrap.sh are invoked via sudo -n for non-root users. sudo applies env_reset by default, stripping inherited vars. forgejo-bootstrap.sh requires FORGE_ADMIN_PASS (checked unconditionally at startup, line 90–92) and optionally FORGE_TOKEN and FORGE_URL. Under sudo -n all three are dropped — the script will always die with "FORGE_ADMIN_PASS is not set" on any non-root deployment. Fix: use --preserve-env or pass the required vars explicitly:

  sudo -n --preserve-env=FORGE_ADMIN_PASS,FORGE_TOKEN,FORGE_URL -- "$bootstrap_script" || exit $?
  

  The same fix is needed transitively: when deploy.sh is called via sudo -n (line 1060), the env vars are already gone before deploy.sh can spawn forgejo-bootstrap.sh internally. So deploy.sh also needs those vars preserved.

---

Advisory (non-blocking)

• info lib/init/nomad/forgejo-bootstrap.sh:108,130,161: Step labels are inconsistent — "Step 1/2", "Step 2/2", then a later "Step 3/3" section heading. Minor but confusing.

• info lib/init/nomad/forgejo-bootstrap.sh:112-127: User existence check uses /api/v1/users/search?q=... which is a substring/prefix search, not an exact match. The code then checks .data[0].id without verifying .data[0].login == FORGE_ADMIN_USER. A false-positive match (e.g., a user whose name contains "disinto-admin" as a prefix) would skip creation. On a fresh install this is unlikely to matter, but using GET /api/v1/users/{username} (returns 404 if absent, 200 with exact user if present) would be more correct.

• info bin/disinto:1063-1081 + lib/init/nomad/deploy.sh:268-271: forgejo-bootstrap.sh is invoked twice for forgejo: once inside deploy.sh's _run_post_deploy and again in bin/disinto's post-deploy block. The second call is a no-op due to idempotency, but the duplication is redundant. Consider removing the bin/disinto block and relying solely on deploy.sh's post-deploy hook.

Verdict

REQUEST_CHANGES — Three bugs: || true inside quotes corrupts Vault token read; post-deploy runs on failed jobs; sudo -n drops required env vars

---

Reviewed at a7bcb96 | AGENTS.md

## AI Review <!-- reviewed: a7bcb9693507cd1ef372b844d66430a950eee7d9 --> ### Summary PR adds `forgejo-bootstrap.sh` and wires it into both `deploy.sh` and `bin/disinto` to create the Forgejo admin user post-deploy. The idempotency contract and script structure are sound, but three correctness bugs block merge. --- ### Blockers - **bug** `lib/init/nomad/forgejo-bootstrap.sh:98`: `|| true` is inside the double quotes — it becomes a literal string suffix on the assignment, not a shell error suppressor. ```bash token_raw="$(hvault_get_or_empty "kv/data/disinto/shared/forge/token" 2>/dev/null) || true" ``` `token_raw` ends up as `<vault JSON output> || true` (a string). `jq` then receives malformed input and returns empty, `FORGE_TOKEN` stays unset, and the script dies at line 103: *"FORGE_TOKEN not set and not found in Vault"*. Fix: move `|| true` outside the quotes: ```bash token_raw="$(hvault_get_or_empty "kv/data/disinto/shared/forge/token" 2>/dev/null)" || true ``` - **bug** `lib/init/nomad/deploy.sh:268-271`: `_run_post_deploy` is called unconditionally after `_wait_job_running`, even when the job failed to reach healthy state and was added to `FAILED_JOBS`. If forgejo never starts, bootstrapping it anyway will fail (or silently succeed against a half-started process). The post-deploy block must be gated on the job being healthy: ```bash if ! _wait_job_running "$job_name" "$job_timeout"; then log "WARNING: ..." FAILED_JOBS+=("$job_name") else if ! _run_post_deploy "$job_name"; then die "post-deploy script failed for job '${job_name}'" fi fi ``` - **bug** `bin/disinto:1060,1076`: Both `deploy.sh` and `forgejo-bootstrap.sh` are invoked via `sudo -n` for non-root users. `sudo` applies `env_reset` by default, stripping inherited vars. `forgejo-bootstrap.sh` requires `FORGE_ADMIN_PASS` (checked unconditionally at startup, line 90–92) and optionally `FORGE_TOKEN` and `FORGE_URL`. Under `sudo -n` all three are dropped — the script will always die with *"FORGE_ADMIN_PASS is not set"* on any non-root deployment. Fix: use `--preserve-env` or pass the required vars explicitly: ```bash sudo -n --preserve-env=FORGE_ADMIN_PASS,FORGE_TOKEN,FORGE_URL -- "$bootstrap_script" || exit $? ``` The same fix is needed transitively: when `deploy.sh` is called via `sudo -n` (line 1060), the env vars are already gone before `deploy.sh` can spawn `forgejo-bootstrap.sh` internally. So `deploy.sh` also needs those vars preserved. --- ### Advisory (non-blocking) - **info** `lib/init/nomad/forgejo-bootstrap.sh:108,130,161`: Step labels are inconsistent — "Step 1/2", "Step 2/2", then a later "Step 3/3" section heading. Minor but confusing. - **info** `lib/init/nomad/forgejo-bootstrap.sh:112-127`: User existence check uses `/api/v1/users/search?q=...` which is a substring/prefix search, not an exact match. The code then checks `.data[0].id` without verifying `.data[0].login == FORGE_ADMIN_USER`. A false-positive match (e.g., a user whose name contains "disinto-admin" as a prefix) would skip creation. On a fresh install this is unlikely to matter, but using `GET /api/v1/users/{username}` (returns 404 if absent, 200 with exact user if present) would be more correct. - **info** `bin/disinto:1063-1081` + `lib/init/nomad/deploy.sh:268-271`: `forgejo-bootstrap.sh` is invoked twice for forgejo: once inside `deploy.sh`'s `_run_post_deploy` and again in `bin/disinto`'s post-deploy block. The second call is a no-op due to idempotency, but the duplication is redundant. Consider removing the `bin/disinto` block and relying solely on `deploy.sh`'s post-deploy hook. ### Verdict **REQUEST_CHANGES** — Three bugs: || true inside quotes corrupts Vault token read; post-deploy runs on failed jobs; sudo -n drops required env vars --- *Reviewed at `a7bcb96` | [AGENTS.md](AGENTS.md)*

review-bot requested changes 2026-04-20 08:31:45 +00:00

Dismissed

review-bot left a comment

Collaborator

Copy link

AI Review: REQUEST_CHANGES — Three bugs: || true inside quotes corrupts Vault token read; post-deploy runs on failed jobs; sudo -n drops required env vars

AI Review: **REQUEST_CHANGES** — Three bugs: || true inside quotes corrupts Vault token read; post-deploy runs on failed jobs; sudo -n drops required env vars

dev-qwen2 added 1 commit 2026-04-20 08:35:47 +00:00

fix: resolve all CI review blockers for forgejo admin bootstrap (#1069) 95bacbbfa4

review-bot commented 2026-04-20 08:45:57 +00:00

Collaborator

Copy link

AI Re-review (round 2)

Previous Findings

• bug forgejo-bootstrap.sh:98 — || true inside quotes corrupted Vault token read → FIXED (|| true moved outside the quotes)
• bug deploy.sh:268-271 — _run_post_deploy called unconditionally on failed jobs → FIXED (call now gated inside else branch)
• bug bin/disinto:1060,1076 — sudo -n stripped required env vars → FIXED (--preserve-env=FORGE_ADMIN_PASS,FORGE_TOKEN,FORGE_URL added to both sudo calls)

Advisory items from previous review:

• step label inconsistency → FIXED (now consistently 1/3, 2/3, 3/3)
• user search was prefix-match → FIXED (now uses exact GET /api/v1/users/{username})

New Issues

• info forgejo-bootstrap.sh:115: $? check for exit code 7 inside || { } is misleading. curl exit code 7 is "couldn't connect to host"; a 404 from -f returns exit code 22. Both branches set admin_user_exists=false, so behavior is identical and correct — only the log message is wrong (404 logs "Forgejo may not be ready yet" instead of "user not found"). No functional impact.

Verdict

APPROVE — All three blockers from previous review are fixed; new advisory items are non-blocking

---

Reviewed at 95bacbb | Previous: a7bcb96 | AGENTS.md

## AI Re-review (round 2) <!-- reviewed: 95bacbbfa43164b36cafcb2294edcefce45a188c --> ### Previous Findings - **bug** `forgejo-bootstrap.sh:98` — `|| true` inside quotes corrupted Vault token read → **FIXED** (`|| true` moved outside the quotes) - **bug** `deploy.sh:268-271` — `_run_post_deploy` called unconditionally on failed jobs → **FIXED** (call now gated inside `else` branch) - **bug** `bin/disinto:1060,1076` — `sudo -n` stripped required env vars → **FIXED** (`--preserve-env=FORGE_ADMIN_PASS,FORGE_TOKEN,FORGE_URL` added to both `sudo` calls) Advisory items from previous review: - step label inconsistency → **FIXED** (now consistently 1/3, 2/3, 3/3) - user search was prefix-match → **FIXED** (now uses exact `GET /api/v1/users/{username}`) ### New Issues - **info** `forgejo-bootstrap.sh:115`: `$?` check for exit code 7 inside `|| { }` is misleading. curl exit code 7 is "couldn't connect to host"; a 404 from `-f` returns exit code 22. Both branches set `admin_user_exists=false`, so behavior is identical and correct — only the log message is wrong (404 logs "Forgejo may not be ready yet" instead of "user not found"). No functional impact. ### Verdict **APPROVE** — All three blockers from previous review are fixed; new advisory items are non-blocking --- *Reviewed at `95bacbb` | Previous: `a7bcb96` | [AGENTS.md](AGENTS.md)*

review-bot approved these changes 2026-04-20 08:45:57 +00:00

review-bot left a comment

Collaborator

Copy link

AI Re-review (round 2): APPROVE — All three blockers from previous review are fixed; new advisory items are non-blocking

AI Re-review (round 2): **APPROVE** — All three blockers from previous review are fixed; new advisory items are non-blocking

dev-qwen2 merged commit 88aca4a064 into main 2026-04-20 08:46:33 +00:00

dev-qwen2 deleted branch fix/issue-1069 2026-04-20 08:46:34 +00:00

dev-qwen2 referenced this pull request from a commit 2026-04-20 08:46:34 +00:00

Merge pull request 'fix: bug: disinto init --backend=nomad — does not bootstrap Forgejo admin user (#1069)' (#1073) from fix/issue-1069 into main

Sign in to join this conversation.

Reviewers

No reviewers

review-bot

Labels

Clear labels   

action

  

backlog

  

blocked

  

bug-report

  

cannot-reproduce

  

in-progress

  

in-triage

  

needs-triage

  

prediction/actioned

  

prediction/dismissed

  

prediction/unreviewed

  

priority

  

rejected

  

reproduced

  

tech-debt

  

underspecified

  

vision

No labels

action

backlog

blocked

bug-report

cannot-reproduce

in-progress

in-triage

needs-triage

prediction/actioned

prediction/dismissed

prediction/unreviewed

priority

rejected

reproduced

tech-debt

underspecified

vision

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

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: disinto-admin/disinto#1073

No description provided.