Conversation
Docs preview (local build)Handbook preview: https://docs-v3-preview.elastic.dev/elastic/docs-builder/pull/4120/ Sample changed pages: |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes: the current product/version notes flow can surface wrong-product notes in both CDN fallback and amend reconciliation paths.
What is this? | From workflow: PR Review
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
| if (!productMissing) | ||
| return []; | ||
|
|
||
| var legacyIndexUri = CombineSegments(baseUri, ["changelog", org, repo, $"notes-{version}.json"]); |
There was a problem hiding this comment.
[HIGH] Legacy fallback can import notes from the wrong product
Falling back from notes-product-version.json to the legacy notes-version.json on product-index 404 causes cross-product leakage when one product has notes for a version and another does not. In that case, bundling for the product with no notes still loads the version-union index and imports unrelated notes.
Concrete failure case: output-products contains two products at the same version where only one has notes; the other product gets those notes via this fallback path. Please gate fallback so it cannot be used when the requested (product, version) has no product-scoped index by design, for example migration-only fallback or post-filtering by requested product.
| if (!entries.Any(e => e.Path == poolRelativePath)) | ||
| entries.Add(new NoteIndexEntry { Path = poolRelativePath, BundleSeq = 0 }); | ||
| AddIndexEntry(byProductVersion, ProductVersionGroupKey(product, version), poolRelativePath); | ||
| AddIndexEntry(byVersion, version, poolRelativePath); |
There was a problem hiding this comment.
[HIGH] Returned notes map drops product scoping before amend reconciliation
ReconcileRepoAsync now builds product-scoped index groups, but it still returns version-to-note-paths via byVersion. ScrubberProcessor passes that map into NoteAmendReconciler, which processes bundles per product. Because product scoping is lost here, a product amend pass can treat another product's notes at the same version as late notes and publish them into the wrong product amend-notes sidecar.
Concrete scenario: one repo has elasticsearch@9.0.0 and kibana@9.0.0 notes, but only an elasticsearch bundle exists; the kibana note can be emitted into the elasticsearch amend-notes sidecar. Please preserve product+version scoping into the amend path, or filter notes by product before amend generation.
1e0bda1 to
80f9d4f
Compare
1 participant
Summary
This PR adds product-scoped notes indexes that are additive.
To ensure we don't break existing changelog users, the legacy
notes-{version}.jsonkeys stay, and{changelog}still does not read those indexes.Background
Relates to #4090, #4103
Since the related issues touch the same areas of the code and have a significant, overlapping impact, this PR is part of a multi-PR phased approach.
Details
Indexes. The scrubber now writes
changelog/{org}/{repo}/notes-{product}-{version}.json(withproductandversionon the body) and still dual-writesnotes-{version}.jsonas a version union without those fields. Stale product-scoped keys can be deleted; a version-only key is kept while any note still declares that version.Consumers.
changelog bundleGETs the product-scoped URL first and falls back to the version-only URL only on HTTP 404. An empty product-scoped index does not fall back.MergeNotesAsyncwalks each(product, version)from--output-productsand still dedups by checksum.Docs (same change set).
docs/development/changelog-bundle-registry.md,docs/cli/changelog/cmd-note.md, anddocs/cli/changelog/cmd-backfill.md.The dual-write path is in the
changelog-scrubberLambda. It will not show up on the live CDN until that Lambda is deployed.Generative AI disclosure
Tool(s) and model(s) used: Cursor Grok 4.6