diff --git a/src/main/database/company-settings.ts b/src/main/database/company-settings.ts index f5cd420c..64959ab8 100644 --- a/src/main/database/company-settings.ts +++ b/src/main/database/company-settings.ts @@ -1,13 +1,21 @@ import type { DatabaseSync } from "node:sqlite"; import type { ProfileSnapshot } from "@shared/types"; -import { - BOARD_READER_ID, - type ReadAll, - type Transaction, -} from "./helpers"; +import type { ReadAll, Transaction } from "./helpers"; +/** + * Company-scoped read endpoints that do not belong to a single domain + * CRUD module. + * + * Sidebar badges used to live here as a server-side aggregation + * (`getSidebarBadges`) — it has been deleted. The desktop app is + * local-first and the renderer already holds every row the badges + * would count via `ProfileSnapshot`, so the aggregation became a + * second source of truth that drifted from the inbox query whenever + * messages were read between snapshot refreshes. Derivation now lives + * in `src/shared/sidebar-badges.ts::deriveSidebarBadges`. + */ export function buildCompanySettingsMethods( - getDb: () => DatabaseSync, + _getDb: () => DatabaseSync, readAll: ReadAll, _transaction: Transaction, _deps: { @@ -52,71 +60,5 @@ export function buildCompanySettingsMethods( ), }; }, - - getSidebarBadges(companyId: string): { - pendingApprovals: number; - activeRuns: number; - failedRuns: number; - todoTasks: number; - budgetWarnings: number; - unreadMessages: number; - } { - const pendingApprovals = - ( - getDb() - .prepare( - "select count(*) as c from approvals where company_id = ? and state = 'pending'", - ) - .get(companyId) as { c: number } - )?.c ?? 0; - const activeRuns = - ( - getDb() - .prepare( - "select count(*) as c from runs where company_id = ? and status in ('queued', 'running')", - ) - .get(companyId) as { c: number } - )?.c ?? 0; - const failedRuns = - ( - getDb() - .prepare( - "select count(*) as c from runs where company_id = ? and status in ('failed', 'timed_out', 'interrupted')", - ) - .get(companyId) as { c: number } - )?.c ?? 0; - const todoTasks = - ( - getDb() - .prepare( - "select count(*) as c from tasks where company_id = ? and status in ('todo', 'in_progress', 'in_review')", - ) - .get(companyId) as { c: number } - )?.c ?? 0; - const budgetWarnings = - ( - getDb() - .prepare( - "select count(*) as c from agents where company_id = ? and budget_monthly_usd > 0 and (spent_monthly_usd * 1.0 / budget_monthly_usd) >= 0.8", - ) - .get(companyId) as { c: number } - )?.c ?? 0; - const unreadMessages = - ( - getDb() - .prepare( - `select count(*) as c - from agent_messages messages - left join agent_message_reads reads - on reads.message_id = messages.id - and reads.agent_id = ? - where messages.company_id = ? - and reads.read_at is null - and (messages.channel = 'company' or messages.channel = 'incident' or messages.priority = 'urgent')`, - ) - .get(BOARD_READER_ID, companyId) as { c: number } - )?.c ?? 0; - return { pendingApprovals, activeRuns, failedRuns, todoTasks, budgetWarnings, unreadMessages }; - }, }; } diff --git a/src/main/ipc-core-read-handlers.ts b/src/main/ipc-core-read-handlers.ts index 269b7de5..caddfe29 100644 --- a/src/main/ipc-core-read-handlers.ts +++ b/src/main/ipc-core-read-handlers.ts @@ -95,13 +95,6 @@ export function registerCoreReadHandlers({ return ok(await checkForUpdates({ manual: true })); }); - registerHandle(IPC_CHANNELS.getSidebarBadges, async (_event, companyId) => { - if (typeof companyId !== "string" || !companyId) { - return fail("INVALID_COMPANY_ID", "A valid company id is required."); - } - return ok(db.getSidebarBadges(companyId)); - }); - registerHandle(IPC_CHANNELS.listConnectorModels, async (_event, connectorId) => { try { const parsed = connectorInputSchema.shape.id.parse(connectorId); diff --git a/src/preload/index.ts b/src/preload/index.ts index 007e4bef..dc878862 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -37,7 +37,6 @@ const api: DesktopApi = { deleteWorkspace: (input) => ipcRenderer.invoke(IPC_CHANNELS.deleteWorkspace, input), deleteApproval: (input) => ipcRenderer.invoke(IPC_CHANNELS.deleteApproval, input), deleteSecret: (input) => ipcRenderer.invoke(IPC_CHANNELS.deleteSecret, input), - getSidebarBadges: (companyId) => ipcRenderer.invoke(IPC_CHANNELS.getSidebarBadges, companyId), listConnectorModels: (connectorId) => ipcRenderer.invoke(IPC_CHANNELS.listConnectorModels, connectorId), getConnectorDisabledModels: (connectorId) => ipcRenderer.invoke(IPC_CHANNELS.getConnectorDisabledModels, connectorId), setConnectorDisabledModels: (input) => ipcRenderer.invoke(IPC_CHANNELS.setConnectorDisabledModels, input), diff --git a/src/renderer/components/communication/MessageList.tsx b/src/renderer/components/communication/MessageList.tsx index 6c3d8609..c16d8c72 100644 --- a/src/renderer/components/communication/MessageList.tsx +++ b/src/renderer/components/communication/MessageList.tsx @@ -41,10 +41,22 @@ export function MessageList({ const parentRef = useRef(null) const threadedMessages = useMemo(() => { - const topLevel = filteredMessages.filter((m) => !m.parentMessageId) + // A message is rendered at the top level when it either has no + // parent, or its parent is outside the current filtered view + // (another channel, beyond the 1000-row snapshot cap, or simply + // not loaded yet). Without this second condition, a DM channel + // whose earliest message has scrolled off the snapshot would + // display as completely empty even though replies for it are + // present — the exact "clicked the channel, got a blank pane" + // symptom. + const idSet = new Set(filteredMessages.map((m) => m.id)) + const topLevel: AgentMessageRecord[] = [] const replyMap = new Map() for (const m of filteredMessages) { - if (m.parentMessageId) { + const parentInView = m.parentMessageId ? idSet.has(m.parentMessageId) : false + if (!m.parentMessageId || !parentInView) { + topLevel.push(m) + } else { const existing = replyMap.get(m.parentMessageId) ?? [] existing.push(m) replyMap.set(m.parentMessageId, existing) diff --git a/src/renderer/hooks/useCompanyController.ts b/src/renderer/hooks/useCompanyController.ts index 9a79bdbd..377ba96e 100644 --- a/src/renderer/hooks/useCompanyController.ts +++ b/src/renderer/hooks/useCompanyController.ts @@ -40,7 +40,6 @@ export function useCompanyController() { store.setStandupReport(null); store.setSocialAccounts([]); store.setBrowserActions([]); - store.setBadges(null); store.setSkills([]); store.setSelectedSkillId(null); store.setSelectedSkillFilePath("SKILL.md"); diff --git a/src/renderer/hooks/useCompanyData.ts b/src/renderer/hooks/useCompanyData.ts index ac825110..1f649801 100644 --- a/src/renderer/hooks/useCompanyData.ts +++ b/src/renderer/hooks/useCompanyData.ts @@ -1,5 +1,6 @@ import { useMemo } from "react"; import type { CompanyRecord, ProfileSnapshot, SectionId } from "@shared/types"; +import { deriveSidebarBadges } from "@shared/sidebar-badges"; import { useAppStore } from "../state/store"; function filterByCompany( @@ -76,18 +77,24 @@ export function useCompanyData( [companyWorkspaces, selectedWorkspaceId], ); - const navBadgesSource = useAppStore((s) => s.badges); - const navBadges = useMemo>>(() => navBadgesSource ? { - runs: - navBadgesSource.failedRuns > 0 - ? { count: navBadgesSource.failedRuns, tone: "danger" } - : navBadgesSource.activeRuns > 0 - ? { count: navBadgesSource.activeRuns, tone: "default" } - : undefined, - tasks: navBadgesSource.todoTasks > 0 ? { count: navBadgesSource.todoTasks, tone: "default" } : undefined, - approvals: navBadgesSource.pendingApprovals > 0 ? { count: navBadgesSource.pendingApprovals, tone: "warn" } : undefined, - communication: navBadgesSource.unreadMessages > 0 ? { count: navBadgesSource.unreadMessages, tone: "warn" } : undefined, - } : {}, [navBadgesSource]); + // Derive sidebar badge counts from the snapshot. Single source of truth — + // no separate `store.badges` cache to drift when messages get marked read + // or approvals get accepted between server-side aggregations. + const navBadges = useMemo>>(() => { + if (!snapshot) return {}; + const badges = deriveSidebarBadges(snapshot, currentCompany?.id ?? null); + return { + runs: + badges.failedRuns > 0 + ? { count: badges.failedRuns, tone: "danger" } + : badges.activeRuns > 0 + ? { count: badges.activeRuns, tone: "default" } + : undefined, + tasks: badges.todoTasks > 0 ? { count: badges.todoTasks, tone: "default" } : undefined, + approvals: badges.pendingApprovals > 0 ? { count: badges.pendingApprovals, tone: "warn" } : undefined, + communication: badges.unreadMessages > 0 ? { count: badges.unreadMessages, tone: "warn" } : undefined, + }; + }, [snapshot]); return useMemo(() => ({ tasks, diff --git a/src/renderer/hooks/useDataLoader.ts b/src/renderer/hooks/useDataLoader.ts index 7e8daf9d..9e709de4 100644 --- a/src/renderer/hooks/useDataLoader.ts +++ b/src/renderer/hooks/useDataLoader.ts @@ -24,8 +24,6 @@ export function useDataLoader( if (!currentSelectedConnectorId && next.connectors[0]) store.setSelectedConnectorId(next.connectors[0].id); if (!currentSelectedRunId && next.runs[0]) store.setSelectedRunId(next.runs[0].id); if (next.currentCompanyId) { - const badgeResult = await window.agentCompany.getSidebarBadges(next.currentCompanyId); - if (badgeResult.ok) store.setBadges(badgeResult.data); const inboxResult = await window.agentCompany.getInbox(next.currentCompanyId); if (inboxResult.ok) store.setInboxItems(inboxResult.data); store.setSocialAccounts(next.socialAccounts?.filter((a) => a.companyId === next.currentCompanyId) ?? []); diff --git a/src/renderer/hooks/useDesktopEvents.ts b/src/renderer/hooks/useDesktopEvents.ts index 2ef2fb18..af70b88e 100644 --- a/src/renderer/hooks/useDesktopEvents.ts +++ b/src/renderer/hooks/useDesktopEvents.ts @@ -106,10 +106,10 @@ export function useDesktopEvents( if (needsFullRefresh) { void refreshRef.current(); - } else { - // Refresh sidebar badges after granular patches (per pitfall 3) - void store.getState().refreshBadges(); } + // Granular patches already update the snapshot arrays, and + // sidebar badges derive from the snapshot live — no separate + // badge refresh is needed. } // 500ms window for full-refresh events (e.g. new-message, diff --git a/src/renderer/state/slices/company.ts b/src/renderer/state/slices/company.ts index 29089636..7c5c8f0c 100644 --- a/src/renderer/state/slices/company.ts +++ b/src/renderer/state/slices/company.ts @@ -10,8 +10,11 @@ import type { StandupReport, TaskRecord, } from "@shared/types"; -import type { SidebarBadges } from "@shared/contracts"; +// Sidebar badge counts live in the renderer — derived from the +// snapshot on every read via `deriveSidebarBadges` rather than cached +// on this slice. One source of truth is the snapshot; adding a +// separate `badges` field here is what let the counters drift. export interface CompanySlice { snapshot: ProfileSnapshot | null; /** @@ -24,13 +27,11 @@ export interface CompanySlice { companyMetrics: CompanyMetrics | null; standupReport: StandupReport | null; inboxItems: InboxItem[]; - badges: SidebarBadges | null; error: string | null; setSnapshot: (snapshot: ProfileSnapshot | null) => void; setCompanyMetrics: (metrics: CompanyMetrics | null) => void; setStandupReport: (report: StandupReport | null) => void; setInboxItems: (items: InboxItem[]) => void; - setBadges: (badges: SidebarBadges | null) => void; setError: (error: string | null) => void; refreshSnapshot: () => Promise; patchTask: (task: TaskRecord) => void; @@ -41,7 +42,6 @@ export interface CompanySlice { removeAgent: (agentId: string) => void; patchApproval: (approval: ApprovalRecord) => void; removeApproval: (approvalId: string) => void; - refreshBadges: () => Promise; } export const createCompanySlice: StateCreator< @@ -49,13 +49,12 @@ export const createCompanySlice: StateCreator< [["zustand/immer", never]], [], CompanySlice -> = (set, get) => ({ +> = (set) => ({ snapshot: null, snapshotRevision: 0, companyMetrics: null, standupReport: null, inboxItems: [], - badges: null, error: null, setSnapshot: (snapshot) => set((state) => { @@ -74,10 +73,6 @@ export const createCompanySlice: StateCreator< set((state) => { state.inboxItems = items; }), - setBadges: (badges) => - set((state) => { - state.badges = badges; - }), setError: (error) => set((state) => { state.error = error; @@ -167,21 +162,4 @@ export const createCompanySlice: StateCreator< if (!state.snapshot) return; state.snapshot.approvals = state.snapshot.approvals.filter((a) => a.id !== approvalId); }), - - refreshBadges: async () => { - const companyId = get().snapshot?.currentCompanyId ?? null; - if (!companyId) return; - try { - const result = await window.agentCompany.getSidebarBadges(companyId); - if (result.ok) { - set((state) => { - state.badges = result.data; - }); - } - } catch (error) { - console.warn( - `[renderer/company] refreshBadges failed for company ${companyId}: ${error instanceof Error ? error.message : String(error)}`, - ); - } - }, }); diff --git a/src/shared/contracts/channels.ts b/src/shared/contracts/channels.ts index 69a1fd8f..b5fd9232 100644 --- a/src/shared/contracts/channels.ts +++ b/src/shared/contracts/channels.ts @@ -30,7 +30,6 @@ export const IPC_CHANNELS = { deleteWorkspace: "agent-company:delete-workspace", deleteApproval: "agent-company:delete-approval", deleteSecret: "agent-company:delete-secret", - getSidebarBadges: "agent-company:get-sidebar-badges", listConnectorModels: "agent-company:list-connector-models", addComment: "agent-company:add-comment", setHeartbeat: "agent-company:set-heartbeat", diff --git a/src/shared/contracts/desktop-api/companies.ts b/src/shared/contracts/desktop-api/companies.ts index b47a9282..0780c3fb 100644 --- a/src/shared/contracts/desktop-api/companies.ts +++ b/src/shared/contracts/desktop-api/companies.ts @@ -1,7 +1,7 @@ import type { z } from "zod"; import type { CompanyMetrics, OnboardingBootstrapResult, ProfileSnapshot, StandupReport, InboxItem } from "../../types"; import type { bootstrapOnboardingSchema, companyInputSchema, connectorDisabledModelsSchema, connectorInputSchema, settingsSchema } from "../schemas-company"; -import type { metricsQuerySchema, SidebarBadges } from "../schemas-badges"; +import type { metricsQuerySchema } from "../schemas-badges"; import type { DesktopResult } from "./common"; export type CompaniesApi = { @@ -15,7 +15,6 @@ export type CompaniesApi = { updateSettings: (input: z.infer) => Promise>; checkForUpdates: () => Promise>; deleteCompany: (companyId: string) => Promise>; - getSidebarBadges: (companyId: string) => Promise>; listConnectorModels: (connectorId: string) => Promise>>; getConnectorDisabledModels: (connectorId: string) => Promise>>; setConnectorDisabledModels: (input: z.infer) => Promise>; diff --git a/src/shared/contracts/schemas-badges.ts b/src/shared/contracts/schemas-badges.ts index 71815fb0..f7858f0a 100644 --- a/src/shared/contracts/schemas-badges.ts +++ b/src/shared/contracts/schemas-badges.ts @@ -5,16 +5,6 @@ export const deleteEntitySchema = z.object({ companyId: z.string().uuid(), }); -export const sidebarBadgesSchema = z.object({ - pendingApprovals: z.number(), - activeRuns: z.number(), - failedRuns: z.number(), - todoTasks: z.number(), - budgetWarnings: z.number(), - unreadMessages: z.number(), -}); -export type SidebarBadges = z.infer; - export const entityGetterSchema = z.object({ companyId: z.string().uuid(), id: z.string().uuid(), @@ -25,3 +15,9 @@ export const metricsQuerySchema = z.object({ agentId: z.string().uuid().optional(), days: z.number().int().min(1).max(90).default(30), }); + +// `SidebarBadges` and its Zod schema used to live here as an IPC +// response type. Sidebar badges are now derived in the renderer from +// `ProfileSnapshot` (see `src/shared/sidebar-badges.ts`), so no wire +// contract is needed — `deriveSidebarBadges(snapshot)` is the only +// path that produces a `SidebarBadges` value. diff --git a/src/shared/sidebar-badges.ts b/src/shared/sidebar-badges.ts new file mode 100644 index 00000000..74258fe0 --- /dev/null +++ b/src/shared/sidebar-badges.ts @@ -0,0 +1,102 @@ +import type { ProfileSnapshot } from "./types"; + +/** + * Sidebar badge counters, derived purely from `ProfileSnapshot`. + * + * This is a local-first desktop app: every row the badges would want + * to count is already in memory via `ProfileSnapshot` (which is scoped + * to the current company and refreshed as an atomic unit). Running a + * second server-side aggregation — as the old `getSidebarBadges` IPC + * did — created two parallel data paths for the same answer, and the + * two drifted whenever one side's data was mutated between refreshes + * (e.g. a message being marked read in Communication). The resulting + * "badge = 1, inbox = empty" mismatch was the visible symptom. + * + * Derivation eliminates the class. Same snapshot in, same badge out. + * No IPC, no state, no drift. + */ +export interface SidebarBadges { + pendingApprovals: number; + activeRuns: number; + failedRuns: number; + todoTasks: number; + budgetWarnings: number; + unreadMessages: number; +} + +const FAILED_RUN_STATUSES: ReadonlyArray = ["failed", "timed_out", "interrupted"]; +const ACTIVE_RUN_STATUSES: ReadonlyArray = ["queued", "running"]; +const OPEN_TASK_STATUSES: ReadonlyArray = ["todo", "in_progress", "in_review"]; +const BUDGET_WARNING_RATIO = 0.8; + +/** + * Derive the sidebar badge counts for the current snapshot. Call this + * from the renderer whenever the snapshot changes; it is cheap + * (O(n) linear filters over arrays that never exceed the snapshot's + * per-company caps). + * + * `snapshot.agentMessages[i].readAt` already reflects the board + * reader's read timestamp (the snapshot query joins + * `agent_message_reads` with `agent_id = BOARD_READER_ID` and projects + * that value into the `readAt` field — see + * `src/main/database/snapshot.ts`). So the unread filter here is just + * "no board read receipt" plus the same channel/priority scope the + * old server query used. + */ +export function deriveSidebarBadges(snapshot: ProfileSnapshot, companyId: string | null): SidebarBadges { + // The snapshot query is already company-scoped when `currentCompanyId` + // is non-null (see `listRendererSnapshot` → `listSnapshot`). The extra + // `companyId` filter here is defensive: on the rare path where the + // caller passes an un-scoped snapshot (e.g., cross-company testing), + // we still count the right company's rows rather than the union. + const matchesCompany = (row: T): boolean => + companyId === null || row.companyId === companyId; + + let pendingApprovals = 0; + for (const approval of snapshot.approvals) { + if (matchesCompany(approval) && approval.state === "pending") pendingApprovals += 1; + } + + let activeRuns = 0; + let failedRuns = 0; + for (const run of snapshot.runs) { + if (!matchesCompany(run)) continue; + if (ACTIVE_RUN_STATUSES.includes(run.status)) activeRuns += 1; + else if (FAILED_RUN_STATUSES.includes(run.status)) failedRuns += 1; + } + + let todoTasks = 0; + for (const task of snapshot.tasks) { + if (matchesCompany(task) && OPEN_TASK_STATUSES.includes(task.status)) todoTasks += 1; + } + + let budgetWarnings = 0; + for (const agent of snapshot.agents) { + if (!matchesCompany(agent)) continue; + if (agent.budgetMonthlyUsd > 0 && agent.spentMonthlyUsd / agent.budgetMonthlyUsd >= BUDGET_WARNING_RATIO) { + budgetWarnings += 1; + } + } + + let unreadMessages = 0; + for (const message of snapshot.agentMessages) { + if (!matchesCompany(message)) continue; + if (message.readAt) continue; + if ( + message.channel === "company" || + message.channel === "incident" || + message.priority === "urgent" + ) { + unreadMessages += 1; + } + } + + return { + pendingApprovals, + activeRuns, + failedRuns, + todoTasks, + budgetWarnings, + unreadMessages, + }; +} diff --git a/tests/unit/store.test.ts b/tests/unit/store.test.ts index f379e502..22394615 100644 --- a/tests/unit/store.test.ts +++ b/tests/unit/store.test.ts @@ -2,13 +2,14 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { useAppStore } from "@renderer/state/store"; import type { ProfileSnapshot, TaskRecord, RunRecord, AgentRecord, ApprovalRecord } from "@shared/types"; -// Mock window.agentCompany for refreshSnapshot and refreshBadges tests +// Mock window.agentCompany for refreshSnapshot tests. Sidebar badges +// no longer traverse IPC — they are derived from the snapshot in +// `@shared/sidebar-badges`, so there is nothing store-level to mock +// for them. const mockLoadSnapshot = vi.fn(); -const mockGetSidebarBadges = vi.fn(); vi.stubGlobal("window", { agentCompany: { loadSnapshot: mockLoadSnapshot, - getSidebarBadges: mockGetSidebarBadges, }, }); @@ -488,20 +489,6 @@ describe("CompanySlice patch methods", () => { expect(approvals[0]!.id).toBe("ap-2"); }); - // -- refreshBadges -- - it("refreshBadges calls window.agentCompany.getSidebarBadges and updates badges", async () => { - const mockBadges = { tasks: 5, approvals: 2, runs: 0, messages: 3 }; - mockGetSidebarBadges.mockResolvedValue({ ok: true, data: mockBadges }); - - // Seed snapshot with currentCompanyId -- refreshBadges reads from snapshot - useAppStore.getState().setSnapshot(makeSnapshot()); - - await useAppStore.getState().refreshBadges(); - - expect(mockGetSidebarBadges).toHaveBeenCalledWith("company-1"); - expect(useAppStore.getState().badges).toEqual(mockBadges); - }); - // -- null-snapshot safety -- it("patch methods are no-ops when snapshot is null", () => { expect(useAppStore.getState().snapshot).toBeNull();