From ab051b0dad0b92c7734ed46c936281c530f857ca Mon Sep 17 00:00:00 2001 From: Koji Wakayama Date: Fri, 14 Aug 2026 18:05:31 +0200 Subject: [PATCH 1/2] fix(generate): warn when scaffolding outside a Veryfront project MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `veryfront generate` writes relative to the invocation directory with no project check, so running it one level above the project (or in the wrong terminal tab) silently created a stray `app/` tree and exited 0: $ cd /tmp && veryfront generate page about ● Created /tmp/app/about/page.tsx `veryfront dev` already detects and reports this situation. Match it: warn when no project marker (veryfront.config.*, or a manifest depending on veryfront) is present, then scaffold anyway so bootstrapping a not-yet- configured directory keeps working. Found while dogfooding the documented journeys against published v0.1.1237. --- cli/commands/generate/command.ts | 57 +++++++++++++++++++ .../generate/generate.integration.test.ts | 57 ++++++++++++++++++- 2 files changed, 113 insertions(+), 1 deletion(-) diff --git a/cli/commands/generate/command.ts b/cli/commands/generate/command.ts index 5b7b2ffc1a..0e8953dc1a 100644 --- a/cli/commands/generate/command.ts +++ b/cli/commands/generate/command.ts @@ -3,6 +3,61 @@ import { cliLogger } from "#cli/utils"; import { createError, toError } from "veryfront/errors"; import { generateIntegration } from "./integration-generator.ts"; import { isScaffoldType, scaffoldProjectFile } from "../../scaffold/engine.ts"; +import { exists, readTextFile } from "#veryfront/compat/fs.ts"; +import { join } from "#veryfront/compat/path"; + +const PROJECT_MARKERS = [ + "veryfront.config.ts", + "veryfront.config.js", + "veryfront.config.mjs", + "veryfront.config.json", +] as const; + +/** + * `generate` writes into whatever directory it is invoked from, so running it + * one level above the project (or in the wrong terminal tab) silently produces + * a stray `app/` tree. `dev` already warns in this situation; match it rather + * than failing, so scaffolding into a not-yet-configured directory still works. + */ +async function looksLikeVeryfrontProject(projectDir: string): Promise { + for (const marker of PROJECT_MARKERS) { + if (await exists(join(projectDir, marker))) return true; + } + + for (const manifest of ["package.json", "deno.json", "deno.jsonc"]) { + const path = join(projectDir, manifest); + if (!(await exists(path))) continue; + try { + const parsed = JSON.parse(await readTextFile(path)) as { + dependencies?: Record; + devDependencies?: Record; + imports?: Record; + }; + const specifiers = [ + ...Object.keys(parsed.dependencies ?? {}), + ...Object.keys(parsed.devDependencies ?? {}), + ...Object.keys(parsed.imports ?? {}), + ]; + if ( + specifiers.some((s) => s === "veryfront" || s.startsWith("veryfront/")) + ) { + return true; + } + } catch { + // An unparseable manifest is not evidence either way; keep looking. + } + } + + return false; +} + +async function warnIfOutsideProject(projectDir: string): Promise { + if (await looksLikeVeryfrontProject(projectDir)) return; + cliLogger.warn( + `${projectDir} does not look like a Veryfront project; scaffolding here anyway. ` + + `Run this from your project root, or create one with "npm create veryfront".`, + ); +} async function getPreferredRouter( projectDir: string, @@ -25,6 +80,8 @@ export async function generateCommand( type: string, name: string, ): Promise { + await warnIfOutsideProject(projectDir); + const preferred = await getPreferredRouter(projectDir); if (type === "integration") { diff --git a/cli/commands/generate/generate.integration.test.ts b/cli/commands/generate/generate.integration.test.ts index ae8724afa3..000f5379de 100644 --- a/cli/commands/generate/generate.integration.test.ts +++ b/cli/commands/generate/generate.integration.test.ts @@ -2,9 +2,15 @@ import "#veryfront/schemas/_test-setup.ts"; import { assert } from "#veryfront/testing/assert"; import { join } from "#veryfront/compat/path"; import { describe, it } from "#veryfront/testing/bdd"; -import { exists, remove, writeTextFile } from "#veryfront/compat/fs.ts"; +import { exists, makeTempDir, remove, writeTextFile } from "#veryfront/compat/fs.ts"; import { generateCommand } from "./index.ts"; import { type TestContext, withTestContext } from "../../../tests/_helpers/context.ts"; +import { + __registerLogRecordEmitter, + __resetLoggerConfigForTests, + __resetLogRecordEmitterForTests, + type LogEntry, +} from "#veryfront/utils/logger/logger.ts"; async function setPreferredRouter( context: TestContext, @@ -101,4 +107,53 @@ describe("CLI generate command", () => { assert(await exists(join(context.projectDir, "skills", "code-review", "SKILL.md"))); }); }); + + describe("outside a Veryfront project", () => { + function captureLogs(): LogEntry[] { + const entries: LogEntry[] = []; + __resetLoggerConfigForTests(); + __registerLogRecordEmitter((entry) => entries.push(entry)); + return entries; + } + + it("warns before scaffolding into a directory that is not a project", async () => { + const bare = await makeTempDir({ prefix: "generate-not-a-project-" }); + const entries = captureLogs(); + try { + await generateCommand(bare, "page", "about"); + + const warning = entries.find((entry) => + entry.level === "warn" && entry.message.includes("does not look like a Veryfront project") + ); + assert( + warning !== undefined, + "expected a warning that the target directory is not a Veryfront project", + ); + // Still scaffolds — the warning informs, it does not block. + assert(await exists(join(bare, "app", "about", "page.tsx"))); + } finally { + __resetLogRecordEmitterForTests(); + __resetLoggerConfigForTests(); + await remove(bare, { recursive: true }); + } + }); + + it("stays quiet inside a real project", async () => { + await withTestContext("generate-in-project-quiet", async (context: TestContext) => { + const entries = captureLogs(); + try { + await generateCommand(context.projectDir, "page", "about"); + + const warning = entries.find((entry) => + entry.level === "warn" && + entry.message.includes("does not look like a Veryfront project") + ); + assert(warning === undefined, "must not warn inside a real project"); + } finally { + __resetLogRecordEmitterForTests(); + __resetLoggerConfigForTests(); + } + }); + }); + }); }); From 4bcdd940cd81a8023c7340c7f3f1a8112b1c4c8b Mon Sep 17 00:00:00 2001 From: Koji Wakayama Date: Fri, 14 Aug 2026 18:37:10 +0200 Subject: [PATCH 2/2] =?UTF-8?q?fix(generate):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20path=20leak,=20legacy=20marker,=20JSONC=20manifests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings, all confirmed against the code before changing anything: 1. The warning printed `projectDir`, an absolute machine path. AGENTS.md forbids local absolute paths in user-facing output. The directory is where the user already is, so the message names no path at all now. 2. `veryfront.config.json` was not a real marker — it appeared only in this file. The name the CLI config loader actually reads is the legacy `veryfront.json` (cli/shared/config.ts), so a project identified only by that file was warned about incorrectly. Swapped. 3. `deno.json` and `deno.jsonc` were parsed with strict `JSON.parse`, so a manifest using the comments and trailing commas Deno permits threw, hit the catch, and counted as no evidence — a false warning on a valid project. Now parsed with `parseExtensionManifest`, matching the JSONC grammar `src/extensions/discovery.ts` already applies to both filenames. Three regression tests added, one per finding. Not changed: the pre-existing `● Created ` line also prints an absolute path. It predates this PR and callers rely on it; worth a separate look rather than widening this change. --- cli/commands/generate/command.ts | 28 +++++++-- .../generate/generate.integration.test.ts | 63 +++++++++++++++++++ 2 files changed, 85 insertions(+), 6 deletions(-) diff --git a/cli/commands/generate/command.ts b/cli/commands/generate/command.ts index 0e8953dc1a..510c66c3a4 100644 --- a/cli/commands/generate/command.ts +++ b/cli/commands/generate/command.ts @@ -5,12 +5,25 @@ import { generateIntegration } from "./integration-generator.ts"; import { isScaffoldType, scaffoldProjectFile } from "../../scaffold/engine.ts"; import { exists, readTextFile } from "#veryfront/compat/fs.ts"; import { join } from "#veryfront/compat/path"; +import { parseExtensionManifest } from "#veryfront/extensions/manifest-reader.ts"; const PROJECT_MARKERS = [ "veryfront.config.ts", "veryfront.config.js", "veryfront.config.mjs", - "veryfront.config.json", + // Legacy, but still read and merged by the CLI config loader, so a project + // identified only by this file is a real project. `veryfront.config.json` is + // deliberately absent — the loader does not recognise that name. + "veryfront.json", +] as const; + +// Deno accepts JSONC grammar for `deno.json` as well as `deno.jsonc`, matching +// `src/extensions/discovery.ts`. Parsing those with strict JSON makes a +// commented manifest look like no evidence at all and fires a false warning. +const PROJECT_MANIFESTS = [ + { name: "package.json", syntax: "json" }, + { name: "deno.json", syntax: "jsonc" }, + { name: "deno.jsonc", syntax: "jsonc" }, ] as const; /** @@ -24,15 +37,15 @@ async function looksLikeVeryfrontProject(projectDir: string): Promise { if (await exists(join(projectDir, marker))) return true; } - for (const manifest of ["package.json", "deno.json", "deno.jsonc"]) { - const path = join(projectDir, manifest); + for (const manifest of PROJECT_MANIFESTS) { + const path = join(projectDir, manifest.name); if (!(await exists(path))) continue; try { - const parsed = JSON.parse(await readTextFile(path)) as { + const parsed = parseExtensionManifest<{ dependencies?: Record; devDependencies?: Record; imports?: Record; - }; + }>(await readTextFile(path), manifest.syntax, manifest.name); const specifiers = [ ...Object.keys(parsed.dependencies ?? {}), ...Object.keys(parsed.devDependencies ?? {}), @@ -53,8 +66,11 @@ async function looksLikeVeryfrontProject(projectDir: string): Promise { async function warnIfOutsideProject(projectDir: string): Promise { if (await looksLikeVeryfrontProject(projectDir)) return; + // Deliberately no path: `projectDir` is an absolute machine path, which + // AGENTS.md forbids in user-facing output. The directory is where the user + // already is, so naming it adds nothing they cannot see. cliLogger.warn( - `${projectDir} does not look like a Veryfront project; scaffolding here anyway. ` + + `The current directory does not look like a Veryfront project; scaffolding here anyway. ` + `Run this from your project root, or create one with "npm create veryfront".`, ); } diff --git a/cli/commands/generate/generate.integration.test.ts b/cli/commands/generate/generate.integration.test.ts index 000f5379de..ae90ee7358 100644 --- a/cli/commands/generate/generate.integration.test.ts +++ b/cli/commands/generate/generate.integration.test.ts @@ -138,6 +138,69 @@ describe("CLI generate command", () => { } }); + it("does not leak an absolute machine path into the warning", async () => { + // AGENTS.md forbids local absolute paths in user-facing output. + const bare = await makeTempDir({ prefix: "generate-no-path-leak-" }); + const entries = captureLogs(); + try { + await generateCommand(bare, "page", "about"); + + const warning = entries.find((entry) => + entry.level === "warn" && entry.message.includes("does not look like a Veryfront project") + ); + assert(warning !== undefined, "expected the outside-project warning"); + assert( + !warning.message.includes(bare), + `warning must not contain the absolute path: ${warning?.message}`, + ); + } finally { + __resetLogRecordEmitterForTests(); + __resetLoggerConfigForTests(); + await remove(bare, { recursive: true }); + } + }); + + it("treats a commented deno.jsonc with a veryfront import as a project", async () => { + // Deno permits comments and trailing commas here; strict JSON parsing + // made such a project look like no project at all. + const dir = await makeTempDir({ prefix: "generate-denojsonc-" }); + const entries = captureLogs(); + try { + await writeTextFile( + join(dir, "deno.jsonc"), + '{\n // the framework\n "imports": {\n "veryfront": "npm:veryfront@^0.1.0",\n },\n}\n', + ); + await generateCommand(dir, "page", "about"); + + const warning = entries.find((entry) => + entry.level === "warn" && entry.message.includes("does not look like a Veryfront project") + ); + assert(warning === undefined, "a commented deno.jsonc must count as project evidence"); + } finally { + __resetLogRecordEmitterForTests(); + __resetLoggerConfigForTests(); + await remove(dir, { recursive: true }); + } + }); + + it("treats a legacy veryfront.json as a project marker", async () => { + const dir = await makeTempDir({ prefix: "generate-legacy-config-" }); + const entries = captureLogs(); + try { + await writeTextFile(join(dir, "veryfront.json"), '{ "projectSlug": "legacy-app" }\n'); + await generateCommand(dir, "page", "about"); + + const warning = entries.find((entry) => + entry.level === "warn" && entry.message.includes("does not look like a Veryfront project") + ); + assert(warning === undefined, "veryfront.json is still read by the config loader"); + } finally { + __resetLogRecordEmitterForTests(); + __resetLoggerConfigForTests(); + await remove(dir, { recursive: true }); + } + }); + it("stays quiet inside a real project", async () => { await withTestContext("generate-in-project-quiet", async (context: TestContext) => { const entries = captureLogs();