# DD-CONTROLLER-001: ObservedGeneration Idempotency Pattern

**Status**: ✅ Approved
**Version**: 4.0
**Date**: March 4, 2026
**Authors**: Kubernaut Team
**Supersedes**: DD-CONTROLLER-001 v3.0 (Pattern A + Pattern B + Pattern C)
**Previous Versions**:
- v3.0 (January 11, 2026): Added Pattern C (Phase Transition Idempotency)
- v2.0 (January 2, 2026): Added Pattern B (Phase-Aware)
- v1.0 (December 2025): Pattern A (Simple) only

---

## Context

Kubernetes controllers are triggered by multiple event types:
- **Spec changes**: User modifies CR (Generation increments)
- **Status updates**: Controller or child CRD updates status (Generation unchanged)
- **Annotation/label changes**: Metadata modifications (Generation unchanged)
- **Requeue timers**: Polling mechanisms (Generation unchanged)
- **Watch events**: Child CRD status changes (Generation unchanged)

Without proper idempotency, controllers execute wasteful reconciles, emit duplicate audit events, and perform redundant work when only metadata changes.

**Problem**: How do we prevent wasteful reconciles while ensuring critical orchestration events are processed?

---

## Decision

Implement **Phase-Aware ObservedGeneration Pattern** with controller-specific strategies:

### **Pattern A: Simple Controllers** (No Child Orchestration)
For controllers managing a single CRD lifecycle without orchestrating child resources.

**Example**: AIAnalysis, SignalProcessing

**Implementation**:
```go
// Simple check at start of Reconcile()
if obj.Status.ObservedGeneration == obj.Generation &&
    obj.Status.Phase != "" &&
    !IsTerminal(obj.Status.Phase) {
    logger.V(1).Info("⏭️  SKIPPED: Generation already processed")
    return ctrl.Result{}, nil
}
```

**Rule**: Set `ObservedGeneration` only AFTER completing phase work, never during initialization.

---

### **Pattern B: Parent Controllers** (Active Child Orchestration)
For controllers orchestrating child CRDs with multi-phase state machines.

**Example**: RemediationOrchestrator

**Implementation**:
```go
// Phase-aware check at start of Reconcile()
if rr.Status.ObservedGeneration == rr.Generation &&
    (rr.Status.OverallPhase == phase.Pending ||         // Pre-orchestration
     phase.IsTerminal(rr.Status.OverallPhase)) {        // Post-orchestration
    logger.V(1).Info("⏭️  SKIPPED: No orchestration needed in this phase",
        "phase", rr.Status.OverallPhase)
    return ctrl.Result{}, nil
}

// Log when proceeding during active orchestration
if rr.Status.ObservedGeneration == rr.Generation &&
    rr.Status.OverallPhase != "" &&
    !phase.IsTerminal(rr.Status.OverallPhase) {
    logger.V(1).Info("✅ PROCEEDING: Active orchestration phase",
        "phase", rr.Status.OverallPhase,
        "reason": "Child orchestration requires reconciliation")
}
```

**Rule**: Allow reconciles during active orchestration phases (Processing/Analyzing/Executing) to process:
- Child CRD status updates
- Polling checks (RequeueAfter)
- Watch events

> **⚠️ CAVEAT (v4.0): Pending-phase `RequeueAfter` incompatibility**
>
> Pattern B assumes Pending phase has no active work and is safe to skip.
> This assumption is **invalid** for controllers that use `RequeueAfter`
> during Pending (e.g., cooldown enforcement). If a Pending WFE is blocked
> by cooldown and the controller returns `RequeueAfter: remaining`, the
> ObservedGeneration guard will kill the follow-up reconcile because
> `ObservedGeneration == Generation` was stamped by `PhaseManager.TransitionTo`
> during the cooldown block. The WFE is then stuck in Pending permanently.
>
> **Rule**: Controllers that issue `RequeueAfter` during Pending phase
> **MUST NOT** include Pending in the skip condition. See Pattern B-WE below.

### **Pattern B-WE: WorkflowExecution** (Pending-Phase Requeues)

