pal-e-docs

Review: Bug: update-kustomize-tag skipped when CI tests fail (re-review)

review-882-2026-04-07-v2 Doc

review ready
Project

Verdict: READY

Re-review after refinement from review-882-2026-04-07 (NEEDS_REFINEMENT). All blocking issues resolved. Remaining items are non-blocking scope recommendations.

Template Completeness

  • [x] Type — Bug
  • [x] Lineage — Standalone, discovered during CRM incident response 2026-04-07
  • [x] Repo — forgejo_admin/pal-e-platform
  • [x] What Broke — clear description with pipeline numbers and root cause
  • [x] Repro Steps — 5-step reproduction
  • [x] Expected Behavior — rewritten for Option A (no longer ambiguous)
  • [x] Design Decision — new section, Option A rationale documented
  • [x] Environment — pipelines #382, #384, #385 identified
  • [x] Acceptance Criteria — 5 concrete, testable criteria
  • [x] File Targets — 2 files with line numbers
  • [ ] Test Expectations — no explicit test commands (nit, not blocker — AC item 4 covers verification intent)
  • [x] Related — project, companion bug #273, deployment-lessons, rollout #206

Traceability

  • [x] story:superuser-deploy — verified in project-pal-e-platform user-stories section. Story: "I can deploy infrastructure changes via tofu plan/apply and see them succeed in Woodpecker CI without manual intervention."
  • [ ] arch:woodpecker — label present on board item but no arch-woodpecker note exists in pal-e-docs. [SCOPE] Create architecture note arch-woodpecker. Non-blocking — the arch label correctly identifies the component even without the backing note.
  • [x] Forgejo issue — #274, open

File Targets

  • [x] scripts/woodpecker-update-tag-step.yaml — verified: canonical template exists (65 lines), line 37-38 shows depends_on: [build-and-push]. This is the correct location for the template fix.
  • [x] ~/basketball-api/.woodpecker.yaml — verified: lines 60-83 show update-kustomize-tag step with depends_on: [build-and-push]. Consumer file correctly identified.

Repo Placement

Issue is filed on pal-e-platform which owns the canonical template. The fix also touches basketball-api's .woodpecker.yaml. The issue correctly identifies both files in File Targets. Since the basketball-api change is a one-line config change that mirrors the template, a single PR in pal-e-platform (template) + a single PR in basketball-api (consumer) is the right approach. The issue body could be clearer that this is a multi-repo change, but File Targets section makes this implicit. Acceptable.

Dependencies

  • #206 — "Rollout: wire update-kustomize-tag step into all 8 app repos" — open. This fix MUST land before rollout continues. Issue body correctly references this. Ordering dependency, not a blocker.
  • #254 — "Woodpecker pipeline restart skips deploy steps" — related but distinct bug. Not a blocker.
  • #273 — "Woodpecker webhook not firing on squash merge" — companion bug, correctly referenced in Related.

Acceptance Criteria

All 5 criteria are agent-actionable:
  • "update-kustomize-tag step uses Woodpecker failure: ignore or when: status: [success, failure]" — concrete Woodpecker config change. failure: ignore is already used in pal-e-platform's own pipeline (line 332), establishing precedent.
  • "Canonical template updated: scripts/woodpecker-update-tag-step.yaml" — verifiable file change.
  • "basketball-api .woodpecker.yaml updated to match" — verifiable file change.
  • "Verify: push with failing test -> image deploys anyway" — integration verification. Agent can't trigger a real pipeline, but can verify the config is correct.
  • "Document decision in deployment-lessons" — verifiable documentation change.
Note on AC #1: The Woodpecker approach needs clarification. failure: ignore on the test step would let the pipeline continue but would also mark the overall pipeline as success even with test failures. The more precise fix is to add failure: ignore to the test step OR restructure depends_on. The agent implementing this should verify Woodpecker's exact semantics. The AC gives two options which is appropriate.

Blast Radius

  • basketball-api — primary consumer, explicitly targeted. Same pattern at lines 79-80.
  • pal-e-docs, westside-app, pal-e-app, twitch-2k-wager — all have update-kustomize-tag with depends_on: [build-and-push]. These repos have the same latent bug but are NOT in scope for this ticket. Issue #206 tracks the broader rollout.
  • twitch-2k-wager has a different pattern: build-and-push depends on test (line 66-68), so test failure blocks the build itself. Different bug surface.
  • Rollback: Straightforward — revert the config change in affected repos.

Decomposition Assessment

  • 2 file targets across 2 repos
  • 5 AC (at the limit but all are small config changes)
  • Template update is a 1-2 line change; consumer update mirrors it
  • deployment-lessons doc update is minimal
  • Estimated: under 5 minutes per repo
No decomposition needed. Two parallel agents (one per repo) or one sequential agent can handle this.

Refinement Delta (from v1)

  • [x] Option A selected and documented in new Design Decision section
  • [x] Story label fixed: story:PLAT-S2 → story:superuser-deploy
  • [x] AC rewritten as 5 concrete, testable statements
  • [x] File Targets section added with specific paths and line numbers
  • [ ] arch-woodpecker note still missing (non-blocking scope item)
  • [ ] Test Expectations section not added (non-blocking — AC #4 covers intent)
  • [ ] Repo field still says pal-e-platform only (non-blocking — File Targets makes multi-repo implicit)

Recommendation

  • [SCOPE] Create architecture note arch-woodpecker in pal-e-docs. Non-blocking — can be done in parallel with implementation.
  • [BODY] Minor: consider noting in Repo section that basketball-api is also affected. Non-blocking.
All blocking issues from v1 review are resolved. Ticket is ready for dispatch.