Surface GitHub PR checks in the sidebar - #111
Conversation
There was a problem hiding this comment.
ℹ️ No critical issues — minor suggestions inline.
Reviewed changes
Reviewed the full 14-file diff at 245de6eb: the new GitHub PR/check service, its workspace-scoped IPC surface, and the sidebar badge.
- New
gh-backed PR status service —main/services/github-pull-request.tsshells out togh pr view --json …with a resolved binary, Git-routing-stripped environment, 15s timeout, and bounded output, then normalizes checks and redacts renderer-facing errors. - Workspace-scoped IPC —
git:pullRequestStatusresolves the workspace throughwithOptionalWorkspaceOperation, short-circuits non-repos and remote-less repos, and passes the operation signal into the service. - Sidebar badge and popover —
WorkspacePullRequestBadge, rendered only while a workspace is explicitly expanded, polls every 30s, shows the check rollup, and opens PR/check URLs through the existing main-window external-open handler. - Refresh wiring — branch switch, commit, and push invalidate
gitPullRequestStatus; main/renderer types and the sidebar and phase-3 contract tests were extended, and the new service test is registered intestandtest:coverage.
ℹ️ Nitpicks
publicCommandMessage(main/services/github-pull-request.ts:124-134) redacts only the exactcwd/home and a fixed query-param list, so absolute paths outside the workspace (including theCommand failed: /opt/homebrew/bin/gh …prefixexecFilebuilds) and param names likeprivate_tokenreach the popover verbatim. Low severity since the string is only shown locally, but parsing URLs and redacting absolute paths generally would match what the redaction test implies.
DeepSeek Flash | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed the delta since the prior pullfrog review (245de6eb → 164bcb66): a redaction-hardening commit for the GitHub CLI error path and its test.
- Hardened CLI error redaction —
publicCommandMessagenow masks multi-segment absolute paths viaredactAbsolutePaths(covering theCommand failed: /opt/homebrew/bin/gh …prefixexecFilebuilds) and redactsprivate_tokenalongside the existing query-param list. - Extended the redaction test — the service test now feeds a realistic
Command failed: …message and asserts the binary path, workspace path, andprivate_tokenvalue never reach the renderer-facing message.
The earlier publicCommandMessage nitpick is addressed. The previously flagged queued-rerun ordering in shouldReplaceCheck is unchanged and its review thread remains open, so this is not an approval.
DeepSeek Flash | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed the delta since the prior pullfrog review (164bcb66 → b5d4a90): the queued-rerun ordering fix in check dedupe and its test.
- Fixed order-dependent queued-rerun selection —
shouldReplaceChecknow evaluates the zero-timestamppending/action-requiredcase in both directions before comparing timestamps, so a queued rerun beats a superseded completed failure regardless of rollup position. - Pinned both rollup orders in the test — the rerun test now feeds the same completed/queued pair in both orders and asserts
pendingeither way.
The prior review's shouldReplaceCheck thread is addressed and resolved. Verification this run: npx tsx --test main/services/github-pull-request.test.ts (15 pass), npx tsc --noEmit, and npx eslint on both changed files all pass.
DeepSeek Flash | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed the delta since the prior pullfrog review (b5d4a90 → 4e9200f8): a check-rollup behavior fix plus GitHub CLI environment isolation, with matching tests.
- Cancelled checks now count as failing —
rollupGitHubChecksStatereturnsfailingwhen any check iscancelled, so a cancelled-only rollup no longer falls through to the "Checks did not run" default; the test pins both the cancelled-only and success-plus-cancelled rollups. - Stripped
GH_HOSTandGH_REPOfrom the CLI environment —githubCliEnvironmentnow deletes both so an inherited value cannot redirectgh pr viewto a different host or repository; the env test asserts they are removed whileGH_TOKENis preserved.
All prior pullfrog threads are resolved, and verification this run passes: npx tsx --test main/services/github-pull-request.test.ts (15), npx tsc --noEmit, and npx eslint on both changed files.
DeepSeek Flash | 𝕏

Summary
Verification
Review