API docs: infer git root for Edit on GitHub/source links

[PrzemyslawKlys]

If SourceUrlPattern is set but SourceRootPath is not, infer SourceRootPath from the nearest git root (walking up from the built assembly). This fixes Edit-on-GitHub/source URLs for repos where projects live in subfolders (e.g. IntelligenceX/...). If no git root is found, source URLs are omitted with a warning instead of producing broken absolute-path URLs.

[claude]

Code Review for PR #76: API docs: infer git root for Edit on GitHub/source links

Summary

This PR improves the source link generation for API documentation by automatically inferring the git root when SourceUrlPattern is set but SourceRootPath is not provided. This is a solid improvement for projects living in subfolders of a repository.

Code Quality ✅

Strengths:

• Clear, well-commented logic explaining the rationale for inferring the git root
• Proper defensive programming with null checks and exception handling
• Improved warning messages that are more informative than the original
• The TryFindGitRoot method follows the "Try" pattern convention correctly

Suggestions:

1. Consider adding XML documentation to TryFindGitRoot (line 4524):

/// <summary>
/// Walks up from the specified path to find the nearest .git folder or file.
/// </summary>
/// <param name="path">Starting path (file or directory).</param>
/// <returns>The git root directory, or null if not found.</returns>
private static string? TryFindGitRoot(string path)

2. The empty catch block (lines 4541-4544) could log the exception for debugging purposes:

catch (Exception ex)
{
    // best-effort - silently return null on any I/O errors
    Trace.TraceWarning($"TryFindGitRoot failed: {ex.Message}");
}

Potential Issues 🟡

1. Git worktree support: The code checks for both .git as a directory and file (line 4532), which correctly handles git worktrees. ✅

2. Path traversal edge case: The loop termination check (lines 4536-4537) looks solid, but consider the rare case where Directory.Exists(path) might throw on network paths or permission issues - this is handled by the outer try-catch. ✅

3. Case sensitivity: Using StringComparison.OrdinalIgnoreCase for path comparison (line 4536) is appropriate for cross-platform compatibility. ✅

Performance Considerations ✅

• The TryFindGitRoot method has minimal performance impact as it only walks up directories (typically 2-5 iterations)
• Only called once during SourceLinkContext.Create, not in hot paths
• No recursive calls or expensive operations

Security Concerns ✅

• No path traversal vulnerabilities - uses Path.GetDirectoryName safely
• No unvalidated file operations - only checks existence
• The catch-all exception handler prevents any errors from causing crashes

Test Coverage ⚠️

Missing tests: There are no tests added for the new TryFindGitRoot functionality. Consider adding:

1. Happy path test: Verify git root is found when starting from a subfolder
2. No git root test: Verify null is returned when no .git exists
3. Assembly in git root test: Verify behavior when assembly is at root level
4. Git worktree test: Verify .git file (worktree) is handled correctly

Example test structure:

[Fact]
public void SourceLink_InfersGitRoot_WhenSourceRootPathNotSet()
{
    // Create temp directory structure with .git folder
    // Generate docs with SourceUrlPattern but no SourceRootPath
    // Assert source URLs are generated correctly with proper relative paths
}

Best Practices ✅

• Follows existing code patterns in the file
• Maintains backward compatibility
• Proper warning messages for debugging
• Defensive coding with null checks

Architecture Considerations ✅

The change is well-isolated and doesn't affect other parts of the codebase. The logic flow is:

1. Try explicit SourceRootPath first (unchanged behavior)
2. Fall back to git root inference (new behavior)
3. Gracefully disable source URLs if neither works (improved warning)

This is a good progressive enhancement approach.

Recommendations

1. Add unit tests for the new functionality (priority: medium)
2. Consider adding XML documentation to TryFindGitRoot (priority: low)
3. Optional: Log exceptions in the catch block for debugging (priority: low)

Overall Assessment

This is a well-implemented feature that solves a real problem for multi-project repositories. The code is defensive, well-structured, and maintains backward compatibility. The main gap is test coverage.

Verdict: Approve with suggestion to add tests ✅

---

Review generated with Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 639944b9d0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Normalize path before searching for git root

TryFindGitRoot climbs parents from the raw assemblyPath string, so when the assembly is passed as a relative path like bin/Release/net10.0/MyLib.dll (a valid CLI input), Path.GetDirectoryName can terminate at an empty segment before ever checking the working directory’s .git. In that case Create treats git root discovery as failed and clears SourceUrlPattern, so source URLs are omitted even though the repo root is available.

Useful? React with 👍 / 👎.

[Copilot]

Pull request overview

Updates API docs source-link generation so that when SourceUrlPattern is set but SourceRootPath is not, the generator attempts to infer a suitable SourceRootPath by walking up to the nearest git root from the assembly location. If a git root cannot be found, it omits source URLs and emits a warning instead of generating broken URLs.

Changes:

• Infer SourceRootPath via a new TryFindGitRoot helper when only SourceUrlPattern is provided.
• Suppress URL generation (set pattern to null) with a warning when neither SourceRootPath nor a git root can be determined.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

When no SourceRootPath is provided and git root detection fails, this code sets pattern = null but still creates a SourceLinkContext. BuildSourceLink will then return ApiSourceLink objects whose Path is the absolute local file path from the PDB, and RenderSourceLink/BuildSourceJson will expose that path in the output even though the URL is omitted. Consider suppressing the entire source link (return null from BuildSourceLink or SourceLinkContext.Create) or at least sanitizing Path (e.g., make it relative/filename-only) when _sourceRoot is null to avoid leaking build-machine paths.

Suggested change

	if (string.IsNullOrWhiteSpace(root) && !string.IsNullOrWhiteSpace(pattern))
	{
	warnings.Add("SourceUrlPattern set without SourceRootPath (and git root not found); source URLs will be omitted.");
	pattern = null;
	if (string.IsNullOrWhiteSpace(root))
	{
	if (!string.IsNullOrWhiteSpace(pattern))
	{
	warnings.Add("SourceUrlPattern set without SourceRootPath (and git root not found); source URLs will be omitted.");
	}
	// Without a resolved source root, exposing paths from the PDB may leak
	// absolute build-machine paths. Suppress source links in this case.
	provider.Dispose();
	stream.Dispose();
	return null;

[Copilot]

TryFindGitRoot may fail unexpectedly when options.AssemblyPath is relative or when the current working directory differs from the assembly location, because it walks up from the input path without normalizing it. Consider calling Path.GetFullPath on assemblyPath (or on current) before searching so git root inference is deterministic.

[Copilot]

In TryFindGitRoot, var current = Directory.Exists(path) ? path : Path.GetDirectoryName(path); can leave current as a relative path and can also be null if Path.GetDirectoryName returns null. It would be clearer/safer to early-return when current is null and to normalize current to a full path before entering the loop, so the .git probing is consistent.