diff --git a/src/Elastic.Documentation.Configuration/ReleaseNotes/ChangelogEntry.cs b/src/Elastic.Documentation.Configuration/ReleaseNotes/ChangelogEntry.cs index f1f0571b66..f0358a50aa 100644 --- a/src/Elastic.Documentation.Configuration/ReleaseNotes/ChangelogEntry.cs +++ b/src/Elastic.Documentation.Configuration/ReleaseNotes/ChangelogEntry.cs @@ -34,6 +34,14 @@ public record ChangelogEntryDto /// Written by the pipeline for non-primary PRs in a multi-PR entry; never hand-authored. /// public string? Link { get; set; } + + /// + /// When true, this public-bucket object is a scrubber-written source pointer that traces back + /// to a canonical public key. Distinguishes source pointers from ordinary link-only PR markers + /// so the delete path does not spuriously follow a regular marker to its canonical target. + /// + [YamlMember(Alias = "source-redirect", ApplyNamingConventions = false)] + public bool? SourceRedirect { get; set; } } /// diff --git a/src/Elastic.Documentation.Configuration/ReleaseNotes/ReleaseNotesSerialization.cs b/src/Elastic.Documentation.Configuration/ReleaseNotes/ReleaseNotesSerialization.cs index 236a9cafbd..0aae497c7f 100644 --- a/src/Elastic.Documentation.Configuration/ReleaseNotes/ReleaseNotesSerialization.cs +++ b/src/Elastic.Documentation.Configuration/ReleaseNotes/ReleaseNotesSerialization.cs @@ -166,7 +166,8 @@ private static string ToYamlDoubleQuotedString(string s) Action = dto.Action, FeatureId = dto.FeatureId, Highlight = dto.Highlight, - Link = dto.Link + Link = dto.Link, + SourceRedirect = dto.SourceRedirect ?? false }; private static ChangelogEntry ToEntry(BundledEntry entry) => new() @@ -287,8 +288,10 @@ private static ChangelogEntryType ParseEntryType(string? value) private static ChangelogEntryDto ToDto(ChangelogEntry entry) { // Marker entries are link-only; emitting any other field would violate the marker contract. + // Source pointers also carry source-redirect: true so the delete path can distinguish them + // from ordinary PR markers without ambiguity. if (entry.IsMarker) - return new ChangelogEntryDto { Link = entry.Link }; + return new ChangelogEntryDto { Link = entry.Link, SourceRedirect = entry.SourceRedirect ? true : null }; return new ChangelogEntryDto { diff --git a/src/Elastic.Documentation/ReleaseNotes/ChangelogEntry.cs b/src/Elastic.Documentation/ReleaseNotes/ChangelogEntry.cs index 25062776c6..ec1a5a549f 100644 --- a/src/Elastic.Documentation/ReleaseNotes/ChangelogEntry.cs +++ b/src/Elastic.Documentation/ReleaseNotes/ChangelogEntry.cs @@ -56,6 +56,12 @@ public record ChangelogEntry /// True when this entry is a pipeline-written marker that redirects to another PR's entry. public bool IsMarker => Link is not null; + /// + /// When true, this entry is a scrubber-written source pointer in the public bucket that traces + /// to a canonical key. Distinguishes source pointers from ordinary link-only PR markers. + /// + public bool SourceRedirect { get; init; } + /// /// Converts this ChangelogEntry to a BundledEntry for embedding in bundles. /// File property is set to null; set it separately using a 'with' expression. diff --git a/src/services/Elastic.Changelog/Scrubbing/ChangelogContentScrubber.cs b/src/services/Elastic.Changelog/Scrubbing/ChangelogContentScrubber.cs index 73152bbac5..ad1ccc22a6 100644 --- a/src/services/Elastic.Changelog/Scrubbing/ChangelogContentScrubber.cs +++ b/src/services/Elastic.Changelog/Scrubbing/ChangelogContentScrubber.cs @@ -31,6 +31,14 @@ public record ScrubResult /// Empty for single-PR entries and bundles. /// public IReadOnlyList<(string Key, string Content)> Markers { get; init; } = []; + + /// + /// True when the content is a pass-through private marker (link: only). + /// The processor must not overwrite existing canonical public content at the same key + /// with a marker — private markers derived from raw (pre-allowlist) PR lists can be + /// stale and their canonical target may already be written to a different public key. + /// + public bool IsMarker { get; init; } } /// Rewrites private-bucket changelog YAML into its public, allowlist-scrubbed form. @@ -97,7 +105,11 @@ private async Task ScrubChangelog(string key, string content, Cance var normalized = ReleaseNotesSerialization.NormalizeYaml(content); var entry = ReleaseNotesSerialization.DeserializeEntry(normalized); - // Marker: link: with no other content. Return unchanged — there is no URL to scrub. + // Marker: link: with no other content. + // Re-serialize rather than passing raw content through so private-authored fields + // (e.g. source-redirect: true) are stripped. source-redirect is processor-owned and + // must never be settable by private input — if it were passed through, a forged marker + // could impersonate a scrubber-written source pointer and trigger canonical deletion. if (entry.Link != null) { var hasContent = !string.IsNullOrEmpty(entry.Title) @@ -107,7 +119,8 @@ private async Task ScrubChangelog(string key, string content, Cance if (hasContent) throw new InvalidOperationException( "Changelog entry has both 'link:' and content fields. A marker must contain only 'link: '."); - return new ScrubResult { Content = content }; + var safeContent = ReleaseNotesSerialization.SerializeEntry(new ChangelogEntry { Link = entry.Link }); + return new ScrubResult { Content = safeContent, IsMarker = true }; } var bundledEntry = new BundledEntry diff --git a/src/services/Elastic.Changelog/Scrubbing/ScrubberProcessor.cs b/src/services/Elastic.Changelog/Scrubbing/ScrubberProcessor.cs index 4568a91882..1c4c5bcfcb 100644 --- a/src/services/Elastic.Changelog/Scrubbing/ScrubberProcessor.cs +++ b/src/services/Elastic.Changelog/Scrubbing/ScrubberProcessor.cs @@ -2,6 +2,7 @@ // 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.Globalization; using System.Net; using Amazon.S3; using Amazon.S3.Model; @@ -321,25 +322,81 @@ private async Task ReconcileObjectAsync(string sourceBucket, string key, bool pa // marker objects it previously produced and delete any that are no longer needed. var oldPublicContent = await TryGetPublicObject(publicKey, ctx); - await PutPublicObject(publicKey, scrubResult.Content, "application/yaml", ctx); - if (scrubResult.CanonicalKey is not null) - _logger.LogInformation("Scrubbed {Key} → canonical public key {CanonicalKey}", key, scrubResult.CanonicalKey); + // Issue 1 guard: a private marker derived from raw (pre-allowlist) PRs can race + // with the scrubber writing canonical public content at the same key. If canonical + // content already occupies the public key, skip the marker write so it cannot + // overwrite the canonical entry that the primary object's scrub already produced. + // A null return from TryDeserializeEntry (unparseable content) is treated as + // canonical — the safest assumption when we cannot classify the existing object. + var skipWrite = scrubResult.IsMarker + && oldPublicContent is not null + && TryDeserializeEntry(oldPublicContent)?.IsMarker != true; + + if (skipWrite) + { + _logger.LogInformation( + "Skipped pass-through marker {Key}: canonical content in public bucket takes precedence", key); + } else - _logger.LogInformation("Scrubbed and wrote {Key} to public bucket", key); - - foreach (var (markerKey, markerContent) in scrubResult.Markers) { - await PutPublicObject(markerKey, markerContent, "application/yaml", ctx); - _logger.LogInformation("Wrote marker {MarkerKey} → {PrimaryKey}", markerKey, publicKey); + await PutPublicObject(publicKey, scrubResult.Content, "application/yaml", ctx); + if (scrubResult.CanonicalKey is not null) + { + _logger.LogInformation("Scrubbed {Key} → canonical public key {CanonicalKey}", key, scrubResult.CanonicalKey); + // Issue 2: write a source pointer at the source key in the public bucket so + // that a delete event for 'key' can trace back to the canonical key and clean + // it up (see delete path below). The pointer uses the same link: format as + // secondary-PR markers; the non-canonical filename makes it distinguishable. + await WriteSourcePointerAsync(key, scrubResult.CanonicalKey, ctx); + } + else + _logger.LogInformation("Scrubbed and wrote {Key} to public bucket", key); + + foreach (var (markerKey, markerContent) in scrubResult.Markers) + { + await PutPublicObject(markerKey, markerContent, "application/yaml", ctx); + _logger.LogInformation("Wrote marker {MarkerKey} → {PrimaryKey}", markerKey, publicKey); + } + + // Remove marker objects that existed in the previous scrub result but are no + // longer produced (e.g. an entry shrank from 3 PRs to 1). + await DeleteStaleMarkersAsync(publicKey, oldPublicContent, scrubResult.Markers, ctx); } - - // Remove marker objects that existed in the previous scrub result but are no - // longer produced (e.g. an entry shrank from 3 PRs to 1). - await DeleteStaleMarkersAsync(publicKey, oldPublicContent, scrubResult.Markers, ctx); } } else { + // Private object is gone. Read the public bucket to discover what was previously + // written for this source key so we can clean it up completely. + // + // Two sub-cases: + // (a) Source key carries a scrubber-written source pointer (SourceRedirect == true): + // the scrubber routed canonical content to a different public key and left a + // breadcrumb here. Trace the pointer to the canonical key, delete it and all its + // markers, then delete the pointer itself. SourceRedirect is processor-owned — + // the scrubber strips it from private-authored markers on the way through. + // (b) Any other key (canonical entry, secondary-PR marker, note-* file): clean up + // any markers the canonical may have emitted and delete the key itself. + var publicContent = await TryGetPublicObject(key, ctx); + var publicEntry = publicContent is not null ? TryDeserializeEntry(publicContent) : null; + + if (publicEntry?.SourceRedirect == true) + { + var lastSlash = key.LastIndexOf('/'); + var keyPrefix = lastSlash >= 0 ? key[..(lastSlash + 1)] : string.Empty; + var canonicalKey = $"{keyPrefix}{publicEntry.Link}.yaml"; + var canonicalContent = await TryGetPublicObject(canonicalKey, ctx); + await DeleteStaleMarkersAsync(canonicalKey, canonicalContent, [], ctx); + await DeletePublicObject(canonicalKey, ctx); + _logger.LogInformation( + "Source pointer {Key} traced to canonical {CanonicalKey}; deleted canonical and its markers", + key, canonicalKey); + } + else if (publicContent is not null) + { + await DeleteStaleMarkersAsync(key, publicContent, [], ctx); + } + await DeletePublicObject(key, ctx); _logger.LogInformation("Private {Key} is gone; removed its public copy", key); } @@ -404,6 +461,28 @@ private async Task ReconcileObjectAsync(string sourceBucket, string key, bool pa } } + private static ChangelogEntry? TryDeserializeEntry(string? content) + { + if (content is null) + return null; + try + { return ReleaseNotesSerialization.DeserializeEntry(content); } + catch { return null; } + } + + private async Task WriteSourcePointerAsync(string sourceKey, string canonicalKey, Cancel ctx) + { + var lastSlash = canonicalKey.LastIndexOf('/'); + var canonicalFileName = lastSlash >= 0 ? canonicalKey[(lastSlash + 1)..] : canonicalKey; + var stem = Path.GetFileNameWithoutExtension(canonicalFileName); + if (!int.TryParse(stem, NumberStyles.None, CultureInfo.InvariantCulture, out _)) + return; // Not a PR-based canonical key; nothing to point to. + var pointerContent = ReleaseNotesSerialization.SerializeEntry(new ChangelogEntry { Link = stem, SourceRedirect = true }); + await PutPublicObject(sourceKey, pointerContent, "application/yaml", ctx); + _logger.LogInformation("Wrote source pointer {SourceKey} → canonical {CanonicalKey}", sourceKey, canonicalKey); + } + + private async Task TryGetPublicObject(string key, Cancel ctx) { try diff --git a/tests/Elastic.Changelog.Tests/Changelogs/MarkerScrubTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/MarkerScrubTests.cs index 966232ad06..cbf7f16064 100644 --- a/tests/Elastic.Changelog.Tests/Changelogs/MarkerScrubTests.cs +++ b/tests/Elastic.Changelog.Tests/Changelogs/MarkerScrubTests.cs @@ -4,13 +4,16 @@ using AwesomeAssertions; using Elastic.Changelog.Scrubbing; +using Elastic.Documentation.Configuration.ReleaseNotes; using Microsoft.Extensions.Logging.Abstractions; namespace Elastic.Changelog.Tests.Changelogs; /// /// Verifies that handles link: markers correctly: -/// a bare marker passes through unchanged; a marker with content fields throws. +/// a bare marker is re-serialized with the link value preserved; a marker with content fields throws. +/// Re-serialization (rather than pass-through) strips private-authored fields such as +/// source-redirect: true so forged markers cannot impersonate scrubber-written source pointers. /// public class MarkerScrubTests { @@ -20,25 +23,30 @@ public class MarkerScrubTests private Cancel Ctx => TestContext.Current.CancellationToken; [Fact] - public async Task Marker_OnlyLink_PassesThroughUnchanged() + public async Task Marker_OnlyLink_PreservesLinkValue() { const string key = "changelog/elastic/elasticsearch/main/200.yaml"; const string content = "link: 100\n"; var result = await _scrubber.ScrubAsync(key, content, Ctx); - result.Content.Should().Be(content); + var entry = ReleaseNotesSerialization.DeserializeEntry(result.Content); + entry.Link.Should().Be("100", "link: value must survive the scrub"); + entry.SourceRedirect.Should().BeFalse("source-redirect must not appear in scrubbed markers"); + result.IsMarker.Should().BeTrue(); } [Fact] - public async Task Marker_OnlyLink_NoTrailingNewline_PassesThroughUnchanged() + public async Task Marker_OnlyLink_NoTrailingNewline_PreservesLinkValue() { const string key = "changelog/elastic/elasticsearch/main/200.yaml"; const string content = "link: 100"; var result = await _scrubber.ScrubAsync(key, content, Ctx); - result.Content.Should().Be(content); + var entry = ReleaseNotesSerialization.DeserializeEntry(result.Content); + entry.Link.Should().Be("100", "link: value must survive the scrub even without a trailing newline"); + result.IsMarker.Should().BeTrue(); } [Fact] diff --git a/tests/Elastic.Changelog.Tests/Scrubbing/ChangelogContentScrubberTests.cs b/tests/Elastic.Changelog.Tests/Scrubbing/ChangelogContentScrubberTests.cs index ad2d993252..224bc1ab8c 100644 --- a/tests/Elastic.Changelog.Tests/Scrubbing/ChangelogContentScrubberTests.cs +++ b/tests/Elastic.Changelog.Tests/Scrubbing/ChangelogContentScrubberTests.cs @@ -19,7 +19,8 @@ private static ChangelogContentScrubber Scrubber(IReadOnlyList? allowRep [Fact] public async Task ScrubAsync_MarkerEntry_LinkFieldPreserved() { - // A marker is link: only — the scrubber must pass it through unchanged. + // A marker is link: only — the scrubber must pass it through (re-serialized to strip + // any private-authored fields, but link: itself is preserved). var yaml = "link: \"12345\"\n"; var scrubber = Scrubber(); @@ -32,6 +33,23 @@ public async Task ScrubAsync_MarkerEntry_LinkFieldPreserved() result.Markers.Should().BeEmpty(); } + [Fact] + public async Task ScrubAsync_MarkerEntryWithSourceRedirect_StripsSourceRedirectFromOutput() + { + // source-redirect: true is processor-owned metadata. If a private author adds it to a + // marker file, the scrubber must strip it so forged markers cannot impersonate + // scrubber-written source pointers and trigger spurious canonical deletion on delete. + var yaml = "link: \"12345\"\nsource-redirect: true\n"; + var scrubber = Scrubber(); + + var result = await scrubber.ScrubAsync("changelog/elastic/elasticsearch/main/12346.yaml", yaml, CancellationToken.None); + + var entry = ReleaseNotesSerialization.DeserializeEntry(result.Content); + entry.Link.Should().Be("12345", "link: must survive the scrub round-trip"); + entry.SourceRedirect.Should().BeFalse("source-redirect must be stripped from private-authored markers"); + result.IsMarker.Should().BeTrue(); + } + [Fact] public async Task ScrubAsync_NormalEntry_LinkIsNull() { diff --git a/tests/Elastic.Changelog.Tests/Scrubbing/ScrubberProcessorTests.cs b/tests/Elastic.Changelog.Tests/Scrubbing/ScrubberProcessorTests.cs index b70e763430..88485b469b 100644 --- a/tests/Elastic.Changelog.Tests/Scrubbing/ScrubberProcessorTests.cs +++ b/tests/Elastic.Changelog.Tests/Scrubbing/ScrubberProcessorTests.cs @@ -8,6 +8,8 @@ using Elastic.Changelog.Scrubbing; using Elastic.Changelog.Tests.Reconciliation; using Elastic.Changelog.Uploading; +using Elastic.Documentation.Configuration.ReleaseNotes; +using Elastic.Documentation.ReleaseNotes; using FakeItEasy; using Microsoft.Extensions.Logging.Abstractions; @@ -275,7 +277,7 @@ public async Task Process_SourceChangingMidFlight_IsDetectedByPostWriteValidatio } [Fact] - public async Task Process_CanonicalKeyEntry_WritesToCanonicalPublicKey() + public async Task Process_CanonicalKeyEntry_WritesToCanonicalPublicKeyAndSourcePointer() { // Scrubber says the private key 12345-fix.yaml should be written to public as 12345.yaml. const string privateKey = "changelog/elastic/elasticsearch/main/12345-fix.yaml"; @@ -290,8 +292,12 @@ public async Task Process_CanonicalKeyEntry_WritesToCanonicalPublicKey() failed.Should().BeEmpty(); _s3.ContentOf(PublicBucket, canonicalKey).Should().Be("scrubbed: entry-content", "canonical key must receive the scrubbed content"); - _s3.Exists(PublicBucket, privateKey).Should().BeFalse( - "private (non-canonical) key must not appear in the public bucket"); + // A source pointer is written at the source key so the delete path can trace back to the + // canonical key when the private object is eventually removed. + _s3.Exists(PublicBucket, privateKey).Should().BeTrue( + "source pointer must exist so delete events can trace to the canonical key"); + _s3.ContentOf(PublicBucket, privateKey).Should().Contain("link:", + "source pointer must be a link marker pointing to the canonical PR number"); } [Fact] @@ -441,6 +447,150 @@ public async Task Process_NoteFile_ScrubbedAndNotesReconcileTriggered() _s3.Exists(PublicBucket, "changelog/elastic/elasticsearch/notes-9.0.0.json").Should().BeTrue(); } + [Fact] + public async Task Process_PassThroughMarker_DoesNotOverwriteExistingCanonicalContent() + { + // Issue 1: a private marker derived from raw (pre-allowlist) PRs can arrive after the + // canonical public entry has already been written. The marker write must be skipped so + // it cannot overwrite canonical content. + const string markerKey = "changelog/elastic/elasticsearch/main/20.yaml"; + _ = _s3.Seed(PrivateBucket, markerKey, "link: \"10\"\n"); + _ = _s3.Seed(PublicBucket, markerKey, "scrubbed canonical content at 20"); + _ = A.CallTo(() => _scrubber.ScrubAsync(markerKey, A._, A._)) + .ReturnsLazily((string _, string content, Cancel _) => + Task.FromResult(new ScrubResult { Content = content, IsMarker = true })); + + var failed = await _processor.ProcessAsync([Message("ObjectCreated:Put", markerKey)], Ctx); + + failed.Should().BeEmpty(); + _s3.ContentOf(PublicBucket, markerKey).Should().Be( + "scrubbed canonical content at 20", + "pass-through marker must not overwrite existing canonical content"); + } + + [Fact] + public async Task Process_DeleteOfNonCanonicalSource_DeletesCanonicalAndMarkersThroughSourcePointer() + { + // Issue 2: when the private source key is non-canonical (e.g. 12345-fix.yaml), the + // scrubber writes canonical content to a different public key (12345.yaml) and leaves a + // source pointer at the source key. On delete, the processor must trace that pointer and + // remove the canonical and all its markers. + const string privateKey = "changelog/elastic/elasticsearch/main/12345-fix.yaml"; + const string canonicalKey = "changelog/elastic/elasticsearch/main/12345.yaml"; + const string markerKey = "changelog/elastic/elasticsearch/main/67890.yaml"; + // Simulate state left by the prior write: source pointer at privateKey, canonical at + // canonicalKey, and a secondary-PR marker at markerKey. + // The source pointer must carry source-redirect: true to be distinguishable from a plain marker. + _ = _s3.Seed(PublicBucket, privateKey, + ReleaseNotesSerialization.SerializeEntry(new ChangelogEntry { Link = "12345", SourceRedirect = true })); + _ = _s3.Seed(PublicBucket, canonicalKey, + // language=yaml + """ + type: enhancement + title: "Example" + prs: + - https://github.com/elastic/elasticsearch/pull/12345 + - https://github.com/elastic/elasticsearch/pull/67890 + """); + _ = _s3.Seed(PublicBucket, markerKey, "link: \"12345\"\n"); + + var failed = await _processor.ProcessAsync([Message("ObjectRemoved:Delete", privateKey)], Ctx); + + failed.Should().BeEmpty(); + _s3.Exists(PublicBucket, privateKey).Should().BeFalse("source pointer must be deleted"); + _s3.Exists(PublicBucket, canonicalKey).Should().BeFalse("canonical entry must be deleted"); + _s3.Exists(PublicBucket, markerKey).Should().BeFalse("secondary-PR marker must be deleted via DeleteStaleMarkersAsync"); + } + + [Fact] + public async Task Process_DeleteOfCanonicalEntryWithMarkers_DeletesMarkersBeforeCanonical() + { + // When the canonical entry's source key is itself the canonical key (numeric filename), + // the delete path must also clean up its secondary-PR markers in the public bucket. + const string canonicalKey = "changelog/elastic/elasticsearch/main/100.yaml"; + const string marker200 = "changelog/elastic/elasticsearch/main/200.yaml"; + const string marker300 = "changelog/elastic/elasticsearch/main/300.yaml"; + _ = _s3.Seed(PublicBucket, canonicalKey, + // language=yaml + """ + type: enhancement + title: "Multi-PR entry" + prs: + - https://github.com/elastic/elasticsearch/pull/100 + - https://github.com/elastic/elasticsearch/pull/200 + - https://github.com/elastic/elasticsearch/pull/300 + """); + _ = _s3.Seed(PublicBucket, marker200, "link: \"100\"\n"); + _ = _s3.Seed(PublicBucket, marker300, "link: \"100\"\n"); + + var failed = await _processor.ProcessAsync([Message("ObjectRemoved:Delete", canonicalKey)], Ctx); + + failed.Should().BeEmpty(); + _s3.Exists(PublicBucket, canonicalKey).Should().BeFalse("canonical entry must be deleted"); + _s3.Exists(PublicBucket, marker200).Should().BeFalse("PR-200 marker must be deleted"); + _s3.Exists(PublicBucket, marker300).Should().BeFalse("PR-300 marker must be deleted"); + } + + [Fact] + public async Task Process_DeleteOfNonCanonicalSourceWithPlainMarker_DoesNotDeleteCanonical() + { + // Regression guard for source-pointer ambiguity: a plain link: marker at a non-numeric key + // must NOT be treated as a source pointer. Only objects with source-redirect: true trigger + // canonical deletion; otherwise any migrated marker could accidentally nuke live content. + const string privateKey = "changelog/elastic/elasticsearch/main/12345-fix.yaml"; + const string canonicalKey = "changelog/elastic/elasticsearch/main/12345.yaml"; + // Public object has link: but no source-redirect: true — it's a plain marker, not a pointer. + _ = _s3.Seed(PublicBucket, privateKey, "link: \"12345\"\n"); + _ = _s3.Seed(PublicBucket, canonicalKey, "type: enhancement\ntitle: Real\n"); + + var failed = await _processor.ProcessAsync([Message("ObjectRemoved:Delete", privateKey)], Ctx); + + failed.Should().BeEmpty(); + _s3.Exists(PublicBucket, privateKey).Should().BeFalse("the source-key public object is deleted"); + _s3.Exists(PublicBucket, canonicalKey).Should().BeTrue( + "a plain link: marker must not trigger canonical deletion — only source-redirect: true does"); + } + + [Fact] + public async Task Process_DeleteOfYmlSourceKey_TracesSourcePointerToCanonical() + { + // A .yml source key can have a source pointer even though the stem looks numeric. + // The delete path must check SourceRedirect first, not the filename shape. + const string privateKey = "changelog/elastic/elasticsearch/main/12345.yml"; + const string canonicalKey = "changelog/elastic/elasticsearch/main/12345.yaml"; + _ = _s3.Seed(PublicBucket, privateKey, + ReleaseNotesSerialization.SerializeEntry(new ChangelogEntry { Link = "12345", SourceRedirect = true })); + _ = _s3.Seed(PublicBucket, canonicalKey, "type: enhancement\ntitle: Real\n"); + + var failed = await _processor.ProcessAsync([Message("ObjectRemoved:Delete", privateKey)], Ctx); + + failed.Should().BeEmpty(); + _s3.Exists(PublicBucket, privateKey).Should().BeFalse("source pointer is cleaned up"); + _s3.Exists(PublicBucket, canonicalKey).Should().BeFalse( + "canonical must be deleted via pointer tracing — .yml stem being numeric must not block this"); + } + + [Fact] + public async Task Process_DeleteOfNumericYamlSourceKeyWithSourcePointer_TracesPointerToCanonical() + { + // Comment 4 regression: a numeric .yaml source key (e.g. 12345.yaml) can itself have a + // source pointer when the entry's min PR is smaller than 12345 (e.g. PRs [100, 12345] → + // canonical 100.yaml). The delete path must check SourceRedirect first — before any + // filename-shape heuristic — so canonical 100.yaml is not orphaned. + const string privateKey = "changelog/elastic/elasticsearch/main/12345.yaml"; + const string canonicalKey = "changelog/elastic/elasticsearch/main/100.yaml"; + _ = _s3.Seed(PublicBucket, privateKey, + ReleaseNotesSerialization.SerializeEntry(new ChangelogEntry { Link = "100", SourceRedirect = true })); + _ = _s3.Seed(PublicBucket, canonicalKey, "type: enhancement\ntitle: Real\n"); + + var failed = await _processor.ProcessAsync([Message("ObjectRemoved:Delete", privateKey)], Ctx); + + failed.Should().BeEmpty(); + _s3.Exists(PublicBucket, privateKey).Should().BeFalse("source pointer is cleaned up"); + _s3.Exists(PublicBucket, canonicalKey).Should().BeFalse( + "canonical must be deleted via pointer tracing even when the source key is numeric"); + } + // language=yaml private static string BundleYaml() => """ products: