From 11fb3d6c3f2d67b6553d48e2e02ff6e0415afef9 Mon Sep 17 00:00:00 2001 From: 0neStep <146049978+AperturePlus@users.noreply.github.com> Date: Mon, 25 May 2026 20:53:12 +0800 Subject: [PATCH 01/10] fix(agent): prevent duplicate execute for same plan via idempotency key check Add idempotency_key-based guard in ExecuteService.enqueue_execute to reject 409 when a execute BackgroundJob already exists for the same planJobId. Update test to mock the second scalar return and add test_execute_rejects_repeat_when_existing_execute_job_exists. --- .../services/agent/execute_service.py | 21 +++++++ app/tests/test_agent_plan_execute_runtime.py | 58 ++++++++++++++++++- 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/app/src/fileflash/services/agent/execute_service.py b/app/src/fileflash/services/agent/execute_service.py index b06ace5..e3f7c41 100644 --- a/app/src/fileflash/services/agent/execute_service.py +++ b/app/src/fileflash/services/agent/execute_service.py @@ -69,10 +69,31 @@ async def enqueue_execute( data={"highRiskActions": high_risk_actions}, ) + idempotency_key = f"agent.execute:{plan_job_id}" + existing_execute = await self.db.scalar( + select(BackgroundJob).where( + and_( + BackgroundJob.task_type == "agent.execute", + BackgroundJob.idempotency_key == idempotency_key, + ) + ) + ) + if existing_execute is not None: + raise ApiError( + status_code=409, + code=409, + message="Plan has already been executed", + data={ + "jobId": str(existing_execute.job_id), + "status": str(existing_execute.status), + }, + ) + job = await self.jobs.enqueue( self.db, task_type="agent.execute", payload=payload.model_dump(by_alias=True, mode="json"), + idempotency_key=idempotency_key, requested_by=user_id, max_attempts=1, priority=100, diff --git a/app/tests/test_agent_plan_execute_runtime.py b/app/tests/test_agent_plan_execute_runtime.py index a07d5f7..62ba503 100644 --- a/app/tests/test_agent_plan_execute_runtime.py +++ b/app/tests/test_agent_plan_execute_runtime.py @@ -272,7 +272,7 @@ async def test_execute_rejects_high_risk_plan_without_confirmation(): @pytest.mark.asyncio async def test_execute_enqueue_serializes_approval_datetime_as_json_string(): db = DummyDb() - db.scalar.return_value = BackgroundJob( + plan_job = BackgroundJob( job_id=99, task_type="agent.plan", status="succeeded", @@ -283,6 +283,7 @@ async def test_execute_enqueue_serializes_approval_datetime_as_json_string(): created_at=datetime.now(UTC), updated_at=datetime.now(UTC), ) + db.scalar = AsyncMock(side_effect=[plan_job, None]) plans = AgentPlanRepository(db) # type: ignore[arg-type] plans.get_for_execute_binding = AsyncMock(return_value=SimpleNamespace(proposed_actions_json=[])) jobs = FakeJobs() @@ -306,6 +307,61 @@ async def test_execute_enqueue_serializes_approval_datetime_as_json_string(): approval_payload = jobs.kwargs["payload"]["approval"] assert isinstance(approval_payload["confirmedAt"], str) assert approval_payload["confirmedAt"] == "2026-05-25T10:00:00Z" + assert jobs.kwargs["idempotency_key"] == "agent.execute:99" + + +@pytest.mark.asyncio +async def test_execute_rejects_repeat_when_existing_execute_job_exists(): + db = DummyDb() + plan_job = BackgroundJob( + job_id=99, + task_type="agent.plan", + status="succeeded", + payload={}, + result={}, + requested_by=7, + scheduled_at=datetime.now(UTC), + created_at=datetime.now(UTC), + updated_at=datetime.now(UTC), + ) + existing_execute = BackgroundJob( + job_id=200, + task_type="agent.execute", + status="running", + payload={}, + result={}, + requested_by=7, + idempotency_key="agent.execute:99", + scheduled_at=datetime.now(UTC), + created_at=datetime.now(UTC), + updated_at=datetime.now(UTC), + ) + db.scalar = AsyncMock(side_effect=[plan_job, existing_execute]) + plans = AgentPlanRepository(db) # type: ignore[arg-type] + plans.get_for_execute_binding = AsyncMock(return_value=SimpleNamespace(proposed_actions_json=[])) + jobs = FakeJobs() + service = ExecuteService( + db=db, + settings=settings(), + jobs=jobs, # type: ignore[arg-type] + plans=plans, + work_sessions=AgentWorkSessionRepository(db), # type: ignore[arg-type] + ) + payload = ExecuteAgentRequest.model_validate( + { + "planJobId": "99", + "planHash": "sha256:test", + "approval": {"confirmedBy": "7", "confirmedAt": "2026-05-25T10:00:00Z"}, + } + ) + + with pytest.raises(ApiError) as exc: + await service.enqueue_execute(user_id=7, payload=payload) + + assert exc.value.status_code == 409 + assert "already been executed" in exc.value.message.lower() or "already" in exc.value.message.lower() + assert exc.value.data["jobId"] == "200" + assert jobs.kwargs == {} @pytest.mark.asyncio From b8b827d98c329009d64707974df64745a570d35a Mon Sep 17 00:00:00 2001 From: 0neStep <146049978+AperturePlus@users.noreply.github.com> Date: Mon, 25 May 2026 20:53:16 +0800 Subject: [PATCH 02/10] feat(web): add viewport-aware smart placement to Select dropdown compute whether to render menu above or below trigger based on available space. Recalculate on resize/scroll. Add specs for both up and down placements. --- web/src/components/molecules/Select.spec.ts | 46 +++++++++++++++- web/src/components/molecules/Select.vue | 61 ++++++++++++++++++--- 2 files changed, 99 insertions(+), 8 deletions(-) diff --git a/web/src/components/molecules/Select.spec.ts b/web/src/components/molecules/Select.spec.ts index 04af91b..48aa7c0 100644 --- a/web/src/components/molecules/Select.spec.ts +++ b/web/src/components/molecules/Select.spec.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, afterEach } from 'vitest'; +import { describe, it, expect, afterEach, vi } from 'vitest'; import { mount } from '../../test/mount'; import Select from './Select.vue'; @@ -50,4 +50,48 @@ describe('molecules/Select', () => { await w.find('.ff-select').trigger('keydown', { key: 'Escape' }); expect(w.find('.ff-select__menu').exists()).toBe(false); }); + + it('opens upward when there is not enough room below the trigger', async () => { + const originalInner = window.innerHeight; + Object.defineProperty(window, 'innerHeight', { value: 600, configurable: true }); + const w = mount(Select, { + props: { modelValue: 'a', options: OPTS }, + attachTo: document.body, + }); + const root = w.find('.ff-select').element as HTMLElement; + vi.spyOn(root, 'getBoundingClientRect').mockReturnValue({ + x: 0, y: 580, top: 580, bottom: 596, + left: 0, right: 120, width: 120, height: 16, + toJSON: () => undefined, + } as DOMRect); + + await w.find('.ff-select__trigger').trigger('click'); + const menu = w.find('.ff-select__menu'); + expect(menu.exists()).toBe(true); + expect(menu.classes()).toContain('ff-select__menu--up'); + + Object.defineProperty(window, 'innerHeight', { value: originalInner, configurable: true }); + }); + + it('opens downward when there is room below the trigger', async () => { + const originalInner = window.innerHeight; + Object.defineProperty(window, 'innerHeight', { value: 800, configurable: true }); + const w = mount(Select, { + props: { modelValue: 'a', options: OPTS }, + attachTo: document.body, + }); + const root = w.find('.ff-select').element as HTMLElement; + vi.spyOn(root, 'getBoundingClientRect').mockReturnValue({ + x: 0, y: 100, top: 100, bottom: 132, + left: 0, right: 120, width: 120, height: 32, + toJSON: () => undefined, + } as DOMRect); + + await w.find('.ff-select__trigger').trigger('click'); + const menu = w.find('.ff-select__menu'); + expect(menu.exists()).toBe(true); + expect(menu.classes()).toContain('ff-select__menu--down'); + + Object.defineProperty(window, 'innerHeight', { value: originalInner, configurable: true }); + }); }); diff --git a/web/src/components/molecules/Select.vue b/web/src/components/molecules/Select.vue index a0a65ad..28330ad 100644 --- a/web/src/components/molecules/Select.vue +++ b/web/src/components/molecules/Select.vue @@ -1,5 +1,5 @@