pal-e-docs

Review: Create read-only Postgres role for westside-streamlit (R2)

review-936-2026-04-10-r2 Doc

review ready
Project

Verdict: READY

Round 2 review of board item #936 on board-westside-streamlit. Forgejo issue: forgejo_admin/westside-streamlit#5 (state: open). All three R1 findings from review-936-2026-04-10 are addressed in the revised body.

R1 Findings Disposition

  • [x] Filename convention — R1 flagged timestamp vs sequential prefix. R2 body now specifies 030_add_westside_streamlit_ro_role.py with explicit guidance: "sequential prefix (NOT a timestamp) ... verify at PR time and adjust if 030 is taken." Resolved.
  • [x] DELETE denial AC — R1 flagged missing DELETE denial. R2 body now adds DELETE, TRUNCATE, and DROP denial acceptance criteria (plus existing INSERT/UPDATE/CREATE TABLE), giving full symmetry across all mutation and DDL verbs. Constraints section also lists TRUNCATE + DDL denials explicitly. Resolved.
  • [x] Cross-repo scope clarification — R1 flagged implicit cross-repo pattern. R2 body now has an explicit ### Repo header stating PR lands in basketball-api while the issue is filed on westside-streamlit for project-tracking. Lineage and context also call out the split. Resolved.

Template Completeness

Type: Feature. All required sections per template-issue-feature:
  • [x] Type, Lineage, Repo, User Story, Context, File Targets, Acceptance Criteria, Test Expectations, Constraints, Checklist, Related
  • [x] Revision history captured in Lineage (cites review-936-2026-04-10)
  • [x] Incident cross-reference added: incident-2026-04-10-pal-e-streamlit-public-funnel as defense-in-depth rationale

Traceability

  • [x] story:reachable label — verified story-westside-streamlit-reachable listed in project-westside-streamlit user-stories (confirmed R1).
  • [x] arch:postgres label — present on board item.
  • [!] arch note missing — arch-postgres does not exist in pal-e-docs. Waiver applied per precedent established for #933 (arch-ci-pipeline) and #938 (arch-keycloak-oidc): platform-wide docs gap, not a ticket defect. Ticket body explicitly acknowledges the gap and queues follow-up. Does not block dispatch.
  • [x] Forgejo issue — westside-streamlit#5, open, correctly filed.

File Targets

  • [!] alembic/versions/030_add_westside_streamlit_ro_role.py — directory verified. However, slot 030_ is now occupied by 030_add_registration_type_to_registrations.py (landed between R1 and R2). Next free slot is 031_. The ticket body preemptively handles this: "verify at PR time and adjust if 030 is taken." Dev agent will pick the correct prefix at branch time. Not a blocker.
  • [x] tests/test_westside_streamlit_ro_role.py — clean add into existing ~/basketball-api/tests/.
  • [x] "Files NOT to touch" section explicit.

Repo Placement

Explicit in the ### Repo header — PR lands in basketball-api, issue tracked on westside-streamlit. Acceptable cross-repo pattern, now documented.

Dependencies

Independent. No in_progress blockers on board-westside-streamlit. Downstream consumers (#4 kustomize overlay) wait on this but are not blocked on each other.

Acceptance Criteria

10 criteria, all machine-verifiable. Full denial symmetry across INSERT / UPDATE / DELETE / TRUNCATE / CREATE / DROP. Positive path (all app.py queries succeed) and reversibility (alembic downgrade -1) also asserted. Test command is real and runnable.

Blast Radius

Establishes the per-consumer read-only role pattern. westside-ops should follow later with westside_ops_ro. Additive, non-breaking. No live consumers yet.

Decomposition

5-minute rule: 2 file targets, 1 repo, 10 AC (all same domain — Postgres grants). Estimated <5 min. No decomposition needed.

Recommendation

No action needed. Ticket is READY for todo and dispatch. All R1 findings resolved; arch-postgres gap waived per precedent. The only residual nuance — migration slot 030 is now taken — is already handled by the ticket's "verify at PR time and adjust" instruction.