diff --git a/.changeset/cli-repeatable-surface-flags.md b/.changeset/cli-repeatable-surface-flags.md new file mode 100644 index 0000000..6927b13 --- /dev/null +++ b/.changeset/cli-repeatable-surface-flags.md @@ -0,0 +1,21 @@ +--- +"sideshow": minor +--- + +`sideshow publish` and `sideshow surface add` now accept repeated surface +flags to add several surfaces of the same kind. Previously a repeated +non-multiple flag (`--diff a --diff b`) was silently dropped to the last +value with no error. + +- `sideshow publish --diff a.patch --code c.ts --diff b.patch` now + produces `[html, diff, code, diff]` — each repeat adds a surface, in + command-line flag order. +- `sideshow surface add --md a.md --md b.md` appends two markdown + surfaces (one append call per surface, so `--before`/`--after` positioning + still applies per surface). +- The seven surface flags (`--md`, `--mermaid`, `--diff`, `--terminal`, + `--json`, `--code`, `--image`) are now `repeatable` in both commands. + +This closes the remaining gap from #151 (multiple surfaces of the same kind +on the CLI); surface order control was already fixed in 0.9.x via the +token-walk for #158. diff --git a/.changeset/id-never-leading-separator.md b/.changeset/id-never-leading-separator.md new file mode 100644 index 0000000..d00c700 --- /dev/null +++ b/.changeset/id-never-leading-separator.md @@ -0,0 +1,17 @@ +--- +"sideshow": patch +--- + +Fixed a latent bug where post, surface, and session ids could start with `-` +or `_` (URL-safe base64 maps `+`→`-`, `/`→`_`, so ~1/64 of ids began with a +separator). Any id starting with `-` broke CLI commands that take an id as a +positional — `node:util` `parseArgs` treated it as an unknown option +(`Unknown option '-6'` for an id like `-6K4AJsKD4M`), affecting `sideshow +update`, `show`, and `surface add/remove/edit/move`. Two fixes: + +- `newId` now swaps a leading separator for an alphanumeric, so new ids are + always CLI-safe. +- The CLI's `parse()` wrapper swaps id-shaped `-`/`_`-prefixed tokens for a + sentinel before `parseArgs` sees them, then restores them in the result + (positionals, tokens, option values). This rescues already-stored ids that + start with a separator. diff --git a/bin/sideshow.js b/bin/sideshow.js index 263798e..6c9058f 100755 --- a/bin/sideshow.js +++ b/bin/sideshow.js @@ -21,15 +21,16 @@ usage: sideshow serve [--port N] [--open] start the surface (API + viewer) sideshow publish [options] publish an HTML post (one html surface) --title post title - --md add a markdown surface (prose) — combine with html - --mermaid add a mermaid surface (diagram source → SVG) — combine with html - --diff add a diff surface from a unified/git patch (combine with html) - --terminal add a terminal surface from monospace/ANSI output - --json add a json surface from a JSON file (collapsible tree) - --code add a code surface from a file (shiki-highlighted) + --md add a markdown surface (prose) — repeatable + --mermaid add a mermaid surface (diagram source → SVG) — repeatable + --diff add a diff surface from a unified/git patch — repeatable + --terminal add a terminal surface from monospace/ANSI output — repeatable + --json add a json surface from a JSON file (collapsible tree) — repeatable + --code add a code surface from a file (shiki-highlighted) — repeatable --kit opt the html surface into a kit (repeatable; see "sideshow kits") - --image upload an image and append it as an image surface + --image upload an image and append it as an image surface — repeatable --session target session (default: auto per agent session) + surfaces appear in command-line flag order; repeat a flag to add several of one kind --session-title name for a newly created session — name the task, e.g. "Auth refactor" (ignored if the session exists) --agent agent name for new sessions (default: $SIDESHOW_AGENT or "agent") @@ -78,16 +79,17 @@ usage: --surface target surface N (id or 0-based index) in a multi-surface post sideshow surface [options] edit individual surfaces of a post surface add [flags] append a surface to an existing post - --md markdown surface - --code code surface (language inferred from filename) - --diff diff surface from a patch - --terminal terminal surface - --mermaid mermaid surface - --json json surface - --image image surface (uploads the file first) + --md markdown surface (repeatable) + --code code surface (language inferred from filename; repeatable) + --diff diff surface from a patch (repeatable) + --terminal terminal surface (repeatable) + --mermaid mermaid surface (repeatable) + --json json surface (repeatable) + --image image surface (uploads the file first; repeatable) --layout split split layout for --diff surfaces --before insert before surface N (id or index) --after insert after surface N (id or index) + surfaces append in command-line flag order; repeat a flag for several of one kind surface remove remove surface N (id or 0-based index) surface edit replace surface N's content (kind preserved) surface move --to move surface N to position M @@ -478,6 +480,77 @@ function normalizeKits(flag) { return ids.length > 0 ? [...new Set(ids)] : undefined; } +// Surface-kind flags accepted by `publish` and `surface add` (the two commands +// that compose a post from one flag per surface kind). Each is declared +// `multiple: true` in the parser, so a repeated flag yields an array — letting +// an author emit several surfaces of the same kind (--diff a --diff b). +const SURFACE_FLAGS = new Map([ + ["md", "markdown"], + ["mermaid", "mermaid"], + ["diff", "diff"], + ["terminal", "terminal"], + ["json", "json"], + ["code", "code"], + ["image", "image"], +]); + +// Build a single surface object from one flag value. Mirrors the per-kind +// construction that used to be inlined in `publish` and `surface add`. +async function buildSurface(kind, value, { session, layout }) { + const file = value || "-"; + if (kind === "markdown") return { kind: "markdown", markdown: readContent(file) }; + if (kind === "mermaid") return { kind: "mermaid", mermaid: readContent(file) }; + if (kind === "diff") + return { + kind: "diff", + patch: readContent(file), + ...(layout === "split" && { layout: "split" }), + }; + if (kind === "terminal") return { kind: "terminal", text: readContent(file) }; + if (kind === "json") { + const text = readContent(file); + try { + return { kind: "json", data: JSON.parse(text) }; + } catch { + fail(`--json: invalid JSON${value && value !== "-" ? ` in ${value}` : ""}`); + } + } + if (kind === "code") { + const part = { kind: "code", code: readContent(file) }; + const codeLang = value && value !== "-" ? inferLang(value) : undefined; + if (codeLang) part.language = codeLang; + if (value && value !== "-") part.title = value.split("/").pop() || value; + return part; + } + if (kind === "image") { + const asset = await uploadFile(value, { session, kind: "image" }); + return { kind: "image", assetId: asset.id }; + } + fail(`unknown surface kind: ${kind}`); +} + +// Walk parseArgs `tokens` (which preserve command-line order, including +// repeats when a surface flag is `multiple: true`) and build one surface per +// flag occurrence, pulling successive values from each flag's value array. +// Surfaces render top-to-bottom, so order is user-visible — this honors the +// order the author wrote the flags, repeats included. +async function surfacesFromFlags(flags, tokens, { session, layout }) { + const idx = new Map(); + const out = []; + for (const t of tokens ?? []) { + if (t.kind !== "option" || !SURFACE_FLAGS.has(t.name)) continue; + const flagName = t.name; + const arr = flags[flagName]; + if (!Array.isArray(arr)) continue; + const i = idx.get(flagName) ?? 0; + const value = arr[i]; + if (value === undefined) continue; + idx.set(flagName, i + 1); + out.push(await buildSurface(SURFACE_FLAGS.get(flagName), value, { session, layout })); + } + return out; +} + async function publishSurface(parts, flags) { const session = await resolveSession(flags, { create: true }); return api("/api/surfaces", { @@ -545,11 +618,27 @@ const [cmd, ...rest] = process.argv.slice(2); // Subcommand flag parsing. parseArgs is strict, so without this --help (or // any typo) throws a raw stack trace; instead --help/-h prints usage and // exits 0, and an unknown option fails with a one-line hint. +// +// Ids are base64url and can start with - or _ (~1/64 each). parseArgs strict +// mode treats those as unknown options ("Unknown option '-6'" for an id like +// "-6K4AJsKD4M"). We swap any id-shaped token that starts with a separator +// for a sentinel before parsing, then restore it in the result — so positionals, +// tokens, and option values all get the original id back, in the right order. +const ID_LIKE = /^[-_](?![-_])[A-Za-z0-9_-]{7,}$/; function parse(config = {}) { + const rescued = new Map(); + const args = rest.map((a) => { + if (ID_LIKE.test(a)) { + const s = `\x00${rescued.size}\x00`; + rescued.set(s, a); + return s; + } + return a; + }); let parsed; try { parsed = parseArgs({ - args: rest, + args, ...config, options: { ...config.options, help: { type: "boolean", short: "h" } }, }); @@ -561,6 +650,17 @@ function parse(config = {}) { console.log(HELP); process.exit(0); } + const restore = (v) => (typeof v === "string" && rescued.has(v) ? rescued.get(v) : v); + if (parsed.positionals) parsed.positionals = parsed.positionals.map(restore); + if (parsed.tokens) { + parsed.tokens = parsed.tokens.map((t) => + t.kind === "positional" && rescued.has(t.value) ? { ...t, value: rescued.get(t.value) } : t, + ); + } + for (const k of Object.keys(parsed.values ?? {})) { + const v = parsed.values[k]; + parsed.values[k] = Array.isArray(v) ? v.map(restore) : restore(v); + } return parsed; } @@ -825,13 +925,13 @@ const commands = { allowPositionals: true, options: { title: { type: "string" }, - md: { type: "string" }, - mermaid: { type: "string" }, - diff: { type: "string" }, - image: { type: "string" }, - terminal: { type: "string" }, - json: { type: "string" }, - code: { type: "string" }, + md: { type: "string", multiple: true }, + mermaid: { type: "string", multiple: true }, + diff: { type: "string", multiple: true }, + image: { type: "string", multiple: true }, + terminal: { type: "string", multiple: true }, + json: { type: "string", multiple: true }, + code: { type: "string", multiple: true }, kit: { type: "string", multiple: true }, layout: { type: "string" }, session: { type: "string" }, @@ -843,61 +943,15 @@ const commands = { const htmlPart = { kind: "html", html: readContent(positionals[0]) }; const kits = normalizeKits(flags.kit); if (kits) htmlPart.kits = kits; - // Surfaces render top-to-bottom, so order is user-visible. Walk the - // parseArgs tokens (which preserve command-line order) and append each - // surface flag the first time it appears, instead of a fixed if-ladder. - const SURFACE_FLAGS = new Map([ - ["md", "markdown"], - ["mermaid", "mermaid"], - ["diff", "diff"], - ["terminal", "terminal"], - ["json", "json"], - ["code", "code"], - ["image", "image"], - ]); - const orderedKinds = []; - const seen = new Set(); - for (const t of tokens ?? []) { - if (t.kind === "option" && SURFACE_FLAGS.has(t.name) && !seen.has(t.name)) { - seen.add(t.name); - orderedKinds.push(SURFACE_FLAGS.get(t.name)); - } - } // Resolve the session first so image uploads and the post share it. const session = await resolveSession(flags, { create: true }); - const parts = [htmlPart]; - for (const kind of orderedKinds) { - if (kind === "markdown") { - parts.push({ kind: "markdown", markdown: readContent(flags.md || "-") }); - } else if (kind === "mermaid") { - parts.push({ kind: "mermaid", mermaid: readContent(flags.mermaid || "-") }); - } else if (kind === "diff") { - parts.push({ - kind: "diff", - patch: readContent(flags.diff || "-"), - ...(flags.layout === "split" && { layout: "split" }), - }); - } else if (kind === "terminal") { - parts.push({ kind: "terminal", text: readContent(flags.terminal || "-") }); - } else if (kind === "json") { - const text = readContent(flags.json || "-"); - try { - parts.push({ kind: "json", data: JSON.parse(text) }); - } catch { - fail(`--json: invalid JSON${flags.json ? ` in ${flags.json}` : ""}`); - } - } else if (kind === "code") { - const codeFile = flags.code || "-"; - const part = { kind: "code", code: readContent(codeFile) }; - const codeLang = codeFile !== "-" ? inferLang(codeFile) : undefined; - if (codeLang) part.language = codeLang; - if (codeFile !== "-") part.title = codeFile.split("/").pop() || codeFile; - parts.push(part); - } else if (kind === "image") { - const asset = await uploadFile(flags.image, { session, kind: "image" }); - parts.push({ kind: "image", assetId: asset.id }); - } - } + // Surfaces render top-to-bottom, so order is user-visible. `surfacesFromFlags` + // walks parseArgs tokens (command-line order, repeats included) and builds + // one surface per flag occurrence — so --diff a --diff b yields two diffs. + const parts = [ + htmlPart, + ...(await surfacesFromFlags(flags, tokens, { session, layout: flags.layout })), + ]; outSurface(await publishSurface(parts, { ...flags, session })); }, @@ -1148,13 +1202,13 @@ const commands = { tokens: true, allowPositionals: true, options: { - md: { type: "string" }, - mermaid: { type: "string" }, - diff: { type: "string" }, - terminal: { type: "string" }, - json: { type: "string" }, - code: { type: "string" }, - image: { type: "string" }, + md: { type: "string", multiple: true }, + mermaid: { type: "string", multiple: true }, + diff: { type: "string", multiple: true }, + terminal: { type: "string", multiple: true }, + json: { type: "string", multiple: true }, + code: { type: "string", multiple: true }, + image: { type: "string", multiple: true }, before: { type: "string" }, after: { type: "string" }, layout: { type: "string" }, @@ -1164,57 +1218,13 @@ const commands = { const postId = positionals[0]; if (!postId) fail("usage: sideshow surface add [--md f] [--code f] ..."); - const SURFACE_FLAGS = new Map([ - ["md", "markdown"], - ["mermaid", "mermaid"], - ["diff", "diff"], - ["terminal", "terminal"], - ["json", "json"], - ["code", "code"], - ["image", "image"], - ]); - const orderedKinds = []; - const seen = new Set(); - for (const t of tokens ?? []) { - if (t.kind === "option" && SURFACE_FLAGS.has(t.name) && !seen.has(t.name)) { - seen.add(t.name); - orderedKinds.push(SURFACE_FLAGS.get(t.name)); - } - } - if (orderedKinds.length === 0) fail("provide at least one surface flag (--md, --code, ...)"); const session = await resolveSession(flags, { create: true }); + const surfaces = await surfacesFromFlags(flags, tokens, { session, layout: flags.layout }); + if (surfaces.length === 0) fail("provide at least one surface flag (--md, --code, ...)"); + // Each surface is a separate append call so --before/--after positioning + // applies per surface (repeats append in command-line order). let lastResult; - for (const kind of orderedKinds) { - let surface; - if (kind === "markdown") { - surface = { kind: "markdown", markdown: readContent(flags.md || "-") }; - } else if (kind === "mermaid") { - surface = { kind: "mermaid", mermaid: readContent(flags.mermaid || "-") }; - } else if (kind === "diff") { - surface = { - kind: "diff", - patch: readContent(flags.diff || "-"), - ...(flags.layout === "split" && { layout: "split" }), - }; - } else if (kind === "terminal") { - surface = { kind: "terminal", text: readContent(flags.terminal || "-") }; - } else if (kind === "json") { - const text = readContent(flags.json || "-"); - try { - surface = { kind: "json", data: JSON.parse(text) }; - } catch { - fail(`--json: invalid JSON${flags.json ? ` in ${flags.json}` : ""}`); - } - } else if (kind === "code") { - const codeFile = flags.code || "-"; - surface = { kind: "code", code: readContent(codeFile) }; - const codeLang = codeFile !== "-" ? inferLang(codeFile) : undefined; - if (codeLang) surface.language = codeLang; - if (codeFile !== "-") surface.title = codeFile.split("/").pop() || codeFile; - } else if (kind === "image") { - const asset = await uploadFile(flags.image, { session, kind: "image" }); - surface = { kind: "image", assetId: asset.id }; - } + for (const surface of surfaces) { const body = { surface }; if (flags.before !== undefined) body.before = flags.before; if (flags.after !== undefined) body.after = flags.after; diff --git a/server/types.ts b/server/types.ts index 3d4c4a4..9391bc9 100644 --- a/server/types.ts +++ b/server/types.ts @@ -371,11 +371,19 @@ export const MAX_WORKSPACE_ASSET_BYTES = 2 * 1024 * 1024 * 1024; // kept only the first 32-bit segment (~4e9), brute-forceable in about an hour. // (Assets use a separate content-hash id, not this.) btoa is a global in both // Node and Workers, same as the atob the asset path already relies on. -export const newId = () => - btoa(String.fromCharCode(...crypto.getRandomValues(new Uint8Array(8)))) +export const newId = () => { + let id = btoa(String.fromCharCode(...crypto.getRandomValues(new Uint8Array(8)))) .replace(/\+/g, "-") .replace(/\//g, "_") .replace(/=+$/, ""); + // Ids are used as CLI positional args and path segments. A leading "-" or + // "_" makes node:util parseArgs treat them as options ("Unknown option '-6'" + // for an id like "-6K4AJsKD4M"), so swap a leading separator for an + // alphanumeric. Collision risk is negligible (the remaining ~10 chars hold + // ~8e17 possibilities). + if (id[0] === "-" || id[0] === "_") id = "0" + id.slice(1); + return id; +}; // Content-addressed asset id: the lowercase hex SHA-256 of the bytes. Because // it depends only on the content, an agent can derive `/a/:id` from the bytes diff --git a/test/cli.test.ts b/test/cli.test.ts index eaf8479..59f58e8 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -496,6 +496,40 @@ test("publish surfaces with --terminal before --md produces terminal-then-markdo } }); +test("publish repeats a surface flag to add several of the same kind, in order", async () => { + const server = await serveSession(); + try { + const html = tmpFile("h.html", "
x
"); + const a = tmpFile("a.patch", "--- a/f\n+++ b/f\n@@ -1 +1 @@\n-a\n+aa\n"); + const b = tmpFile("b.patch", "--- a/f\n+++ b/f\n@@ -1 +1 @@\n-b\n+bb\n"); + const code = tmpFile("c.ts", "const x = 1;"); + const { code: exit, stdout } = await cli( + server, + "publish", + html, + "--diff", + a, + "--code", + code, + "--diff", + b, + ); + assert.equal(exit, 0); + const out = JSON.parse(stdout); + // Two diff surfaces appear, with the code surface between them in argv order. + assert.deepEqual(out.kinds, ["html", "diff", "code", "diff"]); + const full = (await fetch(`${server.url}/api/surfaces/${out.id}`).then((r) => r.json())) as any; + const diffs = full.surfaces.filter((s: any) => s.kind === "diff"); + assert.equal(diffs.length, 2); + assert.deepEqual( + diffs.map((s: any) => s.patch), + [readFileSync(a, "utf8"), readFileSync(b, "utf8")], + ); + } finally { + await server.close(); + } +}); + test("publish --code infers the language from the filename", async () => { const server = await serveSession(); try { @@ -735,6 +769,32 @@ test("surface add appends a diff surface with --layout split", async () => { } }); +test("surface add repeats a flag to append several of the same kind, in order", async () => { + const server = await serveSession(); + try { + const html = tmpFile("h.html", "

first

"); + const pub = await cli(server, "publish", html); + const id = JSON.parse(pub.stdout).id; + + const a = tmpFile("a.md", "# first append"); + const b = tmpFile("b.md", "# second append"); + const { code } = await cli(server, "surface", "add", id, "--md", a, "--md", b); + assert.equal(code, 0); + + const full = (await fetch(`${server.url}/api/posts/${id}`).then((r) => r.json())) as any; + assert.deepEqual( + full.surfaces.map((s: any) => s.kind), + ["html", "markdown", "markdown"], + ); + assert.deepEqual( + full.surfaces.slice(1).map((s: any) => s.markdown), + ["# first append", "# second append"], + ); + } finally { + await server.close(); + } +}); + test("surface remove deletes a surface by index", async () => { const server = await serveSession(); try {