Skip to content

Commit 351899b

Browse files
committed
🤖 fix heartbeat modal config defaults
Use global heartbeat defaults in the workspace modal when no workspace override exists.
1 parent 215128a commit 351899b

3 files changed

Lines changed: 150 additions & 16 deletions

File tree

src/browser/components/WorkspaceHeartbeatModal/WorkspaceHeartbeatModal.test.tsx

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import type { ReactNode } from "react";
44
import { afterEach, beforeEach, describe, expect, mock, spyOn, test } from "bun:test";
55
import { cleanup, fireEvent, render, waitFor } from "@testing-library/react";
66
import { installDom } from "../../../../tests/ui/dom";
7+
import * as APIModule from "@/browser/contexts/API";
8+
import type { APIClient, UseAPIResult } from "@/browser/contexts/API";
79
import * as WorkspaceHeartbeatHookModule from "@/browser/hooks/useWorkspaceHeartbeat";
810
import type { HeartbeatFormSettings } from "@/browser/hooks/useWorkspaceHeartbeat";
911
import {
@@ -33,6 +35,27 @@ let saveResult = true;
3335
let hookError: string | null = null;
3436
let hookIsLoading = false;
3537
let hookIsSaving = false;
38+
let useWorkspaceHeartbeatSpy: ReturnType<
39+
typeof spyOn<typeof WorkspaceHeartbeatHookModule, "useWorkspaceHeartbeat">
40+
>;
41+
42+
type ConnectedUseAPIResult = Extract<UseAPIResult, { status: "connected" }>;
43+
interface WorkspaceHeartbeatTestAPI {
44+
workspace: {
45+
heartbeat: {
46+
get: (input: { workspaceId: string }) => Promise<HeartbeatFormSettings | null>;
47+
set: (
48+
_input: unknown
49+
) => Promise<{ success: true; data: void } | { success: false; error: string }>;
50+
};
51+
};
52+
config: {
53+
getConfig: () => Promise<{
54+
heartbeatDefaultIntervalMs?: number;
55+
heartbeatDefaultPrompt?: string;
56+
}>;
57+
};
58+
}
3659

3760
function createHeartbeatSettings(
3861
overrides: Partial<HeartbeatFormSettings> = {}
@@ -45,6 +68,16 @@ function createHeartbeatSettings(
4568
};
4669
}
4770

71+
function createConnectedUseAPIResult(api: WorkspaceHeartbeatTestAPI): ConnectedUseAPIResult {
72+
return {
73+
api: api as APIClient,
74+
status: "connected",
75+
error: null,
76+
authenticate: () => undefined,
77+
retry: () => undefined,
78+
};
79+
}
80+
4881
const LONG_HEARTBEAT_MESSAGE = "Review pending work and summarize next steps. ".repeat(30).trim();
4982

5083
describe("WorkspaceHeartbeatModal", () => {
@@ -57,7 +90,10 @@ describe("WorkspaceHeartbeatModal", () => {
5790
hookIsLoading = false;
5891
hookIsSaving = false;
5992

60-
spyOn(WorkspaceHeartbeatHookModule, "useWorkspaceHeartbeat").mockImplementation((params) => {
93+
useWorkspaceHeartbeatSpy = spyOn(
94+
WorkspaceHeartbeatHookModule,
95+
"useWorkspaceHeartbeat"
96+
).mockImplementation((params) => {
6197
const workspaceId = params.workspaceId;
6298
return {
6399
settings:
@@ -136,6 +172,59 @@ describe("WorkspaceHeartbeatModal", () => {
136172
expect(onOpenChange).toHaveBeenCalledWith(false);
137173
});
138174

175+
test("loads global heartbeat defaults when the workspace has no saved heartbeat config", async () => {
176+
useWorkspaceHeartbeatSpy.mockRestore();
177+
178+
const globalIntervalMs = 6 * 60_000;
179+
const globalPrompt = "test";
180+
const workspaceHeartbeatGetMock = mock(() => Promise.resolve(null));
181+
const workspaceHeartbeatSetMock = mock(() =>
182+
Promise.resolve({ success: true as const, data: undefined })
183+
);
184+
const getConfigMock = mock(() =>
185+
Promise.resolve({
186+
heartbeatDefaultIntervalMs: globalIntervalMs,
187+
heartbeatDefaultPrompt: globalPrompt,
188+
})
189+
);
190+
const mockApi: WorkspaceHeartbeatTestAPI = {
191+
workspace: {
192+
heartbeat: {
193+
get: workspaceHeartbeatGetMock,
194+
set: workspaceHeartbeatSetMock,
195+
},
196+
},
197+
config: {
198+
getConfig: getConfigMock,
199+
},
200+
};
201+
spyOn(APIModule, "useAPI").mockImplementation(() => createConnectedUseAPIResult(mockApi));
202+
203+
const view = render(
204+
<WorkspaceHeartbeatModal
205+
workspaceId="ws-1"
206+
open={true}
207+
onOpenChange={mock((_open: boolean) => undefined)}
208+
/>
209+
);
210+
211+
const intervalField = (await waitFor(() =>
212+
view.getByLabelText("Heartbeat interval in minutes")
213+
)) as HTMLInputElement;
214+
expect(intervalField.value).toBe("6");
215+
expect(workspaceHeartbeatGetMock).toHaveBeenCalledWith({ workspaceId: "ws-1" });
216+
expect(getConfigMock).toHaveBeenCalled();
217+
218+
fireEvent.click(view.getByRole("switch", { name: "Enable workspace heartbeats" }));
219+
220+
const messageField = (await waitFor(() =>
221+
view.getByLabelText("Heartbeat message")
222+
)) as HTMLTextAreaElement;
223+
// Global prompt is not seeded into the form to avoid persisting it as a workspace
224+
// override on save. The backend handles prompt fallback at execution time.
225+
expect(messageField.value).toBe("");
226+
});
227+
139228
test("saves the selected heartbeat context mode and updates helper copy", async () => {
140229
settingsByWorkspaceId.set(
141230
"ws-1",

src/browser/features/Settings/Sections/HeartbeatSection.tsx

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,29 @@ export function HeartbeatSection() {
154154
return;
155155
}
156156

157+
if (!heartbeatDefaultIntervalLoadedOk && !heartbeatDefaultIntervalEditedSinceLoadRef.current) {
158+
return;
159+
}
160+
157161
const parsedMinutes = parseIntervalMinutes(draftIntervalMinutes);
162+
163+
// Empty or invalid input: clear the global override so the hardcoded default applies.
158164
if (parsedMinutes == null) {
165+
setDraftIntervalMinutes("");
166+
167+
heartbeatDefaultIntervalUpdateChainRef.current =
168+
heartbeatDefaultIntervalUpdateChainRef.current
169+
.catch(() => {
170+
/* Best-effort. */
171+
})
172+
.then(() => api.config.updateHeartbeatDefaultIntervalMs({ intervalMs: null }))
173+
.then(() => {
174+
setHeartbeatDefaultIntervalLoadedOk(true);
175+
heartbeatDefaultIntervalEditedSinceLoadRef.current = false;
176+
})
177+
.catch(() => {
178+
/* Best-effort. */
179+
});
159180
return;
160181
}
161182

@@ -165,10 +186,6 @@ export function HeartbeatSection() {
165186
setDraftIntervalMinutes(clampedMinutesValue);
166187
}
167188

168-
if (!heartbeatDefaultIntervalLoadedOk && !heartbeatDefaultIntervalEditedSinceLoadRef.current) {
169-
return;
170-
}
171-
172189
heartbeatDefaultIntervalUpdateChainRef.current = heartbeatDefaultIntervalUpdateChainRef.current
173190
.catch(() => {
174191
// Best-effort only.

src/browser/hooks/useWorkspaceHeartbeat.ts

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ import {
88

99
type WorkspaceHeartbeatSettings = NonNullable<FrontendWorkspaceMetadata["heartbeat"]>;
1010

11+
interface HeartbeatGlobalDefaults {
12+
intervalMs: number;
13+
}
14+
1115
export type HeartbeatFormSettings = WorkspaceHeartbeatSettings;
1216

1317
interface UseWorkspaceHeartbeatParams {
@@ -22,25 +26,41 @@ export interface UseWorkspaceHeartbeatResult {
2226
save: (next: HeartbeatFormSettings) => Promise<boolean>;
2327
}
2428

25-
function getDefaultHeartbeatSettings(): HeartbeatFormSettings {
29+
function normalizeHeartbeatDefaultMessage(message?: string): string | undefined {
30+
const trimmedMessage = message?.trim();
31+
return trimmedMessage ?? undefined;
32+
}
33+
34+
function getDefaultHeartbeatSettings(
35+
globalDefaults?: HeartbeatGlobalDefaults
36+
): HeartbeatFormSettings {
37+
// Only seed the interval from global defaults. The message is intentionally left
38+
// empty so saving without editing does not persist the global prompt as a
39+
// workspace-level override (the backend handles prompt fallback at execution time).
2640
return {
2741
enabled: false,
28-
intervalMs: HEARTBEAT_DEFAULT_INTERVAL_MS,
42+
intervalMs: globalDefaults?.intervalMs ?? HEARTBEAT_DEFAULT_INTERVAL_MS,
2943
contextMode: HEARTBEAT_DEFAULT_CONTEXT_MODE,
3044
};
3145
}
3246

3347
function normalizeHeartbeatSettings(
34-
heartbeat: WorkspaceHeartbeatSettings | null
48+
heartbeat: WorkspaceHeartbeatSettings | null,
49+
globalDefaults?: HeartbeatGlobalDefaults
3550
): HeartbeatFormSettings {
3651
if (!heartbeat) {
37-
return getDefaultHeartbeatSettings();
52+
return getDefaultHeartbeatSettings(globalDefaults);
3853
}
3954

40-
const trimmedMessage = heartbeat.message?.trim();
55+
const message = normalizeHeartbeatDefaultMessage(heartbeat.message);
4156
const contextMode = heartbeat.contextMode ?? HEARTBEAT_DEFAULT_CONTEXT_MODE;
42-
return trimmedMessage
43-
? { ...heartbeat, message: trimmedMessage, contextMode }
57+
return message
58+
? {
59+
enabled: heartbeat.enabled,
60+
intervalMs: heartbeat.intervalMs,
61+
contextMode,
62+
message,
63+
}
4464
: {
4565
enabled: heartbeat.enabled,
4666
intervalMs: heartbeat.intervalMs,
@@ -89,13 +109,21 @@ export function useWorkspaceHeartbeat(
89109
}
90110

91111
let cancelled = false;
92-
void api.workspace.heartbeat
93-
.get({ workspaceId })
94-
.then((result) => {
112+
void Promise.all([
113+
api.workspace.heartbeat.get({ workspaceId }),
114+
api.config.getConfig().catch(() => null),
115+
])
116+
.then(([heartbeat, config]) => {
95117
if (cancelled) return;
96118
if (currentWorkspaceIdRef.current !== workspaceId) return;
97119

98-
setSettings(normalizeHeartbeatSettings(result));
120+
const globalDefaults = config
121+
? {
122+
intervalMs: config.heartbeatDefaultIntervalMs ?? HEARTBEAT_DEFAULT_INTERVAL_MS,
123+
}
124+
: undefined;
125+
126+
setSettings(normalizeHeartbeatSettings(heartbeat, globalDefaults));
99127
setError(null);
100128
setIsLoading(false);
101129
})

0 commit comments

Comments
 (0)