Summary
Found by Copilot while reviewing #447 (against code from #446).
Early parameter derivation exports GERRIT_SERVER from .gitreview but not
GERRIT_SERVER_PORT. The closed-PR dispatch runs before the orchestrator,
and the SSH abandon path (gerrit_pr_closer._abandon_gerrit_change) reads
the ambient GERRIT_SERVER_PORT, so a repository whose .gitreview says
port=29420 is contacted on 29418 during cleanup even though the pipeline
itself pushes on 29420.
Why exporting the port from derivation is not enough on its own
The composite action passes GERRIT_SERVER_PORT: ${{ inputs.GERRIT_SERVER_PORT }}
with a default of "29418", so the environment variable is always non-blank
in Actions and apply_config_to_env never writes a derived value over it.
This is the provenance gap #444 recorded: an integer port cannot say whether
the operator set it or the default applied. #446 documented the pairing rule
(port travels with the host) for the orchestrator; this issue is about
making the early cleanup path honour the same rule.
Fix
Either:
- Stop defaulting
GERRIT_SERVER_PORT in action.yaml / the reusable
workflow (blank means "not set"), have derivation export the .gitreview
port alongside the host with the same replacement/provenance handling as
GERRIT_SERVER, and let Inputs default to 29418 only when nothing
supplied one; or
- Have the cleanup paths read the port from the resolved
GerritInfo
(host + port as one target) rather than from the ambient environment.
Option 2 keeps the fix inside the tool; option 1 also closes #444's option 2.
Acceptance criteria
Summary
Found by Copilot while reviewing #447 (against code from #446).
Early parameter derivation exports
GERRIT_SERVERfrom.gitreviewbut notGERRIT_SERVER_PORT. The closed-PR dispatch runs before the orchestrator,and the SSH abandon path (
gerrit_pr_closer._abandon_gerrit_change) readsthe ambient
GERRIT_SERVER_PORT, so a repository whose.gitreviewsaysport=29420is contacted on 29418 during cleanup even though the pipelineitself pushes on 29420.
Why exporting the port from derivation is not enough on its own
The composite action passes
GERRIT_SERVER_PORT: ${{ inputs.GERRIT_SERVER_PORT }}with a default of
"29418", so the environment variable is always non-blankin Actions and
apply_config_to_envnever writes a derived value over it.This is the provenance gap #444 recorded: an integer port cannot say whether
the operator set it or the default applied. #446 documented the pairing rule
(port travels with the host) for the orchestrator; this issue is about
making the early cleanup path honour the same rule.
Fix
Either:
GERRIT_SERVER_PORTinaction.yaml/ the reusableworkflow (blank means "not set"), have derivation export the
.gitreviewport alongside the host with the same replacement/provenance handling as
GERRIT_SERVER, and letInputsdefault to 29418 only when nothingsupplied one; or
GerritInfo(host + port as one target) rather than from the ambient environment.
Option 2 keeps the fix inside the tool; option 1 also closes #444's option 2.
Acceptance criteria
.gitreviewwith a non-default portconnects on that port