From c26e7420fbddcc3565ae3aaac5bc501176d1442a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Thu, 23 Apr 2026 19:39:29 -0400 Subject: [PATCH] fix(browser): queue browser action dispatch failures --- src/main/api-server/routes-misc.ts | 23 ++- .../automation/automation-approval-handler.ts | 12 +- src/main/automation/automation-handlers.ts | 2 +- src/main/automation/index.ts | 6 +- src/main/automation/types.ts | 2 +- src/main/bootstrap-application.ts | 25 +++- src/main/bootstrap/recover-scheduled-posts.ts | 4 +- src/main/browser-action-service.ts | 59 +++++++- .../create-background-services.ts | 6 +- tests/unit/approval-hire-routing.test.ts | 2 +- ...approval-review-feedback-writeback.test.ts | 2 +- tests/unit/browser-action-queue.test.ts | 133 ++++++++++++++++++ 12 files changed, 251 insertions(+), 25 deletions(-) create mode 100644 tests/unit/browser-action-queue.test.ts diff --git a/src/main/api-server/routes-misc.ts b/src/main/api-server/routes-misc.ts index c021a784..df44c611 100644 --- a/src/main/api-server/routes-misc.ts +++ b/src/main/api-server/routes-misc.ts @@ -97,7 +97,28 @@ export function registerMiscRoutes(server: AgentApiServer) { const onBrowserAction = server.onBrowserAction; if (onBrowserAction) { - onBrowserAction(actionId, companyId); + try { + onBrowserAction(actionId, companyId); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + db.core.social.updateBrowserAction(actionId, { + status: "failed", + resultSummary: "Browser action dispatch failed before execution completed.", + errorMessage: message, + finishedAt: new Date().toISOString(), + }); + db.core.activity.addActivity({ + companyId, + actor: token.agentId, + action: "browser.action_dispatch_failed", + entityType: "browser_action", + entityId: actionId, + detail: message, + }); + onDomainChanged(); + server.json(res, 500, { id: actionId, status: "failed", error: "Browser action dispatch failed" }); + return; + } } server.json(res, 201, { id: actionId, status: "queued", message: "Browser action queued for execution" }); diff --git a/src/main/automation/automation-approval-handler.ts b/src/main/automation/automation-approval-handler.ts index fc75c5d6..0506ea7f 100644 --- a/src/main/automation/automation-approval-handler.ts +++ b/src/main/automation/automation-approval-handler.ts @@ -62,7 +62,7 @@ export interface ApprovalHandlerDeps { publishDomainChanged: () => void; notify: (options: NotificationOptions) => void; wakeAgentIfPossible: (agentId: string | null | undefined, companyId: string, trigger: string) => string | null; - dispatchBrowserAction: (actionId: string, companyId: string) => Promise; + queueBrowserActionExecution: (actionId: string, companyId: string) => void; isQuitting: () => boolean; } @@ -81,7 +81,7 @@ export function handleApprovalResolved( decision: string, ): void { if (deps.isQuitting()) return; - const { db, logger, publishDomainChanged, notify, wakeAgentIfPossible, dispatchBrowserAction } = deps; + const { db, logger, publishDomainChanged, notify, wakeAgentIfPossible, queueBrowserActionExecution } = deps; const { dispatchAutomation, handleTaskStatusChange, runWorkflowPipeline, trackTimer } = callbacks; const snapshot = db.projection.snapshot.listSnapshot(); @@ -259,9 +259,7 @@ export function handleApprovalResolved( errorMessage: null, }); publishDomainChanged(); - void dispatchBrowserAction(browserAction.id, companyId).catch((error) => { - logger.error(`[browser] Failed to dispatch approved browser action ${browserAction.id}: ${error instanceof Error ? error.message : String(error)}`); - }); + queueBrowserActionExecution(browserAction.id, companyId); } else if (decision === "rejected") { db.core.social.updateBrowserAction(browserAction.id, { status: "cancelled", @@ -301,9 +299,7 @@ export function handleApprovalResolved( approvalId: approval.id, }); publishDomainChanged(); - void dispatchBrowserAction(actionId, approval.companyId).catch((error) => { - logger.error(`[automation] Failed to dispatch approved social post ${actionId}: ${error instanceof Error ? error.message : String(error)}`); - }); + queueBrowserActionExecution(actionId, approval.companyId); }; if (draft.scheduledAt) { diff --git a/src/main/automation/automation-handlers.ts b/src/main/automation/automation-handlers.ts index 7f31742d..b8f0b7ca 100644 --- a/src/main/automation/automation-handlers.ts +++ b/src/main/automation/automation-handlers.ts @@ -13,7 +13,7 @@ export interface HandlerDeps { publishDomainChanged: () => void; notify: (options: NotificationOptions) => void; wakeAgentIfPossible: (agentId: string | null | undefined, companyId: string, trigger: string) => string | null; - dispatchBrowserAction: (actionId: string, companyId: string) => Promise; + queueBrowserActionExecution: (actionId: string, companyId: string) => void; isQuitting: () => boolean; } diff --git a/src/main/automation/index.ts b/src/main/automation/index.ts index 572cec9c..9001c564 100644 --- a/src/main/automation/index.ts +++ b/src/main/automation/index.ts @@ -21,7 +21,7 @@ export function createAutomationService({ publishDomainChanged, notify, wakeAgentIfPossible, - dispatchBrowserAction, + queueBrowserActionExecution, isQuitting, }: AutomationDependencies) { const activeAutomationExecutions = new Set(); @@ -48,7 +48,7 @@ export function createAutomationService({ publishDomainChanged, notify, wakeAgentIfPossible, - dispatchBrowserAction, + queueBrowserActionExecution, isQuitting, }; @@ -58,7 +58,7 @@ export function createAutomationService({ publishDomainChanged, notify, wakeAgentIfPossible, - dispatchBrowserAction, + queueBrowserActionExecution, isQuitting, }; diff --git a/src/main/automation/types.ts b/src/main/automation/types.ts index 66c255a4..0133da22 100644 --- a/src/main/automation/types.ts +++ b/src/main/automation/types.ts @@ -47,7 +47,7 @@ export interface AutomationDependencies { publishDomainChanged: () => void; notify: (options: NotificationOptions) => void; wakeAgentIfPossible: (agentId: string | null | undefined, companyId: string, trigger: string) => string | null; - dispatchBrowserAction: (actionId: string, companyId: string) => Promise; + queueBrowserActionExecution: (actionId: string, companyId: string) => void; isQuitting: () => boolean; } diff --git a/src/main/bootstrap-application.ts b/src/main/bootstrap-application.ts index c4141057..68c2b8b2 100644 --- a/src/main/bootstrap-application.ts +++ b/src/main/bootstrap-application.ts @@ -131,10 +131,29 @@ export async function bootstrapApplication(deps: BootstrapApplicationDependencie app.on("before-quit", () => lifecycleProcessor.stop()); if (apiServer && serviceGraph.getBrowserActionService()) { - apiServer.setBrowserActionHandler(async (actionId, companyId) => { + apiServer.setBrowserActionHandler((actionId, companyId) => { const browserActions = serviceGraph.getBrowserActionService(); - if (!browserActions) throw new Error("Browser action service is not initialized"); - await browserActions.dispatchBrowserAction(actionId, companyId); + if (!browserActions) { + const message = "Browser action service is not initialized."; + log.error(`[browser] ${message} action=${actionId}`); + db.core.social.updateBrowserAction(actionId, { + status: "failed", + resultSummary: "Browser action dispatch failed before execution completed.", + errorMessage: message, + finishedAt: new Date().toISOString(), + }); + db.core.activity.addActivity({ + companyId, + actor: "system", + action: "browser.action_dispatch_failed", + entityType: "browser_action", + entityId: actionId, + detail: message, + }); + publishDomainChanged(); + return; + } + browserActions.queueBrowserActionExecution(actionId, companyId); }); } diff --git a/src/main/bootstrap/recover-scheduled-posts.ts b/src/main/bootstrap/recover-scheduled-posts.ts index 53821991..1d901715 100644 --- a/src/main/bootstrap/recover-scheduled-posts.ts +++ b/src/main/bootstrap/recover-scheduled-posts.ts @@ -9,7 +9,7 @@ export interface RecoverScheduledPostsDeps { db: AppDatabase; logger: Logger; isQuittingRef: { current: boolean }; - getBrowserActionService: () => { dispatchBrowserAction: (actionId: string, companyId: string) => Promise } | null; + getBrowserActionService: () => { queueBrowserActionExecution: (actionId: string, companyId: string) => void } | null; } export function recoverScheduledPosts(deps: RecoverScheduledPostsDeps): Set> { @@ -30,7 +30,7 @@ export function recoverScheduledPosts(deps: RecoverScheduledPostsDeps): Set { + try { + recordDispatchFailure(actionId, companyId, error); + } catch (recordError) { + logAppError(log, recordError, { + kind: "fatal", + code: "browser_action.dispatch_failure_record.failed", + message: `Failed to record browser action ${actionId} dispatch failure`, + cause: recordError, + context: { actionId, companyId, dispatchError: formatError(error) }, + }); + } + }); + } + /** * Returns the approval record for `approvalId`, or null if the record is * missing. Isolates the "approval row disappeared" case so the caller does @@ -157,6 +213,7 @@ export function createBrowserActionService({ return { dispatchBrowserAction, + queueBrowserActionExecution, clearSocialAccountData, cancelAction, launchLoginBrowser, diff --git a/src/main/service-graph/create-background-services.ts b/src/main/service-graph/create-background-services.ts index 53303471..5eb6d40d 100644 --- a/src/main/service-graph/create-background-services.ts +++ b/src/main/service-graph/create-background-services.ts @@ -57,8 +57,8 @@ export function createBackgroundServices({ }); browserActionService.resetBrowserManager(); - const dispatchBrowserAction = async (actionId: string, companyId: string) => - browserActionService.dispatchBrowserAction(actionId, companyId); + const queueBrowserActionExecution = (actionId: string, companyId: string) => + browserActionService.queueBrowserActionExecution(actionId, companyId); const automationService = createAutomationService({ db, @@ -68,7 +68,7 @@ export function createBackgroundServices({ notify, wakeAgentIfPossible: (agentId, companyId, trigger) => orchestrationService.wakeAgentIfPossible(agentId, companyId, trigger), - dispatchBrowserAction, + queueBrowserActionExecution, isQuitting: () => isQuittingRef.current, }); diff --git a/tests/unit/approval-hire-routing.test.ts b/tests/unit/approval-hire-routing.test.ts index 638e98cd..12242af4 100644 --- a/tests/unit/approval-hire-routing.test.ts +++ b/tests/unit/approval-hire-routing.test.ts @@ -107,7 +107,7 @@ describe("hire approval routing", () => { publishDomainChanged: () => {}, notify, wakeAgentIfPossible, - dispatchBrowserAction: async () => {}, + queueBrowserActionExecution: () => {}, isQuitting: () => false, }, { dispatchAutomation, diff --git a/tests/unit/approval-review-feedback-writeback.test.ts b/tests/unit/approval-review-feedback-writeback.test.ts index c2be6735..3aeea474 100644 --- a/tests/unit/approval-review-feedback-writeback.test.ts +++ b/tests/unit/approval-review-feedback-writeback.test.ts @@ -98,7 +98,7 @@ describe("deliverable review feedback writeback", () => { publishDomainChanged: () => {}, notify: vi.fn(), wakeAgentIfPossible, - dispatchBrowserAction: async () => {}, + queueBrowserActionExecution: () => {}, isQuitting: () => false, }, { dispatchAutomation: vi.fn(), diff --git a/tests/unit/browser-action-queue.test.ts b/tests/unit/browser-action-queue.test.ts new file mode 100644 index 00000000..59c8b469 --- /dev/null +++ b/tests/unit/browser-action-queue.test.ts @@ -0,0 +1,133 @@ +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { AppDatabase } from "../../src/main/database"; +import { createBrowserActionService } from "../../src/main/browser-action-service"; +import { recoverScheduledPosts } from "../../src/main/bootstrap/recover-scheduled-posts"; +import type { BrowserManager } from "../../src/main/browser-manager"; + +const tempDirs: string[] = []; + +async function createTempDir(prefix: string) { + const dir = await mkdtemp(join(tmpdir(), prefix)); + tempDirs.push(dir); + return dir; +} + +async function nextTick(): Promise { + await new Promise((resolve) => setImmediate(resolve)); +} + +async function delay(ms: number): Promise { + await new Promise((resolve) => setTimeout(resolve, ms)); +} + +function seedBrowserAction(db: AppDatabase, profileDir: string, input?: { + payloadJson?: string; + approvalId?: string | null; +}): { companyId: string; actionId: string } { + const companyId = db.core.company.saveCompany({ + name: "Browser Queue Co", + description: "Queue contract test", + status: "active", + }); + const workspaceId = db.core.workspaces.saveWorkspace({ + companyId, + name: "Primary", + localPath: profileDir, + repoUrl: "", + repoRef: "", + isPrimary: true, + }); + const agentId = db.core.agents.saveAgent({ + companyId, + name: "CEO", + role: "CEO", + title: "Chief Executive Officer", + status: "active", + reportsTo: null, + connectorId: "openai-codex", + workspaceId, + model: null, + capabilities: "Lead the company", + budgetMonthlyUsd: 250, + }); + const socialAccountId = db.core.social.saveSocialAccount({ + companyId, + platform: "twitter", + accountName: "agentcompany", + status: "ready", + requireApproval: false, + }); + const actionId = db.core.social.saveBrowserAction({ + companyId, + socialAccountId, + agentId, + actionType: "post", + payloadJson: input?.payloadJson ?? JSON.stringify({ content: "hello" }), + status: "queued", + approvalId: input?.approvalId ?? null, + }); + return { companyId, actionId }; +} + +afterEach(async () => { + await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true }))); +}); + +describe("browser action queue", () => { + it("marks queued actions failed and records activity when dispatch fails before execution", async () => { + const profileDir = await createTempDir("agentcompany-browser-queue-"); + const db = new AppDatabase(profileDir); + db.init(); + const { companyId, actionId } = seedBrowserAction(db, profileDir, { + approvalId: "missing-approval", + }); + let published = 0; + const service = createBrowserActionService({ + db, + browserManagerRef: { current: null as BrowserManager | null }, + publishDomainChanged: () => { published += 1; }, + }); + + service.queueBrowserActionExecution(actionId, companyId); + await nextTick(); + + const action = db.core.social.getBrowserAction(actionId, companyId); + expect(action?.status).toBe("failed"); + expect(action?.errorMessage).toContain("missing-approval"); + const activity = db.projection.snapshot.listSnapshot({ companyId }).activity; + expect(activity.some((entry) => entry.action === "browser.action_dispatch_failed" && entry.entityId === actionId)).toBe(true); + expect(published).toBeGreaterThan(0); + db.close(); + }); + + it("startup recovery schedules queued browser actions through the queue wrapper", async () => { + const profileDir = await createTempDir("agentcompany-browser-recovery-"); + const db = new AppDatabase(profileDir); + db.init(); + const scheduledAt = new Date(Date.now() + 40).toISOString(); + const { companyId, actionId } = seedBrowserAction(db, profileDir, { + payloadJson: JSON.stringify({ content: "scheduled", scheduledAt }), + }); + const calls: Array<{ actionId: string; companyId: string }> = []; + + const timers = recoverScheduledPosts({ + db, + logger: { info: () => {}, warn: () => {} }, + isQuittingRef: { current: false }, + getBrowserActionService: () => ({ + queueBrowserActionExecution: (queuedActionId, queuedCompanyId) => { + calls.push({ actionId: queuedActionId, companyId: queuedCompanyId }); + }, + }), + }); + + await delay(80); + for (const timer of timers) clearTimeout(timer); + + expect(calls).toEqual([{ actionId, companyId }]); + db.close(); + }); +});