Skip to content

Commit cee3961

Browse files
os-litantclaude
andauthored
fix(cli): refuse a project name npm rejects in os create, before any write (#15893)
* fix(cli): refuse a project name npm rejects in `os create`, before any write `os create plugin "My App"` exited 0 having written `./plugin-My App/` with a manifest reading `name: "@objectstack/plugin-My App"`, while `os init "My App"` refused the same input and wrote nothing. `create` validated nothing it emitted. The rule set is `init`'s, imported rather than restated — the two scaffolders already shared four symbols, and the one they did not share is the one they disagreed on. `validateProjectName()` and npm's length ceiling are now exported from `init.ts` and called by `create.ts` before its first `mkdirSync`. The commands do not refuse identically, because they do not validate the same string: `init`'s argument IS the package name, while `create` composes its argument into a scoped one and npm's 214-character ceiling counts the scope. A 214-character name is therefore legal for `init` and composes to a 234-character name npm refuses. That one check lives next to the composition, and reads the name back off the RENDERED manifest so it measures the string that would land rather than a second copy of how it is built. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * chore(changeset): declare the `os create` project-name narrowing Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1290156 commit cee3961

4 files changed

Lines changed: 330 additions & 3 deletions

File tree

.changeset/olive-spiders-refuse.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@objectstack/cli": minor
3+
---
4+
5+
**BREAKING** `os create <type> <name>` now refuses a project name that npm refuses, and refuses it before it writes anything.
6+
7+
`os create plugin "My App"` used to exit 0 having written `./plugin-My App/`, carrying a manifest that read `name: "@objectstack/plugin-My App"`. Nothing failed at scaffold time, so the invalid name surfaced later at `npm publish`, in the terminal of whoever ran it next. `os init` has always refused that same input before touching the disk. The rule set is now shared between the two scaffolders rather than restated in one of them, so they answer the same way.
8+
9+
`os create` also refuses a name whose composed scoped package name exceeds npm's 214-character ceiling. `@objectstack/plugin-` spends 20 of those characters before the name begins, so a name that `os init` accepts can still compose to one npm rejects; that check sits next to the composition rather than in the shared rule set.
10+
11+
A scripted invocation that passed an invalid name now exits 1 with the reason on stderr, where it previously exited 0 and produced a project that could not be published.
12+
13+
<!-- adr-0087: not-required (no-migration-prescription) The change narrows what a CLI argument accepts at invocation time. No metadata surface, stored row or spec declaration is touched, so `objectstack migrate meta` has nothing to carry and the ledger has nothing to record. -->

packages/cli/src/commands/create.ts

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,11 @@ import path from 'path';
8282
import { PROTOCOL_MAJOR } from '@objectstack/spec/kernel';
8383
import {
8484
getCliVersion,
85+
NPM_PACKAGE_NAME_MAX_LENGTH,
8586
renderPnpmWorkspaceYaml,
8687
sanitizeNamespace,
8788
SCAFFOLD_PNPM_RANGE,
89+
validateProjectName,
8890
} from './init.js';
8991

9092
/**
@@ -165,6 +167,52 @@ function defineTemplate(t: Omit<CreateTemplate, 'files'>): CreateTemplate {
165167
};
166168
}
167169

170+
/**
171+
* The scoped package name a scaffold is about to write, READ BACK off the
172+
* rendered manifest rather than recomposed here.
173+
*
174+
* Recomposing it would be a second copy of `@objectstack/plugin-${name}` that
175+
* nothing keeps in step with the renderer — the same restatement that let this
176+
* command's emitted name drift away from what `os init` enforces. Reading the
177+
* rendered object measures the string that actually lands on disk, and a
178+
* template added later is covered without being told to declare anything.
179+
*
180+
* `null` when the template emits no `package.json`, or emits one without a
181+
* string `name`: there is then no package name to judge, which is not the same
182+
* as judging one and finding it fine.
183+
*/
184+
export function emittedPackageName(
185+
template: CreateTemplate,
186+
placement: ScaffoldPlacement,
187+
name: string,
188+
): string | null {
189+
const render = template.filesFor(placement)['package.json'];
190+
if (!render) return null;
191+
const manifest = render(name) as { name?: unknown } | null | undefined;
192+
return typeof manifest?.name === 'string' ? manifest.name : null;
193+
}
194+
195+
/**
196+
* The one rule `os create` needs and `os init` cannot.
197+
*
198+
* `init`'s argument IS the package name, so measuring the argument is the same
199+
* measurement. `create` composes its argument into a SCOPED name, and npm's
200+
* 214-character ceiling counts the scope: `@objectstack/plugin-` spends 20 of
201+
* them before the user's first character. A 200-character name is therefore
202+
* legal for `init` (measured: accepted) and illegal for `create` (measured:
203+
* emits a 220-character name npm refuses) — which is why the shared validator
204+
* is shared and this check is not.
205+
*/
206+
export function validateEmittedPackageName(packageName: string): string | null {
207+
const over = packageName.length - NPM_PACKAGE_NAME_MAX_LENGTH;
208+
if (over <= 0) return null;
209+
return (
210+
`The package name this would emit is ${packageName.length} characters; npm's limit is `
211+
+ `${NPM_PACKAGE_NAME_MAX_LENGTH}. Shorten the project name by at least ${over} character`
212+
+ `${over === 1 ? '' : 's'}.`
213+
);
214+
}
215+
168216
function toCamelCase(str: string): string {
169217
return str.replace(/-([a-z])/g, (g) => g[1].toUpperCase());
170218
}
@@ -464,12 +512,39 @@ export default class Create extends Command {
464512
console.log(chalk.dim(`Usage: objectstack create ${args.type} <name>`));
465513
process.exit(1);
466514
}
467-
515+
516+
// ⛔ BEFORE the first write, which is the whole property — a refusal that
517+
// arrives after `mkdirSync` has fixed the message and not the defect.
518+
//
519+
// This command used to validate nothing it emitted, so `os create plugin
520+
// "My App"` exited 0 having written `./plugin-My App/` with a manifest
521+
// reading `name: "@objectstack/plugin-My App"` — a name npm refuses —
522+
// while `os init "My App"` refused the same input and wrote nothing. The
523+
// rule set is `init`'s, imported rather than restated: the two scaffolders
524+
// already share four symbols, and the one they did not share is the one
525+
// they disagreed on.
526+
const nameError = validateProjectName(args.name);
527+
if (nameError) {
528+
console.error(chalk.red(`\n❌ ${nameError}`));
529+
console.log(chalk.dim(` Usage: objectstack create ${args.type} <name>`));
530+
process.exit(1);
531+
}
532+
468533
const template = templates[args.type as keyof typeof templates];
469534
const cwd = process.cwd();
470535
const placement: ScaffoldPlacement = flags['in-repo'] ? 'in-repo' : DEFAULT_PLACEMENT;
471536
const projectDirName = template.dirName(args.name);
472537

538+
// The check `init` cannot need, on the string `init` never composes. Also
539+
// before any write, and read off the rendered manifest so it measures what
540+
// would land rather than a second copy of how it is built.
541+
const willEmit = emittedPackageName(template, placement, args.name);
542+
const packageNameError = willEmit ? validateEmittedPackageName(willEmit) : null;
543+
if (packageNameError) {
544+
console.error(chalk.red(`\n❌ ${packageNameError}`));
545+
process.exit(1);
546+
}
547+
473548
// Refuse `--in-repo` outside a workspace rather than emit the one thing
474549
// this command is no longer allowed to emit: a project that cannot install.
475550
if (placement === 'in-repo' && !fs.existsSync(path.join(cwd, 'pnpm-workspace.yaml'))) {

packages/cli/src/commands/init.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -676,13 +676,34 @@ export function detectPackageManager(env: NodeJS.ProcessEnv = process.env): 'npm
676676
return 'npm';
677677
}
678678

679+
/**
680+
* npm's hard ceiling on a package name, the scope included.
681+
*
682+
* Exported because `os create` does NOT validate the same string this file
683+
* does: it composes its argument into a scoped name
684+
* (`@objectstack/plugin-<name>`) and has to measure the COMPOSED string
685+
* against this limit. Restating the number over there is exactly how the two
686+
* scaffolders came to disagree in the first place.
687+
*/
688+
export const NPM_PACKAGE_NAME_MAX_LENGTH = 214;
689+
679690
/**
680691
* Validate that `name` is a usable npm package name AND a safe directory
681692
* segment. Mirrors the subset of rules used by `npm init`/`create-vite`.
693+
*
694+
* Exported for `os create`, which took none of this and therefore accepted
695+
* names npm refuses — `os create plugin "My App"` wrote `./plugin-My App/`
696+
* carrying `name: "@objectstack/plugin-My App"`, while `os init "My App"`
697+
* refused the same input before touching the disk. The rule set is shared
698+
* rather than copied so a rule added here reaches both scaffolders; the one
699+
* check `create` needs and `init` cannot (the length of the composed scoped
700+
* name) lives next to the composition, in `create.ts`.
682701
*/
683-
function validateProjectName(name: string): string | null {
702+
export function validateProjectName(name: string): string | null {
684703
if (!name) return 'Project name is required';
685-
if (name.length > 214) return 'Project name must be ≤ 214 characters';
704+
if (name.length > NPM_PACKAGE_NAME_MAX_LENGTH) {
705+
return `Project name must be ≤ ${NPM_PACKAGE_NAME_MAX_LENGTH} characters`;
706+
}
686707
if (/[A-Z]/.test(name)) return 'Project name must be lowercase';
687708
if (!/^[a-z0-9][a-z0-9._-]*$/.test(name)) {
688709
return 'Project name must start with a lowercase letter or digit and contain only [a-z0-9._-]';
Lines changed: 218 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,218 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* PIN — `os create` refuses a project name npm refuses, BEFORE it writes.
5+
*
6+
* ## The defect
7+
*
8+
* Measured on `origin/main` e75a9040b02, driving the published entry:
9+
*
10+
* ```
11+
* $ os create plugin "My App"
12+
* exit 0 — wrote ./plugin-My App/, manifest name "@objectstack/plugin-My App"
13+
* $ os init "My App"
14+
* exit 2 — "Project name must be lowercase", wrote NOTHING
15+
* ```
16+
*
17+
* Both spellings are npm-invalid. One scaffolder refused before touching the
18+
* disk; the other emitted a directory and an unpublishable manifest with every
19+
* gate green, so the failure was deferred to `npm publish` in the terminal of
20+
* whoever ran it next.
21+
*
22+
* ## Why the refusal, and not just the message, is what is asserted
23+
*
24+
* `os init` refuses BEFORE the first write. A repair that refuses AFTER
25+
* `mkdirSync` has fixed the message and not the defect — the invalid directory
26+
* still lands. So every refusal case here asserts the directory is ABSENT, and
27+
* `accepts a valid name` is the positive control for that predicate: it runs
28+
* the same `scaffoldDir()` check against the same shape of temp directory and
29+
* finds the directory PRESENT. An absence assertion whose predicate cannot
30+
* fail is not evidence.
31+
*
32+
* ## Why the two commands do NOT refuse identically
33+
*
34+
* `os init`'s argument IS the package name. `os create`'s argument is composed
35+
* into a SCOPED one (`@objectstack/plugin-<name>`), and npm's 214-character
36+
* ceiling counts the scope — so a name that is legal for `init` can compose to
37+
* one npm refuses. `refuses a name only the composed length catches` pins that
38+
* asymmetry from both ends: the shared validator passes the name (asserted
39+
* directly), `create` refuses it, and `init` still accepts it. ⛔ Moving that
40+
* length rule into the shared validator would break `init` for a name npm
41+
* accepts; this test is what says so.
42+
*
43+
* Spawned through `bin/run-dev.js` + tsx, so the suite does not depend on
44+
* `packages/cli/dist` having been built — `@objectstack/cli#test` depends on
45+
* `^build` only (the reason `invocation-loudness.e2e.test.ts` spawns that way).
46+
*/
47+
48+
import { describe, it, expect } from 'vitest';
49+
import { execFile } from 'node:child_process';
50+
import { existsSync, mkdtempSync, rmSync } from 'node:fs';
51+
import { tmpdir } from 'node:os';
52+
import { join, resolve } from 'node:path';
53+
import { fileURLToPath } from 'node:url';
54+
import { childEnv } from './helpers/serve-process.js';
55+
import {
56+
emittedPackageName,
57+
templates,
58+
validateEmittedPackageName,
59+
DEFAULT_PLACEMENT,
60+
type ScaffoldPlacement,
61+
} from '../src/commands/create.js';
62+
import { NPM_PACKAGE_NAME_MAX_LENGTH, validateProjectName } from '../src/commands/init.js';
63+
64+
const HERE = resolve(fileURLToPath(import.meta.url), '..');
65+
const CLI = resolve(HERE, '../bin/run-dev.js');
66+
const TSX = resolve(HERE, '../../../node_modules/.bin/tsx');
67+
68+
/** oclif + tsx cold start with every command module loaded; ~2-10 s when healthy. */
69+
const RUN_TIMEOUT_MS = 180_000;
70+
71+
/** The card's input: a capital and a space, both npm-invalid. */
72+
const INVALID_NAME = 'My App';
73+
const VALID_NAME = 'my-app';
74+
75+
/**
76+
* A name the SHARED validator accepts and the composed one cannot: exactly at
77+
* `init`'s ceiling, so `@objectstack/plugin-` pushes it past the same ceiling.
78+
* Derived from the constant rather than written as a number, so a change to the
79+
* limit moves this case with it.
80+
*/
81+
const COMPOSED_TOO_LONG = 'a'.repeat(NPM_PACKAGE_NAME_MAX_LENGTH);
82+
83+
interface Run {
84+
code: number;
85+
stdout: string;
86+
stderr: string;
87+
}
88+
89+
function runCli(args: string[], cwd: string): Promise<Run> {
90+
return new Promise((resolvePromise) => {
91+
execFile(
92+
TSX,
93+
[CLI, ...args],
94+
{ cwd, maxBuffer: 8 * 1024 * 1024, env: childEnv({ NO_COLOR: '1' }) },
95+
(err, stdout, stderr) => {
96+
resolvePromise({
97+
// `err.code` is the real exit status; null/undefined means the child
98+
// was signalled — a different failure, never reported as 0.
99+
code: err
100+
? typeof (err as { code?: unknown }).code === 'number'
101+
? (err as unknown as { code: number }).code
102+
: 1
103+
: 0,
104+
stdout: String(stdout),
105+
stderr: String(stderr),
106+
});
107+
},
108+
);
109+
});
110+
}
111+
112+
/** A fresh empty directory to scaffold into, removed by the caller. */
113+
function workspace(): string {
114+
return mkdtempSync(join(tmpdir(), 'os-create-name-'));
115+
}
116+
117+
/** The path `os create plugin <name>` writes, present or not. */
118+
function scaffoldDir(cwd: string, name: string): string {
119+
return join(cwd, templates.plugin.dirName(name));
120+
}
121+
122+
describe('os create: a name npm refuses is refused before anything is written', () => {
123+
it(
124+
'refuses the card\'s input and writes NOTHING',
125+
async () => {
126+
const cwd = workspace();
127+
try {
128+
const run = await runCli(['create', 'plugin', INVALID_NAME], cwd);
129+
130+
expect(run.code).not.toBe(0);
131+
// The message is the SHARED validator's own return value, not a second
132+
// copy of it written here — that identity is what stops the two
133+
// scaffolders drifting apart again.
134+
expect(validateProjectName(INVALID_NAME)).not.toBeNull();
135+
expect(run.stderr).toContain(validateProjectName(INVALID_NAME)!);
136+
137+
// The load-bearing half: refused BEFORE the first write.
138+
expect(existsSync(scaffoldDir(cwd, INVALID_NAME))).toBe(false);
139+
} finally {
140+
rmSync(cwd, { recursive: true, force: true });
141+
}
142+
},
143+
RUN_TIMEOUT_MS,
144+
);
145+
146+
it(
147+
'accepts a valid name — the positive control for the absence check above',
148+
async () => {
149+
const cwd = workspace();
150+
try {
151+
const run = await runCli(['create', 'plugin', VALID_NAME], cwd);
152+
153+
expect(run.code).toBe(0);
154+
// Same predicate, same shape of directory, opposite verdict: the
155+
// absence assertion above is capable of failing.
156+
expect(existsSync(scaffoldDir(cwd, VALID_NAME))).toBe(true);
157+
} finally {
158+
rmSync(cwd, { recursive: true, force: true });
159+
}
160+
},
161+
RUN_TIMEOUT_MS,
162+
);
163+
164+
it(
165+
'refuses a name only the COMPOSED length catches, which `os init` must keep accepting',
166+
async () => {
167+
const createCwd = workspace();
168+
const initCwd = workspace();
169+
try {
170+
// The shared validator passes it — so whatever refuses it below is the
171+
// composed-name rule and nothing else.
172+
expect(validateProjectName(COMPOSED_TOO_LONG)).toBeNull();
173+
174+
const created = await runCli(['create', 'plugin', COMPOSED_TOO_LONG], createCwd);
175+
expect(created.code).not.toBe(0);
176+
expect(created.stderr).toContain(String(NPM_PACKAGE_NAME_MAX_LENGTH));
177+
expect(existsSync(scaffoldDir(createCwd, COMPOSED_TOO_LONG))).toBe(false);
178+
179+
// ⛔ The asymmetry is correct, not a second defect: `init`'s argument is
180+
// the package name, so npm's ceiling is already measured against it.
181+
const inited = await runCli(['init', COMPOSED_TOO_LONG], initCwd);
182+
expect(inited.code).toBe(0);
183+
} finally {
184+
rmSync(createCwd, { recursive: true, force: true });
185+
rmSync(initCwd, { recursive: true, force: true });
186+
}
187+
},
188+
RUN_TIMEOUT_MS,
189+
);
190+
});
191+
192+
describe('os create: the composed package name is judged for every template', () => {
193+
const PLACEMENTS: ScaffoldPlacement[] = ['standalone', 'in-repo'];
194+
195+
// Derived from the template map, never a list of `plugin` and `example`: a
196+
// third template must arrive already covered.
197+
for (const key of Object.keys(templates)) {
198+
for (const placement of PLACEMENTS) {
199+
it(`${key} / ${placement}: the emitted name is readable and judged`, () => {
200+
const emitted = emittedPackageName(templates[key], placement, VALID_NAME);
201+
expect(typeof emitted).toBe('string');
202+
expect(emitted).toContain(VALID_NAME);
203+
expect(validateEmittedPackageName(emitted!)).toBeNull();
204+
205+
const overlong = emittedPackageName(templates[key], placement, COMPOSED_TOO_LONG);
206+
expect(overlong!.length).toBeGreaterThan(NPM_PACKAGE_NAME_MAX_LENGTH);
207+
expect(validateEmittedPackageName(overlong!)).toContain(
208+
String(NPM_PACKAGE_NAME_MAX_LENGTH),
209+
);
210+
});
211+
}
212+
}
213+
214+
it('reads the name off the DEFAULT placement the same way the command does', () => {
215+
const emitted = emittedPackageName(templates.plugin, DEFAULT_PLACEMENT, VALID_NAME);
216+
expect(emitted).toBe(`@objectstack/plugin-${VALID_NAME}`);
217+
});
218+
});

0 commit comments

Comments
 (0)