fix: address workflow advisor review issues (bosmain-nj6)

packages/browseros-agent/apps/agent/entrypoints/sidepanel/index/useChatSession.ts

@@ -762,6 +762,12 @@ export const useChatSession = (options?: ChatSessionOptions) => {
   }, [dispatchMessage, isIntegrationsSynced])
 
   const sendMessage = (params: { text: string; action?: ChatAction }) => {
+    const workflowAdvisorCommand = detectWorkflowAdvisorCommand(params.text)
+    if (workflowAdvisorCommand) {
+      void handleWorkflowAdvisorCommand(params.text, workflowAdvisorCommand)
+      return
+    }
+
     const target = selectedChatTargetRef.current
     const llmTargetProvider = toLlmProviderConfig(target)
     const agentTarget = target?.kind === 'acp' ? target : undefined
@@ -780,12 +786,6 @@ export const useChatSession = (options?: ChatSessionOptions) => {
         selectedLlmProvider?.modelId,
     })
 
-    const workflowAdvisorCommand = detectWorkflowAdvisorCommand(params.text)
-    if (workflowAdvisorCommand) {
-      void handleWorkflowAdvisorCommand(params.text, workflowAdvisorCommand)
-      return
-    }
-
     if (!isIntegrationsSyncedRef.current) {
       // Queue the message — will be sent when sync completes
       pendingMessageRef.current = params

packages/browseros-agent/apps/agent/lib/workflow-usage/advisor.test.ts

@@ -10,9 +10,26 @@ import type { WorkflowUsageRecord } from './types'
 describe('workflow usage advisor', () => {
   it('detects explicit workflow advisor commands only', () => {
     expect(detectWorkflowAdvisorCommand('analyze my workflow')).toBe('analyze')
-    expect(detectWorkflowAdvisorCommand('what patterns do you see?')).toBe(
-      'analyze',
-    )
+    expect(
+      detectWorkflowAdvisorCommand('what patterns do you see in my workflow?'),
+    ).toBe('analyze')
+    expect(
+      detectWorkflowAdvisorCommand('what patterns do you see in this log?'),
+    ).toBeNull()
+    expect(detectWorkflowAdvisorCommand('what patterns do you see?')).toBeNull()
+    expect(
+      detectWorkflowAdvisorCommand('what can be automated from my tool usage?'),
+    ).toBe('analyze')
+    expect(
+      detectWorkflowAdvisorCommand('what can be automated in this code?'),
+    ).toBeNull()
+    expect(detectWorkflowAdvisorCommand('suggest skills')).toBeNull()
+    expect(
+      detectWorkflowAdvisorCommand('suggest skills from my workflow usage'),
+    ).toBe('analyze')
+    expect(
+      detectWorkflowAdvisorCommand('what workflow usage data is stored?'),
+    ).toBe('view')
     expect(detectWorkflowAdvisorCommand('show workflow usage data')).toBe(
       'view',
     )
@@ -22,6 +39,41 @@ describe('workflow usage advisor', () => {
     expect(detectWorkflowAdvisorCommand('summarize this page')).toBeNull()
   })
 
+  it('assigns suggestion ids after ranking', () => {
+    const analysis = analyzeWorkflowUsage([
+      record('1', ['search', 'open'], 100),
+      record('2', ['search', 'open'], 200),
+      record('3', ['new_page', 'navigate', 'get_page_content'], 300),
+      record('4', ['new_page', 'navigate', 'get_page_content'], 400),
+      record('5', ['new_page', 'navigate', 'get_page_content'], 500),
+    ])
+
+    expect(analysis.suggestions.map((suggestion) => suggestion.id)).toEqual([
+      'workflow-1',
+      'workflow-2',
+    ])
+    expect(analysis.suggestions[0]).toMatchObject({
+      id: 'workflow-1',
+      runCount: 3,
+      pattern: ['new_page', 'navigate', 'get_page_content'],
+    })
+    expect(analysis.suggestions[1]).toMatchObject({
+      id: 'workflow-2',
+      runCount: 2,
+      pattern: ['search', 'open'],
+    })
+  })
+
+  it('keeps workflow usage view commands separate from analysis', () => {
+    expect(detectWorkflowAdvisorCommand('show workflow usage data')).toBe(
+      'view',
+    )
+    expect(detectWorkflowAdvisorCommand('list workflow patterns')).toBe('view')
+    expect(detectWorkflowAdvisorCommand('analyze workflow patterns')).toBe(
+      'analyze',
+    )
+  })
+
   it('normalizes command sequences without retaining repeated adjacent tools', () => {
     expect(normalizeToolSequence([' new_page ', 'new_page', 'open'])).toEqual([
       'new_page',

packages/browseros-agent/apps/agent/lib/workflow-usage/advisor.ts

@@ -43,6 +43,11 @@ export function detectWorkflowAdvisorCommand(
     normalized.includes('usage pattern') ||
     normalized.includes('workflow pattern') ||
     normalized.includes('skill suggestion')
+  const mentionsWorkflowScope =
+    mentionsWorkflowData ||
+    normalized.includes('my workflow') ||
+    normalized.includes('my workflows') ||
+    normalized.includes('tool usage')
 
   if (
     mentionsWorkflowData &&
@@ -53,7 +58,8 @@ export function detectWorkflowAdvisorCommand(
 
   if (
     mentionsWorkflowData &&
-    /\b(show|view|list|display|what)\b/.test(normalized)
+    (/\b(show|view|list|display)\b/.test(normalized) ||
+      normalized.includes('what workflow usage data'))
   ) {
     return 'view'
   }
@@ -61,10 +67,11 @@ export function detectWorkflowAdvisorCommand(
   if (
     normalized.includes('analyze my workflow') ||
     normalized.includes('analyse my workflow') ||
-    normalized.includes('what patterns do you see') ||
-    normalized.includes('suggest skills') ||
+    (normalized.includes('what patterns do you see') &&
+      mentionsWorkflowScope) ||
+    (normalized.includes('suggest skills') && mentionsWorkflowScope) ||
     normalized.includes('find skill suggestions') ||
-    normalized.includes('what can be automated') ||
+    (normalized.includes('what can be automated') && mentionsWorkflowScope) ||
     normalized.includes('analyze workflow patterns') ||
     normalized.includes('analyse workflow patterns')
   ) {
@@ -117,8 +124,8 @@ function buildBenefit(pattern: string[]): string {
 }
 
 function compareSuggestions(
-  left: WorkflowSkillSuggestion,
-  right: WorkflowSkillSuggestion,
+  left: Omit<WorkflowSkillSuggestion, 'id'>,
+  right: Omit<WorkflowSkillSuggestion, 'id'>,
 ): number {
   return (
     right.runCount - left.runCount ||
@@ -153,10 +160,9 @@ export function analyzeWorkflowUsage(
 
   const suggestions = Array.from(groups.values())
     .filter((group) => group.runCount >= minRuns)
-    .map((group, index): WorkflowSkillSuggestion => {
+    .map((group): Omit<WorkflowSkillSuggestion, 'id'> => {
       const pattern = group.pattern
       return {
-        id: `workflow-${index + 1}`,
         title: buildSuggestionTitle(pattern),
         runCount: group.runCount,
         pattern,
@@ -166,6 +172,12 @@ export function analyzeWorkflowUsage(
     })
     .sort(compareSuggestions)
     .slice(0, limit)
+    .map((suggestion, index): WorkflowSkillSuggestion => {
+      return {
+        ...suggestion,
+        id: `workflow-${index + 1}`,
+      }
+    })
 
   return {
     totalRuns: records.length,

packages/browseros-agent/apps/agent/lib/workflow-usage/storage.test.ts

@@ -0,0 +1,66 @@
+import { beforeEach, describe, expect, it, mock } from 'bun:test'
+import type { WorkflowUsageRecord, WorkflowUsageStore } from './types'
+
+let storedValue: WorkflowUsageStore | null = null
+
+mock.module('@wxt-dev/storage', () => ({
+  storage: {
+    defineItem: () => ({
+      getValue: async () => (storedValue ? structuredClone(storedValue) : null),
+      setValue: async (value: WorkflowUsageStore) => {
+        await Promise.resolve()
+        storedValue = structuredClone(value)
+      },
+    }),
+  },
+}))
+
+const {
+  clearWorkflowUsageRecords,
+  getWorkflowUsageRecords,
+  recordWorkflowUsage,
+} = await import('./storage')
+
+describe('workflow usage storage', () => {
+  beforeEach(() => {
+    storedValue = { version: 1, records: [] }
+  })
+
+  it('serializes concurrent record writes without dropping records', async () => {
+    await Promise.all([
+      recordWorkflowUsage(record('record-1', ['new_page', 'navigate'], 100)),
+      recordWorkflowUsage(
+        record('record-2', ['search', 'get_page_content'], 200),
+      ),
+    ])
+
+    expect((await getWorkflowUsageRecords()).map((item) => item.id)).toEqual([
+      'record-1',
+      'record-2',
+    ])
+  })
+
+  it('keeps clear operations ordered with pending writes', async () => {
+    const write = recordWorkflowUsage(
+      record('record-1', ['new_page', 'navigate'], 100),
+    )
+    const clear = clearWorkflowUsageRecords()
+
+    await Promise.all([write, clear])
+
+    expect(await getWorkflowUsageRecords()).toEqual([])
+  })
+})
+
+function record(
+  id: string,
+  toolNames: string[],
+  recordedAt: number,
+): WorkflowUsageRecord {
+  return {
+    id,
+    source: 'sidepanel-chat',
+    recordedAt,
+    toolNames,
+  }
+}

packages/browseros-agent/apps/agent/lib/workflow-usage/storage.ts

@@ -8,6 +8,7 @@ import type {
 } from './types'
 
 const MAX_WORKFLOW_USAGE_RECORDS = 300
+let pendingWorkflowUsageStorageUpdate: Promise<void> = Promise.resolve()
 
 export const workflowUsageStorage = storage.defineItem<WorkflowUsageStore>(
   'local:workflowUsagePatterns',
@@ -50,12 +51,43 @@ export async function recordWorkflowUsage(
 ): Promise<void> {
   if (!record) return
 
-  const current = (await workflowUsageStorage.getValue()) ?? {
+  await enqueueWorkflowUsageStorageUpdate(async () => {
+    const current = (await workflowUsageStorage.getValue()) ?? {
+      version: 1,
+      records: [],
+    }
+    await workflowUsageStorage.setValue(
+      mergeWorkflowUsageRecord(current, record),
+    )
+  })
+}
+
+function enqueueWorkflowUsageStorageUpdate(
+  update: () => Promise<void>,
+): Promise<void> {
+  const runUpdate = pendingWorkflowUsageStorageUpdate
+    .catch(() => {
+      // Keep later writes moving even if an earlier storage call failed.
+    })
+    .then(update)
+
+  pendingWorkflowUsageStorageUpdate = runUpdate.catch(() => {
+    // Store the rejection for the caller while leaving the queue usable.
+  })
+
+  return runUpdate
+}
+
+function mergeWorkflowUsageRecord(
+  current: WorkflowUsageStore | null | undefined,
+  record: WorkflowUsageRecord,
+): WorkflowUsageStore {
+  const store = current ?? {
     version: 1,
     records: [],
   }
   const recordsById = new Map(
-    current.records.map((existing) => [existing.id, existing]),
+    store.records.map((existing) => [existing.id, existing]),
   )
   recordsById.set(record.id, record)
 
@@ -63,16 +95,21 @@ export async function recordWorkflowUsage(
     .sort((left, right) => left.recordedAt - right.recordedAt)
     .slice(-MAX_WORKFLOW_USAGE_RECORDS)
 
-  await workflowUsageStorage.setValue({ version: 1, records })
+  return { version: 1, records }
 }
 
 export async function getWorkflowUsageRecords(): Promise<
   WorkflowUsageRecord[]
 > {
+  await pendingWorkflowUsageStorageUpdate.catch(() => {
+    // Preserve existing read behavior after a failed write.
+  })
   const current = await workflowUsageStorage.getValue()
   return current?.records ?? []
 }
 
 export async function clearWorkflowUsageRecords(): Promise<void> {
-  await workflowUsageStorage.setValue({ version: 1, records: [] })
+  await enqueueWorkflowUsageStorageUpdate(async () => {
+    await workflowUsageStorage.setValue({ version: 1, records: [] })
+  })
 }