Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import Foundation

/// The repository discovery a pull-request refresh does before it talks to
/// GitHub: resolving a directory to its GitHub slugs and its checked-out branch.
///
/// Both are blocking filesystem work — they walk up for a `.git` directory and
/// read config files — so a refresh must keep them off the main thread. Keeping
/// this boundary separate from GitHub transport lets the pull-request pipeline
/// resolve local candidates before it performs any authenticated network work.
/// ``GitMetadataService`` is the production implementation.
public protocol GitRepositoryDiscovering: Sendable {
/// Ordered, de-duplicated GitHub slugs for the repository containing
/// `directory`; empty when there is no repository or no GitHub remote.
func repositorySlugs(forDirectory directory: String) async -> [String]

/// The ``GitCheckedOutBranch`` for the repository containing `directory`, or
/// ``GitCheckedOutBranch/notARepository`` when there is none.
func checkedOutBranch(forDirectory directory: String) async -> GitCheckedOutBranch
}

extension GitMetadataService: GitRepositoryDiscovering {}
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public struct PullRequestProbeService: Sendable {
/// - Returns: The candidates plus repo-keyed indexes for the fetch stage.
public nonisolated func resolveCandidateSeeds(
_ seeds: [WorkspacePullRequestCandidateSeed],
gitMetadata: GitMetadataService
gitMetadata: any GitRepositoryDiscovering
) async -> WorkspacePullRequestCandidateResolution {
var candidates: [WorkspacePullRequestCandidate] = []
candidates.reserveCapacity(seeds.count)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public final class PullRequestPollService: PullRequestProbing {
// MARK: Dependencies

// Resolves slugs for candidate seeds (stateless CmuxGit reader).
let gitMetadataService: GitMetadataService
let gitMetadataService: any GitRepositoryDiscovering
// Fetches and matches GitHub PRs (stateless CmuxGit pipeline).
let probeService: PullRequestProbeService
// Drives the poll deadline and mobile-host deferral sleeps.
Expand Down Expand Up @@ -67,7 +67,7 @@ public final class PullRequestPollService: PullRequestProbing {
/// - mobileHostDeferral: Mobile-host deferral intervals.
/// - debugLog: Diagnostics sink; defaults to a no-op.
public init(
gitMetadataService: GitMetadataService,
gitMetadataService: any GitRepositoryDiscovering,
probeService: PullRequestProbeService,
clock: any GitPollClock = SystemGitPollClock(),
mobileHostDeferral: MobileHostDeferralPolicy = .standard,
Expand Down
229 changes: 126 additions & 103 deletions cmuxTests/WorkspacePullRequestSidebarTests.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import XCTest
import Darwin
import CmuxFoundation
import CmuxGit
import CmuxSettings
import CmuxSidebarGit

import CmuxSidebar

Expand All @@ -24,43 +27,70 @@ private struct StubCommandRunner: CommandRunning {
}
}

private final class CommandRunnerInvocationCounter: @unchecked Sendable {
private final class BlockingRepositoryDiscovery: GitRepositoryDiscovering, @unchecked Sendable {
private let lock = NSLock()
private var storedValue = 0
private let releaseGate = DispatchSemaphore(value: 0)
private let startedExpectation: XCTestExpectation
private let finishedExpectation: XCTestExpectation
private var storedInvocationCount = 0
private var storedReachedCleanupDeadline = false
private var storedReleased = false

init(
startedExpectation: XCTestExpectation,
finishedExpectation: XCTestExpectation
) {
self.startedExpectation = startedExpectation
self.finishedExpectation = finishedExpectation
}

func increment() {
func repositorySlugs(forDirectory directory: String) async -> [String] {
recordInvocationAndBlockUntilReleased()
return []
}

func checkedOutBranch(forDirectory directory: String) async -> GitCheckedOutBranch {
.notARepository
}

private func recordInvocationAndBlockUntilReleased() {
lock.lock()
storedValue += 1
storedInvocationCount += 1
lock.unlock()

startedExpectation.fulfill()
// This deadline is only deadlock cleanup. The test releases the gate
// after it observes a queued main-run-loop turn; reaching the deadline
// is itself a failure signal.
if releaseGate.wait(timeout: .now() + 5) == .timedOut {
lock.lock()
storedReachedCleanupDeadline = true
storedReleased = true
lock.unlock()
}
finishedExpectation.fulfill()
}

var value: Int {
func release() {
lock.lock()
defer { lock.unlock() }
return storedValue
let shouldSignal = !storedReleased
storedReleased = true
lock.unlock()
if shouldSignal {
releaseGate.signal()
}
}
}

/// Records whether any observation happened on the main thread. Used to assert
/// that off-main work (e.g. PR-refresh git commands) never executes on the main
/// thread, a deterministic signal that does not depend on wall-clock timing.
private final class MainThreadObservationBox: @unchecked Sendable {
private let lock = NSLock()
private var storedObservedOnMainThread = false

func recordCurrentThread() {
let onMain = Thread.isMainThread
var invocationCount: Int {
lock.lock()
if onMain {
storedObservedOnMainThread = true
}
lock.unlock()
defer { lock.unlock() }
return storedInvocationCount
}

var observedOnMainThread: Bool {
var reachedCleanupDeadline: Bool {
lock.lock()
defer { lock.unlock() }
return storedObservedOnMainThread
return storedReachedCleanupDeadline
}
}

Expand Down Expand Up @@ -550,96 +580,89 @@ final class WorkspacePullRequestSidebarTests: XCTestCase {
}

func testPullRequestRefreshRepositoryDiscoveryDoesNotBlockMainRunLoop() throws {
let invocationCounter = CommandRunnerInvocationCounter()
let gitThreadObservation = MainThreadObservationBox()
let commandDelay: TimeInterval = 0.03
let commandRunner = StubCommandRunner { _, executable, arguments, _ in
if executable == "git", arguments == ["remote", "-v"] {
invocationCounter.increment()
gitThreadObservation.recordCurrentThread()
Thread.sleep(forTimeInterval: commandDelay)
return CommandResult(
stdout: "origin\tssh://example.invalid/not-github.git (fetch)\n",
stderr: "",
exitStatus: 0,
timedOut: false,
executionError: nil
)
}
return CommandResult(
stdout: "",
stderr: "",
exitStatus: 0,
timedOut: false,
executionError: nil
)
let defaults = UserDefaults.standard
let sidebarSettings = SidebarCatalogSection()
let hideAllDetailsKey = sidebarSettings.hideAllDetails.userDefaultsKey
let previousWatchGitStatus = defaults.object(forKey: SidebarWorkspaceDetailDefaults.watchGitStatusKey)
let previousShowPullRequests = defaults.object(forKey: SidebarWorkspaceDetailDefaults.showPullRequestsKey)
let previousHideAllDetails = defaults.object(forKey: hideAllDetailsKey)
defer {
restoreUserDefault(previousWatchGitStatus, key: SidebarWorkspaceDetailDefaults.watchGitStatusKey)
restoreUserDefault(previousShowPullRequests, key: SidebarWorkspaceDetailDefaults.showPullRequestsKey)
restoreUserDefault(previousHideAllDetails, key: hideAllDetailsKey)
}
defaults.set(true, forKey: SidebarWorkspaceDetailDefaults.watchGitStatusKey)
defaults.set(true, forKey: SidebarWorkspaceDetailDefaults.showPullRequestsKey)
defaults.set(false, forKey: hideAllDetailsKey)

let manager = TabManager(commandRunner: commandRunner)
var seededPanels: [(workspaceId: UUID, panelId: UUID)] = []
let workspaceCount = 45
var workspaces = manager.tabs
while workspaces.count < workspaceCount {
workspaces.append(manager.addWorkspace(select: false, eagerLoadTerminal: false))
let discoveryStarted = expectation(description: "repository discovery started")
discoveryStarted.assertForOverFulfill = true
let discoveryFinished = expectation(description: "repository discovery finished")
discoveryFinished.assertForOverFulfill = true
let discovery = BlockingRepositoryDiscovery(
startedExpectation: discoveryStarted,
finishedExpectation: discoveryFinished
)
defer {
discovery.release()
}

for (index, workspace) in workspaces.enumerated() {
let panelId = try XCTUnwrap(workspace.focusedPanelId)
workspace.updatePanelDirectory(
panelId: panelId,
directory: "/tmp/cmux-pr-refresh-main-thread-\(index)"
)
workspace.updatePanelGitBranch(
panelId: panelId,
branch: "issue-3033-\(index)",
isDirty: false
)
seededPanels.append((workspace.id, panelId))
}
let manager = TabManager()
let workspace = try XCTUnwrap(manager.selectedWorkspace)
let panelId = try XCTUnwrap(workspace.focusedPanelId)
workspace.updatePanelDirectory(
panelId: panelId,
directory: "/tmp/cmux-pr-refresh-main-run-loop"
)
workspace.updatePanelGitBranch(
panelId: panelId,
branch: "issue-3033-main-run-loop",
isDirty: false
)

let monitorDuration: TimeInterval = 0.7
// Generous bound far above macOS CI scheduling noise (GC, unrelated test
// work, run-loop jitter can stall the main thread well past a few hundred
// ms on a loaded shared runner). This catches gross main-thread blocking
// without failing on routine host jitter; the deterministic non-main-thread
// assertion below is the real regression signal.
let allowedMainThreadGap: TimeInterval = 2.0
let finishedMonitoring = expectation(description: "main run loop remained responsive")
let monitorStartedAt = Date()
var lastTickAt = monitorStartedAt
var maxTickGap: TimeInterval = 0
let timer = Timer.scheduledTimer(withTimeInterval: 0.01, repeats: true) { timer in
let now = Date()
maxTickGap = max(maxTickGap, now.timeIntervalSince(lastTickAt))
lastTickAt = now
if now.timeIntervalSince(monitorStartedAt) >= monitorDuration {
timer.invalidate()
finishedMonitoring.fulfill()
}
}
let pollService = PullRequestPollService(
gitMetadataService: discovery,
probeService: manager.pullRequestProbeService
)
pollService.attach(host: manager)

let triggerPanel = try XCTUnwrap(seededPanels.first)
manager.updateSurfaceShellActivity(
tabId: triggerPanel.workspaceId,
surfaceId: triggerPanel.panelId,
state: .promptIdle
pollService.scheduleWorkspacePullRequestRefresh(
workspaceId: workspace.id,
panelId: panelId,
reason: "testMainRunLoopResponsiveness"
)

let result = XCTWaiter().wait(for: [finishedMonitoring], timeout: monitorDuration + 1.5)
timer.invalidate()
XCTAssertEqual(result, .completed)
XCTAssertGreaterThan(invocationCounter.value, 0)
// Deterministic regression signal: the blocking git work must have run off
// the main thread. This does not depend on wall-clock timing, so it cannot
// flake from host scheduling noise.
// The test releases discovery only after this queued main-run-loop turn
// executes. If discovery occupies the main thread, its cleanup deadline
// opens the gate instead and the assertions below fail.
let mainRunLoopTurnCompleted = expectation(description: "main run loop completed a queued turn")
DispatchQueue.main.async {
mainRunLoopTurnCompleted.fulfill()
}

let responsivenessResult = XCTWaiter().wait(
for: [discoveryStarted, mainRunLoopTurnCompleted],
timeout: 5
)
guard responsivenessResult == .completed else {
XCTFail("Repository discovery did not start while the main run loop remained responsive")
return
}
XCTAssertEqual(
discovery.invocationCount,
1,
"Pull request refresh should resolve repository slugs once for the tracked directory"
)
XCTAssertFalse(
gitThreadObservation.observedOnMainThread,
"Pull request refresh ran its blocking git command on the main thread"
discovery.reachedCleanupDeadline,
"Pull request repository discovery blocked the main run loop"
)
XCTAssertLessThan(
maxTickGap,
allowedMainThreadGap,
"Pull request refresh blocked the main run loop for \(maxTickGap) seconds"

discovery.release()
XCTAssertEqual(
XCTWaiter().wait(for: [discoveryFinished], timeout: 5),
.completed,
"Repository discovery did not finish after the test released it"
)
}

Expand Down