CodeIssuesPull RequestsActionsSecurityInsights
✨ AI
More
Settings

fix(api): ?state=all returned an empty list instead of every issue #5570

MergedXSccantynz wants to mergefix/api-v2-state-allmainopened 2d ago
ccantynzcommented 2d ago

Reported from the Vapron instance, measured against a repo with 13 open issues:

(default)      -> 13   correct
?state=open    -> 13   correct
?state=all     ->  0   WRONG
?state=closed  ->  0   correct, none were closed

all was never special-cased, so it went straight into eq(issues.state, "all") and matched no row. GET /pulls had the identical shape. Both fixed by skipping the state predicate for all rather than adding all to an enum — it isn't a state, it's the absence of a filter.

The failure mode matters more than the one-liner

A documented, valid-looking parameter returned a successful empty result, indistinguishable from "this repo has no issues". The reporting session nearly told its owner that 12 issue creations had failed — the POSTs had returned real ids, and only querying Postgres directly proved the rows were there. ?state=banana answered [] just as confidently.

Unknown values now get a 400 naming what is accepted, so the endpoint can no longer answer a question it didn't understand.

It was already right one directory away

gluecron_list_prs (src/lib/mcp-tools.ts) enum-checks the value and special-cases all. The REST surface serving the same rows did neither — two sources of truth for one contract, and the one with fewer readers was correct. Both list endpoints now share a single helper, so the next state value can't land in one and not the other.

Sibling sweep — four surfaces read ?state=, only this one was broken

Surface Handles all?
api-v2.ts issues + pulls was broken — fixed here
github-compat.ts issues + pulls ✅ (and maps GitHub's closed onto our closed+merged)
issues.tsx ✅ drops the predicate
pulls.tsx inArray([open, closed, merged])

Flagged, not changed: github-compat still answers a mistyped state with an empty list rather than GitHub's 422. That's a deliberate behaviour change to a mimicry surface, so it deserves its own decision rather than riding along here.

Tests

Six, credential-free. Clause assertions call the helper directly — all must produce no predicate — plus wiring guards that both endpoints route through it and that no eq(<table>.state, state) pattern returns to the file.

Verified by mutation: deleting the all branch fails the test that names the bug; restoring it passes six.

The HTTP layer is deliberately not asserted — those routes sit behind middleware needing a database, and excluding this file from CI to test them would trade a guard that runs for one that doesn't.

Reported-by: vapron-0d (cross-instance)

ccantynzAI Reviewcommented 2d ago

AI Triage

(no summary)

Priority: medium Risk area: mixed

Suggested labels: (no label suggestions) Suggested reviewers: (no reviewer suggestions)

Suggestions only — nothing has been applied. The PR author stays in control.

gluecron[bot]🤖 botAI Reviewcommented 2d ago

AI review unavailable

The platform's AI balance is exhausted, so AI generation is temporarily unavailable. Nothing was lost. You can queue this as a repair for the internal agent from the repository's Health page, or try again once the balance is restored. The PR is otherwise unchanged.

Cross-repo impact

See what breaks downstream if this PR merges.

Analyze →
⮌ Merged

This pull request was merged into main.

c comment · e edit title · m merge · a approve · r request changes · ? shortcuts