fix(operator): force source-watcher drift on stuck ArtifactGenerator status - #3182

…re synthesising

Own-audit found the first pass had a spec-update race: if a user
edited PackageSource.spec.variants on a running cluster, cozystack
would bump the derived ArtifactGenerator to Generation=2, and
between "source-watcher started reconciling gen=2" and "source-
watcher wrote new Inventory + ObservedSourcesDigest for gen=2" the
ArtifactGenerator still exposes the previous generation's
Inventory / ObservedSourcesDigest AND Ready=Unknown (Progressing).
The workaround would synthesise Ready=True on stale artifacts, and
downstream Flux would consume the previous-generation
ExternalArtifacts as if the new spec had converged.

Tighten the predicate to require the Ready condition's
ObservedGeneration to match ag.Generation. Only when source-watcher
has actually processed the current spec (which happens strictly
after it writes the current-generation Inventory + Digest) does the
Ready condition carry ObservedGeneration == ag.Generation. That is
exactly the window where Inventory / Digest are guaranteed current-
spec AND only the status patch on Ready itself failed — the actual
upstream bug this workaround exists to paper over.

Consequence: if Ready is missing entirely we return false now
(was true). We cannot verify ObservedGeneration on an absent
condition, and the missing-Ready state is indistinguishable from
mid-reconcile-of-a-newer-generation. On the next reconcile source-
watcher will write at least a Progressing condition, at which point
the current-spec Generation check will be evaluable and the
workaround will kick in if — and only if — Inventory / Digest also
match the current spec.

Tests updated:
  * changed 'no Ready condition' case from want=true to want=false
    (reflecting the tighter invariant);
  * added a case where Ready=Unknown carries an OLD Generation
    (want=false — do not synthesise on stale artifacts);
  * stamped ObservedGeneration on every existing condition-bearing
    case so they exercise the matching-Generation branch;
  * introduced a small agWithGen helper to keep the fixtures
    readable now that Generation is load-bearing.

All 8 subtests pass; existing operator suite unaffected.

Signed-off-by: Ivan Okhotnikov <[email protected]>

… review nits

kvaps and lexfrei confirmed the diagnosis by tracing source-watcher
v2.1.0 (same code paths in v2.2.x / flux-aio latest, so a simple
version bump does not remove the need for this workaround):

  * fluxcd's patch.Helper patches .status and .status.conditions as
    SEPARATE apiserver requests, so an etcd timeout during first-
    install load can land between them — leaving Inventory /
    ObservedSourcesDigest persisted while the Ready-condition write
    is lost.
  * On the next reconcile the !hasDrifted branch returns before
    touching conditions. The deferred summarizeStatus still runs and
    adjusts the Reconciling condition, but it never promotes a
    stuck Unknown Ready to True — that promotion only happens on the
    successful hasDrifted path.

Two substantive changes prompted by the reviews:

  1. Document the semantic change the workaround introduces on
     PackageSource.status.conditions[Ready]. fluxcd/pkg/runtime/
     conditions.Set writes condition.ObservedGeneration =
     ag.Generation on EVERY condition mutation including the
     Progressing/Unknown mark at the start of a rebuild. And
     ArtifactGeneratorStatus does not expose an object-level
     ObservedGeneration (only ReconcileRequestStatus is inlined).
     So Generation-matching cannot distinguish 'current revision
     fully processed' from 'rebuild in progress, previous
     Inventory/digest still persisted' — meaning the predicate can
     (and does) fire during a content-only regeneration (new OCI
     revision, same .metadata.generation).

     Net effect: PackageSource Ready no longer dips through Unknown
     while a new revision is being processed; it now means 'valid
     consumable artifacts exist' rather than 'the current revision
     has been processed'. Kvaps flagged this as the better contract
     for install/upgrade ordering (no InProgress flapping, and a
     genuine regeneration failure still surfaces as Ready=False on
     the successful hasDrifted path). Document it inline instead of
     implying the workaround only fires in the stuck-status window.

  2. Reworked the predicate signature to take the pre-resolved
     Ready condition as a parameter. The caller was already doing
     the FindStatusCondition lookup for the pass-through path, so
     the helper no longer duplicates it (kvaps' nit).

Also refreshed the pre-existing comments:
  * The 'early-exits before touching status again' step is now
    correctly worded as 'returns before touching conditions again;
    the deferred summarizeStatus never promotes a stuck Unknown
    Ready to True'.
  * Bug attribution now names the code paths ('!hasDrifted branch',
    'summarizeStatus deferred'), notes that v2.1.0 and v2.2.x
    ('flux-aio latest') both ship the affected code, and carries a
    TODO(remove once upstream is fixed) marker so the workaround
    cannot silently outlive its cause.

Tests: existing 8 subtests updated to construct the ready condition
manually so the call matches the new signature, plus one new
subtest locking in the intentional 'content-only regeneration
predicate fires' behaviour so a future refactor that tightens the
predicate back to 'current revision processed' — which would
re-introduce the flapping — fails loudly. 9/9 pass.

Signed-off-by: Ivan Okhotnikov <[email protected]>

…issue)

