feat(foreman): open the pull request on review GO

[joryirving]

What

A reviewer GO now opens the Workload's pull request — idempotently — instead of leaving a reviewed branch unopened on the remote. New githubpr package (mirrors githubissue); Workload.spec.openPullRequest gates it (nil = true in issue-batch mode, false opts out; explicit pipelines set payload.openPullRequest per step).

Why

Fixes #937

The pipeline ended one hop short of its artifact: coder GO → gate PASS → review GO → …a branch sitting on the remote until a human noticed. We have hand-opened 12 PRs for completed Workloads in production since filing #937.

How

• githubpr.EnsurePR: list-by-head first (any state → reuse), then create; a 422 create race (two reviewer tasks GOing concurrently) resolves to the winner's PR. Title = branch head's commit subject (fallback Fix #<n>), base = payload.baseBranch (default main), body = Fixes #<n> + workload provenance.
• Executor: after the reviewer enforcement passes settle a GO, maybeOpenPullRequest runs best-effort with the same token the coder pushed with — the approval stands if GitHub is down; pullRequestURL/pullRequestError surface in result extra.
• Operator: synthesizeIssueBatch stamps payload.openPullRequest on review steps from the spec field (three-valued *bool, default true — an issue-batch Workload exists to produce a PR). CRDs + chart CRDs regenerated.
• PREnsurer is injected (like IssueFetcher); nil disables the feature entirely.

Follow-up candidates kept out of scope: draft PR on NO-GO carrying review findings (noted in #937), GHES base-URL config.

AI assistance: authored by Claude (Opus 4.8) operating under my direction; I reviewed the change and test output before submitting.

Checklist

• Tests added/updated (httptest client suite: create / reuse / 422 race / auth failure / title fallback; envtest: default-true, opt-out, review-step-only stamping)
• make test passes locally (agent + controller suites green)
• make lint passes locally (golangci-lint run 0 issues)
• Commit messages follow conventional commits
• All commits are signed off (git commit -s) per DCO
• AI assistance (if any) is disclosed above, per CONTRIBUTING.md
• Documentation updated (if user-facing change) — new fields documented in CRD/API comments

[codecov]

Codecov Report

❌ Patch coverage is 62.58503% with 55 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
pkg/foreman/agent/githubpr/ensure.go	74.52%	16 Missing and 11 partials ⚠️
pkg/foreman/agent/executor_native.go	21.21%	26 Missing ⚠️
cmd/foreman-agent/main.go	0.00%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[Defilan]

Review

Request changes — architecturally sound and safe (degrades to a logged error, never loses a branch), but as written it does not open a PR in our fork workflow, which is the mode the fleet actually runs.

Fork-head bug (blocking): openPullRequest derives the PR head from payload.Repo and passes the bare branch name (pkg/foreman/agent/executor_native.go:1180-1197; EnsurePR head filter owner+":"+head at pkg/foreman/agent/githubpr/ensure.go:129, create at :141). But the coder pushes the branch to the fork — the in-cluster agent runs --git-remote-url=https://github.com/Defilan/LLMKube.git while payload.Repo is the upstream defilantech/LLMKube. So create POSTs to defilantech/LLMKube/pulls with an unqualified head:foreman/issue-N that doesn't exist in the base repo → 422, isAlreadyExists is false → pullRequestError, no PR opened. findByHead and HeadCommitSubject hit the wrong owner too. A cross-fork PR needs head:"Defilan:foreman/issue-N" with base repo = upstream; the code can't express that (it never consults GitRemoteURL/the fork owner). It works in same-repo/#915 push mode, so this is "non-functional in the documented primary workflow," not "corrupts state."

Test gaps: ensure_test.go only uses same-repo heads (branch host == base owner), so the fork case sails through green; and the executor wiring (maybeOpenPullRequest, the GO/Review/flag gating + the fork-owner argument) is untested — a fake githubpr.Ensurer injected into the executor would have caught the above directly.

Nice-to-haves: up to 3 sequential GitHub calls (~30s worst case) run synchronously in the verdict path; findByHead takes prs[0] from a state=all list without an open-first ordering guarantee. CRD/chart/deepcopy sync, the three-valued *bool default, and the nil-Ensurer kill switch all verified clean.