olympus/infra-forge

Fork 0

forgejo role: fix §6 guard so per-repo Actions is actually enabled #6

Merged

pit merged 3 commits from fix/actions-guard-repo-list into main

2026-10-06 08:03:22 +00:00

pit commented

2026-10-06 07:25:42 +00:00

Owner

Follow-up to the prod apply of #5. The per-repo Actions flag (has_actions) did not get enabled: repo_unit has no row of type 10 for pit/infra-forge and no ansible-has-actions-* token was ever minted.

Root cause

community.postgresql's list_to_pg_array (module_utils/postgres.py) converts a list named_arg by str()-ing it and stripping only the brackets, so forgejo_actions_repos (['pit/infra-forge']) is sent as the literal {'pit/infra-forge'}. Its first element is the single-quoted 17-byte string 'pit/infra-forge', which never equals the DB's 15-byte pit/infra-forge. = ANY(%(repos)s) therefore matches nothing, the guard reports "no repo lacks Actions", and the whole enablement block is skipped on every run.

Verified live on prod guest 141 (collection v4.2.0): the literal form returns the row (and eq_literal=true), the list form returns 0 rows, element hex 27...27 (quotes included).

Fix

Pass a scalar — forgejo_actions_repos | join(',') — and split it back into an array in SQL with string_to_array(%(repos)s, ','). Repo slugs are owner/name, so , is not a legal character in one. unit_type is a scalar int and was never affected.

After applying: the guard returns pit/infra-forge, the block mints the transient token, PATCHes has_actions: true, and deletes the token.

Refs #3.

Follow-up to the prod apply of #5. The per-repo Actions flag (`has_actions`) did **not** get enabled: `repo_unit` has no row of type 10 for `pit/infra-forge` and no `ansible-has-actions-*` token was ever minted. ## Root cause `community.postgresql`'s `list_to_pg_array` (`module_utils/postgres.py`) converts a list `named_arg` by `str()`-ing it and stripping only the brackets, so `forgejo_actions_repos` (`['pit/infra-forge']`) is sent as the literal `{'pit/infra-forge'}`. Its first element is the single-quoted 17-byte string `'pit/infra-forge'`, which never equals the DB's 15-byte `pit/infra-forge`. `= ANY(%(repos)s)` therefore matches nothing, the guard reports "no repo lacks Actions", and the whole enablement block is skipped on every run. Verified live on prod guest 141 (collection v4.2.0): the literal form returns the row (and `eq_literal=true`), the list form returns 0 rows, element hex `27...27` (quotes included). ## Fix Pass a scalar — `forgejo_actions_repos | join(',')` — and split it back into an array in SQL with `string_to_array(%(repos)s, ',')`. Repo slugs are `owner/name`, so `,` is not a legal character in one. `unit_type` is a scalar int and was never affected. After applying: the guard returns `pit/infra-forge`, the block mints the transient token, PATCHes `has_actions: true`, and deletes the token. Refs #3.

pit added 1 commit

2026-10-06 07:25:42 +00:00

forgejo role: fix §6 guard so per-repo Actions is actually enabled 35e166a24c

The guard query's `= ANY(%(repos)s)` never matched: community.postgresql's
list_to_pg_array (module_utils/postgres.py) str()s a list named_arg and only
strips the brackets, so `forgejo_actions_repos` (['pit/infra-forge']) was sent
as the literal {'pit/infra-forge'} whose element is the single-quoted 17-byte
"'pit/infra-forge'" -- no row matches. The guard therefore reported "no repo
lacks Actions", the enablement block was skipped on every run, and has_actions
stayed false on pit/infra-forge (verified live on prod guest 141, collection
v4.2.0: literal form returns the repo, the list form returns zero rows).

Pass a scalar (comma-joined) and split it back into an array in SQL. Repo slugs
are owner/name, so ',' is not a legal character in one. Verified: the guard now
returns the repo and the block is entered.

pit added 1 commit

2026-10-06 07:44:14 +00:00

forgejo role: address §6 review — pin separator, assert, honest arg name bbc1fdcaab

Follow-up to 35e166a, from the two-axis review of the branch:

- Data Clumps / Duplicated Code (Standards): the separator ',' was written in
  two places that had to agree (join(',') in named_args, string_to_array(...,','))
  in the query). Hoisted to forgejo_actions_repos_sep in role defaults, the
  convention-4 "pin in one place" spirit applied to a value the two halves share.
- Mysterious Name (Standards): the named_arg was still called `repos` while
  holding a comma-joined scalar. Renamed to `repos_csv`.
