fix(ship): preserve quoted slash arguments and reject invalid flags

docs/generated/commands.md

@@ -1,5 +1,5 @@
 <!-- GENERATED FILE: run `npm run docs:generate`; do not edit by hand. -->
-<!-- manifest-digest: sha256:29ba25dcab1d6c8eedd419a77741bb9db048b0070d1151aa15544248db28e67f -->
+<!-- manifest-digest: sha256:468d917977ef1db81f80d9c4f088cf2aca0421fc80c4af7725f4cc6b298adae8 -->
 # Generated command reference
 
 This reference is generated from the validated, versioned command manifest. Availability is evaluated at runtime; a listed command may still require authentication, a hosted capability, or local tooling.
@@ -126,7 +126,7 @@ Command flags:
 - `--body <value>`
 - `--base <value>`
 
-#### `aether ship [--title t] [--base b]`
+#### `aether ship [--title text] [--body text] [--base branch] [--approve publish] [--yes] [--json]`
 
 publish the head branch and open a pull request
 
@@ -719,7 +719,7 @@ review changes, pick files or hunks, commit
 
 Permission: `unknown` · Availability: `runtime-dependent` · Telemetry: `slash.review`
 
-#### `/ship [--title t] [--base b]`
+#### `/ship [--title text] [--body text] [--base branch] [--approve publish] [--yes] [--json] [--help]`
 
 publish the head branch and open a pull request
 

src/commands/command_manifest_data.ts

