Skip to content

fix(cross-links): use codex path shape in fallback error URL#3174

Open
theletterf wants to merge 1 commit intomainfrom
fix/cross-link-fallback-url-codex
Open

fix(cross-links): use codex path shape in fallback error URL#3174
theletterf wants to merge 1 commit intomainfrom
fix/cross-link-fallback-url-codex

Conversation

@theletterf
Copy link
Copy Markdown
Member

Summary

  • When the codex link index can't be fetched (e.g. CI without credentials to reach the private elastic/codex-link-index repo), the cross-link resolver falls through to an error message containing a best-effort URL to the repo's links.json.
  • That fallback was hardcoded to the public-S3 layout elastic/{scheme}/main/links.json, so errors against a codex/internal registry pointed at a non-existent path under codex-link-index (e.g. https://github.com/elastic/codex-link-index/blob/main/elastic/platform-observability-team/main/links.json), making the error message misleading.
  • This PR threads the per-repo DocSetRegistry through FetchedCrossLinks and picks the codex layout ({env}/elastic/{scheme}/links.json, e.g. internal/elastic/platform-observability-team/links.json) when the entry is non-public, keeping the existing S3 layout for public entries.

Context

Surfaced while debugging elastic/platform-capacity-team#1328, whose preview build fails because its runner can't clone the private codex-link-index repo. That credentials failure (exit 128) is the primary blocker there and isn't addressed here — the consuming workflow needs a token with access to codex-link-index. But whenever the fetch fails for any reason (rate-limit, rotated token, etc.), the error URL should at least point at a path that exists.

Locally the bug almost never surfaces because CrossLinkFetcher.TryGetCachedLinkReference short-circuits to a cached links.json and the git reader is never invoked.

Changes

  • Add optional FrozenDictionary<string, DocSetRegistry>? RegistryByRepository to FetchedCrossLinks.
  • Populate it in DocSetConfigurationCrossLinkFetcher from each CrossLinkEntry.Registry.
  • Switch CrossLinkResolver.TryResolve fallback onto registry type via a small BuildFallbackLinksJsonUrl helper.
  • Add unit tests covering both the codex/internal and public fallback shapes.

Test plan

  • dotnet test tests/Elastic.Markdown.Tests/Elastic.Markdown.Tests.csproj --filter CrossLinks|Codex — 35/35 pass, including the two new CrossLinkResolverFallbackUrlTests.
  • dotnet test tests/Elastic.Documentation.Configuration.Tests — 353/353 pass.
  • Reviewer: confirm the codex path shape ({env}/elastic/{scheme}/links.json) still matches the current layout of elastic/codex-link-index.

🤖 Generated with Claude Code

When the codex link index can't be fetched (e.g. CI without credentials
to reach the private codex-link-index repo), the cross-link resolver
falls through to an error message containing a best-effort URL to the
repo's links.json. That fallback was hardcoded to the public-S3 layout
`elastic/{scheme}/main/links.json`, so errors against a codex/internal
registry pointed at a non-existent path under codex-link-index.

Thread the per-repo DocSetRegistry through FetchedCrossLinks and pick
the codex layout (`{env}/elastic/{scheme}/links.json`) when the entry
is non-public, keeping the existing S3 layout for public entries.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 24, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8a2d5d6c-d519-49d6-98aa-9a69d215a171

📥 Commits

Reviewing files that changed from the base of the PR and between 5decf52 and 9bd241c.

📒 Files selected for processing (4)
  • src/Elastic.Documentation.Links/CrossLinks/CrossLinkFetcher.cs
  • src/Elastic.Documentation.Links/CrossLinks/CrossLinkResolver.cs
  • src/Elastic.Documentation.Links/CrossLinks/DocSetConfigurationCrossLinkFetcher.cs
  • tests/Elastic.Markdown.Tests/CrossLinks/UriEnvironmentResolverTests.cs

📝 Walkthrough

Walkthrough

This pull request extends FetchedCrossLinks with an optional RegistryByRepository mapping that tracks the DocSetRegistry type for each repository. The CrossLinkResolver.TryResolve method now uses this mapping to construct fallback links.json URLs: if a repository's registry is non-public, it generates an internal Codex-style URL; otherwise, it uses the public fallback pattern. The changes update the cross-link fetcher to populate this registry map during configuration processing and add tests validating both internal and public registry fallback scenarios.

Suggested labels

fix

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: fixing the fallback URL in cross-link error messages to use the correct codex path shape instead of the public S3 layout.
Description check ✅ Passed The description provides comprehensive context about the problem, solution, changes made, and test results—all directly related to the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch fix/cross-link-fallback-url-codex

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants