Skip to content

Fix: Reconcile closed changes and bundle action/workflow#330

Merged
tykeal merged 3 commits into
lfreleng-actions:mainfrom
modeseven-lfreleng-actions:fix/final-state-reconciliation
Jul 15, 2026
Merged

Fix: Reconcile closed changes and bundle action/workflow#330
tykeal merged 3 commits into
lfreleng-actions:mainfrom
modeseven-lfreleng-actions:fix/final-state-reconciliation

Conversation

@ModeSevenIndustrialSolutions

Copy link
Copy Markdown
Contributor

Summary

Two related fixes arising from a production failure in onap/policy-opa-pdp (failing run), where a workflow re-run against PR #33 pushed a patchset to Gerrit change 146080 which had already merged, producing ! [remote rejected] (change ... closed) — misreported as an SSH/connection failure (exit 5) with a full traceback in the console logs.

Commit 1: Handle closed Gerrit changes on PR re-runs

  • Pre-push state reconciliation: after Change-Id reuse is resolved, query each change's status in Gerrit. When every resolved change is in a final state (MERGED/ABANDONED), close the GitHub PR with an explanatory comment (honouring CLOSE_MERGED_PRS) and return the existing change details instead of attempting a push Gerrit will reject. Unknown states fail open; mixed open/closed states proceed with a warning.
  • Correct error classification: change ... closed push rejections now map to GERRIT_CHANGE_ALREADY_FINAL (exit 10) with an accurate message, instead of GERRIT_CONNECTION_ERROR (exit 5) / "check SSH keys and server configuration".
  • Cleaner console output: the push-failure traceback only appears in verbose mode; normal runs log a single analyzed error line.
  • 11 new regression tests in tests/test_final_state_reconciliation.py.

Commit 2: Bundle action with reusable workflow at same SHA

The reusable workflow pinned the composite action at v1.3.3 while the repo tagged v1.4.0/v1.4.1 — callers of the v1.4.1 reusable workflow silently ran the v1.3.3 action with a drifted input interface, and no release process step existed to bump the pin.

  • The workflow now checks out the action from its own repository at github.job_workflow_sha (the exact commit the caller pinned) and runs it as a local action — workflow and action ship as one atomic unit, interfaces can never drift, and no release-time pin bump is needed
  • New SKIP_CHECKOUT action input prevents the action's internal checkout from wiping the bundled action checkout (the workflow performs the target-repo checkout itself)
  • The Python tool continues to float on the latest PyPI release via uvx (unchanged behavior for external repos)
  • actionlint config: ignore for the documented-but-unrecognized job_workflow_ref/job_workflow_sha context properties

Validation

  • Full pytest suite passes (0 failures), including 11 new regression tests
  • ruff check/format, mypy, basedpyright clean
  • actionlint (1.7.11 and CI-pinned 1.7.12.24), yamllint, zizmor clean
  • All repo pre-commit hooks pass

When a workflow re-runs against a pull request whose Gerrit change
has already merged (or been abandoned), reconciliation reused the
prior Change-Id and pushed a new patchset, which Gerrit rejected
with 'change ... closed'. The failure then surfaced as a misleading
SSH/connection error (exit code 5) with a full traceback.

Add a pre-push state reconciliation step: after Change-Id reuse is
resolved, query the status of each change in Gerrit. When every
resolved change is in a final state, close the GitHub PR with an
explanatory comment (honouring CLOSE_MERGED_PRS) and return the
existing change details instead of attempting a doomed push.

Classify 'change ... closed' push rejections as
GERRIT_CHANGE_ALREADY_FINAL (exit code 10) rather than a Gerrit
connection failure, and reserve the push-failure traceback for
verbose mode so console logs stay actionable.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>
The reusable workflow pinned the composite action at a fixed
release (v1.3.3), which fell out of step with the workflow itself
as new releases were tagged; callers on v1.4.1 silently ran the
older action with a mismatched input interface.

Remove the version pin entirely: check out the action from the
repository providing this reusable workflow at the exact commit the
caller pinned (github.job_workflow_sha) and run it as a local
action. Workflow and action now always ship as one unit with
matching interfaces, and no release-time pin bump is needed. The
Python tool continues to install as the latest release from PyPI
at runtime via uvx.

