From f9e19bd1ce88850474af61e1bc8fc0dd75f34606 Mon Sep 17 00:00:00 2001 From: Josh Mabry Date: Mon, 14 Sep 2026 02:27:03 -0700 Subject: [PATCH] fix(api): refuse state="merged" for issues with the documented message fetch_issues shared the PR state validator, which accepts "merged". Issues have no merged state, so the value reached `gh issue list --state merged` and came back as a bare gh error instead of "state must be open|closed|all". The validator now takes the allowed set: issues use open|closed|all, and PRs keep all four. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01LSRkzcPyaDrKSY7geFuGr3 --- api.py | 10 +++++++--- tests/test_board_view.py | 10 ++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/api.py b/api.py index 3b70087..35fe1f4 100644 --- a/api.py +++ b/api.py @@ -37,10 +37,14 @@ def gh_available() -> bool: return resolve_gh() is not None -def _norm_state(state: str) -> str | None: +_PR_STATES = ("open", "closed", "merged", "all") +_ISSUE_STATES = ("open", "closed", "all") # issues have no "merged" — `gh issue list --state merged` errors + + +def _norm_state(state: str, allowed: tuple[str, ...] = _PR_STATES) -> str | None: """Normalise the state filter to what `gh` accepts, or None if invalid.""" s = (state or "open").strip().lower() - return s if s in ("open", "closed", "merged", "all") else None + return s if s in allowed else None async def fetch_issues(repo: str, state: str = "open", limit: int = 30) -> dict: @@ -51,7 +55,7 @@ async def fetch_issues(repo: str, state: str = "open", limit: int = 30) -> dict: """ if err := bad_repo(repo): return {"error": err} - norm = _norm_state(state) + norm = _norm_state(state, _ISSUE_STATES) if norm is None: return {"error": f"Error: state must be open|closed|all (got {state!r})."} capped = max(1, min(int(limit), 100)) diff --git a/tests/test_board_view.py b/tests/test_board_view.py index ef64b04..7956107 100644 --- a/tests/test_board_view.py +++ b/tests/test_board_view.py @@ -43,9 +43,19 @@ async def test_fetch_issues_bad_repo_and_bad_state(): with patch("ghplugin.api.run_gh", fake): assert "owner/name" in (await fetch_issues("nope"))["error"] assert "state must be" in (await fetch_issues("o/n", state="weird"))["error"] + # "merged" is a PR state; `gh issue list --state merged` would fail with a bare gh error + assert "state must be" in (await fetch_issues("o/n", state="merged"))["error"] fake.assert_not_called() +async def test_fetch_prs_still_accepts_merged(): + fake = AsyncMock(return_value=(0, "[]", "")) + with patch("ghplugin.api.run_gh", fake): + assert await fetch_prs("o/n", state="merged") == {"items": []} + args = fake.call_args.args[0] + assert args[args.index("--state") + 1] == "merged" + + async def test_fetch_issues_gh_failure(): fake = AsyncMock(return_value=(1, "", "not found")) with patch("ghplugin.api.run_gh", fake):