bug: dispatcher PR lookup fails — --diff-filter=A misses merge commits #129

New issue

Closed

opened 2026-04-01 20:06:18 +00:00 by dev-bot · 0 comments

dev-bot commented

2026-04-01 20:06:18 +00:00

Collaborator

Problem

The dispatcher's get_pr_for_file() function cannot find the PR that introduced a vault action TOML file. It logs WARNING: No PR found for action ... — skipping (possible direct push) even when the file arrived via a properly merged PR.

Root cause

get_pr_for_file() uses:

git log --oneline --diff-filter=A -- "vault/actions/${file_name}"

--diff-filter=A returns the commit that directly added the file (e.g. 4d43b96 vault: release v0.2.0). This is the commit on the PR branch, not the merge commit. The function then checks if this commit message contains "Merge pull request" — it doesn't, because it's a regular commit.

The actual merge commit (1ad1326 Merge pull request ... (#3)) is a separate commit that doesn't show up with --diff-filter=A.

Fix

Replace the current approach with a two-step lookup:

get_pr_for_file() {
  local file_path="$1"
  local file_name
  file_name=$(basename "$file_path")

  # Step 1: find the commit that added the file
  local add_commit
  add_commit=$(git -C "$OPS_REPO_ROOT" log --diff-filter=A --format="%H"     -- "vault/actions/${file_name}" 2>/dev/null | head -1)

  if [ -z "$add_commit" ]; then
    return 1
  fi

  # Step 2: find the merge commit that contains it
  local merge_line
  merge_line=$(git -C "$OPS_REPO_ROOT" log --merges --ancestry-path     "${add_commit}..HEAD" --oneline 2>/dev/null | head -1)

  if [ -z "$merge_line" ]; then
    return 1
  fi

  # Step 3: extract PR number from merge commit message
  # Forgejo format: "Merge pull request 'title' (#N) from branch into main"
  local pr_num
  pr_num=$(echo "$merge_line" | grep -oP '#\d+' | head -1 | tr -d '#')

  if [ -n "$pr_num" ]; then
    echo "$pr_num"
    return 0
  fi

  return 1
}

Tested inside the edge container:

$ git log --diff-filter=A --format="%H" -- vault/actions/release-v0.2.0.toml | head -1
4d43b96...

$ git log --merges --ancestry-path 4d43b96..HEAD --oneline | head -1
1ad1326 Merge pull request 'vault: release v0.2.0' (#3) from vault/release-v0.2.0-proper into main

Also fix: empty secrets should not be an error

The dispatcher currently fails with ERROR: Action ... has no secrets declared when secrets = []. An action with no secrets (like a release tag) is valid — the runner just doesn't get any extra env vars. Remove this check or make it a no-op for empty arrays.

Also fix: edge container env vars

The edge service in docker-compose.yml (and bin/disinto generate_compose) needs these environment variables for the dispatcher to function:

FORGE_URL: http://forgejo:3000
FORGE_REPO: johba/disinto
FORGE_OPS_REPO: dev-bot/disinto-ops
FORGE_TOKEN: ${FORGE_TOKEN:-}
OPS_REPO_ROOT: /opt/disinto-ops
PROJECT_REPO_ROOT: /opt/disinto
PRIMARY_BRANCH: main
DISINTO_VERSION: ${DISINTO_VERSION:-main}

And the entrypoint needs export USER="${USER:-root}" before sourcing lib/env.sh (Alpine doesn't set USER).

Also fix: `FORGE_API` construction

The dispatcher sources lib/env.sh which builds FORGE_API from FORGE_URL and FORGE_REPO. But for ops repo API calls, the dispatcher uses FORGE_API which points to the disinto repo, not the ops repo. PR lookup calls like get_pr_merger use FORGE_API but should use ${FORGE_URL}/api/v1/repos/${FORGE_OPS_REPO}.

Affected files

docker/edge/dispatcher.sh (fix get_pr_for_file, remove empty-secrets error, fix API URL for ops repo)
docker/edge/entrypoint-edge.sh (add USER export)
docker-compose.yml / bin/disinto generate_compose (add env vars to edge service)

Acceptance criteria

get_pr_for_file() finds PRs via ancestry-path merge lookup
Actions with secrets = [] pass validation
Edge container has required env vars
Entrypoint sets USER before sourcing env.sh
Dispatcher uses ops repo API URL for PR lookups (not disinto repo)
End-to-end: vault TOML via merged PR is detected, validated, and runner launched
CI green

## Problem The dispatcher's `get_pr_for_file()` function cannot find the PR that introduced a vault action TOML file. It logs `WARNING: No PR found for action ... — skipping (possible direct push)` even when the file arrived via a properly merged PR. ## Root cause `get_pr_for_file()` uses: ```bash git log --oneline --diff-filter=A -- "vault/actions/${file_name}" ``` `--diff-filter=A` returns the commit that **directly added** the file (e.g. `4d43b96 vault: release v0.2.0`). This is the commit on the PR branch, not the merge commit. The function then checks if this commit message contains "Merge pull request" — it doesn't, because it's a regular commit. The actual merge commit (`1ad1326 Merge pull request ... (#3)`) is a separate commit that doesn't show up with `--diff-filter=A`. ## Fix Replace the current approach with a two-step lookup: ```bash get_pr_for_file() { local file_path="$1" local file_name file_name=$(basename "$file_path") # Step 1: find the commit that added the file local add_commit add_commit=$(git -C "$OPS_REPO_ROOT" log --diff-filter=A --format="%H" -- "vault/actions/${file_name}" 2>/dev/null | head -1) if [ -z "$add_commit" ]; then return 1 fi # Step 2: find the merge commit that contains it local merge_line merge_line=$(git -C "$OPS_REPO_ROOT" log --merges --ancestry-path "${add_commit}..HEAD" --oneline 2>/dev/null | head -1) if [ -z "$merge_line" ]; then return 1 fi # Step 3: extract PR number from merge commit message # Forgejo format: "Merge pull request 'title' (#N) from branch into main" local pr_num pr_num=$(echo "$merge_line" | grep -oP '#\d+' | head -1 | tr -d '#') if [ -n "$pr_num" ]; then echo "$pr_num" return 0 fi return 1 } ``` Tested inside the edge container: ``` $ git log --diff-filter=A --format="%H" -- vault/actions/release-v0.2.0.toml | head -1 4d43b96... $ git log --merges --ancestry-path 4d43b96..HEAD --oneline | head -1 1ad1326 Merge pull request 'vault: release v0.2.0' (#3) from vault/release-v0.2.0-proper into main ``` ## Also fix: empty secrets should not be an error The dispatcher currently fails with `ERROR: Action ... has no secrets declared` when `secrets = []`. An action with no secrets (like a release tag) is valid — the runner just doesn't get any extra env vars. Remove this check or make it a no-op for empty arrays. ## Also fix: edge container env vars The edge service in docker-compose.yml (and `bin/disinto` generate_compose) needs these environment variables for the dispatcher to function: - `FORGE_URL: http://forgejo:3000` - `FORGE_REPO: johba/disinto` - `FORGE_OPS_REPO: dev-bot/disinto-ops` - `FORGE_TOKEN: ${FORGE_TOKEN:-}` - `OPS_REPO_ROOT: /opt/disinto-ops` - `PROJECT_REPO_ROOT: /opt/disinto` - `PRIMARY_BRANCH: main` - `DISINTO_VERSION: ${DISINTO_VERSION:-main}` And the entrypoint needs `export USER="${USER:-root}"` before sourcing lib/env.sh (Alpine doesn't set USER). ## Also fix: `FORGE_API` construction The dispatcher sources lib/env.sh which builds `FORGE_API` from `FORGE_URL` and `FORGE_REPO`. But for ops repo API calls, the dispatcher uses `FORGE_API` which points to the disinto repo, not the ops repo. PR lookup calls like `get_pr_merger` use `FORGE_API` but should use `${FORGE_URL}/api/v1/repos/${FORGE_OPS_REPO}`. ## Affected files - `docker/edge/dispatcher.sh` (fix `get_pr_for_file`, remove empty-secrets error, fix API URL for ops repo) - `docker/edge/entrypoint-edge.sh` (add `USER` export) - `docker-compose.yml` / `bin/disinto` generate_compose (add env vars to edge service) ## Acceptance criteria - [ ] `get_pr_for_file()` finds PRs via ancestry-path merge lookup - [ ] Actions with `secrets = []` pass validation - [ ] Edge container has required env vars - [ ] Entrypoint sets USER before sourcing env.sh - [ ] Dispatcher uses ops repo API URL for PR lookups (not disinto repo) - [ ] End-to-end: vault TOML via merged PR is detected, validated, and runner launched - [ ] CI green