fix(foreman): add payload.branchStrategy to stop stale revisions reverting merged work

[joryirving]

What

Add an explicit payload.branchStrategy (enum reset | rebase, default reset) that controls how an issue-fix task's working branch is cut relative to the current base, so a re-dispatch of the same issue can never revert already-merged work.

Why

Fixes #1029

A re-dispatch of the same issue (a review-feedback revision, or the slicer's repair re-dispatch) restored the prior attempt's branch via reviseFromBranch but never rebased it onto the current base. Its diff was computed against the old base, so opening/refreshing the PR reverted or deleted work merged since — the stale-branch overreach from the issue.

How

• reset (default): cut the working branch fresh from the current base, ignoring any prior attempt. A fresh issue-fix, retry, or repair re-dispatch redoes the work against latest base — a stale branch can never drift from base and revert merged commits.
• rebase: restore the prior attempt and rebase it onto the current base (new repo.RebaseOntoBase; a conflict fails loud rather than reverting merged work), so an in-review PR revision carries its earlier commits forward on top of merged work.
• The WorkloadReconciler's review-feedback re-dispatch is the in-review revision path, so it now stamps branchStrategy=rebase; every other issue-fix defaults to reset. This maps onto the reset=fresh/retry, rebase=in-review-revision split from the issue thread — a repair re-dispatch sets reset.
• Safety (per the force-push concern in the thread): branch ops only ever touch foreman/<workload>/issue-N task branches (never human branches), and the push stays force-with-lease (allowOverwrite) as the compare-and-swap backstop.

CRD regenerated for both AgenticTask and Workload (pipeline steps embed the payload).

Checklist

• Tests added/updated — repo.RebaseOntoBase (prior attempt replayed onto an advanced base preserves the merged file + carries the prior fix forward; empty-upstream no-op); setupTaskBranch (rebase restores+rebases, reset ignores reviseFromBranch and cuts from base, missing-ref falls back)
• make test passes locally — full suite green (controller envtest included) and in CI (Run on Ubuntu ✅)
• make lint passes locally (0 issues)
• Commit messages follow conventional commits
• All commits are signed off (git commit -s) per DCO
• AI assistance disclosed — AI-assisted contribution
• Documentation updated — N/A (internal payload field; self-documented via the CRD description + Go doc comments)

[Defilan]

Really solid, Jory, and it lines up exactly with the #1029 thread. Verified locally: RebaseOntoBase is safe (aborts + fails loud on conflict, no-op on empty upstream, injection guards consistent with the sibling helpers), the reset/rebase dispatch is correct, and backward-compat is clean (the only reviseFromBranch setter now stamps rebase, everything else defaults to reset). go build + go test ./pkg/foreman/agent/... ./api/... are green, and the real-git RebaseOntoBase tests confirm a replay onto an advanced base preserves merged work and carries the prior fix forward. Nice that it sets up the slicer repair re-dispatch to use reset for free.

One thing before it goes green: the chart CRDs are out of sync. branchStrategy landed in config/crd/bases/ but not charts/foreman/templates/crds/ — running make chart-crds && make foreman-chart-crds produces drift on both agentictasks and workloads, so the CRD Sync Check will fail. Run those and commit the chart CRD changes.

Heads up too: my #1037 adds the integrate/reconcile task kinds to the same two CRDs + agentictask_types.go, so whichever of us merges second will need a trivial rebase.

[joryirving]

Thanks for the careful review. Fixed the chart CRD drift in d5076c4 — ran make chart-crds foreman-chart-crds and committed the synced charts/foreman/templates/crds/{agentictasks,workloads}.yaml (branchStrategy is now in both), so the CRD Sync Check should be clean. Also confirmed the full make test (controller envtest included) passes locally, not just the unit suites.

Ack on #1037 — I'll take the trivial rebase on agentictask_types.go + the two CRDs if you merge that first.

[codecov]

Codecov Report

❌ Patch coverage is 58.53659% with 17 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
pkg/foreman/agent/repo/branch.go	40.00%	8 Missing and 7 partials ⚠️
pkg/foreman/agent/executor_native.go	86.66%	1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[joryirving]

Heads-up on the one red check — Run on Ubuntu (MicroShift via MINC, OpenShift SCC): it's an unrelated flake, not this change. It failed at Manager > OpenShift SCC admission > should be admitted on a restricted-v2 namespace (e2e_test.go:2103) because the namespace came back without openshift.io/sa.scc.supplemental-groups — the SCC controller hadn't injected it yet (ran 1 of 48 specs, 6s). The E2E Tests workflow run itself concluded success (the job is continue-on-error), and it was green on the previous commit with byte-identical Go; my only new commit is the chart-CRD yaml sync. I don't have rerun perms here — feel free to re-run that job if you want a clean board before merging.