- Silent failure survives on malformed input (Spec): the join/split cannot
  represent a slug containing the separator, and would skip it exactly as
  silently as the bug being fixed. Added an assert that fails the run before the
  guard if any slug contains the separator. Verified both directions on prod:
  the real config passes ("All assertions passed") and an injected bad,slug
  fails with the intended message.

Primitive Obsession is answered, not silenced, in the guard comment: the list is
joined only because community.postgresql mis-quotes list args, and the assertion
now guards the one input that would make the encoding lossy.

pit commented

2026-10-06 07:44:18 +00:00

Author

Owner

Two-axis review findings, addressed:

Standards — all three were judgement calls; all now addressed

Data Clumps / Duplicated Code (the , written in two places that must agree): hoisted to forgejo_actions_repos_sep in role defaults. join(forgejo_actions_repos_sep) and string_to_array(..., %(sep)s) now read the same value.
Mysterious Name (repos holding a scalar): renamed repos_csv.
Primitive Obsession (list demoted to a scalar to dodge the collection bug): kept, but now answered rather than silenced — the guard comment records why the list is joined, and the new assertion guards the one input that would make the encoding lossy.

Spec

Silent failure survives on malformed input: fixed. A bad,slug could be split and silently skipped, exactly like the bug being fixed. Added an assert that fails the run before the guard if any slug contains the separator. Verified both directions against prod: real config → All assertions passed; injected bad,slug → fails with the intended message.
"ships no proof the PATCH now yields has_actions:true" and "no idempotence/second-run evidence": not addressable in this diff. This is a guard-query repair only; whether has_actions flips and whether a second run is clean is observable only by running the playbook against the guest. Tracked as the post-merge step on #3, not claimable from a code change.
"correctness rests on an in-comment claim": the claim is reproducible — on v4.2.0 the literal/1-element form returns the row and the list form returns zero rows; the mechanism is list_to_pg_array at module_utils/postgres.py:529 (str(elem).strip('[]')). No test harness exists in this repo for a live-DB query, so the comment is the record.

Not done, deliberately: the issue's closing smoke test (push a trivial workflow, observe a green run) and the Prod second-run. Both are runtime observations on the live guest, not code changes; both remain open on #3.

Two-axis review findings, addressed: **Standards — all three were judgement calls; all now addressed** - *Data Clumps / Duplicated Code* (the `,` written in two places that must agree): hoisted to `forgejo_actions_repos_sep` in role defaults. `join(forgejo_actions_repos_sep)` and `string_to_array(..., %(sep)s)` now read the same value. - *Mysterious Name* (`repos` holding a scalar): renamed `repos_csv`. - *Primitive Obsession* (list demoted to a scalar to dodge the collection bug): kept, but now answered rather than silenced — the guard comment records why the list is joined, and the new assertion guards the one input that would make the encoding lossy. **Spec** - *Silent failure survives on malformed input*: fixed. A `bad,slug` could be split and silently skipped, exactly like the bug being fixed. Added an `assert` that fails the run before the guard if any slug contains the separator. Verified both directions against prod: real config → `All assertions passed`; injected `bad,slug` → fails with the intended message. - *"ships no proof the PATCH now yields has_actions:true"* and *"no idempotence/second-run evidence"*: **not addressable in this diff.** This is a guard-query repair only; whether `has_actions` flips and whether a second run is clean is observable only by running the playbook against the guest. Tracked as the post-merge step on #3, not claimable from a code change. - *"correctness rests on an in-comment claim"*: the claim is reproducible — on v4.2.0 the literal/1-element form returns the row and the list form returns zero rows; the mechanism is `list_to_pg_array` at `module_utils/postgres.py:529` (`str(elem).strip('[]')`). No test harness exists in this repo for a live-DB query, so the comment is the record. **Not done, deliberately:** the issue's closing smoke test (`push a trivial workflow, observe a green run`) and the Prod second-run. Both are runtime observations on the live guest, not code changes; both remain open on #3.

pit added 1 commit

2026-10-06 07:56:33 +00:00

forgejo role: fix token cleanup (401) and stale apt index, found on PRE 8c504e12df

Both found by rehearsing the §6 fix on a fresh PRE guest (142), which is the
first time the enablement block ever executed -- the guard bug had always
skipped it.

1. Cleanup 401'd and left a live admin token behind. The delete called
   DELETE /users/{u}/tokens/{name}, the *admin* route: reqBasicOrRevProxyAuth
   rejects token auth there ("auth method not allowed", v15), and v15 has no
   self-service /user/tokens/{name} either (404, verified live). The run failed
   at the last step and the 40-hex admin token stayed in the UI -- the exact
   leftover minting-per-run exists to avoid. Delete the access_token row
   directly instead, over the postgres connection the probes already use.

