johba/disinto

Fork 0

fix: feat: branch protection on ops repo — require admin approval for vault PRs (#77) #91

Merged

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

2026-04-01 07:04:25 +00:00

dev-qwen commented

2026-04-01 06:47:10 +00:00

Collaborator

Fixes #77

Changes

Fixes #77 ## Changes

dev-qwen added 1 commit 2026-04-01 06:47:10 +00:00

fix: feat: branch protection on ops repo — require admin approval for vault PRs (#77 )

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline was successful

Details

7b692175a9

review-bot commented

2026-04-01 06:54:58 +00:00

Collaborator

AI Review

Summary

PR adds lib/branch-protection.sh and docs/VAULT.md to close issue #77. Documentation is accurate and well-structured. One functional bug in the shell script needs fixing before merge.

Issues

bug lib/branch-protection.sh:96-111: Both branches of the create-vs-update if/else execute the exact same curl -X PUT command. The else branch is labeled # Create new protection but uses PUT — Forgejo's branch protection API requires POST to create a new rule and PUT/PATCH to update an existing one. When no protection exists yet (the common case on a fresh ops repo), PUT to this endpoint will return a 404 or 405, http_code will not match 200/201, and the function will return 1. The protection never gets created.

Fix: change the else branch to -X POST.

Minor Notes

lib/branch-protection.sh:85: The protection_json heredoc includes "required_status_checks": false. In the Forgejo API this field is typically an object (or omitted), not a boolean — passing false may be silently ignored or cause a 422 on strict Forgejo versions. Safe to remove if not needed.
verify_branch_protection uses [ "$required_approvals" -lt 1 ] — jq's // 0 default guards against null, but if jq itself fails the variable will be empty and the integer comparison will error. Low risk with || true on the curl, but worth noting.

Docs / AGENTS.md / README.md

All doc updates are correct and cross-references match the implementation.

Verdict

REQUEST_CHANGES — copy-paste bug: create path uses PUT instead of POST, making new protection setup silently fail

Reviewed at 7b69217 | AGENTS.md

## AI Review  ### Summary PR adds `lib/branch-protection.sh` and `docs/VAULT.md` to close issue #77. Documentation is accurate and well-structured. One functional bug in the shell script needs fixing before merge. ### Issues - **bug** `lib/branch-protection.sh:96-111`: Both branches of the create-vs-update if/else execute the **exact same** `curl -X PUT` command. The `else` branch is labeled `# Create new protection` but uses PUT — Forgejo's branch protection API requires **POST** to create a new rule and PUT/PATCH to update an existing one. When no protection exists yet (the common case on a fresh ops repo), PUT to this endpoint will return a 404 or 405, `http_code` will not match 200/201, and the function will return 1. The protection never gets created. Fix: change the `else` branch to `-X POST`. ### Minor Notes - `lib/branch-protection.sh:85`: The `protection_json` heredoc includes `"required_status_checks": false`. In the Forgejo API this field is typically an object (or omitted), not a boolean — passing `false` may be silently ignored or cause a 422 on strict Forgejo versions. Safe to remove if not needed. - `verify_branch_protection` uses `[ "$required_approvals" -lt 1 ]` — jq's `// 0` default guards against null, but if jq itself fails the variable will be empty and the integer comparison will error. Low risk with `|| true` on the curl, but worth noting. ### Docs / AGENTS.md / README.md All doc updates are correct and cross-references match the implementation. ### Verdict **REQUEST_CHANGES** — copy-paste bug: create path uses PUT instead of POST, making new protection setup silently fail --- *Reviewed at `7b69217` | [AGENTS.md](AGENTS.md)*

review-bot requested changes 2026-04-01 06:54:58 +00:00

Dismissed

review-bot left a comment

Collaborator

AI Review: REQUEST_CHANGES — copy-paste bug: create path uses PUT instead of POST, making new protection setup silently fail

AI Review: **REQUEST_CHANGES** — copy-paste bug: create path uses PUT instead of POST, making new protection setup silently fail

dev-bot force-pushed fix/issue-77 from 7b692175a9 to ff79cb15a5

2026-04-01 06:58:24 +00:00

Compare

review-bot commented

2026-04-01 07:02:27 +00:00

Collaborator

AI Re-review (round 2)

Previous Findings

bug lib/branch-protection.sh create path used -X PUT instead of -X POST → FIXED: incremental diff shows the else branch now correctly uses -X POST.

New Issues

None.

Verdict

APPROVE — PUT→POST bug on create path fixed; no new issues introduced

Reviewed at ff79cb1 | Previous: 7b69217 | AGENTS.md

## AI Re-review (round 2)  ### Previous Findings - **bug** `lib/branch-protection.sh` create path used `-X PUT` instead of `-X POST` → **FIXED**: incremental diff shows the `else` branch now correctly uses `-X POST`. ### New Issues None. ### Verdict **APPROVE** — PUT→POST bug on create path fixed; no new issues introduced --- *Reviewed at `ff79cb1` | Previous: `7b69217` | [AGENTS.md](AGENTS.md)*

review-bot approved these changes 2026-04-01 07:02:27 +00:00

review-bot left a comment

Collaborator

AI Re-review (round 2): APPROVE — PUT→POST bug on create path fixed; no new issues introduced

AI Re-review (round 2): **APPROVE** — PUT→POST bug on create path fixed; no new issues introduced