Fix: Bugs related to Github issue #157 by ModeSevenIndustrialSolutions · Pull Request #159 · lfreleng-actions/github2gerrit-action

ModeSevenIndustrialSolutions · 2026-03-13T15:08:08Z

Address issues raised in: #157

The Gerrit REST API hostname was derived as gerrit.{org}.org instead of reading the host field from .gitreview. This caused REST API calls to fail for organizations using a different hostname convention (e.g. git.opendaylight.org). Add _read_gitreview_host() to read the host from the local .gitreview file (available after actions/checkout) or via raw.githubusercontent.com as a fallback. Use it in derive_gerrit_parameters() with priority: 1. Per-org config file entry 2. .gitreview host field 3. Heuristic gerrit.{org}.org fallback Resolves: lfreleng-actions#157 Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>

The cleanup_closed_github_prs function re-raised GerritRestError as a fatal GitHub2GerritError, causing the entire job to fail even when the primary sync operation succeeded. Log cleanup REST errors as warnings and return gracefully so the job reports success when the core sync completed. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>

When an UPDATE operation finds no existing Gerrit change, the error message now tells users they can set CREATE_MISSING=true or use the PR comment command to create a new change instead. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>

Cover all three fixes to prevent regressions: - _read_gitreview_host local/remote parsing (9 tests) - derive_gerrit_parameters .gitreview priority chain (4 tests) - cleanup_closed_github_prs non-fatal REST errors (4 tests) - UPDATE error message mentions CREATE_MISSING (2 tests) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>

Copilot

Pull request overview

Adds regression coverage and implementation tweaks for GitHub issue #157 to improve Gerrit host derivation, make cleanup failures non-fatal, and provide clearer guidance for UPDATE-without-existing-change scenarios.

Changes:

Derive GERRIT_SERVER from .gitreview (local-first, optional remote fallback) before falling back to gerrit.{org}.org.
Make cleanup_closed_github_prs() treat Gerrit REST/connection failures as non-fatal (log + return 0).
Update UPDATE error/help messaging to suggest CREATE_MISSING=true or the @github2gerrit create missing change PR comment, and add regression tests.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`tests/test_issue_157_regressions.py`	New regression suite covering `.gitreview` host parsing, parameter derivation priority, non-fatal cleanup behavior, and improved UPDATE error messaging.
`src/github2gerrit/config.py`	Adds `_read_gitreview_host()` and updates `derive_gerrit_parameters()` to prefer `.gitreview` host over the heuristic default.
`src/github2gerrit/gerrit_pr_closer.py`	Changes cleanup error handling to warn and return success (0) rather than failing the workflow.
`src/github2gerrit/core.py`	Improves UPDATE-without-existing-change error message with `CREATE_MISSING` and PR comment guidance.
`src/github2gerrit/cli.py`	Updates CLI guidance text to match the new CREATE_MISSING / PR comment remedies.

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

tests/test_issue_157_regressions.py

src/github2gerrit/config.py

- Fix test patch targets: use github2gerrit.gerrit_rest.build_client_for_host instead of github2gerrit.gerrit_pr_closer.build_client_for_host since cleanup_closed_github_prs() does an in-function import that bypasses module-level patching (4 tests in TestCleanupNonFatal) - Skip _read_gitreview_host() call when configured_server is already present from the config file, avoiding unnecessary filesystem/network I/O on the common path in derive_gerrit_parameters() Signed-off-by: Matt Black <matt@modeseven.com>

Part of Copilot review feedback for PR lfreleng-actions#159: avoid unnecessary filesystem/network I/O when configured_server is already present. Signed-off-by: Matt Black <matt@modeseven.com>

- Fix test patch targets: use github2gerrit.gerrit_rest.build_client_for_host instead of github2gerrit.gerrit_pr_closer.build_client_for_host since cleanup_closed_github_prs() does an in-function import that bypasses module-level patching (4 tests in TestCleanupNonFatal) - Skip _read_gitreview_host() call when configured_server is already present from the config file, avoiding unnecessary filesystem/network I/O on the common path in derive_gerrit_parameters() Signed-off-by: Matt Black <matt@modeseven.com>

- Fix test patch targets: use github2gerrit.gerrit_rest.build_client_for_host instead of github2gerrit.gerrit_pr_closer.build_client_for_host since cleanup_closed_github_prs() does an in-function import that bypasses module-level patching (4 tests in TestCleanupNonFatal) - Skip _read_gitreview_host() call when configured_server is already present from the config file, avoiding unnecessary filesystem/network I/O on the common path in derive_gerrit_parameters() Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>

- Fix test patch targets: use github2gerrit.gerrit_rest.build_client_for_host instead of github2gerrit.gerrit_pr_closer.build_client_for_host since cleanup_closed_github_prs() does an in-function import that bypasses module-level patching (4 tests in TestCleanupNonFatal) - Skip _read_gitreview_host() call when configured_server is already present from the config file, avoiding unnecessary filesystem/network I/O on the common path in derive_gerrit_parameters() - Make _gitreview_host_re case-insensitive and allow optional whitespace around '=' to handle INI-style 'host = value' and 'Host = value' forms, consistent with extract_gerrit_info_from_gitreview() in ssh_discovery.py - Add regression tests for whitespace and case-insensitive host key parsing Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>

- Fix test patch targets: use github2gerrit.gerrit_rest.build_client_for_host instead of github2gerrit.gerrit_pr_closer.build_client_for_host since cleanup_closed_github_prs() does an in-function import that bypasses module-level patching (4 tests in TestCleanupNonFatal) - Skip _read_gitreview_host() call when configured_server is already present from the config file, avoiding unnecessary filesystem/network I/O on the common path in derive_gerrit_parameters() - Make _gitreview_host_re case-insensitive and allow optional whitespace around '=' to handle INI-style 'host = value' and 'Host = value' forms, consistent with extract_gerrit_info_from_gitreview() in ssh_discovery.py - Add regression tests for whitespace and case-insensitive host key parsing - Add exc_info=True to GerritRestError warning log in cleanup for diagnosability, consistent with the generic Exception handler below it - Replace hard-coded /tmp/fake workspace paths with tmp_path fixture in TestUpdateErrorMentionsCreateMissing for cross-platform portability - Fix basedpyright errors across core.py and ssh_discovery.py: - Dedent reconciliation block out of for-trailer loop (change_ids unbound) - Move args and collected_change_ids init before try block (unbound in except handler) - Move ssh_cmd init before try block (unbound in except handler) - Move cmd init before try block in ssh_discovery.py (unbound in except) - Suppress reportUnnecessaryIsInstance on defensive IPv6Address isinstance - Simplify ssh_cmd locals() guard now that variable is always initialised Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>

PyJWT 2.10.1 has a High severity vulnerability (GHSA-752w-5fwx-jx9f), fixed in 2.12.0. Update transitive dependency (via PyGithub) to 2.12.1 to resolve Grype SBOM scan failure. Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>

Copilot

Pull request overview

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

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

src/github2gerrit/config.py

ModeSevenIndustrialSolutions and others added 4 commits March 13, 2026 14:44

ModeSevenIndustrialSolutions self-assigned this Mar 13, 2026

Copilot AI review requested due to automatic review settings March 13, 2026 15:08

github-actions bot added the bug Something isn't working label Mar 13, 2026

Copilot started reviewing on behalf of ModeSevenIndustrialSolutions March 13, 2026 15:08 View session

Copilot AI reviewed Mar 13, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings March 13, 2026 15:24

Copilot started reviewing on behalf of ModeSevenIndustrialSolutions March 13, 2026 15:25 View session