fix: correct batch of CLI correctness bugs found in critical-bug sweep

- `stanza init --dry-run` no longer creates the project directory, monorepo shell, or `stanza.json`; the dry-run manifest is built with `emptyManifest` and package.json existence checks are skipped throughout `applyModule`
- Direct-fs codemods (`append-to-file`, `add-package-dep`, `set-tsconfig-paths`) now claim their region before writing so a failed `add` rolls back to true pre-apply bytes and `RegionConflictError` fires before any disk change
- `stanza remove` reads the `consumesPackages` snapshot persisted on each installed record (falling back to a live registry fetch only for legacy records), so package-dir protection holds offline and after upstream renames
- `stanza remove` only applies the legacy bare-id owner fallback when no sibling install of the same module remains, preventing one app's removal from sweeping another app's files on pre-composite-owner manifests
- `stanza doctor` no longer reports false drift for package-home modules installed with an `--app` restriction; `expectedConsumerApps` now intersects targeted apps from the manifest records
- `wrap-root-layout` resolves the framework per dispatched app, fixing multi-app projects with differing frameworks
- The codemod render context now carries `packageManager` so `{{pm}}`/`{{run …}}` templates render bun/npm/pnpm correctly instead of always defaulting to pnpm
This commit is contained in:
2026-06-09 19:04:38 -04:00
parent 0cb8d27245
commit 6ada5c0a88
14 changed files with 220 additions and 66 deletions
+13
View File
@@ -0,0 +1,13 @@
---
"stanza-cli": patch
---
Fix a batch of correctness bugs found in a critical-bug sweep:
- `stanza init --dry-run` no longer writes anything (previously it created the project directory, monorepo shell, and `stanza.json` despite saying "no files will be written").
- Direct-fs codemods (`append-to-file`, `add-package-dep`, `set-tsconfig-paths`) claim their region **before** writing, so a failed `add` rolls files back to their true pre-apply bytes and `RegionConflictError` fires before any disk change.
- `stanza remove`'s package-sweep guard reads the `consumesPackages` snapshot persisted on each installed record (falling back to a registry fetch only for legacy records), so a `packages/<dir>/` still imported by an installed module is protected even offline or after an upstream rename.
- `stanza remove` only applies the legacy bare-id owner fallback when no sibling install of the same module remains, so removing one app's install on a pre-composite-owner manifest can no longer sweep another app's files.
- `stanza doctor` no longer reports false drift for package-home modules installed with an `--app` restriction.
- `wrap-root-layout` resolves the framework per dispatched app, fixing multi-app projects with differing frameworks.
- The template/codemod render context carries the project's `packageManager`, so `{{pm}}`/`{{run …}}` render bun/npm commands instead of always pnpm.
+60
View File
@@ -310,6 +310,66 @@ describe("cmdRemove path-traversal hardening", () => {
});
});
describe("cmdRemove legacy bare-owner claims", () => {
it("does not sweep a sibling app's claims when the same module is installed elsewhere", async () => {
const projectRoot = path.join(tmp, "proj");
fs.mkdirSync(path.join(projectRoot, "apps", "web"), { recursive: true });
fs.mkdirSync(path.join(projectRoot, "apps", "native"), { recursive: true });
const webFile = path.join(projectRoot, "apps/web/ghost.config.ts");
const nativeFile = path.join(projectRoot, "apps/native/ghost.config.ts");
fs.writeFileSync(webFile, "// web install\n");
fs.writeFileSync(nativeFile, "// native install\n");
// Pre-composite-owner manifest: both installs' claims use the bare module
// id, so neither claim can be attributed to a single app. `ghost` isn't in
// the registry, mirroring the offline/renamed-upstream scenario.
const manifest = {
version: CURRENT_MANIFEST_VERSION,
projectShape: "monorepo",
packageManager: "pnpm",
name: "proj",
modules: {
testing: [
{ id: "ghost", version: "0.0.0", adapter: "default", apps: ["web"] },
{ id: "ghost", version: "0.0.0", adapter: "default", apps: ["native"] },
],
},
apps: [
{ id: "web", dir: "apps/web", kind: "web" },
{ id: "native", dir: "apps/native", kind: "native" },
],
regions: {
"apps/web/ghost.config.ts": { file: "ghost" },
"apps/native/ghost.config.ts": { file: "ghost" },
},
};
fs.writeFileSync(path.join(projectRoot, "stanza.json"), JSON.stringify(manifest, null, 2));
process.chdir(projectRoot);
await cmdRemove(args({ slot: "testing", moduleId: "ghost", app: "web" }));
process.chdir(tmp);
// The sibling install's file (and even web's own, since the bare claims
// can't be attributed) must survive; only the web record is dropped.
expect(fs.existsSync(nativeFile)).toBe(true);
const after = JSON.parse(fs.readFileSync(path.join(projectRoot, "stanza.json"), "utf8"));
expect(after.modules.testing).toEqual([
{ id: "ghost", version: "0.0.0", adapter: "default", apps: ["native"] },
]);
// Removing the last record (no sibling left) applies the bare-id fallback
// and sweeps the legacy claims.
process.chdir(projectRoot);
await cmdRemove(args({ slot: "testing", moduleId: "ghost", app: "native" }));
process.chdir(tmp);
expect(fs.existsSync(nativeFile)).toBe(false);
const final = JSON.parse(fs.readFileSync(path.join(projectRoot, "stanza.json"), "utf8"));
expect(final.modules.testing).toBeUndefined();
expect(final.regions).toEqual({});
});
});
describe("add-ons (multi-choice testing slot)", () => {
it("init --yes installs two add-ons in one category without a region conflict", async () => {
await cmdInit(
+24 -2
View File
@@ -2,7 +2,8 @@ import fs from "node:fs";
import path from "node:path";
import * as p from "@clack/prompts";
import { PACKAGE_DIRS } from "@withstanza/schema";
import type { AppSpec, StanzaManifest } from "@withstanza/schema";
import { categoryHome, isCategoryId, PACKAGE_DIRS } from "@withstanza/schema";
import { defineCommand } from "citty";
import pc from "picocolors";
@@ -82,7 +83,7 @@ export async function cmdDoctor(): Promise<void> {
continue;
}
const depName = `@${manifest.name}/${dir}`;
for (const app of manifest.apps) {
for (const app of expectedConsumerApps(manifest, dir)) {
const appPkg = path.join(projectRoot, app.dir, "package.json");
if (!fs.existsSync(appPkg)) continue;
if (!pkgHasKey(appPkg, "dependencies", depName)) {
@@ -99,6 +100,27 @@ export async function cmdDoctor(): Promise<void> {
process.exitCode = 1;
}
// The install path wires the workspace dep only into the apps a package-home
// record targets (`apps` omitted = every app), so only expect it there.
// Multiple categories can share a dir (db + orm); union their targets.
function expectedConsumerApps(manifest: StanzaManifest, dir: string): AppSpec[] {
const ids = new Set<string>();
let any = false;
for (const [category, records] of Object.entries(manifest.modules)) {
if (!isCategoryId(category)) continue;
const home = categoryHome(category);
if (home.kind !== "package" || home.dir !== dir) continue;
for (const record of records ?? []) {
any = true;
if (!record.apps?.length) return manifest.apps;
for (const id of record.apps) ids.add(id);
}
}
// Claims with no surviving record is itself drift; fall back to every app.
if (!any) return manifest.apps;
return manifest.apps.filter((a) => ids.has(a.id));
}
function envHasVar(file: string, name: string): boolean {
if (!fs.existsSync(file)) return false;
const content = fs.readFileSync(file, "utf8");
+10 -1
View File
@@ -17,6 +17,7 @@ import type { AppSpec, CategoryId, Module, PackageManager } from "@withstanza/sc
import {
categoryHome,
DEFAULT_NAMESPACE,
emptyManifest,
KNOWN_CATEGORIES,
PEER_CATEGORIES,
} from "@withstanza/schema";
@@ -91,6 +92,7 @@ export async function cmdInit(args: CliArgs): Promise<void> {
return;
}
if (!dryRun) {
fs.mkdirSync(projectRoot, { recursive: true });
// Bootstrap the empty monorepo shell.
@@ -99,8 +101,15 @@ export async function cmdInit(args: CliArgs): Promise<void> {
packageManager: result.packageManager,
apps: result.apps,
});
}
let manifest = initManifest({
let manifest = dryRun
? emptyManifest({
name: result.name,
apps: result.apps,
packageManager: result.packageManager,
})
: initManifest({
projectRoot,
name: result.name,
apps: result.apps,
+21 -9
View File
@@ -189,10 +189,18 @@ export async function cmdRemove(args: CliArgs): Promise<void> {
// driven by whatever region claims remain after the codemod reverts.
// Owner is composite (`<id>@<app>`) for home:app installs so cross-app
// installs of the same module don't collide. Bare `<id>` covers older
// manifests + the non-app homes that never used a composite owner.
// manifests + the non-app homes that never used a composite owner — but
// only when no sibling record of the same id remains: a legacy bare claim
// can't be attributed to one app, so sweeping it while the module is still
// installed elsewhere would delete the other app's files.
const hasSiblingInstall = (manifest.modules[category] ?? []).some(
(r) => r.id === installed.id && !sameAppSet(r.apps, installed.apps),
);
const ownerKeys =
home.kind === "app" && installed.apps?.length === 1
? [`${installed.id}@${installed.apps[0]}`, installed.id]
? hasSiblingInstall
? [`${installed.id}@${installed.apps[0]}`]
: [`${installed.id}@${installed.apps[0]}`, installed.id]
: [installed.id];
const owned = regionsOwnedBy(manifest, ownerKeys);
for (const { file, region } of owned) {
@@ -349,8 +357,11 @@ export async function cmdRemove(args: CliArgs): Promise<void> {
if (dryRun) p.log.info(pc.yellow("[dry-run] no files were written"));
}
// Skips records whose registry entry can't be re-loaded (offline, rename) —
// failing open here beats blocking remove on a transient network error.
// Prefers the `consumesPackages` snapshot persisted on each record so the
// guard holds offline and after upstream registry changes; the live registry
// fetch only backfills legacy records that predate the snapshot. Records that
// can't be resolved either way fail open — better than blocking remove on a
// transient network error.
async function collectConsumesPackagesProtectors(
manifest: StanzaManifest,
registry: Registries,
@@ -361,11 +372,12 @@ async function collectConsumesPackagesProtectors(
for (const record of records ?? []) {
tasks.push(
(async () => {
const mod = await registry
.loadModule(category, record.id, record.namespace)
.catch(() => null);
if (!mod?.consumesPackages?.length) return;
for (const dir of mod.consumesPackages) {
const dirs =
record.consumesPackages ??
(await registry.loadModule(category, record.id, record.namespace).catch(() => null))
?.consumesPackages;
if (!dirs?.length) return;
for (const dir of dirs) {
const arr = protectors.get(dir) ?? [];
arr.push(`${category}/${record.id}`);
protectors.set(dir, arr);
+11 -1
View File
@@ -172,6 +172,7 @@ export async function applyModule(args: {
projectName: manifest.name,
app,
packageName,
packageManager: manifest.packageManager,
peers: activePeerIds(manifest, app.id),
envNames: declaredEnvNames(manifest),
consumesPackages: module.consumesPackages,
@@ -243,6 +244,9 @@ export async function applyModule(args: {
// The slot's package.json may be deferred-bootstrapped by planSlotPackageBootstrap;
// that's fine — the file will exist by the time deferredWrites flush.
const slotBootstrapTarget = packageDir !== null ? `packages/${packageDir}/package.json` : null;
// Skip on dry-run: nothing is written, and `init --dry-run` plans modules
// before the monorepo shell (and these files) would exist.
if (!dryRun) {
for (const target of installTargets) {
if (target === slotBootstrapTarget) continue;
const pkgJsonPath = path.join(projectRoot, target);
@@ -253,6 +257,7 @@ export async function applyModule(args: {
);
}
}
}
// Re-pin declared `^`/`~` ranges to the latest published version that still
// satisfies them (e.g. `^1.6.11` → `^1.8.3`), preserving the modifier. Skip
// the network on dry-run since nothing is written. `resolveRanges` falls
@@ -308,12 +313,14 @@ export async function applyModule(args: {
Object.keys(appFields.scripts).length > 0;
if (hasAppInstall) {
const appTargets = targetApps.map((a) => `${a.dir.replace(/\/+$/, "")}/package.json`);
if (!dryRun) {
for (const target of appTargets) {
const pkgJsonPath = path.join(projectRoot, target);
if (!fs.existsSync(pkgJsonPath)) {
throw new Error(`Cannot apply ${module.id} app overlay: ${target} doesn't exist.`);
}
}
}
const appDeps = dryRun ? appFields.dependencies : await resolveRanges(appFields.dependencies);
const appDevDeps = dryRun
? appFields.devDependencies
@@ -808,6 +815,7 @@ export async function revertCodemods(args: {
projectName: manifest.name,
app,
packageName,
packageManager: manifest.packageManager,
peers: activePeerIds(manifest, app.id),
envNames: declaredEnvNames(manifest),
consumesPackages: args.consumesPackages,
@@ -948,7 +956,9 @@ function realpathAllowingMissing(p: string, depth = 0): string {
*/
export function assertWithinRoot(projectRoot: string, relativePath: string, label: string): void {
const root = path.resolve(projectRoot);
const realRoot = fs.realpathSync(root);
// Missing-tolerant: dry-run init vets paths under a project root that
// doesn't exist yet (equivalent to realpathSync when the root exists).
const realRoot = realpathAllowingMissing(root);
const segments = relativePath.split(/[/\\]+/).filter((s) => s !== "" && s !== ".");
// Reject `..` lexically before any join — a traversal segment must never be
// resolved against the root, and isn't something symlink resolution vets.
+4 -1
View File
@@ -79,11 +79,14 @@ export const Route = createFileRoute("/api/events")({
if (!posthog) return new Response(null, { status: 204 });
for (const e of payload.events) {
// Drop unparseable timestamps — an Invalid Date would throw inside
// posthog-node's serialization, turning bad input into a 500.
const ts = e.timestamp ? new Date(e.timestamp) : undefined;
posthog.capture({
distinctId: payload.distinctId,
event: e.event,
properties: e.properties ?? {},
timestamp: e.timestamp ? new Date(e.timestamp) : undefined,
timestamp: ts && !Number.isNaN(ts.getTime()) ? ts : undefined,
});
}
+7 -5
View File
@@ -1,7 +1,6 @@
import { create, insert, search } from "@orama/orama";
import { createFileRoute } from "@tanstack/react-router";
import type { Module, ModuleMetadata, RegistryIndex } from "@withstanza/schema";
import { cache } from "react";
import { loadRegistryFile } from "@/server/registry-base.server";
@@ -22,10 +21,13 @@ type ModuleIndex = {
metadata: Map<string, ModuleMetadata>;
};
// React `cache()` dedupes the (async) build across any concurrent callers
// within the same server render. The returned promise is reused on warm
// calls so the registry-load + Orama-insert work only runs once.
const getModuleIndex = cache((): Promise<ModuleIndex> => buildIndex());
// Process-lifetime singleton: registry data is immutable per deployment, so
// the registry-load + Orama-insert work runs once per server instance.
let moduleIndexPromise: Promise<ModuleIndex> | undefined;
function getModuleIndex(): Promise<ModuleIndex> {
if (!moduleIndexPromise) moduleIndexPromise = buildIndex();
return moduleIndexPromise;
}
async function buildIndex(): Promise<ModuleIndex> {
const index = await loadRegistryFile<RegistryIndex>("index.json");
@@ -34,11 +34,13 @@ const addPackageDep: Codemod<AddPackageDepArgs> = {
const rel = path.relative(ctx.projectRoot, pkgJsonPath);
const range = args.range ?? "workspace:*";
addPackageDependency(pkgJsonPath, args.name, range, { dev: args.dev });
// Claim before writing: the runner snapshots the file on claim, and the
// snapshot must capture the pre-write bytes for rollback to restore them.
const depKey = args.dev ? "devDependencies" : "dependencies";
ctx.claimRegion(rel, `${depKey}.${args.name}`);
addPackageDependency(pkgJsonPath, args.name, range, { dev: args.dev });
return { touchedFiles: [rel] };
},
@@ -113,10 +113,12 @@ const appendToFile: Codemod<AppendToFileArgs> = {
position === "start"
? prependBlock(current, block, leadingBlank)
: appendBlock(current, block, leadingBlank);
// Claim before writing: the runner snapshots the file on claim, and the
// snapshot must capture the pre-write bytes for rollback to restore them.
ctx.claimRegion(fileRel, `append.${args.marker}`);
if (!exists) fs.mkdirSync(path.dirname(fileAbs), { recursive: true });
fs.writeFileSync(fileAbs, next, "utf8");
ctx.claimRegion(fileRel, `append.${args.marker}`);
return { touchedFiles: [fileRel] };
},
@@ -90,8 +90,10 @@ const setTsconfigPaths: Codemod<SetTsconfigPathsArgs> = {
const allPresent = Object.entries(args.paths).every(
([k, v]) => existing[k] !== undefined && arraysEqual(toArray(existing[k]), v),
);
if (allPresent && compilerOptions.baseUrl !== undefined) {
// Claim before writing: the runner snapshots the file on claim, and the
// snapshot must capture the pre-write bytes for rollback to restore them.
ctx.claimRegion(rel, regionKeyFor(args));
if (allPresent && compilerOptions.baseUrl !== undefined) {
return { touchedFiles: [] };
}
@@ -101,7 +103,6 @@ const setTsconfigPaths: Codemod<SetTsconfigPathsArgs> = {
for (const [key, val] of Object.entries(args.paths)) {
setJsonPathSegments(abs, ["compilerOptions", "paths", key], val);
}
ctx.claimRegion(rel, regionKeyFor(args));
return { touchedFiles: [rel] };
},
@@ -42,10 +42,13 @@ const wrapRootLayout: Codemod<WrapRootLayoutArgs> = {
description: "Wrap the root layout's children with a provider element.",
apply(ctx, args) {
const target = frameworkTarget(selectedOne(ctx.manifest, "framework")?.id);
// App-scoped read: each dispatch targets one app, and apps can run
// different frameworks — the global pick would wrap the wrong layout.
const frameworkId = selectedOne(ctx.manifest, "framework", ctx.app.id)?.id;
const target = frameworkTarget(frameworkId);
if (!target) {
throw new Error(
`wrap-root-layout: framework "${selectedOne(ctx.manifest, "framework")?.id ?? "<unset>"}" not supported. ` +
`wrap-root-layout: framework "${frameworkId ?? "<unset>"}" not supported. ` +
`Add a case in frameworkTarget() to enable.`,
);
}
@@ -92,7 +95,7 @@ const wrapRootLayout: Codemod<WrapRootLayoutArgs> = {
},
revert(ctx, args) {
const target = frameworkTarget(selectedOne(ctx.manifest, "framework")?.id);
const target = frameworkTarget(selectedOne(ctx.manifest, "framework", ctx.app.id)?.id);
if (!target) return { touchedFiles: [] };
const layoutAbs = path.join(ctx.appRoot, target.relPath);
+14 -10
View File
@@ -32,26 +32,30 @@ try {
range = "HEAD~1"; // local fallback
}
const slugs = new Set<string>();
const changedDirs = new Set<string>();
for (const f of git(["diff", "--name-only", range, "HEAD"]).split("\n")) {
const m = /^registry\/modules\/([^/]+)\//.exec(f);
if (m) slugs.add(m[1]!);
if (m) changedDirs.add(m[1]!);
}
if (slugs.size === 0) {
if (changedDirs.size === 0) {
console.log("Module version guard passed (no module changes).");
process.exit(0);
}
const compiledDir = fs.mkdtempSync(path.join(os.tmpdir(), "stanza-check-"));
await compileRegistry({ outDir: compiledDir });
const { modules } = await compileRegistry({ outDir: compiledDir });
// Correlate source dirs with compiled output through the compiler's own
// mapping — inferring `<dir>.json` would silently skip (and so bypass the
// guard for) any module whose dir name doesn't match its `<category>-<id>`.
const byDir = new Map(modules.map((m) => [m.dir, m]));
const errors: string[] = [];
for (const slug of slugs) {
const file = path.join(compiledDir, `${slug}.json`);
if (!fs.existsSync(file)) continue; // module deleted — nothing to pin
const text = fs.readFileSync(file, "utf8");
const { version }: { version: string } = JSON.parse(text);
for (const dir of changedDirs) {
const entry = byDir.get(dir);
if (!entry) continue; // module deleted — nothing to pin
const { slug, version } = entry;
const text = fs.readFileSync(path.join(compiledDir, `${slug}.json`), "utf8");
const res = await fetch(`${registryBase}/${slug}@${version}.json`);
if (res.status === 404) continue; // new version or new module
if (!res.ok) {
@@ -61,7 +65,7 @@ for (const slug of slugs) {
if ((await res.text()) !== text) {
errors.push(
`Module "${slug}" changed but version ${version} is already published with different ` +
`content. Bump "version" in registry/modules/${slug}/package.json.`,
`content. Bump "version" in registry/modules/${dir}/package.json.`,
);
}
}
+16 -5
View File
@@ -22,15 +22,21 @@ import { fileURLToPath } from "node:url";
import { CATEGORIES, type Logo, type Module, type RegistryIndex } from "@withstanza/schema";
import { optimize } from "svgo";
/** One compiled module: its source dir basename, output slug, and version. */
export type CompiledModule = { dir: string; slug: string; version: string };
/**
* Build the static registry into `<outDir>`. `modulesDir` defaults to the
* repo's `registry/modules`; tests can point it elsewhere. Returns the final
* module count.
* module count plus a `dir → slug/version` mapping — the source dir name is a
* convention (`<category>-<id>`), not a contract, so callers correlating
* source dirs with compiled output (e.g. the version-pin guard) must go
* through this mapping rather than infer filenames.
*/
export async function compileRegistry(opts: {
modulesDir?: string;
outDir: string;
}): Promise<{ count: number }> {
}): Promise<{ count: number; modules: CompiledModule[] }> {
const here = path.dirname(fileURLToPath(import.meta.url));
const repoRoot = findRepoRoot(here);
const modulesDir = opts.modulesDir ?? path.join(repoRoot, "registry", "modules");
@@ -49,6 +55,7 @@ export async function compileRegistry(opts: {
.map((d) => d.name);
const metadata = [];
const compiledModules: CompiledModule[] = [];
for (const dir of dirs) {
const entry = path.join(modulesDir, dir, "module.ts");
const imported: { default: Module } = await import(entry);
@@ -87,6 +94,7 @@ export async function compileRegistry(opts: {
// `path`, resolved relative to the index URL.)
const modulePath = `${mod.category}-${mod.id}.json`;
fs.writeFileSync(path.join(opts.outDir, modulePath), JSON.stringify(inlined, null, 2));
compiledModules.push({ dir, slug: `${mod.category}-${mod.id}`, version });
// The index keeps lightweight metadata — no template `content`, no
// per-adapter payloads — but it DOES carry top-level fields like `logo`
@@ -108,7 +116,7 @@ export async function compileRegistry(opts: {
fs.writeFileSync(path.join(opts.outDir, "index.json"), JSON.stringify(index, null, 2));
return { count: metadata.length };
return { count: metadata.length, modules: compiledModules };
}
// Direct invocation: `jiti scripts/compile-registry.ts [outDir]`. Skipped when
@@ -152,8 +160,11 @@ function optimizeLogo(svg: string, prefix: string): string {
plugins: [
"preset-default",
{ name: "prefixIds", params: { prefix, delim: "-" } },
// Strip every `on*` event handler so we can render via dangerouslySetInnerHTML
// without smuggling JS through a hostile (third-party) logo.
// Logos render via dangerouslySetInnerHTML: `removeScripts` drops
// `<script>` elements, every spec-defined event attribute, and
// `javascript:` hrefs; the `removeAttrs` pass additionally catches
// nonstandard `on*` attributes the spec groups don't cover.
"removeScripts",
{ name: "removeAttrs", params: { attrs: "on[a-zA-Z]+" } },
],
}).data;