feat(changelog): add link: field to changelog entries — marker read side (Step 7) - #3928
Conversation
There was a problem hiding this comment.
Requesting changes: marker entries can currently serialize with additional fields (notably title: ''), which conflicts with the link-only marker contract introduced in this PR.
What is this? | From workflow: PR Review
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
690c5c0 to
bcec261
Compare
There was a problem hiding this comment.
Requesting changes: please make probe failures distinguishable from authoritative 404 misses so transient CDN/HTTP errors cannot be treated as absent changelog entries.
What is this? | From workflow: PR Review
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
There was a problem hiding this comment.
Requesting changes: CDN probe selection should preserve PR repo/owner identity so cross-repo PR URLs cannot match by number alone.
What is this? | From workflow: PR Review
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
Adds `link:` to `ChangelogEntryDto`, `ChangelogEntry` (domain type),
`BundledEntry`, and `BundledEntryDto` so the field survives the full
YAML → domain → YAML round-trip and the scrub pass.
`ChangelogEntry.IsMarker` returns true when `Link` is non-null. A marker
is a machine-written object containing only `link: {prNumber}` that
redirects a non-primary PR to the parent entry's key; no markers exist in
S3 today (the writer lands in Step 8), so this is inert on merge.
`ChangelogContentScrubber.ScrubChangelog` explicitly preserves `link:` in
both the `BundledEntry` passed to `LinkAllowlistSanitizer` and the `with`
expression applied after sanitization. `link:` is never a URL, so no
allowlist rule touches it.
Tests: serialization round-trip for marker entries; scrub preserves `link:`
when prs: are stripped; `IsMarker` false on normal entries.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…d other non-link fields ToDto(ChangelogEntry) now short-circuits on IsMarker: only Link is emitted. This enforces the marker contract (link: is the sole field) and prevents 'title: ''' from appearing in round-tripped marker YAML. New test: SerializeEntry_MarkerEntry_YamlContainsOnlyLinkField. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…read (#3931) Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…ly short-circuit pure markers Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ent failures now throw instead of returning null Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…o longer probe elasticsearch pool Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
3c68594 to
9b76977
Compare
There was a problem hiding this comment.
Requesting changes: marker entries can still carry additional populated fields (for example issues) without failing scrub validation, which allows the marker fast-path to bypass private-reference checks.
What is this? | From workflow: PR Review
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
| Highlight = entry.Highlight, | ||
| Subtype = entry.Subtype | ||
| Subtype = entry.Subtype, | ||
| Link = entry.Link |
There was a problem hiding this comment.
link: is now preserved through scrub output, but marker validation above still only treats title, type, products, and prs as disallowed content. That means a payload like:
link: "12345"
issues:
- https://github.com/private-org/private-repo/issues/1is accepted as a marker and returned unchanged, skipping both allowlist scrubbing and ValidateNoPrivateReferences via the early return path.
Given the marker contract in this PR (link must be the only field), this should fail closed for any additional populated field (including issues, description, impact, action, areas, feature-id, highlight, subtype). Could you tighten the marker-content check to cover all non-link fields and add a regression test for link + issues?
Summary
link:toChangelogEntryDto,ChangelogEntry(domain type),BundledEntry, andBundledEntryDtoso the field survives the full YAML → domain → YAML round-trip and the scrub passChangelogEntry.IsMarkerreturnstruewhenLinkis non-nullChangelogContentScrubber.ScrubChangelogexplicitly preserveslink:through both theBundledEntrypassed toLinkAllowlistSanitizerand thewithexpression —link:is a bare PR number (never a URL), so no allowlist rule touches itMust ship before Step 8 (canonical keys + marker writing) so no bundler ever sees a marker it would treat as a malformed empty entry.
Test plan
link: "12345"→ deserialize →entry.Link == "12345",entry.IsMarker == truelink:when privateprs:are strippedIsMarkeris false on normal entriesdotnet test tests/Elastic.Changelog.Tests/ tests/Elastic.Documentation.Configuration.Tests/— all passingdotnet build -c Release— 0 errorsPart of the two-anchor plan (Step 7). Independent of Steps 4–6.
🤖 Generated with Claude Code