@@ -2741,9 +2741,9 @@ export const COMMAND_MANIFEST_SOURCE: readonly CommandManifestEntry[] = [
     "aliases": [],
     "compatibilityAliases": [],
     "deprecatedAliases": [],
-    "args": "[--title t] [--base b]",
+    "args": "[--title text] [--body text] [--base branch] [--approve publish] [--yes] [--json]",
     "summary": "publish the head branch and open a pull request",
-    "detailedHelp": "aether ship [--title t] [--base b]\npublish the head branch and open a pull request",
+    "detailedHelp": "aether ship [--title text] [--body text] [--base branch] [--approve publish] [--yes] [--json]\npublish the head branch and open a pull request\n--title sets the PR title; --body sets the PR body; --base selects the target branch. --approve publish authorizes publication; --yes alone does not. --json previews the planned argv without publishing.",
     "section": "Start",
     "hidden": false,
     "permissionClass": "destructive",
@@ -2823,7 +2823,7 @@ export const COMMAND_MANIFEST_SOURCE: readonly CommandManifestEntry[] = [
       "module": "src/commands/command_manifest_data.ts",
       "symbol": "COMMAND_MANIFEST_SOURCE",
       "target": "ship",
-      "usage": "aether ship [--title t] [--base b]",
+      "usage": "aether ship [--title text] [--body text] [--base branch] [--approve publish] [--yes] [--json]",
       "visible": true,
       "disposition": "generated"
     },
@@ -5634,9 +5634,9 @@ export const COMMAND_MANIFEST_SOURCE: readonly CommandManifestEntry[] = [
     "aliases": [],
     "compatibilityAliases": [],
     "deprecatedAliases": [],
-    "args": "[--title t] [--base b]",
+    "args": "[--title text] [--body text] [--base branch] [--approve publish] [--yes] [--json] [--help]",
     "summary": "publish the head branch and open a pull request",
-    "detailedHelp": "/ship [--title t] [--base b]\npublish the head branch and open a pull request",
+    "detailedHelp": "/ship [--title text] [--body text] [--base branch] [--approve publish] [--yes] [--json] [--help]\npublish the head branch and open a pull request\nQuote values containing spaces or newlines. --title sets the PR title; --body sets the PR body; --base selects the target branch. --approve publish authorizes publication; --yes alone does not. --json previews the planned argv without publishing. --body-file is not supported.",
     "section": "UVT Tools",
     "hidden": false,
     "permissionClass": "unknown",
@@ -5658,7 +5658,7 @@ export const COMMAND_MANIFEST_SOURCE: readonly CommandManifestEntry[] = [
       "module": "src/commands/command_manifest_data.ts",
       "symbol": "COMMAND_MANIFEST_SOURCE",
       "target": "ship",
-      "usage": "/ship [--title t] [--base b]",
+      "usage": "/ship [--title text] [--body text] [--base branch] [--approve publish] [--yes] [--json] [--help]",
       "visible": true,
       "disposition": "generated"
     },

src/commands/ship.ts

@@ -324,20 +324,111 @@ export async function cmdShip(ctx: AppContext, _rest: string[], flags: ShipFlags
   return runShip(ctx, defaultShipDeps(ctx.flags.cwd, process.stdout), flags);
 }
 
-/** `/ship` inside the REPL. */
-export async function shipSlash(ctx: AppContext, out: Writable, arg: string): Promise<void> {
-  const parts = arg.trim().split(/\s+/).filter(Boolean);
-  const valueOf = (name: string): string | undefined => {
-    const at = parts.indexOf(name);
-    return at >= 0 ? parts[at + 1] : undefined;
-  };
-  await runShip(ctx, defaultShipDeps(ctx.flags.cwd, out), {
-    yes: false,
-    json: false,
-    ...(valueOf("--title") !== undefined ? { title: valueOf("--title") } : {}),
-    ...(valueOf("--base") !== undefined ? { base: valueOf("--base") } : {}),
-    ...(valueOf("--approve") !== undefined ? { approve: valueOf("--approve") } : {}),
-  });
+export const SHIP_SLASH_USAGE =
+  'usage: /ship [--title <text>] [--body <text>] [--base <branch>] [--approve publish] [--yes] [--json] [--help]';
+
+interface SlashWord { value: string; quoted: boolean }
+
+/** Split REPL input into argv without invoking a shell or expanding any text. */
+function shipSlashWords(input: string): SlashWord[] {
+  const words: SlashWord[] = [];
+  let value = "";
+  let active = false;
+  let quoted = false;
+  let quote: "'" | '"' | null = null;
+  for (let index = 0; index < input.length; index++) {
+    const char = input[index]!;
+    if (char === "\\" && quote !== "'") {
+      const next = input[index + 1];
+      if (next === undefined) throw new Error("trailing escape");
+      // Escapes quote characters, backslashes and whitespace. Other escapes
+      // remain literal, including Windows paths and shell-looking text.
+      if (next === "\\" || next === '"' || next === "'" || next === "$" || next === "`" || /\s/u.test(next) || (quote === null && next === "-")) {
+        value += next;
+        index++;
+        quoted = true;
+      } else {
+        value += char;
+      }
+      active = true;
+      continue;
+    }
+    if (quote !== null) {
+      if (char === quote) quote = null;
+      else value += char;
+      active = true;
+      continue;
+    }
+    if (char === "'" || char === '"') {
+      quote = char;
+      quoted = true;
+      active = true;
+    } else if (/\s/u.test(char)) {
+      if (active) words.push({ value, quoted });
+      value = "";
+      active = false;
+      quoted = false;
+    } else {
+      value += char;
+      active = true;
+    }
+  }
+  if (quote !== null) throw new Error("unterminated quote");
+  if (active) words.push({ value, quoted });
+  return words;
+}
+
+/** Parse exactly the options `/ship` can pass to the shared ship rail. */
+export function parseShipSlashArgs(arg: string): ShipFlags | "help" {
+  const words = shipSlashWords(arg);
+  const flags: ShipFlags = { yes: false, json: false };
+  const seen = new Set<string>();
+  const valued = new Set(["--title", "--body", "--base", "--approve"]);
+  const boolean = new Set(["--yes", "--json", "--help"]);
+  for (let index = 0; index < words.length; index++) {
+    const word = words[index]!;
+    const equal = word.value.indexOf("=");
+    const name = equal < 0 ? word.value : word.value.slice(0, equal);
+    if (!name.startsWith("-") || (word.quoted && equal < 0)) throw new Error(`unexpected argument: ${word.value}`);
+    if (!valued.has(name) && !boolean.has(name)) throw new Error(`unsupported option: ${name}`);
+    if (seen.has(name)) throw new Error(`duplicate option: ${name}`);
+    seen.add(name);
+    if (boolean.has(name)) {
+      if (equal >= 0) throw new Error(`${name} does not take a value`);
+      if (name === "--yes") flags.yes = true;
+      if (name === "--json") flags.json = true;
+      continue;
+    }
+    const next = equal < 0 ? words[++index] : { value: word.value.slice(equal + 1), quoted: word.quoted };
+    if (!next || !next.value.trim() || (equal < 0 && next.value.startsWith("-") && !next.quoted)) {
+      throw new Error(`${name} needs a value`);
+    }
+    if (name === "--title") flags.title = next.value;
+    if (name === "--body") flags.body = next.value;
+    if (name === "--base") flags.base = next.value;
+    if (name === "--approve") flags.approve = next.value;
+  }
+  if (seen.has("--help")) {
+    if (seen.size !== 1) throw new Error("--help must be used alone");
+    return "help";
+  }
+  return flags;
+}
+
+/** `/ship` inside the REPL. The optional deps also keep wrapper tests offline. */
+export async function shipSlash(ctx: AppContext, out: Writable, arg: string, deps?: ShipDeps): Promise<void> {
+  let flags: ShipFlags | "help";
+  try {
+    flags = parseShipSlashArgs(arg);
+  } catch (error) {
+    out.write(`✗ ${error instanceof Error ? error.message : String(error)}\n${SHIP_SLASH_USAGE}\n`);
+    return;
+  }
+  if (flags === "help") {
+    out.write(`${SHIP_SLASH_USAGE}\n`);
+    return;
+  }
+  await runShip(ctx, deps ?? defaultShipDeps(ctx.flags.cwd, out), flags);
 }
 
 /**

src/commands/slash.ts

@@ -103,15 +103,23 @@ function byKind(cat: CatalogResponse, kind: Kind): CatalogItem[] {
   return cat.models.filter((m) => m.kind === kind);
 }
 
+export function splitSlashCommand(line: string): { cmd: string; arg: string } {
+  const input = line.slice(1).trim();
+  const separator = input.search(/\s/u);
+  const cmd = (separator < 0 ? input : input.slice(0, separator)).toLowerCase();
+  // Keep the argument text intact. /ship parses quoting and may contain a
+  // literal newline in a quoted PR body; splitting here would corrupt it.
+  const arg = separator < 0 ? "" : input.slice(separator).trim();
+  return { cmd, arg };
+}
+
 export async function handleSlash(
   ctx: AppContext,
   line: string,
   out: Writable,
   signal?: AbortSignal,
 ): Promise<SlashResult> {
-  const parts = line.slice(1).trim().split(/\s+/);
-  const cmd = (parts[0] ?? "").toLowerCase();
-  const arg = parts.slice(1).join(" ");
+  const { cmd, arg } = splitSlashCommand(line);
 
   switch (cmd) {
     case "terminal":
@@ -231,7 +239,7 @@ export async function handleSlash(
       break;
     }
     case "agent-create": {
-      await cmdManagedAgents(ctx, ["create", ...parts.slice(1)], { out, err: out, signal, hooks: createAtsHooks({ output: text => { out.write(text); } }) });
+      await cmdManagedAgents(ctx, ["create", ...(arg ? arg.split(/\s+/u) : [])], { out, err: out, signal, hooks: createAtsHooks({ output: text => { out.write(text); } }) });
       break;
     }
     case "doctor": {

test/review_ship_e2e.test.ts

@@ -15,6 +15,7 @@
 
 import { test } from "node:test";
 import assert from "node:assert/strict";
+import { parseArgs } from "node:util";
 import { spawnSync } from "node:child_process";
 import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
 import { join } from "node:path";
@@ -25,7 +26,9 @@ import type { RunResult, Runner } from "../src/core/worktree.js";
 import { defaultRunner } from "../src/core/worktree.js";
 import { readRepoState } from "../src/core/review_state.js";
 import { runReview, type ReviewDeps } from "../src/commands/review.js";
-import { runShip, type ShipDeps } from "../src/commands/ship.js";
+import { parseShipSlashArgs, runShip, shipSlash, type ShipDeps } from "../src/commands/ship.js";
+import { CLI_PARSE_OPTIONS } from "../src/commands/cli_registry.js";
+import { splitSlashCommand } from "../src/commands/slash.js";
 import { spawnAsyncRun } from "../src/commands/review_counts.js";
 import { TEMP_ROOT } from "./tmp_workspace.js";
 
@@ -153,6 +156,98 @@ const remoteBranches = (remote: string): string[] =>
     .filter(Boolean)
     .sort();
 
+test("/ship rejects malformed or unsupported input before any git, push, PR, or confirmation call", async () => {
+  const cases: Array<[string, RegExp]> = [
+    ["--title", /--title needs a value/],
+    ["--title --base main", /--title needs a value/],
+    ['--title "unterminated', /unterminated quote/],
+    ["--title hello\\", /trailing escape/],
+    ["--body-file proposal.md", /unsupported option: --body-file/],
+    ["--title first --title second", /duplicate option: --title/],
+    ["--json --json", /duplicate option: --json/],
+    ["--title good stray", /unexpected argument: stray/],
+    ["--title=", /--title needs a value/],
+    ["--yes=true", /--yes does not take a value/],
+  ];
+  for (const [arg, message] of cases) {
+    const out = sink();
+    let calls = 0;
+    let prompts = 0;
+    const deps: ShipDeps = {
+      run: () => { calls++; throw new Error("git or gh must not run"); },
+      cwd: "unused",
+      out: out.out,
+      io: { tty: true, note: () => {}, question: async () => { prompts++; return "y"; } },
+    };
+    await shipSlash(ctx, out.out, arg, deps);
+    assert.match(out.text(), message, arg);
+    assert.match(out.text(), /usage: \/ship/, arg);
+    assert.equal(calls, 0, arg);
+    assert.equal(prompts, 0, arg);
+  }
+});
+
+test("/ship handles escaped spaces, equals values, and help without running Git", async () => {
+  assert.deepEqual(parseShipSlashArgs('--title=Fix\\ auth\\ and\\ model\\ picker --base="main"'), {
+    yes: false, json: false, title: "Fix auth and model picker", base: "main",
+  });
+  assert.deepEqual(parseShipSlashArgs('--title "C:\\work\\file $(id)"'), {
+    yes: false, json: false, title: "C:\\work\\file $(id)",
+  });
+  const out = sink();
+  await shipSlash(ctx, out.out, "--help", {
+    run: () => { throw new Error("Git must not run for help"); },
+    cwd: "unused", out: out.out, io: io([]),
+  });
+  assert.match(out.text(), /--title.*--body.*--base.*--approve.*--yes.*--json/);
+  assert.equal(out.text().includes("--body-file"), false);
+});
+
+test("/ship and CLI preserve the same quoted Unicode, newlines, and literal shell-looking argv", async () => {
+  const fixture = repoFixture();
+  try {
+    writeFileSync(join(fixture.repo, "kept.ts"), "changed\n");
+    git(fixture.repo, "commit", "-am", "fix: source");
+    const title = 'Fix auth and model picker — "safe" $(whoami) `whoami`';
+    const body = "line one\nline two ☃ $HOME $(id) `id`";
+    const slashArg = '--title "Fix auth and model picker — \\"safe\\" $(whoami) `whoami`" ' +
+      "--body 'line one\nline two ☃ $HOME $(id) `id`' --base main";
+    assert.deepEqual(splitSlashCommand(`/ship ${slashArg}`), { cmd: "ship", arg: slashArg });
+    const cli = parseArgs({
+      args: ["ship", "--title", title, "--body", body, "--base", "main", "--approve", "publish"],
+      allowPositionals: true,
+      strict: true,
+      options: CLI_PARSE_OPTIONS,
+    });
+    assert.deepEqual(cli.positionals, ["ship"]);
+    assert.equal(cli.values["title"], title);
+    assert.equal(cli.values["body"], body);
+    const parsed = parseShipSlashArgs(slashArg);
+    assert.notEqual(parsed, "help");
+    assert.equal((parsed as { title?: string }).title, cli.values["title"]);
+    assert.equal((parsed as { body?: string }).body, cli.values["body"]);
+
+    const { run, calls } = railRunner(okGh);
+    const preview = sink();
+    await shipSlash(ctx, preview.out, `${slashArg} --json`, shipDeps(fixture, run, preview.out));
+    const planned = JSON.parse(preview.text()) as { commands: Array<{ cmd: string; args: string[] }> };
+    const plannedGh = planned.commands.find((command) => command.cmd === "gh")!.args;
+    assert.equal(plannedGh[plannedGh.indexOf("--title") + 1], title);
+    assert.equal(plannedGh[plannedGh.indexOf("--body") + 1], body);
+    assert.equal(calls.some((call) => call[0] === "gh" || call.includes("push")), false);
+
+    const published = sink();
+    await shipSlash(ctx, published.out, `${slashArg} --approve publish`, shipDeps(fixture, run, published.out));
+    const created = calls.find((call) => call[0] === "gh" && call[1] === "pr" && call[2] === "create");
+    assert.ok(created, published.text());
+    assert.deepEqual(created.slice(1), plannedGh);
+    assert.match(published.text(), /Fix auth and model picker/);
+    assert.match(published.text(), /PR opened:/);
+  } finally {
+    fixture.cleanup();
+  }
+});
+
 // ── the whole rail ──────────────────────────────────────────────────────────
 
 test("the whole rail: edit → select → stage → commit → publish → PR", async () => {