Reviewer nit was to file an upstream issue so the workaround does
not silently outlive its cause. Checked first — fluxcd/pkg already
has one open: #934 \"patch.Helper creates race conditions in
downstream controllers\" (May 2025). Same root cause we traced:
patch.Helper writes .status and .status.conditions as separate
apiserver requests, so an etcd blip between the two leaves the
object half-persisted. The upstream issue is broader than just
source-watcher — every fluxcd downstream controller consuming
fluxcd/pkg's runtime patch helper is affected.

Update the TODO(remove) marker on our workaround to point at
that issue and note that both source-watcher v2.1.0 and v2.2.x
(flux-aio latest) pull the same fluxcd/pkg runtime, so bumping
source-watcher alone does not remove the need for this synthesis.
Compile + unit tests unchanged.

Signed-off-by: Ivan Okhotnikov <[email protected]>

…comment

review-branch skill flagged that the comment claimed
meta.SetStatusCondition rewrites LastTransitionTime on Status OR
Reason change. Actually only Status change triggers a
LastTransitionTime rewrite — Reason and Message are updated in
place, LastTransitionTime is preserved. Outcome (repeated
identical calls are no-op-y on persisted state) is unchanged;
just tightening the technical claim to match apimachinery.

Signed-off-by: Ivan Okhotnikov <[email protected]>

…of synthesising PackageSource Ready

Replaces the Ready=True synthesis for PackageSources whose ArtifactGenerator
hit the fluxcd/pkg#934 patch.Helper split-write race with a bounded retry
driver: on detected stall, bump reconcile.fluxcd.io/requestedAt with
exponential backoff (30s to 4m cap) up to 5 attempts, then surface as
PackageSource Ready=False with reason SourceWatcherStalled.

The previous approach could mask a genuine regeneration failure if the same
patch.Helper race that the workaround exists for also swallowed a real
Ready=False write, and it permanently reinterpreted PackageSource.Ready
semantics from "current revision processed" to "valid consumable artifacts
exist". Requeue keeps the CRD contract intact and, on stall exhaustion,
produces an honest failure signal an operator can act on instead of a
false-positive True.

Retry state (attempts + last-bump timestamp) lives on the AG as
cozystack.io/source-watcher-* annotations so it survives operator restarts.

Signed-off-by: Ivan Okhotnikov <[email protected]>

…ches, tighten status

Address self-review nits on the bounded-requeue driver:

* Sample `now := time.Now()` once at Reconcile entry and thread it into
  `updateStatus`, `artifactGeneratorStuck` and `maybeRequeueArtifactGenerator`
  so every time-sensitive check inside a reconcile agrees on the same
  wall-clock reading.
* Set `ObservedGeneration` in the three early Ready-condition branches
  (`SourceRefNotSet`, `ArtifactGeneratorNotFound`, `ArtifactGeneratorNotReady`)
  for parity with the retry-driver branches.
* Patch the ArtifactGenerator via a deep copy in
  `bumpArtifactGeneratorRequeue` and `clearRequeueTracking` so a failed Patch
  no longer leaves the caller's in-memory `ag` desynchronised from the
  apiserver.
* Document the retry driver's dependency on source-watcher's
  ReconcileRequestedPredicate on ArtifactGenerator, so a future upstream
  change dropping the predicate is easier to trace.

