From 3650cc3c41df19dd6cc1890bbdfd5926a464a158 Mon Sep 17 00:00:00 2001 From: Mikael Hugo Date: Tue, 5 May 2026 20:45:47 +0200 Subject: [PATCH] fix: keep notification backlog actionable --- .../extensions/sf/notification-store.js | 74 ++++++++++++++++++- ...ification-detection-headless-high.test.mjs | 20 +++++ ...ion-detection-headless-medium-low.test.mjs | 23 +++++- 3 files changed, 113 insertions(+), 4 deletions(-) diff --git a/src/resources/extensions/sf/notification-store.js b/src/resources/extensions/sf/notification-store.js index 5d9bee308..b3eb88a71 100644 --- a/src/resources/extensions/sf/notification-store.js +++ b/src/resources/extensions/sf/notification-store.js @@ -22,7 +22,33 @@ const MAX_ENTRIES = 500; const FILENAME = "notifications.jsonl"; const LOCKFILE = "notifications.lock"; const DEDUP_WINDOW_MS = 30_000; +const DURABLE_DEDUP_WINDOW_MS = 60 * 60 * 1000; const DEDUP_PRUNE_THRESHOLD = 200; +const DURABLE_DEDUP_SCAN_LIMIT = 100; +const ACTIONABLE_KINDS = new Set([ + "action_required", + "approval_request", + "attention", + "blocker", + "error", + "terminal", +]); +const NOISY_STATUS_PATTERNS = [ + /^MCP client ready\b/, + /^Startup doctor: applied\b/, + /^Flow audit: Inspect session\b/, + /^UOK parity report shows\b/, + /Permission bypassed - all checks disabled!?$/, + /^Dynamic routing:/, + /^Pre-flight:/, + /^\[unit\]/, + /^Model \[/, + /^\[mcp\]/, + /^Step-mode started\b/, + /^Auto-mode resumed\b/, + /^Resuming paused session\b/, + /^Resumed paused session\b/, +]; // ─── Module State ─────────────────────────────────────────────────────── let _basePath = null; let _lineCount = 0; // Hint for rotation — not authoritative for public API @@ -58,9 +84,9 @@ export function appendNotification( ) { if (!_basePath) return; if (_suppressCount > 0) return; - if (!shouldPersistNotification(severity, metadata)) return; const persistedMessage = message.length > 500 ? message.slice(0, 500) + "…" : message; + if (!shouldPersistNotification(severity, metadata, persistedMessage)) return; // Use explicit dedupe_key when provided; fall back to message-hash based key. const dedupKey = metadata?.dedupe_key ? `${_basePath}:${metadata.dedupe_key}` @@ -74,6 +100,15 @@ export function appendNotification( if (now - ts > DEDUP_WINDOW_MS) _recentMessageTimestamps.delete(key); } } + if ( + hasRecentPersistedDuplicate( + _basePath, + metadata?.dedupe_key ?? `${severity}:${source}:${persistedMessage}`, + now, + ) + ) { + return; + } const entry = { id: randomUUID(), ts: new Date().toISOString(), @@ -107,9 +142,17 @@ export function appendNotification( ); } } -function shouldPersistNotification(_severity, metadata) { +function shouldPersistNotification(severity, metadata, message) { if (metadata?.kind === "progress") return false; - return true; + if (metadata?.persist === false) return false; + if (metadata?.persist === true) return true; + if (metadata?.blocking === true || metadata?.requires_action === true) { + return true; + } + if (ACTIONABLE_KINDS.has(metadata?.kind)) return true; + if (severity === "error") return true; + if (isNoisyStatusMessage(message)) return false; + return severity === "warning"; } /** * Read all notification entries from disk. Returns newest-first. @@ -256,6 +299,31 @@ function _readEntriesFromDisk(basePath) { return []; } } +function hasRecentPersistedDuplicate(basePath, keySeed, now) { + const normalizedKey = normalizeDedupKey(keySeed); + const entries = _readEntriesFromDisk(basePath); + for (const entry of entries.slice(-DURABLE_DEDUP_SCAN_LIMIT)) { + if (entry.read === true) continue; + const ts = Date.parse(entry.ts); + if (!Number.isFinite(ts) || now - ts > DURABLE_DEDUP_WINDOW_MS) continue; + const entryKey = entry.metadata?.dedupe_key + ? normalizeDedupKey(entry.metadata.dedupe_key) + : normalizeDedupKey( + `${entry.severity}:${entry.source ?? "notify"}:${entry.message}`, + ); + if (entryKey === normalizedKey) return true; + } + return false; +} +function normalizeDedupKey(value) { + return String(value) + .replace(/\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d+)?Z/g, "") + .replace(/\bsince\s+[^.]+/gi, "since ") + .trim(); +} +function isNoisyStatusMessage(message) { + return NOISY_STATUS_PATTERNS.some((pattern) => pattern.test(message)); +} function _rotate() { if (!_basePath) return; try { diff --git a/src/resources/extensions/sf/tests/notification-detection-headless-high.test.mjs b/src/resources/extensions/sf/tests/notification-detection-headless-high.test.mjs index 1c9d9d05b..f7df270cd 100644 --- a/src/resources/extensions/sf/tests/notification-detection-headless-high.test.mjs +++ b/src/resources/extensions/sf/tests/notification-detection-headless-high.test.mjs @@ -56,6 +56,7 @@ describe("S08 HIGH: notification + detection + headless", () => { it("should deduplicate rapid identical notifications", () => { appendNotification("test message", "info", "test", { dedupe_key: "rapid-test", + blocking: true, }); // Small delay to ensure first write completes const start = Date.now(); @@ -64,6 +65,7 @@ describe("S08 HIGH: notification + detection + headless", () => { } appendNotification("test message", "info", "test", { dedupe_key: "rapid-test", + blocking: true, }); const content = readFileSync( join(testDir, ".sf", "notifications.jsonl"), @@ -72,6 +74,24 @@ describe("S08 HIGH: notification + detection + headless", () => { const lines = content.trim().split("\n").filter(Boolean); expect(lines.length).toBe(1); }); + + it("should suppress repeated persisted notifications across restarts", () => { + appendNotification("quota warning", "warning", "notify", { + dedupe_key: "quota-warning", + }); + _resetNotificationStore(); + initNotificationStore(testDir); + appendNotification("quota warning", "warning", "notify", { + dedupe_key: "quota-warning", + }); + + const content = readFileSync( + join(testDir, ".sf", "notifications.jsonl"), + "utf-8", + ); + const lines = content.trim().split("\n").filter(Boolean); + expect(lines.length).toBe(1); + }); }); describe("ROOT_ONLY_PROJECT_FILES root-only detection", () => { diff --git a/src/resources/extensions/sf/tests/notification-detection-headless-medium-low.test.mjs b/src/resources/extensions/sf/tests/notification-detection-headless-medium-low.test.mjs index cb4539d0c..32a41806b 100644 --- a/src/resources/extensions/sf/tests/notification-detection-headless-medium-low.test.mjs +++ b/src/resources/extensions/sf/tests/notification-detection-headless-medium-low.test.mjs @@ -33,7 +33,28 @@ describe("S08 MEDIUM: notification + detection + headless", () => { writeFileSync(lockPath, "not-a-number", "utf-8"); // This should succeed because NaN lock is treated as stale - appendNotification("test message", "info", "test"); + appendNotification("test message", "info", "test", { blocking: true }); + + const content = readFileSync( + join(testDir, ".sf", "notifications.jsonl"), + "utf-8", + ); + const lines = content.trim().split("\n").filter(Boolean); + expect(lines.length).toBe(1); + }); + + it("should not persist non-actionable startup status notifications", () => { + appendNotification("MCP client ready — 2 server(s) configured", "info"); + + expect(() => + readFileSync(join(testDir, ".sf", "notifications.jsonl"), "utf-8"), + ).toThrow(); + }); + + it("should persist actionable info notifications", () => { + appendNotification("operator input required", "info", "notify", { + kind: "terminal", + }); const content = readFileSync( join(testDir, ".sf", "notifications.jsonl"),