Open pull requests from Pane, and follow them from the review panel

[0x92]

Description

Open a pull request for a session's branch without leaving Pane, and follow it from the review panel afterwards.

A session already is a branch in a worktree, so everything a pull request needs is on hand. Today the flow leaves the app: switch to a browser, find the fork, remember which repository the contribution is meant for. This adds a Create Pull Request entry to the session's git menu that gathers the branch, its commits, the repository's PR template and the possible targets in one round trip, pushes the branch and calls gh pr create.

Once the pull request exists, the same session's Review panel gains a status strip: state, review decision, mergeability, size and reviewers — the questions that otherwise send you back to the browser.

What it does

Creating

• Title and body are proposed from the commits ahead of the base: the first commit's subject becomes the title, the rest become a list, and the repository's pull_request_template.md is appended rather than replacing them.
• Fork-aware target picker. A fork has two candidates, and picking your own by accident is a silent mistake, so the parent is offered first and the head ref is spelled owner:branch when the pull request crosses repositories.
• Searchable base-branch picker. The short list is the branches this clone actually works on that also exist in the target; everything else is one click away. (A busy upstream has ~200 branches — a plain list is unusable, and a native <datalist> hides everything that does not match what is already typed.)
• Changes section: which files the pull request would carry, with per-file +/- and totals, compared against the merge base the way GitHub counts it. "Show diff" loads the patch on demand and renders it in the existing DiffViewer.
• Says what is wrong instead of failing obscurely: no commits ahead, detached HEAD, gh missing or signed out, an existing pull request for this branch (shown as a link — there is nothing to create), a base branch the target repository does not have.
• Uncommitted changes are called out explicitly, because they stay behind.

Following

• A strip under the review header: Open / Draft / Merged / Closed, number and title, review decision, comment count, and a conflict warning when GitHub reports CONFLICTING.
• Expanded: owner:head → repo:base, size, and each reviewer with their newest verdict — a "changes requested" that the same person later approved is not an open objection.
• Refreshes itself every 90s while the panel is on screen, and on demand.
• CI checks continue to be shown by the existing checks chip.

How it works

• PullRequestManager (main process) is the only place that runs commands; everything that decides what to send is a pure function next to it, tested on its own: deriveDraftText, resolveTargets, buildCreateArgs, normalizeBaseBranch, sortBaseBranches, parsePullRequestStatus, parseChecks, …
• gh is invoked through the session's CommandRunner, so a WSL project uses the gh inside the distro rather than the one on the Windows host. When gh is not on the snapshotted PATH — for instance because it was installed while Pane was running — the usual install locations are tried before giving up.
• The body never goes through a shell: it is written to a temp file and passed with --body-file, so backticks, quotes and newlines in markdown survive.
• All channels are daemon-owned (pr:*): the branch, its remote and the gh binary live on the machine that runs the agents, so a remote runtime answers these itself instead of having the laptop push a branch it does not have.

Type of Change

• New feature (non-breaking change which adds functionality)

Checklist

• I have read the CONTRIBUTING.md guidelines
• My code follows the code style of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• I have run pnpm typecheck and pnpm lint locally
• I have tested the Electron app locally with pnpm electron-dev

Critical Areas Modified

• State management/IPC events

Additional Notes

Builds on the per-file commit details PR — it reuses shared/types/git.ts, the numstat/name-status parsers in gitDiffManager and parseUnifiedDiff. Please merge that one first.

Tested against the real GitHub CLI and this repository: draft generation, the target list for a fork, base-branch listing across several API pages, the file comparison cross-checked against git diff --name-only <base>...HEAD, and status for open pull requests. One real pull request was created end to end and closed again.

PR test automation

Status: PASS on 2ab27e158ae6cbe230373bb4c8dd0b2d17158250

• Create Pull Request dialog: PASS for draft values, upstream target, base branch, file totals, and uncommitted-change warning.
• Invalid target base: PASS for clear warning and disabled submit.
• Review status strip: PASS for current state, latest review verdict, conflicts, fork/base coordinates, size, comments, and reviewers.
• Safety: mocked IPC only. No live pull request, push, or other production GitHub object was created by QA.

[parsakhaz]

Verdict: Request changes. Two creation-flow defects violate the PR intent.
Counts: Must Fix: 2 (security: 0) · Should Fix: 0 · pass 1/3

Must Fix

• MF-1 Existing cross-repository pull requests are searched in the checkout context, not the upstream target · main/src/services/pullRequestManager.ts:542, main/src/services/pullRequestManager.ts:1005 · query each resolved target with --repo and use forkOwner:branch for an upstream head · violates the stated existing-PR blocker.
  • Evidence: findExisting runs gh pr list --head <branch> without --repo; the focused test stubs any pr list call and therefore does not prove the command targets dcouple/Pane.
  • Failure scenario: from a fork checkout with an open PR into upstream, gh searches the fork, returns no row, and Pane offers Create Pull Request instead of the existing upstream link.
