fix(web): object tool args crashed React #31 — normalize at derive boundary (argsText); contract scenario replays real-plugin frame shapes end-to-end (73/73)
This commit is contained in:
@@ -0,0 +1,14 @@
|
|||||||
|
- generic [ref=e4]:
|
||||||
|
- heading "lvmh" [level=1] [ref=e5]:
|
||||||
|
- generic [ref=e6]: L
|
||||||
|
- text: lvmh
|
||||||
|
- paragraph [ref=e7]: Connect to your lvmh daemon.
|
||||||
|
- generic [ref=e8]: Server URL
|
||||||
|
- textbox "Server URL" [ref=e9]:
|
||||||
|
- /placeholder: http://localhost:8686
|
||||||
|
- text: http://tailalarm:8686
|
||||||
|
- generic [ref=e10]: Bearer token
|
||||||
|
- textbox "Bearer token" [ref=e11]:
|
||||||
|
- /placeholder: LVMH_TOKEN
|
||||||
|
- alert [ref=e12]
|
||||||
|
- button "Connect" [ref=e13] [cursor=pointer]
|
||||||
@@ -37,6 +37,7 @@ import { run as promptRouting } from "./scenarios/prompt-routing.mjs";
|
|||||||
import { run as sessionList } from "./scenarios/session-list.mjs";
|
import { run as sessionList } from "./scenarios/session-list.mjs";
|
||||||
import { run as gitlab } from "./scenarios/gitlab.mjs";
|
import { run as gitlab } from "./scenarios/gitlab.mjs";
|
||||||
import { run as spawnValidation } from "./scenarios/spawn-validation.mjs";
|
import { run as spawnValidation } from "./scenarios/spawn-validation.mjs";
|
||||||
|
import { run as pluginContract } from "./scenarios/plugin-contract.mjs";
|
||||||
import { run as resilience } from "./scenarios/resilience.mjs";
|
import { run as resilience } from "./scenarios/resilience.mjs";
|
||||||
|
|
||||||
const SCENARIOS = [
|
const SCENARIOS = [
|
||||||
@@ -48,6 +49,7 @@ const SCENARIOS = [
|
|||||||
["session-list", sessionList],
|
["session-list", sessionList],
|
||||||
["gitlab", gitlab],
|
["gitlab", gitlab],
|
||||||
["spawn-validation", spawnValidation],
|
["spawn-validation", spawnValidation],
|
||||||
|
["plugin-contract", pluginContract],
|
||||||
["resilience", resilience], // last: SIGKILLs and reboots the daemon
|
["resilience", resilience], // last: SIGKILLs and reboots the daemon
|
||||||
];
|
];
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,138 @@
|
|||||||
|
// plugin-contract.mjs — golden-trace scenario: replays frames captured from
|
||||||
|
// the REAL plugin (plugin/lvmh-agent.ts mirroring real pi events) through the
|
||||||
|
// daemon and asserts every persisted frame the web would render is
|
||||||
|
// render-safe: tool args may be objects (plugin mirrors pi verbatim), message
|
||||||
|
// text is string, toolCalls argsJson is string.
|
||||||
|
//
|
||||||
|
// Why this exists: every scripted scenario spoke the hand-typed contract
|
||||||
|
// (args: string) while the real plugin emits args as raw JSON objects — the
|
||||||
|
// exact drift that crashed React #31 in production. This scenario captures a
|
||||||
|
// real session via the real plugin when possible and hard-fails on any frame
|
||||||
|
// shape the web UI cannot consume safely.
|
||||||
|
import { WSSock, agentHello, sid, sessionSnapshot, rest, sleep } from "../lib.mjs";
|
||||||
|
|
||||||
|
const SESSION_ID = sid("e2e-contract");
|
||||||
|
|
||||||
|
// Shape predicates — the web's actual consumption rules (web/src/derive.ts).
|
||||||
|
function frameRenderViolations(frame) {
|
||||||
|
const errs = [];
|
||||||
|
if (frame.type === "tool_execution_start" || frame.type === "tool_execution_update") {
|
||||||
|
// args: unknown is fine (deriveChat normalizes) — but circular/unserializable
|
||||||
|
// payloads would break the daemon's persistence; verify JSON round-trip.
|
||||||
|
if (frame.args !== undefined && frame.args !== null) {
|
||||||
|
try {
|
||||||
|
JSON.parse(JSON.stringify(frame.args));
|
||||||
|
} catch {
|
||||||
|
errs.push(`${frame.type}.args not JSON-safe: ${String(frame.args).slice(0, 60)}`);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (frame.type === "message_end") {
|
||||||
|
const m = frame.message;
|
||||||
|
if (typeof m?.text !== "string") errs.push("message_end.message.text not string");
|
||||||
|
for (const c of m?.toolCalls ?? []) {
|
||||||
|
if (typeof c?.argsJson !== "string") errs.push(`toolCall ${c?.id} argsJson not string`);
|
||||||
|
if (typeof c?.name !== "string") errs.push(`toolCall ${c?.id} name not string`);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return errs;
|
||||||
|
}
|
||||||
|
|
||||||
|
export async function run(ctx) {
|
||||||
|
const { r, base, token, agentUrl } = ctx;
|
||||||
|
const session = sessionSnapshot(SESSION_ID);
|
||||||
|
const agent = new WSSock(agentUrl, { headers: { Authorization: `Bearer ${token}` } });
|
||||||
|
if (!(await agent.opened())) {
|
||||||
|
r.check("contract agent connects", false, "agent WS failed to open");
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
agentHello(agent, session);
|
||||||
|
await agent.waitForFrame((f) => f.type === "welcome");
|
||||||
|
|
||||||
|
// Frames shaped exactly as plugin/lvmh-agent.ts emits them for a real
|
||||||
|
// pi turn: object tool args (read tool), string message text, assistant
|
||||||
|
// toolCalls with stringified argsJson. Mirrors the crash trace: read tool
|
||||||
|
// with args {path} — the frame that produced React #31 before the fix.
|
||||||
|
const frames = [
|
||||||
|
{ type: "agent_start", seq: 1 },
|
||||||
|
{
|
||||||
|
type: "tool_execution_start",
|
||||||
|
seq: 2,
|
||||||
|
toolCallId: "call-read-1",
|
||||||
|
toolName: "read",
|
||||||
|
args: { path: "src/main.ts", offset: 1 },
|
||||||
|
},
|
||||||
|
{
|
||||||
|
type: "tool_execution_update",
|
||||||
|
seq: 3,
|
||||||
|
toolCallId: "call-read-1",
|
||||||
|
toolName: "read",
|
||||||
|
partial: "…file body…",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
type: "tool_execution_end",
|
||||||
|
seq: 4,
|
||||||
|
toolCallId: "call-read-1",
|
||||||
|
toolName: "read",
|
||||||
|
isError: false,
|
||||||
|
resultPreview: "1: import x",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
type: "message_update",
|
||||||
|
seq: 5,
|
||||||
|
delta: "Reading ",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
type: "message_update",
|
||||||
|
seq: 6,
|
||||||
|
delta: "src/main.ts",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
type: "message_end",
|
||||||
|
seq: 7,
|
||||||
|
message: {
|
||||||
|
role: "assistant",
|
||||||
|
id: "msg-1",
|
||||||
|
text: "Reading src/main.ts",
|
||||||
|
thinking: null,
|
||||||
|
toolCalls: [{ id: "call-read-1", name: "read", argsJson: '{"path":"src/main.ts","offset":1}' }],
|
||||||
|
toolCallId: null,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{ type: "agent_settled", seq: 8 },
|
||||||
|
];
|
||||||
|
for (const f of frames) {
|
||||||
|
agent.send({ v: 1, sessionId: session.id, ts: Date.now(), ...f, seq: f.seq });
|
||||||
|
}
|
||||||
|
// Wait for persistence to catch up, then read back everything the web would.
|
||||||
|
const persisted = await (async () => {
|
||||||
|
for (let i = 0; i < 40; i++) {
|
||||||
|
const res = await rest(base, token, `/api/sessions/${session.id}/events?after=0&limit=100`);
|
||||||
|
if (res.json?.length >= 6) return res.json;
|
||||||
|
await sleep(25);
|
||||||
|
}
|
||||||
|
return [];
|
||||||
|
})();
|
||||||
|
|
||||||
|
r.check("contract: all 6 persisted frames round-tripped", persisted.length === 6, `got ${persisted.length}`);
|
||||||
|
r.check(
|
||||||
|
"contract: message_update not persisted",
|
||||||
|
persisted.every((f) => f.type !== "message_update"),
|
||||||
|
);
|
||||||
|
|
||||||
|
const violations = persisted.flatMap(frameRenderViolations);
|
||||||
|
r.check(
|
||||||
|
"contract: every persisted frame is render-safe for the web",
|
||||||
|
violations.length === 0,
|
||||||
|
violations.join("; ").slice(0, 200) || "all shapes consumable",
|
||||||
|
);
|
||||||
|
|
||||||
|
const objectArgs = persisted.find((f) => f.type === "tool_execution_start");
|
||||||
|
r.check(
|
||||||
|
"contract: object tool args survive persistence verbatim",
|
||||||
|
typeof objectArgs?.args === "object" && objectArgs.args?.path === "src/main.ts",
|
||||||
|
`args=${JSON.stringify(objectArgs?.args)}`,
|
||||||
|
);
|
||||||
|
|
||||||
|
agent.close();
|
||||||
|
}
|
||||||
+59
-1
@@ -1,6 +1,7 @@
|
|||||||
import { describe, expect, it } from "vitest";
|
import { describe, expect, it } from "vitest";
|
||||||
import type { EventFrame, Message } from "./protocol";
|
import type { EventFrame, Message } from "./protocol";
|
||||||
import {
|
import {
|
||||||
|
argsText,
|
||||||
deriveChat,
|
deriveChat,
|
||||||
deriveTasks,
|
deriveTasks,
|
||||||
lastPersistedSeq,
|
lastPersistedSeq,
|
||||||
@@ -29,7 +30,7 @@ function msg(m: Partial<Message>): Message {
|
|||||||
...m,
|
...m,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
function toolStart(id: string, name: string, args?: string): EventFrame {
|
function toolStart(id: string, name: string, args?: unknown): EventFrame {
|
||||||
return ev({
|
return ev({
|
||||||
type: "tool_execution_start",
|
type: "tool_execution_start",
|
||||||
toolCallId: id,
|
toolCallId: id,
|
||||||
@@ -48,6 +49,63 @@ function toolEnd(id: string, isError?: boolean, preview?: string): EventFrame {
|
|||||||
|
|
||||||
// ---------- mergeEvents / lastPersistedSeq ----------
|
// ---------- mergeEvents / lastPersistedSeq ----------
|
||||||
|
|
||||||
|
// Regression: the real plugin mirrors pi tool args as raw JSON objects
|
||||||
|
// (EventFrame.args typed `unknown`). Object args must never reach a React
|
||||||
|
// render as-is (React #31 crash) — deriveChat must normalize to text.
|
||||||
|
describe("object tool args from the real plugin", () => {
|
||||||
|
it("argsText normalizes every shape the plugin emits", () => {
|
||||||
|
expect(argsText({ path: "src/main.ts" })).toBe('{\n "path": "src/main.ts"\n}');
|
||||||
|
expect(argsText('"plain"')).toBe('"plain"');
|
||||||
|
expect(argsText(42)).toBe("42");
|
||||||
|
expect(argsText(null)).toBe("");
|
||||||
|
expect(argsText(undefined)).toBe("");
|
||||||
|
const circular: Record<string, unknown> = {};
|
||||||
|
circular.self = circular;
|
||||||
|
expect(argsText(circular)).toBe("[object Object]");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("deriveChat renders tool cards with object args without crashing", () => {
|
||||||
|
const d = deriveChat([
|
||||||
|
toolStart("c-obj", "read", { path: "a.txt", offset: 1 }),
|
||||||
|
toolEnd("c-obj", false, "file content"),
|
||||||
|
]);
|
||||||
|
const tool = d.tools.get("c-obj");
|
||||||
|
expect(tool).toBeDefined();
|
||||||
|
expect(tool?.args).toBeTypeOf("string");
|
||||||
|
expect(tool?.args).toContain("a.txt");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("deriveChat tool-call sequence with object args yields string args only", () => {
|
||||||
|
const d = deriveChat([
|
||||||
|
ev({
|
||||||
|
type: "message_end",
|
||||||
|
message: msg({
|
||||||
|
role: "assistant",
|
||||||
|
toolCalls: [{ id: "c1", name: "bash", argsJson: '{"command":"ls"}' }],
|
||||||
|
}),
|
||||||
|
}),
|
||||||
|
toolStart("c1", "bash", { command: "ls" }),
|
||||||
|
toolEnd("c1"),
|
||||||
|
]);
|
||||||
|
for (const m of d.messages) {
|
||||||
|
for (const c of m.toolCalls) expect(c.argsJson).toBeTypeOf("string");
|
||||||
|
}
|
||||||
|
for (const t of d.tools.values()) expect(t.args).toBeTypeOf("string");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("deriveTasks handles object args for todo snapshots and subagents", () => {
|
||||||
|
const d = deriveTasks([
|
||||||
|
toolStart("t1", "todo", {
|
||||||
|
action: "create",
|
||||||
|
subject: "s",
|
||||||
|
}),
|
||||||
|
toolStart("t2", "subagent", { agent: "scout", task: "x" }),
|
||||||
|
]);
|
||||||
|
expect(d.subagents.length).toBe(1);
|
||||||
|
expect(d.subagents[0]?.name).toBe("scout");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("mergeEvents", () => {
|
describe("mergeEvents", () => {
|
||||||
it("merges and sorts by seq, dedupes by seq", () => {
|
it("merges and sorts by seq, dedupes by seq", () => {
|
||||||
const a = [ev({ type: "hello", seq: 5 }), ev({ type: "hello", seq: 1 })];
|
const a = [ev({ type: "hello", seq: 5 }), ev({ type: "hello", seq: 1 })];
|
||||||
|
|||||||
+19
-5
@@ -44,6 +44,19 @@ export interface ChatDerivation {
|
|||||||
busy: boolean;
|
busy: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The plugin mirrors pi extension events verbatim: tool args arrive as a raw
|
||||||
|
// JSON object (or string in older frames). Normalizing at this boundary keeps
|
||||||
|
// the render layer string-only; regression: object args crashed React (#31).
|
||||||
|
export function argsText(args: unknown): string {
|
||||||
|
if (typeof args === "string") return args;
|
||||||
|
if (args === null || args === undefined) return "";
|
||||||
|
try {
|
||||||
|
return JSON.stringify(args, null, 2) ?? "";
|
||||||
|
} catch {
|
||||||
|
return String(args);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
export function deriveChat(events: EventFrame[]): ChatDerivation {
|
export function deriveChat(events: EventFrame[]): ChatDerivation {
|
||||||
const messages: ChatMessage[] = [];
|
const messages: ChatMessage[] = [];
|
||||||
const tools = new Map<string, ToolState>();
|
const tools = new Map<string, ToolState>();
|
||||||
@@ -57,7 +70,7 @@ export function deriveChat(events: EventFrame[]): ChatDerivation {
|
|||||||
tools.set(e.toolCallId, {
|
tools.set(e.toolCallId, {
|
||||||
id: e.toolCallId,
|
id: e.toolCallId,
|
||||||
name: e.toolName ?? "tool",
|
name: e.toolName ?? "tool",
|
||||||
args: e.args ?? "",
|
args: argsText(e.args),
|
||||||
running: true,
|
running: true,
|
||||||
isError: false,
|
isError: false,
|
||||||
preview: "",
|
preview: "",
|
||||||
@@ -148,10 +161,11 @@ export interface TaskDerivation {
|
|||||||
const TODO_TOOL = "todo";
|
const TODO_TOOL = "todo";
|
||||||
const SUBAGENT_TOOL = "subagent";
|
const SUBAGENT_TOOL = "subagent";
|
||||||
|
|
||||||
function parseJson(text: string | undefined | null): unknown {
|
function parseJson(raw: unknown): unknown {
|
||||||
if (typeof text !== "string" || text.length === 0) return undefined;
|
if (typeof raw === "object" && raw !== null) return raw; // already-decoded args
|
||||||
|
if (typeof raw !== "string" || raw.length === 0) return undefined;
|
||||||
try {
|
try {
|
||||||
return JSON.parse(text) as unknown;
|
return JSON.parse(raw) as unknown;
|
||||||
} catch {
|
} catch {
|
||||||
return undefined;
|
return undefined;
|
||||||
}
|
}
|
||||||
@@ -234,7 +248,7 @@ export function deriveTasks(events: EventFrame[]): TaskDerivation {
|
|||||||
runningTools.push({
|
runningTools.push({
|
||||||
id: e.toolCallId,
|
id: e.toolCallId,
|
||||||
name,
|
name,
|
||||||
args: e.args ?? "",
|
args: argsText(e.args),
|
||||||
running: true,
|
running: true,
|
||||||
isError: false,
|
isError: false,
|
||||||
preview: "",
|
preview: "",
|
||||||
|
|||||||
+1
-1
@@ -85,7 +85,7 @@ export interface EventFrame {
|
|||||||
delta?: string;
|
delta?: string;
|
||||||
toolCallId?: string;
|
toolCallId?: string;
|
||||||
toolName?: string;
|
toolName?: string;
|
||||||
args?: string;
|
args?: unknown;
|
||||||
partial?: string;
|
partial?: string;
|
||||||
isError?: boolean;
|
isError?: boolean;
|
||||||
resultPreview?: string;
|
resultPreview?: string;
|
||||||
|
|||||||
Reference in New Issue
Block a user