Harden the egress recovery job after a second review pass (#3937)

- Schedule the recovery job before recording the degraded transition,
  so a crash between the two leaves a stray row (removed on the job's
  first run) rather than a degraded state nothing is polling
- Count any HTTP response, including 4xx/5xx, as reachable for http
  targets; only a transport failure counts against egress
- Cascade a deleted notification out of egressNotifications with an
  atomic $pull in the settings repository, after the team-scoped delete
- Handle two processes racing the first egress state insert
- Address global job rows by one key (deleteGlobalJob on _id) and
  restore the deleteByIdAndType signature
- Inject net into PingProvider like PortProvider
- Integration tests for MongoEgressStateRepository and the global-row
  delete methods on the jobs repository

🔧

server/src/config/services.worker.ts

@@ -78,7 +78,7 @@ export const buildWorker = async (shared: SharedServices, envSettings: EnvConfig
 	// ***********************
 	// Network providers
 	// ***********************
-	const pingProvider = new PingProvider(ping);
+	const pingProvider = new PingProvider(ping, net);
 	const httpProvider = new HttpProvider(got, new AdvancedMatcher(jmespath));
 	const pageSpeedProvider = new PageSpeedProvider(httpProvider, settingsService, logger);
 	const hardwareProvider = new HardwareProvider(httpProvider);

server/src/domain/app-settings/app-settings-repository.interface.ts

@@ -6,6 +6,7 @@ export interface ISettingsRepository {
 	findSingleton(): Promise<Settings | null>;
 	// update
 	update(settings: SettingsUpdate): Promise<Settings>;
+	removeEgressNotification(notificationId: string): Promise<void>; // $pull, so it cannot race a concurrent settings update
 	// delete
 	deleteLegacy: () => Promise<boolean>;
 }

server/src/domain/app-settings/app-settings.repository.mongo.ts

@@ -84,6 +84,10 @@ class MongoSettingsRepository implements ISettingsRepository {
 		return this.toEntity(updatedSettings);
 	};
 
+	removeEgressNotification = async (notificationId: string) => {
+		await AppSettingsModel.updateMany({ egressNotifications: notificationId }, { $pull: { egressNotifications: notificationId } });
+	};
+
 	deleteLegacy = async () => {
 		const res = await AppSettingsModel.deleteMany({ version: { $exists: false } });
 		return res.deletedCount > 0;

server/src/domain/app-settings/app-settings.service.ts

@@ -24,6 +24,7 @@ export interface ISettingsService {
 	areStatusPageThemesEnabled(): boolean;
 	getDBSettings(): Promise<Settings>;
 	updateDbSettings(newSettings: SettingsUpdate): Promise<Settings>;
+	removeEgressNotification(notificationId: string): Promise<void>;
 }
 
 export class SettingsService implements ISettingsService {
@@ -78,6 +79,10 @@ export class SettingsService implements ISettingsService {
 		return await this.getRepository().update(newSettings);
 	};
 
+	removeEgressNotification = async (notificationId: string) => {
+		await this.getRepository().removeEgressNotification(notificationId);
+	};
+
 	getDBSettings = async () => {
 		const repo = this.getRepository();
 		// Remove any old settings

server/src/domain/egress/egress-state.repository.mongo.ts

@@ -30,12 +30,21 @@ class MongoEgressStateRepository implements IEgressStateRepository {
 		// Read first: with timestamps on, every findOneAndUpdate would $set updatedAt, so a read would write.
 		const existing = await EgressStateModel.findOne({ singleton: true }).lean<EgressStateDocument>();
 		if (existing) return this.toEntity(existing);
-		const doc = await EgressStateModel.findOneAndUpdate(
-			{ singleton: true },
-			{ $setOnInsert: { singleton: true, status: "ok" } },
-			{ upsert: true, new: true, setDefaultsOnInsert: true }
-		).lean<EgressStateDocument>();
-		return this.toEntity(doc);
+		try {
+			const doc = await EgressStateModel.findOneAndUpdate(
+				{ singleton: true },
+				{ $setOnInsert: { singleton: true, status: "ok" } },
+				{ upsert: true, new: true, setDefaultsOnInsert: true }
+			).lean<EgressStateDocument>();
+			return this.toEntity(doc);
+		} catch (error: unknown) {
+			// Two processes can race the very first insert; the unique index rejects the loser, who reads the winner's row.
+			if ((error as { code?: number }).code === 11000) {
+				const doc = await EgressStateModel.findOne({ singleton: true }).lean<EgressStateDocument>();
+				if (doc) return this.toEntity(doc);
+			}
+			throw error;
+		}
 	};
 
 	reset = async () => {

server/src/domain/egress/egress-state.service.ts

@@ -25,7 +25,7 @@ export class EgressStateService implements IEgressStateService {
 
 	// Called when the feature is switched on or off: drop any pending recovery job and start from "ok".
 	reset = async (): Promise<EgressState> => {
-		await this.jobsRepository.deleteByIdAndType(null, "egress");
+		await this.jobsRepository.deleteGlobalJob("egress");
 		return this.egressStateRepository.reset();
 	};
 }

server/src/domain/egress/egress.service.ts

@@ -25,6 +25,9 @@ const ASSESSMENT_REUSE_MS = 5000;
 const RECOVERY_JOB_TYPE = "egress" as const;
 const RECOVERY_JOB_ID = jobId(RECOVERY_JOB_TYPE, null);
 
+// HttpProvider reports transport failures with NETWORK_ERROR (outside the HTTP range) and everything else with the real status code.
+const isHttpStatusCode = (code: unknown): boolean => typeof code === "number" && code >= 100 && code <= 599;
+
 // Synthetic identity stamped on the probe monitors so provider responses are recognisable in logs.
 const PROBE_MONITOR_ID = "egress-probe";
 const PROBE_TEAM_ID = "system";
@@ -115,7 +118,9 @@ export class EgressService implements IEgressService {
 
 		return {
 			target: raw,
-			reachable: response.status === true,
+			// Any HTTP response, including 4xx/5xx, proves the instance can reach the target; only a transport
+			// failure counts against egress. Ping and port providers have no such distinction.
+			reachable: response.status === true || (target.kind === "http" && isHttpStatusCode(response.code)),
 			responseTime: response.responseTime ?? responseTime,
 			message: response.message,
 		};
@@ -188,6 +193,9 @@ export class EgressService implements IEgressService {
 				return "ok";
 			}
 
+			// Schedule the recovery job before recording the transition: a crash between the two then leaves a stray row
+			// while ok, which the job's first run removes, rather than a degraded state that nothing is polling.
+			await this.scheduleRecoveryCheck(settings.egressPollIntervalSeconds, true);
 			const degraded = await this.egressStateRepository.markDegraded(results, now);
 			this.logger.warn({
 				message: degraded
@@ -197,8 +205,6 @@ export class EgressService implements IEgressService {
 				method: "assessAfterFailure",
 				details: { results },
 			});
-			// The worker that performed the transition owns the job's schedule; anyone else only makes sure the row exists.
-			await this.scheduleRecoveryCheck(settings.egressPollIntervalSeconds, degraded !== null);
 			return "degraded";
 		} catch (error: unknown) {
 			// The egress check must never break check production; treat an internal failure as "unknown" and carry on.

server/src/domain/jobs/job.repository.interface.ts

@@ -58,9 +58,11 @@ export interface IJobsRepository {
 	// Delete, drop all rows
 	deleteById(refId: string): Promise<boolean>;
 
-	// Delete a single job of type. refId null addresses a global row
-	deleteByIdAndType(refId: string | null, type: JobType): Promise<boolean>;
+	// Delete a single job of type
+	deleteByIdAndType(refId: string, type: JobType): Promise<boolean>;
 
+	// Global rows (refId null) are addressed by their canonical _id, see jobId
+	deleteGlobalJob(type: JobType): Promise<boolean>;
 	// Delete a global row only if nobody has rescheduled it since it was claimed (nextScheduledAt unchanged)
 	deleteGlobalJobIfUnchanged(type: JobType, nextScheduledAt: number): Promise<boolean>;
 

server/src/domain/jobs/job.repository.mongo.ts

@@ -273,8 +273,13 @@ class MongoJobsRepository implements IJobsRepository {
 		return res.deletedCount > 0;
 	};
 
-	deleteByIdAndType = async (refId: string | null, type: JobType) => {
-		const res = await JobModel.deleteOne({ refId, type }); // Delete a single typed row, e.g. just the geo row or the global egress row
+	deleteByIdAndType = async (refId: string, type: JobType) => {
+		const res = await JobModel.deleteOne({ refId, type }); // Delete a single typed row, e.g. just the geo row
+		return res.deletedCount > 0;
+	};
+
+	deleteGlobalJob = async (type: JobType) => {
+		const res = await JobModel.deleteOne({ _id: jobId(type, null) });
 		return res.deletedCount > 0;
 	};
 

server/src/domain/notifications/notification.service.ts

@@ -178,18 +178,10 @@ export class NotificationsService implements INotificationsService {
 
 	deleteById = async (id: string, teamId: string): Promise<Notification> => {
 		await this.monitorsRepository.removeNotificationFromMonitors(id);
-		await this.removeFromEgressNotifications(id);
 		const deleted = await this.notificationsRepository.deleteById(id, teamId);
+		// App settings hold notification ids for the egress recovered alert; drop the deleted one so it never dangles.
+		// After the team-scoped delete, so a request for another team's id (404 above) cannot touch instance settings.
+		await this.settingsService.removeEgressNotification(id);
 		return deleted;
 	};
-
-	// App settings hold notification ids for the egress recovered alert; drop the deleted one so it never dangles
-	private removeFromEgressNotifications = async (id: string) => {
-		const settings = await this.settingsService.getDBSettings();
-		const current = settings.egressNotifications ?? [];
-		const remaining = current.filter((notificationId) => notificationId !== id);
-		if (remaining.length !== current.length) {
-			await this.settingsService.updateDbSettings({ egressNotifications: remaining });
-		}
-	};
 }

server/src/service/network/PingProvider.ts

@@ -4,16 +4,20 @@ import { MonitorType, Monitor } from "@/domain/monitors/monitor.type.js";
 import { MonitorStatusResponse } from "@/types/network.js";
 import { AppError } from "@/utils/AppError.js";
 import ping from "ping";
-import net from "net";
+import * as net from "net";
 import { timeRequest } from "@/service/network/utils.js";
 const SERVICE_NAME = "PingProvider";
 
 type Ping = typeof ping;
+type NetType = typeof net;
 
 export class PingProvider implements IStatusProvider<PingStatusPayload> {
 	readonly type = "ping";
 
-	constructor(private ping: Ping) {}
+	constructor(
+		private ping: Ping,
+		private net: NetType
+	) {}
 
 	supports(type: MonitorType): boolean {
 		return type === "ping";
@@ -25,7 +29,7 @@ export class PingProvider implements IStatusProvider<PingStatusPayload> {
 		const bracketed = /^\[([^\]]+)\](?::\d+)?$/.exec(host);
 		if (bracketed) return bracketed[1]!;
 		// A bare IPv6 address contains colons that are not a port separator
-		if (net.isIPv6(host)) return host;
+		if (this.net.isIPv6(host)) return host;
 		return host.replace(/:.*/, "");
 	}
 

server/test/integration/egressStateRepository.test.ts

@@ -0,0 +1,130 @@
+import { describe, expect, it, beforeAll, afterAll, beforeEach } from "@jest/globals";
+import mongoose from "mongoose";
+import { MongoMemoryServer } from "mongodb-memory-server";
+import MongoEgressStateRepository from "../../src/domain/egress/egress-state.repository.mongo.ts";
+import { EgressStateModel } from "../../src/domain/egress/egress-state.model.ts";
+
+// ── Real-Mongo harness ─────────────────────────────────────────────────────────
+// The transitions here are the guarantees several worker processes rely on: exactly one
+// caller flips ok -> degraded (and degraded -> ok), and a plain read never writes. Both are
+// MongoDB filter/update semantics, so they need a live engine.
+
+let mongod: MongoMemoryServer;
+
+beforeAll(async () => {
+	mongod = await MongoMemoryServer.create();
+	await mongoose.connect(mongod.getUri());
+	await EgressStateModel.init(); // unique index on singleton
+}, 120_000);
+
+afterAll(async () => {
+	await mongoose.disconnect();
+	await mongod.stop();
+});
+
+beforeEach(async () => {
+	await EgressStateModel.deleteMany({});
+});
+
+const probe = (target: string, reachable: boolean) => ({ target, reachable, responseTime: 5, message: reachable ? "Success" : "timeout" });
+const NOW = new Date("2026-01-01T10:00:00.000Z");
+const LATER = new Date("2026-01-01T10:05:00.000Z");
+
+describe("MongoEgressStateRepository", () => {
+	const repo = new MongoEgressStateRepository();
+
+	describe("findSingleton", () => {
+		it("creates the ok singleton on first call and returns the same document afterwards", async () => {
+			const first = await repo.findSingleton();
+			const second = await repo.findSingleton();
+
+			expect(first.status).toBe("ok");
+			expect(second.id).toBe(first.id);
+			expect(await EgressStateModel.countDocuments()).toBe(1);
+		});
+
+		it("does not bump updatedAt on a read", async () => {
+			const created = await repo.findSingleton();
+			await new Promise((resolve) => setTimeout(resolve, 20));
+
+			const read = await repo.findSingleton();
+
+			expect(read.updatedAt).toBe(created.updatedAt);
+		});
+
+		it("survives two processes racing the first insert", async () => {
+			const [a, b] = await Promise.all([repo.findSingleton(), repo.findSingleton()]);
+
+			expect(a.id).toBe(b.id);
+			expect(await EgressStateModel.countDocuments()).toBe(1);
+		});
+	});
+
+	describe("markDegraded / markRecovered", () => {
+		it("markDegraded does nothing when the singleton has never been created (callers read it first)", async () => {
+			expect(await repo.markDegraded([probe("1.1.1.1", false)], NOW)).toBeNull();
+			expect(await EgressStateModel.countDocuments()).toBe(0);
+		});
+
+		it("lets exactly one caller perform each transition", async () => {
+			await repo.findSingleton();
+
+			const [d1, d2] = await Promise.all([repo.markDegraded([probe("1.1.1.1", false)], NOW), repo.markDegraded([probe("1.1.1.1", false)], NOW)]);
+			expect([d1, d2].filter((result) => result !== null)).toHaveLength(1);
+			expect((await repo.findSingleton()).status).toBe("degraded");
+
+			const [r1, r2] = await Promise.all([repo.markRecovered([probe("1.1.1.1", true)], LATER), repo.markRecovered([probe("1.1.1.1", true)], LATER)]);
+			expect([r1, r2].filter((result) => result !== null)).toHaveLength(1);
+			const state = await repo.findSingleton();
+			expect(state.status).toBe("ok");
+			expect(state.degradedSince).toBe(NOW.toISOString());
+			expect(state.lastRecoveredAt).toBe(LATER.toISOString());
+		});
+
+		it("markRecovered is a no-op while ok and markDegraded is a no-op while degraded", async () => {
+			await repo.findSingleton();
+			expect(await repo.markRecovered([probe("1.1.1.1", true)], NOW)).toBeNull();
+
+			await repo.markDegraded([probe("1.1.1.1", false)], NOW);
+			expect(await repo.markDegraded([probe("1.1.1.1", false)], LATER)).toBeNull();
+			expect((await repo.findSingleton()).degradedSince).toBe(NOW.toISOString());
+		});
+	});
+
+	describe("recordProbe", () => {
+		it("stores the probe without changing the status", async () => {
+			await repo.findSingleton();
+			await repo.markDegraded([probe("1.1.1.1", false)], NOW);
+
+			const state = await repo.recordProbe([probe("1.1.1.1", false), probe("8.8.8.8", false)], LATER);
+
+			expect(state.status).toBe("degraded");
+			expect(state.lastProbeAt).toBe(LATER.toISOString());
+			expect(state.lastProbeResults.map((result) => result.target)).toEqual(["1.1.1.1", "8.8.8.8"]);
+		});
+	});
+
+	describe("reset", () => {
+		it("returns a degraded singleton to ok with no episode and keeps the recovery history", async () => {
+			await repo.findSingleton();
+			await repo.markDegraded([probe("1.1.1.1", false)], NOW);
+			await repo.markRecovered([probe("1.1.1.1", true)], LATER);
+			await repo.markDegraded([probe("1.1.1.1", false)], LATER);
+
+			const state = await repo.reset();
+
+			expect(state.status).toBe("ok");
+			expect(state.degradedSince).toBeNull();
+			expect(state.lastProbeAt).toBeNull();
+			expect(state.lastProbeResults).toEqual([]);
+			expect(state.lastRecoveredAt).toBe(LATER.toISOString());
+		});
+
+		it("creates the singleton when none exists", async () => {
+			const state = await repo.reset();
+
+			expect(state.status).toBe("ok");
+			expect(await EgressStateModel.countDocuments()).toBe(1);
+		});
+	});
+});

server/test/integration/jobsRepository.test.ts

@@ -533,4 +533,47 @@ describe("MongoJobsRepository", () => {
 			expect(await repo.countDueBacklog(NOW)).toBe(0);
 		});
 	});
+
+	// ── global rows: egress recovery job ──────────────────────────────────────
+
+	describe("global rows", () => {
+		const repo = new MongoJobsRepository(WORKER_ID);
+		const egressRow = (nextScheduledAt: number) =>
+			seedJob({ _id: "egress", type: "egress" as JobType, refId: null, nextScheduledAt, intervalMs: 30_000 });
+
+		it("deleteGlobalJob removes the row by its canonical _id and leaves other global rows alone", async () => {
+			await egressRow(NOW);
+			await seedJob({ _id: "cleanup-orphaned", type: "cleanup-orphaned" as JobType, refId: null });
+
+			expect(await repo.deleteGlobalJob("egress")).toBe(true);
+
+			expect(await readRow("egress")).toBeNull();
+			expect(await readRow("cleanup-orphaned")).not.toBeNull();
+			expect(await repo.deleteGlobalJob("egress")).toBe(false);
+		});
+
+		it("deleteGlobalJobIfUnchanged removes the row only while nextScheduledAt matches the claimed value", async () => {
+			await egressRow(NOW);
+
+			// Another worker started a new episode and rescheduled the row after this run claimed it
+			await JobModel.updateOne({ _id: "egress" }, { $set: { nextScheduledAt: NOW + 30_000 } });
+			expect(await repo.deleteGlobalJobIfUnchanged("egress", NOW)).toBe(false);
+			expect(await readRow("egress")).not.toBeNull();
+
+			// Nobody touched it: the row goes
+			expect(await repo.deleteGlobalJobIfUnchanged("egress", NOW + 30_000)).toBe(true);
+			expect(await readRow("egress")).toBeNull();
+		});
+
+		it("recordSuccess after a refused conditional delete is a no-op once the lock has moved on", async () => {
+			await egressRow(NOW);
+			const [claimed] = await repo.claimDueBatch("egress", 1, NOW);
+			expect(claimed?.id).toBe("egress");
+
+			// Row deleted by the handler (recovered), then recordSuccess runs from the queue: nothing to update
+			expect(await repo.deleteGlobalJobIfUnchanged("egress", claimed!.nextScheduledAt)).toBe(true);
+			expect(await repo.recordSuccess("egress", claimed!.nextScheduledAt, 30_000, NOW + 1_000)).toBe(false);
+			expect(await readRow("egress")).toBeNull();
+		});
+	});
 });

server/test/unit/providers/network/pingProvider.test.ts

@@ -2,6 +2,7 @@ import { describe, expect, it, jest } from "@jest/globals";
 import { PingProvider } from "../../../../src/service/network/PingProvider.ts";
 import { testStatusProviderContract } from "../../../helpers/statusProviderContract.ts";
 import type { Monitor } from "../../../../src/domain/monitors/monitor.type.ts";
+import * as net from "net";
 
 // ── Helpers ──────────────────────────────────────────────────────────────────
 
@@ -27,7 +28,7 @@ const createMockPing = (result?: Partial<{ alive: boolean; time: number | string
 // ── Contract ─────────────────────────────────────────────────────────────────
 
 testStatusProviderContract("PingProvider", {
-	create: () => new PingProvider(createMockPing() as any),
+	create: () => new PingProvider(createMockPing() as any, net),
 	supportedType: "ping",
 	unsupportedType: "http",
 	makeMonitor: () => makeMonitor(),
@@ -37,7 +38,7 @@ testStatusProviderContract("PingProvider", {
 
 describe("PingProvider", () => {
 	it("returns success when host is alive", async () => {
-		const provider = new PingProvider(createMockPing({ alive: true, time: 30 }) as any);
+		const provider = new PingProvider(createMockPing({ alive: true, time: 30 }) as any, net);
 
 		const result = await provider.handle(makeMonitor());
 
@@ -55,7 +56,7 @@ describe("PingProvider", () => {
 	});
 
 	it("returns failure when host is not alive", async () => {
-		const provider = new PingProvider(createMockPing({ alive: false, time: 0 }) as any);
+		const provider = new PingProvider(createMockPing({ alive: false, time: 0 }) as any, net);
 
 		const result = await provider.handle(makeMonitor());
 
@@ -69,7 +70,7 @@ describe("PingProvider", () => {
 	});
 
 	it("parses string time values", async () => {
-		const provider = new PingProvider(createMockPing({ alive: true, time: "42.5" }) as any);
+		const provider = new PingProvider(createMockPing({ alive: true, time: "42.5" }) as any, net);
 
 		const result = await provider.handle(makeMonitor());
 
@@ -77,7 +78,7 @@ describe("PingProvider", () => {
 	});
 
 	it("defaults responseTime to 0 for non-numeric time", async () => {
-		const provider = new PingProvider(createMockPing({ alive: true, time: "unknown" }) as any);
+		const provider = new PingProvider(createMockPing({ alive: true, time: "unknown" }) as any, net);
 
 		const result = await provider.handle(makeMonitor());
 
@@ -85,7 +86,7 @@ describe("PingProvider", () => {
 	});
 
 	it("defaults status to false when alive is undefined", async () => {
-		const provider = new PingProvider(createMockPing({ alive: undefined as any }) as any);
+		const provider = new PingProvider(createMockPing({ alive: undefined as any }) as any, net);
 
 		const result = await provider.handle(makeMonitor());
 
@@ -94,7 +95,7 @@ describe("PingProvider", () => {
 
 	it("sanitizes URL by stripping protocol and path", async () => {
 		const mockPing = createMockPing();
-		const provider = new PingProvider(mockPing as any);
+		const provider = new PingProvider(mockPing as any, net);
 
 		await provider.handle(makeMonitor({ url: "https://example.com/path?query=1" }));
 
@@ -103,7 +104,7 @@ describe("PingProvider", () => {
 
 	it("sanitizes URL by stripping port", async () => {
 		const mockPing = createMockPing();
-		const provider = new PingProvider(mockPing as any);
+		const provider = new PingProvider(mockPing as any, net);
 
 		await provider.handle(makeMonitor({ url: "example.com:8080" }));
 
@@ -112,7 +113,7 @@ describe("PingProvider", () => {
 
 	it("keeps a bare IPv6 address intact", async () => {
 		const mockPing = createMockPing();
-		const provider = new PingProvider(mockPing as any);
+		const provider = new PingProvider(mockPing as any, net);
 
 		await provider.handle(makeMonitor({ url: "2606:4700:4700::1111" }));
 
@@ -121,7 +122,7 @@ describe("PingProvider", () => {
 
 	it("unwraps a bracketed IPv6 address and drops its port", async () => {
 		const mockPing = createMockPing();
-		const provider = new PingProvider(mockPing as any);
+		const provider = new PingProvider(mockPing as any, net);
 
 		await provider.handle(makeMonitor({ url: "[2606:4700:4700::1111]:53" }));
 
@@ -129,28 +130,28 @@ describe("PingProvider", () => {
 	});
 
 	it("throws AppError when url is missing", async () => {
-		const provider = new PingProvider(createMockPing() as any);
+		const provider = new PingProvider(createMockPing() as any, net);
 
 		await expect(provider.handle(makeMonitor({ url: "" }))).rejects.toThrow("URL is required for ping monitor");
 	});
 
 	it("throws AppError when ping.probe rejects", async () => {
 		const mockPing = { promise: { probe: jest.fn().mockRejectedValue(new Error("Host unreachable")) } };
-		const provider = new PingProvider(mockPing as any);
+		const provider = new PingProvider(mockPing as any, net);
 
 		await expect(provider.handle(makeMonitor())).rejects.toThrow("Host unreachable");
 	});
 
 	it("throws AppError when probe returns no response", async () => {
 		const mockPing = { promise: { probe: jest.fn().mockResolvedValue(null) } };
-		const provider = new PingProvider(mockPing as any);
+		const provider = new PingProvider(mockPing as any, net);
 
 		await expect(provider.handle(makeMonitor())).rejects.toThrow("No response from ping");
 	});
 
 	it("throws AppError with stringified message for non-Error throws", async () => {
 		const mockPing = { promise: { probe: jest.fn().mockRejectedValue(42) } };
-		const provider = new PingProvider(mockPing as any);
+		const provider = new PingProvider(mockPing as any, net);
 
 		await expect(provider.handle(makeMonitor())).rejects.toThrow("42");
 	});

server/test/unit/services/egressService.test.ts

@@ -146,6 +146,27 @@ describe("EgressService", () => {
 			]);
 		});
 
+		it("treats any HTTP response, even 4xx/5xx, as reachable but a transport failure as unreachable", async () => {
+			const requestStatus = jest.fn().mockImplementation(async (monitor: any) => ({
+				monitorId: monitor.id,
+				teamId: monitor.teamId,
+				type: monitor.type,
+				status: false,
+				code: monitor.url.includes("maintenance") ? 503 : 5000, // 5000 is HttpProvider's NETWORK_ERROR
+				message: monitor.url.includes("maintenance") ? "Service Unavailable" : "ECONNREFUSED",
+				responseTime: 3,
+			}));
+			const { service } = createService({ networkService: { requestStatus } });
+
+			const results = await service.probeTargets(["https://example.com/maintenance", "https://example.com/refused", "8.8.8.8:53"]);
+
+			expect(results).toEqual([
+				expect.objectContaining({ target: "https://example.com/maintenance", reachable: true }),
+				expect.objectContaining({ target: "https://example.com/refused", reachable: false }),
+				expect.objectContaining({ target: "8.8.8.8:53", reachable: false }), // non-http: only status true counts
+			]);
+		});
+
 		it("counts an unparseable stored target as unreachable without probing", async () => {
 			const { service, defaults } = createService();
 
@@ -218,7 +239,6 @@ describe("EgressService", () => {
 			);
 			expect(defaults.egressStateRepository.recordProbe).not.toHaveBeenCalled();
 			expect(defaults.logger.warn).toHaveBeenCalledWith(expect.objectContaining({ message: expect.stringContaining("egress degraded") }));
-			// The worker that performed the transition sets the schedule
 			expect(defaults.jobsRepository.upsertCleanupJob).toHaveBeenCalledWith({
 				id: "egress",
 				type: "egress",
@@ -228,9 +248,13 @@ describe("EgressService", () => {
 				intervalMs: 30_000,
 			});
 			expect(defaults.jobsRepository.upsertJob).not.toHaveBeenCalled();
+			// Job first, transition second: a crash in between leaves a stray row, never an unpolled degraded state
+			const scheduleOrder = (defaults.jobsRepository.upsertCleanupJob as jest.Mock).mock.invocationCallOrder[0]!;
+			const transitionOrder = (defaults.egressStateRepository.markDegraded as jest.Mock).mock.invocationCallOrder[0]!;
+			expect(scheduleOrder).toBeLessThan(transitionOrder);
 		});
 
-		it("only makes sure the job row exists when another worker performed the transition", async () => {
+		it("still returns degraded and leaves the job scheduled when another worker performed the transition first", async () => {
 			const { service, defaults } = createService({
 				networkService: { requestStatus: statusFor([]) },
 				egressStateRepository: degradedRepository({
@@ -242,8 +266,22 @@ describe("EgressService", () => {
 			const result = await service.assessAfterFailure();
 
 			expect(result).toBe("degraded");
-			expect(defaults.jobsRepository.upsertJob).toHaveBeenCalledWith(expect.objectContaining({ id: "egress", intervalMs: 30_000 }));
-			expect(defaults.jobsRepository.upsertCleanupJob).not.toHaveBeenCalled();
+			expect(defaults.jobsRepository.upsertCleanupJob).toHaveBeenCalledWith(expect.objectContaining({ id: "egress", intervalMs: 30_000 }));
+			expect(defaults.logger.warn).toHaveBeenCalledWith(expect.objectContaining({ message: expect.stringContaining("already recorded") }));
+		});
+
+		it("does not mark degraded when the recovery job cannot be scheduled", async () => {
+			const { service, defaults } = createService({
+				networkService: { requestStatus: statusFor([]) },
+				jobsRepository: {
+					upsertJob: jest.fn(),
+					upsertCleanupJob: jest.fn().mockRejectedValue(new Error("db down")),
+					deleteGlobalJobIfUnchanged: jest.fn(),
+				},
+			});
+
+			expect(await service.assessAfterFailure()).toBeNull();
+			expect(defaults.egressStateRepository.markDegraded).not.toHaveBeenCalled();
 		});
 
 		it("returns degraded without probing when the persisted state is already degraded, without touching the job's schedule", async () => {

server/test/unit/services/egressStateService.test.ts

@@ -13,7 +13,7 @@ const createEgressStateRepo = () => ({
 });
 
 const createJobsRepo = () => ({
-	deleteByIdAndType: jest.fn().mockResolvedValue(true),
+	deleteGlobalJob: jest.fn().mockResolvedValue(true),
 });
 
 const createService = () => {
@@ -82,7 +82,7 @@ describe("EgressStateService", () => {
 
 			const result = await service.reset();
 
-			expect(jobsRepository.deleteByIdAndType).toHaveBeenCalledWith(null, "egress");
+			expect(jobsRepository.deleteGlobalJob).toHaveBeenCalledWith("egress");
 			expect(egressStateRepository.reset).toHaveBeenCalledTimes(1);
 			expect(result).toEqual(reset);
 		});

server/test/unit/services/notificationsService.test.ts

@@ -26,10 +26,9 @@ const createMonitorsRepo = () => ({
 	removeNotificationFromMonitors: jest.fn(),
 });
 
-const createSettingsService = (clientHost = "https://app.example.com", egressNotifications: string[] = []) => ({
+const createSettingsService = (clientHost = "https://app.example.com") => ({
 	getSettings: jest.fn().mockReturnValue({ clientHost }),
-	getDBSettings: jest.fn().mockResolvedValue({ egressNotifications }),
-	updateDbSettings: jest.fn().mockResolvedValue({ egressNotifications: [] }),
+	removeEgressNotification: jest.fn().mockResolvedValue(undefined),
 });
 
 const createMessageBuilder = () => ({
@@ -374,24 +373,25 @@ describe("NotificationsService", () => {
 			expect(monitorsRepository.removeNotificationFromMonitors).toHaveBeenCalledWith("notif-1");
 		});
 
-		it("removes the notification from the egress recovered recipients in app settings", async () => {
-			const settingsService = createSettingsService("https://app.example.com", ["notif-0", "notif-1"]);
-			const { service, notificationsRepository } = createService({ settingsService });
+		it("removes the notification from the egress recovered recipients after the team-scoped delete succeeds", async () => {
+			const { service, notificationsRepository, settingsService } = createService();
 			(notificationsRepository.deleteById as jest.Mock).mockResolvedValue(makeNotification());
 
 			await service.deleteById("notif-1", "team-1");
 
-			expect(settingsService.updateDbSettings).toHaveBeenCalledWith({ egressNotifications: ["notif-0"] });
+			expect(settingsService.removeEgressNotification).toHaveBeenCalledWith("notif-1");
+			const deleteOrder = (notificationsRepository.deleteById as jest.Mock).mock.invocationCallOrder[0]!;
+			const cascadeOrder = (settingsService.removeEgressNotification as jest.Mock).mock.invocationCallOrder[0]!;
+			expect(deleteOrder).toBeLessThan(cascadeOrder);
 		});
 
-		it("leaves app settings alone when the notification is not an egress recipient", async () => {
-			const settingsService = createSettingsService("https://app.example.com", ["notif-0"]);
-			const { service, notificationsRepository } = createService({ settingsService });
-			(notificationsRepository.deleteById as jest.Mock).mockResolvedValue(makeNotification());
+		it("does not touch app settings when the delete is refused", async () => {
+			const { service, notificationsRepository, settingsService } = createService();
+			(notificationsRepository.deleteById as jest.Mock).mockRejectedValue(new Error("Notification not found"));
 
-			await service.deleteById("notif-1", "team-1");
+			await expect(service.deleteById("notif-1", "other-team")).rejects.toThrow("Notification not found");
 
-			expect(settingsService.updateDbSettings).not.toHaveBeenCalled();
+			expect(settingsService.removeEgressNotification).not.toHaveBeenCalled();
 		});
 	});
 });

server/test/unit/services/settingsService.test.ts

@@ -43,6 +43,7 @@ const createSettingsRepo = () =>
 		create: jest.fn().mockResolvedValue(makeSettings()),
 		findSingleton: jest.fn().mockResolvedValue(makeSettings()),
 		update: jest.fn().mockResolvedValue(makeSettings()),
+		removeEgressNotification: jest.fn().mockResolvedValue(undefined),
 		deleteLegacy: jest.fn().mockResolvedValue(true),
 	}) as unknown as jest.Mocked<ISettingsRepository>;
 
@@ -192,4 +193,20 @@ describe("SettingsService", () => {
 			await expect(service.updateDbSettings({ checkTTL: 60 })).rejects.toThrow("Settings repository not initialized");
 		});
 	});
+
+	describe("removeEgressNotification", () => {
+		it("delegates the pull to the repository", async () => {
+			const { service, settingsRepository } = createService();
+
+			await service.removeEgressNotification("notif-1");
+
+			expect(settingsRepository.removeEgressNotification).toHaveBeenCalledWith("notif-1");
+		});
+
+		it("throws when repository is not set", async () => {
+			const service = new SettingsService(makeEnv());
+
+			await expect(service.removeEgressNotification("notif-1")).rejects.toThrow("Settings repository not initialized");
+		});
+	});
 });