changelog/submit: stop routing fork PRs through the broken commit path

[cotti]

Summary

Fork PRs from verified Elastic org members (the only fork PRs that the workflow currently lets through) have been silently routed into the commit-and-push branch of changelog/submit/apply and dying on git push with fatal: You are not currently on a branch. See elastic/elastic-otel-java#1117's submit run for a concrete reproduction.

Three defects compounded:

1. The CLI flag values are silently misread. changelog/submit/evaluate shells --can-commit "$CAN_COMMIT" (and similar bool flags) into docs-builder changelog prepare-artifact. The underlying CLI framework (Nullean.Argh) treats bool as a presence-only switch: --flag sets true; omission leaves it false. Passing --can-commit "false" therefore set canCommit = TRUE (the flag was present) and silently dropped the literal \"false\" as a stray positional. Confirmed against the :edge image's prepare-artifact --help, which shows those three flags with no <value> placeholder unlike every string flag.
   The artifact metadata then carried can_commit: true for every fork PR from an org member.

2. The apply step had no second line of defense. It trusted should-commit alone, so when should-commit=true came back incorrectly, it checked out the fork at the head SHA (detached HEAD), tried to push to the upstream remote (where GITHUB_TOKEN has write but the branch doesn't exist), and failed hard.

3. No graceful fallback on commit failure. The workflow errored out, leaving the PR with neither a committed entry nor a comment preview.

Changes

• changelog/submit/evaluate/action.yml — build the prepare-artifact argv as an array and append --is-fork, --can-commit, --maintainer-can-modify only when the upstream string value is exactly \"true\". Forward-compatible with the planned docs-builder switch to bool? (which would also add --no-* negations).

• changelog/submit/apply/action.yml —

  • Add a defense-in-depth is-fork != 'true' guard to the checkout / validate-ref / commit steps. Even if should-commit flips to true incorrectly again in the future, fork PRs will skip the commit path entirely and fall through to the comment-only logic instead of producing the detached-HEAD error. (See discussion of "fail closed on the next regression".)
  • Mark the commit step continue-on-error: true, surface any failure via a ::warning:: annotation, and let the Post comment-only changelog step take over when commit.outcome == 'failure'. Same-repo PRs whose push is rejected (branch protection, revoked token scope, etc.) now preserve the generated entry as a PR comment instead of vanishing.

Companion PR

A separate docs-builder PR will switch the affected prepare-artifact parameters to bool? so --can-commit false is no longer ambiguous at the CLI surface, and adds the missing CLI-surface test that lets bool mishandling slip through today.

Test plan

• Lint passes: actionlint .github/workflows/changelog-*.yml, python3 -c \"import yaml; yaml.safe_load(open(...))\", and bash -n on every inline run: block (done locally).
• Manually re-trigger the changelog-submit workflow against a fresh fork PR (e.g. elastic/elastic-otel-java) once this is merged + tagged: should post a single comment with the rendered entry and not attempt any commit/push.
• Same-repo PR: should still commit + push as before (unchanged path for that case).
• (Optional) Synthetic same-repo PR with branch protection blocking the bot: should warn + post a comment instead of erroring.

Related: #69, #129 (both about enabling fork-commit via PAT; this PR only restores the documented "forks always comment-only" path that was silently broken).

Made with Cursor