Skip to content

Commit fcc161f

Browse files
committed
fix(webapp,dashboard-agent): settle the work count on the chat on screen, and let an admin preview's watches alert
The launcher's work count is now taken with the chat the panel has on screen left out — in the panel's own list and in the poll's server-side count alike — instead of subtracting one afterwards, which under-counted whenever that chat held nothing unseen and left a stale count on the closing edge. The alert gate stopped deciding the admin preview differently from the agent's own gate: `canAccessDashboardAgent` reads `admin` off the user row when the caller has no session, so a watch an admin could create can still alert. A `run_start` watch now offers the same Customize variants as the rest of its family.
1 parent b8910fe commit fcc161f

13 files changed

Lines changed: 194 additions & 134 deletions

apps/webapp/app/components/dashboard-agent/DashboardAgent.tsx

Lines changed: 13 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import {
2222
writeAgentFullscreen,
2323
} from "./panel-layout";
2424
import { nextPendingTurnChatId } from "./pending-turn";
25-
import { nextVisibleChat, unreadWorkForDot, unreadWorkOnPanelClose } from "./unread-counts";
25+
import { nextVisibleChat } from "./unread-counts";
2626
import { startWakePolling, wakesToToast } from "./wake-poll";
2727
import { shouldPollWakeFeed, subscribeWatchActivity } from "./watch-activity";
2828
import {
@@ -95,14 +95,9 @@ export function DashboardAgent({
9595
// Same as the read.
9696
}
9797
}, []);
98-
// A wake in the on-screen chat toasts but must not light the dot.
98+
// A wake in the on-screen chat toasts but must not light the dot. Read by the poll callback,
99+
// which outlives the render that started it, so it has to be a ref.
99100
const visibleChat = useRef<string | null>(null);
100-
// Read by the poll callback, which outlives the render that started it: `open` in its closure
101-
// is whatever it was when polling began, and opening the panel does not restart the poll.
102-
const panelOpen = useRef(open);
103-
useEffect(() => {
104-
panelOpen.current = open;
105-
}, [open]);
106101

107102
// Switching environment re-runs the layout loader but does not remount it, so the seeds
108103
// above would keep the old environment's counts.
@@ -146,22 +141,10 @@ export function DashboardAgent({
146141
undefined
147142
);
148143

149-
// The panel's own count, off the chat list it has already marked read. Kept so the closing
150-
// edge can settle the dot instead of waiting a poll for the open-chat subtraction to lift.
151-
const panelWorkCount = useRef<number | null>(null);
152-
const handleUnreadWorkChange = useCallback((count: number) => {
153-
panelWorkCount.current = count;
154-
setUnreadWork(count);
155-
}, []);
156-
157144
const setPanelOpen = useCallback((next: boolean) => {
158145
setOpen(next);
159146
// Pending requests must be dropped or a stale one re-applies on the next open.
160147
if (!next) {
161-
setUnreadWork((shown) =>
162-
unreadWorkOnPanelClose({ shown, panelCount: panelWorkCount.current })
163-
);
164-
panelWorkCount.current = null;
165148
visibleChat.current = null;
166149
setFullscreen(false);
167150
writeAgentFullscreen(false);
@@ -216,10 +199,14 @@ export function DashboardAgent({
216199
let cancelled = false;
217200
const load = async () => {
218201
try {
202+
// The chat on screen is being read, so the server leaves it out of the work count
203+
// rather than the client subtracting it back off afterwards.
204+
const onScreen = visibleChat.current;
219205
// Bounded, so one stuck request can't hold the poll's in-flight guard.
220-
const res = await fetch(`${actionPath}?unread=1`, {
221-
signal: AbortSignal.timeout(UNREAD_REQUEST_TIMEOUT_MS),
222-
});
206+
const res = await fetch(
207+
`${actionPath}?unread=1${onScreen ? `&chatId=${encodeURIComponent(onScreen)}` : ""}`,
208+
{ signal: AbortSignal.timeout(UNREAD_REQUEST_TIMEOUT_MS) }
209+
);
223210
if (!res.ok) return;
224211
const data = (await res.json()) as {
225212
unreadWakes?: number;
@@ -232,13 +219,7 @@ export function DashboardAgent({
232219
(wake) => wake.unread && wake.chatId === visibleChat.current
233220
).length;
234221
setUnreadWakes(Math.max(0, (data.unreadWakes ?? 0) - unreadInView));
235-
setUnreadWork(
236-
unreadWorkForDot({
237-
reported: data.unreadWork,
238-
panelOpen: panelOpen.current,
239-
visibleChatId: visibleChat.current,
240-
})
241-
);
222+
setUnreadWork(Math.max(0, data.unreadWork ?? 0));
242223

243224
const fresh = wakesToToast(data.wakes, toastedWakes.current);
244225
for (const wake of fresh) rememberToasted(wake.watchId);
@@ -357,7 +338,8 @@ export function DashboardAgent({
357338
newChatSeq={newChatSeq}
358339
promotedPrompt={promotedPrompt}
359340
onChatRead={markChatRead}
360-
onUnreadWorkChange={handleUnreadWorkChange}
341+
// The panel's own count, off the chat list it has already marked read.
342+
onUnreadWorkChange={setUnreadWork}
361343
onTurnActivityChange={handleTurnActivityChange}
362344
isFullscreen={fullscreen}
363345
onToggleFullscreen={toggleFullscreen}

apps/webapp/app/components/dashboard-agent/DashboardAgentPanel.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -334,8 +334,8 @@ export function DashboardAgentPanel({
334334
// The one source for the dot's work count: nudging it per open double-subtracts.
335335
useEffect(() => {
336336
if (!chatsLoaded) return;
337-
onUnreadWorkChange?.(unreadWorkCount(chats));
338-
}, [chats, chatsLoaded, onUnreadWorkChange]);
337+
onUnreadWorkChange?.(unreadWorkCount(chats, active?.chatId));
338+
}, [chats, chatsLoaded, active?.chatId, onUnreadWorkChange]);
339339

340340
// Bound to its chat, which remounts with a fresh guard ref on every switch.
341341
const [sendRequest, setSendRequest] = useState<

apps/webapp/app/components/dashboard-agent/unread-counts.test.ts

Lines changed: 19 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,6 @@
11
import { readFileSync } from "node:fs";
22
import { describe, expect, it } from "vitest";
3-
import {
4-
markChatListRead,
5-
nextVisibleChat,
6-
unreadWorkCount,
7-
unreadWorkForDot,
8-
unreadWorkOnPanelClose,
9-
} from "./unread-counts";
3+
import { markChatListRead, nextVisibleChat, unreadWorkCount } from "./unread-counts";
104

115
const list = () => [
126
{ id: "chat_a", hasUnreadWake: true, hasUnreadWork: true },
@@ -59,45 +53,21 @@ describe("nextVisibleChat", () => {
5953
});
6054

6155
/**
62-
* The chat on screen is being read, so it isn't work waiting for anyone — but only while the
63-
* panel is actually open.
56+
* A turn landing in the chat on screen must not light the dot — and the chat is left out of the
57+
* count rather than subtracted off it afterwards, so a chat holding nothing is never subtracted.
6458
*/
65-
describe("unreadWorkForDot", () => {
66-
it("subtracts the chat the panel is showing", () => {
67-
expect(unreadWorkForDot({ reported: 3, panelOpen: true, visibleChatId: "chat_a" })).toBe(2);
59+
describe("the chat on screen", () => {
60+
it("is left out, however much work lands in it", () => {
61+
expect(unreadWorkCount(list(), "chat_a")).toBe(1);
6862
});
6963

70-
it("counts every chat when the panel is closed", () => {
71-
expect(unreadWorkForDot({ reported: 3, panelOpen: false, visibleChatId: "chat_a" })).toBe(3);
72-
expect(unreadWorkForDot({ reported: 3, panelOpen: true, visibleChatId: null })).toBe(3);
64+
it("takes nothing off the count when it holds no unseen work", () => {
65+
expect(unreadWorkCount(list(), "chat_c")).toBe(2);
7366
});
7467

75-
it("never reports a negative count, or one the poll didn't give", () => {
76-
expect(unreadWorkForDot({ reported: 0, panelOpen: true, visibleChatId: "chat_a" })).toBe(0);
77-
expect(unreadWorkForDot({ reported: undefined, panelOpen: false, visibleChatId: null })).toBe(
78-
0
79-
);
80-
});
81-
});
82-
83-
/**
84-
* Closing the panel stops the chat on screen being read, so the one the open panel took off the
85-
* count comes straight back rather than a poll later.
86-
*/
87-
describe("unreadWorkOnPanelClose", () => {
88-
it("settles the dot on the panel's own count", () => {
89-
const open = unreadWorkForDot({ reported: 1, panelOpen: true, visibleChatId: "chat_a" });
90-
expect(open).toBe(0);
91-
// Work landed in another chat while chat_a was on screen: the panel's list still counts it.
92-
expect(unreadWorkOnPanelClose({ shown: open, panelCount: 1 })).toBe(1);
93-
});
94-
95-
it("leaves the shown count alone when the panel closed before its list loaded", () => {
96-
expect(unreadWorkOnPanelClose({ shown: 2, panelCount: null })).toBe(2);
97-
});
98-
99-
it("takes the panel's zero too, so reading the last chat darkens the dot at once", () => {
100-
expect(unreadWorkOnPanelClose({ shown: 1, panelCount: 0 })).toBe(0);
68+
it("counts every chat when there is none on screen", () => {
69+
expect(unreadWorkCount(list(), null)).toBe(2);
70+
expect(unreadWorkCount(list(), undefined)).toBe(2);
10171
});
10272
});
10373

@@ -106,7 +76,7 @@ describe("what the panel and the layout actually do with it", () => {
10676
const layout = readFileSync(new URL("./DashboardAgent.tsx", import.meta.url), "utf8");
10777

10878
it("reports the count from the list, and only from the list", () => {
109-
expect(panel).toContain("onUnreadWorkChange?.(unreadWorkCount(chats));");
79+
expect(panel).toContain("onUnreadWorkChange?.(unreadWorkCount(chats, active?.chatId));");
11080
expect(panel).not.toContain("settled.filter((chat) => chat.hasUnreadWork).length");
11181
expect(layout).not.toContain("setUnreadWork((count) => Math.max(0, count - 1))");
11282
});
@@ -118,22 +88,14 @@ describe("what the panel and the layout actually do with it", () => {
11888
});
11989

12090
/**
121-
* The poll runs for as long as this tab is watching, so anything it reads about the panel has
122-
* to come from a ref. `open` is state: the callback would keep the value it had when polling
123-
* started, which is `false`, and the dot would go on counting the chat on screen.
91+
* Structural: there is no DOM here to open a panel in. The poll runs for as long as this tab
92+
* is watching, so the chat on screen has to be read from a ref at request time — `open` in the
93+
* callback's closure is whatever it was when polling started.
12494
*/
125-
it("reads the panel's state at poll time, not from the closure", () => {
126-
expect(layout).toContain("panelOpen: panelOpen.current,");
127-
expect(layout).not.toContain("open && visibleChat.current");
128-
});
129-
130-
/** Structural: there is no DOM here to close a real panel in. */
131-
it("settles the count on the closing edge instead of waiting for the poll", () => {
132-
expect(panel).toContain("onUnreadWorkChange?.(unreadWorkCount(chats));");
133-
expect(layout).toContain("panelWorkCount.current = count;");
134-
expect(layout).toContain(
135-
"unreadWorkOnPanelClose({ shown, panelCount: panelWorkCount.current })"
136-
);
95+
it("names the chat on screen to the poll instead of correcting the count it gets back", () => {
96+
expect(layout).toContain("const onScreen = visibleChat.current;");
97+
expect(layout).toContain("setUnreadWork(Math.max(0, data.unreadWork ?? 0));");
98+
expect(layout).not.toContain("panelOpen.current");
13799
});
138100

139101
it("re-seeds both counts when the environment changes under the layout", () => {

apps/webapp/app/components/dashboard-agent/unread-counts.ts

Lines changed: 7 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -17,44 +17,18 @@ export function nextVisibleChat(chatId: string, options: { leaving: boolean }):
1717
return options.leaving ? null : chatId;
1818
}
1919

20-
/**
21-
* The work count the launcher's dot shows, given what the poll just reported. A chat open in
22-
* the panel is being read right now, so it is not work anyone is waiting on.
23-
*
24-
* Both inputs have to be read at poll time rather than captured when the poll started: the
25-
* panel opens and closes without restarting it.
26-
*/
27-
export function unreadWorkForDot(params: {
28-
reported: number | undefined;
29-
panelOpen: boolean;
30-
visibleChatId: string | null;
31-
}): number {
32-
const onScreen = params.panelOpen && params.visibleChatId !== null ? 1 : 0;
33-
return Math.max(0, (params.reported ?? 0) - onScreen);
34-
}
35-
36-
/**
37-
* The count the dot settles on the moment the panel closes. While the panel is open the poll
38-
* subtracts the chat on screen, and it only corrects itself a tick later — up to a minute of a
39-
* dark dot over work nobody has seen. The panel's own count is taken off the chat list it has
40-
* already marked read, so it settles the closing edge without waiting. `null` is a panel that
41-
* closed before its list loaded, which leaves the shown count alone.
42-
*/
43-
export function unreadWorkOnPanelClose(params: {
44-
shown: number;
45-
panelCount: number | null;
46-
}): number {
47-
return params.panelCount ?? params.shown;
48-
}
49-
5020
/** Opening a chat settles everything unseen in it, not just the wake. */
5121
export function markChatListRead<T extends UnreadChat>(chats: T[], chatId: string): T[] {
5222
return chats.map((chat) =>
5323
chat.id === chatId ? { ...chat, hasUnreadWake: false, hasUnreadWork: false } : chat
5424
);
5525
}
5626

57-
/** How many chats still hold work their owner hasn't seen. */
58-
export function unreadWorkCount(chats: UnreadChat[]): number {
59-
return chats.filter((chat) => chat.hasUnreadWork).length;
27+
/**
28+
* How many chats still hold work their owner hasn't seen. The chat on screen is being read
29+
* right now, so a turn landing in it is not work anyone is waiting on — every count of this,
30+
* here and on the server, leaves it out, so none of them has to be corrected afterwards.
31+
*/
32+
export function unreadWorkCount(chats: UnreadChat[], visibleChatId?: string | null): number {
33+
return chats.filter((chat) => chat.hasUnreadWork && chat.id !== visibleChatId).length;
6034
}

apps/webapp/app/components/dashboard-agent/watch-card.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,8 @@ describe("cadence limits", () => {
111111
});
112112

113113
describe("condition variants (§3)", () => {
114-
it("offers the run pair and the whole queue family", () => {
115-
expect(variantsOf(runDraft())).toEqual(["run_finished", "run_failed"]);
114+
it("offers the whole run family and the whole queue family", () => {
115+
expect(variantsOf(runDraft())).toEqual(["run_start", "run_finished", "run_failed"]);
116116
expect(variantsOf(queueDraft())).toEqual([
117117
"backlog_drain",
118118
"queue_depth_above",

apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,8 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
139139
countChatsWithUnreadWork(dashboardAgentDb, {
140140
organizationId: scoped.organizationId,
141141
userId,
142+
// The chat the panel has on screen, if any: it is being read as this is counted.
143+
excludeChatId: searchParams.get("chatId") ?? undefined,
142144
}),
143145
]);
144146

apps/webapp/app/services/dashboardAgentWatchAlerts.server.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,12 @@ export async function canUseDashboardAgentAlerts(params: {
8686
userId: string;
8787
organizationSlug: string;
8888
organizationId: string;
89-
isAdmin?: boolean;
9089
orgFeatureFlags?: Record<string, unknown> | null;
9190
}): Promise<DashboardAgentAlertGate> {
9291
const hasAgent = await canAccessDashboardAgent({
92+
// `isAdmin` is left out on purpose: there is no session here, so the gate reads it off
93+
// the user row and a watch an admin could create can still alert.
9394
userId: params.userId,
94-
isAdmin: params.isAdmin ?? false,
9595
// Never an impersonated session: this runs in the background.
9696
isImpersonating: false,
9797
organizationSlug: params.organizationSlug,

apps/webapp/app/v3/canAccessDashboardAgent.server.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,21 @@ import { makeFlag } from "~/v3/featureFlags.server";
1010
*/
1111
export async function canAccessDashboardAgent(options: {
1212
userId: string;
13-
isAdmin: boolean;
13+
// Omitted by a caller with no session (a background job, a token-authenticated route),
14+
// which is read off the user row instead so both answer the preview the same way.
15+
isAdmin?: boolean;
1416
isImpersonating: boolean;
1517
organizationSlug: string;
1618
// The org's already-loaded `featureFlags`. Omitted means we query the org ourselves.
1719
orgFeatureFlags?: Record<string, unknown> | null;
1820
}): Promise<boolean> {
1921
const { userId, isAdmin, isImpersonating, organizationSlug, orgFeatureFlags } = options;
2022

21-
if ((isAdmin || isImpersonating) && env.DASHBOARD_AGENT_ADMIN_PREVIEW === "1") {
22-
return true;
23+
if (env.DASHBOARD_AGENT_ADMIN_PREVIEW === "1") {
24+
const admin =
25+
isAdmin ??
26+
(await prisma.user.findFirst({ where: { id: userId }, select: { admin: true } }))?.admin;
27+
if (admin || isImpersonating) return true;
2328
}
2429

2530
let overrides = orgFeatureFlags;
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/**
2+
* A watch can only exist because its owner could use the agent, so the alert gate has to
3+
* decide access the same way the agent's own gate does. There is no session behind these
4+
* calls — the delivery job, and the agent's own token-authenticated routes — so the admin
5+
* preview the agent honours has to be read off the user row.
6+
*/
7+
8+
import { beforeEach, describe, expect, test, vi } from "vitest";
9+
10+
const ctx = vi.hoisted(() => ({ admin: false }));
11+
12+
vi.mock("~/db.server", () => {
13+
const db = {
14+
user: { findFirst: async () => ({ admin: ctx.admin }) },
15+
organization: { findFirst: async () => ({ featureFlags: {} }) },
16+
featureFlag: { findFirst: async () => null },
17+
};
18+
return { prisma: db, $replica: db, sqlDatabaseSchema: undefined };
19+
});
20+
21+
process.env.SESSION_SECRET = "test-session-secret-for-alert-admin-preview";
22+
// The install this is previewed on: the flag is off for everyone else.
23+
process.env.DASHBOARD_AGENT_ADMIN_PREVIEW = "1";
24+
delete process.env.DASHBOARD_AGENT_ENABLED;
25+
26+
const { canUseDashboardAgentAlerts } = await import("~/services/dashboardAgentWatchAlerts.server");
27+
28+
const params = { userId: "user_1", organizationId: "org_1", organizationSlug: "acme" };
29+
30+
beforeEach(() => {
31+
ctx.admin = false;
32+
});
33+
34+
describe("watch alerts during the admin preview", () => {
35+
test("let an admin's watch alert, exactly as the agent lets them create it", async () => {
36+
ctx.admin = true;
37+
expect(await canUseDashboardAgentAlerts(params)).toEqual({ allowed: true });
38+
});
39+
40+
test("stay shut for everyone the flag is still off for", async () => {
41+
expect(await canUseDashboardAgentAlerts(params)).toEqual({
42+
allowed: false,
43+
reason: "dashboard_agent_disabled",
44+
});
45+
});
46+
});

0 commit comments

Comments
 (0)