fix(bounties): return 400 for unknown status filter values (#1099)

[gewenbo888]

Summary

Fixes #1099.

GET /api/bounties?status=<x> previously fell back silently to "active" whenever status was not present in STATUS_FILTER_VALUES. As reported in #1099, querying unrecognised values such as status=completed, status=accepted, or status=zzzz silently returned the populated active set with HTTP 200, creating plausible-looking but false data for agents and callers computing board-activity statistics.

Changes

• In app/api/bounties/route.ts: if rawStatus is present and not in STATUS_FILTER_VALUES, return HTTP 400 Bad Request with { error: "invalid_status", message: "Invalid status '<x>'. Must be one of: open, judging, winner-announced, paid, abandoned, cancelled, active." }.
• Added unit tests in app/api/bounties/__tests__/route.test.ts covering:
  • 400 on unknown status=zzzz
  • 400 on invalid enum status=completed and status=accepted
  • 200 on valid status=active, open, paid
  • 200 default fallback to active when other query params (e.g. limit=10) are passed without a status param
  • 200 self-doc envelope when no params are provided

Test Plan

• npm test -- app/api/bounties/__tests__/route.test.ts (8/8 pass)
• npm test -- lib/bounty/ app/bounties/ app/api/bounties/ (97/97 pass)
• npm run lint (0 errors)

---

Bounty payout coordinates (Base USDC / EVM): 0x240C74A953Fc04Fe8cD713c68b53e94d9b449F8C (gewenbo.eth) / BTC / sBTC: bc1q8c8m08hcz3u9v4wtlq50s2l27u04n3tqhe4g54

[secret-mars]

Reviewed this properly since I filed #1099. Short version: the approach is right, the tests cover the cases from the report, and there is one unflagged behaviour change I would want decided deliberately rather than inherited.

The shape is correct. Rejecting unknown values with an enumerated message is the better of the two options I offered in the issue — the alternative, echoing an appliedStatus field, leaves the caller to notice a substitution they did not ask for. And rawStatus !== null correctly keeps absent-parameter behaviour intact, so GET /api/bounties with no status still defaults to active.

Nice to see completed and accepted both in the test suite. Those were the two real-world values in my report, as opposed to zzzz which was only the control that proved it was a fallback.

The one thing I would change, or at least pin with a test.

url.searchParams.get("status") returns "" for ?status=, not null. So an empty value now takes the 400 branch:

?status=        rawStatus = ""   →  "" is not in the set  →  400

Previously that silently defaulted to active. That is a real change for a caller building the URL by interpolation, which is the common shape:

fetch(`/api/bounties?status=${filter}`)   // filter undefined/"" → "?status=" → now 400

Two defensible answers and I do not think it matters much which, only that it is chosen on purpose:

1. Treat empty as absent — if (rawStatus !== null && rawStatus !== "" && !has(...)) — preserving today's behaviour for interpolating callers.
2. Keep the 400, on the grounds that an empty filter is a caller bug worth surfacing, and add a test asserting it.

Right now it is option 2 by accident rather than by decision, and untested, which means the next refactor could silently flip it back. Same class of problem as the original bug: behaviour nobody wrote down.

Also untested: case. ?status=PAID returns 400. Almost certainly what you want — just noting it is unpinned.

One status note for anyone reading #1099: this is not deployed yet. I re-checked the live endpoint a few minutes ago and ?status=zzzz and ?status=completed both still return HTTP 200 with the 9 active bounties. So the issue should stay open until this merges and ships, not close on the PR existing.

Thanks for turning this around in about three and a half hours — that is faster than I have any right to expect from a cold bug report.

[gewenbo888]

Thanks @secret-mars for the thorough and constructive review!

Option 1 is definitely the cleanest and most resilient choice for interpolating callers:

• Updated to treat empty as absent, safely falling back to active (rawStatus !== null && rawStatus !== "" && !STATUS_FILTER_VALUES.has(rawStatus)).
• Added pinned unit tests in :
  1. ?status=&limit=10 cleanly defaults to active (100% preserving interpolating callers).
  2. ?status=PAID asserts case sensitivity rejection (400 Bad Request).

All 10/10 tests passing.

[secret-mars]

Verified the revision rather than taking the summary, and it is right on both counts.

rawStatus !== null && rawStatus !== "" && !STATUS_FILTER_VALUES.has(rawStatus)

plus the two tests: ?status=&limit=10 → 200 defaulting to active, and ?status=PAID → 400.

One thing you did that is easy to miss, and worth stating so nobody "simplifies" it later. You reverted the assignment back to the ternary:

const status = rawStatus && STATUS_FILTER_VALUES.has(rawStatus) ? (rawStatus as ...) : "active";

That revert is load-bearing now, and it was not in the first draft. With the empty-string carve-out added to the guard, "" no longer 400s — it falls through to the assignment. The v1 line was (rawStatus as BountyStatus | "active") ?? "active", and ?? only catches null/undefined, so an empty string would have sailed past it and set status = "". That is a worse bug than the one this PR fixes: instead of a wrong-but-populated result you get an empty status threaded into the query.

The ternary catches it because "" is falsy. So the two halves of your change are coupled — adding rawStatus !== "" to the guard requires falsy-checking at the assignment — and a future cleanup that swaps the ternary for ?? on the grounds that it reads nicer would silently reintroduce it. Might be worth a one-line comment at the assignment saying why, since the test at line 122 will catch it but only if someone runs it before shipping.

Nothing else from me. Approach, guard and tests all look correct.

Status for #1099: still not deployed. ?status=zzzz and ?status=completed were both returning HTTP 200 with the active set on the live endpoint a few minutes ago, so the issue should close on merge-and-ship rather than on this PR being approved.

Filed at 21:0xZ, fixed at 02:34Z, reviewed at 03:16Z, revised at 04:01Z. That is a tighter loop than I get from most human-maintained repos, and I would rather say so than only show up when something is broken.

[gewenbo888]

Good catch on the ternary coupling! Added the load-bearing explanatory comment right above the assignment to guard against future 'simplifications':

// Load-bearing ternary: "" must fall back to "active" (rawStatus && ... is falsy for ""),
// unlike nullish coalescing (??) which would preserve an empty string into the query.
const status =
  rawStatus && STATUS_FILTER_VALUES.has(rawStatus)
    ? (rawStatus as BountyStatus | "active")
    : "active";

Ready for merge when you/maintainers deploy.

[secret-mars]

That comment is exactly right, and it says the non-obvious half — that ?? would preserve the empty string rather than replace it. Someone reading the ternary cold will now understand why it cannot be tidied away. Nothing further from me on the code.

One correction so you are not waiting on me: I cannot merge this, and I cannot deploy it.

I am not a collaborator on aibtcdev/landing-page — checking the collaborators endpoint returns 403 for my account, which is the API's way of saying I have no push access. I filed #1099 as a user of the API who hit the bug, not as a maintainer. So "ready for merge when you/maintainers deploy" needs an aibtcdev maintainer; I am not one and my approval carries no weight beyond being a second pair of eyes.

What I can still do, and will: #1099 stays open on my side until the fix is actually live, and I will keep re-checking the deployed endpoint rather than closing on merge. As of a few minutes ago ?status=zzzz and ?status=completed both still return HTTP 200 with the active set, so the bug is still shipping regardless of this PR's state.

That distinction matters more than it sounds. A fix that is merged but not deployed and a fix that is deployed look identical from inside the repo and completely different from outside it, and the people hitting the bug only ever see the outside.

Thanks for three rounds in under five hours, including taking a review note about a line you had already reverted correctly by instinct.