diff --git a/README.md b/README.md index 35e72922..1547557f 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,10 @@ The customer surface is content (`app`, `media`, `compose`), playlists, and scre Discover commands progressively with `screenrig --help`, `screenrig screen --help`, and `screenrig screen assign --help`. Deeper groups work the same way: `screenrig comment show --help`. `screenrig help screen assign` is equivalent. -Add `--json` for structured child command paths, invocation syntax, and option types; +Use `screenrig help --all` for the complete command inventory, or +`screenrig help --all screen` for one group's descendants. Normal help lists +immediate children. Add `--json` for structured command paths, positional arguments, +option choices and defaults, relationships, and examples; help runs without configuration or authentication. Command-specific options follow that command, for example `screenrig screen update ID --name Lobby --if-match 1`. Global options such as `--json` may appear before or after the command. Use @@ -199,7 +202,11 @@ repeat the identical command and input with the same config to resume. If playli creation succeeded before assignment failed, the error identifies the created playlist. Do not delete it or start another create to recover. A revision conflict requires inspecting the screen and reconciling the intended assignment; an already -created playlist can be assigned explicitly with `screen assign`. Publishing never +created playlist can be assigned explicitly with `screen assign`. Assignment-conflict +guidance includes an inspection command and a separate assignment template with +``. Replace that placeholder only after inspecting the screen and +deciding the assignment is still intended. Structured argument arrays preserve the +selected configuration and API origin without requiring shell parsing. Publishing never silently refreshes the expected revision. Unfinished recovery stops after the 24-hour server idempotency window; inspect and reconcile before making more writes. diff --git a/src/argv.ts b/src/argv.ts index 3061622e..813aacd8 100644 --- a/src/argv.ts +++ b/src/argv.ts @@ -18,7 +18,7 @@ export function parseArgv(argv: string[]): ParsedArgs & { help?: HelpDocument } const args = commandInput(tree.selected()); if (tree.versionRequested()) args.flags.version = true; if (tree.helpRequested()) { - const help = describeHelp(tree.selected()); + const help = describeHelp(tree.selected(), tree.inventoryRequested()); return { command: help.path.slice(0, 2), positionals: help.path, flags: { ...args.flags, help: true }, help }; } return args; diff --git a/src/cli-commands/compose.ts b/src/cli-commands/compose.ts index 59321162..2b04737a 100644 --- a/src/cli-commands/compose.ts +++ b/src/cli-commands/compose.ts @@ -1,9 +1,8 @@ -import { usageError } from "../problems.js"; import { positiveInteger } from "./options.js"; import type { CommandActionBinder } from "./types.js"; import { handleComposeCatalog, handleComposeBatch, handleComposeRender } from "../commands.js"; import type { Command } from "commander"; -import { addCommandNotes } from "./notes.js"; +import { addCommandNotes, requireOptionGroup } from "./notes.js"; import { LOOK_AT_THE_CONTACT_SHEET } from "../playlist-preview.js"; export function registerComposeCommands(root: Command, bind: CommandActionBinder): void { @@ -20,7 +19,6 @@ export function registerComposeCommands(root: Command, bind: CommandActionBinder .option("--target-height ", "Set the physical content height", positiveInteger("target-height")) .option("--safe-area", "Apply the display safe area") .option("--lint-only", "Run the command's local validation/lint mode") - .hook("preAction", requireTargetDimensions) .action(bind(handleComposeBatch)), `Render 1 to 2000 pages. ${LOOK_AT_THE_CONTACT_SHEET}`); addCommandNotes(compose.command("render").description("Render a composition locally") @@ -32,12 +30,8 @@ export function registerComposeCommands(root: Command, bind: CommandActionBinder .option("--safe-area", "Apply the display safe area") .option("--open", "Open the result in its viewer") .option("--lint-only", "Run the command's local validation/lint mode") - .hook("preAction", requireTargetDimensions) .action(bind(handleComposeRender)), LOOK_AT_THE_CONTACT_SHEET); -} - -function requireTargetDimensions(command: Command): void { - const width = command.getOptionValueSource("targetWidth") === "cli"; - const height = command.getOptionValueSource("targetHeight") === "cli"; - if (width !== height) throw usageError("Provide both --target-width and --target-height."); + for (const command of compose.commands.filter((command) => ["batch", "render"].includes(command.name()))) { + requireOptionGroup(command, "together", ["--target-width", "--target-height"]); + } } diff --git a/src/cli-commands/media.ts b/src/cli-commands/media.ts index c61d0b16..a2efcffc 100644 --- a/src/cli-commands/media.ts +++ b/src/cli-commands/media.ts @@ -1,9 +1,8 @@ -import { usageError } from "../problems.js"; import { positiveInteger, positiveNumber, revision } from "./options.js"; import type { CommandActionBinder } from "./types.js"; import { handleMediaGenerate, handleMediaUpload, handleMediaUploadBatch, handleMediaShow, handleMediaDownload, handleMediaList, handleMediaUpdate, handleMediaDelete } from "../commands.js"; import { type Command, Option } from "commander"; -import { addCommandNotes } from "./notes.js"; +import { addCommandNotes, addCommandExamples, requireOptionGroup } from "./notes.js"; import { CREDIT_HELP } from "../help-text.js"; export function registerMediaCommands(root: Command, bind: CommandActionBinder): void { @@ -12,14 +11,16 @@ export function registerMediaCommands(root: Command, bind: CommandActionBinder): addCommandNotes(media.command("generate").description("Generate an image from a prompt") .addOption(new Option("--prompt ", "Describe the complete image to generate").conflicts("promptFile")) .addOption(new Option("--prompt-file ", "Read the complete prompt from a file or stdin (-)").conflicts("prompt")) - .option("--aspect-ratio ", "Choose the generated image aspect ratio") - .option("--quality ", "Choose generation quality") + .addOption(new Option("--aspect-ratio ", "Choose the generated image aspect ratio").choices(["1:1", "16:9", "9:16", "4:3", "3:4", "3:2", "2:3"]).default("16:9")) + .addOption(new Option("--quality ", "Choose generation quality").choices(["low", "medium", "high"]).default("medium")) .option("--tag ", "Set the media tag") .option("--no-progress", "Suppress stderr progress") - .hook("preAction", (command) => { - if (command.getOptionValue("prompt") === undefined && command.getOptionValue("promptFile") === undefined) throw usageError("Provide --prompt TEXT or --prompt-file FILE."); - }) .action(bind(handleMediaGenerate)), CREDIT_HELP); + const generate = media.commands.find((command) => command.name() === "generate")!; + requireOptionGroup(generate, "exactlyOne", ["--prompt", "--prompt-file"]); + addCommandExamples(generate, + 'screenrig media generate --prompt "A full-screen welcome sign"', + 'screenrig media generate --prompt-file prompt.txt --aspect-ratio 9:16'); media.command("upload").description("Upload an image or video") .argument("", "Local input file") @@ -28,12 +29,12 @@ export function registerMediaCommands(root: Command, bind: CommandActionBinder): .option("--no-wait", "Return after acceptance without waiting for processing") .option("--poll-ms ", "Set the operation polling interval", positiveInteger("poll-ms")) .addOption(new Option("--no-transcode", "Upload accepted source bytes unchanged").conflicts(["codec", "maxFps", "maxEdge", "webpQuality", "preset", "audio"])) - .addOption(new Option("--codec ", "Choose the video codec (default: h264)").conflicts(["transcode"])) + .addOption(new Option("--codec ", "Choose the video codec").choices(["h264", "hevc"]).default("h264").conflicts(["transcode"])) .addOption(new Option("--max-fps ", "Limit video frames per second").argParser(positiveNumber("max-fps")).conflicts(["transcode"])) .addOption(new Option("--max-edge ", "Limit the longest video edge").argParser(positiveInteger("max-edge")).conflicts(["transcode"])) .addOption(new Option("--webp-quality <1-100>", "Set WebP quality").argParser(positiveInteger("webp-quality")).conflicts(["transcode"])) .option("--no-progress", "Suppress stderr progress") - .addOption(new Option("--preset ", "Use signage-1080p30 or signage-4k30").conflicts(["transcode"])) + .addOption(new Option("--preset ", "Choose a transcoding preset").choices(["signage-1080p30", "signage-4k30"]).conflicts(["transcode"])) .addOption(new Option("--no-audio", "Remove the audio track when transcoding").conflicts(["transcode"])) .action(bind(handleMediaUpload)); @@ -45,11 +46,11 @@ export function registerMediaCommands(root: Command, bind: CommandActionBinder): .option("--tag ", "Set the media tag") .option("--no-progress", "Suppress stderr progress") .option("--poll-ms ", "Set the operation polling interval", positiveInteger("poll-ms")) - .addOption(new Option("--codec ", "Choose the video codec (default: h264)").conflicts(["transcode"])) + .addOption(new Option("--codec ", "Choose the video codec").choices(["h264", "hevc"]).default("h264").conflicts(["transcode"])) .addOption(new Option("--max-fps ", "Limit video frames per second").argParser(positiveNumber("max-fps")).conflicts(["transcode"])) .addOption(new Option("--max-edge ", "Limit the longest video edge").argParser(positiveInteger("max-edge")).conflicts(["transcode"])) .addOption(new Option("--webp-quality <1-100>", "Set WebP quality").argParser(positiveInteger("webp-quality")).conflicts(["transcode"])) - .addOption(new Option("--preset ", "Use signage-1080p30 or signage-4k30").conflicts(["transcode"])) + .addOption(new Option("--preset ", "Choose a transcoding preset").choices(["signage-1080p30", "signage-4k30"]).conflicts(["transcode"])) .addOption(new Option("--no-audio", "Remove the audio track when transcoding").conflicts(["transcode"])) .action(bind(handleMediaUploadBatch)); @@ -64,7 +65,7 @@ export function registerMediaCommands(root: Command, bind: CommandActionBinder): media.command("list").description("List media") .option("--tag ", "Filter media by tag") - .option("--primitive ", "Filter by media primitive") + .addOption(new Option("--primitive ", "Filter by media primitive").choices(["image", "video"])) .action(bind(handleMediaList)); media.command("update").description("Update a media tag") diff --git a/src/cli-commands/notes.ts b/src/cli-commands/notes.ts index 81e771bc..a44d1628 100644 --- a/src/cli-commands/notes.ts +++ b/src/cli-commands/notes.ts @@ -1,4 +1,5 @@ import type { Command } from "commander"; +import { usageError } from "../problems.js"; const notes = new WeakMap(); @@ -11,3 +12,41 @@ export function addCommandNotes(command: Command, note: string): void { export function commandNotes(command: Command): string[] { return [...(notes.get(command) ?? [])]; } + +export interface OptionRelationship { + kind: "exactlyOne" | "together"; + options: string[]; +} +const relationships = new WeakMap(); +const examples = new WeakMap(); + +/** One definition drives both preflight validation and discovery. */ +export function requireOptionGroup(command: Command, kind: OptionRelationship["kind"], names: string[]): void { + const options = names.map((name) => { + const option = command.options.find((candidate) => candidate.long === name); + if (!option) throw new Error(`Unknown option ${name}`); + return option; + }); + relationships.set(command, [...(relationships.get(command) ?? []), { kind, options: names }]); + const message = kind === "exactlyOne" + ? `Provide exactly one of ${names.join(" or ")}.` + : `Provide ${names.join(" and ")} together.`; + addCommandNotes(command, message); + command.hook("preAction", () => { + const count = options.filter((option) => command.getOptionValueSource(option.attributeName()) === "cli").length; + if (kind === "exactlyOne" ? count !== 1 : count !== 0 && count !== options.length) throw usageError(message); + }); +} + +export function commandRelationships(command: Command): OptionRelationship[] { + return relationships.get(command) ?? []; +} + +export function addCommandExamples(command: Command, ...commands: string[]): void { + examples.set(command, [...(examples.get(command) ?? []), ...commands]); + command.addHelpText("after", `\nExamples:\n${commands.map((example) => ` ${example}`).join("\n")}`); +} + +export function commandExamples(command: Command): string[] { + return examples.get(command) ?? []; +} diff --git a/src/cli-commands/playlist.ts b/src/cli-commands/playlist.ts index ed665eb9..3b464535 100644 --- a/src/cli-commands/playlist.ts +++ b/src/cli-commands/playlist.ts @@ -1,24 +1,27 @@ -import { usageError } from "../problems.js"; import { nonnegativeInteger, positiveInteger, revision } from "./options.js"; import type { CommandActionBinder } from "./types.js"; import { handlePlaylistInit, handlePlaylistValidate, handlePlaylistPreview, handlePlaylistTemplates, handlePlaylistCreate, handlePlaylistUpdate, handlePlaylistExport, handlePlaylistImport, handlePlaylistShow, handlePlaylistList, handlePlaylistDelete } from "../commands.js"; -import type { Command } from "commander"; -import { addCommandNotes } from "./notes.js"; +import { type Command, Option } from "commander"; +import { addCommandNotes, addCommandExamples, requireOptionGroup } from "./notes.js"; import { LOOK_AT_THE_CONTACT_SHEET } from "../playlist-preview.js"; export function registerPlaylistCommands(root: Command, bind: CommandActionBinder): void { const playlist = root.command("playlist").description("Author, validate, preview, and manage playlists"); - playlist.command("init").description("Prepare an editable full-screen playlist from ready media") + const init = playlist.command("init").description("Prepare an editable full-screen playlist from ready media") .argument("", "Media identifiers in playback order") .requiredOption("--name ", "Playlist name") .requiredOption("--output ", "Create an editable playlist file") .option("--screen ", "Use this screen's reported playback dimensions") .option("--target-width ", "Override canvas width", positiveInteger("target-width")) .option("--target-height ", "Override canvas height", positiveInteger("target-height")) - .option("--duration-ms ", "Image page duration (default: 8000)", positiveInteger("duration-ms")) - .option("--fit ", "Content fit: contain (default), cover, or fill") + .addOption(new Option("--duration-ms ", "Image page duration").argParser(positiveInteger("duration-ms")).default(8000)) + .addOption(new Option("--fit ", "Content fit").choices(["contain", "cover", "fill"]).default("contain")) .action(bind(handlePlaylistInit)); + requireOptionGroup(init, "together", ["--target-width", "--target-height"]); + addCommandNotes(init, "Provide --screen for reported dimensions, or both target dimensions to set the canvas explicitly."); + addCommandExamples(init, 'screenrig playlist init med_IMAGE --name Lobby --screen scr_SCREEN --output lobby.json', + 'screenrig playlist init med_IMAGE med_VIDEO --name Lobby --target-width 1920 --target-height 1080 --output lobby.json'); playlist.command("validate").description("Validate a playlist file") .argument("", "Local input file") @@ -51,25 +54,23 @@ export function registerPlaylistCommands(root: Command, bind: CommandActionBinde .requiredOption("--output ", "Write the exported bundle to this directory (required)") .action(bind(handlePlaylistExport)); - playlist.command("import").description("Import a playlist bundle") + const importCommand = playlist.command("import").description("Import a playlist bundle") .argument("", "Local directory") .option("--name ", "Set the imported playlist name") .option("--update ", "Update this playlist when importing") .option("--expect-rev ", "Require the current resource revision", revision) .option("--poll-ms ", "Set the operation polling interval", positiveInteger("poll-ms")) - .hook("preAction", (command) => { - const update = command.getOptionValueSource("update") === "cli"; - const ifMatch = command.getOptionValueSource("expectRev") === "cli"; - if (update !== ifMatch) throw usageError("playlist import --update and --expect-rev must be supplied together."); - }) .action(bind(handlePlaylistImport)); + requireOptionGroup(importCommand, "together", ["--update", "--expect-rev"]); - playlist.command("show").description("Inspect a playlist or write an editable document") + const show = playlist.command("show").description("Inspect a playlist or write an editable document") .option("--output ", "Create an editable playlist file; report its revision on stdout") .option("--editable", "Return an editable document and revision in the JSON envelope") .alias("get") .argument("", "Playlist identifier") .action(bind(handlePlaylistShow)); + addCommandExamples(show, 'screenrig playlist show pl_PLAYLIST --output playlist.json', + 'screenrig playlist show pl_PLAYLIST --editable'); playlist.command("list").description("List playlists") .action(bind(handlePlaylistList)); diff --git a/src/cli-errors.ts b/src/cli-errors.ts index 12e1220e..276fff54 100644 --- a/src/cli-errors.ts +++ b/src/cli-errors.ts @@ -17,6 +17,15 @@ export function commandError(error: CommanderError, command: Command): never { } if (path === "media list" && has("kind")) throw usageError("media list uses --primitive image|video, not --kind."); switch (error.code) { + case "commander.invalidArgument": { + const option = command.options.find((candidate) => candidate.argChoices && error.message.startsWith(`error: option '${candidate.flags}'`)); + if (option?.argChoices) { + const choices = option.argChoices; + const allowed = choices.length > 2 ? `${choices.slice(0, -1).join(", ")}, or ${choices.at(-1)}` : choices.join(" or "); + throw usageError(`${option.long} must be ${allowed}.`); + } + throw usageError("Invalid argument. See command help for supported values."); + } case "commander.missingArgument": if (path === "screen set-timezone") throw usageError("screen set-timezone requires --timezone --expect-rev."); throw usageError(`${path} requires ${command.registeredArguments.filter((arg) => arg.required).map((arg) => `<${arg.name()}>`).join(" ")}.`); @@ -38,4 +47,3 @@ export function commandError(error: CommanderError, command: Command): never { throw usageError("Unknown command or unsupported option. See command help for supported arguments.", { command: `screenrig${path ? ` ${path}` : ""} --help`, reason: "List supported commands, arguments, and options." }); } } - diff --git a/src/command-tree.ts b/src/command-tree.ts index a625ed90..c287d091 100644 --- a/src/command-tree.ts +++ b/src/command-tree.ts @@ -14,6 +14,7 @@ export interface CommandTree { selected: () => Command; helpRequested: () => boolean; versionRequested: () => boolean; + inventoryRequested: () => boolean; } export function commandInput(command: Command): ParsedArgs { @@ -60,6 +61,7 @@ export function createCommandTree( let selected = root; let helpRequested = false; let versionRequested = false; + let inventoryRequested = false; const seen = new Set(); registerCommands(root, (handler) => (...values: unknown[]) => { @@ -67,8 +69,10 @@ export function createCommandTree( return execute?.(handler, commandInput(selected)); }); root.command("help [command...]") - .description("Discover commands and their options") - .action((path: string[]) => { + .description("Discover commands; --all lists every command") + .option("--all", "Include all descendant command paths") + .action((path: string[], options: { all?: boolean }) => { + inventoryRequested = options.all === true; const target = findCommand(root, path); if (!target) throw usageError("Unknown help topic."); target.help(); @@ -96,5 +100,5 @@ export function createCommandTree( command.commands.forEach(configure); }; configure(root); - return { root, selected: () => selected, helpRequested: () => helpRequested, versionRequested: () => versionRequested }; + return { root, selected: () => selected, helpRequested: () => helpRequested, versionRequested: () => versionRequested, inventoryRequested: () => inventoryRequested }; } diff --git a/src/envelope.ts b/src/envelope.ts index 89b1697b..8e604386 100644 --- a/src/envelope.ts +++ b/src/envelope.ts @@ -1,6 +1,10 @@ export interface ProblemNext { command: string; reason: string; + /** Arguments excluding the executable; avoids shell quoting ambiguity. */ + argv?: string[]; + /** Conditional template: inspect and reconcile before replacing placeholders. */ + after_inspection?: { command: string; argv: string[]; reason: string }; } export interface NormalizedProblem { diff --git a/src/help.test.ts b/src/help.test.ts index 9fdb8f1a..9d22aadd 100644 --- a/src/help.test.ts +++ b/src/help.test.ts @@ -34,6 +34,9 @@ test("root help is compact and every command is discoverable through immediate c } visit([]); assert.deepEqual(discovered.sort(), [ + "recovery list", + "recovery show", + "recovery reconcile", "account show", "agent enroll", "agent connect", @@ -103,21 +106,20 @@ test("root help is compact and every command is discoverable through immediate c "feedback list", "doctor", "version", - "recovery list", - "recovery show", - "recovery reconcile", ].sort()); + assert.equal(root.allCommands, undefined); + assert.doesNotMatch(root.usage, /All commands:/); const inventory = leafCommandPaths(createCommandTree().root); assert.ok(inventory.includes("help")); for (const path of inventory) { - assert.ok(root.usage.includes(path), path); + assert.ok(commandHelp([], true).usage.includes(path), path); } assert.ok(root.commands.every((child) => child.path.length === 1)); assert.equal(root.commands.some((child) => child.name === "generate"), false); }); test("group help stays focused and still names descendant leaf paths", () => { - const media = commandHelp(["media"]); + const media = commandHelp(["media"], true); assert.match(media.usage, /Usage: screenrig media \[options\] \[command\]/); assert.ok(media.commands.every((child) => child.path[0] === "media" && child.path.length === 2)); assert.deepEqual(media.commands.map((child) => child.name), [ @@ -129,8 +131,8 @@ test("group help stays focused and still names descendant leaf paths", () => { assert.doesNotMatch(media.usage, /screen assign|account show|agent enroll|playlist create/); }); -test("human root --help includes every taught leaf command path", () => { - const text = execFileSync(process.execPath, [bin, "--help"], { +test("explicit human inventory includes every leaf while default root stays compact", () => { + const text = execFileSync(process.execPath, [bin, "help", "--all"], { encoding: "utf8", env: { ...process.env, SCREENRIG_CONFIG: "/nonexistent/screenrig-help-config" }, }); for (const path of leafCommandPaths(createCommandTree().root)) { @@ -195,3 +197,42 @@ test("negative switches are documented as booleans without value placeholders", assert.match(help.usage, new RegExp(`${name}\\s+[^<\\s]`)); } }); + +test("structured discovery exposes positional cardinality, choices, defaults and examples", () => { + const init = invoke(["playlist", "init", "--help"]).data; + assert.deepEqual(init.arguments, [{ name: "media-ids", description: "Media identifiers in playback order", required: true, variadic: true }]); + assert.deepEqual(init.options.find((option) => option.name === "--fit")?.choices, ["contain", "cover", "fill"]); + assert.equal(init.options.find((option) => option.name === "--duration-ms")?.default, 8000); + assert.ok(init.examples.length > 0); + assert.deepEqual(init.relationships, [{ kind: "together", options: ["--target-width", "--target-height"] }]); + const generate = invoke(["media", "generate", "--help"]).data; + assert.equal(generate.options.find((option) => option.name === "--quality")?.default, "medium"); + assert.equal(generate.options.find((option) => option.name === "--aspect-ratio")?.default, "16:9"); + assert.deepEqual(generate.options.find((option) => option.name === "--prompt")?.conflicts, ["--prompt-file"]); + assert.deepEqual(generate.relationships, [{ kind: "exactlyOne", options: ["--prompt", "--prompt-file"] }]); + assert.equal(generate.usage.split("Provide exactly one").length, 2, "notes should appear only once"); +}); + +test("explicit JSON inventory can be scoped to a group", () => { + assert.deepEqual(invoke(["help", "--all"]).data.allCommands, leafCommandPaths(createCommandTree().root)); + const media = invoke(["help", "media", "--all"]).data; + assert.ok(media.allCommands?.every((path) => path.startsWith("media "))); + assert.ok(media.allCommands?.includes("media generate")); +}); + +test("declared relationships and choices reject invalid commands before authentication", () => { + for (const args of [ + ["media", "generate"], + ["media", "generate", "--prompt", "test", "--quality", "ultra"], + ["playlist", "init", "med_TEST", "--name", "Test", "--output", "out.json", "--target-width", "1920"], + ["compose", "render", "input.json", "--target-height", "1080"], + ["playlist", "import", "bundle", "--update", "pl_TEST"], + ]) { + assert.throws(() => invoke(args), (error: unknown) => { + const failure = error as { status: number; stdout: string }; + assert.equal(failure.status, 2); + assert.equal(JSON.parse(failure.stdout).error.code, "usage_error"); + return true; + }); + } +}); diff --git a/src/help.ts b/src/help.ts index c61be5cc..559a07c6 100644 --- a/src/help.ts +++ b/src/help.ts @@ -1,9 +1,21 @@ import type { Command, Option } from "commander"; import { commandPath, createCommandTree, findCommand } from "./command-tree.js"; -import { commandNotes } from "./cli-commands/notes.js"; +import { commandNotes, commandExamples, commandRelationships, type OptionRelationship } from "./cli-commands/notes.js"; import { usageError } from "./problems.js"; export { CREDIT_HELP } from "./help-text.js"; +export interface HelpOption { + name: string; + type: "boolean" | "value"; + description: string; + required: boolean; + valueRequired: boolean; + variadic: boolean; + choices?: string[]; + default?: unknown; + conflicts: string[]; +} + export interface HelpDocument { path: string[]; aliases: string[][]; @@ -11,8 +23,12 @@ export interface HelpDocument { usage: string; synopsis: string[]; commands: Array<{ name: string; path: string[]; kind: "group" | "command"; summary: string; help: string }>; - options: Array<{ name: string; type: "boolean" | "value"; description: string; required: boolean }>; - globalOptions: Array<{ name: string; type: "boolean" | "value"; description: string; required: boolean }>; + arguments: Array<{ name: string; description: string; required: boolean; variadic: boolean; choices?: string[]; default?: unknown }>; + options: HelpOption[]; + globalOptions: HelpOption[]; + relationships: OptionRelationship[]; + examples: string[]; + allCommands?: string[]; notes: string[]; } @@ -40,32 +56,48 @@ function commandInventory(command: Command): string | undefined { } /** JSON discovery and human help both read the actual Commander command tree. */ -export function describeHelp(command: Command): HelpDocument { +export function describeHelp(command: Command, all = false): HelpDocument { const helper = command.createHelp(); const path = commandPath(command); const notes = commandNotes(command); - const inventory = commandInventory(command); - const describeOption = (option: Option) => ({ - name: option.long!, type: (option.negate || option.isBoolean()) ? "boolean" as const : "value" as const, description: option.description, required: option.mandatory, + const inventory = all ? commandInventory(command) : undefined; + const examples = commandExamples(command); + const visibleOptions = [...helper.visibleOptions(command), ...helper.visibleGlobalOptions(command)]; + const describeOption = (option: Option): HelpOption => ({ + name: option.long!, type: (option.negate || option.isBoolean()) ? "boolean" : "value", description: option.description, required: option.mandatory, + valueRequired: option.required, variadic: option.variadic, + ...(option.argChoices ? { choices: option.argChoices } : {}), + ...(option.defaultValue !== undefined ? { default: option.defaultValue } : {}), + // Commander exposes these native conflict names at runtime but omits them from its typings. + conflicts: ((option as Option & { conflictsWith?: string[] }).conflictsWith ?? []) + .flatMap((name) => visibleOptions.filter((candidate) => candidate.attributeName() === name).map((candidate) => candidate.long!)), }); return { path, aliases: command.aliases().map((alias) => [...path.slice(0, -1), alias]), kind: command.commands.length ? "group" : "command", - usage: [command.helpInformation().trimEnd(), inventory, ...notes].filter((part): part is string => part !== undefined).join("\n\n"), + usage: [command.helpInformation().trimEnd(), inventory, ...notes, ...(examples.length ? [`Examples:\n${examples.map((example) => ` ${example}`).join("\n")}`] : [])].filter((part): part is string => part !== undefined).join("\n\n"), synopsis: [helper.commandUsage(command)], commands: helper.visibleCommands(command).map((child) => ({ name: child.name(), path: commandPath(child), kind: child.commands.length ? "group" as const : "command" as const, summary: child.description(), help: `screenrig ${commandPath(child).join(" ")} --help`, })), + arguments: command.registeredArguments.map((argument) => ({ + name: argument.name(), description: argument.description, required: argument.required, variadic: argument.variadic, + ...(argument.argChoices ? { choices: argument.argChoices } : {}), + ...(argument.defaultValue !== undefined ? { default: argument.defaultValue } : {}), + })), + relationships: commandRelationships(command), + examples: commandExamples(command), + ...(all ? { allCommands: leafCommandPaths(command) } : {}), options: helper.visibleOptions(command).map(describeOption), globalOptions: helper.visibleGlobalOptions(command).map(describeOption), notes, }; } -export function commandHelp(path: readonly string[] = []): HelpDocument { +export function commandHelp(path: readonly string[] = [], all = false): HelpDocument { const command = findCommand(createCommandTree().root, path); if (!command) throw usageError("Unknown help topic."); - return describeHelp(command); + return describeHelp(command, all); } diff --git a/src/playlist-workflows.test.ts b/src/playlist-workflows.test.ts index be24cffb..4156069a 100644 --- a/src/playlist-workflows.test.ts +++ b/src/playlist-workflows.test.ts @@ -145,6 +145,36 @@ test("assignment conflicts retain the created playlist and never create another .on('POST','/api/v1/playlists',()=>response({id:'pl_TEST',revision:1})) .on('PATCH','/api/v1/screens/scr_TEST',()=>({status:409,headers:{},body:{code:'revision_conflict',title:'Conflict',status:409,detail:'Screen changed',current_revision:8}})); const options={client:new ApiClient({transport}),runtime:processRuntime(),configPath:dir+'/config.json',apiUrl:'https://api.screenrig.ai',screenId:'scr_TEST',revision:'7',document:document()}; - for(let i=0;i<2;i++) await assert.rejects(()=>publishScreen(options),(e:any)=>{assert.equal(e.problem.errors.at(-1).playlist_id,'pl_TEST');return true;}); + for(let i=0;i<2;i++) await assert.rejects(()=>publishScreen(options),(e:any)=>{ + assert.equal(e.problem.errors.at(-1).playlist_id,'pl_TEST'); + assert.deepEqual(e.problem.next.argv,['screen','show','scr_TEST','--config',options.configPath,'--api-url',options.apiUrl]); + assert.deepEqual(e.problem.next.after_inspection.argv,['screen','assign','scr_TEST','--playlist-id','pl_TEST','--expect-rev','','--config',options.configPath,'--api-url',options.apiUrl]); + assert.match(e.problem.next.reason,/Do not rerun publish with a new revision/); + assert.match(e.problem.next.after_inspection.reason,/Only if this assignment is still intended/); + // The placeholder cannot accidentally execute a write before reconciliation. + assert.throws(()=>parseArgv(e.problem.next.after_inspection.argv)); + const reconciled=e.problem.next.after_inspection.argv.map((arg:string)=>arg===''?'8':arg); + assert.equal(parseArgv(reconciled).flags['if-match'],'8'); + return true; + }); assert.equal(transport.calls.filter(c=>c.method==='POST').length,1); + assert.equal(transport.calls.filter(c=>c.method==='GET'&&c.path==='/api/v1/screens/scr_TEST').length,1); + for(const call of transport.calls.filter(c=>c.method==='PATCH')) assert.equal(call.headers?.['if-match'],'"7"'); +}); + +test("publish recovery arguments preserve paths with spaces without exposing URL credentials",async t=>{ + const dir=await mkdtemp('/tmp/publish guidance-');t.after(()=>rm(dir,{recursive:true,force:true})); + const transport=new FakeTransport().on('GET','/api/v1/account',()=>response({id:'acc_TEST'})).on('GET','/api/v1/screens/scr_TEST',()=>response({id:'scr_TEST',revision:7})) + .on('POST','/api/v1/playlists',()=>response({id:'pl_TEST',revision:1})) + .on('PATCH','/api/v1/screens/scr_TEST',()=>({status:409,headers:{},body:{code:'revision_conflict',title:'Conflict',status:409,detail:'Screen changed',next:{command:'retry',reason:'Try again'}}})); + const options={client:new ApiClient({transport,token:'private-test-token'}),runtime:processRuntime(),configPath:dir+'/selected config.json',apiUrl:'https://user:password@api.screenrig.ai/prefix?secret=value#private',screenId:'scr_TEST',revision:'7',document:document()}; + await assert.rejects(()=>publishScreen(options),(e:any)=>{ + for(const args of [e.problem.next.argv,e.problem.next.after_inspection.argv]) { + assert.equal(args[args.indexOf('--config')+1],options.configPath); + assert.equal(args[args.indexOf('--api-url')+1],'https://api.screenrig.ai/prefix'); + } + assert.doesNotMatch(JSON.stringify(e.problem.next),/password|secret=value|private-test-token/); + assert.equal(e.problem.next.after_inspection.argv.includes('7'),false); + return true; + }); }); diff --git a/src/program.ts b/src/program.ts index 92d16df9..0c9b2c35 100644 --- a/src/program.ts +++ b/src/program.ts @@ -20,7 +20,7 @@ export async function executeCommand(argv: string[], runtime: CliRuntime): Promi if (!(error instanceof CommanderError) || !(tree.helpRequested() || tree.versionRequested())) throw error; } if (tree.helpRequested()) { - const help = describeHelp(tree.selected()); + const help = describeHelp(tree.selected(), tree.inventoryRequested()); return { envelope: successEnvelope(help), exitCode: ExitCode.Success, human: help.usage, output: "help" }; } if (tree.versionRequested()) return handleVersion({ command: ["version"], positionals: ["version"], flags: {} }, runtime); diff --git a/src/screen-publish.ts b/src/screen-publish.ts index f6eb50b2..16258384 100644 --- a/src/screen-publish.ts +++ b/src/screen-publish.ts @@ -100,7 +100,27 @@ export async function publishScreen(options: { } catch (error) { if (error instanceof CliError) { error.problem.errors.push({ stage: state?.assigned ? "verify" : state?.playlist ? "assign" : "create", playlist_id: state?.playlist?.id, journal: journalPath }); - if (!error.problem.next) error.problem.next = { command: `screenrig screen show ${screenId}`, reason: "Inspect the target. After a transport failure, repeat the identical publish command to resume; revision conflicts require reconciliation. The journal records any playlist already created." }; + // Preserve the selected destination without copying credentials or URL parameters. + const api = new URL(options.apiUrl); + api.username = ""; api.password = ""; api.search = ""; api.hash = ""; + const context = ["--config", options.configPath, "--api-url", api.toString().replace(/\/+$/, "")]; + if (!error.problem.next) error.problem.next = { + command: `screenrig screen show ${screenId}`, + argv: ["screen", "show", screenId, ...context], + reason: "Inspect the target using argv to preserve the selected configuration and API. After a transport failure, repeat the identical publish command to resume; revision conflicts require reconciliation. The journal records any playlist already created.", + }; + if (error.problem.code === "revision_conflict" && state?.playlist && !state.assigned) { + error.problem.next = { + command: `screenrig screen show ${screenId}`, + argv: ["screen", "show", screenId, ...context], + reason: "Inspect the current screen and reconcile concurrent changes before assigning the playlist already created. Do not rerun publish with a new revision: that starts a new publish and can create another playlist.", + after_inspection: { + command: `screenrig screen assign ${screenId} --playlist-id ${state.playlist.id} --expect-rev `, + argv: ["screen", "assign", screenId, "--playlist-id", state.playlist.id, "--expect-rev", "", ...context], + reason: "Only if this assignment is still intended, replace with the revision from the inspected screen. Use argv to preserve the configuration and API. The conflict response revision is not approval to overwrite concurrent changes.", + }, + }; + } } throw error; } finally { await rm(lockPath, { recursive: true, force: true }); }