From d7fd68d636c17d5397ddcc2844119292b09174af Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 00:18:25 +0000 Subject: [PATCH] fix(cli): print init's "Created files" summary after install, not before (#10557) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `objectstack init` accumulated the "Created files" list while writing the template — before ` install` ran — so the printed summary could never name `pnpm-lock.yaml` / `package-lock.json` or `node_modules/`. Measured against a real `pnpm install`: 7 entries printed, 9 paths on disk (+ node_modules/, 575 MB). Moves the print to after the install attempt (succeeded or failed) and derives it from a walk of the finished directory, reusing `create-objectstack`'s `created-summary.ts` (now published as the `create-objectstack/created-summary` subpath) instead of a second copy of the same renderer — the two scaffold paths already drifted once (#10499) from carrying separate implementations of this exact list. Fixes #10557 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r --- ...nit-created-files-summary-after-install.md | 14 ++ packages/cli/package.json | 1 + packages/cli/src/commands/init.ts | 70 +++++-- .../init-created-files-summary.e2e.test.ts | 194 ++++++++++++++++++ packages/cli/vitest.config.ts | 30 ++- packages/create-objectstack/package.json | 6 + packages/create-objectstack/tsup.config.ts | 9 +- pnpm-lock.yaml | 3 + 8 files changed, 307 insertions(+), 20 deletions(-) create mode 100644 .changeset/init-created-files-summary-after-install.md create mode 100644 packages/cli/test/init-created-files-summary.e2e.test.ts diff --git a/.changeset/init-created-files-summary-after-install.md b/.changeset/init-created-files-summary-after-install.md new file mode 100644 index 0000000000..67038a4e43 --- /dev/null +++ b/.changeset/init-created-files-summary-after-install.md @@ -0,0 +1,14 @@ +--- +"@objectstack/cli": patch +"create-objectstack": patch +--- + +Fix `objectstack init`'s closing "Created files" summary omitting `pnpm-lock.yaml` / `package-lock.json` and `node_modules/` (#10557). + +The summary used to be printed from a list accumulated while the template +files were written — before ` install` ran — so it could never name what +the package manager wrote. `init` now prints it after the install attempt +(succeeded or failed) from a walk of the finished project directory, reusing +`create-objectstack`'s `created-summary.ts` (now published as the +`create-objectstack/created-summary` subpath) instead of a second copy of the +same renderer. diff --git a/packages/cli/package.json b/packages/cli/package.json index 6549dd0738..2292600547 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -95,6 +95,7 @@ "bundle-require": "^5.1.0", "chalk": "^6.0.0", "chokidar": "^5.0.0", + "create-objectstack": "workspace:*", "dotenv-flow": "^4.1.0", "esbuild": "^0.28.2", "ts-morph": "^28.0.0", diff --git a/packages/cli/src/commands/init.ts b/packages/cli/src/commands/init.ts index 3a9cadee55..8ff3da7219 100644 --- a/packages/cli/src/commands/init.ts +++ b/packages/cli/src/commands/init.ts @@ -8,6 +8,7 @@ import path from 'path'; import { fileURLToPath } from 'url'; import { printHeader, printSuccess, printError, printStep, printKV, printInfo, formatZodErrors } from '../utils/format.js'; import { validateScaffold } from '../utils/scaffold-validate.js'; +import { summarizeTree, describeEntry } from 'create-objectstack/created-summary'; // ─── Version resolution ────────────────────────────────────────────── // @@ -473,6 +474,43 @@ function printWarning(msg: string) { console.log(chalk.yellow(` ⚠ ${msg}`)); } +/** + * Print the closing "Created files" summary from a walk of the FINISHED + * project directory, not from a list accumulated while writing the + * template. A list built during the copy phase is printed before ` + * install` runs and so can never name `pnpm-lock.yaml`, `package-lock.json`, + * `node_modules/`, or anything else the package manager writes — the exact + * gap this replaces (measured: a fresh `init` omitted a 138 KB + * `pnpm-lock.yaml` and a 575 MB `node_modules/` from its own "Created + * files" list). Reuses `create-objectstack`'s `created-summary.ts` (see its + * header for the reachability measurement that made a hand-accumulated + * list untenable for that scaffolder) instead of a second copy of the same + * renderer — the two scaffold paths already drifted once (#10499) from + * carrying separate implementations of the same list. + * + * Called once, after the install attempt (success OR failure) has run its + * course, so it always reports the real state of disk at that point rather + * than a promise: on a failed install it shows whatever partial state the + * failure left behind instead of silently disappearing. + */ +function printCreatedFilesSummary(targetDir: string, wasEmpty: boolean) { + const entries = summarizeTree(targetDir); + if (entries.length === 0) return; + + console.log(chalk.bold(wasEmpty ? ' Created files:' : ' Project contents:')); + if (!wasEmpty) { + console.log(chalk.dim(' (the directory already had contents; this lists all of it)')); + } + + const width = Math.min(44, Math.max(...entries.map((e) => e.path.length)) + 2); + for (const entry of entries) { + const note = describeEntry(entry); + const pad = note ? entry.path.padEnd(width) : entry.path; + console.log(chalk.green(` + ${pad}${note ? chalk.dim(note) : ''}`)); + } + console.log(''); +} + /** * Write a template's `srcFiles` into `targetDir` and return the relative paths * written, in creation order. @@ -635,7 +673,18 @@ export default class Init extends Command { printKV('Directory', targetDir); console.log(''); - const createdFiles: string[] = []; + // Read BEFORE the first write. The named-arg branch above refuses a + // non-empty target, but the no-name branch scaffolds into whatever the + // current directory already is — the closing summary is a walk of that + // directory, so it must know whether it is entitled to call what it + // finds "Created files" (mirrors create-objectstack's own `targetWasEmpty`). + let targetWasEmpty = true; + try { + targetWasEmpty = fs.readdirSync(targetDir).filter((e) => e !== '.git').length === 0; + } catch { + // targetDir does not exist — treated as empty (defensive; both + // branches above already create or verify it before this point). + } let installSucceeded = false; let installAttempted = false; @@ -647,7 +696,6 @@ export default class Init extends Command { if (!fs.existsSync(pkgPath)) { const pkg = renderScaffoldPackageJson(projectName, template); fs.writeFileSync(pkgPath, JSON.stringify(pkg, null, 2) + '\n'); - createdFiles.push('package.json'); } else { printInfo('package.json already exists, skipping'); } @@ -660,7 +708,6 @@ export default class Init extends Command { const pnpmWorkspacePath = path.join(targetDir, 'pnpm-workspace.yaml'); try { fs.writeFileSync(pnpmWorkspacePath, renderPnpmWorkspaceYaml(), { flag: 'wx' }); - createdFiles.push('pnpm-workspace.yaml'); } catch (err: any) { if (err?.code !== 'EEXIST') throw err; } @@ -668,7 +715,6 @@ export default class Init extends Command { // 2. Create objectstack.config.ts const configContent = template.configContent(projectName, namespace); fs.writeFileSync(path.join(targetDir, 'objectstack.config.ts'), configContent); - createdFiles.push('objectstack.config.ts'); // 3. Create tsconfig.json if missing const tsconfigPath = path.join(targetDir, 'tsconfig.json'); @@ -689,27 +735,18 @@ export default class Init extends Command { exclude: ['dist', 'node_modules'], }; fs.writeFileSync(tsconfigPath, JSON.stringify(tsconfig, null, 2) + '\n'); - createdFiles.push('tsconfig.json'); } // 4. Create src files (see `writeTemplateSrcFiles` for the `__name__` // placeholder rule and why the loop is exported). - createdFiles.push(...writeTemplateSrcFiles(template.srcFiles, targetDir, projectName, namespace)); + writeTemplateSrcFiles(template.srcFiles, targetDir, projectName, namespace); // 5. Create .gitignore if missing const gitignorePath = path.join(targetDir, '.gitignore'); if (!fs.existsSync(gitignorePath)) { fs.writeFileSync(gitignorePath, `node_modules/\ndist/\n*.tsbuildinfo\n`); - createdFiles.push('.gitignore'); } - // Summary - console.log(chalk.bold(' Created files:')); - for (const f of createdFiles) { - console.log(chalk.green(` + ${f}`)); - } - console.log(''); - // Install dependencies if (flags.install) { chosenPm = (flags['package-manager'] as typeof chosenPm | undefined) ?? detectPackageManager(); @@ -724,6 +761,11 @@ export default class Init extends Command { } } + // Created-files summary — printed HERE, after the install attempt has + // run its course (whether it succeeded or failed), so it can name what + // ` install` wrote. See `printCreatedFilesSummary` for why. + printCreatedFilesSummary(targetDir, targetWasEmpty); + // Self-test the scaffold so we catch template regressions before the // user discovers them by running `objectstack dev`. Only runs when deps // are present — `defineStack()` validation lives in `@objectstack/spec`. diff --git a/packages/cli/test/init-created-files-summary.e2e.test.ts b/packages/cli/test/init-created-files-summary.e2e.test.ts new file mode 100644 index 0000000000..77d2fc8430 --- /dev/null +++ b/packages/cli/test/init-created-files-summary.e2e.test.ts @@ -0,0 +1,194 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `objectstack init`'s closing "Created files" summary — over the REAL + * command, not a copy of its printing logic (#10557). + * + * ## The defect + * + * The summary used to be printed from an array accumulated WHILE the + * template files were written — and that happens BEFORE ` install` + * runs, so it could never name `pnpm-lock.yaml` / `package-lock.json` / + * `node_modules/`, or anything else the package manager writes. Measured + * against a real `objectstack init demo-app -t app --package-manager pnpm` + * on this tree before the fix: + * + * printed summary entries : 7 (template files only) + * written on disk : 9 (+ pnpm-lock.yaml, node_modules/ — 575 MB) + * + * ## The fork this closes, and why THIS side of it + * + * Two repairs were on the table: (A) move the print after the install + * attempt, or (B) keep the print where it was and predict the paths the + * install will write. (B) can name a path that never lands (a failed + * install, or a package manager that writes something different). (A)'s + * open question was what prints when the install FAILS. + * + * `create-objectstack`'s sibling scaffolder (`packages/create-objectstack/ + * src/index.ts`, the #10323 fix) had already measured and answered exactly + * this for the other scaffold path: print UNCONDITIONALLY once the install + * attempt — succeeded or failed — has run its course, from a WALK of the + * finished directory (`created-summary.ts`'s `summarizeTree`), never from a + * list assembled during the copy. `init` now reuses that exact module + * (imported as `create-objectstack/created-summary`, a published subpath — + * see that package's `exports`) instead of carrying a second copy of the + * same renderer, which is how the two scaffold paths drifted once already + * (#10499). + * + * ## Why a fake `pnpm` on PATH rather than a real install + * + * A real `pnpm install` against a scaffold outside the workspace hits the + * npm registry for every `@objectstack/*` package — slow, and in CI a + * source of flakiness unrelated to this property. The fake below does + * exactly what this test needs from "install" and nothing more: it writes + * `pnpm-lock.yaml` and populates `node_modules/`, including a REAL symlink + * to this repo's already-built `@objectstack/spec`, so the self-test step + * that runs after a successful install (`validateScaffold`, which bundles + * the scaffold's `objectstack.config.ts` and needs `defineStack` to + * resolve) passes too — closer to a real install than a bare touch, and it + * costs nothing extra: `created-summary.ts` never follows a symlink while + * walking (see its header), so this stays fast regardless of how large the + * real `spec` package is on disk. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execFile } from 'node:child_process'; +import { mkdtempSync, rmSync, mkdirSync, writeFileSync, chmodSync, readdirSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const HERE = resolve(fileURLToPath(import.meta.url), '..'); +const CLI = resolve(HERE, '../bin/run-dev.js'); +const REPO_ROOT = resolve(HERE, '../../..'); +const TSX = resolve(REPO_ROOT, 'node_modules/.bin/tsx'); +const SPEC_PKG = resolve(REPO_ROOT, 'packages/spec'); + +interface Run { + code: number; + stdout: string; + stderr: string; +} + +function runCli(args: string[], cwd: string, env: Record): Promise { + return new Promise((resolvePromise) => { + execFile( + TSX, + [CLI, ...args], + { cwd, maxBuffer: 16 * 1024 * 1024, env: { ...process.env, NO_COLOR: '1', ...env } }, + (err, stdout, stderr) => { + resolvePromise({ + // The real exit status, not truthiness of `err` — a non-zero code + // without a signal still lands here as an `error` from `execFile`. + code: err ? (typeof (err as { code?: unknown }).code === 'number' ? (err as unknown as { code: number }).code : 1) : 0, + stdout: String(stdout), + stderr: String(stderr), + }); + }, + ); + }); +} + +/** + * A fake `pnpm` on PATH. `opts.fail` simulates the OTHER path this fix has + * to answer for: install fails, and the summary must still name whatever + * DID land rather than disappearing (see the file header's fork). + */ +function writeFakePnpm(binDir: string, opts: { fail: boolean }) { + mkdirSync(binDir, { recursive: true }); + const bin = join(binDir, 'pnpm'); + + if (opts.fail) { + writeFileSync(bin, '#!/bin/sh\necho "fake pnpm: simulated install failure" >&2\nexit 1\n'); + } else { + // 15 dummy top-level node_modules entries — over created-summary's + // COLLAPSE_AT of 10 — so this also exercises the collapsed-directory + // line a real install's hundreds of packages would produce, plus a + // real symlink to the already-built @objectstack/spec so the + // post-install self-test has a `defineStack` to resolve. + const script = [ + '#!/bin/sh', + 'set -e', + 'mkdir -p node_modules/@objectstack/spec', + `cp ${JSON.stringify(join(SPEC_PKG, 'package.json'))} node_modules/@objectstack/spec/package.json`, + `ln -s ${JSON.stringify(join(SPEC_PKG, 'dist'))} node_modules/@objectstack/spec/dist`, + ': > pnpm-lock.yaml', + 'i=1', + 'while [ "$i" -le 15 ]; do', + ' mkdir -p "node_modules/.fake-pkg-$i"', + ' : > "node_modules/.fake-pkg-$i/index.js"', + ' i=$((i + 1))', + 'done', + '', + ].join('\n'); + writeFileSync(bin, script); + } + chmodSync(bin, 0o755); +} + +let parentDir: string; +let binDir: string; +let successProjectDir: string; +let successRun: Run; +let failProjectDir: string; +let failRun: Run; + +beforeAll(async () => { + parentDir = mkdtempSync(join(tmpdir(), 'os-init-summary-e2e-')); + binDir = join(parentDir, 'fakebin'); + const env = { PATH: `${binDir}:${process.env.PATH ?? ''}` }; + + writeFakePnpm(binDir, { fail: false }); + successProjectDir = join(parentDir, 'demo-success'); + successRun = await runCli(['init', 'demo-success', '-t', 'app', '--package-manager', 'pnpm'], parentDir, env); + + writeFakePnpm(binDir, { fail: true }); + failProjectDir = join(parentDir, 'demo-fail'); + failRun = await runCli(['init', 'demo-fail', '-t', 'app', '--package-manager', 'pnpm'], parentDir, env); +}, 60_000); + +afterAll(() => { + try { rmSync(parentDir, { recursive: true, force: true }); } catch { /* ignore */ } +}); + +describe('objectstack init — "Created files" summary names what `pnpm install` wrote (#10557)', () => { + it('a successful install: exits 0, and the summary names pnpm-lock.yaml and node_modules/', () => { + expect(successRun.code).toBe(0); + expect(successRun.stdout).toContain('pnpm-lock.yaml'); + // Collapsed-directory line: "node_modules/ files, ". + expect(successRun.stdout).toMatch(/node_modules\/\s+\d+ files?,/); + }); + + it('prints the summary AFTER attempting the install, not before', () => { + const installIdx = successRun.stdout.indexOf('Installing dependencies with pnpm'); + const summaryIdx = successRun.stdout.indexOf('Created files:'); + expect(installIdx).toBeGreaterThan(-1); + expect(summaryIdx).toBeGreaterThan(-1); + expect(installIdx).toBeLessThan(summaryIdx); + }); + + it('really wrote what it claims — every top-level path on disk is named in the summary', () => { + const onDisk = readdirSync(successProjectDir); + expect(onDisk).toEqual(expect.arrayContaining(['pnpm-lock.yaml', 'node_modules'])); + for (const name of onDisk) { + const named = successRun.stdout.includes(name) || successRun.stdout.includes(`${name}/`); + expect(named, `"${name}" is on disk but not named anywhere in the printed summary`).toBe(true); + } + }); + + it('a FAILED install still prints a summary — reality, not a promise', () => { + // The other failure mode the fork's Option A had to answer for: a + // summary that never appears on a failed run would be a new gap. + expect(failRun.stdout).toContain('Created files:'); + expect(failRun.stdout).toContain('package.json'); + expect(failRun.stdout).toContain('objectstack.config.ts'); + + // Nothing the fake pnpm would have written landed on disk this time, so + // the summary must not claim it did either. + const onDisk = readdirSync(failProjectDir); + expect(onDisk).not.toContain('pnpm-lock.yaml'); + expect(onDisk).not.toContain('node_modules'); + expect(failRun.stdout).not.toContain('pnpm-lock.yaml'); + expect(failRun.stdout).not.toMatch(/node_modules\//); + }); +}); diff --git a/packages/cli/vitest.config.ts b/packages/cli/vitest.config.ts index ac98b98f7f..58a912ac49 100644 --- a/packages/cli/vitest.config.ts +++ b/packages/cli/vitest.config.ts @@ -3,14 +3,14 @@ // This package had NO vitest config until #9832, and that is the fact this // header exists to keep visible: adding one changes how every test file in // `packages/cli` is configured, not just the file that needed it. So the -// config is deliberately minimal — a single anchored `resolve.alias` entry and -// no `test` block at all, because the 135 test files here run on vitest's -// defaults (`globals: false`, `environment: 'node'`) and a `test` block would -// silently re-specify them. Sibling configs in this repo do carry +// config is deliberately minimal — anchored `resolve.alias` entries and no +// `test` block at all, because the test files here run on vitest's defaults +// (`globals: false`, `environment: 'node'`) and a `test` block would silently +// re-specify them. Sibling configs in this repo do carry // `test: { globals: true, … }`; copying that shape here would have flipped // `globals` for every existing file in the package. // -// ## Why the one entry +// ## Why the service-cache entry // // `serve-observability-registration.test.ts` (#9832) boots the REAL // `CacheServicePlugin` to prove that a consumer registered after @@ -34,6 +34,22 @@ // exactly one file — the new test — and cannot move any existing suite. // Measured again after: 135 files / 1470 tests, unchanged. // +// ## Why the create-objectstack entry (#10557) +// +// `init.ts` prints its "Created files" summary from a walk of the finished +// project directory rather than a list accumulated while writing the +// template (see the command's own header) — reusing `create-objectstack`'s +// `created-summary.ts`, published as the `create-objectstack/created-summary` +// subpath so both scaffold paths share one renderer instead of drifting +// (#10499). Without an alias that bare specifier resolves through +// `create-objectstack`'s `exports` to its **dist**, for the same reason and +// the same danger as the entry above: a stale `dist/created-summary.js` +// would make every test that reaches `init.ts` a verdict about build state. +// `init.ts` is imported (relatively, inside this package) by three existing +// test files — `commands.test.ts`, `init.test.ts`, +// `init-scaffold-authoring-rules.test.ts` — so all three are reachable from +// this entry, none of them new; this alias just keeps them pointed at source. +// // Crossing into `service-cache/src` also makes ITS value imports reachable to // the gate's walk. That is one package, `@objectstack/observability`, which is // already in this package's ledger entry — so the required set is unchanged in @@ -130,6 +146,10 @@ export default defineConfig({ find: /^@objectstack\/service-cache$/, replacement: path.resolve(__dirname, '../services/service-cache/src/index.ts'), }, + { + find: /^create-objectstack\/created-summary$/, + replacement: path.resolve(__dirname, '../create-objectstack/src/created-summary.ts'), + }, ], }, }); diff --git a/packages/create-objectstack/package.json b/packages/create-objectstack/package.json index c6db18aaec..9d62902c8f 100644 --- a/packages/create-objectstack/package.json +++ b/packages/create-objectstack/package.json @@ -5,6 +5,12 @@ "bin": { "create-objectstack": "./bin/create-objectstack.js" }, + "exports": { + "./created-summary": { + "types": "./dist/created-summary.d.ts", + "import": "./dist/created-summary.js" + } + }, "scripts": { "build": "tsup", "typecheck": "tsc --noEmit", diff --git a/packages/create-objectstack/tsup.config.ts b/packages/create-objectstack/tsup.config.ts index c72bc9e93d..cad302081b 100644 --- a/packages/create-objectstack/tsup.config.ts +++ b/packages/create-objectstack/tsup.config.ts @@ -4,10 +4,17 @@ import { defineConfig } from 'tsup'; import { cpSync } from 'fs'; export default defineConfig({ - entry: ['src/index.ts'], + // `index.ts` is the CLI entry point — executed for its side effects via + // `bin/create-objectstack.js` — and stays undeclared. `created-summary.ts` + // is the one file this package exposes as a library subpath (see the + // `exports` map in package.json) so `@objectstack/cli`'s `init` command can + // reuse the same "walk the finished directory" summary renderer instead of + // carrying a second copy of it. + entry: ['src/index.ts', 'src/created-summary.ts'], format: ['esm'], clean: true, shims: true, + dts: { entry: ['src/created-summary.ts'] }, onSuccess: async () => { // Copy template files to dist/ so they sit alongside the bundled JS cpSync('src/templates', 'dist/templates', { recursive: true }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 704b88939e..a2d0b855b7 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -565,6 +565,9 @@ importers: chokidar: specifier: ^5.0.0 version: 5.0.0 + create-objectstack: + specifier: workspace:* + version: link:../create-objectstack dotenv-flow: specifier: ^4.1.0 version: 4.1.0