WorkflowExecution uses `RequeueAfter` during Pending phase for cooldown
enforcement (BR-WE-009). When a second WFE targets a resource that recently
completed remediation, the controller blocks execution and requeues after
the cooldown expires. This requires Pending-phase reconciles to proceed.

**Example**: WorkflowExecution

**Implementation**: WorkflowExecution does **not** apply the Pending-phase
ObservedGeneration skip. `GenerationChangedPredicate` already filters
status-only watch events, so duplicate reconciles from status updates
are prevented at the controller setup level. The only way a Pending WFE
is re-reconciled is via `RequeueAfter` (cooldown retry, intentional) or
a spec change (bumps Generation, intentional).

**Execution Resource Watches**: The controller watches both `PipelineRun`
(Tekton engine) and `Job` (Job engine) resources via the same label-based
mapper (`FindWFEForOwnedResource`). Both executors label their resources
with `kubernaut.ai/workflow-execution` and `kubernaut.ai/source-namespace`,
enabling immediate reconciliation when an execution resource's status
changes (e.g., Job completion). The `RequeueAfter: 10s` polling in
`reconcileRunning` remains as a safety-net fallback. The Ansible engine
uses AWX REST API polling exclusively (no K8s resources to watch).

**Why Pattern B's Pending skip failed for WE (#374, #375)**:
1. WFE2 created for same target as recently-completed WFE1
2. First reconcile: `CheckCooldownActive` blocks → `TransitionTo(Pending)` stamps `ObservedGeneration` → returns `RequeueAfter: remaining`
3. After cooldown: requeue fires, but `ObservedGeneration == Generation && Phase == Pending` → guard skips reconcile entirely
4. No further requeue issued → WFE2 stuck in Pending permanently

**Fix**: Removed the Pending-phase ObservedGeneration skip from WE controller.
Terminal-phase and active-phase handling remains unchanged.

---

### **Pattern C: Phase Transition Idempotency** (Audit Event Prevention)
For preventing duplicate audit events at the phase handler level.

**Problem**: Even with Patterns A/B preventing wasteful reconciles, controllers can still emit duplicate audit events when:
- Status update conflicts trigger immediate re-reconciliation
- Annotation/label changes trigger reconciles when phase hasn't changed
- Multiple controllers racing in multi-controller test environments

**Solution**: Check if phase has ALREADY transitioned before emitting audit events.

**Example**: RemediationOrchestrator (discovered pattern), AIAnalysis (AA-BUG-009)

**Implementation** (at handler/transition function level):
```go
func (r *Reconciler) transitionPhase(ctx context.Context, obj *v1.Object, newPhase Phase) (ctrl.Result, error) {
	logger := log.FromContext(ctx)

	// Capture current phase BEFORE any modifications
	oldPhase := obj.Status.Phase

	// ========================================
	// IDEMPOTENCY CHECK (Prevents Duplicate Audit Events)
	// Per RO_AUDIT_DUPLICATION_RISK_ANALYSIS_JAN_01_2026.md - Option C
	// ========================================
	// Without GenerationChangedPredicate, controller reconciles on annotation/label changes.
	// This check prevents duplicate audit emissions when phase hasn't actually changed.
	// ADR-032 §1: Audit integrity requires exactly-once emission per state change.
	if oldPhase == newPhase {
		logger.V(1).Info("Phase transition skipped - already in target phase",
			"currentPhase", oldPhase,
			"requestedPhase", newPhase)
		return ctrl.Result{Requeue: true}, nil
	}

	// Proceed with phase transition
	obj.Status.Phase = newPhase
	obj.Status.ObservedGeneration = obj.Generation // Set AFTER completing transition

	// Emit audit event (guaranteed exactly-once per phase change)
	if oldPhase != newPhase {
		r.emitPhaseTransitionAudit(ctx, obj, oldPhase, newPhase)
	}

	// Update status
	return r.updateStatus(ctx, obj)
}
```

**Alternative Implementation** (at handler entry):
```go
func (h *AnalyzingHandler) Handle(ctx context.Context, analysis *v1.AIAnalysis) (ctrl.Result, error) {
	// Capture current phase BEFORE any processing
	oldPhase := analysis.Status.Phase

	// AA-BUG-009: Idempotency check - Per RO pattern
	// Skip if we're ALREADY in Completed phase for this generation
	// This prevents duplicate processing and audit events
	if analysis.Status.ObservedGeneration == analysis.Generation &&
	   oldPhase == PhaseCompleted {
		h.log.Info("Already in Completed phase for this generation, skipping",
			"generation", analysis.Generation,
			"phase", oldPhase)
		return ctrl.Result{}, nil
	}

	// ... process phase ...

	// Set new phase
	analysis.Status.Phase = PhaseCompleted
	analysis.Status.ObservedGeneration = analysis.Generation

	// Emit audit ONLY if phase actually changed
	if analysis.Status.Phase != oldPhase {
		h.auditClient.RecordAnalysisComplete(ctx, analysis)
	}

	return ctrl.Result{}, nil
}
```

**Key Principles**:
1. **Capture `oldPhase` FIRST** - before any modifications
2. **Check `oldPhase == targetPhase`** - skip if already in target state
3. **Set `ObservedGeneration` AFTER completing phase** - not mid-transition
4. **Emit audit events conditionally** - only if `oldPhase != newPhase`

**When to Use Pattern C**:
- ✅ Controllers with multi-phase state machines (AIAnalysis, RemediationOrchestrator, WorkflowExecution)
- ✅ Services with SOC2 audit trail requirements (ADR-032 compliance)
- ✅ Multi-controller test environments (DD-TEST-010)
- ❌ Simple stateless controllers (no phase transitions)

**Discovered By**: RemediationOrchestrator team (January 1, 2026)
**Validated By**: AIAnalysis migration (AA-BUG-009, January 11, 2026)
**Reference**: `RO_AUDIT_DUPLICATION_RISK_ANALYSIS_JAN_01_2026.md - Option C`

---

## Rationale

### **Why Simple Pattern Isn't Sufficient for Parent Controllers**

Generation-based idempotency **cannot distinguish** event types:

| Event Type | Generation | ObservedGeneration | Should Reconcile? | Simple Pattern Behavior |
|------------|-----------|-------------------|------------------|------------------------|
| Annotation change | 1 | 1 | ❌ No (wasteful) | ✅ Correctly skips |
| Child completes | 1 | 1 | ✅ **YES (critical!)** | ❌ **Incorrectly skips!** |
| Polling check | 1 | 1 | ✅ **YES (critical!)** | ❌ **Incorrectly skips!** |

**Result**: Simple pattern blocks critical orchestration, breaking parent controllers.

### **Why Phase-Aware Pattern Works**

Uses **phase context** to determine if orchestration is active:

| Phase | Orchestration Active? | Skip Reconcile? | Rationale |
|-------|----------------------|-----------------|-----------|
| **Pending** | ❌ No ¹ | ✅ Yes ¹ | Not yet created children, wasteful reconciles |
| **Processing** | ✅ **Yes** | ❌ **No** | Waiting for SignalProcessing completion |
| **Analyzing** | ✅ **Yes** | ❌ **No** | Waiting for AIAnalysis completion |
| **Executing** | ✅ **Yes** | ❌ **No** | Waiting for WorkflowExecution completion |
| **Completed** | ❌ No | ✅ Yes | Orchestration complete, wasteful reconciles |
| **Failed** | ❌ No | ✅ Yes | Orchestration complete, wasteful reconciles |

> **¹ Exception (v4.0)**: WorkflowExecution uses `RequeueAfter` during Pending
> for cooldown enforcement (BR-WE-009). Skipping Pending reconciles blocks
> cooldown retries and permanently stalls WFEs. WE does **not** skip Pending.
> See Pattern B-WE above.

**Result**: Critical orchestration events processed, wasteful reconciles still prevented.

---

## Implementation Checklist

### **1. CRD Schema** (ALL Controllers)

```go
type [CRD]Status struct {
    // ObservedGeneration is the most recent generation observed by the controller.
    // Used to prevent duplicate reconciliations and ensure idempotency.
    // Per DD-CONTROLLER-001: Standard pattern for all Kubernetes controllers.
    // +optional
    ObservedGeneration int64 `json:"observedGeneration,omitempty"`

    // ... rest of status fields ...
}
```

### **2. Controller Check** (Choose Pattern A or B)

**Pattern A** (Simple Controllers):
```go
if obj.Status.ObservedGeneration == obj.Generation &&
    obj.Status.Phase != "" &&
    !IsTerminal(obj.Status.Phase) {
    return ctrl.Result{}, nil
}
```

**Pattern B** (Parent Controllers — e.g., RemediationOrchestrator):
```go
if obj.Status.ObservedGeneration == obj.Generation &&
    (obj.Status.Phase == "Pending" || IsTerminal(obj.Status.Phase)) {
    return ctrl.Result{}, nil
}
```

**Pattern B-WE** (Controllers with Pending-phase `RequeueAfter` — e.g., WorkflowExecution):

Do **not** skip Pending. `GenerationChangedPredicate` prevents status-only watch duplicates.
```go
// No ObservedGeneration skip for Pending phase.
// Cooldown enforcement (BR-WE-009) uses RequeueAfter during Pending,
// and the skip would block the follow-up reconcile permanently.
```

### **3. Status Updates** (ALL Controllers)

**CRITICAL**: Set `ObservedGeneration` AFTER work completes, NOT during initialization.

```go
// ❌ BAD: During initialization
if currentPhase == "" {
    obj.Status.Phase = "Pending"
    obj.Status.ObservedGeneration = obj.Generation // ❌ Creates deadlock!
    // Update status
}

// ✅ GOOD: After completing phase work
func transitionPhase(ctx, obj, newPhase) error {
    // Do phase work...

    obj.Status.Phase = newPhase
    obj.Status.ObservedGeneration = obj.Generation // ✅ After work done
    // Update status
}

// ✅ GOOD: In terminal states
func transitionToCompleted(ctx, obj) error {
    obj.Status.Phase = "Completed"
    obj.Status.ObservedGeneration = obj.Generation // ✅ No further work
    // Update status
}
```

**Locations to Update**:
- ❌ **NOT** during initialization (Phase="" → "Pending")
- ❌ **NOT** when transitioning TO a new phase (work not done yet)
- ✅ **YES** after phase work completes (transition FROM current phase)
- ✅ **YES** in terminal states (Completed/Failed)

---

## Benefits

### **Correctness**
- ✅ Eliminates duplicate audit events (ADR-032 compliance)
- ✅ Prevents redundant work on annotation/label changes
- ✅ Ensures critical orchestration events are processed

### **Performance**
- ✅ Reduces unnecessary reconciliations by 30-50%
- ✅ Pattern A: ~90% skip rate (simple controllers)
- ✅ Pattern B: ~5% skip rate during orchestration, 95% skip otherwise
- ✅ Overall test suite 45% faster (RO: 329s → 182s)

### **Maintainability**
- ✅ Standard Kubernetes pattern (used by Deployment, StatefulSet, etc.)
- ✅ Consistent across all controllers
- ✅ Explicit visibility into processed generations via logging

---

## Consequences

### **Positive**
- ✅ Correct idempotency without blocking orchestration
- ✅ Follows Kubernetes best practices
- ✅ Measurable performance improvements
- ✅ Clear debugging via structured logging

### **Negative**
- ⚠️ Pattern B accepts extra reconciles during active phases
  - **Mitigation**: Active phases are short-lived (seconds/minutes)
  - **Cost**: ~270 extra reconciles per test suite (~2.7 seconds)
  - **Benefit**: 45% faster overall (net win)

### **Risks**
- ⚠️ Phase-aware logic more complex than simple pattern
  - **Mitigation**: Comprehensive documentation and tests
  - **Fallback**: Can revert to removing check entirely (Option C)

---

## Validation Results

### **Test Coverage**

| Controller | Pattern | Tests | Pass Rate | Before | Improvement |
|-----------|---------|-------|-----------|--------|-------------|
| **RemediationOrchestrator** | B (Phase-Aware) | 41/42 | **98%** | 57% | **+41 points** |
| **AIAnalysis** | A (Simple) | 54/54 | **100%** | 0% (timeouts) | **+100 points** |
| **SignalProcessing** | A (Simple) | 81/81 | **100%** | 0% (timeouts) | **+100 points** |
| **WorkflowExecution** | B-WE (No Pending skip) | IT-WE-374-001, IT-WE-375-002 | **100%** | 0% (stuck Pending) | **+100 points** |

### **Performance Metrics**

| Metric | Before | After | Improvement |
|--------|--------|-------|-------------|
| **RO Test Runtime** | 329.8s | 182.0s | **45% faster** |
| **Duplicates Prevented** | 301 | 31 | Reduced 90% (Pattern B tradeoff) |
| **Active Reconciles** | Blocked | 570 ✅ | Orchestration enabled |

### **Production Evidence**

- ✅ No duplicate audit events in integration tests
- ✅ Child orchestration tests passing
- ✅ Polling mechanisms functional
- ✅ Watch events processed correctly

---

## References

### **Implementation Docs**
- [PHASE_AWARE_OBSERVED_GENERATION_SUCCESS_JAN_02_2026.md](../../handoff/PHASE_AWARE_OBSERVED_GENERATION_SUCCESS_JAN_02_2026.md) - Success report
- [OBSERVED_GENERATION_DEEP_ANALYSIS_JAN_01_2026.md](../../handoff/OBSERVED_GENERATION_DEEP_ANALYSIS_JAN_01_2026.md) - Problem analysis
- [OBSERVED_GENERATION_DEADLOCK_COMPLETE_FIX_JAN_01_2026.md](../../handoff/OBSERVED_GENERATION_DEADLOCK_COMPLETE_FIX_JAN_01_2026.md) - Deadlock fix

### **Related Decisions**
- DD-AUDIT-003: Service audit trace requirements
- DD-PERF-001: Atomic status updates mandate
- DD-RO-002: Centralized routing responsibility

---

## Changelog

### Version 2.0 (January 2, 2026)
- **BREAKING**: Changed from simple pattern to phase-aware pattern for parent controllers
- **Added**: Pattern B (Phase-Aware) for controllers with active child orchestration
- **Added**: Comprehensive validation results (98% RO pass rate)
- **Added**: Performance metrics (45% faster test suite)
- **Fixed**: Critical deadlock issue with initialization
- **Fixed**: Child orchestration blocking (blocking eliminated)

### Version 1.0 (January 1, 2026)
- Initial implementation with simple pattern
- Applied to all 4 controllers
- Documented 97% baseline (documentation error - never achieved with simple pattern)

---

## Implementation Changelog

### 2.0 Implementation (January 1-2, 2026)

#### Phase 1: Initial ObservedGeneration Field Addition
**Date**: January 1, 2026 (Morning)

**Changes**:
- Added `ObservedGeneration` field to CRD status schemas:
  - `api/v1alpha1/aianalysis_types.go`
  - `api/v1alpha1/remediationrequest_types.go`
  - `api/v1alpha1/signalprocessing_types.go`
- Added CEL validation rules to CRD manifests
- Regenerated CRD manifests with `make manifests`

**Issues Encountered**:
- **Error 1**: Field not recognized by Kubernetes API server
  - **Cause**: Ran `make generate` but forgot `make manifests`
  - **Fix**: Ran `make manifests` to regenerate CRD YAMLs
- **Error 2**: CEL validation syntax error (double quotes)
  - **Cause**: Used `!= ""` instead of `!= ''` in CEL rule
  - **Fix**: Changed to single quotes in SignalProcessing CRD

---

#### Phase 2: Simple Pattern Implementation (Failed Approach)
**Date**: January 1, 2026 (Afternoon)

**Changes**:
- Implemented simple `ObservedGeneration` check in all controllers:
  ```go
  if obj.Status.ObservedGeneration == obj.Generation {
      return ctrl.Result{}, nil
  }
  ```
- Set `ObservedGeneration` in all status updates
- Applied to RemediationOrchestrator, AIAnalysis, SignalProcessing

**Issues Encountered**:
- **Error 3**: Complete deadlock in all controllers
  - **Cause**: `GenerationChangedPredicate` + `ObservedGeneration` check = double-blocking
  - **Symptoms**:
    - AIAnalysis: 0/54 tests passing (100% timeouts)
    - SignalProcessing: 0/81 tests passing (100% timeouts)
    - RemediationOrchestrator: ~30% pass rate
  - **Fix**: Removed `GenerationChangedPredicate` from `SetupWithManager` in all controllers

**Files Modified**:
- `internal/controller/aianalysis/reconciler.go`
- `internal/controller/signalprocessing/reconciler.go`
- `internal/controller/remediationorchestrator/reconciler.go`

---

#### Phase 3: Initialization Deadlock Fix
**Date**: January 1, 2026 (Late Afternoon)

**Changes**:
- Removed `ObservedGeneration` assignments during phase initialization
- Rule: Never set `ObservedGeneration` when transitioning FROM empty phase

**Issues Encountered**:
- **Error 4**: Controllers blocked during initialization
  - **Cause**: Setting `ObservedGeneration` when phase was `""` → `"Pending"` blocked subsequent reconciles
  - **Symptoms**: Resources stuck in initialization, never progressed
  - **Fix**: Removed `ObservedGeneration` assignment from initialization blocks

**Example Fix** (AIAnalysis):
```go
// BEFORE (Deadlock)
if currentPhase == "" {
    aa.Status.Phase = "Pending"
    aa.Status.ObservedGeneration = aa.Generation  // ❌ Blocks next reconcile!
}

// AFTER (Fixed)
if currentPhase == "" {
    aa.Status.Phase = "Pending"
    // ObservedGeneration NOT set here ✅
}
```

**Files Modified**:
- `internal/controller/remediationorchestrator/reconciler.go` (line ~250)
- `internal/controller/aianalysis/reconciler.go` (line ~180)
- `internal/controller/signalprocessing/reconciler.go` (line ~160)

**Test Results After Fix**:
- AIAnalysis: 54/54 passing (100%) ✅
- SignalProcessing: 81/81 passing (100%) ✅
- RemediationOrchestrator: Still failing (Phase 4 needed)

---

#### Phase 4: Non-Terminal Phase Transition Deadlock Fix
**Date**: January 1, 2026 (Evening)

**Changes**:
- Removed `ObservedGeneration` assignments during non-terminal phase transitions
- Rule: Never set `ObservedGeneration` when transitioning TO a new active phase

**Issues Encountered**:
- **Error 5**: Controllers blocked after transitioning to new phases
  - **Cause**: Setting `ObservedGeneration` when entering new phase (e.g., `Pending` → `Processing`) blocked reconciles needed to complete that phase
  - **Symptoms**:
    - Resources stuck in newly-entered phases
    - Child status updates not processed
    - Polling mechanisms broken
  - **Fix**: Removed `ObservedGeneration` from non-terminal phase transitions

**Example Fix** (SignalProcessing):
```go
// BEFORE (Deadlock)
func transitionToEnriching(sp) {
    sp.Status.Phase = "Enriching"
    sp.Status.ObservedGeneration = sp.Generation  // ❌ Blocks enrichment work!
}

// AFTER (Fixed)
func transitionToEnriching(sp) {
    sp.Status.Phase = "Enriching"
    // ObservedGeneration NOT set here ✅
}
```

**Files Modified**:
- `internal/controller/aianalysis/reconciler.go`:
  - Removed from `reconcilePending()` (transition to `Analyzing`)
  - Removed from `response_processor.go` (transition to `AnalysisComplete`)
- `internal/controller/signalprocessing/reconciler.go`:
  - Removed from all non-terminal transitions:
    - `Pending` → `Enriching`
    - `Enriching` → `Classifying`
    - `Classifying` → `Categorizing`

