Merge branch 'codex/bugs-gh-266-apply-args'

Author: MajorTal

cli-e2e.test.mjs

@@ -2215,6 +2215,98 @@ describe("CLI e2e happy path", () => {
     return { threw, stderr: capturedStderr(), stdout: capturedStdout(), deployCalled, planBodies };
   }
 
+  function nonEmptyDeploySpec() {
+    return JSON.stringify({ site: { replace: { "index.html": { data: "ok" } } } });
+  }
+
+  async function writeDeployManifest(name, spec = JSON.parse(nonEmptyDeploySpec())) {
+    const { writeFileSync: wf } = await import("node:fs");
+    const manifestPath = join(tempDir, name);
+    wf(manifestPath, JSON.stringify(spec));
+    return manifestPath;
+  }
+
+  async function assertDeployApplyBadUsage(args, messagePattern) {
+    const { threw, stderr, deployCalled } = await deployApplyAndCapture(args);
+    assert.ok(threw && /process\.exit\(1\)/.test(threw.message),
+      `should exit non-zero, got: ${threw && threw.message}`);
+    assert.equal(deployCalled, false, "must not POST to /deploy/v2/plans on bad deploy apply usage");
+    const parsed = parseStderrEnvelope(stderr);
+    assert.equal(parsed.code, "BAD_USAGE");
+    assert.match(parsed.message, messagePattern);
+  }
+
+  // ── GH-266/GH-268: deploy apply argument/source validation ──
+  it("deploy apply rejects unknown flags (GH-266)", async () => {
+    await assertDeployApplyBadUsage(
+      ["--spec", nonEmptyDeploySpec(), "--project", "prj_test123", "--allow-warning"],
+      /Unknown flag.*--allow-warning/,
+    );
+  });
+
+  it("deploy apply rejects extra positional arguments (GH-266)", async () => {
+    const manifestPath = await writeDeployManifest("gh266-positional.json");
+    await assertDeployApplyBadUsage(
+      ["--manifest", manifestPath, "unexpected.json", "--project", "prj_test123"],
+      /Unexpected argument.*unexpected\.json/,
+    );
+  });
+
+  it("deploy apply rejects --manifest combined with --spec (GH-268)", async () => {
+    const manifestPath = await writeDeployManifest("gh268-manifest-and-spec.json");
+    await assertDeployApplyBadUsage(
+      ["--manifest", manifestPath, "--spec", nonEmptyDeploySpec(), "--project", "prj_test123"],
+      /Only one deploy manifest source/,
+    );
+  });
+
+  it("deploy apply rejects repeated --manifest flags (GH-266)", async () => {
+    const firstManifestPath = await writeDeployManifest("gh266-repeated-manifest-1.json");
+    const secondManifestPath = await writeDeployManifest("gh266-repeated-manifest-2.json");
+    await assertDeployApplyBadUsage(
+      ["--manifest", firstManifestPath, "--manifest", secondManifestPath, "--project", "prj_test123"],
+      /--manifest.*only be provided once/,
+    );
+  });
+
+  it("deploy apply rejects repeated --spec flags (GH-266)", async () => {
+    await assertDeployApplyBadUsage(
+      ["--spec", nonEmptyDeploySpec(), "--spec", nonEmptyDeploySpec(), "--project", "prj_test123"],
+      /--spec.*only be provided once/,
+    );
+  });
+
+  it("deploy apply rejects redirected stdin combined with an explicit source (GH-268)", async () => {
+    const { closeSync, openSync, writeFileSync: wf } = await import("node:fs");
+    const { spawnSync } = await import("node:child_process");
+    const stdinPath = join(tempDir, "gh268-stdin.json");
+    wf(stdinPath, nonEmptyDeploySpec());
+    const stdinFd = openSync(stdinPath, "r");
+    let result;
+    try {
+      result = spawnSync(process.execPath, [
+        "cli/cli.mjs",
+        "deploy",
+        "apply",
+        "--spec",
+        "{}",
+        "--project",
+        "prj_test123",
+      ], {
+        cwd: process.cwd(),
+        env: { ...process.env, RUN402_CONFIG_DIR: tempDir, RUN402_API_BASE: API },
+        stdio: [stdinFd, "pipe", "pipe"],
+        encoding: "utf-8",
+      });
+    } finally {
+      closeSync(stdinFd);
+    }
+    assert.notEqual(result.status, 0, `should exit non-zero, stdout: ${result.stdout}, stderr: ${result.stderr}`);
+    const parsed = parseStderrEnvelope(result.stderr);
+    assert.equal(parsed.code, "BAD_USAGE");
+    assert.match(parsed.message, /Only one deploy manifest source/);
+  });
+
   it("deploy apply rejects empty manifest file (GH-232)", async () => {
     const { writeFileSync: wf } = await import("node:fs");
     const manifestPath = join(tempDir, "gh232-empty-manifest.json");

cli/lib/deploy-v2.mjs

@@ -21,7 +21,7 @@
  * UTF-8 is the default; binary files pass `"encoding": "base64"`.
  */
 
-import { readFileSync } from "node:fs";
+import { fstatSync, readFileSync } from "node:fs";
 import { resolve, dirname, isAbsolute } from "node:path";
 import {
   buildDeployResolveSummary,
@@ -266,29 +266,113 @@ async function readStdin() {
   return Buffer.concat(chunks).toString("utf-8");
 }
 
+function hasStdinSource() {
+  try {
+    const stats = fstatSync(0);
+    return stats.isFIFO() || stats.isFile();
+  } catch {
+    return false;
+  }
+}
+
 function makeStderrEventWriter(quiet) {
   if (quiet) return undefined;
   return (event) => {
     console.error(JSON.stringify(event));
   };
 }
 
-async function applyCmd(args) {
+function parseApplyArgs(args) {
   const opts = { manifest: null, spec: null, project: null, quiet: false, allowWarnings: false };
+  const allowedFlags = ["--manifest", "--spec", "--project", "--quiet", "--allow-warnings", "--help", "-h"];
+
   for (let i = 0; i < args.length; i++) {
-    if (args[i] === "--help" || args[i] === "-h") { console.log(APPLY_HELP); process.exit(0); }
-    if (args[i] === "--manifest" && args[i + 1]) { opts.manifest = args[++i]; continue; }
-    if (args[i] === "--spec" && args[i + 1]) { opts.spec = args[++i]; continue; }
-    if (args[i] === "--project" && args[i + 1]) { opts.project = args[++i]; continue; }
-    if (args[i] === "--quiet") { opts.quiet = true; continue; }
-    if (args[i] === "--allow-warnings") { opts.allowWarnings = true; continue; }
+    const arg = args[i];
+    if (arg === "--help" || arg === "-h") {
+      console.log(APPLY_HELP);
+      process.exit(0);
+    }
+    if (arg === "--manifest" || arg === "--spec" || arg === "--project") {
+      const value = args[i + 1];
+      if (value === undefined || (typeof value === "string" && value.startsWith("--"))) {
+        fail({
+          code: "BAD_USAGE",
+          message: `${arg} requires a value`,
+          details: { flag: arg },
+        });
+      }
+      if (arg === "--manifest") {
+        if (opts.manifest !== null) {
+          fail({
+            code: "BAD_USAGE",
+            message: "--manifest may only be provided once",
+            details: { flag: "--manifest" },
+          });
+        }
+        opts.manifest = value;
+      } else if (arg === "--spec") {
+        if (opts.spec !== null) {
+          fail({
+            code: "BAD_USAGE",
+            message: "--spec may only be provided once",
+            details: { flag: "--spec" },
+          });
+        }
+        opts.spec = value;
+      } else {
+        opts.project = value;
+      }
+      i += 1;
+      continue;
+    }
+    if (arg === "--quiet") { opts.quiet = true; continue; }
+    if (arg === "--allow-warnings") { opts.allowWarnings = true; continue; }
+    if (typeof arg === "string" && arg.startsWith("-")) {
+      fail({
+        code: "BAD_USAGE",
+        message: `Unknown flag for deploy apply: ${arg}`,
+        details: { flag: arg, allowed_flags: allowedFlags },
+      });
+    }
+    fail({
+      code: "BAD_USAGE",
+      message: `Unexpected argument for deploy apply: ${arg}`,
+      details: { argument: arg },
+    });
   }
 
+  return opts;
+}
+
+function applySourceField(opts) {
+  if (opts.manifest !== null) return "manifest";
+  if (opts.spec !== null) return "spec";
+  return "stdin";
+}
+
+function validateApplySources(opts) {
+  const sources = [];
+  if (opts.manifest !== null) sources.push("--manifest");
+  if (opts.spec !== null) sources.push("--spec");
+  if (hasStdinSource()) sources.push("stdin");
+  if (sources.length > 1) {
+    fail({
+      code: "BAD_USAGE",
+      message: "Only one deploy manifest source may be provided: --spec, --manifest, or stdin.",
+      details: { sources },
+    });
+  }
+}
+
+async function applyCmd(args) {
+  const opts = parseApplyArgs(args);
+  validateApplySources(opts);
+
   let raw;
   let manifestPath = null;
-  if (opts.spec) {
+  if (opts.spec !== null) {
     raw = opts.spec;
-  } else if (opts.manifest) {
+  } else if (opts.manifest !== null) {
     try {
       manifestPath = isAbsolute(opts.manifest) ? opts.manifest : resolve(process.cwd(), opts.manifest);
       raw = readFileSync(manifestPath, "utf-8");
@@ -310,11 +394,11 @@ async function applyCmd(args) {
     fail({
       code: "BAD_USAGE",
       message: `Manifest is not valid JSON: ${err.message}`,
-      details: { source: opts.manifest ? "manifest" : opts.spec ? "spec" : "stdin", parse_error: err.message },
+      details: { source: applySourceField(opts), parse_error: err.message },
     });
   }
   rejectLegacySecretManifest(spec, {
-    source: opts.manifest ? "manifest" : opts.spec ? "spec" : "stdin",
+    source: applySourceField(opts),
     ...(manifestPath ? { path: manifestPath } : {}),
   });
 
@@ -355,7 +439,7 @@ async function applyCmd(args) {
       message: `Manifest contains no deployable sections. Expected at least one of: ${meaningful.join(", ")}`,
       hint: "Did you mean to write a 'site.replace' or 'database.migrations' block? See https://run402.com/schemas/manifest.v1.json",
       details: {
-        field: opts.manifest ? "manifest" : opts.spec ? "spec" : "stdin",
+        field: applySourceField(opts),
         ...(manifestPath ? { path: manifestPath } : {}),
         meaningful_keys: meaningful,
       },