Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/typed-mermaid-validation-errors.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"sideshow": patch
---

Invalid Mermaid submitted through the posts API now returns a typed validation
error with the failing request field, diagram type, complete parser diagnostic,
and concrete retry steps. The existing human-readable `error` string remains
for compatibility, and rejected diagrams are not persisted.
33 changes: 33 additions & 0 deletions e2e/mermaid.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,39 @@ test("a mermaid part renders inside an opaque-origin frame served from /s", asyn
await expect(frame.locator("body")).toContainText("Choice");
});

test("the native posts endpoint rejects invalid mermaid with actionable details", async ({
page,
request,
server,
}) => {
const response = await request.post(`${server.url}/api/posts`, {
data: {
title: "Broken diagram",
surfaces: [{ kind: "mermaid", mermaid: 'pie title Pets\n "Dogs" : broken !!@@' }],
},
});

expect(response.status()).toBe(400);
const body = await response.json();
expect(body).toMatchObject({
code: "surface_validation_failed",
issues: [
{
code: "invalid_mermaid_syntax",
requestPath: "surfaces[0].mermaid",
diagramType: "pie",
},
],
});
expect(body.issues[0].parserMessage).toContain("Parsing failed");
expect(body.issues[0].nextSteps).toContain(
"Correct the Mermaid syntax described by parserMessage, then retry the request.",
);

await page.goto(server.url);
await expect(page.locator(".card:not(#whatsNew)")).toHaveCount(0);
});

