fix: preflight diagnostic toast — false unknown-hook, doubling, placement (lr-e901)

lib/public/css/diagnostics.css

@@ -154,17 +154,38 @@
 }
 
 /* --------------------------------------------------------
-   Diagnostic Toast
-   Extends the base .toast with richer layout.
-   Sits above the regular toast z-index.
+   Diagnostic Toast container (lr-e901)
+   Fixed top-right corner stack, off the composer/input box.
+   The base .toast family (base.css) is center-bottom over the
+   input — fine for a single transient message, but diagnostic
+   toasts can arrive concurrently (lr-e901 doubling defect) and
+   that spot has no stacking room. Follows the existing corner-
+   anchored, flex-column-stacked container precedent already
+   used by .notif-banner-container (notifications-center.css)
+   rather than introducing a third pattern: normal flow + gap
+   handles stacking with no per-toast JS offset math needed.
    -------------------------------------------------------- */
-.toast-diagnostic {
+#diagnostic-toast-container {
   position: fixed;
-  bottom: 80px;
-  left: 50%;
-  transform: translateX(-50%) translateY(8px);
+  top: 48px;
+  right: 8px;
   z-index: 450;
+  display: flex;
+  flex-direction: column;
+  gap: 8px;
+  pointer-events: none;
+}
+
+/* --------------------------------------------------------
+   Diagnostic Toast
+   Extends the base .toast with richer layout; positioning now
+   comes from the parent #diagnostic-toast-container (lr-e901),
+   not from the element itself.
+   -------------------------------------------------------- */
+.toast-diagnostic {
+  position: relative;
   opacity: 0;
+  transform: translateY(-8px);
   pointer-events: auto;
   transition: opacity 0.2s, transform 0.2s;
 
@@ -181,7 +202,7 @@
 }
 .toast-diagnostic.visible {
   opacity: 1;
-  transform: translateX(-50%) translateY(0);
+  transform: translateY(0);
 }
 
 /* Severity border accent */
@@ -259,19 +280,21 @@
    Mobile
    -------------------------------------------------------- */
 @media (max-width: 768px) {
-  .toast-diagnostic {
-    left: 16px;
+  /* lr-2fdd: bottom:80px occluded the bottom-nav / input box / content on
+     mobile (iOS screenshot showed the PREFLIGHT WARNING toast covering the
+     UPCOMING card just above the Projects/Chat/Team/More nav). Top-anchor
+     instead, matching the base .toast / .sw-update-banner mobile fix so the
+     whole toast family lands the same way. lr-e901 keeps this top-anchor
+     intact — only widens the container to full-width since the desktop
+     top-right corner box is too narrow/cramped on small viewports. */
+  #diagnostic-toast-container {
+    top: calc(var(--safe-top) + 12px);
     right: 16px;
-    transform: translateY(8px);
+    left: 16px;
+  }
+  .toast-diagnostic {
     max-width: none;
-    /* lr-2fdd: bottom:80px (inherited from base .toast) occludes the
-       bottom-nav / input box / content on mobile (iOS screenshot showed
-       the PREFLIGHT WARNING toast covering the UPCOMING card just above
-       the Projects/Chat/Team/More nav). Top-anchor instead, matching the
-       base .toast / .sw-update-banner mobile fix so the whole toast
-       family lands the same way. */
-    top: calc(var(--safe-top) + 12px);
-    bottom: auto;
+    transform: translateY(-8px);
   }
   .toast-diagnostic.visible {
     transform: translateY(0);

lib/public/modules/diagnostics.js

@@ -26,6 +26,32 @@ var panelBadgeEl = null;
 // Severity priority (higher = more prominent).
 var SEVERITY_ORDER = { error: 2, warning: 1, info: 0 };
 
+// lr-e901: dedup window for repeated/duplicate diagnostics. Two independent
+// emitters (CLI-stderr capture + deterministic settings-preflight runPreflight())
+// can legitimately produce a diagnostic for the same underlying condition -
+// addDiagnostic() had no dedup, so both were pushed and both toasted. Identity
+// is (source, scope, message): scope is included because runPreflight()
+// intentionally emits one diagnostic per settings file (user vs project,
+// lr-7e22) - those are distinct conditions and must NOT be deduped together.
+var DEDUP_WINDOW_MS = 5000;
+var _recentDiagnosticKeys = Object.create(null); // key -> last-seen timestamp (ms)
+
+function _diagnosticKey(msg) {
+  return (msg.source || "") + "|" + (msg.scope || "") + "|" + (msg.message || "");
+}
+
+/**
+ * True when an equivalent diagnostic (same source+scope+message) was already
+ * recorded within DEDUP_WINDOW_MS. Also prunes the key's prior timestamp so
+ * the map does not grow unbounded across a long session.
+ */
+function _isDuplicateDiagnostic(msg, now) {
+  var key = _diagnosticKey(msg);
+  var last = _recentDiagnosticKeys[key];
+  _recentDiagnosticKeys[key] = now;
+  return typeof last === "number" && (now - last) < DEDUP_WINDOW_MS;
+}
+
 // ============================================================
 // Init
 // ============================================================
@@ -86,13 +112,19 @@ export function initDiagnostics() {
 // ============================================================
 
 export function addDiagnostic(msg) {
+  var now = Date.now();
+
+  // lr-e901: same condition can arrive via two independent emitters (CLI-stderr
+  // capture + deterministic preflight) - collapse to a single visible entry.
+  if (_isDuplicateDiagnostic(msg, now)) return;
+
   var entry = {
     severity: msg.severity || "info",
     source: msg.source || "",
     scope: msg.scope || "",
     message: msg.message || "",
     actionable: msg.actionable || null,
-    ts: Date.now(),
+    ts: now,
   };
   diagnostics.push(entry);
 
@@ -117,6 +149,23 @@ export { formatDiagnosticSource } from './diagnostic-format.js';
 // Toast
 // ============================================================
 
+// lr-e901: fixed top-right corner stack (see diagnostics.css
+// #diagnostic-toast-container) so concurrent diagnostic toasts stack via
+// normal flex flow instead of piling on top of each other at a single fixed
+// position over the input box.
+var toastContainerEl = null;
+
+function _getToastContainer() {
+  if (toastContainerEl && toastContainerEl.isConnected) return toastContainerEl;
+  toastContainerEl = document.getElementById("diagnostic-toast-container");
+  if (!toastContainerEl) {
+    toastContainerEl = document.createElement("div");
+    toastContainerEl.id = "diagnostic-toast-container";
+    document.body.appendChild(toastContainerEl);
+  }
+  return toastContainerEl;
+}
+
 function _showDiagnosticToast(entry) {
   var el = document.createElement("div");
   // Use a dedicated class so diagnostics are visually distinct from error messages.
@@ -156,7 +205,7 @@ function _showDiagnosticToast(entry) {
   el.appendChild(msgEl);
   el.appendChild(hintEl);
 
-  document.body.appendChild(el);
+  _getToastContainer().appendChild(el);
   refreshIcons();
 
   // Animate in.

lib/yoke/adapters/diagnostic-patterns.js

@@ -11,6 +11,18 @@
 
 var validateDiagnosticEvent = require("../interface").validateDiagnosticEvent;
 
+// VALID_HOOK_EVENTS (lr-e901): the CLI's own "Unknown hook event: X" stderr
+// line is matched generically below (source:'hook') without knowing whether
+// the Console's allowlist actually considers X unknown. settings-preflight.js
+// is the single source of truth for that allowlist (see its header comment —
+// "the ONLY place the valid hook events are defined"). Reuse it here instead
+// of re-deriving a second copy, so a hook event added to the allowlist stops
+// producing a false "unknown hook" toast from BOTH emission paths at once.
+var VALID_HOOK_EVENTS = require("../../settings-preflight").VALID_HOOK_EVENTS;
+
+// Matches the hook event name out of a CLI "Unknown hook event: <Name>" line.
+var UNKNOWN_HOOK_EVENT_RE = /^Unknown hook event:\s*(\S+)/;
+
 // ---------------------------------------------------------------------------
 // Pattern table
 // ---------------------------------------------------------------------------
@@ -91,6 +103,19 @@ function parseDiagnosticLine(rawLine) {
   for (var i = 0; i < PATTERNS.length; i++) {
     var pattern = PATTERNS[i];
     if (matches(line, pattern)) {
+      // lr-e901: reconcile the CLI-stderr "Unknown hook event" path against
+      // the Console's own VALID_HOOK_EVENTS allowlist (settings-preflight.js).
+      // The CLI's stderr message reflects the *installed CLI version's*
+      // knowledge of hook events, which can lag the allowlist verified here
+      // (see settings-preflight.js header, lr-7e22). When the named event IS
+      // in VALID_HOOK_EVENTS, treat this as a stale/false CLI warning and
+      // suppress it rather than surfacing a contradictory toast — do not
+      // widen the allowlist to "fix" this, the allowlist is already correct.
+      var hookMatch = pattern.source === "hook" && UNKNOWN_HOOK_EVENT_RE.exec(line);
+      if (hookMatch && VALID_HOOK_EVENTS.indexOf(hookMatch[1]) !== -1) {
+        return null;
+      }
+
       var event = {
         type: "diagnostic",
         severity: pattern.severity,

test/diagnostic-patterns.test.js

@@ -31,8 +31,13 @@ var SEED_CASES = [
     expectedSource: "settings",
   },
   {
+    // lr-e901: PostToolUse is in VALID_HOOK_EVENTS (settings-preflight.js),
+    // so a CLI "Unknown hook event: PostToolUse" line is now reconciled away
+    // as a stale/false warning rather than forwarded (see 9g below). Use a
+    // genuinely-unknown event name here so this seed case still exercises
+    // the "Unknown hook event" pattern's happy path.
     label: "Unknown hook event → warning/hook",
-    line: "Unknown hook event: PostToolUse",
+    line: "Unknown hook event: SomeFutureHookEvent",
     expectedSeverity: "warning",
     expectedSource: "hook",
   },
@@ -180,3 +185,31 @@ test("9f: message is truncated to 500 chars for very long lines", function() {
   assert.ok(result.message.length <= 500,
     "message must be bounded to 500 chars, got: " + result.message.length);
 });
+
+// ---------------------------------------------------------------------------
+// Test 9g (lr-e901 regression) — CLI "Unknown hook event" reconciled against
+// VALID_HOOK_EVENTS (settings-preflight.js). A hook the Console's own
+// allowlist considers valid must NOT produce a diagnostic, even though the
+// CLI's stderr text says "Unknown" (installed-CLI-version lag, not a real
+// unknown-hook condition). Genuinely unknown events must still pass through.
+// ---------------------------------------------------------------------------
+
+var VALID_HOOK_EVENTS = require("../lib/settings-preflight").VALID_HOOK_EVENTS;
+
+test("9g: 'Unknown hook event' line for an event IN VALID_HOOK_EVENTS is suppressed (null)", function() {
+  for (var i = 0; i < VALID_HOOK_EVENTS.length; i++) {
+    var event = VALID_HOOK_EVENTS[i];
+    var result = parseDiagnosticLine("Unknown hook event: " + event);
+    assert.strictEqual(result, null,
+      "'Unknown hook event: " + event + "' must be suppressed — " + event +
+      " is in VALID_HOOK_EVENTS, so the CLI's warning is stale/false, not a real defect");
+  }
+});
+
+test("9g: 'Unknown hook event' line for an event NOT in VALID_HOOK_EVENTS still produces a diagnostic", function() {
+  var result = parseDiagnosticLine("Unknown hook event: TotallyMadeUpEvent");
+  assert.ok(result !== null,
+    "a genuinely unknown hook event must still produce a diagnostic");
+  assert.strictEqual(result.severity, "warning");
+  assert.strictEqual(result.source, "hook");
+});

test/diagnostics-toast-dedup-placement-lr-e901.test.js

@@ -0,0 +1,136 @@
+// diagnostics-toast-dedup-placement-lr-e901.test.js — lr-e901 regression coverage.
+//
+// Two of the three lr-e901 defects live in lib/public/modules/diagnostics.js
+// and lib/public/css/diagnostics.css:
+//
+//   (2) doubling — addDiagnostic() pushed + toasted every entry unconditionally,
+//       so the same underlying condition arriving via two independent emitters
+//       (CLI-stderr capture + deterministic settings-preflight runPreflight())
+//       produced two toasts. Fix: dedup keyed on (source, scope, message)
+//       within a short time window before pushing/toasting.
+//
+//   (3) placement/stacking — .toast-diagnostic was position:fixed center-bottom,
+//       directly over the composer/input box, with no stacking offset for
+//       concurrent toasts. Fix: re-anchor to a fixed top-right corner stack
+//       (#diagnostic-toast-container), following the existing corner-anchored,
+//       flex-column-stacked precedent already used by .notif-banner-container
+//       (notifications-center.css) — normal flow + gap handles stacking with
+//       no per-toast JS offset math. Mobile keeps the lr-2fdd top-anchor
+//       behavior, widened to full-width.
+//
+// diagnostics.js is an ESM module with DOM + icons.js dependencies that this
+// project's test runner does not exercise via a DOM harness (see the existing
+// diagnostics-panel-pointer-events-lr-b580.test.js convention) — these are
+// source-text regression checks matching that same convention.
+
+"use strict";
+
+var test = require("node:test");
+var assert = require("node:assert/strict");
+var fs = require("fs");
+var path = require("path");
+
+var DIAGNOSTICS_JS = fs.readFileSync(
+  path.join(__dirname, "../lib/public/modules/diagnostics.js"),
+  "utf8"
+);
+var DIAGNOSTICS_CSS = fs.readFileSync(
+  path.join(__dirname, "../lib/public/css/diagnostics.css"),
+  "utf8"
+);
+
+// ---------------------------------------------------------------------------
+// Defect 2 — doubling / dedup
+// ---------------------------------------------------------------------------
+
+test("diagnostics.js: addDiagnostic checks for a duplicate before pushing/toasting", () => {
+  var idx = DIAGNOSTICS_JS.indexOf("export function addDiagnostic");
+  assert.ok(idx !== -1, "expected addDiagnostic to be exported");
+  var block = DIAGNOSTICS_JS.slice(idx, idx + 400);
+
+  assert.match(
+    block,
+    /_isDuplicateDiagnostic\s*\(/,
+    "addDiagnostic must consult a duplicate check before recording a new entry (lr-e901 doubling defect)"
+  );
+  assert.match(
+    block,
+    /return;/,
+    "addDiagnostic must bail out early (return) when a duplicate is detected, skipping both the panel push and the toast"
+  );
+});
+
+test("diagnostics.js: dedup identity includes source, scope, and message", () => {
+  var idx = DIAGNOSTICS_JS.indexOf("function _diagnosticKey");
+  assert.ok(idx !== -1, "expected a _diagnosticKey helper to exist");
+  var block = DIAGNOSTICS_JS.slice(idx, idx + 300);
+
+  assert.match(block, /msg\.source/, "dedup key must include source");
+  assert.match(block, /msg\.scope/,
+    "dedup key must include scope -- runPreflight() intentionally emits one diagnostic " +
+    "per settings file (user vs project, lr-7e22); those are distinct conditions and " +
+    "must NOT be collapsed together by dedup"
+  );
+  assert.match(block, /msg\.message/, "dedup key must include message");
+});
+
+test("diagnostics.js: dedup uses a bounded time window, not a permanent block", () => {
+  assert.match(
+    DIAGNOSTICS_JS,
+    /DEDUP_WINDOW_MS/,
+    "expected a DEDUP_WINDOW_MS constant so identical diagnostics are only " +
+    "collapsed within a short window (not permanently blocked for the rest of the session)"
+  );
+});
+
+// ---------------------------------------------------------------------------
+// Defect 3 — placement / stacking
+// ---------------------------------------------------------------------------
+
+test("diagnostics.css: #diagnostic-toast-container is a fixed, corner-anchored, gap-stacked container", () => {
+  var idx = DIAGNOSTICS_CSS.indexOf("#diagnostic-toast-container {");
+  assert.ok(idx !== -1, "expected #diagnostic-toast-container rule to exist");
+  var block = DIAGNOSTICS_CSS.slice(idx, idx + 400);
+
+  assert.match(block, /position:\s*fixed/, "container must be position:fixed");
+  assert.match(block, /display:\s*flex/, "container must use flex layout for stacking");
+  assert.match(block, /flex-direction:\s*column/, "container must stack children in a column");
+  assert.match(block, /gap:/, "container must define a gap so stacked toasts do not touch/overlap");
+
+  // Off the input box: must NOT be center-bottom (the lr-e901 root cause).
+  assert.ok(block.indexOf("left: 50%") === -1,
+    "container must not be center-anchored (left:50%) -- that was the lr-e901 placement defect, directly over the composer/input box");
+});
+
+test("diagnostics.css: .toast-diagnostic no longer self-positions with position:fixed", () => {
+  var idx = DIAGNOSTICS_CSS.indexOf(".toast-diagnostic {");
+  assert.ok(idx !== -1, "expected .toast-diagnostic rule to exist");
+  var block = DIAGNOSTICS_CSS.slice(idx, idx + 400);
+
+  assert.ok(block.indexOf("position: fixed") === -1,
+    "individual toasts must not be position:fixed anymore -- positioning now comes from the parent #diagnostic-toast-container so multiple concurrent toasts stack instead of overlapping at the same fixed spot");
+});
+
+test("diagnostics.js: toasts are appended into the shared toast container, not document.body directly", () => {
+  var idx = DIAGNOSTICS_JS.indexOf("function _showDiagnosticToast");
+  assert.ok(idx !== -1, "expected _showDiagnosticToast to exist");
+  var block = DIAGNOSTICS_JS.slice(idx, idx + 2000);
+
+  assert.match(
+    block,
+    /_getToastContainer\(\)\.appendChild\(el\)/,
+    "_showDiagnosticToast must append into the shared stacking container, not document.body directly (lr-e901 placement/stacking defect)"
+  );
+});
+
+test("diagnostics.css: mobile media query keeps top-anchor behavior on the container (lr-2fdd intact)", () => {
+  var idx = DIAGNOSTICS_CSS.indexOf("@media (max-width: 768px)");
+  assert.ok(idx !== -1, "expected a mobile media query to exist");
+  var block = DIAGNOSTICS_CSS.slice(idx, idx + 900);
+
+  assert.match(
+    block,
+    /#diagnostic-toast-container\s*\{[^}]*top:\s*calc\(var\(--safe-top\)/,
+    "mobile must keep the lr-2fdd top-anchor fix (clearing the notch/status bar via --safe-top) on the container"
+  );
+});