fix(chat): restore pending approval guard, tighten encapsulation, drop dormant tracker

app/src/main/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegate.kt

@@ -81,8 +81,12 @@ class ChatApprovalsDelegate(
         @Suppress("UNCHECKED_CAST")
         val approvals = (result as? Map<*, *>)?.get("approvals") as? List<*>
         val first = approvals?.filterIsInstance<Map<*, *>>()?.firstOrNull() ?: return
+        val requestId = first["request_id"] as? String ?: return
         val sessionId = runtimeSessionId() ?: uiState.value.currentSessionId
-        maybeSurfacePendingApproval(first, sessionId)
+        val alreadyShown =
+            uiState.value.messages.any { it.approvalInfo?.requestId == requestId }
+        if (alreadyShown) return
+        handleApprovalRequest(parseApprovalMap(first, sessionId))
     }
 
     fun handleApprovalRespondResult(result: Any?) {

app/src/main/java/com/m57/hermescontrol/ui/chat/ChatModelSwitchDelegate.kt

@@ -19,7 +19,7 @@ import java.util.concurrent.ConcurrentHashMap
  * Tracks an in-flight `config.set key=model` RPC and the model it would replace
  * so confirmation dialog dismiss can roll back optimistically.
  */
-data class ActiveModelSwitch(
+private data class ActiveModelSwitch(
     val spec: String,
     val previousModel: String?,
 )
@@ -47,10 +47,10 @@ class ChatModelSwitchDelegate(
     private val getPinnedModels: () -> List<PinnedModel> = { AuthManager.getPinnedModels() },
     private val savePinnedModels: (List<PinnedModel>) -> Unit = { AuthManager.savePinnedModels(it) },
 ) {
-    val pendingModelSwitchRequests = ConcurrentHashMap<String, ActiveModelSwitch>()
+    private val pendingModelSwitchRequests = ConcurrentHashMap<String, ActiveModelSwitch>()
-    var activeModelSwitchConfirmation: ActiveModelSwitch? = null
+    private var activeModelSwitchConfirmation: ActiveModelSwitch? = null
-    var optimisticPreviousModel: String? = null
+    private var optimisticPreviousModel: String? = null
-    var cachedModelOptions: List<ModelProvider> = emptyList()
+    private var cachedModelOptions: List<ModelProvider> = emptyList()
 
     fun isModelPickerCommand(command: String): Boolean {
         val trimmed = command.trim()

app/src/main/java/com/m57/hermescontrol/ui/chat/ChatRpcRequestTracker.kt

@@ -1,111 +0,0 @@
-package com.m57.hermescontrol.ui.chat
-
-import java.util.concurrent.ConcurrentHashMap
-
-/**
- * Tracks an in-flight RPC request associated with a specific session generation
- * and resume sequence to guard against stale results/errors from previous sessions.
- */
-data class TrackedSessionRequest(
-    val generation: Long,
-    val resumeSequence: Long = 0L,
-    val sessionId: String? = null,
-)
-
-/**
- * Owns RPC id-to-method mapping, session request generations, hydration sequences,
- * and staleness detection.
- *
- * Extracted behavior-preservingly from [ChatViewModel] to ensure deterministic
- * RPC tracking across session creation, switches, resumes, and reconnects.
- */
-class ChatRpcRequestTracker(
-    private val getCurrentSessionId: () -> String?,
-) {
-    /** Maps an in-flight RPC id to its method for UI error labeling. */
-    private val idToMethod = ConcurrentHashMap<String, String>()
-    private val sessionRequestById = ConcurrentHashMap<String, TrackedSessionRequest>()
-
-    var sessionGeneration: Long = 0L
-        private set
-
-    var resumeRequestSequence: Long = 0L
-        private set
-    var activeResumeRequestSequence: Long = 0L
-        private set
-
-    var hydrationRequestSequence: Long = 0L
-        private set
-    var activeHydrationRequestSequence: Long = 0L
-        private set
-
-    var resumedGeneration: Long = -1L
-    var hydratedGeneration: Long = -1L
-
-    fun nextSessionGeneration(): Long {
-        sessionGeneration++
-        return sessionGeneration
-    }
-
-    fun nextResumeSequence(): Long {
-        resumeRequestSequence++
-        activeResumeRequestSequence = resumeRequestSequence
-        return resumeRequestSequence
-    }
-
-    fun nextHydrationSequence(): Long {
-        hydrationRequestSequence++
-        activeHydrationRequestSequence = hydrationRequestSequence
-        return hydrationRequestSequence
-    }
-
-    fun trackRequest(
-        id: String,
-        method: String,
-    ) {
-        idToMethod[id] = method
-    }
-
-    fun trackSessionRequest(
-        id: String,
-        method: String,
-        generation: Long,
-        resumeSequence: Long = 0L,
-        sessionId: String? = null,
-    ) {
-        sessionRequestById[id] = TrackedSessionRequest(generation, resumeSequence, sessionId)
-        trackRequest(id, method)
-    }
-
-    fun isCurrentSessionRequest(
-        sessionId: String,
-        generation: Long,
-    ): Boolean = generation == sessionGeneration && sessionId == getCurrentSessionId()
-
-    fun isCurrentHydration(
-        sessionId: String,
-        generation: Long,
-        requestSequence: Long,
-    ): Boolean = requestSequence == activeHydrationRequestSequence && isCurrentSessionRequest(sessionId, generation)
-
-    fun isStaleSessionRequest(id: String): Boolean = sessionRequestById[id]?.let(::isStaleSessionRequest) == true
-
-    fun isStaleSessionRequest(request: TrackedSessionRequest): Boolean =
-        request.generation != sessionGeneration ||
-            (request.sessionId != null && request.sessionId != getCurrentSessionId()) ||
-            (request.resumeSequence != 0L && request.resumeSequence != activeResumeRequestSequence)
-
-    fun removeMethod(id: String): String? = idToMethod.remove(id)
-
-    fun removeSessionRequest(id: String): TrackedSessionRequest? = sessionRequestById.remove(id)
-
-    fun forgetRequest(id: String) {
-        idToMethod.remove(id)
-        sessionRequestById.remove(id)
-    }
-
-    fun resetSessionStateTracking() {
-        resumedGeneration = -1L
-        hydratedGeneration = -1L
-    }
-}

app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt

@@ -510,11 +510,6 @@ class ChatViewModel(
     private val sessionRequestById = ConcurrentHashMap<String, SessionRequest>()
     private var sessionGeneration = 0L
 
-    private data class ActiveModelSwitch(
-        val spec: String,
-        val previousModel: String?,
-    )
-
     private var resumeRequestSequence = 0L
     private var activeResumeRequestSequence = 0L
     private var hydrationRequestSequence = 0L

app/src/test/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegateTest.kt

@@ -161,6 +161,36 @@ class ChatApprovalsDelegateTest {
         )
     }
 
+    @Test
+    fun handleApprovalPendingResult_dropsMissingRequestId_andDedupesAlreadyShown() {
+        val initialCount = uiState.value.messages.size
+
+        // No request_id -> dropped entirely
+        delegate.handleApprovalPendingResult(
+            mapOf("approvals" to listOf(mapOf("command" to "rm -rf /"))),
+        )
+        assertEquals(initialCount, uiState.value.messages.size)
+
+        // Valid request_id -> surfaced
+        delegate.handleApprovalPendingResult(
+            mapOf("approvals" to listOf(mapOf("command" to "git pull", "request_id" to "r-valid"))),
+        )
+        assertEquals(initialCount + 1, uiState.value.messages.size)
+        assertEquals(
+            "r-valid",
+            uiState.value.messages
+                .last()
+                .approvalInfo
+                ?.requestId,
+        )
+
+        // Duplicate request_id -> ignored
+        delegate.handleApprovalPendingResult(
+            mapOf("approvals" to listOf(mapOf("command" to "git pull", "request_id" to "r-valid"))),
+        )
+        assertEquals(initialCount + 1, uiState.value.messages.size)
+    }
+
     @Test
     fun handleApprovalRespondResult_appendsDesktopParitySystemMessage() {
         delegate.handleApprovalRespondResult(mapOf("resolved" to 1))

app/src/test/java/com/m57/hermescontrol/ui/chat/ChatRpcRequestTrackerTest.kt

@@ -1,94 +0,0 @@
-package com.m57.hermescontrol.ui.chat
-
-import org.junit.Assert.assertEquals
-import org.junit.Assert.assertFalse
-import org.junit.Assert.assertNull
-import org.junit.Assert.assertTrue
-import org.junit.Test
-
-class ChatRpcRequestTrackerTest {
-    private var currentSessionId: String? = "session-a"
-    private val tracker = ChatRpcRequestTracker { currentSessionId }
-
-    @Test
-    fun trackRequest_andRemoveMethod() {
-        tracker.trackRequest("req-1", "session.create")
-        assertEquals("session.create", tracker.removeMethod("req-1"))
-        assertNull(tracker.removeMethod("req-1"))
-    }
-
-    @Test
-    fun generationIncrement_detectsStaleGenerations() {
-        val gen1 = tracker.nextSessionGeneration()
-        assertEquals(1L, gen1)
-
-        tracker.trackSessionRequest(
-            id = "req-resume-1",
-            method = "session.resume",
-            generation = gen1,
-            sessionId = "session-a",
-        )
-        assertFalse(tracker.isStaleSessionRequest("req-resume-1"))
-
-        val gen2 = tracker.nextSessionGeneration()
-        assertEquals(2L, gen2)
-        assertTrue(tracker.isStaleSessionRequest("req-resume-1"))
-    }
-
-    @Test
-    fun sessionMismatch_marksRequestStale() {
-        val gen = tracker.nextSessionGeneration()
-        tracker.trackSessionRequest(
-            id = "req-2",
-            method = "session.resume",
-            generation = gen,
-            sessionId = "session-a",
-        )
-        assertFalse(tracker.isStaleSessionRequest("req-2"))
-
-        currentSessionId = "session-b"
-        assertTrue(tracker.isStaleSessionRequest("req-2"))
-    }
-
-    @Test
-    fun resumeSequence_marksSupersededResumeStale() {
-        val gen = tracker.nextSessionGeneration()
-        val seq1 = tracker.nextResumeSequence()
-        tracker.trackSessionRequest(
-            id = "req-resume-seq1",
-            method = "session.resume",
-            generation = gen,
-            resumeSequence = seq1,
-            sessionId = "session-a",
-        )
-        assertFalse(tracker.isStaleSessionRequest("req-resume-seq1"))
-
-        val seq2 = tracker.nextResumeSequence()
-        assertEquals(2L, seq2)
-        assertTrue(tracker.isStaleSessionRequest("req-resume-seq1"))
-    }
-
-    @Test
-    fun hydrationSequence_guardsActiveHydration() {
-        val gen = tracker.nextSessionGeneration()
-        val hyd1 = tracker.nextHydrationSequence()
-        assertTrue(tracker.isCurrentHydration("session-a", gen, hyd1))
-
-        val hyd2 = tracker.nextHydrationSequence()
-        assertFalse(tracker.isCurrentHydration("session-a", gen, hyd1))
-        assertTrue(tracker.isCurrentHydration("session-a", gen, hyd2))
-    }
-
-    @Test
-    fun forgetRequest_removesBothTrackingEntries() {
-        tracker.trackSessionRequest(
-            id = "req-forget",
-            method = "session.resume",
-            generation = tracker.sessionGeneration,
-            sessionId = "session-a",
-        )
-        tracker.forgetRequest("req-forget")
-        assertNull(tracker.removeMethod("req-forget"))
-        assertNull(tracker.removeSessionRequest("req-forget"))
-    }
-}