changelog: switch prepare-artifact bool CLI params to bool?

docs/cli-schema.json

@@ -3394,7 +3394,7 @@
           ],
           "name": "prepare-artifact",
           "summary": "(CI) Package changelog artifact for cross-workflow transfer.",
-          "notes": "Resolves final status from evaluate-pr \u002B changelog add outcomes, copies generated YAML,\nwrites metadata.json, and sets GitHub Actions outputs. Always succeeds (exit 0) so the upload step runs.",
+          "notes": "Resolves final status from evaluate-pr \u002B changelog add outcomes, copies generated YAML,\nwrites metadata.json, and sets GitHub Actions outputs. Always succeeds (exit 0) so the upload step runs.\n\n\nThe isFork, canCommit and maintainerCanModify parameters are declared\nas bool? so the generated CLI emits both --flag and --no-flag pairs\n(Argh convention). A plain bool would expose presence-only switches: passing\n--can-commit \u0022false\u0022 would set canCommit = true (the flag is present) and\nsilently discard the literal \u0022false\u0022 as a stray positional. Callers that forward a\ndynamic value (--can-commit \u0022$VAR\u0022) would then misroute fork PRs into the\ncommit-and-push branch and die on a detached-HEAD push. See elastic/docs-actions#172\nfor the workflow-side fix.",
           "usage": "docs-builder changelog prepare-artifact --staging-dir \u003Cstring\u003E --output-dir \u003Cstring\u003E --evaluate-status \u003Cstring\u003E --generate-outcome \u003Cstring\u003E --pr-number \u003Cint\u003E --head-ref \u003Cstring\u003E --head-sha \u003Cstring\u003E [options]",
           "examples": [],
           "parameters": [
@@ -3452,24 +3452,24 @@
               "name": "is-fork",
               "type": "boolean",
               "required": false,
-              "summary": "Whether the PR is from a fork",
-              "defaultValue": "false"
+              "summary": "Whether the PR is from a fork (pass --is-fork / --no-is-fork; omit to leave null which is treated as false)",
+              "defaultValue": "default"
             },
             {
               "role": "flag",
               "name": "can-commit",
               "type": "boolean",
               "required": false,
-              "summary": "Whether the commit strategy allows committing",
-              "defaultValue": "false"
+              "summary": "Whether the commit strategy allows committing (pass --can-commit / --no-can-commit; omit to leave null which is treated as false)",
+              "defaultValue": "default"
             },
             {
               "role": "flag",
               "name": "maintainer-can-modify",
               "type": "boolean",
               "required": false,
-              "summary": "Whether the fork PR allows maintainer edits",
-              "defaultValue": "false"
+              "summary": "Whether the fork PR allows maintainer edits (pass --maintainer-can-modify / --no-maintainer-can-modify; omit to leave null which is treated as false)",
+              "defaultValue": "default"
             },
             {
               "role": "flag",

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

@@ -79,16 +79,20 @@ public async Task<bool> PrepareArtifact(IDiagnosticsCollector collector, Prepare
 		var changelogDir = config?.Bundle?.Directory ?? "docs/changelog";
 
 		var statusString = status.ToStringFast(true);
+		// Null-coalesce the nullable bool inputs into the metadata's concrete
+		// `bool` fields. Treating "unspecified" as `false` keeps downstream
+		// consumers (apply step) failing closed: an unrecognised or omitted
+		// CLI flag never grants commit permission.
 		var metadata = new ChangelogArtifactMetadata
 		{
 			PrNumber = input.PrNumber,
 			HeadRef = input.HeadRef,
 			HeadSha = input.HeadSha,
 			Status = statusString,
-			IsFork = input.IsFork,
+			IsFork = input.IsFork ?? false,
 			HeadRepo = input.HeadRepo,
-			CanCommit = input.CanCommit,
-			MaintainerCanModify = input.MaintainerCanModify,
+			CanCommit = input.CanCommit ?? false,
+			MaintainerCanModify = input.MaintainerCanModify ?? false,
 			LabelTable = input.LabelTable,
 			ProductLabelTable = input.ProductLabelTable,
 			SkipLabels = input.SkipLabels,

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

@@ -14,10 +14,16 @@ public record PrepareArtifactArguments
 	public required int PrNumber { get; init; }
 	public required string HeadRef { get; init; }
 	public required string HeadSha { get; init; }
-	public bool IsFork { get; init; }
+	// Nullable bool mirrors the CLI surface: null = "flag not specified",
+	// normalized to false when serialized into ChangelogArtifactMetadata.
+	// Using non-nullable bool here would force every call site to choose
+	// between true/false and lose the "not specified" signal, which is what
+	// allowed --can-commit "false" to silently set CanCommit = true at the
+	// CLI before this change.
+	public bool? IsFork { get; init; }
 	public string? HeadRepo { get; init; }
-	public bool CanCommit { get; init; }
-	public bool MaintainerCanModify { get; init; }
+	public bool? CanCommit { get; init; }
+	public bool? MaintainerCanModify { get; init; }
 	public string? LabelTable { get; init; }
 	public string? ProductLabelTable { get; init; }
 	public string? SkipLabels { get; init; }

src/tooling/docs-builder/Commands/ChangelogCommand.cs

@@ -1401,6 +1401,17 @@ async static (s, collector, state, ctx) => await s.EvaluatePr(collector, state,
 	/// <remarks>
 	/// Resolves final status from evaluate-pr + changelog add outcomes, copies generated YAML,
 	/// writes metadata.json, and sets GitHub Actions outputs. Always succeeds (exit 0) so the upload step runs.
+	///
+	/// <para>
+	/// The <c>isFork</c>, <c>canCommit</c> and <c>maintainerCanModify</c> parameters are declared
+	/// as <c>bool?</c> so the generated CLI emits both <c>--flag</c> and <c>--no-flag</c> pairs
+	/// (Argh convention). A plain <c>bool</c> would expose presence-only switches: passing
+	/// <c>--can-commit "false"</c> would set <c>canCommit = true</c> (the flag is present) and
+	/// silently discard the literal <c>"false"</c> as a stray positional. Callers that forward a
+	/// dynamic value (<c>--can-commit "$VAR"</c>) would then misroute fork PRs into the
+	/// commit-and-push branch and die on a detached-HEAD push. See elastic/docs-actions#172
+	/// for the workflow-side fix.
+	/// </para>
 	/// </remarks>
 	/// <param name="stagingDir">Directory where changelog add wrote the generated YAML</param>
 	/// <param name="outputDir">Directory to write the artifact (metadata.json + YAML)</param>
@@ -1409,9 +1420,9 @@ async static (s, collector, state, ctx) => await s.EvaluatePr(collector, state,
 	/// <param name="prNumber">Pull request number</param>
 	/// <param name="headRef">PR head branch ref</param>
 	/// <param name="headSha">PR head commit SHA</param>
-	/// <param name="isFork">Whether the PR is from a fork</param>
-	/// <param name="canCommit">Whether the commit strategy allows committing</param>
-	/// <param name="maintainerCanModify">Whether the fork PR allows maintainer edits</param>
+	/// <param name="isFork">Whether the PR is from a fork (pass --is-fork / --no-is-fork; omit to leave null which is treated as false)</param>
+	/// <param name="canCommit">Whether the commit strategy allows committing (pass --can-commit / --no-can-commit; omit to leave null which is treated as false)</param>
+	/// <param name="maintainerCanModify">Whether the fork PR allows maintainer edits (pass --maintainer-can-modify / --no-maintainer-can-modify; omit to leave null which is treated as false)</param>
 	/// <param name="headRepo">Fork repository full name (owner/repo)</param>
 	/// <param name="labelTable">Optional: markdown label table from evaluate-pr</param>
 	/// <param name="productLabelTable">Optional: markdown product label table from evaluate-pr</param>
@@ -1427,9 +1438,9 @@ public async Task<int> PrepareArtifact(
 		int prNumber,
 		string headRef,
 		string headSha,
-		bool isFork = false,
-		bool canCommit = false,
-		bool maintainerCanModify = false,
+		bool? isFork = null,
+		bool? canCommit = null,
+		bool? maintainerCanModify = null,
 		string? headRepo = null,
 		string? labelTable = null,
 		string? productLabelTable = null,

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

@@ -149,6 +149,34 @@ public async Task PrepareArtifact_ForkNoMaintainerEdits_CanCommitFalse()
 		metadata.MaintainerCanModify.Should().BeFalse();
 	}
 
+	// Regression for the docs-actions / docs-builder fork-PR detached-HEAD failure
+	// (see elastic/docs-actions#172). The bool? CLI parameters mean an *omitted*
+	// --can-commit flag arrives at the service as null. The service must coerce
+	// that to false in the persisted metadata so the downstream apply step does
+	// NOT attempt a commit+push. Failing closed on "unspecified" is the whole
+	// point of moving these flags to nullable bool.
+	[Fact]
+	public async Task PrepareArtifact_NullableBoolsUnspecified_CoerceToFalseInMetadata()
+	{
+		await SetupStagingYaml();
+		await SetupConfig();
+		var service = CreateService();
+		var args = DefaultArgs() with
+		{
+			IsFork = null,
+			CanCommit = null,
+			MaintainerCanModify = null,
+			HeadRepo = null
+		};
+
+		await service.PrepareArtifact(Collector, args, CancellationToken.None);
+
+		var metadata = ReadMetadata();
+		metadata.IsFork.Should().BeFalse("an omitted --is-fork flag must not be treated as 'fork'");
+		metadata.CanCommit.Should().BeFalse("an omitted --can-commit flag must never grant commit permission to the apply step");
+		metadata.MaintainerCanModify.Should().BeFalse("an omitted --maintainer-can-modify flag must not be treated as granted");
+	}
+
 	[Fact]
 	public async Task PrepareArtifact_ProductLabelTableAndSkipLabels_PersistedInMetadata()
 	{