diff --git a/.changeset/cli-strip-arg-separator.md b/.changeset/cli-strip-arg-separator.md new file mode 100644 index 0000000000..b7e8589055 --- /dev/null +++ b/.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 ` 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 `-- ` 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 `-`. diff --git a/packages/cli/package.json b/packages/cli/package.json index 5f8f2ae036..f4bd244f72 100644 --- a/packages/cli/package.json +++ b/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" diff --git a/packages/cli/src/hooks/preparse/strip-arg-separator.test.ts b/packages/cli/src/hooks/preparse/strip-arg-separator.test.ts new file mode 100644 index 0000000000..b57210aaf1 --- /dev/null +++ b/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 => + (await (hook as unknown as (opts: { argv: string[] }) => Promise)({ 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 ` reaches the CLI as + // `dev --seed-admin -- --fresh -p `. 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/, + ); + }); +}); diff --git a/packages/cli/src/hooks/preparse/strip-arg-separator.ts b/packages/cli/src/hooks/preparse/strip-arg-separator.ts new file mode 100644 index 0000000000..63b6618c29 --- /dev/null +++ b/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 `-- ` 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;