Repository navigation
Refresh after a clone on the render thread, and allow one clone per folder [patch] - #474
Merged
Merged
Conversation
…older [patch] The Clone button's continuation ran RefreshPage on the worker thread that did the clone, since the app has no SynchronizationContext and the continuation asked to run synchronously. From there it mutated ClonedRepos, enumerated Repos and replaced the repo browser while the render thread was reading them, and a second click started another clone into the same folder. A new CloneTracker records the clones in flight and a refresh request. The worker only marks its clone complete; Tick takes the request and refreshes on the render thread, and the button is disabled while its clone runs. Fixes #439 Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01KLpJqJzDAe9WoXTVLt65CJ
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01KLpJqJzDAe9WoXTVLt65CJ
Moves the background clone, its completion and the tick's refresh into CloneTracker.TryRun, CloneTracker.RefreshIfRequested and ProjectDirector.MakeClone, so the parts with a rule in them are tested rather than sitting in the ImGui layer. The Clone button is now hidden while its clone runs rather than disabled. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01KLpJqJzDAe9WoXTVLt65CJ
Sonar S4586: a method returning Task should never return null. The task is now an out parameter, completed when the clone is refused. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01KLpJqJzDAe9WoXTVLt65CJ
|
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
2 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 #439
Problem
The Clone button's continuation used
TaskScheduler.CurrentwithExecuteSynchronously. The app installs noSynchronizationContext, soRefreshPage()ran on the worker thread that did the clone. From there it changedOptions.ClonedRepos, enumeratedOptions.Reposand replacedBrowserContentsBasewhile the render thread was reading all three every frame. Nothing stopped a second click from starting anothergit cloneinto the same folder.Changes
CloneTracker:TryRun(localPath, clone)runs the clone on the thread pool, unless a clone into that folder is already running.finallyblock, so a clone that fails or throws can be retried.Interlocked. It never touches UI state.TickcallsClones.RefreshIfRequested(RefreshPage), so the page is refreshed on the render thread. Other background results, such asOwnerPendingTokenPopup, are already handed over the same way.ProjectDirector.MakeClonebuilds the clone action. Only a few lines of ImGui wiring are left untested.Tests
CloneTrackerTestscovers:TryRunrefusing a second clone while the first is blocked, then requesting a refresh when it ends;RefreshIfRequestedrefreshing once per completion;MakeClonecloning a real local repository and logging the result.With
CloneTrackerstubbed back to the old behaviour (no in-flight guard, no handoff to the render thread), the duplicate-clone test and the handoff test fail.The race itself can't be reproduced deterministically, so the tests pin down the handoff and the guard it depends on.
Full suite: 141 of 142 pass locally. The exception,
GitCliTests.CloningAnLfsRepositoryRestoresTheFileContentRatherThanThePointer, fails only in this sandbox because of its git-lfs setup. It passes in CI on all three platforms.dotnet format --verify-no-changesreports only the two findingsProjectDirector.csalready has onmain.This PR stands alone on
main, and it merges cleanly with #473 (for #438).🤖 Generated with Claude Code
https://claude.ai/code/session_01KLpJqJzDAe9WoXTVLt65CJ