Review: Bug: validation duplication + CI test step + JSON parse

Verdict: NEEDS_REFINEMENT

Template Completeness

• [x] Type — Bug
• [x] Lineage — plan-wkq Phase 14 QA nits from PR #2
• [x] Repo — forgejo_admin/westside-contracts
• [x] What Broke — three issues described
• [x] Repro Steps — present
• [x] Expected Behavior — present
• [x] Environment — present with namespace and file paths
• [x] Acceptance Criteria — 5 criteria listed
• [x] Related — project and PR references

Traceability

• [ ] story:X label — missing. Foundational QA cleanup; acceptable for a bug ticket.
• [ ] arch:X label — ISSUE: board item has arch:westside-app but work is in westside-contracts repo. Other contract-flow items on this board use arch:contract-flow. Should be arch:contract-flow.
• [x] Forgejo issue — westside-contracts#5, open

File Targets

• [x] src/routes/contract/[token]/sign/+server.ts — verified: file exists, line 30 has unguarded request.json(), lines 32-46 have inline validation that duplicates validation.ts logic. validateSignRequest is never imported.
• [x] src/lib/validation.ts — verified: exports validateSignRequest and validateSignatureData. 12 tests exist in tests/validation.test.ts and all pass.
• [ ] .woodpecker.yml — ISSUE: wrong file extension. Actual file is .woodpecker.yaml. Furthermore, npm test is ALREADY present in the CI pipeline (line 23). This sub-issue has been resolved since the ticket was filed.

Repo Placement

Dependencies

• #312 (Bug: signature pad init after checkbox) is also in todo for westside-contracts. Independent — different files. No blocking relationship.
• Related done items (#328 outbox bug, #330 E2E, #331 date bug, #334 mobile date) are all complete. No blockers.

Acceptance Criteria

• [x] "Sign endpoint imports validateSignRequest from validation.ts" — verifiable via grep
• [x] "Inline validation removed from +server.ts" — verifiable via grep
• [x] "Malformed JSON returns 400" — verifiable via test
• [ ] "npm run test in CI pipeline" — ALREADY DONE. This criterion is pre-satisfied. Should be removed or updated.
• [x] "Existing 12 tests pass" — verifiable (confirmed: 12 tests pass now)

Blast Radius

• No other SvelteKit server endpoints in westside-contracts use request.json() — this is the only one.
• No similar unguarded request.json() found in westside-app or basketball-api.
• Downstream: basketball-api outbox consumer is not affected (it reads from the outbox table, not from this endpoint directly).

Decomposition Check

• 2 file targets in 1 repo (after removing CI sub-issue) — under threshold
• 3 remaining acceptance criteria (after removing CI) — under threshold
• Estimated agent work under 5 minutes — single-pass appropriate

Recommendation

• Fix arch label: Change arch:westside-app to arch:contract-flow on board item #313.
• Remove CI sub-issue: The .woodpecker.yaml already contains npm test. Remove acceptance criterion #4 ("npm run test in CI pipeline") and update the issue title/body to reflect only two bugs remain (validation duplication + JSON parse). Fix the file extension reference from .woodpecker.yml to .woodpecker.yaml.
• Add missing acceptance criterion: "New test added for malformed JSON body returning 400" — the fix should include a test, not just the try/catch.