Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Elastic.Documentation;
using Elastic.Documentation.AppliesTo;
using Elastic.Documentation.Configuration.Inference;
using Elastic.Documentation.Extensions;
using Elastic.Documentation.Navigation;
using Elastic.Documentation.Search;
using Elastic.Ingest.Elasticsearch.Indices;
Expand Down Expand Up @@ -157,6 +158,19 @@
}).ToArray()
: null;

var gitHubRepo = fileContext.BuildContext.Git.GitHubRepository;
var branch = fileContext.BuildContext.Git.Branch;
if (gitHubRepo is not null
&& fileContext.BuildContext.Git != GitCheckoutInformation.Unavailable
&& fileContext.BuildContext.DocumentationCheckoutDirectory is { } checkoutDirectory)
{
var relativeSourcePath = Path.GetRelativePath(
checkoutDirectory.FullName,
fileContext.BuildContext.DocumentationSourceDirectory.FullName);
var path = UrlPath.Join(relativeSourcePath, file.RelativePath);
doc.SourceUrl = $"https://github.com/{gitHubRepo}/blob/{branch}/{path}";

Check failure on line 171 in src/Elastic.Markdown/Exporters/Elasticsearch/ElasticsearchMarkdownExporter.Export.cs

View workflow job for this annotation

GitHub Actions / build (macos-latest)

'DocumentationDocument' does not contain a definition for 'SourceUrl' and no accessible extension method 'SourceUrl' accepting a first argument of type 'DocumentationDocument' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 171 in src/Elastic.Markdown/Exporters/Elasticsearch/ElasticsearchMarkdownExporter.Export.cs

View workflow job for this annotation

GitHub Actions / build (macos-latest)

'DocumentationDocument' does not contain a definition for 'SourceUrl' and no accessible extension method 'SourceUrl' accepting a first argument of type 'DocumentationDocument' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 171 in src/Elastic.Markdown/Exporters/Elasticsearch/ElasticsearchMarkdownExporter.Export.cs

View workflow job for this annotation

GitHub Actions / build (windows-latest)

'DocumentationDocument' does not contain a definition for 'SourceUrl' and no accessible extension method 'SourceUrl' accepting a first argument of type 'DocumentationDocument' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 171 in src/Elastic.Markdown/Exporters/Elasticsearch/ElasticsearchMarkdownExporter.Export.cs

View workflow job for this annotation

GitHub Actions / build (windows-latest)

'DocumentationDocument' does not contain a definition for 'SourceUrl' and no accessible extension method 'SourceUrl' accepting a first argument of type 'DocumentationDocument' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 171 in src/Elastic.Markdown/Exporters/Elasticsearch/ElasticsearchMarkdownExporter.Export.cs

View workflow job for this annotation

GitHub Actions / build (ubuntu-latest)

'DocumentationDocument' does not contain a definition for 'SourceUrl' and no accessible extension method 'SourceUrl' accepting a first argument of type 'DocumentationDocument' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 171 in src/Elastic.Markdown/Exporters/Elasticsearch/ElasticsearchMarkdownExporter.Export.cs

View workflow job for this annotation

GitHub Actions / build (ubuntu-latest)

'DocumentationDocument' does not contain a definition for 'SourceUrl' and no accessible extension method 'SourceUrl' accepting a first argument of type 'DocumentationDocument' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 171 in src/Elastic.Markdown/Exporters/Elasticsearch/ElasticsearchMarkdownExporter.Export.cs

View workflow job for this annotation

GitHub Actions / validate-assembler

'DocumentationDocument' does not contain a definition for 'SourceUrl' and no accessible extension method 'SourceUrl' accepting a first argument of type 'DocumentationDocument' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 171 in src/Elastic.Markdown/Exporters/Elasticsearch/ElasticsearchMarkdownExporter.Export.cs

View workflow job for this annotation

GitHub Actions / synthetics

'DocumentationDocument' does not contain a definition for 'SourceUrl' and no accessible extension method 'SourceUrl' accepting a first argument of type 'DocumentationDocument' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 171 in src/Elastic.Markdown/Exporters/Elasticsearch/ElasticsearchMarkdownExporter.Export.cs

View workflow job for this annotation

GitHub Actions / build (elastic/oblt-actions)

'DocumentationDocument' does not contain a definition for 'SourceUrl' and no accessible extension method 'SourceUrl' accepting a first argument of type 'DocumentationDocument' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 171 in src/Elastic.Markdown/Exporters/Elasticsearch/ElasticsearchMarkdownExporter.Export.cs

View workflow job for this annotation

GitHub Actions / build (elastic/opentelemetry)

'DocumentationDocument' does not contain a definition for 'SourceUrl' and no accessible extension method 'SourceUrl' accepting a first argument of type 'DocumentationDocument' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 171 in src/Elastic.Markdown/Exporters/Elasticsearch/ElasticsearchMarkdownExporter.Export.cs

View workflow job for this annotation

GitHub Actions / build (elastic/elasticsearch)

'DocumentationDocument' does not contain a definition for 'SourceUrl' and no accessible extension method 'SourceUrl' accepting a first argument of type 'DocumentationDocument' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 171 in src/Elastic.Markdown/Exporters/Elasticsearch/ElasticsearchMarkdownExporter.Export.cs

View workflow job for this annotation

GitHub Actions / build (elastic/docs-content)

'DocumentationDocument' does not contain a definition for 'SourceUrl' and no accessible extension method 'SourceUrl' accepting a first argument of type 'DocumentationDocument' could be found (are you missing a using directive or an assembly reference?)
}

