Repository navigation
Surface per-PR CI status in sidebar - #6983
Closed
austinywang wants to merge 19 commits into
Closed
austinywang wants to merge 19 commits into
austinywang wants to merge 19 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds CI check-status models, GraphQL fetch plumbing, workspace/sidebar propagation, sidebar rendering, and related tests and support code. ChangesCI Check-Status Rollup Feature
Estimated code review effort: 4 (Complex) | ~60 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 3 warnings)
✅ Passed checks (20 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Greptile SummaryThis PR surfaces per-PR CI check status in the sidebar, threading
Confidence Score: 5/5Safe to merge — CI status is a best-effort cosmetic layer that degrades to neutral on any failure, so no authoritative PR badge data is at risk. Every edge case in the GraphQL path (timeout, null aliases, unknown rollup states, malformed CI raw values in the resolved item) is handled with a safe neutral fallback and confirmed by tests. The three previously flagged issues (REST/GraphQL coverage mismatch, dual source of truth in the cache, badge suppression on unknown CI values) are all resolved in this revision. Internationalization covers all 20 supported locales. The URLSession-per-call pattern matches existing REST fetch code. No production logic bugs, actor isolation issues, ambient global state, or test seams in production source were found. No files require special attention. Important Files Changed
Reviews (17): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…r-pr-ci-cd-check-status-in-the-side # Conflicts: # .github/swift-file-length-budget.tsv
- Remove `checkStatusesByNormalizedBranch`, which had no production caller (the per-branch path reads the embedded `ciStatus` via `cachedCheckStatus`); drop its now-orphaned test assertions. - Document why individual aliased-node decode failures are tolerated so the `try?` is intentional rather than silent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extract the per-branch-lookup fold into a pure `foldingBranchLookupResults` helper (behavior-preserving: still cached-status-only) and add a regression test asserting that a PR resolved only via the targeted `head=` lookup — i.e. outside the recent REST window the window-wide GraphQL fetch covers — gets its fetched CI rollup applied rather than defaulting to `.neutral`. This commit is the failing half of the two-commit regression structure; the fold ignores `fetchedCheckStatuses` here so the new test goes red. The fix follows in the next commit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…imeout P2 (autoreview): a PR resolved only via the targeted `head=` branch lookup is absent from the recent REST window, so the window-wide GraphQL rollup fetch never covered it and its badge defaulted to `.neutral` permanently. Now `branchLookupOutcome` fetches rollups for the found branch-lookup PRs in one batched GraphQL call, and `foldingBranchLookupResults` prefers a freshly fetched status over a cached one. Turns the regression test added in the prior commit green. (`pullRequestCheckStatuses` skips the network when there are no open PRs, so the cache-hit fast path stays free.) P3 (autoreview): restore a bounded timeout to `GatedMetadataReader.waitForTrackedPathEventGenerationProbe`. The continuation rewrite dropped the old bounded-yield giveup, so a probe that never arrives hung the whole suite. The continuation approach is kept (no yield spin loop); a timeout task resolves the waiter by id so it is resumed exactly once. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/macOS/CmuxSidebarGit/Sources/CmuxSidebarGit/Service/PullRequestPollService+Apply.swift (1)
92-105: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDon't make CI parsing a hard requirement for resolved PR updates.
If
ciStatusRawValueis unknown, thiscontinuedrops the entire resolved PR badge update and also skipsscheduleNextWorkspacePullRequestPoll(...)for that key on this pass. The PR status and URL are still authoritative here; only the CI glyph should degrade to.neutral, which also matches the fallback already used in the app-side bridge.As per path instructions,
.github/review-bot-rules/reliability-single-source-of-truth.mdsays the GraphQL-unavailable path must not map to a false state, and the PR objective says unavailable CI should remain neutral/hidden rather than suppress the PR row.Proposed fix
- guard let status = PullRequestStatus(rawValue: resolvedPullRequest.statusRawValue), - let ciStatus = PullRequestCheckStatus(rawValue: resolvedPullRequest.ciStatusRawValue), - let url = URL(string: resolvedPullRequest.urlString) else { + guard let status = PullRequestStatus(rawValue: resolvedPullRequest.statusRawValue), + let url = URL(string: resolvedPullRequest.urlString) else { continue } + let ciStatus = PullRequestCheckStatus(rawValue: resolvedPullRequest.ciStatusRawValue) ?? .neutral host.updatePanelPullRequest( workspaceId: result.workspaceId, panelId: result.panelId,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/macOS/CmuxSidebarGit/Sources/CmuxSidebarGit/Service/PullRequestPollService`+Apply.swift around lines 92 - 105, The resolved PR update path in PullRequestPollService+Apply should not abort when ciStatusRawValue cannot be parsed, since that drops the whole badge update and the next poll scheduling for that workspace key. Update the logic around the guard in the resolved PR handling to require only PullRequestStatus and URL, then map an unknown PullRequestCheckStatus to .neutral before calling host.updatePanelPullRequest and scheduleNextWorkspacePullRequestPoll, matching the existing fallback behavior in the app-side bridge.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/macOS/CmuxGit/Sources/CmuxGit/Probe/PullRequestProbeService`+CheckStatusGraphQL.swift:
- Around line 5-10: The branch-miss fallback in pull request check status lookup
is still doing a linear scan through pullRequestsByBranch.values, which makes
each miss O(n). Update PullRequestProbeService+CheckStatusGraphQL’s
pullRequestCheckStatuses path to build or reuse a second PR-number index (or
equivalent lookup map) alongside the existing branch cache so cachedCheckStatus
can resolve by pull request number in O(1) without re-scanning the whole
repository cache.
In
`@Packages/macOS/CmuxGit/Sources/CmuxGit/Probe/PullRequestProbeService`+Fetch.swift:
- Around line 248-278: The branch-lookup folding path is ignoring the fetched
GraphQL CI rollups, so PRs discovered via head= lookups stay neutral. Update
branchLookupOutcome and foldingBranchLookupResults to pass through and consult
fetchedCheckStatuses before falling back, using the existing cachedCheckStatus
and applyingCheckStatus helpers to attach the authoritative status to found pull
requests.
In `@Packages/macOS/CmuxSidebarGit/Tests/CmuxSidebarGitTests/Support/Fakes.swift`:
- Around line 35-50: Restore a bounded failure path in
`waitForTrackedPathEventGenerationProbe` so test callers do not block
indefinitely when no probe is ever recorded. Update the `Fakes`/`ProbeWaiter`
flow to keep a deterministic timeout or bounded polling fallback (or accept an
explicit deadline) and ensure `probeWaiters` still resumes only within that
bound. Keep the waiting logic in `waitForTrackedPathEventGenerationProbe` and
the resume path in the `probeWaiters` handling aligned so the method returns
`false` on timeout instead of hanging the suite.
---
Outside diff comments:
In
`@Packages/macOS/CmuxSidebarGit/Sources/CmuxSidebarGit/Service/PullRequestPollService`+Apply.swift:
- Around line 92-105: The resolved PR update path in
PullRequestPollService+Apply should not abort when ciStatusRawValue cannot be
parsed, since that drops the whole badge update and the next poll scheduling for
that workspace key. Update the logic around the guard in the resolved PR
handling to require only PullRequestStatus and URL, then map an unknown
PullRequestCheckStatus to .neutral before calling host.updatePanelPullRequest
and scheduleNextWorkspacePullRequestPoll, matching the existing fallback
behavior in the app-side bridge.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c3a7f8d4-48bf-4af9-b423-5789893feedc
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (33)
Packages/macOS/CmuxGit/Sources/CmuxGit/Model/GitHubPullRequestProbeItem.swiftPackages/macOS/CmuxGit/Sources/CmuxGit/Model/PullRequestCheckStatus.swiftPackages/macOS/CmuxGit/Sources/CmuxGit/Model/WorkspacePullRequestGraphQLCommit.swiftPackages/macOS/CmuxGit/Sources/CmuxGit/Model/WorkspacePullRequestGraphQLCommitConnection.swiftPackages/macOS/CmuxGit/Sources/CmuxGit/Model/WorkspacePullRequestGraphQLCommitNode.swiftPackages/macOS/CmuxGit/Sources/CmuxGit/Model/WorkspacePullRequestGraphQLData.swiftPackages/macOS/CmuxGit/Sources/CmuxGit/Model/WorkspacePullRequestGraphQLPullRequestConnection.swiftPackages/macOS/CmuxGit/Sources/CmuxGit/Model/WorkspacePullRequestGraphQLPullRequestNode.swiftPackages/macOS/CmuxGit/Sources/CmuxGit/Model/WorkspacePullRequestGraphQLRepository.swiftPackages/macOS/CmuxGit/Sources/CmuxGit/Model/WorkspacePullRequestGraphQLRequestBody.swiftPackages/macOS/CmuxGit/Sources/CmuxGit/Model/WorkspacePullRequestGraphQLResponse.swiftPackages/macOS/CmuxGit/Sources/CmuxGit/Model/WorkspacePullRequestGraphQLStatusCheckRollup.swiftPackages/macOS/CmuxGit/Sources/CmuxGit/Model/WorkspacePullRequestGraphQLVariables.swiftPackages/macOS/CmuxGit/Sources/CmuxGit/Model/WorkspacePullRequestRepoCacheEntry.swiftPackages/macOS/CmuxGit/Sources/CmuxGit/Model/WorkspacePullRequestResolvedItem.swiftPackages/macOS/CmuxGit/Sources/CmuxGit/Probe/PullRequestProbeService+CheckStatusGraphQL.swiftPackages/macOS/CmuxGit/Sources/CmuxGit/Probe/PullRequestProbeService+Fetch.swiftPackages/macOS/CmuxGit/Sources/CmuxGit/PullRequestProbeService.swiftPackages/macOS/CmuxGit/Tests/CmuxGitTests/PullRequestProbeServiceTests.swiftPackages/macOS/CmuxSidebar/Sources/CmuxSidebar/Git/SidebarPullRequestCIStatus.swiftPackages/macOS/CmuxSidebar/Sources/CmuxSidebar/Git/SidebarPullRequestState.swiftPackages/macOS/CmuxSidebar/Tests/CmuxSidebarTests/SidebarValueVocabularyTests.swiftPackages/macOS/CmuxSidebarGit/Sources/CmuxSidebarGit/Model/SidebarPullRequestBadge.swiftPackages/macOS/CmuxSidebarGit/Sources/CmuxSidebarGit/Service/PullRequestPollService+Apply.swiftPackages/macOS/CmuxSidebarGit/Sources/CmuxSidebarGit/Service/PullRequestPollService+CommandHints.swiftPackages/macOS/CmuxSidebarGit/Tests/CmuxSidebarGitTests/PullRequestPollServiceTests.swiftPackages/macOS/CmuxSidebarGit/Tests/CmuxSidebarGitTests/Support/Fakes.swiftResources/Localizable.xcstringsSources/ContentView.swiftSources/TabManager+SidebarGitHosting.swiftSources/Workspace+CustomSidebarPullRequests.swiftSources/Workspace.swiftcmuxTests/WorkspaceCustomSidebarPullRequestContextTests.swift
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/macOS/CmuxSidebarGit/Sources/CmuxSidebarGit/Service/PullRequestPollService`+Apply.swift:
- Around line 92-98: The PR badge is being dropped when an unrecognized CI value
is encountered because `Self.resolvedPullRequestBadge(from:)` currently fails
the whole badge build; update that path so `ciStatusRawValue` defaults to
`.neutral` instead of returning nil, while keeping the existing fatal handling
for invalid status and URL values. Make the change in
`resolvedPullRequestBadge(from:)` so `PullRequestPollService+Apply` can still
call `host.updatePanelPullRequest(...)` with a badge for resolved PRs even when
CI is unknown.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 71379e25-a206-4a4b-95b4-5230f25bf672
📒 Files selected for processing (4)
Packages/macOS/CmuxGit/Sources/CmuxGit/Probe/PullRequestProbeService+Fetch.swiftPackages/macOS/CmuxSidebarGit/Sources/CmuxSidebarGit/Service/PullRequestPollService+Apply.swiftPackages/macOS/CmuxSidebarGit/Tests/CmuxSidebarGitTests/PullRequestPollServiceTests.swiftPackages/macOS/CmuxSidebarGit/Tests/CmuxSidebarGitTests/Support/Fakes.swift
… PR row Extract the resolved-badge construction into a pure `resolvedPullRequestBadge` helper and degrade an unparseable `ciStatusRawValue` to `.neutral` rather than failing the guard. Previously an unknown CI value made the `.resolved` apply path `continue`, dropping the entire PR badge update and skipping the next poll scheduling for that key — even though the PR's status and URL are authoritative. Aligns with .github/review-bot-rules/reliability-single-source-of-truth.md and the PR objective (unavailable CI stays neutral, never suppresses the row), and matches the `.neutral` fallback already used on the app-side bridge. Adds a regression test for the degrade and the nil-on-unusable-status/URL paths. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
austinywang
force-pushed
the
issue-5748-surface-per-pr-ci-cd-check-status-in-the-side
branch
from
June 30, 2026 01:30
e8c1198 to
8faf7ae
Compare
…r-pr-ci-cd-check-status-in-the-side # Conflicts: # .github/swift-file-length-budget.tsv
…r-pr-ci-cd-check-status-in-the-side # Conflicts: # .github/swift-file-length-budget.tsv
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Workspace.swift (1)
4958-4966: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
updatePanelPullRequestshould not default CI state to.neutral
Sources/TerminalController+ControlSidebarContext2.swift:114updates the panel PR without forwardingciStatus, so a branch/status refresh can silently reset a known pass/fail badge to neutral. MakeciStatusrequired here or thread through the current panel value instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/Workspace.swift` around lines 4958 - 4966, updatePanelPullRequest is currently defaulting ciStatus to .neutral, which lets callers like TerminalController+ControlSidebarContext2.updatePanelPullRequest accidentally overwrite an existing pass/fail badge during refresh. Make ciStatus required in Workspace.updatePanelPullRequest, or change the caller to pass through the panel’s current CI value instead of relying on the default. Update any related call sites and the updatePanelPullRequest signature so the existing CI state is preserved.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Sources/Workspace.swift`:
- Around line 4958-4966: updatePanelPullRequest is currently defaulting ciStatus
to .neutral, which lets callers like
TerminalController+ControlSidebarContext2.updatePanelPullRequest accidentally
overwrite an existing pass/fail badge during refresh. Make ciStatus required in
Workspace.updatePanelPullRequest, or change the caller to pass through the
panel’s current CI value instead of relying on the default. Update any related
call sites and the updatePanelPullRequest signature so the existing CI state is
preserved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 90eda4af-e1df-4e0a-a972-2218dbea25a3
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (3)
Resources/Localizable.xcstringsSources/ContentView.swiftSources/Workspace.swift
The per-repo refresh awaited the CI status GraphQL call on the shared REST session (8s timeout) before building the cache entry and returning any PR results. Because CI status is optional UI metadata layered on the authoritative REST PR data, a slow or unavailable rollup should never delay the core PR status/URL badge update. Give the CI lookup its own dedicated, much-shorter-timeout session (checkStatusProbeTimeout = 3s). On timeout it yields no statuses and PRs keep their neutral/cached CI glyph — the authoritative badge refresh is bounded by a small fraction of the REST budget instead of the full 8s. No return-contract change, so branch-lookup PRs still receive fetched CI when it responds promptly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 34 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…r-pr-ci-cd-check-status-in-the-side # Conflicts: # .github/swift-file-length-budget.tsv
…r-pr-ci-cd-check-status-in-the-side # Conflicts: # .github/swift-file-length-budget.tsv
…r-pr-ci-cd-check-status-in-the-side # Conflicts: # .github/swift-file-length-budget.tsv
…r-pr-ci-cd-check-status-in-the-side
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
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
3 participants
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.
Fixes #5748
Summary
Testing
Feature work; skipped crash reproduction/failing-regression-first flow. Per request, no reload.sh/dev app build/xcodebuild was run.
Summary by cubic
Shows CI status (pending/pass/fail) beside open PRs in the sidebar. Batch-fetches rollups via GraphQL with a dedicated 3s timeout so badges never wait on CI; threads status through the poller, cache, and UI with localized help and JSON export. Addresses #5748.
New Features
statusCheckRollupfor candidate open PRs only; dedupe and preserve order; map toneutral/success/failure; embedciStatusin probe/resolved items, badges, sidebar state, and app bridges; render icons for open rows with localized help; includeciStatusin custom sidebar JSON.head=lookups, fetch rollups for found PRs and prefer freshly fetched status over cached.Bug Fixes
neutralinstead of suppressing the PR row (including resolved-badge builds); command-hint reconciliation preserves CI status.Written for commit 414ed64. Summary will update on new commits.
Summary by CodeRabbit