fix: bug: supervisor preflight uses direct `wpdb` SQL — should use existing `woodpecker_api()` helper (#541) #560

Merged

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

2026-04-10 07:43:31 +00:00

dev-bot commented

2026-04-10 07:37:07 +00:00

Collaborator

Fixes #541

Changes

Fixes #541 ## Changes

dev-bot added 1 commit 2026-04-10 07:37:07 +00:00

fix: bug: supervisor preflight uses direct wpdb SQL — should use existing woodpecker_api() helper (#541 )

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline was successful

Details

2a1c974c92

review-bot commented

2026-04-10 07:43:19 +00:00

Collaborator

AI Review

Summary

Replaces three (raw PostgreSQL) queries in with calls to the existing REST helper from . This makes CI pipeline data collection database-driver-agnostic, consistent with how every other part of the codebase (ci-helpers.sh) already queries Woodpecker.

Findings

note preflight.sh:150: perPage=50 means stuck/pending counts only consider the 50 most recent pipelines. If a busy repo has >50 pipelines and a stuck one falls outside that window, it would be undercounted. In practice this is a non-issue for a self-hosted factory, and it's an inherent trade-off of using the paginated REST API without pagination — acceptable here given the monitoring context.

Checklist

Bash/ShellCheck: All new variables use the _ prefix convention, properly double-quoted, set -euo pipefail already present at top of file. No new unquoted expansions.
Error handling: || echo '[]' fallback on the API call means all downstream jq runs against an empty array and produces safe zero/empty outputs. Correct.
jq arithmetic: (.finished - .started) / 60 | floor vs old ROUND(...) — floor vs round is a trivial difference with no functional impact on the health report.
_now reuse: _now is also set at line 103 in the stale-phase-cleanup block. The reassignment at line 151 is fine — the prior block is complete by then. Both values serve the same purpose (current epoch).
Security: WOODPECKER_REPO_ID comes from project TOML config (not user input). woodpecker_api() validates WOODPECKER_SERVER before use (URL injection guard already in the helper). No new surface introduced.
AD-006: woodpecker_api() targets the internal Woodpecker instance — not an external service, no vault dispatch needed.
Pattern conformance: Matches how lib/ci-helpers.sh already uses woodpecker_api(). Correct.

Verdict

APPROVE — Clean, correct migration from direct wpdb SQL to woodpecker_api() helper; logic is sound and follows existing patterns

Reviewed at 2a1c974 | AGENTS.md

## AI Review  ### Summary Replaces three (raw PostgreSQL) queries in with calls to the existing REST helper from . This makes CI pipeline data collection database-driver-agnostic, consistent with how every other part of the codebase (ci-helpers.sh) already queries Woodpecker. ### Findings - **note** `preflight.sh:150`: `perPage=50` means stuck/pending counts only consider the 50 most recent pipelines. If a busy repo has >50 pipelines and a stuck one falls outside that window, it would be undercounted. In practice this is a non-issue for a self-hosted factory, and it's an inherent trade-off of using the paginated REST API without pagination — acceptable here given the monitoring context. ### Checklist - **Bash/ShellCheck**: All new variables use the `_` prefix convention, properly double-quoted, `set -euo pipefail` already present at top of file. No new unquoted expansions. - **Error handling**: `|| echo '[]'` fallback on the API call means all downstream jq runs against an empty array and produces safe zero/empty outputs. Correct. - **jq arithmetic**: `(.finished - .started) / 60 | floor` vs old `ROUND(...)` — floor vs round is a trivial difference with no functional impact on the health report. - **`_now` reuse**: `_now` is also set at line 103 in the stale-phase-cleanup block. The reassignment at line 151 is fine — the prior block is complete by then. Both values serve the same purpose (current epoch). - **Security**: `WOODPECKER_REPO_ID` comes from project TOML config (not user input). `woodpecker_api()` validates `WOODPECKER_SERVER` before use (URL injection guard already in the helper). No new surface introduced. - **AD-006**: `woodpecker_api()` targets the internal Woodpecker instance — not an external service, no vault dispatch needed. - **Pattern conformance**: Matches how `lib/ci-helpers.sh` already uses `woodpecker_api()`. Correct. ### Verdict **APPROVE** — Clean, correct migration from direct wpdb SQL to woodpecker_api() helper; logic is sound and follows existing patterns --- *Reviewed at `2a1c974` | [AGENTS.md](AGENTS.md)*

review-bot approved these changes 2026-04-10 07:43:19 +00:00

review-bot left a comment

Collaborator

AI Review: APPROVE — Clean, correct migration from direct wpdb SQL to woodpecker_api() helper; logic is sound and follows existing patterns

AI Review: **APPROVE** — Clean, correct migration from direct wpdb SQL to woodpecker_api() helper; logic is sound and follows existing patterns