From 0840b565e33b8f20f96f6900ef59b4203b323401 Mon Sep 17 00:00:00 2001 From: anvyle Date: Tue, 14 Apr 2026 22:01:26 +0200 Subject: [PATCH] fix(copilot): clear stale Redis cancel flag when scheduling new auto-approve Sentry correctly identified that after a user clicks Modify (which sets a Redis cancel flag) and then the LLM calls decompose_goal again for a new plan, the stale cancel flag would suppress the new auto-approve. _schedule_auto_approve is now async and DELETEs the Redis cancel key before scheduling the new task. Also fixes the autouse test fixture to use an async no-op (matching the now-async function signature). Co-Authored-By: Claude Sonnet 4.6 --- .../backend/copilot/tools/decompose_goal.py | 11 +++++--- .../copilot/tools/decompose_goal_test.py | 28 ++++++++++++++----- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/autogpt_platform/backend/backend/copilot/tools/decompose_goal.py b/autogpt_platform/backend/backend/copilot/tools/decompose_goal.py index 185cedacee..cb2f33e855 100644 --- a/autogpt_platform/backend/backend/copilot/tools/decompose_goal.py +++ b/autogpt_platform/backend/backend/copilot/tools/decompose_goal.py @@ -163,7 +163,7 @@ async def cancel_auto_approve(session_id: str) -> bool: return True -def _schedule_auto_approve( +async def _schedule_auto_approve( session_id: str | None, user_id: str | None, session: ChatSession ) -> None: """Schedule the fire-and-forget auto-approve task for this session. @@ -176,11 +176,14 @@ def _schedule_auto_approve( if not session_id: return # Cancel any existing pending approval for this session (e.g. if the - # LLM called decompose_goal twice in one turn). Best-effort in-process - # cancel only — skip the async Redis call here to keep scheduling fast. + # LLM called decompose_goal twice in one turn). old_task = _pending_auto_approvals.pop(session_id, None) if old_task is not None and not old_task.done(): old_task.cancel() + # Clear any stale Redis cancel flag from a previous Modify click so + # the new auto-approve task isn't incorrectly suppressed. + redis = await get_redis_async() + await redis.delete(f"{_CANCEL_KEY_PREFIX}{session_id}") baseline_index = len(session.messages) task = asyncio.create_task(_run_auto_approve(session_id, user_id, baseline_index)) _pending_auto_approvals[session_id] = task @@ -301,7 +304,7 @@ class DecomposeGoalTool(BaseTool): ) ) - _schedule_auto_approve(session_id, user_id, session) + await _schedule_auto_approve(session_id, user_id, session) return TaskDecompositionResponse( message=f"Here's the plan to build your agent ({len(decomposition_steps)} steps):", diff --git a/autogpt_platform/backend/backend/copilot/tools/decompose_goal_test.py b/autogpt_platform/backend/backend/copilot/tools/decompose_goal_test.py index 514114b43c..e234aa3a5f 100644 --- a/autogpt_platform/backend/backend/copilot/tools/decompose_goal_test.py +++ b/autogpt_platform/backend/backend/copilot/tools/decompose_goal_test.py @@ -42,9 +42,11 @@ def _stub_auto_approve_scheduler(): fire-and-forget scheduler so they don't kick off real timers that try to hit Redis/Postgres. Tests that exercise auto-approve override this with their own patches inside the test body.""" - with patch.object( - decompose_goal_module, "_schedule_auto_approve", lambda *a, **kw: None - ): + + async def _noop(*a, **kw): + pass + + with patch.object(decompose_goal_module, "_schedule_auto_approve", _noop): yield @@ -390,6 +392,9 @@ class _FakeRedisNoCancelFlag: async def set(self, key, value, ex=None): pass + async def delete(self, key): + pass + def _stub_redis(): """Patch get_redis_async to return a fake Redis (no real connection).""" @@ -523,6 +528,11 @@ async def test_schedule_auto_approve_creates_task(monkeypatch): monkeypatch.setattr(decompose_goal_module, "AUTO_APPROVE_SERVER_SECONDS", 0) fake_run = AsyncMock() monkeypatch.setattr(decompose_goal_module, "_run_auto_approve", fake_run) + monkeypatch.setattr( + decompose_goal_module, + "get_redis_async", + AsyncMock(return_value=_FakeRedisNoCancelFlag()), + ) session = make_session(_USER_ID) # Two messages already in the session — the next one (the tool result) @@ -530,7 +540,7 @@ async def test_schedule_auto_approve_creates_task(monkeypatch): session.messages.append(ChatMessage(role="user", content="initial")) session.messages.append(ChatMessage(role="assistant", content="tool call")) - _REAL_SCHEDULE_AUTO_APPROVE( + await _REAL_SCHEDULE_AUTO_APPROVE( session_id="session-schedule", user_id=_USER_ID, session=session, @@ -544,10 +554,11 @@ async def test_schedule_auto_approve_creates_task(monkeypatch): fake_run.assert_awaited_once_with("session-schedule", _USER_ID, 2) -def test_schedule_auto_approve_no_op_without_session_id(): +@pytest.mark.asyncio +async def test_schedule_auto_approve_no_op_without_session_id(): """Empty session id should be a no-op (defensive).""" session = make_session(_USER_ID) - decompose_goal_module._schedule_auto_approve( + await decompose_goal_module._schedule_auto_approve( session_id=None, user_id=_USER_ID, session=session ) assert len(decompose_goal_module._pending_auto_approvals) == 0 @@ -575,12 +586,15 @@ async def test_cancel_auto_approve_sets_redis_flag_and_cancels_task(monkeypatch) async def get(self, key): return None + async def delete(self, key): + pass + monkeypatch.setattr( decompose_goal_module, "get_redis_async", AsyncMock(return_value=FakeRedis()) ) session = make_session(_USER_ID) - _REAL_SCHEDULE_AUTO_APPROVE( + await _REAL_SCHEDULE_AUTO_APPROVE( session_id="session-cancel-test", user_id=_USER_ID, session=session,