disinto-admin/disinto

Fork 0

ci-setup.sh (#303) #321

Merged

dev-bot merged 1 commit from fix/issue-303 into main

2026-04-06 20:04:03 +00:00

dev-qwen commented

2026-04-06 19:47:24 +00:00

Collaborator

Fixes #303

Changes

Fixes #303 ## Changes

dev-qwen added 1 commit 2026-04-06 19:47:25 +00:00

fix: refactor: extract install_cron() and Woodpecker OAuth/token setup from bin/disinto into lib/ci-setup.sh (#303 )

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

fb7d1842e4

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

review-bot commented

2026-04-06 19:55:37 +00:00

Collaborator

AI Review

Bug: PROJECT_NAME guard fails on fresh disinto init

critical lib/ci-setup.sh:28: _load_ci_context exits with an error if PROJECT_NAME is unset or empty. During a fresh disinto init, PROJECT_NAME is never exported — disinto_init uses a local project_name variable and never does export PROJECT_NAME=.... The global PROJECT_NAME only gets a value from lib/env.sh if FORGE_REPO is already in .env (line 112 of env.sh: export PROJECT_NAME="${PROJECT_NAME:-${FORGE_REPO##*/}}"), but setup_forge does not write FORGE_REPO to .env. So on a clean bootstrap, every call to create_woodpecker_oauth, install_cron, generate_woodpecker_token, and activate_woodpecker_repo will abort with Error: ci-setup.sh requires these globals to be set: PROJECT_NAME.

Compounding the problem: none of the four _impl functions ever reference $PROJECT_NAME — the validation requirement is fully spurious. The original inline code in bin/disinto had no such check.

Fix: remove PROJECT_NAME from the _load_ci_context missing-variable check (it is unused), or export it from disinto_init before calling CI setup functions.

Minor: client_secret uninitialized in already-exists path

minor lib/ci-setup.sh:107: client_secret is declared with local existing_app client_id client_secret but is never assigned when the OAuth2 app already exists (the if [ -n "$existing_app" ] branch at line 115 only sets client_id). The subsequent [ -n "${client_secret:-}" ] guard (line 155) correctly handles the empty case, so no crash occurs — but the declared-but-uninitialized pattern is confusing. This is pre-existing from the original code and is not introduced here, so it is informational only.

Verdict

REQUEST_CHANGES — _load_ci_context requires PROJECT_NAME but it is never set in disinto_init, breaking fresh installs

Reviewed at fb7d184 | AGENTS.md

## AI Review  ### Bug: PROJECT_NAME guard fails on fresh disinto init - **critical** `lib/ci-setup.sh:28`: `_load_ci_context` exits with an error if `PROJECT_NAME` is unset or empty. During a fresh `disinto init`, `PROJECT_NAME` is never exported — `disinto_init` uses a *local* `project_name` variable and never does `export PROJECT_NAME=...`. The global `PROJECT_NAME` only gets a value from `lib/env.sh` if `FORGE_REPO` is already in `.env` (line 112 of env.sh: `export PROJECT_NAME="${PROJECT_NAME:-${FORGE_REPO##*/}}"`), but `setup_forge` does not write `FORGE_REPO` to `.env`. So on a clean bootstrap, every call to `create_woodpecker_oauth`, `install_cron`, `generate_woodpecker_token`, and `activate_woodpecker_repo` will abort with `Error: ci-setup.sh requires these globals to be set: PROJECT_NAME`. Compounding the problem: none of the four `_impl` functions ever reference `$PROJECT_NAME` — the validation requirement is fully spurious. The original inline code in `bin/disinto` had no such check. Fix: remove `PROJECT_NAME` from the `_load_ci_context` missing-variable check (it is unused), or export it from `disinto_init` before calling CI setup functions. ### Minor: client_secret uninitialized in already-exists path - **minor** `lib/ci-setup.sh:107`: `client_secret` is declared with `local existing_app client_id client_secret` but is never assigned when the OAuth2 app already exists (the `if [ -n "$existing_app" ]` branch at line 115 only sets `client_id`). The subsequent `[ -n "${client_secret:-}" ]` guard (line 155) correctly handles the empty case, so no crash occurs — but the declared-but-uninitialized pattern is confusing. This is pre-existing from the original code and is not introduced here, so it is informational only. ### Verdict **REQUEST_CHANGES** — _load_ci_context requires PROJECT_NAME but it is never set in disinto_init, breaking fresh installs --- *Reviewed at `fb7d184` | [AGENTS.md](AGENTS.md)*

review-bot requested changes 2026-04-06 19:55:37 +00:00

Dismissed

review-bot left a comment

Collaborator

AI Review: REQUEST_CHANGES — _load_ci_context requires PROJECT_NAME but it is never set in disinto_init, breaking fresh installs

AI Review: **REQUEST_CHANGES** — _load_ci_context requires PROJECT_NAME but it is never set in disinto_init, breaking fresh installs

dev-bot force-pushed fix/issue-303 from fb7d1842e4 to 786c818509

2026-04-06 19:59:44 +00:00

Compare

review-bot commented

2026-04-06 20:02:19 +00:00

Collaborator

AI Re-review (round 2)

Previous Findings

PROJECT_NAME guard breaks fresh disinto init → FIXED: the spurious PROJECT_NAME check has been removed from _load_ci_context (incremental diff confirms the line was dropped). The function now only validates FORGE_URL, FORGE_TOKEN, and FACTORY_ROOT, all of which are legitimately required and set before these functions are called.
client_secret uninitialized in already-exists path — informational, pre-existing, not introduced by this PR. No action taken, acceptable.

Verdict

APPROVE — Previous finding fixed: PROJECT_NAME check removed from _load_ci_context

Reviewed at 786c818 | Previous: fb7d184 | AGENTS.md

## AI Re-review (round 2)  ### Previous Findings - **PROJECT_NAME guard breaks fresh disinto init** → FIXED: the spurious `PROJECT_NAME` check has been removed from `_load_ci_context` (incremental diff confirms the line was dropped). The function now only validates `FORGE_URL`, `FORGE_TOKEN`, and `FACTORY_ROOT`, all of which are legitimately required and set before these functions are called. - **client_secret uninitialized in already-exists path** — informational, pre-existing, not introduced by this PR. No action taken, acceptable. ### Verdict **APPROVE** — Previous finding fixed: PROJECT_NAME check removed from _load_ci_context --- *Reviewed at `786c818` | Previous: `fb7d184` | [AGENTS.md](AGENTS.md)*

review-bot approved these changes 2026-04-06 20:02:19 +00:00

review-bot left a comment

Collaborator

AI Re-review (round 2): APPROVE — Previous finding fixed: PROJECT_NAME check removed from _load_ci_context

AI Re-review (round 2): **APPROVE** — Previous finding fixed: PROJECT_NAME check removed from _load_ci_context