pal-e-docs

Review: westside-admin#29 (board 1146) — Edit route + audit — 2026-05-04 (v2, APPROVED)

review-1146-2026-05-04-v2 Doc

review approved
Project

Scope Review (v2): westside-admin#29 (board item 1146) — Players edit route + transactional audit

Verdict: APPROVED

Re-review of body v2 (edited 2026-05-04, in response to v1 verdict NEEDS_REFINEMENT (minor)). All three minor refinements cleanly applied; no new gaps introduced. Greenlit by Lucas for backlog → todo.

v2 changes verified

  • GET-side leak check. Body now contains two matching artifacts:
  • old_state / new_state shape. Pinned in Constraints: "FULL ROW snapshots, not diffs. jsonb of every column on players (excluding contract_token) at the moment of UPDATE — both before and after." Removes ambiguity for the dev agent.
  • arch-page-server backing note gap. Documented in Related as KNOWN PLATFORM GAP (not blocking #29); shared with #4; tracked as a separate Ava task. Correctly classified as non-blocker.

Template Completeness

  • [x] Type, Lineage, Repo, User Story, Context, File Targets, AC, Test Expectations, Constraints, Checklist, Related, Edit Log — all 12 sections present.
  • [x] Edit Log section properly maintained (v1 + v2 entries).

Traceability

  • [x] story:admin-row-crud label — story note story-westside-admin-admin-row-crud verified on project-westside-admin#user-stories table.
  • [x] arch:page-server label — backing note arch-page-server still missing in pal-e-docs (shared gap with #4, board item 1092). Behavior IS documented in arch-dataflow-westside-admin Flow 2, which IS listed on the project page architecture section. Non-blocker, tracked separately by Ava.
  • [x] Forgejo issue forgejo_admin/westside-admin#29 — open, body v2 last updated 2026-05-05T01:25:08Z.
  • [x] type:feature + split-from:5 labels present.

File Targets

  • [x] ~/westside-admin/src/routes/players/[id]/+page.server.ts — verified greenfield (no players/ route exists yet).
  • [x] ~/westside-admin/src/routes/players/[id]/+page.svelte — verified greenfield.
  • [x] ~/westside-admin/src/lib/server/audit.ts — verified greenfield (src/lib/server/ currently contains only keycloak.ts + keycloak.test.ts).
  • [x] Modify ~/westside-admin/src/routes/players/+page.svelte — file ships in #4 (board item 1092, currently in todo); modification only valid after #4 merges. Already declared as a hard-dep in body Lineage. Correct.

Repo Placement

OK. All targets in forgejo_admin/westside-admin; matches the issue's ### Repo declaration and the board item's labels.

Dependencies

Hard deps #1, #3, #4, #28 — all open and correctly enumerated in body Lineage. Board state confirmed:
  • #1 (1089) — todo (scopedDb / Drizzle base)
  • #3 (1091) — todo (tenant-scoped scopedDb)
  • #4 (1092) — todo (players list — supplies the row in +page.svelte we mutate to make rows clickable)
  • #28 (1145) — backlog (input components — sibling, parallel)
Cannot advance past todo until at least #1, #3, #4 clear (and ideally #28 to import inputs). Body is explicit about this.

Acceptance Criteria

11 ACs, all individually verifiable by an agent. The new contract_token grep AC is testable via integration test (response body assertion) — both AC and test are stated. monthly_fee dollars-not-cents constraint preserved from v1. Tenant-scoping 404-not-403 (existence-leak avoidance) preserved. Atomicity-by-failure-injection in checklist preserved.

Blast Radius

WRITE surface for players table. Audit log target contract_audit_log is the canonical mutation ledger. Replaces banned ad-hoc psql UPDATEs (feedback_never_write_prod_db). No sibling services consume this route — internal admin only. Helper signature writeAudit({ table, rowId, actor, sub, oldState, newState }, tx) is generic enough that future tables (#4 list rows, future row-edit routes) reuse it without re-design — good seam.

Decomposition Assessment

4 file targets, 11 AC. Already a decomposition (#5 split into #28 + #29 on 2026-05-04). Further splitting would create useless intermediate states (a +page.server.ts with no +page.svelte is not testable). Hold at this granularity. Estimated agent work: ~5 min — at the limit but justified by atomicity/leak/tenant requirements that must land together.

Recommendation

No action needed. APPROVED for backlog → todo move. Cannot advance to next_up / in_progress until #1, #3, #4 (and ideally #28) clear — that's a downstream concern, not a refinement gap.
  • v1 review: review-1146-2026-05-04 (NEEDS_REFINEMENT minor)
  • Forgejo v1 verdict comment: issue #29 comment 16229
  • Forgejo issue: forgejo_admin/westside-admin#29
  • Board item: 1146 on board-westside-admin
  • Sibling: #28 (board 1145) — input components, also backlog
  • Parent (closed): #5