disinto-admin/disinto

Fork 0

fix: feat: vault PRs should auto-merge after approval (#170) #173

Merged

dev-qwen merged 2 commits from fix/issue-170 into main

2026-04-03 06:42:54 +00:00

dev-qwen commented

2026-04-03 06:29:50 +00:00

Collaborator

Fixes #170

Changes

Fixes #170 ## Changes

dev-qwen added 1 commit 2026-04-03 06:29:51 +00:00

fix: feat: vault PRs should auto-merge after approval (#170 )

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

7b9c483477

review-bot commented

2026-04-03 06:33:39 +00:00

Collaborator

AI Review

Summary

This PR adds auto-merge to vault PRs so that human approval triggers automatic merge without a separate merge click. The intent is correct and the approach (calling the Forgejo merge API right after PR creation) is the right pattern.

However, there is a critical bug in the API payload that renders the feature entirely non-functional.

Findings

bug lib/vault.sh:196 / bin/disinto:2983: "merge_when_checks_succeed":false is the wrong flag value. With false, the API call attempts an immediate merge (not a scheduled/auto-merge). The PR was just created with no approval, so Forgejo rejects the request (HTTP 405 or similar) and the error is silently swallowed by the || { _vault_log ... } handler. Auto-merge is never actually scheduled. The correct value is true — that is the Forgejo signal to schedule the merge once the required approval conditions are satisfied. Docs in VAULT.md describe the auto-merge behaviour as if it works, but it will never trigger.

Required Fix

Change both occurrences of the payload from:

{"Do":"merge","merge_when_checks_succeed":false}

to:

{"Do":"merge","merge_when_checks_succeed":true}

One occurrence is in lib/vault.sh (the main vault_request helper) and one is in bin/disinto (the release vault path). Both must be fixed.

Verdict

REQUEST_CHANGES — merge_when_checks_succeed is false — this attempts an immediate merge (which fails and is swallowed), not auto-merge after approval; the feature is broken as shipped

Reviewed at 7b9c483 | AGENTS.md

## AI Review  ### Summary This PR adds auto-merge to vault PRs so that human approval triggers automatic merge without a separate merge click. The intent is correct and the approach (calling the Forgejo merge API right after PR creation) is the right pattern. However, there is a critical bug in the API payload that renders the feature entirely non-functional. ### Findings - **bug** `lib/vault.sh:196` / `bin/disinto:2983`: `"merge_when_checks_succeed":false` is the wrong flag value. With `false`, the API call attempts an **immediate merge** (not a scheduled/auto-merge). The PR was just created with no approval, so Forgejo rejects the request (HTTP 405 or similar) and the error is silently swallowed by the `|| { _vault_log ... }` handler. Auto-merge is **never actually scheduled**. The correct value is `true` — that is the Forgejo signal to schedule the merge once the required approval conditions are satisfied. Docs in `VAULT.md` describe the auto-merge behaviour as if it works, but it will never trigger. ### Required Fix Change both occurrences of the payload from: ```json {"Do":"merge","merge_when_checks_succeed":false} ``` to: ```json {"Do":"merge","merge_when_checks_succeed":true} ``` One occurrence is in `lib/vault.sh` (the main vault_request helper) and one is in `bin/disinto` (the release vault path). Both must be fixed. ### Verdict **REQUEST_CHANGES** — merge_when_checks_succeed is false — this attempts an immediate merge (which fails and is swallowed), not auto-merge after approval; the feature is broken as shipped --- *Reviewed at `7b9c483` | [AGENTS.md](AGENTS.md)*

review-bot requested changes 2026-04-03 06:33:40 +00:00

Dismissed

review-bot left a comment

Collaborator

AI Review: REQUEST_CHANGES — merge_when_checks_succeed is false — this attempts an immediate merge (which fails and is swallowed), not auto-merge after approval; the feature is broken as shipped

AI Review: **REQUEST_CHANGES** — merge_when_checks_succeed is false — this attempts an immediate merge (which fails and is swallowed), not auto-merge after approval; the feature is broken as shipped

dev-bot added 1 commit 2026-04-03 06:37:20 +00:00

fix: correct merge_when_checks_succeed to true for auto-merge (#170 )

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

a916904e76

review-bot commented

2026-04-03 06:42:25 +00:00

Collaborator

AI Re-review (round 2)

Previous Findings

bug lib/vault.sh:196 / bin/disinto:2983: merge_when_checks_succeed:false attempted immediate merge instead of scheduling auto-merge → FIXED: both occurrences updated to merge_when_checks_succeed:true

New Issues

None.

Verdict

APPROVE — previous finding fixed: merge_when_checks_succeed is now true in both lib/vault.sh and bin/disinto

Reviewed at a916904 | Previous: 7b9c483 | AGENTS.md

## AI Re-review (round 2)  ### Previous Findings - **bug** `lib/vault.sh:196` / `bin/disinto:2983`: `merge_when_checks_succeed:false` attempted immediate merge instead of scheduling auto-merge → **FIXED**: both occurrences updated to `merge_when_checks_succeed:true` ### New Issues None. ### Verdict **APPROVE** — previous finding fixed: merge_when_checks_succeed is now true in both lib/vault.sh and bin/disinto --- *Reviewed at `a916904` | Previous: `7b9c483` | [AGENTS.md](AGENTS.md)*

review-bot approved these changes 2026-04-03 06:42:26 +00:00

review-bot left a comment

Collaborator

AI Re-review (round 2): APPROVE — previous finding fixed: merge_when_checks_succeed is now true in both lib/vault.sh and bin/disinto

AI Re-review (round 2): **APPROVE** — previous finding fixed: merge_when_checks_succeed is now true in both lib/vault.sh and bin/disinto