diff --git a/android/app/src/main/kotlin/app/hapi/companion/feature/chat/ChatScreen.kt b/android/app/src/main/kotlin/app/hapi/companion/feature/chat/ChatScreen.kt index 2e9304dd..bf7f1420 100644 --- a/android/app/src/main/kotlin/app/hapi/companion/feature/chat/ChatScreen.kt +++ b/android/app/src/main/kotlin/app/hapi/companion/feature/chat/ChatScreen.kt @@ -352,6 +352,7 @@ fun ChatScreen( QueuedMessagesBar( rows = queuedRows, onSteer = viewModel::steerQueuedMessage, + onRetry = viewModel::retryIndeterminateMessage, onEdit = viewModel::editQueuedMessage, onCancel = viewModel::cancelQueuedMessage, ) diff --git a/android/app/src/main/kotlin/app/hapi/companion/feature/chat/ChatViewModel.kt b/android/app/src/main/kotlin/app/hapi/companion/feature/chat/ChatViewModel.kt index 49f4962a..83d0a15b 100644 --- a/android/app/src/main/kotlin/app/hapi/companion/feature/chat/ChatViewModel.kt +++ b/android/app/src/main/kotlin/app/hapi/companion/feature/chat/ChatViewModel.kt @@ -154,6 +154,8 @@ data class QueuedRowUi( val canAct: Boolean, /** Steer offered: turn active, not future-scheduled, actionable. */ val canSteer: Boolean, + /** Native delivery outcome is unknown; show explicit retry instead of normal Steer. */ + val indeterminate: Boolean = false, ) /** Session config sheet model (M3b switching). */ @@ -952,6 +954,9 @@ class ChatViewModel( val invokedMessage = response.message if (response.status == "invoked" && invokedMessage != null) { store.appendOptimistic(invokedMessage.asWindowMessage(MessageStatus.Sent)) + } else if (response.status == "busy") { + store.appendOptimistic(row.copy(status = MessageStatus.Indeterminate)) + runCatching { store.reconcileQueuedState() } } response.status } catch (cancellation: CancellationException) { @@ -965,6 +970,35 @@ class ChatViewModel( } } + fun retryIndeterminateMessage(messageId: String) { + if (!queuedOpPending.compareAndSet(expect = false, update = true)) return + scope.launch { + try { + val response = api.retryIndeterminateMessage(sessionId, messageId) + val message = response.message + if (response.status == "invoked" && message != null) { + val localId = message.localId + val invokedAt = message.invokedAtOrNull + if (localId != null && invokedAt != null) { + awaitWindowStore().markConsumed(listOf(localId), invokedAt) + } + } + if (response.status == "retried" || response.status == "already-queued") { + response.localId?.let { awaitWindowStore().markRequeued(listOf(it)) } + } else if (response.status == "not-found") { + awaitWindowStore().removeMessage(messageId) + _events.tryEmit(ChatEvent.Notice(ChatNotice.CancelQueuedFailed("Message is no longer available"))) + } else if (response.status == "retry-unavailable") { + _events.tryEmit(ChatEvent.Notice(ChatNotice.CancelQueuedFailed("Delivery is still being resolved"))) + } + } catch (error: Exception) { + _events.tryEmit(ChatEvent.Notice(ChatNotice.CancelQueuedFailed(error.message))) + } finally { + queuedOpPending.value = false + } + } + } + /** Edit = cancel + prefill composer (kept when the operator typed meanwhile). */ fun editQueuedMessage(messageId: String) { scope.launch { @@ -1063,7 +1097,9 @@ class ChatViewModel( attachmentNames = preview.attachmentNames, scheduledAt = row.wire.scheduledAt, canAct = canAct, - canSteer = canAct && thinking && row.wire.scheduledAt == null, + canSteer = canAct && thinking && row.wire.scheduledAt == null + && row.status != MessageStatus.Indeterminate, + indeterminate = row.status == MessageStatus.Indeterminate, ) } } diff --git a/android/app/src/main/kotlin/app/hapi/companion/feature/chat/composer/QueuedMessagesBar.kt b/android/app/src/main/kotlin/app/hapi/companion/feature/chat/composer/QueuedMessagesBar.kt index d7c4b6be..db3ed6f5 100644 --- a/android/app/src/main/kotlin/app/hapi/companion/feature/chat/composer/QueuedMessagesBar.kt +++ b/android/app/src/main/kotlin/app/hapi/companion/feature/chat/composer/QueuedMessagesBar.kt @@ -37,6 +37,7 @@ import java.util.Date fun QueuedMessagesBar( rows: List, onSteer: (messageId: String) -> Unit, + onRetry: (messageId: String) -> Unit, onEdit: (messageId: String) -> Unit, onCancel: (messageId: String) -> Unit, modifier: Modifier = Modifier, @@ -65,7 +66,7 @@ fun QueuedMessagesBar( verticalArrangement = Arrangement.spacedBy(4.dp), ) { rows.forEach { row -> - QueuedRow(row, onSteer, onEdit, onCancel) + QueuedRow(row, onSteer, onRetry, onEdit, onCancel) } } } @@ -75,6 +76,7 @@ fun QueuedMessagesBar( private fun QueuedRow( row: QueuedRowUi, onSteer: (String) -> Unit, + onRetry: (String) -> Unit, onEdit: (String) -> Unit, onCancel: (String) -> Unit, ) { @@ -94,6 +96,13 @@ private fun QueuedRow( maxLines = 2, overflow = TextOverflow.Ellipsis, ) + if (row.indeterminate) { + Text( + text = "Delivery outcome unknown", + style = MaterialTheme.typography.labelSmall, + color = MaterialTheme.colorScheme.error, + ) + } row.scheduledAt?.let { scheduledAt -> Text( text = stringResource( @@ -105,7 +114,9 @@ private fun QueuedRow( ) } } - if (row.canSteer) { + if (row.indeterminate) { + TextButton(onClick = { onRetry(row.id) }, enabled = row.canAct) { Text("Retry") } + } else if (row.canSteer) { TextButton(onClick = { onSteer(row.id) }) { Text(stringResource(R.string.chat_queued_steer)) } } TextButton(onClick = { onEdit(row.id) }, enabled = row.canAct) { Text(stringResource(R.string.chat_queued_edit)) } @@ -145,7 +156,7 @@ private fun QueuedMessagesBarPreview() { canAct = true, canSteer = false, ), ), - onSteer = {}, onEdit = {}, onCancel = {}, + onSteer = {}, onRetry = {}, onEdit = {}, onCancel = {}, ) } } diff --git a/android/app/src/test/kotlin/app/hapi/companion/feature/chat/ChatViewModelInteractionTest.kt b/android/app/src/test/kotlin/app/hapi/companion/feature/chat/ChatViewModelInteractionTest.kt index 1fc5d3df..bcfd1449 100644 --- a/android/app/src/test/kotlin/app/hapi/companion/feature/chat/ChatViewModelInteractionTest.kt +++ b/android/app/src/test/kotlin/app/hapi/companion/feature/chat/ChatViewModelInteractionTest.kt @@ -39,6 +39,7 @@ import app.hapi.protocol.wire.OptionalField import app.hapi.protocol.wire.QueuedStateResponse import app.hapi.protocol.wire.ReopenSessionResponse import app.hapi.protocol.wire.ResumeSessionResponse +import app.hapi.protocol.wire.RetryIndeterminateMessageResponse import app.hapi.protocol.wire.SendMessageRequest import app.hapi.protocol.wire.Session import app.hapi.protocol.wire.SessionMetadata @@ -205,6 +206,9 @@ private class RecordingChatApi : ChatSessionApi { return cancelResult } + override suspend fun retryIndeterminateMessage(sessionId: String, messageId: String): RetryIndeterminateMessageResponse = + RetryIndeterminateMessageResponse(status = "retried", localId = messageId) + override suspend fun steerMessage(sessionId: String, messageId: String): SteerQueuedMessageResponse { steerCalls.value = steerCalls.value + messageId return steerResult diff --git a/android/app/src/test/kotlin/app/hapi/companion/feature/chat/ChatViewModelTest.kt b/android/app/src/test/kotlin/app/hapi/companion/feature/chat/ChatViewModelTest.kt index 973afc30..9ab8a3c4 100644 --- a/android/app/src/test/kotlin/app/hapi/companion/feature/chat/ChatViewModelTest.kt +++ b/android/app/src/test/kotlin/app/hapi/companion/feature/chat/ChatViewModelTest.kt @@ -20,6 +20,7 @@ import app.hapi.protocol.wire.CodexModelsResponse import app.hapi.protocol.wire.DecryptedMessage import app.hapi.protocol.wire.HapiJson import app.hapi.protocol.wire.ResumeSessionResponse +import app.hapi.protocol.wire.RetryIndeterminateMessageResponse import app.hapi.protocol.wire.SendMessageRequest import app.hapi.protocol.wire.SteerQueuedMessageResponse import app.hapi.protocol.wire.Machine @@ -144,6 +145,9 @@ private class FakeMessagesApi : ChatSessionApi { override suspend fun sendMessage(sessionId: String, message: SendMessageRequest) {} override suspend fun cancelMessage(sessionId: String, messageId: String): CancelMessageResponse = CancelMessageResponse(status = "cancelled", localId = messageId) + override suspend fun retryIndeterminateMessage(sessionId: String, messageId: String): RetryIndeterminateMessageResponse = + RetryIndeterminateMessageResponse(status = "retried", localId = messageId) + override suspend fun steerMessage(sessionId: String, messageId: String): SteerQueuedMessageResponse = SteerQueuedMessageResponse(status = "steered", localId = messageId) override suspend fun abortSession(sessionId: String) {} diff --git a/android/core/data/src/main/kotlin/app/hapi/data/api/ChatSessionApi.kt b/android/core/data/src/main/kotlin/app/hapi/data/api/ChatSessionApi.kt index 0daff609..9abd47b9 100644 --- a/android/core/data/src/main/kotlin/app/hapi/data/api/ChatSessionApi.kt +++ b/android/core/data/src/main/kotlin/app/hapi/data/api/ChatSessionApi.kt @@ -2,6 +2,7 @@ package app.hapi.data.api import app.hapi.protocol.wire.ApprovePermissionRequest import app.hapi.protocol.wire.CancelMessageResponse +import app.hapi.protocol.wire.RetryIndeterminateMessageResponse import app.hapi.protocol.wire.CodexModelsResponse import app.hapi.protocol.wire.DeleteUploadResponse import app.hapi.protocol.wire.ResumeSessionResponse @@ -47,6 +48,9 @@ interface ChatSessionApi : MessagesApi, AttachmentUploadApi { /** `DELETE /api/sessions/:id/messages/:messageId` — `cancelled` or `invoked` (too late). */ suspend fun cancelMessage(sessionId: String, messageId: String): CancelMessageResponse + /** `POST /api/sessions/:id/messages/:messageId/retry` — explicit unknown-delivery retry. */ + suspend fun retryIndeterminateMessage(sessionId: String, messageId: String): RetryIndeterminateMessageResponse + /** `POST /api/sessions/:id/messages/:messageId/steer` — `steered`/`invoked`/`failed`. */ suspend fun steerMessage(sessionId: String, messageId: String): SteerQueuedMessageResponse diff --git a/android/core/data/src/main/kotlin/app/hapi/data/api/HapiApi.kt b/android/core/data/src/main/kotlin/app/hapi/data/api/HapiApi.kt index 226b16a0..23211570 100644 --- a/android/core/data/src/main/kotlin/app/hapi/data/api/HapiApi.kt +++ b/android/core/data/src/main/kotlin/app/hapi/data/api/HapiApi.kt @@ -5,6 +5,7 @@ import app.hapi.protocol.wire.ApprovePermissionRequest import app.hapi.protocol.wire.AuthRequest import app.hapi.protocol.wire.AuthResponse import app.hapi.protocol.wire.CancelMessageResponse +import app.hapi.protocol.wire.RetryIndeterminateMessageResponse import app.hapi.protocol.wire.CodexModelsResponse import app.hapi.protocol.wire.DeleteUploadRequest import app.hapi.protocol.wire.DeleteUploadResponse @@ -247,6 +248,9 @@ class HapiApi( override suspend fun cancelMessage(sessionId: String, messageId: String): CancelMessageResponse = request("DELETE", url("api", "sessions", sessionId, "messages", messageId).build()) + override suspend fun retryIndeterminateMessage(sessionId: String, messageId: String): RetryIndeterminateMessageResponse = + request("POST", url("api", "sessions", sessionId, "messages", messageId, "retry").build(), EMPTY_JSON) + /** `POST /api/sessions/:id/messages/:messageId/steer`. */ override suspend fun steerMessage(sessionId: String, messageId: String): SteerQueuedMessageResponse = request("POST", url("api", "sessions", sessionId, "messages", messageId, "steer").build(), EMPTY_JSON) diff --git a/android/core/data/src/main/kotlin/app/hapi/data/sse/SyncEventRouter.kt b/android/core/data/src/main/kotlin/app/hapi/data/sse/SyncEventRouter.kt index 389f84e0..d63664df 100644 --- a/android/core/data/src/main/kotlin/app/hapi/data/sse/SyncEventRouter.kt +++ b/android/core/data/src/main/kotlin/app/hapi/data/sse/SyncEventRouter.kt @@ -79,6 +79,8 @@ class SyncEventRouter(private val targets: SyncTargets) { is SyncEvent.MessageReceived, is SyncEvent.MessagesConsumed, + is SyncEvent.MessagesIndeterminate, + is SyncEvent.MessagesRequeued, is SyncEvent.MessageCancelled, is SyncEvent.ScheduledMatured, is SyncEvent.MessagesInvalidated, diff --git a/android/core/data/src/main/kotlin/app/hapi/data/store/MessageWindowStore.kt b/android/core/data/src/main/kotlin/app/hapi/data/store/MessageWindowStore.kt index 97ec8740..133469f0 100644 --- a/android/core/data/src/main/kotlin/app/hapi/data/store/MessageWindowStore.kt +++ b/android/core/data/src/main/kotlin/app/hapi/data/store/MessageWindowStore.kt @@ -371,6 +371,8 @@ class MessageWindowStore( when (event) { is SyncEvent.MessageReceived -> ingestSseMessages(listOf(event.message.asWindowMessage())) is SyncEvent.MessagesConsumed -> markConsumed(event.localIds, event.invokedAt) + is SyncEvent.MessagesIndeterminate -> markIndeterminate(event.localIds) + is SyncEvent.MessagesRequeued -> markRequeued(event.localIds) is SyncEvent.MessageCancelled -> removeMessage(event.messageId) is SyncEvent.MessagesInvalidated -> { clear() @@ -395,6 +397,16 @@ class MessageWindowStore( persist() } + suspend fun markIndeterminate(localIds: List) { + update { MessageWindowLogic.markIndeterminate(it, localIds) } + persist() + } + + suspend fun markRequeued(localIds: List) { + update { MessageWindowLogic.markRequeued(it, localIds) } + persist() + } + /** SSE `message-cancelled` / optimistic DELETE removal (web `removeOptimisticMessage`). */ suspend fun removeMessage(localIdOrId: String) { update { MessageWindowLogic.removeByLocalIdOrId(it, localIdOrId) } @@ -471,10 +483,12 @@ class MessageWindowStore( val candidateLocalIds = queuedReconcileCandidateLocalIds() if (candidateLocalIds.isEmpty()) return val queuedLocalIds = mutableListOf() + val indeterminateLocalIds = mutableListOf() val invokedLocalMessages = mutableListOf>() candidateLocalIds.chunked(QUEUED_STATE_BATCH_SIZE).forEach { batch -> val response = api.getQueuedState(sessionId, batch) queuedLocalIds += response.queuedLocalIds + indeterminateLocalIds += response.indeterminateLocalIds invokedLocalMessages += response.invokedLocalMessages.map { it.localId to it.invokedAt } } val invokedByTimestamp = LinkedHashMap>() @@ -484,7 +498,8 @@ class MessageWindowStore( for ((invokedAt, localIds) in invokedByTimestamp) { markConsumed(localIds, invokedAt) } - reconcileQueuedLocalIds(candidateLocalIds, queuedLocalIds) + markIndeterminate(indeterminateLocalIds) + reconcileQueuedLocalIds(candidateLocalIds, queuedLocalIds + indeterminateLocalIds) } // ------------------------------------------------------------ lifecycle -- diff --git a/android/core/data/src/main/kotlin/app/hapi/data/store/StoreSyncTargets.kt b/android/core/data/src/main/kotlin/app/hapi/data/store/StoreSyncTargets.kt index 861e4ed8..396a67fc 100644 --- a/android/core/data/src/main/kotlin/app/hapi/data/store/StoreSyncTargets.kt +++ b/android/core/data/src/main/kotlin/app/hapi/data/store/StoreSyncTargets.kt @@ -83,6 +83,8 @@ class StoreSyncTargets( when (event) { is SyncEvent.MessagesInvalidated, is SyncEvent.MessagesConsumed, + is SyncEvent.MessagesIndeterminate, + is SyncEvent.MessagesRequeued, is SyncEvent.MessageCancelled, is SyncEvent.ScheduledMatured, -> sessions.scheduleRefresh() @@ -156,6 +158,8 @@ class StoreSyncTargets( is SseSubscriptionKey.Global -> when (event) { // Global pipe: queued/optimistic bookkeeping only (see class doc). is SyncEvent.MessagesConsumed -> store.markConsumed(event.localIds, event.invokedAt) + is SyncEvent.MessagesIndeterminate -> store.markIndeterminate(event.localIds) + is SyncEvent.MessagesRequeued -> store.markRequeued(event.localIds) is SyncEvent.MessageCancelled -> store.removeMessage(event.messageId) else -> Unit } @@ -165,6 +169,8 @@ class StoreSyncTargets( private fun SyncEvent.sessionIdOrNull(): String? = when (this) { is SyncEvent.MessageReceived -> sessionId is SyncEvent.MessagesConsumed -> sessionId + is SyncEvent.MessagesIndeterminate -> sessionId + is SyncEvent.MessagesRequeued -> sessionId is SyncEvent.MessageCancelled -> sessionId is SyncEvent.MessagesInvalidated -> sessionId is SyncEvent.ScheduledMatured -> sessionId diff --git a/android/core/protocol/src/main/kotlin/app/hapi/protocol/window/MessageWindowLogic.kt b/android/core/protocol/src/main/kotlin/app/hapi/protocol/window/MessageWindowLogic.kt index 2996ebec..204081d1 100644 --- a/android/core/protocol/src/main/kotlin/app/hapi/protocol/window/MessageWindowLogic.kt +++ b/android/core/protocol/src/main/kotlin/app/hapi/protocol/window/MessageWindowLogic.kt @@ -509,6 +509,37 @@ object MessageWindowLogic { return if (changed) previous.withMessages(messages) else previous } + /** Mark a row's native delivery outcome as unknown without invoking it. */ + fun markIndeterminate(previous: MessageWindowState, localIds: List): MessageWindowState { + if (localIds.isEmpty()) return previous + val ids = localIds.toSet() + var changed = false + val messages = previous.messages.map { message -> + if (message.localId == null || message.localId !in ids || message.status == MessageStatus.Indeterminate) { + message + } else { + changed = true + message.copy(status = MessageStatus.Indeterminate) + } + } + return if (changed) previous.withMessages(messages) else previous + } + + fun markRequeued(previous: MessageWindowState, localIds: List): MessageWindowState { + if (localIds.isEmpty()) return previous + val ids = localIds.toSet() + var changed = false + val messages = previous.messages.map { message -> + if (message.localId == null || message.localId !in ids || message.status != MessageStatus.Indeterminate) { + message + } else { + changed = true + message.copy(status = MessageStatus.Queued) + } + } + return if (changed) previous.withMessages(messages) else previous + } + /** `message-cancelled` / optimistic DELETE removal: matches localId OR id; idempotent. */ fun removeByLocalIdOrId(previous: MessageWindowState, localId: String): MessageWindowState { if (localId.isEmpty()) return previous diff --git a/android/core/protocol/src/main/kotlin/app/hapi/protocol/window/WindowMessage.kt b/android/core/protocol/src/main/kotlin/app/hapi/protocol/window/WindowMessage.kt index 00adb284..cb43d77a 100644 --- a/android/core/protocol/src/main/kotlin/app/hapi/protocol/window/WindowMessage.kt +++ b/android/core/protocol/src/main/kotlin/app/hapi/protocol/window/WindowMessage.kt @@ -31,6 +31,7 @@ enum class MessageStatus(val wire: String) { Sending("sending"), Sent("sent"), Failed("failed"), + Indeterminate("indeterminate"), ; companion object { @@ -79,7 +80,10 @@ data class WindowMessage( } fun DecryptedMessage.asWindowMessage(status: MessageStatus? = null): WindowMessage = - WindowMessage(this, status) + WindowMessage( + this, + status ?: if (deliveryState == "indeterminate") MessageStatus.Indeterminate else null, + ) /** * Serializes as the wire object plus an optional `status` key — the same diff --git a/android/core/protocol/src/main/kotlin/app/hapi/protocol/wire/ApiResponses.kt b/android/core/protocol/src/main/kotlin/app/hapi/protocol/wire/ApiResponses.kt index 0b078314..35f1188c 100644 --- a/android/core/protocol/src/main/kotlin/app/hapi/protocol/wire/ApiResponses.kt +++ b/android/core/protocol/src/main/kotlin/app/hapi/protocol/wire/ApiResponses.kt @@ -119,6 +119,13 @@ data class CancelMessageResponse( val message: DecryptedMessage? = null, ) +@Serializable +data class RetryIndeterminateMessageResponse( + val status: String, + val localId: String? = null, + val message: DecryptedMessage? = null, +) + /** * `POST /api/sessions/:id/messages/:messageId/steer` * (`SteerQueuedMessageResponseSchema` — discriminated on [status]): @@ -140,6 +147,7 @@ data class SteerQueuedMessageResponse( data class QueuedStateResponse( val queuedLocalIds: List, val invokedLocalMessages: List, + val indeterminateLocalIds: List = emptyList(), ) @Serializable diff --git a/android/core/protocol/src/main/kotlin/app/hapi/protocol/wire/DecryptedMessage.kt b/android/core/protocol/src/main/kotlin/app/hapi/protocol/wire/DecryptedMessage.kt index d20962de..a721d62b 100644 --- a/android/core/protocol/src/main/kotlin/app/hapi/protocol/wire/DecryptedMessage.kt +++ b/android/core/protocol/src/main/kotlin/app/hapi/protocol/wire/DecryptedMessage.kt @@ -39,6 +39,7 @@ data class DecryptedMessage( val createdAt: Long, val invokedAt: OptionalField = OptionalField.Absent, val scheduledAt: Long? = null, + val deliveryState: String? = null, ) { /** `invokedAt` collapsed to a plain value (both absent and `null` → null). */ val invokedAtOrNull: Long? get() = invokedAt.valueOrNull() @@ -70,6 +71,7 @@ internal object DecryptedMessageSerializer : KSerializer { OptionalField.Absent }, scheduledAt = obj["scheduledAt"].longOrNull, + deliveryState = obj["deliveryState"].stringOrNull, ) } @@ -86,6 +88,7 @@ internal object DecryptedMessageSerializer : KSerializer { put("invokedAt", value.invokedAt.value?.let(::JsonPrimitive) ?: JsonNull) } value.scheduledAt?.let { put("scheduledAt", JsonPrimitive(it)) } + value.deliveryState?.let { put("deliveryState", JsonPrimitive(it)) } }) } } diff --git a/android/core/protocol/src/main/kotlin/app/hapi/protocol/wire/SyncEvent.kt b/android/core/protocol/src/main/kotlin/app/hapi/protocol/wire/SyncEvent.kt index e008317b..af30bf9d 100644 --- a/android/core/protocol/src/main/kotlin/app/hapi/protocol/wire/SyncEvent.kt +++ b/android/core/protocol/src/main/kotlin/app/hapi/protocol/wire/SyncEvent.kt @@ -105,6 +105,20 @@ sealed interface SyncEvent { val invokedAt: Long, ) : SyncEvent + @Serializable + data class MessagesIndeterminate( + override val namespace: String? = null, + val sessionId: String, + val localIds: List, + ) : SyncEvent + + @Serializable + data class MessagesRequeued( + override val namespace: String? = null, + val sessionId: String, + val localIds: List, + ) : SyncEvent + @Serializable data class MessageCancelled( override val namespace: String? = null, @@ -183,6 +197,8 @@ object SyncEvents { "machine-updated" -> return parseMachineUpdated(obj) "toast" -> SyncEvent.Toast.serializer() "messages-consumed" -> SyncEvent.MessagesConsumed.serializer() + "messages-indeterminate" -> SyncEvent.MessagesIndeterminate.serializer() + "messages-requeued" -> SyncEvent.MessagesRequeued.serializer() "message-cancelled" -> SyncEvent.MessageCancelled.serializer() "heartbeat" -> SyncEvent.Heartbeat.serializer() "connection-changed" -> SyncEvent.ConnectionChanged.serializer() diff --git a/cli/src/api/apiSession.ts b/cli/src/api/apiSession.ts index a138667e..64766f17 100644 --- a/cli/src/api/apiSession.ts +++ b/cli/src/api/apiSession.ts @@ -241,7 +241,8 @@ export class ApiSessionClient extends EventEmitter { private pendingMessages: { message: UserMessage; localId?: string }[] = [] private pendingHubPromptEchoes: { text: string; localIds: string[] }[] = [] private pendingMessageCallback: ((message: UserMessage, localId?: string) => void) | null = null - private cancelQueuedMessageCallback: ((localId: string) => boolean) | null = null + private cancelQueuedMessageCallback: ((localId: string) => boolean | 'in-flight' | 'indeterminate' | 'consumed') | null = null + private retryQueuedMessageCallback: ((localId: string) => boolean) | null = null private readonly incomingFilter = new IncomingMessageFilter() private backfillInFlight: Promise | null = null private needsBackfill = false @@ -421,7 +422,7 @@ export class ApiSessionClient extends EventEmitter { this.agentTerminalActive = false })) - this.socket.on('update', (data: Update, ack?: (response: { removed: boolean }) => void) => { + this.socket.on('update', (data: Update, ack?: (response: { removed: boolean; inFlight?: boolean; indeterminate?: boolean; accepted?: boolean; consumed?: boolean }) => void) => { try { if (!data.body) return @@ -430,11 +431,41 @@ export class ApiSessionClient extends EventEmitter { return } + if (data.body.t === 'retry-queued-message') { + // Explicit user retry: release any held in-memory + // reservation and bypass the normal message-id dedup. + let accepted = true + if (data.body.localId && this.cancelQueuedMessageCallback) { + const cancelled = this.cancelQueuedMessageCallback(data.body.localId) + accepted = cancelled !== 'in-flight' && cancelled !== 'consumed' + if (cancelled === 'indeterminate') { + accepted = this.retryQueuedMessageCallback?.(data.body.localId) === true + } + if (cancelled === 'consumed') { + ack?.({ removed: false, accepted: false, consumed: true }) + return + } + } + if (accepted) { + this.handleIncomingMessage(data.body.message, true) + } + ack?.({ removed: false, accepted }) + return + } + if (data.body.t === 'cancel-queued-message') { - const removed = (data.body.localId && this.cancelQueuedMessageCallback) + const result = (data.body.localId && this.cancelQueuedMessageCallback) ? this.cancelQueuedMessageCallback(data.body.localId) : false - ack?.({ removed }) + // 'in-flight' = the row is inside an async steer: not + // removed, but also NOT consumed — the hub must neither + // delete it nor stamp invoked_at. + ack?.({ + removed: result === true, + inFlight: result === 'in-flight', + indeterminate: result === 'indeterminate', + consumed: result === 'consumed' + }) return } @@ -665,10 +696,14 @@ export class ApiSessionClient extends EventEmitter { } } - onCancelQueuedMessage(callback: (localId: string) => boolean): void { + onCancelQueuedMessage(callback: (localId: string) => boolean | 'in-flight' | 'indeterminate' | 'consumed'): void { this.cancelQueuedMessageCallback = callback } + onRetryQueuedMessage(callback: (localId: string) => boolean): void { + this.retryQueuedMessageCallback = callback + } + private enqueueUserMessage(message: UserMessage, localId?: string): void { if (this.pendingMessageCallback) { this.pendingMessageCallback(message, localId) @@ -733,8 +768,12 @@ export class ApiSessionClient extends EventEmitter { return true } - private handleIncomingMessage(message: { id?: string; seq?: number; localId?: string | null; content: unknown }): void { - if (!this.incomingFilter.accept({ id: message.id, seq: message.seq })) { + private handleIncomingMessage( + message: { id?: string; seq?: number; localId?: string | null; content: unknown }, + force = false + ): void { + const accepted = this.incomingFilter.accept({ id: message.id, seq: message.seq }) + if (!force && !accepted) { return } @@ -1174,7 +1213,7 @@ export class ApiSessionClient extends EventEmitter { }, 'droppable') } - emitMessagesConsumed(localIds: string[], options?: { clearQueuedThinkingGrace?: boolean }): void { + emitMessagesConsumed(localIds: string[], options?: { clearQueuedThinkingGrace?: boolean; steered?: boolean }): void { if (localIds.length === 0) return // `clearQueuedThinkingGrace` is an opt-in signal for the hub to drop // the 15s queued-thinking grace immediately. Only synchronous handlers @@ -1182,16 +1221,49 @@ export class ApiSessionClient extends EventEmitter { // inside `onUserMessage`) should set it — normal queue drains still // need the grace so the spinner doesn't flicker between drain and // backend.prompt start. - const payload: { sid: string; localIds: string[]; clearQueuedThinkingGrace?: boolean } = { + const payload: { + sid: string + localIds: string[] + clearQueuedThinkingGrace?: boolean + steered?: boolean + } = { sid: this.sessionId, localIds } if (options?.clearQueuedThinkingGrace) { payload.clearQueuedThinkingGrace = true } + if (options?.steered) { + payload.steered = true + } this.emitOrQueue(() => this.socket.emit('messages-consumed', payload)) } + /** Persist the durable pre-dispatch/restore state with a hub ACK. */ + async setSteerDeliveryState(localIds: string[], state: 'queued' | 'dispatching'): Promise { + if (localIds.length === 0 || this.state !== 'active') return false + try { + const response = await this.socket.timeout(5_000).emitWithAck('messages-steer-state', { + sid: this.sessionId, + localIds, + state + }) + return response?.ok === true + } catch (error) { + logger.debug('[API] Failed to persist steer delivery state', error) + return false + } + } + + /** Persist a steer whose transport completed ambiguously; no consumed ACK. */ + emitSteerIndeterminate(localIds: string[]): void { + if (localIds.length === 0) return + this.emitOrQueue(() => this.socket.emit('messages-indeterminate', { + sid: this.sessionId, + localIds + })) + } + sendSessionDeath(reason?: SessionEndReason): void { if (this.state === 'active') { void cleanupUploadDir(this.sessionId) diff --git a/cli/src/codex/appServerTypes.ts b/cli/src/codex/appServerTypes.ts index 27cbfe97..7753ef74 100644 --- a/cli/src/codex/appServerTypes.ts +++ b/cli/src/codex/appServerTypes.ts @@ -295,6 +295,18 @@ export interface ThreadRollbackResponse { [key: string]: unknown; } +export interface TurnSteerParams { + threadId: string; + input: UserInput[]; + expectedTurnId: string; + clientUserMessageId?: string | null; +} + +export interface TurnSteerResponse { + turnId: string; + [key: string]: unknown; +} + export interface ThreadCompactStartParams { threadId: string; } diff --git a/cli/src/codex/codexAppServerClient.test.ts b/cli/src/codex/codexAppServerClient.test.ts index b144265e..bd9c0ad5 100644 --- a/cli/src/codex/codexAppServerClient.test.ts +++ b/cli/src/codex/codexAppServerClient.test.ts @@ -28,7 +28,7 @@ vi.mock('@/ui/logger', () => ({ logger: { debug: vi.fn() } })); -import { CodexAppServerClient } from './codexAppServerClient'; +import { CodexAppServerClient, isIndeterminateError } from './codexAppServerClient'; function fakeStream(): EventEmitter & { setEncoding: ReturnType } { return Object.assign(new EventEmitter(), { setEncoding: vi.fn() }); @@ -61,4 +61,87 @@ describe('CodexAppServerClient process cwd', () => { ); await client.disconnect(); }); + + it('steerTurn resolves dispatch on stdin accept and completes with the turn response', async () => { + const child = fakeChild(); + child.stdin.write = vi.fn((_data: unknown, cb?: (error?: Error | null) => void) => { + cb?.(); + return true; + }); + spawnMock.mockReturnValue(child); + const client = new CodexAppServerClient({ cwd: '/neutral-home' }); + + await client.connect(); + const steer = await client.steerTurn({ + threadId: 'thread-1', + input: [{ type: 'text', text: 'pivot now' }], + expectedTurnId: 'turn-1', + clientUserMessageId: 'local-1' + }); + await steer.dispatched; + + const written = child.stdin.write.mock.calls[0]?.[0] as string; + const payload = JSON.parse(written); + expect(payload).toEqual(expect.objectContaining({ + method: 'turn/steer', + params: expect.objectContaining({ clientUserMessageId: 'local-1' }) + })); + + // App-server completes the turn after the inject. + child.stdout.emit('data', Buffer.from(JSON.stringify({ id: payload.id, result: { turnId: 'turn-1' } }) + '\n')); + await expect(steer.completed).resolves.toEqual({ turnId: 'turn-1' }); + await client.disconnect(); + }); + + it('times out a stalled stdin dispatch instead of leaving it pending', async () => { + vi.useFakeTimers(); + try { + const child = fakeChild(); + child.stdin.write = vi.fn(() => true); + spawnMock.mockReturnValue(child); + const client = new CodexAppServerClient({ cwd: '/neutral-home' }); + + await client.connect(); + const steer = await client.steerTurn({ + threadId: 'thread-1', + input: [{ type: 'text', text: 'x' }], + expectedTurnId: 'turn-1' + }); + const dispatched = expect(steer.dispatched).rejects.toThrow("timed out after 20000ms"); + const completed = expect(steer.completed).rejects.toThrow("timed out after 20000ms"); + await vi.advanceTimersByTimeAsync(20_000); + await dispatched; + await completed; + await client.disconnect(); + } finally { + vi.useRealTimers(); + } + }); + + it('steerTurn rejects dispatch when stdin write fails', async () => { + const child = fakeChild(); + child.stdin.write = vi.fn((_data: unknown, cb?: (error?: Error | null) => void) => { + cb?.(new Error('stdin closed')); + return true; + }); + spawnMock.mockReturnValue(child); + const client = new CodexAppServerClient({ cwd: '/neutral-home' }); + + await client.connect(); + const steer = await client.steerTurn({ + threadId: 'thread-1', + input: [{ type: 'text', text: 'x' }], + expectedTurnId: 'turn-1' + }); + let dispatchedError: unknown; + try { + await steer.dispatched; + } catch (error) { + dispatchedError = error; + } + expect(dispatchedError).toBeInstanceOf(Error); + expect(isIndeterminateError(dispatchedError)).toBe(true); + await expect(steer.completed).rejects.toThrow('stdin closed'); + await client.disconnect(); + }); }); diff --git a/cli/src/codex/codexAppServerClient.ts b/cli/src/codex/codexAppServerClient.ts index a70cf06c..801c32d5 100644 --- a/cli/src/codex/codexAppServerClient.ts +++ b/cli/src/codex/codexAppServerClient.ts @@ -25,6 +25,8 @@ import type { TurnInterruptResponse, ThreadRollbackParams, ThreadRollbackResponse, + TurnSteerParams, + TurnSteerResponse, ThreadCompactStartParams, ThreadCompactStartResponse, ThreadGoalSetParams, @@ -63,9 +65,18 @@ type RequestHandler = (params: unknown) => Promise | unknown; type PendingRequest = { resolve: (value: unknown) => void; reject: (error: Error) => void; + rejectDispatched: (error: Error) => void; cleanup: () => void; }; +/** Marks transport-level failures whose steer outcome is unknown. */ +export const INDETERMINATE_SYMBOL = Symbol('codex-app-server-indeterminate'); + +export function isIndeterminateError(error: unknown): boolean { + return typeof error === 'object' && error !== null + && (error as Record)[INDETERMINATE_SYMBOL] === true; +} + type CodexAppServerClientOptions = { cwd?: string; }; @@ -164,10 +175,22 @@ function resolveCodexAppServerCommand(): string { export class CodexAppServerClient extends JsonLineParser { private process: ChildProcessWithoutNullStreams | null = null; private connected = false; + private initialized = false; + + /** True while the app-server process is connected. */ + isConnected(): boolean { + return this.connected; + } + + /** True after a successful initialize round-trip on the current process. */ + isInitialized(): boolean { + return this.initialized; + } private nextId = 1; private readonly pending = new Map(); private readonly requestHandlers = new Map(); private notificationHandler: ((method: string, params: unknown) => void) | null = null; + private transportAbandonedHandler: (() => void) | null = null; private stderrHandler: ((text: string) => void) | null = null; private protocolError: Error | null = null; @@ -188,7 +211,7 @@ export class CodexAppServerClient extends JsonLineParser { const codexCommand = resolveCodexAppServerCommand(); logger.debug(`[CodexAppServer] Starting ${codexCommand} app-server`); - this.process = spawn(codexCommand, ['app-server'], { + const child = spawn(codexCommand, ['app-server'], { cwd: this.options.cwd, env: Object.keys(process.env).reduce((acc, key) => { const value = process.env[key]; @@ -199,12 +222,16 @@ export class CodexAppServerClient extends JsonLineParser { shell: process.platform === 'win32', windowsHide: process.platform === 'win32' }); + this.process = child; - this.process.stdout.setEncoding('utf8'); - this.process.stdout.on('data', (chunk) => this.feed(chunk)); + child.stdout.setEncoding('utf8'); + child.stdout.on('data', (chunk) => { + if (this.process === child) this.feed(chunk); + }); - this.process.stderr.setEncoding('utf8'); - this.process.stderr.on('data', (chunk) => { + child.stderr.setEncoding('utf8'); + child.stderr.on('data', (chunk) => { + if (this.process !== child) return; const text = chunk.toString().trim(); if (text.length > 0) { logger.debug(`[CodexAppServer][stderr] ${text}`); @@ -212,16 +239,20 @@ export class CodexAppServerClient extends JsonLineParser { } }); - this.process.on('exit', (code, signal) => { + child.on('exit', (code, signal) => { + if (this.process !== child) return; const message = `Codex app-server exited (code=${code ?? 'null'}, signal=${signal ?? 'null'})`; logger.debug(message); this.rejectAllPending(new Error(message)); this.connected = false; + this.initialized = false; this.resetParserState(); this.process = null; + this.transportAbandonedHandler?.(); }); - this.process.on('error', (error) => { + child.on('error', (error) => { + if (this.process !== child) return; logger.debug('[CodexAppServer] Process error', error); const message = error instanceof Error ? error.message : String(error); this.rejectAllPending(new Error( @@ -231,6 +262,7 @@ export class CodexAppServerClient extends JsonLineParser { this.connected = false; this.resetParserState(); this.process = null; + this.transportAbandonedHandler?.(); }); this.connected = true; @@ -241,6 +273,10 @@ export class CodexAppServerClient extends JsonLineParser { this.notificationHandler = handler; } + setTransportAbandonedHandler(handler: (() => void) | null): void { + this.transportAbandonedHandler = handler; + } + registerRequestHandler(method: string, handler: RequestHandler): void { this.requestHandlers.set(method, handler); } @@ -248,6 +284,7 @@ export class CodexAppServerClient extends JsonLineParser { async initialize(params: InitializeParams): Promise { const response = await this.sendRequest('initialize', params, { timeoutMs: 30_000 }); this.sendNotification('initialized'); + this.initialized = true; return response as InitializeResponse; } @@ -351,6 +388,24 @@ export class CodexAppServerClient extends JsonLineParser { return response as ThreadRollbackResponse; } + async steerTurn( + params: TurnSteerParams, + options?: { signal?: AbortSignal } + ): Promise<{ dispatched: Promise; completed: Promise }> { + // Dispatch/complete split: the caller awaits acceptance before + // reporting success. Bound the wait below the hub's 30s RPC timeout so + // a lost response cannot strand the row in dispatching — a timeout is + // indeterminate and funnels into thread reconciliation. + const request = await this.sendRequestWithDispatch('turn/steer', params, { + signal: options?.signal, + timeoutMs: 20_000 + }); + return { + dispatched: request.dispatched, + completed: request.completed.then((response) => response as TurnSteerResponse) + }; + } + async compactThread( params: ThreadCompactStartParams, options?: { signal?: AbortSignal } @@ -413,6 +468,7 @@ export class CodexAppServerClient extends JsonLineParser { } finally { this.rejectAllPending(new Error('Codex app-server disconnected')); this.connected = false; + this.initialized = false; this.resetParserState(); } @@ -424,9 +480,30 @@ export class CodexAppServerClient extends JsonLineParser { params?: unknown, options?: { signal?: AbortSignal; timeoutMs?: number } ): Promise { + const request = await this.sendRequestWithDispatch(method, params, options); + void request.dispatched.catch(() => {}); + return request.completed; + } + + /** + * Split a request into transport dispatch (stdin accepted) and completion + * (JSON-RPC response). Lets callers commit queue state once stdin accepted + * the request without waiting for the (possibly long-running) response. + */ + private async sendRequestWithDispatch( + method: string, + params?: unknown, + options?: { signal?: AbortSignal; timeoutMs?: number } + ): Promise<{ dispatched: Promise; completed: Promise }> { + if (options?.signal?.aborted) { + throw createAbortError(); + } if (!this.connected) { await this.connect(); } + if (options?.signal?.aborted) { + throw createAbortError(); + } const id = this.nextId++; const payload: JsonRpcLiteRequest = { @@ -437,60 +514,134 @@ export class CodexAppServerClient extends JsonLineParser { const timeoutMs = options?.timeoutMs ?? CodexAppServerClient.DEFAULT_TIMEOUT_MS; - return new Promise((resolve, reject) => { - let timeout: ReturnType | null = null; - let aborted = false; - - const cleanup = () => { - if (timeout) { - clearTimeout(timeout); - } - if (options?.signal) { - options.signal.removeEventListener('abort', onAbort); - } - }; - - const onAbort = () => { - if (aborted) return; - aborted = true; - this.pending.delete(id); - cleanup(); - reject(createAbortError()); - }; + let timeout: ReturnType | null = null; + let resolveDispatched!: () => void; + let rejectDispatched!: (error: Error) => void; + let resolveCompleted!: (value: unknown) => void; + let rejectCompleted!: (error: Error) => void; + const dispatched = new Promise((resolve, reject) => { + resolveDispatched = resolve; + rejectDispatched = reject; + }); + const completed = new Promise((resolve, reject) => { + resolveCompleted = resolve; + rejectCompleted = reject; + }); + let aborted = false; + let dispatchSettled = false; + const cleanup = () => { + if (timeout) { + clearTimeout(timeout); + } if (options?.signal) { - if (options.signal.aborted) { - onAbort(); + options.signal.removeEventListener('abort', onAbort); + } + }; + + const abandonUnconfirmedDispatch = (error: Error) => { + const child = this.process; + this.process = null; + this.connected = false; + this.initialized = false; + try { + child?.stdin.destroy(); + } catch (destroyError) { + logger.debug('[CodexAppServer] Error destroying stalled stdin', destroyError); + } + this.rejectAllPending(error); + this.resetParserState(); + this.transportAbandonedHandler?.(); + }; + + const failRequest = (error: Error, abandon = false) => { + if (abandon) { + abandonUnconfirmedDispatch(error); + return; + } + this.pending.delete(id); + cleanup(); + if (!dispatchSettled) { + dispatchSettled = true; + rejectDispatched(error); + } + rejectCompleted(error); + }; + + const onAbort = () => { + if (aborted) return; + aborted = true; + const error = this.markIndeterminate(createAbortError()); + if (dispatchSettled) { + failRequest(error); + } else { + failRequest(error, true); + } + }; + + if (options?.signal) { + if (options.signal.aborted) { + onAbort(); + return { dispatched, completed }; + } + options.signal.addEventListener('abort', onAbort, { once: true }); + } + + if (Number.isFinite(timeoutMs) && !aborted) { + timeout = setTimeout(() => { + if (this.pending.has(id)) { + const error = this.markIndeterminate(new Error(`Codex app-server request '${method}' timed out after ${timeoutMs}ms`)); + failRequest(error, !dispatchSettled); + } + }, timeoutMs); + timeout.unref(); + } + + this.pending.set(id, { + resolve: (value) => { + cleanup(); + if (!dispatchSettled) { + dispatchSettled = true; + resolveDispatched(); + } + resolveCompleted(value); + }, + reject: (error) => { + cleanup(); + if (!dispatchSettled) { + dispatchSettled = true; + resolveDispatched(); + } + rejectCompleted(error); + }, + rejectDispatched: (error) => { + if (!dispatchSettled) { + dispatchSettled = true; + rejectDispatched(error); + } + }, + cleanup + }); + + try { + const serialized = JSON.stringify(payload); + this.process?.stdin.write(`${serialized}\n`, (error) => { + if (error) { + const writeError = error instanceof Error ? error : new Error(String(error)); + failRequest(this.markIndeterminate(writeError), !dispatchSettled); return; } - options.signal.addEventListener('abort', onAbort, { once: true }); - } - - if (Number.isFinite(timeoutMs)) { - timeout = setTimeout(() => { - if (this.pending.has(id)) { - this.pending.delete(id); - cleanup(); - reject(new Error(`Codex app-server request '${method}' timed out after ${timeoutMs}ms`)); - } - }, timeoutMs); - timeout.unref(); - } - - this.pending.set(id, { - resolve: (value) => { - cleanup(); - resolve(value); - }, - reject: (error) => { - cleanup(); - reject(error); - }, - cleanup + if (!dispatchSettled) { + dispatchSettled = true; + resolveDispatched(); + } }); + } catch (error) { + const writeError = error instanceof Error ? error : new Error(String(error)); + failRequest(writeError); + } - this.writePayload(payload); - }); + return { dispatched, completed }; } private sendNotification(method: string, params?: unknown): void { @@ -604,6 +755,15 @@ export class CodexAppServerClient extends JsonLineParser { pending.resolve(response.result); } + /** Marks transport-level failures (timeout, disconnect, spawn, protocol) + * whose outcome is unknown — unlike an explicit JSON-RPC error response. + * Steer completion uses this to distinguish definite rejection from + * indeterminate outcomes. */ + private markIndeterminate(error: Error): Error { + Object.defineProperty(error, INDETERMINATE_SYMBOL, { value: true }); + return error; + } + private writePayload(payload: JsonRpcLiteRequest | JsonRpcLiteNotification | JsonRpcLiteResponse): void { const serialized = JSON.stringify(payload); this.process?.stdin.write(`${serialized}\n`); @@ -615,8 +775,10 @@ export class CodexAppServerClient extends JsonLineParser { } private rejectAllPending(error: Error): void { - for (const { reject, cleanup } of this.pending.values()) { + error = this.markIndeterminate(error); + for (const { reject, rejectDispatched, cleanup } of this.pending.values()) { cleanup(); + rejectDispatched(error); reject(error); } this.pending.clear(); diff --git a/cli/src/codex/codexRemoteLauncher.test.ts b/cli/src/codex/codexRemoteLauncher.test.ts index 7d325e8b..a8d9d83b 100644 --- a/cli/src/codex/codexRemoteLauncher.test.ts +++ b/cli/src/codex/codexRemoteLauncher.test.ts @@ -57,6 +57,12 @@ const harness = vi.hoisted(() => ({ failNextCompact: false, deferCompactCompletion: false, deferThreadStatusNotifications: false, + steerTurnParams: [] as Array>, + steerDispatchError: null as Error | null, + steerCompletionError: null as Error | null, + readThreadParams: [] as Array>, + readThreadError: null as Error | null, + readThreadResponse: { thread: { turns: [] as unknown[] } } as { thread: { turns?: Array<{ items?: Array> }> } }, emitStaleTaskCompleteAfterRetry: false, emitStaleTaskFailedAfterRetry: false, emitStaleThreadStatusFailureAfterRetry: false, @@ -90,12 +96,21 @@ const harness = vi.hoisted(() => ({ })); vi.mock('./codexAppServerClient', () => { + const INDETERMINATE_SYMBOL = Symbol('codex-app-server-indeterminate'); class MockCodexAppServerClient { private notificationHandler: ((method: string, params: unknown) => void) | null = null; private stderrHandler: ((text: string) => void) | null = null; async connect(): Promise {} + isConnected(): boolean { + return true; + } + + isInitialized(): boolean { + return true; + } + async initialize(params: unknown): Promise<{ protocolVersion: number }> { harness.initializeCalls.push(params); return { protocolVersion: 1 }; @@ -106,6 +121,8 @@ vi.mock('./codexAppServerClient', () => { harness.dispatchNotification = handler; } + setTransportAbandonedHandler(_handler: (() => void) | null): void {} + setStderrHandler(handler: ((text: string) => void) | null): void { this.stderrHandler = handler; } @@ -195,6 +212,33 @@ vi.mock('./codexAppServerClient', () => { return { goal: harness.goal }; } + async steerTurn(params?: { threadId?: string; input?: unknown[]; expectedTurnId?: string; clientUserMessageId?: string }): Promise<{ + dispatched: Promise; + completed: Promise; + }> { + harness.steerTurnParams.push((params ?? {}) as Record); + if (harness.steerDispatchError) { + return { + dispatched: Promise.reject(harness.steerDispatchError), + completed: Promise.reject(harness.steerDispatchError) + }; + } + return { + dispatched: Promise.resolve(), + completed: harness.steerCompletionError + ? Promise.reject(harness.steerCompletionError) + : Promise.resolve({ turnId: 'steered-turn' }) + }; + } + + async readThread(params?: { threadId?: string; includeTurns?: boolean }): Promise { + harness.readThreadParams.push((params ?? {}) as Record); + if (harness.readThreadError) { + throw harness.readThreadError; + } + return harness.readThreadResponse; + } + async clearThreadGoal(params?: { threadId?: string }): Promise<{ cleared: boolean }> { harness.goalClearCalls.push(params ?? {}); const cleared = harness.goal !== null; @@ -1028,7 +1072,13 @@ vi.mock('./codexAppServerClient', () => { async disconnect(): Promise {} } - return { CodexAppServerClient: MockCodexAppServerClient }; + return { + CodexAppServerClient: MockCodexAppServerClient, + INDETERMINATE_SYMBOL, + isIndeterminateError: (error: unknown) => Boolean( + error && (error as Record)[INDETERMINATE_SYMBOL] === true + ) + }; }); vi.mock('./utils/buildHapiMcpBridge', () => ({ @@ -1043,7 +1093,9 @@ vi.mock('./utils/buildHapiMcpBridge', () => ({ } })); -import { codexRemoteLauncher } from './codexRemoteLauncher'; +import { codexRemoteLauncher, isCurrentSteerHandler } from './codexRemoteLauncher'; +import { INDETERMINATE_SYMBOL } from './codexAppServerClient'; +import { RPC_METHODS } from '@hapi/protocol/rpcMethods'; type FakeAgentState = { requests: Record; @@ -1061,7 +1113,8 @@ function createMode(): EnhancedMode { function createSessionStub( messages = ['hello from launcher test'], mode = createMode(), - isolateMessages = false + isolateMessages = false, + closeQueue = true ) { const queue = new MessageQueue2((mode) => JSON.stringify(mode)); messages.forEach((message, index) => { @@ -1073,7 +1126,9 @@ function createSessionStub( queue.push(message, mode); } }); - queue.close(); + if (closeQueue) { + queue.close(); + } const sessionEvents: Array<{ type: string; [key: string]: unknown }> = []; const codexMessages: unknown[] = []; @@ -1111,7 +1166,10 @@ function createSessionStub( }, sendSessionEvent(event: { type: string; [key: string]: unknown }) { sessionEvents.push(event); - } + }, + emitMessagesConsumed: vi.fn(), + emitSteerIndeterminate: vi.fn(), + setSteerDeliveryState: vi.fn(async () => true) }; const session = { @@ -1174,6 +1232,8 @@ function createSessionStub( foundSessionIds, resetThreadCalls, rpcHandlers, + emitMessagesConsumed: client.emitMessagesConsumed, + emitSteerIndeterminate: client.emitSteerIndeterminate, setPermissionMode: (nextMode: EnhancedMode['permissionMode']) => { currentPermissionMode = nextMode; }, @@ -1186,6 +1246,147 @@ function createSessionStub( } describe('codexRemoteLauncher', () => { + it('invalidates queued steer handlers after abort or cleanup', () => { + expect(isCurrentSteerHandler(3, 3, false)).toBe(true); + expect(isCurrentSteerHandler(4, 3, false)).toBe(false); + expect(isCurrentSteerHandler(3, 3, true)).toBe(false); + }); + + it('steers a queued message into the active turn and acks on dispatch', async () => { + harness.suppressTurnCompletion = true; + const { session, rpcHandlers, emitMessagesConsumed } = createSessionStub(['first'], createMode(), false, false); + const runPromise = codexRemoteLauncher(session as never); + await vi.waitFor(() => expect(harness.startTurnThreadIds.length).toBe(1)); + + session.queue.push('steer me', createMode(), 'local-1'); + const handler = rpcHandlers.get(RPC_METHODS.SteerQueuedMessage)!; + const result = await handler({ localId: 'local-1' }); + + expect(result).toEqual({ steered: true }); + expect(harness.steerTurnParams).toHaveLength(1); + expect(harness.steerTurnParams[0]).toMatchObject({ + expectedTurnId: 'turn-1', + clientUserMessageId: 'local-1' + }); + await vi.waitFor(() => expect(emitMessagesConsumed).toHaveBeenCalledWith(['local-1'], { steered: true })); + // The suppressed turn never completes, so the launcher stays busy; + // close the queue and leave the run promise unawaited. + session.queue.close(); + }); + + it('restores the row when the app-server explicitly rejects after dispatch', async () => { + harness.suppressTurnCompletion = true; + // Plain Error without the indeterminate marker = definite JSON-RPC + // rejection: the instruction was not accepted, so restoring is safe. + harness.steerCompletionError = new Error('turn/steer not allowed here'); + const { session, rpcHandlers, emitMessagesConsumed } = createSessionStub(['first'], createMode(), false, false); + const runPromise = codexRemoteLauncher(session as never); + await vi.waitFor(() => expect(harness.startTurnThreadIds.length).toBe(1)); + + session.queue.push('steer me', createMode(), 'local-1'); + const handler = rpcHandlers.get(RPC_METHODS.SteerQueuedMessage)!; + const result = await handler({ localId: 'local-1' }); + + // Explicit rejection surfaces as failed (not a false steered), and the + // row is restored so the normal turn/start path delivers it later. + expect(result).toEqual({ steered: false, error: 'turn/steer not allowed here' }); + await vi.waitFor(() => expect(session.queue.cancelByLocalId('local-1')).toBe(true)); + expect(emitMessagesConsumed).not.toHaveBeenCalledWith(['local-1'], { steered: true }); + session.queue.close(); + }); + + it('holds an indeterminate steer for explicit retry or cancel', async () => { + harness.suppressTurnCompletion = true; + const indeterminate = new Error('Codex app-server disconnected'); + Object.defineProperty(indeterminate, INDETERMINATE_SYMBOL, { value: true }); + harness.steerCompletionError = indeterminate; + // Reconciliation cannot read the thread either. The row is held out of + // automatic replay and the user can explicitly retry or cancel it. + harness.readThreadError = new Error('app-server unreachable'); + const { session, rpcHandlers, emitMessagesConsumed, emitSteerIndeterminate } = createSessionStub(['first'], createMode(), false, false); + const runPromise = codexRemoteLauncher(session as never); + await vi.waitFor(() => expect(harness.startTurnThreadIds.length).toBe(1)); + + session.queue.push('steer me', createMode(), 'local-1'); + const handler = rpcHandlers.get(RPC_METHODS.SteerQueuedMessage)!; + const result = await handler({ localId: 'local-1' }); + + // Transport failure after dispatch: report reconciliation, persist the + // ambiguous state, and remove it from the automatic queue. + expect(result).toEqual({ steered: false, error: 'Steer outcome is being reconciled' }); + expect(emitSteerIndeterminate).toHaveBeenCalledWith(['local-1']); + expect(session.queue.cancelByLocalId('local-1')).toBe(true); + expect(emitMessagesConsumed).not.toHaveBeenCalledWith(['local-1'], { steered: true }); + session.queue.close(); + }); + + it('reconciles an indeterminate steer to accepted once the thread shows the message', async () => { + harness.suppressTurnCompletion = true; + const indeterminate = new Error('Codex app-server disconnected'); + Object.defineProperty(indeterminate, INDETERMINATE_SYMBOL, { value: true }); + harness.steerCompletionError = indeterminate; + harness.readThreadResponse = { + thread: { + turns: [{ + items: [{ type: 'userMessage', clientId: 'local-1', text: 'steer me' }] + }] + } + }; + const { session, rpcHandlers, emitMessagesConsumed } = createSessionStub(['first'], createMode(), false, false); + const runPromise = codexRemoteLauncher(session as never); + await vi.waitFor(() => expect(harness.startTurnThreadIds.length).toBe(1)); + + session.queue.push('steer me', createMode(), 'local-1'); + const handler = rpcHandlers.get(RPC_METHODS.SteerQueuedMessage)!; + const result = await handler({ localId: 'local-1' }); + + expect(result).toEqual({ steered: false, error: 'Steer outcome is being reconciled' }); + // The scheduled reconciliation (1s timer) reads the thread, finds the + // message, commits the row and fires the badge — no restore, no + // duplicate. + await vi.waitFor(() => expect(emitMessagesConsumed).toHaveBeenCalledWith(['local-1'], { steered: true }), { timeout: 5_000 }); + // Row committed and consumed — the tombstone blocks a stale retry. + expect(session.queue.cancelByLocalId('local-1')).toBe('consumed'); + session.queue.close(); + }); + + it('reconciles when stdin dispatch fails indeterminately', async () => { + harness.suppressTurnCompletion = true; + const indeterminate = new Error('stdin callback timed out'); + Object.defineProperty(indeterminate, INDETERMINATE_SYMBOL, { value: true }); + harness.steerDispatchError = indeterminate; + harness.readThreadError = new Error('app-server unreachable'); + const { session, rpcHandlers, emitMessagesConsumed, emitSteerIndeterminate } = createSessionStub(['first'], createMode(), false, false); + const runPromise = codexRemoteLauncher(session as never); + await vi.waitFor(() => expect(harness.startTurnThreadIds.length).toBe(1)); + + session.queue.push('steer me', createMode(), 'local-1'); + const handler = rpcHandlers.get(RPC_METHODS.SteerQueuedMessage)!; + const result = await handler({ localId: 'local-1' }); + + expect(result).toEqual({ steered: false, error: 'Steer outcome is being reconciled' }); + expect(emitSteerIndeterminate).toHaveBeenCalledWith(['local-1']); + expect(session.queue.cancelByLocalId('local-1')).toBe(true); + expect(emitMessagesConsumed).not.toHaveBeenCalledWith(['local-1'], { steered: true }); + session.queue.close(); + }); + + it('restores the queued row when stdin dispatch fails', async () => { + harness.suppressTurnCompletion = true; + harness.steerDispatchError = new Error('stdin closed'); + const { session, rpcHandlers, emitMessagesConsumed } = createSessionStub(['first'], createMode(), false, false); + const runPromise = codexRemoteLauncher(session as never); + await vi.waitFor(() => expect(harness.startTurnThreadIds.length).toBe(1)); + + session.queue.push('steer me', createMode(), 'local-1'); + const handler = rpcHandlers.get(RPC_METHODS.SteerQueuedMessage)!; + const result = await handler({ localId: 'local-1' }); + + expect(result).toEqual({ steered: false, error: 'stdin closed' }); + await vi.waitFor(() => expect(session.queue.cancelByLocalId('local-1')).toBe(true)); + expect(emitMessagesConsumed).not.toHaveBeenCalledWith(['local-1'], { steered: true }); + session.queue.close(); + }); afterEach(() => { harness.notifications = []; harness.dispatchNotification = null; @@ -1241,6 +1442,12 @@ describe('codexRemoteLauncher', () => { harness.failNextCompact = false; harness.deferCompactCompletion = false; harness.deferThreadStatusNotifications = false; + harness.steerTurnParams = []; + harness.steerDispatchError = null; + harness.steerCompletionError = null; + harness.readThreadParams = []; + harness.readThreadError = null; + harness.readThreadResponse = { thread: { turns: [] } }; harness.emitStaleTaskCompleteAfterRetry = false; harness.emitStaleTaskFailedAfterRetry = false; harness.emitStaleThreadStatusFailureAfterRetry = false; diff --git a/cli/src/codex/codexRemoteLauncher.ts b/cli/src/codex/codexRemoteLauncher.ts index 2e145044..46a7706b 100644 --- a/cli/src/codex/codexRemoteLauncher.ts +++ b/cli/src/codex/codexRemoteLauncher.ts @@ -1,7 +1,7 @@ import React from 'react'; import { randomUUID } from 'node:crypto'; -import { CodexAppServerClient } from './codexAppServerClient'; +import { CodexAppServerClient, isIndeterminateError } from './codexAppServerClient'; import { CodexPermissionHandler } from './utils/permissionHandler'; import { ReasoningProcessor } from './utils/reasoningProcessor'; import { DiffProcessor } from './utils/diffProcessor'; @@ -11,6 +11,7 @@ import { buildHapiMcpBridge } from './utils/buildHapiMcpBridge'; import { emitReadyIfIdle } from './utils/emitReadyIfIdle'; import type { CodexSession } from './session'; import type { EnhancedMode } from './loop'; +import type { QueueReservation } from '@/utils/MessageQueue2'; import { hasCodexCliOverrides } from './utils/codexCliOverrides'; import { AppServerEventConverter } from './utils/appServerEventConverter'; import { registerGeneratedImageFromPath } from '@/modules/common/generatedImages'; @@ -212,6 +213,14 @@ function stripAnsi(value: string): string { return value.replace(/\u001b\[[0-9;]*m/g, ''); } +export function isCurrentSteerHandler( + currentEpoch: number, + handlerEpoch: number, + shouldExit: boolean +): boolean { + return currentEpoch === handlerEpoch && !shouldExit; +} + class CodexRemoteLauncher extends RemoteLauncherBase { private readonly session: CodexSession; private readonly appServerClient: CodexAppServerClient; @@ -220,6 +229,9 @@ class CodexRemoteLauncher extends RemoteLauncherBase { private diffProcessor: DiffProcessor | null = null; private happyServer: HappyServer | null = null; private abortController: AbortController = new AbortController(); + /** Invalidates queued-message steer handlers after abort or cleanup. */ + private steerEpoch = 0; + private abortInProgress = false; private currentThreadId: string | null = null; private currentTurnId: string | null = null; private readonly activeChildTurns = new Map(); @@ -276,13 +288,17 @@ class CodexRemoteLauncher extends RemoteLauncherBase { } private async handleAbort(): Promise { + this.abortInProgress = true; + this.steerEpoch++; logger.debug('[Codex] Abort requested - stopping current task'); try { await this.interruptActiveTurns('abort'); this.currentTurnId = null; this.abortController.abort(); - this.session.queue.reset(); + // A dispatched steer may still reconcile after Abort; preserve its + // reservation so a positive thread/read result can acknowledge it. + this.session.queue.reset({ preserveDispatchingReservations: true }); this.permissionHandler?.reset(); this.reasoningProcessor?.abort(); this.diffProcessor?.reset(); @@ -291,6 +307,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase { logger.debug('[Codex] Error during abort:', error); } finally { this.abortController = new AbortController(); + this.abortInProgress = false; } } @@ -671,6 +688,11 @@ class CodexRemoteLauncher extends RemoteLauncherBase { let scheduleReadyAfterTurn: (() => void) | null = null; let clearReadyAfterTurnTimer: (() => void) | null = null; let turnInFlight = false; + const setTurnInFlight = (value: boolean) => { + turnInFlight = value; + session.client.updateAgentState((state) => ({ ...state, steeringActive: value })); + }; + setTurnInFlight(false); let usageModel: string | null = null; let allowAnonymousTerminalEvent = false; let invalidThreadId: string | null = null; @@ -1875,6 +1897,18 @@ class CodexRemoteLauncher extends RemoteLauncherBase { waiter(); }; + appServerClient.setTransportAbandonedHandler(() => { + // The old process is gone; its foreground turn cannot deliver more + // notifications. Let the loop recover on a fresh app-server. + setTurnInFlight(false); + this.conversationHistory.setBusy(false); + if (session.thinking) session.onThinkingChange(false); + recoveryInFlight = false; + activeMessage = null; + this.currentTurnId = null; + wakeLoop(); + }); + const waitForTurnOrRecovery = (signal: AbortSignal): Promise => new Promise((resolve) => { if (!turnInFlight && !recoveryInFlight) { resolve(); @@ -1893,6 +1927,316 @@ class CodexRemoteLauncher extends RemoteLauncherBase { signal.addEventListener('abort', finish, { once: true }); }); + // Steers whose transport outcome is indeterminate (disconnect / protocol + // failure after dispatch). Reconciled independently of the main loop by + // a self-rescheduling timer: accepted → commit + consumed; still + // unreadable or no matching client id → retry later (absence of a + // durable client id is NOT proof of rejection). + const pendingSteerReconciliations = new Map; + batch: QueuedMessage; + expiresAt: number; + }>(); + let steerReconcileTimer: ReturnType | null = null; + let steerReconcileInProgress = false; + let shuttingDown = false; + + const settleReconcileEntry = (localId: string, entry: { taken: QueueReservation; batch: QueuedMessage }): void => { + // An explicit retry/cancel may have superseded this poll while its + // readThread request was in flight. + if (pendingSteerReconciliations.get(localId) !== entry) return; + // The hub already knows this row is indeterminate. Only a positive + // thread/read match may stamp it delivered. + const committed = session.queue.commitReservation(entry.taken); + pendingSteerReconciliations.delete(localId); + if (!committed) return; + messageBuffer.addMessage(entry.batch.message, 'user'); + session.client.emitMessagesConsumed([localId], { steered: true }); + }; + + const markReconcileEntryIndeterminate = (localId: string, entry: { taken: QueueReservation; batch: QueuedMessage }): void => { + // Keep the reservation out of the automatic queue, but leave an + // explicit retry/cancel path. The hub persists the same state so a + // runner restart cannot replay an unproven steer. + session.queue.markReservationIndeterminate(entry.taken); + session.client.emitSteerIndeterminate([localId]); + }; + + const runSteerReconciliation = async (): Promise => { + if (steerReconcileTimer) { + clearTimeout(steerReconcileTimer); + steerReconcileTimer = null; + } + if (shuttingDown || steerReconcileInProgress || pendingSteerReconciliations.size === 0) { + return; + } + steerReconcileInProgress = true; + try { + for (const [localId, entry] of Array.from(pendingSteerReconciliations.entries())) { + if (pendingSteerReconciliations.get(localId) !== entry) continue; + if (Date.now() >= entry.expiresAt) { + // Terminal outcome: keep the durable indeterminate state. + // Never convert missing client-id evidence into a + // messages-consumed ACK; the user must explicitly retry + // or cancel the held row. + logger.debug(`[Codex] steer ${localId} reconciliation expired; keeping indeterminate`); + pendingSteerReconciliations.delete(localId); + continue; + } + const outcome = await reconcileSteerByClientId(entry.threadId, localId); + if (outcome === 'accepted') { + settleReconcileEntry(localId, entry); + } + // 'unknown' → keep the entry; the timer reschedules below. + } + } finally { + steerReconcileInProgress = false; + if (!shuttingDown && pendingSteerReconciliations.size > 0) { + steerReconcileTimer = setTimeout(() => { + steerReconcileTimer = null; + void runSteerReconciliation(); + // Wake the main loop too (it may sit in + // waitForTurnOrRecovery while the turn is in flight). + wakeLoop(); + }, 1_000); + steerReconcileTimer.unref?.(); + } + } + }; + + const scheduleSteerReconcileRetry = () => { + if (shuttingDown) { + return; + } + if (steerReconcileTimer || steerReconcileInProgress) { + return; + } + steerReconcileTimer = setTimeout(() => { + steerReconcileTimer = null; + void runSteerReconciliation(); + }, 1_000); + steerReconcileTimer.unref?.(); + }; + + // thread/read auto-connects after a disconnect, but a freshly spawned + // app-server must be initialized before any request — the startup block + // only ran for the original process. + const ensureAppServerInitialized = async () => { + if (appServerClient.isConnected() && appServerClient.isInitialized()) { + return; + } + await appServerClient.connect(); + await appServerClient.initialize({ + clientInfo: { + name: 'hapi-codex-client', + version: '1.0.0' + }, + capabilities: { + experimentalApi: true + } + }); + }; + + // Returns 'accepted' when the thread contains the steered user message + // (echoed as userMessage.clientId). Absence of the id is ambiguous + // (client ids are not guaranteed durable), so anything unreadable or + // unmatched stays 'unknown' and is retried. + const reconcileSteerByClientId = async (threadId: string, localId: string): Promise<'accepted' | 'unknown'> => { + try { + await ensureAppServerInitialized(); + const response = await appServerClient.readThread( + { threadId, includeTurns: true }, + { signal: AbortSignal.timeout(5_000) } + ); + for (const turn of response.thread.turns ?? []) { + for (const item of turn.items ?? []) { + const record = asRecord(item); + // Accept both wire shapes the app-server has used for + // user messages (see conversationHistory.ts parsing). + const type = asString(record?.type) ?? asString(record?.itemType); + const clientId = asString(record?.clientId) ?? asString(record?.client_id); + if ((type === 'userMessage' || type === 'user_message') && clientId === localId) { + return 'accepted'; + } + } + } + return 'unknown'; + } catch (error) { + logger.debug(`[Codex] steer reconcile unavailable (${error instanceof Error ? error.message : String(error)})`); + return 'unknown'; + } + }; + + // Non-interrupting mid-turn inject via app-server `turn/steer`. + // Returns null when the turn ended or is review/compact (not steerable). + const trySteerActiveTurn = async ( + batch: QueuedMessage, + localId: string + ): Promise<{ dispatched: Promise; completed: Promise } | null> => { + const threadId = this.currentThreadId; + const turnId = this.currentTurnId; + if (!threadId || !turnId || !turnInFlight) { + return null; + } + try { + return await appServerClient.steerTurn({ + threadId, + input: [{ type: 'text', text: batch.message }], + expectedTurnId: turnId, + // Echoed back as userMessage.clientId — lets an ambiguous + // transport failure reconcile the thread later. + clientUserMessageId: localId + }, { signal: this.abortController.signal }); + } catch (error) { + const detail = error instanceof Error ? error.message : String(error); + logger.debug(`[Codex] turn/steer failed (${detail})`); + return null; + } + }; + + // Per-message steer from the waiting queue (web "Steer" button). + session.client.rpcHandlerManager.registerHandler( + RPC_METHODS.SteerQueuedMessage, + async (payload: unknown) => { + const localId = typeof (payload as { localId?: unknown } | null)?.localId === 'string' + ? (payload as { localId: string }).localId + : ''; + if (!localId) { + return { steered: false, error: 'Missing localId' }; + } + if (this.abortInProgress || !turnInFlight || !this.currentThreadId || !this.currentTurnId) { + return { steered: false, error: 'No active steerable turn' }; + } + // Reserve before awaiting turn/steer so the main loop cannot + // collect the same row for turn/start while steer is in flight. + const taken = session.queue.takeByLocalId(localId); + if (!taken) { + return { steered: false, error: 'Message not in queue' }; + } + // An explicit retry supersedes any old reconciliation poll for + // the same held row. + pendingSteerReconciliations.delete(localId); + const isControlCommand = Boolean(taken.item.isolate) + || Boolean(parseCodexSpecialCommand(taken.item.message).type); + if (isControlCommand) { + session.queue.restoreReservation(taken); + return { steered: false, error: 'Control commands cannot be steered' }; + } + if (activeMessage?.hash !== taken.item.modeHash) { + session.queue.restoreReservation(taken); + return { steered: false, error: 'Queued message mode differs from the active turn' }; + } + const batch: QueuedMessage = { + message: taken.item.message, + mode: taken.item.mode, + isolate: Boolean(taken.item.isolate), + hash: taken.item.modeHash + }; + const steerEpoch = this.steerEpoch; + // Pin the thread this steer targets: reconciliation must look at + // the steer's thread, not whichever turn is current later. + const steerThreadId = this.currentThreadId; + const steerTurnId = this.currentTurnId; + if (!session.queue.beginReservationDispatch(taken)) { + return { steered: false, error: 'Steer cancelled' }; + } + const dispatchStatePersisted = await session.client.setSteerDeliveryState([localId], 'dispatching'); + if (!dispatchStatePersisted) { + session.queue.markReservationIndeterminate(taken); + session.client.emitSteerIndeterminate([localId]); + return { steered: false, error: 'Steer state is indeterminate' }; + } + const restoreQueuedReservation = async (): Promise => { + if (!taken.originIndeterminate) { + const persisted = await session.client.setSteerDeliveryState([localId], 'queued'); + if (!persisted) { + session.queue.markReservationIndeterminate(taken); + session.client.emitSteerIndeterminate([localId]); + return false; + } + } + if (taken.state !== 'dispatching' || !session.queue.restoreReservation(taken)) { + session.client.emitSteerIndeterminate([localId]); + return false; + } + return true; + }; + if (taken.state !== 'dispatching') { + session.client.emitSteerIndeterminate([localId]); + return { steered: false, error: 'Steer cancelled' }; + } + if (!turnInFlight + || this.currentThreadId !== steerThreadId + || this.currentTurnId !== steerTurnId + || !isCurrentSteerHandler(this.steerEpoch, steerEpoch, this.shouldExit)) { + await restoreQueuedReservation(); + return { steered: false, error: 'Active turn changed' }; + } + const steer = await trySteerActiveTurn(batch, localId); + if (steer) { + const reconcileDispatchedSteer = ( + localId: string, + taken: QueueReservation, + batch: QueuedMessage + ) => { + const threadId = steerThreadId; + if (!threadId) { + session.queue.markReservationIndeterminate(taken); + session.client.emitSteerIndeterminate([localId]); + return; + } + const entry = { threadId, taken, batch, expiresAt: Date.now() + 60_000 }; + pendingSteerReconciliations.set(localId, entry); + markReconcileEntryIndeterminate(localId, entry); + scheduleSteerReconcileRetry(); + }; + try { + await steer.dispatched; + } catch (error) { + // A stalled/aborted stdin callback is indeterminate: the + // bytes may have reached Codex even though dispatch did + // not report success. Reconcile instead of replaying. + void steer.completed.catch(() => {}); + if (isIndeterminateError(error)) { + reconcileDispatchedSteer(localId, taken, batch); + return { steered: false, error: 'Steer outcome is being reconciled' }; + } + await restoreQueuedReservation(); + return { steered: false, error: error instanceof Error ? error.message : 'Steer failed' }; + } + try { + // Await app-server acceptance before reporting success: + // an explicit JSON-RPC rejection must surface as failed + // (row restored, normal turn/start delivers it later) + // rather than a false steered. turn/steer's response is + // the inject acceptance, not the full turn completion. + await steer.completed; + } catch (error) { + if (isIndeterminateError(error)) { + // Transport failure after dispatch: the outcome is + // unknown — keep the row reserved and reconcile the + // thread once the app-server is reachable again. + reconcileDispatchedSteer(localId, taken, batch); + return { steered: false, error: 'Steer outcome is being reconciled' }; + } + await restoreQueuedReservation(); + return { steered: false, error: error instanceof Error ? error.message : 'Steer failed' }; + } + session.queue.commitReservation(taken); + messageBuffer.addMessage(batch.message, 'user'); + session.client.emitMessagesConsumed([localId], { steered: true }); + return { steered: true }; + } + if (!isCurrentSteerHandler(this.steerEpoch, steerEpoch, this.shouldExit)) { + await restoreQueuedReservation(); + return { steered: false, error: 'Steer cancelled' }; + } + await restoreQueuedReservation(); + return { steered: false, error: 'Active turn is not steerable' }; + } + ); + const clearCompactRecovery = (recovery: typeof compactRecovery) => { if (!recovery) { return; @@ -2198,7 +2542,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase { }); lastFinalizedTurnId = request.turnId; - turnInFlight = false; + setTurnInFlight(false); allowAnonymousTerminalEvent = false; this.currentTurnId = null; sameThreadRetryAttempt = 0; @@ -2221,7 +2565,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase { } catch (error) { if (interrupted) { lastFinalizedTurnId = request.turnId; - turnInFlight = false; + setTurnInFlight(false); allowAnonymousTerminalEvent = false; this.currentTurnId = null; activeMessage = null; @@ -2746,7 +3090,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase { if (msgType === 'task_started') { clearReadyAfterTurnTimer?.(); - turnInFlight = true; + setTurnInFlight(true); if (!eventTurnId && !this.currentTurnId) { allowAnonymousTerminalEvent = true; } @@ -2756,7 +3100,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase { } } if (isTerminalEvent) { - turnInFlight = false; + setTurnInFlight(false); this.conversationHistory.setBusy(false); allowAnonymousTerminalEvent = false; if (session.thinking) { @@ -3340,7 +3684,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase { const resetCurrentTurnState = () => { clearDeferredThreadStatusFailure(); cancelSafetyBufferingRequest('Session reset'); - turnInFlight = false; + setTurnInFlight(false); allowAnonymousTerminalEvent = false; this.currentTurnId = null; permissionHandler.reset(); @@ -3646,6 +3990,12 @@ class CodexRemoteLauncher extends RemoteLauncherBase { while (!this.shouldExit) { logActiveHandles('loop-top'); + if (!appServerClient.isConnected() || !appServerClient.isInitialized()) { + await ensureAppServerInitialized(); + } + if (pendingSteerReconciliations.size > 0) { + await runSteerReconciliation(); + } if (!pending && recoveryInFlight) { await waitForTurnOrRecovery(this.abortController.signal); if (this.abortController.signal.aborted && !this.shouldExit) { @@ -3790,7 +4140,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase { } } - turnInFlight = true; + setTurnInFlight(true); this.conversationHistory.setBusy(true); allowAnonymousTerminalEvent = false; const mode = { @@ -3870,7 +4220,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase { } catch (error) { logger.warn('Error in codex session:', error); const isAbortError = error instanceof Error && error.name === 'AbortError'; - turnInFlight = false; + setTurnInFlight(false); this.conversationHistory.setBusy(false); allowAnonymousTerminalEvent = false; this.currentTurnId = null; @@ -3922,10 +4272,19 @@ class CodexRemoteLauncher extends RemoteLauncherBase { clearDeferredThreadStatusFailure(); cancelSafetyBufferingRequest('Session ended'); cancelAllPendingThrottledAgentRunUpdates(); + // Stop reconciliation: the launcher is leaving; no pending steer may + // spawn a fresh app-server after cleanup. + shuttingDown = true; + if (steerReconcileTimer) { + clearTimeout(steerReconcileTimer); + steerReconcileTimer = null; + } + pendingSteerReconciliations.clear(); } protected async cleanup(): Promise { logger.debug('[codex-remote]: cleanup start'); + this.appServerClient.setTransportAbandonedHandler(null); this.appServerClient.setStderrHandler(null); try { await this.appServerClient.disconnect(); diff --git a/cli/src/codex/runCodex.test.ts b/cli/src/codex/runCodex.test.ts index 6449f27c..94a59269 100644 --- a/cli/src/codex/runCodex.test.ts +++ b/cli/src/codex/runCodex.test.ts @@ -17,6 +17,7 @@ const harness = vi.hoisted(() => ({ session: { onUserMessage: vi.fn(), onCancelQueuedMessage: vi.fn(), + onRetryQueuedMessage: vi.fn(), rpcHandlerManager: { registerHandler: vi.fn() } diff --git a/cli/src/codex/runCodex.ts b/cli/src/codex/runCodex.ts index aa3da7cb..a931b197 100644 --- a/cli/src/codex/runCodex.ts +++ b/cli/src/codex/runCodex.ts @@ -307,6 +307,7 @@ export async function runCodex(opts: { logger.debug(`[codex] cancelByLocalId(${localId}): ${removed ? 'removed' : 'not found (best-effort)'}`); return removed; }); + session.onRetryQueuedMessage((localId) => messageQueue.releaseIndeterminateReservation(localId)); const formatFailureReason = (message: string): string => { const maxLength = 200; diff --git a/cli/src/utils/MessageQueue2.test.ts b/cli/src/utils/MessageQueue2.test.ts index d54d621d..d1dee25e 100644 --- a/cli/src/utils/MessageQueue2.test.ts +++ b/cli/src/utils/MessageQueue2.test.ts @@ -573,6 +573,95 @@ describe('MessageQueue2', () => { }); }); + it('restores a reserved item relative to surviving neighbors', async () => { + const queue = new MessageQueue2(mode => mode); + queue.push('a', 'A', 'a'); + queue.push('b', 'B', 'b'); + queue.push('c', 'B', 'c'); + + const reservation = queue.takeByLocalId('b'); + expect(reservation).not.toBeNull(); + expect(queue.cancelByLocalId('a')).toBe(true); + expect(queue.restoreReservation(reservation!)).toBe(true); + expect(queue.queue.map((item) => item.localId)).toEqual(['b', 'c']); + }); + + it('restores multiple reservations in FIFO order regardless of completion order', () => { + const queue = new MessageQueue2(mode => mode); + queue.push('a', 'A', 'a'); + queue.push('b', 'A', 'b'); + queue.push('c', 'A', 'c'); + queue.push('d', 'A', 'd'); + const b = queue.takeByLocalId('b'); + const c = queue.takeByLocalId('c'); + expect(b).not.toBeNull(); + expect(c).not.toBeNull(); + expect(queue.restoreReservation(c!)).toBe(true); + expect(queue.restoreReservation(b!)).toBe(true); + expect(queue.queue.map((item) => item.localId)).toEqual(['a', 'b', 'c', 'd']); + }); + + it('retries an indeterminate reservation without automatic replay', () => { + const queue = new MessageQueue2(mode => mode); + queue.push('b', 'B', 'b'); + const reservation = queue.takeByLocalId('b'); + expect(reservation).not.toBeNull(); + expect(queue.beginReservationDispatch(reservation!)).toBe(true); + expect(queue.markReservationIndeterminate(reservation!)).toBe(true); + expect(queue.size()).toBe(0); + expect(queue.takeByLocalId('b')).toBe(reservation); + expect(queue.beginReservationDispatch(reservation!)).toBe(true); + expect(queue.restoreReservation(reservation!)).toBe(true); + expect(queue.takeByLocalId('b')).toBe(reservation); + }); + + it('allows a positive reconciliation after a queue reset', () => { + const queue = new MessageQueue2(mode => mode); + queue.push('steer', 'B', 'steer'); + const reservation = queue.takeByLocalId('steer'); + expect(reservation).not.toBeNull(); + expect(queue.beginReservationDispatch(reservation!)).toBe(true); + expect(queue.markReservationIndeterminate(reservation!)).toBe(true); + queue.reset(); + expect(queue.commitReservation(reservation!)).toBe(true); + }); + + it('preserves a dispatched reservation across an abort reset', () => { + const queue = new MessageQueue2(mode => mode); + queue.push('b', 'B', 'b'); + const reservation = queue.takeByLocalId('b'); + expect(reservation).not.toBeNull(); + expect(queue.beginReservationDispatch(reservation!)).toBe(true); + queue.reset({ preserveDispatchingReservations: true }); + expect(queue.cancelByLocalId('b')).toBe('in-flight'); + expect(queue.markReservationIndeterminate(reservation!)).toBe(true); + expect(queue.commitReservation(reservation!)).toBe(true); + }); + + it('releases an indeterminate reservation on explicit cancel', () => { + const queue = new MessageQueue2(mode => mode); + queue.push('old', 'B', 'b'); + const reservation = queue.takeByLocalId('b'); + expect(reservation).not.toBeNull(); + expect(queue.beginReservationDispatch(reservation!)).toBe(true); + expect(queue.markReservationIndeterminate(reservation!)).toBe(true); + expect(queue.cancelByLocalId('b')).toBe(true); + queue.push('replacement', 'B', 'b'); + expect(queue.takeByLocalId('b')?.item.message).toBe('replacement'); + }); + + it('does not restore an indeterminate reservation after clear', () => { + const queue = new MessageQueue2(mode => mode); + queue.push('b', 'B', 'b'); + const reservation = queue.takeByLocalId('b'); + expect(reservation).not.toBeNull(); + expect(queue.beginReservationDispatch(reservation!)).toBe(true); + expect(queue.markReservationIndeterminate(reservation!)).toBe(true); + queue.pushIsolateAndClear('clear', 'B'); + expect(queue.restoreReservation(reservation!)).toBe(false); + expect(queue.queue.map((item) => item.message)).toEqual(['clear']); + }); + it('should differentiate between pushImmediate and pushIsolateAndClear behavior', async () => { const queue = new MessageQueue2<{ type: string }>((mode) => mode.type); diff --git a/cli/src/utils/MessageQueue2.ts b/cli/src/utils/MessageQueue2.ts index 0e4ffe88..6c724ba1 100644 --- a/cli/src/utils/MessageQueue2.ts +++ b/cli/src/utils/MessageQueue2.ts @@ -1,13 +1,26 @@ import { logger } from "@/ui/logger"; -interface QueueItem { +export interface QueueItem { message: string; mode: T; modeHash: string; localId?: string; isolate?: boolean; // If true, this message must be processed alone + /** Stable FIFO key used when an async reservation is restored later. */ + enqueueOrder?: number; } +export type QueueReservation = { + item: QueueItem; + index: number; + previousItem: QueueItem | null; + nextItem: QueueItem | null; + /** Once ambiguous, retries must remain held unless explicitly committed. */ + originIndeterminate: boolean; + cancelReason?: 'explicit' | 'queue-reset'; + state: 'reserved' | 'dispatching' | 'indeterminate' | 'cancelled'; +}; + /** * A mode-aware message queue that stores messages with their modes. * Returns consistent batches of messages with the same mode. @@ -19,6 +32,10 @@ export class MessageQueue2 { private onMessageHandler: ((message: string, mode: T) => void) | null = null; onBatchConsumed: ((localIds: string[]) => void) | null = null; modeHasher: (mode: T) => string; + private readonly reservations = new Map>(); + private readonly consumedReservations = new Set(); + private nextEnqueueOrder = 0; + private previousEnqueueOrder = -1; constructor( modeHasher: (mode: T) => string, @@ -47,13 +64,16 @@ export class MessageQueue2 { const modeHash = this.modeHasher(mode); logger.debug(`[MessageQueue2] push() called with mode hash: ${modeHash}`); - this.queue.push({ + const item = { message, mode, modeHash, localId, - isolate: false - }); + isolate: false, + enqueueOrder: this.nextEnqueueOrder++ + }; + Object.defineProperty(item, 'enqueueOrder', { value: item.enqueueOrder, enumerable: false, writable: true }); + this.queue.push(item); // Trigger message handler if set if (this.onMessageHandler) { @@ -83,13 +103,16 @@ export class MessageQueue2 { const modeHash = this.modeHasher(mode); logger.debug(`[MessageQueue2] pushImmediate() called with mode hash: ${modeHash}`); - this.queue.push({ + const item = { message, mode, modeHash, localId, - isolate: false - }); + isolate: false, + enqueueOrder: this.nextEnqueueOrder++ + }; + Object.defineProperty(item, 'enqueueOrder', { value: item.enqueueOrder, enumerable: false, writable: true }); + this.queue.push(item); // Trigger message handler if set if (this.onMessageHandler) { @@ -122,13 +145,16 @@ export class MessageQueue2 { const modeHash = this.modeHasher(mode); logger.debug(`[MessageQueue2] pushIsolated() called with mode hash: ${modeHash} - preserving ${this.queue.length} pending messages`); - this.queue.push({ + const item = { message, mode, modeHash, localId, - isolate: true - }); + isolate: true, + enqueueOrder: this.nextEnqueueOrder++ + }; + Object.defineProperty(item, 'enqueueOrder', { value: item.enqueueOrder, enumerable: false, writable: true }); + this.queue.push(item); if (this.onMessageHandler) { this.onMessageHandler(message, mode); @@ -159,14 +185,20 @@ export class MessageQueue2 { // Clear any pending messages to ensure this message is processed in complete isolation this.queue = []; + // Reservations live outside this.queue; a steer awaiting an explicit + // rejection must not restore a prompt the clear command discarded. + this.cancelReservations(); - this.queue.push({ + const item = { message, mode, modeHash, localId, - isolate: true - }); + isolate: true, + enqueueOrder: this.nextEnqueueOrder++ + }; + Object.defineProperty(item, 'enqueueOrder', { value: item.enqueueOrder, enumerable: false, writable: true }); + this.queue.push(item); // Trigger message handler if set if (this.onMessageHandler) { @@ -195,13 +227,16 @@ export class MessageQueue2 { const modeHash = this.modeHasher(mode); logger.debug(`[MessageQueue2] unshift() called with mode hash: ${modeHash}`); - this.queue.unshift({ + const item = { message, mode, modeHash, localId, - isolate: false - }); + isolate: false, + enqueueOrder: this.previousEnqueueOrder-- + }; + Object.defineProperty(item, 'enqueueOrder', { value: item.enqueueOrder, enumerable: false, writable: true }); + this.queue.unshift(item); // Trigger message handler if set if (this.onMessageHandler) { @@ -234,13 +269,16 @@ export class MessageQueue2 { const modeHash = this.modeHasher(mode); logger.debug(`[MessageQueue2] unshiftIsolated() called with mode hash: ${modeHash}`); - this.queue.unshift({ + const item = { message, mode, modeHash, localId, - isolate: true - }); + isolate: true, + enqueueOrder: this.previousEnqueueOrder-- + }; + Object.defineProperty(item, 'enqueueOrder', { value: item.enqueueOrder, enumerable: false, writable: true }); + this.queue.unshift(item); if (this.onMessageHandler) { this.onMessageHandler(message, mode); @@ -261,20 +299,207 @@ export class MessageQueue2 { * Best-effort: if the CLI is offline when cancel is issued, the message * may already have been collected for invocation and won't be found here. */ - cancelByLocalId(localId: string): boolean { + cancelByLocalId(localId: string): boolean | 'in-flight' | 'indeterminate' | 'consumed' { if (!localId) return false; const idx = this.queue.findIndex(item => item.localId === localId); - if (idx === -1) return false; - this.queue.splice(idx, 1); + if (idx !== -1) { + this.queue.splice(idx, 1); + return true; + } + const reservation = this.reservations.get(localId); + if (!reservation) { + return this.consumedReservations.has(localId) ? 'consumed' : false; + } + if (reservation.state === 'dispatching') { + // The row is inside an async steer: it cannot be removed, but it is + // also NOT already consumed — the hub must not stamp invoked_at. + return 'in-flight'; + } + if (reservation.state === 'indeterminate') { + // Explicit Cancel/ Edit is the user's resolution of an unknown + // outcome; release the held reservation so it cannot settle later. + reservation.cancelReason = 'explicit'; + reservation.state = 'cancelled'; + this.reservations.delete(localId); + return true; + } + reservation.cancelReason = 'explicit'; + reservation.state = 'cancelled'; + this.reservations.delete(localId); return true; } + /** + * Look up a queued item by localId without removing it. + */ + peekByLocalId(localId: string): QueueItem | null { + if (!localId) return null; + return this.queue.find(item => item.localId === localId) ?? null; + } + + /** + * Remove and return a queued item by localId (with its original index), + * or null if not found. Pair with {@link restoreTakenItem} when an async + * operation may need to put the item back in order. + */ + takeByLocalId(localId: string): QueueReservation | null { + if (!localId) return null; + + // An indeterminate steer is deliberately held outside the normal queue. + // A later explicit Steer retries that same reservation; automatic queue + // drains never see it. + const existing = this.reservations.get(localId); + if (existing) { + if (existing.state !== 'indeterminate') return null; + existing.state = 'reserved'; + return existing; + } + + const idx = this.queue.findIndex(item => item.localId === localId); + if (idx === -1) return null; + const [item] = this.queue.splice(idx, 1); + if (!item) return null; + const reservation: QueueReservation = { + item, + index: idx, + previousItem: this.queue[idx - 1] ?? null, + nextItem: this.queue[idx] ?? null, + originIndeterminate: false, + state: 'reserved' + }; + if (item.localId) { + this.reservations.set(item.localId, reservation); + } + return reservation; + } + + /** + * Re-insert an item previously removed by {@link takeByLocalId} at its + * original index (clamped if the queue shrank). + */ + restoreReservation(reservation: QueueReservation): boolean { + if (reservation.state === 'cancelled') { + return false; + } + if (reservation.originIndeterminate) { + reservation.state = 'indeterminate'; + return true; + } + if (this.closed) { + throw new Error('Cannot restore into closed queue'); + } + if (reservation.item.localId) { + if (this.reservations.get(reservation.item.localId) !== reservation) { + return false; + } + this.reservations.delete(reservation.item.localId); + } + const order = reservation.item.enqueueOrder; + const orderedIndex = order === undefined + ? -1 + : this.queue.findIndex((item) => item.enqueueOrder !== undefined && item.enqueueOrder > order); + const nextIndex = reservation.nextItem + ? this.queue.indexOf(reservation.nextItem) + : -1; + const previousIndex = reservation.previousItem + ? this.queue.indexOf(reservation.previousItem) + : -1; + const idx = orderedIndex >= 0 + ? orderedIndex + : nextIndex >= 0 + ? nextIndex + : previousIndex >= 0 + ? previousIndex + 1 + : this.queue.length; + this.queue.splice(idx, 0, reservation.item); + if (this.waiter) { + const waiter = this.waiter; + this.waiter = null; + waiter(true); + } + return true; + } + + commitReservation(reservation: QueueReservation): boolean { + if (reservation.state === 'cancelled' && reservation.cancelReason !== 'queue-reset') { + return false; + } + if (reservation.item.localId) { + const active = this.reservations.get(reservation.item.localId); + if (active === reservation) { + this.reservations.delete(reservation.item.localId); + } else if (reservation.cancelReason !== 'queue-reset') { + return false; + } + this.rememberConsumedReservation(reservation.item.localId); + } + return true; + } + + beginReservationDispatch(reservation: QueueReservation): boolean { + if (reservation.state !== 'reserved') { + return false; + } + if (reservation.item.localId && this.reservations.get(reservation.item.localId) !== reservation) { + return false; + } + reservation.state = 'dispatching'; + return true; + } + + restoreTakenItem(taken: QueueReservation): void { + this.restoreReservation(taken); + } + + /** Release a held reservation only for the explicit retry path. */ + releaseIndeterminateReservation(localId: string): boolean { + const reservation = this.reservations.get(localId); + if (!reservation || reservation.state !== 'indeterminate') return false; + reservation.cancelReason = 'explicit'; + reservation.state = 'cancelled'; + this.reservations.delete(localId); + return true; + } + + /** Hold a dispatched steer for explicit retry/cancel without replaying it. */ + markReservationIndeterminate(reservation: QueueReservation): boolean { + if (reservation.state !== 'dispatching') return false; + if (reservation.item.localId && this.reservations.get(reservation.item.localId) !== reservation) { + return false; + } + reservation.originIndeterminate = true; + reservation.state = 'indeterminate'; + return true; + } + + private rememberConsumedReservation(localId: string): void { + this.consumedReservations.delete(localId); + this.consumedReservations.add(localId); + while (this.consumedReservations.size > 256) { + const oldest = this.consumedReservations.values().next().value; + if (oldest === undefined) break; + this.consumedReservations.delete(oldest); + } + } + + private cancelReservations(preserveDispatching = false): void { + for (const [localId, reservation] of this.reservations) { + if (preserveDispatching && (reservation.state === 'dispatching' || reservation.state === 'indeterminate')) { + continue; + } + reservation.cancelReason = 'queue-reset'; + reservation.state = 'cancelled'; + this.reservations.delete(localId); + } + } + /** * Reset the queue - clears all messages and resets to empty state */ - reset(): void { + reset(options?: { preserveDispatchingReservations?: boolean }): void { logger.debug(`[MessageQueue2] reset() called. Clearing ${this.queue.length} messages`); this.queue = []; + this.cancelReservations(options?.preserveDispatchingReservations === true); this.closed = false; // Clear waiter without calling it since we're not closing @@ -298,6 +523,7 @@ export class MessageQueue2 { close(): void { logger.debug(`[MessageQueue2] close() called`); this.closed = true; + this.cancelReservations(); // Notify any waiting caller if (this.waiter) { diff --git a/docs/api/client-contract/messages.md b/docs/api/client-contract/messages.md index eb2d41a8..75bc3e56 100644 --- a/docs/api/client-contract/messages.md +++ b/docs/api/client-contract/messages.md @@ -17,6 +17,7 @@ type DecryptedMessage = { createdAt: number // hub receive time (epoch ms) invokedAt?: number | null // when the agent consumed it (null = still queued) scheduledAt?: number | null // future-scheduled sends + deliveryState?: 'indeterminate' // steer dispatched, outcome unproven; explicit retry/cancel required } ``` diff --git a/docs/api/client-contract/pagination.md b/docs/api/client-contract/pagination.md index 52f7bae2..6dd5e4bc 100644 --- a/docs/api/client-contract/pagination.md +++ b/docs/api/client-contract/pagination.md @@ -140,9 +140,11 @@ Lifecycle: 2. On POST success: status → `queued` if the session is currently thinking, else `sent`. On failure: drop the row and restore the composer (or keep it as `failed` with a retry affordance when attachments are involved). 3. **Echo**: the hub emits `message-received` carrying the stored row (server `id`, real `seq`, same `localId`). Merging a stored row whose `localId` matches an optimistic row **replaces** the optimistic one, preserving the client-side `status` and any already-known `invokedAt` the server row lacks. Fallback when no `localId` echo matches: drop an optimistic `sent` row when a server user message lands within **10 s** of the same position. 4. **`messages-consumed {localIds, invokedAt}`** (SSE): stamp `invokedAt` and flip status to `sent` on matching rows (skip `failed` ones). This is what moves a message out of the queued bar and into the thread at its invocation position. -5. **`message-cancelled {messageId, localId?}`** (SSE): remove the row (match either id). +5. **`messages-indeterminate {localIds}`** (SSE): the steer outcome is unknown. Keep `invokedAt: null`, mark `deliveryState:'indeterminate'`, exclude the row from automatic replay, and show explicit Retry/Cancel actions. +6. **`messages-requeued {localIds}`** (SSE): an explicit Retry restored normal queue delivery; clear `deliveryState`. +7. **`message-cancelled {messageId, localId?}`** (SSE): remove the row (match either id). -**Queued semantics**: a user message is "queued" iff `invokedAt === null` **strictly** and `status !== 'failed'`. `undefined` means already-invoked (rows from pre-V8 hubs omit the field) — only rows explicitly carrying `null` belong in the queued bar. Server-side, rows sent without a `localId` are stamped invoked at insert and can never be queued. +**Queued semantics**: a user message is "queued" iff `invokedAt === null` **strictly**, `deliveryState !== 'indeterminate'`, and `status !== 'failed'`. An indeterminate row remains visible in the unresolved-delivery bar but is not eligible for automatic delivery. `undefined` means already-invoked (rows from pre-V8 hubs omit the field) — only rows explicitly carrying `null` belong in the queued bar. Server-side, rows sent without a `localId` are stamped invoked at insert and can never be queued. ### Queued-state recovery @@ -179,6 +181,7 @@ Response `{"ok": true}`. Sending to an inactive session returns `409 {"error":"S |---|---|---| | `{"status":"cancelled","localId":string\|null}` | Row deleted (or already gone). Bumps the epoch. | Remove the row. | | `{"status":"invoked","message":DecryptedMessage}` | Too late — the agent consumed it before the cancel landed. | **Ingest the returned message** as the authoritative row (correct `invokedAt`, status `sent`); do not resurrect the queued snapshot. | +| `{"status":"busy","localId":string}` | A live steer is still resolving. | Restore the row as indeterminate; reconcile queued state before allowing Retry/Cancel. | Other subscribers learn the same outcome via `message-cancelled` / `messages-consumed` SSE events. diff --git a/docs/api/client-contract/rest.md b/docs/api/client-contract/rest.md index 067fe997..7810e450 100644 --- a/docs/api/client-contract/rest.md +++ b/docs/api/client-contract/rest.md @@ -59,14 +59,15 @@ Optional for v1 (endpoints exist; the v1 native scope does not require them): `P ### Messages -Source: `hub/src/web/routes/messages.ts`; schemas `MessagesQuerySchema`, `SendMessageRequestSchema`, `QueuedStateRequestSchema` (`shared/src/apiTypes.ts`), responses `CancelMessageResponseSchema`, `SteerQueuedMessageResponseSchema` (`shared/src/schemas.ts`). +Source: `hub/src/web/routes/messages.ts`; schemas `MessagesQuerySchema`, `SendMessageRequestSchema`, `QueuedStateRequestSchema` (`shared/src/apiTypes.ts`), responses `CancelMessageResponseSchema`, `SteerQueuedMessageResponseSchema` (`shared/src/schemas.ts`). Unknown steer delivery is durable and user-resolvable; native clients must implement the same transitions. | Method & path | Request | Response | |---|---|---| | `GET /api/sessions/:id/messages` | Query: `limit?` (1–200, default 50), cursor pairs `beforeSeq+beforeAt` \| `afterSeq+afterAt` (+ optional `untilSeq+untilAt`, `epoch` with `after`) | `MessagesResponse` `{messages: DecryptedMessage[], page: {direction, limit, epoch, reset, nextBefore*/nextAfter*, snapshotHead*, hasMore}}` — full cursor semantics in [Pagination](./pagination.md) | | `POST /api/sessions/:id/messages` | `{text, localId?, attachments?, scheduledAt?, deliveryMode?: 'queue'\|'steer'}` — text or attachments required; `scheduledAt` requires `localId`, must be ≤ 7 days out, excludes attachments and steer | `{ok: true}` — the message itself arrives via SSE (`message-received`), reconciled by `localId` | -| `DELETE /api/sessions/:id/messages/:messageId` | — | `{status: 'cancelled', localId}` \| `{status: 'invoked', message}` (cancel a queued message; `invoked` = too late) | +| `DELETE /api/sessions/:id/messages/:messageId` | — | `{status: 'cancelled', localId}` \| `{status: 'invoked', message}` \| `{status: 'busy', localId}` (cancel; `busy` = steer still resolving) | | `POST /api/sessions/:id/messages/:messageId/steer` | — | `{status: 'steered', localId}` \| `{status: 'invoked', message}` \| `{status: 'failed', error, localId}` | +| `POST /api/sessions/:id/messages/:messageId/retry` | — | `{status: 'retried', localId}` \| `{status: 'already-queued', localId}` \| `{status: 'retry-unavailable', localId}` \| `{status: 'invoked', message}` \| `{status: 'not-found'}` — explicit retry only; never automatic replay | | `POST /api/sessions/:id/messages/queued-state` | `{localIds: string[]}` (≤ 1000, deduped) | `{queuedLocalIds: string[], invokedLocalMessages: [{localId, invokedAt}]}` — resync optimistic sends after reconnect | The hub stamps `sentFrom: 'webapp'` on REST-sent messages server-side; the request body has no such field. diff --git a/docs/api/client-contract/sse.md b/docs/api/client-contract/sse.md index 19d8f8b1..99836b73 100644 --- a/docs/api/client-contract/sse.md +++ b/docs/api/client-contract/sse.md @@ -121,16 +121,16 @@ Hub-side delivery (`SSEManager.shouldSend`): | `heartbeat` | every connection | | `toast` | every **visible** connection in the namespace, regardless of filter (no `id`, never replayed) | | `message-received`, `scheduled-matured` | `all=true` connections + matching `sessionId` connections | -| `session-added` / `session-updated` / `session-removed` / `session-ended` / `messages-invalidated` / `messages-consumed` / `message-cancelled` | `all=true` connections + matching `sessionId` connections | +| `session-added` / `session-updated` / `session-removed` / `session-ended` / `messages-invalidated` / `messages-consumed` / `messages-indeterminate` / `messages-requeued` / `message-cancelled` | `all=true` connections + matching `sessionId` connections | | `machine-updated` | `all=true` connections + matching `machineId` connections | -**The global connection must also handle the message-stream events** (`message-received`, `messages-consumed`, `message-cancelled`, `scheduled-matured`): while a session connection is down (reconnect gap) or the session isn't open, the global pipe is the only one alive, and it must still keep queued/optimistic bookkeeping correct — mark local messages consumed, remove cancelled rows, and refresh session-list scheduled counts. The session-scoped connection additionally ingests `message-received` into the message window. +**The global connection must also handle the message-stream events** (`message-received`, `messages-consumed`, `messages-indeterminate`, `messages-requeued`, `message-cancelled`, `scheduled-matured`): while a session connection is down (reconnect gap) or the session isn't open, the global pipe is the only one alive, and it must still keep queued/optimistic bookkeeping correct — mark local messages consumed, remove cancelled rows, and refresh session-list scheduled counts. The session-scoped connection additionally ingests `message-received` into the message window. The two connections have **no ordering relationship with each other** — the same `session-updated` patch can arrive on both, in either order. That is why the versioned-patch gate below exists. --- -## SyncEvent union (13 types) +## SyncEvent union (15 types) Schema: `SyncEventSchema` in `shared/src/schemas.ts` (discriminated on `type`). All events except `connection-changed` carry `namespace?: string`. Ignore unknown event types. @@ -146,6 +146,8 @@ Schema: `SyncEventSchema` in `shared/src/schemas.ts` (discriminated on `type`). | `machine-updated` | `machineId`, `data?: Machine \| MachinePatch \| null` | Full `Machine`: upsert (remove when `active:false`). `null`: machine removed. Patch `{active?, activeAt?, updatedAt?}`: `active:false` ⇒ remove, otherwise refetch machines. `data` absent ⇒ refetch. | | `toast` | `data: {title, body, sessionId, url}` | Show as in-app toast/banner. Only delivered to visible connections (see [Visibility](#visibility)). | | `messages-consumed` | `sessionId`, `localIds: string[]`, `invokedAt: number` | The agent consumed queued user messages: stamp `invokedAt`, flip status to `sent`, remove from the queued bar. | +| `messages-indeterminate` | `sessionId`, `localIds: string[]` | A steer was dispatched but its outcome is unknown. Keep the row uninvoked, show an explicit Retry/Cancel resolution, and do not auto-replay it. | +| `messages-requeued` | `sessionId`, `localIds: string[]` | An explicit Retry restored delivery to the normal queue. Clear the indeterminate marker. | | `message-cancelled` | `sessionId`, `messageId`, `localId?` | A queued message was cancelled: remove the row (match by `messageId` **or** `localId`). | | `heartbeat` | `data?: {timestamp}` | Feed the staleness watchdog. No other action. **Carries no `id`.** | | `connection-changed` | `data?: {status, subscriptionId?, resume?: 'ok'\|'gap'}` | Handshake; see [Handshake and resume](#handshake-and-resume). Store `subscriptionId` for visibility reporting. **Carries no `id`.** | diff --git a/hub/src/socket/handlers/cli/sessionHandlers.ts b/hub/src/socket/handlers/cli/sessionHandlers.ts index 36a5b636..aa22c34f 100644 --- a/hub/src/socket/handlers/cli/sessionHandlers.ts +++ b/hub/src/socket/handlers/cli/sessionHandlers.ts @@ -364,7 +364,7 @@ export function registerSessionHandlers(socket: CliSocketWithData, deps: Session onSessionReady?.(data) }) - socket.on('messages-consumed', (data: { sid: string; localIds: string[]; clearQueuedThinkingGrace?: boolean }) => { + socket.on('messages-consumed', (data: { sid: string; localIds: string[]; clearQueuedThinkingGrace?: boolean; steered?: boolean }) => { if (!data || typeof data.sid !== 'string' || !Array.isArray(data.localIds)) { return } @@ -407,8 +407,67 @@ export function registerSessionHandlers(socket: CliSocketWithData, deps: Session } // Emit only after the DB transaction succeeds. This is an ACK-level // batch contract, so preserve its original timestamp even when IDs are - // heterogeneous, replayed, or unknown. - onWebappEvent?.({ type: 'messages-consumed', sessionId: data.sid, localIds, invokedAt }) + // heterogeneous, replayed, or unknown. `steered` is a live-only signal + // (never persisted) that marks mid-turn delivery for the web badge. + onWebappEvent?.({ type: 'messages-consumed', sessionId: data.sid, localIds, invokedAt, ...(data.steered === true ? { steered: true } : {}) }) + }) + + socket.on('messages-indeterminate', (data: { sid: string; localIds: string[] }) => { + if (!data || typeof data.sid !== 'string' || !Array.isArray(data.localIds)) { + return + } + const localIds = data.localIds.filter((id): id is string => typeof id === 'string') + if (localIds.length === 0) return + const sessionAccess = resolveSessionAccess(data.sid) + if (!sessionAccess.ok) { + emitAccessError('session', data.sid, sessionAccess.reason) + return + } + try { + store.recordMessagesIndeterminate(data.sid, localIds, sessionAccess.value.namespace) + } catch (err) { + console.error('recordMessagesIndeterminate failed', err) + return + } + onWebappEvent?.({ type: 'messages-indeterminate', sessionId: data.sid, localIds }) + }) + + socket.on('messages-steer-state', ( + data: { sid: string; localIds: string[]; state: 'queued' | 'dispatching' }, + ack?: (response: { ok: boolean }) => void + ) => { + const reply = typeof ack === 'function' ? ack : () => {} + if (!data || typeof data.sid !== 'string' || !Array.isArray(data.localIds) + || (data.state !== 'queued' && data.state !== 'dispatching')) { + reply({ ok: false }) + return + } + const localIds = data.localIds.filter((id): id is string => typeof id === 'string') + if (localIds.length === 0) { + reply({ ok: false }) + return + } + const sessionAccess = resolveSessionAccess(data.sid) + if (!sessionAccess.ok) { + emitAccessError('session', data.sid, sessionAccess.reason) + reply({ ok: false }) + return + } + try { + const ok = store.recordSteerDeliveryState( + data.sid, + localIds, + data.state, + sessionAccess.value.namespace + ) + reply({ ok }) + if (ok && data.state === 'queued') { + onWebappEvent?.({ type: 'messages-requeued', sessionId: data.sid, localIds }) + } + } catch (err) { + console.error('recordSteerDeliveryState failed', err) + reply({ ok: false }) + } }) socket.on('session-end', (data: SessionEndPayload) => { diff --git a/hub/src/store/index.ts b/hub/src/store/index.ts index 12869230..5c12c5eb 100644 --- a/hub/src/store/index.ts +++ b/hub/src/store/index.ts @@ -18,6 +18,7 @@ export type { NativeDevicePlatform, StoredMachine, StoredMessage, + MessageDeliveryState, StoredPushSubscription, StoredFcmDevice, StoredScratchlistEntry, @@ -41,7 +42,7 @@ export { WorkGraphValidationError } from './workGraph' -const SCHEMA_VERSION: number = 24 +const SCHEMA_VERSION: number = 25 const REQUIRED_TABLES = [ 'sessions', 'machines', @@ -157,6 +158,47 @@ export class Store { })() } + /** Persist a steer delivery state before/after the native request. */ + recordSteerDeliveryState( + sessionId: string, + localIds: string[], + state: 'queued' | 'dispatching' | 'indeterminate', + namespace: string + ): boolean { + return this.db.transaction(() => { + const changes = this.messages.setMessagesDeliveryState(sessionId, localIds, state) + const now = Date.now() + if (changes > 0) { + this.sessions.touchSessionUpdatedAt(sessionId, now, namespace) + } + const session = this.sessions.getSessionByNamespace(sessionId, namespace) + if (!session) { + throw new Error('session not found after steer delivery transition') + } + return changes > 0 + })() + } + + /** Persist an ambiguous agent steer without stamping it delivered. */ + recordMessagesIndeterminate( + sessionId: string, + localIds: string[], + namespace: string + ): number { + return this.db.transaction(() => { + const changes = this.messages.markMessagesIndeterminate(sessionId, localIds) + const now = Date.now() + if (changes > 0) { + this.sessions.touchSessionUpdatedAt(sessionId, now, namespace) + } + const session = this.sessions.getSessionByNamespace(sessionId, namespace) + if (!session) { + throw new Error('session not found after indeterminate transition') + } + return session.updatedAt + })() + } + /** Resolve a durable OpenCode clear reservation and insert in one SQLite transaction. */ addMessageForCurrentSession( sessionId: string, @@ -304,6 +346,7 @@ export class Store { 21: () => this.migrateFromV21ToV22(), 22: () => this.migrateFromV22ToV23(), 23: () => this.migrateFromV23ToV24(), + 24: () => this.migrateFromV24ToV25(), }) if (currentVersion === 0) { @@ -403,6 +446,7 @@ export class Store { local_id TEXT, invoked_at INTEGER, scheduled_at INTEGER, + delivery_state TEXT NOT NULL DEFAULT 'queued', FOREIGN KEY (session_id) REFERENCES sessions(id) ON DELETE CASCADE ); CREATE INDEX IF NOT EXISTS idx_messages_session ON messages(session_id, seq); @@ -917,6 +961,22 @@ export class Store { } } + /** v23→v24: add the iOS push envelope key. */ + private migrateFromV23ToV24(): void { + const fcmColumns = this.db.prepare('PRAGMA table_info(fcm_devices)').all() as Array<{ name: string }> + if (fcmColumns.length > 0 && !fcmColumns.some((column) => column.name === 'push_key')) { + this.db.exec('ALTER TABLE fcm_devices ADD COLUMN push_key TEXT') + } + } + + /** v24→v25: add durable unknown-delivery state for steers. */ + private migrateFromV24ToV25(): void { + const messageColumns = this.getMessageColumnNames() + if (messageColumns.size > 0 && !messageColumns.has('delivery_state')) { + this.db.exec("ALTER TABLE messages ADD COLUMN delivery_state TEXT NOT NULL DEFAULT 'queued'") + } + } + /** * A2A Layer 1 / P1 (#1374) + P3 substrate: hub work-graph ledger tables. * Namespace + principal_json required on every events row. @@ -975,25 +1035,6 @@ export class Store { `) } - /** - * iOS push (P1, PUSH SPEC v1): `fcm_devices.push_key` stores the - * device-generated 32-byte E2E envelope key (base64) for `platform = - * 'ios'` rows; phone/wear rows keep NULL. Nullable ALTER keeps existing - * Android registrations untouched. - * - * Rollback: `ALTER TABLE fcm_devices DROP COLUMN push_key` (SQLite - * 3.35+) or leave the column unused; `PRAGMA user_version = 23`. - */ - private migrateFromV23ToV24(): void { - const columns = this.db.prepare('PRAGMA table_info(fcm_devices)').all() as Array<{ name: string }> - // Legacy branch may reach this step before fcm_devices exists; - // createSchema afterwards builds the table with the column included. - if (columns.length === 0) return - if (!columns.some((col) => col.name === 'push_key')) { - this.db.exec('ALTER TABLE fcm_devices ADD COLUMN push_key TEXT') - } - } - private getSessionColumnNames(): Set { const rows = this.db.prepare('PRAGMA table_info(sessions)').all() as Array<{ name: string }> return new Set(rows.map((row) => row.name)) diff --git a/hub/src/store/messageStore.ts b/hub/src/store/messageStore.ts index 5652d30b..d39b2fc3 100644 --- a/hub/src/store/messageStore.ts +++ b/hub/src/store/messageStore.ts @@ -6,6 +6,7 @@ import { addImportedMessage, cancelQueuedMessage, deleteQueuedMessageById, + claimIndeterminateMessage, lookupQueuedMessage, getMessages, getFirstMessages, @@ -24,6 +25,8 @@ import { minFutureScheduledAtBySessionIds, countMessages, markMessagesInvoked, + markMessagesIndeterminate, + setMessagesDeliveryState, markUninvokedImmediateMessages, mergeSessionMessages, moveUninvokedScheduledMessages, @@ -57,7 +60,7 @@ export class MessageStore { copyMessageToSession( sessionId: string, - message: Pick + message: Pick ): StoredMessage { // 中文注释：重复会话合并时需要保留源消息的时间戳和排队信息，因此走专门的复制入口而不是普通 addMessage。 return copyStoredMessageToSession(this.db, sessionId, message) @@ -65,7 +68,7 @@ export class MessageStore { copyMessagesToSession( sessionId: string, - messages: Array> + messages: Array> ): number { return copyStoredMessagesToSession(this.db, sessionId, messages) } @@ -123,8 +126,8 @@ export class MessageStore { return getLocalMessageStates(this.db, sessionId, localIds) } - getUninvokedLocalMessages(sessionId: string): StoredMessage[] { - return getUninvokedLocalMessages(this.db, sessionId) + getUninvokedLocalMessages(sessionId: string, options?: { deliverableOnly?: boolean }): StoredMessage[] { + return getUninvokedLocalMessages(this.db, sessionId, options) } getMatureScheduledMessages(beforeTime: number): StoredMessage[] { @@ -163,10 +166,22 @@ export class MessageStore { return deleteQueuedMessageById(this.db, sessionId, messageId) } + claimIndeterminateMessage(sessionId: string, messageId: string): StoredMessage | null { + return claimIndeterminateMessage(this.db, sessionId, messageId) + } + markMessagesInvoked(sessionId: string, localIds: string[], invokedAt: number): number { return markMessagesInvoked(this.db, sessionId, localIds, invokedAt) } + markMessagesIndeterminate(sessionId: string, localIds: string[]): number { + return markMessagesIndeterminate(this.db, sessionId, localIds) + } + + setMessagesDeliveryState(sessionId: string, localIds: string[], state: 'queued' | 'dispatching' | 'indeterminate'): number { + return setMessagesDeliveryState(this.db, sessionId, localIds, state) + } + markUninvokedImmediateMessages(sessionId: string, invokedAt: number): string[] { return markUninvokedImmediateMessages(this.db, sessionId, invokedAt) } diff --git a/hub/src/store/messages.test.ts b/hub/src/store/messages.test.ts index 4011c806..4790f4c2 100644 --- a/hub/src/store/messages.test.ts +++ b/hub/src/store/messages.test.ts @@ -546,4 +546,53 @@ describe('content codec integration', () => { const delivered = scheduled[0]!.content as { content: { text: string } } expect(delivered.content.text).toBe(text) }) + + it('requeues an indeterminate delivery only on explicit request', () => { + const store = makeStore() + const session = makeSession(store, 'requeue-indeterminate') + const msg = store.messages.addMessage( + session.id, + { role: 'user', content: { type: 'text', text: 'retry me' } }, + 'lid-retry' + ) + expect(store.messages.setMessagesDeliveryState(session.id, ['lid-retry'], 'dispatching')).toBe(1) + expect(store.messages.setMessagesDeliveryState(session.id, ['lid-retry'], 'indeterminate')).toBe(1) + expect(store.messages.getDeliverableMessagesAfter(session.id, 0, Date.now())).toHaveLength(0) + const claimed = store.messages.claimIndeterminateMessage(session.id, msg.id) + expect(claimed).toMatchObject({ id: msg.id, invokedAt: null, deliveryState: 'indeterminate' }) + expect(store.messages.claimIndeterminateMessage(session.id, msg.id)).toBeNull() + expect(store.messages.getDeliverableMessagesAfter(session.id, 0, Date.now())).toHaveLength(0) + expect(store.messages.setMessagesDeliveryState(session.id, ['lid-retry'], 'queued')).toBe(1) + expect(store.messages.getDeliverableMessagesAfter(session.id, 0, Date.now())).toHaveLength(1) + }) + + it('holds indeterminate delivery out of replay without stamping invoked_at', () => { + const store = makeStore() + const session = makeSession(store, 'indeterminate') + const msg = store.messages.addMessage( + session.id, + { role: 'user', content: { type: 'text', text: 'uncertain' } }, + 'lid-uncertain' + ) + + expect(store.messages.markMessagesIndeterminate(session.id, ['lid-uncertain'])).toBe(1) + expect(store.messages.markMessagesIndeterminate(session.id, ['lid-uncertain'])).toBe(0) + expect(store.messages.getDeliverableMessagesAfter(session.id, 0, Date.now())).toHaveLength(0) + expect(store.messages.getImmediateQueuedLocalMessages(session.id)).toHaveLength(0) + expect(store.messages.getUninvokedLocalMessages(session.id)[0]).toMatchObject({ + id: msg.id, + deliveryState: 'indeterminate', + invokedAt: null + }) + expect(store.messages.lookupQueuedMessage(session.id, msg.id)).toEqual({ + status: 'indeterminate', + localId: 'lid-uncertain', + resolvedId: msg.id, + scheduledAt: null + }) + + const invokedAt = Date.now() + expect(store.messages.markMessagesInvoked(session.id, ['lid-uncertain'], invokedAt)).toBe(1) + expect(store.messages.getMessages(session.id)[0]).toMatchObject({ invokedAt }) + }) }) diff --git a/hub/src/store/messages.ts b/hub/src/store/messages.ts index 5f10ad7e..86fa34d6 100644 --- a/hub/src/store/messages.ts +++ b/hub/src/store/messages.ts @@ -15,6 +15,7 @@ type DbMessageRow = { local_id: string | null invoked_at: number | null scheduled_at: number | null + delivery_state: string | null } export type MessagePosition = { @@ -22,6 +23,8 @@ export type MessagePosition = { seq: number } +export type SteerDeliveryState = 'queued' | 'dispatching' | 'indeterminate' + export class ImportedMessageConflictError extends Error { constructor(readonly localId: string) { super(`Imported message content changed for localId: ${localId}`) @@ -85,13 +88,14 @@ function toStoredMessage(row: DbMessageRow): StoredMessage { seq: row.seq, localId: row.local_id, invokedAt: row.invoked_at ?? null, - scheduledAt: row.scheduled_at ?? null + scheduledAt: row.scheduled_at ?? null, + ...(row.delivery_state && row.delivery_state !== 'queued' ? { deliveryState: 'indeterminate' as const } : {}) } } export type CopyStoredMessageInput = Pick< StoredMessage, - 'content' | 'createdAt' | 'localId' | 'invokedAt' | 'scheduledAt' + 'content' | 'createdAt' | 'localId' | 'invokedAt' | 'scheduledAt' | 'deliveryState' > export function addMessage( @@ -199,9 +203,9 @@ export function copyMessageToSession( const id = randomUUID() db.prepare(` INSERT INTO messages ( - id, session_id, content, created_at, seq, local_id, invoked_at, scheduled_at + id, session_id, content, created_at, seq, local_id, invoked_at, scheduled_at, delivery_state ) VALUES ( - @id, @session_id, @content, @created_at, @seq, @local_id, @invoked_at, @scheduled_at + @id, @session_id, @content, @created_at, @seq, @local_id, @invoked_at, @scheduled_at, @delivery_state ) `).run({ id, @@ -213,7 +217,8 @@ export function copyMessageToSession( seq: nextSeq, local_id: localId ?? null, invoked_at: invokedAt ?? null, - scheduled_at: message.scheduledAt ?? null + scheduled_at: message.scheduledAt ?? null, + delivery_state: message.deliveryState ?? 'queued' }) const row = db.prepare('SELECT * FROM messages WHERE id = ?').get(id) as DbMessageRow | undefined @@ -243,9 +248,9 @@ export function copyMessagesToSession( let nextSeq = getMaxSeq(db, sessionId) + 1 const insert = db.prepare(` INSERT INTO messages ( - id, session_id, content, created_at, seq, local_id, invoked_at, scheduled_at + id, session_id, content, created_at, seq, local_id, invoked_at, scheduled_at, delivery_state ) VALUES ( - @id, @session_id, @content, @created_at, @seq, @local_id, @invoked_at, @scheduled_at + @id, @session_id, @content, @created_at, @seq, @local_id, @invoked_at, @scheduled_at, @delivery_state ) `) const collisionCheck = db.prepare( @@ -273,7 +278,8 @@ export function copyMessagesToSession( seq: nextSeq, local_id: localId ?? null, invoked_at: invokedAt ?? null, - scheduled_at: message.scheduledAt ?? null + scheduled_at: message.scheduledAt ?? null, + delivery_state: message.deliveryState ?? 'queued' }) nextSeq += 1 } @@ -369,6 +375,7 @@ export function getDeliverableMessagesAfter( WHERE session_id = ? AND seq > ? AND (scheduled_at IS NULL OR scheduled_at <= ?) + AND delivery_state = 'queued' ORDER BY seq ASC LIMIT ? `).all(sessionId, safeAfterSeq, now, safeLimit) as DbMessageRow[] @@ -476,10 +483,12 @@ export function bumpMessageEpoch(db: Database, sessionId: string): number { * (including scheduled) for the Web floating bar on refresh / secondary clients. */ export function getUninvokedLocalMessages( db: Database, - sessionId: string + sessionId: string, + options?: { deliverableOnly?: boolean } ): StoredMessage[] { + const deliverableClause = options?.deliverableOnly ? " AND delivery_state = 'queued'" : '' const rows = db.prepare( - 'SELECT * FROM messages WHERE session_id = ? AND invoked_at IS NULL AND local_id IS NOT NULL ORDER BY seq ASC' + `SELECT * FROM messages WHERE session_id = ? AND invoked_at IS NULL AND local_id IS NOT NULL${deliverableClause} ORDER BY seq ASC` ).all(sessionId) as DbMessageRow[] return rows.map(toStoredMessage) } @@ -487,6 +496,7 @@ export function getUninvokedLocalMessages( export type LocalMessageState = { localId: string invokedAt: number | null + deliveryState?: string | null } export function getLocalMessageStates( @@ -499,17 +509,21 @@ export function getLocalMessageStates( } const placeholders = localIds.map(() => '?').join(', ') const rows = db.prepare(` - SELECT local_id, invoked_at + SELECT local_id, invoked_at, delivery_state FROM messages WHERE session_id = ? AND local_id IN (${placeholders}) ORDER BY seq ASC `).all(sessionId, ...localIds) as Array<{ local_id: string invoked_at: number | null + delivery_state: string | null }> return rows.map((row) => ({ localId: row.local_id, - invokedAt: row.invoked_at + invokedAt: row.invoked_at, + ...(row.delivery_state && row.delivery_state !== 'queued' + ? { deliveryState: row.delivery_state } + : {}) })) } @@ -521,7 +535,7 @@ export function getMatureScheduledMessages( beforeTime: number ): StoredMessage[] { const rows = db.prepare( - 'SELECT * FROM messages WHERE scheduled_at IS NOT NULL AND scheduled_at <= ? AND invoked_at IS NULL ORDER BY scheduled_at ASC' + "SELECT * FROM messages WHERE scheduled_at IS NOT NULL AND scheduled_at <= ? AND invoked_at IS NULL AND delivery_state = 'queued' ORDER BY scheduled_at ASC" ).all(beforeTime) as DbMessageRow[] return rows.map(toStoredMessage) } @@ -549,6 +563,7 @@ export function getImmediateQueuedLocalMessages( AND invoked_at IS NULL AND local_id IS NOT NULL AND scheduled_at IS NULL + AND delivery_state = 'queued' ORDER BY seq ASC `).all(sessionId) as DbMessageRow[] return rows.map(toStoredMessage) @@ -583,6 +598,7 @@ export function countFutureScheduledLocalMessages( AND local_id IS NOT NULL AND scheduled_at IS NOT NULL AND scheduled_at > ? + AND delivery_state = 'queued' `).get(sessionId, now) as { count: number } | undefined return row?.count ?? 0 } @@ -607,6 +623,7 @@ export function countFutureScheduledBySessionIds( AND local_id IS NOT NULL AND scheduled_at IS NOT NULL AND scheduled_at > ? + AND delivery_state = 'queued' GROUP BY session_id `).all(...sessionIds, now) as { session_id: string; count: number }[] @@ -636,6 +653,7 @@ export function minFutureScheduledAtBySessionIds( AND local_id IS NOT NULL AND scheduled_at IS NOT NULL AND scheduled_at > ? + AND delivery_state = 'queued' GROUP BY session_id `).all(...sessionIds, now) as { session_id: string; next_at: number }[] @@ -655,6 +673,7 @@ export function getMaxSeq(db: Database, sessionId: string): number { export type CancelQueuedMessageResult = | { status: 'cancelled'; localId: string | null } | { status: 'invoked'; message: StoredMessage } + | { status: 'busy'; localId: string } /** Delete a queued (invoked_at IS NULL) message by session + message id. * @@ -714,6 +733,8 @@ export type LookupQueuedMessageResult = | { status: 'absent' } | { status: 'invoked'; message: StoredMessage } | { status: 'queued'; localId: string | null; resolvedId: string; scheduledAt: number | null } + | { status: 'dispatching'; localId: string | null; resolvedId: string; scheduledAt: number | null } + | { status: 'indeterminate'; localId: string | null; resolvedId: string; scheduledAt: number | null } /** Look up a queued message without deleting it. * @@ -743,6 +764,13 @@ export function lookupQueuedMessage( return { status: 'invoked' as const, message: toStoredMessage(row) } } + if (row.delivery_state === 'dispatching') { + return { status: 'dispatching' as const, localId: row.local_id, resolvedId: row.id, scheduledAt: row.scheduled_at } + } + if (row.delivery_state === 'indeterminate') { + return { status: 'indeterminate' as const, localId: row.local_id, resolvedId: row.id, scheduledAt: row.scheduled_at } + } + return { status: 'queued' as const, localId: row.local_id, resolvedId: row.id, scheduledAt: row.scheduled_at } } @@ -751,6 +779,33 @@ export function lookupQueuedMessage( * This is the "confirmed DELETE" step after the service layer has received a * CLI ack with removed:true. Uses the same first-write-wins guard as the * original cancelQueuedMessage. */ +/** Claim a durable unknown-delivery row for an explicit retry. */ +export function claimIndeterminateMessage( + db: Database, + sessionId: string, + messageId: string +): StoredMessage | null { + return db.transaction(() => { + const claimed = db.prepare(` + UPDATE messages + SET delivery_state = 'dispatching' + WHERE session_id = ? + AND (id = ? OR local_id = ?) + AND invoked_at IS NULL + AND delivery_state = 'indeterminate' + `).run(sessionId, messageId, messageId) + if (claimed.changes === 0) return null + const updated = db.prepare(` + SELECT * FROM messages + WHERE session_id = ? AND (id = ? OR local_id = ?) + AND invoked_at IS NULL + AND delivery_state = 'dispatching' + LIMIT 1 + `).get(sessionId, messageId, messageId) as DbMessageRow | undefined + return updated ? toStoredMessage(updated) : null + })() +} + export function deleteQueuedMessageById( db: Database, sessionId: string, @@ -784,13 +839,46 @@ export function markMessagesInvoked( const placeholders = localIds.map(() => '?').join(', ') return db.prepare( `UPDATE messages - SET invoked_at = ? + SET invoked_at = ?, delivery_state = 'queued' WHERE session_id = ? AND local_id IN (${placeholders}) AND invoked_at IS NULL` ).run(invokedAt, sessionId, ...localIds).changes } +/** Move an uninvoked steer through its durable delivery states. */ +export function setMessagesDeliveryState( + db: Database, + sessionId: string, + localIds: string[], + state: SteerDeliveryState +): number { + if (localIds.length === 0) return 0 + const placeholders = localIds.map(() => '?').join(', ') + const fromStates = state === 'queued' + ? "'dispatching', 'indeterminate'" + : state === 'dispatching' + ? "'queued', 'indeterminate'" + : "'queued', 'dispatching'" + return db.prepare( + `UPDATE messages + SET delivery_state = ? + WHERE session_id = ? + AND local_id IN (${placeholders}) + AND invoked_at IS NULL + AND delivery_state IN (${fromStates})` + ).run(state, sessionId, ...localIds).changes +} + +/** Hold an ambiguous steer out of automatic replay without claiming delivery. */ +export function markMessagesIndeterminate( + db: Database, + sessionId: string, + localIds: string[] +): number { + return setMessagesDeliveryState(db, sessionId, localIds, 'indeterminate') +} + /** Settle immediate queued rows on an archived clear source without touching * scheduled rows, which must remain uninvoked for transfer to the replacement. */ export function markUninvokedImmediateMessages( @@ -804,6 +892,7 @@ export function markUninvokedImmediateMessages( AND local_id IS NOT NULL AND scheduled_at IS NULL AND invoked_at IS NULL + AND delivery_state = 'queued' ORDER BY seq ASC `).all(sessionId) as Array<{ local_id: string }> if (rows.length === 0) return [] @@ -815,6 +904,7 @@ export function markUninvokedImmediateMessages( AND local_id IS NOT NULL AND scheduled_at IS NULL AND invoked_at IS NULL + AND delivery_state = 'queued' `).run(invokedAt, sessionId) return rows.map((row) => row.local_id) } diff --git a/hub/src/store/migration-v13.test.ts b/hub/src/store/migration-v13.test.ts index d11eeb90..ec0ff42c 100644 --- a/hub/src/store/migration-v13.test.ts +++ b/hub/src/store/migration-v13.test.ts @@ -10,7 +10,7 @@ describe('Store V12/V13→V14 schema reconciliation', () => { const store = new Store(':memory:') expect(tableExists(store, 'message_epochs')).toBe(true) expect(tableExists(store, 'session_scratchlist')).toBe(true) - expect(getUserVersion(store)).toBe(24) + expect(getUserVersion(store)).toBe(25) store.close() }) @@ -34,7 +34,7 @@ describe('Store V12/V13→V14 schema reconciliation', () => { store = new Store(dbPath) expect(tableExists(store, 'message_epochs')).toBe(true) expect(tableExists(store, 'session_scratchlist')).toBe(true) - expect(getUserVersion(store)).toBe(24) + expect(getUserVersion(store)).toBe(25) expect(store.messages.getMessageEpoch('session-1')).toBe(0) expect(store.messages.getMessages('session-1')).toHaveLength(1) } finally { @@ -72,7 +72,7 @@ describe('Store V12/V13→V14 schema reconciliation', () => { store = new Store(dbPath) expect(tableExists(store, 'message_epochs')).toBe(true) expect(tableExists(store, 'session_scratchlist')).toBe(true) - expect(getUserVersion(store)).toBe(24) + expect(getUserVersion(store)).toBe(25) expect(store.messages.getMessages('session-1')).toHaveLength(1) } finally { store?.close() diff --git a/hub/src/store/migration-v15.test.ts b/hub/src/store/migration-v15.test.ts index 57fd4079..57113fe4 100644 --- a/hub/src/store/migration-v15.test.ts +++ b/hub/src/store/migration-v15.test.ts @@ -19,7 +19,7 @@ describe('Store V14→V15 migration: scratchlist attachments column', () => { expect(cols).toContain('attachments') expect(getColumns(store, 'usage_events')).toContain('last_input_tokens') expect(getColumns(store, 'usage_scan_state')).toContain('last_seq') - expect(getUserVersion(store)).toBe(24) + expect(getUserVersion(store)).toBe(25) store.close() }) @@ -40,7 +40,7 @@ describe('Store V14→V15 migration: scratchlist attachments column', () => { expect(cols).toContain('attachments') expect(getColumns(store, 'usage_events')).toContain('last_input_tokens') expect(getColumns(store, 'usage_scan_state')).toContain('last_seq') - expect(getUserVersion(store)).toBe(24) + expect(getUserVersion(store)).toBe(25) } finally { store?.close() rmSync(dir, { recursive: true, force: true }) @@ -60,7 +60,7 @@ describe('Store V14→V15 migration: scratchlist attachments column', () => { store2 = new Store(dbPath) const cols2 = getColumns(store2, 'session_scratchlist') expect(cols2).toEqual(cols1) - expect(getUserVersion(store2)).toBe(24) + expect(getUserVersion(store2)).toBe(25) } finally { store2?.close() store1?.close() diff --git a/hub/src/store/migration-v18.test.ts b/hub/src/store/migration-v18.test.ts index 7b0b971c..3e6b4912 100644 --- a/hub/src/store/migration-v18.test.ts +++ b/hub/src/store/migration-v18.test.ts @@ -38,7 +38,7 @@ describe('Store V18->V19 migration: usage scan state', () => { const usageRows = internalDb.prepare('SELECT COUNT(*) AS count FROM usage_events').get() as { count: number } expect(table?.name).toBe('usage_scan_state') - expect(version.user_version).toBe(24) + expect(version.user_version).toBe(25) expect(usageRows.count).toBe(0) } finally { store?.close() diff --git a/hub/src/store/migration-v19.test.ts b/hub/src/store/migration-v19.test.ts index 49f82164..ba3d802d 100644 --- a/hub/src/store/migration-v19.test.ts +++ b/hub/src/store/migration-v19.test.ts @@ -37,7 +37,7 @@ describe('Store V19->current migration: usage re-index', () => { const scanRows = internalDb.prepare('SELECT COUNT(*) AS count FROM usage_scan_state').get() as { count: number } const usageRows = internalDb.prepare('SELECT COUNT(*) AS count FROM usage_events').get() as { count: number } - expect(version.user_version).toBe(24) + expect(version.user_version).toBe(25) expect(scanRows.count).toBe(0) expect(usageRows.count).toBe(0) } finally { diff --git a/hub/src/store/migration-v20.test.ts b/hub/src/store/migration-v20.test.ts index b3a62cb3..2af4f259 100644 --- a/hub/src/store/migration-v20.test.ts +++ b/hub/src/store/migration-v20.test.ts @@ -43,7 +43,7 @@ describe('Store V20->V21 migration: usage semantics re-index', () => { store = new Store(dbPath) const internalDb = (store as unknown as { db: Database }).db const count = (table: string): number => (internalDb.prepare(`SELECT COUNT(*) AS count FROM ${table}`).get() as { count: number }).count - expect((internalDb.prepare('PRAGMA user_version').get() as { user_version: number }).user_version).toBe(24) + expect((internalDb.prepare('PRAGMA user_version').get() as { user_version: number }).user_version).toBe(25) expect(count('usage_events')).toBe(0) expect(count('usage_scan_state')).toBe(0) expect(store.messages.getMessages(session.id)).toHaveLength(1) diff --git a/hub/src/store/migration-v23.test.ts b/hub/src/store/migration-v23.test.ts index f587518c..cd2733b3 100644 --- a/hub/src/store/migration-v23.test.ts +++ b/hub/src/store/migration-v23.test.ts @@ -13,7 +13,7 @@ afterEach(() => { } }) -describe('schema migration v22 to v23', () => { +describe('schema migration v22 to v25', () => { it('adds events and event_links tables to a V22 database', () => { const dir = mkdtempSync(join(tmpdir(), 'hapi-migration-v23-')) tempDirs.push(dir) @@ -40,7 +40,9 @@ describe('schema migration v22 to v23', () => { expect(events?.name).toBe('events') expect(links?.name).toBe('event_links') - expect(version.user_version).toBe(24) + const columns = internalDb.prepare('PRAGMA table_info(messages)').all() as Array<{ name: string }> + expect(columns.map((column) => column.name)).toContain('delivery_state') + expect(version.user_version).toBe(25) migrated.close() }) }) diff --git a/hub/src/store/migration-v24.test.ts b/hub/src/store/migration-v24.test.ts index 94e437bc..7dd95179 100644 --- a/hub/src/store/migration-v24.test.ts +++ b/hub/src/store/migration-v24.test.ts @@ -13,7 +13,7 @@ afterEach(() => { } }) -describe('schema migration v23 to v24', () => { +describe('schema migration v23 to v25', () => { it('adds fcm_devices.push_key to a V23 database and keeps existing rows', () => { const dir = mkdtempSync(join(tmpdir(), 'hapi-migration-v24-')) tempDirs.push(dir) @@ -35,7 +35,9 @@ describe('schema migration v23 to v24', () => { const version = internalDb.prepare('PRAGMA user_version').get() as { user_version: number } expect(columns.some((col) => col.name === 'push_key')).toBe(true) - expect(version.user_version).toBe(24) + const messageColumns = internalDb.prepare('PRAGMA table_info(messages)').all() as Array<{ name: string }> + expect(messageColumns.some((col) => col.name === 'delivery_state')).toBe(true) + expect(version.user_version).toBe(25) // Existing Android rows survive with a NULL push key. const devices = migrated.fcm.getDevicesByNamespace('default') @@ -53,4 +55,26 @@ describe('schema migration v23 to v24', () => { expect(migrated.fcm.getDevicesByNamespace('default', ['ios'])).toHaveLength(1) migrated.close() }) + + it('adds messages.delivery_state to an already-upgraded V24 database', () => { + const dir = mkdtempSync(join(tmpdir(), 'hapi-migration-v24-delivery-')) + tempDirs.push(dir) + const dbPath = join(dir, 'hapi.db') + + new Store(dbPath).close() + const legacy = new Database(dbPath) + legacy.exec(` + ALTER TABLE messages DROP COLUMN delivery_state; + PRAGMA user_version = 24; + `) + legacy.close() + + const migrated = new Store(dbPath) + const internalDb = (migrated as unknown as { db: Database }).db + const columns = internalDb.prepare('PRAGMA table_info(messages)').all() as Array<{ name: string }> + const version = internalDb.prepare('PRAGMA user_version').get() as { user_version: number } + expect(columns.some((col) => col.name === 'delivery_state')).toBe(true) + expect(version.user_version).toBe(25) + migrated.close() + }) }) diff --git a/hub/src/store/types.ts b/hub/src/store/types.ts index 9a84a9fc..d22773b8 100644 --- a/hub/src/store/types.ts +++ b/hub/src/store/types.ts @@ -38,6 +38,8 @@ export type StoredMachine = { seq: number } +export type MessageDeliveryState = 'indeterminate' + export type StoredMessage = { id: string sessionId: string @@ -47,6 +49,8 @@ export type StoredMessage = { localId: string | null invokedAt: number | null scheduledAt: number | null + /** Omitted for ordinary queued/delivered rows; set when steer outcome is unknown. */ + deliveryState?: MessageDeliveryState } export type StoredUser = { diff --git a/hub/src/sync/messageService.test.ts b/hub/src/sync/messageService.test.ts index 8d76c9ec..1c7d6648 100644 --- a/hub/src/sync/messageService.test.ts +++ b/hub/src/sync/messageService.test.ts @@ -497,16 +497,64 @@ describe('MessageService.getQueuedState', () => { 'local-other' ])).toEqual({ queuedLocalIds: ['local-queued', 'local-future'], + indeterminateLocalIds: [], invokedLocalMessages: [{ localId: 'local-invoked', invokedAt: 1_000 }] }) expect(service.getQueuedState(session.id, [])).toEqual({ queuedLocalIds: [], + indeterminateLocalIds: [], invokedLocalMessages: [] }) }) }) describe('MessageService.cancelQueuedMessage race scenarios', () => { + describe('indeterminate cancel recheck', () => { + it('returns invoked when consumption wins during the cancel ACK wait', async () => { + const store = makeStore() + const session = makeSession(store, 'race-indeterminate-cancel') + const msg = store.messages.addMessage( + session.id, + { role: 'user', content: { type: 'text', text: 'hello' } }, + 'local-indeterminate-cancel' + ) + store.messages.markMessagesIndeterminate(session.id, [msg.localId!]) + const publisher = makePublisher() + const io = makeIo((callback) => { + store.messages.markMessagesInvoked(session.id, [msg.localId!], 2_000) + callback(null, [{ removed: false }]) + }) + + const service = new MessageService(store, io, publisher as any) + const result = await service.cancelQueuedMessage(session.id, msg.id) + + expect(result.status).toBe('invoked') + expect(publisher.events.some((event) => event.type === 'message-cancelled')).toBe(false) + }) + }) + + describe('indeterminate explicit cancel', () => { + it('releases and deletes a held reservation after confirmed removal', async () => { + const store = makeStore() + const session = makeSession(store, 'indeterminate-cancel') + const msg = store.messages.addMessage( + session.id, + { role: 'user', content: { type: 'text', text: 'discard' } }, + 'local-indeterminate' + ) + store.messages.markMessagesIndeterminate(session.id, [msg.localId!]) + const publisher = makePublisher() + const service = new MessageService(store, makeIo((callback) => { + callback(null, [{ removed: true }]) + }), publisher as any) + + const result = await service.cancelQueuedMessage(session.id, msg.id) + expect(result).toEqual({ status: 'cancelled', localId: 'local-indeterminate' }) + expect(store.messages.lookupQueuedMessage(session.id, msg.id)).toEqual({ status: 'absent' }) + expect(publisher.events.some((event) => event.type === 'message-cancelled')).toBe(true) + }) + }) + describe('Race-A: CLI ack removed:true → DELETE + status=cancelled', () => { it('returns cancelled and emits message-cancelled SSE after CLI confirms removal', async () => { const store = makeStore() diff --git a/hub/src/sync/messageService.ts b/hub/src/sync/messageService.ts index b7ff690c..3b714fa3 100644 --- a/hub/src/sync/messageService.ts +++ b/hub/src/sync/messageService.ts @@ -42,7 +42,8 @@ function toDecryptedMessage(message: StoredMessageForDelivery): DecryptedMessage content: message.content, createdAt: message.createdAt, invokedAt: message.invokedAt, - scheduledAt: message.scheduledAt + scheduledAt: message.scheduledAt, + ...(message.deliveryState ? { deliveryState: message.deliveryState } : {}) } } @@ -100,6 +101,13 @@ function getNormalizedDeliveryMode( * provenance for Web diagnostics, but make deferred CLI delivery an ordinary * queue item so it cannot steer a later generation. */ +export type RetryIndeterminateMessageResult = + | { status: 'retried'; localId: string } + | { status: 'already-queued'; localId: string | null } + | { status: 'retry-unavailable'; localId: string } + | { status: 'invoked'; message: DecryptedMessage } + | { status: 'not-found' } + function contentForDeferredDelivery(content: unknown): unknown { if (!isObject(content) || content.role !== 'user' || !isObject(content.meta)) { return content @@ -117,6 +125,7 @@ function contentForDeferredDelivery(content: unknown): unknown { export class MessageService { /** One scheduled-matured SSE per localId per hub process (cleared on cancel/consume paths here). */ private readonly scheduledMatureNotifiedLocalIds = new Set() + private readonly activeIndeterminateRetries = new Set() constructor( private readonly store: Store, @@ -132,6 +141,19 @@ export class MessageService { } } + private recordConsumedAcknowledgement( + sessionId: string, + localId: string, + ): CancelQueuedMessageResult { + const invokedAt = Date.now() + this.store.messages.markMessagesInvoked(sessionId, [localId], invokedAt) + this.publisher.emit({ type: 'messages-consumed', sessionId, localIds: [localId], invokedAt }) + const settled = this.store.messages.lookupQueuedMessage(sessionId, localId) + return settled.status === 'invoked' + ? settled + : { status: 'cancelled', localId } + } + getMessages(sessionId: string, limit: number = 200): DecryptedMessage[] { const stored = this.store.messages.getMessages(sessionId, limit) return toVisibleDecryptedMessages(stored) @@ -141,7 +163,10 @@ export class MessageService { const states = this.store.messages.getLocalMessageStates(sessionId, localIds) return { queuedLocalIds: states - .filter((state) => state.invokedAt === null) + .filter((state) => state.invokedAt === null && state.deliveryState !== 'indeterminate' && state.deliveryState !== 'dispatching') + .map((state) => state.localId), + indeterminateLocalIds: states + .filter((state) => state.invokedAt === null && (state.deliveryState === 'indeterminate' || state.deliveryState === 'dispatching')) .map((state) => state.localId), invokedLocalMessages: states.flatMap((state) => state.invokedAt === null ? [] @@ -399,7 +424,8 @@ export class MessageService { content: contentForDeferredDelivery(message.content), createdAt: message.createdAt, invokedAt: message.invokedAt, - scheduledAt: message.scheduledAt + scheduledAt: message.scheduledAt, + ...(message.deliveryState ? { deliveryState: message.deliveryState } : {}) })) } @@ -423,9 +449,11 @@ export class MessageService { return lookup } - // Phase 2: row is still queued. Ask the CLI whether it already shifted the item + // Phase 2: row is still queued. Ask the CLI whether it already shifted the item // (race window between collectBatch() shift and messages-consumed ack). const { localId, resolvedId, scheduledAt } = lookup + const isDispatching = lookup.status === 'dispatching' + const isIndeterminate = lookup.status === 'indeterminate' if (!localId) { // No localId — row exists but has no cancel path; treat as cancelled. @@ -434,6 +462,56 @@ export class MessageService { return { status: 'cancelled', localId: null } } + // A live dispatch is not cancellable by timeout. Convert it to the + // durable unknown state and require a second explicit resolution. + if (isDispatching) { + const ackResult = await this.requestCliCancelAck(sessionId, localId, messageId, 500) + if (ackResult === 'consumed') { + return this.recordConsumedAcknowledgement(sessionId, localId) + } + // The native request may have reached the agent while the cancel + // round-trip was pending. Never delete a live dispatch; hold it as + // unknown and let the user explicitly retry or discard afterwards. + const changed = this.store.messages.setMessagesDeliveryState(sessionId, [localId], 'indeterminate') + if (changed === 0) { + const settled = this.store.messages.lookupQueuedMessage(sessionId, resolvedId) + if (settled.status === 'invoked') return settled + if (settled.status === 'absent') return { status: 'cancelled', localId } + } else { + this.publisher.emit({ type: 'messages-indeterminate', sessionId, localIds: [localId] }) + } + return { status: 'busy', localId } + } + + // An indeterminate steer is never converted to invoked by a cancel + // timeout. Explicit cancel resolves it by discarding the durable row; + // an online CLI still gets a chance to remove its held reservation. + if (isIndeterminate) { + const roomName = `session:${sessionId}` + const cliCount = this.io.of('/cli').adapter.rooms.get(roomName)?.size ?? 0 + const ackResult = cliCount > 0 + ? await this.requestCliCancelAck(sessionId, localId, messageId, 500) + : 'timeout' as const + if (ackResult === 'consumed') { + return this.recordConsumedAcknowledgement(sessionId, localId) + } + if (ackResult === 'in-flight' || ackResult === 'indeterminate' || (ackResult === 'timeout' && cliCount > 0)) { + return { status: 'busy', localId } + } + this.store.messages.deleteQueuedMessageById(sessionId, resolvedId) + const recheck = this.store.messages.lookupQueuedMessage(sessionId, resolvedId) + if (recheck.status === 'invoked') { + // The steer won the race while the cancel ACK was in flight; + // never broadcast cancellation over a delivered row. + return recheck + } + if (recheck.status !== 'absent') { + return { status: 'busy', localId } + } + this.publisher.emit({ type: 'message-cancelled', sessionId, messageId, localId }) + return { status: 'cancelled', localId } + } + // Phase 2b: future-scheduled messages were never emitted to the CLI, so they // are not in the CLI's in-memory queue. Asking the CLI whether it can remove // the item would always return 'not-found', which the normal ack path @@ -499,6 +577,16 @@ export class MessageService { const ackResult = await this.requestCliCancelAck(sessionId, localId, messageId, 500) + if (ackResult === 'consumed') { + return this.recordConsumedAcknowledgement(sessionId, localId) + } + if (ackResult === 'in-flight') { + // The row is inside an async steer (mid-turn delivery): it can + // neither be removed nor stamped invoked — the steer's eventual + // accept/reject decides. Report busy so the caller keeps the row. + return { status: 'busy', localId } + } + if (ackResult === 'not-found' || ackResult === 'timeout') { // CLI could not remove the item — it was already shift()-ed or CLI is // offline. Stamp invoked_at immediately so the message lands in the thread @@ -548,6 +636,104 @@ export class MessageService { return { status: 'cancelled', localId } } + async retryIndeterminateMessage( + sessionId: string, + messageId: string + ): Promise { + const lookup = this.store.messages.lookupQueuedMessage(sessionId, messageId) + if (lookup.status === 'absent') return { status: 'not-found' } + if (lookup.status === 'invoked') { + return { + status: 'invoked', + message: toDecryptedMessage(lookup.message) + } + } + if (lookup.status === 'queued') { + return { status: 'already-queued', localId: lookup.localId } + } + if (!lookup.localId) return { status: 'not-found' } + const retryKey = `${sessionId}:${lookup.localId}` + if (this.activeIndeterminateRetries.has(retryKey)) { + return { status: 'retry-unavailable', localId: lookup.localId } + } + this.activeIndeterminateRetries.add(retryKey) + + try { + const roomName = `session:${sessionId}` + const cliCount = this.io.of('/cli').adapter.rooms.get(roomName)?.size ?? 0 + if (this.store.isOpenCodeClearDeliveryGated(sessionId) || cliCount !== 1) { + return { status: 'retry-unavailable', localId: lookup.localId } + } + + const cancelResult = await this.requestCliCancelAck(sessionId, lookup.localId, messageId, 500) + if (cancelResult === 'consumed') { + const settled = this.recordConsumedAcknowledgement(sessionId, lookup.localId) + return settled.status === 'invoked' + ? { status: 'invoked', message: toDecryptedMessage(settled.message) } + : { status: 'not-found' } + } + if (cancelResult === 'in-flight' || cancelResult === 'timeout') { + return { status: 'retry-unavailable', localId: lookup.localId } + } + const refreshed = this.store.messages.lookupQueuedMessage(sessionId, messageId) + if (refreshed.status === 'invoked') { + return { status: 'invoked', message: toDecryptedMessage(refreshed.message) } + } + if (refreshed.status === 'absent') return { status: 'not-found' } + if (refreshed.status === 'dispatching') { + const changed = this.store.messages.setMessagesDeliveryState(sessionId, [lookup.localId], 'indeterminate') + if (changed === 0) return { status: 'retry-unavailable', localId: lookup.localId } + } + + const message = this.store.messages.claimIndeterminateMessage(sessionId, messageId) + if (!message || !message.localId) return { status: 'not-found' } + + const update = { + id: message.id, + seq: message.seq, + createdAt: message.createdAt, + body: { + t: 'retry-queued-message' as const, + sid: sessionId, + messageId: message.id, + localId: message.localId, + message: { + id: message.id, + seq: message.seq, + createdAt: message.createdAt, + localId: message.localId, + content: contentForDeferredDelivery(message.content) + } + } + } + const room = this.io.of('/cli').to(roomName) + const accepted = await new Promise((resolve) => { + room.timeout(500).emit( + 'update', + update, + (_err: Error | null, responses: Array<{ accepted?: boolean }>) => { + resolve(responses?.some((response) => response.accepted === true) ?? false) + } + ) + }) + if (!accepted) { + this.store.messages.setMessagesDeliveryState(sessionId, [message.localId], 'indeterminate') + this.publisher.emit({ type: 'messages-indeterminate', sessionId, localIds: [message.localId] }) + return { status: 'retry-unavailable', localId: message.localId } + } + const requeued = this.store.messages.setMessagesDeliveryState(sessionId, [message.localId], 'queued') + if (requeued === 0) { + const settled = this.store.messages.lookupQueuedMessage(sessionId, message.id) + if (settled.status === 'invoked') return { status: 'invoked', message: toDecryptedMessage(settled.message) } + return { status: 'retry-unavailable', localId: message.localId } + } + this.publisher.emit({ type: 'messages-requeued', sessionId, localIds: [message.localId] }) + return { status: 'retried', localId: message.localId } + } finally { + this.activeIndeterminateRetries.delete(retryKey) + } + } + /** * Ask the CLI (via socket.io ack) whether it removed the in-memory queue item. * Returns 'removed', 'not-found', or 'timeout'. @@ -561,7 +747,7 @@ export class MessageService { localId: string, messageId: string, timeoutMs: number - ): Promise<'removed' | 'not-found' | 'timeout'> { + ): Promise<'removed' | 'in-flight' | 'indeterminate' | 'consumed' | 'not-found' | 'timeout'> { return new Promise((resolve) => { const room = this.io.of('/cli').to(`session:${sessionId}`) // socket.io v4 BroadcastOperator: .timeout(ms).emit(event, data, ackCb) @@ -579,11 +765,25 @@ export class MessageService { localId } }, - (err: Error | null, responses: Array<{ removed: boolean }>) => { + (err: Error | null, responses: Array<{ removed: boolean; inFlight?: boolean; indeterminate?: boolean; consumed?: boolean }>) => { // Check responses before err: in a reconnect overlap or any room with // multiple CLI sockets, Socket.IO may set err (one socket timed out) // while still delivering successful responses from the sockets that did - // ack. Any confirmed removal wins over the partial timeout. + // ack. An explicit in-flight report dominates: one socket may be + // dispatching the steer while a stale duplicate socket reports + // removed — deleting the row then would orphan the executing message. + if (responses?.some((r) => r.consumed === true)) { + resolve('consumed') + return + } + if (responses?.some((r) => r.indeterminate === true)) { + resolve('indeterminate') + return + } + if (responses?.some((r) => r.inFlight === true)) { + resolve('in-flight') + return + } const removed = responses?.some((r) => r.removed === true) ?? false if (removed) { resolve('removed') @@ -657,6 +857,7 @@ export class MessageService { const cliContent = inserted.inserted ? msg.content : contentForDeferredDelivery(msg.content) + const shouldEmitToCli = msg.deliveryState !== 'indeterminate' this.onSessionActivity?.(actualSessionId, msg.createdAt) // Only emit to CLI if the message is not scheduled for the future. @@ -666,7 +867,7 @@ export class MessageService { // the pre-insert `now` capture could misclassify a borderline scheduledAt // as future when it has already become past by the time we check. const isFutureScheduled = msg.scheduledAt !== null && msg.scheduledAt > Date.now() - if (!isFutureScheduled && !this.store.isOpenCodeClearDeliveryGated(actualSessionId)) { + if (shouldEmitToCli && !isFutureScheduled && !this.store.isOpenCodeClearDeliveryGated(actualSessionId)) { const update = { id: msg.id, seq: msg.seq, @@ -697,7 +898,8 @@ export class MessageService { content: msg.content, createdAt: msg.createdAt, invokedAt: msg.invokedAt, - scheduledAt: msg.scheduledAt + scheduledAt: msg.scheduledAt, + ...(msg.deliveryState ? { deliveryState: msg.deliveryState } : {}) } }) return { actualSessionId, createdAt: msg.createdAt } @@ -763,7 +965,7 @@ export class MessageService { /** Release a completed clear handoff in finalized seq order. */ releaseDeliverableQueuedMessages(sessionId: string, now: number = Date.now()): number { if (this.store.isOpenCodeClearDeliveryGated(sessionId)) return 0 - const queued = this.store.messages.getUninvokedLocalMessages(sessionId) + const queued = this.store.messages.getUninvokedLocalMessages(sessionId, { deliverableOnly: true }) .filter((msg) => msg.scheduledAt === null || msg.scheduledAt <= now) for (const msg of queued) { const update = { diff --git a/hub/src/sync/steerQueuedMessage.test.ts b/hub/src/sync/steerQueuedMessage.test.ts index 78b51e87..8515054a 100644 --- a/hub/src/sync/steerQueuedMessage.test.ts +++ b/hub/src/sync/steerQueuedMessage.test.ts @@ -48,12 +48,12 @@ describe('SyncEngine.steerQueuedMessage', () => { } }) - it('rejects non-pi sessions without invoking the CLI', async () => { + it('rejects unsupported flavors without invoking the CLI', async () => { const { store, engine } = createEngine() try { const session = engine.getOrCreateSession( - 'steer-codex', - { path: '/tmp/project', host: 'localhost', flavor: 'codex' }, + 'steer-claude', + { path: '/tmp/project', host: 'localhost', flavor: 'claude' }, { requests: {}, completedRequests: {} }, 'default' ) @@ -63,7 +63,7 @@ describe('SyncEngine.steerQueuedMessage', () => { expect(result).toEqual({ status: 'failed', - error: 'Steering is only supported for Pi sessions', + error: 'Steering is only supported for Pi and Codex sessions', localId: null }) } finally { diff --git a/hub/src/sync/syncEngine.ts b/hub/src/sync/syncEngine.ts index e0c4f2e1..28212f2b 100644 --- a/hub/src/sync/syncEngine.ts +++ b/hub/src/sync/syncEngine.ts @@ -7,7 +7,7 @@ * - No E2E encryption; data is stored as JSON in SQLite */ -import { isKnownFlavor, type LocalResumeTarget, type ResumableSession, type SessionEndReason } from '@hapi/protocol' +import { isKnownFlavor, isSteeringSupportedForSession, type LocalResumeTarget, type ResumableSession, type SessionEndReason } from '@hapi/protocol' import { cliBinaryUpdatedOnDisk, isMachineCapabilitySkewed, @@ -27,7 +27,7 @@ import { CursorLegacyMigrator, type CursorLegacyMigratorOptions } from '../curso import { EventPublisher, type SyncEventListener } from './eventPublisher' import { MachineCache, type Machine } from './machineCache' -import { MessageService } from './messageService' +import { MessageService, type RetryIndeterminateMessageResult } from './messageService' import { createTitleSuggestionService, type TitleSuggestionService } from './titleSuggestion' import { selectForkTranscriptPrefix } from './forkTranscript' import { @@ -1027,6 +1027,13 @@ export class SyncEngine { return this.messageService.cancelQueuedMessage(sessionId, messageId) } + async retryIndeterminateMessage( + sessionId: string, + messageId: string + ): Promise { + return this.messageService.retryIndeterminateMessage(sessionId, messageId) + } + /** * Ask the CLI to deliver one waiting-queue message into the active Pi turn * (Pi native steer). Only pi sessions support this today; the CLI's @@ -1040,8 +1047,8 @@ export class SyncEngine { if (!session) { return { status: 'failed', error: 'Session not found', localId: null } } - if (session.metadata?.flavor !== 'pi') { - return { status: 'failed', error: 'Steering is only supported for Pi sessions', localId: null } + if (!isSteeringSupportedForSession(session.metadata)) { + return { status: 'failed', error: 'Steering is only supported for Pi and Codex sessions', localId: null } } if (session.agentState?.controlledByUser === true) { return { status: 'failed', error: 'Steering is only available for remote sessions', localId: null } diff --git a/hub/src/web/routes/messages.test.ts b/hub/src/web/routes/messages.test.ts index 55b96cdd..331b51bd 100644 --- a/hub/src/web/routes/messages.test.ts +++ b/hub/src/web/routes/messages.test.ts @@ -26,6 +26,7 @@ function createApp(opts: { getMessagesPage?: GetMessagesPage getQueuedState?: (sessionId: string, localIds: string[]) => { queuedLocalIds: string[] + indeterminateLocalIds: string[] invokedLocalMessages: Array<{ localId: string; invokedAt: number }> } steerQueuedMessage?: (sessionId: string, messageId: string) => Promise @@ -39,6 +40,7 @@ function createApp(opts: { queuedStateCalls.push({ sessionId, localIds }) return { queuedLocalIds: localIds.filter((localId) => localId.startsWith('queued-')), + indeterminateLocalIds: [], invokedLocalMessages: localIds .filter((localId) => localId.startsWith('invoked-')) .map((localId) => ({ localId, invokedAt: 1_000 })) @@ -447,6 +449,7 @@ describe('POST /api/sessions/:id/messages/queued-state', () => { expect(response.status).toBe(200) expect(await response.json()).toEqual({ queuedLocalIds: ['queued-2', 'queued-1'], + indeterminateLocalIds: [], invokedLocalMessages: [{ localId: 'invoked-1', invokedAt: 1_000 }] }) expect(queuedStateCalls).toEqual([{ @@ -465,7 +468,7 @@ describe('POST /api/sessions/:id/messages/queued-state', () => { }) expect(response.status).toBe(200) - expect(await response.json()).toEqual({ queuedLocalIds: [], invokedLocalMessages: [] }) + expect(await response.json()).toEqual({ queuedLocalIds: [], indeterminateLocalIds: [], invokedLocalMessages: [] }) expect(queuedStateCalls).toHaveLength(0) }) diff --git a/hub/src/web/routes/messages.ts b/hub/src/web/routes/messages.ts index dcd33d7c..92e46e22 100644 --- a/hub/src/web/routes/messages.ts +++ b/hub/src/web/routes/messages.ts @@ -77,6 +77,22 @@ export function createMessagesRoutes(getSyncEngine: () => SyncEngine | null): Ho return c.json(result) }) + app.post('/sessions/:id/messages/:messageId/retry', async (c) => { + const engine = requireSyncEngine(c, getSyncEngine) + if (engine instanceof Response) { + return engine + } + + const sessionResult = requireSessionFromParam(c, engine) + if (sessionResult instanceof Response) { + return sessionResult + } + return c.json(await engine.retryIndeterminateMessage( + sessionResult.sessionId, + c.req.param('messageId') + )) + }) + app.post('/sessions/:id/messages/queued-state', async (c) => { const engine = requireSyncEngine(c, getSyncEngine) if (engine instanceof Response) { @@ -97,7 +113,7 @@ export function createMessagesRoutes(getSyncEngine: () => SyncEngine | null): Ho const localIds = [...new Set(parsed.data.localIds)] if (localIds.length === 0) { - return c.json({ queuedLocalIds: [], invokedLocalMessages: [] }) + return c.json({ queuedLocalIds: [], indeterminateLocalIds: [], invokedLocalMessages: [] }) } return c.json(engine.getQueuedState(sessionId, localIds)) }) diff --git a/ios/Hapi/Features/Chat/Composer/QueuedMessagesBarView.swift b/ios/Hapi/Features/Chat/Composer/QueuedMessagesBarView.swift index b0dc0c0a..5c8dc630 100644 --- a/ios/Hapi/Features/Chat/Composer/QueuedMessagesBarView.swift +++ b/ios/Hapi/Features/Chat/Composer/QueuedMessagesBarView.swift @@ -46,6 +46,11 @@ private struct QueuedRowView: View { Text(row.text.isEmpty ? row.attachmentNames.joined(separator: ", ") : row.text) .font(.footnote) .lineLimit(2) + if row.indeterminate { + Text("Delivery outcome unknown") + .font(.caption2) + .foregroundStyle(.red) + } if let scheduledAt = row.scheduledAt { Text("Scheduled · \(Self.timeLabel(scheduledAt))") .font(.caption2) @@ -53,7 +58,13 @@ private struct QueuedRowView: View { } } Spacer(minLength: 8) - if row.canSteer { + if row.indeterminate { + Button("Retry") { + interactor.retryIndeterminateMessage(row.id) + } + .font(.footnote.weight(.medium)) + .disabled(!row.canAct) + } else if row.canSteer { Button("Steer") { interactor.steerQueuedMessage(row.id) } diff --git a/ios/Packages/HapiKit/Sources/HapiClient/Chat/ChatInteractionState.swift b/ios/Packages/HapiKit/Sources/HapiClient/Chat/ChatInteractionState.swift index f12438d4..e415986b 100644 --- a/ios/Packages/HapiKit/Sources/HapiClient/Chat/ChatInteractionState.swift +++ b/ios/Packages/HapiKit/Sources/HapiClient/Chat/ChatInteractionState.swift @@ -35,6 +35,7 @@ public struct QueuedMessageRow: Equatable, Sendable, Identifiable { public let canAct: Bool /// Steer offered: turn active, not future-scheduled, actionable. public let canSteer: Bool + public let indeterminate: Bool public init( id: String, @@ -43,7 +44,8 @@ public struct QueuedMessageRow: Equatable, Sendable, Identifiable { attachmentNames: [String], scheduledAt: Int?, canAct: Bool, - canSteer: Bool + canSteer: Bool, + indeterminate: Bool = false ) { self.id = id self.localId = localId @@ -52,6 +54,7 @@ public struct QueuedMessageRow: Equatable, Sendable, Identifiable { self.scheduledAt = scheduledAt self.canAct = canAct self.canSteer = canSteer + self.indeterminate = indeterminate } } diff --git a/ios/Packages/HapiKit/Sources/HapiClient/Chat/ChatInteractor.swift b/ios/Packages/HapiKit/Sources/HapiClient/Chat/ChatInteractor.swift index ec9db066..c53708fa 100644 --- a/ios/Packages/HapiKit/Sources/HapiClient/Chat/ChatInteractor.swift +++ b/ios/Packages/HapiKit/Sources/HapiClient/Chat/ChatInteractor.swift @@ -417,7 +417,8 @@ public final class ChatInteractor { attachmentNames: preview.attachmentNames, scheduledAt: row.scheduledAt, canAct: canAct, - canSteer: canAct && thinking && row.scheduledAt == nil + canSteer: canAct && thinking && row.scheduledAt == nil && row.status != .indeterminate, + indeterminate: row.status == .indeterminate ) } } @@ -434,6 +435,7 @@ public final class ChatInteractor { private enum CancelVerdict { case cancelled case invoked + case busy } /// The cancel verdict, or nil on guard/error. @@ -454,6 +456,10 @@ public final class ChatInteractor { case .invoked(let message): await store.applyCancelInvoked(localId: localId, message: WindowMessage(wire: message)) return .invoked + case .busy: + await store.appendOptimistic(row.withDeliveryState("indeterminate")) + try? await store.reconcileQueuedState() + return .busy } } catch { await store.appendOptimistic(row) @@ -462,6 +468,32 @@ public final class ChatInteractor { } } + public func retryIndeterminateMessage(_ messageId: String) { + guard !queuedOpPending else { return } + queuedOpPending = true + Task { [weak self] in + guard let self else { return } + defer { self.queuedOpPending = false } + do { + let response = try await api.retryIndeterminateMessage(sessionId: sessionId, messageId: messageId) + if response.status == "invoked", let message = response.message, + let localId = message.localId, let invokedAt = message.invokedAt { + await (windowController()).markConsumed(localIds: [localId], invokedAt: invokedAt) + } else if response.status == "retried" || response.status == "already-queued", + let localId = response.localId { + await (windowController()).markRequeued(localIds: [localId]) + } else if response.status == "not-found" { + await (windowController()).removeMessage(localIdOrId: messageId) + emit(.notice("Message is no longer available")) + } else if response.status == "retry-unavailable" { + emit(.notice("Delivery is still being resolved")) + } + } catch { + emit(.notice(Self.errorMessage(error, fallback: "Failed to retry message"))) + } + } + } + /// Edit = cancel + prefill composer (kept when the operator typed /// meanwhile). public func editQueuedMessage(_ messageId: String) { @@ -485,6 +517,8 @@ public final class ChatInteractor { } case .invoked: self.emit(.notice("Already delivered to the agent")) + case .busy: + self.emit(.notice("Delivery outcome is unknown; message remains queued")) case nil: break } diff --git a/ios/Packages/HapiKit/Sources/HapiClient/Endpoints/MessageEndpoints.swift b/ios/Packages/HapiKit/Sources/HapiClient/Endpoints/MessageEndpoints.swift index 4d2f325c..337867f3 100644 --- a/ios/Packages/HapiKit/Sources/HapiClient/Endpoints/MessageEndpoints.swift +++ b/ios/Packages/HapiKit/Sources/HapiClient/Endpoints/MessageEndpoints.swift @@ -79,6 +79,15 @@ extension APIClient { ) } + /// `POST /api/sessions/:id/messages/:messageId/retry` — explicitly retry + /// an indeterminate delivery. + public func retryIndeterminateMessage(sessionId: String, messageId: String) async throws -> RetryIndeterminateMessageResponse { + try await request( + .post, + "/api/sessions/\(encodePathComponent(sessionId))/messages/\(encodePathComponent(messageId))/retry" + ) + } + /// `POST /api/sessions/:id/messages/:messageId/steer` — promote a queued /// message to steer delivery. public func steerMessage(sessionId: String, messageId: String) async throws -> SteerQueuedMessageResponse { diff --git a/ios/Packages/HapiKit/Sources/HapiClient/Stores/MessageWindowController.swift b/ios/Packages/HapiKit/Sources/HapiClient/Stores/MessageWindowController.swift index a1613ff9..dbcbcf74 100644 --- a/ios/Packages/HapiKit/Sources/HapiClient/Stores/MessageWindowController.swift +++ b/ios/Packages/HapiKit/Sources/HapiClient/Stores/MessageWindowController.swift @@ -434,6 +434,12 @@ public actor MessageWindowController { case .messagesConsumed(_, let eventSessionId, let localIds, let invokedAt) where eventSessionId == sessionId: markConsumed(localIds: localIds, invokedAt: invokedAt) + case .messagesIndeterminate(_, let eventSessionId, let localIds) + where eventSessionId == sessionId: + markIndeterminate(localIds: localIds) + case .messagesRequeued(_, let eventSessionId, let localIds) + where eventSessionId == sessionId: + markRequeued(localIds: localIds) case .messageCancelled(_, let eventSessionId, let messageId, _) where eventSessionId == sessionId: removeMessage(localIdOrId: messageId) case .messagesInvalidated(_, let eventSessionId) where eventSessionId == sessionId: @@ -460,6 +466,16 @@ public actor MessageWindowController { persist() } + public func markIndeterminate(localIds: [String]) { + update { MessageWindowLogic.markIndeterminate($0, localIds: localIds) } + persist() + } + + public func markRequeued(localIds: [String]) { + update { MessageWindowLogic.markRequeued($0, localIds: localIds) } + persist() + } + /// SSE `message-cancelled` / optimistic DELETE removal /// (web `removeOptimisticMessage`). public func removeMessage(localIdOrId: String) { @@ -539,6 +555,7 @@ public actor MessageWindowController { let candidateLocalIds = queuedReconcileCandidateLocalIds() guard !candidateLocalIds.isEmpty else { return } var queuedLocalIds: [String] = [] + var indeterminateLocalIds: [String] = [] var invokedLocalMessages: [(localId: String, invokedAt: Int)] = [] var start = 0 while start < candidateLocalIds.count { @@ -546,6 +563,7 @@ public actor MessageWindowController { let batch = Array(candidateLocalIds[start.. MessageWindowState { + if localIds.isEmpty { return previous } + let idSet = Set(localIds) + var changed = false + let messages = previous.messages.map { message -> WindowMessage in + guard let localId = message.localId, idSet.contains(localId), message.status != .indeterminate else { + return message + } + changed = true + return message.withStatus(.indeterminate) + } + return changed ? settingMessages(previous, messages) : previous + } + + public static func markRequeued( + _ previous: MessageWindowState, + localIds: [String] + ) -> MessageWindowState { + if localIds.isEmpty { return previous } + let idSet = Set(localIds) + var changed = false + let messages = previous.messages.map { message -> WindowMessage in + guard let localId = message.localId, idSet.contains(localId), message.status == .indeterminate else { + return message + } + changed = true + return message.withStatus(.queued) + } + return changed ? settingMessages(previous, messages) : previous + } + /// `message-cancelled` / optimistic DELETE removal: matches localId OR /// id; idempotent. public static func removeByLocalIdOrId( diff --git a/ios/Packages/HapiKit/Sources/HapiProtocol/Window/WindowMessage.swift b/ios/Packages/HapiKit/Sources/HapiProtocol/Window/WindowMessage.swift index f31f97f2..ebfd93b6 100644 --- a/ios/Packages/HapiKit/Sources/HapiProtocol/Window/WindowMessage.swift +++ b/ios/Packages/HapiKit/Sources/HapiProtocol/Window/WindowMessage.swift @@ -9,6 +9,7 @@ public enum MessageStatus: String, Codable, Sendable { case sending case sent case failed + case indeterminate } /// Wire tri-state of `invokedAt` (`pagination.md` "Queued semantics"): @@ -101,7 +102,7 @@ public final class WindowMessage: Sendable, Equatable { createdAt: wire.createdAt, invokedAt: wire.invokedAt.map(InvokedAtField.number) ?? .null, scheduledAt: wire.scheduledAt, - status: status + status: status ?? (wire.deliveryState == "indeterminate" ? .indeterminate : nil) ) } @@ -151,6 +152,10 @@ public final class WindowMessage: Sendable, Equatable { ) } + public func withDeliveryState(_ state: String?) -> WindowMessage { + withStatus(state == "indeterminate" ? .indeterminate : (status ?? .queued)) + } + /// Copy with the client-side status replaced. public func withStatus(_ status: MessageStatus) -> WindowMessage { WindowMessage( @@ -170,7 +175,8 @@ public final class WindowMessage: Sendable, Equatable { content: content, createdAt: createdAt, invokedAt: invokedAt.numberValue, - scheduledAt: scheduledAt + scheduledAt: scheduledAt, + deliveryState: status == .indeterminate ? "indeterminate" : nil ) } } diff --git a/shared/src/apiTypes.ts b/shared/src/apiTypes.ts index 00106769..735d8a5a 100644 --- a/shared/src/apiTypes.ts +++ b/shared/src/apiTypes.ts @@ -595,6 +595,7 @@ export type QueuedStateRequest = z.infer export type QueuedStateResponse = { queuedLocalIds: string[] + indeterminateLocalIds?: string[] invokedLocalMessages: Array<{ localId: string invokedAt: number diff --git a/shared/src/modes.test.ts b/shared/src/modes.test.ts index db4534ed..0e60af3b 100644 --- a/shared/src/modes.test.ts +++ b/shared/src/modes.test.ts @@ -8,6 +8,8 @@ import { getPermissionModeTone, getPermissionModesForFlavor, isPermissionModeAllowedForFlavor, + isSteeringSupportedForFlavor, + isSteeringSupportedForSession, } from './modes' describe('Gemini CLI sunset (read-only, not creatable)', () => { @@ -129,3 +131,36 @@ describe('claude auto permission mode', () => { expect(getPermissionModeTone('auto')).toBe('warning') }) }) + +describe('isSteeringSupportedForFlavor', () => { + it('supports codex and pi only', () => { + expect(isSteeringSupportedForFlavor('codex')).toBe(true) + expect(isSteeringSupportedForFlavor('pi')).toBe(true) + expect(isSteeringSupportedForFlavor('cursor')).toBe(false) + expect(isSteeringSupportedForFlavor('claude')).toBe(false) + expect(isSteeringSupportedForFlavor('opencode')).toBe(false) + expect(isSteeringSupportedForFlavor(undefined)).toBe(false) + expect(isSteeringSupportedForFlavor(null)).toBe(false) + }) +}) + +describe('isSteeringSupportedForSession', () => { + it('supports codex and pi sessions', () => { + expect(isSteeringSupportedForSession({ flavor: 'codex' })).toBe(true) + expect(isSteeringSupportedForSession({ flavor: 'pi' })).toBe(true) + }) + + it('rejects cursor until its steer handler lands', () => { + expect(isSteeringSupportedForSession({ + flavor: 'cursor', + cursorSessionProtocol: 'acp', + cursorSessionId: 'sess-1', + })).toBe(false) + expect(isSteeringSupportedForSession({ flavor: 'cursor' })).toBe(false) + }) + + it('rejects non-steerable flavors', () => { + expect(isSteeringSupportedForSession({ flavor: 'claude' })).toBe(false) + expect(isSteeringSupportedForSession(null)).toBe(false) + }) +}) diff --git a/shared/src/modes.ts b/shared/src/modes.ts index fc2ac24b..8f18e63f 100644 --- a/shared/src/modes.ts +++ b/shared/src/modes.ts @@ -180,3 +180,42 @@ export function getCodexCollaborationModeOptions(): CodexCollaborationModeOption label: getCodexCollaborationModeLabel(mode) })) } + +/** + * Flavors that can deliver a queued message into an active turn on demand + * (per-message "Steer" from the waiting queue), without waiting for full-turn end. + * + * Steer = soft mid-turn delivery (same idea as Cursor GUI default "Send"): + * - Pi: native steer over the Pi runtime (first-class since #1466) + * - Codex: app-server `turn/steer` (true mid-turn inject) + * - Cursor ACP: concurrent `session/prompt` soft-send (no cancel). Legacy + * stream-json Cursor sessions are NOT steerable — gate with + * {@link isSteeringSupportedForSession}. (Cursor joins this list once its + * soft-steer handler lands.) + * + * Claude / others: not supported (no reachable soft-steer path) — UI hides Steer. + */ +export const STEERING_SUPPORTED_FLAVORS = ['codex', 'pi'] as const + +export function isSteeringSupportedForFlavor(flavor?: string | null): boolean { + return (STEERING_SUPPORTED_FLAVORS as readonly string[]).includes(flavor ?? '') +} + +/** + * Session-aware steer gate. Prefer this over {@link isSteeringSupportedForFlavor} + * when metadata is available: legacy Cursor stream-json sessions advertise + * flavor `cursor` but cannot steer. + * + * Matches CLI legacy detection: explicit `stream-json`, or a pre-ACP session + * that has `cursorSessionId` without `cursorSessionProtocol: 'acp'`. + */ +export function isSteeringSupportedForSession(metadata?: { + flavor?: string | null + cursorSessionId?: string | null + cursorSessionProtocol?: 'acp' | 'stream-json' | null +} | null): boolean { + if (metadata?.flavor === 'codex' || metadata?.flavor === 'pi') { + return true + } + return false +} diff --git a/shared/src/schemas.ts b/shared/src/schemas.ts index ec496671..167f95d6 100644 --- a/shared/src/schemas.ts +++ b/shared/src/schemas.ts @@ -194,6 +194,9 @@ export type AgentStateCompletedRequest = z.infer @@ -575,7 +583,17 @@ export const SyncEventSchema = z.discriminatedUnion('type', [ SessionChangedSchema.extend({ type: z.literal('messages-consumed'), localIds: z.array(z.string()), - invokedAt: z.number() + invokedAt: z.number(), + // True when messages were steered into an active turn (not a normal queue drain). + steered: z.boolean().optional() + }), + SessionChangedSchema.extend({ + type: z.literal('messages-indeterminate'), + localIds: z.array(z.string()) + }), + SessionChangedSchema.extend({ + type: z.literal('messages-requeued'), + localIds: z.array(z.string()) }), SessionChangedSchema.extend({ type: z.literal('message-cancelled'), @@ -609,6 +627,8 @@ export type SyncEvent = z.infer export const CancelMessageResponseSchema = z.discriminatedUnion('status', [ z.object({ status: z.literal('cancelled'), localId: z.string().nullable() }), z.object({ status: z.literal('invoked'), message: DecryptedMessageSchema }), + // The row is inside an async steer: not removed, but not consumed either. + z.object({ status: z.literal('busy'), localId: z.string() }), ]) export type CancelMessageResponse = z.infer diff --git a/shared/src/socket.ts b/shared/src/socket.ts index eccb9e9f..aa3f3483 100644 --- a/shared/src/socket.ts +++ b/shared/src/socket.ts @@ -143,6 +143,16 @@ export const UpdateMachineBodySchema = z.object({ export type UpdateMachineBody = z.infer +export const UpdateRetryQueuedMessageBodySchema = z.object({ + t: z.literal('retry-queued-message'), + sid: z.string(), + messageId: z.string(), + localId: z.string().nullable(), + message: UpdateNewMessageBodySchema.shape.message +}) + +export type UpdateRetryQueuedMessageBody = z.infer + export const UpdateCancelQueuedMessageBodySchema = z.object({ t: z.literal('cancel-queued-message'), sid: z.string(), @@ -153,7 +163,11 @@ export const UpdateCancelQueuedMessageBodySchema = z.object({ export type UpdateCancelQueuedMessageBody = z.infer export const CancelQueuedMessageAckSchema = z.object({ - removed: z.boolean() + removed: z.boolean(), + inFlight: z.boolean().optional(), + indeterminate: z.boolean().optional(), + accepted: z.boolean().optional(), + consumed: z.boolean().optional() }) export type CancelQueuedMessageAck = z.infer @@ -161,7 +175,7 @@ export type CancelQueuedMessageAck = z.infer void) => void + update: (data: Update, ack?: (response: CancelQueuedMessageAck & { accepted?: boolean }) => void) => void 'rpc-request': (data: { method: string; params: string }, callback: (response: string) => void) => void 'terminal:open': (data: TerminalOpenPayload) => void 'terminal:write': (data: TerminalWritePayload) => void @@ -259,7 +273,9 @@ export interface ClientToServerEvents { /** CLI agent finished session/load (or equivalent) and can accept prompts. */ 'session-ready': (data: { sid: string; time: number }) => void 'session-end': (data: { sid: string; time: number; reason?: SessionEndReason }) => void - 'messages-consumed': (data: { sid: string; localIds: string[] }) => void + 'messages-consumed': (data: { sid: string; localIds: string[]; clearQueuedThinkingGrace?: boolean; steered?: boolean }) => void + 'messages-indeterminate': (data: { sid: string; localIds: string[] }) => void + 'messages-steer-state': (data: { sid: string; localIds: string[]; state: 'queued' | 'dispatching' }, cb: (response: { ok: boolean }) => void) => void 'update-metadata': (data: { sid: string; expectedVersion: number; metadata: unknown }, cb: (answer: UpdateMetadataAck) => void) => void 'update-state': (data: { sid: string; expectedVersion: number; agentState: unknown | null }, cb: (answer: UpdateStateAck) => void) => void 'machine-alive': (data: { machineId: string; time: number; health?: unknown }) => void diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 95751c34..6f9af31e 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -8,6 +8,7 @@ import type { CodexDesktopSyncRequest, CodexDesktopStatusResponse, CodexArchiveSessionResponse, + DecryptedMessage, CodexCollaborationMode, CopilotAgentMode, FileSearchResponse, @@ -60,6 +61,11 @@ import type { AgentFlavor, MessageDeliveryMode } from '@hapi/protocol' import type { CancelMessageResponse, SteerQueuedMessageResponse } from '@hapi/protocol/schemas' import type { TranscriptionMode, TranscriptionProvider, TranscriptionProviderInfo } from '@hapi/protocol/voice' +export type RetryIndeterminateMessageResponse = + | { status: 'retried' | 'already-queued' | 'retry-unavailable'; localId: string | null } + | { status: 'invoked'; message: DecryptedMessage } + | { status: 'not-found' } + export type ProviderCredentialSource = 'env' | 'settings' | 'none' export interface MaskedCredentialStatus { @@ -555,6 +561,13 @@ export class ApiClient { return response as SteerQueuedMessageResponse } + async retryIndeterminateMessage(sessionId: string, messageId: string): Promise { + return await this.request( + `/api/sessions/${encodeURIComponent(sessionId)}/messages/${encodeURIComponent(messageId)}/retry`, + { method: 'POST' } + ) + } + async abortSession(sessionId: string): Promise { await this.request(`/api/sessions/${encodeURIComponent(sessionId)}/abort`, { method: 'POST', diff --git a/web/src/chat/normalize.ts b/web/src/chat/normalize.ts index 7b6b73fb..8040a2dc 100644 --- a/web/src/chat/normalize.ts +++ b/web/src/chat/normalize.ts @@ -23,7 +23,7 @@ export function normalizeDecryptedMessage(message: DecryptedMessage): Normalized if (record.role === 'user') { const normalized = normalizeUserRecord(message.id, message.localId, message.createdAt, record.content, record.meta) return normalized - ? { ...normalized, status: message.status, originalText: message.originalText, invokedAt: message.invokedAt } + ? { ...normalized, status: message.status, originalText: message.originalText, invokedAt: message.invokedAt, steered: message.steered } : { id: message.id, localId: message.localId, @@ -34,7 +34,8 @@ export function normalizeDecryptedMessage(message: DecryptedMessage): Normalized meta: record.meta, status: message.status, originalText: message.originalText, - invokedAt: message.invokedAt + invokedAt: message.invokedAt, + steered: message.steered } } if (record.role === 'agent') { diff --git a/web/src/chat/reducerTimeline.ts b/web/src/chat/reducerTimeline.ts index 9cb12a9c..dc2181f1 100644 --- a/web/src/chat/reducerTimeline.ts +++ b/web/src/chat/reducerTimeline.ts @@ -706,7 +706,8 @@ export function reduceTimeline( attachments: msg.content.attachments, status: msg.status, originalText: msg.originalText, - meta: msg.meta + meta: msg.meta, + steered: msg.steered }) continue } diff --git a/web/src/chat/types.ts b/web/src/chat/types.ts index 5bd8c7de..4b7f75ea 100644 --- a/web/src/chat/types.ts +++ b/web/src/chat/types.ts @@ -154,6 +154,8 @@ export type NormalizedMessage = ({ * flavors) — consumers should fall back to `createdAt` in that case. */ agentTimestamp?: number | null + /** True when a user message was steered into an active turn (mid-turn). */ + steered?: boolean } export type ToolPermission = { @@ -207,6 +209,8 @@ export type UserTextBlock = { status?: MessageStatus originalText?: string meta?: unknown + /** True when this message was steered into an active turn (mid-turn). */ + steered?: boolean } export type AgentTextBlock = { diff --git a/web/src/components/AssistantChat/QueuedMessagesBar.tsx b/web/src/components/AssistantChat/QueuedMessagesBar.tsx index 5bcf148f..68887d7f 100644 --- a/web/src/components/AssistantChat/QueuedMessagesBar.tsx +++ b/web/src/components/AssistantChat/QueuedMessagesBar.tsx @@ -8,6 +8,7 @@ import { normalizeDecryptedMessage } from '@/chat/normalize' import type { DecryptedMessage } from '@/types/api' import { useCancelQueuedMessage } from '@/hooks/mutations/useCancelQueuedMessage' import { useSteerQueuedMessage } from '@/hooks/mutations/useSteerQueuedMessage' +import { useRetryIndeterminateMessage } from '@/hooks/mutations/useRetryIndeterminateMessage' import { useTranslation } from '@/lib/use-translation' import { useToast } from '@/lib/toast-context' import type { PendingSchedule } from '@/components/AssistantChat/ScheduleTimePicker' @@ -221,6 +222,7 @@ export function QueuedMessagesBar({ const composerText = useAuiState((state) => state.composer.text) const cancelMutation = useCancelQueuedMessage(api) const steerMutation = useSteerQueuedMessage(api) + const retryMutation = useRetryIndeterminateMessage(api) const { t } = useTranslation() const { addToast } = useToast() const pendingScheduleRef = useRef(pendingSchedule) @@ -372,6 +374,7 @@ export function QueuedMessagesBar({ // (the hub rejects those). const canSteerRow = Boolean( canSteer + && msg.deliveryState !== 'indeterminate' && msg.scheduledAt == null && canCancel ) @@ -391,6 +394,22 @@ export function QueuedMessagesBar({ }) } + const retryPending = retryMutation.isPending + && retryMutation.variables?.messageId === msg.id + const handleRetry = () => { + if (msg.deliveryState !== 'indeterminate' || !canCancel) return + const token = beginQueuedOperation(sessionId) + if (!token) return + void retryMutation.mutateAsync({ + sessionId, + messageId: msg.id, + }).catch(() => { + // The row remains held if the explicit retry fails. + }).finally(() => { + endQueuedOperation(sessionId, token) + }) + } + const handleEdit = async () => { if (!canCancel) return // Edit = cancel + restore composer (text + schedule). @@ -413,6 +432,12 @@ export function QueuedMessagesBar({ }) // Race guard: if the agent already consumed this message, skip prefill // and inform the user so they aren't confused by the row disappearing. + // A 'busy' cancel means the row is inside an async steer — it was + // NOT cancelled, so never prefill (the instruction may still be + // delivered; prefilling invites a duplicate send). + if (result.status === 'busy') { + return + } if (result.status === 'invoked') { if (mountedRef.current) { addToast({ @@ -480,6 +505,11 @@ export function QueuedMessagesBar({ {text} ) : null} + {msg.deliveryState === 'indeterminate' ? ( +
+ {t('queuedMessages.steerOutcomeUnknown')} +
+ ) : null} {hasAttachments ? (
{attachmentNames.map((name, index) => ( @@ -504,6 +534,19 @@ export function QueuedMessagesBar({ )}
+ {msg.deliveryState === 'indeterminate' ? ( + + ) : null} {canSteerRow ? (
)} + {steered ? ( + + {t('queuedMessages.steeredBadge')} + + ) : null} diff --git a/web/src/hooks/mutations/useCancelQueuedMessage.ts b/web/src/hooks/mutations/useCancelQueuedMessage.ts index 2fc172f5..e99fd53a 100644 --- a/web/src/hooks/mutations/useCancelQueuedMessage.ts +++ b/web/src/hooks/mutations/useCancelQueuedMessage.ts @@ -3,6 +3,7 @@ import type { ApiClient } from '@/api/client' import type { DecryptedMessage } from '@/types/api' import { appendOptimisticMessage, + markMessagesConsumed, removeOptimisticMessage, } from '@/lib/message-window-store' import { usePlatform } from '@/hooks/usePlatform' @@ -46,7 +47,28 @@ export function useCancelQueuedMessage(api: ApiClient | null) { // Optimistic: remove from the floating bar immediately. removeOptimisticMessage(input.sessionId, input.localId) }, - onSuccess: (result, input) => { + onSuccess: async (result, input) => { + if (result.status === 'busy') { + // The row is inside an async steer: restore a held copy, not a + // normal FIFO row. A concurrent consumed ACK may have arrived + // while the optimistic row was absent, so reconcile once. + appendOptimisticMessage(input.sessionId, { + ...input.snapshot, + deliveryState: 'indeterminate', + }) + if (api) { + try { + const state = await api.getQueuedState(input.sessionId, [input.localId]) + const invoked = state.invokedLocalMessages.find((item) => item.localId === input.localId) + if (invoked) { + markMessagesConsumed(input.sessionId, [input.localId], invoked.invokedAt) + } + } catch { + // SSE / the next reconnect will reconcile the held row. + } + } + return + } if (result.status === 'invoked') { // Race: CLI consumed this message before cancel arrived. // Restore using the server-validated invoked row so invokedAt is correct. diff --git a/web/src/hooks/mutations/useRetryIndeterminateMessage.ts b/web/src/hooks/mutations/useRetryIndeterminateMessage.ts new file mode 100644 index 00000000..e86bfa29 --- /dev/null +++ b/web/src/hooks/mutations/useRetryIndeterminateMessage.ts @@ -0,0 +1,50 @@ +import { useMutation } from '@tanstack/react-query' +import type { ApiClient } from '@/api/client' +import { markMessagesConsumed, markMessagesRequeued, removeOptimisticMessage } from '@/lib/message-window-store' +import { useTranslation } from '@/lib/use-translation' +import { useToast } from '@/lib/toast-context' + +type RetryIndeterminateMessageInput = { + sessionId: string + messageId: string +} + +export function useRetryIndeterminateMessage(api: ApiClient | null) { + const { t } = useTranslation() + const { addToast } = useToast() + return useMutation({ + mutationFn: async (input: RetryIndeterminateMessageInput) => { + if (!api) throw new Error('API unavailable') + return api.retryIndeterminateMessage(input.sessionId, input.messageId) + }, + onSuccess: (result, input) => { + if (result.status === 'retried' || result.status === 'already-queued') { + markMessagesRequeued(input.sessionId, result.localId ? [result.localId] : []) + } + if (result.status === 'invoked' && result.message.localId && typeof result.message.invokedAt === 'number') { + markMessagesConsumed(input.sessionId, [result.message.localId], result.message.invokedAt) + } + if (result.status === 'retry-unavailable' || result.status === 'not-found') { + if (result.status === 'not-found') { + removeOptimisticMessage(input.sessionId, input.messageId) + } + addToast({ + title: t('queuedMessages.retryFailed'), + body: result.status === 'not-found' + ? t('queuedMessages.retryMissing') + : t('queuedMessages.retryUnavailable'), + sessionId: input.sessionId, + url: window.location.href, + }) + } + }, + onError: (error, input) => { + addToast({ + title: t('queuedMessages.retryFailed'), + body: error instanceof Error ? error.message : '', + sessionId: input.sessionId, + url: window.location.href, + }) + }, + }) +} diff --git a/web/src/hooks/useSSE.ts b/web/src/hooks/useSSE.ts index 273dff56..d7494fc0 100644 --- a/web/src/hooks/useSSE.ts +++ b/web/src/hooks/useSSE.ts @@ -21,7 +21,7 @@ import type { SyncEvent } from '@/types/api' import { queryKeys } from '@/lib/query-keys' -import { clearMessageWindow, getMessageWindowState, ingestIncomingMessages, markMessagesConsumed, removeOptimisticMessage, updateMessageStatus } from '@/lib/message-window-store' +import { clearMessageWindow, getMessageWindowState, ingestIncomingMessages, markMessagesConsumed, markMessagesIndeterminate, markMessagesRequeued, removeOptimisticMessage, updateMessageStatus } from '@/lib/message-window-store' import { applySessionDetailPatch } from '@/lib/sessionPatch' // Pure patch-application rules live in @/lib/sessionPatch (React-free, shared @@ -40,6 +40,8 @@ export type SSEScope = 'global' | 'full' const MESSAGE_STREAM_EVENT_TYPES = new Set([ 'message-received', 'messages-consumed', + 'messages-indeterminate', + 'messages-requeued', 'message-cancelled', 'scheduled-matured' ]) @@ -680,6 +682,8 @@ export function useSSE(options: { if ( event.type === 'message-cancelled' || event.type === 'messages-consumed' + || event.type === 'messages-indeterminate' + || event.type === 'messages-requeued' || event.type === 'scheduled-matured' ) { queueSessionListInvalidation() @@ -689,7 +693,13 @@ export function useSSE(options: { // reconnect gaps or while another session is selected, only the global // connection may be alive — still clear the queued bar / optimistic rows. if (event.type === 'messages-consumed') { - markMessagesConsumed(event.sessionId, event.localIds, event.invokedAt) + markMessagesConsumed(event.sessionId, event.localIds, event.invokedAt, event.steered) + } + if (event.type === 'messages-indeterminate') { + markMessagesIndeterminate(event.sessionId, event.localIds) + } + if (event.type === 'messages-requeued') { + markMessagesRequeued(event.sessionId, event.localIds) } if (event.type === 'message-cancelled') { removeOptimisticMessage(event.sessionId, event.messageId) @@ -699,7 +709,15 @@ export function useSSE(options: { } if (event.type === 'messages-consumed') { - markMessagesConsumed(event.sessionId, event.localIds, event.invokedAt) + markMessagesConsumed(event.sessionId, event.localIds, event.invokedAt, event.steered) + } + + if (event.type === 'messages-indeterminate') { + markMessagesIndeterminate(event.sessionId, event.localIds) + } + + if (event.type === 'messages-requeued') { + markMessagesRequeued(event.sessionId, event.localIds) } if (event.type === 'message-cancelled') { diff --git a/web/src/lib/assistant-runtime.ts b/web/src/lib/assistant-runtime.ts index c98973bc..ca0d5fc9 100644 --- a/web/src/lib/assistant-runtime.ts +++ b/web/src/lib/assistant-runtime.ts @@ -39,6 +39,7 @@ export type HappyChatMessageMetadata = { source?: CliOutputBlock['source'] attachments?: AttachmentMetadata[] invokedAt?: number | null + steered?: boolean durationMs?: number usage?: UsageData model?: string | null @@ -474,7 +475,8 @@ function toThreadMessageLike( localId: block.localId, originalText: block.originalText, attachments: block.attachments, - invokedAt: block.invokedAt + invokedAt: block.invokedAt, + steered: block.steered } satisfies HappyChatMessageMetadata } } diff --git a/web/src/lib/locales/en.ts b/web/src/lib/locales/en.ts index 2865d06e..e2ed9476 100644 --- a/web/src/lib/locales/en.ts +++ b/web/src/lib/locales/en.ts @@ -637,6 +637,13 @@ export default { 'queuedMessages.editCurrentDraftKept': 'Queued message cancelled — current draft and schedule were kept.', 'queuedMessages.steer': 'Deliver into the running turn now', 'queuedMessages.steerFailed': 'Steer failed — message stays queued', + 'queuedMessages.steerOutcomeUnknown': 'Delivery outcome unknown — retry or cancel explicitly', + 'queuedMessages.retryOutcome': 'Retry uncertain delivery', + 'queuedMessages.retryFailed': 'Retry failed', + 'queuedMessages.retryMissing': 'The message is no longer available', + 'queuedMessages.retryUnavailable': 'The agent is still resolving this delivery', + 'queuedMessages.steeredBadge': '↳ Steered', + 'queuedMessages.steeredBadgeTitle': 'Steered into the active turn', // Scratchlist (per-session workbench, issue #11) 'scratchlist.title': 'Scratchlist', diff --git a/web/src/lib/locales/zh-CN.ts b/web/src/lib/locales/zh-CN.ts index 00a2e3f3..2efb95c2 100644 --- a/web/src/lib/locales/zh-CN.ts +++ b/web/src/lib/locales/zh-CN.ts @@ -636,6 +636,13 @@ export default { 'queuedMessages.editCurrentDraftKept': '队列消息已取消，已保留当前草稿和定时设置。', 'queuedMessages.steer': '立即插入当前回合', 'queuedMessages.steerFailed': '插入失败，消息仍在队列中', + 'queuedMessages.steerOutcomeUnknown': '发送结果未知，请明确重试或取消', + 'queuedMessages.retryOutcome': '重试不确定的消息发送', + 'queuedMessages.retryFailed': '重试失败', + 'queuedMessages.retryMissing': '消息已不存在', + 'queuedMessages.retryUnavailable': '代理仍在处理这条消息的发送结果', + 'queuedMessages.steeredBadge': '↳ 已介入', + 'queuedMessages.steeredBadgeTitle': '已介入当前进行中的回合', // Scratchlist (per-session workbench, issue #11) 'scratchlist.title': '草稿夹', diff --git a/web/src/lib/message-window-store.ts b/web/src/lib/message-window-store.ts index 41afa885..39092c61 100644 --- a/web/src/lib/message-window-store.ts +++ b/web/src/lib/message-window-store.ts @@ -1140,7 +1140,45 @@ export function removeOptimisticMessage(sessionId: string, localId: string): voi }, true) } -export function markMessagesConsumed(sessionId: string, localIds: string[], invokedAt: number): void { +export function markMessagesIndeterminate(sessionId: string, localIds: string[]): void { + if (localIds.length === 0) return + const idSet = new Set(localIds) + updateState(sessionId, (previous) => { + let changed = false + const messages = previous.messages.map((message) => { + if (!message.localId || !idSet.has(message.localId) || message.deliveryState === 'indeterminate') { + return message + } + changed = true + return { ...message, deliveryState: 'indeterminate' as const } + }) + return changed ? buildState(previous, { messages }) : previous + }, true) +} + +export function markMessagesRequeued(sessionId: string, localIds: string[]): void { + if (localIds.length === 0) return + const idSet = new Set(localIds) + updateState(sessionId, (previous) => { + let changed = false + const messages = previous.messages.map((message) => { + if (!message.localId || !idSet.has(message.localId) || message.deliveryState === undefined) { + return message + } + changed = true + const { deliveryState: _deliveryState, ...requeued } = message + return requeued + }) + return changed ? buildState(previous, { messages }) : previous + }, true) +} + +export function markMessagesConsumed( + sessionId: string, + localIds: string[], + invokedAt: number, + steered?: boolean +): void { if (localIds.length === 0) return const idSet = new Set(localIds) updateState(sessionId, (previous) => { @@ -1151,12 +1189,15 @@ export function markMessagesConsumed(sessionId: string, localIds: string[], invo } const needsStatus = message.status !== 'sent' const needsInvokedAt = message.invokedAt === null - if (!needsStatus && !needsInvokedAt) return message + const needsSteered = steered === true && message.steered !== true + if (!needsStatus && !needsInvokedAt && !needsSteered) return message changed = true + const { deliveryState: _deliveryState, ...withoutDeliveryState } = message return { - ...message, + ...withoutDeliveryState, ...(needsStatus ? { status: 'sent' as MessageStatus } : {}), - ...(needsInvokedAt ? { invokedAt } : {}) + ...(needsInvokedAt ? { invokedAt } : {}), + ...(needsSteered ? { steered: true } : {}) } }) if (!changed) return previous diff --git a/web/src/lib/messages.test.ts b/web/src/lib/messages.test.ts index d1b64963..b162f7d8 100644 --- a/web/src/lib/messages.test.ts +++ b/web/src/lib/messages.test.ts @@ -10,6 +10,7 @@ function userMessage(partial: Partial & { id: string }): Decry createdAt: partial.createdAt ?? 1_000, invokedAt: partial.invokedAt ?? null, status: partial.status, + steered: partial.steered, content: { role: 'user', content: [{ type: 'text', text: 'hi' }] }, } } @@ -24,4 +25,49 @@ describe('mergeMessages', () => { expect(merged).toHaveLength(1) expect(merged[0]?.invokedAt).toBe(invokedAt) }) + + it('does not inherit optimistic "queued" onto an already-invoked server echo (steered message)', () => { + const optimistic = userMessage({ id: 'local-1', localId: 'local-1', invokedAt: null, status: 'queued' }) + const serverInvoked = userMessage({ id: 'server-1', localId: 'local-1', invokedAt: 2_000 }) + + const merged = mergeMessages([optimistic], [serverInvoked]) + expect(merged).toHaveLength(1) + expect(merged[0]?.invokedAt).toBe(2_000) + expect(merged[0]?.status).not.toBe('queued') + }) + + it('keeps "queued" while the message is not yet invoked (normal queue mode)', () => { + const optimistic = userMessage({ id: 'local-2', localId: 'local-2', invokedAt: null, status: 'queued' }) + const serverPending = userMessage({ id: 'server-2', localId: 'local-2', invokedAt: null }) + + const merged = mergeMessages([optimistic], [serverPending]) + expect(merged).toHaveLength(1) + expect(merged[0]?.status).toBe('queued') + }) + + it('carries the live "steered" marker from the optimistic row onto the server echo', () => { + const optimisticSteered = userMessage({ id: 'local-s', localId: 'local-s', invokedAt: 4_000, status: 'sent', steered: true }) + const serverEcho = userMessage({ id: 'server-s', localId: 'local-s', invokedAt: 4_000 }) + + const merged = mergeMessages([optimisticSteered], [serverEcho]) + expect(merged).toHaveLength(1) + expect(merged[0]?.steered).toBe(true) + }) + + it('preserves "steered" across a refetch that omits it', () => { + const existing = [userMessage({ id: 'server-s2', localId: 'local-s2', invokedAt: 5_000, steered: true })] + const incoming = [userMessage({ id: 'server-s2', localId: 'local-s2', invokedAt: 5_000 })] + + const merged = mergeMessages(existing, incoming) + expect(merged).toHaveLength(1) + expect(merged[0]?.steered).toBe(true) + }) + + it('normalizes a stuck queued status on an invoked message', () => { + const inconsistent = userMessage({ id: 'server-3', localId: 'local-3', invokedAt: 3_000, status: 'queued' }) + + const merged = mergeMessages([inconsistent], []) + expect(merged).toHaveLength(1) + expect(merged[0]?.status).not.toBe('queued') + }) }) diff --git a/web/src/lib/messages.ts b/web/src/lib/messages.ts index b4304f46..2f21fc85 100644 --- a/web/src/lib/messages.ts +++ b/web/src/lib/messages.ts @@ -43,12 +43,24 @@ function compareMessages(a: DecryptedMessage, b: DecryptedMessage): number { return a.id.localeCompare(b.id) } +// Invariant: a message the CLI has consumed (invokedAt set) is delivered, not +// queued. A message delivered immediately — e.g. steered into the active turn — +// can carry a stale optimistic 'queued' status while its invokedAt is already +// set; normalize so the queued clock never lingers on a delivered message. +function clearStaleQueuedStatus(list: DecryptedMessage[]): DecryptedMessage[] { + return list.map((msg) => + msg.status === 'queued' && msg.invokedAt != null + ? { ...msg, status: 'sent' as DecryptedMessage['status'] } + : msg + ) +} + export function mergeMessages(existing: DecryptedMessage[], incoming: DecryptedMessage[]): DecryptedMessage[] { if (existing.length === 0) { - return [...incoming].sort(compareMessages) + return clearStaleQueuedStatus([...incoming]).sort(compareMessages) } if (incoming.length === 0) { - return [...existing].sort(compareMessages) + return clearStaleQueuedStatus([...existing]).sort(compareMessages) } const byId = new Map() @@ -57,8 +69,17 @@ export function mergeMessages(existing: DecryptedMessage[], incoming: DecryptedM } for (const msg of incoming) { const existing = byId.get(msg.id) - if (existing && existing.invokedAt != null && msg.invokedAt == null) { - byId.set(msg.id, { ...msg, invokedAt: existing.invokedAt }) + if (existing) { + // Preserve client-only signals the incoming (server) copy can't carry: + // a late ack timestamp and the live 'steered' marker (not persisted). + const preserved: Partial = {} + if (existing.invokedAt != null && msg.invokedAt == null) { + preserved.invokedAt = existing.invokedAt + } + if (existing.steered && !msg.steered) { + preserved.steered = true + } + byId.set(msg.id, Object.keys(preserved).length > 0 ? { ...msg, ...preserved } : msg) } else { byId.set(msg.id, msg) } @@ -78,6 +99,7 @@ export function mergeMessages(existing: DecryptedMessage[], incoming: DecryptedM if (incomingStoredLocalIds.size > 0) { const optimisticStatusByLocalId = new Map() const optimisticInvokedAtByLocalId = new Map() + const optimisticSteeredByLocalId = new Map() for (const msg of merged) { if (msg.localId && isOptimisticMessage(msg) && incomingStoredLocalIds.has(msg.localId)) { if (msg.status) { @@ -86,6 +108,9 @@ export function mergeMessages(existing: DecryptedMessage[], incoming: DecryptedM if (msg.invokedAt !== undefined) { optimisticInvokedAtByLocalId.set(msg.localId, msg.invokedAt) } + if (msg.steered) { + optimisticSteeredByLocalId.set(msg.localId, true) + } } } merged = merged.filter((msg) => { @@ -94,12 +119,20 @@ export function mergeMessages(existing: DecryptedMessage[], incoming: DecryptedM } return !isOptimisticMessage(msg) }) - if (optimisticStatusByLocalId.size > 0 || optimisticInvokedAtByLocalId.size > 0) { + if (optimisticStatusByLocalId.size > 0 || optimisticInvokedAtByLocalId.size > 0 || optimisticSteeredByLocalId.size > 0) { merged = merged.map((msg) => { if (!msg.localId) return msg const update: Partial = {} if (optimisticStatusByLocalId.has(msg.localId) && !msg.status) { - update.status = optimisticStatusByLocalId.get(msg.localId) + const optimisticStatus = optimisticStatusByLocalId.get(msg.localId) + // Don't carry an optimistic 'queued' status onto a server message + // the CLI has already consumed (invokedAt set). This happens when a + // message is delivered immediately — e.g. steered into the active + // turn — so its server echo arrives pre-invoked; inheriting 'queued' + // would pin the queued clock on an already-delivered message. + if (optimisticStatus !== 'queued' || msg.invokedAt == null) { + update.status = optimisticStatus + } } if (optimisticInvokedAtByLocalId.has(msg.localId) && msg.invokedAt == null) { const optimisticInvokedAt = optimisticInvokedAtByLocalId.get(msg.localId) @@ -107,6 +140,12 @@ export function mergeMessages(existing: DecryptedMessage[], incoming: DecryptedM update.invokedAt = optimisticInvokedAt } } + // The 'steered' marker is live-only (the hub never persists it), so + // carry it from the optimistic row onto the replacing server echo — + // otherwise the ↳ Steered badge vanishes the moment the echo lands. + if (optimisticSteeredByLocalId.has(msg.localId) && !msg.steered) { + update.steered = true + } if (Object.keys(update).length > 0) { return { ...msg, ...update } } @@ -139,6 +178,7 @@ export function mergeMessages(existing: DecryptedMessage[], incoming: DecryptedM result.push(optimistic) } - result.sort(compareMessages) - return result + const normalized = clearStaleQueuedStatus(result) + normalized.sort(compareMessages) + return normalized } diff --git a/web/src/lib/queued-state-reconciliation.test.ts b/web/src/lib/queued-state-reconciliation.test.ts index 52040496..493bbe86 100644 --- a/web/src/lib/queued-state-reconciliation.test.ts +++ b/web/src/lib/queued-state-reconciliation.test.ts @@ -4,6 +4,7 @@ import type { ApiClient } from '@/api/client' vi.mock('./message-window-store', () => ({ getQueuedReconcileCandidateLocalIds: vi.fn(), markMessagesConsumed: vi.fn(), + markMessagesIndeterminate: vi.fn(), reconcileQueuedLocalIds: vi.fn(), syncTailMessages: vi.fn(), })) @@ -11,6 +12,7 @@ vi.mock('./message-window-store', () => ({ import { getQueuedReconcileCandidateLocalIds, markMessagesConsumed, + markMessagesIndeterminate, reconcileQueuedLocalIds, syncTailMessages, } from './message-window-store' @@ -19,6 +21,7 @@ import { reconcileQueuedStateAfterConnect } from './queued-state-reconciliation' const mockSyncTailMessages = vi.mocked(syncTailMessages) const mockGetCandidates = vi.mocked(getQueuedReconcileCandidateLocalIds) const mockMarkMessagesConsumed = vi.mocked(markMessagesConsumed) +const mockMarkMessagesIndeterminate = vi.mocked(markMessagesIndeterminate) const mockReconcileQueuedLocalIds = vi.mocked(reconcileQueuedLocalIds) function createMockApi( @@ -35,6 +38,7 @@ describe('reconcileQueuedStateAfterConnect', () => { vi.clearAllMocks() mockSyncTailMessages.mockResolvedValue(undefined) mockGetCandidates.mockReturnValue([]) + mockMarkMessagesIndeterminate.mockClear() }) it('waits for the latest messages before snapshotting and querying queued state', async () => { diff --git a/web/src/lib/queued-state-reconciliation.ts b/web/src/lib/queued-state-reconciliation.ts index a66dc0de..e9246946 100644 --- a/web/src/lib/queued-state-reconciliation.ts +++ b/web/src/lib/queued-state-reconciliation.ts @@ -2,6 +2,7 @@ import type { ApiClient } from '@/api/client' import { getQueuedReconcileCandidateLocalIds, markMessagesConsumed, + markMessagesIndeterminate, reconcileQueuedLocalIds, syncTailMessages, } from './message-window-store' @@ -18,11 +19,13 @@ export async function reconcileQueuedStateAfterConnect( return } const queuedLocalIds: string[] = [] + const indeterminateLocalIds: string[] = [] const invokedLocalMessages: Array<{ localId: string; invokedAt: number }> = [] for (let index = 0; index < candidateLocalIds.length; index += QUEUED_STATE_BATCH_SIZE) { const batch = candidateLocalIds.slice(index, index + QUEUED_STATE_BATCH_SIZE) const state = await api.getQueuedState(sessionId, batch) queuedLocalIds.push(...state.queuedLocalIds) + indeterminateLocalIds.push(...(state.indeterminateLocalIds ?? [])) invokedLocalMessages.push(...state.invokedLocalMessages) } const invokedByTimestamp = new Map() @@ -34,5 +37,6 @@ export async function reconcileQueuedStateAfterConnect( for (const [invokedAt, localIds] of invokedByTimestamp) { markMessagesConsumed(sessionId, localIds, invokedAt) } - reconcileQueuedLocalIds(sessionId, candidateLocalIds, queuedLocalIds) + markMessagesIndeterminate(sessionId, indeterminateLocalIds) + reconcileQueuedLocalIds(sessionId, candidateLocalIds, [...queuedLocalIds, ...indeterminateLocalIds]) }