2. First task failed on a fresh guest: the template's apt index is stale and
   `apt install git` 404s on a version it still names (libcurl3t64-gnutls
   ...deb13u4). Added one cache refresh at the top of §1 (cache_valid_time
   3600, so re-runs inside the hour skip the network) and dropped the lone
   update_cache from the postgres task.

Verified end-to-end on a destroyed-and-recreated PRE guest: full run green
(29 ok / 0 failed), §6 flips has_actions False->True, the transient token is
deleted (0 leftover rows), and a second run is 0 changed.

pit force-pushed fix/actions-guard-repo-list from 8c504e12df to 3cd28c59e0

2026-10-06 07:57:04 +00:00

Compare

pit commented

2026-10-06 07:57:09 +00:00

Author

Owner

Tested on PRE (fresh LXC 142, destroyed and recreated). Answering the question directly: the earlier verification was check-mode + read-only DB probes only — that was not a real PRE run, and it missed two bugs because the enablement block had never actually executed. A proper rehearsal found them.

Verified on PRE, end-to-end

Full role run on a genuinely fresh guest: green (ok=29 changed=22 failed=0).
§6 now actually runs: guard returns the repo → transient token minted → PATCH has_actions: true → has_actions flips False→True (confirmed via the API).
Idempotence: second run changed=0 (story 12 satisfied for this section).
Negative test of the new assertion: a slug containing the separator fails the run loudly instead of being silently skipped.

Two further bugs the rehearsal exposed (both fixed on this branch)

Transient token cleanup 401'd, leaving a live admin token. DELETE /users/{u}/tokens/{name} is the admin route and rejects token auth (auth method not allowed); v15 has no self-service /user/tokens/{name} either (404). The last task of the block failed and the 40-hex admin token stayed in access_token — the exact leftover per-run minting is meant to prevent. Now the access_token row is deleted directly over the postgres connection the probes already use. Verified: 0 leftover ansible-has-actions-* rows after a run.
First task failed on a fresh guest. The template's apt index is stale and apt install git 404s on a version it still names (libcurl3t64-gnutls ...deb13u4). Added one update_cache refresh at the top of §1 (cache_valid_time: 3600).

Still not done (runtime, not code): the issue's closing smoke test — push a trivial .forgejo/workflows/ file and observe a green run with logs in the UI — has not been run. It remains the last gate on #3 after merge.

PRE guest 142 has been destroyed and the parked rig files restored to the forgejo-pre branch.

Tested on PRE (fresh LXC 142, destroyed and recreated). Answering the question directly: the earlier verification was check-mode + read-only DB probes only — that was **not** a real PRE run, and it missed two bugs because the enablement block had never actually executed. A proper rehearsal found them. **Verified on PRE, end-to-end** - Full role run on a genuinely fresh guest: green (`ok=29 changed=22 failed=0`). - §6 now actually runs: guard returns the repo → transient token minted → `PATCH has_actions: true` → `has_actions` flips `False`→`True` (confirmed via the API). - Idempotence: second run `changed=0` (story 12 satisfied for this section). - Negative test of the new assertion: a slug containing the separator fails the run loudly instead of being silently skipped. **Two further bugs the rehearsal exposed (both fixed on this branch)** 1. *Transient token cleanup 401'd, leaving a live admin token.* `DELETE /users/{u}/tokens/{name}` is the admin route and rejects token auth (`auth method not allowed`); v15 has no self-service `/user/tokens/{name}` either (404). The last task of the block failed and the 40-hex admin token stayed in `access_token` — the exact leftover per-run minting is meant to prevent. Now the `access_token` row is deleted directly over the postgres connection the probes already use. Verified: 0 leftover `ansible-has-actions-*` rows after a run. 2. *First task failed on a fresh guest.* The template's apt index is stale and `apt install git` 404s on a version it still names (`libcurl3t64-gnutls ...deb13u4`). Added one `update_cache` refresh at the top of §1 (`cache_valid_time: 3600`). **Still not done (runtime, not code):** the issue's closing smoke test — push a trivial `.forgejo/workflows/` file and observe a green run with logs in the UI — has not been run. It remains the last gate on #3 after merge. PRE guest 142 has been destroyed and the parked rig files restored to the `forgejo-pre` branch.

pit merged commit 85366ee903 into main

2026-10-06 08:03:22 +00:00

pit deleted branch fix/actions-guard-repo-list

2026-10-06 08:03:22 +00:00

pit referenced this pull request from a commit

2026-10-06 08:03:24 +00:00

forgejo role: fix §6 guard so per-repo Actions is actually enabled (#6)

pit referenced this pull request

2026-10-06 08:05:00 +00:00

Enable Forgejo Actions: app.ini toggle, per-repo flag, pinned runner on the Prod Guest #3

pit referenced this pull request

2026-10-06 08:28:37 +00:00