Review: Players row edit: form action UPDATE + audit log in same transaction

Verdict: READY

Template Completeness

• [x] Type: Feature
• [x] Lineage (depends on players list #4)
• [x] Repo
• [x] User Story (story-westside-admin-admin-row-crud)
• [x] Context
• [x] File Targets (Create / Modify / Do NOT create)
• [x] Acceptance Criteria (9 items)
• [x] Test Expectations (3 integration + 1 manual)
• [x] Constraints (with monthly_fee=DOLLARS reminder)
• [x] Checklist
• [x] Related

Traceability

• [x] story:admin-row-crud — Admin Row CRUD (the WRITE half)
• [x] story note verified — story-westside-admin-admin-row-crud exists; listed in project-westside-admin user-stories table
• [x] arch:page-server label present
• [ ] arch note coverage — No standalone arch-page-server note, but the page-server pattern is documented as part of arch-dataflow-westside-admin Flow 2 (sequence diagram covers SvelteKit Server -> BEGIN -> UPDATE -> INSERT audit -> COMMIT). The label is acting as a component pointer into the dataflow note rather than a separate node. Acceptable for v1 since the Flow 2 diagram is the spec; flag below.
• [x] Forgejo issue — https://forgejo.tail5b443a.ts.net/forgejo_admin/westside-admin/issues/5, open

File Targets

• [x] src/routes/players/[id]/+page.server.ts — standard SvelteKit server module path
• [x] src/routes/players/[id]/+page.svelte — standard SvelteKit page path
• [x] src/lib/components/inputs/EnumSelect.svelte, JsonbEditor.svelte, DatePicker.svelte — conventional $lib component locations
• [x] src/lib/server/audit.ts — server-only helper module path is correct ($lib/server is the SvelteKit private convention)
• [?] src/routes/players/+page.svelte — Modify. This file is created by dependency ticket #4 (players list view). Modify directive is correct but only valid AFTER #4 lands. Captured under Dependencies.

Repo Placement

Dependencies

• Blocked by #4 (players list view) — explicitly stated in Lineage. The Modify target src/routes/players/+page.svelte does not exist until #4 ships. Do not start #1093 until #4 is in done.
• Likely needs #1 (scoped DB helper) — story group includes arch:scopeddb-helper tickets (#1, #3) that produce the tenant_id-scoped Drizzle wrapper. The AC "tenant_id mismatch returns 404" + "WHERE id=123 AND tenant_id=1" in Flow 2 imply the scoped helper is the canonical access path. If #1 is not landed first, this ticket will inline a one-off scope check, which contradicts the helper's purpose. Recommend ordering: #6 (sveltekit-server bootstrap) -> #1 (scopeddb) -> #4 (list) -> #1093 (edit).
• Auth dependency on #301/#1096 (keycloak) and #6/#1087 (sveltekit-server) — locals.user.email referenced in audit AC requires hooks.server.ts populating locals.user from Keycloak JWT (covered by #2 hooks-server ticket). Must land before #1093 to satisfy AC.

Acceptance Criteria

• Transaction atomicity AC ("UPDATE + audit log INSERT happen in the SAME transaction (both or neither)") is paired with a Test Expectation that injects deliberate failure — verifiable by an agent.
• Audit row shape is fully specified (actor, table, row_id, old_state, new_state, timestamp).
• Tenant scoping returns 404 not 403 — security-conscious, prevents enumeration.
• Sensitive column exclusion (contract_token NOT editable AND NOT shown) is concrete.
• Progressive enhancement constraint (works without JS) is testable by disabling JS.

• "flash message Saved" — mechanism not specified (cookie? URL param? load() return?). SvelteKit idiom is the &saved=1 query param or a flash cookie. Dev judgment OK.
• "validates client-side and server-side using Drizzle column metadata" — reuse path from #4 list view validation is implied but not explicit; assume shared util.

Blast Radius

• contract_audit_log table schema must already exist in the westside Postgres database. The ticket assumes this (writes to it without referencing a migration). Per the never_alter_prod_directly + never_write_prod_db conventions, if the table is not yet present, a separate migration ticket is required. Recommend Dev verify schema exists in step 1; if missing, ticket BLOCKS pending migration scoping. (Flag below as [SCOPE] for Ava to confirm pre-flight.)
• This is the first legitimized write surface for westside DB. Per feedback_never_write_prod_db, this ticket replaces ad-hoc psql UPDATEs. Bug here = audit gap = compliance regression. Justifies extra QA rigor.
• Drizzle.transaction() semantics — must use the tx parameter for both UPDATE and audit INSERT; using the outer db handle inside the callback silently breaks atomicity. Worth calling out in a code review checklist.

Decomposition Assessment

• Single repo, single transaction unit — splitting would break the atomicity story.
• The 3 input components (EnumSelect, JsonbEditor, DatePicker) are small and tightly coupled to the form. Pulling them into a sub-ticket creates coordination cost without benefit.
• Audit helper is a single function (writeAudit) — natural fit alongside the transaction it serves.

Recommendation

• [SCOPE] Ava to confirm contract_audit_log table exists in westside DB before this ticket leaves next_up. If missing, scope a migration ticket (must land before #1093). Capture in Dependencies note on the issue if not already covered by #1 (scopeddb-helper) ticket body.
• [BODY] Optional: add a one-line note in Constraints clarifying that Drizzle's db.transaction(async (tx) => { ... }) requires using tx (not the outer db) for both statements. Common footgun, worth pre-empting.
• [BODY] Optional: clarify the "Saved" flash message mechanism (query param vs cookie vs load() return) — Dev judgment otherwise.