Skip to content

Finish a sibling comparison when a shared entry cannot be read [patch] - #472

Merged
matt-edmondson merged 4 commits into
mainfrom
fix/436-unreadable-sibling-entries
Oct 6, 2026
Merged

matt-edmondson merged 4 commits into
mainfrom
fix/436-unreadable-sibling-entries

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #436

What was wrong

DiffRepos read every tracked path that both siblings share through ReadFileOrEmpty, which caught only FileNotFoundException and DirectoryNotFoundException. A submodule's gitlink is a directory on disk, so File.ReadAllText threw UnauthorizedAccessException out of the background task CompareSiblingsAsync starts. Nothing observes that task, so SimilarReposPending stayed true, the panels showed "Comparing repositories..." indefinitely, and nothing was logged. A locked or permission-denied file took the same path.

Change

  • TryReadTrackedFile replaces the read. It never throws:
    • a directory (gitlink, or a tracked link to a directory) reports no content and no reason
    • a missing file still reads as empty, as before
    • an IOException or UnauthorizedAccessException reports no content, with the exception's message as the reason
  • DiffRepos leaves out any shared path that either side can't supply content for, and logs the reason when there is one: Left <file> out of the comparison with <remote>: <reason>. Gitlinks are skipped silently, since a shared submodule is normal. DiffAgainstAll gained an optional log parameter, which CompareSiblingsAsync passes through.
  • CompareSiblingsAsync now catches anything that still escapes (IOException, UnauthorizedAccessException, InvalidOperationException, ArgumentException). It publishes an empty answer so the pending state always ends, and logs Comparing <remote> failed: <message>.
  • DiffSingleFile, which runs on the render thread, goes through the same non-throwing read via ReadFileOrEmpty.

Tests

Two new tests in SimilarReposTests, both covering the acceptance criteria:

  • SiblingsSharingASubmodulePathCompareWithTheGitlinkLeftOut adds a 160000 gitlink at external to both repos with git update-index --cacheinfo. It checks that the comparison finishes, that shared.txt is diffed, and that external is not.
  • ASharedFileThatCannotBeReadIsLeftOutAndLogged holds the sibling's locked.txt open with FileShare.None, which is refused even under root. It checks that the comparison finishes, the readable file is still diffed, the locked one is left out, and the log names it.

Results:

  • With the ProjectDirector.cs change reverted, both tests fail with the issue's exceptions: UnauthorizedAccessException: Access to the path '.../external' is denied and IOException: ... being used by another process.
  • With the change, 105 of 107 pass and 1 is skipped. The one failure is CloningAnLfsRepositoryRestoresTheFileContentRatherThanThePointer, which fails the same way on unchanged main in this sandbox.
  • The build has no warnings. dotnet format reports only the two issues that already exist on main (import ordering, IDE0001 at line 1218).

🤖 Generated with Claude Code

https://claude.ai/code/session_01CaZMWc5xm4vDRcyMefDXyV


Generated by Claude Code

DiffRepos read every tracked path both siblings share. A submodule's
gitlink is a directory on disk, so File.ReadAllText threw
UnauthorizedAccessException out of the background comparison task. Nothing
observes that task, so SimilarReposPending stayed true and the panels
showed "Comparing repositories..." forever, with nothing logged.

Shared paths that are directories are now left out of the comparison, and
a file that cannot be read (locked, permission denied) is left out with
the reason logged. Any remaining failure still publishes an empty answer
and logs why, so the pending state always ends. The single-file re-diff
on the render thread uses the same non-throwing read.

Fixes #436

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01CaZMWc5xm4vDRcyMefDXyV
Comment thread ProjectDirector/ProjectDirector.cs Fixed
claude added 3 commits October 6, 2026 10:35
Path.Combine drops repoPath if relativePath is rooted; Path.Join never does.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01CaZMWc5xm4vDRcyMefDXyV
…ests

The quality gate measured 77.8% coverage on new code; these two paths
were the uncovered ones.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01CaZMWc5xm4vDRcyMefDXyV
… new tests

Addresses MSTEST0037, MSTEST0046 and MSTEST0061 reported on this PR.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01CaZMWc5xm4vDRcyMefDXyV
@sonarqubecloud

sonarqubecloud Bot commented Oct 6, 2026

Copy link
Copy Markdown

@matt-edmondson
matt-edmondson merged commit b708cf6 into main Oct 6, 2026
14 checks passed
@matt-edmondson
matt-edmondson deleted the fix/436-unreadable-sibling-entries branch October 6, 2026 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Similar-repo comparison stays on "Comparing repositories..." forever when siblings share a submodule path (or any unreadable tracked entry)

2 participants