Skip to content

fix(api): refuse state="merged" for issues with the documented message - #33

Merged
mabry1985 merged 1 commit into
mainfrom
fix/issue-state-no-merged
Sep 14, 2026
Merged

mabry1985 merged 1 commit into
mainfrom
fix/issue-state-no-merged

Conversation

@mabry1985

Copy link
Copy Markdown
Member

Follow-up to the one finding on #32. It was out of scope for that PR, which didn't touch api.py.

Bug: fetch_issues used the PR state validator, which accepts "merged". Issues have no merged state, so state="merged" passed validation and reached gh issue list --state merged. It came back as a bare gh error instead of the documented state must be open|closed|all.

Fix: _norm_state now takes the allowed set. Issues use open|closed|all; PRs keep all four (default), so fetch_prs is unchanged.

Tests

  • fetch_issues(state="merged") returns the validation error and never calls gh.
  • New test_fetch_prs_still_accepts_merged covers the PR path.
  • Teeth check: with main's api.py, the issue assertion fails.
  • The full suite passes (314), and ruff check / ruff format --check are clean.

No version bump. This ships with the next release.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LSRkzcPyaDrKSY7geFuGr3

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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LSRkzcPyaDrKSY7geFuGr3

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA panel review — PASS

code-review · head f9e19bd1ce88 · formal

Low-risk change: _norm_state is parameterized with an allowed tuple so that fetch_issues rejects "merged" while fetch_prs retains it. The verifier confirmed the logic is correct, both callers are in the same file with no other call sites, and tests cover both the rejection and preserved paths. No defects found; the panel's empty findings array is confirmed.

No findings — the review came back clean.

findings JSON (machine-readable)
[]

@mabry1985
mabry1985 merged commit c042762 into main Sep 14, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant