Merge pull request #4077 from jeremymcs/fix/tui-notification-overlay-wiring

fix(tui): overlay subscription + Ctrl+Shift+P shortcut conflict

src/resources/extensions/gsd/bootstrap/register-shortcuts.ts

@@ -93,9 +93,6 @@ export function registerShortcuts(pi: ExtensionAPI): void {
     handler: openParallelOverlay,
   });
 
-  // Fallback for terminals where Ctrl+Alt letter chords are not forwarded reliably.
-  pi.registerShortcut(Key.ctrlShift(GSD_SHORTCUTS.parallel.key), {
-    description: shortcutDesc(`${GSD_SHORTCUTS.parallel.action} (fallback)`, GSD_SHORTCUTS.parallel.command),
-    handler: openParallelOverlay,
-  });
+  // No Ctrl+Shift+P fallback — conflicts with cycleModelBackward (shift+ctrl+p).
+  // Use Ctrl+Alt+P or /gsd parallel watch instead.
 }

src/resources/extensions/gsd/notification-overlay.ts

@@ -9,6 +9,7 @@ import {
   readNotifications,
   markAllRead,
   clearNotifications,
+  onNotificationStoreChange,
   type NotificationEntry,
   type NotifySeverity,
 } from "./notification-store.js";
@@ -82,6 +83,7 @@ export class GSDNotificationOverlay {
   private refreshTimer: ReturnType<typeof setInterval>;
   private disposed = false;
   private resizeHandler: (() => void) | null = null;
+  private unsubscribeStore: (() => void) | null = null;
 
   constructor(
     tui: { requestRender: () => void },
@@ -105,19 +107,17 @@ export class GSDNotificationOverlay {
     };
     process.stdout.on("resize", this.resizeHandler);
 
-    // Refresh every 3s for new notifications
+    // Subscribe to store mutations for immediate updates
+    this.unsubscribeStore = onNotificationStoreChange(() => {
+      if (this.disposed) return;
+      this._refreshFromDisk();
+    });
+
+    // 30s safety-net for cross-process edits (web subprocess, parallel workers)
     this.refreshTimer = setInterval(() => {
       if (this.disposed) return;
-      const fresh = readNotifications();
-      const signature = notificationSignature(fresh);
-      if (signature !== this.entriesSignature) {
-        markAllRead();
-        this.entries = readNotifications();
-        this.entriesSignature = notificationSignature(this.entries);
-        this.invalidate();
-        this.tui.requestRender();
-      }
-    }, 3000);
+      this._refreshFromDisk();
+    }, 30_000);
   }
 
   private get filter(): FilterMode {
@@ -215,12 +215,28 @@ export class GSDNotificationOverlay {
   dispose(): void {
     this.disposed = true;
     clearInterval(this.refreshTimer);
+    if (this.unsubscribeStore) {
+      this.unsubscribeStore();
+      this.unsubscribeStore = null;
+    }
     if (this.resizeHandler) {
       process.stdout.removeListener("resize", this.resizeHandler);
       this.resizeHandler = null;
     }
   }
 
+  private _refreshFromDisk(): void {
+    const fresh = readNotifications();
+    const signature = notificationSignature(fresh);
+    if (signature !== this.entriesSignature) {
+      markAllRead();
+      this.entries = readNotifications();
+      this.entriesSignature = notificationSignature(this.entries);
+      this.invalidate();
+      this.tui.requestRender();
+    }
+  }
+
   private wrapInBox(inner: string[], width: number): string[] {
     const th = this.theme;
     const border = (s: string) => th.fg("borderAccent", s);

src/resources/extensions/gsd/notification-store.ts

@@ -323,10 +323,11 @@ function _withLock<T>(basePath: string, fn: () => T): T {
     }
   }
 
-  // Only run the mutation if we actually own the lock
-  const ownsLock = fd !== null;
+  // Best-effort: mutation runs regardless of lock status (idempotent overwrites).
+  // createdLock gates cleanup only — never skip fn() on lock failure.
+  const createdLock = fd !== null;
   try {
-    if (ownsLock && fd !== null) {
+    if (createdLock && fd !== null) {
       // Write our PID timestamp into the lock for stale detection
       writeFileSync(lockPath, String(Date.now()), "utf-8");
       closeSync(fd);
@@ -334,7 +335,7 @@ function _withLock<T>(basePath: string, fn: () => T): T {
     return fn();
   } finally {
     // Only delete the lock if we created it — never remove another process's lock
-    if (ownsLock) {
+    if (createdLock) {
       try { unlinkSync(lockPath); } catch { /* best-effort cleanup */ }
     }
   }

src/resources/extensions/gsd/shortcut-defs.ts

@@ -8,6 +8,8 @@ type GSDShortcutDef = {
   key: "g" | "n" | "p";
   action: string;
   command: string;
+  /** Whether the Ctrl+Shift fallback is registered (false when it conflicts with an app keybinding). */
+  hasFallback: boolean;
 };
 
 export const GSD_SHORTCUTS: Record<GSDShortcutId, GSDShortcutDef> = {
@@ -15,16 +17,19 @@ export const GSD_SHORTCUTS: Record<GSDShortcutId, GSDShortcutDef> = {
     key: "g",
     action: "Open GSD dashboard",
     command: "/gsd status",
+    hasFallback: true,
   },
   notifications: {
     key: "n",
     action: "Open notification history",
     command: "/gsd notifications",
+    hasFallback: true,
   },
   parallel: {
     key: "p",
     action: "Open parallel worker monitor",
     command: "/gsd parallel watch",
+    hasFallback: false, // Ctrl+Shift+P conflicts with cycleModelBackward
   },
 };
 
@@ -41,7 +46,9 @@ export function fallbackShortcutCombo(id: GSDShortcutId): string {
 }
 
 export function shortcutPair(id: GSDShortcutId, formatter: (combo: string) => string = (combo) => combo): string {
-  return `${formatter(primaryShortcutCombo(id))} / ${formatter(fallbackShortcutCombo(id))}`;
+  const primary = formatter(primaryShortcutCombo(id));
+  if (!GSD_SHORTCUTS[id].hasFallback) return primary;
+  return `${primary} / ${formatter(fallbackShortcutCombo(id))}`;
 }
 
 export function formattedShortcutPair(id: GSDShortcutId): string {

src/resources/extensions/gsd/tests/format-shortcut.test.ts

@@ -82,3 +82,19 @@ test("shortcut-defs: formats shortcut pair using platform symbols", () => {
     assert.equal(pair, "Ctrl+Alt+N / Ctrl+Shift+N");
   }
 });
+
+test("shortcut-defs: parallel shortcut omits fallback (hasFallback: false)", () => {
+  const pair = formattedShortcutPair("parallel");
+  if (process.platform === "darwin") {
+    assert.equal(pair, "⌃⌥P", "parallel should only show primary combo");
+  } else {
+    assert.equal(pair, "Ctrl+Alt+P", "parallel should only show primary combo");
+  }
+  // Verify it does NOT contain the fallback separator
+  assert.ok(!pair.includes("/"), "parallel pair should not contain fallback separator");
+});
+
+test("shortcut-defs: dashboard shortcut includes fallback (hasFallback: true)", () => {
+  const pair = formattedShortcutPair("dashboard");
+  assert.ok(pair.includes("/"), "dashboard pair should contain fallback separator");
+});

src/resources/extensions/gsd/tests/register-shortcuts.test.ts

@@ -69,14 +69,15 @@ test("dashboard shortcut resolves the project root instead of the current worktr
 
   assert.ok(customCalls > 0, "shortcut opens the dashboard overlay when project root is resolved");
   assert.equal(notices.length, 0, "shortcut does not fall back to the missing-.gsd warning");
-  assert.equal(shortcuts.length, 6, "all GSD shortcuts are still registered");
+  assert.equal(shortcuts.length, 5, "all GSD shortcuts are still registered");
   const keys = shortcuts.map((shortcut) => shortcut.key);
   assert.ok(keys.includes("ctrl+alt+g"), "primary dashboard shortcut is registered");
   assert.ok(keys.includes("ctrl+shift+g"), "fallback dashboard shortcut is registered");
   assert.ok(keys.includes("ctrl+alt+n"), "primary notifications shortcut is registered");
   assert.ok(keys.includes("ctrl+shift+n"), "fallback notifications shortcut is registered");
   assert.ok(keys.includes("ctrl+alt+p"), "primary parallel shortcut is registered");
-  assert.ok(keys.includes("ctrl+shift+p"), "fallback parallel shortcut is registered");
+  // No Ctrl+Shift+P fallback — conflicts with cycleModelBackward (shift+ctrl+p)
+  assert.ok(!keys.includes("ctrl+shift+p"), "parallel fallback must not be registered (conflicts with cycleModelBackward)");
 });
 
 test("parallel shortcut passes resolved project root into overlay", async (t) => {