Skip to content

Commit 2ffaec7

Browse files
cotticursoragent
andauthored
changelog: surface missing product label as no-label failure (#3304)
When `pivot.products` is configured with multiple products but a PR carries no matching product label (and no `products.default` is set), `changelog evaluate-pr` would still report `proceed`. The downstream `changelog add` step then failed with "At least one product is required", which is a workflow-level hard error and skips the helpful PR comment that surfaces the missing labels. Treat this case the same as a missing type label: return `NoLabel` and emit the product-label-table so the existing comment-failure path posts an actionable hint to the contributor. When `pivot.products` defines only one distinct product, assign it implicitly — no point requiring a redundant label when there is only one possible answer. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 0b2d6e8 commit 2ffaec7

3 files changed

Lines changed: 140 additions & 6 deletions

File tree

src/services/Elastic.Changelog/Evaluation/ChangelogPrEvaluationService.cs

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,16 +114,31 @@ public async Task<bool> EvaluatePr(IDiagnosticsCollector collector, EvaluatePrAr
114114
// Resolve products from labels
115115
string? resolvedProducts = null;
116116
string? productLabelTable = null;
117-
if (config.LabelToProducts is { Count: > 0 })
117+
if (config.LabelToProducts is { Count: > 0 } labelToProducts)
118118
{
119-
var products = PrInfoProcessor.MapLabelsToProducts(input.PrLabels, config.LabelToProducts);
119+
var products = PrInfoProcessor.MapLabelsToProducts(input.PrLabels, labelToProducts);
120120
if (products.Count > 0)
121121
{
122122
resolvedProducts = ProductArgument.FormatProductSpecs(products);
123123
_logger.LogInformation("Mapped PR labels to products: {Products}", resolvedProducts);
124124
}
125125
else
126-
productLabelTable = BuildProductLabelTable(config.LabelToProducts);
126+
{
127+
// When only one distinct product is configured, assigning it implicitly is
128+
// unambiguous — no point requiring contributors to add a redundant label.
129+
var distinctSpecs = labelToProducts.Values
130+
.Distinct(StringComparer.OrdinalIgnoreCase)
131+
.ToList();
132+
if (distinctSpecs.Count == 1)
133+
{
134+
resolvedProducts = ProductArgument.FormatProductSpecs(
135+
ProductArgument.ParseProductSpecs(distinctSpecs[0])
136+
);
137+
_logger.LogInformation("Single product configured; assigning implicitly: {Products}", resolvedProducts);
138+
}
139+
else
140+
productLabelTable = BuildProductLabelTable(labelToProducts);
141+
}
127142
}
128143

129144
if (resolvedType == null)
@@ -138,6 +153,22 @@ public async Task<bool> EvaluatePr(IDiagnosticsCollector collector, EvaluatePrAr
138153
);
139154
}
140155

156+
// Multiple products are configured via labels but none matched, and no defaults are
157+
// available to fill in via inference at 'changelog add' time. Surface this as a
158+
// missing-label failure so the contributor sees an actionable hint instead of a hard
159+
// error later in the workflow.
160+
if (productLabelTable != null
161+
&& (config.ProductsConfiguration?.Default is null or { Count: 0 }))
162+
{
163+
_logger.LogInformation("Multiple products configured but no matching product label on PR; no default products configured");
164+
return await SetOutputs(
165+
PrEvaluationResult.NoLabel, title,
166+
resolvedDescription: description,
167+
productLabelTable: productLabelTable,
168+
skipLabels: skipLabels
169+
);
170+
}
171+
141172
_logger.LogInformation("PR evaluation complete: title={Title}, type={Type}, products={Products}, existingFile={File}", title, resolvedType, resolvedProducts, existingFilename);
142173
return await SetOutputs(
143174
PrEvaluationResult.Success, title, resolvedType,

src/services/Elastic.Changelog/Evaluation/PrEvaluationResult.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public enum PrEvaluationResult
1515
[Display(Name = "success")]
1616
Success,
1717

18-
/// <summary>No label matching a changelog type was found on the PR.</summary>
18+
/// <summary>A required label (type or product) is missing from the PR.</summary>
1919
[Display(Name = "no-label")]
2020
NoLabel,
2121

tests/Elastic.Changelog.Tests/Evaluation/ChangelogPrEvaluationServiceTests.cs

Lines changed: 105 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,7 @@ public async Task EvaluatePr_WithProductLabels_OutputsProductsAndNoTable()
520520
}
521521

522522
[Fact]
523-
public async Task EvaluatePr_WithoutProductLabels_OutputsProductLabelTable()
523+
public async Task EvaluatePr_WithoutProductLabels_ReturnsNoLabelAndOutputsProductLabelTable()
524524
{
525525
await WriteMinimalConfig(Path.Join(Paths.WorkingDirectoryRoot.FullName, "config", "changelog.yml"), ConfigWithProducts);
526526
var service = CreateService();
@@ -532,10 +532,113 @@ public async Task EvaluatePr_WithoutProductLabels_OutputsProductLabelTable()
532532
var result = await service.EvaluatePr(Collector, args, CancellationToken.None);
533533

534534
result.Should().BeTrue();
535-
VerifyOutputSet("status", "proceed");
535+
VerifyOutputSet("status", "no-label");
536+
VerifyOutputSet("should-generate", "false");
536537
A.CallTo(() => _mockCore.SetOutputAsync("products", A<string>._)).MustNotHaveHappened();
537538
A.CallTo(() => _mockCore.SetOutputAsync("product-label-table", A<string>.That.Contains("@Product:ECH"))).MustHaveHappened();
538539
A.CallTo(() => _mockCore.SetOutputAsync("product-label-table", A<string>.That.Contains("cloud-hosted"))).MustHaveHappened();
540+
A.CallTo(() => _mockCore.SetOutputAsync("label-table", A<string>._)).MustNotHaveHappened();
541+
}
542+
543+
[Fact]
544+
public async Task EvaluatePr_SingleProductConfigured_WithoutLabel_AutoAssignsProduct()
545+
{
546+
var singleProductConfig = """
547+
pivot:
548+
types:
549+
feature: "type:feature"
550+
bug-fix: "type:bug"
551+
breaking-change: "type:breaking"
552+
enhancement:
553+
deprecation:
554+
docs:
555+
known-issue:
556+
other:
557+
regression:
558+
security:
559+
products:
560+
kibana: "@Product:Kibana"
561+
""";
562+
await WriteMinimalConfig(content: singleProductConfig);
563+
var service = CreateService();
564+
var args = DefaultArgs(prLabels: ["type:feature"]);
565+
566+
var result = await service.EvaluatePr(Collector, args, CancellationToken.None);
567+
568+
result.Should().BeTrue();
569+
VerifyOutputSet("status", "proceed");
570+
VerifyOutputSet("products", "kibana");
571+
A.CallTo(() => _mockCore.SetOutputAsync("product-label-table", A<string>._)).MustNotHaveHappened();
572+
}
573+
574+
[Fact]
575+
public async Task EvaluatePr_SingleProductConfigured_WithMultipleLabelAliases_AutoAssignsProduct()
576+
{
577+
// Multi-label aliasing for the same product is still a single-product config — should
578+
// not require any explicit label on the PR.
579+
var configWithAliases = """
580+
pivot:
581+
types:
582+
feature: "type:feature"
583+
bug-fix: "type:bug"
584+
breaking-change: "type:breaking"
585+
enhancement:
586+
deprecation:
587+
docs:
588+
known-issue:
589+
other:
590+
regression:
591+
security:
592+
products:
593+
kibana:
594+
- "@Product:Kibana"
595+
- "@kibana"
596+
""";
597+
await WriteMinimalConfig(content: configWithAliases);
598+
var service = CreateService();
599+
var args = DefaultArgs(prLabels: ["type:feature"]);
600+
601+
var result = await service.EvaluatePr(Collector, args, CancellationToken.None);
602+
603+
result.Should().BeTrue();
604+
VerifyOutputSet("status", "proceed");
605+
VerifyOutputSet("products", "kibana");
606+
A.CallTo(() => _mockCore.SetOutputAsync("product-label-table", A<string>._)).MustNotHaveHappened();
607+
}
608+
609+
[Fact]
610+
public async Task EvaluatePr_WithoutProductLabels_WithDefaultProducts_ReturnsProceed()
611+
{
612+
var configWithDefaults = """
613+
pivot:
614+
types:
615+
feature: "type:feature"
616+
bug-fix: "type:bug"
617+
breaking-change: "type:breaking"
618+
enhancement:
619+
deprecation:
620+
docs:
621+
known-issue:
622+
other:
623+
regression:
624+
security:
625+
products:
626+
cloud-hosted: "@Product:ECH"
627+
cloud-serverless: "@Product:ESS"
628+
products:
629+
default:
630+
- product: cloud-hosted
631+
lifecycle: ga
632+
""";
633+
await WriteMinimalConfig(content: configWithDefaults);
634+
var service = CreateService();
635+
var args = DefaultArgs(prLabels: ["type:feature"]);
636+
637+
var result = await service.EvaluatePr(Collector, args, CancellationToken.None);
638+
639+
result.Should().BeTrue();
640+
VerifyOutputSet("status", "proceed");
641+
VerifyOutputSet("should-generate", "true");
539642
}
540643

541644
[Fact]

0 commit comments

Comments
 (0)