diff --git a/.changeset/scaffold-next-steps-package-manager.md b/.changeset/scaffold-next-steps-package-manager.md new file mode 100644 index 0000000000..59723d8c2d --- /dev/null +++ b/.changeset/scaffold-next-steps-package-manager.md @@ -0,0 +1,23 @@ +--- +"create-objectstack": patch +--- + +Fix `create-objectstack`'s closing "Next steps" and install-failure remedy +hardcoding `npm` regardless of which package manager the run actually used +(#10322). `detectPackageManager()` already prefers `pnpm` and falls back to +`npm` only when `pnpm` is unreachable — confirmed still true at HEAD, and +confirmed empirically: a real run with `pnpm` on `PATH` installs with `pnpm` +(`pnpm-lock.yaml`, "Done in … using pnpm vX") and then told the newcomer to +run `npm run dev` / `npm run validate` next, a package manager the run never +touched. The detected package manager is now read once, up front, and reused +consistently for the install command, the install-failure remedy, and every +line of "Next steps" — so the printed guidance always names the tool the run +actually used, in both the `pnpm` and the `npm`-fallback case. + +Also names `validate` — the step the generated `AGENTS.md` calls +unskippable — in the "Getting started" section of the generated `blank` +template's README, not only in its later "Verify your changes" section, so a +newcomer reading top-to-bottom sees it at first touch. + +No install behaviour changes: the scaffolder still installs by default and +still supports `--skip-install`; this is a messaging-only fix. diff --git a/packages/create-objectstack/src/blank-readme-validate-disclosure.test.ts b/packages/create-objectstack/src/blank-readme-validate-disclosure.test.ts new file mode 100644 index 0000000000..8fb30539de --- /dev/null +++ b/packages/create-objectstack/src/blank-readme-validate-disclosure.test.ts @@ -0,0 +1,58 @@ +// Copyright (c) 2026 ObjectStack contributors. Apache-2.0 license. +// +// Pins #10322 part 3 — the substantive half, per triage: the generated +// `AGENTS.md` calls `validate` the command you must never skip ("Never report +// a metadata change as done until `npm run validate` passes"), and the +// newcomer's primary doc, the blank template's own README, must name it where +// a newcomer reading top-to-bottom actually sees it, not only in a section +// further down the file. This template is a STATIC file — `index.ts` copies +// it byte-for-byte (only the first H1 line is rewritten, by +// `rewriteProjectIdentity`) — so there is no code path to unit-test; this +// source-text pin is what covers it. A fuller explanation of *why* to run it +// already lives in the "## Verify your changes" section further down; this +// pin is deliberately about the FIRST section a newcomer reads, not a +// duplicate of that explanation. + +import { describe, it, expect } from 'vitest'; +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const HERE = path.dirname(fileURLToPath(import.meta.url)); +const blankRoot = path.resolve(HERE, 'templates', 'blank'); +const readme = fs.readFileSync(path.join(blankRoot, 'README.md'), 'utf8'); + +describe('blank template README names `validate` at first touch (#10322)', () => { + it('reads the real template README (vacuity guard)', () => { + expect(readme).toMatch(/^## Getting started$/m); + }); + + it('mentions `validate` in or immediately after "Getting started" — not only further down', () => { + // Everything from the "Getting started" heading up to (not including) the + // next `## ` heading after it, minus the heading's own code fence — this + // is what a newcomer reads before scrolling past the first section. + const gettingStarted = readme.split(/^## Getting started$/m)[1]?.split(/^## /m)[0] ?? ''; + expect( + gettingStarted, + '"Getting started" must mention `validate` — otherwise a newcomer who ' + + 'only reads the first section never learns about the command ' + + "AGENTS.md calls unskippable.", + ).toMatch(/\bvalidate\b/); + }); + + it('the validate step named at first touch matches the fuller explanation below', () => { + expect(readme).toMatch(/^## Verify your changes$/m); + const verifySection = readme.split(/^## Verify your changes$/m)[1]?.split(/^## /m)[0] ?? ''; + expect(verifySection).toMatch(/\bvalidate\b/); + }); + + it('names one consistent package manager throughout — no bare npm mixed into a pnpm doc', () => { + // #10322 part 1: pick one and say it everywhere. The blank template + // already used pnpm consistently; this pin keeps it that way. Excludes + // the `engines.pnpm` prose about pnpm-version floors living in + // template-consistency.test.ts, and non-pm words like "npm" never occur + // here at all today — so a plain absence check is the right shape. + expect(readme).not.toMatch(/\bnpm run\b/); + expect(readme).not.toMatch(/\bnpm install\b/); + }); +}); diff --git a/packages/create-objectstack/src/index.ts b/packages/create-objectstack/src/index.ts index e54bfdcb47..1255748dfc 100644 --- a/packages/create-objectstack/src/index.ts +++ b/packages/create-objectstack/src/index.ts @@ -429,6 +429,17 @@ const program = new Command() const targetDir = name ? path.resolve(cwd, name) : cwd; const isCurrentDir = targetDir === cwd; + // Detected once, up front, and reused for every command this run prints or + // runs — the install line, the failure remedy and the closing "Next + // steps" all name the SAME package manager. Detecting it here (rather than + // only inside the install branch) means `--skip-install` still gets an + // accurate "Next steps" instead of a guess: the probe is a read-only + // ` --version` check, so running it costs nothing even when there is + // no install to drive. Previously "Next steps" hardcoded `npm` regardless + // of which package manager actually ran (#10322) — a newcomer who just + // watched `pnpm install` run was then told `npm run dev`. + const pm = detectPackageManager(); + printKV('Environment', projectName); printKV('Namespace', namespace); printKV('Template', `${options.template} — ${template.description}`); @@ -469,12 +480,11 @@ const program = new Command() printStep('Installing dependencies...'); let installed = false; try { - const pm = detectPackageManager(); execSync(`${pm} install`, { stdio: 'inherit', cwd: targetDir }); installed = true; console.log(''); } catch { - printWarning('Dependency installation failed. Run `npm install` manually.'); + printWarning(`Dependency installation failed. Run \`${pm} install\` manually.`); console.log(''); } @@ -539,11 +549,18 @@ const program = new Command() console.log(chalk.dim(` cd ${name}`)); } if (options.skipInstall) { - console.log(chalk.dim(' npm install')); + console.log(chalk.dim(` ${pm} install`)); } - console.log(chalk.dim(' npm run dev # Start development server')); - console.log(chalk.dim(' npm run validate # Verify metadata: schema + predicates + bindings')); - console.log(chalk.dim(' # (run after every metadata edit — see AGENTS.md)')); + // Same `${pm} run …` shape for both commands, padded to the longer of + // the two labels so the trailing comments still line up — for either + // package manager name, not just the `npm`-length one the literal + // strings above were hand-kerned for. + const devLabel = `${pm} run dev`; + const validateLabel = `${pm} run validate`; + const labelWidth = Math.max(devLabel.length, validateLabel.length) + 3; + console.log(chalk.dim(` ${devLabel.padEnd(labelWidth)}# Start development server`)); + console.log(chalk.dim(` ${validateLabel.padEnd(labelWidth)}# Verify metadata: schema + predicates + bindings`)); + console.log(chalk.dim(` ${' '.repeat(labelWidth)}# (run after every metadata edit — see AGENTS.md)`)); if (options.skipInstall || options.skipSkills) { console.log(''); console.log(chalk.bold(' AI Skills (recommended):')); diff --git a/packages/create-objectstack/src/scaffold-next-steps-pm.test.ts b/packages/create-objectstack/src/scaffold-next-steps-pm.test.ts new file mode 100644 index 0000000000..e72eb31880 --- /dev/null +++ b/packages/create-objectstack/src/scaffold-next-steps-pm.test.ts @@ -0,0 +1,131 @@ +// Copyright (c) 2026 ObjectStack contributors. Apache-2.0 license. +// +// Pins #10322: the printed "Next steps" (and the install-failure remedy) must +// name the SAME package manager the run actually detected — never a +// hardcoded `npm` regardless of what ran. Before this fix, a newcomer whose +// install ran with `pnpm` (confirmed empirically: this scaffolder prefers +// pnpm and only falls back to npm when pnpm is unreachable — see +// `detectPackageManager()`) was told to run `npm run dev` / `npm run +// validate` afterwards — the third of the "three different answers" #10322 +// measured. `packages/cli/src/commands/init.ts`'s own "Next steps" already +// threads its detected `chosenPm` through; this file is the same contract for +// `create-objectstack`. +// +// `index.ts` calls `program.parse()` at import time, so it cannot be +// unit-tested directly — this exercises the real CLI end to end via `tsx`, +// the same no-build subprocess pattern `scaffold-description.test.ts` uses. +// `--skip-install` keeps every run here fast and offline: `detectPackageManager()` +// is a read-only ` --version` probe (see index.ts), so its result — and +// therefore what "Next steps" prints — does not depend on an install actually +// following it. The *real* install path (both the pnpm and npm-fallback +// cases) was additionally verified by hand against the built CLI; see this +// issue's PR body for the transcripts. +// +// Both branches of the detector are exercised by controlling PATH: +// - pnpm reachable -> "pnpm run dev" / "pnpm run validate" +// - pnpm unreachable -> "npm run dev" / "npm run validate" (the fallback +// this scaffolder has always had for machines without pnpm) +// +// The "no bare npm when pnpm ran" assertion is deliberately a WORD-BOUNDARY +// match, not a substring one: the literal text "pnpm run" itself contains the +// substring "npm run" (p-N-P-M-space-r-u-n has "npm run" starting at its +// second character), so a naive `.not.toContain('npm run')` would fail +// against correct pnpm output. + +import { describe, it, expect } from 'vitest'; +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const HERE = path.dirname(fileURLToPath(import.meta.url)); +const PKG_ROOT = path.resolve(HERE, '..'); +const REPO_ROOT = path.resolve(PKG_ROOT, '..', '..'); +const TSX = path.join(REPO_ROOT, 'node_modules', '.bin', 'tsx'); +const INDEX_TS = path.join(PKG_ROOT, 'src', 'index.ts'); + +function which(cmd: string): string { + return execFileSync('sh', ['-c', `command -v ${cmd}`], { encoding: 'utf8' }).trim(); +} + +/** A PATH entry with `node` + `npm` reachable and `pnpm` deliberately absent. */ +function makePnpmlessBin(): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'create-objectstack-nopnpm-bin-')); + fs.symlinkSync(which('node'), path.join(dir, 'node')); + fs.symlinkSync(which('npm'), path.join(dir, 'npm')); + return dir; +} + +/** + * The "Next steps:" block of a run's stdout — deliberately narrower than the + * whole transcript. The "Created files" listing above it names + * `pnpm-workspace.yaml` regardless of which package manager ran the install + * (it is a static template file, not install output), so a bare + * `stdout.not.toMatch(/pnpm/)` would false-positive on that filename in the + * npm-fallback case. What actually matters is what the run tells the reader + * to type next. + */ +function nextStepsSection(stdout: string): string { + return stdout.split('Next steps:')[1] ?? ''; +} + +/** Run the real CLI with --skip-install --skip-skills and return its stdout. */ +function runScaffold(env: NodeJS.ProcessEnv): string { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'create-objectstack-nextsteps-')); + try { + return execFileSync( + TSX, + [INDEX_TS, 'my-app', '--template', 'blank', '--skip-install', '--skip-skills'], + { cwd: tmp, env, encoding: 'utf8' }, + ); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } +} + +describe('scaffolder "Next steps" names the package manager it actually detected (#10322)', () => { + it('with pnpm on PATH: prints pnpm consistently, never bare npm', () => { + // Sanity: this container really does have pnpm reachable, or the + // "consistently pnpm" assertion below would be vacuous. + expect(() => which('pnpm')).not.toThrow(); + + const nextSteps = nextStepsSection(runScaffold(process.env)); + expect(nextSteps).toMatch(/\bpnpm run dev\b/); + expect(nextSteps).toMatch(/\bpnpm run validate\b/); + expect(nextSteps).not.toMatch(/\bnpm run\b/); + expect(nextSteps).not.toMatch(/\bnpm install\b/); + }, 20_000); + + it('with pnpm unreachable: falls back to npm — consistently, not a stale pnpm mention', () => { + const bin = makePnpmlessBin(); + try { + // Sanity: the fake PATH really does hide pnpm (and really does still + // expose node/npm — otherwise tsx itself could not launch). + expect(() => + execFileSync('sh', ['-c', 'command -v pnpm'], { + env: { ...process.env, PATH: bin }, + }), + ).toThrow(); + + const nextSteps = nextStepsSection( + runScaffold({ ...process.env, PATH: `${bin}:/usr/bin:/bin` }), + ); + expect(nextSteps).toMatch(/\bnpm run dev\b/); + expect(nextSteps).toMatch(/\bnpm run validate\b/); + expect(nextSteps).not.toMatch(/pnpm/); + } finally { + fs.rmSync(bin, { recursive: true, force: true }); + } + }, 20_000); + + it('both branches still name the unskippable validate step (#10322 pt. 3)', () => { + expect(runScaffold(process.env)).toMatch(/run validate/); + }, 20_000); + + it('the install-failure remedy also names the detected package manager, not a hardcoded npm', () => { + const source = fs.readFileSync(INDEX_TS, 'utf8'); + expect(source).toMatch(/Dependency installation failed\. Run .*\$\{pm\} install.* manually\./); + expect(source).not.toMatch(/Run `npm install` manually/); + }); +}); diff --git a/packages/create-objectstack/src/templates/blank/README.md b/packages/create-objectstack/src/templates/blank/README.md index 6f6409fc90..36f8d3fb1d 100644 --- a/packages/create-objectstack/src/templates/blank/README.md +++ b/packages/create-objectstack/src/templates/blank/README.md @@ -9,6 +9,11 @@ pnpm install pnpm dev ``` +After editing any metadata (an object, view, flow, …), run `pnpm validate` — +see [Verify your changes](#verify-your-changes) below. It is the one command +this project's `AGENTS.md` calls unskippable: it catches mistakes that +otherwise fail silently at runtime. + The REST API is served at `http://localhost:3000/api/v1`. Data endpoints require a session — the dev server seeds a login-ready admin (`admin@objectos.ai` / `admin123`) on an empty database: diff --git a/scripts/check-cross-package-test-inputs.mjs b/scripts/check-cross-package-test-inputs.mjs index 529ca9d64f..60fbf72624 100644 --- a/scripts/check-cross-package-test-inputs.mjs +++ b/scripts/check-cross-package-test-inputs.mjs @@ -643,16 +643,24 @@ export const CROSS_PACKAGE_TEST_INPUTS = { // (its `paths:` filter is `packages/create-objectstack/**`), which is why // the test lives here rather than beside a shell script in spec (#9779). // - // The last three are NAMED in that test's header rather than read, the same - // shape as `check-nul-bytes.mjs` above and settled the same way: the literal - // collector takes quoted paths without parsing, so a mention forces a - // declaration, and declaring three rarely-touched files is cheaper than - // rewording prose to dodge a scanner. `serve.ts` earns it on the merits too - // — its `flags.dev || NODE_ENV === 'development'` port-shift gate is the - // single fact that decides which fix those workflow blocks need, so a change - // to that branch is exactly the change the test's premise would need - // re-measuring against. The two sibling scripts are cited for the contrast - // that keeps the fixes from being copied between them. + // Three of the remaining four are NAMED in a test's header rather than + // read, the same shape as `check-nul-bytes.mjs` above and settled the + // same way: the literal collector takes quoted paths without parsing, so + // a mention forces a declaration, and declaring a rarely-touched file is + // cheaper than rewording prose to dodge a scanner. `serve.ts` earns it on + // the merits too — its `flags.dev || NODE_ENV === 'development'` + // port-shift gate is the single fact that decides which fix those + // workflow blocks need, so a change to that branch is exactly the change + // the test's premise would need re-measuring against. The two sibling + // scripts are cited for the contrast that keeps the fixes from being + // copied between them. + // + // `packages/cli/src/commands/init.ts` is the fourth of that shape (#10322): + // scaffold-next-steps-pm.test.ts's header quotes it in backticks while + // explaining that `init.ts`'s own "Next steps" output already threads its + // detected `chosenPm` the same way this package's scaffolder now does — + // it is cited for the contrast, never read. The test execs + // `create-objectstack`'s own CLI via `tsx`, not `init.ts`. globs: [ 'content/**', 'scripts/sync-template-versions.mjs', @@ -670,6 +678,7 @@ export const CROSS_PACKAGE_TEST_INPUTS = { 'packages/cli/src/commands/serve.ts', 'scripts/gen-sdui-manifest.sh', 'scripts/publish-smoke.sh', + 'packages/cli/src/commands/init.ts', ], heldBy: { // Read through `git grep -- content/docs` and `git ls-files`, so the diff --git a/turbo.json b/turbo.json index ce24d57b19..667339bcae 100644 --- a/turbo.json +++ b/turbo.json @@ -231,7 +231,8 @@ "$TURBO_ROOT$/.github/workflows/scaffold-e2e.yml", "$TURBO_ROOT$/packages/cli/src/commands/serve.ts", "$TURBO_ROOT$/scripts/gen-sdui-manifest.sh", - "$TURBO_ROOT$/scripts/publish-smoke.sh" + "$TURBO_ROOT$/scripts/publish-smoke.sh", + "$TURBO_ROOT$/packages/cli/src/commands/init.ts" ] }, "test:e2e": {