fix(operator): reconcile the warm pool on ADDED, not just MODIFIED

AGENTS.md

@@ -122,6 +122,24 @@ sidecar Deployment and Service are created **before** the Theia deployment so
 DNS resolves at pod startup, and releasing an eager session restarts the sidecar
 pods rather than deleting them.
 
+**Every operator start replays existing AppDefinitions as ADDED, so ADDED is not
+"new".** `BasicTheiaCloudOperator` lists the AppDefinitions at startup and feeds
+each one through `handleAppDefnitionEvent(ADDED, ...)` before the watch begins;
+there is no periodic resync, and the in-memory cache dies with the process. A
+helm upgrade restarts the operator in the same release that rewrites the
+AppDefinition, so an image change arrives on the ADDED path, never MODIFIED.
+Both paths must therefore reconcile. They used to differ - ADDED called
+`ensureCapacity`, which creates missing ids and never compares generations - and
+production served a pull request's image from its warm pool for weeks because
+ten instances existed and none were missing.
+
+**Pool membership is a name count, not a state check.**
+`TheiaCloudHandlerUtil.computeIdsOfMissing*` decides an instance is present if a
+resource whose name yields id *N* exists. Image, generation and readiness are
+not consulted, and `reserveInstance` hands out the lowest free instance without
+looking either. Anything that should replace an instance has to say so through
+`isOutdated`; nothing else will notice.
+
 **The pool's email ConfigMaps carry live session state.** `instance-N-email-…`
 holds the `authenticated-emails-list` oauth2-proxy admits; it is written when a
 session reserves the instance and cleared when the instance is released. That

java/operator/org.eclipse.theia.cloud.operator/src/main/java/org/eclipse/theia/cloud/operator/handler/appdef/EagerStartAppDefinitionAddedHandler.java

@@ -83,13 +83,18 @@ public boolean appDefinitionAdded(AppDefinition appDefinition, String correlatio
 
             LOGGER.trace(formatLogMessage(correlationId, "HTTPRoute available"));
 
-            // Ensure pool has minimum capacity
-            ISpan poolSpan = Tracing.childSpan(tx, "appdef.ensure_capacity", "Ensure pool capacity");
-            poolSpan.setData("min_instances", minInstances);
+            // Reconcile rather than only filling gaps. Every operator start replays the
+            // existing AppDefinitions as ADDED, and a helm upgrade restarts the operator
+            // in the same release that rewrites the AppDefinition - so ADDED is the path
+            // an image change actually arrives on. Counting instances and finding none
+            // missing left the pool on the previous image until something else happened
+            // to touch the AppDefinition.
+            ISpan poolSpan = Tracing.childSpan(tx, "appdef.reconcile_pool", "Reconcile pool to target");
+            poolSpan.setData("target_instances", minInstances);
 
-            boolean success = pool.ensureCapacity(appDefinition, minInstances, correlationId);
+            boolean success = pool.reconcile(appDefinition, minInstances, correlationId);
 
-            poolSpan.setTag("pool.operation", "ensure_capacity");
+            poolSpan.setTag("pool.operation", "reconcile");
             poolSpan.setTag("outcome", success ? "success" : "failure");
             Tracing.finish(poolSpan, success ? SpanStatus.OK : SpanStatus.INTERNAL_ERROR);
 

java/operator/org.eclipse.theia.cloud.operator/src/test/java/org/eclipse/theia/cloud/operator/pool/PrewarmedResourcePoolTests.java

@@ -10,6 +10,7 @@
 package org.eclipse.theia.cloud.operator.pool;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
@@ -18,6 +19,7 @@
 import static org.mockito.Mockito.when;
 
 import java.lang.reflect.Field;
+import java.lang.reflect.Method;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -38,6 +40,7 @@
 import org.mockito.Mockito;
 
 import io.fabric8.kubernetes.api.model.ConfigMap;
+import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.fabric8.kubernetes.api.model.ConfigMapBuilder;
 import io.fabric8.kubernetes.api.model.ConfigMapList;
 import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
@@ -397,6 +400,68 @@ private Session createSession(String appDefinition, String user, String instance
         return session;
     }
 
+    // ========== isOutdated ==========
+    //
+    // The generation label is what tells a pool instance apart from the AppDefinition
+    // it was built from. appDefinitionAdded reconciles rather than only filling gaps,
+    // and reconcile decides what to recreate entirely on this comparison, so these are
+    // the cases that decide whether a released image actually reaches the warm pool.
+
+    @Test
+    void isOutdated_generationMatches() {
+        assertFalse(isOutdated(deploymentWithLabels(Map.of(
+                PrewarmedResourcePool.APPDEFINITION_GENERATION_LABEL, "6")), 6L));
+    }
+
+    @Test
+    void isOutdated_generationIsOlder() {
+        assertTrue(isOutdated(deploymentWithLabels(Map.of(
+                PrewarmedResourcePool.APPDEFINITION_GENERATION_LABEL, "5")), 6L));
+    }
+
+    @Test
+    void isOutdated_labelMissing() {
+        // Instances created before the label existed. Production had ten of them still
+        // serving a pull request's image weeks after a release.
+        assertTrue(isOutdated(deploymentWithLabels(Map.of("app", APP_DEFINITION)), 6L));
+    }
+
+    @Test
+    void isOutdated_noLabelsAtAll() {
+        assertTrue(isOutdated(deploymentWithLabels(null), 6L));
+    }
+
+    @Test
+    void isOutdated_labelIsNotANumber() {
+        assertTrue(isOutdated(deploymentWithLabels(Map.of(
+                PrewarmedResourcePool.APPDEFINITION_GENERATION_LABEL, "not-a-generation")), 6L));
+    }
+
+    private Deployment deploymentWithLabels(Map<String, String> labels) {
+        ObjectMetaBuilder metadata = new ObjectMetaBuilder().withName("instance-1-" + APP_DEFINITION);
+        if (labels != null) {
+            metadata.withLabels(labels);
+        }
+        Deployment deployment = new DeploymentBuilder().build();
+        deployment.setMetadata(metadata.build());
+        return deployment;
+    }
+
+    private boolean isOutdated(Deployment deployment, long currentGeneration) {
+        try {
+            Method method = PrewarmedResourcePool.class.getDeclaredMethod("isOutdated", HasMetadata.class,
+                    long.class);
+            method.setAccessible(true);
+            return (boolean) method.invoke(pool(), deployment, currentGeneration);
+        } catch (Exception e) {
+            throw new IllegalStateException(e);
+        }
+    }
+
+    private PrewarmedResourcePool pool() {
+        return Mockito.mock(PrewarmedResourcePool.class, Mockito.CALLS_REAL_METHODS);
+    }
+
     private void setField(Object target, String fieldName, Object value) throws Exception {
         Field field = target.getClass().getDeclaredField(fieldName);
         field.setAccessible(true);