**Test Results After Fix**:
- AIAnalysis: 54/54 passing (100%) ✅
- SignalProcessing: 81/81 passing (100%) ✅
- RemediationOrchestrator: Still ~80% pass rate (Phase 5 needed)

---

#### Phase 5: Phase-Aware Pattern Implementation
**Date**: January 2, 2026 (Morning)

**Changes**:
- Implemented phase-aware `ObservedGeneration` check for RemediationOrchestrator
- Pattern: Skip ONLY in `Pending` or `Terminal` phases, allow active orchestration

**Issues Encountered**:
- **Error 6**: RemediationOrchestrator regression after initialization fix
  - **Cause**: `ObservedGeneration` check missing `rr.Status.OverallPhase != ""` condition
  - **Symptoms**: Resources blocked even during initialization
  - **Fix**: Added phase check to condition

**Example Implementation** (RemediationOrchestrator):
```go
// Phase-aware check
if rr.Status.ObservedGeneration == rr.Generation &&
    rr.Status.OverallPhase != "" &&  // ✅ Added this condition
    (rr.Status.OverallPhase == phase.Pending ||
     phase.IsTerminal(rr.Status.OverallPhase)) {
    logger.V(1).Info("⏭️  SKIPPED: No orchestration needed")
    return ctrl.Result{}, nil
}
```

**Files Modified**:
- `internal/controller/remediationorchestrator/reconciler.go` (lines 272-286)

**Test Results After Fix**:
- RemediationOrchestrator: 41/42 passing (98%) ✅
  - 1 failure: Audit infrastructure issue (unrelated to `ObservedGeneration`)

---

#### Phase 6: Design Decision Documentation
**Date**: January 2, 2026 (Afternoon)

**Changes**:
- Created `DD-CONTROLLER-001-observed-generation-idempotency-pattern.md`
- Documented Pattern A (Simple) and Pattern B (Phase-Aware)
- Added implementation checklist and validation results
- Documented all edge cases and failure modes

**Files Created**:
- `docs/architecture/decisions/DD-CONTROLLER-001-observed-generation-idempotency-pattern.md`

---

### Summary of Implementation Journey

**Total Implementation Time**: ~8 hours (January 1-2, 2026)

**Iterations Required**: 6 major phases
1. Field addition (2 bugs fixed)
2. Simple pattern (1 bug: double-blocking)
3. Initialization deadlock fix (1 bug: early `ObservedGeneration` assignment)
4. Phase transition deadlock fix (1 bug: transition-time assignment)
5. Phase-aware pattern (1 bug: missing phase condition)
6. Documentation

**Total Bugs Fixed**: 6 critical issues

**Final Test Results**:
- AIAnalysis: 54/54 (100%) ✅
- SignalProcessing: 81/81 (100%) ✅
- RemediationOrchestrator: 41/42 (98%) ✅
- **Overall: 176/177 tests passing (99.4%)**

**Key Lessons Learned**:
1. **Never set `ObservedGeneration` during initialization** - blocks first meaningful reconcile
2. **Never set `ObservedGeneration` during phase transitions** - blocks phase completion work
3. **Parent controllers need phase-aware logic** - simple pattern breaks child orchestration
4. **Remove `GenerationChangedPredicate`** - conflicts with `ObservedGeneration` check
5. **Test systematically** - each controller has unique orchestration patterns

**Performance Impact**:
- Test suite speedup: 45% faster (329s → 182s)
- Duplicate reconciles prevented: 90% reduction
- Zero functional regressions

---

## Approval

- ✅ **Technical Lead**: Approved (January 2, 2026 - v2.0, January 11, 2026 - v3.0, March 4, 2026 - v4.0)
- ✅ **Test Validation**: 98% pass rate achieved (v2.0), AIAnalysis validated (v3.0), WE cooldown fix validated (v4.0)
- ✅ **Performance Validation**: 45% speedup measured
- ✅ **Production Readiness**: Approved for deployment

**Status**: **AUTHORITATIVE** - All controllers must implement this pattern

---

## Changelog

### v4.0 (March 4, 2026)
**Fixed**: Pattern B Pending-phase skip incompatible with `RequeueAfter` (WorkflowExecution)