Add a SKIP_CHECKOUT input to the action so the workflow's own
repository checkout is not wiped by the action's internal checkout,
and teach actionlint about the job_workflow_ref/job_workflow_sha
context properties its schema does not yet include.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>

Copilot AI left a comment

Copy link
Copy Markdown

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 addresses two production issues in the GitHub2Gerrit action/workflow: (1) handling Gerrit “change … closed” rejections correctly (and reconciling PR state before attempting a doomed push), and (2) preventing drift between the reusable workflow and the composite action by bundling them at the same SHA.

Changes:

  • Add pre-push reconciliation to detect when all reused Change-Ids are already in a final state (MERGED/ABANDONED) and stop early (optionally closing/commenting on the GitHub PR).
  • Improve push-failure analysis and exit-code classification so “change … closed” maps to GERRIT_CHANGE_ALREADY_FINAL, and reduce non-verbose traceback noise.
  • Update the reusable workflow to check out and run the composite action from the same commit SHA as the workflow, and add SKIP_CHECKOUT to avoid nested checkout wiping the bundled action directory.

Reviewed changes

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

Show a summary per file
File Description
tests/test_final_state_reconciliation.py Adds regression tests for closed-change rejection analysis, exit-code mapping, and final-state reconciliation behavior.
src/github2gerrit/error_codes.py Classifies “change closed / cannot accept new patchsets” as GERRIT_CHANGE_ALREADY_FINAL and surfaces a clearer user-facing message.
src/github2gerrit/core.py Implements Gerrit change status lookup + final-state reconciliation, improves push failure logging verbosity behavior, and adds specific “change closed” analysis.
action.yaml Introduces SKIP_CHECKOUT input and conditionally skips the action’s internal checkout when the caller/workflow performs checkout.
.github/workflows/github2gerrit.yaml Bundles workflow + action by checking out the action repo at github.job_workflow_sha and running it locally.
.github/actionlint.yaml Suppresses actionlint schema warnings for documented github.job_workflow_ref/github.job_workflow_sha context properties.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/github2gerrit.yaml Outdated
Copilot AI review requested due to automatic review settings July 15, 2026 11:17

This comment was marked as resolved.

Copilot AI review requested due to automatic review settings July 15, 2026 11:32
@ModeSevenIndustrialSolutions
ModeSevenIndustrialSolutions force-pushed the fix/final-state-reconciliation branch from e719b7e to 2a75728 Compare July 15, 2026 11:32

Copilot AI left a comment

Copy link
Copy Markdown

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 6 out of 6 changed files in this pull request and generated 1 comment.

Comment thread src/github2gerrit/core.py
Copilot AI review requested due to automatic review settings July 15, 2026 11:39
@ModeSevenIndustrialSolutions
ModeSevenIndustrialSolutions force-pushed the fix/final-state-reconciliation branch from 2a75728 to 79bde99 Compare July 15, 2026 11:39

Copilot AI left a comment

Copy link
Copy Markdown

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 6 out of 6 changed files in this pull request and generated 2 comments.

Comment thread src/github2gerrit/core.py Outdated
Comment thread src/github2gerrit/core.py Outdated
Validate the owner/repo value parsed from job_workflow_ref in the
workflow-source resolution step and fail fast with a clear error
when the format is unexpected, rather than proceeding to a
confusing checkout failure.

Comment on the pull request when final-state reconciliation stops
the pipeline for a merged change and CLOSE_MERGED_PRS is false;
close_pr_with_status posts no comment for merged changes in that
configuration, which left the early stop unexplained. Add a
regression test covering the comment-only path.

Type the overall reconciliation status as a Literal instead of
suppressing the close_pr_with_status argument check with a
type-ignore comment.

Reference every reconciled change in log messages and PR
close/comment text when multiple changes are in a final state,
rather than only the first change URL. Add a regression test
asserting all change URLs appear in the PR closure call.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>
Copilot AI review requested due to automatic review settings July 15, 2026 11:53
@ModeSevenIndustrialSolutions
ModeSevenIndustrialSolutions force-pushed the fix/final-state-reconciliation branch from 79bde99 to 17e6520 Compare July 15, 2026 11:53

Copilot AI left a comment

Copy link
Copy Markdown

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 6 out of 6 changed files in this pull request and generated no new comments.

@tykeal
tykeal merged commit 2e15991 into lfreleng-actions:main Jul 15, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants