diff --git a/packages/node/octobot_node/scheduler/api.py b/packages/node/octobot_node/scheduler/api.py index d2caf6211f..2fdea5df6a 100644 --- a/packages/node/octobot_node/scheduler/api.py +++ b/packages/node/octobot_node/scheduler/api.py @@ -132,7 +132,13 @@ def _get_active_execution( def _build_tasks_from_executions( executions: list[octobot_node.models.Execution], + include_content: bool = True, ) -> list[octobot_node.models.Task]: + """ + ``Task.content`` is a copy of the active execution's ``actions``, which is already + part of the returned executions. List callers don't read it, so ``include_content=False`` + drops that duplicate: on a polled list endpoint it is the largest string in the payload. + """ grouped: dict[str, list[octobot_node.models.Execution]] = {} for execution in executions: parent_id = execution.id[:octobot_node.constants.PARENT_WORKFLOW_ID_LENGTH] @@ -142,7 +148,7 @@ def _build_tasks_from_executions( for parent_id, group in grouped.items(): active = _get_active_execution(group) active_name = active.name if active else None - active_content = active.actions if active else None + active_content = (active.actions if active else None) if include_content else None error = active.error if active else None active_wallet = active.user_id if active else None is_encrypted = any(e.is_encrypted for e in group) @@ -208,6 +214,7 @@ async def _enrich_tasks_with_child_octobot_process( async def get_all_tasks( user_id: typing.Optional[str] = None, + include_content: bool = True, ) -> list[octobot_node.models.Task]: executions: list[octobot_node.models.Execution] = [] try: @@ -225,7 +232,7 @@ async def get_all_tasks( logger.error("Failed to retrieve tasks from scheduler: %s", e) return [] - tasks = _build_tasks_from_executions(executions) + tasks = _build_tasks_from_executions(executions, include_content=include_content) await _enrich_tasks_with_child_octobot_process(tasks, user_id) logger.debug("Returning %d total tasks from %d executions", len(tasks), len(executions)) return tasks diff --git a/packages/node/octobot_node/scheduler/workflows_util.py b/packages/node/octobot_node/scheduler/workflows_util.py index 46a1730faf..8c73dbaa85 100644 --- a/packages/node/octobot_node/scheduler/workflows_util.py +++ b/packages/node/octobot_node/scheduler/workflows_util.py @@ -360,8 +360,9 @@ def get_automation_workflow_inputs(workflow_status: dbos_lib.WorkflowStatus) -> parsed_inputs = params.AutomationWorkflowInputs.from_dict(input) return parsed_inputs except TypeError: - print(f"Failed to parse inputs: {input}") - pass + # not an automation input: this runs on every listed workflow row, + # so keep it quiet and out of stdout. + logger.debug(f"Failed to parse inputs: {input}") return None diff --git a/packages/node/tests/scheduler/test_api.py b/packages/node/tests/scheduler/test_api.py index ce3a95eb6f..d35f06ff45 100644 --- a/packages/node/tests/scheduler/test_api.py +++ b/packages/node/tests/scheduler/test_api.py @@ -349,6 +349,34 @@ async def test_get_all_tasks_success(self, temp_dbos_scheduler) -> None: assert "cccccccc-cccc-cccc-cccc-cccccccccccc" in task_ids assert "dddddddd-dddd-dddd-dddd-dddddddddddd" in task_ids + @pytest.mark.asyncio + async def test_get_all_tasks_content_mirrors_active_execution_actions(self, temp_dbos_scheduler) -> None: + """content duplicates the active execution's actions, so it is opt-in.""" + actions = '[{"action": "buy"}]' + pending_executions = [ + Execution(id="aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", status=TaskStatus.PENDING, actions=actions) + ] + + async def _all_tasks(include_content: bool): + with mock.patch.object( + temp_dbos_scheduler, "get_periodic_tasks", mock.AsyncMock(return_value=[]) + ), mock.patch.object( + temp_dbos_scheduler, "get_pending_tasks", mock.AsyncMock(return_value=pending_executions) + ), mock.patch.object( + temp_dbos_scheduler, "get_scheduled_tasks", mock.AsyncMock(return_value=[]) + ), mock.patch.object( + temp_dbos_scheduler, "get_results", mock.AsyncMock(return_value=[]) + ): + return await get_all_tasks(include_content=include_content) + + included = await _all_tasks(True) + assert included[0].content == actions + + excluded = await _all_tasks(False) + assert excluded[0].content is None + # the actions are still reachable through the execution itself + assert excluded[0].executions[0].actions == actions + @pytest.mark.asyncio async def test_get_all_tasks_merges_same_id(self, temp_dbos_scheduler) -> None: """Test that executions sharing the same parent ID are merged into a single Task.""" diff --git a/packages/tentacles/Services/Interfaces/node_api_interface/api/routes/tasks.py b/packages/tentacles/Services/Interfaces/node_api_interface/api/routes/tasks.py index f53718f753..8d97e5f9a4 100644 --- a/packages/tentacles/Services/Interfaces/node_api_interface/api/routes/tasks.py +++ b/packages/tentacles/Services/Interfaces/node_api_interface/api/routes/tasks.py @@ -82,10 +82,18 @@ async def get_tasks( current_user: CurrentUser, page: int = 1, limit: int = 100, + include_content: bool = False, ) -> typing.Any: + """ + ``include_content`` is off by default: ``Task.content`` duplicates the active execution's + ``actions``, which is already returned in ``executions``. Callers that need the duplicate + (rather than reading it off the execution) can opt back in. + """ limit = max(1, min(limit, _MAX_PAGE_LIMIT)) user_id_filter = None if current_user.is_superuser else evm_to_user_id(current_user.email) - tasks_data = await octobot_node.scheduler.api.get_all_tasks(user_id=user_id_filter) + tasks_data = await octobot_node.scheduler.api.get_all_tasks( + user_id=user_id_filter, include_content=include_content + ) start_idx = (page - 1) * limit end_idx = start_idx + limit @@ -119,7 +127,9 @@ async def delete_tasks( requested_ids = [str(t) for t in taskIds] if not current_user.is_superuser: # Ownership check: only allow deleting own tasks - owned_tasks = await octobot_node.scheduler.api.get_all_tasks(user_id=evm_to_user_id(current_user.email)) + owned_tasks = await octobot_node.scheduler.api.get_all_tasks( + user_id=evm_to_user_id(current_user.email), include_content=False + ) owned_ids = {t.id for t in owned_tasks if t.id is not None} unauthorized = [tid for tid in requested_ids if tid not in owned_ids] if unauthorized: @@ -144,7 +154,9 @@ async def cancel_tasks( current_user: CurrentUser, ) -> list[str]: if not current_user.is_superuser: - owned_tasks = await octobot_node.scheduler.api.get_all_tasks(user_id=evm_to_user_id(current_user.email)) + owned_tasks = await octobot_node.scheduler.api.get_all_tasks( + user_id=evm_to_user_id(current_user.email), include_content=False + ) owned_ids = {t.id for t in owned_tasks if t.id is not None} unauthorized = [tid for tid in body.task_ids if tid not in owned_ids] if unauthorized: diff --git a/packages/tentacles/Services/Interfaces/node_api_interface/tests/test_routes_tasks.py b/packages/tentacles/Services/Interfaces/node_api_interface/tests/test_routes_tasks.py index b3c8c66412..455369b3c0 100644 --- a/packages/tentacles/Services/Interfaces/node_api_interface/tests/test_routes_tasks.py +++ b/packages/tentacles/Services/Interfaces/node_api_interface/tests/test_routes_tasks.py @@ -44,7 +44,7 @@ def test_admin_sees_all_tasks(admin_client, mock_auth): assert resp.status_code == 200 assert len(resp.json()) == 2 # Admin passes no user_id filter - mock_get.assert_called_once_with(user_id=None) + mock_get.assert_called_once_with(user_id=None, include_content=False) def test_tenant_sees_only_own_tasks(tenant_client, mock_auth): @@ -56,7 +56,24 @@ def test_tenant_sees_only_own_tasks(tenant_client, mock_auth): assert len(data) == 1 assert data[0]["user_id"] == TENANT_USER_ID # Tenant's user_id is passed as filter - mock_get.assert_called_once_with(user_id=TENANT_USER_ID) + mock_get.assert_called_once_with(user_id=TENANT_USER_ID, include_content=False) + + +def test_tasks_list_skips_content_by_default(admin_client, mock_auth): + """Task.content duplicates the active execution's actions: the list must not carry it.""" + mock_get = AsyncMock(return_value=[_admin_task()]) + with patch("octobot_node.scheduler.api.get_all_tasks", new=mock_get): + resp = admin_client.get("/api/v1/tasks/") + assert resp.status_code == 200 + mock_get.assert_called_once_with(user_id=None, include_content=False) + + +def test_tasks_list_can_opt_into_content(admin_client, mock_auth): + mock_get = AsyncMock(return_value=[_admin_task()]) + with patch("octobot_node.scheduler.api.get_all_tasks", new=mock_get): + resp = admin_client.get("/api/v1/tasks/?include_content=true") + assert resp.status_code == 200 + mock_get.assert_called_once_with(user_id=None, include_content=True) def test_task_creation_stamps_tenant_wallet(tenant_client, mock_auth): diff --git a/packages/tentacles/Services/Interfaces/node_web_interface/src/components/OctoBots/BotCard.tsx b/packages/tentacles/Services/Interfaces/node_web_interface/src/components/OctoBots/BotCard.tsx index 1525a5cae6..045bb81f43 100644 --- a/packages/tentacles/Services/Interfaces/node_web_interface/src/components/OctoBots/BotCard.tsx +++ b/packages/tentacles/Services/Interfaces/node_web_interface/src/components/OctoBots/BotCard.tsx @@ -263,21 +263,48 @@ function BotCardBody({ ) } +function executionsEqual( + prev: Task["executions"], + next: Task["executions"], +): boolean { + if (prev === next) return true + if ((prev?.length ?? 0) !== (next?.length ?? 0)) return false + if (!prev || !next) return true + for (let i = 0; i < prev.length; i++) { + const a = prev[i] + const b = next[i] + if ( + a.id !== b.id || + a.name !== b.name || + a.status !== b.status || + a.actions !== b.actions || + a.scheduled_at !== b.scheduled_at || + a.completed_at !== b.completed_at || + a.error !== b.error || + a.error_message !== b.error_message + ) { + return false + } + } + return true +} + function areTaskPropsEqual( prev: { task: Task; selected: boolean }, next: { task: Task; selected: boolean }, ): boolean { + if (prev.selected !== next.selected) return false + // react-query reuses unchanged objects, so identity is the common fast path. + if (prev.task === next.task) return true return ( - prev.selected === next.selected && prev.task.id === next.task.id && prev.task.name === next.task.name && + prev.task.is_encrypted === next.task.is_encrypted && prev.task.error === next.task.error && prev.task.error_message === next.task.error_message && - prev.task.executions?.length === next.task.executions?.length && - JSON.stringify(prev.task.executions) === - JSON.stringify(next.task.executions) && - JSON.stringify(prev.task.metadata?.child_octobot_process) === - JSON.stringify(next.task.metadata?.child_octobot_process) + prev.task.metadata?.child_octobot_process?.web_port === + next.task.metadata?.child_octobot_process?.web_port && + executionsEqual(prev.task.executions, next.task.executions) ) } diff --git a/packages/tentacles/Services/Interfaces/node_web_interface/src/lib/__tests__/device-key.test.ts b/packages/tentacles/Services/Interfaces/node_web_interface/src/lib/__tests__/device-key.test.ts index 08bc1dfe0e..d736554ca2 100644 --- a/packages/tentacles/Services/Interfaces/node_web_interface/src/lib/__tests__/device-key.test.ts +++ b/packages/tentacles/Services/Interfaces/node_web_interface/src/lib/__tests__/device-key.test.ts @@ -7,6 +7,7 @@ import { derivePassphraseKey, hasStoredClientKeys, loadClientKeys, + loadPassword, saveClientKeys, savePassword, } from "../device-key" @@ -77,11 +78,13 @@ async function clearIDB(): Promise { beforeEach(async () => { localStorageMock.clear() + await clearPassword() await clearIDB() }) afterEach(async () => { localStorageMock.clear() + await clearPassword() await clearIDB() }) @@ -350,3 +353,41 @@ describe("wallet switch restores keys on login", () => { expect(await loadClientKeys()).toEqual(SAMPLE_KEYS) }) }) + +// ─── connection reuse / password cache ────────────────────────────────────── + +describe("IndexedDB connection reuse", () => { + it("does not reopen the database on repeated reads", async () => { + setWallet(WALLET_A) + await savePassword(PASSPHRASE_A) + await saveClientKeys(SAMPLE_KEYS) + // warm up so the shared connection is already established + await loadClientKeys() + + // loadClientKeys always hits the store, so any reopen would show up here + const openSpy = vi.spyOn(indexedDB, "open") + for (let i = 0; i < 5; i++) { + expect(await loadClientKeys()).toEqual(SAMPLE_KEYS) + } + expect(openSpy).not.toHaveBeenCalled() + openSpy.mockRestore() + }) +}) + +describe("password cache", () => { + it("clearPassword invalidates the cached value", async () => { + await savePassword(PASSPHRASE_A) + expect(await loadPassword()).toBe(PASSPHRASE_A) + + await clearPassword() + expect(await loadPassword()).toBeNull() + }) + + it("savePassword refreshes the cached value", async () => { + await savePassword(PASSPHRASE_A) + expect(await loadPassword()).toBe(PASSPHRASE_A) + + await savePassword(PASSPHRASE_B) + expect(await loadPassword()).toBe(PASSPHRASE_B) + }) +}) diff --git a/packages/tentacles/Services/Interfaces/node_web_interface/src/lib/device-key.ts b/packages/tentacles/Services/Interfaces/node_web_interface/src/lib/device-key.ts index dd12fd9f56..bf05a4f221 100644 --- a/packages/tentacles/Services/Interfaces/node_web_interface/src/lib/device-key.ts +++ b/packages/tentacles/Services/Interfaces/node_web_interface/src/lib/device-key.ts @@ -10,15 +10,37 @@ interface EncryptedRecord { ciphertext: ArrayBuffer } +// One connection per tab. An IndexedDB connection stays alive until it is closed, +// and openDB() runs on every API request (OpenAPI.PASSWORD is resolved per +// request), so opening a fresh one each time leaks a connection each time. +let dbPromise: Promise | null = null + function openDB(): Promise { - return new Promise((resolve, reject) => { + if (dbPromise) return dbPromise + dbPromise = new Promise((resolve, reject) => { const req = indexedDB.open(DB_NAME, 1) req.onupgradeneeded = () => { req.result.createObjectStore(STORE_NAME) } - req.onsuccess = () => resolve(req.result) - req.onerror = () => reject(req.error) + req.onsuccess = () => { + const db = req.result + // Forget the handle if it dies so the next call reopens instead of + // reusing a dead connection. + db.onclose = () => { + dbPromise = null + } + db.onversionchange = () => { + db.close() + dbPromise = null + } + resolve(db) + } + req.onerror = () => { + dbPromise = null + reject(req.error) + } }) + return dbPromise } function idbGet(store: IDBObjectStore, key: string): Promise { @@ -49,10 +71,24 @@ function idbDelete(store: IDBObjectStore, key: string): Promise { }) } -async function getOrCreateDeviceKey(): Promise { - const readDb = await openDB() +// The device key never changes for a given browser profile, so resolve it once +// instead of reading and re-reading it on every encrypt/decrypt. +let deviceKeyPromise: Promise | null = null + +function getOrCreateDeviceKey(): Promise { + if (!deviceKeyPromise) { + deviceKeyPromise = loadOrCreateDeviceKey().catch((err) => { + deviceKeyPromise = null + throw err + }) + } + return deviceKeyPromise +} + +async function loadOrCreateDeviceKey(): Promise { + const db = await openDB() const existing = await idbGet( - readDb.transaction(STORE_NAME, "readonly").objectStore(STORE_NAME), + db.transaction(STORE_NAME, "readonly").objectStore(STORE_NAME), DEVICE_KEY_RECORD, ) if (existing) return existing @@ -62,9 +98,9 @@ async function getOrCreateDeviceKey(): Promise { false, ["encrypt", "decrypt"], ) - const writeDb = await openDB() + // A new transaction: the generateKey() await above ends the previous one. await idbPut( - writeDb.transaction(STORE_NAME, "readwrite").objectStore(STORE_NAME), + db.transaction(STORE_NAME, "readwrite").objectStore(STORE_NAME), DEVICE_KEY_RECORD, key, ) @@ -141,15 +177,25 @@ async function idbClearRecord(recordKey: string): Promise { ) } +// Kept in memory for the tab lifetime: loadPassword() is called for every API +// request, and re-reading plus AES-GCM decrypting it each time is pure overhead. +let cachedPassword: string | null = null + export async function savePassword(password: string): Promise { await idbSaveRecord(AUTH_PASSWORD_RECORD, password) + cachedPassword = password } export async function loadPassword(): Promise { - return idbLoadRecord(AUTH_PASSWORD_RECORD) + if (cachedPassword !== null) return cachedPassword + cachedPassword = await idbLoadRecord(AUTH_PASSWORD_RECORD) + return cachedPassword } export async function clearPassword(): Promise { + // Drop the cache first so a concurrent loadPassword() cannot repopulate it + // from the record before the delete lands. + cachedPassword = null await idbClearRecord(AUTH_PASSWORD_RECORD) } diff --git a/packages/tentacles/Services/Interfaces/node_web_interface/src/lib/task-queries.ts b/packages/tentacles/Services/Interfaces/node_web_interface/src/lib/task-queries.ts index 9bdaa884d8..d54d69b284 100644 --- a/packages/tentacles/Services/Interfaces/node_web_interface/src/lib/task-queries.ts +++ b/packages/tentacles/Services/Interfaces/node_web_interface/src/lib/task-queries.ts @@ -4,6 +4,8 @@ export function getTasksQueryOptions() { return { queryFn: () => TasksService.getTasks({ page: 1, limit: 100 }), queryKey: ["tasks"], - refetchInterval: 2_000, + refetchInterval: 5_000, + // A hidden tab has nothing to show: stop polling until it is visible again. + refetchIntervalInBackground: false, } }