From 918adc45b2ddeb8a11e60758ca322c5801602f6a Mon Sep 17 00:00:00 2001 From: Yu-ga <74749461+yuga-hashimoto@users.noreply.github.com> Date: Sun, 9 Aug 2026 23:26:30 +0000 Subject: [PATCH] fix: show pending questions reliably and open the right chat from notifications Questions asked while the user was in another chat, or long before they opened the session, never rendered: - Recovery fetched `GET /question?directory=`, but the selected workspace is rarely the session's own directory, so the query found nothing. Resolve the directory from the session being opened and retry transient failures. - Claude Code had no recovery path at all: `pendingQuestions` returned an empty list and the one-shot bridge event was the only delivery. Read the pending files back (only for sessions whose process is alive) and answer them, and make the decline button work via the bridge's deny response. - The Claude hook expired questions after ~75 s: the loop counted iterations of a quarter-second sleep as seconds. Measure real time, keep permissions at five minutes, and let questions wait up to the hook timeout (raised to an hour). - Dismissed cards came back on every refetch; remember the dismissal for the current visit to the session. Notification taps opened the wrong chat or nothing: - The deep link was a bare session id that was never consumed, so a second tap for the same session changed nothing. Deliver a one-shot tokenised link and consume the intent extras. - Question/complete/error notifications carried no runtime id, so a chat on another agent opened against the wrong backend. Include the runtime id in every notification and switch to it before opening (falling back to the session catalog for notifications posted before this change). - On a cold start with no runtime selected yet, `openSession` no-opped and the request was dropped; keep it pending until a runtime exists. - Answered questions now cancel their notification, and the wider notification-id/requestCode space stops unrelated sessions colliding. --- .../and-code-claude-permission-hook.sh | 16 +- .../andcode/AndCodeApplication.kt | 12 +- .../com/yugahashimoto/andcode/MainActivity.kt | 25 ++- .../notification/RuntimeNotificationHelper.kt | 28 +++- .../repository/RuntimeActivityRepository.kt | 11 +- .../com/yugahashimoto/andcode/di/AppModule.kt | 12 +- .../andcode/feature/chat/ChatViewModel.kt | 67 ++++++-- .../schedule/ScheduleExecutionService.kt | 6 +- .../runtime/local/ClaudeCodeRuntime.kt | 17 ++ .../andcode/runtime/local/ClaudeCodeTarget.kt | 19 +++ .../runtime/local/ClaudePermissionBridge.kt | 52 +++--- .../runtime/local/ClaudePermissionHooks.kt | 12 +- .../yugahashimoto/andcode/ui/AndCodeApp.kt | 37 +++-- .../yugahashimoto/andcode/ui/ChatDeepLink.kt | 13 ++ .../RuntimeActivityRepositoryTest.kt | 12 +- .../feature/chat/ChatViewModelQuestionTest.kt | 150 +++++++++++++++++- .../local/ClaudePermissionBridgeTest.kt | 39 +++++ 17 files changed, 451 insertions(+), 77 deletions(-) create mode 100644 app/src/main/java/com/yugahashimoto/andcode/ui/ChatDeepLink.kt diff --git a/app/src/main/assets/scripts/and-code-claude-permission-hook.sh b/app/src/main/assets/scripts/and-code-claude-permission-hook.sh index f73f56cb..31e8cff8 100644 --- a/app/src/main/assets/scripts/and-code-claude-permission-hook.sh +++ b/app/src/main/assets/scripts/and-code-claude-permission-hook.sh @@ -9,6 +9,10 @@ PENDING="$BRIDGE/pending" RESPONSES="$BRIDGE/responses" ALWAYS="$BRIDGE/always-rules.json" TIMEOUT_SEC="${ANDCODE_PERMISSION_TIMEOUT_SEC:-300}" +# A question can legitimately wait much longer than a permission: the user may be away from the +# device and the turn is blocked until they answer. Keep it under the hook timeout Claude Code +# itself applies (see ClaudePermissionHooks), so this script still gets to deny gracefully. +QUESTION_TIMEOUT_SEC="${ANDCODE_QUESTION_TIMEOUT_SEC:-3540}" SLEEP_SEC=0.25 mkdir -p "$PENDING" "$RESPONSES" @@ -88,8 +92,15 @@ else printf '%s\n' "{\"v\":1,\"kind\":\"$KIND\",\"requestId\":\"$REQUEST_ID\",\"androidSessionId\":\"$ANDROID_SESSION\",\"claudeSessionId\":\"$SESSION_ID\",\"toolName\":\"$TOOL_NAME\",\"toolInput\":{},\"permissionLabel\":\"$LABEL\",\"createdAtMs\":0}" >"$REQUEST_FILE" fi -elapsed=0 -while [ "$elapsed" -lt "$TIMEOUT_SEC" ]; do +if [ "$KIND" = "question" ]; then + WAIT_SEC="$QUESTION_TIMEOUT_SEC" +else + WAIT_SEC="$TIMEOUT_SEC" +fi +# Measure real seconds: the loop used to count iterations of a quarter-second sleep, which made +# the effective timeout a quarter of the configured one and expired questions far too early. +deadline=$(( $(date +%s) + WAIT_SEC )) +while [ "$(date +%s)" -lt "$deadline" ]; do if [ -f "$RESPONSE_FILE" ]; then if command -v jq >/dev/null 2>&1; then DECISION=$(jq -r '.decision // "deny"' "$RESPONSE_FILE") @@ -125,7 +136,6 @@ while [ "$elapsed" -lt "$TIMEOUT_SEC" ]; do fi # shellcheck disable=SC2039 sleep "$SLEEP_SEC" 2>/dev/null || sleep 1 - elapsed=$((elapsed + 1)) done rm -f "$REQUEST_FILE" 2>/dev/null || true diff --git a/app/src/main/java/com/yugahashimoto/andcode/AndCodeApplication.kt b/app/src/main/java/com/yugahashimoto/andcode/AndCodeApplication.kt index 3f67cb75..60bab9d5 100644 --- a/app/src/main/java/com/yugahashimoto/andcode/AndCodeApplication.kt +++ b/app/src/main/java/com/yugahashimoto/andcode/AndCodeApplication.kt @@ -347,21 +347,23 @@ class AndCodeApplication : Application() { notifications.notifyPermission(request, title, runtimeId) }, onPermissionResolved = notifications::cancelPermission, - onSessionIdle = { sessionId, title -> + onSessionIdle = { sessionId, title, runtimeId -> AnalyticsReporter.recordRuntimeSessionCompleted() - notifications.notifySessionComplete(sessionId, title) + notifications.notifySessionComplete(sessionId, title, runtimeId) githubStarCoordinator.onSessionCompleted() }, - onSessionError = { sessionId, message -> + onSessionError = { sessionId, message, runtimeId -> AnalyticsReporter.recordRuntimeSessionError() CrashReporter.recordException( error = IllegalStateException(SecretRedaction.redact(message ?: "Runtime session failed")), message = "Runtime session error", customKeys = mapOf("session_id" to (sessionId ?: "unknown")), ) - notifications.notifySessionError(sessionId, message) + notifications.notifySessionError(sessionId, message, runtimeId) + }, + onQuestionAsked = { request, title, runtimeId -> + notifications.notifyQuestion(request, title, runtimeId) }, - onQuestionAsked = notifications::notifyQuestion, unreadStore = settings, messages = AndroidRuntimeActivityMessages(this), ) diff --git a/app/src/main/java/com/yugahashimoto/andcode/MainActivity.kt b/app/src/main/java/com/yugahashimoto/andcode/MainActivity.kt index fce77ebf..f647f50c 100644 --- a/app/src/main/java/com/yugahashimoto/andcode/MainActivity.kt +++ b/app/src/main/java/com/yugahashimoto/andcode/MainActivity.kt @@ -23,15 +23,18 @@ import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.dp import com.yugahashimoto.andcode.core.ProjectLinks import com.yugahashimoto.andcode.core.locale.AppLanguage +import com.yugahashimoto.andcode.core.notification.RuntimeNotificationHelper import com.yugahashimoto.andcode.feature.assistant.AndCodeVoiceInteractionService import com.yugahashimoto.andcode.feature.assistant.AssistantStatus import com.yugahashimoto.andcode.feature.support.GitHubStarPromptDialog import com.yugahashimoto.andcode.feature.support.openProjectLink import com.yugahashimoto.andcode.ui.AndCodeApp +import com.yugahashimoto.andcode.ui.ChatDeepLink import java.util.UUID class MainActivity : ComponentActivity() { - private var targetSessionId by mutableStateOf(null) + private var chatDeepLink by mutableStateOf(null) + private var deepLinkToken = 0L private var showInitialStarPrompt by mutableStateOf(false) private var assistantActive by mutableStateOf(false) @@ -73,7 +76,8 @@ class MainActivity : ComponentActivity() { AndCodeApp( onOpenAssistantSettings = ::openAssistantSettings, assistantActive = assistantActive, - targetSessionId = targetSessionId, + chatDeepLink = chatDeepLink, + onChatDeepLinkConsumed = { chatDeepLink = null }, ) SnackbarHost( hostState = snackbarHostState, @@ -127,9 +131,20 @@ class MainActivity : ComponentActivity() { private fun handleDeepLink(intent: Intent?) { intent ?: return - intent.getStringExtra("target_session_id")?.let { id -> - targetSessionId = id - } + val sessionId = + intent.getStringExtra(RuntimeNotificationHelper.EXTRA_TARGET_SESSION_ID) + ?.takeIf(String::isNotBlank) ?: return + deepLinkToken += 1 + chatDeepLink = + ChatDeepLink( + sessionId = sessionId, + runtimeId = intent.getStringExtra(RuntimeNotificationHelper.EXTRA_RUNTIME_ID), + token = deepLinkToken, + ) + // Consume the extras at once: the activity keeps this intent across configuration changes, + // and re-delivering it would yank the user back to a chat they have already left. + intent.removeExtra(RuntimeNotificationHelper.EXTRA_TARGET_SESSION_ID) + intent.removeExtra(RuntimeNotificationHelper.EXTRA_RUNTIME_ID) } private fun openAssistantSettings() { diff --git a/app/src/main/java/com/yugahashimoto/andcode/core/notification/RuntimeNotificationHelper.kt b/app/src/main/java/com/yugahashimoto/andcode/core/notification/RuntimeNotificationHelper.kt index 342a3c71..e55f52fd 100644 --- a/app/src/main/java/com/yugahashimoto/andcode/core/notification/RuntimeNotificationHelper.kt +++ b/app/src/main/java/com/yugahashimoto/andcode/core/notification/RuntimeNotificationHelper.kt @@ -81,15 +81,17 @@ class RuntimeNotificationHelper(private val context: Context) { fun notifyQuestion( request: QuestionRequest, chatTitle: String?, + runtimeId: String, ) { if (!canPostNotifications()) return val openIntent = pendingActivityIntent( - requestCode = request.id.hashCode(), + requestCode = ("question:" + request.id).hashCode(), extras = mapOf( EXTRA_OPEN_CHAT to true, EXTRA_TARGET_SESSION_ID to request.sessionId, + EXTRA_RUNTIME_ID to runtimeId, ), ) val prompt = request.questions.firstOrNull()?.question?.takeIf(String::isNotBlank) @@ -123,15 +125,17 @@ class RuntimeNotificationHelper(private val context: Context) { fun notifySessionComplete( sessionId: String, chatTitle: String?, + runtimeId: String, ) { if (!canPostNotifications()) return val intent = pendingActivityIntent( - requestCode = sessionId.hashCode(), + requestCode = ("complete:" + sessionId).hashCode(), extras = mapOf( EXTRA_OPEN_CHAT to true, EXTRA_TARGET_SESSION_ID to sessionId, + EXTRA_RUNTIME_ID to runtimeId, ), ) val notification = @@ -153,15 +157,17 @@ class RuntimeNotificationHelper(private val context: Context) { fun notifySessionError( sessionId: String?, message: String?, + runtimeId: String, ) { if (!canPostNotifications()) return val intent = pendingActivityIntent( - requestCode = (sessionId ?: "error").hashCode(), + requestCode = ("error:" + (sessionId ?: "error")).hashCode(), extras = mapOf( EXTRA_OPEN_ACTIVITY to true, EXTRA_TARGET_SESSION_ID to (sessionId.orEmpty()), + EXTRA_RUNTIME_ID to runtimeId, ), ) val notification = @@ -266,19 +272,29 @@ class RuntimeNotificationHelper(private val context: Context) { ) } - private fun permissionNotificationId(permissionId: String): Int = 20_000 + (permissionId.hashCode() and 0x0FFF) + // A narrow hash window made unrelated sessions collide onto one notification id, so one chat's + // notice visibly replaced another's and tapping it carried the wrong content intent. The wider + // mask makes that collision vanishingly unlikely. + private fun permissionNotificationId(permissionId: String): Int = + NOTIFICATION_ID_BASE_PERMISSION + (permissionId.hashCode() and NOTIFICATION_ID_MASK) - private fun questionNotificationId(questionId: String): Int = 25_000 + (questionId.hashCode() and 0x0FFF) + private fun questionNotificationId(questionId: String): Int = + NOTIFICATION_ID_BASE_QUESTION + (questionId.hashCode() and NOTIFICATION_ID_MASK) private fun statusNotificationId( sessionId: String, kind: String, - ): Int = 30_000 + ((sessionId + kind).hashCode() and 0x0FFF) + ): Int = NOTIFICATION_ID_BASE_STATUS + ((sessionId + kind).hashCode() and NOTIFICATION_ID_MASK) companion object { const val CHANNEL_APPROVALS = "opencode_approvals" const val CHANNEL_STATUS = "opencode_status" const val ACTION_PERMISSION_RESPONSE = "com.yugahashimoto.andcode.PERMISSION_RESPONSE" + + private const val NOTIFICATION_ID_MASK = 0x000FFFFF + private const val NOTIFICATION_ID_BASE_PERMISSION = 1_000_000 + private const val NOTIFICATION_ID_BASE_QUESTION = 3_000_000 + private const val NOTIFICATION_ID_BASE_STATUS = 5_000_000 const val EXTRA_OPEN_ACTIVITY = "open_activity" const val EXTRA_OPEN_CHAT = "open_chat" const val EXTRA_SESSION_ID = "session_id" diff --git a/app/src/main/java/com/yugahashimoto/andcode/data/repository/RuntimeActivityRepository.kt b/app/src/main/java/com/yugahashimoto/andcode/data/repository/RuntimeActivityRepository.kt index f0585cf2..a3991861 100644 --- a/app/src/main/java/com/yugahashimoto/andcode/data/repository/RuntimeActivityRepository.kt +++ b/app/src/main/java/com/yugahashimoto/andcode/data/repository/RuntimeActivityRepository.kt @@ -51,9 +51,9 @@ class RuntimeActivityRepository( private val maxRetryDelayMillis: Long = 30_000L, private val onPermissionAsked: ((PermissionRequest, String?, String) -> Unit)? = null, private val onPermissionResolved: ((String) -> Unit)? = null, - private val onSessionIdle: ((String, String?) -> Unit)? = null, - private val onSessionError: ((String?, String?) -> Unit)? = null, - private val onQuestionAsked: ((QuestionRequest, String?) -> Unit)? = null, + private val onSessionIdle: ((String, String?, String) -> Unit)? = null, + private val onSessionError: ((String?, String?, String) -> Unit)? = null, + private val onQuestionAsked: ((QuestionRequest, String?, String) -> Unit)? = null, private val unreadStore: UnreadSessionStore? = null, private val messages: RuntimeActivityMessages = RuntimeActivityMessages, ) { @@ -254,7 +254,7 @@ class RuntimeActivityRepository( runCatching { target.session(event.sessionId).parentId != null } .getOrDefault(false) if (!isSubagent) { - onSessionIdle?.invoke(event.sessionId, sessionTitle(target, event.sessionId)) + onSessionIdle?.invoke(event.sessionId, sessionTitle(target, event.sessionId), target.id) } } is OpenCodeEvent.MessageUpdated -> { @@ -309,7 +309,7 @@ class RuntimeActivityRepository( } } appendLog(messages.eventError, event.message, event.sessionId) - onSessionError?.invoke(event.sessionId, event.message) + onSessionError?.invoke(event.sessionId, event.message, target.id) } is OpenCodeEvent.QuestionAsked -> { mutableState.update { current -> @@ -319,6 +319,7 @@ class RuntimeActivityRepository( onQuestionAsked?.invoke( event.request, sessionTitle(target, event.request.sessionId), + target.id, ) } is OpenCodeEvent.Unknown -> appendLog(messages.eventUnknown, event.type) diff --git a/app/src/main/java/com/yugahashimoto/andcode/di/AppModule.kt b/app/src/main/java/com/yugahashimoto/andcode/di/AppModule.kt index cc0e6c8a..f16e9968 100644 --- a/app/src/main/java/com/yugahashimoto/andcode/di/AppModule.kt +++ b/app/src/main/java/com/yugahashimoto/andcode/di/AppModule.kt @@ -193,9 +193,15 @@ val appModule = onPermissionAsked = { request, title, runtimeId -> notifications.notifyPermission(request, title, runtimeId) }, - onSessionIdle = notifications::notifySessionComplete, - onSessionError = notifications::notifySessionError, - onQuestionAsked = notifications::notifyQuestion, + onSessionIdle = { sessionId, title, runtimeId -> + notifications.notifySessionComplete(sessionId, title, runtimeId) + }, + onSessionError = { sessionId, message, runtimeId -> + notifications.notifySessionError(sessionId, message, runtimeId) + }, + onQuestionAsked = { request, title, runtimeId -> + notifications.notifyQuestion(request, title, runtimeId) + }, messages = get(), ) } diff --git a/app/src/main/java/com/yugahashimoto/andcode/feature/chat/ChatViewModel.kt b/app/src/main/java/com/yugahashimoto/andcode/feature/chat/ChatViewModel.kt index 8ecaeeaf..5e52c317 100644 --- a/app/src/main/java/com/yugahashimoto/andcode/feature/chat/ChatViewModel.kt +++ b/app/src/main/java/com/yugahashimoto/andcode/feature/chat/ChatViewModel.kt @@ -163,6 +163,8 @@ private const val TRANSIENT_RECOVERY_MAX_BACKOFF_MS = 30_000L private const val TRANSIENT_RECOVERY_MAX_ATTEMPTS = 20 private const val HEALTH_CHECK_ATTEMPTS = 15 private const val HEALTH_CHECK_DELAY_MS = 2000L +private const val PENDING_QUESTION_REFRESH_ATTEMPTS = 3 +private const val PENDING_QUESTION_REFRESH_RETRY_DELAY_MS = 3000L /** Floor between transcript scans for pull request links, so streaming does not drive them. */ private const val PULL_REQUEST_SCAN_THROTTLE_MS = 300L @@ -302,6 +304,8 @@ data class ChatUiState( val selectedModelId: String? = null, val selectedAgentId: String? = null, val selectedWorkspacePath: String? = null, + /** Workspace directory of the open session, learned from the backend when it was opened. */ + val sessionDirectory: String? = null, val slashCommands: List = emptyList(), val slashSkills: List = emptyList(), val offlineQueue: List = emptyList(), @@ -316,6 +320,8 @@ class ChatViewModel( private val backend: OpenCodeBackend? = null, private val eventFlow: Flow? = null, private val onPermissionResolved: (String) -> Unit = {}, + /** Reports a question that was answered or declined, so its notification can be cancelled. */ + private val onQuestionResolved: (String) -> Unit = {}, private val onSessionCreated: () -> Unit = {}, /** * Reports whether this chat is working, so the drawer shows real state even when no stream @@ -360,6 +366,12 @@ class ChatViewModel( /** Message id to role, learned from `message.updated`, so user echoes can be skipped. */ private val messageRoles = mutableMapOf() + + /** + * Questions the user hid with [dismissQuestion]. A recovery fetch must not resurrect them: + * they are still open server-side, so without this every refetch would put the card back. + */ + private val dismissedQuestionIds = mutableSetOf() private val connectionMonitor = ConnectionQualityMonitor(viewModelScope) init { @@ -618,6 +630,9 @@ class ChatViewModel( val switchingSession = _uiState.value.sessionId != sessionId streamedParts.clear() messageRoles.clear() + // Opening the chat is an explicit act of attention, so questions the user hid earlier are + // offered again rather than staying suppressed by a stale dismissal. + dismissedQuestionIds.clear() _uiState.update { it.copy( sessionId = sessionId, @@ -627,6 +642,7 @@ class ChatViewModel( messages = emptyList(), permissions = emptyList(), pendingQuestions = emptyList(), + sessionDirectory = null, isRunning = if (switchingSession) false else it.isRunning, isThinking = if (switchingSession) false else it.isThinking, error = null, @@ -699,6 +715,7 @@ class ChatViewModel( fun newSession() { streamedParts.clear() messageRoles.clear() + dismissedQuestionIds.clear() _uiState.update { it.copy( sessionId = null, @@ -707,6 +724,7 @@ class ChatViewModel( messages = emptyList(), permissions = emptyList(), pendingQuestions = emptyList(), + sessionDirectory = null, isRunning = false, isThinking = false, isListening = false, @@ -1242,6 +1260,7 @@ class ChatViewModel( directory = pendingQuestion.workspaceDirectory(), ) }.onSuccess { accepted -> + if (accepted) onQuestionResolved(questionId) _uiState.update { state -> if (accepted) { state.copy( @@ -1279,31 +1298,53 @@ class ChatViewModel( /** * The directory the question routes have to be scoped to. What the event stream reported is - * authoritative; the selected workspace is the fallback for the older per-instance `/event` - * stream, whose frames carry no directory. + * authoritative; the open session's own directory is the next best source; the selected + * workspace is the last fallback for the older per-instance `/event` stream, whose frames + * carry no directory. */ - private fun PendingQuestionUi.workspaceDirectory(): String? = request.directory ?: _uiState.value.selectedWorkspacePath + private fun PendingQuestionUi.workspaceDirectory(): String? = + request.directory + ?: _uiState.value.sessionDirectory + ?: _uiState.value.selectedWorkspacePath /** * Recovers questions that are already waiting for an answer. A question reaches the chat as an - * event and nowhere else, so one asked while this client was not listening — before the app - * opened the session, or across a dropped event stream — would otherwise leave the turn - * blocked with nothing on screen to unblock it. + * event and nowhere else, so one asked while this client was not listening — in another chat, + * before the app opened the session, or across a dropped event stream — would otherwise leave + * the turn blocked with nothing on screen to unblock it. + * + * The question routes are scoped to the instance that owns the session, so the directory is + * resolved from the session itself: the workspace the composer happens to have selected often + * belongs to another project entirely, and querying with it finds nothing. */ private fun refreshPendingQuestions(sessionId: String) { val currentBackend = backend ?: return viewModelScope.launch { - val pending = - runCatching { currentBackend.pendingQuestions(_uiState.value.selectedWorkspacePath) } - .getOrElse { return@launch } - .filter { it.sessionId == sessionId } + val directory = + runCatching { currentBackend.session(sessionId).directory } + .getOrNull() + ?: _uiState.value.selectedWorkspacePath + if (directory != null && _uiState.value.sessionId == sessionId) { + _uiState.update { it.copy(sessionDirectory = directory) } + } + var fetched: List? = null + for (attempt in 0 until PENDING_QUESTION_REFRESH_ATTEMPTS) { + if (_uiState.value.sessionId != sessionId) return@launch + fetched = runCatching { currentBackend.pendingQuestions(directory) }.getOrNull() + if (fetched != null) break + if (attempt < PENDING_QUESTION_REFRESH_ATTEMPTS - 1) delay(PENDING_QUESTION_REFRESH_RETRY_DELAY_MS) + } + val pending = (fetched ?: return@launch).filter { it.sessionId == sessionId } if (pending.isEmpty()) return@launch _uiState.update { state -> if (state.sessionId != sessionId) return@update state val known = state.pendingQuestions.map { it.request.id }.toSet() state.copy( pendingQuestions = - state.pendingQuestions + pending.filterNot { it.id in known }.map(PendingQuestionUi::from), + state.pendingQuestions + + pending + .filterNot { it.id in known || it.id in dismissedQuestionIds } + .map(PendingQuestionUi::from), ) } } @@ -1315,6 +1356,7 @@ class ChatViewModel( * chat shows. */ fun dismissQuestion(questionId: String) { + dismissedQuestionIds += questionId _uiState.update { state -> state.copy( pendingQuestions = state.pendingQuestions.filterNot { it.request.id == questionId }, @@ -1341,6 +1383,8 @@ class ChatViewModel( requestId = questionId, directory = pendingQuestion.workspaceDirectory(), ) + }.onSuccess { rejected -> + if (rejected) onQuestionResolved(questionId) }.onFailure { error -> _uiState.update { it.copy(error = error.safeMessage()) } } @@ -1447,6 +1491,7 @@ class ChatViewModel( } is OpenCodeEvent.QuestionAsked -> { if (event.request.sessionId != activeSession) return + if (event.request.id in dismissedQuestionIds) return _uiState.update { state -> state.copy( pendingQuestions = diff --git a/app/src/main/java/com/yugahashimoto/andcode/feature/schedule/ScheduleExecutionService.kt b/app/src/main/java/com/yugahashimoto/andcode/feature/schedule/ScheduleExecutionService.kt index 726bface..599dc8eb 100644 --- a/app/src/main/java/com/yugahashimoto/andcode/feature/schedule/ScheduleExecutionService.kt +++ b/app/src/main/java/com/yugahashimoto/andcode/feature/schedule/ScheduleExecutionService.kt @@ -179,7 +179,7 @@ class ScheduleExecutionService : Service() { val title = runCatching { target.session(targetSessionId).title } .getOrDefault(schedule.displayName) - app.notifications.notifySessionComplete(targetSessionId, title) + app.notifications.notifySessionComplete(targetSessionId, title, target.id) } throw CompletionSignal() } @@ -187,7 +187,7 @@ class ScheduleExecutionService : Service() { is OpenCodeEvent.SessionError -> { if (event.sessionId == null || event.sessionId == targetSessionId) { recordFailed(run, event.message) - if (notifyUser) app.notifications.notifySessionError(targetSessionId, event.message) + if (notifyUser) app.notifications.notifySessionError(targetSessionId, event.message, target.id) throw CompletionSignal() } } @@ -208,7 +208,7 @@ class ScheduleExecutionService : Service() { } is OpenCodeEvent.QuestionAsked -> { if (event.request.sessionId == targetSessionId && notifyUser) { - app.notifications.notifyQuestion(event.request, schedule.displayName) + app.notifications.notifyQuestion(event.request, schedule.displayName, target.id) } } else -> Unit diff --git a/app/src/main/java/com/yugahashimoto/andcode/runtime/local/ClaudeCodeRuntime.kt b/app/src/main/java/com/yugahashimoto/andcode/runtime/local/ClaudeCodeRuntime.kt index f57915cf..e7ba0cdc 100644 --- a/app/src/main/java/com/yugahashimoto/andcode/runtime/local/ClaudeCodeRuntime.kt +++ b/app/src/main/java/com/yugahashimoto/andcode/runtime/local/ClaudeCodeRuntime.kt @@ -7,6 +7,7 @@ import com.yugahashimoto.andcode.core.api.OpenCodePart import com.yugahashimoto.andcode.core.api.OpenCodeTime import com.yugahashimoto.andcode.core.api.OpenCodeTodo import com.yugahashimoto.andcode.core.api.PromptAttachment +import com.yugahashimoto.andcode.core.api.QuestionRequest import com.yugahashimoto.andcode.runtime.PermissionResponse import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -215,6 +216,22 @@ class ClaudeCodeRuntime( return permissionBridge.answerQuestion(requestId, questionsJson, mapped) } + /** + * Questions still waiting for an answer, so a chat opened after the event was missed can + * recover them. + * + * Only requests whose session process is alive qualify: the hook that parks a request dies + * with the CLI process, so a file left behind by a dead session (an app restart, an aborted + * run) can never be answered and must not be shown. + */ + @Synchronized + fun pendingQuestions(): List = + permissionBridge + .pendingRequests() + .filter { it.kind == ClaudePermissionBridge.Kind.QUESTION } + .filter { sessions[it.androidSessionId]?.process?.isAlive == true } + .mapNotNull(permissionBridge::toQuestionRequest) + private fun ensureBridgeWatcher() { if (bridgeWatchJob?.isActive == true) return bridgeWatchJob = diff --git a/app/src/main/java/com/yugahashimoto/andcode/runtime/local/ClaudeCodeTarget.kt b/app/src/main/java/com/yugahashimoto/andcode/runtime/local/ClaudeCodeTarget.kt index 2e87efb7..1e8698a5 100644 --- a/app/src/main/java/com/yugahashimoto/andcode/runtime/local/ClaudeCodeTarget.kt +++ b/app/src/main/java/com/yugahashimoto/andcode/runtime/local/ClaudeCodeTarget.kt @@ -17,6 +17,7 @@ import com.yugahashimoto.andcode.core.api.OpenCodeTime import com.yugahashimoto.andcode.core.api.OpenCodeTodo import com.yugahashimoto.andcode.core.api.OpenCodeVcsInfo import com.yugahashimoto.andcode.core.api.PromptRequest +import com.yugahashimoto.andcode.core.api.QuestionRequest import com.yugahashimoto.andcode.runtime.BackendKind import com.yugahashimoto.andcode.runtime.LocalAgent import com.yugahashimoto.andcode.runtime.PermissionResponse @@ -332,6 +333,24 @@ class ClaudeCodeTarget( directory: String?, ): Boolean = withContext(Dispatchers.IO) { runtime.answerQuestion(requestId, answers) } + /** + * Declines a question parked on the file bridge. Without this the card's decline button threw + * "unsupported", leaving no way to unblock a turn whose question the user did not want. + */ + override suspend fun rejectQuestion( + requestId: String, + directory: String?, + ): Boolean = withContext(Dispatchers.IO) { runtime.respondToPermission(requestId, PermissionResponse.REJECT, remember = false) } + + /** + * Questions still waiting on the bridge. The event that announces a question is emitted once + * and can be missed — another chat on screen, an app restart — and the file bridge is the only + * place left to find it. [directory] is ignored: a Claude Code question belongs to a session, + * not a workspace. + */ + override suspend fun pendingQuestions(directory: String?): List = + withContext(Dispatchers.IO) { runtime.pendingQuestions() } + override fun events(): Flow = runtime.events() /** diff --git a/app/src/main/java/com/yugahashimoto/andcode/runtime/local/ClaudePermissionBridge.kt b/app/src/main/java/com/yugahashimoto/andcode/runtime/local/ClaudePermissionBridge.kt index aad99a32..28011f54 100644 --- a/app/src/main/java/com/yugahashimoto/andcode/runtime/local/ClaudePermissionBridge.kt +++ b/app/src/main/java/com/yugahashimoto/andcode/runtime/local/ClaudePermissionBridge.kt @@ -116,30 +116,46 @@ class ClaudePermissionBridge( } fun pollPending(): List { - val files = pendingDir.listFiles().orEmpty().filter { it.extension == "json" }.sortedBy { it.name } val out = mutableListOf() - for (file in files) { - val stored = runCatching { json.decodeFromString(file.readText()) }.getOrNull() ?: continue + for (stored in readPendingFiles()) { if (!emitted.add(stored.requestId)) continue - out += - Request( - requestId = stored.requestId, - androidSessionId = stored.androidSessionId, - kind = - when (stored.kind.lowercase()) { - "question" -> Kind.QUESTION - "elicitation" -> Kind.ELICITATION - else -> Kind.PERMISSION - }, - toolName = stored.toolName, - toolInputJson = stored.toolInput.toString(), - permissionLabel = stored.permissionLabel.ifBlank { stored.toolName }, - claudeSessionId = stored.claudeSessionId, - ) + out += toRequest(stored) } return out } + /** + * Everything still waiting on disk, regardless of whether it was already reported as an event. + * + * Recovery needs this: a request whose event was missed — the app showed another chat, or was + * restarted — is only findable through its pending file. Unlike [pollPending] it does not mark + * requests as emitted, so the watcher still emits them as events afterwards. + */ + fun pendingRequests(): List = readPendingFiles().map(::toRequest) + + private fun readPendingFiles(): List = + pendingDir + .listFiles().orEmpty() + .filter { it.extension == "json" } + .sortedBy { it.name } + .mapNotNull { file -> runCatching { json.decodeFromString(file.readText()) }.getOrNull() } + + private fun toRequest(stored: StoredRequest): Request = + Request( + requestId = stored.requestId, + androidSessionId = stored.androidSessionId, + kind = + when (stored.kind.lowercase()) { + "question" -> Kind.QUESTION + "elicitation" -> Kind.ELICITATION + else -> Kind.PERMISSION + }, + toolName = stored.toolName, + toolInputJson = stored.toolInput.toString(), + permissionLabel = stored.permissionLabel.ifBlank { stored.toolName }, + claudeSessionId = stored.claudeSessionId, + ) + fun respond( requestId: String, response: PermissionResponse, diff --git a/app/src/main/java/com/yugahashimoto/andcode/runtime/local/ClaudePermissionHooks.kt b/app/src/main/java/com/yugahashimoto/andcode/runtime/local/ClaudePermissionHooks.kt index 032f33b8..25dfa494 100644 --- a/app/src/main/java/com/yugahashimoto/andcode/runtime/local/ClaudePermissionHooks.kt +++ b/app/src/main/java/com/yugahashimoto/andcode/runtime/local/ClaudePermissionHooks.kt @@ -19,6 +19,14 @@ import java.io.File object ClaudePermissionHooks { const val HOOK_GUEST_PATH = "/usr/local/bin/and-code-claude-permission-hook.sh" const val HOOK_MARKER = "and-code-claude-permission" + + /** + * How long Claude Code lets the hook block a tool call, in seconds. A question may wait almost + * an hour for a user who is away from the device; the old six minutes expired long before + * that, denying the turn while the card was still on its way. The hook script's own timeout + * stays just under this so it gets to deny gracefully first. + */ + const val HOOK_TIMEOUT_SEC = 3600 private const val SETTINGS_RELATIVE = "root/.claude/settings.json" private const val HOOK_RELATIVE = "usr/local/bin/and-code-claude-permission-hook.sh" @@ -40,7 +48,7 @@ object ClaudePermissionHooks { { "type": "command", "command": "$HOOK_GUEST_PATH", - "timeout": 360, + "timeout": $HOOK_TIMEOUT_SEC, "statusMessage": "Waiting for AndCode approval" } ] @@ -75,7 +83,7 @@ object ClaudePermissionHooks { buildJsonObject { put("type", "command") put("command", HOOK_GUEST_PATH) - put("timeout", 360) + put("timeout", HOOK_TIMEOUT_SEC) put("statusMessage", "Waiting for AndCode approval") }, ) diff --git a/app/src/main/java/com/yugahashimoto/andcode/ui/AndCodeApp.kt b/app/src/main/java/com/yugahashimoto/andcode/ui/AndCodeApp.kt index dc3973e6..a9558bd2 100644 --- a/app/src/main/java/com/yugahashimoto/andcode/ui/AndCodeApp.kt +++ b/app/src/main/java/com/yugahashimoto/andcode/ui/AndCodeApp.kt @@ -160,7 +160,8 @@ fun AndCodeApp( assistantActive: Boolean = false, appTheme: AppTheme = AppTheme.DARK, uiFontSize: Int = 16, - targetSessionId: String? = null, + chatDeepLink: ChatDeepLink? = null, + onChatDeepLinkConsumed: () -> Unit = {}, ) { val context = LocalContext.current val keyboardController = LocalSoftwareKeyboardController.current @@ -273,6 +274,7 @@ fun AndCodeApp( backend = selectedRuntime, eventFlow = app.activityRepository.events, onPermissionResolved = app.activityRepository::resolvePermission, + onQuestionResolved = app.notifications::cancelQuestion, onSessionCreated = app.catalogRepository::refreshSessionsOnly, onRunStateChanged = { sessionId, running -> if (running) { @@ -558,11 +560,22 @@ fun AndCodeApp( } } - LaunchedEffect(targetSessionId) { - targetSessionId?.let { id -> - pendingSession = id to id - navController.navigate(ROUTE_CHAT) { launchSingleTop = true } + // A notification tap. The chat may belong to another agent than the selected one, so move to + // its runtime first — opening it against the wrong backend only loads an error. The effect is + // keyed on the whole link (token included), so tapping twice for the same session still works, + // and consuming resets the state so a stale link never navigates again. + LaunchedEffect(chatDeepLink) { + val link = chatDeepLink ?: return@LaunchedEffect + onChatDeepLinkConsumed() + val knownSession = app.catalogRepository.allSessions.value.firstOrNull { it.session.id == link.sessionId } + val runtimeId = link.runtimeId ?: knownSession?.runtimeId + if (runtimeId != null && runtimeId != selectedRuntime?.id) { + app.runtimeRegistry.select(runtimeId) } + app.activityRepository.markSessionRead(link.sessionId) + val title = knownSession?.session?.title?.takeIf(String::isNotBlank) ?: link.sessionId + pendingSession = link.sessionId to title + navController.navigate(ROUTE_CHAT) { launchSingleTop = true } } // Lets the in-guest agent pop the guest browser open for the user by dropping a command @@ -881,11 +894,15 @@ fun AndCodeApp( } composable(ROUTE_CHAT) { - LaunchedEffect(pendingSession) { - pendingSession?.let { (id, title) -> - chatViewModel.openSession(id, title) - pendingSession = null - } + // Keyed on the runtime too: a deep link that also switches the runtime must run + // against the view model of the new runtime, not the previous one's. While no + // runtime is selected yet (cold start) the chat backend does not exist and + // openSession would silently no-op — wait for one instead of dropping the request. + LaunchedEffect(pendingSession, selectedRuntime?.id) { + val pending = pendingSession ?: return@LaunchedEffect + if (selectedRuntime == null) return@LaunchedEffect + chatViewModel.openSession(pending.first, pending.second) + pendingSession = null } LaunchedEffect(pendingHandoffPrompt, selectedRuntime?.id, handoffReady) { val pending = pendingHandoffPrompt diff --git a/app/src/main/java/com/yugahashimoto/andcode/ui/ChatDeepLink.kt b/app/src/main/java/com/yugahashimoto/andcode/ui/ChatDeepLink.kt new file mode 100644 index 00000000..12eb5a8f --- /dev/null +++ b/app/src/main/java/com/yugahashimoto/andcode/ui/ChatDeepLink.kt @@ -0,0 +1,13 @@ +package com.yugahashimoto.andcode.ui + +/** + * A notification tap asking the app to open a chat. + * + * [token] makes every tap a distinct value even when it names the same session twice in a row, so + * the one-shot handler re-runs instead of seeing an unchanged state and doing nothing. + */ +data class ChatDeepLink( + val sessionId: String, + val runtimeId: String?, + val token: Long, +) diff --git a/app/src/test/java/com/yugahashimoto/andcode/data/repository/RuntimeActivityRepositoryTest.kt b/app/src/test/java/com/yugahashimoto/andcode/data/repository/RuntimeActivityRepositoryTest.kt index 0d92d801..1a79aa81 100644 --- a/app/src/test/java/com/yugahashimoto/andcode/data/repository/RuntimeActivityRepositoryTest.kt +++ b/app/src/test/java/com/yugahashimoto/andcode/data/repository/RuntimeActivityRepositoryTest.kt @@ -328,11 +328,11 @@ class RuntimeActivityRepositoryTest { localTarget = target, remoteFactory = { error("unused") }, ) - val asked = mutableListOf() + val asked = mutableListOf>() RuntimeActivityRepository( registry = registry, scope = TestScope(dispatcher), - onQuestionAsked = { request, _ -> asked += request }, + onQuestionAsked = { request, _, runtimeId -> asked += request to runtimeId }, ) advanceUntilIdle() @@ -347,7 +347,9 @@ class RuntimeActivityRepositoryTest { ) advanceUntilIdle() - assertEquals(listOf("q-1"), asked.map { it.id }) + assertEquals(listOf("q-1"), asked.map { it.first.id }) + // The notification needs the runtime to open the right chat, so it travels with the ask. + assertEquals(listOf(target.id), asked.map { it.second }) } @Test @@ -366,7 +368,7 @@ class RuntimeActivityRepositoryTest { RuntimeActivityRepository( registry = registry, scope = TestScope(dispatcher), - onSessionIdle = { sessionId, _ -> completed += sessionId }, + onSessionIdle = { sessionId, _, _ -> completed += sessionId }, ) advanceUntilIdle() @@ -393,7 +395,7 @@ class RuntimeActivityRepositoryTest { RuntimeActivityRepository( registry = registry, scope = TestScope(dispatcher), - onSessionIdle = { sessionId, _ -> completed += sessionId }, + onSessionIdle = { sessionId, _, _ -> completed += sessionId }, ) advanceUntilIdle() diff --git a/app/src/test/java/com/yugahashimoto/andcode/feature/chat/ChatViewModelQuestionTest.kt b/app/src/test/java/com/yugahashimoto/andcode/feature/chat/ChatViewModelQuestionTest.kt index e8a3b969..e888c6d0 100644 --- a/app/src/test/java/com/yugahashimoto/andcode/feature/chat/ChatViewModelQuestionTest.kt +++ b/app/src/test/java/com/yugahashimoto/andcode/feature/chat/ChatViewModelQuestionTest.kt @@ -322,6 +322,141 @@ class ChatViewModelQuestionTest { assertEquals(listOf("q-1"), viewModel.uiState.value.pendingQuestions.map { it.request.id }) } + @Test + fun `recovery asks for the directory of the session being opened`() = + runTest(dispatcher) { + val backend = + FakeBackend( + pending = listOf(request(id = "q-1", sessionId = "session-1")), + sessionDirectory = "/workspace/repo", + ) + val viewModel = ChatViewModel(backend) + + // The composer's workspace belongs to another project; it must not scope the query. + viewModel.selectWorkspace("/workspace/other") + viewModel.openSession("session-1") + advanceUntilIdle() + + assertEquals(listOf("q-1"), viewModel.uiState.value.pendingQuestions.map { it.request.id }) + assertEquals(listOf("/workspace/repo"), backend.pendingQuestionDirectories) + } + + @Test + fun `a recovered question is answered against the session's own workspace`() = + runTest(dispatcher) { + val backend = + FakeBackend( + pending = listOf(request(id = "q-1", sessionId = "session-1")), + sessionDirectory = "/workspace/repo", + ) + val viewModel = ChatViewModel(backend) + + viewModel.selectWorkspace("/workspace/other") + viewModel.openSession("session-1") + advanceUntilIdle() + + viewModel.selectQuestionAnswer("q-1", 0, "src") + viewModel.submitQuestion("q-1") + advanceUntilIdle() + + assertEquals("/workspace/repo", backend.answeredQuestions.single().directory) + } + + @Test + fun `a dismissed question stays hidden when the stream reconnects`() = + runTest(dispatcher) { + val backend = FakeBackend(pending = listOf(request(id = "q-1", sessionId = "session-1"))) + val viewModel = ChatViewModel(backend) + + viewModel.openSession("session-1") + advanceUntilIdle() + assertEquals(listOf("q-1"), viewModel.uiState.value.pendingQuestions.map { it.request.id }) + + viewModel.dismissQuestion("q-1") + backend.events.emit(OpenCodeEvent.ServerConnected) + advanceUntilIdle() + + assertTrue(viewModel.uiState.value.pendingQuestions.isEmpty()) + } + + @Test + fun `reopening the session offers a dismissed question again`() = + runTest(dispatcher) { + val backend = FakeBackend(pending = listOf(request(id = "q-1", sessionId = "session-1"))) + val viewModel = ChatViewModel(backend) + + viewModel.openSession("session-1") + advanceUntilIdle() + viewModel.dismissQuestion("q-1") + viewModel.openSession("session-2") + advanceUntilIdle() + + viewModel.openSession("session-1") + advanceUntilIdle() + + assertEquals(listOf("q-1"), viewModel.uiState.value.pendingQuestions.map { it.request.id }) + } + + @Test + fun `answering a question reports it as resolved`() = + runTest(dispatcher) { + val backend = FakeBackend() + val resolved = mutableListOf() + val viewModel = ChatViewModel(backend, onQuestionResolved = { resolved += it }) + + viewModel.openSession("session-1") + advanceUntilIdle() + backend.events.emit( + OpenCodeEvent.QuestionAsked(request(id = "q-1", sessionId = "session-1", options = listOf("src"))), + ) + advanceUntilIdle() + + viewModel.selectQuestionAnswer("q-1", 0, "src") + viewModel.submitQuestion("q-1") + advanceUntilIdle() + + assertEquals(listOf("q-1"), resolved) + } + + @Test + fun `cancelling a question reports it as resolved`() = + runTest(dispatcher) { + val backend = FakeBackend() + val resolved = mutableListOf() + val viewModel = ChatViewModel(backend, onQuestionResolved = { resolved += it }) + + viewModel.openSession("session-1") + advanceUntilIdle() + backend.events.emit(OpenCodeEvent.QuestionAsked(request(id = "q-1", sessionId = "session-1"))) + advanceUntilIdle() + + viewModel.cancelQuestion("q-1") + advanceUntilIdle() + + assertEquals(listOf("q-1"), resolved) + } + + @Test + fun `a failed answer does not report the question as resolved`() = + runTest(dispatcher) { + val backend = FakeBackend(answerResult = false) + val resolved = mutableListOf() + val viewModel = ChatViewModel(backend, onQuestionResolved = { resolved += it }) + + viewModel.openSession("session-1") + advanceUntilIdle() + backend.events.emit( + OpenCodeEvent.QuestionAsked(request(id = "q-1", sessionId = "session-1", options = listOf("src"))), + ) + advanceUntilIdle() + + viewModel.selectQuestionAnswer("q-1", 0, "src") + viewModel.submitQuestion("q-1") + advanceUntilIdle() + + assertTrue(resolved.isEmpty()) + } + private fun request( id: String, sessionId: String, @@ -351,6 +486,7 @@ class ChatViewModelQuestionTest { private class FakeBackend( private val answerResult: Boolean = true, private val pending: List = emptyList(), + private val sessionDirectory: String? = null, ) : OpenCodeBackend { override val id: String = "fake" override val displayName: String = "Fake" @@ -359,11 +495,20 @@ class ChatViewModelQuestionTest { val answeredQuestions = mutableListOf() val rejectedQuestions = mutableListOf>() val abortedSessions = mutableListOf() + val pendingQuestionDirectories = mutableListOf() override suspend fun health(): OpenCodeHealth = OpenCodeHealth(true, "test") override suspend fun listSessions(directory: String?): List = emptyList() + override suspend fun session(sessionId: String): OpenCodeSession = + OpenCodeSession( + id = sessionId, + directory = sessionDirectory, + title = "", + time = OpenCodeTime(created = 1), + ) + override suspend fun createSession( title: String?, directory: String?, @@ -415,7 +560,10 @@ class ChatViewModelQuestionTest { return true } - override suspend fun pendingQuestions(directory: String?): List = pending + override suspend fun pendingQuestions(directory: String?): List { + pendingQuestionDirectories += directory + return pending + } override fun events(): Flow = events } diff --git a/app/src/test/java/com/yugahashimoto/andcode/runtime/local/ClaudePermissionBridgeTest.kt b/app/src/test/java/com/yugahashimoto/andcode/runtime/local/ClaudePermissionBridgeTest.kt index fe66f645..419e1d69 100644 --- a/app/src/test/java/com/yugahashimoto/andcode/runtime/local/ClaudePermissionBridgeTest.kt +++ b/app/src/test/java/com/yugahashimoto/andcode/runtime/local/ClaudePermissionBridgeTest.kt @@ -39,6 +39,45 @@ class ClaudePermissionBridgeTest { assertTrue(bridge.pollPending().isEmpty()) } + @Test + fun `pendingRequests recovers a request the event stream already carried`() { + val bridge = ClaudePermissionBridge(folder.root) + val requestId = UUID.randomUUID().toString() + bridge.writeGuestRequest( + ClaudePermissionBridge.Request( + requestId = requestId, + androidSessionId = "session-1", + kind = ClaudePermissionBridge.Kind.QUESTION, + toolName = "AskUserQuestion", + toolInputJson = """{"questions":[{"question":"Pick?","options":[{"label":"A"}]}]}""", + permissionLabel = "AskUserQuestion", + ), + ) + + // The watcher already emitted it; a chat opened later must still find it on disk. + bridge.pollPending() + assertEquals(listOf(requestId), bridge.pendingRequests().map { it.requestId }) + } + + @Test + fun `pendingRequests does not consume the watcher's one-shot emission`() { + val bridge = ClaudePermissionBridge(folder.root) + val requestId = UUID.randomUUID().toString() + bridge.writeGuestRequest( + ClaudePermissionBridge.Request( + requestId = requestId, + androidSessionId = "session-1", + kind = ClaudePermissionBridge.Kind.PERMISSION, + toolName = "Bash", + toolInputJson = "{}", + permissionLabel = "Bash", + ), + ) + + assertEquals(listOf(requestId), bridge.pendingRequests().map { it.requestId }) + assertEquals(listOf(requestId), bridge.pollPending().map { it.requestId }) + } + @Test fun `respond writes a response the hook can read`() { val bridge = ClaudePermissionBridge(folder.root)