test("an invalid mermaid part shows the source in an error fallback, not a crash", async ({
page,
server,
Expand Down
22 changes: 14 additions & 8 deletions server/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ import {
type TerminalSurface,
type TraceStep,
} from "./types.ts";
import { validateSurfaces } from "./postSurfaces.ts";
import { type SurfaceValidationFailure, validateSurfaces } from "./postSurfaces.ts";
import {
findWelcomePost,
WELCOME_POST_TITLE,
Expand Down Expand Up @@ -77,6 +77,12 @@ const MAX_STEP_LABEL = 500;
// edge to keep one oversize value from bloating the agent's context forever.
const MAX_COMMENT_TEXT = 8000;
const MAX_TITLE = 500;

const surfaceValidationErrorBody = (failure: SurfaceValidationFailure) => ({
error: failure.error,
code: failure.code,
issues: failure.issues,
});
// Ceiling on concurrently-held SSE + long-poll connections. Both are GETs that
// pin a connection open (the event stream indefinitely, /api/comments?wait up
// to MAX_WAIT_SECONDS); on a publicRead workspace they're reachable unauthenticated,
Expand Down Expand Up @@ -1198,7 +1204,7 @@ export function createApp({
return c.json({ error: 'body must include a "surfaces" (or legacy "parts") array' }, 400);
}
const parsed = await validateSurfaces(blocks);
if (!parsed.ok) return c.json({ error: parsed.error }, 400);
if (!parsed.ok) return c.json(surfaceValidationErrorBody(parsed), 400);
return publish(c, body, parsed.surfaces);
};
app.post("/api/posts", publishPost); // canonical
Expand All @@ -1213,7 +1219,7 @@ export function createApp({
return c.json({ error: 'body must include non-empty "html" string' }, 400);
}
const parsed = await validateSurfaces([htmlSurface(body.html, body.kits)]);
if (!parsed.ok) return c.json({ error: parsed.error }, 400);
if (!parsed.ok) return c.json(surfaceValidationErrorBody(parsed), 400);
return publish(c, body, parsed.surfaces);
});

Expand Down Expand Up @@ -1278,11 +1284,11 @@ export function createApp({
return c.json({ error: '"surfaces" (or legacy "parts") must be an array' }, 400);
}
const parsed = await validateSurfaces(blocks);
if (!parsed.ok) return c.json({ error: parsed.error }, 400);
if (!parsed.ok) return c.json(surfaceValidationErrorBody(parsed), 400);
surfaces = parsed.surfaces;
} else if (typeof body.html === "string") {
const parsed = await validateSurfaces([htmlSurface(body.html, body.kits)]);
if (!parsed.ok) return c.json({ error: parsed.error }, 400);
if (!parsed.ok) return c.json(surfaceValidationErrorBody(parsed), 400);
surfaces = parsed.surfaces;
}
const result = await revisePost(c.req.param("id"), {
Expand Down Expand Up @@ -1346,7 +1352,7 @@ export function createApp({
);
}
const parsed = await validateSurfaces([updated]);
if (!parsed.ok) return c.json({ error: parsed.error }, 400);
if (!parsed.ok) return c.json(surfaceValidationErrorBody(parsed), 400);
surfaces = [...existing.surfaces];
surfaces[targetIdx] = { ...parsed.surfaces[0], id: existing.surfaces[targetIdx].id };
}
Expand All @@ -1371,7 +1377,7 @@ export function createApp({
return c.json({ error: 'body must include a "surface" object' }, 400);
}
const parsed = await validateSurfaces([body.surface]);
if (!parsed.ok) return c.json({ error: parsed.error }, 400);
if (!parsed.ok) return c.json(surfaceValidationErrorBody(parsed), 400);
const result = await appendPostSurface(c.req.param("id"), parsed.surfaces[0], {
before: body.before,
after: body.after,
Expand All @@ -1394,7 +1400,7 @@ export function createApp({
let surface: Surface | undefined;
if (body.surface !== undefined) {
const parsed = await validateSurfaces([body.surface]);
if (!parsed.ok) return c.json({ error: parsed.error }, 400);
if (!parsed.ok) return c.json(surfaceValidationErrorBody(parsed), 400);
surface = parsed.surfaces[0];
}
const result = await replacePostSurface(c.req.param("id"), c.req.param("target"), {
Expand Down
152 changes: 127 additions & 25 deletions server/postSurfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,49 @@ import { z } from "zod";
import { isKnownKit, KIT_IDS } from "./kits.ts";
import { isSurfaceKind, type Surface, type SurfaceKind } from "./types.ts";

/** Holds parsed surfaces and any validation issues found at the HTTP/MCP boundary. */
export interface SurfaceParseResult {
surfaces: Surface[];
// Deprecated compatibility alias for older deep imports.
parts: Surface[];
errors: string[];
errors: SurfaceValidationIssue[];
}

/** Identifies a rejected surface field and, for Mermaid syntax, how the agent can recover. */
export type SurfaceValidationIssue =
| {
code: "invalid_surface";
requestPath: string;
message: string;
}
| {
code: "invalid_mermaid_syntax";
requestPath: string;
message: string;
diagramType: string | null;
parserMessage: string;
nextSteps: string[];
};

/** Describes a failed surface validation without losing machine-readable issue details. */
export interface SurfaceValidationFailure {
ok: false;
error: string;
code: "surface_validation_failed";
issues: SurfaceValidationIssue[];
}

/** Returns either validated surfaces or a typed surface validation failure. */
export type SurfaceValidationResult =
| { ok: true; surfaces: Surface[]; parts: Surface[] }
| SurfaceValidationFailure;

const MERMAID_PARSE_NEXT_STEPS = [
"Correct the Mermaid syntax described by parserMessage, then retry the request.",
'Keep the diagram declaration on the first non-comment line (for example, "flowchart TD").',
"Reduce the diagram to a minimal valid form, then add statements back until the failing line is isolated.",
];

const requiredString = (name: string) =>
z.string({
required_error: `requires string "${name}"`,
Expand Down Expand Up @@ -261,7 +297,12 @@ async function parseSurfaceList(
raw: unknown,
opts: { strict?: boolean } = {},
): Promise<SurfaceParseResult> {
if (!Array.isArray(raw)) return surfaceResult([], ["surfaces must be an array"]);
if (!Array.isArray(raw)) {
return surfaceResult(
[],
[{ code: "invalid_surface", requestPath: "surfaces", message: "must be an array" }],
);
}

if (opts.strict === true) {
const results = await Promise.all(raw.map((surface, i) => parseStrictSurface(surface, i)));
Expand All @@ -272,28 +313,32 @@ async function parseSurfaceList(
}

const surfaces: Surface[] = [];
for (const surface of raw) {
for (const [index, surface] of raw.entries()) {
const parsed = looseSurfaceSchema.safeParse(surface);
if (!parsed.success) continue;
if ((await validateSemantics(parsed.data as Surface)).length === 0)
if ((await validateSemantics(parsed.data as Surface, `surfaces[${index}]`)).length === 0)
surfaces.push(parsed.data as Surface);
}
return surfaceResult(surfaces, []);
}

function surfaceResult(surfaces: Surface[], errors: string[]): SurfaceParseResult {
function surfaceResult(surfaces: Surface[], errors: SurfaceValidationIssue[]): SurfaceParseResult {
return { surfaces, parts: surfaces, errors };
}

export const coerceSurfaces = (raw: unknown): Promise<Surface[]> =>
parseSurfaceList(raw).then((r) => r.surfaces);

export async function validateSurfaces(
raw: unknown,
): Promise<{ ok: true; surfaces: Surface[]; parts: Surface[] } | { ok: false; error: string }> {
/** Strictly validates surfaces before a native API write reaches persistence. */
export async function validateSurfaces(raw: unknown): Promise<SurfaceValidationResult> {
const result = await parseSurfaceList(raw, { strict: true });
return result.errors.length > 0
? { ok: false, error: result.errors.join("; ") }
? {
ok: false,
error: result.errors.map((issue) => `${issue.requestPath}: ${issue.message}`).join("; "),
code: "surface_validation_failed",
issues: result.errors,
}
: { ok: true, surfaces: result.surfaces, parts: result.surfaces };
}

Expand Down Expand Up @@ -345,24 +390,46 @@ function mermaidDiagramType(src: string): string | null {
//
// This is the other half of app.ts's lazy richRender import: leaving either half
// static keeps the whole dependency graph resident and neither one saves anything.
async function validateSemantics(surface: Surface): Promise<string[]> {
async function validateSemantics(
surface: Surface,
surfaceRequestPath: string,
): Promise<SurfaceValidationIssue[]> {
if (surface.kind === "diff" && surface.patch) {
const diffParsers = await import("@pierre/diffs");
try {
if (!diffPatchHasContent(surface.patch, diffParsers))
return [
'diff surface "patch" did not parse to any file — expected a unified/git patch with --- /+++ headers and @@ hunks',
{
code: "invalid_surface",
requestPath: `${surfaceRequestPath}.patch`,
message:
"did not parse to any file — expected a unified/git patch with --- /+++ headers and @@ hunks",
},
];
} catch (e) {
return [
'diff surface "patch" failed to parse: ' + (e instanceof Error ? e.message : "error"),
{
code: "invalid_surface",
requestPath: `${surfaceRequestPath}.patch`,
message: "failed to parse: " + (e instanceof Error ? e.message : "error"),
},
];
}
}
if (surface.kind === "mermaid") {
const diagramType = mermaidDiagramType(surface.mermaid);
if (!diagramType)
return ['mermaid surface has no diagram type (first line should be e.g. "flowchart TD")'];
if (!diagramType) {
return [
{
code: "invalid_mermaid_syntax",
requestPath: `${surfaceRequestPath}.mermaid`,
message: 'has no diagram type (first line should be e.g. "flowchart TD")',
diagramType: null,
parserMessage: "No Mermaid diagram type was found.",
nextSteps: MERMAID_PARSE_NEXT_STEPS,
},
];
}
// Outside the try: the catch below turns a throw into "your mermaid is
// invalid", which would be a wrong answer (and a 400 instead of a 500) if what
// actually failed was loading the parser.
Expand All @@ -373,8 +440,18 @@ async function validateSemantics(surface: Surface): Promise<string[]> {
// Unsupported diagram types (flowchart, sequence, etc. — still on Jison)
// skip validation; the viewer's graceful fallback handles render failures.
if (e instanceof Error && /unknown diagram type/i.test(e.message)) return [];
const msg = e instanceof Error ? (e.message.split("\n")[0] ?? "parse error") : "parse error";
return [`mermaid surface failed to parse: ${msg}`];
const parserMessage = e instanceof Error ? e.message : "parse error";
const summary = parserMessage.split("\n")[0] ?? "parse error";
return [
{
code: "invalid_mermaid_syntax",
requestPath: `${surfaceRequestPath}.mermaid`,
message: `failed to parse: ${summary}`,
diagramType,
parserMessage,
nextSteps: MERMAID_PARSE_NEXT_STEPS,
},
];
}
}
return [];
Expand All @@ -383,30 +460,55 @@ async function validateSemantics(surface: Surface): Promise<string[]> {
async function parseStrictSurface(
raw: unknown,
index: number,
): Promise<{ surface: Surface | null; errors: string[] }> {
const path = `surfaces[${index}]`;
): Promise<{ surface: Surface | null; errors: SurfaceValidationIssue[] }> {
const surfaceRequestPath = `surfaces[${index}]`;
if (!raw || typeof raw !== "object")
return { surface: null, errors: [`${path}: must be an object`] };
return {
surface: null,
errors: [
{ code: "invalid_surface", requestPath: surfaceRequestPath, message: "must be an object" },
],
};

const kind = (raw as { kind?: unknown }).kind;
const schema = schemaForKind(kind);
if (!schema) return { surface: null, errors: [`${path}: unknown surface kind`] };
if (!schema) {
return {
surface: null,
errors: [
{
code: "invalid_surface",
requestPath: surfaceRequestPath,
message: "unknown surface kind",
},
],
};
}

const parsed = schema.safeParse(raw);
if (!parsed.success) return { surface: null, errors: formatZodErrors(parsed.error, path) };
const semantic = await validateSemantics(parsed.data);
if (!parsed.success) {
return { surface: null, errors: formatZodErrors(parsed.error, surfaceRequestPath) };
}
const semantic = await validateSemantics(parsed.data, surfaceRequestPath);
return semantic.length > 0
? { surface: null, errors: semantic.map((m) => `${path}: ${m}`) }
? { surface: null, errors: semantic }
: { surface: parsed.data, errors: [] };
}

function schemaForKind(kind: unknown): z.ZodType<Surface, z.ZodTypeDef, any> | null {
return isSurfaceKind(kind) ? strictSurfaceSchemas[kind] : null;
}

function formatZodErrors(error: z.ZodError, prefix = "surfaces"): string[] {
function formatZodErrors(
error: z.ZodError,
requestPathPrefix = "surfaces",
): SurfaceValidationIssue[] {
return error.issues.map((issue) => {
const suffix = issue.path.length > 0 ? `.${issue.path.join(".")}` : "";
return `${prefix}${suffix}: ${issue.message}`;
return {
code: "invalid_surface",
requestPath: `${requestPathPrefix}${suffix}`,
message: issue.message,
};
});
}
5 changes: 5 additions & 0 deletions server/public.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,9 @@ export { createApp, type AppOptions, type AuthenticateHook, type FeedEvent } fro
export { SqlStore } from "./sqlStore.js";
export { createSqliteStorage, migrateJsonToSqlite } from "./sqliteStorage.js";
export { JsonFileStore } from "./storage.js";
export type {
SurfaceValidationFailure,
SurfaceValidationIssue,
SurfaceValidationResult,
} from "./postSurfaces.js";
export type * from "./types.js";
Loading
Loading