Skip to content

Commit 28adf34

Browse files
os-zhuangclaude
andcommitted
fix(cli): tolerate the -- separator pnpm injects when forwarding script args (#3114)
The AGENTS.md-documented `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. 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. No `os` command takes passthrough args (none sets `strict = false`, none reads raw argv), so a `--` is always a package-manager artifact here. Not fixable via oclif's `'--': false` option: that keeps flag-parsing on past the separator but re-appends the `--` into argv, so strict commands fail with `Unexpected argument: --` instead. Verified end-to-end: `pnpm dev -- --fresh -p 44639` now boots the showcase on :44639 with an ephemeral tempdir and a seeded admin. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 5f05de2 commit 28adf34

4 files changed

Lines changed: 125 additions & 0 deletions

File tree

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

packages/cli/package.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@
3232
"target": "./dist/commands",
3333
"glob": "**/*.js"
3434
},
35+
"hooks": {
36+
"preparse": "./dist/hooks/preparse/strip-arg-separator.js"
37+
},
3538
"plugins": [
3639
"@oclif/plugin-help",
3740
"@oclif/plugin-plugins"
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect } from 'vitest';
4+
import { Parser } from '@oclif/core';
5+
import Dev from '../../commands/dev.js';
6+
import hook from './strip-arg-separator.js';
7+
8+
/** Run the preparse hook the way oclif's Command.parse does. */
9+
const preparse = async (argv: string[]): Promise<string[]> =>
10+
(await (hook as unknown as (opts: { argv: string[] }) => Promise<string[]>)({ argv })) ?? argv;
11+
12+
const parseDev = (argv: string[]) =>
13+
Parser.parse(argv, { flags: Dev.flags, args: Dev.args, strict: true });
14+
15+
describe('preparse: strip `--` argument separators', () => {
16+
it('drops a pnpm-injected separator', async () => {
17+
expect(await preparse(['dev', '--seed-admin', '--', '--fresh', '-p', '44637'])).toEqual([
18+
'dev',
19+
'--seed-admin',
20+
'--fresh',
21+
'-p',
22+
'44637',
23+
]);
24+
});
25+
26+
it('leaves argv without a separator untouched', async () => {
27+
const argv = ['dev', '--seed-admin', '--fresh', '-p', '44637'];
28+
expect(await preparse(argv)).toEqual(argv);
29+
});
30+
31+
it('does not touch flags that merely contain dashes', async () => {
32+
const argv = ['dev', '--log-level', 'debug', '--admin-email', 'a@b.co'];
33+
expect(await preparse(argv)).toEqual(argv);
34+
});
35+
36+
// The actual bug (#3114): `pnpm dev -- --fresh -p <port>` reaches the CLI as
37+
// `dev --seed-admin -- --fresh -p <port>`. Assert against the real Dev flags.
38+
it('makes `os dev --seed-admin -- --fresh -p N` parse identically to the bare form', async () => {
39+
const withSeparator = await parseDev(
40+
await preparse(['--seed-admin', '--', '--fresh', '-p', '44637']),
41+
);
42+
const bare = await parseDev(['--seed-admin', '--fresh', '-p', '44637']);
43+
44+
expect(withSeparator.flags).toEqual(bare.flags);
45+
expect(withSeparator.flags.fresh).toBe(true);
46+
expect(withSeparator.flags['seed-admin']).toBe(true);
47+
expect(withSeparator.flags.port).toBe('44637');
48+
// `--fresh` must not be swallowed as the `package` positional.
49+
expect(withSeparator.args.package).toBe('all');
50+
});
51+
52+
it('regression: without the hook the same argv loses --fresh and throws', async () => {
53+
await expect(parseDev(['--seed-admin', '--', '--fresh', '-p', '44637'])).rejects.toThrow(
54+
/Unexpected argument/,
55+
);
56+
});
57+
});
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import type { Hook } from '@oclif/core';
4+
5+
/**
6+
* Drop `--` argument separators before oclif parses argv.
7+
*
8+
* pnpm appends forwarded args to a script *verbatim, including the `--`*, and
9+
* every nested `pnpm --filter` hop preserves it. So the documented
10+
* `pnpm dev -- --fresh -p 44637` reaches us as:
11+
*
12+
* objectstack dev --seed-admin -- --fresh -p 44637
13+
*
14+
* oclif reads `--` as POSIX end-of-flags, so everything after it becomes
15+
* positional: `--fresh` is silently swallowed as the `package` arg and the
16+
* command dies with the opaque `Unexpected arguments: -p, 44637` (exit 2 +
17+
* a help dump). The flags the user asked for are simply dropped.
18+
*
19+
* No `os` command takes passthrough args — none sets `strict = false`, and none
20+
* reads raw argv — so a `--` carries no meaning here and is always a
21+
* package-manager artifact. Dropping it makes the npm-style `-- <flags>` form
22+
* and the bare form behave identically.
23+
*
24+
* The tradeoff: you can no longer force a `-`-prefixed token to be read as a
25+
* positional value. Every `os` positional is a config path, a metadata /
26+
* datasource / package name, or an id, none of which start with `-`. Revisit
27+
* this if a command ever needs true passthrough (e.g. wrapping another CLI).
28+
*
29+
* Note this cannot be fixed with oclif's `'--': false` parser option: that
30+
* option keeps flag-parsing on past the separator but then re-appends the `--`
31+
* into argv, so strict commands fail with `Unexpected argument: --` instead.
32+
*/
33+
const hook: Hook<'preparse'> = async ({ argv }) => argv.filter((arg) => arg !== '--');
34+
35+
export default hook;

0 commit comments

Comments
 (0)