fix(cli): tolerate the -- separator pnpm injects when forwarding script args (#3114)

.changeset/cli-strip-arg-separator.md

@@ -0,0 +1,30 @@
+---
+"@objectstack/cli": patch
+---
+
+fix(cli): tolerate the `--` separator pnpm injects when forwarding script args (#3114)
+
+The AGENTS.md-documented backend-debug flow `pnpm dev -- --fresh -p <port>` failed at
+the repo root with an opaque `Unexpected arguments: -p, 44637` (exit 2 + a help dump).
+
+pnpm appends forwarded args to a script **verbatim, including the `--`**, and each
+nested `pnpm --filter` hop preserves it, so the showcase's `objectstack dev
+--seed-admin` ran as `objectstack dev --seed-admin -- --fresh -p 44637`. oclif reads
+`--` as POSIX end-of-flags, so everything after it became positional: `--fresh` was
+silently swallowed as the `package` arg and `-p 44637` overflowed the arg list. Every
+flag the user asked for was dropped — the failure was opaque precisely because the
+`--` looks inert.
+
+A `preparse` hook now drops `--` separators before oclif parses argv, so the
+npm-style `-- <flags>` form and the bare form behave identically, for every command
+and both bins (`run.js`, `run-dev.js`). No `os` command takes passthrough args (none
+sets `strict = false`, none reads raw argv), so a `--` carries no meaning here and is
+always a package-manager artifact.
+
+Note this is not fixable via oclif's `'--': false` parser option: that keeps
+flag-parsing on past the separator but re-appends the `--` into argv, so strict
+commands fail with `Unexpected argument: --` instead.
+
+Tradeoff: a `-`-prefixed token can no longer be forced to parse as a positional
+value. Every `os` positional is a config path, a metadata / datasource / package
+name, or an id — none start with `-`.

packages/cli/package.json

@@ -32,6 +32,9 @@
       "target": "./dist/commands",
       "glob": "**/*.js"
     },
+    "hooks": {
+      "preparse": "./dist/hooks/preparse/strip-arg-separator.js"
+    },
     "plugins": [
       "@oclif/plugin-help",
       "@oclif/plugin-plugins"

packages/cli/src/hooks/preparse/strip-arg-separator.test.ts

@@ -0,0 +1,57 @@
+// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
+
+import { describe, it, expect } from 'vitest';
+import { Parser } from '@oclif/core';
+import Dev from '../../commands/dev.js';
+import hook from './strip-arg-separator.js';
+
+/** Run the preparse hook the way oclif's Command.parse does. */
+const preparse = async (argv: string[]): Promise<string[]> =>
+  (await (hook as unknown as (opts: { argv: string[] }) => Promise<string[]>)({ argv })) ?? argv;
+
+const parseDev = (argv: string[]) =>
+  Parser.parse(argv, { flags: Dev.flags, args: Dev.args, strict: true });
+
+describe('preparse: strip `--` argument separators', () => {
+  it('drops a pnpm-injected separator', async () => {
+    expect(await preparse(['dev', '--seed-admin', '--', '--fresh', '-p', '44637'])).toEqual([
+      'dev',
+      '--seed-admin',
+      '--fresh',
+      '-p',
+      '44637',
+    ]);
+  });
+
+  it('leaves argv without a separator untouched', async () => {
+    const argv = ['dev', '--seed-admin', '--fresh', '-p', '44637'];
+    expect(await preparse(argv)).toEqual(argv);
+  });
+
+  it('does not touch flags that merely contain dashes', async () => {
+    const argv = ['dev', '--log-level', 'debug', '--admin-email', 'a@b.co'];
+    expect(await preparse(argv)).toEqual(argv);
+  });
+
+  // The actual bug (#3114): `pnpm dev -- --fresh -p <port>` reaches the CLI as
+  // `dev --seed-admin -- --fresh -p <port>`. Assert against the real Dev flags.
+  it('makes `os dev --seed-admin -- --fresh -p N` parse identically to the bare form', async () => {
+    const withSeparator = await parseDev(
+      await preparse(['--seed-admin', '--', '--fresh', '-p', '44637']),
+    );
+    const bare = await parseDev(['--seed-admin', '--fresh', '-p', '44637']);
+
+    expect(withSeparator.flags).toEqual(bare.flags);
+    expect(withSeparator.flags.fresh).toBe(true);
+    expect(withSeparator.flags['seed-admin']).toBe(true);
+    expect(withSeparator.flags.port).toBe('44637');
+    // `--fresh` must not be swallowed as the `package` positional.
+    expect(withSeparator.args.package).toBe('all');
+  });
+
+  it('regression: without the hook the same argv loses --fresh and throws', async () => {
+    await expect(parseDev(['--seed-admin', '--', '--fresh', '-p', '44637'])).rejects.toThrow(
+      /Unexpected argument/,
+    );
+  });
+});

packages/cli/src/hooks/preparse/strip-arg-separator.ts

@@ -0,0 +1,35 @@
+// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
+
+import type { Hook } from '@oclif/core';
+
+/**
+ * Drop `--` argument separators before oclif parses argv.
+ *
+ * pnpm appends forwarded args to a script *verbatim, including the `--`*, and
+ * every nested `pnpm --filter` hop preserves it. So the documented
+ * `pnpm dev -- --fresh -p 44637` reaches us as:
+ *
+ *   objectstack dev --seed-admin -- --fresh -p 44637
+ *
+ * oclif reads `--` as POSIX end-of-flags, so everything after it becomes
+ * positional: `--fresh` is silently swallowed as the `package` arg and the
+ * command dies with the opaque `Unexpected arguments: -p, 44637` (exit 2 +
+ * a help dump). The flags the user asked for are simply dropped.
+ *
+ * No `os` command takes passthrough args — none sets `strict = false`, and none
+ * reads raw argv — so a `--` carries no meaning here and is always a
+ * package-manager artifact. Dropping it makes the npm-style `-- <flags>` form
+ * and the bare form behave identically.
+ *
+ * The tradeoff: you can no longer force a `-`-prefixed token to be read as a
+ * positional value. Every `os` positional is a config path, a metadata /
+ * datasource / package name, or an id, none of which start with `-`. Revisit
+ * this if a command ever needs true passthrough (e.g. wrapping another CLI).
+ *
+ * Note this cannot be fixed with oclif's `'--': false` parser option: that
+ * option keeps flag-parsing on past the separator but then re-appends the `--`
+ * into argv, so strict commands fail with `Unexpected argument: --` instead.
+ */
+const hook: Hook<'preparse'> = async ({ argv }) => argv.filter((arg) => arg !== '--');
+
+export default hook;