fix(applications): enforce one-way review state transitions

[cold-13]

Closes #1302

What this protects

NgoApplication.status is a three-state machine with exactly one legal
transition: PENDING -> APPROVED | REJECTED. Both targets are terminal — a
recorded decision is the record of what the review concluded, and it is what
an admin acts on and what an applicant is told by
GET /ngo-applications/status. Today nothing enforces that: the
already_reviewed check in both review handlers sits after the return
inside the try, so it never runs, and it reads an application variable
that no longer exists in that scope.

The observable consequences on main today:

• POST /ngo-applications/:id/approve on a rejected application returns
  200 and flips it to APPROVED.
• POST /ngo-applications/:id/reject on an approved application returns
  200 and flips it to REJECTED.
• Replaying either request (a captured admin signature stays valid for the
  whole 5-minute clock-skew window, and an admin dashboard retrying a request
  it never saw the answer to is an ordinary accident) silently overwrites
  the decision and the review note, leaving no trace that the outcome had
  ever been anything else.

This is the invariant the reachable-looking check was meant to hold. The two
tests that covered it are not in the current suite.

The fix

src/routes/ngoApplications.ts:95 — the "is it still pending?" test is folded
into the UPDATE itself:

const { count } = await prisma.ngoApplication.updateMany({
  where: { id, status: 'PENDING' },
  data: { status: decision, reviewNote: parsed.data.reviewNote },
});

This is a compare-and-swap, not a read-then-write. Postgres re-evaluates the
predicate against the latest committed row when a concurrent writer holds the
row lock, so of two reviews that race, exactly one matches and the loser
updates zero rows. A findUnique followed by an update — the shape #80
introduced — cannot do that: both callers read PENDING and both write.

updateMany also cannot raise the "record not found" error that update
does, so the two reasons for matching nothing are told apart by one
follow-up read that selects only the status:

situation	response
no application with that id	404 {"error":"not_found"} (unchanged)
exists, but not PENDING	409 {"error":"already_reviewed","status":<current>}

Both routes now go through a single reviewHandler(decision), so approve and
reject cannot drift apart in how they are applied — the two hand-copied
handlers are what let the guard rot in the first place. The invariant is
documented at the top of that function.

Failure modes, deliberately

• Retry after a lost response. The review committed but the client never
  saw it. The retry gets 409 with the status on file, which distinguishes
  "your review landed" from "another reviewer got there first". (A successful
  review also cannot be silently lost: updateMany only ever moves a row out
  of PENDING, and nothing else in the codebase writes this column.)
• Partial failure. If the re-read after a won compare-and-swap comes back
  empty, the handler answers 404 rather than 200 with an empty body, so
  the response is never inconsistent with the row.
• Real database errors still propagate to the global error handler and
  become 500 internal_server_error; only a genuinely missing row is a 404.

Security

• The requireAdminSignature preHandler is untouched — authorization and the
  state machine stay separate gates, and a validly signed request that tries
  to re-review is still refused.
• The refusal log line carries applicationId, currentStatus and the
  attempted decision only. The applicant's contactEmail/description and
  the reviewer's reviewNote are never logged or echoed on the failure path.
• Nothing is weakened: validation, the admin signature check and the id
  validation are all unchanged.

Compatibility

Unchanged: 400 for a malformed id or body, 401/503 from the auth
middleware, 404 for an unknown id, 200 with the same application body on
success. The one change is additive — the 409 body gains status.

Tests

test/routes/ngoApplications.test.ts, new application review state machine
block plus one added to the existing review block:

• Success — approve a pending application (existing), and reject a
  pending application with its note stored (new; the reject happy path had no
  coverage at all).
• Rejection — refuse to approve an already-approved application, an
  already-rejected one, and to reject an approved one; each asserts the stored
  status is unchanged, not just the status code.
• Regression — a refused re-review must not write the losing note: a
  rejected application with Original reviewer note. stays rejected with that
  exact note after an approve carrying Overwriting note..
• Response contract — the 409 body is exactly
  {error, status}.
• Concurrency — two overlapping approves with different notes: exactly one
  200 and one 409, and the stored note is the winner's, so the loser's
  note never lands. Plus the worst case, two admins submitting opposite
  decisions at once: the application lands in exactly one terminal state.
• Boundary — a 2000-character note is accepted; 2001 is 400 and the
  application is still PENDING, i.e. a request that fails validation has not
  consumed the one transition the application is allowed.

Notes for review

• POST /ngo-applications still has a read-then-write duplicate check
  (two concurrent submissions can create two PENDING applications). That is
  a separate invariant, already tracked and being fixed in fix(applications): prevent duplicate pending applications #182, so it is left
  alone here rather than duplicated.
• 409 for a repeat (rather than an idempotent 200) is the contract Already-reviewed applications can be approved or rejected again #49
  specified and Status #80 implemented, so this keeps it. If you would rather a
  same-decision retry return 200, that is a one-line change now that the
  transition is atomic — say the word.

[Ashborne267]

@cold-13 the issue you linked doesn't exist