Finish a file propagation the user confirmed, and say what it did - #418
Conversation
Propagate() ran an unguarded File.Copy per repository, so the first locked destination or read-only directory ended the loop: every repository after it silently never got the file, and nothing was written to the log panel either way. Because every other long-running action in this application reports through QueueLog/QueueGitLog, that silence read as success. The exception also escaped a render callback, which can take the application down and lose unsaved options state. The copy loop moves to FilePropagation, apart from the ImGui layer so it can be driven without a live context -- the same shape as DecidePull. Each repository's copy is guarded and recorded, the batch runs to the end, and the summary line carries the counts and names the repositories that missed out, so a partial run does not have to be reconstructed from fifteen individual log lines. A missing source is checked once before anything is copied, because asking for the wrong file is a different failure from a locked destination and reporting it fifteen times would bury that. Fixes #415 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgX8kpAMnBEumW6PDBfynE
The build fails before it reaches a single test: NU1902 is an error here, and
Microsoft.SourceLink.{AzureRepos.Git,GitHub} 10.0.102 -- along with the
Microsoft.Build.Tasks.Git 10.0.102 they bring with them -- carries
GHSA-23fw-v26w-5fgq. Both are pinned to that version by VersionOverride, so
every build in this repository has been red on it, main and the open Dependabot
PRs included. 10.0.401 is the current release of the same package and clears the
advisory.
This is not what #415 is about, and it is only here because a red base branch
cannot be handed to a reviewer as a green PR. It is a self-contained two-line
bump and can be split out or dropped without touching the propagation change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FgX8kpAMnBEumW6PDBfynE
CI:
|
SonarCloud put new-code coverage at 66.7% against a gate of 80%. The uncovered half was not incidental: it was the part of the rule still sitting inside the ImGui layer, plus the failure arms of the guard that no test drove. ResolveDestinations and DescribeForLog move to FilePropagation. Both are real decisions worth pinning -- where a checked repository's copy lands, and the fact that only the summary carries a timestamp while the detail stays indented under it -- and neither needs a live ImGui context to run. Propagate() is left doing only what it has to: read the options, call them, write the lines. The NotSupportedException arm goes. File.Copy documents it for a path in an invalid format, but modern .NET raises ArgumentException for everything that reaches it here, so it was a catch no input could exercise. Four new tests cover the arms that were missing -- a destination whose parent is a file, one with no containing directory, one that is not a usable path, and the two extracted methods. FilePropagation.cs is now fully covered and new-code coverage is 86%. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgX8kpAMnBEumW6PDBfynE
SonarCloud raised 17 INFO findings on the new tests, all the same shape: a count compared with AreEqual where HasCount or IsEmpty says what is meant, and StringAssert where Assert now carries the equivalent. They cost nothing to take and the failure messages read better for it -- HasCount prints the collection, where AreEqual prints two bare integers. GitCliTests already uses Assert.IsEmpty, so this is the direction the tests are already moving in rather than a new convention. Note that Assert.Contains and Assert.StartsWith take the substring and the prefix first, which is the reverse of StringAssert. Each converted assertion was checked by breaking its expectation and confirming it still fails, so none of them passes vacuously on swapped arguments. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgX8kpAMnBEumW6PDBfynE
|
1 participant
Fixes #415
What was wrong
PopupPropagateFile.Propagate()ran an unguardedDirectory.CreateDirectory+File.Copyper repository, with notry/catchanywhere in the method. A user who selects 15 repositories and confirms gets an arbitrary prefix of them: the first locked destination or read-only directory ends the loop, the remaining repositories silently never get the file, and nothing is written to the log panel either way. Because every other long-running action in this application reports throughQueueLog/QueueGitLog, that silence reads as success. The exception also escaped a render callback, which can take the application down and lose unsaved options state.What changed
ProjectDirector/FilePropagation.csholds the copy loop, apart from the ImGui layer so it can be driven without a live context — the same shapeDecidePullalready has, and whatCLAUDE.mdasks for when something worth testing is tangled up with drawing. It returns aFilePropagationReport: oneFilePropagationResultper requested repository, carrying either success or the reason the copy failed.Summarize()produces a summary line that answers "did that work" on its own —Propagated .editorconfig to 14 of 15 repos; failed: ktsu-dev/foo— followed by one indented line per failure with its reason. That is the same shapeQueueGitLoggives a git command and its output, and it means a partial run does not have to be reconstructed from fifteen individual lines.PopupPropagateFile.Opennow takes the log callback so the popup can reachQueueLog.Tests
New
ProjectDirector.Test/FilePropagationTests.cs, driven against real throwaway directories under the temp directory the wayGitCliTestsandPullDecisionTestsare. A destination occupied by an existing directory is the portable way to force a refused copy — both Windows and Linux reject it, with no lock or permission change to undo afterwards. CI has since confirmed that portability: the suite passes on all three of ubuntu, macOS and Windows.ARefusedCopyDoesNotStopTheRepositoriesAfterIt— the regression test: three ordered destinations with the middle one blocked; the first and third must both end up with the file, and all three must be accounted for in the report.TheSummaryCountsTheRunAndNamesTheRepositoriesThatMissedOut— the summary line reports2 of 3and names the repository that failed.ARunWhereEveryCopyWorksSaysSoInOneLine— a clean run is one line with no failure text.AMissingSourceIsReportedOnceAndLeavesEveryRepositoryAlone— no repository is touched, no destination directory is created, and it is one log line rather than one per repository.Verified by reverting: with the original unguarded loop restored in place of the new one, three of the four new tests fail with the exact escaping
UnauthorizedAccessException/DirectoryNotFoundExceptionthe issue describes; the fourth (the all-succeed path) passes either way, as it should. With the fix in place the full suite is green: 35 tests, 33 passed, 2 skipped (the two git-lfs tests self-skip where git-lfs is not installed).Second commit: the SourceLink bump, which is not about #415
6678ea8movesMicrosoft.SourceLink.AzureRepos.GitandMicrosoft.SourceLink.GitHubfrom10.0.102to10.0.401.It is here because the build was failing before it reached a single test.
NU1902is an error in this repository, and those two packages were pinned byVersionOverrideto exactly the version GHSA-23fw-v26w-5fgq names, draggingMicrosoft.Build.Tasks.Git 10.0.102along with them. That reproduces on a pristine clone ofmainwith nothing applied, and it is also why the.NET Workflowruns on the open Dependabot PRs #416 and #417 are red — so it is a base-branch break, not this PR's.It is a self-contained two-line change in its own commit, so it can be split out or dropped in favour of a separate packaging fix without disturbing the propagation change, which remains the substance of this PR. Note that landing it here does not unblock #416 and #417; they each need the same bump.
🤖 Generated with Claude Code
https://claude.ai/code/session_01FgX8kpAMnBEumW6PDBfynE