api/cmd: let implicit pr_ci resolve closed and merged PRs #450

New issue

Closed

opened 2026-04-27 02:09:56 +00:00 by barrettruth · 1 comment

barrettruth commented

2026-04-27 02:09:56 +00:00

Searched existing issues; the closest related work is #389 and #391.

Problem

Today the implicit current-ref PR CI entrypoints are open-PR-only:

require('forge').pr_ci()
:Forge pr ci

They resolve through current_pr(), which currently warns no open PR found for this branch when the current branch has no open match.

That behavior is intentional in the current implementation:

resolve_action_pr() routes implicit pr_ci() through current_pr()
current_pr() returns nil when no open PR matches
backend pr_for_branch_cmd() lookups use open-only PR/MR listing

This makes post-merge/post-close CI inspection awkward. If I am still on the branch for a PR that was just merged or closed, pr_ci() can no longer recover that PR's checks from branch context, even though viewing CI for a closed/merged PR is still useful.

Explicit targeting remains possible with pr_ci({ num = '42' }), but that requires already knowing the PR number.

Proposed solution

Revisit the semantics of implicit pr_ci() / :Forge pr ci specifically.

A reasonable behavior would be:

first prefer the unique open PR for the branch, as today
if there is no open match, optionally fall back to the unique closed/merged PR for that branch
keep ambiguity errors when multiple candidates exist
keep pr() and review() open-PR-only if that still makes sense for edit/review flows

In other words: CI inspection may want broader branch->PR resolution than edit/review actions do.

Alternatives considered

Keep the current behavior and require explicit num= / {num} for closed or merged PRs. This is precise, but it makes the common "I just merged this, show me its checks again" flow unnecessarily manual.
Broaden current_pr() globally to include closed/merged PRs. That may be too broad for flows like pr() and review() that conceptually target open PRs.
Add a separate helper for "latest PR for this branch" and keep pr_ci() unchanged. That avoids changing current-ref semantics, but adds another near-duplicate entrypoint.

Searched existing issues; the closest related work is #389 and #391. ## Problem Today the implicit current-ref PR CI entrypoints are open-PR-only: - `require('forge').pr_ci()` - `:Forge pr ci` They resolve through `current_pr()`, which currently warns `no open PR found for this branch` when the current branch has no open match. That behavior is intentional in the current implementation: - `resolve_action_pr()` routes implicit `pr_ci()` through `current_pr()` - `current_pr()` returns `nil` when no open PR matches - backend `pr_for_branch_cmd()` lookups use open-only PR/MR listing This makes post-merge/post-close CI inspection awkward. If I am still on the branch for a PR that was just merged or closed, `pr_ci()` can no longer recover that PR's checks from branch context, even though viewing CI for a closed/merged PR is still useful. Explicit targeting remains possible with `pr_ci({ num = '42' })`, but that requires already knowing the PR number. ## Proposed solution Revisit the semantics of implicit `pr_ci()` / `:Forge pr ci` specifically. A reasonable behavior would be: - first prefer the unique open PR for the branch, as today - if there is no open match, optionally fall back to the unique closed/merged PR for that branch - keep ambiguity errors when multiple candidates exist - keep `pr()` and `review()` open-PR-only if that still makes sense for edit/review flows In other words: CI inspection may want broader branch->PR resolution than edit/review actions do. ## Alternatives considered - Keep the current behavior and require explicit `num=` / `{num}` for closed or merged PRs. This is precise, but it makes the common "I just merged this, show me its checks again" flow unnecessarily manual. - Broaden `current_pr()` globally to include closed/merged PRs. That may be too broad for flows like `pr()` and `review()` that conceptually target open PRs. - Add a separate helper for "latest PR for this branch" and keep `pr_ci()` unchanged. That avoids changing current-ref semantics, but adds another near-duplicate entrypoint.

barrettruth commented

2026-04-27 17:21:21 +00:00

Tracking note: I’m implementing #453 first as the shared resolver/backend layer. This issue should then consume it with open-first, then closed/merged fallback for implicit pr_ci(), while keeping pr() and review() open-only.

Tracking note: I’m implementing #453 first as the shared resolver/backend layer. This issue should then consume it with open-first, then closed/merged fallback for implicit `pr_ci()`, while keeping `pr()` and `review()` open-only.