pal-e-docs

Review: Fix CI test failures and gem audit for initial Harbor image build

review-1616-2026-06-26 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type -- Bug
  • [x] Lineage -- Sprint 5 blocking, discovered during pipeline validation
  • [x] Repo -- ldraney/westside-basketball
  • [x] What Broke -- Detailed description of pipeline failures
  • [x] Repro Steps -- 6-step reproduction with specific pipeline output
  • [x] Expected Behavior -- Clear target state
  • [x] Acceptance Criteria -- Comprehensive (test fixes, gem audit, pipeline)
  • [x] Environment -- Woodpecker 3.13.0, Ruby 3.4, Rails 8.1.3
  • [x] Related -- Sprint 5 tickets and dependencies listed
Template is complete -- all required sections for a Bug issue are present.

Traceability

  • [ ] story:admin-player-management label -- INVALID. Project page user stories use WS-S{N} format. admin-player-management is a freeform label not matching any documented story. This CI fix is infrastructure/pipeline work that blocks all deployments. Best match: story:WS-S1 ("As superadmin, I want to deploy platform changes via IaC so that infrastructure is reproducible and auditable").
  • [ ] story note NOT verified -- no admin-player-management story entry exists on project-westside-basketball user-stories section. [SCOPE] Create user story entry on project-westside-basketball OR remap to existing WS-S1.
  • [x] arch:ci-pipeline label -- CI pipeline architecture component
  • [ ] arch note MISSING -- search_notes("arch-ci-pipeline") returned no results. No architecture note exists for the ci-pipeline component. [SCOPE] Create architecture note arch-ci-pipeline for component ci-pipeline.
  • [x] Forgejo issue -- ldraney/westside-basketball#69, state: open

File Targets

  • [x] test/controllers/admin/players_controller_test.rb -- verified: file exists, contains all 3 failing test methods referenced in the issue (player links L54-58, status badges L47-52, table wrap L60-65)
  • [x] CSS selectors verified in views -- app/views/admin/players/index.html.erb contains .status-badge (L77), .player-list-link (L66), .player-list-table-wrap (L51), .player-list-cards (L88). These selectors match the test assertions, so the tests should pass if the view is rendered correctly. The failures likely stem from DB/fixture issues preventing player data from rendering.
  • [x] test/mailers/tournament_mailer_test.rb -- verified: file exists, contains the 3 failing tests referenced in the issue
  • [x] Transportation content mismatch CONFIRMED -- test asserts "15-passenger van" and "Travel to Las Vegas together as a team" (L54-55) but mailer view says "We will not be traveling as a team, and there will be no team transportation" (PR #64 content update). Issue correctly identifies this.
  • [x] Tenant lookup in tests CONFIRMED -- TournamentMailerTest uses Tenant.find_by!(slug: "westside-kings-queens") (L108, L124). No fixtures directory exists, db/seeds.rb has no Tenant records. These tests will fail in CI with ActiveRecord::RecordNotFound. However, note that tests already have rescue ActiveRecord::ConnectionNotEstablished, PG::ConnectionBad skip guards (L118-119, L141-142), but RecordNotFound is NOT caught by these guards.
  • [x] .woodpecker.yaml -- verified: pipeline config exists, build-and-push depends on lint, test, audit (L97-99). Issue accurately describes the dependency chain.
  • [x] Gemfile.lock gem versions verified:

Repo Placement

OK. Issue is filed on ldraney/westside-basketball, all file targets are in the same repo. No cross-repo changes required -- gem updates and test fixes are repo-local.

Dependencies

  • This ticket is a BLOCKER for all downstream Sprint 5 work: #55 (player list), #58 (player detail) cannot deploy without a passing pipeline.
  • Board item 1612 (1pt, backlog) also targets arch:ci-pipeline with sprint:4 label -- possible overlap or predecessor. Its title is null (no Forgejo link visible), so scope is unclear.
  • Board item 1609 (todo, 5pts, "Admin player detail page") has depends:55 -- the dependency chain is: #69 (pipeline fix) → #55 (player list) → #58 (player detail).
  • No items explicitly depend on #69 via depends: label, but #69 is an implicit blocker for all deployment.

Acceptance Criteria

AC is comprehensive and agent-verifiable:
  • 7 test fix criteria with specific test method names -- verifiable by running bundle exec rails test
  • 6 gem audit criteria with specific version targets -- verifiable by checking Gemfile.lock and bundle-audit check
  • 4 pipeline criteria (Woodpecker pass, Harbor image, ArgoCD sync, site 200) -- partially verifiable (pipeline pass is CI, ArgoCD/site require post-deploy validation)
  • CONCERN: The "ArgoCD syncs and deploys pods" and "site returns 200" criteria are post-deploy validation, not verifiable by the implementing agent. These belong in post-merge validation, not AC. However, they are reasonable aspirational AC.

Blast Radius

WARNING: The same vulnerable gem versions exist in at least 5 sibling Rails repos:
  • concurrent-ruby 1.3.6 -- pal-enterprises, palinks, landscaping-assistant, believers-elite, paldocs
  • nokogiri 1.19.3 -- same repos
  • faraday 2.14.1-2 -- pal-enterprises, landscaping-assistant
These repos will have the same CVEs. However, this ticket correctly scopes to westside-basketball only -- sibling repo updates should be separate tickets. The base image (ruby-rails-build) does not pin gem versions, so each repo manages its own Gemfile.lock independently.

Decomposition Assessment

File targets: ~4 files in 1 repo (2 test files, Gemfile, Gemfile.lock). AC count: 18 total (7 test + 6 gem + 4 pipeline + 1 overall). Estimated agent work: ~10-15 minutes (test fixes require understanding view rendering, gem updates require resolving dependency conflicts).
The 18 AC count exceeds the 5 AC threshold, BUT the work is mechanically straightforward -- bundle update for gems, test assertion edits for test fixes. All changes are in a single repo. The gem updates are a single bundle update command. The test fixes are independent edits to 2 test files. A skilled agent can do this in one pass.
Borderline. No decomposition needed -- work is mechanical and single-repo despite high AC count.

Recommendation

  • [LABEL] Fix story label: story:admin-player-management is not a valid story code. Replace with story:WS-S1 (superadmin deploy via IaC) which is the closest match for CI/pipeline infrastructure work.
  • [SCOPE] Create architecture note arch-ci-pipeline for the CI pipeline component. Multiple board items reference arch:ci-pipeline but no backing note exists.