feat(ui): add Cmd+P pull request dropdown - #309
Merged
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb76d7cfb4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
When the focused terminal sits inside a GitHub-cloned repo, ⌘P now expands a pill that lists open pull requests fetched via `gh pr list` on a worker thread. Selecting an entry sends `gh pr checkout <N>` to the focused shell. The collapsed pill shows the current branch's PR number (e.g. #123) when the HEAD ref matches a fetched PR's head branch. Detection of the GitHub origin and current branch is done synchronously on the main thread from `.git/config` and `.git/HEAD` (including worktree indirection), so the pill is hidden cheaply when the cwd is not a GitHub repo. Listing PRs hits the network through gh, so that work runs on a per-open std.Thread with results delivered via a mutex-guarded slot and an atomic completion flag, matching the spirit of ADR-009. https://claude.ai/code/session_01BUkFdLJ8vdB36bfZ13MksA
forketyfork
added a commit
that referenced
this pull request
Aug 21, 2026
Issue: PR #309 was based on stale main, failed against the current UI APIs, and could apply pull request data from a previous repository or leave its branch badge stale.\nSolution: Rebase the feature onto current main, adapt its search field and test registration to the current UI foundations, and keep repository-keyed fetch jobs so stale results are discarded while the newly focused repository fetches immediately. Refresh the branch from HEAD on each update and cover repository/result and branch/PR matching with tests.
forketyfork
force-pushed
the
claude/cmd-p-pr-dropdown-ydwn5
branch
from
August 21, 2026 14:43
cb76d7c to
3183b79
Compare
Issue: PR #309 was based on stale main, failed against the current UI APIs, and could apply pull request data from a previous repository or leave its branch badge stale.\nSolution: Rebase the feature onto current main, adapt its search field and test registration to the current UI foundations, and keep repository-keyed fetch jobs so stale results are discarded while the newly focused repository fetches immediately. Refresh the branch from HEAD on each update and cover repository/result and branch/PR matching with tests.
forketyfork
force-pushed
the
claude/cmd-p-pr-dropdown-ydwn5
branch
from
August 21, 2026 14:49
3183b79 to
8b0040d
Compare
Issue: PR dropdown fetch failures only surfaced as a generic UI message, leaving no diagnostic record for malformed gh responses or CLI failures.\nSolution: Add scoped error logs for gh lifecycle failures and JSON processing, including the repository path and escaped, bounded output previews. Document the new diagnostics so future dropdown failures can be investigated from the configured log.
Issue: Reproduced PR fetch failures caused the diagnostic output preview to exceed Architect's 4 KiB logging buffer, so the malformed gh response was still omitted from the log.\nSolution: Format escaped output into a fixed 2 KiB buffer, escaping structured-log delimiters and stopping at an encoded boundary. Add a regression test for the bound and document the effective preview limit.
Issue: gh can emit ANSI color sequences around --json output, causing the pull request dropdown to report a JSON parse failure.\n\nSolution: remove ANSI CSI terminal formatting before parsing gh output and cover the captured colorized response with a regression test. Document the normalization in the logging configuration reference.
Issue: four-digit pull request numbers can exceed the compact PR pill and render outside its bounds.\n\nSolution: constrain oversized PR number textures to the pill's available width while preserving their aspect ratio, leaving the default Cmd+P glyph unchanged. Add a regression test for proportional fitting and document the behavior.
Issue: the PR overlay rendered above sibling pills during their expansion, and the collapsed badge stayed at Cmd+P until the overlay was opened after entering a repository.\n\nSolution: render the PR component below the other pill overlays and start the initial repository-keyed PR fetch as soon as a GitHub repository becomes focused. The resulting branch match updates the collapsed badge without requiring an overlay interaction.
Issue: pr_dropdown.zig combined UI orchestration, repository inspection, gh process handling, JSON parsing, and SDL rendering in one large component. Solution: split those responsibilities into model, repository, fetch, and view modules while keeping background-job ownership and UI lifecycle in the component. Register every new test-bearing module and document the boundaries.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
PR #309 was created against an old
main, conflicted with the current branch, and its original CI run failed. The PR dropdown also had correctness issues around stale repository results and branch badges, and fetch failures were reduced to a generic UI message with no diagnostic output. Its implementation had grown into a large file combining unrelated responsibilities.Solution
Rebased the feature onto current
mainand adapted it to the current UI text-input and test-registration infrastructure. Pull-request fetches now use repository-keyed worker jobs; results are applied only for the focused repository, while stale jobs are discarded and a new repository fetch starts immediately. The component rereads.git/HEADduring updates so branch badges follow checkouts without cwd changes.ghlaunch, output collection, exit, JSON-shape, and parsing failures now emit scopedERRORlogs with the repository path and escaped stdout/stderr previews capped at 2 KiB after escaping per stream. The preview is formatted into a fixed buffer so it stays below the logger's 4 KiB record limit. The logging behavior is documented indocs/configuration.md.The PR dropdown is now decomposed into model, repository, fetch, and view modules. The component retains UI lifecycle, input handling, and fetch-job ownership, while the new modules isolate domain helpers, Git inspection,
ghparsing, and rendering/cache code. Every test-bearing module is registered explicitly, and the boundaries are documented indocs/ARCHITECTURE.md.Context
Original implementation context: https://claude.ai/code/session_01BUkFdLJ8vdB36bfZ13MksA
Both original review threads were addressed, replied to, and resolved. The branch now contains the refactor in commit
060aa0band was force-pushed after verification.Test plan
pr_dropdownentry includes the repository path and bounded escaped output preview without amessage exceeded 4096-byte logging bufferentry.