feat: structured notification event model with metadata-first classification
Replace brittle string-matching in headless-events.ts with structured
source/kind/blocking/dedupe_key metadata on notify() events. String
matching is preserved as a fallback for the ~940 untagged call sites.
- Add NotificationMetadata type to headless-types.ts (canonical definition)
- Extend rpc-types.ts notify event with optional metadata field
- Extend ExtensionUIContext.notify() signature with optional 3rd arg
- Pass metadata through RPC notify implementation in rpc-mode.ts
- Update headless-events.ts: isTerminalNotification, isBlockedNotification,
isMilestoneReadyNotification, isPauseNotification all check metadata first
- Update notification-store.ts: store metadata on NotificationEntry; use
metadata.dedupe_key as dedup key when provided (falls back to message hash)
- Update notify-interceptor.ts to thread metadata through to store + original
- Tag critical emit sites with structured metadata:
stopAuto → { kind: "terminal" } (+ blocking: true when reason includes "block")
pauseAuto → { kind: "terminal", blocking: true }
guided-flow milestone ready → { kind: "approval_request", blocking: true }
- Update notification-overlay.ts to prefer metadata.source for [label] display
- Add 17-test regression suite (notification-event-model.test.ts)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
6f877b61ab
commit
a055b3adf2
11 changed files with 323 additions and 10 deletions
|
|
@ -120,7 +120,16 @@ export interface ExtensionUIContext {
|
|||
input(title: string, placeholder?: string, opts?: ExtensionUIDialogOptions): Promise<string | undefined>;
|
||||
|
||||
/** 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;
|
||||
|
|
|
|||
|
|
@ -245,7 +245,7 @@ export async function runRpcMode(session: AgentSession): Promise<never> {
|
|||
"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<string, unknown>): 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<never> {
|
|||
method: "notify",
|
||||
message,
|
||||
notifyType: type,
|
||||
...(metadata ? { metadata } : {}),
|
||||
} as RpcExtensionUIRequest);
|
||||
void withEmbeddedUiContext((ui) => {
|
||||
ui.notify(message, type);
|
||||
|
|
|
|||
|
|
@ -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";
|
||||
|
|
|
|||
|
|
@ -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<string, unknown>,
|
||||
): Record<string, unknown> | undefined {
|
||||
const meta = event.metadata;
|
||||
if (meta == null || typeof meta !== "object" || Array.isArray(meta))
|
||||
return undefined;
|
||||
return meta as Record<string, unknown>;
|
||||
}
|
||||
|
||||
export function isTerminalNotification(
|
||||
event: Record<string, unknown>,
|
||||
): 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<string, unknown>): 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<string, unknown>): 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 ?? ""));
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -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" },
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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<string, unknown>);
|
||||
};
|
||||
|
||||
_wrappedContexts.add(ctx.ui);
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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} `;
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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<string, unknown>,
|
||||
): Record<string, unknown> {
|
||||
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);
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue