Skip to content

fix(kanban): convert PR web URL to API URL in _fetch_pr_state - #29545

Closed
lin-hao wants to merge 1 commit into
NousResearch:mainfrom
lin-hao:fix/kanban-active-pr-guard-api-url-v2
Closed

fix(kanban): convert PR web URL to API URL in _fetch_pr_state#29545
lin-hao wants to merge 1 commit into
NousResearch:mainfrom
lin-hao:fix/kanban-active-pr-guard-api-url-v2

Conversation

@lin-hao

@lin-hao lin-hao commented May 21, 2026

Copy link
Copy Markdown

What

The original _fetch_pr_state implementation passed the GitHub web URL (https://github.com/owner/repo/pull/N) directly to urllib.urlopen, which 404s because that is not a REST API endpoint.

This converts the web URL to the GitHub REST API URL (https://api.github.com/repos/owner/repo/pulls/N) before querying, so the PR state check can distinguish open PRs from closed/merged PRs.

Testing

  • python3 -m pytest tests/hermes_cli/test_kanban_db.py -k 'respawn_guard_merged_pr_not_guarded or respawn_guard_open_pr_still_guarded or respawn_guard_network_error_falls_back_to_guarding' -q -o 'addopts='

@lin-hao lin-hao left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Review

The fix is correct. Key points:

  1. URL conversion bug (the main fix): was passing directly to — this 404s. The fix converts it to via regex substitution before calling the API.

  2. Logic is sound: now calls per URL; → block, → skip (merged), → fallback block (safe-by-default on errors/no-token).

  3. Tests: 18/18 respawn_guard tests pass including 3 new tests for merged PR (no guard), open PR (guard), and network error (fallback guard).

  4. No behavior change when GitHub API is unreachable — falls back to legacy URL-only guard (block).

Verdict: LGTM. The fix actually makes the feature work as intended.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard labels May 21, 2026
@lin-hao
lin-hao force-pushed the fix/kanban-active-pr-guard-api-url-v2 branch from c438583 to 0fe5553 Compare May 22, 2026 21:25
Convert GitHub pull request web URLs to REST API URLs before fetching PR state, and keep respawn guard behavior safe-by-default when state cannot be resolved.
@lin-hao
lin-hao force-pushed the fix/kanban-active-pr-guard-api-url-v2 branch from 0fe5553 to cd66511 Compare May 22, 2026 21:26
@lin-hao lin-hao closed this May 22, 2026
@lin-hao
lin-hao deleted the fix/kanban-active-pr-guard-api-url-v2 branch May 22, 2026 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants