feat(FN-3003): merge fusion/fn-3003
- Unify notification service ownership across the engine: `Notifier` now operates on `ProjectEngine` rather than its own `Core` reference, resolving stale-closure issues during engine lifecycle - Add regression tests in `notifier.test.ts` covering merge dedupe wiring and ownership initialization - Expand `project-engine.test.ts` to cover ownership state transitions and `ProjectEngine`-notifier integration - Fix TypeScript regressions in `ModelOnboardingModal.tsx` and `SettingsModal.tsx` introduced by custom provider types - Document the notification ownership model in `docs/architecture.md` Commits merged: - feat(FN-3003): complete Step 4 — document notification ownership model - fix(FN-3003): resolve dashboard custom provider typecheck regressions - test(FN-3003): complete Step 2 — add merge dedupe wiring regressions - feat(FN-3003): complete Step 1 — unify notification service ownership Files changed: docs/architecture.md | 2 + .../app/components/ModelOnboardingModal.tsx | 13 +++++- .../dashboard/app/components/SettingsModal.tsx | 17 +++++-- packages/engine/src/__tests__/notifier.test.ts | 33 ++++++++++++++ .../engine/src/__tests__/project-engine.test.ts | 53 +++++++++++++++++++--- packages/engine/src/notifier.ts | 3 +- packages/engine/src/project-engine.ts | 12 +++-- 7 files changed, 116 insertions(+), 17 deletions(-) Fusion-Task-Id: FN-3003
This commit is contained in:
@@ -2,7 +2,7 @@ import "./ModelOnboardingModal.css";
|
||||
import { useState, useEffect, useCallback, useRef } from "react";
|
||||
import { X, Loader2, CheckCircle, Key, Zap, GitPullRequest, Rocket, Plus, ChevronRight } from "lucide-react";
|
||||
import { getErrorMessage, type Task } from "@fusion/core";
|
||||
import type { AuthProvider, ModelInfo, CustomProviderConfig } from "../api";
|
||||
import type { AuthProvider, ModelInfo, CustomProvider, CustomProviderConfig } from "../api";
|
||||
import {
|
||||
fetchAuthStatus,
|
||||
fetchGlobalSettings,
|
||||
@@ -25,6 +25,15 @@ import { LoginInstructions } from "./LoginInstructions";
|
||||
import { CustomProviderForm } from "./CustomProviderForm";
|
||||
import { appendTokenQuery } from "../auth";
|
||||
|
||||
const mapLegacyCustomProviderToConfig = (provider: CustomProvider): CustomProviderConfig => ({
|
||||
id: provider.id,
|
||||
name: provider.name,
|
||||
baseUrl: provider.baseUrl,
|
||||
api: provider.apiType === "anthropic-compatible" ? "anthropic-messages" : "openai-responses",
|
||||
apiKey: provider.apiKey,
|
||||
models: provider.models?.map((model) => ({ id: model.id, name: model.name })) ?? [],
|
||||
});
|
||||
|
||||
/** Provider-specific API key setup metadata for onboarding form rendering */
|
||||
interface ApiKeyInfo {
|
||||
/** Label shown above the input field, e.g. "OpenAI API Key" */
|
||||
@@ -746,14 +755,7 @@ export function ModelOnboardingModal({
|
||||
const loadCustomProviders = useCallback(async () => {
|
||||
try {
|
||||
const data = await fetchCustomProviders();
|
||||
setCustomProviders((data.providers ?? []).map((provider) => ({
|
||||
id: provider.id,
|
||||
name: provider.name,
|
||||
baseUrl: provider.baseUrl,
|
||||
api: provider.apiType === "anthropic-compatible" ? "anthropic-messages" : "openai-completions",
|
||||
apiKey: provider.apiKey,
|
||||
models: (provider.models ?? []).map((model) => ({ id: model.id, name: model.name })),
|
||||
})));
|
||||
setCustomProviders((data.providers ?? []).map(mapLegacyCustomProviderToConfig));
|
||||
} catch {
|
||||
// best effort
|
||||
}
|
||||
|
||||
@@ -11,7 +11,7 @@ import {
|
||||
} from "@fusion/core";
|
||||
import type { Settings, GlobalSettings, ThemeMode, ColorTheme, ModelPreset, NtfyNotificationEvent, AgentPromptsConfig, ThinkingLevel } from "@fusion/core";
|
||||
import { fetchSettings, fetchSettingsByScope, updateSettings, updateGlobalSettings, fetchAuthStatus, loginProvider, logoutProvider, saveApiKey, clearApiKey, fetchModels, testNotification, fetchBackups, createBackup, exportSettings, importSettings, fetchMemoryFile, fetchMemoryFiles, saveMemoryFile, compactMemory, fetchGlobalConcurrency, updateGlobalConcurrency, installQmd, testMemoryRetrieval, triggerMemoryDreams, fetchGitRemotesDetailed, fetchDashboardHealth, checkForUpdates, fetchRemoteSettings, updateRemoteSettings, fetchRemoteStatus, installCloudflared, startRemoteTunnel, stopRemoteTunnel, regenerateRemotePersistentToken, generateShortLivedRemoteToken, fetchRemoteQr, fetchRemoteUrl, fetchCustomProviders, createCustomProvider, updateCustomProvider, deleteCustomProvider } from "../api";
|
||||
import type { AuthProvider, ModelInfo, BackupListResponse, SettingsExportData, MemoryFileInfo, MemoryRetrievalTestResult, GitRemoteDetailed, RemoteSettings, RemoteStatus, UpdateCheckResponse, CustomProviderConfig } from "../api";
|
||||
import type { AuthProvider, ModelInfo, BackupListResponse, SettingsExportData, MemoryFileInfo, MemoryRetrievalTestResult, GitRemoteDetailed, RemoteSettings, RemoteStatus, UpdateCheckResponse, CustomProvider, CustomProviderConfig } from "../api";
|
||||
import { useMemoryBackendStatus } from "../hooks/useMemoryBackendStatus";
|
||||
import { useOverlayDismiss } from "../hooks/useOverlayDismiss";
|
||||
import type { ToastType } from "../hooks/useToast";
|
||||
@@ -46,6 +46,15 @@ const GITHUB_STAR_CACHE_KEY = "fusion_github_star_count";
|
||||
const GITHUB_STAR_CACHE_TTL_MS = 60 * 60 * 1000; // 1 hour
|
||||
const GITHUB_STAR_CLICKED_KEY = "fusion:github-star-clicked";
|
||||
|
||||
const mapLegacyCustomProviderToConfig = (provider: CustomProvider): CustomProviderConfig => ({
|
||||
id: provider.id,
|
||||
name: provider.name,
|
||||
baseUrl: provider.baseUrl,
|
||||
api: provider.apiType === "anthropic-compatible" ? "anthropic-messages" : "openai-responses",
|
||||
apiKey: provider.apiKey,
|
||||
models: provider.models?.map((model) => ({ id: model.id, name: model.name })) ?? [],
|
||||
});
|
||||
|
||||
function getNodeStatusLabel(status: "online" | "offline" | "connecting" | "error"): string {
|
||||
if (status === "online") return "Online";
|
||||
if (status === "connecting") return "Connecting";
|
||||
@@ -786,14 +795,9 @@ export function SettingsModal({
|
||||
if (activeSection === "authentication") {
|
||||
setAuthLoading(true);
|
||||
loadAuthStatus().finally(() => setAuthLoading(false));
|
||||
void fetchCustomProviders().then((data) => setCustomProviders((data.providers ?? []).map((provider) => ({
|
||||
id: provider.id,
|
||||
name: provider.name,
|
||||
baseUrl: provider.baseUrl,
|
||||
api: provider.apiType === "anthropic-compatible" ? "anthropic-messages" : "openai-completions",
|
||||
apiKey: provider.apiKey,
|
||||
models: (provider.models ?? []).map((model) => ({ id: model.id, name: model.name })),
|
||||
})))).catch(() => undefined);
|
||||
void fetchCustomProviders()
|
||||
.then((data) => setCustomProviders((data.providers ?? []).map(mapLegacyCustomProviderToConfig)))
|
||||
.catch(() => undefined);
|
||||
}
|
||||
// Clean up polling when leaving auth section
|
||||
return () => {
|
||||
@@ -806,14 +810,7 @@ export function SettingsModal({
|
||||
|
||||
const loadCustomProviders = useCallback(async () => {
|
||||
const data = await fetchCustomProviders();
|
||||
setCustomProviders((data.providers ?? []).map((provider) => ({
|
||||
id: provider.id,
|
||||
name: provider.name,
|
||||
baseUrl: provider.baseUrl,
|
||||
api: provider.apiType === "anthropic-compatible" ? "anthropic-messages" : "openai-completions",
|
||||
apiKey: provider.apiKey,
|
||||
models: (provider.models ?? []).map((model) => ({ id: model.id, name: model.name })),
|
||||
})));
|
||||
setCustomProviders((data.providers ?? []).map(mapLegacyCustomProviderToConfig));
|
||||
}, []);
|
||||
|
||||
const handleSaveCustomProvider = useCallback(async (config: CustomProviderConfig) => {
|
||||
|
||||
@@ -586,7 +586,7 @@ describe("TaskCard", () => {
|
||||
expect(timer).not.toBeNull();
|
||||
// 8m workflow + 4m timed = 12m
|
||||
expect(timer?.textContent).toContain("12m");
|
||||
expect(timer?.getAttribute("title")).toContain("Execution time 12m");
|
||||
expect(timer?.getAttribute("title")).toContain("In progress 12m");
|
||||
});
|
||||
|
||||
it("updates the in-progress timer when timedExecutionMs changes", () => {
|
||||
|
||||
@@ -50,6 +50,7 @@ vi.mock("@fusion/core", () => {
|
||||
parseSingleAgentManifest: (...args: unknown[]) => mockParseSingleAgentManifest(...args),
|
||||
prepareAgentCompaniesImport: (...args: unknown[]) => mockPrepareAgentCompaniesImport(...args),
|
||||
AgentCompaniesParseError: MockAgentCompaniesParseError,
|
||||
DEFAULT_HEARTBEAT_PROCEDURE_PATH: ".fusion/agents/heartbeat-procedure.md",
|
||||
};
|
||||
});
|
||||
|
||||
|
||||
@@ -8,6 +8,7 @@ import {
|
||||
isNtfyEventEnabled,
|
||||
resolveNtfyEvents,
|
||||
} from "../notifier.js";
|
||||
import { NotificationService } from "../notification/notification-service.js";
|
||||
|
||||
// Mock the logger
|
||||
vi.mock("../logger.js", () => ({
|
||||
@@ -1053,6 +1054,38 @@ describe("NtfyNotifier", () => {
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("emits a single merged notification when notifier shares an already-started NotificationService", async () => {
|
||||
const sharedService = new NotificationService(store, { projectId: "proj-1" });
|
||||
await sharedService.start();
|
||||
|
||||
notifier = new NtfyNotifier(store, { projectId: "proj-1" }, sharedService);
|
||||
await notifier.start();
|
||||
|
||||
const task = createTask("FN-777", "Single Merge Notification");
|
||||
const mergeResult: MergeResult = {
|
||||
task,
|
||||
branch: "fusion/fn-777",
|
||||
merged: true,
|
||||
worktreeRemoved: true,
|
||||
branchDeleted: true,
|
||||
};
|
||||
|
||||
store.triggerTaskMerged(mergeResult);
|
||||
await flushAsyncWork();
|
||||
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
expect(fetchMock).toHaveBeenCalledWith(
|
||||
"https://ntfy.sh/test-topic",
|
||||
expect.objectContaining({
|
||||
headers: expect.objectContaining({
|
||||
Title: "Task FN-777 merged",
|
||||
}),
|
||||
}),
|
||||
);
|
||||
|
||||
await sharedService.stop();
|
||||
});
|
||||
|
||||
it("allows notifications for different tasks independently", async () => {
|
||||
notifier = new NtfyNotifier(store);
|
||||
await notifier.start();
|
||||
|
||||
@@ -2,6 +2,8 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { ProjectEngine } from "../project-engine.js";
|
||||
import { runtimeLog } from "../logger.js";
|
||||
import { TunnelProcessManager } from "../remote-access/tunnel-process-manager.js";
|
||||
import { NtfyNotifier } from "../notifier.js";
|
||||
import { NotificationService } from "../notification/index.js";
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
syncInsightExtractionAutomation: vi.fn(),
|
||||
@@ -16,6 +18,11 @@ const mocks = vi.hoisted(() => ({
|
||||
aiMergeTask: vi.fn(),
|
||||
execFile: vi.fn(),
|
||||
currentStore: null as Record<string, unknown> | null,
|
||||
notifierStart: vi.fn(async () => undefined),
|
||||
notifierStop: vi.fn(),
|
||||
notifierNotifyGridlock: vi.fn(),
|
||||
notificationServiceStart: vi.fn(async () => undefined),
|
||||
notificationServiceStop: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@fusion/core", async () => {
|
||||
@@ -69,16 +76,16 @@ vi.mock("../pr-comment-handler.js", () => ({
|
||||
|
||||
vi.mock("../notifier.js", () => ({
|
||||
NtfyNotifier: vi.fn().mockImplementation(() => ({
|
||||
start: vi.fn(async () => undefined),
|
||||
stop: vi.fn(),
|
||||
notifyGridlock: vi.fn(),
|
||||
start: mocks.notifierStart,
|
||||
stop: mocks.notifierStop,
|
||||
notifyGridlock: mocks.notifierNotifyGridlock,
|
||||
})),
|
||||
}));
|
||||
|
||||
vi.mock("../notification/index.js", () => ({
|
||||
NotificationService: vi.fn().mockImplementation(() => ({
|
||||
start: vi.fn().mockResolvedValue(undefined),
|
||||
stop: vi.fn(),
|
||||
start: mocks.notificationServiceStart,
|
||||
stop: mocks.notificationServiceStop,
|
||||
})),
|
||||
}));
|
||||
|
||||
@@ -209,7 +216,7 @@ const baseSettings: Record<string, unknown> = {
|
||||
remoteAccess: baseRemoteAccess,
|
||||
};
|
||||
|
||||
function createEngine() {
|
||||
function createEngine(options?: ConstructorParameters<typeof ProjectEngine>[2]) {
|
||||
return new ProjectEngine(
|
||||
{
|
||||
projectId: "proj_test",
|
||||
@@ -219,11 +226,17 @@ function createEngine() {
|
||||
maxWorktrees: 2,
|
||||
},
|
||||
{} as never,
|
||||
{ skipNotifier: true },
|
||||
{ skipNotifier: true, ...options },
|
||||
);
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
mocks.notifierStart.mockClear();
|
||||
mocks.notifierStop.mockClear();
|
||||
mocks.notifierNotifyGridlock.mockClear();
|
||||
mocks.notificationServiceStart.mockClear();
|
||||
mocks.notificationServiceStop.mockClear();
|
||||
|
||||
mocks.execFile.mockImplementation((
|
||||
_file: string,
|
||||
_args: string[],
|
||||
@@ -243,6 +256,32 @@ beforeEach(() => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("ProjectEngine notification ownership wiring", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
const mockStore = createMockStore(baseSettings);
|
||||
mocks.currentStore = mockStore.store;
|
||||
});
|
||||
|
||||
it("constructs NtfyNotifier with the same NotificationService instance and starts canonical listeners once", async () => {
|
||||
const engine = createEngine({ skipNotifier: false, projectId: "proj_for_notifier" });
|
||||
|
||||
await engine.start();
|
||||
|
||||
expect(NotificationService).toHaveBeenCalledTimes(1);
|
||||
expect(NtfyNotifier).toHaveBeenCalledTimes(1);
|
||||
const notifierCtorArgs = vi.mocked(NtfyNotifier).mock.calls[0];
|
||||
expect(notifierCtorArgs?.[2]).toBe(vi.mocked(NotificationService).mock.results[0]?.value);
|
||||
|
||||
expect(mocks.notificationServiceStart).toHaveBeenCalledTimes(1);
|
||||
expect(mocks.notifierStart).toHaveBeenCalledTimes(1);
|
||||
|
||||
await engine.stop();
|
||||
expect(mocks.notificationServiceStop).toHaveBeenCalledTimes(1);
|
||||
expect(mocks.notifierStop).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe("ProjectEngine auto-summarize wiring", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
|
||||
@@ -182,11 +182,12 @@ export class NtfyNotifier {
|
||||
constructor(
|
||||
private store: NtfyNotifierStore,
|
||||
options: NtfyNotifierOptions = {},
|
||||
notificationService?: NotificationService,
|
||||
) {
|
||||
this.defaultNtfyBaseUrl = resolveNtfyBaseUrl(options.ntfyBaseUrl);
|
||||
this.ntfyBaseUrl = this.defaultNtfyBaseUrl;
|
||||
this.projectId = options.projectId;
|
||||
this.notificationService = new NotificationService(store, {
|
||||
this.notificationService = notificationService ?? new NotificationService(store, {
|
||||
projectId: this.projectId,
|
||||
ntfyBaseUrl: options.ntfyBaseUrl,
|
||||
});
|
||||
|
||||
@@ -269,10 +269,14 @@ export class ProjectEngine {
|
||||
await this.notificationService.start();
|
||||
|
||||
// Backward-compatibility shim for gridlock notifications.
|
||||
this.notifier = new NtfyNotifier(store, {
|
||||
projectId: this.options.projectId,
|
||||
ntfyBaseUrl: this.options.ntfyBaseUrl,
|
||||
});
|
||||
this.notifier = new NtfyNotifier(
|
||||
store,
|
||||
{
|
||||
projectId: this.options.projectId,
|
||||
ntfyBaseUrl: this.options.ntfyBaseUrl,
|
||||
},
|
||||
this.notificationService,
|
||||
);
|
||||
await this.notifier.start();
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user