diff --git a/packages/pi-tui/src/__tests__/overlay-layout.test.ts b/packages/pi-tui/src/__tests__/overlay-layout.test.ts new file mode 100644 index 000000000..4f7d7817c --- /dev/null +++ b/packages/pi-tui/src/__tests__/overlay-layout.test.ts @@ -0,0 +1,65 @@ +// pi-tui — Overlay Layout Tests (backdrop dimming) + +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; +import { compositeOverlays, type OverlayEntry } from "../overlay-layout.js"; + +function makeEntry( + lines: string[], + options?: OverlayEntry["options"], +): OverlayEntry { + return { + component: { render: () => lines }, + options, + hidden: false, + focusOrder: 1, + }; +} + +describe("compositeOverlays — backdrop", () => { + it("dims base lines when backdrop is true", () => { + const base = ["hello world", "second line"]; + const overlay = makeEntry(["OVERLAY"], { + width: 7, + anchor: "top-left", + backdrop: true, + }); + + const result = compositeOverlays(base, [overlay], 20, 20, 2); + + // All base lines in viewport should contain dim escape (\x1b[2m) + // The overlay line itself is composited on top, but underlying lines get dimmed + const dimmedLine = result.find((l) => l.includes("second line")); + assert.ok(dimmedLine, "should have a line containing 'second line'"); + assert.ok(dimmedLine.includes("\x1b[2m"), "base line should be dimmed"); + }); + + it("does not dim when backdrop is false/absent", () => { + const base = ["hello world", "second line"]; + const overlay = makeEntry(["OVERLAY"], { + width: 7, + anchor: "top-left", + }); + + const result = compositeOverlays(base, [overlay], 20, 20, 2); + + // Lines not covered by overlay should remain undimmed + const secondLine = result.find((l) => l.includes("second line")); + assert.ok(secondLine, "should have a line containing 'second line'"); + assert.ok(!secondLine.includes("\x1b[2m"), "base line should not be dimmed"); + }); + + it("overlay content renders on top of dimmed background", () => { + const base = ["aaaaaaaaaa"]; + const overlay = makeEntry(["XX"], { + width: 2, + anchor: "top-left", + backdrop: true, + }); + + const result = compositeOverlays(base, [overlay], 10, 10, 1); + + // The first line should contain the overlay text + assert.ok(result[0].includes("XX"), "overlay text should be composited"); + }); +}); diff --git a/packages/pi-tui/src/overlay-layout.ts b/packages/pi-tui/src/overlay-layout.ts index 1896c5bba..d3c2d8b6a 100644 --- a/packages/pi-tui/src/overlay-layout.ts +++ b/packages/pi-tui/src/overlay-layout.ts @@ -6,7 +6,7 @@ */ import type { OverlayAnchor, OverlayOptions, SizeValue } from "./tui.js"; -import { extractSegments, sliceByColumn, sliceWithWidth, truncateToWidth, visibleWidth } from "./utils.js"; +import { applyBackgroundToLine, extractSegments, sliceByColumn, sliceWithWidth, truncateToWidth, visibleWidth } from "./utils.js"; import { isImageLine } from "./terminal-image.js"; import { CURSOR_MARKER } from "./tui.js"; @@ -324,6 +324,17 @@ export function compositeOverlays( const viewportStart = Math.max(0, workingHeight - termHeight); + // Apply backdrop dimming if any visible overlay requests it + const hasBackdrop = visibleEntries.some((e) => e.options?.backdrop); + if (hasBackdrop) { + const dimFn = (text: string) => `\x1b[2m${text}\x1b[22m`; + for (let i = viewportStart; i < result.length; i++) { + if (!isImageLine(result[i]) && result[i].length > 0) { + result[i] = applyBackgroundToLine(result[i], termWidth, dimFn); + } + } + } + // Composite each overlay for (const { overlayLines, row, col, w } of rendered) { for (let i = 0; i < overlayLines.length; i++) { diff --git a/packages/pi-tui/src/tui.ts b/packages/pi-tui/src/tui.ts index 493c1c727..7a078f2d3 100644 --- a/packages/pi-tui/src/tui.ts +++ b/packages/pi-tui/src/tui.ts @@ -141,6 +141,8 @@ export interface OverlayOptions { visible?: (termWidth: number, termHeight: number) => boolean; /** If true, don't capture keyboard focus when shown */ nonCapturing?: boolean; + /** If true, dim the background behind the overlay */ + backdrop?: boolean; } /** diff --git a/src/resources/extensions/gsd/bootstrap/register-shortcuts.ts b/src/resources/extensions/gsd/bootstrap/register-shortcuts.ts index 9abe3fbb8..1e0faf7a0 100644 --- a/src/resources/extensions/gsd/bootstrap/register-shortcuts.ts +++ b/src/resources/extensions/gsd/bootstrap/register-shortcuts.ts @@ -44,6 +44,7 @@ export function registerShortcuts(pi: ExtensionAPI): void { minWidth: 60, maxHeight: "88%", anchor: "center", + backdrop: true, }, }, ); diff --git a/src/resources/extensions/gsd/commands/handlers/notifications-handler.ts b/src/resources/extensions/gsd/commands/handlers/notifications-handler.ts index e66309b95..16d30d49a 100644 --- a/src/resources/extensions/gsd/commands/handlers/notifications-handler.ts +++ b/src/resources/extensions/gsd/commands/handlers/notifications-handler.ts @@ -105,6 +105,7 @@ export async function handleNotificationsCommand( minWidth: 60, maxHeight: "88%", anchor: "center", + backdrop: true, }, }, ); diff --git a/src/resources/extensions/gsd/notification-overlay.ts b/src/resources/extensions/gsd/notification-overlay.ts index ec25c440a..6ab73f729 100644 --- a/src/resources/extensions/gsd/notification-overlay.ts +++ b/src/resources/extensions/gsd/notification-overlay.ts @@ -164,6 +164,11 @@ export class GSDNotificationOverlay { this.scrollOffset = Math.min(this.scrollOffset, maxScroll); const visibleContent = content.slice(this.scrollOffset, this.scrollOffset + visibleContentRows); + // Pad to fill viewport so the overlay covers underlying content + while (visibleContent.length < visibleContentRows) { + visibleContent.push(""); + } + const lines = this.wrapInBox(visibleContent, width); this.cachedWidth = width; diff --git a/src/resources/extensions/gsd/notification-store.ts b/src/resources/extensions/gsd/notification-store.ts index 54a600061..d79d4a33c 100644 --- a/src/resources/extensions/gsd/notification-store.ts +++ b/src/resources/extensions/gsd/notification-store.ts @@ -275,14 +275,19 @@ function _withLock(basePath: string, fn: () => T): T { } } + // Only run the mutation if we actually own the lock + const ownsLock = fd !== null; try { - // Write our PID timestamp into the lock for stale detection - if (fd !== null) { + if (ownsLock && fd !== null) { + // Write our PID timestamp into the lock for stale detection writeFileSync(lockPath, String(Date.now()), "utf-8"); closeSync(fd); } return fn(); } finally { - try { unlinkSync(lockPath); } catch { /* best-effort cleanup */ } + // Only delete the lock if we created it — never remove another process's lock + if (ownsLock) { + try { unlinkSync(lockPath); } catch { /* best-effort cleanup */ } + } } } diff --git a/src/resources/extensions/gsd/tests/notification-store.test.ts b/src/resources/extensions/gsd/tests/notification-store.test.ts index 2d8dd105a..8f13fb873 100644 --- a/src/resources/extensions/gsd/tests/notification-store.test.ts +++ b/src/resources/extensions/gsd/tests/notification-store.test.ts @@ -2,7 +2,7 @@ import { describe, test, beforeEach, afterEach } from "node:test"; import assert from "node:assert/strict"; -import { mkdtempSync, mkdirSync, rmSync, readFileSync, existsSync } from "node:fs"; +import { mkdtempSync, mkdirSync, rmSync, readFileSync, existsSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; @@ -246,4 +246,37 @@ describe("notification-store", () => { assert.equal(getUnreadCount(), 0); assert.equal(getLineCount(), 0); }); + + test("markAllRead does not delete a foreign lock file", () => { + initNotificationStore(tmp); + appendNotification("msg1", "info"); + + // Simulate another process holding the lock + const lockPath = join(tmp, ".gsd", "notifications.lock"); + writeFileSync(lockPath, String(Date.now()), "utf-8"); + + // markAllRead should still work (best-effort) but not delete the foreign lock + markAllRead(); + + assert.ok(existsSync(lockPath), "foreign lock file should not be deleted"); + + // Clean up the lock so afterEach doesn't leave artifacts + rmSync(lockPath, { force: true }); + }); + + test("clearNotifications does not delete a foreign lock file", () => { + initNotificationStore(tmp); + appendNotification("msg1", "info"); + + // Simulate another process holding the lock + const lockPath = join(tmp, ".gsd", "notifications.lock"); + writeFileSync(lockPath, String(Date.now()), "utf-8"); + + // clearNotifications should still work but not delete the foreign lock + clearNotifications(); + + assert.ok(existsSync(lockPath), "foreign lock file should not be deleted"); + + rmSync(lockPath, { force: true }); + }); });