• MF-2 A base branch the target repository does not have is described as invalid but still submit-enabled · frontend/src/components/git/CreatePullRequestDialog.tsx:145, frontend/src/components/git/CreatePullRequestDialog.tsx:150, main/src/services/pullRequestManager.ts:581 · block known-invalid bases and validate the target base before pushing · violates the stated readable blocker for a missing target base.
  • Evidence: baseIsUnknown only renders warning copy; canSubmit ignores it, and the backend pushes before gh pr create performs validation.
  • Failure scenario: the user selects a target where the typed base is absent, Pane pushes the branch, then fails through raw gh rejection despite already knowing the base is invalid.

Praise

• Shell arguments are quoted for the actual WSL or host target, PR bodies use a temporary file, and hostile branch/title coverage passed.
• Focused backend review tests passed, including PR manager, WSL quoting, commit-file parsing, untracked hostile filenames, and daemon channel registration.

[parsakhaz]

Verdict: Request changes. One additional Must-Fix surfaced during the full touched-file pass.
Counts: Must Fix: 1 (security: 0) · Should Fix: 0 · pass 2/3

Must Fix

• MF-3 The lazy patch request cancels itself when loading starts · frontend/src/components/git/PullRequestChanges.tsx:63 · do not depend on the loading state that the effect itself mutates.
  • Evidence: the effect includes diffLoading in its dependency list, then calls setDiffLoading(true). That rerun invokes cleanup, sets the first request's cancelled flag, and the replacement run exits because loading is true.
  • Failure scenario: clicking Show diff leaves the panel on Reading the patch forever, even when pr:get-diff succeeds.

[parsakhaz]

Verdict: Request changes. Rebase verification found one additional blocking test-wiring defect.
Counts: Must Fix: 1 (security: 0) · Should Fix: 0 · pass 3/3

Must Fix

• MF-4 Pull request daemon registration executes outside a test with undefined fixtures · main/src/ipc/daemonRegistryBindings.test.ts:359 · restore the named test block and remove stale imports from handlers no longer covered here.
  • Evidence: focused Vitest collection failed with ReferenceError: ipcMain is not defined; after restoring the block, all 12 daemon registry tests pass.
  • Failure scenario: CI cannot collect this suite, so the new daemon-owned PR channel registration has no executable verification.

[parsakhaz]

All four review defects are fixed on the current head: target-aware existing PR lookup, pre-push target base validation, stable lazy diff loading, and restored daemon IPC coverage. Current-head unit, browser, lint, and typecheck evidence is recorded in the PR summary.

[parsakhaz]

Final three-pass report

Current pushed head: 2ab27e158ae6cbe230373bb4c8dd0b2d17158250

1. Review

The correctness and security review found four Must-Fix defects and posted each with file and line evidence:

• Existing PR lookup could select a PR from the wrong target repository.
• A known-invalid target base could remain submittable, and backend validation happened after pushing.
• Lazy diff loading cancelled itself when loading state changed.
• The rebase left a daemon registry assertion outside its test block.

Fixed in 1cd85c61, a09d5f12, and f6c6142c. The follow-up review is APPROVED. No security defect remained after the fixes.

2. Simplify

Commit 2ddd81ff reduced boundary ambiguity and test indirection across the touched files:

• Decoded GitHub CLI JSON at the main-process boundary.
• Replaced filesystem module mocking with injected file operations.
• Removed dead exports, stale imports, conditional empty spreads, and redundant widening.
• Moved pull request URL parsing out of the React component.
• Updated tests to use typed mocks and focused assertions.

The current branch has zero blocking lint, Knip, boundary-conformance, or advisory anti-slop findings.

3. Refactor

Commit ab3f6541 extracted pure git output parsing from GitDiffManager into gitDiffParsers.ts, so pull request code no longer depends on the git manager implementation for parser utilities. It also makes latest reviewer verdict selection explicit by submission time and adds a reversed-order regression test.

Commit 2ab27e15 adds permanent Playwright coverage and PR IPC mock boundaries for the Create Pull Request dialog and review status strip.

Verification

• pnpm lint: PASS, including Oxlint, ESLint, Knip, and boundary conformance.
• pnpm typecheck: PASS across frontend, main, runpane, and shared workspaces.
• Focused main Vitest coverage: 120 tests PASS.
• Focused frontend Vitest coverage: 14 tests PASS.
• Focused unit total: 134 tests PASS.
• Playwright, Create Pull Request target and invalid-base flow: PASS.
• Playwright, expanded pull request review status flow: PASS.
• Existing pr-assets evidence published:
  • Create dialog
  • Invalid base blocked
  • Review status expanded

The branch is rebased on current main, pushed, MERGEABLE, and APPROVED. The currently reported GitHub checks are successful.

Follow-ups

• Consolidate the duplicated polling lifecycle and stale-state reset in PullRequestChecks and PullRequestStatusBar after a deterministic React component timing harness is available.
• Parsa should do one optional live smoke test against a disposable fork branch to exercise real gh pr create credentials and network behavior. Automated QA intentionally used mocked IPC and created no production GitHub object.
• Parsa retains the merge step. I did not merge.