Signed-off-by: Ivan Okhotnikov <[email protected]>

…ct, document HA + timing rationale

Address second-pass self-review findings on the bounded-requeue driver:

* Skip reconcile early when the PackageSource has a deletionTimestamp so
  the retry driver stops writing "AwaitingSourceWatcherRequeue" onto an
  object about to be garbage collected. ownerReference cascade handles
  the ArtifactGenerator cleanup.
* Fix the mutation contract on bumpArtifactGeneratorRequeue and
  clearRequeueTracking: the previous deep-copy shape left the caller's
  `ag` desynchronised from the apiserver on success. Now mutations land
  on `ag` itself with a rollback to the pre-call annotation snapshot on
  Patch failure — so success reflects persisted state and failure does
  not leak unpersisted state to subsequent code paths.
* Convert the "unreachable" default in maybeRequeueArtifactGenerator to
  an explicit panic so a future requeueAction added without a matching
  case fails loudly instead of freezing the Ready condition silently.
* Document the timing constants' rationale (grace period matches the
  observed <10s source-watcher settle time on dev3; total ~11.5m budget
  keeps SourceWatcherStalled inside the 15m HR install timeout) and note
  the operator must run with leader election when replicas > 1 so the
  annotation-based retry state does not race.

Signed-off-by: Ivan Okhotnikov <[email protected]>

…lper for the requeue driver

Address the third-pass self-review's blocker: the mutation contract on
bumpArtifactGeneratorRequeue / clearRequeueTracking, the DeletionTimestamp
early-return in Reconcile, and the nil-preserving cloneAnnotations helper
had no regression tests, so a future refactor could silently break the
contracts documented in comments.

New fake-client + interceptor-based tests:

* TestCloneAnnotations — pins the "nil in / nil out" sentinel behaviour
  and mutation-independence of the shallow copy.
* TestBumpArtifactGeneratorRequeue_Success — asserts the persisted state
  matches the caller's in-memory `ag.Annotations` on success, including
  that reconcile.fluxcd.io/requestedAt is actually written (previously
  never asserted anywhere).
* TestBumpArtifactGeneratorRequeue_PatchFailure_Rollback and its
  PreservesUnrelated variant — drive the Patch-failure branch via an
  interceptor client and assert both that our tracking annotations are
  rolled back and that pre-existing unrelated annotations survive.
* TestClearRequeueTracking_Noop — asserts we don't issue a no-op Patch on
  every healthy reconcile when there are no tracking annotations to clear.
* TestClearRequeueTracking_PatchFailure_Rollback — same rollback contract
  as bump, on the clear path.
* TestReconcile_DeletionTimestamp_EarlyReturn — asserts a PackageSource
  with a deletionTimestamp does not receive any writes and does not
  schedule a RequeueAfter.

Doc tightening in the same commit:

* Reword clearRequeueTracking's comment on the flux annotation semantics —
  standard flux controllers do not clear reconcile.fluxcd.io/requestedAt,
  they only advance .status.lastHandledReconcileAt via the
  ReconcileRequestedPredicate.
* Note controller-runtime v0.15+'s default RecoverPanic behaviour where
  the retry-driver's unhandled-action panic is raised, so a future
  SetupWithManager flip that disables it is easier to spot.
* Explain why cloneAnnotations' shallow copy is sound (annotation values
  are immutable strings).

Signed-off-by: Ivan Okhotnikov <[email protected]>

Address the fourth-pass self-review's non-blocking NITs:

* Add TestBumpArtifactGeneratorRequeue_PatchFailure_RollbackRestoresPriorBump
  to cover the subtle case the two existing rollback tests miss: when the
  tracking annotations were already set from a prior bump, a failed Patch
  must restore the PRIOR values, not delete the keys. A refactor that
  naively swapped rollback for `delete(ag.Annotations, key)` would pass
  the nil-rollback tests but silently regress this contract.
* Rename the cloneAnnotations subtest from "deep copy" to "independent
  copy" — the code documents shallow copy semantics and the misnomer was
  the only thing pointing readers at the wrong mental model.
* Hoist `now.UTC().Format(time.RFC3339Nano)` to a local so the two
  timestamp annotations use one formatted value instead of two identical
  Format calls.

