diff --git a/packages/pi-coding-agent/src/core/extensions/types.ts b/packages/pi-coding-agent/src/core/extensions/types.ts index a533323ce..2d4cd6eaa 100644 --- a/packages/pi-coding-agent/src/core/extensions/types.ts +++ b/packages/pi-coding-agent/src/core/extensions/types.ts @@ -120,7 +120,16 @@ export interface ExtensionUIContext { input(title: string, placeholder?: string, opts?: ExtensionUIDialogOptions): Promise; /** Show a notification to the user. */ - notify(message: string, type?: "info" | "warning" | "error" | "success"): void; + notify( + message: string, + type?: "info" | "warning" | "error" | "success", + metadata?: { + kind?: "notice" | "approval_request" | "progress" | "terminal"; + blocking?: boolean; + dedupe_key?: string; + source?: string; + }, + ): void; /** Listen to raw terminal input (interactive mode only). Returns an unsubscribe function. */ onTerminalInput(handler: TerminalInputHandler): () => void; diff --git a/packages/pi-coding-agent/src/modes/rpc/rpc-mode.ts b/packages/pi-coding-agent/src/modes/rpc/rpc-mode.ts index e6666f3a7..c01d04088 100644 --- a/packages/pi-coding-agent/src/modes/rpc/rpc-mode.ts +++ b/packages/pi-coding-agent/src/modes/rpc/rpc-mode.ts @@ -245,7 +245,7 @@ export async function runRpcMode(session: AgentSession): Promise { "cancelled" in r && r.cancelled ? undefined : "value" in r ? r.value : undefined, ), - notify(message: string, type?: "info" | "warning" | "error" | "success"): void { + notify(message: string, type?: "info" | "warning" | "error" | "success", metadata?: Record): void { startupNotifications.push({ message, type }); if (startupNotifications.length > 20) { startupNotifications.splice(0, startupNotifications.length - 20); @@ -257,6 +257,7 @@ export async function runRpcMode(session: AgentSession): Promise { method: "notify", message, notifyType: type, + ...(metadata ? { metadata } : {}), } as RpcExtensionUIRequest); void withEmbeddedUiContext((ui) => { ui.notify(message, type); diff --git a/packages/rpc-client/src/rpc-types.ts b/packages/rpc-client/src/rpc-types.ts index be8bca73b..eb9be0acf 100644 --- a/packages/rpc-client/src/rpc-types.ts +++ b/packages/rpc-client/src/rpc-types.ts @@ -362,6 +362,13 @@ export type RpcExtensionUIRequest = method: "notify"; message: string; notifyType?: "info" | "warning" | "error"; + /** Optional structured metadata for metadata-first classification in headless-events.ts. */ + metadata?: { + kind?: "notice" | "approval_request" | "progress" | "terminal"; + blocking?: boolean; + dedupe_key?: string; + source?: string; + }; } | { type: "extension_ui_request"; diff --git a/src/headless-events.ts b/src/headless-events.ts index a93f84373..8a64e96c0 100644 --- a/src/headless-events.ts +++ b/src/headless-events.ts @@ -109,11 +109,29 @@ const INTERACTIVE_HEADLESS_TOOLS = new Set([ "secure_env_collect", ]); +/** + * Extract structured metadata from a notify event. + * Returns undefined when absent or malformed, so callers fall through to + * the legacy string-matching heuristics. + */ +function getEventMetadata( + event: Record, +): Record | undefined { + const meta = event.metadata; + if (meta == null || typeof meta !== "object" || Array.isArray(meta)) + return undefined; + return meta as Record; +} + export function isTerminalNotification( event: Record, ): boolean { if (event.type !== "extension_ui_request" || event.method !== "notify") return false; + // Structured metadata takes precedence over text matching. + const meta = getEventMetadata(event); + if (meta?.kind === "terminal") return true; + // Fallback: legacy text heuristics for untagged notifications. const message = String(event.message ?? "").toLowerCase(); return TERMINAL_PREFIXES.some((prefix) => message.startsWith(prefix)); } @@ -121,6 +139,10 @@ export function isTerminalNotification( export function isPauseNotification(event: Record): boolean { if (event.type !== "extension_ui_request" || event.method !== "notify") return false; + // Structured: a terminal+blocking notice is a pause. + const meta = getEventMetadata(event); + if (meta?.kind === "terminal" && meta.blocking === true) return true; + // Fallback: legacy text heuristics. const message = String(event.message ?? "").toLowerCase(); return ( message.startsWith("auto-mode paused") || @@ -139,8 +161,11 @@ export function isAutoResumeScheduledNotification( export function isBlockedNotification(event: Record): boolean { if (event.type !== "extension_ui_request" || event.method !== "notify") return false; + // Structured: explicit blocking flag, excluding non-blocking progress notices. + const meta = getEventMetadata(event); + if (meta?.blocking === true && meta.kind !== "progress") return true; + // Fallback: legacy text heuristics. const message = String(event.message ?? "").toLowerCase(); - // Blocked notifications come through stopAuto as "Auto-mode stopped (Blocked: ...)" return message.includes("blocked:") || isPauseNotification(event); } @@ -149,6 +174,10 @@ export function isMilestoneReadyNotification( ): boolean { if (event.type !== "extension_ui_request" || event.method !== "notify") return false; + // Structured: approval_request+blocking is the milestone-ready signal. + const meta = getEventMetadata(event); + if (meta?.kind === "approval_request" && meta.blocking === true) return true; + // Fallback: legacy text heuristics. return isMilestoneReadyText(String(event.message ?? "")); } diff --git a/src/headless-types.ts b/src/headless-types.ts index 589cf8680..010f86e14 100644 --- a/src/headless-types.ts +++ b/src/headless-types.ts @@ -8,6 +8,30 @@ * Consumer: headless.ts (orchestrator), external CI scripts parsing batch JSON. */ +// --------------------------------------------------------------------------- +// Notification Metadata +// --------------------------------------------------------------------------- + +/** + * Optional structured metadata that can be attached to a notify() call to + * allow classification without brittle string matching. + * + * When present, headless-events.ts checks these fields first and falls back + * to the legacy text heuristics only for untagged (metadata-absent) events. + * + * Consumer: headless-events.ts classifiers, notification-store.ts dedup. + */ +export interface NotificationMetadata { + /** Semantic kind of this notification. */ + kind?: "notice" | "approval_request" | "progress" | "terminal"; + /** Whether this notification halts the workflow and requires user action. */ + blocking?: boolean; + /** Stable key for deduplication — collapses repeated notices of the same type. */ + dedupe_key?: string; + /** Emission source label (extension name, "workflow", etc.). */ + source?: string; +} + // --------------------------------------------------------------------------- // Output Format // --------------------------------------------------------------------------- diff --git a/src/resources/extensions/sf/auto.ts b/src/resources/extensions/sf/auto.ts index 5998f1d80..6c18d24a0 100644 --- a/src/resources/extensions/sf/auto.ts +++ b/src/resources/extensions/sf/auto.ts @@ -948,15 +948,27 @@ export async function stopAuto( // ── Step 8: Ledger notification ── try { + // Tag with structured metadata so headless-events.ts classifies via + // metadata.kind rather than text matching. blocking=true when the + // stop reason includes "blocked" (e.g. write-gate, guardrail block). + const isBlocked = + reason !== undefined && + reason.toLowerCase().includes("block"); + const stopMeta = { + kind: "terminal" as const, + ...(isBlocked ? { blocking: true } : {}), + source: "workflow", + }; const ledger = getLedger(); if (ledger && ledger.units.length > 0) { const totals = getProjectTotals(ledger.units); ctx?.ui.notify( `Auto-mode stopped${reasonSuffix}. Session: ${formatCost(totals.cost)} · ${formatTokenCount(totals.tokens.total)} tokens · ${ledger.units.length} units`, "info", + stopMeta, ); } else { - ctx?.ui.notify(`Auto-mode stopped${reasonSuffix}.`, "info"); + ctx?.ui.notify(`Auto-mode stopped${reasonSuffix}.`, "info", stopMeta); } } catch (e) { debugLog("stop-cleanup-ledger", { @@ -1259,6 +1271,7 @@ export async function pauseAuto( ctx?.ui.notify( `${s.stepMode ? "Step" : "Autonomous"} mode paused (Escape). Type to interact, or ${resumeCmd} to resume.`, "info", + { kind: "terminal", blocking: true, source: "workflow" }, ); } diff --git a/src/resources/extensions/sf/bootstrap/notify-interceptor.ts b/src/resources/extensions/sf/bootstrap/notify-interceptor.ts index e3b446897..39f1fd819 100644 --- a/src/resources/extensions/sf/bootstrap/notify-interceptor.ts +++ b/src/resources/extensions/sf/bootstrap/notify-interceptor.ts @@ -7,6 +7,7 @@ import type { ExtensionContext } from "@singularity-forge/pi-coding-agent"; import { appendNotification, + type NotificationMetadata, type NotifySeverity, } from "../notification-store.js"; @@ -27,13 +28,19 @@ export function installNotifyInterceptor(ctx: ExtensionContext): void { (ctx.ui as any).notify = ( message: string, type?: "info" | "warning" | "error" | "success", + metadata?: NotificationMetadata, ): void => { try { - appendNotification(message, (type ?? "info") as NotifySeverity, "notify"); + appendNotification( + message, + (type ?? "info") as NotifySeverity, + "notify", + metadata, + ); } catch { // Non-fatal — never let persistence break the UI } - originalNotify(message, type); + originalNotify(message, type, metadata as Record); }; _wrappedContexts.add(ctx.ui); diff --git a/src/resources/extensions/sf/guided-flow.ts b/src/resources/extensions/sf/guided-flow.ts index f6e2ed690..874387acb 100644 --- a/src/resources/extensions/sf/guided-flow.ts +++ b/src/resources/extensions/sf/guided-flow.ts @@ -395,7 +395,12 @@ export function checkAutoStartAfterDiscuss(): boolean { } pendingAutoStartMap.delete(basePath); - ctx.ui.notify(`Milestone ${milestoneId} ready.`, "info"); + ctx.ui.notify(`Milestone ${milestoneId} ready.`, "info", { + kind: "approval_request", + blocking: true, + source: "workflow", + dedupe_key: `milestone-ready:${milestoneId}`, + }); startAutoDetached(ctx, pi, basePath, false, { step }); return true; } diff --git a/src/resources/extensions/sf/notification-overlay.ts b/src/resources/extensions/sf/notification-overlay.ts index 6af7b6eeb..0d7277316 100644 --- a/src/resources/extensions/sf/notification-overlay.ts +++ b/src/resources/extensions/sf/notification-overlay.ts @@ -357,8 +357,10 @@ export class SFNotificationOverlay { ? th.fg("success", icon) : th.fg("dim", icon); const time = th.fg("dim", formatTimestamp(entry.ts)); - const source = - entry.source === "workflow-logger" ? th.fg("dim", " [engine]") : ""; + const sourceLabel = + entry.metadata?.source ?? + (entry.source === "workflow-logger" ? "engine" : undefined); + const source = sourceLabel ? th.fg("dim", ` [${sourceLabel}]`) : ""; // Measure actual prefix width for wrapping const prefix = `${coloredIcon} ${time}${source} `; diff --git a/src/resources/extensions/sf/notification-store.ts b/src/resources/extensions/sf/notification-store.ts index 2852d3ba6..b9af718ae 100644 --- a/src/resources/extensions/sf/notification-store.ts +++ b/src/resources/extensions/sf/notification-store.ts @@ -22,6 +22,13 @@ import { join } from "node:path"; export type NotifySeverity = "info" | "success" | "warning" | "error"; export type NotificationSource = "notify" | "workflow-logger"; +export interface NotificationMetadata { + kind?: "notice" | "approval_request" | "progress" | "terminal"; + blocking?: boolean; + dedupe_key?: string; + source?: string; +} + export interface NotificationEntry { id: string; ts: string; @@ -29,6 +36,7 @@ export interface NotificationEntry { message: string; source: NotificationSource; read: boolean; + metadata?: NotificationMetadata; } // ─── Constants ────────────────────────────────────────────────────────── @@ -70,12 +78,16 @@ export function appendNotification( message: string, severity: NotifySeverity, source: NotificationSource = "notify", + metadata?: NotificationMetadata, ): void { if (!_basePath) return; if (_suppressCount > 0) return; const persistedMessage = message.length > 500 ? message.slice(0, 500) + "…" : message; - const dedupKey = `${_basePath}:${severity}:${source}:${persistedMessage}`; + // Use explicit dedupe_key when provided; fall back to message-hash based key. + const dedupKey = metadata?.dedupe_key + ? `${_basePath}:${metadata.dedupe_key}` + : `${_basePath}:${severity}:${source}:${persistedMessage}`; const now = Date.now(); const lastSeen = _recentMessageTimestamps.get(dedupKey); if (lastSeen !== undefined && now - lastSeen < DEDUP_WINDOW_MS) return; @@ -93,6 +105,7 @@ export function appendNotification( message: persistedMessage, source, read: false, + ...(metadata ? { metadata } : {}), }; try { diff --git a/src/resources/extensions/sf/tests/notification-event-model.test.ts b/src/resources/extensions/sf/tests/notification-event-model.test.ts new file mode 100644 index 000000000..120288aa0 --- /dev/null +++ b/src/resources/extensions/sf/tests/notification-event-model.test.ts @@ -0,0 +1,203 @@ +/** + * Notification Event Model — regression tests + * + * Verifies that: + * 1. headless-events.ts classification functions use metadata fields first. + * 2. String-matching fallbacks still work for untagged notifications. + * 3. notification-store deduplicates by dedupe_key when provided. + * 4. Automated (non-blocking, non-terminal) notices do NOT produce terminal/blocked results. + */ + +import assert from "node:assert/strict"; +import { mkdirSync, mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, test } from "node:test"; + +import { + isBlockedNotification, + isMilestoneReadyNotification, + isPauseNotification, + isTerminalNotification, +} from "../../../../headless-events.ts"; +import { + appendNotification, + initNotificationStore, + readNotifications, +} from "../notification-store.ts"; + +// ── Helpers ────────────────────────────────────────────────────────────────── + +function notifyEvent( + message: string, + metadata?: Record, +): Record { + return { + type: "extension_ui_request", + method: "notify", + message, + ...(metadata ? { metadata } : {}), + }; +} + +// ── headless-events: metadata-first classification ──────────────────────────── + +describe("isTerminalNotification — metadata-first", () => { + test("returns true when metadata.kind === 'terminal', even with non-matching text", () => { + const event = notifyEvent("Some unrelated message.", { kind: "terminal" }); + assert.equal(isTerminalNotification(event), true); + }); + + test("returns true for legacy 'auto-mode stopped' prefix (fallback)", () => { + const event = notifyEvent("Auto-mode stopped. Session: ..."); + assert.equal(isTerminalNotification(event), true); + }); + + test("returns true for legacy 'step-mode stopped' prefix (fallback)", () => { + const event = notifyEvent("Step-mode stopped."); + assert.equal(isTerminalNotification(event), true); + }); + + test("returns false for unrelated notification without metadata", () => { + const event = notifyEvent("Slice S01 complete."); + assert.equal(isTerminalNotification(event), false); + }); + + test("returns false for progress metadata (non-terminal)", () => { + const event = notifyEvent("Compiling...", { kind: "progress" }); + assert.equal(isTerminalNotification(event), false); + }); +}); + +describe("isBlockedNotification — metadata-first", () => { + test("returns true when metadata.blocking === true", () => { + const event = notifyEvent("Write gate prevented this action.", { + blocking: true, + kind: "notice", + }); + assert.equal(isBlockedNotification(event), true); + }); + + test("returns false for progress notice even with blocking=true", () => { + const event = notifyEvent("Loading...", { + blocking: true, + kind: "progress", + }); + assert.equal(isBlockedNotification(event), false); + }); + + test("returns true for legacy 'blocked:' text (fallback)", () => { + const event = notifyEvent("Auto-mode stopped — Blocked: write gate."); + assert.equal(isBlockedNotification(event), true); + }); + + test("returns false for non-blocking automated notice (the key regression)", () => { + // An automated status notice must NOT be classified as blocked. + const event = notifyEvent("Iteration 3 complete. Running verification...", { + kind: "progress", + source: "workflow", + }); + assert.equal(isBlockedNotification(event), false); + }); +}); + +describe("isMilestoneReadyNotification — metadata-first", () => { + test("returns true when metadata.kind === 'approval_request' and blocking === true", () => { + const event = notifyEvent("Milestone M001 ready.", { + kind: "approval_request", + blocking: true, + }); + assert.equal(isMilestoneReadyNotification(event), true); + }); + + test("returns false when approval_request but blocking not set (text doesn't match fallback)", () => { + // Use text that doesn't match the legacy regex so we can test metadata-only path. + const event = notifyEvent("Review needed.", { + kind: "approval_request", + }); + assert.equal(isMilestoneReadyNotification(event), false); + }); + + test("returns true for legacy 'Milestone M001 ready' text (fallback)", () => { + const event = notifyEvent("Milestone M001 is ready for review."); + assert.equal(isMilestoneReadyNotification(event), true); + }); +}); + +describe("isPauseNotification — metadata-first", () => { + test("returns true when metadata.kind=terminal and blocking=true", () => { + const event = notifyEvent( + "Autonomous mode paused. Type to interact.", + { kind: "terminal", blocking: true }, + ); + assert.equal(isPauseNotification(event), true); + }); +}); + +// ── notification-store: dedupe_key deduplication ───────────────────────────── + +describe("notification-store — dedupe_key", () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = mkdtempSync(join(tmpdir(), "sf-notif-test-")); + mkdirSync(join(tmpDir, ".sf"), { recursive: true }); + initNotificationStore(tmpDir); + }); + + afterEach(() => { + rmSync(tmpDir, { recursive: true, force: true }); + }); + + test("deduplicates by dedupe_key within the window", () => { + appendNotification("Syncing...", "info", "notify", { + dedupe_key: "sync:progress", + }); + appendNotification("Syncing with different text...", "info", "notify", { + dedupe_key: "sync:progress", + }); + const entries = readNotifications(tmpDir); + assert.equal(entries.length, 1, "second entry with same dedupe_key should be dropped"); + }); + + test("does not deduplicate across different dedupe_keys", () => { + appendNotification("Syncing A...", "info", "notify", { + dedupe_key: "sync:A", + }); + appendNotification("Syncing B...", "info", "notify", { + dedupe_key: "sync:B", + }); + const entries = readNotifications(tmpDir); + assert.equal(entries.length, 2, "different dedupe_keys should produce separate entries"); + }); + + test("stores metadata on the entry", () => { + appendNotification("Ready.", "info", "notify", { + kind: "approval_request", + blocking: true, + source: "workflow", + }); + const entries = readNotifications(tmpDir); + assert.equal(entries.length, 1); + assert.equal(entries[0].metadata?.kind, "approval_request"); + assert.equal(entries[0].metadata?.blocking, true); + assert.equal(entries[0].metadata?.source, "workflow"); + }); + + test("automated progress notice does not affect blocking classification", () => { + appendNotification("Running checks...", "info", "notify", { + kind: "progress", + source: "workflow", + }); + const entries = readNotifications(tmpDir); + assert.equal(entries.length, 1); + // The notice is stored, but kind=progress means headless will not treat it as blocked. + assert.equal(entries[0].metadata?.kind, "progress"); + // Confirm headless classification: this event should NOT be blocked + const fakeEvent = notifyEvent("Running checks...", { + kind: "progress", + source: "workflow", + }); + assert.equal(isBlockedNotification(fakeEvent), false); + }); +});