CommonEnrichments(doc, currentNavigation);
AssignContentHash(doc);
AssignDocumentMetadata(doc);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ public class DocumentGateway(
e => e.AiQuestions,
e => e.AiUseCases,
e => e.LastUpdated,
e => e.SourceUrl,
e => e.Product,
e => e.RelatedProducts
))),
Expand Down Expand Up @@ -79,6 +80,7 @@ public class DocumentGateway(
AiQuestions = doc.AiQuestions,
AiUseCases = doc.AiUseCases,
LastUpdated = doc.LastUpdated,
SourceUrl = doc.SourceUrl,
Product = doc.Product?.Id != null ? new DocumentProduct
{
Id = doc.Product.Id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ public record DocumentResult
public string[]? AiQuestions { get; init; }
public string[]? AiUseCases { get; init; }
public DateTimeOffset? LastUpdated { get; init; }
public string? SourceUrl { get; init; }
public DocumentParent[] Parents { get; init; } = [];
public string[] Headings { get; init; } = [];
public string[] Links { get; init; } = [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ public sealed record DocumentResponse
public string[]? AiQuestions { get; init; }
public string[]? AiUseCases { get; init; }
public DateTimeOffset? LastUpdated { get; init; }
public string? SourceUrl { get; init; }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

SourceUrl currently serializes as sourceUrl, not source_url (contract mismatch).

Line 113 needs an explicit JSON name override if the MCP contract is source_url as stated in the PR objective. With current options, this will emit sourceUrl and can break clients expecting source_url.

Proposed fix
 public sealed record DocumentResponse
 {
@@
+	[JsonPropertyName("source_url")]
 	public string? SourceUrl { get; init; }
@@
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public string? SourceUrl { get; init; }
[JsonPropertyName("source_url")]
public string? SourceUrl { get; init; }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/api/Elastic.Documentation.Mcp.Remote/Responses/McpResponses.cs` at line
113, The SourceUrl property in McpResponses.cs currently serializes to
"sourceUrl" but the MCP contract requires "source_url"; update the SourceUrl
property to explicitly specify the JSON name (e.g., add
[JsonPropertyName("source_url")] above the public string? SourceUrl { get; init;
}) so the serializer emits "source_url" (ensure the System.Text.Json attribute
is imported or use [JsonProperty("source_url")] if the project uses
Newtonsoft.Json).

public required List<ParentDto> Parents { get; init; }
public required List<string> Headings { get; init; }
public ProductDto? Product { get; init; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ public async Task<string> GetDocumentByUrl(
AiQuestions = result.AiQuestions,
AiUseCases = result.AiUseCases,
LastUpdated = result.LastUpdated,
SourceUrl = result.SourceUrl,
Parents = result.Parents.Select(p => new ParentDto
{
Title = p.Title,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,33 @@
Output.WriteLine($"Error message: {errorResponse.Error}");
}

[Fact]
public async Task GetDocumentByUrl_SourceUrlIsGitHubBlobUrlWhenPresent()
{
// Arrange
var (documentTools, clientAccessor) = CreateDocumentTools();
Assert.SkipUnless(documentTools is not null, "Elasticsearch is not configured");
LogDiagnostics(clientAccessor);
var canConnect = await clientAccessor!.CanConnect(TestContext.Current.CancellationToken);
Assert.SkipUnless(canConnect, "Elasticsearch is not connected");

// Act
var resultJson = await documentTools.GetDocumentByUrl(
"/docs/reference/elasticsearch",
cancellationToken: TestContext.Current.CancellationToken);

if (resultJson.Contains("\"error\""))
Assert.Skip("Test document not found in index");

var response = JsonSerializer.Deserialize(resultJson, McpJsonContext.Default.DocumentResponse);
response.Should().NotBeNull();
Output.WriteLine($"source_url: {response!.SourceUrl ?? "(null — document predates indexing of this field)"}");

// source_url is null for docs indexed before this field was added; when present it must be a GitHub blob URL
if (response.SourceUrl is not null)

Check warning on line 98 in tests-integration/Mcp.Remote.IntegrationTests/DocumentToolsIntegrationTests.cs

View workflow job for this annotation

GitHub Actions / lint

Null check can be simplified

Check warning on line 98 in tests-integration/Mcp.Remote.IntegrationTests/DocumentToolsIntegrationTests.cs

View workflow job for this annotation

GitHub Actions / lint

Null check can be simplified
response.SourceUrl.Should().StartWith("https://github.com/").And.Contain("/blob/");
}

[Fact]
public async Task AnalyzeDocumentStructure_ReturnsStructure()
{
Expand Down
63 changes: 63 additions & 0 deletions tests/Mcp.Remote.Tests/DocumentToolsTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Licensed to Elasticsearch B.V under one or more agreements.
// Elasticsearch B.V licenses this file to you under the Apache 2.0 License.
// See the LICENSE file in the project root for more information

using System.Text.Json;
using AwesomeAssertions;
using Elastic.Documentation.Mcp.Remote.Gateways;
using Elastic.Documentation.Mcp.Remote.Responses;
using Elastic.Documentation.Mcp.Remote.Tools;
using Microsoft.Extensions.Logging.Abstractions;

namespace Mcp.Remote.Tests;

public class DocumentToolsTests
{
[Fact]
public async Task GetDocumentByUrl_MapsSourceUrlIntoResponse()
{
const string expectedSourceUrl = "https://github.com/elastic/docs-content/blob/main/docs/some-page.md";
var gateway = new StubDocumentGateway(new DocumentResult
{
Url = "/docs/some-page",
Title = "Some Page",
Type = "doc",
SourceUrl = expectedSourceUrl
});
var tools = new DocumentTools(gateway, NullLogger<DocumentTools>.Instance);

var json = await tools.GetDocumentByUrl("/docs/some-page");

var response = JsonSerializer.Deserialize(json, McpJsonContext.Default.DocumentResponse);
response.Should().NotBeNull();
response!.SourceUrl.Should().Be(expectedSourceUrl);
Comment on lines +31 to +33

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

These tests won’t catch a JSON field-name contract regression.

Because both assertions deserialize into DocumentResponse, they validate value mapping but not the emitted key name. Add a raw JSON assertion for "source_url" (or explicit absence when null) so contract breaks are caught.

Also applies to: 50-52

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/Mcp.Remote.Tests/DocumentToolsTests.cs` around lines 31 - 33, The tests
currently deserialize JSON into DocumentResponse (via JsonSerializer.Deserialize
with McpJsonContext.Default.DocumentResponse) and assert response.SourceUrl,
which won't detect if the emitted JSON property name changed; update the tests
to also assert the raw JSON string contains the expected key "source_url" (or
that "source_url" is absent when expected null) before/after deserialization so
a field-name contract regression is caught—locate the assertions around
response/DocumentResponse (lines with JsonSerializer.Deserialize and
response!.SourceUrl) and add a string-based assertion against the raw json
variable for the "source_url" property (and mirror the same change for the other
test at the noted lines 50-52).

}

[Fact]
public async Task GetDocumentByUrl_OmitsSourceUrlWhenNull()
{
var gateway = new StubDocumentGateway(new DocumentResult
{
Url = "/docs/some-page",
Title = "Some Page",
Type = "doc",
SourceUrl = null
});
var tools = new DocumentTools(gateway, NullLogger<DocumentTools>.Instance);

var json = await tools.GetDocumentByUrl("/docs/some-page");

var response = JsonSerializer.Deserialize(json, McpJsonContext.Default.DocumentResponse);
response.Should().NotBeNull();
response!.SourceUrl.Should().BeNull();
}

private sealed class StubDocumentGateway(DocumentResult? result) : IDocumentGateway
{
public Task<DocumentResult?> GetByUrlAsync(string url, CancellationToken ct = default) =>
Task.FromResult(result);

public Task<DocumentStructure?> GetStructureAsync(string url, CancellationToken ct = default) =>
Task.FromResult<DocumentStructure?>(null);
}
}
Loading