From 0c1ad3a20e900ed50841bb6c8c6d2f0b5ba1ea92 Mon Sep 17 00:00:00 2001 From: JMAN730 Date: Tue, 28 Jul 2026 12:09:37 -0400 Subject: [PATCH] fix: address auto approval review findings --- .../com/opendroid/ai/core/agent/AutoApprovalPolicy.kt | 2 +- .../com/opendroid/ai/core/service/OpenDroidService.kt | 6 +++--- .../main/java/com/opendroid/ai/ui/screens/ChatScreen.kt | 4 +++- .../java/com/opendroid/ai/ui/screens/SettingsScreen.kt | 5 +++-- .../opendroid/ai/core/agent/AutoApprovalPolicyTest.kt | 9 +++++++++ 5 files changed, 19 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/com/opendroid/ai/core/agent/AutoApprovalPolicy.kt b/app/src/main/java/com/opendroid/ai/core/agent/AutoApprovalPolicy.kt index b8b3c96..d8bd233 100644 --- a/app/src/main/java/com/opendroid/ai/core/agent/AutoApprovalPolicy.kt +++ b/app/src/main/java/com/opendroid/ai/core/agent/AutoApprovalPolicy.kt @@ -25,7 +25,7 @@ object AutoApprovalPolicy { */ fun blockedActions(granted: Set, steps: List): List = steps.flatMap { step -> - listOfNotNull(step.action, step.fallback.takeIf { it.isNotBlank() }) + listOfNotNull(step.action.takeIf { it.isNotBlank() }, step.fallback.takeIf { it.isNotBlank() }) } .distinct() .filter { it !in granted || ActionSchema.isNeverAutoApprove(it) } diff --git a/app/src/main/java/com/opendroid/ai/core/service/OpenDroidService.kt b/app/src/main/java/com/opendroid/ai/core/service/OpenDroidService.kt index f4f7b1b..98cfcf8 100644 --- a/app/src/main/java/com/opendroid/ai/core/service/OpenDroidService.kt +++ b/app/src/main/java/com/opendroid/ai/core/service/OpenDroidService.kt @@ -108,13 +108,13 @@ class OpenDroidService : Service() { agentLoop.agentState.collectLatest { state -> if (state is AgentState.PlanProposed && state.plan.planId != promptedPlanId) { promptedPlanId = state.plan.planId + if (!showFloatingButton) { + pendingApprovalListen = true + } textToSpeechEngine.speak( "I've planned: ${state.plan.goal}, ${state.plan.estimatedSteps} steps. " + "Say approve to run, or cancel." ) - if (!showFloatingButton) { - pendingApprovalListen = true - } } } } diff --git a/app/src/main/java/com/opendroid/ai/ui/screens/ChatScreen.kt b/app/src/main/java/com/opendroid/ai/ui/screens/ChatScreen.kt index dd398ec..99f3128 100644 --- a/app/src/main/java/com/opendroid/ai/ui/screens/ChatScreen.kt +++ b/app/src/main/java/com/opendroid/ai/ui/screens/ChatScreen.kt @@ -389,6 +389,7 @@ fun ChatScreen( ) } else emptyList() ProposedPlanPrompt( + planId = proposedPlan.planId, goal = proposedPlan.goal, stepsCount = proposedPlan.estimatedSteps, blockedActions = blocked, @@ -838,6 +839,7 @@ fun ThinkingBubble() { @Composable fun ProposedPlanPrompt( + planId: String, goal: String, stepsCount: Int, blockedActions: List = emptyList(), @@ -845,7 +847,7 @@ fun ProposedPlanPrompt( onApprove: (Set) -> Unit, onReject: () -> Unit ) { - var checkedGrants by remember { mutableStateOf(setOf()) } + var checkedGrants by remember(planId) { mutableStateOf(setOf()) } Card( modifier = Modifier .fillMaxWidth() diff --git a/app/src/main/java/com/opendroid/ai/ui/screens/SettingsScreen.kt b/app/src/main/java/com/opendroid/ai/ui/screens/SettingsScreen.kt index 3a4f228..a928a87 100644 --- a/app/src/main/java/com/opendroid/ai/ui/screens/SettingsScreen.kt +++ b/app/src/main/java/com/opendroid/ai/ui/screens/SettingsScreen.kt @@ -1459,15 +1459,16 @@ fun SettingsScreen( } Spacer(modifier = Modifier.height(16.dp)) + val grantedActions = config.effectiveGrantedActions() Text( - text = "ALLOWED ACTIONS (${config.effectiveGrantedActions().size})", + text = "ALLOWED ACTIONS (${grantedActions.size})", fontSize = 11.sp, fontFamily = FontFamily.Monospace, color = AccentCyan ) Spacer(modifier = Modifier.height(4.dp)) val dateFormat = remember { java.text.SimpleDateFormat("d MMM yyyy", java.util.Locale.getDefault()) } - config.effectiveGrantedActions().entries + grantedActions.entries .sortedBy { it.key } .forEach { (action, grantedAt) -> Row( diff --git a/app/src/test/java/com/opendroid/ai/core/agent/AutoApprovalPolicyTest.kt b/app/src/test/java/com/opendroid/ai/core/agent/AutoApprovalPolicyTest.kt index 2368318..421bbd0 100644 --- a/app/src/test/java/com/opendroid/ai/core/agent/AutoApprovalPolicyTest.kt +++ b/app/src/test/java/com/opendroid/ai/core/agent/AutoApprovalPolicyTest.kt @@ -98,4 +98,13 @@ class AutoApprovalPolicyTest { AutoApprovalPolicy.shouldAutoApprove(AutoMode.AUTO, granted, Plan("p1", "test", "1m", 1, steps)) ) } + + @Test + fun `blank primary action is ignored`() { + val steps = listOf(step("s0", " ")) + assertTrue( + AutoApprovalPolicy.shouldAutoApprove(AutoMode.AUTO, emptySet(), Plan("p1", "test", "1m", 1, steps)) + ) + assertEquals(emptyList(), AutoApprovalPolicy.blockedActions(emptySet(), steps)) + } }