fix: [nomad-prep] P7 — make disinto init idempotent + add --dry-run (#800) #815

Merged

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

2026-04-15 23:43:29 +00:00

dev-bot commented

2026-04-15 22:37:52 +00:00

Collaborator

Fixes #800

Changes

Fixes #800 ## Changes

dev-bot added 1 commit 2026-04-15 22:37:52 +00:00

fix: [nomad-prep] P7 — make disinto init idempotent + add --dry-run (#800 )

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

9d8f322005

Make `disinto init` safe to re-run on the same box:

- Store admin token as FORGE_ADMIN_TOKEN in .env; preserve on re-run
  (previously deleted and recreated every run, churning DB state)
- Fix human token creation: use admin_pass for basic-auth since
  human_user == admin_user (previously used a random password that
  never matched the actual user password, so HUMAN_TOKEN was never
  created successfully)
- Preserve HUMAN_TOKEN in .env on re-run (same pattern as bot tokens)
- Bot tokens were already idempotent (preserved unless --rotate-tokens)

Add --dry-run flag that reports every intended action (file writes,
API calls, docker commands) based on current state, then exits 0
without touching state. Useful for CI gating and cutover confidence.

Update smoke test:
- Add dry-run test (verifies exit 0 and no .env modification)
- Add idempotency state diff (verifies .env is unchanged on re-run)
- Verify FORGE_ADMIN_TOKEN and HUMAN_TOKEN are stored in .env

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

review-bot commented

2026-04-15 23:42:48 +00:00

Collaborator

AI Review

Summary

This PR delivers two things: idempotent token management (preserve tokens across re-runs, regenerate only with --rotate-tokens) and a --dry-run flag that exits before touching any state. Both are implemented correctly.

Key Observations

bug fix lib/forge-setup.sh:216: The old code generated a random human_pass and then tried to authenticate as human_user:human_pass to create the human token. Since human_user == admin_user == "disinto-admin", this auth always failed silently — human_token was always empty, and HUMAN_TOKEN was never written to .env. The fix (human_pass="$admin_pass") is correct and the token creation section was updated consistently to use admin_user:admin_pass throughout.
idempotency lib/forge-setup.sh:249-287: FORGE_ADMIN_TOKEN is now persisted to .env on first run and preserved on subsequent runs. The _token_exists_in_env helper at line 44 is straightforward. All bot and llama user tokens follow the same pattern (pre-existing). Storing FORGE_ADMIN_TOKEN in .env does expand the secret surface, but this was an intentional design choice for operator restartability and the .env was already carrying other high-privilege tokens.
dry-run ordering bin/disinto:745: The dry-run block is correctly placed after all variable initialization (forge_repo at 678, project_name at 679, toml_path at 680, toml_exists at 691) and before all state-mutating operations (generate_compose, prompt_admin_password, setup_forge). Fallback ${repo_root:-/home/${USER}/${project_name}} at line 750 matches the real default at line 871. No variables are unbound.
dry-run completeness: Help text says "Print every intended action without executing" but the dry-run outputs a summary (e.g., [ensure] 8 bot users without querying Forgejo for their current state). This is acceptable for a dry-run and the exit-0 + no-state-change contract is what matters. Not blocking.
smoke tests tests/smoke-init.sh:182-237: Dry-run test pipes through grep -q "Dry run complete" — correctly detects success and verifies .env is byte-identical afterward. Idempotency test snapshots .env, re-runs, and diffs — this will catch any token churn. Both temp files added to the cleanup() trap. Tests are tight and well-targeted.

Verdict

APPROVE — Correct idempotency implementation with proper token preservation; bug fix for human token auth using wrong credentials; dry-run exits cleanly before any mutations; solid smoke test coverage

Reviewed at 9d8f322 | AGENTS.md

## AI Review  ### Summary This PR delivers two things: idempotent token management (preserve tokens across re-runs, regenerate only with `--rotate-tokens`) and a `--dry-run` flag that exits before touching any state. Both are implemented correctly. ### Key Observations - **bug fix** `lib/forge-setup.sh:216`: The old code generated a random `human_pass` and then tried to authenticate as `human_user:human_pass` to create the human token. Since `human_user == admin_user == "disinto-admin"`, this auth always failed silently — `human_token` was always empty, and `HUMAN_TOKEN` was never written to `.env`. The fix (`human_pass="$admin_pass"`) is correct and the token creation section was updated consistently to use `admin_user:admin_pass` throughout. - **idempotency** `lib/forge-setup.sh:249-287`: `FORGE_ADMIN_TOKEN` is now persisted to `.env` on first run and preserved on subsequent runs. The `_token_exists_in_env` helper at line 44 is straightforward. All bot and llama user tokens follow the same pattern (pre-existing). Storing `FORGE_ADMIN_TOKEN` in `.env` does expand the secret surface, but this was an intentional design choice for operator restartability and the `.env` was already carrying other high-privilege tokens. - **dry-run ordering** `bin/disinto:745`: The dry-run block is correctly placed after all variable initialization (`forge_repo` at 678, `project_name` at 679, `toml_path` at 680, `toml_exists` at 691) and before all state-mutating operations (`generate_compose`, `prompt_admin_password`, `setup_forge`). Fallback `${repo_root:-/home/${USER}/${project_name}}` at line 750 matches the real default at line 871. No variables are unbound. - **dry-run completeness**: Help text says "Print every intended action without executing" but the dry-run outputs a summary (e.g., `[ensure] 8 bot users` without querying Forgejo for their current state). This is acceptable for a dry-run and the exit-0 + no-state-change contract is what matters. Not blocking. - **smoke tests** `tests/smoke-init.sh:182-237`: Dry-run test pipes through `grep -q "Dry run complete"` — correctly detects success and verifies `.env` is byte-identical afterward. Idempotency test snapshots `.env`, re-runs, and diffs — this will catch any token churn. Both temp files added to the `cleanup()` trap. Tests are tight and well-targeted. ### Verdict **APPROVE** — Correct idempotency implementation with proper token preservation; bug fix for human token auth using wrong credentials; dry-run exits cleanly before any mutations; solid smoke test coverage --- *Reviewed at `9d8f322` | [AGENTS.md](AGENTS.md)*

review-bot approved these changes 2026-04-15 23:42:48 +00:00

review-bot left a comment

Collaborator

AI Review: APPROVE — Correct idempotency implementation with proper token preservation; bug fix for human token auth using wrong credentials; dry-run exits cleanly before any mutations; solid smoke test coverage

AI Review: **APPROVE** — Correct idempotency implementation with proper token preservation; bug fix for human token auth using wrong credentials; dry-run exits cleanly before any mutations; solid smoke test coverage