**Motivation**: Integration tests IT-WE-374-001 and IT-WE-375-002 (Issues #374, #375) revealed
that Pattern B's Pending-phase skip permanently stalls WFEs that use `RequeueAfter` during
Pending for cooldown enforcement (BR-WE-009). The `PhaseManager.TransitionTo(Pending)` call
in the cooldown block stamps `ObservedGeneration = Generation`, and when the requeue fires
the guard at `Reconcile()` entry sees `ObservedGeneration == Generation && Phase == Pending`
and skips the reconcile -- preventing cooldown retry and Job creation.

**Changes**:
- **Added**: Pattern B-WE variant for controllers with Pending-phase requeues
- **Added**: Caveat to Pattern B documenting the `RequeueAfter` incompatibility
- **Added**: Footnote ¹ to rationale table for Pending-phase exception
- **Added**: `Job` watch in `SetupWithManager` for immediate completion detection (BR-WE-014)
- **Renamed**: `FindWFEForPipelineRun` → `FindWFEForOwnedResource` (serves both PipelineRun and Job)
- **Updated**: WE validation results (previously TBD)
- **Removed**: Pending-phase ObservedGeneration skip from WE controller

**Root Cause (Pending skip)**: `PhaseManager.TransitionTo` unconditionally stamps
`ObservedGeneration` on every transition (including no-op `Pending → Pending`). Combined
with `RequeueAfter`, the DD-CONTROLLER-001 guard kills the follow-up reconcile because
nothing changed from its perspective. The fix removes the Pending skip for WE;
`GenerationChangedPredicate` already prevents status-only watch duplicates.

**Root Cause (Job detection delay)**: The Job executor relied solely on `RequeueAfter: 10s`
polling to detect Job completion, unlike the Tekton executor which had a dedicated
`PipelineRun` watch. This caused IT-WE-374-001 to flake when the 15s test timeout
was insufficient. Adding a `Job` watch using the same label-based mapper enables
immediate reconciliation on Job status changes.

**Implementation Files**:
- `internal/controller/workflowexecution/workflowexecution_controller.go` (removed Pending skip, added Job watch, renamed mapper)

**Validated By**:
- IT-WE-374-001: Second WFE proceeds after first completes for same target
- IT-WE-375-002: Lock release and cooldown expiry allow blocked WFE to proceed

---

### v3.0 (January 11, 2026)
**Added**: Pattern C - Phase Transition Idempotency (Audit Event Prevention)

**Motivation**: AIAnalysis migration to multi-controller (DD-TEST-010) exposed duplicate audit event emission when status update conflicts trigger re-reconciliation.

**Discovery**: RemediationOrchestrator team had already solved this with `oldPhase == newPhase` check (RO_AUDIT_DUPLICATION_RISK_ANALYSIS_JAN_01_2026.md - Option C).

**Impact**:
- Prevents duplicate `aianalysis.analysis.completed` events (AA-BUG-009)
- Ensures SOC2 audit trail integrity (ADR-032 compliance)
- Critical for multi-controller test environments (DD-TEST-010)

**Implementation Files**:
- `pkg/aianalysis/handlers/analyzing.go` (AA-BUG-009 fix)
- `pkg/aianalysis/handlers/investigating.go` (AA-BUG-009 fix)
- `internal/controller/remediationorchestrator/reconciler.go` (reference implementation)

**Validated By**:
- RemediationOrchestrator (production use since January 1, 2026)
- AIAnalysis (validation in progress as of January 11, 2026)

---

### v2.0 (January 2, 2026)
**Added**: Pattern B - Phase-Aware Idempotency (Parent Controllers)

**Motivation**: RemediationOrchestrator requires child orchestration during active phases, but Pattern A blocked all reconciles once `ObservedGeneration` was set.

**Impact**: 45% test speedup, 90% duplicate reconcile reduction while enabling child orchestration.

---

### v1.0 (December 2025)
**Initial**: Pattern A - Simple Idempotency (Leaf Controllers)

**Scope**: AIAnalysis, SignalProcessing (controllers without child orchestration)


