elastic / elastic/docs-builder

diff validate: redirect check silently skips all files when docset.yml is at repo root

Open
#3,808 0 comments 0 reactions 0 assignees View on GitHub
needs triage
Dominant language
C#
Stars
24
Forks
44
Avg merge
1d 7h
Merged PRs (30d)
146

Description

## Summary

The `diff validate` redirect health check (added in #1340) silently passes on repositories where `docset.yml` lives at the git root (e.g. `elastic/docs-content`). Deleted and renamed files are never checked against `redirects.yml`, so the CI step always exits 0 regardless of whether redirects are missing.

## Root cause

`GetDocsetPathFromKnownLocations` correctly finds `docset.yml` at the repo root, so `DocumentationSourceDirectory` is set to the git root itself. This means:

```csharp
var relativePath = Path.GetRelativePath(root.FullName, buildContext.DocumentationSourceDirectory.FullName);
// root == docSourceDir → relativePath = "."
```

`IntegrationGitRepositoryTracker` then builds its lookup prefix as:

```csharp
LookupPath = $"{"." .Trim(['/', '\\'])}/"; // = "./"
```

But `DELETED_FILES` from the workflow contains paths like `solutions/search/ai-search/ai-search.md` — no leading `./` — so **every file is filtered out** and the loop finds nothing to validate.

## Impact

Confirmed against `elastic/docs-content` PR [elastic/docs-content#5567](https://github.com/elastic/docs-content/pull/5567), which deleted `solutions/search/ai-search/ai-search.md` with no redirect entry in `redirects.yml`. The `Validate redirect rules` CI step ran and reported success. The missing redirect was only caught later and fixed manually in a follow-up PR ([elastic/docs-content#7859](https://github.com/elastic/docs-content/pull/7859)), after the broken URL had already been live.

## Proposed fix

In `IntegrationGitRepositoryTracker`, treat `.` or empty `lookupPath` as "no prefix filter" (match all files):

```csharp
// Before
private string LookupPath { get; } = $"{lookupPath.Trim(['/', '\\'])}/";

// After
private string LookupPath { get; } = lookupPath.Trim(['/', '\\']) is "." or ""
? string.Empty
: $"{lookupPath.Trim(['/', '\\'])}/";
```

And update the filter predicate:

```csharp
.Where(f => string.IsNullOrEmpty(LookupPath) || f.StartsWith(LookupPath, OrdinalIgnoreCase))
```

The same fix should be applied symmetrically in `LocalGitRepositoryTracker` if it uses the same prefix logic.

## Affected repos

Any docs repo with `docset.yml` at the git root rather than in a `docs/` subdirectory. `elastic/docs-content` is the primary example.

Contributor guide

Open the contributing guide

Research direction

Start with IntegrationGitRepositoryTracker and inspect how LookupPath is built and applied when DocumentationSourceDirectory is the git root. Check LocalGitRepositoryTracker for the same prefix logic, then run the diff validate workflow against root-level and nested docset paths; done means deleted or renamed files are checked for redirect entries in both layouts.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.