Improve OMP state compatibility and advisor rendering (#2219)

* Accept OMP session state without thinkingLevel Models whose reasoning effort is encoded in the model ID (e.g. cursor-grok-4.5-high-fast) are marked reasoning: false by OMP, which omits thinkingLevel from get_state. The required schema field made session creation throw a ZodError. Make it optional and resolve the thinking option to null when absent. * Render OMP advisor messages as blocks
2026-07-23 03:25:12 +08:00
parent 512c9b31a8
commit 10da5ca169
9 changed files with 243 additions and 8 deletions
@@ -0,0 +1,98 @@
+import { createHash } from "node:crypto";
+
+import type { AgentTimelineItem } from "../../agent-sdk-types.js";
+import type { OmpAgentMessage } from "./rpc-types.js";
+
+type OmpCustomMessage = Extract<OmpAgentMessage, { role: "custom" }>;
+type OmpAdvisorToolCallItem = Extract<AgentTimelineItem, { type: "tool_call" }>;
+type OmpAdvisorSeverity = "nit" | "concern" | "blocker";
+
+interface OmpAdvisorNote {
+  note: string;
+  severity?: OmpAdvisorSeverity;
+  advisor?: string;
+}
+
+const ADVISOR_SEVERITIES: Record<OmpAdvisorSeverity, true> = {
+  nit: true,
+  concern: true,
+  blocker: true,
+};
+
+function readOptionalString(value: unknown): string | undefined {
+  return typeof value === "string" && value.trim() ? value.trim() : undefined;
+}
+
+function readAdvisorNotes(message: OmpCustomMessage): OmpAdvisorNote[] {
+  const details = Reflect.get(message, "details");
+  if (!details || typeof details !== "object") return [];
+  const notes = Reflect.get(details, "notes");
+  if (!Array.isArray(notes)) return [];
+
+  return notes.flatMap((value): OmpAdvisorNote[] => {
+    if (!value || typeof value !== "object") return [];
+    const note = readOptionalString(Reflect.get(value, "note"));
+    if (!note) return [];
+    const rawSeverity = Reflect.get(value, "severity");
+    const severity =
+      typeof rawSeverity === "string" && rawSeverity in ADVISOR_SEVERITIES
+        ? (rawSeverity as OmpAdvisorSeverity)
+        : undefined;
+    const advisor = readOptionalString(Reflect.get(value, "advisor"));
+    return [{ note, ...(severity ? { severity } : {}), ...(advisor ? { advisor } : {}) }];
+  });
+}
+
+function formatAdvisorNote(note: OmpAdvisorNote): string {
+  const prefix = [
+    note.severity ? `[${note.severity}]` : null,
+    note.advisor ? `[${note.advisor}]` : null,
+  ]
+    .filter(Boolean)
+    .join(" ");
+  return prefix ? `${prefix} ${note.note}` : note.note;
+}
+
+function buildAdvisorLabel(noteCount: number, blockerCount: number): string {
+  if (noteCount === 0) return "Advisor";
+  const label = `Advisor · ${noteCount} ${noteCount === 1 ? "note" : "notes"}`;
+  return blockerCount > 0
+    ? `${label} · ${blockerCount} ${blockerCount === 1 ? "blocker" : "blockers"}`
+    : label;
+}
+
+function buildAdvisorCallId(message: OmpCustomMessage, text: string): string {
+  const id = readOptionalString(Reflect.get(message, "id"));
+  if (id) return `omp-advisor:${id}`;
+  const digest = createHash("sha1").update(text.trim()).digest("hex").slice(0, 12);
+  return `omp-advisor:${digest}`;
+}
+
+export function mapOmpAdvisorMessageToToolCall(
+  message: OmpCustomMessage,
+  text: string,
+): OmpAdvisorToolCallItem | null {
+  if (Reflect.get(message, "customType") !== "advisor") return null;
+
+  const notes = readAdvisorNotes(message);
+  const blockerCount = notes.filter((note) => note.severity === "blocker").length;
+  return {
+    type: "tool_call",
+    callId: buildAdvisorCallId(message, text),
+    name: "advisor",
+    status: "completed",
+    detail: {
+      type: "plain_text",
+      label: buildAdvisorLabel(notes.length, blockerCount),
+      text: notes.length > 0 ? notes.map(formatAdvisorNote).join("\n\n") : text,
+      icon: "brain",
+    },
+    metadata: {
+      synthetic: true,
+      source: "omp_advisor",
+      noteCount: notes.length,
+      blockerCount,
+    },
+    error: null,
+  };
+}
@@ -88,6 +88,50 @@ describe("OMP agent client and session", () => {
    expect(omp.completedTurnCount()).toBe(1);
  });

+  test("streams OMP advisor messages as distinct tool-call blocks", async () => {
+    const omp = new OmpHarness();
+    await omp.start();
+
+    await omp.runPromptWithCustomMessage(
+      "review this",
+      {
+        role: "custom",
+        content: '<advisory severity="concern">Exercise the failure path.</advisory>',
+        customType: "advisor",
+        id: "advisor-live-1",
+        display: true,
+        details: {
+          notes: [{ note: "Exercise the failure path.", severity: "concern" }],
+        },
+      },
+      "fixed",
+    );
+
+    expect(omp.timeline()).toEqual([
+      { type: "user_message", text: "review this", messageId: "user-1" },
+      {
+        type: "tool_call",
+        callId: "omp-advisor:advisor-live-1",
+        name: "advisor",
+        status: "completed",
+        detail: {
+          type: "plain_text",
+          label: "Advisor · 1 note",
+          text: "[concern] Exercise the failure path.",
+          icon: "brain",
+        },
+        metadata: {
+          synthetic: true,
+          source: "omp_advisor",
+          noteCount: 1,
+          blockerCount: 0,
+        },
+        error: null,
+      },
+      { type: "assistant_message", text: "fixed", messageId: "omp-assistant-1" },
+    ]);
+  });
+
  test("does not accept a follow-up until OMP reports stable idle", async () => {
    const omp = new OmpHarness();
    await omp.start();
@@ -91,6 +91,7 @@ import { mapOmpAvailableCommandsUpdate, mapOmpRuntimeSlashCommands } from "./com
 import { streamOmpHistory } from "./history.js";
 import { mapOmpTodoReminderEvent, mapOmpTodoState, mapOmpTodoToolResult } from "./todo-mapper.js";
 import { mapOmpRuntimeEventToTimelineItem } from "./event-mapper.js";
+import { mapOmpAdvisorMessageToToolCall } from "./advisor-message.js";
 import {
  clearOmpHostToolState,
  handleOmpHostToolRuntimeEvent,
@@ -511,7 +512,7 @@ function isOmpRequestAbortError(error: unknown): boolean {

 function resolveThinkingOptionId(
  cachedThinkingOptionId: string | null,
-  sessionThinkingLevel: OmpThinkingLevel,
+  sessionThinkingLevel: OmpThinkingLevel | undefined,
 ): OmpThinkingLevel | null {
  const currentThinking = cachedThinkingOptionId ?? sessionThinkingLevel;
  return normalizeOmpThinkingOption(currentThinking);
@@ -1987,11 +1988,12 @@ export class OmpAgentSession implements AgentSession {
    if (event.message.role === "custom") {
      const text = getUserMessageText(event.message.content);
      if (text) {
+        const advisorItem = mapOmpAdvisorMessageToToolCall(event.message, text);
        this.emit({
          type: "timeline",
          provider: this.provider,
          turnId,
-          item: { type: "assistant_message", text },
+          item: advisorItem ?? { type: "assistant_message", text },
        });
      }
      if (!this.activeTurnHasUserMessage) {
@@ -96,6 +96,23 @@ describe("OMP CLI runtime", () => {
    });
  });

+  test("accepts session state without thinkingLevel for non-reasoning models", async () => {
+    const child = createOmpChild();
+    // Models like cursor-grok-4.5-high-fast encode effort in the model ID, so
+    // OMP marks them reasoning: false and omits thinkingLevel from get_state.
+    replyToCommands(child, () => ({
+      model: null,
+      isStreaming: false,
+      isCompacting: false,
+      sessionId: "session-1",
+      messageCount: 0,
+      queuedMessageCount: 0,
+    }));
+    const session = await createRuntime(child).startSession({ cwd: "/workspace/project" });
+
+    await expect(session.getState()).resolves.toMatchObject({ sessionId: "session-1" });
+  });
+
  test("rejects malformed RPC results instead of trusting transport data", async () => {
    const child = createOmpChild();
    replyToCommands(child, () => ({
@@ -1,13 +1,15 @@
 import type { OmpHistoryMapperHooks } from "./message-history.js";
+import { mapOmpAdvisorMessageToToolCall } from "./advisor-message.js";
 import { mapOmpSystemNoticeToToolCall } from "./system-notice.js";
 import { mapOmpToolDetail } from "./tool-call-mapper.js";
 import { resolveOmpEmittedToolCallId } from "./tool-call-id.js";

 export const OMP_HISTORY_MAPPER_HOOKS: OmpHistoryMapperHooks = {
  mapToolDetail: mapOmpToolDetail,
-  mapCustomMessage: (text, provider) => {
-    const noticeItem = mapOmpSystemNoticeToToolCall(text);
-    return noticeItem ? { type: "timeline", provider, item: noticeItem } : null;
+  mapCustomMessage: (message, text, provider) => {
+    const item =
+      mapOmpAdvisorMessageToToolCall(message, text) ?? mapOmpSystemNoticeToToolCall(text);
+    return item ? { type: "timeline", provider, item } : null;
  },
  resolveToolCallId: resolveOmpEmittedToolCallId,
 };
@@ -156,6 +156,59 @@ describe("OMP history mapper", () => {
    ]);
  });

+  test("renders replayed OMP advisor messages as synthetic tool-call blocks", async () => {
+    await expect(
+      collectHistory([
+        {
+          role: "custom",
+          content: [
+            {
+              type: "text",
+              text: '<advisory severity="blocker">Add an authorization check.</advisory>',
+            },
+          ],
+          customType: "advisor",
+          id: "advisor-message-1",
+          display: true,
+          details: {
+            notes: [
+              {
+                note: "Add an authorization check.",
+                severity: "blocker",
+                advisor: "security",
+              },
+              { note: "Exercise the failure path.", severity: "concern" },
+            ],
+          },
+        },
+      ]),
+    ).resolves.toEqual([
+      {
+        type: "timeline",
+        provider: "omp",
+        item: {
+          type: "tool_call",
+          callId: "omp-advisor:advisor-message-1",
+          name: "advisor",
+          status: "completed",
+          detail: {
+            type: "plain_text",
+            label: "Advisor · 2 notes · 1 blocker",
+            text: "[blocker] [security] Add an authorization check.\n\n[concern] Exercise the failure path.",
+            icon: "brain",
+          },
+          metadata: {
+            synthetic: true,
+            source: "omp_advisor",
+            noteCount: 2,
+            blockerCount: 1,
+          },
+          error: null,
+        },
+      },
+    ]);
+  });
+
  test("suppresses replayed raw todo tool calls through the OMP detail hook", async () => {
    await expect(
      collectHistory([
@@ -17,6 +17,7 @@ export interface OmpCapturedUserMessageEntry {

 export interface OmpHistoryMapperHooks {
  mapCustomMessage?: (
+    message: Extract<OmpAgentMessage, { role: "custom" }>,
    text: string,
    provider: string,
  ) => Extract<AgentStreamEvent, { type: "timeline" }> | null;
@@ -117,7 +118,7 @@ export class OmpHistoryMapper {
    message: Extract<OmpAgentMessage, { role: "custom" }>,
  ): AgentStreamEvent[] {
    const text = getUserMessageText(message.content);
-    const mappedEvent = text ? this.hooks.mapCustomMessage?.(text, this.provider) : null;
+    const mappedEvent = text ? this.hooks.mapCustomMessage?.(message, text, this.provider) : null;
    if (mappedEvent) {
      return [mappedEvent];
    }
@@ -114,7 +114,7 @@ const OmpContextUsageSchema = z
 export const OmpSessionStateSchema = z
  .object({
    model: OmpModelSchema.nullable().optional(),
-    thinkingLevel: OmpThinkingLevelSchema,
+    thinkingLevel: OmpThinkingLevelSchema.optional(),
    isStreaming: z.boolean(),
    isCompacting: z.boolean(),
    autoCompactionEnabled: z.boolean().optional(),
@@ -12,7 +12,7 @@ import type {
 } from "../../../agent-sdk-types.js";
 import type { PaseoToolCatalog } from "../../../tools/types.js";
 import { OmpAgentClient, OmpAgentSession, type OmpProviderIdleScheduler } from "../agent.js";
-import type { OmpRpcSlashCommand } from "../rpc-types.js";
+import type { OmpAgentMessage, OmpRpcSlashCommand } from "../rpc-types.js";
 import { FakeOmp } from "./fake-omp.js";

 const CWD = "/tmp/paseo-omp-agent-test";
@@ -155,6 +155,24 @@ export class OmpHarness {
    return await run;
  }

+  async runPromptWithCustomMessage(
+    input: string,
+    customMessage: Extract<OmpAgentMessage, { role: "custom" }>,
+    output: string,
+  ): Promise<unknown> {
+    const session = this.requireSession();
+    const promptStarted = this.omp.latestSession().nextPrompt();
+    const run = session.run(input);
+    await promptStarted;
+    const runtime = this.omp.latestSession();
+    runtime.beginTurn();
+    runtime.acceptPrompt(input, "user-1");
+    runtime.emit({ type: "message_end", message: customMessage });
+    runtime.streamAssistantText(output);
+    runtime.finishTurn();
+    return await run;
+  }
+
  async runPromptAfterExtensionNotice(input: string, output: string): Promise<unknown> {
    const session = this.requireSession();
    const promptStarted = this.omp.latestSession().nextPrompt();