Signed-off-by: Ivan Okhotnikov <[email protected]>

…ue bump

Rework the fluxcd/pkg#934 recovery driver after myasnikovdaniil's B1
finding: bumping reconcile.fluxcd.io/requestedAt cannot recover a stuck
ArtifactGenerator because source-watcher v2.1.0's reconcile is
drift-gated. detectDrift (artifactgenerator_drift.go:52) returns
NoDriftDetected for exactly the stuck signature we target — Inventory
populated, ObservedSourcesDigest set, Ready=Unknown on the current
generation — so the ReconcileRequestedPredicate fires a reconcile that
lands on the no-drift early-return at controller.go:164 and MarkTrue at
:254 is never reached. Every attempt in the previous shape was a no-op.

New mechanism: force drift by patching Ready=False on the AG's status
subresource. detectDrift picks this up as NotReady drift, source-watcher
takes the drifted branch, and either MarkTrue (on healthy rebuild) or
MarkFalse with a real reason (on genuine failure) replaces our synthetic
False. Owns() watch fires this reconciler to copy the resolved condition
through. Same bounded-retry harness (30s / 60s / 2m / 4m / 4m budget
inside the 15m HR install timeout, SourceWatcherStalled surfaced on
exhaustion), same annotation-based state on the AG, same rollback
contract on Patch failure — only the action itself changed.

Additional fixes bundled in:

* Reset attempt counter when the AG's Ready.LastTransitionTime is newer
  than our lastRecoveryAt, so a fresh stall episode after a previous
  give-up gets a fresh budget rather than an immediate re-give-up
  (myasnikovdaniil's follow-up #3).
* Propagate updateStatus errors from Reconcile instead of swallowing them,
  so controller-runtime's exponential backoff retries transient failures
  on force-drift / status-update writes rather than dropping them silently
  (coderabbit's inline finding).
* forceArtifactGeneratorDrift documents the two-subresource shape (status
  patch first, metadata patch second) and the partial-failure semantics.
* New tests cover the status-patch success / status-patch failure /
  metadata-patch failure paths, all with fake client + interceptor. The
  successful case asserts Ready=False actually lands in the apiserver via
  WithStatusSubresource.
* All annotation / action / decision / helper names renamed from
  *Requeue* to *Recovery* to match the new semantics.

Signed-off-by: Ivan Okhotnikov <[email protected]>

…ait assertions, polish comments

Address the fourth-pass self-review's non-blocking findings:

* Add TestMaybeRecoverArtifactGenerator_{ForceBranch, WaitBranch,
  GiveUpBranch, ResetsCounterOnFreshTransition}. The three switch
  outcomes of the driver (force / wait / give-up) and the reset-counter
  guard on a fresh Ready.LastTransitionTime transition were previously
  only covered indirectly through their pure-function components. Now
  each outcome is asserted end-to-end against a fake client with a
  status subresource, verifying the Ready condition, tracking
  annotations, RequeueAfter, and — for the wait branch — that no AG
  writes are issued at all.
* Tighten TestDecideRecovery's wait-branch assertions from a loose lower
  bound to exact equality. decideRecovery is deterministic
  (needed - elapsed) so an exact assertion catches subtle backoff
  regressions a lower bound would miss (coderabbit's inline nit).
* Rename the local var `wantWaitMin` to `wantWait` for the tighter
  assertion shape.
* Fix two stale identifier references in comments
  (maybeRequeueArtifactGenerator, AwaitingSourceWatcherRequeue) left
  over from the rename pass.
* Add a docstring paragraph on forceArtifactGeneratorDrift explaining
  why the JSON-merge-patch full-array-replace on
  `.status.conditions[]` is net-neutral in this workaround's context.

Signed-off-by: Ivan Okhotnikov <[email protected]>

…hin grace

E2E on dev3 caught a dormancy bug that renders the whole recovery driver
inactive in practice: when updateStatus copies Ready=Unknown across but
the AG's LastTransitionTime is still within stuckGracePeriod,
artifactGeneratorStuck returns false and we fall through to the copy
path with no RequeueAfter set. If the reconcile happens to fire in the
[stuck_start, stuck_start+30s) window — a routine case since operator
restarts touch every PackageSource — the controller then sleeps
waiting for an AG-status change that will never come (that IS the
fluxcd/pkg#934 lost-write race), up to the 10h informer resync.

Observed on dev3: `cozystack.networking` stuck at Ready=Unknown for 9+
minutes with zero recovery attempts. Two reconciles fired at
stuck_start+23s and +24s — both correctly detected "not yet stuck" —
and then nothing until observation ended. Upstream OCIRepository
revision was unchanged throughout, ruling out a coincidental source
update as the cause.

Fix: whenever we copy Ready=Unknown to the PackageSource (or set a
synthetic Unknown because no Ready condition exists yet), compute the
remaining grace period against the AG's LastTransitionTime (or
CreationTimestamp for the missing-Ready branch) and schedule a
RequeueAfter at that expiry. A 1s floor prevents a zero-delay tight
loop when the anchor is right at or past grace expiry.

Extracted the delay computation to `agFollowUpDelay` for direct unit
testing. Adds three tests: the follow-up scheduling on Ready=Unknown
in-grace, the same on the missing-Ready branch, and a table test on
agFollowUpDelay covering the floor edge cases.

Signed-off-by: Ivan Okhotnikov <[email protected]>

…remove reset-counter loop

Address lexfrei's three blockers on the force-drift approach — as
committed, the recovery driver did not function on a live cluster.

**B1**: `forceArtifactGeneratorDrift` wrote `Ready=False` on the AG's
status subresource but did not touch its metadata annotations.
source-watcher v2.1.0 registers its ArtifactGenerator watch with
`predicate.Or(GenerationChangedPredicate, ReconcileRequestedPredicate)`
— a status-only patch changes neither the generation nor
`reconcile.fluxcd.io/requestedAt`, so it does NOT enqueue a
source-watcher reconcile. The AG's periodic self-requeue is hardcoded
to `time.Hour`, well outside the 15m HR install timeout. Fix: also
stamp `reconcile.fluxcd.io/requestedAt` in the metadata patch so the
ReconcileRequestedPredicate fires and source-watcher's next reconcile
picks up the `IsFalse(Ready)` drift signal.

**B2**: `Owns(&ArtifactGenerator{})` re-fires this reconciler on the
force-drift's own writes. Both the status patch (Ready=False, reason
`SourceWatcherRecoveryForced`) and the metadata patch enqueue a
PackageSource reconcile. Blindly running `clearRecoveryTracking` on
those re-entrant fires nuked the attempts counter — the give-up
budget was unreachable, and the synthetic marker leaked to
`PackageSource.status.conditions[Ready]`. Fix: in `updateStatus`,
detect our own reason and route into a new `awaitSourceWatcherResponse`
helper that holds tracking and reports Unknown /
AwaitingSourceWatcherRecovery on the PS. The same helper covers the
Progressing/Unknown write source-watcher issues during rebuild once
tracking is present (attempts > 0) — otherwise the transient
Progressing would trigger the same premature tracking clear.

**B3**: The attempt-counter reset keyed on
`Ready.LastTransitionTime > lastRecoveryAt`. That inequality is true
on every source-watcher response during recovery (the Progressing
write is always newer than our force), so under the very race this
driver targets the counter would reset every cycle and
`recoveryActionGiveUp` was unreachable. Fix: remove the reset. The
canonical counter reset is the natural `clearRecoveryTracking` on the
not-stuck path (Ready=True from source-watcher, or a real upstream
Ready=False), and the HA/leader-election requirement already
contains the "missed recovery event on operator restart" edge case
the reset was trying to paper over.

Extract `reasonRecoveryForced` to a named constant so both the
writer (forceArtifactGeneratorDrift) and the reader (updateStatus'
own-marker filter) use the same symbol.

New tests pin all three fixes:
- ForceArtifactGeneratorDrift_Success now asserts the
  reconcile.fluxcd.io/requestedAt annotation is set (B1).
- UpdateStatus_OwnMarkerRoutesToAwait asserts we do NOT clear
  tracking and do NOT copy the synthetic Ready=False through when we
  see our own marker (B2).
- UpdateStatus_ProgressingDuringRecoveryRoutesToAwait covers the
  second Owns() re-entrancy path — source-watcher's Progressing
  write during rebuild (B2).
- UpdateStatus_ReadyTrueClearsTracking pins that the natural not-
  stuck path IS the canonical counter reset after B3.
- The old ResetsCounterOnFreshTransition test is replaced with
  DoesNotResetCounterOnFreshTransition, asserting the opposite
  behaviour that closes B3.

Signed-off-by: Ivan Okhotnikov <[email protected]>

…rce-watcher-unresponsive dormancy loop

Self-review on 467b788 caught a MAJOR bug introduced by the separate
awaitSourceWatcherResponse helper: if source-watcher never responds to
a forced drift (network partition, missing predicate, dead pod), our
own reasonRecoveryForced marker keeps reflecting back via Owns() forever,
and the wait-forever helper schedules a 1-second requeue tight loop
without ever advancing the retry state machine. SourceWatcherStalled
give-up was unreachable.

Fix: remove awaitSourceWatcherResponse and unify all three recovery
entry conditions — our own marker, source-watcher's Progressing during
rebuild, and the classic first-time stuck detection — through a single
gate that routes to maybeRecoverArtifactGenerator. The state machine's
decideRecovery is the single arbiter of wait/force/give-up, and its
Wait branch is bounded by the same backoff schedule as Force. When
backoff elapses on a still-reflecting own-marker the machine escalates
to another force (bumps attempts, re-stamps requestedAt); when attempts
exhaust it surfaces SourceWatcherStalled honestly.

The own-marker filter also no longer requires attempts>0, so an
operator crash between step 1 (status patch) and step 2 (metadata
patch) of forceArtifactGeneratorDrift is still routed through the
state machine on the next reconcile rather than leaking the synthetic
marker onto the PackageSource.

Also address the non-blocking notes from the same self-review:

* Docstring on maybeRecoverArtifactGenerator was still describing the
  removed reset-counter branch and the "requestedAt alone is a no-op"
  framing. Rewrite to explain the current shape: status write triggers
  the drift signal, requestedAt bump triggers the reconcile event —
  both are required, neither alone works.
* TestForceArtifactGeneratorDrift_MetadataPatchFailure_RollbackAnnotations
  now asserts reconcile.fluxcd.io/requestedAt is also rolled back on
  the metadata-patch failure branch (symmetric with the tracking
  annotations).
* TestUpdateStatus_OwnMarkerRoutesToAwait asserts an exact 25s
  RequeueAfter (backoffFor(1)=30s minus 5s elapsed) so a silent
  regression in the backoff arithmetic is caught.

New regression tests for the source-watcher-unresponsive path:

* TestUpdateStatus_OwnMarkerStale_TriggersRetry: with attempts=1 and
  lastRecoveryAt older than backoffFor(1), the reconcile must
  escalate through the Force branch and bump attempts to 2 rather
  than idle in a tight requeue loop.
* TestUpdateStatus_OwnMarkerAttemptsExhausted_GivesUp: with
  attempts=maxRecoveryAttempts and the marker still reflecting back,
  the reconcile must set PackageSource Ready=False /
  SourceWatcherStalled instead of extending the loop indefinitely.

Signed-off-by: Ivan Okhotnikov <[email protected]>

…m comments and test names

Self-review after the previous commit removed the awaitSourceWatcherResponse
helper (routing unified through maybeRecoverArtifactGenerator's state machine
via a single own-marker / progressing-in-recovery / stuck gate). Four
comments and two test names still referenced the now-deleted symbol —
misleading anyone tracing the architecture.

* maybeRecoverArtifactGenerator docstring: rewrote the trade-off paragraph
  to describe the unified gate and Wait branch instead of the removed
  helper.
* TestMaybeRecoverArtifactGenerator_DoesNotResetCounterOnFreshTransition
  comment: same rewrite.
* TestUpdateStatus_OwnMarkerRoutesToAwait renamed to
  TestUpdateStatus_OwnMarkerHoldsTrackingViaStateMachine + comment
  rewritten to describe the state-machine Wait branch.
* TestUpdateStatus_ProgressingDuringRecoveryRoutesToAwait renamed to
  TestUpdateStatus_ProgressingDuringRecoveryHoldsTracking + comment
  rewritten in parallel.

Pure doc / naming; no code or test-body changes. Tests still pass with
-race, vet clean, golangci-lint 0 issues.

Signed-off-by: Ivan Okhotnikov <[email protected]>

…OG check, warn on partial force-drift

Self-review nits raised in a skeptical pass over the current tip:

* The status-patch docstring in forceArtifactGeneratorDrift described
  the JSON-merge-patch full-array-replace on `status.conditions[]` as
  "net-neutral". That was too optimistic: if source-watcher writes a
  concurrent Reconciling condition between our Get and our Patch, our
  patch does transiently wipe it. source-watcher restores it on its
  next reconcile (which our requestedAt bump is about to trigger), but
  downstream tooling watching for Reconciling can observe the momentary
  absence. Rewrite the comment to name that trade-off explicitly and
  point at server-side apply as the fuller fix if the operator ever
  migrates to SSA globally.

* The unified own-marker/progressing/stuck gate in updateStatus is
  asymmetric on ObservedGeneration — isProgressingInRecovery checks it,
  isOwnMarker does not. That asymmetry is deliberate (a stale own-marker
  after a spec edit must still route to the state machine to avoid
  leaking the synthetic reason to the PackageSource — exactly the B2
  regression), not an oversight. Add a comment explaining the
  reasoning so a future edit doesn't "symmetrise" the check and re-open
  B2.

* forceArtifactGeneratorDrift's step-2 (metadata patch) failure was
  silent when it happened after a successful step-1 status patch. In
  that state source-watcher has been nudged, but the tracking
  annotations never persisted, so the next reconcile reads attempts=0
  and re-forces from scratch instead of accumulating toward
  SourceWatcherStalled — a persistent metadata-write-only apiserver
  malfunction would loop forever. Log the failure loudly with the
  intended attempt count so operators can spot this specific mode
  from cozystack-operator logs.

Signed-off-by: Ivan Okhotnikov <[email protected]>

…m version skew

Address lexfrei's non-blocking follow-ups on the LGTM approval:

Follow-up #1 — give-up preempted the final backoff. `decideRecovery`
returned `recoveryActionGiveUp` as soon as `attempts >= maxRecoveryAttempts`,
without checking whether the final-force backoff window had elapsed. The
Owns()-watch re-fire from the N-th force-drift's own writes triggered a
reconcile milliseconds after the force, so the effective time-to-give-up
was ~7.5m (30s+60s+2m+4m) — not the ~11.5m documented on the constants
block. The final 4-minute response window the last nudge was supposed to
grant source-watcher was never actually granted.

Fix: extend the exhaustion branch to hold `Wait` until
`now - lastRecoveryAt >= backoffFor(maxRecoveryAttempts)` before switching
to GiveUp. Now the documented ~11.5m budget matches the observed
behaviour, and the last force gets a real chance to succeed (Owns
watch → source-watcher Ready=True copy-through) before we surface
SourceWatcherStalled.

Tests:
* `TestDecideRecovery` gains a case pinning `Wait` when
  attempts=maxRecoveryAttempts but the final backoff has not yet
  elapsed (the exact re-fire shape the previous behaviour tripped on).
* `TestUpdateStatus_OwnMarkerAttemptsExhaustedButFinalBackoffPending`
  drives the full updateStatus path against the same shape, asserting
  the PackageSource stays in Unknown/AwaitingSourceWatcherRecovery and
  the RequeueAfter equals the remaining backoff, NOT zero (GiveUp).

Follow-up #3 — upstream version-skew note. The Go module dependency
tracks `fluxcd/source-watcher/api/v2 v2.0.3` (CRD types package only),
but the deployed binary this workaround targets is pinned to v2.1.0
at `internal/fluxinstall/manifests/fluxcd.yaml`. Comments referencing
`drift.go:52` / `controller.go:164/254` describe the deployed binary,
not the compiled API package. Documented on the constants block so a
future flux bump re-verifies the anchors AND the watch predicate /
drift-detection heuristics, which are the load-bearing upstream
assumptions.

Follow-up #2 (envtest over full Reconcile→Owns→updateStatus cycle) is
architectural and out of scope for this PR — noted for a follow-up.

Signed-off-by: Ivan Okhotnikov <[email protected]>