Skip to content

Commit 539f597

Browse files
committed
fix(create-objectstack): a failed install lists the install step in "Next steps"
"Next steps" printed `<pm> install` only under `--skip-install`, so a run whose install was attempted and failed printed the same list as a successful one, and the next command it named ran against an empty node_modules. The condition is now `!installed`: skipped or failed. Pins: a stub `pnpm` whose `install` exits 1 lists the install step before the run commands; CONTROL legs for a succeeding install (no install step) and `--skip-install` (install step, as before). Claude-Session: https://claude.ai/code/session_018t6a5rvqPRvEDMCVVW9kYT Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8f4260f commit 539f597

3 files changed

Lines changed: 123 additions & 4 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'create-objectstack': patch
3+
---
4+
5+
Clause-②: no
6+
7+
When the dependency install fails (a network error, a registry outage, an engine mismatch, an npm crash), the
8+
scaffolder's closing "Next steps" now lists `npm install` (or `pnpm install`, whichever package manager the run
9+
detected) before `npm run dev` and `npm run validate`, the same as it does under `--skip-install`. Before, a
10+
failed install printed the same list as a successful one, so the next command it named ran against an empty
11+
`node_modules`. A successful install still prints no install step. No flag, exit code, prompt or scaffolded file
12+
changes.

‎packages/create-objectstack/src/index.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -631,7 +631,11 @@ const program = new Command()
631631
if (!isCurrentDir) {
632632
console.log(chalk.dim(` cd ${name}`));
633633
}
634-
if (options.skipInstall) {
634+
// `!installed`, not `options.skipInstall`: an install that was attempted
635+
// and FAILED leaves the same empty node_modules as a skipped one, so the
636+
// list a newcomer follows must name the install before the run commands
637+
// in both cases — the warning line above is easy to scroll past.
638+
if (!installed) {
635639
console.log(chalk.dim(` ${pm} install`));
636640
}
637641
// Same `${pm} run …` shape for both commands, padded to the longer of

‎packages/create-objectstack/src/scaffold-next-steps-pm.test.ts‎

Lines changed: 106 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
// `--skip-install` keeps every run here fast and offline: `detectPackageManager()`
1616
// is a read-only `<pm> --version` probe, so its result — and therefore what
1717
// "Next steps" prints — does not depend on an install actually following it.
18+
// The install-outcome legs at the foot of the file run without it, against a
19+
// stub `pnpm` that answers `install` itself, so they stay offline too.
1820
//
1921
// ─── WHY EVERY LEG STUBS pnpm, INCLUDING THE ONE WHERE pnpm WORKS ───────────
2022
//
@@ -146,13 +148,56 @@ function nextStepsSection(stdout: string): string {
146148
return stdout.split('Next steps:')[1] ?? '';
147149
}
148150

149-
/** Run the real CLI with --skip-install --skip-skills under `bin`'s PATH. */
150-
function runScaffold(bin: string): string {
151+
/**
152+
* Only the "Next steps:" command list — the lines up to the blank line that
153+
* ends it. Narrower again than `nextStepsSection`: the "AI Skills" block that
154+
* follows says "After `<pm> install`:" under `--skip-install`, so an install
155+
* assertion against the whole section would pass without the list naming it.
156+
*/
157+
function nextStepsList(stdout: string): string {
158+
return nextStepsSection(stdout).split(/\n\s*\n/)[0] ?? '';
159+
}
160+
161+
/**
162+
* `node` + `npm` reachable and a stub `pnpm` whose `--version` succeeds and
163+
* whose `install` exits with `installExit`. Every call's argv is appended to
164+
* `calls.log` beside it, so a leg can prove the install was (or was not)
165+
* attempted from the record rather than from the wording of a warning.
166+
*/
167+
function makeInstallStubPnpmBin(installExit: number): string {
168+
const dir = makeBin();
169+
const script =
170+
'#!/bin/sh\n' +
171+
`echo "$*" >> "${path.join(dir, 'calls.log')}"\n` +
172+
'case "$1" in\n' +
173+
' --version) echo "10.31.0"; exit 0 ;;\n' +
174+
` install) exit ${installExit} ;;\n` +
175+
'esac\n' +
176+
'exit 64\n';
177+
fs.writeFileSync(path.join(dir, 'pnpm'), script, { mode: 0o755 });
178+
return dir;
179+
}
180+
181+
/** The argv of every stub `pnpm` call a leg made, one entry per call. */
182+
function stubPnpmCalls(bin: string): string[] {
183+
const log = path.join(bin, 'calls.log');
184+
return fs.existsSync(log) ? fs.readFileSync(log, 'utf8').split('\n').filter(Boolean) : [];
185+
}
186+
187+
/** Run the real CLI with --skip-skills (and, by default, --skip-install) under `bin`'s PATH. */
188+
function runScaffold(bin: string, { skipInstall = true }: { skipInstall?: boolean } = {}): string {
151189
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'create-objectstack-nextsteps-'));
152190
try {
153191
return execFileSync(
154192
TSX,
155-
[INDEX_TS, 'my-app', '--template', 'blank', '--skip-install', '--skip-skills'],
193+
[
194+
INDEX_TS,
195+
'my-app',
196+
'--template',
197+
'blank',
198+
...(skipInstall ? ['--skip-install'] : []),
199+
'--skip-skills',
200+
],
156201
{ cwd: tmp, env: { ...process.env, PATH: childPath(bin) }, encoding: 'utf8' },
157202
);
158203
} finally {
@@ -259,3 +304,61 @@ describe('a failed pnpm probe is reported as a fallback, not as a choice', () =>
259304
});
260305
}, 20_000);
261306
});
307+
308+
// ─── A failed install leaves the same empty node_modules as a skipped one ───
309+
//
310+
// "Next steps" used to print `<pm> install` only under `--skip-install`, so a
311+
// run whose install was ATTEMPTED and FAILED (a network error, a registry
312+
// outage, an engine mismatch, an npm crash) printed the same list as a
313+
// successful one, and the next command it named ran against an empty
314+
// node_modules. The condition is now "the install did not complete". The
315+
// install's outcome is the stub's exit status, never the network, for the
316+
// reason the header gives; the stub's call log proves each leg took the path
317+
// it claims.
318+
319+
/** The index of the first line of `list` matching `re`, or -1. */
320+
function lineIndex(list: string, re: RegExp): number {
321+
return list.split('\n').findIndex((line) => re.test(line));
322+
}
323+
324+
describe('a failed install still lists the install step in "Next steps" (#23865)', () => {
325+
it('install attempted and failed: the list names `pnpm install`, before the run commands', () => {
326+
withBin(() => makeInstallStubPnpmBin(1), (bin) => {
327+
expectPnpmResolvesToStub(bin);
328+
329+
const list = nextStepsList(runScaffold(bin, { skipInstall: false }));
330+
331+
// Attempted, and the stub answered it with exit 1.
332+
expect(stubPnpmCalls(bin)).toContain('install');
333+
const install = lineIndex(list, /\bpnpm install\b/);
334+
expect(install).toBeGreaterThanOrEqual(0);
335+
expect(lineIndex(list, /\bpnpm run dev\b/)).toBeGreaterThan(install);
336+
});
337+
}, 20_000);
338+
339+
it('CONTROL: install attempted and succeeded, so no install step in the list', () => {
340+
withBin(() => makeInstallStubPnpmBin(0), (bin) => {
341+
expectPnpmResolvesToStub(bin);
342+
343+
const list = nextStepsList(runScaffold(bin, { skipInstall: false }));
344+
345+
expect(stubPnpmCalls(bin)).toContain('install');
346+
expect(list).toMatch(/\bpnpm run dev\b/);
347+
expect(list).not.toMatch(/\bpnpm install\b/);
348+
});
349+
}, 20_000);
350+
351+
it('CONTROL: `--skip-install` still names `pnpm install` before the run commands', () => {
352+
withBin(() => makeInstallStubPnpmBin(0), (bin) => {
353+
expectPnpmResolvesToStub(bin);
354+
355+
const list = nextStepsList(runScaffold(bin));
356+
357+
// Skipped, not attempted: the only call the stub saw is the probe.
358+
expect(stubPnpmCalls(bin)).toEqual(['--version']);
359+
const install = lineIndex(list, /\bpnpm install\b/);
360+
expect(install).toBeGreaterThanOrEqual(0);
361+
expect(lineIndex(list, /\bpnpm run dev\b/)).toBeGreaterThan(install);
362+
});
363+
}, 20_000);
364+
});

0 commit comments

Comments
 (0)