Skip to content

publish-plan: annotate a non-publishing pin push; codify fail-loud gates (#354) - #355

Merged
ptr727 merged 2 commits into
developfrom
publish-plan-fail-loud-354
Jul 18, 2026
Merged

publish-plan: annotate a non-publishing pin push; codify fail-loud gates (#354)#355
ptr727 merged 2 commits into
developfrom
publish-plan-fail-loud-354

Conversation

@ptr727

@ptr727 ptr727 commented Jul 18, 2026

Copy link
Copy Markdown
Owner

Fixes #354, taking option 1 (the minimal, recommended one) plus the generalization.

The fix

A main pin push by an actor outside the allowlist now emits a ::warning:: rather than falling through silently:

elif [[ "$REF" == "main" ]]; then
  echo "::warning::Pin push on main by unrecognized actor '$ACTOR'; not publishing. If this is the codegen App under a new identity, update the allowlist in publish-plan-task.yml."
fi

Verification: the publish decision is unchanged

Simulated the patched logic across the full matrix - the gate behaves identically for every legitimate case, and the warning fires only on the silent-failure path:

push               ptr727-codegen[bot]      main     -> publish=true
push               dependabot[bot]          main     -> publish=true
push               ptr727-codegen-v2[bot]   main     -> publish=false  + ::warning::   <- the renamed-App case
push               ptr727                   main     -> publish=false  + ::warning::   <- hand edit, worth seeing
push               ptr727-codegen[bot]      develop  -> publish=false
schedule           github                   main     -> publish=true
workflow_dispatch  ptr727                   develop  -> publish=true

Generalization codified: WORKFLOW.md D8.4

An identity allowlist used as a gate fails loud.

with the failure mode it prevents, and two judgment calls worth recording:

  • Where an annotation is optional: the merge-bot hard-codes the same identities, but its failure is self-announcing - it stops merging and bot PRs visibly pile up. The publish gate is the acute case precisely because the weekly schedule masks it, leaving only a timeliness symptom.
  • The escalation: option 2 (mint an App token, resolve the identity via GET /app) removes the hard-coded string entirely, and is the right move only if an allowlist proves fragile in practice - noted rather than adopted, per the issue.

Validation

actionlint clean on the snippet; spec/validate.py, markdownlint, editorconfig-checker clean; LF preserved on the workflow YAML.

🤖 Generated with Claude Code

…tes (#354)

The push gate matches github.actor against a hard-coded bot allowlist.
The allowlist is correct today, but if the App is renamed, replaced, or
reinstalled under a new slug the comparison quietly evaluates false,
publish is skipped, and the run stays green. The weekly schedule keeps
publishing, so the only symptom is lost release timeliness - easy to
miss for months, and it defeats the tracker's same-day purpose.

Option 1 from the issue (minimal): a main pin push by an unrecognized
actor now emits a ::warning:: instead of falling through silently. The
publish decision is unchanged for every case - simulated across the
matrix (codegen/dependabot/renamed-App/human x main/develop, plus
schedule and dispatch): identical publish= results, the warning firing
only on the silent-failure path.

Also codifies the transferable rule as WORKFLOW.md D8.4: an identity
allowlist used as a gate fails loud. It notes where an annotation is
optional (the merge-bot's failure is self-announcing - bot PRs visibly
pile up) and names the run-time identity resolution (GET /app) as the
escalation if an allowlist proves fragile.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 18, 2026 19:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the release gating guidance and the reusable publish-plan workflow snippet so that identity-based allowlists do not fail silently when an unexpected github.actor appears (for example, if an App is renamed/reinstalled under a different slug).

Changes:

  • Add a new WORKFLOW.md guarantee (D8.4) documenting that identity allowlists used as gates must emit a ::warning:: on the non-matching path.
  • Update publish-plan-task.yml to warn (instead of silently no-op) when a push to main does not match the bot allowlist.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
WORKFLOW.md Codifies the “fail loud” rule for identity allowlists used as workflow gates (D8.4).
catalog/snippets/workflows/publish-plan-task.yml Emits a ::warning:: when a main push is ignored due to an unrecognized actor, preventing silent non-publishing runs.

Comment thread catalog/snippets/workflows/publish-plan-task.yml Outdated
…355)

publish-plan-task is the generic publish gate every publish-release job
reuses, not only the upstream-version pin flow, so "Pin push on main"
would misread on any other push trigger. The message and its comment now
say "Push to main ... release bot", which is accurate for every consumer.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 18, 2026 19:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

catalog/snippets/workflows/publish-plan-task.yml:80

  • The new rationale comment spans three lines; repo guidance says one line is the default and extra lines should be reserved for constraints the code cannot express. This can be condensed without losing intent.
                # Fail loud: an unrecognized actor pushing to main is either a human commit (legitimately not
                # publishing, but worth seeing) or a release bot under a new identity, which would otherwise
                # stop publishing silently while a schedule keeps releasing - lost timeliness, no error.

@ptr727
ptr727 merged commit 4280881 into develop Jul 18, 2026
7 checks passed
@ptr727
ptr727 deleted the publish-plan-fail-loud-354 branch July 18, 2026 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants